Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
84 changes: 54 additions & 30 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -427,12 +427,16 @@ export class SignedXml {
return this.getCanonXml([this.canonicalizationAlgorithm], signedInfo[0], c14nOptions);
}

private getCanonReferenceXml(doc: Document, ref: Reference, node: Node) {
private getCanonReferenceXml(_doc: Document, ref: Reference, node: Node) {
/**
* Search for ancestor namespaces before canonicalization.
* Derive ancestor namespaces from the specific node being digested, not by
* re-running ref.xpath. findAncestorNs uses only the first XPath match, so
* when multiple references are created for the same xpath pattern (e.g. via
* addAllReferences), references beyond the first are digested with the wrong
* ancestor namespace scope.
*/
if (Array.isArray(ref.transforms)) {
ref.ancestorNamespaces = utils.findAncestorNs(doc, ref.xpath, this.namespaceResolver);
if (isDomNode.isElementNode(node)) {
ref.ancestorNamespaces = utils.findAncestorNsForNode(node);
}

const c14nOptions = {
Expand Down Expand Up @@ -819,10 +823,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 +1155,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 +1205,6 @@ export class SignedXml {
);
digestValueElem.textContent = digestAlgorithm.getHash(canonXml);

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

Expand Down Expand Up @@ -1272,7 +1275,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 +1290,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
71 changes: 44 additions & 27 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,49 @@ function isElementSubset(docSubset: Node[]): docSubset is Element[] {
return docSubset.every((node) => isDomNode.isElementNode(node));
}

/** Deduplicate and filter ancestor namespaces for a given subset element. */
function buildAncestorNsForElement(element: Element): NamespacePrefix[] {
const ancestorNs = collectAncestorNamespaces(element);
const ancestorNsWithoutDuplicate: NamespacePrefix[] = [];
for (let i = 0; i < ancestorNs.length; i++) {
let notOnTheList = true;
for (const v in ancestorNsWithoutDuplicate) {
if (ancestorNsWithoutDuplicate[v].prefix === ancestorNs[i].prefix) {
notOnTheList = false;
break;
}
}

if (notOnTheList) {
ancestorNsWithoutDuplicate.push(ancestorNs[i]);
}
}

// Remove namespaces which are already declared in the subset with the same prefix
const returningNs: NamespacePrefix[] = [];
const subsetNsPrefix = findNSPrefix(element);
for (const ns of ancestorNsWithoutDuplicate) {
if (ns.prefix !== subsetNsPrefix) {
returningNs.push(ns);
}
}

return returningNs;
}

/**
* Extract ancestor namespaces for a specific element node.
* Prefer this over `findAncestorNs` when the target element is already known,
* since `findAncestorNs` re-executes an XPath and uses the first match —
* which is incorrect when multiple nodes are referenced individually.
*
* @param element - The element whose ancestor namespace declarations to collect
* @returns i.e. [{prefix: "saml", namespaceURI: "urn:oasis:names:tc:SAML:2.0:assertion"}]
*/
export function findAncestorNsForNode(element: Element): NamespacePrefix[] {
return buildAncestorNsForElement(element);
}

/**
* Extract ancestor namespaces in order to import it to root of document subset
* which is being canonicalized for non-exclusive c14n.
Expand Down Expand Up @@ -264,33 +307,7 @@ export function findAncestorNs(
throw new Error("Document subset must be list of elements");
}

// Remove duplicate on ancestor namespace
const ancestorNs = collectAncestorNamespaces(docSubset[0]);
const ancestorNsWithoutDuplicate: NamespacePrefix[] = [];
for (let i = 0; i < ancestorNs.length; i++) {
let notOnTheList = true;
for (const v in ancestorNsWithoutDuplicate) {
if (ancestorNsWithoutDuplicate[v].prefix === ancestorNs[i].prefix) {
notOnTheList = false;
break;
}
}

if (notOnTheList) {
ancestorNsWithoutDuplicate.push(ancestorNs[i]);
}
}

// Remove namespaces which are already declared in the subset with the same prefix
const returningNs: NamespacePrefix[] = [];
const subsetNsPrefix = findNSPrefix(docSubset[0]);
for (const ancestorNs of ancestorNsWithoutDuplicate) {
if (ancestorNs.prefix !== subsetNsPrefix) {
returningNs.push(ancestorNs);
}
}

return returningNs;
return buildAncestorNsForElement(docSubset[0]);
}

export function validateDigestValue(digest, expectedDigest) {
Expand Down
105 changes: 103 additions & 2 deletions test/signature-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -908,8 +908,7 @@ 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).to.deep.equal(["http://www.w3.org/2001/10/xml-exc-c14n#"]);
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 +1073,108 @@ describe("Signature unit tests", function () {
expect(URI.value, `uri should be empty but instead was ${URI.value}`).to.equal("");
});

for (const { label, transforms } of [
{ label: "omitted transforms property", transforms: undefined },
{ label: "empty transforms array", transforms: [] as string[] },
]) {
it(`omits Transforms element when no transforms are specified (${label})`, 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,
...(transforms !== undefined ? { transforms } : {}),
});
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 transformNodes = xpath.select("//*[local-name(.)='Transforms']", doc);
expect(transformNodes, "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("correctly canonicalizes no-transform references under different ancestor namespace scopes", function () {
// Two <item> elements live under different namespace scopes. Without the
// fix, findAncestorNs(doc, ref.xpath) always uses the first XPath match
// (item1's scope), so item2 is digested with the wrong ancestor namespaces
// and verification fails.
const xml =
"<root>" +
"<section xmlns:ns1='http://ns1.example.com'><item Id='item1'>one</item></section>" +
"<section xmlns:ns2='http://ns2.example.com'><item Id='item2'>two</item></section>" +
"</root>";
const sig = new SignedXml();
sig.privateKey = fs.readFileSync("./test/static/client.pem");
sig.publicCert = fs.readFileSync("./test/static/client_public.pem");
for (const id of ["item1", "item2"]) {
sig.addReference({
xpath: `//*[@Id='${id}']`,
digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256",
uri: `#${id}`,
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