Add support for directly querying a node to see if it has passed validation

[cjbarth]

Previously, the API for checking if a reference was valid was very complex. This adds support for allow for a single method call on the container that represents the reference to determine if it passed validation. The idea is that this can clean up some code as found at node-saml:

export const validateXmlSignatureForCert = (
  signature: Node,
  certPem: string,
  fullXml: string,
  currentNode: Element
): boolean => {
  const sig = new xmlCrypto.SignedXml();
  sig.keyInfoProvider = {
    file: "",
    getKeyInfo: () => "<X509Data></X509Data>",
    getKey: () => Buffer.from(certPem),
  };
  sig.loadSignature(signature);
  // 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;
  assertRequired(refUri, "signature reference uri not found");
  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;
  }
  fullXml = normalizeNewlines(fullXml);
  return sig.checkSignature(fullXml);
};

Which code follows the recommendation in the README:

// Roll your own
var elem = xpath.select("/xpath_to_interesting_element", doc);
var uri = sig.references[0].uri; // might not be 0 - depending on the document you verify
var id = uri[0] === "#" ? uri.substring(1) : uri;
if (elem.getAttribute("ID") != id && elem.getAttribute("Id") != id && elem.getAttribute("id") != id)
  throw new Error("the interesting element was not the one verified by the signature");

// Use the built-in method
let elem = xpath.select("/xpath_to_interesting_element", doc);
try {
  const matchingReference = sig.validateElementAgainstReferences(elem, doc);
} catch {
  throw new Error("the interesting element was not the one verified by the signature");
}

Since we already know about the id internally, this shouldn't have to be re-implemented by a consumer.

[codecov]

Codecov Report

Merging #389 (6399cc6) into master (f0237e9) will increase coverage by 0.20%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
+ Coverage   74.03%   74.24%   +0.20%     
==========================================
  Files           9        9              
  Lines         882      889       +7     
  Branches      233      236       +3     
==========================================
+ Hits          653      660       +7     
+ Misses        136      135       -1     
- Partials       93       94       +1     

Files	Coverage Δ
src/types.ts	90.00% <ø> (ø)
src/signed-xml.ts	76.84% <85.71%> (+0.39%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[cjbarth]

@LoneRifle If you like this approach, or think a feature like this has merit, I'll add some tests.

[LoneRifle]

My dev tooling is not within reach currently; will be a few days before I can get round to looking!

[cjbarth]

@LoneRifle , I'm eager to hear what you think. I'd like to get this landed and released so that I can use it in the pending release of node-saml. I appreciate you buying out time to have a look.

[LoneRifle]

Sorry for taking so long!

lgtm, but how is this going to be used? Should we update the README to explain the new feature?

[cjbarth]

Yes, I'd update the README with this new process. I'll actually write a test that shows the old way working to validate and the new way working to validate. Now that you're OK with the general direction, I'll start to move forward on the README and tests.

[cjbarth]

I've finished with this code and am breaking it into smaller pieces to land as functional commits and to make review easier.

[cjbarth]

@LoneRifle Hopefully this gives a clearer idea of how this would be used. Please have a look and approve this again if you agree with this methodology. I'm 100% read to land this unless you have feedback.

After this, I'll make a PR for pulling isDomNode from NPM instead of GitHub, and then we can release this as a semver-major and get on to node-saml work :)

[LoneRifle]

Lgtm