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

WsFederationMessage.GetToken() is appending "
" to SAML (Formatting is breaking signature check - Sign in fails) #1258

Closed
smiket opened this issue Sep 26, 2019 · 43 comments
Assignees
Labels
Bug Product is not functioning as expected Customer reported Indicates issue was opened by customer Overflow Customer ask that was not fixed in previous release, has higher priority P2 Little less important, but we would like to do this WsTrust Related to WsTrust

Comments

@smiket
Copy link

smiket commented Sep 26, 2019

WS Federation was working for me prior to upgrading to .NET Core 3. I have a typical setup shown below, followed by exception that began after the upgrade. Not sure if there is any changes I should make to affect / resolve this.

Also it would be nice if I could step into the code (could not load module / find symbols). I may be missing something there - let me know if there is a way for me to step through and troubleshoot further.

I'm using Microsoft.AspNetCore.Authentication.WsFederation 3.0.0, which depends on / brings in Microsoft.IdentityModel.Protocols.WsFederation (>= 5.5.0).

            services.AddAuthentication(sharedOptions =>
            {
                sharedOptions.DefaultScheme = CookieAuthenticationDefaults.AuthenticationScheme;
                sharedOptions.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
                sharedOptions.DefaultChallengeScheme = WsFederationDefaults.AuthenticationScheme;
            })
            .AddWsFederation(federationOptions =>
            {
                federationOptions.MetadataAddress = _options.MetadataAddress;
                federationOptions.Wtrealm = _options.Realm;
                federationOptions.Events.OnSecurityTokenValidated = OnSecurityTokenValidated;
                federationOptions.Events.OnRedirectToIdentityProvider = OnRedirectToIdentityProvider;
            })
            .AddCookie(cookieOptions =>
            {
                cookieOptions.ExpireTimeSpan = TimeSpan.FromDays(1);
                cookieOptions.Events.OnValidatePrincipal = OnValidateCookiePrincipal;
                cookieOptions.Events.OnRedirectToAccessDenied = OnRedirectToAccessDenied;
            });
Exception occurred while processing message. [Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler] 
 Microsoft.IdentityModel.Tokens.SecurityTokenInvalidSignatureException: IDX10503: Signature validation failed. Keys tried: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'. , KeyId: 0F0AC48C0B8FB903E0F49823DD2C133908EF139E
'.
Exceptions caught:
 'Microsoft.IdentityModel.Xml.XmlValidationException: IDX30200: The 'Signature' did not validate. CryptoProviderFactory: 'Microsoft.IdentityModel.Tokens.CryptoProviderFactory', SecurityKey: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'.'.
   at Microsoft.IdentityModel.Xml.Signature.Verify(SecurityKey key, CryptoProviderFactory cryptoProviderFactory)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(SamlSecurityToken samlToken, String token, TokenValidationParameters validationParameters)
'.
token: 'Microsoft.IdentityModel.Tokens.Saml.SamlSecurityToken'.
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(SamlSecurityToken samlToken, String token, TokenValidationParameters validationParameters)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(String token, TokenValidationParameters validationParameters)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateToken(String token, TokenValidationParameters validationParameters, SecurityToken& validatedToken)
   at Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler.HandleRemoteAuthenticateAsync()
09-26-2019 11:08:04.729 INF [0105C316619L, ] Error from RemoteAuthentication: IDX10503: Signature validation failed. Keys tried: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'. , KeyId: 0F0AC48C0B8FB903E0F49823DD2C133908EF139E
'.
Exceptions caught:
 'Microsoft.IdentityModel.Xml.XmlValidationException: IDX30200: The 'Signature' did not validate. CryptoProviderFactory: 'Microsoft.IdentityModel.Tokens.CryptoProviderFactory', SecurityKey: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'.'.
   at Microsoft.IdentityModel.Xml.Signature.Verify(SecurityKey key, CryptoProviderFactory cryptoProviderFactory)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(SamlSecurityToken samlToken, String token, TokenValidationParameters validationParameters)
'.
token: 'Microsoft.IdentityModel.Tokens.Saml.SamlSecurityToken'.. [Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler] 
 09-26-2019 11:08:04.875 ERR [0105C316619L, ] An unhandled exception has occurred while executing the request. [Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware] 
 System.Exception: An error was encountered while handling the remote login.
 ---> Microsoft.IdentityModel.Tokens.SecurityTokenInvalidSignatureException: IDX10503: Signature validation failed. Keys tried: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'. , KeyId: 0F0AC48C0B8FB903E0F49823DD2C133908EF139E
