Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_sign_case5 fails with xmlsec 1.2.36 #244

Closed
danigm opened this issue Nov 25, 2022 · 7 comments
Closed

test_sign_case5 fails with xmlsec 1.2.36 #244

danigm opened this issue Nov 25, 2022 · 7 comments

Comments

@danigm
Copy link

danigm commented Nov 25, 2022

  • xmlsec: 1.2.36
  • libxmlsec1: 1.2.36
  • libxml2: 2.10.3
  • pkg-config --cflags xmlsec1: -DXMLSEC_CRYPTO_DYNAMIC_LOADING=1 -D__XMLSEC_FUNCTION__=__func__ -DXMLSEC_NO_SIZE_T -DXMLSEC_NO_MD5=1 -DXMLSEC_NO_GOST=1 -DXMLSEC_NO_GOST2012=1 -DXMLSEC_DL_LIBLTDL=1 -I/usr/include/xmlsec1 -I/usr/include/libxml2
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
Copy link

kapouer commented Jan 4, 2023

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
Copy link

kapouer commented Jan 4, 2023

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
Copy link

kapouer commented Jan 5, 2023

Could it be related to openssl 3.0 ?

@kapouer
Copy link

kapouer commented Jan 6, 2023

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

@nbastin
Copy link

nbastin commented Feb 12, 2023

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
Copy link

mgorny commented Feb 17, 2023

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
Copy link
Contributor

tdivis commented Mar 21, 2023

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.

tdivis added a commit to tdivis/python-xmlsec that referenced this issue Mar 22, 2023
tdivis added a commit to tdivis/python-xmlsec that referenced this issue Mar 22, 2023
tdivis added a commit to tdivis/python-xmlsec that referenced this issue Mar 23, 2023
tdivis added a commit to tdivis/python-xmlsec that referenced this issue Mar 27, 2023
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Apr 6, 2023
Disable another broken test: xmlsec/python-xmlsec#244
Switch to PEP517.

git-svn-id: file:///srv/repos/svn-community/svn@1440326 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this issue Apr 6, 2023
Disable another broken test: xmlsec/python-xmlsec#244
Switch to PEP517.

git-svn-id: file:///srv/repos/svn-community/svn@1440326 9fca08f4-af9d-4005-b8df-a31f2cc04f65
tdivis added a commit to tdivis/python-xmlsec that referenced this issue Apr 17, 2023
brennanneoh pushed a commit to brennanneoh/python-xmlsec that referenced this issue Feb 16, 2024
brennanneoh added a commit to brennanneoh/python-xmlsec that referenced this issue Feb 16, 2024
* Added changes to enable 3.11 builds
* Fix xmlsec#244 - Fix failing test with libxmlsec-1.2.36, also make libxmlsec version available from Python.
* Fix xmlsec#164 - Add support for loading keys from engine (e.g. pkcs11).
* Fix xmlsec#164 - Add tests for pkcs11 (softhsm) key.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
---------
Co-authored-by: Dan Vella <[email protected]>
Co-authored-by: Tomas Divis <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@jimjag jimjag closed this as completed in 2c58d43 Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants