Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method for checking if element is signed #368

Merged
merged 3 commits into from
Aug 2, 2023
Merged
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
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,20 @@ If the verification process fails `sig.validationErrors` will contain the errors
In order to protect from some attacks we must check the content we want to use is the one that has been signed:

```javascript
var elem = select(doc, "/xpath_to_interesting_element");
// Roll your own
var elem = xpath.select("/xpath_to_interesting_element", doc);
var uri = sig.references[0].uri; // might not be 0 - depending on the document you verify
var id = uri[0] === "#" ? uri.substring(1) : uri;
if (elem.getAttribute("ID") != id && elem.getAttribute("Id") != id && elem.getAttribute("id") != id)
throw new Error("the interesting element was not the one verified by the signature");

// Use the built-in method
let elem = xpath.select("/xpath_to_interesting_element", doc);
try {
const matchingReference = sig.validateElementAgainstReferences(elem, doc);
} catch {
throw new Error("the interesting element was not the one verified by the signature");
}
```

Note:
Expand Down
51 changes: 43 additions & 8 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
privateKey?: crypto.KeyLike;
publicCert?: crypto.KeyLike;
/**
* One of the supported signature algorithms. See {@link SignatureAlgorithmType}
* One of the supported signature algorithms.
* @see {@link SignatureAlgorithmType}
*/
signatureAlgorithm: SignatureAlgorithmType = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
/**
Expand All @@ -56,21 +57,24 @@
getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;

// Internal state
/**
* Specifies the data to be signed within an XML document. See {@link Reference}
*/
private references: Reference[] = [];
private id = 0;
private signedXml = "";
private signatureXml = "";
private signatureNode: Node | null = null;
private signatureValue = "";
private originalXmlWithIds = "";
private keyInfo: Node | null = null;

/**
* Contains the references that were signed.
* @see {@link Reference}
*/
references: Reference[] = [];

/**
* Contains validation errors (if any) after {@link checkSignature} method is called
*/
validationErrors: string[] = [];
private keyInfo: Node | null = null;

/**
* To add a new transformation algorithm create a new class that implements the {@link TransformationAlgorithm} interface, and register it here. More info: {@link https://github.com/node-saml/xml-crypto#customizing-algorithms|Customizing Algorithms}
Expand Down Expand Up @@ -205,7 +209,7 @@
* Returns the value of the signing certificate based on the contents of the
* specified KeyInfo.
*
* @param keyInfo KeyInfo element (see https://www.w3.org/TR/2008/REC-xmldsig-core-20080610/#sec-X509Data)
* @param keyInfo KeyInfo element (@see https://www.w3.org/TR/2008/REC-xmldsig-core-20080610/#sec-X509Data)
* @return the signing certificate as a string in PEM format
*/
static getCertFromKeyInfo(keyInfo?: Node | null): string | null {
Expand Down Expand Up @@ -389,6 +393,37 @@
}
}

validateElementAgainstReferences(elem: Element, doc: Document): Reference {
for (const ref of this.references) {

Check warning on line 397 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L397

Added line #L397 was not covered by tests
const uri = ref.uri?.[0] === "#" ? ref.uri.substring(1) : ref.uri;
let targetElem: xpath.SelectSingleReturnType;

for (const attr of this.idAttributes) {
const elemId = elem.getAttribute(attr);

Check warning on line 402 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L401-L402

Added lines #L401 - L402 were not covered by tests
if (uri === elemId) {
targetElem = elem;
ref.xpath = `//*[@*[local-name(.)='${attr}']='${uri}']`;
break; // found the correct element, no need to check further

Check warning on line 406 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L404-L406

Added lines #L404 - L406 were not covered by tests
}
}

// @ts-expect-error This is a problem with the types on `xpath`
if (!xpath.isNodeLike(targetElem)) {
continue;

Check warning on line 412 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L412

Added line #L412 was not covered by tests
}

const canonXml = this.getCanonReferenceXml(doc, ref, targetElem);
const hash = this.findHashAlgorithm(ref.digestAlgorithm);
const digest = hash.getHash(canonXml);

Check warning on line 417 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L415-L417

Added lines #L415 - L417 were not covered by tests

if (utils.validateDigestValue(digest, ref.digestValue)) {
return ref;

Check warning on line 420 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L420

Added line #L420 was not covered by tests
}
}

throw new Error("No references passed validation");

Check warning on line 424 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L424

Added line #L424 was not covered by tests
}

validateReferences(doc) {
for (const ref of this.references) {
let elemXpath;
Expand Down Expand Up @@ -517,7 +552,7 @@
loadReference(refNode: Node) {
let nodes = utils.findChildren(refNode, "DigestMethod");
if (nodes.length === 0) {
throw new Error(`could not find DigestMethod in reference ${refNode.toString()}`);

Check warning on line 555 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L555

Added line #L555 was not covered by tests
}
const digestAlgoNode = nodes[0];

Expand All @@ -529,7 +564,7 @@

nodes = utils.findChildren(refNode, "DigestValue");
if (nodes.length === 0) {
throw new Error(`could not find DigestValue node in reference ${refNode.toString()}`);

Check warning on line 567 in src/signed-xml.ts

View check run for this annotation

Codecov / codecov/patch

src/signed-xml.ts#L567

Added line #L567 was not covered by tests
}
const firstChild = nodes[0].firstChild;
if (!firstChild || !("data" in firstChild)) {
Expand Down Expand Up @@ -573,7 +608,7 @@
* 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:
* @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
Expand Down
2 changes: 1 addition & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type SignatureAlgorithmType =
| string;

/**
* @param cert the certificate as a string or array of strings (see https://www.w3.org/TR/2008/REC-xmldsig-core-20080610/#sec-X509Data)
* @param cert the certificate as a string or array of strings (@see https://www.w3.org/TR/2008/REC-xmldsig-core-20080610/#sec-X509Data)
* @param prefix an optional namespace alias to be used for the generated XML
*/
export interface GetKeyInfoContentArgs {
Expand Down
16 changes: 10 additions & 6 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,22 @@ const xml_special_to_encoded_text = {

export function encodeSpecialCharactersInAttribute(attributeValue) {
return attributeValue.replace(/([&<"\r\n\t])/g, function (str, item) {
// Special character normalization. See:
// - https://www.w3.org/TR/xml-c14n#ProcessingModel (Attribute Nodes)
// - https://www.w3.org/TR/xml-c14n#Example-Chars
/** Special character normalization.
* @see:
* - https://www.w3.org/TR/xml-c14n#ProcessingModel (Attribute Nodes)
* - https://www.w3.org/TR/xml-c14n#Example-Chars
*/
return xml_special_to_encoded_attribute[item];
});
}

export function encodeSpecialCharactersInText(text: string): string {
return text.replace(/([&<>\r])/g, function (str, item) {
// Special character normalization. See:
// - https://www.w3.org/TR/xml-c14n#ProcessingModel (Text Nodes)
// - https://www.w3.org/TR/xml-c14n#Example-Chars
/** Special character normalization.
* @see:
* - https://www.w3.org/TR/xml-c14n#ProcessingModel (Text Nodes)
* - https://www.w3.org/TR/xml-c14n#Example-Chars
*/
return xml_special_to_encoded_text[item];
});
}
Expand Down
10 changes: 6 additions & 4 deletions test/signature-integration-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ describe("Signature integration tests", function () {
it("add canonicalization if output of transforms will be a node-set rather than an octet stream", function () {
let xml = fs.readFileSync("./test/static/windows_store_signature.xml", "utf-8");

// Make sure that whitespace in the source document is removed -- see xml-crypto issue #23 and post at
// http://webservices20.blogspot.co.il/2013/06/validating-windows-mobile-app-store.html
// This regex is naive but works for this test case; for a more general solution consider
// the xmldom-fork-fixed library which can pass {ignoreWhiteSpace: true} into the Dom constructor.
/** Make sure that whitespace in the source document is removed --
* @see xml-crypto issue #23 and post at
* http://webservices20.blogspot.co.il/2013/06/validating-windows-mobile-app-store.html
* This regex is naive but works for this test case; for a more general solution consider
* the xmldom-fork-fixed library which can pass {ignoreWhiteSpace: true} into the Dom constructor.
*/
xml = xml.replace(/>\s*</g, "><");

const doc = new xmldom.DOMParser().parseFromString(xml);
Expand Down
11 changes: 9 additions & 2 deletions test/signature-unit-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ describe("Signature unit tests", function () {
const checkedSignature = sig.checkSignature(xml);
expect(checkedSignature).to.be.true;

// @ts-expect-error FIXME
expect(sig.references.length).to.equal(3);

const digests = [
Expand All @@ -91,9 +90,17 @@ describe("Signature unit tests", function () {
"sH1gxKve8wlU8LlFVa2l6w3HMJ0=",
];

const firstGrandchild = doc.firstChild?.firstChild;

// @ts-expect-error FIXME
for (let i = 0; i < sig.references.length; i++) {
if (xpath.isElement(firstGrandchild)) {
expect(() => sig.validateElementAgainstReferences(firstGrandchild, doc)).to.not.throw;
} else {
// @ts-expect-error FIXME
expect(xpath.isElement(firstGrandchild)).to.be.true;
}

for (let i = 0; i < sig.references.length; i++) {
const ref = sig.references[i];
const expectedUri = `#_${i}`;
expect(
Expand Down
Loading