Verification with InclusiveNamespaces

[majvan]

Hi guys,

[Issue - short]
an xml document having this ds:SignedInfo
<ds:SignedInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#" xmlns:soapenv="http://www.w3.org/2003/05/soap-envelope"><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"><ec:InclusiveNamespaces PrefixList="soapenv" xmlns:ec="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:CanonicalizationMethod><ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/><ds:Reference URI="#id-D4754E6D65BB527E86154893374299759"><ds:Transforms></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><ds:DigestValue>/+OXEO9svqFKP48QWZ6dBgswc7Mk6EIlLGPOljjPpAE=</ds:DigestValue></ds:Reference></ds:SignedInfo>
is not correctly canonized (c14n-exc) => hence the signature digest is not correctly computed => hence it fails with verification.

[Reasoning]
The document with above SignedInfo was given to me as a verified document (the signature verification should pass).
After I digged into issue more, I used this tool:
https://www.cryptosys.net/sc14n/

So I ran 2 commands with 2 different results:

1. Excluding the InclusiveNamespaces (not correct): sc14n -e -s "ds:SignedInfo" -i signed_info.xml -o signed_info.bin
   This computes the same c14n-exc output as signxml;
2. Including the InclusiveNamespaces (correct): sc14n -e --prefix-list "soapenv" -s "ds:SignedInfo" -i signed_info.xml -o signed_info.bin
   This computes another c14n-exc output, but the output shall be correct and also the SignatureValue of such output would lead into correctly computed SignatureValue.

[Findings]
Please correct me if I am wrong, but if the ds:SignedInfo contains <ec:InclusiveNamespaces ...>, these must be included into the canonization.
So far I see this is not taking into account when verifying the document.
In fact, what is verified:
signed_info_c14n = self._c14n(signed_info, algorithm=c14n_algorithm)
where the _c14n function:
def c14n(cls, nodes, algorithm, inclusive_ns_prefixes=None): ... c14n = b"" for node in nodes: c14n += etree.tostring(node, method="c14n", exclusive=exclusive, with_comments=with_comments, inclusive_ns_prefixes=inclusive_ns_prefixes)
can take a parameter for inclusive prefixes, but none is given, because the inclusive prefixes are not searched for- for verification.

[kislyuk]

After a brief look, it seems you are correct - the verify method never retrieves/passes inclusive namespaces. I'll take a look at fixing this shortly, but PRs are welcome.

[kislyuk]

Upon further examination, SignXML correctly retrieves and applies InclusiveNamespaces PrefixList, and your analysis is incorrect. There is something else wrong with your example.

[kislyuk]

If you require further assistance, provide a complete reproduction with the full document, expected output, and output produced by SignXML.

[majvan]

Closing too fast :) Reopening.
In the attached you find:

test.py - python script to reproduce the issue
signed_request.xml - an xml which should pass the verification
trusted.pem - a trusted CA certificate
Expected output of the script:
Passed, ok.
Output of the script:
Bug in the signxml, the doc did not pass
Output produced by SignXML:
Signature verification failed: bad signature

test.zip

[kislyuk]

Thank you for providing a full reproduction.

Using the information you gave, I was able to diagnose the issue. The SOAP protocol, or at least its implementations, violate the XML Signature standard by placing the InclusiveNamespaces tag in SignedInfo. The XML Signature schema does not permit it there, which is why you had to supply the validate_schema=False keyword argument.

Nevertheless, it looks like this tag is used in practice, and is required to pass validation as you noted. I can't see any security issues with supporting this tag in this context, and it does not seem to disrupt any other functionality, so I went ahead and added support for it in the verify() method, and added your case to the test suite.