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

"invalid signature" when using xml-crypto > 1.4.0 #431

Closed
mightypenguin opened this issue Apr 15, 2020 · 4 comments · Fixed by #498
Closed

"invalid signature" when using xml-crypto > 1.4.0 #431

mightypenguin opened this issue Apr 15, 2020 · 4 comments · Fixed by #498

Comments

@mightypenguin
Copy link

Not sure why, but xml-crypto > 1.4.0 gives me an invalid signature.
Fix is to force xml-crypto to v1.4.0

But perhaps there's something programmatic that needs to change in how passport-saml is using it?

@mightypenguin
Copy link
Author

Possibly related bug report on that package.

node-saml/xml-crypto#200

@thenengah
Copy link

thenengah commented May 15, 2020

@mightypenguin

We also ran into this. I submitted a fix here:
node-saml/xml-crypto#211

But the xml-crypto team thinks this update doesn't belong in xml-crypto.

@bergie what are your thoughts about normalizing line-breaks before checkSignature. Something like this should work:

// windows line breaks botch sha digests algos
fullXml = fullXml.string.replace(/\r\n?/g, '\n');
sig.checkSignature(fullXml);

@markstos
Copy link
Contributor

@thenengah Please create a PR for this which includes a test case that illustrates the failure. You should hopefully be able to find a matching test, copy/paste and add some Carriage Returns to it.

I looked up the XML spec for newlines. It is here:

https://www.w3.org/TR/REC-xml/#sec-line-ends

It says that the XML processor should normalize the newlines as you describe before parsing.

I look up related references in xml-crypto and it sounds like xml-crypto expects us to be the "XML Processor" while it serves as the "parser".

So, it does seem to be correct according to the spec for us to normalize the newlines before handing off XML to xml-crypto.

@mhassan1
Copy link
Contributor

mhassan1 commented Nov 5, 2020

@markstos Here is a PR that normalizes line breaks as part of signature validation: #498. Let me know how I can help push this through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants