Support verification of multiple signatures

[kislyuk]

• Use breadth-first traversal when looking for signatures
• Silently discard signatures that don't use the expected certificate
• If multiple signatures remain and current interface is used, raise an error
• Introduce iterator interface for situations where multiple signatures are expected

[dflorijn]

Hi Andrey Kislyuk,

I would like to propose an alternative method to solve this problem:
Perhaps an easier method would be to include an x_path parameter in the verify function that resolves any conflicts if multiple Signatures are present in the document. The user can use the x_path parameter to select the correct Signature element that belongs to the certificate. This might also be a more efficient way compared to checking every Signature element.

This method is also used in the XML Security Library of Aleksey e.g.
xmlsec1 --verify --pub-certkey cert.cer --node-xpath <XPATH EXPRESSION> file.xml. This allows me to succesfully verify documents with two signatures, where the first result of the breadth-first traversal does not yield the desired Signature element.

I'm willing to commit to this open issue considering this is a function I desire in my implementation (using the signxml library).

I look forward to hearing from you soon.

Kind regards,

Diederik Florijn

[kislyuk]

Hi @dflorijn, I'm fine with this approach, you're welcome to submit a PR.

[dflorijn]

Hi Andrey @kislyuk ,

I've been quite busy at work so I wasn't able to make much progression lately. Nonetheless, it seems my addition of the xpath expression in the verify function to distinguished between signature elements works (or at least on the XML files I need to verify that contain multiple signatures).

What are the next steps before I can make a PR? I suppose make some additions to the test file so to ensure it works properly? I have ran the testfile but I receive 1 error on an expired certificate:

  File "test.py", line 233, in test_xmldsig_interop
    ca_pem_file=get_ca_pem_file(signature_file))
  File "/Users/Diederik/git/signxml/signxml/__init__.py", line 732, in verify
    signing_cert = verify_x509_cert_chain(cert_chain, ca_pem_file=ca_pem_file, ca_path=ca_path)
  File "/Users/Diederik/git/signxml/signxml/util/__init__.py", line 234, in verify_x509_cert_chain
    raise last_error
  File "/Users/Diederik/git/signxml/signxml/util/__init__.py", line 223, in verify_x509_cert_chain
    end_of_chain = _add_cert_to_store(store, cert)
  File "/Users/Diederik/git/signxml/signxml/util/__init__.py", line 193, in _add_cert_to_store
    raise InvalidCertificate(e)
signxml.exceptions.InvalidCertificate: [10, 0, 'certificate has expired']

Kind regards,

Diederik

[kislyuk]

I just committed a fix for the test failure, please try again.

You are correct that you will need to add a test case and change the docstring to document your new functionality.

[dflorijn]

Hi Andrey @kislyuk ,

Thank you for your quick response and action! I ran the test yesterday and it was successful with the alteration I made. The last step is adding cases to the unit tests. Do you have a suggestion on what testcases I should create?

My idea is to create some sample xml files containing multiple signatures and check these with the xpath parameter. However, I can also copy and change several existing testcases, and simply add the xnode parameter. This ensures that the xpath works, however it excludes documents with multiple signatures.

Looking forward to hearing from you.

Kind regards,

Diederik

[kislyuk]

Yes, you should add new test files that contain multiple signatures, and test selecting each of the signatures and any error conditions you can think of (xpath not resolving to anything, etc.)