shouldn't the XML be canonicalized before calculating the digest/hash value?
dcu opened this issue · 8 comments
from what I see in the code it is only canonicalized when calculating the signature
What you're saying makes sense, but its been quite awhile since I've committed to or used this library. So, I can't recall all the details. Its possible that the tests and my use cases at the time didn't catch a potential issue here.
Its unlikely that I'll be able to spend much time looking into this, but I'd be happy to accept a PR that addresses your use cases/concerns.
I've been looking at this but I still don't have a solution, the transform block is supposed not canonicalize the block but apparently it isn't doing it.
I'll try to continue investigating on weekend
so after commenting out this if https://github.com/ma314smith/signedxml/blob/master/exclusivecanonicalization.go#L199 requests to SOAP servers work, at least in my case.
but that condition should be there for a reason so I'm not sure how to proceed
I'm comparing the signature procedure against:
xmlsec1 --sign --store-signatures --store-references --print-debug --privkey-pem priv.pem --id-attr:Id Body example3.xml
actually xmlsec creates an attribute with the namespace, signedxml do not creates it and if you add it to the original document it is removed
Another issue I had was the id attribute when finding a referenced section is case sensitive, must be ID
That might be an issue with your InclusiveNamespaces or it might be a bug. Its hard to say without the xml.
https://www.w3.org/TR/xml-exc-c14n/
namespace nodes that are not on the InclusiveNamespaces PrefixList are expressed only in start tags where they are visible and if they are not in effect from an output ancestor of that tag.
This was contributed awhile back which may help with your reference issue:
https://github.com/ma314smith/signedxml#using-a-custom-reference-id-attribute
I tried adding inclusive namespaces but it didn't work and xmlsec which is what everybody uses doesn't need it, it automatically adds the missing namespace
I haven't tested but my forked repo works at least for my case dcu/signedxml