Skip to content

Commit

Permalink
KeyID should be present in exception messages logs and should not be…
Browse files Browse the repository at this point in the history
… considered PII. (#3102)

* mark keysAttempted as non PII

* mark kid's as non PII

* mark key as non pii in .Tokens classes

* final marks

* only log KID
  • Loading branch information
kllysng authored Jan 28, 2025
1 parent bef98ca commit 3b32fd9
Showing 25 changed files with 190 additions and 99 deletions.
Original file line number Diff line number Diff line change
@@ -1274,7 +1274,10 @@ internal static string EncryptToken(byte[] innerTokenUtf8Bytes, SecurityTokenDes
}
catch (Exception ex)
{
throw LogHelper.LogExceptionMessage(new SecurityTokenEncryptionFailedException(LogHelper.FormatInvariant(TokenLogMessages.IDX10616, LogHelper.MarkAsNonPII(encryptingCredentials.Enc), encryptingCredentials.Key), ex));
throw LogHelper.LogExceptionMessage(
new SecurityTokenEncryptionFailedException(
LogHelper.FormatInvariant(TokenLogMessages.IDX10616, LogHelper.MarkAsNonPII(encryptingCredentials.Enc), LogHelper.MarkAsNonPII(encryptingCredentials.Key.KeyId)),
ex));
}
}
}
@@ -1296,13 +1299,13 @@ internal IEnumerable<SecurityKey> GetContentEncryptionKeys(JsonWebToken jwtToken
if (key != null)
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(TokenLogMessages.IDX10904, key);
LogHelper.LogInformation(TokenLogMessages.IDX10904, LogHelper.MarkAsNonPII(key.KeyId));
}
else if (configuration != null)
{
key = ResolveTokenDecryptionKeyFromConfig(jwtToken, configuration);
if (key != null && LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(TokenLogMessages.IDX10905, key);
LogHelper.LogInformation(TokenLogMessages.IDX10905, LogHelper.MarkAsNonPII(key.KeyId));
}

if (key != null)
@@ -1383,13 +1386,19 @@ internal IEnumerable<SecurityKey> GetContentEncryptionKeys(JsonWebToken jwtToken
(exceptionStrings ??= new StringBuilder()).AppendLine(ex.ToString());
}

(keysAttempted ??= new StringBuilder()).AppendLine(key.ToString());
(keysAttempted ??= new StringBuilder()).AppendLine(key.KeyId);
}

if (unwrappedKeys.Count > 0 || exceptionStrings is null)
return unwrappedKeys;
else
throw LogHelper.LogExceptionMessage(new SecurityTokenKeyWrapException(LogHelper.FormatInvariant(TokenLogMessages.IDX10618, (object)keysAttempted ?? "", (object)exceptionStrings ?? "", jwtToken)));
throw LogHelper.LogExceptionMessage(
new SecurityTokenKeyWrapException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10618,
LogHelper.MarkAsNonPII((object)keysAttempted ?? ""),
LogHelper.MarkAsNonPII((object)exceptionStrings ?? ""),
jwtToken)));
}
}
}
Original file line number Diff line number Diff line change
@@ -196,7 +196,7 @@ internal ValidationResult<string> DecryptToken(
(exceptionStrings ??= new StringBuilder()).AppendLine(ex.ToString());
}

(keysAttempted ??= new StringBuilder()).AppendLine(key.ToString());
(keysAttempted ??= new StringBuilder()).AppendLine(key.KeyId);
}

