Remove the default for getKeyInfoContent forcing a consumer to choose

[cjbarth]

Per the discussion here, remove the default for getKeyInfoContent. This will force the consumer to make a decision here. The default implementation is still available.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (741240f) 73.36% compared to head (e1072d7) 73.17%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
- Coverage   73.36%   73.17%   -0.20%     
==========================================
  Files           9        9              
  Lines         901      902       +1     
  Branches      239      239              
==========================================
- Hits          661      660       -1     
- Misses        142      143       +1     
- Partials       98       99       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[srd90]

@cjbarth This PR did not fix vulnerability. Discussion you referenced at the description of this PR suggested setting getCertFromKeyInfo by default to "noop" but this PR set getKeyInfoContent to "noop" (IMHO getKeyInfoContent is not involved at signature validation execution path and it does not have any effect to issue described at #399). I.e. if someone initiates SignedXML like this:

 const sig = new xmlCrypto.SignedXml();
 sig.publicCert = pemFile;
 sig.loadSignature(signature);

(that code was copy paste from node-saml/node-saml#341 's version https://github.com/node-saml/node-saml/blob/eaaae9f6ced68cdda5b5e7c79f136abc9a07039d/src/xml.ts#L113-L115 )

Then untrusted cert is used by default (i.e. if attacker re-signs XML message with own private key and attaches that key's cert then attacker can by pass signature validation). One example of this is described at node-saml/node-saml#341 (comment) (note that example applies to version https://github.com/node-saml/node-saml/tree/eaaae9f6ced68cdda5b5e7c79f136abc9a07039d of that PR branch)

So once again consider setting at least getKeyInfoFromCert (instead of/in addition to getKeyInfoContent) to noop and in addition to that consider switching precedence order of

xml-crypto/src/signed-xml.ts

Line 267 in f8cbbb7

const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey;