Azure-activedirectory-identitymodel-extensions-for-dotnet: WsFederationMessage.GetToken() is appending "
" to SAML (Formatting is breaking signature check - Sign in fails)

[smiket]

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]

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]

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:

``` // once RequestedSecurityToken element is found, it's written into a token.
// 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();

```

[brentschmaltz]

@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]

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

[everettcomstock]

@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]

@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]

@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]

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]

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

[smiket]

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]

@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]

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

[everettcomstock]

@brentschmaltz , yes.. 5.5.0 and Windows / Azure

[brentschmaltz]

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

[everettcomstock]

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

[cathalpern]

@brentschmaltz
Any ETA on this? Having the same issue.

[brentschmaltz]

@cathalpern I moved this to the current release, which was marked as today.
The team has a planning meeting on Monday, we will set a realistic release date.

[brentschmaltz]

@cathalpern we will get this into the July release.

[brentschmaltz]

this has been around for a while and should be fixed in 6.7.1.
this will become more important for WsFederation / WsTrust support in Core.

[brentschmaltz]

@everettcomstock would you be able to share a wsfed signin message that has this issue?

[brentschmaltz]

@everettcomstock no need to provide a sample, i was able to reproduce locally

[everettcomstock]

Thanks @brentschmaltz!

[brentschmaltz]

@everettcomstock @smiket here is what I determined. White space is significant between elements when obtaining the C14n (https://www.w3.org/TR/xml-c14n/) octets that the signature is generated from. If the original xml had this whitespace, then we must read the token (xml) in a manner that will ensure that when calculating the C14n octets the signature will validate.

Sometimes as the xml flows through the cloud nodes, whitespace is added. C14n removes whitespace inside elements, orders attributes, fixes namespaces, and other stuff. But does not remove the outer element whitespace.
We can't just remove such whitespace automatically as that will break other users. You can see that by making the change suggested above and run all tests.

We can however, add a switch that will allow you to ignore whitespace. The logic would force a call graph represented with this logic:

using (var ms = new MemoryStream())
{
    using (var writer = XmlDictionaryWriter.Create(
                ms, 
                new XmlWriterSettings { 
                    OmitXmlDeclaration = true, 
                    Encoding = Encoding.UTF8, 
                    NewLineHandling = NewLineHandling.None}))
    {
        writer.WriteNode(xmlReader, true);
        writer.Flush();
        ms.Seek(0, SeekOrigin.Begin);
        token = Encoding.UTF8.GetString(ms.ToArray());
    }
}

`

[smiket]

Hi @brentschmaltz,

In my case it was the additional 
 which caused the issue, which occurs inside WsFederationMessage.GetToken().
As I stepped through the debugger, I could see the original SAML payload did not have the appended escape sequence. Only after entering that function did the format change, adding the new lines.

When I did this, in the debugger, the signature check worked:
token = token.Replace("
", "");

Is it possible to avoid adding those newlines, as I point out in my 3rd comment at top of this thread?
That would not require any kind of switch etc - I imagine the additional 
 is actually a bug on its own.
I may be missing something though - just my perspective.

Thanks Brent,
Steve

[brentschmaltz]

@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?

[krishnajampana]

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]

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

[krishnajampana]

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

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

[krishnajampana]

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

(&#xD);

ds:X509Certificate

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

[brentschmaltz]

@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]

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]

@smiket the difference i believe may be related to this small line of code here: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/blob/66c26046a96ca23caf45540ca30b86dec88307be/src/Microsoft.IdentityModel.Protocols.WsFederation/WsFederationMessage.cs#L326

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]

@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]

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

[brentschmaltz]

@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]

@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]

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

[brentschmaltz]

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

[northof490]

@brentschmaltz any progress?

[brentschmaltz]

@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(" "))
{
string securityTokenTrimmed = securityToken.Replace(" ", "");
try
{
return base.ValidateToken(securityTokenTrimmed, validationParameters, out validatedToken);
}
catch (Exception ex)
{
if (!(ex is SecurityTokenInvalidSignatureException))
throw;
}
}

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

}

[northof490]

Thank you @brentschmaltz!

[brentschmaltz]

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