Merge pull request #2 from overleaf/tm-normalization-patch

Patch validateXmlSignatureForCert to attempt alternate normalization

src/node-saml/xml.ts

@@ -72,35 +72,43 @@ export const validateXmlSignatureForCert = (
   currentNode: Element
 ): boolean => {
   const sig = new xmlCrypto.SignedXml();
-  sig.keyInfoProvider = {
-    file: "",
-    getKeyInfo: () => "<X509Data></X509Data>",
-    getKey: () => Buffer.from(certPem),
-  };
-  const signatureStr = normalizeNewlines(signature.toString());
-  sig.loadSignature(signatureStr);
-  // We expect each signature to contain exactly one reference to the top level of the xml we
-  //   are validating, so if we see anything else, reject.
-  if (sig.references.length != 1) return false;
-  const refUri = sig.references[0].uri!;
-  const refId = refUri[0] === "#" ? refUri.substring(1) : refUri;
-  // If we can't find the reference at the top level, reject
-  const idAttribute = currentNode.getAttribute("ID") ? "ID" : "Id";
-  if (currentNode.getAttribute(idAttribute) != refId) return false;
-  // If we find any extra referenced nodes, reject.  (xml-crypto only verifies one digest, so
-  //   multiple candidate references is bad news)
-  const totalReferencedNodes = xpath.selectElements(
-    currentNode.ownerDocument,
-    "//*[@" + idAttribute + "='" + refId + "']"
-  );
-
-  if (totalReferencedNodes.length > 1) {
-    return false;
+  const originalXml = fullXml; // save original xml for restoring before second attempt
+  for(const overrideNormalization of [false, true]) {
+    fullXml = originalXml;
+    sig.keyInfoProvider = {
+      file: "",
+      getKeyInfo: () => "<X509Data></X509Data>",
+      getKey: () => Buffer.from(certPem),
+    };
+    const signatureStr = overrideNormalization ? signature.toString() : normalizeNewlines(signature.toString());
+    sig.loadSignature(signatureStr);
+    // We expect each signature to contain exactly one reference to the top level of the xml we
+    //   are validating, so if we see anything else, reject.
+    if (sig.references.length != 1) return false;
+    const refUri = sig.references[0].uri!;
+    const refId = refUri[0] === "#" ? refUri.substring(1) : refUri;
+    // If we can't find the reference at the top level, reject
+    const idAttribute = currentNode.getAttribute("ID") ? "ID" : "Id";
+    if (currentNode.getAttribute(idAttribute) != refId) return false;
+    // If we find any extra referenced nodes, reject.  (xml-crypto only verifies one digest, so
+    //   multiple candidate references is bad news)
+    const totalReferencedNodes = xpath.selectElements(
+      currentNode.ownerDocument,
+      "//*[@" + idAttribute + "='" + refId + "']"
+    );
+
+    if (totalReferencedNodes.length > 1) {
+      return false;
+    }
+    // normalize XML to replace XML-encoded carriage returns with actual carriage returns
+    fullXml = normalizeXml(fullXml);
+    if(!overrideNormalization) {
+      fullXml = normalizeNewlines(fullXml);
+    }
+    const isValid = sig.checkSignature(fullXml);
+    if(isValid) { return isValid; }
   }
-  // normalize XML to replace XML-encoded carriage returns with actual carriage returns
-  fullXml = normalizeXml(fullXml);
-  fullXml = normalizeNewlines(fullXml);
-  return sig.checkSignature(fullXml);
+  return false; // maintain behavior of returning false if not valid
 };
 
 interface XmlSignatureLocation {