samlify
samlify copied to clipboard
tngan
Signature verification - libsaml.ts throws ERR_FAILED_TO_VERIFY_SIGNATURE
Hello, i encountered a weird issue. When i'm receiving response for AuthnRequest ERR_FAILED_TO_VERIFY_SIGNATURE error is thrown. I experimented around package and found out that if i comment out a single line of code, the signature verification passes.
https://github.com/tngan/samlify/blob/06a8b9cb92221d03727c74ed1bc4a64b095d0b1d/src/libsaml.ts#L395
Also, i was looking at xml-crypto docs and in the section for signature verification didn't see that they use that approach with removing sig node.
Is this a bug or did i misconfigured something?
I will gladly provide more info if it's needed.
I am running into the same issue. Commenting out the line of code is not helping me.
@dsimic-coco @rkkatariya Which IdP are you using ? I have confirmed a test case that if the response contains CRLF character, it will cause the failed signature verification.
I am running into the same issue. Commenting out the line of code is not helping me.
@rkkatariya FYI, I referenced ts file for clarification. Try commenting out line in dist folder if you haven't.
@dsimic-coco @rkkatariya Which IdP are you using ? I have confirmed a test case that if the response contains CRLF character, it will cause the failed signature verification.
@tngan I'm using third party IdP (croatian government - NIAS service), but i don't think that info will be helpful. From my knowledge, they usualy spin their apps on win instances with .net/java stack on backend, so your deduction might prove plausible.
@tngan @dsimic-coco There was an issue with the certificate I was using. It is working now. I was using the Samlify as Idp and Sp.
@d3simic Yes, it's a known issue that the extra CRLF will cause the failed signature verification with using samlify, from the case that I have worked with someone in the community couple months ago, he eventually made a workaround for it by removing CRLF before the parsing process begins.
@rkkatariya Would you mind to share what kind of issue? It could be a hint for debugging that someone may encounter later on. :)
@d3simic Yes, it's a known issue that the extra CRLF will cause the failed signature verification with using samlify, from the case that I have worked with someone in the community couple months ago, he eventually made a workaround for it by removing CRLF before the parsing process begins.
@tngan Since there are no \r\n in decoded string, I think something else might be the issue. What would you need to try to debug? Also, since you didn't mention it, what is the purpose of the line i referenced. If there is no use or if it's some kind of deprecated flow, maybe remove it?
Any new ideas for that. I have also same issue: ERR_FAILED_TO_VERIFY_SIGNATURE at /Users/esa/devel/systaylor/iris3/server/node_modules/samlify/src/libsaml.ts:401:17
Is there some workaround for this ??
hi @netmiller , have you sorted out the issue? i'm having same issue after updating my nodeJS version from 10 to 12
No, I have not. But this issue is going to be some-kind-of-blocker for my project very soon. I have to solve this some way. Any ideas are very appreciative !
@tngan can you please guide about extra CRLF in which file? because i am stuck on this thing as well
@tngan Any news, ideas or plans to resolve this? If not, is it ok/safe to use forked library build with mentioned line removed?
Debugging this error while onboarding a new IDP led me to discover they weren't signing their responses with an exclusive canonicalization method.
When it's signed in a non-exclusive canonical form, the parent document of an XML node makes a big difference to the signature. Calling removeChild(signatureNode) manually strips away those effects, which causes a mismatch with the original signature. An exclusive canonical form would strip them away from both sides of the equation.
I'm not totally convinced this is a samlify bug since the SAML 2.0 spec admits implementations should use exclusive canonicalization with or without comments, but for what it's worth, removing the line in question doesn't seem to break validation for either method.
