Use distinct default for payload c14n. Fixes #217

Author: kislyuk

signxml/signer.py

@@ -374,6 +374,17 @@ 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):
+        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)
+        else:
+            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):
         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)
@@ -388,16 +399,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"))
-            if self.construction_method == SignatureConstructionMethod.enveloped:
-                SubElement(transforms, ds_tag("Transform"), Algorithm=SignatureConstructionMethod.enveloped.value)
-                SubElement(transforms, ds_tag("Transform"), Algorithm=reference.c14n_method.value)
-            else:
-                c14n_xform = SubElement(transforms, 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)
-                    )
-
+            self._build_transforms_for_reference(transforms_node=transforms, reference=reference)
             SubElement(reference_node, ds_tag("DigestMethod"), Algorithm=self.digest_alg.value)
             digest_value = SubElement(reference_node, ds_tag("DigestValue"))
             payload_c14n = self._c14n(

signxml/verifier.py

@@ -107,6 +107,8 @@ 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
@@ -188,7 +190,7 @@ def _get_inclusive_ns_prefixes(self, transform_node):
         else:
             return inclusive_namespaces.get("PrefixList").split(" ")
 
-    def _apply_transforms(self, payload, transforms_node, signature, c14n_algorithm: CanonicalizationMethod):
+    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")
@@ -214,7 +216,7 @@ def _apply_transforms(self, payload, transforms_node, signature, c14n_algorithm:
             c14n_applied = True
 
         if not c14n_applied and not isinstance(payload, (str, bytes)):
-            payload = self._c14n(payload, algorithm=c14n_algorithm)
+            payload = self._c14n(payload, algorithm=self._default_reference_c14n_method)
 
         return payload
 
@@ -464,8 +466,7 @@ def _verify_reference(self, reference, index, root, uri_resolver, c14n_algorithm
         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)
-        # TODO: payload-specific c14n alg
-        payload_c14n = self._apply_transforms(payload, transforms, copied_signature_ref, c14n_algorithm)
+        payload_c14n = self._apply_transforms(payload, transforms_node=transforms, signature=copied_signature_ref)
         digest_alg = DigestAlgorithm(digest_method_alg_name)
         if digest_alg not in self.config.digest_algorithms:
             raise InvalidInput(f"Digest algorithm {digest_alg.name} forbidden by configuration")

test/test.py

@@ -34,6 +34,7 @@
     methods,
     namespaces,
 )
+from signxml.util import ds_tag  # noqa:E402
 from signxml.xades import (  # noqa:E402
     XAdESDataObjectFormat,
     XAdESSignatureConfiguration,
@@ -557,6 +558,20 @@ def test_inclusive_namespaces_signing(self):
             "8GPAVJstDxHyuoJqec8C0ssji4zfdXanu1YHGlWbfx0=",
         )
 
+        # Test correct default c14n method for payload when c14n transform metadata is omitted
+        def _build_transforms_for_reference(transforms_node, reference):
+            etree.SubElement(
+                transforms_node, ds_tag("Transform"), Algorithm=SignatureConstructionMethod.enveloped.value
+            )
+
+        signer._build_transforms_for_reference = _build_transforms_for_reference
+        signed3 = signer.sign(**sign_args)
+        self.assertEqual(
+            signed3.find(".//ds:SignatureValue", namespaces=namespaces).text,
+            "/iezjApGBVMMUspj5WyZwIOEw30qLX3Gv576vwFMAbQ=",
+        )
+        XMLVerifier().verify(signed3, hmac_key=b"secret", require_x509=False, validate_schema=False)
+
     def test_excision_of_untrusted_comments(self):
         pass  # TODO: test comments excision