if (unwrappedKeys.Count > 0 || exceptionStrings is null)
@@ -206,7 +206,7 @@ internal ValidationResult<string> DecryptToken(
ValidationError validationError = new(
new MessageDetail(
TokenLogMessages.IDX10618,
keysAttempted?.ToString() ?? "",
LogHelper.MarkAsNonPII(keysAttempted?.ToString() ?? ""),
exceptionStrings?.ToString() ?? "",
LogHelper.MarkAsSecurityArtifact(jwtToken, JwtTokenUtilities.SafeLogJwtToken)),
ValidationFailureType.TokenDecryptionFailed,
Original file line number Diff line number Diff line change
@@ -297,7 +297,7 @@ private static ValidationResult<SecurityKey> ValidateSignatureWithKey(
return new SignatureValidationError(
new MessageDetail(
TokenLogMessages.IDX10636,
key?.ToString() ?? "Null",
LogHelper.MarkAsNonPII(key?.KeyId ?? "Null"),
LogHelper.MarkAsNonPII(jsonWebToken.Alg)),
ValidationFailureType.SignatureValidationFailed,
typeof(SecurityTokenInvalidOperationException),
Original file line number Diff line number Diff line change
@@ -320,7 +320,7 @@ internal static bool ValidateSignature(JsonWebToken jsonWebToken, SecurityKey ke
if (!cryptoProviderFactory.IsSupportedAlgorithm(jsonWebToken.Alg, key))
{
if (LogHelper.IsEnabled(EventLogLevel.Informational))
LogHelper.LogInformation(LogMessages.IDX14000, LogHelper.MarkAsNonPII(jsonWebToken.Alg), key);
LogHelper.LogInformation(LogMessages.IDX14000, LogHelper.MarkAsNonPII(jsonWebToken.Alg), LogHelper.MarkAsNonPII(key.KeyId));

return false;
}
@@ -330,7 +330,12 @@ internal static bool ValidateSignature(JsonWebToken jsonWebToken, SecurityKey ke
try
{
if (signatureProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(TokenLogMessages.IDX10636, key == null ? "Null" : key.ToString(), LogHelper.MarkAsNonPII(jsonWebToken.Alg))));
throw LogHelper.LogExceptionMessage
(new InvalidOperationException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10636,
LogHelper.MarkAsNonPII(key == null ? "Null" : key.KeyId),
LogHelper.MarkAsNonPII(jsonWebToken.Alg))));

return EncodingUtils.PerformEncodingDependentOperation<bool, string, int, SignatureProvider>(
jsonWebToken.EncodedToken,
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ internal static ValidationResult<string> DecryptJwtToken(
}

if (key != null)
(keysAttempted ??= new StringBuilder()).AppendLine(key.ToString());
(keysAttempted ??= new StringBuilder()).AppendLine(key.KeyId);
}

if (!decryptionSucceeded)
37 changes: 25 additions & 12 deletions src/Microsoft.IdentityModel.JsonWebTokens/JwtTokenUtilities.cs
Original file line number Diff line number Diff line change
@@ -73,7 +73,12 @@ public static string CreateEncodedSignature(string input, SigningCredentials sig
var cryptoProviderFactory = signingCredentials.CryptoProviderFactory ?? signingCredentials.Key.CryptoProviderFactory;
var signatureProvider = cryptoProviderFactory.CreateForSigning(signingCredentials.Key, signingCredentials.Algorithm);
if (signatureProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(TokenLogMessages.IDX10637, signingCredentials.Key == null ? "Null" : signingCredentials.Key.ToString(), LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10637,
LogHelper.MarkAsNonPII(signingCredentials.Key == null ? "Null" : signingCredentials.Key.KeyId),
LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));

try
{
@@ -105,7 +110,12 @@ public static string CreateEncodedSignature(string input, SigningCredentials sig
var cryptoProviderFactory = signingCredentials.CryptoProviderFactory ?? signingCredentials.Key.CryptoProviderFactory;
var signatureProvider = cryptoProviderFactory.CreateForSigning(signingCredentials.Key, signingCredentials.Algorithm, cacheProvider);
if (signatureProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(TokenLogMessages.IDX10637, signingCredentials.Key == null ? "Null" : signingCredentials.Key.ToString(), LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10637,
LogHelper.MarkAsNonPII(signingCredentials.Key == null ? "Null" : signingCredentials.Key.KeyId),
LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));

try
{
@@ -138,7 +148,7 @@ internal static byte[] CreateEncodedSignature(
new InvalidOperationException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10637,
signingCredentials.Key == null ? "Null" : signingCredentials.Key.ToString(),
LogHelper.MarkAsNonPII(signingCredentials.Key == null ? "Null" : signingCredentials.Key.KeyId),
LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));

try
@@ -179,7 +189,8 @@ internal static bool CreateSignature(
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(
TokenLogMessages.IDX10637, signingCredentials.Key == null ? "Null" : signingCredentials.Key.ToString(),
TokenLogMessages.IDX10637,
LogHelper.MarkAsNonPII(signingCredentials.Key == null ? "Null" : signingCredentials.Key.KeyId),
LogHelper.MarkAsNonPII(signingCredentials.Algorithm))));

try
@@ -257,7 +268,7 @@ internal static string DecryptJwtToken(
if (cryptoProviderFactory == null)
{
if (LogHelper.IsEnabled(EventLogLevel.Warning))
LogHelper.LogWarning(TokenLogMessages.IDX10607, key);
LogHelper.LogWarning(TokenLogMessages.IDX10607, LogHelper.MarkAsNonPII(key.KeyId));

continue;
}
@@ -273,7 +284,7 @@ internal static string DecryptJwtToken(
if (!cryptoProviderFactory.IsSupportedAlgorithm(jsonWebToken.Enc, key))
{
if (LogHelper.IsEnabled(EventLogLevel.Warning))
LogHelper.LogWarning(TokenLogMessages.IDX10611, LogHelper.MarkAsNonPII(decryptionParameters.Enc), key);
LogHelper.LogWarning(TokenLogMessages.IDX10611, LogHelper.MarkAsNonPII(decryptionParameters.Enc), LogHelper.MarkAsNonPII(key.KeyId));

algorithmNotSupportedByCryptoProvider = true;
continue;
@@ -299,7 +310,7 @@ internal static string DecryptJwtToken(
if (!cryptoProviderFactory.IsSupportedAlgorithm(decryptionParameters.Enc, key))
{
if (LogHelper.IsEnabled(EventLogLevel.Warning))
LogHelper.LogWarning(TokenLogMessages.IDX10611, LogHelper.MarkAsNonPII(decryptionParameters.Enc), key);
LogHelper.LogWarning(TokenLogMessages.IDX10611, LogHelper.MarkAsNonPII(decryptionParameters.Enc), LogHelper.MarkAsNonPII(key.KeyId));

algorithmNotSupportedByCryptoProvider = true;
continue;
@@ -326,7 +337,7 @@ internal static string DecryptJwtToken(
}

if (key != null)
(keysAttempted ??= new StringBuilder()).AppendLine(key.ToString());
(keysAttempted ??= new StringBuilder()).AppendLine(key.KeyId);
}

if (!decryptionSucceeded)
@@ -367,7 +378,7 @@ private static ValidationError GetDecryptionError(
return new ValidationError(
new MessageDetail(
TokenLogMessages.IDX10603,
keysAttempted.ToString(),
LogHelper.MarkAsNonPII(keysAttempted.ToString()),
exceptionStrings?.ToString() ?? string.Empty,
LogHelper.MarkAsSecurityArtifact(decryptionParameters.EncodedToken, SafeLogJwtToken)),
ValidationFailureType.TokenDecryptionFailed,
@@ -397,7 +408,7 @@ private static byte[] DecryptToken(CryptoProviderFactory cryptoProviderFactory,
using (AuthenticatedEncryptionProvider decryptionProvider = cryptoProviderFactory.CreateAuthenticatedEncryptionProvider(key, encAlg))
{
if (decryptionProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(TokenLogMessages.IDX10610, key, LogHelper.MarkAsNonPII(encAlg))));
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(TokenLogMessages.IDX10610, LogHelper.MarkAsNonPII(key.KeyId), LogHelper.MarkAsNonPII(encAlg))));

return decryptionProvider.Decrypt(
ciphertext,
@@ -445,7 +456,8 @@ internal static SecurityKey GetSecurityKey(
if (JwtConstants.DirectKeyUseAlg.Equals(encryptingCredentials.Alg))
{
if (!cryptoProviderFactory.IsSupportedAlgorithm(encryptingCredentials.Enc, encryptingCredentials.Key))
throw LogHelper.LogExceptionMessage(new SecurityTokenEncryptionFailedException(LogHelper.FormatInvariant(TokenLogMessages.IDX10615, LogHelper.MarkAsNonPII(encryptingCredentials.Enc), encryptingCredentials.Key)));
throw LogHelper.LogExceptionMessage(new SecurityTokenEncryptionFailedException(
LogHelper.FormatInvariant(TokenLogMessages.IDX10615, LogHelper.MarkAsNonPII(encryptingCredentials.Enc), LogHelper.MarkAsNonPII(encryptingCredentials.Key.KeyId))));

securityKey = encryptingCredentials.Key;
}
@@ -484,7 +496,8 @@ internal static SecurityKey GetSecurityKey(
else
{
if (!cryptoProviderFactory.IsSupportedAlgorithm(encryptingCredentials.Alg, encryptingCredentials.Key))
throw LogHelper.LogExceptionMessage(new SecurityTokenEncryptionFailedException(LogHelper.FormatInvariant(TokenLogMessages.IDX10615, LogHelper.MarkAsNonPII(encryptingCredentials.Alg), encryptingCredentials.Key)));
throw LogHelper.LogExceptionMessage(new SecurityTokenEncryptionFailedException(
LogHelper.FormatInvariant(TokenLogMessages.IDX10615, LogHelper.MarkAsNonPII(encryptingCredentials.Alg), LogHelper.MarkAsNonPII(encryptingCredentials.Key.KeyId))));

// only 128, 384 and 512 AesCbcHmac for CEK algorithm
if (SecurityAlgorithms.Aes128CbcHmacSha256.Equals(encryptingCredentials.Enc))
Original file line number Diff line number Diff line change
@@ -57,7 +57,7 @@ public ConfigurationValidationResult Validate(OpenIdConnectConfiguration openIdC
LogMessages.IDX21818,
LogHelper.MarkAsNonPII(MinimumNumberOfKeys),
LogHelper.MarkAsNonPII(numberOfValidKeys),
string.IsNullOrEmpty(convertKeyInfos) ? "None" : convertKeyInfos),
string.IsNullOrEmpty(convertKeyInfos) ? "None" : LogHelper.MarkAsNonPII(convertKeyInfos)),
Succeeded = false
};
}
Original file line number Diff line number Diff line change
@@ -656,7 +656,9 @@ internal virtual async Task<SecurityKey> ValidateSignatureAsync(JsonWebToken sig
{
signatureProvider = popKey.CryptoProviderFactory.CreateForVerifying(popKey, signedHttpRequest.Alg, false);
if (signatureProvider == null)
throw LogHelper.LogExceptionMessage(new InvalidOperationException(LogHelper.FormatInvariant(Tokens.LogMessages.IDX10636, popKey.ToString(), LogHelper.MarkAsNonPII(signedHttpRequest.Alg))));
throw LogHelper.LogExceptionMessage(
new InvalidOperationException(
LogHelper.FormatInvariant(Tokens.LogMessages.IDX10636, LogHelper.MarkAsNonPII(popKey.KeyId), LogHelper.MarkAsNonPII(signedHttpRequest.Alg))));

if (EncodingUtils.PerformEncodingDependentOperation<bool, string, int, SignatureProvider>(
signedHttpRequest.EncodedToken,
@@ -1099,7 +1101,7 @@ internal virtual SecurityKey ResolvePopKeyFromJwk(JsonWebKey jsonWebKey, SignedH
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23015, LogHelper.MarkAsNonPII(key.GetType().ToString()))));
}
else
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23016, jsonWebKey.ToString())));
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23016, LogHelper.MarkAsNonPII(jsonWebKey.KeyId))));
}

/// <summary>
@@ -1164,7 +1166,8 @@ internal virtual async Task<SecurityKey> ResolvePopKeyFromJkuAsync(string jkuSet
new SignedHttpRequestInvalidPopKeyException(
LogHelper.FormatInvariant(
LogMessages.IDX23021,
LogHelper.MarkAsNonPII(cnf.Kid), string.Join(", ", popKeys.Select(x => x.KeyId ?? "Null")))));
LogHelper.MarkAsNonPII(cnf.Kid),
LogHelper.MarkAsNonPII(string.Join(", ", popKeys.Select(x => x.KeyId ?? "Null"))))));
}
else
{
Original file line number Diff line number Diff line change
@@ -106,7 +106,12 @@ internal static async Task<JsonWebKey> DecryptSymmetricPopKeyAsync(JsonWebTokenH
}
catch (Exception e)
{
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(LogHelper.FormatInvariant(LogMessages.IDX23018, string.Join(", ", decryptionKeys.Select(x => x?.KeyId ?? "Null")), e), e));
throw LogHelper.LogExceptionMessage(new SignedHttpRequestInvalidPopKeyException(
LogHelper.FormatInvariant(
LogMessages.IDX23018,
LogHelper.MarkAsNonPII(string.Join(", ", decryptionKeys.Select(x => x?.KeyId ?? "Null"))),
e),
e));
}
}
}
Loading

0 comments on commit 3b32fd9

Please sign in to comment.