diff --git a/README.md b/README.md index 891e91c..1f2e02b 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,48 @@ ## Upgrading +### Upgrading to 7.0 + +Asynchronous work is done through its own entry points rather than by passing a callback. +`computeSignature(xml, callback)`, `computeSignature(xml, options, callback)` and +`checkSignature(xml, callback)` are gone, along with the `createOptionalCallbackFunction` +helper and the `ErrorFirstCallback` type: + +```js +// before +sig.computeSignature(xml, (err) => { + if (err) return done(err); + use(sig.getSignedXml()); +}); + +// after +const signedXml = (await sig.computeSignatureAsync(xml)).getSignedXml(); +``` + +Passing a callback now throws a `TypeError` naming the replacement, rather than signing +successfully and never calling back. + +"Synchronous unless you pass a callback" was observable to the caller: switching +`signatureAlgorithm` changed whether the caller's own `try`/`catch` caught a handler's error and +whether state assigned after the call was visible to the handler. Node's own answer to this is a +pair of separately named functions, and that is what this is. + +`checkSignature`'s asynchronous path did not work at all: it called `verifySignature` in its +three-argument synchronous form and never passed the callback down, so an async-only verifier +could not report a valid signature. `checkSignatureAsync` replaces it. + +`HashAlgorithm.getHash`, `SignatureAlgorithm.getSignature` and +`SignatureAlgorithm.verifySignature` are now optional, joined by `getHashAsync`, +`getSignatureAsync` and `verifySignatureAsync`. An implementation provides whichever forms its +backend supports — see [asynchronous signing and verification](#asynchronous-signing-and-verification). +Existing synchronous implementations keep working unchanged; TypeScript code that _calls_ these +methods through the interface type now has to account for them being optional. + +`validateElementAgainstReferences` gained an asynchronous twin, +`validateElementAgainstReferencesAsync`. + +### Upgrading to 6.0 + The `.getReferences()` AND the `.references` APIs are deprecated. Please do not attempt to access them. The content in them should be treated as unsigned. @@ -278,6 +320,7 @@ To sign xml documents: - `attrs` - a hash of attributes and values `attrName: value` to add to the signature root node - `location` - customize the location of the signature, pass an object with a `reference` key which should contain a XPath expression to a reference node, an `action` key which should contain one of the following values: `append`, `prepend`, `before`, `after` - `existingPrefixes` - A hash of prefixes and namespaces `prefix: namespace` that shouldn't be in the signature because they already exist in the xml +- `computeSignatureAsync(xml, [options])` - as `computeSignature`, but returns a promise for this instance and awaits any [algorithm that can only work asynchronously](#algorithms-that-can-only-work-asynchronously) - `getSignedXml()` - returns the original xml document with the signature in it, **must be called only after `computeSignature`** - `getSignatureXml()` - returns just the signature part, **must be called only after `computeSignature`** - `getOriginalXmlWithIds()` - **[deprecated]** returns the original xml with Id attributes added on relevant elements, **must be called only after `computeSignature`**. Use the `location` option of `computeSignature()` to place the signature, then `getSignedXml()`. See [how to specify the location of the signature](#how-to-specify-the-location-of-the-signature). @@ -287,6 +330,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 +- `checkSignatureAsync(xml)` - as `checkSignature`, but returns a promise and awaits any [algorithm that can only work asynchronously](#algorithms-that-can-only-work-asynchronously) ## Customizing Algorithms @@ -332,12 +376,21 @@ function MySignatureAlgorithm() { return "signature of signedInfo as base64..."; }; + /*verify the given signature over the given material. return a boolean*/ + this.verifySignature = function (material, key, signatureValue) { + return true; + }; + this.getAlgorithmName = function () { return "http://mySigningAlgorithm"; }; } ``` +If the backend cannot answer synchronously — Web Crypto, an HSM, a KMS, a signing server — +implement `getHashAsync`, `getSignatureAsync` and `verifySignatureAsync` instead. See +[asynchronous signing and verification](#asynchronous-signing-and-verification). + Custom transformation algorithm. ```javascript @@ -422,32 +475,78 @@ You can always look at the actual code as a sample. ## Asynchronous signing and verification -If the private key is not stored locally, and you wish to use a signing server or Hardware Security Module (HSM) to sign documents, you can create a custom signing algorithm that uses an asynchronous callback. +Every entry point comes in two forms. `computeSignature` and `checkSignature` are synchronous; +`computeSignatureAsync` and `checkSignatureAsync` return promises and await the crypto. ```javascript -function AsyncSignatureAlgorithm() { - this.getSignature = function (signedInfo, privateKey, callback) { - var signer = crypto.createSign("RSA-SHA1"); - signer.update(signedInfo); - var res = signer.sign(privateKey, "base64"); - //Do some asynchronous things here - callback(null, res); +const sig = new SignedXml({ privateKey, signatureAlgorithm, canonicalizationAlgorithm }); +sig.addReference({ xpath, digestAlgorithm, transforms }); + +await sig.computeSignatureAsync(xml); +const signedXml = sig.getSignedXml(); +``` + +`computeSignatureAsync` resolves with the instance, so it can be chained: + +```javascript +const signedXml = (await sig.computeSignatureAsync(xml)).getSignedXml(); +``` + +Only three operations can be asynchronous — hashing, signing, verifying — and they are the +only places the asynchronous flow differs from the synchronous one. Canonicalization, XPath +selection, digest comparison and DOM assembly are shared, so the two entry points produce +byte-identical output. + +### Algorithms that can only work asynchronously + +If the private key is not held locally — a Hardware Security Module, a KMS, a signing server — +or if the backend is the Web Crypto API, whose `crypto.subtle` has no synchronous form, the +algorithm cannot answer synchronously. Implement the `Async` twin instead of the synchronous +method: + +```javascript +function WebCryptoSha256() { + this.getAlgorithmName = function () { + return "http://www.w3.org/2001/04/xmlenc#sha256"; + }; + this.getHashAsync = async function (xml) { + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(xml)); + return Buffer.from(digest).toString("base64"); }; +} + +function RemoteRsaSha256() { this.getAlgorithmName = function () { - return "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; + return "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + }; + this.getSignatureAsync = async function (signedInfo, privateKey) { + return await signingServer.sign(signedInfo); + }; + this.verifySignatureAsync = async function (material, key, signatureValue) { + return await signingServer.verify(material, signatureValue); }; } +``` -var sig = new SignedXml({ signatureAlgorithm: "http://asyncSignatureAlgorithm" }); -sig.SignatureAlgorithms["http://asyncSignatureAlgorithm"] = AsyncSignatureAlgorithm; -sig.signatureAlgorithm = "http://asyncSignatureAlgorithm"; -sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#"; -sig.computeSignature(xml, opts, function (err) { - var signedResponse = sig.getSignedXml(); -}); +Provide whichever forms the backend supports, and no more: + +| Interface | Synchronous | Asynchronous | +| -------------------- | ----------------- | ---------------------- | +| `HashAlgorithm` | `getHash` | `getHashAsync` | +| `SignatureAlgorithm` | `getSignature` | `getSignatureAsync` | +| `SignatureAlgorithm` | `verifySignature` | `verifySignatureAsync` | + +The asynchronous entry points use the synchronous method when only that one exists, so they +accept every algorithm the synchronous entry points do — the bundled `node:crypto` algorithms +included. The reverse cannot work, so reaching an async-only algorithm from `computeSignature` +or `checkSignature` fails immediately and names the entry point to use: + +```text +WebCryptoSha256 is async-only; use computeSignatureAsync() ``` -The function `sig.checkSignature` may also use a callback if asynchronous verification is needed. +An algorithm that implements neither form of an operation is reported the same way, rather than +surfacing as `undefined is not a function` from inside the flow. ## X.509 / Key formats diff --git a/src/signature-algorithms.ts b/src/signature-algorithms.ts index 52e0928..5ec6045 100644 --- a/src/signature-algorithms.ts +++ b/src/signature-algorithms.ts @@ -1,26 +1,22 @@ import * as crypto from "crypto"; -import { type SignatureAlgorithm, createOptionalCallbackFunction } from "./types"; +import type { SignatureAlgorithm } from "./types"; export class RsaSha1 implements SignatureAlgorithm { - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { - const signer = crypto.createSign("RSA-SHA1"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); - - return res; - }, - ); - - verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - const verifier = crypto.createVerify("RSA-SHA1"); - verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); - - return res; - }, - ); + getSignature = (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + const signer = crypto.createSign("RSA-SHA1"); + signer.update(signedInfo); + const res = signer.sign(privateKey, "base64"); + + return res; + }; + + verifySignature = (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + const verifier = crypto.createVerify("RSA-SHA1"); + verifier.update(material); + const res = verifier.verify(key, signatureValue, "base64"); + + return res; + }; getAlgorithmName = () => { return "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; @@ -28,25 +24,21 @@ export class RsaSha1 implements SignatureAlgorithm { } export class RsaSha256 implements SignatureAlgorithm { - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { - const signer = crypto.createSign("RSA-SHA256"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); - - return res; - }, - ); - - verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - const verifier = crypto.createVerify("RSA-SHA256"); - verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); - - return res; - }, - ); + getSignature = (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + const signer = crypto.createSign("RSA-SHA256"); + signer.update(signedInfo); + const res = signer.sign(privateKey, "base64"); + + return res; + }; + + verifySignature = (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + const verifier = crypto.createVerify("RSA-SHA256"); + verifier.update(material); + const res = verifier.verify(key, signatureValue, "base64"); + + return res; + }; getAlgorithmName = () => { return "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; @@ -54,46 +46,42 @@ export class RsaSha256 implements SignatureAlgorithm { } export class RsaSha256Mgf1 implements SignatureAlgorithm { - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { - if (!(typeof privateKey === "string" || Buffer.isBuffer(privateKey))) { - throw new Error("keys must be strings or buffers"); - } - const signer = crypto.createSign("RSA-SHA256"); - signer.update(signedInfo); - const res = signer.sign( - { - key: privateKey, - padding: crypto.constants.RSA_PKCS1_PSS_PADDING, - saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, - }, - "base64", - ); - - return res; - }, - ); - - verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - if (!(typeof key === "string" || Buffer.isBuffer(key))) { - throw new Error("keys must be strings or buffers"); - } - const verifier = crypto.createVerify("RSA-SHA256"); - verifier.update(material); - const res = verifier.verify( - { - key: key, - padding: crypto.constants.RSA_PKCS1_PSS_PADDING, - saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, - }, - signatureValue, - "base64", - ); + getSignature = (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + if (!(typeof privateKey === "string" || Buffer.isBuffer(privateKey))) { + throw new Error("keys must be strings or buffers"); + } + const signer = crypto.createSign("RSA-SHA256"); + signer.update(signedInfo); + const res = signer.sign( + { + key: privateKey, + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, + saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, + }, + "base64", + ); + + return res; + }; - return res; - }, - ); + verifySignature = (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + if (!(typeof key === "string" || Buffer.isBuffer(key))) { + throw new Error("keys must be strings or buffers"); + } + const verifier = crypto.createVerify("RSA-SHA256"); + verifier.update(material); + const res = verifier.verify( + { + key: key, + padding: crypto.constants.RSA_PKCS1_PSS_PADDING, + saltLength: crypto.constants.RSA_PSS_SALTLEN_DIGEST, + }, + signatureValue, + "base64", + ); + + return res; + }; getAlgorithmName = () => { return "http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1"; @@ -101,25 +89,21 @@ export class RsaSha256Mgf1 implements SignatureAlgorithm { } export class RsaSha512 implements SignatureAlgorithm { - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { - const signer = crypto.createSign("RSA-SHA512"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); - - return res; - }, - ); - - verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - const verifier = crypto.createVerify("RSA-SHA512"); - verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); - - return res; - }, - ); + getSignature = (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + const signer = crypto.createSign("RSA-SHA512"); + signer.update(signedInfo); + const res = signer.sign(privateKey, "base64"); + + return res; + }; + + verifySignature = (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + const verifier = crypto.createVerify("RSA-SHA512"); + verifier.update(material); + const res = verifier.verify(key, signatureValue, "base64"); + + return res; + }; getAlgorithmName = () => { return "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512"; @@ -127,35 +111,31 @@ export class RsaSha512 implements SignatureAlgorithm { } export class HmacSha1 implements SignatureAlgorithm { - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { - const signer = crypto.createHmac("SHA1", privateKey); - signer.update(signedInfo); - const res = signer.digest("base64"); - - return res; - }, - ); - - verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - const verifier = crypto.createHmac("SHA1", key); - verifier.update(material); - const res = verifier.digest("base64"); - - // Use constant-time comparison to prevent timing attacks (CWE-208) - // See: https://github.com/node-saml/xml-crypto/issues/522 - try { - return crypto.timingSafeEqual( - Buffer.from(res, "base64"), - Buffer.from(signatureValue, "base64"), - ); - } catch (e) { - // timingSafeEqual throws if buffer lengths don't match - return false; - } - }, - ); + getSignature = (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + const signer = crypto.createHmac("SHA1", privateKey); + signer.update(signedInfo); + const res = signer.digest("base64"); + + return res; + }; + + verifySignature = (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + const verifier = crypto.createHmac("SHA1", key); + verifier.update(material); + const res = verifier.digest("base64"); + + // Use constant-time comparison to prevent timing attacks (CWE-208) + // See: https://github.com/node-saml/xml-crypto/issues/522 + try { + return crypto.timingSafeEqual( + Buffer.from(res, "base64"), + Buffer.from(signatureValue, "base64"), + ); + } catch (e) { + // timingSafeEqual throws if buffer lengths don't match + return false; + } + }; getAlgorithmName = () => { return "http://www.w3.org/2000/09/xmldsig#hmac-sha1"; diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 843a53b..c14f65c 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -4,7 +4,6 @@ import type { CanonicalizationOrTransformationAlgorithm, CanonicalizationOrTransformationAlgorithmProcessOptions, ComputeSignatureOptions, - ErrorFirstCallback, GetKeyInfoContentArgs, HashAlgorithm, HashAlgorithmType, @@ -33,6 +32,87 @@ const warnOriginalXmlWithIds = deprecate( "XML_CRYPTO_GET_ORIGINAL_XML_WITH_IDS", ); +/* + * The signing and verification flows are written once, as synchronous phases separated by the + * only three operations that can be asynchronous: hashing, signing, verifying. An algorithm + * provides the synchronous form of an operation, the asynchronous form, or both, so each + * barrier resolves what the entry point it was reached from is able to use. + * https://github.com/node-saml/xml-crypto/issues/546 + */ + +type AlgorithmLike = { getAlgorithmName(): string }; + +/** Everything the synchronous phases of a signing flow hand to each other. */ +interface SigningContext { + doc: Document; + signatureElem: Element; + prefix?: string; + previousSignatureNode: Node | null; +} + +/** A reference whose canonical form is known but whose digest has not been computed yet. */ +interface PendingDigest { + algorithm: HashAlgorithm; + canonXml: string; + digestValueElem: Element; +} + +/** A loaded reference resolved to the canonical XML its `DigestValue` has to match. */ +interface LocatedReference { + ref: Reference; + algorithm: HashAlgorithm; + canonXml: string; +} + +/** + * The callback overloads were removed in 7.0 in favour of the `*Async` entry points. A + * JavaScript caller that still passes one would otherwise get the work done and a callback that + * never fires, which is the silent break the removal was meant to avoid. + * https://github.com/node-saml/xml-crypto/issues/546 + */ +function rejectRemovedCallback(argument: unknown, asyncEntryPoint: string): void { + if (typeof argument === "function") { + throw new TypeError( + `The callback form was removed in 7.0; use ${asyncEntryPoint}(), which returns a promise`, + ); + } +} + +function applyDigests(pending: PendingDigest[], digests: string[]): void { + pending.forEach((digest, index) => { + digest.digestValueElem.textContent = digests[index]; + }); +} + +function describeAlgorithm(algorithm: AlgorithmLike): string { + const className = algorithm.constructor?.name; + + return className && className !== "Object" ? className : algorithm.getAlgorithmName(); +} + +function notImplemented(algorithm: AlgorithmLike, operation: string): Error { + return new Error( + `${describeAlgorithm(algorithm)} implements neither ${operation}() nor ${operation}Async()`, + ); +} + +/** + * Reaching a missing method as `undefined is not a function` from the middle of a flow tells + * the implementer nothing, so name the entry point that would have worked. + */ +function unavailableSynchronously( + algorithm: AlgorithmLike, + operation: string, + asyncMethod: unknown, + asyncEntryPoint: string, +): Error { + if (typeof asyncMethod !== "function") { + return notImplemented(algorithm, operation); + } + + return new Error(`${describeAlgorithm(algorithm)} is async-only; use ${asyncEntryPoint}()`); +} + export class SignedXml { idMode?: "wssecurity"; idAttributes: string[]; @@ -250,29 +330,75 @@ export class SignedXml { } /** - * Validates the signature of the provided XML document synchronously using the configured key info provider. + * Validates the signature of the provided XML document using the configured key info + * provider. * * @param xml The XML document containing the signature to be validated. * @returns `true` if the signature is valid - * @throws Error if no key info resolver is provided. + * @throws TypeError if a callback is passed — the callback overloads were removed in 7.0 in + * favour of {@link checkSignatureAsync}. + * @throws Error if no key info resolver is provided, if the signature value is incorrect, or + * if a configured algorithm can only work asynchronously — use {@link checkSignatureAsync} + * for that. */ - checkSignature(xml: string): boolean; + checkSignature(xml: string, removedCallback?: never): boolean { + rejectRemovedCallback(removedCallback, "checkSignatureAsync"); + + const { doc, unverifiedSignedInfoCanon } = this.prepareVerification(xml); + + const located = this.locateReferences(doc); + if (located == null) { + return this.rejectUnverifiedReferences(); + } + + const digests = located.map((reference) => + this.hashSync(reference.algorithm, reference.canonXml, "checkSignatureAsync"), + ); + if (!this.acceptReferenceDigests(located, digests)) { + return this.rejectUnverifiedReferences(); + } + + return this.concludeVerification(this.verifySignedInfoSync(unverifiedSignedInfoCanon)); + } + /** - * Validates the signature of the provided XML document synchronously using the configured key info provider. + * Validates the signature of the provided XML document, awaiting any algorithm that can only + * answer asynchronously — Web Crypto, an HSM, a KMS, a signing server. + * + * Algorithms that work synchronously are used as-is, so this entry point accepts every + * algorithm {@link checkSignature} does. * * @param xml The XML document containing the signature to be validated. - * @param callback Callback function to handle the validation result asynchronously. - * @throws Error if the last parameter is provided and is not a function, or if no key info resolver is provided. + * @returns a promise for `true` if the signature is valid + * @throws Error rejects if no key info resolver is provided, if the signature value is + * incorrect, or if a configured algorithm implements neither form of an operation. */ - checkSignature(xml: string, callback: (error: Error | null, isValid?: boolean) => void): void; - checkSignature( - xml: string, - callback?: (error: Error | null, isValid?: boolean) => void, - ): unknown { - if (callback != null && typeof callback !== "function") { - throw new Error("Last parameter must be a callback function"); + async checkSignatureAsync(xml: string): Promise { + const { doc, unverifiedSignedInfoCanon } = this.prepareVerification(xml); + + const located = this.locateReferences(doc); + if (located == null) { + return this.rejectUnverifiedReferences(); + } + + const digests = await Promise.all( + located.map((reference) => this.hashAsync(reference.algorithm, reference.canonXml)), + ); + if (!this.acceptReferenceDigests(located, digests)) { + return this.rejectUnverifiedReferences(); } + return this.concludeVerification(await this.verifySignedInfoAsync(unverifiedSignedInfoCanon)); + } + + /** + * Parses the document and loads the references named by its `SignedInfo`, returning the + * canonical `SignedInfo` whose signature has still to be checked. + */ + private prepareVerification(xml: string): { + doc: Document; + unverifiedSignedInfoCanon: string; + } { this.signedXml = xml; const doc = new xmldom.DOMParser().parseFromString(xml); @@ -282,15 +408,9 @@ export class SignedXml { const unverifiedSignedInfoCanon = this.getCanonSignedInfoXml(doc); if (!unverifiedSignedInfoCanon) { - if (callback) { - callback(new Error("Canonical signed info cannot be empty"), false); - return; - } - throw new Error("Canonical signed info cannot be empty"); } - // unsigned, verify later to keep with consistent callback behavior const parsedUnverifiedSignedInfo = new xmldom.DOMParser().parseFromString( unverifiedSignedInfoCanon, "text/xml", @@ -298,21 +418,11 @@ export class SignedXml { const unverifiedSignedInfoDoc = parsedUnverifiedSignedInfo.documentElement; if (!unverifiedSignedInfoDoc) { - if (callback) { - callback(new Error("Could not parse unverifiedSignedInfoCanon into a document"), false); - return; - } - throw new Error("Could not parse unverifiedSignedInfoCanon into a document"); } const references = utils.findChildren(unverifiedSignedInfoDoc, "Reference"); if (!utils.isArrayHasLength(references)) { - if (callback) { - callback(new Error("could not find any Reference elements"), false); - return; - } - throw new Error("could not find any Reference elements"); } @@ -323,72 +433,76 @@ export class SignedXml { this.loadReference(reference); } - /* eslint-disable-next-line deprecation/deprecation */ - if (!this.getReferences().every((ref) => this.validateReference(ref, doc))) { - /* Trustworthiness can only be determined if SignedInfo's (which holds References' DigestValue(s) - which were validated at this stage) signature is valid. Execution does not proceed to validate - signature phase thus each References' DigestValue must be considered to be untrusted (attacker - might have injected any data with new new references and/or recalculated new DigestValue for - altered Reference(s)). Returning any content via `signedReferences` would give false sense of - trustworthiness if/when SignedInfo's (which holds references' DigestValues) signature is not - valid(ated). Put simply: if one fails, they are all not trustworthy. - */ - this.signedReferences = []; - this.references.forEach((ref) => { - ref.signedReference = undefined; - }); - // TODO: add this breaking change here later on for even more security: `this.references = [];` + return { doc, unverifiedSignedInfoCanon }; + } + + /** + * Resolves every loaded reference to the canonical XML its `DigestValue` has to match, or + * `null` as soon as one cannot be resolved at all — in which case the reference carries the + * reason in `validationError`. + */ + private locateReferences(doc: Document): LocatedReference[] | null { + const located: LocatedReference[] = []; - if (callback) { - callback(new Error("Could not validate all references"), false); - return; + /* eslint-disable-next-line deprecation/deprecation */ + for (const ref of this.getReferences()) { + const canonXml = this.locateReference(ref, doc); + if (canonXml == null) { + return null; } - // We return false because some references validated, but not all - // We should actually be throwing an error here, but that would be a breaking change - // See https://www.w3.org/TR/xmldsig-core/#sec-CoreValidation - return false; + located.push({ + ref, + canonXml, + algorithm: this.findHashAlgorithm(ref.digestAlgorithm), + }); } - // (Stage B authentication step, show that the `signedInfoCanon` is signed) + return located; + } - // First find the key & signature algorithm, these should match - // Stage B: Take the signature algorithm and key and verify the `SignatureValue` against the canonicalized `SignedInfo` - const signer = this.findSignatureAlgorithm(this.signatureAlgorithm); - const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey; - if (key == null) { - throw new Error("KeyInfo or publicCert or privateKey is required to validate signature"); - } + private acceptReferenceDigests(located: LocatedReference[], digests: string[]): boolean { + return located.every((reference, index) => + this.acceptReferenceDigest(reference.ref, reference.canonXml, digests[index]), + ); + } - // Check the signature verification to know whether to reset signature value or not. - const sigRes = signer.verifySignature(unverifiedSignedInfoCanon, key, this.signatureValue); - if (sigRes === true) { - if (callback) { - callback(null, true); - } else { - return true; - } - } else { - // Ideally, we would start by verifying the `signedInfoCanon` first, - // but that may cause some breaking changes, so we'll handle that in v7.x. - // If we were validating `signedInfoCanon` first, we wouldn't have to reset this array. - this.signedReferences = []; - this.references.forEach((ref) => { - ref.signedReference = undefined; - }); - // TODO: add this breaking change here later on for even more security: `this.references = [];` + /* Trustworthiness can only be determined if SignedInfo's (which holds References' DigestValue(s) + which were validated at this stage) signature is valid. Execution does not proceed to validate + signature phase thus each References' DigestValue must be considered to be untrusted (attacker + might have injected any data with new new references and/or recalculated new DigestValue for + altered Reference(s)). Returning any content via `signedReferences` would give false sense of + trustworthiness if/when SignedInfo's (which holds references' DigestValues) signature is not + valid(ated). Put simply: if one fails, they are all not trustworthy. + */ + private discardUnverifiedReferences(): void { + this.signedReferences = []; + this.references.forEach((ref) => { + ref.signedReference = undefined; + }); + // TODO: add this breaking change here later on for even more security: `this.references = [];` + } - if (callback) { - callback( - new Error(`invalid signature: the signature value ${this.signatureValue} is incorrect`), - ); - return; // return early - } else { - throw new Error( - `invalid signature: the signature value ${this.signatureValue} is incorrect`, - ); - } + private rejectUnverifiedReferences(): false { + this.discardUnverifiedReferences(); + + // We return false because some references validated, but not all + // We should actually be throwing an error here, but that would be a breaking change + // See https://www.w3.org/TR/xmldsig-core/#sec-CoreValidation + return false; + } + + private concludeVerification(isValid: boolean): boolean { + if (isValid) { + return true; } + + // Ideally, we would start by verifying the `signedInfoCanon` first, + // but that may cause some breaking changes, so we'll handle that in v7.x. + // If we were validating `signedInfoCanon` first, we wouldn't have to reset this array. + this.discardUnverifiedReferences(); + + throw new Error(`invalid signature: the signature value ${this.signatureValue} is incorrect`); } private getCanonSignedInfoXml(doc: Document) { @@ -449,17 +563,110 @@ export class SignedXml { return this.getCanonXml(ref.transforms, node, c14nOptions); } - private calculateSignatureValue(doc: Document, callback?: ErrorFirstCallback) { - const signedInfoCanon = this.getCanonSignedInfoXml(doc); - const signer = this.findSignatureAlgorithm(this.signatureAlgorithm); + private hashSync(algorithm: HashAlgorithm, canonXml: string, asyncEntryPoint: string): string { + if (typeof algorithm.getHash !== "function") { + throw unavailableSynchronously(algorithm, "getHash", algorithm.getHashAsync, asyncEntryPoint); + } + + return algorithm.getHash(canonXml); + } + + private async hashAsync(algorithm: HashAlgorithm, canonXml: string): Promise { + if (typeof algorithm.getHashAsync === "function") { + return algorithm.getHashAsync(canonXml); + } + if (typeof algorithm.getHash === "function") { + return algorithm.getHash(canonXml); + } + + throw notImplemented(algorithm, "getHash"); + } + + private signingKey(): crypto.KeyLike { if (this.privateKey == null) { throw new Error("Private key is required to compute signature"); } - if (typeof callback === "function") { - signer.getSignature(signedInfoCanon, this.privateKey, callback); - } else { - this.signatureValue = signer.getSignature(signedInfoCanon, this.privateKey); + + return this.privateKey; + } + + /** + * The certificate is taken from `KeyInfo` if `getCertFromKeyInfo` yields one, then + * `publicCert`, then `privateKey` for symmetric signatures. + */ + private verificationKey(): crypto.KeyLike { + const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey; + if (key == null) { + throw new Error("KeyInfo or publicCert or privateKey is required to validate signature"); + } + + return key; + } + + private signSync(signedInfoCanon: string): string { + const algorithm = this.findSignatureAlgorithm(this.signatureAlgorithm); + const privateKey = this.signingKey(); + + if (typeof algorithm.getSignature !== "function") { + throw unavailableSynchronously( + algorithm, + "getSignature", + algorithm.getSignatureAsync, + "computeSignatureAsync", + ); + } + + return algorithm.getSignature(signedInfoCanon, privateKey); + } + + private async signAsync(signedInfoCanon: string): Promise { + const algorithm = this.findSignatureAlgorithm(this.signatureAlgorithm); + const privateKey = this.signingKey(); + + if (typeof algorithm.getSignatureAsync === "function") { + return algorithm.getSignatureAsync(signedInfoCanon, privateKey); + } + if (typeof algorithm.getSignature === "function") { + return algorithm.getSignature(signedInfoCanon, privateKey); } + + throw notImplemented(algorithm, "getSignature"); + } + + /* + * Stage B of core validation. The reference digests only show that the document matches what + * `SignedInfo` claims; this is the step that shows `SignedInfo` itself is signed, which is what + * makes those digests worth anything. + * https://www.w3.org/TR/xmldsig-core/#sec-CoreValidation + */ + private verifySignedInfoSync(unverifiedSignedInfoCanon: string): boolean { + const algorithm = this.findSignatureAlgorithm(this.signatureAlgorithm); + const key = this.verificationKey(); + + if (typeof algorithm.verifySignature !== "function") { + throw unavailableSynchronously( + algorithm, + "verifySignature", + algorithm.verifySignatureAsync, + "checkSignatureAsync", + ); + } + + return algorithm.verifySignature(unverifiedSignedInfoCanon, key, this.signatureValue); + } + + private async verifySignedInfoAsync(unverifiedSignedInfoCanon: string): Promise { + const algorithm = this.findSignatureAlgorithm(this.signatureAlgorithm); + const key = this.verificationKey(); + + if (typeof algorithm.verifySignatureAsync === "function") { + return algorithm.verifySignatureAsync(unverifiedSignedInfoCanon, key, this.signatureValue); + } + if (typeof algorithm.verifySignature === "function") { + return algorithm.verifySignature(unverifiedSignedInfoCanon, key, this.signatureValue); + } + + throw notImplemented(algorithm, "verifySignature"); } private findSignatureAlgorithm(name?: SignatureAlgorithmType) { @@ -494,7 +701,55 @@ export class SignedXml { } } + /** + * Finds the loaded reference whose `DigestValue` matches the given element. + * + * @throws Error if no reference matches, or if the digest algorithm can only work + * asynchronously — use {@link validateElementAgainstReferencesAsync} for that. + */ validateElementAgainstReferences(elemOrXpath: Element | string, doc: Document): Reference { + for (const candidate of this.referenceCandidatesFor(elemOrXpath, doc)) { + const digest = this.hashSync( + candidate.algorithm, + candidate.canonXml, + "validateElementAgainstReferencesAsync", + ); + if (utils.validateDigestValue(digest, candidate.ref.digestValue)) { + return candidate.ref; + } + } + + throw new Error("No references passed validation"); + } + + /** + * Finds the loaded reference whose `DigestValue` matches the given element, awaiting a digest + * algorithm that can only answer asynchronously. + * + * @throws Error rejects if no reference matches. + */ + async validateElementAgainstReferencesAsync( + elemOrXpath: Element | string, + doc: Document, + ): Promise { + for (const candidate of this.referenceCandidatesFor(elemOrXpath, doc)) { + const digest = await this.hashAsync(candidate.algorithm, candidate.canonXml); + if (utils.validateDigestValue(digest, candidate.ref.digestValue)) { + return candidate.ref; + } + } + + throw new Error("No references passed validation"); + } + + /** + * Canonicalizes the element once per loaded reference, lazily, so a caller that finds its + * match on the first reference does no work for the rest. + */ + private *referenceCandidatesFor( + elemOrXpath: Element | string, + doc: Document, + ): Generator { let elem: Element; if (typeof elemOrXpath === "string") { const firstElem = xpath.select1(elemOrXpath, doc); @@ -516,19 +771,19 @@ export class SignedXml { } } - const canonXml = this.getCanonReferenceXml(doc, ref, elem); - const hash = this.findHashAlgorithm(ref.digestAlgorithm); - const digest = hash.getHash(canonXml); - - if (utils.validateDigestValue(digest, ref.digestValue)) { - return ref; - } + yield { + ref, + canonXml: this.getCanonReferenceXml(doc, ref, elem), + algorithm: this.findHashAlgorithm(ref.digestAlgorithm), + }; } - - throw new Error("No references passed validation"); } - private validateReference(ref: Reference, doc: Document) { + /** + * Resolves the element a reference points at and canonicalizes it, or returns `null` and + * records why on the reference. + */ + private locateReference(ref: Reference, doc: Document): string | null { const uri = ref.uri?.[0] === "#" ? ref.uri.substring(1) : ref.uri; let elem: xpath.SelectSingleReturnType = null; @@ -569,25 +824,28 @@ export class SignedXml { }, "`ref.getValidatedNode()` is deprecated and insecure. Use `ref.signedReference` or `this.getSignedReferences()` instead."); if (!isDomNode.isNodeLike(elem)) { - const validationError = new Error( + ref.validationError = new Error( `invalid signature: the signature references an element with uri ${ref.uri} but could not find such element in the xml`, ); - ref.validationError = validationError; - return false; + return null; } - const canonXml = this.getCanonReferenceXml(doc, ref, elem); - const hash = this.findHashAlgorithm(ref.digestAlgorithm); - const digest = hash.getHash(canonXml); + return this.getCanonReferenceXml(doc, ref, elem); + } + /** + * Compares a computed digest against the one the document supplies, and on a match records + * the canonical XML as signed content. + */ + private acceptReferenceDigest(ref: Reference, canonXml: string, digest: string): boolean { if (!utils.validateDigestValue(digest, ref.digestValue)) { - const validationError = new Error( + ref.validationError = new Error( `invalid signature: for uri ${ref.uri} calculated digest is ${digest} but the xml to validate supplies digest ${ref.digestValue}`, ); - ref.validationError = validationError; return false; } + // This step can only be done after we have verified the `signedInfo`. // We verified that they have same hash, // thus the `canonXml` and _only_ the `canonXml` can be trusted. @@ -869,61 +1127,89 @@ export class SignedXml { * Compute the signature of the given XML (using the already defined settings). * * @param xml The XML to compute the signature for. - * @param callback A callback function to handle the signature computation asynchronously. - * @returns void - * @throws TypeError If the xml can not be parsed. + * @param options An object containing options for the signature computation. + * @throws TypeError If the xml can not be parsed, or if a callback is passed — the callback + * overloads were removed in 7.0 in favour of {@link computeSignatureAsync}. + * @throws Error if there were invalid options passed, or if a configured algorithm can only + * work asynchronously — use {@link computeSignatureAsync} for that. */ - computeSignature(xml: string): void; + computeSignature( + xml: string, + options: ComputeSignatureOptions = {}, + removedCallback?: never, + ): void { + rejectRemovedCallback(options, "computeSignatureAsync"); + rejectRemovedCallback(removedCallback, "computeSignatureAsync"); - /** - * Compute the signature of the given XML (using the already defined settings). - * - * @param xml The XML to compute the signature for. - * @param callback A callback function to handle the signature computation asynchronously. - * @returns void - * @throws TypeError If the xml can not be parsed. - */ - computeSignature(xml: string, callback: ErrorFirstCallback): void; + const context = this.prepareSignature(xml, options); - /** - * Compute the signature of the given XML (using the already defined settings). - * - * @param xml The XML to compute the signature for. - * @param opts An object containing options for the signature computation. - * @returns If no callback is provided, returns `this` (the instance of SignedXml). - * @throws TypeError If the xml can not be parsed, or Error if there were invalid options passed. - */ - computeSignature(xml: string, options: ComputeSignatureOptions): void; + try { + const pending = this.collectReferenceDigests(context); + applyDigests( + pending, + pending.map((digest) => + this.hashSync(digest.algorithm, digest.canonXml, "computeSignatureAsync"), + ), + ); + } catch (error) { + // A failure here leaves a half-built `Signature` in the document. + this.signatureNode = context.previousSignatureNode; + throw error; + } + + const signedInfoNode = this.locateSignedInfoToSign(context); + const signatureValue = this.signSync(this.getCanonSignedInfoXml(context.doc)); + + this.finalizeSignature(context, signedInfoNode, signatureValue); + } /** - * Compute the signature of the given XML (using the already defined settings). + * Compute the signature of the given XML, awaiting any algorithm that can only answer + * asynchronously — Web Crypto, an HSM, a KMS, a signing server. + * + * Algorithms that work synchronously are used as-is, so this entry point accepts every + * algorithm {@link computeSignature} does. * * @param xml The XML to compute the signature for. - * @param opts An object containing options for the signature computation. - * @param callback A callback function to handle the signature computation asynchronously. - * @returns void - * @throws TypeError If the xml can not be parsed, or Error if there were invalid options passed. + * @param options An object containing options for the signature computation. + * @returns a promise for this instance, so `getSignedXml()` can be chained off it + * @throws TypeError rejects if the xml can not be parsed. + * @throws Error rejects if there were invalid options passed, or if a configured algorithm + * implements neither form of an operation. */ - computeSignature( + async computeSignatureAsync( xml: string, - options: ComputeSignatureOptions, - callback: ErrorFirstCallback, - ): void; + options: ComputeSignatureOptions = {}, + ): Promise { + const context = this.prepareSignature(xml, options); - computeSignature( - xml: string, - options?: ComputeSignatureOptions | ErrorFirstCallback, - callbackParam?: ErrorFirstCallback, - ): void { - let callback: ErrorFirstCallback; - if (typeof options === "function" && callbackParam == null) { - callback = options as ErrorFirstCallback; - options = {} as ComputeSignatureOptions; - } else { - callback = callbackParam as ErrorFirstCallback; - options = (options ?? {}) as ComputeSignatureOptions; + try { + const pending = this.collectReferenceDigests(context); + applyDigests( + pending, + await Promise.all( + pending.map((digest) => this.hashAsync(digest.algorithm, digest.canonXml)), + ), + ); + } catch (error) { + // A failure here leaves a half-built `Signature` in the document. + this.signatureNode = context.previousSignatureNode; + throw error; } + const signedInfoNode = this.locateSignedInfoToSign(context); + const signatureValue = await this.signAsync(this.getCanonSignedInfoXml(context.doc)); + + this.finalizeSignature(context, signedInfoNode, signatureValue); + + return this; + } + + /** + * Parses the document, gives the referenced elements IDs, and inserts an empty `Signature` + * element at the configured location. + */ + private prepareSignature(xml: string, options: ComputeSignatureOptions): SigningContext { const doc = new xmldom.DOMParser().parseFromString(xml); let xmlNsAttr = "xmlns"; const signatureAttrs: string[] = []; @@ -948,17 +1234,11 @@ export class SignedXml { location.action = location.action || "append"; if (validActions.indexOf(location.action) === -1) { - const err = new Error( + throw new Error( `location.action option has an invalid action: ${ location.action }, must be any of the following values: ${validActions.join(", ")}`, ); - if (!callback) { - throw err; - } else { - callback(err); - return; - } } // Add IDs for all non-self references upfront @@ -1023,15 +1303,9 @@ export class SignedXml { const referenceNode = xpath.select1(location.reference, doc); if (!isDomNode.isNodeLike(referenceNode)) { - const err2 = new Error( + throw new Error( `the following xpath cannot be used because it was not found: ${location.reference}`, ); - if (!callback) { - throw err2; - } else { - callback(err2); - return; - } } if (location.action === "append") { @@ -1056,53 +1330,44 @@ export class SignedXml { const previousSignatureNode = this.signatureNode; this.signatureNode = signatureElem; - try { - this.addAllReferences(doc, signatureElem, prefix); - } catch (error) { - this.signatureNode = previousSignatureNode; - throw error; - } - const signedInfoNodes = utils.findChildren(this.signatureNode, "SignedInfo"); + return { doc, signatureElem, prefix, previousSignatureNode }; + } + + private locateSignedInfoToSign(context: SigningContext): Element { + const signedInfoNodes = utils.findChildren(context.signatureElem, "SignedInfo"); if (signedInfoNodes.length === 0) { - const err3 = new Error("could not find SignedInfo element in the message"); - if (!callback) { - throw err3; - } else { - callback(err3); - return; - } + throw new Error("could not find SignedInfo element in the message"); } - const signedInfoNode = signedInfoNodes[0]; - if (typeof callback === "function") { - // Asynchronous flow - this.calculateSignatureValue(doc, (err, signature) => { - if (err) { - callback(err); - } else { - this.signatureValue = signature || ""; - signatureElem.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling); - this.signatureXml = signatureElem.toString(); - this.signedXml = doc.toString(); - callback(null, this); - } - }); - } else { - // Synchronous flow - this.calculateSignatureValue(doc); - signatureElem.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling); - this.signatureXml = signatureElem.toString(); - this.signedXml = doc.toString(); - } + return signedInfoNodes[0]; + } + + private finalizeSignature( + context: SigningContext, + signedInfoNode: Element, + signatureValue: string, + ): void { + this.signatureValue = signatureValue; + context.signatureElem.insertBefore( + this.createSignature(context.prefix), + signedInfoNode.nextSibling, + ); + this.signatureXml = context.signatureElem.toString(); + this.signedXml = context.doc.toString(); } /** - * Adds all references to the SignedInfo after the signature placeholder is inserted. + * Adds all references to the SignedInfo after the signature placeholder is inserted, leaving + * each `DigestValue` empty. The digest is the one part of this phase an algorithm may only be + * able to produce asynchronously, so the caller computes it and fills them in. */ - private addAllReferences(doc: Document, signatureElem: Element, prefix?: string): void { + private collectReferenceDigests(context: SigningContext): PendingDigest[] { + const { doc, signatureElem, prefix } = context; + const pending: PendingDigest[] = []; + if (!utils.isArrayHasLength(this.references)) { - return; + return pending; } const currentPrefix = prefix ? `${prefix}:` : ""; @@ -1197,7 +1462,6 @@ export class SignedXml { // Get the canonicalized XML const canonXml = this.getCanonReferenceXml(doc, ref, node); - // Get the digest algorithm and compute the digest value const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm); const digestMethodElem = signatureDoc.createElementNS( @@ -1210,7 +1474,7 @@ export class SignedXml { signatureNamespace, `${currentPrefix}DigestValue`, ); - digestValueElem.textContent = digestAlgorithm.getHash(canonXml); + pending.push({ algorithm: digestAlgorithm, canonXml, digestValueElem }); referenceElem.appendChild(transformsElem); referenceElem.appendChild(digestMethodElem); @@ -1220,6 +1484,8 @@ export class SignedXml { signedInfoNode.appendChild(referenceElem); } } + + return pending; } private getKeyInfo(prefix) { diff --git a/src/types.ts b/src/types.ts index 08c4300..c4bf234 100644 --- a/src/types.ts +++ b/src/types.ts @@ -8,8 +8,6 @@ import * as crypto from "crypto"; -export type ErrorFirstCallback = (err: Error | null, result?: T) => void; - export type CanonicalizationAlgorithmType = | "http://www.w3.org/TR/2001/REC-xml-c14n-20010315" | "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments" @@ -169,36 +167,65 @@ export interface CanonicalizationOrTransformationAlgorithm { getAlgorithmName(): CanonicalizationOrTransformAlgorithmType; } -/** Implement this to create a new HashAlgorithm */ +/** + * Implement this to create a new HashAlgorithm. + * + * Provide `getHash`, `getHashAsync`, or both — an implementation backed by `node:crypto` can + * answer synchronously, one backed by `crypto.subtle` cannot. Providing only `getHashAsync` + * makes the algorithm usable from {@link SignedXml.computeSignatureAsync} and + * {@link SignedXml.checkSignatureAsync}, and the synchronous entry points will say so rather + * than failing obscurely. + * + * @see https://github.com/node-saml/xml-crypto/issues/546 + */ export interface HashAlgorithm { getAlgorithmName(): HashAlgorithmType; - getHash(xml: string): string; + getHash?(xml: string): string; + + getHashAsync?(xml: string): Promise; } -/** Extend this to create a new SignatureAlgorithm */ +/** + * Extend this to create a new SignatureAlgorithm. + * + * Each operation comes in a synchronous and an asynchronous form; provide whichever the + * backing implementation can support. A `node:crypto`-backed algorithm implements + * `getSignature` and `verifySignature`; one backed by `crypto.subtle`, an HSM, a KMS or a + * signing server implements `getSignatureAsync` and `verifySignatureAsync` and is used + * through {@link SignedXml.computeSignatureAsync} and {@link SignedXml.checkSignatureAsync}. + * An algorithm that only signs need not implement verification, and vice versa. + * + * @see https://github.com/node-saml/xml-crypto/issues/546 + */ export interface SignatureAlgorithm { /** * Sign the given string using the given key */ - getSignature(signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string; - getSignature( - signedInfo: crypto.BinaryLike, - privateKey: crypto.KeyLike, - callback?: ErrorFirstCallback, - ): void; + getSignature?(signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string; + + /** + * Sign the given string using the given key, resolving when the signature is available + */ + getSignatureAsync?(signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): Promise; + /** * Verify the given signature of the given string using key * * @param key a public cert, public key, or private key can be passed here */ - verifySignature(material: string, key: crypto.KeyLike, signatureValue: string): boolean; - verifySignature( + verifySignature?(material: string, key: crypto.KeyLike, signatureValue: string): boolean; + + /** + * Verify the given signature of the given string using key, resolving with the verdict + * + * @param key a public cert, public key, or private key can be passed here + */ + verifySignatureAsync?( material: string, key: crypto.KeyLike, signatureValue: string, - callback?: ErrorFirstCallback, - ): void; + ): Promise; getAlgorithmName(): SignatureAlgorithmType; } @@ -231,39 +258,3 @@ export interface TransformAlgorithm { * - {@link SignedXml#loadSignature} * - {@link SignedXml#checkSignature} */ - -function isErrorFirstCallback( - possibleCallback: unknown, -): possibleCallback is ErrorFirstCallback { - return typeof possibleCallback === "function"; -} - -/** - * This function will add a callback version of a sync function. - * - * This follows the factory pattern. - * Just call this function, passing the function that you'd like to add a callback version of. - */ -export function createOptionalCallbackFunction( - syncVersion: (...args: A) => T, -): { - (...args: A): T; - (...args: [...A, ErrorFirstCallback]): void; -} { - return ((...args: A | [...A, ErrorFirstCallback]) => { - const possibleCallback = args[args.length - 1]; - if (isErrorFirstCallback(possibleCallback)) { - try { - const result = syncVersion(...(args.slice(0, -1) as A)); - possibleCallback(null, result); - } catch (err) { - possibleCallback(err instanceof Error ? err : new Error("Unknown error")); - } - } else { - return syncVersion(...(args as A)); - } - }) as { - (...args: A): T; - (...args: [...A, ErrorFirstCallback]): void; - }; -} diff --git a/test/async-model-tests.spec.ts b/test/async-model-tests.spec.ts new file mode 100644 index 0000000..09d6ef7 --- /dev/null +++ b/test/async-model-tests.spec.ts @@ -0,0 +1,229 @@ +import * as crypto from "crypto"; +import * as fs from "fs"; +import * as xmldom from "@xmldom/xmldom"; +import * as xpath from "xpath"; +import * as isDomNode from "@xmldom/is-dom-node"; +import { expect } from "chai"; +import { SignedXml } from "../src/index"; + +const RSA_SHA256 = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; +const SHA256 = "http://www.w3.org/2001/04/xmlenc#sha256"; +const EXC_C14N = "http://www.w3.org/2001/10/xml-exc-c14n#"; + +/** Only answers asynchronously, the way `crypto.subtle` or a signing server has to. */ +class AsyncOnlySha256 { + getAlgorithmName = () => SHA256; + getHashAsync = async (xml: string) => + crypto.createHash("sha256").update(xml, "utf8").digest("base64"); +} + +class AsyncOnlyRsaSha256 { + getAlgorithmName = () => RSA_SHA256; + getSignatureAsync = async (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike) => + crypto.createSign("RSA-SHA256").update(signedInfo).sign(privateKey, "base64"); + verifySignatureAsync = async (material: string, key: crypto.KeyLike, signatureValue: string) => + crypto.createVerify("RSA-SHA256").update(material).verify(key, signatureValue, "base64"); +} + +class NeitherFormSha256 { + getAlgorithmName = () => SHA256; +} + +const signatureAlgorithms = [ + "http://www.w3.org/2000/09/xmldsig#rsa-sha1", + RSA_SHA256, + "http://www.w3.org/2007/05/xmldsig-more#sha256-rsa-MGF1", + "http://www.w3.org/2001/04/xmldsig-more#rsa-sha512", +]; + +function signer(signatureAlgorithm: string = RSA_SHA256): SignedXml { + const sig = new SignedXml({ privateKey: fs.readFileSync("./test/static/client.pem") }); + sig.canonicalizationAlgorithm = EXC_C14N; + sig.signatureAlgorithm = signatureAlgorithm; + sig.addReference({ + xpath: "//*[local-name(.)='x']", + digestAlgorithm: SHA256, + transforms: [EXC_C14N], + }); + return sig; +} + +function signedInfoOf(signedXml: string): string { + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const node = xpath.select1("//*[local-name(.)='SignedInfo']", doc); + isDomNode.assertIsNodeLike(node); + return node.toString(); +} + +function verifier(signedXml: string): SignedXml { + const doc = new xmldom.DOMParser().parseFromString(signedXml); + const node = xpath.select1( + "//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']", + doc, + ); + isDomNode.assertIsNodeLike(node); + const sig = new SignedXml({ publicCert: fs.readFileSync("./test/static/client_public.pem") }); + sig.canonicalizationAlgorithm = EXC_C14N; + sig.loadSignature(node); + return sig; +} + +describe("Synchronous and asynchronous entry points", function () { + const xml = ''; + + signatureAlgorithms.forEach((signatureAlgorithm) => { + it(`signs identically through either entry point with ${signatureAlgorithm}`, async function () { + const sync = signer(signatureAlgorithm); + sync.computeSignature(xml); + + const async = signer(signatureAlgorithm); + await async.computeSignatureAsync(xml); + + // RSASSA-PSS salts every signature, so only the material being signed is comparable. + expect(signedInfoOf(async.getSignedXml())).to.equal(signedInfoOf(sync.getSignedXml())); + + expect(verifier(sync.getSignedXml()).checkSignature(sync.getSignedXml())).to.be.true; + expect(verifier(async.getSignedXml()).checkSignature(async.getSignedXml())).to.be.true; + }); + }); + + it("resolves with the instance, so getSignedXml() can be chained", async function () { + const sig = signer(); + expect(await sig.computeSignatureAsync(xml)).to.equal(sig); + }); + + it("verifies a valid signature through either entry point", async function () { + const sig = signer(); + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + expect(verifier(signedXml).checkSignature(signedXml)).to.be.true; + expect(await verifier(signedXml).checkSignatureAsync(signedXml)).to.be.true; + }); + + it("reports a tampered document as invalid through either entry point", async function () { + const sig = signer(); + sig.computeSignature(xml); + const doc = new xmldom.DOMParser().parseFromString(sig.getSignedXml()); + const node = xpath.select1("//*[local-name(.)='x']", doc); + isDomNode.assertIsElementNode(node); + node.setAttribute("attr", "tampered"); + const tampered = new xmldom.XMLSerializer().serializeToString(doc); + + expect(verifier(tampered).checkSignature(tampered)).to.be.false; + expect(await verifier(tampered).checkSignatureAsync(tampered)).to.be.false; + }); + + describe("an async-only algorithm", function () { + it("signs through computeSignatureAsync and verifies against the synchronous path", async function () { + const sig = signer(); + sig.HashAlgorithms[SHA256] = AsyncOnlySha256; + sig.SignatureAlgorithms[RSA_SHA256] = AsyncOnlyRsaSha256; + await sig.computeSignatureAsync(xml); + const signedXml = sig.getSignedXml(); + + expect(verifier(signedXml).checkSignature(signedXml)).to.be.true; + }); + + // Regressed in v6.1.0: `checkSignature` called `verifySignature` in its synchronous + // three-argument form and never passed the callback down, so an async-only verifier + // could not report a valid signature at all. + // https://github.com/node-saml/xml-crypto/issues/546 + it("verifies through checkSignatureAsync", async function () { + const sig = signer(); + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + const check = verifier(signedXml); + check.HashAlgorithms[SHA256] = AsyncOnlySha256; + check.SignatureAlgorithms[RSA_SHA256] = AsyncOnlyRsaSha256; + + expect(await check.checkSignatureAsync(signedXml)).to.be.true; + }); + + it("names the entry point to use when reached from computeSignature", function () { + const sig = signer(); + sig.HashAlgorithms[SHA256] = AsyncOnlySha256; + + expect(() => sig.computeSignature(xml)).to.throw( + "AsyncOnlySha256 is async-only; use computeSignatureAsync()", + ); + }); + + it("names the entry point to use when reached from checkSignature", function () { + const sig = signer(); + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + const check = verifier(signedXml); + check.SignatureAlgorithms[RSA_SHA256] = AsyncOnlyRsaSha256; + + expect(() => check.checkSignature(signedXml)).to.throw( + "AsyncOnlyRsaSha256 is async-only; use checkSignatureAsync()", + ); + }); + + it("leaves no half-built Signature behind when signing fails", function () { + const sig = signer(); + sig.HashAlgorithms[SHA256] = AsyncOnlySha256; + + expect(() => sig.computeSignature(xml)).to.throw(); + expect(() => sig.getSignatureXml()).to.not.throw(); + expect(sig.getSignatureXml()).to.equal(""); + }); + }); + + it("says so when an algorithm implements neither form", async function () { + const sig = signer(); + sig.HashAlgorithms[SHA256] = NeitherFormSha256; + + const message = "NeitherFormSha256 implements neither getHash() nor getHashAsync()"; + expect(() => sig.computeSignature(xml)).to.throw(message); + + const asyncSig = signer(); + asyncSig.HashAlgorithms[SHA256] = NeitherFormSha256; + let rejection: Error | undefined; + try { + await asyncSig.computeSignatureAsync(xml); + } catch (error) { + rejection = error as Error; + } + expect(rejection?.message).to.equal(message); + }); + + describe("the removed callback form", function () { + it("throws from computeSignature rather than never calling back", function () { + expect(() => signer().computeSignature(xml, (() => undefined) as never)).to.throw( + TypeError, + "The callback form was removed in 7.0; use computeSignatureAsync()", + ); + expect(() => signer().computeSignature(xml, {}, (() => undefined) as never)).to.throw( + TypeError, + "The callback form was removed in 7.0; use computeSignatureAsync()", + ); + }); + + it("throws from checkSignature rather than never calling back", function () { + const sig = signer(); + sig.computeSignature(xml); + const signedXml = sig.getSignedXml(); + + expect(() => + verifier(signedXml).checkSignature(signedXml, (() => undefined) as never), + ).to.throw(TypeError, "The callback form was removed in 7.0; use checkSignatureAsync()"); + }); + }); + + it("rejects rather than throwing synchronously for a configuration error", async function () { + const sig = signer(); + sig.signatureAlgorithm = undefined; + + let rejection: Error | undefined; + const promise = sig.computeSignatureAsync(xml).catch((error: Error) => { + rejection = error; + }); + expect(rejection, "should not have thrown before the promise settled").to.be.undefined; + await promise; + expect(rejection?.message).to.equal("signatureAlgorithm is required"); + }); +}); diff --git a/test/signature-unit-tests.spec.ts b/test/signature-unit-tests.spec.ts index 2ce3ced..5990e9f 100644 --- a/test/signature-unit-tests.spec.ts +++ b/test/signature-unit-tests.spec.ts @@ -1,6 +1,6 @@ import * as xpath from "xpath"; import * as xmldom from "@xmldom/xmldom"; -import { SignedXml, createOptionalCallbackFunction } from "../src/index"; +import { SignedXml } from "../src/index"; import * as fs from "fs"; import * as crypto from "crypto"; import { expect } from "chai"; @@ -780,20 +780,17 @@ describe("Signature unit tests", function () { expect(expected, "wrong signature format").to.equal(signedXml); }); - it("signer creates correct signature values using async callback", function () { + it("signer creates correct signature values with an async-only algorithm", async function () { class DummySignatureAlgorithm { verifySignature = function () { return true; }; - getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike) => { - const signer = crypto.createSign("RSA-SHA1"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); - return res; - }, - ); + getSignatureAsync = async (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike) => { + const signer = crypto.createSign("RSA-SHA1"); + signer.update(signedInfo); + return signer.sign(privateKey, "base64"); + }; getAlgorithmName = function () { return "http://www.w3.org/2000/09/xmldsig#rsa-sha1"; @@ -824,41 +821,41 @@ describe("Signature unit tests", function () { }); sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#"; - sig.computeSignature(xml, function () { - const signedXml = sig.getSignedXml(); - const expected = - '' + - '' + - "" + - '' + - '' + - '' + - "" + - '' + - '' + - "b5GCZ2xpP5T7tbLWBTkOl4CYupQ=" + - "" + - '' + - "" + - '' + - "" + - '' + - "4Pq/sBri+AyOtxtSFsPSOyylyzk=" + - "" + - '' + - "" + - '' + - "" + - '' + - "6I7SDu1iV2YOajTlf+iMLIBfLnE=" + - "" + - "" + - "NejzGB9MDUddKCt3GL2vJhEd5q6NBuhLdQc3W4bJI5q34hk7Hk6zBRoW3OliX+/f7Hpi9y0INYoqMSUfrsAVm3IuPzUETKlI6xiNZo07ULRj1DwxRo6cU66ar1EKUQLRuCZas795FjB8jvUI2lyhcax/00uMJ+Cjf4bwAQ+9gOQ=" + - "" + - ""; - - expect(expected, "wrong signature format").to.equal(signedXml); - }); + await sig.computeSignatureAsync(xml); + + const signedXml = sig.getSignedXml(); + const expected = + '' + + '' + + "" + + '' + + '' + + '' + + "" + + '' + + '' + + "b5GCZ2xpP5T7tbLWBTkOl4CYupQ=" + + "" + + '' + + "" + + '' + + "" + + '' + + "4Pq/sBri+AyOtxtSFsPSOyylyzk=" + + "" + + '' + + "" + + '' + + "" + + '' + + "6I7SDu1iV2YOajTlf+iMLIBfLnE=" + + "" + + "" + + "NejzGB9MDUddKCt3GL2vJhEd5q6NBuhLdQc3W4bJI5q34hk7Hk6zBRoW3OliX+/f7Hpi9y0INYoqMSUfrsAVm3IuPzUETKlI6xiNZo07ULRj1DwxRo6cU66ar1EKUQLRuCZas795FjB8jvUI2lyhcax/00uMJ+Cjf4bwAQ+9gOQ=" + + "" + + ""; + + expect(expected, "wrong signature format").to.equal(signedXml); }); describe("verify existing signature", function () {