diff --git a/signxml/signer.py b/signxml/signer.py index ac39002..0e780a9 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,29 @@ 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 +420,11 @@ 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..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): + 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,8 +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)): - payload = self._c14n(payload, algorithm=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=self.config.default_reference_c14n_method) return payload def get_cert_chain_verifier(self, ca_pem_file): @@ -523,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): + 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) + 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 6559478..a663062 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,56 @@ 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_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) + def test_verify_config(self): data = etree.parse(self.example_xml_files[0]).getroot() cert, key = self.load_example_keys()