Skip to content

Commit

Permalink
Merge pull request #736 from cordada/dependabot/pip/signxml-4.0.3
Browse files Browse the repository at this point in the history
chore(deps): Bump signxml from 3.2.2 to 4.0.3
  • Loading branch information
svillegas-cdd authored Dec 12, 2024
2 parents 7ed6b21 + 3b8f2c9 commit 87be259
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 72 deletions.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ dependencies = [
"pydantic>=2.3.0,!=1.7.*,!=1.8.*,!=1.9.*",
"pyOpenSSL>=22.0.0",
"pytz>=2019.3",
"signxml>=3.1.0",
"signxml>=4.0.0",
]
requires-python = ">=3.8, <3.11"
authors = [
Expand Down
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ marshmallow==3.22.0
pydantic==2.10.3
pyOpenSSL==24.2.1
pytz==2024.2
signxml==3.2.2
signxml==4.0.3
6 changes: 2 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,7 @@ pydantic==2.10.3
pydantic-core==2.27.1
# via pydantic
pyopenssl==24.2.1
# via
# -r requirements.in
# signxml
# via -r requirements.in
pytz==2024.2
# via -r requirements.in
referencing==0.35.1
Expand All @@ -77,7 +75,7 @@ rpds-py==0.19.0
# via
# jsonschema
# referencing
signxml==3.2.2
signxml==4.0.3
# via -r requirements.in
sqlparse==0.5.0
# via django
Expand Down
3 changes: 1 addition & 2 deletions src/cl_sii/libs/crypto_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@ def add_pem_cert_header_footer(pem_cert: bytes) -> bytes:
"""
pem_value_str = pem_cert.decode('ascii')
# note: it would be great if 'add_pem_header' did not forcefully convert bytes to str.
mod_pem_value_str = signxml.util.add_pem_header(pem_value_str)
mod_pem_value: bytes = mod_pem_value_str.encode('ascii')
mod_pem_value: bytes = signxml.util.add_pem_header(pem_value_str)
return mod_pem_value


Expand Down
16 changes: 5 additions & 11 deletions src/cl_sii/libs/xml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,14 +440,8 @@ def verify_xml_signature(
)

if isinstance(trusted_x509_cert, crypto_utils._X509CertOpenSsl):
trusted_x509_cert_open_ssl = trusted_x509_cert
elif isinstance(trusted_x509_cert, crypto_utils.X509Cert):
trusted_x509_cert_open_ssl = crypto_utils._X509CertOpenSsl.from_cryptography(
trusted_x509_cert
)
elif trusted_x509_cert is None:
trusted_x509_cert_open_ssl = None
else:
trusted_x509_cert = trusted_x509_cert.to_cryptography()
elif not (isinstance(trusted_x509_cert, crypto_utils.X509Cert) or trusted_x509_cert is None):
# A 'crypto_utils._X509CertOpenSsl' is ok but we prefer 'crypto_utils.X509Cert'.
raise TypeError("'trusted_x509_cert' must be a 'crypto_utils.X509Cert' instance, or None.")

Expand Down Expand Up @@ -481,10 +475,10 @@ def verify_xml_signature(
# https://github.com/XML-Security/signxml/commit/ef15da8dbb904f1dedfdd210ae3e0df5da535612
result = xml_verifier.verify(
data=tmp_bytes,
require_x509=True,
x509_cert=trusted_x509_cert_open_ssl,
ignore_ambiguous_key_info=True,
x509_cert=trusted_x509_cert,
expect_config=signxml.verifier.SignatureConfiguration(
require_x509=True,
ignore_ambiguous_key_info=True,
signature_methods=frozenset([signxml.algorithms.SignatureMethod.RSA_SHA1]),
digest_algorithms=frozenset([signxml.algorithms.DigestAlgorithm.SHA1]),
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,11 @@
</Reference>
</SignedInfo>
<SignatureValue>
fsYP5p/lNfofAz8POShrJjqXdBTNNtvv4/TWCxbvwTIAXr7BLrlvX3C/Hpfo4viqaxSu1OGFgPnk
ddDIFwj/ZsVdbdB+MhpKkyha83RxhJpYBVBY3c+y9J6oMfdIdMAYXhEkFw8w63KHyhdf2E9dnbKi
wqSxDcYjTT6vXsLPrZk=
wwOMQuFqa6c5gzYSJ5PWfo0OiAf+yNcJK6wx4xJ3VNehlAcMrUB2q+rK/DDhCvjxAoX4NxBACiFD
MrTMIfvxrwXjLd1oX37lSFOtsWX6JxL0SV+tLF7qvWCu1Yzw8ypUf7GDkbymJkoTYDF9JFF8kYU4
FdU2wttiwne9XH8QFHgXsocKP/aygwiOeGqiNX9o/O5XS2GWpt+KM20jrvtYn7UFMED/3aPacCb1
GABizr8mlVEZggZgJunMDChpFQyEigSXMK5I737Ac8D2bw7WB47Wj1WBL3sCFRDlXUXtnMvChBVp
0HRUXYuKHyfpCzqIBXygYrIZexxXgOSnKu/yGg==
</SignatureValue>
<KeyInfo>
<KeyValue>
Expand All @@ -87,50 +89,35 @@ Uavs/9J+gR9BBMs/eYE=
</KeyValue>
<X509Data>
<X509Certificate>
MIIIDTCCBvWgAwIBAgIQXD9eCvh/44P1ET5RI1LuJjANBgkqhkiG9w0BAQsFADBU
MQswCQYDVQQGEwJVUzEeMBwGA1UEChMVR29vZ2xlIFRydXN0IFNlcnZpY2VzMSUw
IwYDVQQDExxHb29nbGUgSW50ZXJuZXQgQXV0aG9yaXR5IEczMB4XDTE5MDMyNjEz
NDA0MFoXDTE5MDYxODEzMjQwMFowZjELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNh
bGlmb3JuaWExFjAUBgNVBAcMDU1vdW50YWluIFZpZXcxEzARBgNVBAoMCkdvb2ds
ZSBMTEMxFTATBgNVBAMMDCouZ29vZ2xlLmNvbTBZMBMGByqGSM49AgEGCCqGSM49
AwEHA0IABANpWSLXLbJm5eRzc1EJmvSIbz0nANT+b11r+XhSUCAbfQhS+4M/91YJ
gVE6UtZJrLO7GGxvp1tV/DL857NaLEWjggWSMIIFjjATBgNVHSUEDDAKBggrBgEF
BQcDATAOBgNVHQ8BAf8EBAMCB4AwggRXBgNVHREEggROMIIESoIMKi5nb29nbGUu
Y29tgg0qLmFuZHJvaWQuY29tghYqLmFwcGVuZ2luZS5nb29nbGUuY29tghIqLmNs
b3VkLmdvb2dsZS5jb22CGCouY3Jvd2Rzb3VyY2UuZ29vZ2xlLmNvbYIGKi5nLmNv
gg4qLmdjcC5ndnQyLmNvbYIKKi5nZ3BodC5jboIWKi5nb29nbGUtYW5hbHl0aWNz
LmNvbYILKi5nb29nbGUuY2GCCyouZ29vZ2xlLmNsgg4qLmdvb2dsZS5jby5pboIO
Ki5nb29nbGUuY28uanCCDiouZ29vZ2xlLmNvLnVrgg8qLmdvb2dsZS5jb20uYXKC
DyouZ29vZ2xlLmNvbS5hdYIPKi5nb29nbGUuY29tLmJygg8qLmdvb2dsZS5jb20u
Y2+CDyouZ29vZ2xlLmNvbS5teIIPKi5nb29nbGUuY29tLnRygg8qLmdvb2dsZS5j
b20udm6CCyouZ29vZ2xlLmRlggsqLmdvb2dsZS5lc4ILKi5nb29nbGUuZnKCCyou
Z29vZ2xlLmh1ggsqLmdvb2dsZS5pdIILKi5nb29nbGUubmyCCyouZ29vZ2xlLnBs
ggsqLmdvb2dsZS5wdIISKi5nb29nbGVhZGFwaXMuY29tgg8qLmdvb2dsZWFwaXMu
Y26CESouZ29vZ2xlY25hcHBzLmNughQqLmdvb2dsZWNvbW1lcmNlLmNvbYIRKi5n
b29nbGV2aWRlby5jb22CDCouZ3N0YXRpYy5jboINKi5nc3RhdGljLmNvbYISKi5n
c3RhdGljY25hcHBzLmNuggoqLmd2dDEuY29tggoqLmd2dDIuY29tghQqLm1ldHJp
Yy5nc3RhdGljLmNvbYIMKi51cmNoaW4uY29tghAqLnVybC5nb29nbGUuY29tghYq
LnlvdXR1YmUtbm9jb29raWUuY29tgg0qLnlvdXR1YmUuY29tghYqLnlvdXR1YmVl
ZHVjYXRpb24uY29tghEqLnlvdXR1YmVraWRzLmNvbYIHKi55dC5iZYILKi55dGlt
Zy5jb22CGmFuZHJvaWQuY2xpZW50cy5nb29nbGUuY29tggthbmRyb2lkLmNvbYIb
ZGV2ZWxvcGVyLmFuZHJvaWQuZ29vZ2xlLmNughxkZXZlbG9wZXJzLmFuZHJvaWQu
Z29vZ2xlLmNuggRnLmNvgghnZ3BodC5jboIGZ29vLmdsghRnb29nbGUtYW5hbHl0
aWNzLmNvbYIKZ29vZ2xlLmNvbYIPZ29vZ2xlY25hcHBzLmNughJnb29nbGVjb21t
ZXJjZS5jb22CGHNvdXJjZS5hbmRyb2lkLmdvb2dsZS5jboIKdXJjaGluLmNvbYIK
d3d3Lmdvby5nbIIIeW91dHUuYmWCC3lvdXR1YmUuY29tghR5b3V0dWJlZWR1Y2F0
aW9uLmNvbYIPeW91dHViZWtpZHMuY29tggV5dC5iZTBoBggrBgEFBQcBAQRcMFow
LQYIKwYBBQUHMAKGIWh0dHA6Ly9wa2kuZ29vZy9nc3IyL0dUU0dJQUczLmNydDAp
BggrBgEFBQcwAYYdaHR0cDovL29jc3AucGtpLmdvb2cvR1RTR0lBRzMwHQYDVR0O
BBYEFM8C2hpNgJL/BEX/yzeB408dhba2MAwGA1UdEwEB/wQCMAAwHwYDVR0jBBgw
FoAUd8K4UJpndnaxLcKG0IOgfqZ+ukswIQYDVR0gBBowGDAMBgorBgEEAdZ5AgUD
MAgGBmeBDAECAjAxBgNVHR8EKjAoMCagJKAihiBodHRwOi8vY3JsLnBraS5nb29n
L0dUU0dJQUczLmNybDANBgkqhkiG9w0BAQsFAAOCAQEAF9PM41ShwCbhtJG7tj2y
ZvF2sHbQ5YuZrMfJc6eeCG+nCKm1U5iJzXnXctFGvfJnUCZpj9YrfwDswdEddWyZ
IG6m6wONF3ZiQifQrcDi0oDA+0BwjEuzYGCGkbfE+Xxb30bVEyDRe51DpJf+cqsb
+DW2pYdikbdrPem5/hwdNerc7nqrQOJ93sqwbVNGktuyJsTOGNKkSwSaejxdN7yl
g5aa4CJsE94gy4+mCywWjnnsjcLGJM3RBUxDdAdTGMldU/r33HCUCXl33Qxc4nvP
MlE9LyFOTIJoajWcpGOsbKWiL3Zr19DKNBSn4Xof0onbtCH7dbpyMwP8XcA2O1dA
ow==
MIIGVDCCBTygAwIBAgIKMUWmvgAAAAjUHTANBgkqhkiG9w0BAQUFADCB0jELMAkGA1UEBhMCQ0wx
HTAbBgNVBAgTFFJlZ2lvbiBNZXRyb3BvbGl0YW5hMREwDwYDVQQHEwhTYW50aWFnbzEUMBIGA1UE
ChMLRS1DRVJUQ0hJTEUxIDAeBgNVBAsTF0F1dG9yaWRhZCBDZXJ0aWZpY2Fkb3JhMTAwLgYDVQQD
EydFLUNFUlRDSElMRSBDQSBGSVJNQSBFTEVDVFJPTklDQSBTSU1QTEUxJzAlBgkqhkiG9w0BCQEW
GHNjbGllbnRlc0BlLWNlcnRjaGlsZS5jbDAeFw0xNzA5MDQyMTExMTJaFw0yMDA5MDMyMTExMTJa
MIHXMQswCQYDVQQGEwJDTDEUMBIGA1UECBMLVkFMUEFSQUlTTyAxETAPBgNVBAcTCFF1aWxsb3Rh
MS8wLQYDVQQKEyZTZXJ2aWNpb3MgQm9uaWxsYSB5IExvcGV6IHkgQ2lhLiBMdGRhLjEkMCIGA1UE
CwwbSW5nZW5pZXLDrWEgeSBDb25zdHJ1Y2Npw7NuMSMwIQYDVQQDExpSYW1vbiBodW1iZXJ0byBM
b3BleiAgSmFyYTEjMCEGCSqGSIb3DQEJARYUZW5hY29ubHRkYUBnbWFpbC5jb20wgZ8wDQYJKoZI
hvcNAQEBBQADgY0AMIGJAoGBAKQeAbNDqfi9M2v86RUGAYgq1ZSDioFC6OLr0SwiOaYnLsSOl+Kx
O394PVwSGa6rZk1ErIZonyi15fU/0nHZLi8iHLB49EB5G3tCwh0s8NfqR9ck0/3Z+TXhVUdiJyJC
/z8x5I5lSUfzNEedJRidVvp6jVGr7P/SfoEfQQTLP3mBAgMBAAGjggKnMIICozA9BgkrBgEEAYI3
FQcEMDAuBiYrBgEEAYI3FQiC3IMvhZOMZoXVnReC4twnge/sPGGBy54UhqiCWAIBZAIBBDAdBgNV
HQ4EFgQU1dVHhF0UVe7RXIz4cjl3/Vew+qowCwYDVR0PBAQDAgTwMB8GA1UdIwQYMBaAFHjhPp/S
ErN6PI3NMA5Ts0MpB7NVMD4GA1UdHwQ3MDUwM6AxoC+GLWh0dHA6Ly9jcmwuZS1jZXJ0Y2hpbGUu
Y2wvZWNlcnRjaGlsZWNhRkVTLmNybDA6BggrBgEFBQcBAQQuMCwwKgYIKwYBBQUHMAGGHmh0dHA6
Ly9vY3NwLmVjZXJ0Y2hpbGUuY2wvb2NzcDAjBgNVHREEHDAaoBgGCCsGAQQBwQEBoAwWCjEzMTg1
MDk1LTYwIwYDVR0SBBwwGqAYBggrBgEEAcEBAqAMFgo5NjkyODE4MC01MIIBTQYDVR0gBIIBRDCC
AUAwggE8BggrBgEEAcNSBTCCAS4wLQYIKwYBBQUHAgEWIWh0dHA6Ly93d3cuZS1jZXJ0Y2hpbGUu
Y2wvQ1BTLmh0bTCB/AYIKwYBBQUHAgIwge8egewAQwBlAHIAdABpAGYAaQBjAGEAZABvACAARgBp
AHIAbQBhACAAUwBpAG0AcABsAGUALgAgAEgAYQAgAHMAaQBkAG8AIAB2AGEAbABpAGQAYQBkAG8A
IABlAG4AIABmAG8AcgBtAGEAIABwAHIAZQBzAGUAbgBjAGkAYQBsACwAIABxAHUAZQBkAGEAbgBk
AG8AIABoAGEAYgBpAGwAaQB0AGEAZABvACAAZQBsACAAQwBlAHIAdABpAGYAaQBjAGEAZABvACAA
cABhAHIAYQAgAHUAcwBvACAAdAByAGkAYgB1AHQAYQByAGkAbzANBgkqhkiG9w0BAQUFAAOCAQEA
mxtPpXWslwI0+uJbyuS9s/S3/Vs0imn758xMU8t4BHUd+OlMdNAMQI1G2+q/OugdLQ/a9Sg3clKD
qXR4lHGl8d/Yq4yoJzDD3Ceez8qenY3JwGUhPzw9oDpg4mXWvxQDXSFeW/u/BgdadhfGnpwx61Un
+/fU24ZgU1dDJ4GKj5oIPHUIjmoSBhnstEhIr6GJWSTcDKTyzRdqBlaVhenH2Qs6Mw6FrOvRPuud
B7lo1+OgxMb/Gjyu6XnEaPu7Vq4XlLYMoCD2xrV7WEADaDTm7KcNLczVAYqWSF1WUqYSxmPoQDFY
+kMTThJyCXBlE0NADInrkwWgLLygkKI7zXkwaw==
</X509Certificate>
</X509Data>
</KeyInfo>
Expand Down
72 changes: 66 additions & 6 deletions src/tests/test_libs_xml_utils.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import datetime
import io
import unittest
from typing import Any
from unittest import mock

import lxml.etree
import signxml

from cl_sii.libs.crypto_utils import load_pem_x509_cert
from cl_sii.base.constants import SII_OFFICIAL_TZ
from cl_sii.libs.crypto_utils import _X509CertOpenSsl, load_pem_x509_cert
from cl_sii.libs.tz_utils import convert_naive_dt_to_tz_aware
from cl_sii.libs.xml_utils import ( # noqa: F401
XmlElement,
XmlFeatureForbidden,
Expand Down Expand Up @@ -185,6 +191,28 @@ def test_ok_external_trusted_cert(self) -> None:
signature_xml_bytes = f.getvalue()
self.assertEqual(signature_xml_bytes, self.with_valid_signature_signature_xml)

def test_ok_external_trusted_open_ssl_cert_with_signature(self) -> None:
xml_doc = parse_untrusted_xml(self.with_valid_signature)
cert = load_pem_x509_cert(self.xml_doc_cert_pem_bytes)

open_ssl_cert = _X509CertOpenSsl.from_cryptography(cert)

signed_data, signed_xml, signature_xml = verify_xml_signature(
xml_doc, trusted_x509_cert=open_ssl_cert
)

self.assertEqual(signed_data, self.with_valid_signature_signed_data)

f = io.BytesIO()
write_xml_doc(signed_xml, f)
signed_xml_bytes = f.getvalue()
self.assertEqual(signed_xml_bytes, self.with_valid_signature_signed_xml)

f = io.BytesIO()
write_xml_doc(signature_xml, f)
signature_xml_bytes = f.getvalue()
self.assertEqual(signature_xml_bytes, self.with_valid_signature_signature_xml)

def test_ok_cert_in_signature(self) -> None:
# TODO: implement!

Expand Down Expand Up @@ -221,7 +249,7 @@ def test_fail_verify_with_other_cert(self) -> None:
verify_xml_signature(xml_doc, trusted_x509_cert=cert)
self.assertEqual(
cm.exception.args,
("Signature verification failed: wrong signature length",),
("Signature verification failed: ",),
)

def test_bad_cert_included(self) -> None:
Expand All @@ -244,26 +272,58 @@ def test_bad_cert_included(self) -> None:
)

def test_fail_replaced_cert(self) -> None:
"""
Tests that the signature verification fails
when the certificate is not the one that was used to sign the document.
"""
xml_doc = parse_untrusted_xml(self.with_replaced_cert)
cert = load_pem_x509_cert(self.any_x509_cert_pem_file)
cert = load_pem_x509_cert(self.xml_doc_cert_pem_bytes)

with self.assertRaises(XmlSignatureInvalid) as cm:
verify_xml_signature(xml_doc, trusted_x509_cert=cert)
self.assertEqual(
cm.exception.args,
("Signature verification failed: []",),
("Signature verification failed: ",),
)

def test_fail_included_cert_not_from_a_known_ca(self) -> None:
xml_doc = parse_untrusted_xml(self.with_valid_signature)
xml_doc_signature_timestamp = convert_naive_dt_to_tz_aware(
dt=datetime.datetime.fromisoformat('2019-04-01T01:36:40'), # From XML doc’s <TmstFirma>
tz=SII_OFFICIAL_TZ,
)

def _get_cert_chain_verifier(
*args: Any, **kwargs: Any
) -> signxml.util.X509CertChainVerifier:
# The default signature verification time is the current time (see
# https://cryptography.io/en/43.0.3/x509/verification/#cryptography.x509.verification.PolicyBuilder.time
# ), but that causes verification to fail with the message
# “validation failed: cert is not valid at validation time”.
# To avoid that, we set the verification time to the time of the signature.
return signxml.util.X509CertChainVerifier(
ca_pem_file=kwargs['ca_pem_file'], verification_time=xml_doc_signature_timestamp
)

# Without cert: fails because the issuer of the cert in the signature is not a known CA.
with self.assertRaises(XmlSignatureInvalidCertificate) as cm:
with self.assertRaises(XmlSignatureInvalidCertificate) as cm, mock.patch.object(
signxml.verifier.XMLVerifier,
'get_cert_chain_verifier',
side_effect=_get_cert_chain_verifier,
) as mock_get_cert_chain_verifier:
verify_xml_signature(xml_doc, trusted_x509_cert=None)
self.assertEqual(
cm.exception.args,
('unable to get local issuer certificate',),
# According to some test cases from https://x509-limbo.com/, OpenSSL’s error message
# “unable to get local issuer certificate” seems to be equivalent to PyCA Cryptography’s
# error message below:
(
'validation failed:'
' candidates exhausted:'
' all candidates exhausted with no interior errors',
),
)
mock_get_cert_chain_verifier.assert_called_once_with(ca_pem_file=None)

def test_fail_signed_data_modified(self) -> None:
xml_doc = parse_untrusted_xml(self.with_signature_and_modified)
Expand Down

0 comments on commit 87be259

Please sign in to comment.