From 3d03d53fd11fff03eedf7c168b08816d9b0d4544 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 25 Jun 2025 16:39:44 +0200 Subject: [PATCH 1/2] Add options to exclude the C14N Transform element in signatures This patch adds a flag 'exclude_c14n_transform_element' (boolean, defaults to False) to XMLSigner.sign(). If set to True, the transform is still applied, but it is not added to the Transforms element in ds:SignedInfo/ds:Reference. Conversely, this patch adds an option to XMLVerifier.verify() to specify a default canonicalization algorithm in the case where SignedInfo does not include the c14n Transform information. Rationale: In most cases, the Transforms element should indeed contain every modification made to the source data before creating or verifying the signature, and indeed, the necessity to add a default algorithm in verify() shows why. However, there are specifications that make use of XML Signatures, where the canonicalization algorithm is fixed, and which even go so far as to forbid naming the c14n algorithm in the Transforms element. This patch aims to make it possible to support those specifications. --- signxml/signer.py | 27 ++++++++++++++++++--------- signxml/verifier.py | 18 ++++++++++++------ test/test.py | 32 +++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 16 deletions(-) diff --git a/signxml/signer.py b/signxml/signer.py index ac39002..2d3fe7d 100644 --- a/signxml/signer.py +++ b/signxml/signer.py @@ -136,6 +136,7 @@ def sign( always_add_key_value: bool = False, inclusive_ns_prefixes: Optional[List[str]] = None, signature_properties: Optional[Union[_Element, List[_Element]]] = None, + exclude_c14n_transform_element: bool = False ) -> _Element: """ Sign the data and return the root element of the resulting XML tree. @@ -183,6 +184,11 @@ def sign( :param signature_properties: One or more Elements that are to be included in the SignatureProperies section when using the detached method. + :param exclude_c14n_transform_element: + When set to True, the c14n algorithm is not added to the signature's Transforms element. By default, + all transforms including canonicalization are added to the Transforms element, and in most cases this + is the correct approach. Only set this to True if you need to comply with a specification where the + c14n algorithm is fixed, and the specification forbids mentioning the algorithm. :returns: A :class:`lxml.etree._Element` object representing the root of the XML tree containing the signature and @@ -235,6 +241,7 @@ def sign( references=references, c14n_inputs=c14n_inputs, inclusive_ns_prefixes=inclusive_ns_prefixes, + exclude_c14n_transform_element=exclude_c14n_transform_element ) for signature_annotator in self.signature_annotators: @@ -377,23 +384,25 @@ def _unpack(self, data, references: List[SignatureReference]): references = [SignatureReference(URI="#object")] return sig_root, doc_root, c14n_inputs, references - def _build_transforms_for_reference(self, *, transforms_node: _Element, reference: SignatureReference): + def _build_transforms_for_reference(self, *, transforms_node: _Element, reference: SignatureReference, exclude_c14n_transform_element: bool = True): assert reference.c14n_method is not None if self.construction_method == SignatureConstructionMethod.enveloped: SubElement(transforms_node, ds_tag("Transform"), Algorithm=SignatureConstructionMethod.enveloped.value) - SubElement(transforms_node, ds_tag("Transform"), Algorithm=reference.c14n_method.value) + if not exclude_c14n_transform_element: + SubElement(transforms_node, ds_tag("Transform"), Algorithm=reference.c14n_method.value) else: - c14n_xform = SubElement( - transforms_node, - ds_tag("Transform"), - Algorithm=reference.c14n_method.value, - ) + if not exclude_c14n_transform_element: + c14n_xform = SubElement( + transforms_node, + ds_tag("Transform"), + Algorithm=reference.c14n_method.value, + ) if reference.inclusive_ns_prefixes: SubElement( c14n_xform, ec_tag("InclusiveNamespaces"), PrefixList=" ".join(reference.inclusive_ns_prefixes) ) - def _build_sig(self, sig_root, references, c14n_inputs, inclusive_ns_prefixes): + def _build_sig(self, sig_root, references, c14n_inputs, inclusive_ns_prefixes, exclude_c14n_transform_element=False): signed_info = SubElement(sig_root, ds_tag("SignedInfo"), nsmap=self.namespaces) sig_c14n_method = SubElement(signed_info, ds_tag("CanonicalizationMethod"), Algorithm=self.c14n_alg.value) if inclusive_ns_prefixes: @@ -407,7 +416,7 @@ def _build_sig(self, sig_root, references, c14n_inputs, inclusive_ns_prefixes): reference = replace(reference, inclusive_ns_prefixes=inclusive_ns_prefixes) reference_node = SubElement(signed_info, ds_tag("Reference"), URI=reference.URI) transforms = SubElement(reference_node, ds_tag("Transforms")) - self._build_transforms_for_reference(transforms_node=transforms, reference=reference) + self._build_transforms_for_reference(transforms_node=transforms, reference=reference, exclude_c14n_transform_element=exclude_c14n_transform_element) SubElement(reference_node, ds_tag("DigestMethod"), Algorithm=self.digest_alg.value) digest_value = SubElement(reference_node, ds_tag("DigestValue")) payload_c14n = self._c14n( diff --git a/signxml/verifier.py b/signxml/verifier.py index 29e3fa8..723333b 100644 --- a/signxml/verifier.py +++ b/signxml/verifier.py @@ -207,7 +207,7 @@ def _get_inclusive_ns_prefixes(self, transform_node): else: return inclusive_namespaces.get("PrefixList").split(" ") - def _apply_transforms(self, payload, *, transforms_node: etree._Element, signature: etree._Element): + def _apply_transforms(self, payload, *, transforms_node: etree._Element, signature: etree._Element, default_c14n_algorithm: Optional[CanonicalizationMethod] = None): transforms, c14n_applied = [], False if transforms_node is not None: transforms = self._findall(transforms_node, "Transform") @@ -239,8 +239,10 @@ def _apply_transforms(self, payload, *, transforms_node: etree._Element, signatu c14n_applied = True if not c14n_applied and not isinstance(payload, (str, bytes)): - payload = self._c14n(payload, algorithm=self._default_reference_c14n_method) - + c14n_algorithm = default_c14n_algorithm if default_c14n_algorithm is not None else self._default_reference_c14n_method + # Create a separate copy of the node, see above + payload = self._fromstring(self._tostring(payload)) + payload = self._c14n(payload, algorithm=c14n_algorithm) return payload def get_cert_chain_verifier(self, ca_pem_file): @@ -295,6 +297,7 @@ def verify( uri_resolver: Optional[Callable] = None, id_attribute: Optional[str] = None, expect_config: SignatureConfiguration = SignatureConfiguration(), + default_c14n_algorithm: Optional[CanonicalizationMethod] = None, **deprecated_kwargs, ) -> Union[VerifyResult, List[VerifyResult]]: """ @@ -372,6 +375,9 @@ def verify( :param deprecated_kwargs: Direct application of the parameters **require_x509**, **expect_references**, and **ignore_ambiguous_key_info** is deprecated. Use **expect_config** instead. + :param default_c14n_algorithm: + Specify a canonicalization algorithm to use if the signature itself does not contain a transform element + for canonicalization. If not set, defaults to CANONICAL_XML_1_0 :raises: :class:`signxml.exceptions.InvalidSignature` """ @@ -514,7 +520,7 @@ def verify( verify_results: List[VerifyResult] = [] for idx, reference in enumerate(self._findall(verified_signed_info, "Reference")): verify_results.append( - self._verify_reference(reference, idx, root, uri_resolver, c14n_algorithm, signature, key_used) + self._verify_reference(reference, idx, root, uri_resolver, c14n_algorithm, signature, key_used, default_c14n_algorithm) ) if type(self.config.expect_references) is int and len(verify_results) != self.config.expect_references: @@ -523,14 +529,14 @@ def verify( return verify_results if self.config.expect_references > 1 else verify_results[0] - def _verify_reference(self, reference, index, root, uri_resolver, c14n_algorithm, signature, signature_key_used): + def _verify_reference(self, reference, index, root, uri_resolver, c14n_algorithm, signature, signature_key_used, default_c14n_algorithm=None): copied_root = self._fromstring(self._tostring(root)) copied_signature_ref = self._get_signature(copied_root) transforms = self._find(reference, "Transforms", require=False) digest_method_alg_name = self._find(reference, "DigestMethod").get("Algorithm") digest_value = self._find(reference, "DigestValue") payload = self._resolve_reference(copied_root, reference, uri_resolver=uri_resolver) - payload_c14n = self._apply_transforms(payload, transforms_node=transforms, signature=copied_signature_ref) + payload_c14n = self._apply_transforms(payload, transforms_node=transforms, signature=copied_signature_ref, default_c14n_algorithm=default_c14n_algorithm) digest_alg = DigestAlgorithm(digest_method_alg_name) self.check_digest_alg_expected(digest_alg) diff --git a/test/test.py b/test/test.py index 6559478..9c9c6ff 100755 --- a/test/test.py +++ b/test/test.py @@ -585,7 +585,7 @@ def test_inclusive_namespaces_signing(self): ) # Test correct default c14n method for payload when c14n transform metadata is omitted - def _build_transforms_for_reference(transforms_node, reference): + def _build_transforms_for_reference(transforms_node, reference, exclude_c14n_transform_element=False): etree.SubElement( transforms_node, ds_tag("Transform"), Algorithm=SignatureConstructionMethod.enveloped.value ) @@ -676,6 +676,36 @@ def test_xmlns_insulation_of_reference_c14n(self): root = XMLSigner().sign(doc, cert=cert, key=key, reference_uri="#target") XMLVerifier().verify(root, x509_cert=cert) + def test_include_c14n_transform_element_by_default(self): + cert, key = self.load_example_keys() + doc = etree.fromstring( + '' + '91' + "" + ) + root = XMLSigner().sign(doc, cert=cert, key=key, reference_uri="#target") + XMLVerifier().verify(root, x509_cert=cert) + transform_elements = root.findall("ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces) + transform_algorithms = [el.attrib['Algorithm'] for el in transform_elements] + self.assertEqual(len(transform_elements), 2) + self.assertIn('http://www.w3.org/2000/09/xmldsig#enveloped-signature', transform_algorithms) + self.assertIn('http://www.w3.org/2006/12/xml-c14n11', transform_algorithms) + + def test_exclude_c14n_transform_element_option(self): + cert, key = self.load_example_keys() + doc = etree.fromstring( + '' + '91' + "" + ) + root = XMLSigner(c14n_algorithm=CanonicalizationMethod.CANONICAL_XML_1_1).sign(doc, cert=cert, key=key, reference_uri="#target", exclude_c14n_transform_element=True) + XMLVerifier().verify(root, x509_cert=cert, default_c14n_algorithm=CanonicalizationMethod.CANONICAL_XML_1_1) + transform_elements = root.findall("ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces) + transform_algorithms = [el.attrib['Algorithm'] for el in transform_elements] + self.assertEqual(len(transform_elements), 1) + self.assertIn('http://www.w3.org/2000/09/xmldsig#enveloped-signature', transform_algorithms) + self.assertNotIn('http://www.w3.org/2006/12/xml-c14n11', transform_algorithms) + def test_verify_config(self): data = etree.parse(self.example_xml_files[0]).getroot() cert, key = self.load_example_keys() From e6a6ee31bf62aed16d0458773697a7f1af91c8d3 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 25 Jun 2025 22:39:39 +0200 Subject: [PATCH 2/2] Move the default_reference_c14n_method in verify() to config Rather than a named argument, the option which default c14n method should be used for references can now be set as a SignatureConfiguration value Additional changes: - ruff formatted the changes from the previous commit - amended the test to also test for a scenario where verification would fail --- signxml/signer.py | 18 +++++++++++++----- signxml/verifier.py | 45 +++++++++++++++++++++++++++++++++------------ test/test.py | 44 ++++++++++++++++++++++++++++++++------------ 3 files changed, 78 insertions(+), 29 deletions(-) diff --git a/signxml/signer.py b/signxml/signer.py index 2d3fe7d..0e780a9 100644 --- a/signxml/signer.py +++ b/signxml/signer.py @@ -136,7 +136,7 @@ def sign( always_add_key_value: bool = False, inclusive_ns_prefixes: Optional[List[str]] = None, signature_properties: Optional[Union[_Element, List[_Element]]] = None, - exclude_c14n_transform_element: bool = False + exclude_c14n_transform_element: bool = False, ) -> _Element: """ Sign the data and return the root element of the resulting XML tree. @@ -241,7 +241,7 @@ def sign( references=references, c14n_inputs=c14n_inputs, inclusive_ns_prefixes=inclusive_ns_prefixes, - exclude_c14n_transform_element=exclude_c14n_transform_element + exclude_c14n_transform_element=exclude_c14n_transform_element, ) for signature_annotator in self.signature_annotators: @@ -384,7 +384,9 @@ def _unpack(self, data, references: List[SignatureReference]): references = [SignatureReference(URI="#object")] return sig_root, doc_root, c14n_inputs, references - def _build_transforms_for_reference(self, *, transforms_node: _Element, reference: SignatureReference, exclude_c14n_transform_element: bool = True): + def _build_transforms_for_reference( + self, *, transforms_node: _Element, reference: SignatureReference, exclude_c14n_transform_element: bool = True + ): assert reference.c14n_method is not None if self.construction_method == SignatureConstructionMethod.enveloped: SubElement(transforms_node, ds_tag("Transform"), Algorithm=SignatureConstructionMethod.enveloped.value) @@ -402,7 +404,9 @@ def _build_transforms_for_reference(self, *, transforms_node: _Element, referenc c14n_xform, ec_tag("InclusiveNamespaces"), PrefixList=" ".join(reference.inclusive_ns_prefixes) ) - def _build_sig(self, sig_root, references, c14n_inputs, inclusive_ns_prefixes, exclude_c14n_transform_element=False): + def _build_sig( + self, sig_root, references, c14n_inputs, inclusive_ns_prefixes, exclude_c14n_transform_element=False + ): signed_info = SubElement(sig_root, ds_tag("SignedInfo"), nsmap=self.namespaces) sig_c14n_method = SubElement(signed_info, ds_tag("CanonicalizationMethod"), Algorithm=self.c14n_alg.value) if inclusive_ns_prefixes: @@ -416,7 +420,11 @@ def _build_sig(self, sig_root, references, c14n_inputs, inclusive_ns_prefixes, e reference = replace(reference, inclusive_ns_prefixes=inclusive_ns_prefixes) reference_node = SubElement(signed_info, ds_tag("Reference"), URI=reference.URI) transforms = SubElement(reference_node, ds_tag("Transforms")) - self._build_transforms_for_reference(transforms_node=transforms, reference=reference, exclude_c14n_transform_element=exclude_c14n_transform_element) + self._build_transforms_for_reference( + transforms_node=transforms, + reference=reference, + exclude_c14n_transform_element=exclude_c14n_transform_element, + ) SubElement(reference_node, ds_tag("DigestMethod"), Algorithm=self.digest_alg.value) digest_value = SubElement(reference_node, ds_tag("DigestValue")) payload_c14n = self._c14n( diff --git a/signxml/verifier.py b/signxml/verifier.py index 723333b..3e6b24b 100644 --- a/signxml/verifier.py +++ b/signxml/verifier.py @@ -87,6 +87,15 @@ class SignatureConfiguration: and validate the signature using X509Data only. """ + default_reference_c14n_method: CanonicalizationMethod = CanonicalizationMethod.CANONICAL_XML_1_1 + """ + The default canonicalization method to use for referenced data structures, if there is no canonicalization + algorithm specified in the Transforms element. In most cases, it should not be necessary to override this + setting; SignedInfo should include every transformation that were applied to the reference before creating + the digests and signature, but there are some uses of XML Signatures where the canonicalization method is + fixed, and not added to the Transforms. This setting can be used to validate signatures in those use-cases. + """ + @dataclass(frozen=True) class VerifyResult: @@ -116,8 +125,6 @@ class XMLVerifier(XMLSignatureProcessor): Create a new XML Signature Verifier object, which can be used to verify multiple pieces of data. """ - _default_reference_c14n_method = CanonicalizationMethod.CANONICAL_XML_1_0 - def _get_signature(self, root): if root.tag == ds_tag("Signature"): return root @@ -207,7 +214,13 @@ def _get_inclusive_ns_prefixes(self, transform_node): else: return inclusive_namespaces.get("PrefixList").split(" ") - def _apply_transforms(self, payload, *, transforms_node: etree._Element, signature: etree._Element, default_c14n_algorithm: Optional[CanonicalizationMethod] = None): + def _apply_transforms( + self, + payload, + *, + transforms_node: etree._Element, + signature: etree._Element, + ): transforms, c14n_applied = [], False if transforms_node is not None: transforms = self._findall(transforms_node, "Transform") @@ -239,10 +252,9 @@ def _apply_transforms(self, payload, *, transforms_node: etree._Element, signatu c14n_applied = True if not c14n_applied and not isinstance(payload, (str, bytes)): - c14n_algorithm = default_c14n_algorithm if default_c14n_algorithm is not None else self._default_reference_c14n_method # Create a separate copy of the node, see above payload = self._fromstring(self._tostring(payload)) - payload = self._c14n(payload, algorithm=c14n_algorithm) + payload = self._c14n(payload, algorithm=self.config.default_reference_c14n_method) return payload def get_cert_chain_verifier(self, ca_pem_file): @@ -297,7 +309,6 @@ def verify( uri_resolver: Optional[Callable] = None, id_attribute: Optional[str] = None, expect_config: SignatureConfiguration = SignatureConfiguration(), - default_c14n_algorithm: Optional[CanonicalizationMethod] = None, **deprecated_kwargs, ) -> Union[VerifyResult, List[VerifyResult]]: """ @@ -375,9 +386,6 @@ def verify( :param deprecated_kwargs: Direct application of the parameters **require_x509**, **expect_references**, and **ignore_ambiguous_key_info** is deprecated. Use **expect_config** instead. - :param default_c14n_algorithm: - Specify a canonicalization algorithm to use if the signature itself does not contain a transform element - for canonicalization. If not set, defaults to CANONICAL_XML_1_0 :raises: :class:`signxml.exceptions.InvalidSignature` """ @@ -520,7 +528,7 @@ def verify( verify_results: List[VerifyResult] = [] for idx, reference in enumerate(self._findall(verified_signed_info, "Reference")): verify_results.append( - self._verify_reference(reference, idx, root, uri_resolver, c14n_algorithm, signature, key_used, default_c14n_algorithm) + self._verify_reference(reference, idx, root, uri_resolver, c14n_algorithm, signature, key_used) ) if type(self.config.expect_references) is int and len(verify_results) != self.config.expect_references: @@ -529,14 +537,27 @@ def verify( return verify_results if self.config.expect_references > 1 else verify_results[0] - def _verify_reference(self, reference, index, root, uri_resolver, c14n_algorithm, signature, signature_key_used, default_c14n_algorithm=None): + def _verify_reference( + self, + reference, + index, + root, + uri_resolver, + c14n_algorithm, + signature, + signature_key_used, + ): copied_root = self._fromstring(self._tostring(root)) copied_signature_ref = self._get_signature(copied_root) transforms = self._find(reference, "Transforms", require=False) digest_method_alg_name = self._find(reference, "DigestMethod").get("Algorithm") digest_value = self._find(reference, "DigestValue") payload = self._resolve_reference(copied_root, reference, uri_resolver=uri_resolver) - payload_c14n = self._apply_transforms(payload, transforms_node=transforms, signature=copied_signature_ref, default_c14n_algorithm=default_c14n_algorithm) + payload_c14n = self._apply_transforms( + payload, + transforms_node=transforms, + signature=copied_signature_ref, + ) digest_alg = DigestAlgorithm(digest_method_alg_name) self.check_digest_alg_expected(digest_alg) diff --git a/test/test.py b/test/test.py index 9c9c6ff..a663062 100755 --- a/test/test.py +++ b/test/test.py @@ -680,31 +680,51 @@ def test_include_c14n_transform_element_by_default(self): cert, key = self.load_example_keys() doc = etree.fromstring( '' - '91' + '91' "" ) root = XMLSigner().sign(doc, cert=cert, key=key, reference_uri="#target") XMLVerifier().verify(root, x509_cert=cert) - transform_elements = root.findall("ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces) - transform_algorithms = [el.attrib['Algorithm'] for el in transform_elements] + transform_elements = root.findall( + "ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces + ) + transform_algorithms = [el.attrib["Algorithm"] for el in transform_elements] self.assertEqual(len(transform_elements), 2) - self.assertIn('http://www.w3.org/2000/09/xmldsig#enveloped-signature', transform_algorithms) - self.assertIn('http://www.w3.org/2006/12/xml-c14n11', transform_algorithms) + self.assertIn("http://www.w3.org/2000/09/xmldsig#enveloped-signature", transform_algorithms) + self.assertIn("http://www.w3.org/2006/12/xml-c14n11", transform_algorithms) def test_exclude_c14n_transform_element_option(self): cert, key = self.load_example_keys() doc = etree.fromstring( '' - '91' + '91' "" ) - root = XMLSigner(c14n_algorithm=CanonicalizationMethod.CANONICAL_XML_1_1).sign(doc, cert=cert, key=key, reference_uri="#target", exclude_c14n_transform_element=True) - XMLVerifier().verify(root, x509_cert=cert, default_c14n_algorithm=CanonicalizationMethod.CANONICAL_XML_1_1) - transform_elements = root.findall("ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces) - transform_algorithms = [el.attrib['Algorithm'] for el in transform_elements] + root = XMLSigner(c14n_algorithm=CanonicalizationMethod.CANONICAL_XML_1_0_WITH_COMMENTS).sign( + doc, cert=cert, key=key, reference_uri="#target", exclude_c14n_transform_element=True + ) + + # The default to use is CANONICAL_XML_1_1 (no comments), and since it's not specified in Transforms, + # verification without specifying the reference canonicalization algorithm should fail. + self.assertRaises( + InvalidDigest, + XMLVerifier().verify, + root, + x509_cert=cert, + ) + + # However, if we use the right configuration, it should verify correctly + config = SignatureConfiguration( + default_reference_c14n_method=CanonicalizationMethod.CANONICAL_XML_1_0_WITH_COMMENTS + ) + XMLVerifier().verify(root, x509_cert=cert, expect_config=config) + transform_elements = root.findall( + "ds:Signature/ds:SignedInfo/ds:Reference/ds:Transforms/ds:Transform", namespaces=namespaces + ) + transform_algorithms = [el.attrib["Algorithm"] for el in transform_elements] self.assertEqual(len(transform_elements), 1) - self.assertIn('http://www.w3.org/2000/09/xmldsig#enveloped-signature', transform_algorithms) - self.assertNotIn('http://www.w3.org/2006/12/xml-c14n11', transform_algorithms) + self.assertIn("http://www.w3.org/2000/09/xmldsig#enveloped-signature", transform_algorithms) + self.assertNotIn("http://www.w3.org/2006/12/xml-c14n11", transform_algorithms) def test_verify_config(self): data = etree.parse(self.example_xml_files[0]).getroot()