jitbit / AspNetSaml

Very simple SAML 2.0 consumer module for ASP.NET/C#
https://www.jitbit.com
Apache License 2.0
361 stars 118 forks source link

NullReferenceException on checksignature #49

Closed apexdodge closed 3 years ago

apexdodge commented 3 years ago

Hello,

I am getting a NullReferenceException on this line of code.

image

I am using a valid certificate and valid SAMLResponse payload. I narrowed it down to the CheckSignature function. It fails whether I use CheckSignature(_certificate, true) or just CheckSignature() by itself.

I confirmed validity here: https://www.samltool.com/validate_response.php so I'm at a loss as to what might be happening at this point.

Tried on both .net core 3.1 and .net 5. Failed on both, if that matters.

Is there a known issue or bug? Any ideas?

Thanks

alex-jitbit commented 3 years ago

So, signedXml is null in debug?

PS. The onelogin's samltool.com you mentioned does not verify signatures at all, so checking the response their - does not actually confirm validity

apexdodge commented 3 years ago

@alex-jitbit signedXml is not null:

image

ValidateSignatureReference() returns true and !isExpired == true, so we are good on those.

Noted on samltool.com. I tested it on https://8gwifi.org/samlverifysign.jsp - not sure if that site is better.

When I select these two options: https://s3.amazonaws.com/vo-random/ShareX/2020/11/chrome_RGe1ylqkfL.png - it passes.

image

alex-jitbit commented 3 years ago

Then probably the certificate is null. I haven't tested this on .NET Core a lot, will look into this

apexdodge commented 3 years ago

@alex-jitbit I got it working by removing all references to these:

RSAPKCS1SHA256SignatureDescription.Init()

There are two places in the code where that is called, and I commented them out and it works now.

image

I suspect that code conflicts with .net core / .net 5.

I hope that helps you and anyone else running into the problem.

admalledd commented 3 years ago

Note, the root cause is that the types registered at the ctor() RSAPKCS1SHA256SignatureDescription() are using "typeof(...).FullNameand deep in the signature verification on MS's side they doType.GetType(string)`. As this stack overflow hints, using the assembly qualified name instead works. Thus changing the ctor() to:

        public RSAPKCS1SHA256SignatureDescription()
        {
            //https://stackoverflow.com/a/1825156/494125
            KeyAlgorithm = typeof(RSACryptoServiceProvider).AssemblyQualifiedName;
            DigestAlgorithm = typeof(SHA256Managed).AssemblyQualifiedName;   // Note - SHA256CryptoServiceProvider is not registered with CryptoConfig
            FormatterAlgorithm = typeof(RSAPKCS1SignatureFormatter).AssemblyQualifiedName;
            DeformatterAlgorithm = typeof(RSAPKCS1SignatureDeformatter).AssemblyQualifiedName;
        }
alex-jitbit commented 3 years ago

We've actually removed this code altogether, needed for pre-4.5 .NET versions anyway. Currently testing in production, will push updates here next week.

ppmBeebie commented 3 years ago

@alex-jitbit I'm waiting for your update

ppmBeebie commented 3 years ago

@ @admalledd Thank you

RayWillett commented 2 years ago

see #64. Looks like the referenced updates are available from source on github, but have not been published to nuget yet. It's fairly simple to swap out the nuget reference for a reference to the source code directly in your project.

shotttik commented 8 months ago

Thank you, that worked