Roundtrip referenced XML nodes before c14n to detach them from parent

When signing, we roundtrip referenced XML nodes before passing them to
the c14n algorithm. This effectively detaches the node from the parent
document into its own document subset.

We need to do the same when verifying, otherwise our own c14n is
inconsistent when a default namespace is declared on a parent node and
the element node namespace axis processing algorithm causes empty
namespace declarations to appear. See
https://www.w3.org/TR/xml-c14n11/ sections 2.3, 4.6, 4.7

Fixes #225

signxml/processor.py

@@ -1,6 +1,5 @@
 import logging
 import os
-import warnings
 from typing import Any, List, Tuple
 from xml.etree import ElementTree as stdlibElementTree
 
@@ -53,10 +52,10 @@ def get_root(self, data):
             # TODO: add debug level logging statement re: performance impact here
             return self._fromstring(stdlibElementTree.tostring(data, encoding="utf-8"))
         else:
-            # HACK: deep copy won't keep root's namespaces resulting in an invalid digest
-            # We use a copy so we can modify the tree
-            # TODO: turn this off for xmlenc
-            return self._fromstring(etree.tostring(data))
+            # Create a separate copy of the node so we can modify the tree and avoid any c14n inconsistencies from
+            # namespaces propagating from parent nodes. The lxml docs recommend using copy.deepcopy for this, but it
+            # doesn't seem to preserve namespaces. It would be nice to find a less heavy-handed way of doing this.
+            return self._fromstring(self._tostring(data))
 
 
 class XMLSignatureProcessor(XMLProcessor):
@@ -126,10 +125,6 @@ def _c14n(self, nodes, algorithm: CanonicalizationMethod, inclusive_ns_prefixes=
                 inclusive_ns_prefixes=inclusive_ns_prefixes,
             )
         if exclusive is False and self.excise_empty_xmlns_declarations is True:
-            warnings.warn(
-                "excise_empty_xmlns_declarations is deprecated and will be removed in a future SignXML release",
-                DeprecationWarning,
-            )
             # Incorrect legacy behavior. See also:
             # - https://github.com/XML-Security/signxml/issues/193
             # - http://www.w3.org/TR/xml-c14n, "namespace axis"

signxml/verifier.py

@@ -210,6 +210,12 @@ def _apply_transforms(self, payload, *, transforms_node: etree._Element, signatu
             except ValueError:
                 continue
             inclusive_ns_prefixes = self._get_inclusive_ns_prefixes(transform)
+
+            # Create a separate copy of the node so we can modify the tree and avoid any c14n inconsistencies from
+            # namespaces propagating from parent nodes. The lxml docs recommend using copy.deepcopy for this, but it
+            # doesn't seem to preserve namespaces. It would be nice to find a less heavy-handed way of doing this.
+            payload = self._fromstring(self._tostring(payload))
+
             payload = self._c14n(
                 payload, algorithm=c14n_algorithm_from_transform, inclusive_ns_prefixes=inclusive_ns_prefixes
             )

test/test.py

@@ -640,6 +640,16 @@ def test_payload_c14n(self):
             b'<abc xmlns="http://example.com"><foo xmlns="">bar</foo></abc>',
         )
 
+    def test_xmlns_insulation_of_reference_c14n(self):
+        cert, key = self.load_example_keys()
+        doc = etree.fromstring(
+            '<rDE xmlns="http://example.com/ns1">'
+            '<DE Id="target"><dDVId>9</dDVId><gOpeDE><iTipEmi>1</iTipEmi></gOpeDE></DE>'
+            "</rDE>"
+        )
+        root = XMLSigner().sign(doc, cert=cert, key=key, reference_uri="#target")
+        XMLVerifier().verify(root, x509_cert=cert)
+
     def test_verify_config(self):
         data = etree.parse(self.example_xml_files[0]).getroot()
         cert, key = self.load_example_keys()