diff --git a/signxml/processor.py b/signxml/processor.py index 1903b36..011d588 100644 --- a/signxml/processor.py +++ b/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" diff --git a/signxml/verifier.py b/signxml/verifier.py index 841ecd5..7f43c5a 100644 --- a/signxml/verifier.py +++ b/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 ) diff --git a/test/test.py b/test/test.py index 50b5ec0..69782d1 100755 --- a/test/test.py +++ b/test/test.py @@ -640,6 +640,16 @@ def test_payload_c14n(self): b'bar', ) + def test_xmlns_insulation_of_reference_c14n(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) + def test_verify_config(self): data = etree.parse(self.example_xml_files[0]).getroot() cert, key = self.load_example_keys()