xmlsec / python-xmlsec

Python bindings for the XML Security Library.
MIT License
95 stars 99 forks source link

test_sign_case5 fails with xmlsec 1.2.36 #244

Closed danigm closed 8 months ago

danigm commented 1 year ago
self = <tests.test_ds.TestSignContext testMethod=test_sign_case5>

    def test_sign_case5(self):
        """Should sign a file using a dynamicaly created template, key from PEM file and an X509 certificate."""
        root = self.load_xml("sign5-in.xml")
        sign = xmlsec.template.create(root, consts.TransformExclC14N, consts.TransformRsaSha1)
        self.assertIsNotNone(sign)
        root.append(sign)
        ref = xmlsec.template.add_reference(sign, consts.TransformSha1)
        xmlsec.template.add_transform(ref, consts.TransformEnveloped)

        ki = xmlsec.template.ensure_key_info(sign)
        x509 = xmlsec.template.add_x509_data(ki)
        xmlsec.template.x509_data_add_subject_name(x509)
        xmlsec.template.x509_data_add_certificate(x509)
        xmlsec.template.x509_data_add_ski(x509)
        x509_issuer_serial = xmlsec.template.x509_data_add_issuer_serial(x509)
        xmlsec.template.x509_issuer_serial_add_issuer_name(x509_issuer_serial, 'Test Issuer')
        xmlsec.template.x509_issuer_serial_add_serial_number(x509_issuer_serial, '1')

        ctx = xmlsec.SignatureContext()
        ctx.key = xmlsec.Key.from_file(self.path("rsakey.pem"), format=consts.KeyDataFormatPem)
        self.assertIsNotNone(ctx.key)
        ctx.key.load_cert_from_file(self.path('rsacert.pem'), consts.KeyDataFormatPem)
        ctx.key.name = 'rsakey.pem'
        self.assertEqual("rsakey.pem", ctx.key.name)

        ctx.sign(sign)
>       self.assertEqual(self.load_xml("sign5-out.xml"), root)

tests/test_ds.py:186: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
tests/base.py:128: in assertXmlEqual
    self.assertXmlEqual(c1, c2)
tests/base.py:128: in assertXmlEqual
    self.assertXmlEqual(c1, c2)
tests/base.py:128: in assertXmlEqual
    self.assertXmlEqual(c1, c2)
tests/base.py:128: in assertXmlEqual
    self.assertXmlEqual(c1, c2)
tests/base.py:118: in assertXmlEqual
    self.fail('text: {!r} != {!r}. {}'.format(first.text, second.text, msg))
E   AssertionError: text: 'JIQs8tRZIGKLLlyGkKOqMLonGpw=' != None.
kapouer commented 1 year ago

More precisely, xmlsec.template.x509_data_add_ski(x509) does not fill <X509SKI>:

<Envelope xmlns="urn:envelope">
  <Data>
    Hello, World!
  </Data>
<Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
<SignedInfo>
<CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<Reference>
<Transforms>
<Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/>
</Transforms>
<DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/>
<DigestValue>HjY8ilZAIEM2tBbPn5mYO1ieIX4=</DigestValue>
</Reference>
</SignedInfo>
<SignatureValue>SIaj/6KY3C1SmDXU2++Gm31U1xTadFp04WhBgfsJFbxrL+q7GKSKN9kfQ+UpN9+i
D5fWmuavXEHe4Gw6RMaMEkq2URQo7F68+d5J/ajq8/l4n+xE6/reGScVwT6L4dEP
XXVJcAi2ZnQ3O7GTNvNGCPibL9mUcyCWBFZ92Uemtc/vJFCQ7ZyKMdMfACgxOwyN
T/9971oog241/2doudhonc0I/3mgPYWkZdX6yvr62mEjnG+oUZkhWYJ4ewZJ4hM4
JjbFqZO+OEzDRSbw3DkmuBA/mtlx+3t13SESfEub5hqoMdVmtth/eTb64dsPdl9r
3k1ACVX9f8aHfQQdJOmLFQ==</SignatureValue>
<KeyInfo>
<X509Data>
<X509IssuerSerial>
<X509IssuerName>Test Issuer</X509IssuerName>
<X509SerialNumber>1</X509SerialNumber>
</X509IssuerSerial>
<X509SubjectName/>
<X509Certificate>MIIE3zCCBEigAwIBAgIBBTANBgkqhkiG9w0BAQQFADCByzELMAkGA1UEBhMCVVMx
EzARBgNVBAgTCkNhbGlmb3JuaWExEjAQBgNVBAcTCVN1bm55dmFsZTE9MDsGA1UE
ChM0WE1MIFNlY3VyaXR5IExpYnJhcnkgKGh0dHA6Ly93d3cuYWxla3NleS5jb20v
eG1sc2VjKTEZMBcGA1UECxMQUm9vdCBDZXJ0aWZpY2F0ZTEWMBQGA1UEAxMNQWxl
a3NleSBTYW5pbjEhMB8GCSqGSIb3DQEJARYSeG1sc2VjQGFsZWtzZXkuY29tMB4X
DTAzMDMzMTA0MDIyMloXDTEzMDMyODA0MDIyMlowgb8xCzAJBgNVBAYTAlVTMRMw
EQYDVQQIEwpDYWxpZm9ybmlhMT0wOwYDVQQKEzRYTUwgU2VjdXJpdHkgTGlicmFy
eSAoaHR0cDovL3d3dy5hbGVrc2V5LmNvbS94bWxzZWMpMSEwHwYDVQQLExhFeGFt
cGxlcyBSU0EgQ2VydGlmaWNhdGUxFjAUBgNVBAMTDUFsZWtzZXkgU2FuaW4xITAf
BgkqhkiG9w0BCQEWEnhtbHNlY0BhbGVrc2V5LmNvbTCCASIwDQYJKoZIhvcNAQEB
BQADggEPADCCAQoCggEBAJe4/rQ/gzV4FokE7CthjL/EXwCBSkXm2c3p4jyXO0Wt
quaNC3dxBwFPfPl94hmq3ZFZ9PHPPbp4RpYRnLZbRjlzVSOq954AXOXpSew7nD+E
mTqQrd9+ZIbGJnLOMQh5fhMVuOW/1lYCjWAhTCcYZPv7VXD2M70vVXDVXn6ZrqTg
qkVHE6gw1aCKncwg7OSOUclUxX8+Zi10v6N6+PPslFc5tKwAdWJhVLTQ4FKG+F53
7FBDnNK6p4xiWryy/vPMYn4jYGvHUUk3eH4lFTCr+rSuJY8i/KNIf/IKim7g/o3w
Ae3GM8xrof2mgO8GjK/2QDqOQhQgYRIf4/wFsQXVZcMCAwEAAaOCAVcwggFTMAkG
A1UdEwQCMAAwLAYJYIZIAYb4QgENBB8WHU9wZW5TU0wgR2VuZXJhdGVkIENlcnRp
ZmljYXRlMB0GA1UdDgQWBBQkhCzy1FkgYosuXIaQo6owuicanDCB+AYDVR0jBIHw
MIHtgBS0ue+a5pcOaGUemM76VQ2JBttMfKGB0aSBzjCByzELMAkGA1UEBhMCVVMx
EzARBgNVBAgTCkNhbGlmb3JuaWExEjAQBgNVBAcTCVN1bm55dmFsZTE9MDsGA1UE
ChM0WE1MIFNlY3VyaXR5IExpYnJhcnkgKGh0dHA6Ly93d3cuYWxla3NleS5jb20v
eG1sc2VjKTEZMBcGA1UECxMQUm9vdCBDZXJ0aWZpY2F0ZTEWMBQGA1UEAxMNQWxl
a3NleSBTYW5pbjEhMB8GCSqGSIb3DQEJARYSeG1sc2VjQGFsZWtzZXkuY29tggEA
MA0GCSqGSIb3DQEBBAUAA4GBALU/mzIxSv8vhDuomxFcplzwdlLZbvSQrfoNkMGY
1UoS3YJrN+jZLWKSyWE3mIaPpElqXiXQGGkwD5iPQ1iJMbI7BeLvx6ZxX/f+c8Wn
ss0uc1NxfahMaBoyG15IL4+beqO182fosaKJTrJNG3mc//ANGU9OsQM9mfBEt4oL
NJ2D
</X509Certificate>
<X509SKI/>
</X509Data>
</KeyInfo>
</Signature></Envelope>
kapouer commented 1 year ago

Compiled with PYXMLSEC_ENABLE_DEBUG=1 and there is something odd logged right on the test:

tests/test_ds.py ......FE[./src/ds.c:61 PyXmlSec_SignatureContext__del__] 0x7fe9f9fca6d0: delete sign context   [ 58%]
tests/test_enc.py ........................                                                                                                             [ 67%]

Full test log: test.log

kapouer commented 1 year ago

Could it be related to openssl 3.0 ?

kapouer commented 1 year ago

Update: this test is expected to fail in debian bookworm, since libxmlsec1 in debian bookworm is also expecting this test to fail.

nbastin commented 1 year ago

I ran into an issue with a signed document of this format, and it turns out the problem (at least in our case, and it might also be here), is that there is a spurious newline in the document at the end of the X509Certificate tag data (the close tag should be on the same line as the end of the data, not on the next line). I have no idea what is triggering this change in generation, but if you remove the newline the document is properly handled.

mgorny commented 1 year ago

Could it be related to openssl 3.0 ?

I don't think so.

I've verified that the test passes if the extension is built against libxmlsec 1.2.34 or older, and fails with 1.2.36 and .37. I haven't been able to test 1.2.35 since it fails to build.

tdivis commented 1 year ago

I diffed the expected and actulat xml in the test and difference is, that <X509SubjectName> and <X509SKI> are empty for xmlsec>=1.2.36, and when I looked to the git diff xmlsec-1_2_33..xmlsec-1_2_36 i found them newly marked as "DEPRECATED", so it's probably intended change.

I will try to make change of the test expected XML depending on version of libxmlsec1.