'.
Exceptions caught:
 'Microsoft.IdentityModel.Xml.XmlValidationException: IDX30200: The 'Signature' did not validate. CryptoProviderFactory: 'Microsoft.IdentityModel.Tokens.CryptoProviderFactory', SecurityKey: 'Microsoft.IdentityModel.Tokens.X509SecurityKey, KeyId: '0F0AC48C0B8FB903E0F49823DD2C133908EF139E', InternalId: '6322e1ea-2f46-4843-89d6-539a9828803b'.'.
   at Microsoft.IdentityModel.Xml.Signature.Verify(SecurityKey key, CryptoProviderFactory cryptoProviderFactory)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(SamlSecurityToken samlToken, String token, TokenValidationParameters validationParameters)
'.
token: 'Microsoft.IdentityModel.Tokens.Saml.SamlSecurityToken'.
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(SamlSecurityToken samlToken, String token, TokenValidationParameters validationParameters)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateSignature(String token, TokenValidationParameters validationParameters)
   at Microsoft.IdentityModel.Tokens.Saml.SamlSecurityTokenHandler.ValidateToken(String token, TokenValidationParameters validationParameters, SecurityToken& validatedToken)
   at Microsoft.AspNetCore.Authentication.WsFederation.WsFederationHandler.HandleRemoteAuthenticateAsync()
   --- End of inner exception stack trace ---
   at Microsoft.AspNetCore.Authentication.RemoteAuthenticationHandler`1.HandleRequestAsync()
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Disney.DTCI.AdSales.Common.Middleware.AdVisorCorrelationLogging.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
@smiket
Copy link
Author

smiket commented Sep 27, 2019

I found the issue by putting a breakpoint in WsFederationHandler (line 213).
var token = wsFederationMessage.GetToken();

The above token winds up with newline sequences appended on every line as such:


However if I look directly at the above wsFederationMessage, I can see the incoming wresult.
The response message does not have any newline sequences - i.e. the raw payload is clean.

If I do the following while in the debugger (after that line) I no longer get the signature error.
token = token.Replace("
", "");

So it would seem there is some code that is appending the newline escapes and causing the issue.
Microsoft.IdentityModel.Protocols.WsFederation.WsFederationMessage.GetToken()

It's kind of a show stopper for us - we cannot use V3.
Happy to help resolve this, let me know if there is anything I can do to move this forward.

Thanks,
Steve

@smiket
Copy link
Author

smiket commented Sep 27, 2019

And finally, if you change the code for GetToken as follows, you no longer get the appended newline chars etc. Not sure this is the proper approach performance-wise. But it does eliminate the extra chars:

                                // as the current node might not be a content node, the reader should skip ahead to the next content node.
                                xmlReader.MoveToContent();

                                using (var ms = new MemoryStream())
                                {
                                    var settings = new XmlWriterSettings();
                                    settings.NewLineHandling = NewLineHandling.None;
                                    settings.Indent = false;
                                    settings.Encoding = Encoding.UTF8;
                                    settings.OmitXmlDeclaration = true;

                                    using (var writer = XmlWriter.Create(ms, settings))
                                    {
                                        writer.WriteNode(xmlReader, true);
                                        writer.Flush();
                                    }

                                    ms.Seek(0, SeekOrigin.Begin);
                                    var tokenBytes = ms.ToArray();
                                    token = Encoding.UTF8.GetString(tokenBytes);
                                }

                                // </RequestedSecurityToken>
                                xmlReader.ReadEndElement();

@smiket smiket changed the title SAML Signature failing after upgrade to .NET Core 3 WsFederationMessage.GetToken() is appending "&#xD;" to SAML (Formatting is breaking signature check - Sign in fails) Sep 27, 2019
@brentschmaltz brentschmaltz added Bug Product is not functioning as expected P1 More important, prioritize highly Customer reported Indicates issue was opened by customer labels Sep 30, 2019
@brentschmaltz
Copy link
Member

@smiket thanks for the detailed work.
We are planning a complete overhaul of our WsFederation work and are adding WsTrust support.
We are planning a release soon with a feature for Proof-Of-Possession, we will investigate this issue and include it in that release.

Sorry for the hassle.

@brentschmaltz brentschmaltz self-assigned this Sep 30, 2019
@brentschmaltz brentschmaltz added this to the 5.x Release milestone Oct 10, 2019
@brentschmaltz brentschmaltz added 5.x and 6.x WsTrust Related to WsTrust labels Oct 10, 2019
@brentschmaltz brentschmaltz modified the milestones: 5.x Release, 5.6.1 Oct 10, 2019
@brentschmaltz
Copy link
Member

@smiket is this still a blocker?
If so, can you share a WsFed message that fails?

@everettcomstock
Copy link

@brentschmaltz is there any update on this issue? I am experiencing the exact same issue as @smiket, and I'd like to know your recommended course of action? If the updated release is right around the corner, I think we could wait a week or two.

@brentschmaltz
Copy link
Member

@everettcomstock the release with undated WsFed is unlikely in a couple of weeks.
We may be able to issue a . release.
Can you share a WsFedMessage with me that has the issue?

@everettcomstock
Copy link

@brentschmaltz , here is a heavily redacted example... I can PM you a real message if necessary:

<saml:Assertion MajorVersion="1" MinorVersion="1" AssertionID="IOjlZrMgiugArEj4_szrzFbk1UE" IssueInstant="2020-01-14T14:04:40.796Z" Issuer="https://foo.com" xmlns:saml="urn:oasis:names:tc:SAML:1.0:assertion">
<ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">&#xD;
<ds:SignedInfo>&#xD;
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>&#xD;
<ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>&#xD;
<ds:Reference URI="#IOjlZrMgArEj4_szrzdOIFbk1UE">&#xD;
<ds:Transforms>&#xD;
<ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>&#xD;
<ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>&#xD;
</ds:Transforms>&#xD;
<ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>&#xD;
<ds:DigestValue></ds:DigestValue>&#xD;
</ds:Reference>&#xD;
</ds:SignedInfo>&#xD;
<ds:SignatureValue>&#xD;
</ds:SignatureValue>&#xD;
<ds:KeyInfo>&#xD;
<ds:X509Data>&#xD;
<ds:X509Certificate>&#xD;
</ds:X509Certificate>&#xD;
</ds:X509Data>&#xD;
</ds:KeyInfo>&#xD;
</ds:Signature></saml:Assertion>

@smiket
Copy link
Author

smiket commented Jan 15, 2020

Hi @brentschmaltz, I have not tried it since originally reporting the issue. It looks like Everett has identical problem. Happy to try any updates you may have, thanks for keeping it on your radar 👍

@brentschmaltz
Copy link
Member

Are you saying that our method adds "&#xD" to the SamlToken?
If so that's not good, you can't replace it.

@smiket
Copy link
Author

smiket commented Jan 15, 2020

Yeah, it adds those characters. See my 3rd comment where I suggest a fix. I forked and tried that locally and it worked for me.
Basically I changed the XML Writer settings to use ‘NewLineHandling.None‘ and that helped

@everettcomstock
Copy link

@brentschmaltz ... just for clarity, yes it is adding those characters in our application too. It seems like Stephen has identified the precise point where the issue is occurring. If we can help in any way, please don't hesitate to reach out. Thanks!

@brentschmaltz
Copy link
Member

@smiket @everettcomstock we will fix this.
Just to confirm you are using 5.5.0 and you have a Windows OS right?

@everettcomstock
Copy link

@brentschmaltz , yes.. 5.5.0 and Windows / Azure

@brentschmaltz
Copy link
Member

@everettcomstock we are planning on fixing this in our 6.x release.
We are working through that deliverable schedule.

@everettcomstock
Copy link

@brentschmaltz, thanks for the update. We'll add a card on our board for the 6.x release.

@brentschmaltz brentschmaltz modified the milestones: 5.7.0, 6.6.0 May 1, 2020
@henrik-me henrik-me added this to To do in IdentityModel Board via automation May 5, 2020
@henrik-me henrik-me removed this from To do in IdentityModel Board May 15, 2020
@brentschmaltz
Copy link
Member

brentschmaltz commented Aug 10, 2020

@smiket the most likely reason that " " is added is because a "\n" character is found in the xml that arrives. Can you share a copy of the wasignin message that was received so that i can be sure?

@henrik-me henrik-me moved this from To do to In progress in IdentityModel Board Aug 10, 2020
@krishnajampana
Copy link

krishnajampana commented Aug 12, 2020

I have also similar issue when update wsfederation library to 3.1.5. I am tried but not sure how to override get token method.
could you please help me.
let me know when this is fixed. if the fix available which library needs to update.

@brentschmaltz
Copy link
Member

@krishnajampana would you be able to share the wasignin message that exhibits the issue?

@krishnajampana
Copy link

@krishnajampana would you be able to share the wasignin message that exhibits the issue?

ds:X509Data
ds:X509Certificate

we are getting the token similar above due to that we have the signature validation failed exception.

@krishnajampana
Copy link

@krishnajampana would you be able to share the wasignin message that exhibits the issue?

ds:X509Data(&#xD);
ds:X509Certificate

ds:X509Certificate

we are getting the token similar above due to that we have the signature validation failed exception.

@brentschmaltz
Copy link
Member

@krishnajampana if I understand you, in the SignedInfo element in the signed saml, shows up with a "&#xD" in the string returned from WsFederationMessage.GetToken().
Am I understanding correctly?

@smiket
Copy link
Author

smiket commented Aug 12, 2020

Hi @brentschmaltz thanks for keeping up on this one. If the signature validation works in prior versions, would it make sense to compare the older method that does work and use that code (from here):

var token = wsFederationMessage.GetToken();

Just trying to be helpful - I know there's lots of permutations to consider. I'd have a hard time sharing a SAML payload, but as I recall it was formatted with newlines etc. Still, the same SAML validated in the older code.

@brentschmaltz
Copy link
Member

brentschmaltz commented Aug 13, 2020

@smiket the difference i believe may be related to this small line of code here:

That line was removed because it caused other issues, signature failures when the whitespace was significant. The idea is that when the 'token' is obtained from reading the string, the '\n' is not translated by the xmlReader.

@krishnajampana
Copy link

@krishnajampana if I understand you, in the SignedInfo element in the signed saml, shows up with a "&#xD" in the string returned from WsFederationMessage.GetToken().
Am I understanding correctly?

yes @brentschmaltz we are getting "&#xD" from WsFederationMessage.GetToken() method. Could you please help me how to resolve it.

@northof490
Copy link

Hi @brentschmaltz thanks for keeping this open. Is there any update on this issue?

@brentschmaltz
Copy link
Member

@northof490 i would like to propose a change where the user has the ability to 'ignore' whitespace outside of elements. This allows a user to have control.

@northof490
Copy link

@brentschmaltz would you just add a flag to WsFederationOptions?

If you have anything you need tested in a pre-release just let me know.

Thanks again!

@everettcomstock
Copy link

@brentschmaltz in theory I think your proposition would work for us too.

@brentschmaltz
Copy link
Member

@everettcomstock @northof490 we will ping you when we have something to test. Most likely the week of Sept 7th.

@northof490
Copy link

@brentschmaltz any progress?

@brentschmaltz
Copy link
Member

@northof490 @everettcomstock @smiket @krishnajampana there is a workaround here:

In startup.cs replace the SamlSecurityTokenHandler with one the retries the validation if SecurityTokenInvalidSignatureException is thrown.

The same code would work Saml2 tokens.

public void ConfigureServices(IServiceCollection services)
{
    ...

    services.AddAuthentication(options =>
    {
        options.DefaultSignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
        options.DefaultAuthenticateScheme = CookieAuthenticationDefaults.AuthenticationScheme;
        options.DefaultChallengeScheme = WsFederationDefaults.AuthenticationScheme;
    })
    .AddCookie()
    .AddWsFederation(options =>
    {
        options.SecurityTokenHandlers.Clear();
        options.SecurityTokenHandlers.Add(new CustomSamlSecurityTokenHandler());
        options.SecurityTokenHandlers.Add(new Saml2SecurityTokenHandler());
        options.SecurityTokenHandlers.Add(new JwtSecurityTokenHandler());
        options.MetadataAddress = "<METADATA address here: https://login.microsoftonline.com/<YOUR tenant>/FederationMetadata/2007-06/FederationMetadata.xml";
        options.Wtrealm = "<YOUR ClientId>";
        options.Wreply = "<Reply url registered with AAD>";
    });

     ...
}

public class CustomSamlSecurityTokenHandler : SamlSecurityTokenHandler
{
    public override ClaimsPrincipal ValidateToken(string securityToken, TokenValidationParameters validationParameters, out SecurityToken validatedToken)
    {
        if (securityToken.Contains("&#xD;"))
        {
            string securityTokenTrimmed = securityToken.Replace("&#xD;", "");
            try
            {
                return base.ValidateToken(securityTokenTrimmed, validationParameters, out validatedToken);
            }
            catch (Exception ex)
            {
                if (!(ex is SecurityTokenInvalidSignatureException))
                    throw;
            }
        }

        return base.ValidateToken(securityToken, validationParameters, out validatedToken);
    }
}

@northof490
Copy link

Thank you @brentschmaltz!

@brentschmaltz brentschmaltz modified the milestones: 6.7.2, v6 Backlog Sep 29, 2020
@brentschmaltz
Copy link
Member

Since there is a workaround, i am moving this to the next milestone.

@henrik-me henrik-me moved this from In progress to To do in IdentityModel Board Oct 5, 2020
@henrik-me henrik-me added P2 Little less important, but we would like to do this and removed P1 More important, prioritize highly labels Oct 5, 2020
@henrik-me henrik-me removed this from To do in IdentityModel Board Nov 2, 2020
@jennyf19 jennyf19 removed this from the v6 Backlog milestone Sep 19, 2023
@jennyf19
Copy link
Collaborator

closing as there is a workaround. Please reopen if you disagree. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Product is not functioning as expected Customer reported Indicates issue was opened by customer Overflow Customer ask that was not fixed in previous release, has higher priority P2 Little less important, but we would like to do this WsTrust Related to WsTrust
Projects
None yet
Development

No branches or pull requests

8 participants