From eaeb7480bdd6f6dd5cd84b4b9c63555697582401 Mon Sep 17 00:00:00 2001 From: Keegan Caruso Date: Mon, 8 Jul 2024 17:41:15 -0700 Subject: [PATCH] Remove dependency on AadIssuerValidator.GetTenantIdFromToken in ValidateIssuerSigningKey (#2680) (#2686) * Remove dep on AadIssuerValidator * Add DontFailOnMissingTidValidateIssuerSigning AppContextSwitch * Fail if multiple tids --------- Co-authored-by: Keegan Caruso --- .../InternalsVisibleTo.cs | 1 + .../JsonWebToken.cs | 5 + .../AadTokenValidationParametersExtension.cs | 64 +++++++++- .../LogMessages.cs | 4 + .../AadSigningKeyIssuerValidatorTests.cs | 112 +++++++++++++++++- 5 files changed, 182 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs b/src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs index 05a39737b8..d5dbc25a51 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/InternalsVisibleTo.cs @@ -1,4 +1,5 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.Validators, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("Microsoft.IdentityModel.JsonWebTokens.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100b5fc90e7027f67871e773a8fde8938c81dd402ba65b9201d60593e96c492651e889cc13f1415ebb53fac1131ae0bd333c5ee6021672d9718ea31a8aebd0da0072f25d87dba6fc90ffd598ed4da35e44c398c454307e8e33b8426143daec9f596836f97c8f74750e5975c64e2189f45def46b2a2b1247adc3652bf5c308055da9")] diff --git a/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs b/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs index b0d89ba30a..d3c0cd3986 100644 --- a/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs +++ b/src/Microsoft.IdentityModel.JsonWebTokens/JsonWebToken.cs @@ -625,6 +625,11 @@ public Claim GetClaim(string key) return Payload.GetClaim(key, Issuer ?? ClaimsIdentity.DefaultIssuer); } + /// + /// Gets the names of the payload claims on the JsonWebToken. + /// + internal IReadOnlyCollection PayloadClaimNames => Payload._jsonClaims.Keys; + internal ClaimsIdentity ClaimsIdentity { get diff --git a/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs b/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs index d745bf124c..dd5f011484 100644 --- a/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs +++ b/src/Microsoft.IdentityModel.Validators/AadTokenValidationParametersExtension.cs @@ -2,7 +2,11 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; +using System.IdentityModel.Tokens.Jwt; using System.Linq; +using System.Security.Claims; +using Microsoft.IdentityModel.JsonWebTokens; using Microsoft.IdentityModel.Logging; using Microsoft.IdentityModel.Protocols.OpenIdConnect; using Microsoft.IdentityModel.Tokens; @@ -41,6 +45,13 @@ public static void EnableAadSigningKeyIssuerValidation(this TokenValidationParam }; } + internal const string DontFailOnMissingTidSwitch = "Switch.Microsoft.IdentityModel.DontFailOnMissingTidValidateIssuerSigning"; + + private static bool DontFailOnMissingTid() + { + return (AppContext.TryGetSwitch(DontFailOnMissingTidSwitch, out bool dontFailOnMissingTid) && dontFailOnMissingTid); + } + /// /// Validates the issuer signing key. /// @@ -67,9 +78,14 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT if (string.IsNullOrWhiteSpace(signingKeyIssuer)) return true; - var tenantIdFromToken = AadIssuerValidator.GetTenantIdFromToken(securityToken); + var tenantIdFromToken = GetTid(securityToken); if (string.IsNullOrEmpty(tenantIdFromToken)) - return true; + { + if (DontFailOnMissingTid()) + return true; + + throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogMessages.IDX40009)); + } var tokenIssuer = securityToken.Issuer; @@ -91,7 +107,7 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT // comparing effectiveSigningKeyIssuer with v2TokenIssuer is required as well because of the following scenario: // 1. service trusts /common/v2.0 endpoint - // 2. service receieves a v1 token that has issuer like sts.windows.net + // 2. service receives a v1 token that has issuer like sts.windows.net // 3. signing key issuers will never match sts.windows.net as v1 endpoint doesn't have issuers attached to keys // v2TokenIssuer is the representation of Token.Issuer (if it was a v2 issuer) if (effectiveSigningKeyIssuer != tokenIssuer && effectiveSigningKeyIssuer != v2TokenIssuer) @@ -101,6 +117,48 @@ internal static bool ValidateIssuerSigningKey(SecurityKey securityKey, SecurityT return true; } + private static string GetTid(SecurityToken securityToken) + { + switch (securityToken) + { + case JsonWebToken jsonWebToken: + if (jsonWebToken.TryGetPayloadValue(AadIssuerValidatorConstants.Tid, out string tid)) + { + EnforceSingleClaimCaseInsensitive(jsonWebToken.PayloadClaimNames, AadIssuerValidatorConstants.Tid); + return tid; + } + + return string.Empty; + + case JwtSecurityToken jwtSecurityToken: + if ((jwtSecurityToken.Payload.TryGetValue(AadIssuerValidatorConstants.Tid, out object tidObject) && tidObject is string jwtTid)) + { + EnforceSingleClaimCaseInsensitive(jwtSecurityToken.Payload.Keys, AadIssuerValidatorConstants.Tid); + return jwtTid; + } + + return string.Empty; + + default: + throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogMessages.IDX40010)); + } + } + + private static void EnforceSingleClaimCaseInsensitive(IEnumerable keys, string claimType) + { + bool claimSeen = false; + foreach (var key in keys) + { + if (string.Equals(key, claimType, StringComparison.OrdinalIgnoreCase)) + { + if (claimSeen) + throw LogHelper.LogExceptionMessage(new SecurityTokenInvalidIssuerException(LogHelper.FormatInvariant(LogMessages.IDX40011, claimType))); + + claimSeen = true; + } + } + } + /// /// Validates the issuer signing key certificate. /// diff --git a/src/Microsoft.IdentityModel.Validators/LogMessages.cs b/src/Microsoft.IdentityModel.Validators/LogMessages.cs index 9279b2e1e4..bca2668903 100644 --- a/src/Microsoft.IdentityModel.Validators/LogMessages.cs +++ b/src/Microsoft.IdentityModel.Validators/LogMessages.cs @@ -24,5 +24,9 @@ internal static class LogMessages public const string IDX40005 = "IDX40005: Token issuer: '{0}', does not match the signing key issuer: '{1}'."; public const string IDX40007 = "IDX40007: RequireSignedTokens property on ValidationParameters is set to true, but the issuer signing key is null."; public const string IDX40008 = "IDX40008: When setting LastKnownGoodLifetime, the value must be greater than or equal to zero. value: '{0}'."; + + public const string IDX40009 = "IDX40009: Either the 'tid' claim was not found or it didn't have a value."; + public const string IDX40010 = "IDX40010: The SecurityToken must be a 'JsonWebToken' or 'JwtSecurityToken'"; + public const string IDX40011 = "IDX40011: The SecurityToken has multiple instances of the '{0}' claim."; } } diff --git a/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs b/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs index e768a2372f..a0a70fddf8 100644 --- a/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs +++ b/test/Microsoft.IdentityModel.Validators.Tests/AadSigningKeyIssuerValidatorTests.cs @@ -11,12 +11,15 @@ using Microsoft.IdentityModel.Protocols.OpenIdConnect; using Microsoft.IdentityModel.TestUtils; using Microsoft.IdentityModel.Tokens; +using Microsoft.IdentityModel.Tokens.Saml2; using Xunit; #pragma warning disable CS3016 // Arrays as attribute arguments is not CLS-compliant namespace Microsoft.IdentityModel.Validators.Tests { + // Serialize as one of the tests depends on static state (app context) + [Collection(nameof(AadSigningKeyIssuerValidatorTests))] public class AadSigningKeyIssuerValidatorTests { [Theory, MemberData(nameof(EnableAadSigningKeyIssuerValidationTestCases))] @@ -167,6 +170,7 @@ public void ValidateIssuerSigningKeyTests(AadSigningKeyIssuerTheoryData theoryDa try { + theoryData.SetupAction?.Invoke(); var result = AadTokenValidationParametersExtension.ValidateIssuerSigningKey(theoryData.SecurityKey, theoryData.SecurityToken, theoryData.OpenIdConnectConfiguration); theoryData.ExpectedException.ProcessNoException(context); Assert.True(result); @@ -175,6 +179,10 @@ public void ValidateIssuerSigningKeyTests(AadSigningKeyIssuerTheoryData theoryDa { theoryData.ExpectedException.ProcessException(ex, context); } + finally + { + theoryData.TearDownAction?.Invoke(); + } TestUtilities.AssertFailIfErrors(context); } @@ -294,7 +302,17 @@ public static TheoryData ValidateIssuerSigningKey TestId = "MissingTenantIdClaimInToken", SecurityKey = KeyingMaterial.JsonWebKeyP256, SecurityToken = new JwtSecurityToken(), - OpenIdConnectConfiguration = mockConfiguration + OpenIdConnectConfiguration = mockConfiguration, + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009") + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "WrongSecurityKeyType", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new Saml2SecurityToken(new Saml2Assertion(new Saml2NameIdentifier("nameIdentifier"))), + OpenIdConnectConfiguration = mockConfiguration, + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40010") }); theoryData.Add(new AadSigningKeyIssuerTheoryData @@ -321,6 +339,94 @@ public static TheoryData ValidateIssuerSigningKey ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40004") }); + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Doesnt_Fail_With_Switch", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JwtSecurityToken(), + OpenIdConnectConfiguration = mockConfiguration, + SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true), + TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false) + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Fail_With_Switch_False", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JwtSecurityToken(), + OpenIdConnectConfiguration = mockConfiguration, + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"), + SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false), + TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, isEnabled: false) + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Doesnt_Fail_With_Switch", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JwtSecurityToken(), + OpenIdConnectConfiguration = mockConfiguration, + SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true), + TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false) + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Fail_With_Switch_False_JsonWebToken", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JsonWebToken(Default.Jwt(Default.SecurityTokenDescriptor(Default.SymmetricSigningCredentials, [issClaim]))), + OpenIdConnectConfiguration = mockConfiguration, + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"), + SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false), + TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, isEnabled: false) + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Doesnt_Fail_With_Switch_JsonWebToken", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JsonWebToken(Default.Jwt(Default.SecurityTokenDescriptor(Default.SymmetricSigningCredentials, [issClaim]))), + OpenIdConnectConfiguration = mockConfiguration, + SetupAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, true), + TearDownAction = () => AppContext.SetSwitch(AadTokenValidationParametersExtension.DontFailOnMissingTidSwitch, false) + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Fails_With_Multiple_tids", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JsonWebToken( + Default.Jwt(Default.SecurityTokenDescriptor( + Default.SymmetricSigningCredentials, + [tidClaim, issClaim, new Claim("TID", Guid.NewGuid().ToString())]))), + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40011"), + OpenIdConnectConfiguration = mockConfiguration + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Fails_With_Multiple_tids_alternate_order", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JsonWebToken( + Default.Jwt(Default.SecurityTokenDescriptor( + Default.SymmetricSigningCredentials, + [issClaim, new Claim("TID", Guid.NewGuid().ToString()), tidClaim]))), + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40011"), + OpenIdConnectConfiguration = mockConfiguration + }); + + theoryData.Add(new AadSigningKeyIssuerTheoryData + { + TestId = "Fails_With_no standard_tid", + SecurityKey = KeyingMaterial.JsonWebKeyP256, + SecurityToken = new JsonWebToken( + Default.Jwt(Default.SecurityTokenDescriptor( + Default.SymmetricSigningCredentials, + [issClaim, new Claim("TID", Guid.NewGuid().ToString())]))), + ExpectedException = ExpectedException.SecurityTokenInvalidIssuerException("IDX40009"), + OpenIdConnectConfiguration = mockConfiguration + }); + return theoryData; } } @@ -346,6 +452,10 @@ public class AadSigningKeyIssuerTheoryData : TheoryDataBase public bool SetDelegateUsingConfig { get; set; } = false; public bool SetDelegateWithoutConfig { get; set; } = false; + + public Action SetupAction { get; set; } + + public Action TearDownAction { get; set; } } } }