Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 47 additions & 29 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,7 @@ export class SignedXml {
/**
* Search for ancestor namespaces before canonicalization.
*/
if (Array.isArray(ref.transforms)) {
ref.ancestorNamespaces = utils.findAncestorNs(doc, ref.xpath, this.namespaceResolver);
}
ref.ancestorNamespaces = utils.findAncestorNs(doc, ref.xpath, this.namespaceResolver);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

const c14nOptions = {
inclusiveNamespacesPrefixList: ref.inclusiveNamespacesPrefixList,
Expand Down Expand Up @@ -819,10 +817,6 @@ export class SignedXml {
throw new Error("digestAlgorithm is required");
}

if (!utils.isArrayHasLength(transforms)) {
throw new Error("transforms must contain at least one transform algorithm");
}

this.references.push({
xpath,
transforms,
Expand Down Expand Up @@ -1155,32 +1149,36 @@ export class SignedXml {
referenceElem.setAttribute("Type", ref.type);
}

const transformsElem = signatureDoc.createElementNS(
signatureNamespace,
`${currentPrefix}Transforms`,
);

for (const trans of ref.transforms || []) {
const transform = this.findCanonicalizationAlgorithm(trans);
const transformElem = signatureDoc.createElementNS(
if (utils.isArrayHasLength(ref.transforms)) {
const transformsElem = signatureDoc.createElementNS(
signatureNamespace,
`${currentPrefix}Transform`,
`${currentPrefix}Transforms`,
);
transformElem.setAttribute("Algorithm", transform.getAlgorithmName());

if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) {
const inclusiveNamespacesElem = signatureDoc.createElementNS(
transform.getAlgorithmName(),
"InclusiveNamespaces",
for (const trans of ref.transforms) {
const transform = this.findCanonicalizationAlgorithm(trans);
const transformElem = signatureDoc.createElementNS(
signatureNamespace,
`${currentPrefix}Transform`,
);
inclusiveNamespacesElem.setAttribute(
"PrefixList",
ref.inclusiveNamespacesPrefixList.join(" "),
);
transformElem.appendChild(inclusiveNamespacesElem);
transformElem.setAttribute("Algorithm", transform.getAlgorithmName());

if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) {
const inclusiveNamespacesElem = signatureDoc.createElementNS(
transform.getAlgorithmName(),
"InclusiveNamespaces",
);
inclusiveNamespacesElem.setAttribute(
"PrefixList",
ref.inclusiveNamespacesPrefixList.join(" "),
);
transformElem.appendChild(inclusiveNamespacesElem);
}

transformsElem.appendChild(transformElem);
}

transformsElem.appendChild(transformElem);
referenceElem.appendChild(transformsElem);
}

// Get the canonicalized XML
Expand All @@ -1201,7 +1199,6 @@ export class SignedXml {
);
digestValueElem.textContent = digestAlgorithm.getHash(canonXml);

referenceElem.appendChild(transformsElem);
referenceElem.appendChild(digestMethodElem);
referenceElem.appendChild(digestValueElem);

Expand Down Expand Up @@ -1272,7 +1269,7 @@ export class SignedXml {
const canonXml = node.cloneNode(true); // Deep clone
let transformedXml: Node | string = canonXml;

transforms.forEach((transformName) => {
(transforms ?? []).forEach((transformName) => {
if (isDomNode.isNodeLike(transformedXml)) {
// If, after processing, `transformedNode` is a string, we can't do anymore transforms on it
const transform = this.findCanonicalizationAlgorithm(transformName);
Expand All @@ -1287,6 +1284,27 @@ export class SignedXml {
//if only y is the node to sign then a string would be <p:y/> without the definition of the p namespace. probably xmldom toString() should have added it.
});

// When the transform chain produces a DOM node (including the no-transform
// case), apply C14N so that the digest is computed over a canonical byte
// sequence. This mirrors what loadReference does on the verification side:
// it appends C14N when the transform list is empty or ends with
// enveloped-signature, ensuring signing and verification agree on the bytes.
if (typeof transformedXml === "string") {
return transformedXml;
}

// When there are no transforms, the XMLDSig processing model requires the
// node-set to be serialized via C14N before digesting. This mirrors what
// loadReference already does on the verification side (it appends C14N when
// the transform list is empty), so that signing and verification compute the
// same digest.
if (!utils.isArrayHasLength(transforms)) {
const c14n = this.findCanonicalizationAlgorithm(
"http://www.w3.org/TR/2001/REC-xml-c14n-20010315",
);
return String(c14n.process(transformedXml, options));
}

return transformedXml.toString();
}

Expand Down
4 changes: 3 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ export interface Reference {
xpath?: string;

// An array of transforms to be applied to the data before signing.
transforms: ReadonlyArray<CanonicalizationOrTransformAlgorithmType>;
// When absent or empty, no Transforms element is emitted and the referenced
// node is digested directly (after C14N, per the XMLDSig processing model).
transforms?: ReadonlyArray<CanonicalizationOrTransformAlgorithmType>;

// The algorithm used to calculate the digest value of the data.
digestAlgorithm: HashAlgorithmType;
Expand Down
61 changes: 59 additions & 2 deletions test/signature-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,8 @@ describe("Signature unit tests", function () {
ref.uri,
`wrong uri for index ${i}. expected: ${expectedUri} actual: ${ref.uri}`,
).to.equal(expectedUri);
expect(ref.transforms.length).to.equal(1);
expect(ref.transforms[0]).to.equal("http://www.w3.org/2001/10/xml-exc-c14n#");
expect(ref.transforms!.length).to.equal(1);
expect(ref.transforms![0]).to.equal("http://www.w3.org/2001/10/xml-exc-c14n#");
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
expect(ref.digestValue).to.equal(digests[i]);
expect(ref.digestAlgorithm).to.equal("http://www.w3.org/2000/09/xmldsig#sha1");
}
Expand Down Expand Up @@ -1074,6 +1074,63 @@ describe("Signature unit tests", function () {
expect(URI.value, `uri should be empty but instead was ${URI.value}`).to.equal("");
});

it("omits Transforms element when no transforms are specified", function () {
const xml = "<root><x Id='ref1'/></root>";
const sig = new SignedXml();
sig.privateKey = fs.readFileSync("./test/static/client.pem");
sig.addReference({
xpath: "//*[local-name(.)='x']",
digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256",
uri: "#ref1",
digestValue: "",
inclusiveNamespacesPrefixList: [],
isEmptyUri: false,
});
sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";
sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256";
sig.computeSignature(xml);
const signedXml = sig.getSignedXml();
const doc = new xmldom.DOMParser().parseFromString(signedXml);
const transforms = xpath.select(
"//*[local-name(.)='Transforms']",
doc,
);
expect(transforms, "Transforms element should be absent when no transforms specified").to.have
.length(0);
});

it("signs and verifies correctly with no transforms (round-trip)", function () {
const xml = "<root><x Id='ref1'/></root>";
const sig = new SignedXml();
sig.privateKey = fs.readFileSync("./test/static/client.pem");
sig.publicCert = fs.readFileSync("./test/static/client_public.pem");
sig.addReference({
xpath: "//*[local-name(.)='x']",
digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256",
uri: "#ref1",
digestValue: "",
inclusiveNamespacesPrefixList: [],
isEmptyUri: false,
});
sig.canonicalizationAlgorithm = "http://www.w3.org/TR/2001/REC-xml-c14n-20010315";
sig.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256";
sig.computeSignature(xml);
const signedXml = sig.getSignedXml();

const doc = new xmldom.DOMParser().parseFromString(signedXml);
const sigNode = xpath.select1(
"//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
doc,
);
isDomNode.assertIsNodeLike(sigNode);

const verifySig = new SignedXml();
verifySig.publicCert = fs.readFileSync("./test/static/client_public.pem");
verifySig.loadSignature(sigNode);
const result = verifySig.checkSignature(signedXml);
expect(result, "expected signature to verify successfully").to.be.true;
});

it("signer appends signature to a non-existing reference node", function () {
const xml = "<root><name>xml-crypto</name><repository>github</repository></root>";
const sig = new SignedXml();
Expand Down