Problem with both X509Data and KeyValue present

[arthurdejong]

Hi,

Thanks for providing and maintaining singxml! I am using signxml in the python-pskc module and since release 2.7.2 of signxml my tests fail.

The code that triggers it basically consists of generating a signature without adding a certificate (so only a key, which results in key information being saved in the signature) and then using a (self signed) certificate to do the verification:

>>> import signxml
>>> from lxml import etree
>>> 
>>> signing_key = open('tests/certificate/key.pem', 'rb').read()
>>> self_signed_certificate = open('tests/certificate/ss-certificate.pem', 'rb').read()
>>> tree = etree.Element('root')
>>> 
>>> signer = signxml.XMLSigner(
...     method=signxml.methods.enveloped,
...     signature_algorithm='rsa-sha256',
...     digest_algorithm='sha256',
...     c14n_algorithm=signxml.XMLSignatureProcessor.default_c14n_algorithm)
>>> verifier = signxml.XMLVerifier()
>>> 
>>> signed_tree = signer.sign(tree, key=signing_key)
>>> print(etree.tostring(signed_tree, pretty_print=True))
<root>
  <ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
    <ds:SignedInfo>
      <ds:CanonicalizationMethod Algorithm="http://www.w3.org/2006/12/xml-c14n11"/>
      <ds:SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/>
      <ds:Reference URI="">
        <ds:Transforms>
          <ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
          <ds:Transform Algorithm="http://www.w3.org/2006/12/xml-c14n11"/>
        </ds:Transforms>
        <ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/>
        <ds:DigestValue>tx5NFydGNrlxebotl8dCc1tlEOtU8iiT06La/yzrKNs=</ds:DigestValue>
      </ds:Reference>
    </ds:SignedInfo>
    <ds:SignatureValue>sEz0pcHTomxGiC+1TdoieFtAVrmzke8qhqVYlottp37hEsYGhij1o/3QiH5CN+KZIyItAZ+LJIVPEI1hFPSe+X1nRNS+D5WpT3NMUXs5utdf+RXh7+WU6/+wdziHiAnvocNDfdHwbyAwFeoUjvjYPyp8rYlvrKBjaIM4IFnMRwIrH9xs4ae/MyJg/ZjcWoBxUkWUnCiEgDp+VWlQY+JUFWK6CqWG9Xoe1ATIBqA+HUhU49WWxUvv/Z+gpIKKurV9tv9UoLkbw3Iuyzh4otRuGee/xNJX6unlCVeHnR3kiAGwHAFUDyMDagt8zpjt6XAwHT2DqFU76I9DGEavUN4I7g==</ds:SignatureValue>
    <ds:KeyInfo>
      <ds:KeyValue>
        <ds:RSAKeyValue>
          <ds:Modulus>5tndgKHUJsULyv6Uq/+iwHY+FNkTN7FIHdD+n9pQHLOfjk69rPuklpDBJ6ZuMc2pnxa+IWa04uvO04eNOJNevlQ4A9hXZYWtYnOBEx9VmAJANNNRy13o2fyXNbWGG/5hg/S9X5u05Yk3Ptpi+3n0zMkRcyBky5oQu733J6EwUczQRoOLstM9Fyviqw3J2/Eta21y0Fms7EABIvRQAdwK2JLxPIGIA7Ouult34ORETm9zoRh3CpqhAjaD2sOGXhJR06mxE6emAnWtzRoLLYqPGHNezgrjYP8S+DUsaLCVmYgUP2gw7Fz9cwZhdsbYWjwKNYJQUwAp2n9Ct+6oERNV0Q==</ds:Modulus>
          <ds:Exponent>AQAB</ds:Exponent>
        </ds:RSAKeyValue>
      </ds:KeyValue>
    </ds:KeyInfo>
  </ds:Signature>
