diff --git a/identity-server/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs b/identity-server/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs index c4f1692dd..bf616e0ec 100644 --- a/identity-server/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs +++ b/identity-server/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs @@ -69,6 +69,11 @@ public class IdentityServerOptions /// public bool StrictJarValidation { get; set; } = false; + /// + /// When clients authenticate with private_key_jwt assertions, validate the audience of the assertion strictly: the audience must be this IdentityServer's issuer identifier as a single string. + /// + public bool StrictClientAssertionAudienceValidation { get; set; } = false; + /// /// Specifies if a user's tenant claim is compared to the tenant acr_values parameter value to determine if the login page is displayed. Defaults to false. /// diff --git a/identity-server/src/IdentityServer/Validation/Default/PrivateKeyJwtSecretValidator.cs b/identity-server/src/IdentityServer/Validation/Default/PrivateKeyJwtSecretValidator.cs index afe93fc99..6fe43daf4 100644 --- a/identity-server/src/IdentityServer/Validation/Default/PrivateKeyJwtSecretValidator.cs +++ b/identity-server/src/IdentityServer/Validation/Default/PrivateKeyJwtSecretValidator.cs @@ -89,20 +89,7 @@ public async Task ValidateAsync(IEnumerable secr return fail; } - var validAudiences = new[] - { - // token endpoint URL - string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.Token), - // issuer URL + token (legacy support) - string.Concat((await _issuerNameService.GetCurrentAsync()).EnsureTrailingSlash(), ProtocolRoutePaths.Token), - // issuer URL - await _issuerNameService.GetCurrentAsync(), - // CIBA endpoint: https://openid.net/specs/openid-client-initiated-backchannel-authentication-core-1_0.html#auth_request - string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.BackchannelAuthentication), - // PAR endpoint: https://datatracker.ietf.org/doc/html/rfc9126#name-request - string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.PushedAuthorization), - - }.Distinct(); + var issuer = await _issuerNameService.GetCurrentAsync(); var tokenValidationParameters = new TokenValidationParameters { @@ -111,15 +98,46 @@ await _issuerNameService.GetCurrentAsync(), ValidIssuer = parsedSecret.Id, ValidateIssuer = true, - - ValidAudiences = validAudiences, - ValidateAudience = true, - + RequireSignedTokens = true, RequireExpirationTime = true, ClockSkew = TimeSpan.FromMinutes(5) }; + + if (_options.StrictClientAssertionAudienceValidation) + { + // New strict audience validation requires that the audience be the issuer identifier, disallows multiple + // audiences in an array, and even disallows wrapping even a single audience in an array + tokenValidationParameters.AudienceValidator = (audiences, token, parameters) => + { + // There isn't a particularly nice way to distinguish between a claim that is a single string wrapped in + // an array and just a single string when using a JsonWebToken. The jwt.GetClaim function and jwt.Claims + // collection both convert that into a string valued claim. However, GetPayloadValue does not do + // any type inferencing, so we can call that, and then check if the result is actually a string + var audValue = ((JsonWebToken)token).GetPayloadValue("aud"); + return audValue is string audString && + AudiencesMatch(audString, issuer); + }; + } + else + { + tokenValidationParameters.ValidateAudience = true; + tokenValidationParameters.ValidAudiences = new[] + { + // token endpoint URL + string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.Token), + // issuer URL + token (legacy support) + string.Concat((await _issuerNameService.GetCurrentAsync()).EnsureTrailingSlash(), ProtocolRoutePaths.Token), + // issuer URL + issuer, + // CIBA endpoint: https://openid.net/specs/openid-client-initiated-backchannel-authentication-core-1_0.html#auth_request + string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.BackchannelAuthentication), + // PAR endpoint: https://datatracker.ietf.org/doc/html/rfc9126#name-request + string.Concat(_urls.BaseUrl.EnsureTrailingSlash(), ProtocolRoutePaths.PushedAuthorization), + + }.Distinct(); + } var handler = new JsonWebTokenHandler() { MaximumTokenSizeInBytes = _options.InputLengthRestrictions.Jwt }; var result = await handler.ValidateTokenAsync(jwtTokenString, tokenValidationParameters); @@ -162,4 +180,50 @@ await _issuerNameService.GetCurrentAsync(), return success; } + + // AudiencesMatch and AudiencesMatchIgnoringTrailingSlash are based on code from + // https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/bef98ca10ae55603ce6d37dfb7cd5af27791527c/src/Microsoft.IdentityModel.Tokens/Validators.cs#L158-L193 + private bool AudiencesMatch(string tokenAudience, string validAudience) + { + if (validAudience.Length == tokenAudience.Length) + { + if (string.Equals(validAudience, tokenAudience)) + { + return true; + } + } + + return AudiencesMatchIgnoringTrailingSlash(tokenAudience, validAudience); + } + + private bool AudiencesMatchIgnoringTrailingSlash(string tokenAudience, string validAudience) + { + int length = -1; + + if (validAudience.Length == tokenAudience.Length + 1 && + validAudience.EndsWith("/", StringComparison.InvariantCulture)) + { + length = validAudience.Length - 1; + } + else if (tokenAudience.Length == validAudience.Length + 1 && + tokenAudience.EndsWith("/", StringComparison.InvariantCulture)) + { + length = tokenAudience.Length - 1; + } + + // the length of the audiences is different by more than 1 and neither ends in a "/" + if (length == -1) + { + return false; + } + + if (string.CompareOrdinal(validAudience, 0, tokenAudience, 0, length) == 0) + { + _logger.LogInformation("Audience Validated.Audience: '{audience}'", tokenAudience); + + return true; + } + + return false; + } } \ No newline at end of file diff --git a/identity-server/test/IdentityServer.UnitTests/Validation/Secrets/PrivateKeyJwtSecretValidation.cs b/identity-server/test/IdentityServer.UnitTests/Validation/Secrets/PrivateKeyJwtSecretValidation.cs index 2fc940c3c..327f9f437 100644 --- a/identity-server/test/IdentityServer.UnitTests/Validation/Secrets/PrivateKeyJwtSecretValidation.cs +++ b/identity-server/test/IdentityServer.UnitTests/Validation/Secrets/PrivateKeyJwtSecretValidation.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.IdentityModel.Tokens.Jwt; using System.Security.Claims; +using System.Text.Json; using System.Threading.Tasks; using Duende.IdentityServer; using Duende.IdentityServer.Configuration; @@ -28,32 +29,47 @@ public class PrivateKeyJwtSecretValidation { private readonly ISecretValidator _validator; private readonly IClientStore _clients; + private readonly IdentityServerOptions _options; public PrivateKeyJwtSecretValidation() { + _options = new IdentityServerOptions(); + _validator = new PrivateKeyJwtSecretValidator( - new TestIssuerNameService("https://idsrv3.com"), + new TestIssuerNameService("https://idsrv.com"), new DefaultReplayCache(new TestCache()), - new MockServerUrls() { Origin = "https://idsrv3.com" }, - new IdentityServerOptions(), + new MockServerUrls() { Origin = "https://idsrv.com" }, + _options, new LoggerFactory().CreateLogger()); _clients = new InMemoryClientStore(ClientValidationTestClients.Get()); } - private JwtSecurityToken CreateToken(string clientId, DateTime? nowOverride = null, string aud = "https://idsrv3.com/connect/token") + private JwtSecurityToken CreateToken(string clientId, string aud = "https://idsrv.com/", DateTime? nowOverride = null) + { + return CreateTokenHelper(clientId, new Claim(JwtClaimTypes.Audience, aud), nowOverride); + } + + private JwtSecurityToken CreateToken(string clientId, string[] audiences, DateTime? nowOverride = null) + { + return CreateTokenHelper(clientId, new Claim(JwtClaimTypes.Audience, JsonSerializer.Serialize(audiences), JsonClaimValueTypes.JsonArray), nowOverride); + } + + private JwtSecurityToken CreateTokenHelper(string clientId, Claim aud = null, DateTime? nowOverride = null) { var certificate = TestCert.Load(); var now = nowOverride ?? DateTime.UtcNow; - + aud ??= new Claim("aud", "https://idsrv.com"); + var token = new JwtSecurityToken( clientId, - aud, + null, new List() { - new Claim("jti", Guid.NewGuid().ToString()), - new Claim(JwtClaimTypes.Subject, clientId), - new Claim(JwtClaimTypes.IssuedAt, new DateTimeOffset(now).ToUnixTimeSeconds().ToString(), ClaimValueTypes.Integer64) + new("jti", Guid.NewGuid().ToString()), + new(JwtClaimTypes.Subject, clientId), + new(JwtClaimTypes.IssuedAt, new DateTimeOffset(now).ToUnixTimeSeconds().ToString(), ClaimValueTypes.Integer64), + aud }, now, now.AddMinutes(1), @@ -121,12 +137,12 @@ public async Task Valid_Certificate_Base64() } [Theory] - [InlineData("https://idsrv3.com")] - [InlineData("https://idsrv3.com/")] - [InlineData("https://idsrv3.com/connect/token")] - [InlineData("https://idsrv3.com/connect/ciba")] - [InlineData("https://idsrv3.com/connect/par")] - public async Task Valid_aud(string aud) + [InlineData("https://idsrv.com")] + [InlineData("https://idsrv.com/")] + [InlineData("https://idsrv.com/connect/token")] + [InlineData("https://idsrv.com/connect/ciba")] + [InlineData("https://idsrv.com/connect/par")] + public async Task Valid_legacy_aud(string aud) { var clientId = "certificate_base64_valid"; var client = await _clients.FindEnabledClientByIdAsync(clientId); @@ -143,6 +159,78 @@ public async Task Valid_aud(string aud) result.Success.Should().BeTrue(); } + [Theory] + [InlineData("https://idsrv.com", true)] + [InlineData("https://idsrv.com/", true)] + [InlineData("https://idsrv.com/connect/token", false)] + [InlineData("https://idsrv.com/connect/ciba", false)] + [InlineData("", false)] + [InlineData("https://idsrv.com/connect/par", false)] + public async Task Valid_strict_aud(string aud, bool expectSuccess) + { + _options.StrictClientAssertionAudienceValidation = true; + + var clientId = "certificate_base64_valid"; + var client = await _clients.FindEnabledClientByIdAsync(clientId); + + var secret = new ParsedSecret + { + Id = clientId, + Credential = new JwtSecurityTokenHandler().WriteToken(CreateToken(clientId, aud:aud)), + Type = IdentityServerConstants.ParsedSecretTypes.JwtBearer + }; + + var result = await _validator.ValidateAsync(client.ClientSecrets, secret); + + result.Success.Should().Be(expectSuccess, result.Error); + } + + [Theory] + [InlineData("https://idsrv.com")] + [InlineData("https://idsrv.com/")] + [InlineData("https://idsrv.com/connect/token")] + [InlineData("https://idsrv.com/connect/ciba")] + [InlineData("https://idsrv.com/connect/par")] + public async Task StrictAudience_does_not_allow_single_valued_arrays(string aud) + { + _options.StrictClientAssertionAudienceValidation = true; + + var clientId = "certificate_base64_valid"; + var client = await _clients.FindEnabledClientByIdAsync(clientId); + + var secret = new ParsedSecret + { + Id = clientId, + Credential = new JwtSecurityTokenHandler().WriteToken(CreateToken(clientId, [aud])), + Type = IdentityServerConstants.ParsedSecretTypes.JwtBearer + }; + + var result = await _validator.ValidateAsync(client.ClientSecrets, secret); + + result.Success.Should().BeFalse(); + } + + [Fact] + public async Task StrictAudience_does_not_allow_multi_valued_arrays() + { + _options.StrictClientAssertionAudienceValidation = true; + + var clientId = "certificate_base64_valid"; + var client = await _clients.FindEnabledClientByIdAsync(clientId); + + var secret = new ParsedSecret + { + Id = clientId, + Credential = new JwtSecurityTokenHandler().WriteToken(CreateToken(clientId, + ["https://idsrv.com", "https://idsrv.com/"])), + Type = IdentityServerConstants.ParsedSecretTypes.JwtBearer + }; + + var result = await _validator.ValidateAsync(client.ClientSecrets, secret); + + result.Success.Should().BeFalse(); + } + [Fact] public async Task Invalid_Replay() { @@ -230,7 +318,7 @@ public async Task Invalid_Expired_Token() var clientId = "certificate_valid"; var client = await _clients.FindEnabledClientByIdAsync(clientId); - var token = CreateToken(clientId, DateTime.UtcNow.AddHours(-1)); + var token = CreateToken(clientId, nowOverride: DateTime.UtcNow.AddHours(-1)); var secret = new ParsedSecret { Id = clientId,