Skip to content

Commit 73db72d

Browse files
authored
fix: id attribute detection and generation during signing (resolves #520) (#521)
- Run ensureHasId(node) in addAllReferences to: 1. have consistent Id matching logic as the initial pass 2. add Id attributes to elements which are present inside the Signature itself (KeyInfo, Object) - Added tests for autogenerated ids within the Signature and prefixed Ids
1 parent 4f4e0ed commit 73db72d

File tree

3 files changed

+156
-21
lines changed

3 files changed

+156
-21
lines changed

src/signed-xml.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,27 +1135,7 @@ export class SignedXml {
11351135
if (ref.isEmptyUri) {
11361136
targetUri = "";
11371137
} else {
1138-
let id: string | null = null;
1139-
if (this.idMode === "wssecurity") {
1140-
const attr = utils.findAttr(
1141-
node,
1142-
"Id",
1143-
"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd",
1144-
);
1145-
if (attr) {
1146-
id = attr.value;
1147-
}
1148-
} else {
1149-
for (const attr of this.idAttributes) {
1150-
id = node.getAttribute(attr);
1151-
if (id) {
1152-
break;
1153-
}
1154-
}
1155-
}
1156-
if (!id) {
1157-
throw new Error(`No ID attribute found on node for reference: ${ref.xpath}`);
1158-
}
1138+
const id = this.ensureHasId(node);
11591139
ref.uri = id;
11601140
targetUri = `#${id}`;
11611141
}

test/signature-object-tests.spec.ts

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,130 @@ describe("Valid signatures with ds:Object elements", function () {
366366
const { valid, errorMessage } = checkSignature(signedXml, doc);
367367
expect(valid, errorMessage).to.be.true;
368368
});
369+
370+
it("should create valid signature and generate Id attribute for ds:Object when not provided", function () {
371+
const xml = "<root></root>";
372+
const sig = new SignedXml({
373+
privateKey,
374+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
375+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
376+
objects: [
377+
{
378+
content: "<Data>Test data in Object element</Data>",
379+
},
380+
],
381+
});
382+
383+
sig.addReference({
384+
xpath: "//*[local-name(.)='Data']",
385+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
386+
transforms: [
387+
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
388+
"http://www.w3.org/2001/10/xml-exc-c14n#",
389+
],
390+
});
391+
392+
sig.computeSignature(xml, { prefix: "ds" });
393+
const signedXml = sig.getSignedXml();
394+
const doc = new xmldom.DOMParser().parseFromString(signedXml);
395+
396+
// Find the ds:Object/Data element and get the value of its Id attribute (ensuring it was generated)
397+
const dataEl = select1Ns("/root/ds:Signature/ds:Object/Data[@Id]", doc);
398+
isDomNode.assertIsElementNode(dataEl);
399+
const idValue = dataEl.getAttribute("Id");
400+
expect(idValue).to.be.a("string").that.is.not.empty;
401+
402+
// Verify that there is a Reference pointing to the generated Id
403+
const uri = `#${idValue}`;
404+
const refEl = select1Ns(`/root/ds:Signature/ds:SignedInfo/ds:Reference[@URI='${uri}']`, doc);
405+
isDomNode.assertIsElementNode(refEl);
406+
407+
// Verify that the signature is valid
408+
const { valid, errorMessage } = checkSignature(signedXml, doc);
409+
expect(valid, errorMessage).to.be.true;
410+
});
411+
});
412+
413+
describe("Should successfuly sign references to ds:KeyInfo elements", function () {
414+
it("should create valid signatures with references to ds:KeyInfo when the Id attribute is provided", function () {
415+
const xml = "<root><x /></root>";
416+
const sig = new SignedXml({
417+
privateKey,
418+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
419+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
420+
keyInfoAttributes: {
421+
Id: "key-info-1",
422+
},
423+
getKeyInfoContent: () => "<dummy></dummy>",
424+
});
425+
426+
sig.addReference({
427+
xpath: "//*[local-name(.)='KeyInfo']",
428+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
429+
transforms: [
430+
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
431+
"http://www.w3.org/2001/10/xml-exc-c14n#",
432+
],
433+
});
434+
435+
sig.computeSignature(xml);
436+
const signedXml = sig.getSignedXml();
437+
438+
const doc = new xmldom.DOMParser().parseFromString(signedXml);
439+
440+
// Verify that there is a Reference to KeyInfo
441+
const referenceEl = select1Ns(
442+
"/root/ds:Signature/ds:SignedInfo/ds:Reference[@URI='#key-info-1']",
443+
doc,
444+
);
445+
isDomNode.assertIsElementNode(referenceEl);
446+
447+
// Verify that the signature is valid
448+
const { valid, errorMessage } = checkSignature(signedXml, doc);
449+
expect(valid, errorMessage).to.be.true;
450+
});
451+
452+
it("should create valid signatures with references to ds:KeyInfo when the Id attribute is autogenerated", function () {
453+
const xml = "<root><x /></root>";
454+
const sig = new SignedXml({
455+
privateKey,
456+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
457+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
458+
getKeyInfoContent: () => "<dummy></dummy>",
459+
});
460+
461+
sig.addReference({
462+
xpath: "//*[local-name(.)='KeyInfo']",
463+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
464+
transforms: [
465+
"http://www.w3.org/2000/09/xmldsig#enveloped-signature",
466+
"http://www.w3.org/2001/10/xml-exc-c14n#",
467+
],
468+
});
469+
470+
sig.computeSignature(xml);
471+
const signedXml = sig.getSignedXml();
472+
473+
const doc = new xmldom.DOMParser().parseFromString(signedXml);
474+
475+
// Find the KeyInfo element and get the value of its Id attribute (ensuring it was generated)
476+
const keyInfoEl = select1Ns("/root/ds:Signature/ds:KeyInfo[@Id]", doc);
477+
isDomNode.assertIsElementNode(keyInfoEl);
478+
const idValue = keyInfoEl.getAttribute("Id");
479+
expect(idValue).to.be.a("string").that.is.not.empty;
480+
481+
// Find a Reference with URI=`#${idValue}`
482+
const uri = `#${idValue}`;
483+
const referenceEl = select1Ns(
484+
`/root/ds:Signature/ds:SignedInfo/ds:Reference[@URI='${uri}']`,
485+
doc,
486+
);
487+
isDomNode.assertIsElementNode(referenceEl);
488+
489+
// Verify that the signature is valid
490+
const { valid, errorMessage } = checkSignature(signedXml, doc);
491+
expect(valid, errorMessage).to.be.true;
492+
});
369493
});
370494

