invalid signature: for uri calculated digest is '*' but the xml to validate supplies digest '*'

[CarlaMck77]

Good Day,

I have seen this reported a few different time with no resolution. I am hoping I would get some assistance in resolving this.
I am new to this type of feature, so any guidance or assistance is more than welcome. I have battling this issue for a few days now and I have no idea what to do next.

I am using version "xml-crypto": "^4.0.1"

node version v16.14.0

Error displayed as

invalid signature: for uri calculated digest is 2U1suBt1sOA2olbnbMK1gC/3FHk= but the xml to validate supplies digest OGXcEIgUP1W+Hv9ghexl8gdMtrI=

Generated XML

<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:1.0:protocol">
    <saml:Assertion MajorVersion="1" MinorVersion="1"
        AssertionID="_1" Issuer="mydomain.com"
        IssueInstant="2023-08-13T03:01:27.265Z" xmlns:saml="urn:oasis:names:tc:SAML:1.0:assertion"
        Id="_0">
        <saml:Conditions NotBefore="2023-08-13T03:01:27.266Z"
            NotOnOrAfter="2023-08-14T03:01:27.263Z" />
        <saml:AuthenticationStatement AuthenticationMethod="urn:oasis:names:tc:SAML:1.0:am:password"
            AuthenticationInstant="2023-08-13T03:01:27.268Z">
            <saml:Subject>
                <saml:NameIdentifier Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"
                    NameQualifier="urn:mydomain.com">123356</saml:NameIdentifier>
                <saml:SubjectConfirmation>
                    <saml:ConfirmationMethod>urn:oasis:names:tc:SAML:1.0:cm:bearer</saml:ConfirmationMethod>
                </saml:SubjectConfirmation>
            </saml:Subject>
        </saml:AuthenticationStatement>
        <saml:AttributeStatement>
            <saml:Subject>
                <saml:NameIdentifier Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"
                    NameQualifier="urn:mydomain.com">123356</saml:NameIdentifier>
                <saml:SubjectConfirmation>
                    <saml:ConfirmationMethod>urn:oasis:names:tc:SAML:1.0:cm:bearer</saml:ConfirmationMethod>
                </saml:SubjectConfirmation>
            </saml:Subject>
            <saml:Attribute AttributeName="version" AttributeNamespace="mydomain.com">
                <saml:AttributeValue>1</saml:AttributeValue>
            </saml:Attribute>
        </saml:AttributeStatement>
    </saml:Assertion>
    <samlp:Status>
        <samlp:StatusCode Value="samlp:Success" />
    </samlp:Status>
    <Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
        <SignedInfo>
            <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
            <SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
            <Reference URI="#_0">
                <Transforms>
                    <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
                </Transforms>
                <DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
                <DigestValue>OGXcEIgUP1W+Hv9ghexl8gdMtrI=</DigestValue>
            </Reference>
        </SignedInfo>
        <SignatureValue>
            HKJ/iE4XpJlF219H0zP8yK6SZO/Haz1234E09GTwD...[redacted]...tsRlIJX9nMtdg==</SignatureValue>
    </Signature>
</samlp:Response>

I used the following function to strip the spaces from the XML

function removeXMLSpaces(string){
  let xmlString = string;
  xmlString = xmlString.replace(/>\s*/g, ">");
  xmlString = xmlString.replace(/\s*</g, "<");
  xmlString = xmlString.replace(/\r\n?/g, "\n");
  xmlString = xmlString.replace(/(\r\n\t|\n|\r\t)/gm, "");
  return xmlString;
}

Here is my SignXML function used

function signXml(xml, options) {
  console.log({ options });

  const sig = new SignedXml(options);

  // sig.keyInfoProvider = new KeyProvider();

  sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";

  sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";

  sig.addReference({
    xpath: "//*[local-name(.)='Assertion']",
    transforms: ["http://www.w3.org/2000/09/xmldsig#enveloped-signature"]
  });

  sig.computeSignature(xml);

  fs.writeFileSync(process.cwd() + "/xml/signed.xml", sig.getSignedXml());

  let currentSignedXML = sig.getSignedXml();
  currentSignedXML = currentSignedXML.replace("Id=\"_0\"","").replace("URI=\"#_0\"","URI=\"\"");

  return currentSignedXML;
}

