-
Notifications
You must be signed in to change notification settings - Fork 432
SAML2 new model validation: Signature #2961
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
Merged
iNinja
merged 24 commits into
dev
from
iinglese/new-model-validation-for-saml2-signature
Nov 8, 2024
Merged
Changes from 14 commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
f92bafe
Added XmlValidationError. Added ValidationError property to XmlValida…
iNinja b141a1a
Added alternative versions using ValidationParameters to XML signatur…
iNinja 9731a37
Added XmlValidationFailure to ValidationFailureType
iNinja 5e106f7
Merge branch 'dev' into iinglese/new-model-validation-for-saml-signature
iNinja 08a2f19
Added refactored ValidateSignature method to SamlSecurityTokenHandler.
iNinja 5fee17a
Added tests to compare signature validation between the legacy and ne…
iNinja f9aeac7
Re-added API lost in merge to InternalAPI.Unshipped.txt
iNinja ccae551
Migrated refactored ValidateSignature from SamlSecurityTokenHandler t…
iNinja bdb6133
Updated Saml2SecurityTokenHandler's ValidateTokenAsync to validate si…
iNinja 668eae3
Added tests
iNinja 0b505f6
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 0487e34
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 0dd0f5b
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 3f58a63
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 0a7d1c4
Update src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTok…
iNinja 6bc69cd
Addressed PR feedback in both SAML and SAML2 signature validations to…
iNinja 3560e15
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 9c505b6
Optimised signature validation in SAML and SAML2 for the expected mos…
iNinja b00716c
Removed debug information
iNinja c95968b
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja 3eef64e
Addressed PR feedback
iNinja 7116d24
Addressed PR feedback
iNinja 2b86734
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja e726b75
Merge branch 'dev' into iinglese/new-model-validation-for-saml2-signa…
iNinja File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
168 changes: 168 additions & 0 deletions
168
src/Microsoft.IdentityModel.Tokens.Saml/Saml2/Saml2SecurityTokenHandler.ValidateSignature.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
| using System.Text; | ||
| using Microsoft.IdentityModel.Tokens.Saml; | ||
| using Microsoft.IdentityModel.Xml; | ||
| using TokenLogMessages = Microsoft.IdentityModel.Tokens.LogMessages; | ||
|
|
||
| #nullable enable | ||
| namespace Microsoft.IdentityModel.Tokens.Saml2 | ||
| { | ||
| public partial class Saml2SecurityTokenHandler : SecurityTokenHandler | ||
| { | ||
| internal static ValidationResult<SecurityKey> ValidateSignature( | ||
| Saml2SecurityToken samlToken, | ||
| ValidationParameters validationParameters, | ||
| #pragma warning disable CA1801 // Review unused parameters | ||
| CallContext callContext) | ||
| #pragma warning restore CA1801 // Review unused parameters | ||
| { | ||
| if (samlToken is null) | ||
| { | ||
| return ValidationError.NullParameter( | ||
| nameof(samlToken), | ||
| new StackFrame(true)); | ||
| } | ||
|
|
||
| if (validationParameters is null) | ||
| { | ||
| return ValidationError.NullParameter( | ||
| nameof(validationParameters), | ||
| new StackFrame(true)); | ||
| } | ||
|
|
||
| // Delegate is set by the user, we call it and return the result. | ||
| if (validationParameters.SignatureValidator is not null) | ||
iNinja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return validationParameters.SignatureValidator(samlToken, validationParameters, null, callContext); | ||
|
|
||
| // If the user wants to accept unsigned tokens, they must implement the delegate | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (samlToken.Assertion.Signature is null) | ||
iNinja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return new XmlValidationError( | ||
| new MessageDetail( | ||
| TokenLogMessages.IDX10504, | ||
| samlToken.Assertion.CanonicalString), | ||
| ValidationFailureType.SignatureValidationFailed, | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| typeof(SecurityTokenValidationException), | ||
| new StackFrame(true)); | ||
|
|
||
| IList<SecurityKey>? keys = null; | ||
brentschmaltz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| SecurityKey? resolvedKey = null; | ||
| bool keyMatched = false; | ||
|
|
||
| if (validationParameters.IssuerSigningKeyResolver is not null) | ||
| { | ||
| resolvedKey = validationParameters.IssuerSigningKeyResolver( | ||
| samlToken.Assertion.CanonicalString, | ||
| samlToken, | ||
| samlToken.Assertion.Signature.KeyInfo?.Id, | ||
| validationParameters, | ||
| null, | ||
| callContext); | ||
| } | ||
| else | ||
iNinja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| resolvedKey = SamlTokenUtilities.ResolveTokenSigningKey(samlToken.Assertion.Signature.KeyInfo, validationParameters); | ||
| } | ||
|
|
||
| if (resolvedKey is null) | ||
| { | ||
| if (validationParameters.TryAllIssuerSigningKeys) | ||
| keys = validationParameters.IssuerSigningKeys; | ||
| } | ||
| else | ||
| { | ||
| keys = [resolvedKey]; | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| keyMatched = true; | ||
| } | ||
|
|
||
| bool canMatchKey = samlToken.Assertion.Signature.KeyInfo != null; | ||
| List<ValidationError> errors = new(); | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| StringBuilder keysAttempted = new(); | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| if (keys is not null) | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { | ||
| for (int i = 0; i < keys.Count; i++) | ||
| { | ||
| SecurityKey key = keys[i]; | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| ValidationResult<string> algorithmValidationResult = validationParameters.AlgorithmValidator( | ||
| samlToken.Assertion.Signature.SignedInfo.SignatureMethod, | ||
| key, | ||
| samlToken, | ||
| validationParameters, | ||
| callContext); | ||
|
|
||
| if (!algorithmValidationResult.IsValid) | ||
| { | ||
| errors.Add(algorithmValidationResult.UnwrapError()); | ||
| } | ||
| else | ||
| { | ||
| var validationError = samlToken.Assertion.Signature.Verify( | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| key, | ||
| validationParameters.CryptoProviderFactory ?? key.CryptoProviderFactory, | ||
| callContext); | ||
|
|
||
| if (validationError is null) | ||
| { | ||
| samlToken.SigningKey = key; | ||
|
|
||
| return key; | ||
| } | ||
| else | ||
| { | ||
| errors.Add(validationError.AddStackFrame(new StackFrame())); | ||
iNinja marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| keysAttempted.Append(key.ToString()); | ||
| if (canMatchKey && !keyMatched && key.KeyId is not null && samlToken.Assertion.Signature.KeyInfo is not null) | ||
| keyMatched = samlToken.Assertion.Signature.KeyInfo.MatchesKey(key); | ||
| } | ||
| } | ||
|
|
||
| if (canMatchKey && keyMatched) | ||
msbw2 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return new XmlValidationError( | ||
| new MessageDetail( | ||
| TokenLogMessages.IDX10514, | ||
| keysAttempted.ToString(), | ||
| samlToken.Assertion.Signature.KeyInfo, | ||
| GetErrorStrings(errors), | ||
| samlToken), | ||
| ValidationFailureType.SignatureValidationFailed, | ||
| typeof(SecurityTokenInvalidSignatureException), | ||
| new StackFrame(true)); | ||
|
|
||
| if (keysAttempted.Length > 0) | ||
msbw2 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return new XmlValidationError( | ||
| new MessageDetail( | ||
| TokenLogMessages.IDX10512, | ||
| keysAttempted.ToString(), | ||
| GetErrorStrings(errors), | ||
| samlToken), | ||
| ValidationFailureType.SignatureValidationFailed, | ||
| typeof(SecurityTokenSignatureKeyNotFoundException), | ||
| new StackFrame(true)); | ||
|
|
||
| return new XmlValidationError( | ||
| new MessageDetail(TokenLogMessages.IDX10500), | ||
| ValidationFailureType.SignatureValidationFailed, | ||
| typeof(SecurityTokenSignatureKeyNotFoundException), | ||
| new StackFrame(true)); | ||
| } | ||
|
|
||
| private static string GetErrorStrings(List<ValidationError> errors) | ||
| { | ||
| StringBuilder sb = new(); | ||
iNinja marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for (int i = 0; i < errors.Count; i++) | ||
| { | ||
| sb.AppendLine(errors[i].ToString()); | ||
| } | ||
|
|
||
| return sb.ToString(); | ||
| } | ||
| } | ||
| } | ||
| #nullable restore | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.