From ccb15012a193c9bf1d1b5e43d03208af38ad382e Mon Sep 17 00:00:00 2001 From: Michael Morales Date: Mon, 24 Aug 2026 13:46:29 -0700 Subject: [PATCH 1/2] fix: keep RSA-OAEP-256 digest correct in runtimes that ignore oaepHash --- lib/mcapi/crypto/jwe-crypto.js | 139 +++++++++++++++++++++++++++++---- test/jwe-crypto.test.js | 91 +++++++++++++++++++++ 2 files changed, 214 insertions(+), 16 deletions(-) diff --git a/lib/mcapi/crypto/jwe-crypto.js b/lib/mcapi/crypto/jwe-crypto.js index 78bf509..96f0da6 100644 --- a/lib/mcapi/crypto/jwe-crypto.js +++ b/lib/mcapi/crypto/jwe-crypto.js @@ -26,6 +26,12 @@ function JweCrypto(config) { this.encryptedValueFieldName = config.encryptedValueFieldName; + // Only parsed if this runtime needs the node-forge fallback, so callers on + // node never pay for it. A config may also legitimately carry only one of + // the two keys. + let parsedPublicKey = null; + let parsedPrivateKey = null; + /** * Perform data encryption * @@ -52,14 +58,31 @@ function JweCrypto(config) { const secretKey = nodeCrypto.randomBytes(32); const secretKeyBuffer = Buffer.from(secretKey, c.BINARY); - const encryptedSecretKey = nodeCrypto.publicEncrypt( - { - key: this.encryptionCertificate, - padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, - oaepHash: "sha256", - }, - secretKeyBuffer - ); + let encryptedSecretKey; + if (honoursOaepHash(this.encryptionCertificate)) { + encryptedSecretKey = nodeCrypto.publicEncrypt( + { + key: this.encryptionCertificate, + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: "sha256", + }, + secretKeyBuffer + ); + } else { + if (!parsedPublicKey) { + parsedPublicKey = forge.pki.certificateFromPem( + this.encryptionCertificate + ).publicKey; + } + encryptedSecretKey = Buffer.from( + parsedPublicKey.encrypt( + secretKeyBuffer.toString(c.BINARY), + "RSA-OAEP", + createOAEPOptions() + ), + c.BINARY + ); + } const iv = nodeCrypto.randomBytes(16); @@ -110,14 +133,29 @@ function JweCrypto(config) { const encryptedText = Buffer.from(jweTokenParts[3], c.BASE64); const authTag = Buffer.from(jweTokenParts[4], c.BASE64); - let secretKey = nodeCrypto.privateDecrypt( - { - key: this.privateKey, - padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, - oaepHash: "sha256", - }, - Buffer.from(encryptedSecretKey, c.BINARY) - ); + let secretKey; + if (honoursOaepHash(this.privateKey)) { + secretKey = nodeCrypto.privateDecrypt( + { + key: this.privateKey, + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: "sha256", + }, + Buffer.from(encryptedSecretKey, c.BINARY) + ); + } else { + if (!parsedPrivateKey) { + parsedPrivateKey = forge.pki.privateKeyFromPem(this.privateKey); + } + secretKey = Buffer.from( + parsedPrivateKey.decrypt( + Buffer.from(encryptedSecretKey, c.BINARY).toString(c.BINARY), + "RSA-OAEP", + createOAEPOptions() + ), + c.BINARY + ); + } let decryptionEncoding = JSON.parse(jweHeader).enc; let gcmMode = true; @@ -267,6 +305,75 @@ function validateFingerprint(config, contains) { } } +/** + * Does this runtime's crypto honour the `oaepHash` option? + * + * node validates the digest name and throws on an unrecognised one. The + * crypto-browserify polyfill ignores `oaepHash` altogether and hard-codes + * SHA-1, so it accepts a nonsense value without complaint. Attempting a wrap + * under a bogus digest is therefore a cheap and side-effect free probe. + * + * This matters because a polyfilled `publicEncrypt` reports no error while + * producing a token whose header advertises RSA-OAEP-256 over a key that was + * actually wrapped with SHA-1. The failure only surfaces at the recipient. + * + * Memoised: the answer is a property of the bundle, not of the key. + * + * @private + */ +let nativeOaepHash = null; +function honoursOaepHash(keyPem) { + if (nativeOaepHash !== null) { + return nativeOaepHash; + } + if ( + typeof nodeCrypto.publicEncrypt !== "function" || + !nodeCrypto.constants || + !keyPem + ) { + // `resolve.fallback.crypto = false`, as set in this repo's webpack config, + // leaves an empty module behind. Treat anything unusable as unsupported. + nativeOaepHash = false; + return nativeOaepHash; + } + try { + nodeCrypto.publicEncrypt( + { + key: keyPem, + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: "mcapi-oaep-support-probe", + }, + Buffer.alloc(1) + ); + nativeOaepHash = false; + } catch { + nativeOaepHash = true; + } + return nativeOaepHash; +} + +/** + * Build node-forge OAEP options for RSA-OAEP-256. + * + * The JWE header emitted by this module is always `"alg": "RSA-OAEP-256"`, so + * the label digest and the MGF1 digest are both SHA-256. MGF1 has to be set + * explicitly: node-forge defaults it to SHA-1, which yields a key wrap that no + * conforming RSA-OAEP-256 implementation can undo. + * + * Mirrors createOAEPOptions in field-level-crypto.js. + * + * @private + */ +function createOAEPOptions() { + const md = forge.md.sha256.create(); + return { + md: md, + mgf1: { + md: md, + }, + }; +} + /** * @private */ diff --git a/test/jwe-crypto.test.js b/test/jwe-crypto.test.js index 439d6cf..130a68c 100644 --- a/test/jwe-crypto.test.js +++ b/test/jwe-crypto.test.js @@ -1,5 +1,7 @@ const assert = require("assert"); const rewire = require("rewire"); +const forge = require("node-forge"); +const nodeCrypto = require("crypto"); const Crypto = rewire("../lib/mcapi/crypto/jwe-crypto"); const utils = require("../lib/mcapi/utils/utils"); @@ -167,6 +169,18 @@ describe("JWE Crypto", () => { crypto = new Crypto(testConfig); }); + // Unwrap a JWE encrypted-key segment with an explicit digest, using node + // directly so the assertion does not depend on the code under test. + const unwrap = (encryptedKey, oaepHash) => + nodeCrypto.privateDecrypt( + { + key: forge.pki.privateKeyToPem(utils.getPrivateKey(testConfig)), + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: oaepHash, + }, + encryptedKey + ); + it("with empty string", () => { assert.throws(() => { crypto.encryptData({ data: "" }); @@ -226,6 +240,83 @@ describe("JWE Crypto", () => { }); } }); + + it("still wraps with SHA-256 when the runtime ignores oaepHash", () => { + // Simulate crypto-browserify: publicEncrypt accepts oaepHash and always + // wraps with SHA-1 regardless. On the unpatched module this silently + // produced a token whose header claimed RSA-OAEP-256 over a SHA-1 wrap. + const sha1Only = Object.assign({}, nodeCrypto, { + publicEncrypt: (options, buffer) => + nodeCrypto.publicEncrypt( + { + key: options.key, + padding: options.padding, + oaepHash: "sha1", + }, + buffer + ), + }); + const revertCrypto = Crypto.__set__("nodeCrypto", sha1Only); + const revertProbe = Crypto.__set__("nativeOaepHash", null); + try { + const isolated = new Crypto(testConfig); + const encrypted = isolated.encryptData({ + data: JSON.stringify({ text: "message" }), + }); + const encryptedKey = Buffer.from( + encrypted[testConfig.encryptedValueFieldName].split(".")[1], + "base64" + ); + assert.strictEqual(unwrap(encryptedKey, "sha256").length, 32); + assert.throws(() => unwrap(encryptedKey, "sha1")); + } finally { + revertProbe(); + revertCrypto(); + } + }); + + it("detects whether the runtime honours oaepHash", () => { + const honoursOaepHash = Crypto.__get__("honoursOaepHash"); + const certPem = forge.pki.certificateToPem( + utils.readPublicCertificate(testConfig.encryptionCertificate) + ); + + let reset = Crypto.__set__("nativeOaepHash", null); + assert.strictEqual(honoursOaepHash(certPem), true, "node validates it"); + reset(); + + reset = Crypto.__set__("nativeOaepHash", null); + const revert = Crypto.__set__( + "nodeCrypto", + Object.assign({}, nodeCrypto, { publicEncrypt: () => Buffer.alloc(0) }) + ); + assert.strictEqual( + honoursOaepHash(certPem), + false, + "a polyfill accepts a bogus digest" + ); + revert(); + reset(); + + // An absent crypto module, as webpack's resolve.fallback produces. + reset = Crypto.__set__("nativeOaepHash", null); + const revertEmpty = Crypto.__set__("nodeCrypto", {}); + assert.strictEqual(honoursOaepHash(certPem), false, "empty module"); + revertEmpty(); + reset(); + }); + + it("wraps the content encryption key with RSA-OAEP-256, not SHA-1", () => { + const resp = crypto.encryptData({ + data: JSON.stringify({ text: "message" }), + }); + const encryptedKey = Buffer.from( + resp[testConfig.encryptedValueFieldName].split(".")[1], + "base64" + ); + assert.strictEqual(unwrap(encryptedKey, "sha256").length, 32); + assert.throws(() => unwrap(encryptedKey, "sha1")); + }); }); describe("#toEncodedString()", () => { From 077736d7b74c6141e9f439dccc3814502c062e84 Mon Sep 17 00:00:00 2001 From: Michael Morales Date: Mon, 2 Nov 2026 15:08:14 -0800 Subject: [PATCH 2/2] fix: only cache the OAEP probe result when it is conclusive; cover forge decrypt --- lib/mcapi/crypto/jwe-crypto.js | 32 ++++-- test/jwe-crypto.test.js | 173 ++++++++++++++++++++++++++++----- 2 files changed, 170 insertions(+), 35 deletions(-) diff --git a/lib/mcapi/crypto/jwe-crypto.js b/lib/mcapi/crypto/jwe-crypto.js index 96f0da6..cc74788 100644 --- a/lib/mcapi/crypto/jwe-crypto.js +++ b/lib/mcapi/crypto/jwe-crypto.js @@ -27,8 +27,8 @@ function JweCrypto(config) { this.encryptedValueFieldName = config.encryptedValueFieldName; // Only parsed if this runtime needs the node-forge fallback, so callers on - // node never pay for it. A config may also legitimately carry only one of - // the two keys. + // node never pay for it. A config may also legitimately omit the private + // key. let parsedPublicKey = null; let parsedPrivateKey = null; @@ -134,7 +134,7 @@ function JweCrypto(config) { const authTag = Buffer.from(jweTokenParts[4], c.BASE64); let secretKey; - if (honoursOaepHash(this.privateKey)) { + if (honoursOaepHash(this.encryptionCertificate)) { secretKey = nodeCrypto.privateDecrypt( { key: this.privateKey, @@ -317,19 +317,29 @@ function validateFingerprint(config, contains) { * producing a token whose header advertises RSA-OAEP-256 over a key that was * actually wrapped with SHA-1. The failure only surfaces at the recipient. * - * Memoised: the answer is a property of the bundle, not of the key. + * node parses the key before it resolves the digest, so the probe needs a + * usable key to reach the digest check at all. It takes the encryption + * certificate, which config validation requires, for both directions; a + * decrypt-only caller still has one. + * + * The native path is chosen only on positive evidence: node's own + * invalid-digest error. Any other failure, such as a key the runtime cannot + * parse, says nothing about the runtime. It is not memoised and that call + * takes the node-forge path, which applies SHA-256 regardless. + * + * Memoised once the answer is known: it is a property of the bundle, not of + * the key. * * @private */ let nativeOaepHash = null; -function honoursOaepHash(keyPem) { +function honoursOaepHash(certificatePem) { if (nativeOaepHash !== null) { return nativeOaepHash; } if ( typeof nodeCrypto.publicEncrypt !== "function" || - !nodeCrypto.constants || - !keyPem + !nodeCrypto.constants ) { // `resolve.fallback.crypto = false`, as set in this repo's webpack config, // leaves an empty module behind. Treat anything unusable as unsupported. @@ -339,14 +349,18 @@ function honoursOaepHash(keyPem) { try { nodeCrypto.publicEncrypt( { - key: keyPem, + key: certificatePem, padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash: "mcapi-oaep-support-probe", }, Buffer.alloc(1) ); + // Accepted a digest that does not exist, so it is not using the option. nativeOaepHash = false; - } catch { + } catch (err) { + if (!err || err.code !== "ERR_OSSL_EVP_INVALID_DIGEST") { + return false; + } nativeOaepHash = true; } return nativeOaepHash; diff --git a/test/jwe-crypto.test.js b/test/jwe-crypto.test.js index 130a68c..a1008f7 100644 --- a/test/jwe-crypto.test.js +++ b/test/jwe-crypto.test.js @@ -7,6 +7,27 @@ const utils = require("../lib/mcapi/utils/utils"); const testConfig = require("./mock/jwe-config"); +// Unwrap a JWE encrypted-key segment with an explicit digest, using node +// directly so the assertion does not depend on the code under test. +const unwrap = (encryptedKey, oaepHash) => + nodeCrypto.privateDecrypt( + { + key: forge.pki.privateKeyToPem(utils.getPrivateKey(testConfig)), + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: oaepHash, + }, + encryptedKey + ); + +// Simulate crypto-browserify: publicEncrypt and privateDecrypt accept +// oaepHash and always use SHA-1 regardless. +const forceSha1 = (fn) => (options, buffer) => + fn({ key: options.key, padding: options.padding, oaepHash: "sha1" }, buffer); +const sha1Only = Object.assign({}, nodeCrypto, { + publicEncrypt: forceSha1(nodeCrypto.publicEncrypt), + privateDecrypt: forceSha1(nodeCrypto.privateDecrypt), +}); + describe("JWE Crypto", () => { before(function () { if (!utils.nodeVersionSupportsJWE()) { @@ -169,18 +190,6 @@ describe("JWE Crypto", () => { crypto = new Crypto(testConfig); }); - // Unwrap a JWE encrypted-key segment with an explicit digest, using node - // directly so the assertion does not depend on the code under test. - const unwrap = (encryptedKey, oaepHash) => - nodeCrypto.privateDecrypt( - { - key: forge.pki.privateKeyToPem(utils.getPrivateKey(testConfig)), - padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, - oaepHash: oaepHash, - }, - encryptedKey - ); - it("with empty string", () => { assert.throws(() => { crypto.encryptData({ data: "" }); @@ -242,20 +251,8 @@ describe("JWE Crypto", () => { }); it("still wraps with SHA-256 when the runtime ignores oaepHash", () => { - // Simulate crypto-browserify: publicEncrypt accepts oaepHash and always - // wraps with SHA-1 regardless. On the unpatched module this silently - // produced a token whose header claimed RSA-OAEP-256 over a SHA-1 wrap. - const sha1Only = Object.assign({}, nodeCrypto, { - publicEncrypt: (options, buffer) => - nodeCrypto.publicEncrypt( - { - key: options.key, - padding: options.padding, - oaepHash: "sha1", - }, - buffer - ), - }); + // On the unpatched module this silently produced a token whose header + // claimed RSA-OAEP-256 over a SHA-1 wrap. const revertCrypto = Crypto.__set__("nodeCrypto", sha1Only); const revertProbe = Crypto.__set__("nativeOaepHash", null); try { @@ -306,6 +303,45 @@ describe("JWE Crypto", () => { reset(); }); + it("does not memoise a probe that fails for a reason other than the digest", () => { + const honoursOaepHash = Crypto.__get__("honoursOaepHash"); + const certPem = forge.pki.certificateToPem( + utils.readPublicCertificate(testConfig.encryptionCertificate) + ); + + // Real node: a missing key and an unparseable key both throw before the + // digest is looked at. Neither says anything about the runtime, so they + // take the forge path and leave the answer open. + let reset = Crypto.__set__("nativeOaepHash", null); + assert.strictEqual(honoursOaepHash(null), false, "missing key"); + assert.strictEqual( + honoursOaepHash( + "-----BEGIN CERTIFICATE-----\nAAAA\n-----END CERTIFICATE-----" + ), + false, + "unparseable key" + ); + assert.strictEqual(Crypto.__get__("nativeOaepHash"), null); + assert.strictEqual(honoursOaepHash(certPem), true, "usable key"); + reset(); + + // Anything else that throws, short of node's invalid-digest error, must + // not be read as support for oaepHash. + reset = Crypto.__set__("nativeOaepHash", null); + const revert = Crypto.__set__( + "nodeCrypto", + Object.assign({}, nodeCrypto, { + publicEncrypt: () => { + throw new Error("unrelated"); + }, + }) + ); + assert.strictEqual(honoursOaepHash(certPem), false, "unrelated error"); + assert.strictEqual(Crypto.__get__("nativeOaepHash"), null); + revert(); + reset(); + }); + it("wraps the content encryption key with RSA-OAEP-256, not SHA-1", () => { const resp = crypto.encryptData({ data: JSON.stringify({ text: "message" }), @@ -394,6 +430,91 @@ describe("JWE Crypto", () => { assert.strictEqual(resp, "message"); }); + it("does not let an instance without a private key downgrade the runtime check", () => { + const encryptOnlyConfig = Object.assign({}, testConfig); + delete encryptOnlyConfig.privateKey; + const token = crypto.encryptData({ data: "message" })[ + testConfig.encryptedValueFieldName + ]; + const reset = Crypto.__set__("nativeOaepHash", null); + try { + const encryptOnly = new Crypto(encryptOnlyConfig); + // Same failure as before the forge fallback existed: node rejects the + // missing key. + assert.throws(() => encryptOnly.decryptData(token), { + code: "ERR_INVALID_ARG_TYPE", + }); + assert.strictEqual(Crypto.__get__("nativeOaepHash"), true); + } finally { + reset(); + } + }); + + describe("when the runtime ignores oaepHash", () => { + const certPem = forge.pki.certificateToPem( + utils.readPublicCertificate(testConfig.encryptionCertificate) + ); + let nativeToken; + let revertCrypto; + let revertProbe; + + before(() => { + nativeToken = crypto.encryptData({ data: "message" })[ + testConfig.encryptedValueFieldName + ]; + }); + + // The stubbed privateDecrypt unwraps with SHA-1, so it cannot read any + // of these tokens. A successful decrypt can only have come from forge. + beforeEach(() => { + revertCrypto = Crypto.__set__("nodeCrypto", sha1Only); + revertProbe = Crypto.__set__("nativeOaepHash", null); + }); + + afterEach(() => { + revertProbe(); + revertCrypto(); + }); + + it("decrypts a token wrapped by node crypto", () => { + const isolated = new Crypto(testConfig); + assert.strictEqual(isolated.decryptData(nativeToken), "message"); + assert.strictEqual(Crypto.__get__("nativeOaepHash"), false); + }); + + it("decrypts a token wrapped by node-forge", () => { + const isolated = new Crypto(testConfig); + const token = isolated.encryptData({ data: "message" })[ + testConfig.encryptedValueFieldName + ]; + assert.strictEqual(isolated.decryptData(token), "message"); + }); + + it("rejects a content encryption key wrapped with SHA-1", () => { + const rewrap = (oaepHash) => { + const parts = nativeToken.split("."); + const cek = unwrap(Buffer.from(parts[1], "base64"), "sha256"); + parts[1] = nodeCrypto + .publicEncrypt( + { + key: certPem, + padding: nodeCrypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: oaepHash, + }, + cek + ) + .toString("base64url"); + return parts.join("."); + }; + const isolated = new Crypto(testConfig); + // Control: the rebuilt token is well formed when wrapped correctly. + assert.strictEqual(isolated.decryptData(rewrap("sha256")), "message"); + assert.throws( + () => isolated.decryptData(rewrap("sha1")), + /Invalid RSAES-OAEP padding/ + ); + }); + }); }); describe("#readPublicCertificate", () => {