Let me know if I need to provide more details

[srd90]

This is not an answer for your question. More like "side notes":

I do not quite understand what you are trying to accomplish here with all the Id and URI replacements (there should be no need to do that).

Based on example XML snippet at issue description and addReference's xpath clause at your example code it looks like you are trying to sign assertion part of the SAML 1.1 protocol's authn response message but placement of the signature element is incorrect (it can be defined with computeSignature's location option).

I.e. IMHO SAML 1.1 document at the issue report is not even valid from XML schema validation point of view (signature calculated from the assertion should be inside assertion element at the position defined in assertion schema and if you are trying to sign whole message that signature's position is defined at protocol schema).

• SAML 1.1 protocol schema https://www.oasis-open.org/committees/download.php/3407/oasis-sstc-saml-schema-protocol-1.1.xsd
• SAML 1.1 assertion schema https://www.oasis-open.org/committees/download.php/3408/oasis-sstc-saml-schema-assertion-1.1.xsd

---

You wrote:

...I have seen this reported a few different time with no resolution....

Could you provide references to those earlier issues that you were looking before writing this issue report.

---

As a random side note:

There are lot of online examples and tools for SAML 2.0 but those are not usable for your case due to SAML 1.1

Here is example of signing SAML 2.0 response with xml-crypto (version < 4.0.0): node-saml/passport-saml#836 (comment) NOTE: SAML 1.1 and SAML 2.0 are different things but you can pick some hints how to place calculated signature to correct position at the resulting XML (see from SAML 1.1 schemas correct position for SAML 1.1 message's / assertion's signature).

[CarlaMck77]

So...i found the issue. I am not sure if this requires an update to validation.

For it to work, I had to add the transform for C14n. On reading the documentation, I believe that this was a default value but on inspecting the code in the package, I saw the value was missing in the validationXML which caused the mismatch.

Also note that I changes the xpath to "Response" and took @srd90 suggestion for appending the signature.

function signXml(xml, options) {
  const sig = new SignedXml(options);

  sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";

  sig.canonicalizationAlgorithm =
    "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";

  sig.addReference({
    xpath: "//*[local-name(.)='Response']",
    transforms: ["http://www.w3.org/2000/09/xmldsig#enveloped-signature", "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"],
  });

  sig.computeSignature(xml, {
    location: {
      reference: "//*[local-name(.)='Status']",
      action: "after",
    },
  });

  let currentSignedXML = sig.getSignedXml();
  
  return currentSignedXML;
}

[cjbarth]

Can you site where in the documentation it mentions that default value, or, even better, put up a PR with a test that makes sure the defaults correspond to the spec?

[srd90]

I was also wondering back in the days what @CarlaMck77 might have meant with this

For it to work, I had to add the transform for C14n. On reading the documentation, I believe that this was a default value but on inspecting the code in the package, I saw the value was missing in the validationXML which caused the mismatch.

And just in case author of that comment has lost interest to this issue I share what I figured / speculated about aforementioned comment:

On comment #376 (comment) 's example javascript had this:

...
sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";

sig.addReference({
  xpath: "//*[local-name(.)='Assertion']",
  transforms: ["http://www.w3.org/2000/09/xmldsig#enveloped-signature"]
});
...

and same comment's example XML had this:

...
       <SignedInfo>
           <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
...
               <Transforms>
                   <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
               </Transforms>
...
          </Reference>
      </SignedInfo>
...

On follow up comment #376 (comment) author had these

...
 sig.canonicalizationAlgorithm =
   "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";

 sig.addReference({
   xpath: "//*[local-name(.)='Response']",
   transforms: ["http://www.w3.org/2000/09/xmldsig#enveloped-signature", "http://www.w3.org/TR/2001/REC-xml-c14n-20010315"],
 });
...

and XML (if such example would have been provided) would probably had looked like this:

 ...
        <SignedInfo>
            <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
 ...
                <Transforms>
                    <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
                    <Transform Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
                </Transforms>
 ...
           </Reference>
       </SignedInfo>
 ...

So what was supposedly added ("...I had to add the transform for C14n...") was <Transform Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />...but I failed to spot where README says that it would be by default in transforms list.

---

Side note:
IMHO there is at least mismatch / "fuzzyness" in README (from 4.1.0):

xml-crypto/README.md

Lines 52 to 54 in 06cab68

	by default the following algorithms are used:
	_Canonicalization/Transformation Algorithm:_ Exclusive Canonicalization http://www.w3.org/2001/10/xml-exc-c14n#

i.e. http://www.w3.org/2001/10/xml-exc-c14n#

versus

xml-crypto/README.md

Line 209 in 06cab68

- `canonicalizationAlgorithm` - string - default `http://www.w3.org/TR/2001/REC-xml-c14n-20010315` - the canonicalization algorithm to use

i.e. http://www.w3.org/TR/2001/REC-xml-c14n-20010315

This side note applies only to 4.x codebase (at least up to 4.1.0)

---

Side note 2:

Another fuzziness:

loadSignature adds implicitly ...REC-xml-c14n-20010315 to transforms list under certain circumstances:

xml-crypto/src/signed-xml.ts

Lines 573 to 588 in 06cab68

	/**
	* DigestMethods take an octet stream rather than a node set. If the output of the last transform is a node set, we
	* need to canonicalize the node set to an octet stream using non-exclusive canonicalization. If there are no
	* transforms, we need to canonicalize because URI dereferencing for a same-document reference will return a node-set.
	* See:
	* https://www.w3.org/TR/xmldsig-core1/#sec-DigestMethod
	* https://www.w3.org/TR/xmldsig-core1/#sec-ReferenceProcessingModel
	* https://www.w3.org/TR/xmldsig-core1/#sec-Same-Document
	*/
	if (
	transforms.length === 0 ||
	transforms[transforms.length - 1] ===
	"http://www.w3.org/2000/09/xmldsig#enveloped-signature"
	) {
	transforms.push("http://www.w3.org/TR/2001/REC-xml-c14n-20010315");
	}

but I fail to see that implicit addition in addReference and/or computeSignature execution path.

addReference adds by default only this (if transformations list is empty):

xml-crypto/src/signed-xml.ts

Line 614 in 06cab68

transforms = ["http://www.w3.org/2001/10/xml-exc-c14n#"],

...maybe this "side note 2" was something that author of issue meant because author of the issue had initially only http://www.w3.org/2000/09/xmldsig#enveloped-signature at addReference's transfomationslist and if xml-crypto was used to verify signature then loadSignature would have added (prior to performing c14n operations) this http://www.w3.org/TR/2001/REC-xml-c14n-20010315

[cjbarth]

@LoneRifle , do you have any thoughts on this? In all the refactoring, I don't think we touched anything related to any of these defaults in the code, but I'd have to double-check. In any case, the README should at least be clear to a reader, which it appears it currently isn't.

[CarlaMck77]

@srd90: You're speculations are accurate
@cjbarth
Apologies to everyone if my details/explanation wasn't very clear

Adding default at the beginning gave me the impression that if I did not set the value, this would be automatically chosen. Also in the example provided, nothing suggests that adding a value for canonicalizationAlgorithm may need to be added to addReference. If it did, it wasn't obvious to me

Sidenote: I've never submitted any PR or MRs to a public project before but I'll give it a go. All I will ask is just to give me a bit of time :)

[cjbarth]

@CarlaMck77 , we're more than happy to help anyone through the process of submitting PRs. Having said that, in harmony with the approach taken with node-saml and passport-saml, we are removing as many defaults as possible from this project to force the consumer to think about the actions they are taking. Hopefully this will address some of the concerns/issues you have (had).

In any case, more PRs, especially those that clarify the documentation or increase test coverage, are very welcome.

[cjbarth]

@CarlaMck77 Again, thanks for pointing this out and @srd90 , thank you for providing specific details. I believe with the latest PRs that have landed there are no longer any confusing defaults set. If so, I believe we are ready for a semver-major release (pending a round of dependency updates). Please let me know if I've missed anything.

[srd90]

... we are ready for a semver-major release (pending a round of dependency updates). Please let me know if I've missed anything.

@cjbarth commenting here even though this is not related to this issue: Did you schedule this #399 (reply in thread) to this semver major release?