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

New line and whitespace normalisation in canonicalisation operation #238

Open
dventurait opened this issue Oct 8, 2021 · 3 comments
Open

Comments

@dventurait
Copy link

dventurait commented Oct 8, 2021

I have and error related to unmatching digests.
After debugging some signature validation errors on SAML responses, I would like to discuss with you a couple of questions.

The SAML response we have is like this:


<bar>\r\n
  <foo>Something Here</foo>\r\n
</bar>\r\n

In the Canonical process the \r chars are replaced by &#xD; ( see this function: https://github.com/yaronn/xml-crypto/blob/master/lib/utils.js#L66)

This code will modify the SAML response generating this output:


<bar>&#xD;\n
  <foo>Something Here</foo>&#xD;\n
</bar>&#xD;\n

The digest is calculated on this modified XML which generates a digest different from the one in the original SAML response. The consequence is the validation process fails.

I have a couple of questions:

@cjbarth
Copy link
Contributor

cjbarth commented Jul 24, 2022

Unless I'm missing something, that replacement is only for Attribute Nodes.

@cjbarth
Copy link
Contributor

cjbarth commented Jul 24, 2022

  • Otherwise what should be the right approach to follow?

Have you looked at #195 for thoughts?

@cjbarth
Copy link
Contributor

cjbarth commented May 29, 2023

We'd welcome a PR that includes some tests of what the problem is. It would then be easier to determine what the appropriate fix would be. Having said that, you might also want to look at #286 where we have several broken tests. Perhaps one of those addresses your issue and you could submit a PR to fix the test.

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

No branches or pull requests

2 participants