Skip to content

Commit

Permalink
Move validation messages to each reference (#396)
Browse files Browse the repository at this point in the history
  • Loading branch information
cjbarth authored Oct 6, 2023
1 parent 0d01641 commit d98128a
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 24 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,13 @@ var signature = select(
)[0];
var sig = new SignedXml({ publicCert: fs.readFileSync("client_public.pem") });
sig.loadSignature(signature);
var res = sig.checkSignature(xml);
if (!res) console.log(sig.validationErrors);
try {
var res = sig.checkSignature(xml);
} catch (ex) {
console.log(ex);
}
```

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
Expand Down Expand Up @@ -247,8 +248,7 @@ To verify xml documents:

- `loadSignature(signatureXml)` - loads the signature where:
- `signatureXml` - a string or node object (like an [xmldom](https://github.com/xmldom/xmldom) node) containing the xml representation of the signature
- `checkSignature(xml)` - validates the given xml document and returns true if the validation was successful, `sig.validationErrors` will have the validation errors if any, where:
- `xml` - a string containing a xml document
- `checkSignature(xml)` - validates the given xml document and returns `true` if the validation was successful

## Customizing Algorithms

Expand Down
21 changes: 8 additions & 13 deletions src/signed-xml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@ export class SignedXml {
*/
private references: Reference[] = [];

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

/**
* 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 @@ -248,7 +243,6 @@ export class SignedXml {
throw new Error("Last parameter must be a callback function");
}

this.validationErrors = [];
this.signedXml = xml;

const doc = new xmldom.DOMParser().parseFromString(xml);
Expand All @@ -271,14 +265,15 @@ export class SignedXml {
if (callback) {
signer.verifySignature(signedInfoCanon, key, this.signatureValue, callback);
} else {
const res = signer.verifySignature(signedInfoCanon, key, this.signatureValue);
if (res === false) {
this.validationErrors.push(
`invalid signature: the signature value ${this.signatureValue} is incorrect`,
const verified = signer.verifySignature(signedInfoCanon, key, this.signatureValue);

if (verified === false) {
throw new Error(
"invalid signature: the signature value ${this.signatureValue} is incorrect",
);
}

return res;
return true;
}
}

Expand Down Expand Up @@ -438,7 +433,7 @@ export class SignedXml {
const validationError = new Error(
`invalid signature: the signature references an element with uri ${ref.uri} but could not find such element in the xml`,
);
this.validationErrors.push(validationError.message);
ref.validationError = validationError;
return false;
}

Expand All @@ -450,7 +445,7 @@ export class SignedXml {
const validationError = new Error(
`invalid signature: for uri ${ref.uri} calculated digest is ${digest} but the xml to validate supplies digest ${ref.digestValue}`,
);
this.validationErrors.push(validationError.message);
ref.validationError = validationError;

return false;
}
Expand Down
3 changes: 2 additions & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export interface Reference {

// Optional. The type of the reference node.
ancestorNamespaces?: NamespacePrefix[];

validationError?: Error;
}

/** Implement this to create a new CanonicalizationOrTransformationAlgorithm */
Expand Down Expand Up @@ -204,7 +206,6 @@ export interface TransformAlgorithm {
* #### Api
* - {@link SignedXml#loadSignature}
* - {@link SignedXml#checkSignature}
* - {@link SignedXml#validationErrors}
*/

function isErrorFirstCallback<T>(
Expand Down
3 changes: 1 addition & 2 deletions test/hmac-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ describe("HMAC tests", function () {
sig.enableHMAC();
sig.publicCert = fs.readFileSync("./test/static/hmac-foobar.key");
sig.loadSignature(signature);
const result = sig.checkSignature(xml);

expect(result).to.be.false;
expect(() => sig.checkSignature(xml)).to.throw(/^invalid signature/);
});

it("test create and validate HMAC signature", function () {
Expand Down
3 changes: 1 addition & 2 deletions test/saml-response-tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ describe("SAML response tests", function () {
const sig = new SignedXml();
sig.publicCert = fs.readFileSync("./test/static/feide_public.pem");
sig.loadSignature(signature);
const result = sig.checkSignature(xml);
// This doesn't matter, just want to make sure that we don't fail due to unknown algorithm
expect(result).to.be.false;
expect(() => sig.checkSignature(xml)).to.throw(/^invalid signature/);
});
});

0 comments on commit d98128a

Please sign in to comment.