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

Handle when IDP does not use namespace prefix on CanonicalizationMethod #36

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

james-async
Copy link

The "Novell Access Manager" does not put a namespace prefix on the CanonicalizationMethod element. (or at least the installed version we are working with does not). We tried to write a test. This is a minimalist change rather than changing childPath.

In fact the official documentation shows that the namespace prefix is not applied to CanonicalizationMethod.
https://www.netiq.com/documentation/access-manager-43/admin/data/b65ogn0.html
<ds:SignedInfo> <CanonicalizationMethod xmlns="http://www.w3.org/2000/09/xmldsig#" Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/> <ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/> <ds:Reference URI="#idF5JceWGWYwS3bOkmJS2wJuNqitU"> <ds:Transforms> <ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/> <ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/> </ds:Transforms> <ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/> <DigestValue xmlns="http://www.w3.org/2000/09/xmldsig#">ZocFiEUYcda0cKGRNcZYZqvmnlM=</DigestValue> </ds:Reference> </ds:SignedInfo>

@russellhaering
Copy link
Owner

Good find, you uncovered some old code that has an actual bug in it. That CanonicalizationMethod has a namespace declared and we should have been respecting it.

Can you test this branch and confirm that it fixes the issue for you?

https://github.com/russellhaering/goxmldsig/tree/russell_h/improved_namespace_handling_2

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

Successfully merging this pull request may close these issues.

2 participants