371495
describe("XAdES Object support in XML signatures", function () {

test/signature-unit-tests.spec.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,4 +1403,35 @@ describe("Signature unit tests", function () {
14031403
/the following xpath cannot be signed because it was not found/,
14041404
);
14051405
});
1406+
1407+
it("should sign references when the Id attribute is prefixed", () => {
1408+
const xml = '<root><x xmlns:ns="urn:example" ns:Id="unique-id"/></root>';
1409+
const sig = new SignedXml({
1410+
privateKey: fs.readFileSync("./test/static/client.pem"),
1411+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
1412+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
1413+
});
1414+
1415+
sig.addReference({
1416+
xpath: "//*[local-name(.)='x']",
1417+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
1418+
transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
1419+
});
1420+
1421+
sig.computeSignature(xml);
1422+
const signedXml = sig.getSignedXml();
1423+
1424+
const doc = new xmldom.DOMParser().parseFromString(signedXml);
1425+
const referenceElements = xpath.select("//*[local-name(.)='Reference']", doc);
1426+
isDomNode.assertIsArrayOfNodes(referenceElements);
1427+
expect(referenceElements.length, "Reference element should exist").to.equal(1);
1428+
1429+
const referenceElement = referenceElements[0];
1430+
isDomNode.assertIsElementNode(referenceElement);
1431+
1432+
const uriAttribute = referenceElement.getAttribute("URI");
1433+
expect(uriAttribute, "Reference element should have the correct URI attribute value").to.equal(
1434+
"#unique-id",
1435+
);
1436+
});
14061437
});

0 commit comments

Comments
 (0)