From f19a8345a011046181f09305669b97334d03b5c7 Mon Sep 17 00:00:00 2001 From: Michael Morales Date: Mon, 24 Aug 2026 13:46:29 -0700 Subject: [PATCH] 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 e14e6a9..5d8adbb 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 f209d7a..5b6ef1b 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: "" }); @@ -208,6 +222,83 @@ describe("JWE Crypto", () => { assert.ok(resp[3].length === 10); assert.ok(resp[4].length === 22); }); + + 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("#decryptData()", () => {