EnvelopedSignatureReader can only process one <Signature>

cveld commented 5 years ago

Non Security Issues

Information needed

What nuget packages are you using 2.2.0
What is the expected behaviour

Our saml2p:Response payload contains one main saml2:Assertion with a saml2:Advice in it. This saml2:Advice on its turn also contains one saml2:Assertion. The payload contains three signatures on the following levels:

saml2p:Response
The main saml2:Assertion
And the saml2:Assertion within the saml2:Advice. We expect that the library just reads the payload without errors.

What happens instead.

The library throws the following exception:

IDX13102: Exception thrown while reading 'NameIDType' for Saml2SecurityToken. Inner exception: 'Microsoft.IdentityModel.Xml.XmlReadException: IDX30019: Unable to read XML. A second <Signature> element was found. The EnvelopedSignatureReader can only process one <Signature>.

Additional info

Please include

.Net Framework your application is compiled against (e.g. net472, netcoreap2.1) net471
.Net Framework installed. This might be different than above. You can compile with net452, but have 4.7.2 installed. net472
Version of Asp.Net MVC / Asp.NET Core used. We tested in a net471 console app.

cveld commented 5 years ago

I believe this issue is related: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/issues/1012

AndersAbel commented 5 years ago

Yes, that issue is definitely related. I also created a PR which would fix it: https://github.com/AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet/pull/1041 Unfortunately it's been stuck in discussions. One thing I've commented there is that a SAML assertion could contain nested signed data (such as an assertion) in any of the extension points. And obviously it is used in the wild too.

I'll bring this issue up as an example of why disallowing more signatures is a bad idea.

AndersAbel commented 5 years ago

And as a workaround, the 1.0.0 version does not have this issue - if targeting the full .NET Framework is okay for you.

AndersAbel commented 4 years ago

The AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#1012 is fixed, so I'm closing this.

Sustainsys / Saml2