Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rm SkipAuthenticationTagLengthValidation and UseShortNameForRsaOaepKey App Context Switches #2644

Merged
merged 4 commits into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ See the [releases](https://github.com/AzureAD/azure-activedirectory-identitymode
### Breaking changes:
- IdentityModel 8x no longer supports .net461, which has reached end of life and is no longer supported. See issue [#2544](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2544) for details.
- Two IdentityModel extension dlls `Microsoft.IdentityModel.KeyVaultExtensions` and `Microsoft.IdentityModel.ManagedKeyVaultSecurityKey` were using ADAL, which is no longer supported . The affected packages have been removed, as the replacement is to use [Microsoft.Identity.Web](https://github.com/AzureAD/microsoft-identity-web/wiki/Certificates). See issue [#2454](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2454) for details.
- `AppContext.SetSwitch` which were included in IdentityModel 7x, have been removed and are the default in IdentityModel 8x. The result is a more performant IdentityModel by default. See issue [#2629](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2629) for details.
- `AppContext.SetSwitch` which were included in IdentityModel 7x, have been removed and are the default in IdentityModel 8x. The result is a more performant IdentityModel by default. See issue [#2629](https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/2629) and https://aka.ms/IdentityModel8x for details.

7.6.1
=====
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ private struct AuthenticatedKeys
private DecryptionDelegate DecryptFunction;
private EncryptionDelegate EncryptFunction;
private const string _className = "Microsoft.IdentityModel.Tokens.AuthenticatedEncryptionProvider";
internal const string _skipValidationOfAuthenticationTagLength = "Switch.Microsoft.IdentityModel.SkipAuthenticationTagLengthValidation";

/// <summary>
/// Initializes a new instance of the <see cref="AuthenticatedEncryptionProvider"/> class used for encryption and decryption.
Expand Down Expand Up @@ -167,8 +166,7 @@ private AuthenticatedEncryptionResult EncryptWithAesCbc(byte[] plaintext, byte[]
private byte[] DecryptWithAesCbc(byte[] ciphertext, byte[] authenticatedData, byte[] iv, byte[] authenticationTag)
{
// Verify authentication Tag
if (ShouldValidateAuthenticationTagLength()
&& SymmetricSignatureProvider.ExpectedSignatureSizeInBytes.TryGetValue(Algorithm, out int expectedTagLength)
if (SymmetricSignatureProvider.ExpectedSignatureSizeInBytes.TryGetValue(Algorithm, out int expectedTagLength)
&& expectedTagLength != authenticationTag.Length)
throw LogHelper.LogExceptionMessage(new SecurityTokenDecryptionFailedException(
LogHelper.FormatInvariant(LogMessages.IDX10625, authenticationTag.Length, expectedTagLength, Base64UrlEncoder.Encode(authenticationTag), Algorithm)));
Expand Down Expand Up @@ -197,11 +195,6 @@ private byte[] DecryptWithAesCbc(byte[] ciphertext, byte[] authenticatedData, by
}
}

private static bool ShouldValidateAuthenticationTagLength()
{
return !(AppContext.TryGetSwitch(_skipValidationOfAuthenticationTagLength, out bool skipValidation) && skipValidation);
}

private AuthenticatedKeys CreateAuthenticatedKeys()
{
ValidateKeySize(Key, Algorithm);
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.IdentityModel.Tokens/LogMessages.cs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ internal static class LogMessages
// public const string IDX10622 = "IDX10622:";
// public const string IDX10623 = "IDX10623:";
// public const string IDX10624 = "IDX10624:";
public const string IDX10625 = "IDX10625: Failed to verify the authenticationTag length, the actual tag length '{0}' does not match the expected tag length '{1}'. authenticationTag: '{2}', algorithm: '{3}' See: https://aka.ms/IdentityModel/SkipAuthenticationTagLengthValidation";
kllysng marked this conversation as resolved.
Show resolved Hide resolved
public const string IDX10625 = "IDX10625: Failed to verify the authenticationTag length, the actual tag length '{0}' does not match the expected tag length '{1}'. authenticationTag: '{2}', algorithm: '{3}'.";
// public const string IDX10627 = "IDX10627:";
public const string IDX10628 = "IDX10628: Cannot set the MinimumSymmetricKeySizeInBits to less than '{0}'.";
public const string IDX10630 = "IDX10630: The '{0}' for signing cannot be smaller than '{1}' bits. KeySize: '{2}'.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ namespace Microsoft.IdentityModel.Tokens
/// </summary>
public class X509EncryptingCredentials : EncryptingCredentials
{
internal const string _useShortNameForRsaOaepKey = "Switch.Microsoft.IdentityModel.UseShortNameForRsaOaepKey";

/// <summary>
/// Designed to construct <see cref="EncryptingCredentials"/> based on a x509 certificate.
/// </summary>
Expand All @@ -23,7 +21,7 @@ public class X509EncryptingCredentials : EncryptingCredentials
/// </remarks>
/// <exception cref="ArgumentNullException">if 'certificate' is null.</exception>
public X509EncryptingCredentials(X509Certificate2 certificate)
: this(certificate, GetEncryptionAlgorithm(), SecurityAlgorithms.DefaultSymmetricEncryptionAlgorithm)
: this(certificate, SecurityAlgorithms.RsaOAEP, SecurityAlgorithms.DefaultSymmetricEncryptionAlgorithm)
{
}

Expand All @@ -50,15 +48,5 @@ public X509Certificate2 Certificate
get;
private set;
}

private static string GetEncryptionAlgorithm()
{
return ShouldUseShortNameForRsaOaepKey() ? SecurityAlgorithms.RsaOAEP : SecurityAlgorithms.DefaultAsymmetricKeyWrapAlgorithm;
}

private static bool ShouldUseShortNameForRsaOaepKey()
{
return AppContext.TryGetSwitch(_useShortNameForRsaOaepKey, out var useKeyWrap) && useKeyWrap;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4190,10 +4190,9 @@ public static TheoryData<CreateTokenTheoryData> IncludeSecurityTokenOnFailureTes
}

[Theory, MemberData(nameof(ValidateAuthenticationTagLengthTheoryData))]
public void ValidateTokenAsync_ModifiedAuthNTag(CreateTokenTheoryData theoryData)
public async Task ValidateTokenAsync_ModifiedAuthNTag(CreateTokenTheoryData theoryData)
{
// arrange
AppContext.SetSwitch(AuthenticatedEncryptionProvider._skipValidationOfAuthenticationTagLength, theoryData.EnableAppContextSwitch);
var payload = new JObject()
{
{ JwtRegisteredClaimNames.Email, "[email protected]" },
Expand All @@ -4217,9 +4216,7 @@ public void ValidateTokenAsync_ModifiedAuthNTag(CreateTokenTheoryData theoryData
var jweWithExtraCharacters = jwe + "_cannoli_hunts_truffles_";

// act
// calling ValidateTokenAsync.Result to prevent tests from sharing app context switch property
// normally, we would want to await ValidateTokenAsync().ConfigureAwait(false)
var tokenValidationResult = jsonWebTokenHandler.ValidateTokenAsync(jweWithExtraCharacters, theoryData.ValidationParameters).Result;
var tokenValidationResult = await jsonWebTokenHandler.ValidateTokenAsync(jweWithExtraCharacters, theoryData.ValidationParameters).ConfigureAwait(false);

// assert
Assert.Equal(theoryData.IsValid, tokenValidationResult.IsValid);
Expand Down Expand Up @@ -4281,47 +4278,6 @@ public static TheoryData<CreateTokenTheoryData> ValidateAuthenticationTagLengthT
ValidIssuer = "http://Default.Issuer.com",
},
IsValid = false
},
new("A128CBC-HS256_SkipTagLengthValidationAppContextSwitchOn_IsValid")
{
EnableAppContextSwitch = true,
Algorithm = SecurityAlgorithms.Aes128CbcHmacSha256,
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaPKCS1, SecurityAlgorithms.Aes128CbcHmacSha256),
ValidationParameters = new TokenValidationParameters
{
TokenDecryptionKey = KeyingMaterial.JsonWebKeyRsa256SigningCredentials.Key,
IssuerSigningKey = Default.SymmetricSigningKey256,
ValidAudience = "http://Default.Audience.com",
ValidIssuer = "http://Default.Issuer.com",
},
IsValid = true
},
new("A192CBC-HS384_SkipTagLengthValidationAppContextSwitchOn_IsValid")
{
EnableAppContextSwitch = true,
Algorithm = SecurityAlgorithms.Aes192CbcHmacSha384,
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaPKCS1, SecurityAlgorithms.Aes192CbcHmacSha384),
ValidationParameters = new TokenValidationParameters
{
TokenDecryptionKey = KeyingMaterial.JsonWebKeyRsa256SigningCredentials.Key,
IssuerSigningKey = Default.SymmetricSigningKey256,
ValidAudience = "http://Default.Audience.com",
ValidIssuer = "http://Default.Issuer.com",
},
IsValid = true
},
new("A256CBC-HS512_SkipTagLengthValidationAppContextSwitchOn_IsValid")
{
EnableAppContextSwitch = true,
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaPKCS1, SecurityAlgorithms.Aes256CbcHmacSha512),
ValidationParameters = new TokenValidationParameters
{
TokenDecryptionKey = signingCredentials512.Key,
IssuerSigningKey = Default.SymmetricSigningKey256,
ValidAudience = "http://Default.Audience.com",
ValidIssuer = "http://Default.Issuer.com",
},
IsValid = true
}
};
}
Expand Down Expand Up @@ -4370,8 +4326,6 @@ public CreateTokenTheoryData(string testId) : base(testId)
public IEnumerable<SecurityKey> ExpectedDecryptionKeys { get; set; }

public Dictionary<string, object> ExpectedClaims { get; set; }

public bool EnableAppContextSwitch { get; set; } = false;
}

// Overrides CryptoProviderFactory.CreateAuthenticatedEncryptionProvider to create AuthenticatedEncryptionProviderMock that provides AesGcm encryption.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static TheoryData<EncryptingCredentialsTheoryData> ConstructorATheoryData
new EncryptingCredentialsTheoryData
{
Key = null,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = SecurityAlgorithms.Aes128CbcHmacSha256,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'key'"),
TestId = "NullKey"
Expand All @@ -73,7 +73,7 @@ public static TheoryData<EncryptingCredentialsTheoryData> ConstructorATheoryData
new EncryptingCredentialsTheoryData
{
Key = Default.AsymmetricEncryptionKeyPublic,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = String.Empty,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'enc'"),
TestId = "EmptyEncString"
Expand All @@ -89,15 +89,15 @@ public static TheoryData<EncryptingCredentialsTheoryData> ConstructorATheoryData
new EncryptingCredentialsTheoryData
{
Key = Default.AsymmetricEncryptionKeyPublic,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = null,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'enc'"),
TestId = "NullEncString"
},
new EncryptingCredentialsTheoryData
{
Key = Default.AsymmetricEncryptionKeyPublic,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = SecurityAlgorithms.Aes128CbcHmacSha256,
TestId = "ValidTest"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public static TheoryData<MultiThreadingTheoryData> MultiThreadingCreateAndVerify
{
Claims = Default.PayloadDictionary,
SigningCredentials = new SigningCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaSha256, SecurityAlgorithms.Sha256),
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaOaepKeyWrap, SecurityAlgorithms.Aes128CbcHmacSha256)
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKey_2048, SecurityAlgorithms.RsaOAEP, SecurityAlgorithms.Aes128CbcHmacSha256)
};

var tokenValidationParametersEncryptedRsaKW = new TokenValidationParameters
Expand Down Expand Up @@ -174,7 +174,7 @@ public static TheoryData<MultiThreadingTheoryData> MultiThreadingCreateAndVerify
{
Claims = Default.PayloadDictionary,
SigningCredentials = new SigningCredentials(KeyingMaterial.RsaSecurityKeyCng_2048, SecurityAlgorithms.RsaSha256, SecurityAlgorithms.Sha256),
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKeyCng_2048, SecurityAlgorithms.RsaOaepKeyWrap, SecurityAlgorithms.Aes128CbcHmacSha256)
EncryptingCredentials = new EncryptingCredentials(KeyingMaterial.RsaSecurityKeyCng_2048, SecurityAlgorithms.RsaOAEP, SecurityAlgorithms.Aes128CbcHmacSha256)
};

var tokenValidationParametersEncryptedRsaKWCng = new TokenValidationParameters
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public static TheoryData<X509EncryptingCredentialsTheoryData> ConstructorsTheory
new X509EncryptingCredentialsTheoryData
{
Certificate = null,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = SecurityAlgorithms.Aes128CbcHmacSha256,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'certificate'"),
TestId = "NullCertificate"
Expand All @@ -58,7 +58,7 @@ public static TheoryData<X509EncryptingCredentialsTheoryData> ConstructorsTheory
new X509EncryptingCredentialsTheoryData
{
Certificate = Default.Certificate,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = String.Empty,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'enc'"),
TestId = "EmptyEncString"
Expand All @@ -74,15 +74,15 @@ public static TheoryData<X509EncryptingCredentialsTheoryData> ConstructorsTheory
new X509EncryptingCredentialsTheoryData
{
Certificate = Default.Certificate,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = null,
ExpectedException = ExpectedException.ArgumentNullException("IDX10000: The parameter 'enc'"),
TestId = "NullEncString"
},
new X509EncryptingCredentialsTheoryData
{
Certificate = Default.Certificate,
Alg = SecurityAlgorithms.RsaOaepKeyWrap,
Alg = SecurityAlgorithms.RsaOAEP,
Enc = SecurityAlgorithms.Aes128CbcHmacSha256,
TestId = "ValidTest"
}
Expand Down

This file was deleted.