</root>
>>> verified_tree = verifier.verify(signed_tree, x509_cert=self_signed_certificate)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/arthur/devel/python-pskc/.tox/py27-signxml/local/lib/python2.7/site-packages/signxml/__init__.py", line 769, in verify
    raise InvalidInput("Both X509Data and KeyValue found. Use verify(ignore_ambiguous_key_info=True) "
signxml.exceptions.InvalidInput: Both X509Data and KeyValue found. Use verify(ignore_ambiguous_key_info=True) to ignore KeyValue and validate using X509Data only.

Is my use case wrong, am I passing the incorrect options (and do I need to fix my code) or is this a bug in signxml?

I must say that X509 certificates always confuse me somewhat.

(the key and certificate used above are available in the python-pskc test suite)

Thanks!

[nimish]

As far as I understand it you are providing the same info twice: the x509_cert will have the public key embedded in it, and the RsaKeyValue element also includes that public key. If you want to override the keyinfo element, see the error message. Otherwise, you should first validate trust in the RSA key stored in the KeyInfo element, then ensure that that the signature is valid by verifying it.

[kislyuk]

Sorry about the delay in responding to this.

The reason you're getting the error is that you're signing using just the raw key, but then verifying using a certificate. The raw key signing method implies that you will verify the internal consistency of the signature and then verify trust yourself (extract and match the public key against trusted keys using some other method; actually, there is a gap in the API here - signxml provides no recommended canonical/safe way to extract the key that was used for verification).

To avoid the error, pass the cert parameter to sign() in addition to the key, or pass the ignore_ambiguous_key_info parameter as recommended in the error message.

[kislyuk]

I should say I added this safety check to try to avoid situations where a user could become confused about what is trusted in the process of verification (in your case, since you're passing x509_cert yourself, that is trusted, and the ds:KeyValue information is NOT trusted or matched against x509_cert). I am open to well-reasoned suggestions on the more intuitive thing to do here. One possible solution is to actually match the KeyValue against the public key in the cert and raise an error only if they don't match.

[nimish]

@kislyuk The java solution is to have a truststore that handles validation: the key must match a certificate that's in the trust chain -- either it's in the trust store directly, or you trust the chain to the certificate that matches the key.

This allows for trusting root certs only vs attempting to catalog the entire issued key set.

[kislyuk]

@nimish yes, that is supported by signxml out of the box by default. The issue described here arises only when signing with the raw key pair instead of X509.

[nimish]

That's how the java store works: if it only finds the RSA public key it will attempt to match it with an entry (certificate or private key) in the truststore before validating that entry against the trust chain.

[arthurdejong]

@kislyuk Thanks for your response.

To avoid the error, pass the cert parameter to sign() in addition to the key, or pass the ignore_ambiguous_key_info parameter as recommended in the error message.

I can't pass the cert parameter because I would like to have a test case for when someone provides a signed PSKC file without an embedded certificate.

I also can't pass ignore_ambiguous_key_info to verify() because that would mean my library would depend on the latest signxml which may be different from the version of signxml that the user has available.

I am open to well-reasoned suggestions on the more intuitive thing to do here. One possible solution is to actually match the KeyValue against the public key in the cert and raise an error only if they don't match.

I always have issues with certificate and key trust paths but the above does sound like a reasonable approach. For this kind of API it is probably a good idea to make it as convenient as possible for the caller to do the right thing.

I'm not 100% sure my use case is a reasonable one but it doesn't sound that unreasonable to sign an XML file without embedded a certificate. From my reading of the XML signature specs all elements within KeyInfo are optional so it least it sounds possible (it should probably also be possible with signxml to generate a signature without embedded the public key and validate with a certificate).

[kislyuk]

Totally agree with the entirety of your comment, and would welcome any help implementing the consistency check between X509 cert data and KeyInfo. It may take me a few weeks to get to it myself.

From my reading of the XML signature specs all elements within KeyInfo are optional so it least it sounds possible

The fundamental, serious problem with the XML Signature spec is that it gives everyone far more than enough rope to hang themselves with. A lot of things sound possible, but it's not at all clear that they should be and what hazards they may cause. The spec is huge and leaves way too many details to the implementer as optional or underdefined. This is why working with XML Signature is hazardous; you need several pages of warnings and a thoughtfully designed API before you can start using it safely.

[lscorcia]

Hi, I discovered this issue when troubleshooting a regression in the testsuite for the Italian nationwide SAML authentication provider. One of the compliance check tools uses this library in order to validate XML signatures, and it recently got upgraded and started exhibiting this error message.

While I agree on the principle that XML validation should be unambiguous, there really should be no error if the two representations of the public key are the same. I just wanted to cast my vote for the more accurate check. Meanwhile, I'll try to see if the tool maintainers can activate the ignore_ambiguous_key_info option.

[kislyuk]

@lscorcia thanks for your comment. I completely agree that the matching of the two representations is a missing feature, and that there should be no error if the keys match. Unfortunately, this library is not sponsored by anybody. I maintain it in my spare time, and my other obligations have taken priority recently. Contributions in the form of complete single-feature review-friendly PRs are always welcome, and if my work is needed, the sponsor button is at the top of the page.

[lscorcia]

Hello @kislyuk ! I completely understand your point, I'm sorry if I sounded demanding, it wasn't my intention at all.
I have submitted a PR to try and implement the feature, but please keep in mind that it is my first python code ever and it will probably be ugly, wrong or both. If you or anyone else has the time to review it, maybe it can be used as a starting point.
Thanks for all of your time and effort!

[kislyuk]

Thank you for stepping up to do the work @lscorcia! Your PR addresses the issue and I will be moving forward with the testing and preparing this work for release. I appreciate your contributions to SignXML.