xml-crypto icon indicating copy to clipboard operation
xml-crypto copied to clipboard

[BUG]: keyInfo usage

Open IlyaRazuvaev opened this issue 1 year ago • 4 comments

Is your feature request related to a problem? Please describe...

I have just updated to 4 version and confused by validateSignatureValue function. I have duplicated <KeyInfo> inside SamlRequest and SamlMetadata. Similar to Okta example http://saml.oktadev.com/. That's mean that loadSignature functions will initialize this.keyInfo by request key, and validateSignatureValue will use it preferable over metadata certificate without any option to choose another behavior.

       // loadSignature 
        const keyInfo = xpath.select1(".//*[local-name(.)='KeyInfo']", signatureNode);
        // TODO: should this just be a single return instead of an array that we always take the first entry of?
        if (xpath.isNodeLike(keyInfo)) {
            this.keyInfo = keyInfo;
        }
       
        // validateSignatureValue 
        const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey;

Describe teh solution you'd like...

Another order of keys.

Describe the alternatives you've considered...

Configurable keyInfo

IlyaRazuvaev avatar Aug 11 '23 14:08 IlyaRazuvaev

Thanks for flagging this. This was also picked up by another user and discussed in #399. The user pointed out that there is a viable workaround in this comment.

tldr: consider doing the following to declare getCertFromKeyInfo() a no-op:

new SignedXml(
  {
    // ...
    getCertFromKeyInfo: () => undefined
  }
);

4.x introduced a breaking change where we now adhere to section 3.2.2 of W3C's XML Signature Syntax and Processing (Second Edition)^1, ie, if a certificate is present in the KeyInfo element of an XML document, we will use that for validation.

LoneRifle avatar Oct 08 '23 09:10 LoneRifle

getKeyInfoContent is now a noop by default. This will show in the next semver-major release.

cjbarth avatar Nov 26 '23 14:11 cjbarth

Thanks for flagging this. This was also picked up by another user and discussed in #399. The user pointed out that there is a viable workaround in this comment.

tldr: consider doing the following to declare getCertFromKeyInfo() a no-op:

new SignedXml(
  {
    // ...
    getCertFromKeyInfo: () => undefined
  }
);

4.x introduced a breaking change where we now adhere to section 3.2.2 of W3C's XML Signature Syntax and Processing (Second Edition)1, ie, if a certificate is present in the KeyInfo element of an XML document, we will use that for validation.

Footnotes

1. https://www.w3.org/TR/2008/REC-xmldsig-core-20080610/#sec-CoreValidation [↩](#user-content-fnref-1-6bd00c995b16e7b73ddf894a6d7c52fd)

By that logic you should also resolve the SecurityTokenReference when in wssecurity mode.

https://docs.oasis-open.org/wss/v1.1/wss-v1.1-spec-pr-x509TokenProfile-01.htm

shunkica avatar Dec 22 '23 07:12 shunkica

By that logic you should also resolve the SecurityTokenReference when in wssecurity mode.

https://docs.oasis-open.org/wss/v1.1/wss-v1.1-spec-pr-x509TokenProfile-01.htm

Feel free to submit a PR.

cjbarth avatar Dec 23 '23 20:12 cjbarth