fix: apply transforms after a canonicalization and remove only the verified Signature

README.md

@@ -56,6 +56,32 @@ Earlier releases serialized both with xmldom instead.
   different digest or signature than 6.1.x, so a signature created by one will not verify with the
   other. Upgrade signers and verifiers that use it together.
 
+### Transforms that follow a canonicalization
+
+When a transform returns a string and another transform follows it in the same `Reference`, the
+string is parsed into a new document and the next transform is applied to that, as the
+[reference processing model](https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel)
+requires. Every built-in canonicalization algorithm returns a string, so:
+
+- `enveloped-signature` after a canonicalization removes the `Signature`, including one inside the
+  referenced element.
+- The result of a `#WithComments` canonicalization followed by `enveloped-signature` is
+  [canonicalized](#transforms-that-end-in-a-dom-node) without comments.
+- Exclusive canonicalization after inclusive canonicalization omits inherited namespace declarations
+  that the referenced element does not use.
+- A custom transform after a canonicalization receives the parsed document.
+- A transform throws when the string returned by the transform before it is not well-formed XML.
+
+### Copies of an enveloped signature
+
+The `enveloped-signature` transform removes only the `Signature` element being verified, as
+[XMLDSig](https://www.w3.org/TR/xmldsig-core1/#sec-EnvelopedSignature) requires.
+
+- `checkSignature()` finds that element by its `SignatureValue`, and throws when the document
+  contains more than one `Signature` element with that value.
+- `getCanonXml()` finds the loaded signature in the node's document the same way, and removes nothing
+  when the signature is not there.
+
 ### Deprecated ahead of 7.0
 
 The package used to re-export everything in its internal `utils` module, so helpers written for

src/enveloped-signature.ts

@@ -1,6 +1,6 @@
 import * as xpath from "xpath";
 import * as isDomNode from "@xmldom/is-dom-node";
-import { findChildren, isDescendantOf } from "./utils";
+import { isDescendantOf } from "./utils";
 
 import type {
   CanonicalizationOrTransformationAlgorithm,
@@ -29,32 +29,6 @@ export class EnvelopedSignature implements CanonicalizationOrTransformationAlgor
     const signatureNode = options.signatureNode;
     if (isDescendantOf(signatureNode, node) && signatureNode.parentNode) {
       signatureNode.parentNode.removeChild(signatureNode);
-      return node;
-    }
-    const signatureValueNode = findChildren(signatureNode, "SignatureValue")[0];
-    const expectedSignatureValue =
-      signatureValueNode && xpath.select1("text()", signatureValueNode);
-    if (isDomNode.isTextNode(expectedSignatureValue)) {
-      const expectedSignatureValueData = expectedSignatureValue.data;
-
-      const signatures = xpath.select(
-        ".//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
-        node,
-      );
-      for (const nodeSignature of Array.isArray(signatures) ? signatures : []) {
-        const signatureValue = xpath.select1(
-          ".//*[local-name(.)='SignatureValue']/text()",
-          nodeSignature,
-        );
-        if (isDomNode.isTextNode(signatureValue)) {
-          const signatureValueData = signatureValue.data;
-          if (expectedSignatureValueData === signatureValueData) {
-            if (nodeSignature.parentNode) {
-              nodeSignature.parentNode.removeChild(nodeSignature);
-            }
-          }
-        }
-      }
     }
     return node;
   }

src/signed-xml.ts

@@ -29,6 +29,15 @@ import * as utils from "./utils";
 
 type SigningReferenceTarget = { node: Element; digestValue?: string };
 
+function findSignatureElements(node: Node): Element[] {
+  const signatures = xpath.select(
+    ".//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
+    node,
+  );
+  isDomNode.assertIsArrayOfNodes(signatures);
+  return signatures.filter(isDomNode.isElementNode);
+}
+
 const warnOriginalXmlWithIds = deprecate(
   () => {},
   "`getOriginalXmlWithIds()` is deprecated and will be removed in a future version. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`. For a detached signature, put an ID attribute the signer recognizes on each referenced element (`wsu:Id` for WS-Security), sign that document, and send it alongside `getSignatureXml()`.",
@@ -1311,7 +1320,7 @@ export class SignedXml {
     const canonXml = node.cloneNode(true); // Deep clone
     if (transforms.includes("http://www.w3.org/2000/09/xmldsig#enveloped-signature")) {
       const signaturePath: number[] = [];
-      let signatureAncestor = this.signatureNode;
+      let signatureAncestor = this.findLoadedSignature(node);
       while (signatureAncestor?.parentNode && signatureAncestor !== node) {
         signaturePath.push(
           Array.from<Node>(signatureAncestor.parentNode.childNodes).indexOf(signatureAncestor),
@@ -1325,26 +1334,77 @@ export class SignedXml {
       }
     }
     let transformedXml: Node | string = canonXml;
-
-    transforms.forEach((transformName) => {
-      if (isDomNode.isNodeLike(transformedXml)) {
-        // If, after processing, `transformedNode` is a string, we can't do anymore transforms on it
-        const transform = this.findCanonicalizationAlgorithm(transformName);
-        transformedXml = transform.process(transformedXml, options);
+    let transformOptions = options;
+
+    // Octets are parsed into a node-set for the next transform, and a node-set left at the end is
+    // converted to octets with C14N: https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
+    for (const transformName of transforms) {
+      if (!isDomNode.isNodeLike(transformedXml)) {
+        transformedXml = this.parseTransformInput(transformedXml, transformName);
+        // The parsed octets are a new document, so the referenced node's ancestors are gone.
+        transformOptions = {
+          ...options,
+          ancestorNamespaces: [],
+          defaultNs: "",
+          signatureNode: this.findLoadedSignature(transformedXml) ?? options.signatureNode,
+        };
       }
-    });
+      transformedXml = this.findCanonicalizationAlgorithm(transformName).process(
+        transformedXml,
+        transformOptions,
+      );
+    }
 
-    // A node-set is converted to octets with C14N, never with a DOM serializer:
-    // https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
     if (isDomNode.isNodeLike(transformedXml)) {
       transformedXml = this.findCanonicalizationAlgorithm(
         "http://www.w3.org/TR/2001/REC-xml-c14n-20010315",
-      ).process(transformedXml, options);
+      ).process(transformedXml, transformOptions);
     }
 
     return transformedXml.toString();
   }
 
+  // checkSignature() parses its own copy of the document, and octets a transform returns are parsed
+  // again, so the loaded signature is found by its SignatureValue. A copy carrying the same value
+  // could stand in for it, so refuse to guess.
+  private findLoadedSignature(node: Node): Node | null {
+    const doc = node.ownerDocument ?? node;
+    if (this.signatureNode == null || this.signatureNode.ownerDocument === doc) {
+      return this.signatureNode;
+    }
+
+    const signatureValue = utils.findChildren(this.signatureNode, "SignatureValue")[0]?.textContent;
+    if (!signatureValue) {
+      return null;
+    }
+
+    const matches = findSignatureElements(doc).filter(
+      (signature) =>
+        utils.findChildren(signature, "SignatureValue")[0]?.textContent === signatureValue,
+    );
+    if (matches.length > 1) {
+      throw new Error(
+        "Cannot validate a document which contains multiple Signature elements with the same SignatureValue, in order to prevent signature wrapping attack.",
+      );
+    }
+    return matches[0] ?? null;
+  }
+
+  private parseTransformInput(octets: string, transformName: string): Element {
+    const parseErrors: string[] = [];
+    const doc = new xmldom.DOMParser({
+      errorHandler: (_level, message) => parseErrors.push(String(message)),
+    }).parseFromString(octets);
+
+    if (parseErrors.length > 0 || doc.documentElement == null) {
+      throw new Error(
+        `Cannot apply transform ${transformName}: the output of the previous transform is not well-formed XML`,
+      );
+    }
+
+    return doc.documentElement;
+  }
+
   /**
    * Ensure an element has Id attribute. If not create it with unique value.
    * Work with both normal and wssecurity Id flavour

test/canonicalization-unit-tests.spec.ts

@@ -463,28 +463,48 @@ describe("Canonicalization unit tests", function () {
     expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
   });
 
-  it("Shouldn't continue processing transforms if we end up with a string as a result of a transform", function () {
+  it("Parses the string a transform returns for the transform that follows it", function () {
     const doc = new xmldom.DOMParser().parseFromString(
       '<x xmlns:p="myns"><p:y><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y></x>',
     );
-    const node1 = xpath.select1("//*[local-name(.)='y']", doc);
-    const node2 = xpath.select1("//*[local-name(.)='y']", doc);
-    isDomNode.assertIsNodeLike(node1);
-    isDomNode.assertIsNodeLike(node2);
+    const node = xpath.select1("//*[local-name(.)='y']", doc);
+    isDomNode.assertIsNodeLike(node);
+
     const sig = new SignedXml();
-    const res1 = sig.getCanonXml(
+    const res = sig.getCanonXml(
       [
         "http://www.w3.org/2001/10/xml-exc-c14n#",
         "http://www.w3.org/2000/09/xmldsig#enveloped-signature",
       ],
-      node1,
+      node,
+    );
+    expect(res).to.equal('<p:y xmlns:p="myns"></p:y>');
+  });
+
+  it("Throws when a transform follows one that returns malformed XML", function () {
+    const doc = new xmldom.DOMParser().parseFromString("<x><y></y></x>");
+    const node = xpath.select1("//*[local-name(.)='y']", doc);
+    isDomNode.assertIsNodeLike(node);
+
+    const sig = new SignedXml();
+    sig.CanonicalizationAlgorithms["urn:test:malformed"] = class {
+      process() {
+        return "<y>";
+      }
+
+      getAlgorithmName() {
+        return "urn:test:malformed";
+      }
+    };
+
+    expect(() =>
+      sig.getCanonXml(
+        ["urn:test:malformed", "http://www.w3.org/2000/09/xmldsig#enveloped-signature"],
+        node,
+      ),
+    ).to.throw(
+      "Cannot apply transform http://www.w3.org/2000/09/xmldsig#enveloped-signature: the output of the previous transform is not well-formed XML",
     );
-    const res2 = sig.getCanonXml(["http://www.w3.org/2001/10/xml-exc-c14n#"], node2);
-    expect(res1)
-      .to.equal(res2)
-      .to.equal(
-        '<p:y xmlns:p="myns"><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"></ds:Signature></p:y>',
-      );
   });
 
   it("Enveloped-signature canonicalization respects current node", function () {