From 2ce236f7ec665e3016935be87c9943bb9d629b14 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Wed, 9 Sep 2026 16:51:30 -0500 Subject: [PATCH] feat!: decouple public key and data types from node:crypto Every public signature that touched key material named `node:crypto` directly, which hard-coded Node into the type surface: a caller could not pass a `CryptoKey`, the only key representation Web Crypto produces, so the types blocked a Web Crypto backend before any implementation existed. Introduce `BinaryLike` and `KeyLike` in `src/types.ts` and use them for `SignedXmlOptions.privateKey`/`publicCert`, `GetKeyInfoContentArgs.publicCert` and the `SignatureAlgorithm` interface. `SignatureAlgorithm` takes the accepted key type as a parameter rather than declaring the whole union, so each implementation states what it can really use and nothing casts back out. The bundled algorithms declare `crypto.KeyLike | Uint8Array` (`string | Buffer` for MGF1, which needs a key it can put in a `SignPrivateKeyInput`), and a `Uint8Array` is now viewed as a `Buffer` instead of reaching OpenSSL as `ERR_OSSL_UNSUPPORTED`. An `ArrayBuffer` of data is likewise viewed rather than cast, so the widened data type is true for them. The algorithm is looked up by a URI read from the document, so a JavaScript caller can still pair a `CryptoKey` with a Node algorithm without the compiler seeing it. Node answers that by accepting the key through its DEP0203 shim: it signs, the tests pass, and the signature is attributed to a key the algorithm never supported. Reject it explicitly instead. BREAKING CHANGE: implementers of `SignatureAlgorithm` should declare the key type they accept, e.g. `implements SignatureAlgorithm`. The bundled algorithms now throw on key material `node:crypto` cannot use rather than silently accepting a `CryptoKey`. Closes #545 Co-Authored-By: Claude Opus 5 --- README.md | 67 +++++++++++++++++++++-- src/signature-algorithms.ts | 94 +++++++++++++++++++++++---------- src/signed-xml.ts | 6 +-- src/types.ts | 50 ++++++++++++++---- test/key-material-tests.spec.ts | 79 +++++++++++++++++++++++++++ 5 files changed, 249 insertions(+), 47 deletions(-) create mode 100644 test/key-material-tests.spec.ts diff --git a/README.md b/README.md index 891e91c5..e1c070cd 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,23 @@ ## Upgrading +### Upgrading to 7.0 + +Key material and data types no longer name `node:crypto` in the public API. `privateKey`, +`publicCert` and the `SignatureAlgorithm` key arguments are now `KeyLike`, and the data to +be signed is `BinaryLike`; both are exported from this package. + +This is source-compatible for callers. It is a break for anyone who **implements** +`SignatureAlgorithm`: the interface now takes the accepted key type as a parameter, so an +implementation should declare it — `implements SignatureAlgorithm` for a +`node:crypto`-backed one. See +[declaring the key material your algorithm accepts](#declaring-the-key-material-your-algorithm-accepts). + +The bundled algorithms now throw when handed key material `node:crypto` cannot use, instead +of letting a `CryptoKey` through Node's DEP0203 shim as if it were supported. + +### 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. @@ -61,8 +78,8 @@ signature algorithms enabled at same time. When signing a xml document you can pass the following options to the `SignedXml` constructor to customize the signature process: -- `privateKey` - **[required]** a `Buffer` or pem encoded `String` containing your private key -- `publicCert` - **[optional]** a `Buffer` or pem encoded `String` containing your public key +- `privateKey` - **[required]** your private key, as a pem encoded `String`, a `Buffer`, a `Uint8Array`, or a `KeyObject`. Typed [`KeyLike`](#declaring-the-key-material-your-algorithm-accepts), which is wider than any one algorithm accepts +- `publicCert` - **[optional]** your public certificate, in the same forms - `signatureAlgorithm` - **[required]** one of the supported [signature algorithms](#signature-algorithms). Ex: `sign.signatureAlgorithm = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"` - `canonicalizationAlgorithm` - **[required]** one of the supported [canonicalization algorithms](#canonicalization-and-transformation-algorithms). Ex: `sign.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#WithComments"` @@ -139,6 +156,8 @@ When verifying a xml document you can pass the following options to the `SignedX - `publicCert` - **[optional]** your certificate as a string, a string of multiple certs in PEM format, or a Buffer - `privateKey` - **[optional]** your private key as a string or a Buffer - used for verifying symmetrical signatures (HMAC) +Both are typed [`KeyLike`](#declaring-the-key-material-your-algorithm-accepts). + The certificate that will be used to check the signature will first be determined by calling `this.getCertFromKeyInfo()`, which function you can customize as you see fit. If that returns `null`, then `publicCert` is used. If that is `null`, then `privateKey` is used (for symmetrical signing applications). Example: @@ -249,8 +268,8 @@ The `SignedXml` constructor provides an abstraction for sign and verify xml docu - `idMode` - default `null` - if the value of `wssecurity` is passed it will create/validate id's with the ws-security namespace. - `idAttribute` - string - default `Id` or `ID` or `id` - the name of the attribute that contains the id of the element -- `privateKey` - string or Buffer - default `null` - the private key to use for signing -- `publicCert` - string or Buffer - default `null` - the public certificate to use for verifying +- `privateKey` - [`KeyLike`](#declaring-the-key-material-your-algorithm-accepts) - default `null` - the private key to use for signing +- `publicCert` - [`KeyLike`](#declaring-the-key-material-your-algorithm-accepts) - default `null` - the public certificate to use for verifying - `signatureAlgorithm` - string - the signature algorithm to use - `canonicalizationAlgorithm` - string - default `undefined` - the canonicalization algorithm to use - `inclusiveNamespacesPrefixList` - string - default `null` - a list of namespace prefixes to include during canonicalization @@ -338,6 +357,44 @@ function MySignatureAlgorithm() { } ``` +#### Declaring the key material your algorithm accepts + +`privateKey`, `publicCert` and the key arguments to `SignatureAlgorithm` are typed as +`KeyLike`, which is the widest set the library can carry: + +```ts +type KeyLike = crypto.KeyLike | crypto.webcrypto.CryptoKey | Uint8Array; +``` + +That is deliberately wider than any single algorithm can use. `CryptoKey` is the only key +representation the Web Crypto API produces, so it has to be nameable — but nothing built on +`node:crypto` can use one. So `SignatureAlgorithm` takes the key type as a parameter, and an +implementation declares the subset it actually accepts: + +```ts +import * as crypto from "crypto"; +import type { BinaryLike, SignatureAlgorithm } from "xml-crypto"; + +class MySignatureAlgorithm implements SignatureAlgorithm { + getSignature = (signedInfo: BinaryLike, privateKey: crypto.KeyLike): string => { + // `privateKey` is narrowed to what node:crypto takes — no cast needed. + }; + + getAlgorithmName = () => "http://mySigningAlgorithm"; +} +``` + +Declare the narrowest type that works rather than leaving it at `KeyLike`. Leaving it wide +compiles, but it advertises support the implementation does not have, and callers get no +diagnostic when they pair your algorithm with a key it cannot use. Handed a `CryptoKey`, +`node:crypto` does not fail — it accepts it through the +[DEP0203](https://nodejs.org/api/deprecations.html#DEP0203) shim, so a signature appears to +verify against a key the algorithm never really supported. The bundled algorithms reject +non-Node key material with an explicit error for that reason. + +`signedInfo` is typed `BinaryLike` (`crypto.BinaryLike | ArrayBuffer`). `node:crypto` accepts +every arm except a bare `ArrayBuffer`, which `Buffer.from(data)` views without copying. + Custom transformation algorithm. ```javascript @@ -451,7 +508,7 @@ The function `sig.checkSignature` may also use a callback if asynchronous verifi ## X.509 / Key formats -Xml-Crypto internally relies on node's crypto module. This means pem encoded certificates are supported. So to sign an xml use key.pem that looks like this (only the beginning of the key content is shown): +The bundled algorithms are backed by node's crypto module, so pem encoded certificates are supported. To sign an xml use key.pem that looks like this (only the beginning of the key content is shown): ```text -----BEGIN PRIVATE KEY----- diff --git a/src/signature-algorithms.ts b/src/signature-algorithms.ts index 52e09280..3bf33e27 100644 --- a/src/signature-algorithms.ts +++ b/src/signature-algorithms.ts @@ -1,22 +1,58 @@ import * as crypto from "crypto"; -import { type SignatureAlgorithm, createOptionalCallbackFunction } from "./types"; +import { + type BinaryLike, + type KeyLike, + type SignatureAlgorithm, + createOptionalCallbackFunction, +} from "./types"; + +/** + * `node:crypto` takes any `ArrayBufferView` but not a bare `ArrayBuffer`, which is what Web + * Crypto produces. Wrap rather than copy: `Buffer.from` over the three arguments is a view. + */ +function toNodeData(data: BinaryLike): crypto.BinaryLike { + return data instanceof ArrayBuffer ? Buffer.from(data) : data; +} + +/** + * The signature algorithm is looked up by a URI read from the document under inspection, so a + * JavaScript caller can pair any key representation with any algorithm and the compiler never + * sees it. Handed a `CryptoKey`, Node does not fail: it accepts it through the DEP0203 shim, + * so the signature appears to verify against a key this algorithm never really supported. + * Reject it here instead. + * + * @see https://github.com/node-saml/xml-crypto/issues/545 + */ +function toNodeKey(key: KeyLike, algorithmName: string): crypto.KeyLike { + if (typeof key === "string" || Buffer.isBuffer(key) || key instanceof crypto.KeyObject) { + return key; + } + + if (key instanceof Uint8Array) { + return Buffer.from(key.buffer, key.byteOffset, key.byteLength); + } + + throw new Error( + `${algorithmName} needs a key that node:crypto accepts: a string, a Buffer, a Uint8Array, or a KeyObject`, + ); +} -export class RsaSha1 implements SignatureAlgorithm { +export class RsaSha1 implements SignatureAlgorithm { getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + (signedInfo: BinaryLike, privateKey: crypto.KeyLike | Uint8Array): string => { const signer = crypto.createSign("RSA-SHA1"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); + signer.update(toNodeData(signedInfo)); + const res = signer.sign(toNodeKey(privateKey, "RsaSha1"), "base64"); return res; }, ); verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + (material: string, key: crypto.KeyLike | Uint8Array, signatureValue: string): boolean => { const verifier = crypto.createVerify("RSA-SHA1"); verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); + const res = verifier.verify(toNodeKey(key, "RsaSha1"), signatureValue, "base64"); return res; }, @@ -27,22 +63,22 @@ export class RsaSha1 implements SignatureAlgorithm { }; } -export class RsaSha256 implements SignatureAlgorithm { +export class RsaSha256 implements SignatureAlgorithm { getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + (signedInfo: BinaryLike, privateKey: crypto.KeyLike | Uint8Array): string => { const signer = crypto.createSign("RSA-SHA256"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); + signer.update(toNodeData(signedInfo)); + const res = signer.sign(toNodeKey(privateKey, "RsaSha256"), "base64"); return res; }, ); verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + (material: string, key: crypto.KeyLike | Uint8Array, signatureValue: string): boolean => { const verifier = crypto.createVerify("RSA-SHA256"); verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); + const res = verifier.verify(toNodeKey(key, "RsaSha256"), signatureValue, "base64"); return res; }, @@ -53,14 +89,14 @@ export class RsaSha256 implements SignatureAlgorithm { }; } -export class RsaSha256Mgf1 implements SignatureAlgorithm { +export class RsaSha256Mgf1 implements SignatureAlgorithm { getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + (signedInfo: BinaryLike, privateKey: string | Buffer): 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); + signer.update(toNodeData(signedInfo)); const res = signer.sign( { key: privateKey, @@ -75,7 +111,7 @@ export class RsaSha256Mgf1 implements SignatureAlgorithm { ); verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + (material: string, key: string | Buffer, signatureValue: string): boolean => { if (!(typeof key === "string" || Buffer.isBuffer(key))) { throw new Error("keys must be strings or buffers"); } @@ -100,22 +136,22 @@ export class RsaSha256Mgf1 implements SignatureAlgorithm { }; } -export class RsaSha512 implements SignatureAlgorithm { +export class RsaSha512 implements SignatureAlgorithm { getSignature = createOptionalCallbackFunction( - (signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string => { + (signedInfo: BinaryLike, privateKey: crypto.KeyLike | Uint8Array): string => { const signer = crypto.createSign("RSA-SHA512"); - signer.update(signedInfo); - const res = signer.sign(privateKey, "base64"); + signer.update(toNodeData(signedInfo)); + const res = signer.sign(toNodeKey(privateKey, "RsaSha512"), "base64"); return res; }, ); verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { + (material: string, key: crypto.KeyLike | Uint8Array, signatureValue: string): boolean => { const verifier = crypto.createVerify("RSA-SHA512"); verifier.update(material); - const res = verifier.verify(key, signatureValue, "base64"); + const res = verifier.verify(toNodeKey(key, "RsaSha512"), signatureValue, "base64"); return res; }, @@ -126,11 +162,11 @@ export class RsaSha512 implements SignatureAlgorithm { }; } -export class HmacSha1 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); + (signedInfo: BinaryLike, privateKey: crypto.KeyLike | Uint8Array): string => { + const signer = crypto.createHmac("SHA1", toNodeKey(privateKey, "HmacSha1")); + signer.update(toNodeData(signedInfo)); const res = signer.digest("base64"); return res; @@ -138,8 +174,8 @@ export class HmacSha1 implements SignatureAlgorithm { ); verifySignature = createOptionalCallbackFunction( - (material: string, key: crypto.KeyLike, signatureValue: string): boolean => { - const verifier = crypto.createHmac("SHA1", key); + (material: string, key: crypto.KeyLike | Uint8Array, signatureValue: string): boolean => { + const verifier = crypto.createHmac("SHA1", toNodeKey(key, "HmacSha1")); verifier.update(material); const res = verifier.digest("base64"); diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 843a53bf..4159e61d 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -8,6 +8,7 @@ import type { GetKeyInfoContentArgs, HashAlgorithm, HashAlgorithmType, + KeyLike, ObjectAttributes, Reference, SignatureAlgorithm, @@ -17,7 +18,6 @@ import type { import * as isDomNode from "@xmldom/is-dom-node"; import * as xmldom from "@xmldom/xmldom"; -import * as crypto from "crypto"; import { deprecate } from "util"; import * as xpath from "xpath"; import * as c14n from "./c14n-canonicalization"; @@ -39,8 +39,8 @@ export class SignedXml { /** * A {@link Buffer} or pem encoded {@link String} containing your private key */ - privateKey?: crypto.KeyLike; - publicCert?: crypto.KeyLike; + privateKey?: KeyLike; + publicCert?: KeyLike; /** * One of the supported signature algorithms. * @see {@link SignatureAlgorithmType} diff --git a/src/types.ts b/src/types.ts index 08c4300f..b803a34c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,6 +10,28 @@ import * as crypto from "crypto"; export type ErrorFirstCallback = (err: Error | null, result?: T) => void; +/** + * Data that can be signed or verified, expressed without assuming a Node runtime. + * + * `node:crypto` accepts a string or any `ArrayBufferView`. A bare `ArrayBuffer` is included + * here because that is what the Web Crypto API produces; an implementation that ends up in + * `node:crypto` has to view it as a `Buffer` first. + */ +export type BinaryLike = crypto.BinaryLike | ArrayBuffer; + +/** + * Key material a {@link SignatureAlgorithm} may accept, expressed without assuming a Node + * runtime. `CryptoKey` is the only key representation the Web Crypto API produces, so it has + * to be nameable here even though no bundled algorithm can use one. + * + * This is the widest set, not a promise that any given algorithm handles all of it. An + * implementation declares the subset it can actually use — see {@link SignatureAlgorithm}. + * + * The Web Crypto type is spelled `crypto.webcrypto.CryptoKey` so that it resolves without + * `lib.dom`; a global or DOM `CryptoKey` is structurally identical and assignable to it. + */ +export type KeyLike = crypto.KeyLike | crypto.webcrypto.CryptoKey | Uint8Array; + export type CanonicalizationAlgorithmType = | "http://www.w3.org/TR/2001/REC-xml-c14n-20010315" | "http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments" @@ -39,7 +61,7 @@ export type SignatureAlgorithmType = * @param prefix an optional namespace alias to be used for the generated XML */ export interface GetKeyInfoContentArgs { - publicCert?: crypto.KeyLike; + publicCert?: KeyLike; prefix?: string | null; } @@ -64,8 +86,8 @@ export interface ObjectAttributes { export interface SignedXmlOptions { idMode?: "wssecurity"; idAttribute?: string; - privateKey?: crypto.KeyLike; - publicCert?: crypto.KeyLike; + privateKey?: KeyLike; + publicCert?: KeyLike; signatureAlgorithm?: SignatureAlgorithmType; canonicalizationAlgorithm?: CanonicalizationAlgorithmType; inclusiveNamespacesPrefixList?: string | string[]; @@ -176,15 +198,23 @@ export interface HashAlgorithm { getHash(xml: string): string; } -/** Extend this to create a new SignatureAlgorithm */ -export interface SignatureAlgorithm { +/** + * Extend this to create a new SignatureAlgorithm. + * + * `TKey` is the key material the implementation accepts. Declare the narrowest type that + * actually works rather than leaving it at {@link KeyLike}: a `node:crypto`-backed algorithm + * cannot use a Web Crypto `CryptoKey`, and saying `KeyLike` would claim that it can. + * + * @see https://github.com/node-saml/xml-crypto/issues/545 + */ +export interface SignatureAlgorithm { /** * Sign the given string using the given key */ - getSignature(signedInfo: crypto.BinaryLike, privateKey: crypto.KeyLike): string; + getSignature(signedInfo: BinaryLike, privateKey: TKey): string; getSignature( - signedInfo: crypto.BinaryLike, - privateKey: crypto.KeyLike, + signedInfo: BinaryLike, + privateKey: TKey, callback?: ErrorFirstCallback, ): void; /** @@ -192,10 +222,10 @@ export interface SignatureAlgorithm { * * @param key a public cert, public key, or private key can be passed here */ - verifySignature(material: string, key: crypto.KeyLike, signatureValue: string): boolean; + verifySignature(material: string, key: TKey, signatureValue: string): boolean; verifySignature( material: string, - key: crypto.KeyLike, + key: TKey, signatureValue: string, callback?: ErrorFirstCallback, ): void; diff --git a/test/key-material-tests.spec.ts b/test/key-material-tests.spec.ts new file mode 100644 index 00000000..60fb24fa --- /dev/null +++ b/test/key-material-tests.spec.ts @@ -0,0 +1,79 @@ +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, pemToDer } from "../src/index"; + +const RSA_SHA256 = "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"; + +function signerFor(privateKey: crypto.KeyLike | Uint8Array): SignedXml { + const sig = new SignedXml(); + sig.privateKey = privateKey; + sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#"; + sig.signatureAlgorithm = RSA_SHA256; + sig.addReference({ + xpath: "//*[local-name(.)='x']", + digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256", + transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"], + }); + return sig; +} + +function verify(signedXml: string): boolean { + 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(); + sig.publicCert = fs.readFileSync("./test/static/client_public.pem"); + sig.loadSignature(node); + return sig.checkSignature(signedXml); +} + +describe("Key material accepted by the bundled algorithms", function () { + const xml = ''; + + it("signs and verifies with a private key given as a Uint8Array", function () { + const pem = fs.readFileSync("./test/static/client.pem"); + const sig = signerFor(new Uint8Array(pem)); + sig.computeSignature(xml); + + expect(verify(sig.getSignedXml())).to.be.true; + }); + + it("refuses a Web Crypto key rather than signing through the DEP0203 shim", async function () { + const cryptoKey = await crypto.webcrypto.subtle.importKey( + "pkcs8", + pemToDer(fs.readFileSync("./test/static/client.pem", "latin1")), + { name: "RSASSA-PKCS1-v1_5", hash: "SHA-256" }, + false, + ["sign"], + ); + + // Node accepts a CryptoKey here and only emits a deprecation warning, so the signature + // would look valid until the shim is removed and would never work off-Node. + // https://github.com/node-saml/xml-crypto/issues/545 + const sig = signerFor(cryptoKey as unknown as crypto.KeyLike); + + expect(() => sig.computeSignature(xml)).to.throw(/RsaSha256 needs a key that node:crypto/); + }); + + it("signs an ArrayBuffer identically to the equivalent string", function () { + const signatureAlgorithms = new SignedXml().SignatureAlgorithms; + const algorithm = new signatureAlgorithms[RSA_SHA256](); + const privateKey = fs.readFileSync("./test/static/client.pem"); + const signedInfo = ""; + const bytes = Buffer.from(signedInfo, "utf8"); + + expect( + algorithm.getSignature( + bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength), + privateKey, + ), + ).to.equal(algorithm.getSignature(signedInfo, privateKey)); + }); +});