From 764c40c69da826d27af234c4a9fa89278b22848f Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 16:01:31 -0400 Subject: [PATCH 01/14] fix: resolve XML-Enc elements by DOM traversal instead of XPath decryptKeyInfo located elements with XPath expressions that were built by string concatenation and scoped by a leading "//". Both properties made the lookups behave differently from how they read. "//" is descendant-or-self over the whole document regardless of the context node passed to select(), so lookups that appear scoped were document-wide: - the wrapped key was taken from the first CipherValue anywhere in the document rather than from the EncryptedKey being processed - DigestMethod was resolved by an absolute path, so with several EncryptedKey elements it could return a digest declared by one key while the ciphertext came from another - the absolute DigestMethod path also matched nothing for documents using EncryptedData/KeyInfo/RetrievalMethod, where EncryptedKey sits outside KeyInfo, silently defaulting oaepHash to sha1 Predicates were also assembled by concatenating the RetrievalMethod URI into the expression text, so an Id containing a quote changed what the expression meant instead of being compared as a value. lib/dom-select.js walks the DOM explicitly. Document values are compared, never concatenated into anything that gets parsed, and scope is a node enforced by the walk itself. EncryptionMethod and CipherValue are now read from one resolved EncryptedKey, so they always describe the same key. Two input-validation changes come with this: - a RetrievalMethod URI must be a same-document fragment reference. substring(1) removed the first character whatever it was, so "/ek1" and "Xek1" both resolved to Id "ek1". - a duplicated Id is reported rather than resolved to its first match. Duplicate Ids are invalid XML and resolving them makes the result depend on document order. xpath moves to devDependencies; the runtime no longer parses expressions. Verified with a packed tarball installed via --omit=dev, where require('xpath') throws MODULE_NOT_FOUND and both fixtures still decrypt. 65 passing (44 before). Each property is pinned by a test that also asserts the previous XPath behaviour for contrast, and 11 mutations of the traversal and the guards were each confirmed to fail a named test. Co-Authored-By: Claude Opus 5 --- lib/dom-select.js | 115 ++++++++++++++++ lib/xmlenc.js | 54 +++++--- package-lock.json | 36 ++--- package.json | 6 +- test/dom-select.js | 320 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 485 insertions(+), 46 deletions(-) create mode 100644 lib/dom-select.js create mode 100644 test/dom-select.js diff --git a/lib/dom-select.js b/lib/dom-select.js new file mode 100644 index 0000000..1e9fc6e --- /dev/null +++ b/lib/dom-select.js @@ -0,0 +1,115 @@ +// Explicit DOM traversal for locating XML-Enc elements. +// +// This module deliberately does not use XPath. Two properties of the previous +// XPath lookups made this key-selection path fragile: +// +// 1. Expressions were built from document content. The RetrievalMethod URI +// was spliced into a predicate, so a URI of "#x' or '1'='1" closed the +// quote and changed what the expression meant, matching every +// EncryptedKey in the document instead of comparing an Id. Here a +// document value is only ever compared, never concatenated into +// anything that gets parsed. +// +// 2. Scope was a string, not a node. "//" is descendant-or-self over the +// whole document regardless of the context node handed to select(), so +// lookups that read as relative were silently document-wide, and could +// pair one EncryptedKey's ciphertext with a different one's DigestMethod. +// Here scope is the root argument, and it is enforced by the walk itself. +// +// Only element nodes are candidates; text, comments and PIs never match. + +const ELEMENT_NODE = 1; + +// Element children of node, in document order. +function children(node, localName, namespaceUri) { + const out = []; + if (!node) return out; + for (let n = node.firstChild; n; n = n.nextSibling) { + if (n.nodeType !== ELEMENT_NODE) continue; + if (n.localName !== localName) continue; + if (namespaceUri !== undefined && n.namespaceURI !== namespaceUri) continue; + out.push(n); + } + return out; +} + +function child(node, localName, namespaceUri) { + return children(node, localName, namespaceUri)[0]; +} + +// Pre-order depth-first walk of root's subtree, root included. Pre-order is +// document order, so the first element visited that satisfies a predicate is +// the same element XPath's [0] would have returned. +function walk(root, visit) { + if (!root) return undefined; + const start = root.nodeType === ELEMENT_NODE ? root : root.documentElement; + if (!start) return undefined; + const stack = [start]; + while (stack.length) { + const node = stack.pop(); + const found = visit(node); + if (found !== undefined) return found; + // Push in reverse so the first child is visited first. + const kids = []; + for (let n = node.firstChild; n; n = n.nextSibling) { + if (n.nodeType === ELEMENT_NODE) kids.push(n); + } + for (let i = kids.length - 1; i >= 0; i--) stack.push(kids[i]); + } + return undefined; +} + +// True when node is the tail of localNames, each earlier name being its +// parent, and the head of the chain lies within root's subtree. This is the +// node-wise reading of "A/B/C": C's parent is B, B's parent is A, A anywhere +// under root. +function matchesPath(node, localNames, root) { + let current = node; + for (let i = localNames.length - 1; i >= 0; i--) { + if (!current || current.nodeType !== ELEMENT_NODE) return false; + if (current.localName !== localNames[i]) return false; + if (i > 0) current = current.parentNode; + } + // Confirm the head of the chain is inside the requested scope. + const scope = root.nodeType === ELEMENT_NODE ? root : root.documentElement; + for (let n = current; n; n = n.parentNode) { + if (n === scope) return true; + } + return false; +} + +// First element in root's subtree matching the child-step path, e.g. +// byPath(doc, ['EncryptedData', 'CipherData', 'CipherValue']). +function byPath(root, localNames) { + return walk(root, function (node) { + return matchesPath(node, localNames, root) ? node : undefined; + }); +} + +// First descendant-or-self of root with this name, optionally namespace-qualified. +function descendant(root, localName, namespaceUri) { + return walk(root, function (node) { + if (node.localName !== localName) return undefined; + if (namespaceUri !== undefined && node.namespaceURI !== namespaceUri) return undefined; + return node; + }); +} + +// The single element named localName carrying Id === id. +// +// Duplicate Id values are invalid XML. Resolving to the first of several makes +// the result depend on document order, so ambiguity is refused rather than +// resolved. +function elementById(root, localName, id) { + const matches = []; + walk(root, function (node) { + if (node.localName === localName && node.getAttribute('Id') === id) matches.push(node); + return undefined; // visit every node + }); + if (matches.length > 1) { + throw new Error('multiple ' + localName + ' elements share Id ' + id); + } + return matches[0]; +} + +module.exports = { children, child, byPath, descendant, elementById }; diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 50fa39e..349eec0 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -1,6 +1,6 @@ var crypto = require('crypto'); var xmldom = require('@xmldom/xmldom'); -var xpath = require('xpath'); +var dom = require('./dom-select'); var utils = require('./utils'); const insecureAlgorithms = [ @@ -198,14 +198,14 @@ function decrypt(xml, options, callback) { var doc = typeof xml === 'string' ? new xmldom.DOMParser().parseFromString(xml) : xml; var symmetricKey = decryptKeyInfo(doc, options); - var encryptionMethod = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='EncryptionMethod']", doc)[0]; + var encryptionMethod = dom.byPath(doc, ['EncryptedData', 'EncryptionMethod']); var encryptionAlgorithm = encryptionMethod.getAttribute('Algorithm'); if (options.disallowDecryptionWithInsecureAlgorithm !== false && insecureAlgorithms.indexOf(encryptionAlgorithm) >= 0) { return callback(new Error('encryption algorithm ' + encryptionAlgorithm + ' is not secure, fail to decrypt')); } - var encryptedContent = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='CipherData']/*[local-name(.)='CipherValue']", doc)[0]; + var encryptedContent = dom.byPath(doc, ['EncryptedData', 'CipherData', 'CipherValue']); var encrypted = Buffer.from(encryptedContent.textContent, 'base64'); switch (encryptionAlgorithm) { @@ -233,25 +233,38 @@ function decrypt(xml, options, callback) { function decryptKeyInfo(doc, options) { if (typeof doc === 'string') doc = new xmldom.DOMParser().parseFromString(doc); - var keyRetrievalMethodUri; - var keyInfo = xpath.select("//*[local-name(.)='KeyInfo' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']", doc)[0]; - if (!keyInfo) { - keyInfo = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='KeyInfo']", doc)[0]; + // The EncryptedKey holding the wrapped symmetric key. Either it is nested in + // KeyInfo, or KeyInfo points at it by Id via RetrievalMethod. Resolve the + // element once and read both the EncryptionMethod and the ciphertext from it: + // the two describe the same key, and taking them from one parent is what + // keeps them consistent. + var encryptedKeyElement = dom.byPath(doc, ['KeyInfo', 'EncryptedKey']); + + if (!encryptedKeyElement) { // try with EncryptedData->KeyInfo->RetrievalMethod + var keyRetrievalMethod = dom.byPath(doc, ['EncryptedData', 'KeyInfo', 'RetrievalMethod']); + var keyRetrievalMethodUri = keyRetrievalMethod ? keyRetrievalMethod.getAttribute('URI') : null; + if (keyRetrievalMethodUri) { + // A same-document reference only. An external URL or a bare fragment is + // not something this resolves. + if (keyRetrievalMethodUri.charAt(0) !== '#' || keyRetrievalMethodUri.length < 2) { + throw new Error('RetrievalMethod URI must be a same-document reference'); + } + encryptedKeyElement = dom.elementById(doc, 'EncryptedKey', keyRetrievalMethodUri.substring(1)); + } } - var keyEncryptionMethod = xpath.select("//*[local-name(.)='KeyInfo']/*[local-name(.)='EncryptedKey']/*[local-name(.)='EncryptionMethod']", doc)[0]; - if (!keyEncryptionMethod) { // try with EncryptedData->KeyInfo->RetrievalMethod - var keyRetrievalMethod = xpath.select("//*[local-name(.)='EncryptedData']/*[local-name(.)='KeyInfo']/*[local-name(.)='RetrievalMethod']", doc)[0]; - keyRetrievalMethodUri = keyRetrievalMethod ? keyRetrievalMethod.getAttribute('URI') : null; - keyEncryptionMethod = keyRetrievalMethodUri ? xpath.select("//*[local-name(.)='EncryptedKey' and @Id='" + keyRetrievalMethodUri.substring(1) + "']/*[local-name(.)='EncryptionMethod']", doc)[0] : null; - } + var keyEncryptionMethod = encryptedKeyElement + ? dom.child(encryptedKeyElement, 'EncryptionMethod') + : undefined; if (!keyEncryptionMethod) { throw new Error('cant find encryption algorithm'); } let oaepHash = 'sha1'; - const keyDigestMethod = xpath.select("//*[local-name(.)='KeyInfo']/*[local-name(.)='EncryptedKey']/*[local-name(.)='EncryptionMethod']/*[local-name(.)='DigestMethod']", doc)[0]; + // A direct child of the EncryptionMethod in use. A document-wide lookup can + // return a DigestMethod belonging to a different EncryptedKey. + const keyDigestMethod = dom.child(keyEncryptionMethod, 'DigestMethod'); if (keyDigestMethod) { const keyDigestMethodAlgorithm = keyDigestMethod.getAttribute('Algorithm'); switch (keyDigestMethodAlgorithm) { @@ -271,9 +284,16 @@ function decryptKeyInfo(doc, options) { && insecureAlgorithms.indexOf(keyEncryptionAlgorithm) >= 0) { throw new Error('encryption algorithm ' + keyEncryptionAlgorithm + ' is not secure, fail to decrypt'); } - var encryptedKey = keyRetrievalMethodUri ? - xpath.select("//*[local-name(.)='EncryptedKey' and @Id='" + keyRetrievalMethodUri.substring(1) + "']/*[local-name(.)='CipherData']/*[local-name(.)='CipherValue']", keyInfo)[0] : - xpath.select("//*[local-name(.)='CipherValue']", keyInfo)[0]; + // Scoped to the EncryptedKey the EncryptionMethod came from, so the ciphertext + // and the algorithm used to unwrap it cannot come from different elements. + var encryptedKeyCipherData = dom.child(encryptedKeyElement, 'CipherData'); + var encryptedKey = encryptedKeyCipherData + ? dom.child(encryptedKeyCipherData, 'CipherValue') + : undefined; + + if (!encryptedKey) { + throw new Error('cant find encrypted key'); + } switch (keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': diff --git a/package-lock.json b/package-lock.json index 9c32768..8469131 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,8 +10,7 @@ "license": "MIT", "dependencies": { "@xmldom/xmldom": "^0.8.13", - "escape-html": "^1.0.3", - "xpath": "0.0.32" + "escape-html": "^1.0.3" }, "devDependencies": { "@commitlint/cli": "^20.3.1", @@ -22,7 +21,8 @@ "husky": "^9.1.7", "mocha": "^7.1.2", "semantic-release": "^25.0.2", - "sinon": "^9.0.2" + "sinon": "^9.0.2", + "xpath": "0.0.32" } }, "node_modules/@actions/core": { @@ -713,7 +713,6 @@ "integrity": "sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.3", @@ -1936,7 +1935,6 @@ "integrity": "sha512-zWW5KPngR/yvakJgGOmZ5vTBemDoSqF3AcV/LrO5u5wTWyEAVVh+IT39G4gtyAkh3CtTZs8aX/yRM82OfzHJRg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -2766,7 +2764,6 @@ "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@simple-libs/stream-utils": "^1.2.0", "meow": "^13.0.0" @@ -2845,7 +2842,6 @@ "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=18" } @@ -2895,7 +2891,6 @@ "integrity": "sha512-itvL5h8RETACmOTFc4UfIyB2RfEHi71Ax6E/PivVxq9NseKbOWpeyHEOIbmAw1rs8Ak0VursQNww7lf7YtUwzg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "env-paths": "^2.2.1", "import-fresh": "^3.3.0", @@ -4642,7 +4637,6 @@ "integrity": "sha512-8dD6FusOQSrpv9Z1rdNMdlSgQOIP880DHqnohobOmYLElGEqAL/JvxvuxZO16r4HtjTlfPRDC1hbvxC9dPN2nA==", "dev": true, "license": "MIT", - "peer": true, "bin": { "marked": "bin/marked.js" }, @@ -6956,7 +6950,6 @@ "dev": true, "inBundle": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -7729,7 +7722,6 @@ "integrity": "sha512-6qGjWccl5yoyugHt3jTgztJ9Y0JVzyH8/Voc/D8PlLat9pwxQYXz7W1Dpnq5h0/G5GCYGUaDSlYcyk3AMh5A6g==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@semantic-release/commit-analyzer": "^13.0.1", "@semantic-release/error": "^4.0.0", @@ -8732,7 +8724,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -9050,6 +9041,7 @@ "version": "0.0.32", "resolved": "https://registry.npmjs.org/xpath/-/xpath-0.0.32.tgz", "integrity": "sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw==", + "dev": true, "license": "MIT", "engines": { "node": ">=0.6.0" @@ -9664,7 +9656,6 @@ "resolved": "https://registry.npmjs.org/@octokit/core/-/core-7.0.6.tgz", "integrity": "sha512-DhGl4xMVFGVIyMwswXeyzdL4uXD5OGILGX5N8Y+f6W7LhC1Ze2poSNrkF/fedpVDHEEZ+PHFW0vL14I+mm8K3Q==", "dev": true, - "peer": true, "requires": { "@octokit/auth-token": "^6.0.0", "@octokit/graphql": "^9.0.3", @@ -10437,7 +10428,6 @@ "resolved": "https://registry.npmjs.org/@types/node/-/node-25.0.10.tgz", "integrity": "sha512-zWW5KPngR/yvakJgGOmZ5vTBemDoSqF3AcV/LrO5u5wTWyEAVVh+IT39G4gtyAkh3CtTZs8aX/yRM82OfzHJRg==", "dev": true, - "peer": true, "requires": { "undici-types": "~7.16.0" } @@ -10973,7 +10963,6 @@ "resolved": "https://registry.npmjs.org/conventional-commits-parser/-/conventional-commits-parser-6.4.0.tgz", "integrity": "sha512-tvRg7FIBNlyPzjdG8wWRlPHQJJHI7DylhtRGeU9Lq+JuoPh5BKpPRX83ZdLrvXuOSu5Eo/e7SzOQhU4Hd2Miuw==", "dev": true, - "peer": true, "requires": { "@simple-libs/stream-utils": "^1.2.0", "meow": "^13.0.0" @@ -11074,8 +11063,7 @@ "version": "5.0.0", "resolved": "https://registry.npmjs.org/conventional-commits-filter/-/conventional-commits-filter-5.0.0.tgz", "integrity": "sha512-tQMagCOC59EVgNZcC5zl7XqO30Wki9i9J3acbUvkaosCT6JX3EeFwJD7Qqp4MCikRnzS18WXV3BLIQ66ytu6+Q==", - "dev": true, - "peer": true + "dev": true }, "conventional-commits-parser": { "version": "5.0.0", @@ -11106,7 +11094,6 @@ "resolved": "https://registry.npmjs.org/cosmiconfig/-/cosmiconfig-9.0.0.tgz", "integrity": "sha512-itvL5h8RETACmOTFc4UfIyB2RfEHi71Ax6E/PivVxq9NseKbOWpeyHEOIbmAw1rs8Ak0VursQNww7lf7YtUwzg==", "dev": true, - "peer": true, "requires": { "env-paths": "^2.2.1", "import-fresh": "^3.3.0", @@ -12244,8 +12231,7 @@ "version": "15.0.12", "resolved": "https://registry.npmjs.org/marked/-/marked-15.0.12.tgz", "integrity": "sha512-8dD6FusOQSrpv9Z1rdNMdlSgQOIP880DHqnohobOmYLElGEqAL/JvxvuxZO16r4HtjTlfPRDC1hbvxC9dPN2nA==", - "dev": true, - "peer": true + "dev": true }, "marked-terminal": { "version": "7.3.0", @@ -13769,8 +13755,7 @@ "picomatch": { "version": "4.0.3", "bundled": true, - "dev": true, - "peer": true + "dev": true } } }, @@ -14297,7 +14282,6 @@ "resolved": "https://registry.npmjs.org/semantic-release/-/semantic-release-25.0.2.tgz", "integrity": "sha512-6qGjWccl5yoyugHt3jTgztJ9Y0JVzyH8/Voc/D8PlLat9pwxQYXz7W1Dpnq5h0/G5GCYGUaDSlYcyk3AMh5A6g==", "dev": true, - "peer": true, "requires": { "@semantic-release/commit-analyzer": "^13.0.1", "@semantic-release/error": "^4.0.0", @@ -14942,8 +14926,7 @@ "version": "4.0.3", "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", - "dev": true, - "peer": true + "dev": true } } }, @@ -15154,7 +15137,8 @@ "xpath": { "version": "0.0.32", "resolved": "https://registry.npmjs.org/xpath/-/xpath-0.0.32.tgz", - "integrity": "sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw==" + "integrity": "sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw==", + "dev": true }, "xtend": { "version": "4.0.2", diff --git a/package.json b/package.json index 0506a03..97b8688 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,8 @@ "husky": "^9.1.7", "mocha": "^7.1.2", "semantic-release": "^25.0.2", - "sinon": "^9.0.2" + "sinon": "^9.0.2", + "xpath": "0.0.32" }, "main": "./lib", "repository": "https://github.com/auth0/node-xml-encryption", @@ -27,8 +28,7 @@ "license": "MIT", "dependencies": { "@xmldom/xmldom": "^0.8.13", - "escape-html": "^1.0.3", - "xpath": "0.0.32" + "escape-html": "^1.0.3" }, "files": [ "lib", diff --git a/test/dom-select.js b/test/dom-select.js new file mode 100644 index 0000000..1d84829 --- /dev/null +++ b/test/dom-select.js @@ -0,0 +1,320 @@ +var assert = require('assert'); +var crypto = require('crypto'); +var fs = require('fs'); +var xmldom = require('@xmldom/xmldom'); +var xpath = require('xpath'); +var dom = require('../lib/dom-select'); +var xmlenc = require('../lib/xmlenc'); + +var XENC = 'http://www.w3.org/2001/04/xmlenc#'; +var DSIG = 'http://www.w3.org/2000/09/xmldsig#'; +var SHA256 = XENC + 'sha256'; +var SHA512 = XENC + 'sha512'; +var MGF1P = XENC + 'rsa-oaep-mgf1p'; + +function parse(xml) { + return new xmldom.DOMParser().parseFromString(xml); +} + +// An EncryptedKey wrapping ct, optionally declaring an OAEP DigestMethod. +function encryptedKey(id, digestUri, ct) { + return '' + + '' + + (digestUri ? '' : '') + + '' + + '' + ct.toString('base64') + '' + + ''; +} + +// EncryptedData whose KeyInfo points at an out-of-line EncryptedKey by Id. +function retrievalDoc(targetId, keys) { + return '' + + '' + + keys.join('') + ''; +} + +describe('dom-select', function () { + describe('scoping', function () { + it('confines byPath to the subtree of the root it is given', function () { + // The property XPath's "//" does not have: passing a context node + // actually restricts the search. + var doc = parse( + '' + + 'OUTSIDE' + + 'INSIDE' + + ''); + var b = dom.descendant(doc, 'b'); + + assert.equal(dom.byPath(b, ['target']).textContent, 'INSIDE'); + assert.equal(dom.descendant(b, 'target').textContent, 'INSIDE'); + // Contrast: the XPath this replaced ignored the context node entirely. + assert.equal(xpath.select("//*[local-name(.)='target']", b)[0].textContent, 'OUTSIDE'); + }); + + it('returns children only, never deeper descendants', function () { + var doc = parse(''); + var root = doc.documentElement; + + assert.equal(dom.child(root, 'DigestMethod'), undefined); + assert.equal(dom.descendant(root, 'DigestMethod').getAttribute('Algorithm'), 'deep'); + }); + + it('requires each step of a path to be a parent-child link', function () { + // 'a/c' must not match a c that sits under a/b. + var doc = parse('NESTED'); + assert.equal(dom.byPath(doc, ['a', 'c']), undefined); + assert.equal(dom.byPath(doc, ['a', 'b', 'c']).textContent, 'NESTED'); + }); + + it('matches in document order', function () { + var doc = parse('FIRSTSECOND'); + assert.equal(dom.byPath(doc, ['a', 'x']).textContent, 'FIRST'); + assert.equal(dom.descendant(doc, 'x').textContent, 'FIRST'); + }); + + it('does not climb above the scope to satisfy an earlier path step', function () { + // Scope is 'a'. The path 'x/a/target' matches by name walking parentNode + // up from target -- but that chain reaches x, which sits outside a's + // subtree. Matching it would let a path escape the scope it was given. + var doc = parse('LEAKED'); + var a = dom.descendant(doc, 'a'); + assert.equal(dom.byPath(a, ['x', 'a', 'target']), undefined); + // Every step is in scope when anchored where the chain actually lives. + assert.equal(dom.byPath(doc, ['x', 'a', 'target']).textContent, 'LEAKED'); + assert.equal(dom.byPath(a, ['target']).textContent, 'LEAKED'); + }); + + it('only ever returns element nodes', function () { + // Non-element nodes report localName === null, so a name match cannot + // select them; the nodeType guard is belt-and-braces over that. + var doc = parse('text'); + var el = dom.descendant(doc, 'a'); + assert.equal(el.nodeType, 1); + assert.equal(el.textContent, 'text'); + assert.equal(dom.descendant(doc, '#comment'), undefined); + assert.equal(dom.children(doc.documentElement, null).length, 0); + }); + + it('filters child() on namespace, not just descendant()', function () { + var doc = parse( + '' + + 'WRONG-NS' + + 'RIGHT-NS' + + ''); + var root = doc.documentElement; + assert.equal(dom.child(root, 'DigestMethod', DSIG).textContent, 'RIGHT-NS'); + assert.equal(dom.child(root, 'DigestMethod', XENC).textContent, 'WRONG-NS'); + assert.equal(dom.child(root, 'DigestMethod', 'urn:absent'), undefined); + assert.equal(dom.children(root, 'DigestMethod', DSIG).length, 1); + assert.equal(dom.children(root, 'DigestMethod').length, 2); + }); + + it('filters on namespace when one is given, and ignores prefix', function () { + var doc = parse( + '' + + 'WRONG-NS' + + 'RIGHT-NS' + + ''); + assert.equal(dom.descendant(doc, 'KeyInfo', DSIG).textContent, 'RIGHT-NS'); + assert.equal(dom.descendant(doc, 'KeyInfo', XENC).textContent, 'WRONG-NS'); + // Without a namespace argument, name alone decides. + assert.equal(dom.descendant(doc, 'KeyInfo').textContent, 'WRONG-NS'); + }); + }); + + describe('elementById', function () { + it('finds the element carrying the Id', function () { + var doc = parse( + '' + + 'A' + + 'B' + + ''); + assert.equal(dom.elementById(doc, 'EncryptedKey', 'b').textContent, 'B'); + assert.equal(dom.elementById(doc, 'EncryptedKey', 'nope'), undefined); + }); + + it('requires the element name to match, not the Id alone', function () { + // An Id is only unique within a document, not per element type, so + // resolving on Id alone lets an unrelated element answer for an + // EncryptedKey. + var doc = parse( + '' + + 'DECOY' + + 'REAL' + + ''); + assert.equal(dom.elementById(doc, 'EncryptedKey', 'k1'), undefined); + assert.equal(dom.elementById(doc, 'EncryptedKey', 'k2').textContent, 'REAL'); + }); + + it('treats an Id as an opaque value, not as expression syntax', function () { + // The XPath this replaced built its predicate by string concatenation, so + // an Id containing quotes changed the expression's meaning rather than + // being compared as a value. + var quoted = "x' or '1'='1"; + var doc = parse( + '' + + 'FIRST' + + 'SECOND' + + ''); + + // No element carries that Id, so nothing resolves. + assert.equal(dom.elementById(doc, 'EncryptedKey', quoted), undefined); + + // Contrast: concatenated into an expression, the same value made the + // predicate true for every element and [0] returned an arbitrary one. + var expr = "//*[local-name(.)='EncryptedKey' and @Id='" + quoted + "']"; + var matched = xpath.select(expr, doc); + assert(matched.length > 1, 'a concatenated Id changes the expression'); + assert(matched.indexOf(dom.elementById(doc, 'EncryptedKey', 'first')) >= 0); + }); + + it('refuses a duplicated Id instead of silently picking one', function () { + // Duplicate Id values are invalid XML. Resolving to the first match makes + // the choice depend on document order, so ambiguity is refused instead. + var doc = parse( + '' + + 'FIRST' + + 'SECOND' + + ''); + assert.throws(function () { + dom.elementById(doc, 'EncryptedKey', 'dup'); + }, /share Id dup/); + }); + }); +}); + +describe('decryptKeyInfo element resolution', function () { + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + + // master's decrypt path uses crypto directly; oaepHash follows DigestMethod. + function wrap(symmetricKey, oaepHash) { + return crypto.publicEncrypt({ + key: pub, + padding: crypto.constants.RSA_PKCS1_OAEP_PADDING, + oaepHash: oaepHash + }, symmetricKey); + } + + it('pairs DigestMethod with the EncryptedKey actually in use', function () { + // Two EncryptedKeys under one KeyInfo. The one in use declares no digest + // (so sha1); the other declares sha512. A document-wide DigestMethod lookup + // returns the sha512 belonging to the second, and decryption fails. + var inUse = Buffer.alloc(32, 0x11); + var other = Buffer.alloc(32, 0x22); + var xml = + '' + + encryptedKey(null, null, wrap(inUse, 'sha1')) + + encryptedKey(null, SHA512, wrap(other, 'sha512')) + + ''; + + var recovered = xmlenc.decryptKeyInfo(xml, { key: key }); + assert.equal(Buffer.compare(Buffer.from(recovered), inUse), 0); + + // Contrast: the document-wide lookup returns a digest from a different key. + var doc = parse(xml); + var em = dom.byPath(doc, ['KeyInfo', 'EncryptedKey', 'EncryptionMethod']); + var strayDigest = xpath.select( + "//*[local-name(.)='KeyInfo']/*[local-name(.)='EncryptedKey']/*[local-name(.)='EncryptionMethod']/*[local-name(.)='DigestMethod']", + doc)[0]; + assert.equal(strayDigest.getAttribute('Algorithm'), SHA512); + assert.notEqual(strayDigest.parentNode, em, 'stray digest belongs to a different EncryptionMethod'); + }); + + it('takes the wrapped key from the EncryptedKey, not the first CipherValue in the document', function () { + // EncryptedData's own CipherData precedes KeyInfo. A document-wide + // CipherValue lookup returns the content ciphertext as the wrapped key. + var symmetricKey = Buffer.alloc(32, 0x33); + var xml = + '' + + 'Q09OVEVOVA==' + + '' + encryptedKey(null, SHA256, wrap(symmetricKey, 'sha256')) + '' + + ''; + + var recovered = xmlenc.decryptKeyInfo(xml, { key: key }); + assert.equal(Buffer.compare(Buffer.from(recovered), symmetricKey), 0); + + // Contrast: unscoped, the content ciphertext comes back first. + var doc = parse(xml); + var keyInfo = dom.descendant(doc, 'KeyInfo', DSIG); + assert.equal(xpath.select("//*[local-name(.)='CipherValue']", keyInfo)[0].textContent, 'Q09OVEVOVA=='); + }); + + describe('RetrievalMethod', function () { + it('resolves the referenced key when it is not the first in the document', function () { + var first = Buffer.alloc(32, 0x44); + var target = Buffer.alloc(32, 0x55); + var xml = retrievalDoc('ek2', [ + encryptedKey('ek1', null, wrap(first, 'sha1')), + encryptedKey('ek2', SHA256, wrap(target, 'sha256')) + ]); + var recovered = xmlenc.decryptKeyInfo(xml, { key: key }); + assert.equal(Buffer.compare(Buffer.from(recovered), target), 0); + }); + + it('resolves the referenced key when a decoy follows it', function () { + var target = Buffer.alloc(32, 0x66); + var decoy = Buffer.alloc(32, 0x77); + var xml = retrievalDoc('ek1', [ + encryptedKey('ek1', SHA512, wrap(target, 'sha512')), + encryptedKey('ek2', SHA256, wrap(decoy, 'sha256')) + ]); + var recovered = xmlenc.decryptKeyInfo(xml, { key: key }); + assert.equal(Buffer.compare(Buffer.from(recovered), target), 0); + }); + + it('accepts a well-formed fragment reference', function () { + // Pins the guard's accept side, so a change that rejects everything + // cannot pass by making the rejection test below trivially true. + var symmetricKey = Buffer.alloc(32, 0xDD); + var xml = retrievalDoc('ek1', [encryptedKey('ek1', SHA256, wrap(symmetricKey, 'sha256'))]); + var recovered = xmlenc.decryptKeyInfo(xml, { key: key }); + assert.equal(Buffer.compare(Buffer.from(recovered), symmetricKey), 0); + }); + + it('rejects a URI that is not a same-document reference', function () { + // substring(1) chops the first character whatever it is, so without an + // explicit '#' check "/ek1" and "Xek1" both resolve to Id "ek1" -- a + // non-fragment URI treated as a local one. + ['/ek1', 'Xek1', 'ek1', 'https://example.org/keys#ek1', '#', ''].forEach(function (uri) { + var xml = '' + + '' + + encryptedKey('ek1', null, wrap(Buffer.alloc(32, 0x88), 'sha1')) + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(xml, { key: key }); + }, /same-document reference|cant find encryption algorithm/, 'URI ' + JSON.stringify(uri) + ' must be refused'); + }); + }); + + it('refuses a reference that resolves to more than one EncryptedKey', function () { + var xml = retrievalDoc('dup', [ + encryptedKey('dup', null, wrap(Buffer.alloc(32, 0x99), 'sha1')), + encryptedKey('dup', SHA256, wrap(Buffer.alloc(32, 0xAA), 'sha256')) + ]); + assert.throws(function () { + xmlenc.decryptKeyInfo(xml, { key: key }); + }, /share Id dup/); + }); + + it('does not let an Id change the meaning of resolution', function () { + var xml = '' + + '' + + encryptedKey('first', null, wrap(Buffer.alloc(32, 0xBB), 'sha1')) + + encryptedKey('second', SHA256, wrap(Buffer.alloc(32, 0xCC), 'sha256')) + ''; + // No element has that Id, so resolution finds nothing rather than matching all. + assert.throws(function () { + xmlenc.decryptKeyInfo(xml, { key: key }); + }, /cant find encryption algorithm/); + }); + }); + + it('reports a missing CipherValue rather than throwing on a null read', function () { + var xml = + '' + + '' + + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(xml, { key: key }); + }, /cant find encrypted key/); + }); +}); From 04f4a773d83b2e6acb8dca7896847a93b860c6a9 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 08:41:57 -0400 Subject: [PATCH 02/14] feat: add OAEP encode/decode with independent MGF1 digest Node's crypto cannot set the MGF1 digest separately from the OAEP digest, so implement EME-OAEP over the raw RSA primitive. Verified against OpenSSL-generated ciphertext in both directions. Co-Authored-By: Claude Opus 5 --- lib/oaep.js | 115 +++++++++++++++++++++++++++++++++++++++++++++++++++ test/oaep.js | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+) create mode 100644 lib/oaep.js create mode 100644 test/oaep.js diff --git a/lib/oaep.js b/lib/oaep.js new file mode 100644 index 0000000..5063954 --- /dev/null +++ b/lib/oaep.js @@ -0,0 +1,115 @@ +var crypto = require('crypto'); + +// MGF1 mask generation function (RFC 8017 B.2.1). +function mgf1(seed, length, hash) { + var hLen = crypto.createHash(hash).digest().length; + var out = Buffer.alloc(Math.ceil(length / hLen) * hLen); + var counter = Buffer.alloc(4); + for (var i = 0; i * hLen < length; i++) { + counter.writeUInt32BE(i, 0); + crypto.createHash(hash).update(seed).update(counter).digest().copy(out, i * hLen); + } + return out.subarray(0, length); +} + +function xor(a, b) { + var out = Buffer.allocUnsafe(a.length); + for (var i = 0; i < a.length; i++) { + out[i] = a[i] ^ b[i]; + } + return out; +} + +function decodingError() { + var err = new Error('oaep decoding error'); + err.code = 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; + return err; +} + +// EME-OAEP-DECODE (RFC 8017 7.1.2) with the message digest and the MGF1 digest +// chosen independently. Node's privateDecrypt cannot express that combination: +// it only sets the OAEP digest, and OpenSSL then defaults MGF1 to match it. +function privateDecryptOaep(privateKey, ciphertext, options) { + var opts = options || {}; + var oaepHash = opts.oaepHash || 'sha1'; + var mgf1Hash = opts.mgf1Hash || oaepHash; + var label = opts.oaepLabel || Buffer.alloc(0); + + var em; + try { + em = crypto.privateDecrypt( + { key: privateKey, padding: crypto.constants.RSA_NO_PADDING }, + ciphertext + ); + } catch (e) { + // Random bytes or malformed ciphertext can exceed the modulus and fail + // before OAEP decode even starts. Don't reveal which check failed. + throw decodingError(); + } + + var hLen = crypto.createHash(oaepHash).digest().length; + if (em.length < 2 * hLen + 2) throw decodingError(); + + var maskedSeed = em.subarray(1, 1 + hLen); + var maskedDB = em.subarray(1 + hLen); + var seed = xor(maskedSeed, mgf1(maskedDB, hLen, mgf1Hash)); + var db = xor(maskedDB, mgf1(seed, maskedDB.length, mgf1Hash)); + var lHash = crypto.createHash(oaepHash).update(label).digest(); + + // Accumulate every failure condition, then throw one generic error. Do not + // branch out early and do not report which check failed: the RFC 8017 + // 7.1.2 checks must be indistinguishable from outside. + var bad = em[0] | (crypto.timingSafeEqual(db.subarray(0, hLen), lHash) ? 0 : 1); + var found = 0; + var messageStart = 0; + for (var i = hLen; i < db.length; i++) { + var isOne = (db[i] ^ 0x01) === 0 ? 1 : 0; + var isZero = db[i] === 0 ? 1 : 0; + var first = isOne & (found ^ 1); + messageStart |= first * (i + 1); + found |= isOne; + bad |= (found ^ 1) & (isZero ^ 1); + } + if (bad || !found) throw decodingError(); + + return Buffer.from(db.subarray(messageStart)); +} + +// EME-OAEP-ENCODE (RFC 8017 7.1.1) followed by the raw RSA public operation. +function publicEncryptOaep(publicKey, message, options) { + var opts = options || {}; + var oaepHash = opts.oaepHash || 'sha1'; + var mgf1Hash = opts.mgf1Hash || oaepHash; + var label = opts.oaepLabel || Buffer.alloc(0); + + var key = crypto.createPublicKey(publicKey); + var k = Math.ceil(key.asymmetricKeyDetails.modulusLength / 8); + var hLen = crypto.createHash(oaepHash).digest().length; + var msg = Buffer.isBuffer(message) ? message : Buffer.from(message); + if (msg.length > k - 2 * hLen - 2) { + throw new Error('message too long for the given key size'); + } + + var lHash = crypto.createHash(oaepHash).update(label).digest(); + var db = Buffer.concat([ + lHash, + Buffer.alloc(k - msg.length - 2 * hLen - 2), + Buffer.from([0x01]), + msg + ]); + var seed = crypto.randomBytes(hLen); + var maskedDB = xor(db, mgf1(seed, db.length, mgf1Hash)); + var maskedSeed = xor(seed, mgf1(maskedDB, hLen, mgf1Hash)); + var em = Buffer.concat([Buffer.alloc(1), maskedSeed, maskedDB]); + + return crypto.publicEncrypt( + { key: key, padding: crypto.constants.RSA_NO_PADDING }, + em + ); +} + +module.exports = { + mgf1: mgf1, + publicEncryptOaep: publicEncryptOaep, + privateDecryptOaep: privateDecryptOaep +}; diff --git a/test/oaep.js b/test/oaep.js new file mode 100644 index 0000000..259e585 --- /dev/null +++ b/test/oaep.js @@ -0,0 +1,113 @@ +var assert = require('assert'); +var crypto = require('crypto'); +var oaep = require('../lib/oaep'); + +// Throwaway 2048-bit key + ciphertext from the ESD-63620 repro. Produced by: +// openssl pkeyutl -encrypt -pubin -inkey pub.pem -in key.bin \ +// -pkeyopt rsa_padding_mode:oaep -pkeyopt rsa_oaep_md:sha256 -pkeyopt rsa_mgf1_md:sha1 +var VECTOR_PLAINTEXT = 'AES-128-key-1234'; +var VECTOR_KEY = Buffer.from( + 'LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQzA5QmY1NTRXR0VxRXYKMmZyUWxOUG9ycWRMbld5RVAyTGlybGwvekZuUHFnK0c5RkdxYnNNb3o3UG1CUDRpZlRhVFRtSkViaUx1ajkyWQpQM3FieU9JUmN6MkFZQXJkZlE3M0RiSXhYaUZsazNObjlvVnRISVJUcHVvZkwzc2FkVlozMGg5c3JVeTZ4N0Z1ClZvL0ErNDJlSEVRNEdhaGJOSjJsMlh2QU5ydGQwUG5jNlc2MS9pVTdQK3Z4WDJyM0Fqb2VLMjNTWVRxbTkxRTkKMlN0WmVKQjJuSm4rSGxaamV6WTVUblhCZy9HRmFCZGNvR1JMb1diYzFsV2Q0SHNYa1BuVExyTW5UL0xiV1pQZQo4QVpyU253R1Fpa3dud245ZjF3K01ZQ1h6QU0yNWE4STkrZXRacEl0cFN5VUVtWE9yMnEzRkVkMG5RUUVzMHNFCmkrbENlaW5oQWdNQkFBRUNnZ0VBRUpDdDV6YytIblZ6SXczSjY3Rk1LdWRlTWtwaGhrUEZPaW9xMEV1MVJ4RHkKNWZCVXo0emZPY3UxMU05Tk1ud1M5RzQvQ2JPcFoveHNsVVR1WlBlQlZvYWRzVFJabWtnYUNCek5YTDZZd1JNOApBOTdwL1FDWXpvMmZyaVlyRjFONWpIT0VZKzhEY0svYU90Y2F4dGhnY1FKMmJrcFBBclp3M2g5b09FTHFhUjZTClJyNDgxSUZtS0JNdmhyVUQxVFU0MG5jWG43MTdvazlxalR4bFNuOElONElxSmVMTDFPTkFTMDlNSkhISTZPdG8KbHRZUjNWc1RFdE9YTGNsQ2ZubU5ZT2xpeVgrL1VoMTZBak0rSlJmOFRNL2lDYjNkUGFxMyt4UUFCL1oxRnZPZAo0UEFpa01LNVFTc09jdHhxYThwbm10MUlLK1N4MEZ0aUIweThzbWdYM3dLQmdRRHIyNmxLMlIwbmRpWVZDK3hLCjB2SmxYZ0FZeG9TU1IxOS9EdEFQbDdNMkFVVFRzblVFNmlpSCtDbU1ZK1k5Qm0wblkrNGsveThNUG9sdHZ5OUsKQ1ZVT21Ka0hFY3IvZmo3WHl2OEdkTTJSeXVQOHFhRVZxUlh5WU9PMzM5OUt0NzBFb3FFWVJsS0MxQllXcTVWcQovRExURXphSEowSHFXSk85QjB5eW00cDJId0tCZ1FERWFCdEZGN0NqbG1lMVVjUXRDZ0pqZnZPVWF6UTQ3WGdxClpkNTZ6emcyWm5vejZjYzkvTE40WnV5OC8rcFRYcS9GL3E5RjZtb1pyYktHVDBlNFg4dlVtQVZxd3NmQ01TOGcKTWk4Ui8zeGRZdG54Ly9IbW5DUmpYYTJTOUoraWU1Wks3RHpZWGl0Vi9yamEwRHhvWG9RMm82TG9URXQ1WVNTdQpFY2dEclNkZi93S0JnQXZ5dk1qRjV1d3cyQTBJNVplRXlETEthRWJaQjY1QlgxMFlhd0hmTlh6dTQ0VzE3S2VyCkZSS09SOHlNNHdVRVpsTXdoTWZyQlg4aFMrVDdZbkhsdHlGZUthSnFERmFWRnFubjVyTjFCMVR6YWtsS2JwYWkKVWpKTkpqd1NZMFZ0dVcyYXIzNkRVWHEvTTc5Q1FmZUJmekdpTDRqNVBDV2JCeUQwVmJaV210VVJBb0dBY3ZncAo5bUR1c21QSm8zY1FxZml3KzBNR0hMeEFYbzZMaCs0SHRNWDJOc24wQU1sNUt3endsYXRTS3pSM0c0UlN5a2pTCm1zK2tlaEdXYms2Y1FnNDVoK0hSVWZSZzhJalArRDNJRmZZQys3dHdydHRPNDlwRTVyR2dlR1NmeVlJa3NRam0KZVJWdXNyRWZ6bDZVN2RkZDk0b0VRNHpkcFZpN0d2WW5xaGRDOUVzQ2dZRUFwSkJ1SEpFZXYwd1lxUStnNERzVwpxZTVQYWJOY1dVaDJrNUswMGwza0lmMlpweVdWRWs5bnhRVzJIOFVmNHNVc0d0VTBvMmUycEtUanJ1WDRWcVR5CitoeWxrOXZEam1MU29CenFEREU4bFdOK1llU3hBYzJ0WnE2TnpmT0FPZGdHOVN1ODI0MDlEcUtuR1RlQzlKKzAKWHJ6Z21WOGp5K013cU9kQXJ2QXJ0MXM9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K', + 'base64' +).toString('utf8'); +var VECTOR_CT = Buffer.from( + 'fcAWGHe0HIxC3LcLBwwrkts3005XTSznQZTZZU6EiLOSh/fAfPoe0vF60RcK0IYGW1oDUfuwCl3W+C3HOPTRvFGHiI6AfKCKkj8pTna6WuAZP5x4lBdSKxkIoECgBp+GYko2TMlRn6aW0mOhMCw60P1lT5x93blbbYf4nh0reOtODA8VQBCHnS0wu+qFqIzG/x2UgIbrasnlHo45UlbxdfpOYR08ckKZZrltMZrLcoQnTgrwevwafOg9OvfpY9Kw5Aml+aBhdsabr2aQC5quE6nho0ar/QobPmG5+WzEB5eHn59fTQExDdV2KDcyi7E8xACOjkFFWr+VZmf6t1l59Q==', + 'base64' +); + +describe('oaep', function () { + describe('privateDecryptOaep', function () { + it('decrypts an OpenSSL OAEP(sha256)/MGF1(sha1) ciphertext', function () { + var pt = oaep.privateDecryptOaep(VECTOR_KEY, VECTOR_CT, { + oaepHash: 'sha256', + mgf1Hash: 'sha1' + }); + assert.equal(pt.toString('utf8'), VECTOR_PLAINTEXT); + }); + + it('rejects the same ciphertext when MGF1 is wrong', function () { + assert.throws(function () { + oaep.privateDecryptOaep(VECTOR_KEY, VECTOR_CT, { + oaepHash: 'sha256', + mgf1Hash: 'sha256' + }); + }, /oaep decoding error/); + }); + + it('rejects random bytes', function () { + assert.throws(function () { + oaep.privateDecryptOaep(VECTOR_KEY, crypto.randomBytes(256), { + oaepHash: 'sha256', + mgf1Hash: 'sha1' + }); + }, /oaep decoding error/); + }); + + it('sets code ERR_OSSL_RSA_OAEP_DECODING_ERROR on failure', function () { + try { + oaep.privateDecryptOaep(VECTOR_KEY, crypto.randomBytes(256), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + assert.fail('should have thrown'); + } catch (e) { + assert.equal(e.code, 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'); + } + }); + }); + + describe('mgf1', function () { + it('matches the RFC 8017 counter construction for one block', function () { + var seed = Buffer.from('abc'); + var ctr = Buffer.alloc(4); // i = 0 + var expected = crypto.createHash('sha1').update(seed).update(ctr).digest(); + assert.equal(oaep.mgf1(seed, 20, 'sha1').toString('hex'), expected.toString('hex')); + }); + + it('spans multiple blocks and truncates to the requested length', function () { + var out = oaep.mgf1(Buffer.from('seed'), 50, 'sha1'); + assert.equal(out.length, 50); + }); + }); + + describe('round trips', function () { + var fs = require('fs'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + var combos = [ + ['sha256', 'sha1'], + ['sha512', 'sha1'], + ['sha1', 'sha256'], + ['sha384', 'sha256'], + ['sha256', 'sha256'] + ]; + + combos.forEach(function (combo) { + var oaepHash = combo[0]; + var mgf1Hash = combo[1]; + // 2048-bit key => k = 256 bytes; longest legal message is k - 2*hLen - 2. + var hLen = crypto.createHash(oaepHash).digest().length; + [0, 1, 17, 256 - 2 * hLen - 2].forEach(function (len) { + it('round trips oaep=' + oaepHash + ' mgf1=' + mgf1Hash + ' len=' + len, function () { + var msg = crypto.randomBytes(len); + var ct = oaep.publicEncryptOaep(pub, msg, { oaepHash: oaepHash, mgf1Hash: mgf1Hash }); + var pt = oaep.privateDecryptOaep(key, ct, { oaepHash: oaepHash, mgf1Hash: mgf1Hash }); + assert.equal(Buffer.compare(pt, msg), 0); + }); + }); + }); + + it('rejects a message longer than the key allows', function () { + assert.throws(function () { + oaep.publicEncryptOaep(pub, crypto.randomBytes(256), { oaepHash: 'sha256' }); + }, /message too long/); + }); + + it('round trips a non-empty oaepLabel and rejects the wrong label', function () { + var label = Buffer.from('MYLABEL'); + var ct = oaep.publicEncryptOaep(pub, Buffer.from('labelled'), { oaepHash: 'sha256', mgf1Hash: 'sha1', oaepLabel: label }); + var pt = oaep.privateDecryptOaep(key, ct, { oaepHash: 'sha256', mgf1Hash: 'sha1', oaepLabel: label }); + assert.equal(pt.toString(), 'labelled'); + assert.throws(function () { + oaep.privateDecryptOaep(key, ct, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, /oaep decoding error/); + }); + }); +}); From 08472e56cb664e82c48369f07767818e8efdea80 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 09:02:37 -0400 Subject: [PATCH 03/14] fix(oaep): hoist key parsing, pin rejection guards, use branch-free isZero/isOne MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses code-review findings for Task 1: Finding 1 (Important): Hoist key parsing and ciphertext-length validation above the try-catch so operational failures (bad PEM, wrong key type) surface immediately rather than masquerading as OAEP decode errors. Only the modular exponentiation remains masked so that the RFC 8017 7.1.2 checks stay indistinguishable from outside. Finding 2 (Important): Add four tests that pin each RFC 8017 §7.1.2 rejection guard (leading byte, lHash mismatch, no separator, non-zero PS). Mutation testing confirms each guard is now covered. Minor: Replace isZero/isOne ternaries with branch-free arithmetic form to eliminate data-dependent branching in the separator scan. Minor: Derive k from the fixture key instead of hardcoding 256, so max- length test cases remain correct if the key changes. All 77 tests passing. OpenSSL interop verified. Co-Authored-By: Claude Opus 5 --- lib/oaep.js | 17 ++++++--- test/oaep.js | 102 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/lib/oaep.js b/lib/oaep.js index 5063954..78d3659 100644 --- a/lib/oaep.js +++ b/lib/oaep.js @@ -35,15 +35,22 @@ function privateDecryptOaep(privateKey, ciphertext, options) { var mgf1Hash = opts.mgf1Hash || oaepHash; var label = opts.oaepLabel || Buffer.alloc(0); + // Parse key before masking errors — operational failures (bad PEM, public-key-for-private) + // must surface, not masquerade as OAEP decode failures. + var keyObj = crypto.createPrivateKey(privateKey); + var k = Math.ceil(keyObj.asymmetricKeyDetails.modulusLength / 8); + // Ciphertext length comes from the document, so keep the failure generic. + if (ciphertext.length !== k) throw decodingError(); + var em; try { em = crypto.privateDecrypt( - { key: privateKey, padding: crypto.constants.RSA_NO_PADDING }, + { key: keyObj, padding: crypto.constants.RSA_NO_PADDING }, ciphertext ); } catch (e) { - // Random bytes or malformed ciphertext can exceed the modulus and fail - // before OAEP decode even starts. Don't reveal which check failed. + // Ciphertext ≥ modulus fails before OAEP decode starts. Also from the + // document, so stay generic. throw decodingError(); } @@ -63,8 +70,8 @@ function privateDecryptOaep(privateKey, ciphertext, options) { var found = 0; var messageStart = 0; for (var i = hLen; i < db.length; i++) { - var isOne = (db[i] ^ 0x01) === 0 ? 1 : 0; - var isZero = db[i] === 0 ? 1 : 0; + var isZero = ((db[i] - 1) >>> 31) & 1; + var isOne = (((db[i] ^ 1) - 1) >>> 31) & 1; var first = isOne & (found ^ 1); messageStart |= first * (i + 1); found |= isOne; diff --git a/test/oaep.js b/test/oaep.js index 259e585..4941f46 100644 --- a/test/oaep.js +++ b/test/oaep.js @@ -51,6 +51,101 @@ describe('oaep', function () { assert.equal(e.code, 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'); } }); + + // RFC 8017 §7.1.2 rejection guards: encrypt a valid message, corrupt one thing, decrypt. + it('rejects when leading byte is not 0x00', function () { + var fs = require('fs'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + var ct = oaep.publicEncryptOaep(pub, Buffer.from('test'), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + // Decrypt to EM, corrupt leading byte, re-encrypt. + var em = crypto.privateDecrypt({ key: key, padding: crypto.constants.RSA_NO_PADDING }, ct); + em[0] = 0x01; + var badCt = crypto.publicEncrypt({ key: pub, padding: crypto.constants.RSA_NO_PADDING }, em); + assert.throws(function () { + oaep.privateDecryptOaep(key, badCt, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, function (e) { return e.code === 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; }); + }); + + it('rejects when lHash does not match', function () { + var fs = require('fs'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + var ct = oaep.publicEncryptOaep(pub, Buffer.from('test'), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var em = crypto.privateDecrypt({ key: key, padding: crypto.constants.RSA_NO_PADDING }, ct); + // Corrupt a byte in the lHash region (db[0..31] after unmasking). Flip bit in maskedDB[0]. + var hLen = 32; // SHA-256 + var maskedDB = em.subarray(1 + hLen); + maskedDB[0] ^= 1; + var badCt = crypto.publicEncrypt({ key: pub, padding: crypto.constants.RSA_NO_PADDING }, em); + assert.throws(function () { + oaep.privateDecryptOaep(key, badCt, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, function (e) { return e.code === 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; }); + }); + + it('rejects when no 0x01 separator is found', function () { + var fs = require('fs'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + // Craft an EM where db = lHash || all-zeros (no separator, no message). + var hLen = 32; + var keyObj = crypto.createPublicKey(pub); + var k = Math.ceil(keyObj.asymmetricKeyDetails.modulusLength / 8); + var lHash = crypto.createHash('sha256').digest(); + var db = Buffer.concat([lHash, Buffer.alloc(k - 1 - hLen - hLen)]); + var seed = crypto.randomBytes(hLen); + var maskedDB = oaep.mgf1(seed, db.length, 'sha1'); + for (var i = 0; i < db.length; i++) maskedDB[i] ^= db[i]; + var maskedSeed = oaep.mgf1(maskedDB, hLen, 'sha1'); + for (var j = 0; j < seed.length; j++) maskedSeed[j] ^= seed[j]; + var em = Buffer.concat([Buffer.from([0x00]), maskedSeed, maskedDB]); + var badCt = crypto.publicEncrypt({ key: pub, padding: crypto.constants.RSA_NO_PADDING }, em); + assert.throws(function () { + oaep.privateDecryptOaep(key, badCt, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, function (e) { return e.code === 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; }); + }); + + it('rejects a too-short ciphertext', function () { + var fs = require('fs'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + // Ciphertext length must equal k (modulus size in bytes). A shorter ciphertext + // is caught by the ciphertext-length check before raw RSA decrypt. + var keyObj = crypto.createPrivateKey(key); + var k = Math.ceil(keyObj.asymmetricKeyDetails.modulusLength / 8); + var shortCt = Buffer.alloc(k - 1); + assert.throws(function () { + oaep.privateDecryptOaep(key, shortCt, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, function (e) { return e.code === 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; }); + }); + + it('rejects when PS contains non-zero bytes before the separator', function () { + var fs = require('fs'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var key = fs.readFileSync(__dirname + '/test-auth0.key'); + // Small message => long PS, so there's guaranteed space to inject 0x02 before the separator. + var ct = oaep.publicEncryptOaep(pub, Buffer.from('x'), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var em = crypto.privateDecrypt({ key: key, padding: crypto.constants.RSA_NO_PADDING }, ct); + var hLen = 32; + var maskedSeed = em.subarray(1, 1 + hLen); + var maskedDB = em.subarray(1 + hLen); + var seed = oaep.mgf1(maskedDB, hLen, 'sha1'); + for (var i = 0; i < seed.length; i++) seed[i] ^= maskedSeed[i]; + var db = oaep.mgf1(seed, maskedDB.length, 'sha1'); + for (var j = 0; j < db.length; j++) db[j] ^= maskedDB[j]; + // db: lHash (32) || PS || 0x01 || message. Inject 0x02 in PS well before the separator. + db[hLen + 10] = 0x02; + // Re-mask both DB and seed so the decode will recover this corrupted db. + var newMaskedDB = oaep.mgf1(seed, db.length, 'sha1'); + for (var m = 0; m < db.length; m++) newMaskedDB[m] ^= db[m]; + var newMaskedSeed = oaep.mgf1(newMaskedDB, hLen, 'sha1'); + for (var n = 0; n < seed.length; n++) newMaskedSeed[n] ^= seed[n]; + // Rebuild EM with the new masked values. + em = Buffer.concat([Buffer.from([0x00]), newMaskedSeed, newMaskedDB]); + var badCt = crypto.publicEncrypt({ key: pub, padding: crypto.constants.RSA_NO_PADDING }, em); + assert.throws(function () { + oaep.privateDecryptOaep(key, badCt, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + }, function (e) { return e.code === 'ERR_OSSL_RSA_OAEP_DECODING_ERROR'; }); + }); }); describe('mgf1', function () { @@ -71,6 +166,8 @@ describe('oaep', function () { var fs = require('fs'); var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); var key = fs.readFileSync(__dirname + '/test-auth0.key'); + var keyObj = crypto.createPublicKey(pub); + var k = Math.ceil(keyObj.asymmetricKeyDetails.modulusLength / 8); var combos = [ ['sha256', 'sha1'], ['sha512', 'sha1'], @@ -82,9 +179,8 @@ describe('oaep', function () { combos.forEach(function (combo) { var oaepHash = combo[0]; var mgf1Hash = combo[1]; - // 2048-bit key => k = 256 bytes; longest legal message is k - 2*hLen - 2. var hLen = crypto.createHash(oaepHash).digest().length; - [0, 1, 17, 256 - 2 * hLen - 2].forEach(function (len) { + [0, 1, 17, k - 2 * hLen - 2].forEach(function (len) { it('round trips oaep=' + oaepHash + ' mgf1=' + mgf1Hash + ' len=' + len, function () { var msg = crypto.randomBytes(len); var ct = oaep.publicEncryptOaep(pub, msg, { oaepHash: oaepHash, mgf1Hash: mgf1Hash }); @@ -96,7 +192,7 @@ describe('oaep', function () { it('rejects a message longer than the key allows', function () { assert.throws(function () { - oaep.publicEncryptOaep(pub, crypto.randomBytes(256), { oaepHash: 'sha256' }); + oaep.publicEncryptOaep(pub, crypto.randomBytes(k), { oaepHash: 'sha256' }); }, /message too long/); }); From 4b4e8d6e9a04be1fc44f0dd2ec76161c79642434 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 09:13:46 -0400 Subject: [PATCH 04/14] fix!: pin MGF1 to SHA-1 for rsa-oaep-mgf1p on decrypt BREAKING CHANGE: the rsa-oaep-mgf1p identifier fixes MGF1 to SHA-1 per XML-Enc 1.1 section 5.5.2, and DigestMethod selects only the OAEP message digest. Ciphertext produced by this library with keyEncryptionDigest sha256 or sha512 (v3.1.0 through v5.0.0) used MGF1 matching the digest and no longer decrypts; it was never interoperable with compliant peers. --- lib/xmlenc.js | 19 ++++++++--- test/xmlenc.digest.js | 74 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 349eec0..86b381b 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -2,6 +2,7 @@ var crypto = require('crypto'); var xmldom = require('@xmldom/xmldom'); var dom = require('./dom-select'); var utils = require('./utils'); +var oaep = require('./oaep'); const insecureAlgorithms = [ //https://www.w3.org/TR/xmlenc-core1/#rsav15note @@ -297,7 +298,12 @@ function decryptKeyInfo(doc, options) { switch (keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': - return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash); + // The identifier fixes MGF1 to SHA-1 (XML-Enc 1.1 5.5.2); DigestMethod + // selects only the OAEP message digest. An xenc11:MGF child is a MUST NOT. + if (xpath.select("./*[local-name(.)='MGF']", keyEncryptionMethod)[0]) { + throw new Error('MGF element must not be present with ' + keyEncryptionAlgorithm); + } + return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, 'sha1'); case 'http://www.w3.org/2001/04/xmlenc#rsa-1_5': utils.warnInsecureAlgorithm(keyEncryptionAlgorithm, options.warnInsecureAlgorithm); return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_PADDING); @@ -306,10 +312,15 @@ function decryptKeyInfo(doc, options) { } } -function decryptKeyInfoWithScheme(encryptedKey, options, padding, oaepHash) { +function decryptKeyInfoWithScheme(encryptedKey, options, padding, oaepHash, mgf1Hash) { const key = Buffer.from(encryptedKey.textContent, 'base64'); - const decrypted = crypto.privateDecrypt({ key: options.key, padding, oaepHash}, key); - return Buffer.from(decrypted, 'binary'); + // Node cannot set the MGF1 digest separately, so only fall back to the JS + // implementation when it actually differs. Everything else stays in OpenSSL. + if (padding !== crypto.constants.RSA_PKCS1_OAEP_PADDING || !mgf1Hash || mgf1Hash === oaepHash) { + const decrypted = crypto.privateDecrypt({ key: options.key, padding, oaepHash }, key); + return Buffer.from(decrypted, 'binary'); + } + return oaep.privateDecryptOaep(options.key, key, { oaepHash, mgf1Hash }); } function encryptWithAlgorithm(algorithm, symmetricKey, ivLength, content, encoding, callback) { diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 4994415..2e3138f 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -150,3 +150,77 @@ describe('keyEncryptionDigest', function () { }); }); }); + +describe('DigestMethod resolution with RetrievalMethod', function () { + it('decrypts a RetrievalMethod document whose DigestMethod is sha256', function (done) { + // The RetrievalMethod shape with a sha256 digest: the OAEP message digest + // must be read from the referenced EncryptedKey, not defaulted to sha1. + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP, + keyEncryptionDigest: 'sha256' + }; + xmlenc.encrypt('retrieval method content', options, function (err, result) { + if (err) return done(err); + // Move EncryptedKey out of KeyInfo and point at it with a RetrievalMethod. + var m = //.exec(result); + assert(m, 'expected an EncryptedKey element'); + var encryptedKey = m[0].replace('') + .replace('', encryptedKey + ''); + + xmlenc.decrypt(rewritten, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { + if (err2) return done(err2); + assert.equal(decrypted, 'retrieval method content'); + done(); + }); + }); + }); +}); + +describe('rsa-oaep-mgf1p pins MGF1 to sha1', function () { + var crypto = require('crypto'); + var oaep = require('../lib/oaep'); + + // Build a KeyInfo whose EncryptedKey was wrapped with OAEP(sha256)/MGF1(sha1), + // i.e. what a spec-compliant IdP such as ADFS or Okta actually sends. + function specCompliantKeyInfo(symmetricKey, digest) { + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var wrapped = oaep.publicEncryptOaep(pub, symmetricKey, { oaepHash: digest, mgf1Hash: 'sha1' }); + return '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + } + + ['sha256', 'sha512'].forEach(function (digest) { + it('decrypts a spec-correct MGF1-sha1 key with DigestMethod ' + digest, function () { + var symmetricKey = crypto.randomBytes(32); + var recovered = xmlenc.decryptKeyInfo(specCompliantKeyInfo(symmetricKey, digest), { + key: fs.readFileSync(__dirname + '/test-auth0.key') + }); + assert.equal(Buffer.compare(Buffer.from(recovered), symmetricKey), 0); + }); + }); + + it('rejects a key wrapped with the non-spec MGF1=sha256', function () { + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var wrapped = oaep.publicEncryptOaep(pub, crypto.randomBytes(32), { oaepHash: 'sha256', mgf1Hash: 'sha256' }); + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + }, /oaep decoding error/); + }); +}); From 3f05a6297e5027671e743486d25abf61b1934a63 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 09:34:09 -0400 Subject: [PATCH 05/14] fix!: emit MGF1-SHA1 ciphertext for rsa-oaep-mgf1p BREAKING CHANGE: encrypting with keyEncryptionDigest sha256 or sha512 under rsa-oaep-mgf1p now wraps the key with MGF1-SHA1, as the identifier requires. Peers that adapted to the previous non-compliant output must switch to the xmlenc11#rsa-oaep identifier with keyEncryptionMgf. --- lib/xmlenc.js | 28 ++++++++++++----- test/xmlenc.digest.js | 71 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 86b381b..3ea9008 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -14,15 +14,26 @@ const insecureAlgorithms = [ 'http://www.w3.org/2001/04/xmlenc#aes128-cbc', ]; -function encryptKeyInfoWithScheme(symmetricKey, options, padding, callback) { +function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, callback) { const symmetricKeyBuffer = Buffer.isBuffer(symmetricKey) ? symmetricKey : Buffer.from(symmetricKey, 'utf-8'); try { - var encrypted = crypto.publicEncrypt({ - key: options.rsa_pub, - oaepHash: padding == crypto.constants.RSA_PKCS1_OAEP_PADDING ? options.keyEncryptionDigest : undefined, - padding: padding - }, symmetricKeyBuffer); + const isOAEP = padding == crypto.constants.RSA_PKCS1_OAEP_PADDING; + const oaepHash = isOAEP ? options.keyEncryptionDigest : undefined; + let encrypted; + if (isOAEP && mgf1Hash && mgf1Hash !== oaepHash) { + // Node cannot set MGF1 separately from the OAEP digest. + encrypted = oaep.publicEncryptOaep(options.rsa_pub, symmetricKeyBuffer, { + oaepHash: oaepHash, + mgf1Hash: mgf1Hash + }); + } else { + encrypted = crypto.publicEncrypt({ + key: options.rsa_pub, + oaepHash: oaepHash, + padding: padding + }, symmetricKeyBuffer); + } var base64EncodedEncryptedKey = encrypted.toString('base64'); var params = { @@ -56,11 +67,12 @@ function encryptKeyInfo(symmetricKey, options, callback) { options.keyEncryptionDigest = options.keyEncryptionDigest || 'sha1'; switch (options.keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': - return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, callback); + // MGF1 is fixed to SHA-1 by this identifier (XML-Enc 1.1 5.5.2). + return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, 'sha1', callback); case 'http://www.w3.org/2001/04/xmlenc#rsa-1_5': utils.warnInsecureAlgorithm(options.keyEncryptionAlgorithm, options.warnInsecureAlgorithm); - return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_PADDING, callback); + return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_PADDING, undefined, callback); default: return callback(new Error('encryption key algorithm not supported')); diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 2e3138f..0e04835 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -223,4 +223,75 @@ describe('rsa-oaep-mgf1p pins MGF1 to sha1', function () { xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); }, /oaep decoding error/); }); + + it('decrypts the external OpenSSL vector through the public API', function () { + // VECTOR from test/oaep.js (originally from repro-oaep-mgf1.cjs): OpenSSL OAEP(sha256)/MGF1(sha1) + var VECTOR_KEY = Buffer.from( + 'LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQzA5QmY1NTRXR0VxRXYKMmZyUWxOUG9ycWRMbld5RVAyTGlybGwvekZuUHFnK0c5RkdxYnNNb3o3UG1CUDRpZlRhVFRtSkViaUx1ajkyWQpQM3FieU9JUmN6MkFZQXJkZlE3M0RiSXhYaUZsazNObjlvVnRISVJUcHVvZkwzc2FkVlozMGg5c3JVeTZ4N0Z1ClZvL0ErNDJlSEVRNEdhaGJOSjJsMlh2QU5ydGQwUG5jNlc2MS9pVTdQK3Z4WDJyM0Fqb2VLMjNTWVRxbTkxRTkKMlN0WmVKQjJuSm4rSGxaamV6WTVUblhCZy9HRmFCZGNvR1JMb1diYzFsV2Q0SHNYa1BuVExyTW5UL0xiV1pQZQo4QVpyU253R1Fpa3dud245ZjF3K01ZQ1h6QU0yNWE4STkrZXRacEl0cFN5VUVtWE9yMnEzRkVkMG5RUUVzMHNFCmkrbENlaW5oQWdNQkFBRUNnZ0VBRUpDdDV6YytIblZ6SXczSjY3Rk1LdWRlTWtwaGhrUEZPaW9xMEV1MVJ4RHkKNWZCVXo0emZPY3UxMU05Tk1ud1M5RzQvQ2JPcFoveHNsVVR1WlBlQlZvYWRzVFJabWtnYUNCek5YTDZZd1JNOApBOTdwL1FDWXpvMmZyaVlyRjFONWpIT0VZKzhEY0svYU90Y2F4dGhnY1FKMmJrcFBBclp3M2g5b09FTHFhUjZTClJyNDgxSUZtS0JNdmhyVUQxVFU0MG5jWG43MTdvazlxalR4bFNuOElONElxSmVMTDFPTkFTMDlNSkhISTZPdG8KbHRZUjNWc1RFdE9YTGNsQ2ZubU5ZT2xpeVgrL1VoMTZBak0rSlJmOFRNL2lDYjNkUGFxMyt4UUFCL1oxRnZPZAo0UEFpa01LNVFTc09jdHhxYThwbm10MUlLK1N4MEZ0aUIweThzbWdYM3dLQmdRRHIyNmxLMlIwbmRpWVZDK3hLCjB2SmxYZ0FZeG9TU1IxOS9EdEFQbDdNMkFVVFRzblVFNmlpSCtDbU1ZK1k5Qm0wblkrNGsveThNUG9sdHZ5OUsKQ1ZVT21Ka0hFY3IvZmo3WHl2OEdkTTJSeXVQOHFhRVZxUlh5WU9PMzM5OUt0NzBFb3FFWVJsS0MxQllXcTVWcQovRExURXphSEowSHFXSk85QjB5eW00cDJId0tCZ1FERWFCdEZGN0NqbG1lMVVjUXRDZ0pqZnZPVWF6UTQ3WGdxClpkNTZ6emcyWm5vejZjYzkvTE40WnV5OC8rcFRYcS9GL3E5RjZtb1pyYktHVDBlNFg4dlVtQVZxd3NmQ01TOGcKTWk4Ui8zeGRZdG54Ly9IbW5DUmpYYTJTOUoraWU1Wks3RHpZWGl0Vi9yamEwRHhvWG9RMm82TG9URXQ1WVNTdQpFY2dEclNkZi93S0JnQXZ5dk1qRjV1d3cyQTBJNVplRXlETEthRWJaQjY1QlgxMFlhd0hmTlh6dTQ0VzE3S2VyCkZSS09SOHlNNHdVRVpsTXdoTWZyQlg4aFMrVDdZbkhsdHlGZUthSnFERmFWRnFubjVyTjFCMVR6YWtsS2JwYWkKVWpKTkpqd1NZMFZ0dVcyYXIzNkRVWHEvTTc5Q1FmZUJmekdpTDRqNVBDV2JCeUQwVmJaV210VVJBb0dBY3ZncAo5bUR1c21QSm8zY1FxZml3KzBNR0hMeEFYbzZMaCs0SHRNWDJOc24wQU1sNUt3endsYXRTS3pSM0c0UlN5a2pTCm1zK2tlaEdXYms2Y1FnNDVoK0hSVWZSZzhJalArRDNJRmZZQys3dHdydHRPNDlwRTVyR2dlR1NmeVlJa3NRam0KZVJWdXNyRWZ6bDZVN2RkZDk0b0VRNHpkcFZpN0d2WW5xaGRDOUVzQ2dZRUFwSkJ1SEpFZXYwd1lxUStnNERzVwpxZTVQYWJOY1dVaDJrNUswMGwza0lmMlpweVdWRWs5bnhRVzJIOFVmNHNVc0d0VTBvMmUycEtUanJ1WDRWcVR5CitoeWxrOXZEam1MU29CenFEREU4bFdOK1llU3hBYzJ0WnE2TnpmT0FPZGdHOVN1ODI0MDlEcUtuR1RlQzlKKzAKWHJ6Z21WOGp5K013cU9kQXJ2QXJ0MXM9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K', + 'base64' + ).toString('utf8'); + var VECTOR_CT = Buffer.from( + 'fcAWGHe0HIxC3LcLBwwrkts3005XTSznQZTZZU6EiLOSh/fAfPoe0vF60RcK0IYGW1oDUfuwCl3W+C3HOPTRvFGHiI6AfKCKkj8pTna6WuAZP5x4lBdSKxkIoECgBp+GYko2TMlRn6aW0mOhMCw60P1lT5x93blbbYf4nh0reOtODA8VQBCHnS0wu+qFqIzG/x2UgIbrasnlHo45UlbxdfpOYR08ckKZZrltMZrLcoQnTgrwevwafOg9OvfpY9Kw5Aml+aBhdsabr2aQC5quE6nho0ar/QobPmG5+WzEB5eHn59fTQExDdV2KDcyi7E8xACOjkFFWr+VZmf6t1l59Q==', + 'base64' + ); + // Wrap it with mgf1p + sha256 DigestMethod, which the library should decrypt with MGF1-sha1. + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + VECTOR_CT.toString('base64') + '' + + ''; + var recovered = xmlenc.decryptKeyInfo(keyInfo, { key: VECTOR_KEY }); + assert.equal(recovered.toString('utf8'), 'AES-128-key-1234'); + }); +}); + +describe('rsa-oaep-mgf1p emits MGF1-sha1 ciphertext', function () { + var oaep = require('../lib/oaep'); + var xpath = require('xpath'); + var xmldom = require('@xmldom/xmldom'); + + ['sha256', 'sha512'].forEach(function (digest) { + it('wraps the key with MGF1-sha1 when keyEncryptionDigest is ' + digest, function (done) { + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP, + keyEncryptionDigest: digest + }; + xmlenc.encrypt('mgf1 sha1 content', options, function (err, result) { + if (err) return done(err); + var doc = new xmldom.DOMParser().parseFromString(result); + var cipherValue = xpath.select("//*[local-name(.)='EncryptedKey']/*[local-name(.)='CipherData']/*[local-name(.)='CipherValue']", doc)[0]; + var wrapped = Buffer.from(cipherValue.textContent, 'base64'); + + // Unwrap with MGF1-sha1: succeeds only if encrypt used the spec MGF. + var withSha1 = oaep.privateDecryptOaep(fs.readFileSync(__dirname + '/test-auth0.key'), wrapped, { oaepHash: digest, mgf1Hash: 'sha1' }); + assert(withSha1.length > 0); + + // And the old non-spec MGF1=digest must no longer parse. + assert.throws(function () { + oaep.privateDecryptOaep(fs.readFileSync(__dirname + '/test-auth0.key'), wrapped, { oaepHash: digest, mgf1Hash: digest }); + }, /oaep decoding error/); + done(); + }); + }); + }); + + it('never emits an MGF element for mgf1p', function (done) { + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP, + keyEncryptionDigest: 'sha256' + }; + xmlenc.encrypt('x', options, function (err, result) { + if (err) return done(err); + assert(!/MGF/.test(result), 'MGF element must not be present with mgf1p'); + done(); + }); + }); }); From 29559dc1b4e4a90ce8895a8680195d197c59b49b Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 09:46:35 -0400 Subject: [PATCH 06/14] test: fix flaky MGF assertion and strengthen key length check - Parse XML to detect MGF elements instead of string matching base64 - Pin wrapped key length to 32 bytes (aes256-gcm key size) Co-Authored-By: Claude Opus 5 --- test/xmlenc.digest.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 0e04835..1e547b7 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -269,7 +269,7 @@ describe('rsa-oaep-mgf1p emits MGF1-sha1 ciphertext', function () { // Unwrap with MGF1-sha1: succeeds only if encrypt used the spec MGF. var withSha1 = oaep.privateDecryptOaep(fs.readFileSync(__dirname + '/test-auth0.key'), wrapped, { oaepHash: digest, mgf1Hash: 'sha1' }); - assert(withSha1.length > 0); + assert.equal(withSha1.length, 32); // And the old non-spec MGF1=digest must no longer parse. assert.throws(function () { @@ -290,7 +290,9 @@ describe('rsa-oaep-mgf1p emits MGF1-sha1 ciphertext', function () { }; xmlenc.encrypt('x', options, function (err, result) { if (err) return done(err); - assert(!/MGF/.test(result), 'MGF element must not be present with mgf1p'); + var doc = new xmldom.DOMParser().parseFromString(result); + var mgf = xpath.select("//*[local-name(.)='EncryptedKey']/*[local-name(.)='EncryptionMethod']/*[local-name(.)='MGF']", doc); + assert.equal(mgf.length, 0, 'MGF element must not be present with mgf1p'); done(); }); }); From fba272ef5435904968c9681bf8227b79dfd191b5 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 09:53:43 -0400 Subject: [PATCH 07/14] feat: support xmlenc11#rsa-oaep with an explicit MGF element Adds the XML-Enc 1.1 key transport identifier whose xenc11:MGF child selects the mask generation function, which is how a digest other than SHA-1 for MGF1 is expressed. Unknown MGF URIs are rejected rather than defaulting to SHA-1. Co-Authored-By: Claude Opus 5 --- lib/templates/keyinfo.tpl.xml.js | 15 +++- lib/xmlenc.js | 60 ++++++++++++- test/xmlenc.digest.js | 146 +++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 4 deletions(-) diff --git a/lib/templates/keyinfo.tpl.xml.js b/lib/templates/keyinfo.tpl.xml.js index 9859d5f..8321521 100644 --- a/lib/templates/keyinfo.tpl.xml.js +++ b/lib/templates/keyinfo.tpl.xml.js @@ -7,15 +7,28 @@ const DIGEST_ALGORITHMS = { 'sha512': 'http://www.w3.org/2001/04/xmlenc#sha512' }; -module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest }) => { +const MGF_ALGORITHMS = { + 'sha1': 'http://www.w3.org/2009/xmlenc11#mgf1sha1', + 'sha224': 'http://www.w3.org/2009/xmlenc11#mgf1sha224', + 'sha256': 'http://www.w3.org/2009/xmlenc11#mgf1sha256', + 'sha384': 'http://www.w3.org/2009/xmlenc11#mgf1sha384', + 'sha512': 'http://www.w3.org/2009/xmlenc11#mgf1sha512' +}; + +module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest, keyEncryptionMgf }) => { const digestUri = DIGEST_ALGORITHMS[keyEncryptionDigest] || keyEncryptionDigest; // RSA-1.5 doesn't hash the key, so it has no digest or DigestMethod. RSA-OAEP does. const isOAEP = keyEncryptionMethod && keyEncryptionMethod.includes('rsa-oaep'); + // Only xmlenc11#rsa-oaep carries an MGF element. For rsa-oaep-mgf1p the MGF + // is fixed to SHA-1 and the element MUST NOT be present (XML-Enc 1.1 5.5.2). + const isOAEP11 = keyEncryptionMethod === 'http://www.w3.org/2009/xmlenc11#rsa-oaep'; + const mgfUri = MGF_ALGORITHMS[keyEncryptionMgf] || keyEncryptionMgf; return ` + ${isOAEP11 && mgfUri ? `` : ''} ${isOAEP ? `` : ''} diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 3ea9008..84db49e 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -14,14 +14,29 @@ const insecureAlgorithms = [ 'http://www.w3.org/2001/04/xmlenc#aes128-cbc', ]; +// XML-Enc 1.1 5.5.2. The normative list uses the xmlenc11#mgf1* URIs; the +// xmlenc#MGF1withSHA1 spelling is accepted on decrypt because Example 33 in +// that same section uses it and implementations copied it. +const MGF_ALGORITHMS = { + 'http://www.w3.org/2009/xmlenc11#mgf1sha1': 'sha1', + 'http://www.w3.org/2009/xmlenc11#mgf1sha224': 'sha224', + 'http://www.w3.org/2009/xmlenc11#mgf1sha256': 'sha256', + 'http://www.w3.org/2009/xmlenc11#mgf1sha384': 'sha384', + 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512', + 'http://www.w3.org/2001/04/xmlenc#MGF1withSHA1': 'sha1' +}; + function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, callback) { const symmetricKeyBuffer = Buffer.isBuffer(symmetricKey) ? symmetricKey : Buffer.from(symmetricKey, 'utf-8'); try { const isOAEP = padding == crypto.constants.RSA_PKCS1_OAEP_PADDING; const oaepHash = isOAEP ? options.keyEncryptionDigest : undefined; + if (isOAEP && !mgf1Hash) { + return callback(new Error('mgf1Hash is required for OAEP padding')); + } let encrypted; - if (isOAEP && mgf1Hash && mgf1Hash !== oaepHash) { + if (isOAEP && mgf1Hash !== oaepHash) { // Node cannot set MGF1 separately from the OAEP digest. encrypted = oaep.publicEncryptOaep(options.rsa_pub, symmetricKeyBuffer, { oaepHash: oaepHash, @@ -41,6 +56,7 @@ function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, call encryptionPublicCert: '' + utils.pemToCert(options.pem.toString()) + '', keyEncryptionMethod: options.keyEncryptionAlgorithm, keyEncryptionDigest: options.keyEncryptionDigest, + keyEncryptionMgf: mgf1Hash, }; var result = utils.renderTemplate('keyinfo', params); @@ -60,16 +76,36 @@ function encryptKeyInfo(symmetricKey, options, callback) { if (!options.keyEncryptionAlgorithm) return callback(new Error('encryption without encrypted key is not supported yet')); - if (options.disallowEncryptionWithInsecureAlgorithm !== false + if (options.disallowEncryptionWithInsecureAlgorithm !== false && insecureAlgorithms.indexOf(options.keyEncryptionAlgorithm) >= 0) { return callback(new Error('encryption algorithm ' + options.keyEncryptionAlgorithm + 'is not secure')); } options.keyEncryptionDigest = options.keyEncryptionDigest || 'sha1'; + + if (options.keyEncryptionMgf + && options.keyEncryptionAlgorithm !== 'http://www.w3.org/2009/xmlenc11#rsa-oaep') { + return callback(new Error('keyEncryptionMgf is only supported with http://www.w3.org/2009/xmlenc11#rsa-oaep; ' + + options.keyEncryptionAlgorithm + ' fixes the mask generation function')); + } + switch (options.keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': // MGF1 is fixed to SHA-1 by this identifier (XML-Enc 1.1 5.5.2). return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, 'sha1', callback); + case 'http://www.w3.org/2009/xmlenc11#rsa-oaep': { + // Normalize keyEncryptionMgf to a short digest name. + let mgf1Hash = options.keyEncryptionMgf || 'sha1'; + if (MGF_ALGORITHMS[mgf1Hash]) { + // It's a full URI, map to short name. + mgf1Hash = MGF_ALGORITHMS[mgf1Hash]; + } else if (!['sha1', 'sha224', 'sha256', 'sha384', 'sha512'].includes(mgf1Hash)) { + // It's neither a known URI nor a valid short name. + return callback(new Error('keyEncryptionMgf value ' + mgf1Hash + ' is not supported')); + } + return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, mgf1Hash, callback); + } + case 'http://www.w3.org/2001/04/xmlenc#rsa-1_5': utils.warnInsecureAlgorithm(options.keyEncryptionAlgorithm, options.warnInsecureAlgorithm); return encryptKeyInfoWithScheme(symmetricKey, options, crypto.constants.RSA_PKCS1_PADDING, undefined, callback); @@ -316,6 +352,21 @@ function decryptKeyInfo(doc, options) { throw new Error('MGF element must not be present with ' + keyEncryptionAlgorithm); } return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, 'sha1'); + + case 'http://www.w3.org/2009/xmlenc11#rsa-oaep': { + // MGF1 comes from the optional xenc11:MGF child; default MGF1-SHA1. + const mgfElement = xpath.select("./*[local-name(.)='MGF']", keyEncryptionMethod)[0]; + let mgf1Hash = 'sha1'; + if (mgfElement) { + const mgfAlgorithm = mgfElement.getAttribute('Algorithm'); + mgf1Hash = MGF_ALGORITHMS[mgfAlgorithm]; + if (!mgf1Hash) { + throw new Error('mask generation function ' + mgfAlgorithm + ' not supported'); + } + } + return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, mgf1Hash); + } + case 'http://www.w3.org/2001/04/xmlenc#rsa-1_5': utils.warnInsecureAlgorithm(keyEncryptionAlgorithm, options.warnInsecureAlgorithm); return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_PADDING); @@ -328,7 +379,10 @@ function decryptKeyInfoWithScheme(encryptedKey, options, padding, oaepHash, mgf1 const key = Buffer.from(encryptedKey.textContent, 'base64'); // Node cannot set the MGF1 digest separately, so only fall back to the JS // implementation when it actually differs. Everything else stays in OpenSSL. - if (padding !== crypto.constants.RSA_PKCS1_OAEP_PADDING || !mgf1Hash || mgf1Hash === oaepHash) { + if (padding === crypto.constants.RSA_PKCS1_OAEP_PADDING && !mgf1Hash) { + throw new Error('mgf1Hash is required for OAEP padding'); + } + if (padding !== crypto.constants.RSA_PKCS1_OAEP_PADDING || mgf1Hash === oaepHash) { const decrypted = crypto.privateDecrypt({ key: options.key, padding, oaepHash }, key); return Buffer.from(decrypted, 'binary'); } diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 1e547b7..959cbbd 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -296,4 +296,150 @@ describe('rsa-oaep-mgf1p emits MGF1-sha1 ciphertext', function () { done(); }); }); + + it('rejects MGF element when present with mgf1p', function () { + var oaep = require('../lib/oaep'); + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var wrapped = oaep.publicEncryptOaep(pub, Buffer.alloc(32), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + }, /MGF element must not be present/); + }); +}); + +describe('xmlenc11#rsa-oaep with explicit MGF', function () { + var RSA_OAEP_11 = 'http://www.w3.org/2009/xmlenc11#rsa-oaep'; + var oaep = require('../lib/oaep'); + + it('round trips sha256 digest with an explicit mgf1sha256', function (done) { + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP_11, + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'sha256' + }; + xmlenc.encrypt('xmlenc11 content', options, function (err, result) { + if (err) return done(err); + assert(result.includes('http://www.w3.org/2009/xmlenc11#mgf1sha256'), 'expected MGF element'); + xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { + if (err2) return done(err2); + assert.equal(decrypted, 'xmlenc11 content'); + done(); + }); + }); + }); + + it('round trips sha256 digest with mgf1sha1 (the default)', function (done) { + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP_11, + keyEncryptionDigest: 'sha256' + }; + xmlenc.encrypt('default mgf', options, function (err, result) { + if (err) return done(err); + assert(result.includes('http://www.w3.org/2009/xmlenc11#mgf1sha1')); + xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { + if (err2) return done(err2); + assert.equal(decrypted, 'default mgf'); + done(); + }); + }); + }); + + it('rejects an unknown MGF URI rather than defaulting to sha1', function () { + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var wrapped = oaep.publicEncryptOaep(pub, Buffer.alloc(32), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + }, /mask generation function/); + }); + + it('accepts the MGF1withSHA1 spelling from spec Example 33', function () { + // 5.5.2's normative list says xmlenc11#mgf1sha1, but Example 33 in the same + // section writes xmlenc#MGF1withSHA1. Implementations copied the example. + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var sym = require('crypto').randomBytes(32); + var wrapped = oaep.publicEncryptOaep(pub, sym, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + var recovered = xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + assert.equal(Buffer.compare(Buffer.from(recovered), sym), 0); + }); + + it('rejects keyEncryptionMgf under mgf1p instead of silently ignoring it', function (done) { + xmlenc.encrypt('x', { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP, + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'sha256' + }, function (err) { + assert(err, 'expected an error'); + assert(/keyEncryptionMgf/.test(err.message)); + done(); + }); + }); + + it('accepts keyEncryptionMgf as a full MGF URI', function (done) { + var options = { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP_11, + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'http://www.w3.org/2009/xmlenc11#mgf1sha256' + }; + xmlenc.encrypt('uri form', options, function (err, result) { + if (err) return done(err); + assert(result.includes('http://www.w3.org/2009/xmlenc11#mgf1sha256')); + xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { + if (err2) return done(err2); + assert.equal(decrypted, 'uri form'); + done(); + }); + }); + }); + + it('rejects an unsupported keyEncryptionMgf value', function (done) { + xmlenc.encrypt('x', { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP_11, + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'md5' + }, function (err) { + assert(err, 'expected an error'); + assert(/keyEncryptionMgf/.test(err.message)); + assert(/md5/.test(err.message)); + done(); + }); + }); }); From 251865fe707a4553ed10d9c67911c317cd71f5bb Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 10:12:04 -0400 Subject: [PATCH 08/14] feat: honour OAEPparams as the RSA-OAEP label XML-Enc 1.1 5.5.2 permits OAEPparams as the base64 PSourceAlgorithm value. Documents carrying one previously failed with an opaque OAEP decoding error. Co-Authored-By: Claude Opus 5 --- lib/templates/keyinfo.tpl.xml.js | 11 ++--- lib/xmlenc.js | 42 ++++++++++++------ test/xmlenc.digest.js | 73 ++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 19 deletions(-) diff --git a/lib/templates/keyinfo.tpl.xml.js b/lib/templates/keyinfo.tpl.xml.js index 8321521..8d8172d 100644 --- a/lib/templates/keyinfo.tpl.xml.js +++ b/lib/templates/keyinfo.tpl.xml.js @@ -1,21 +1,21 @@ var escapehtml = require('escape-html'); -const DIGEST_ALGORITHMS = { +const DIGEST_ALGORITHMS = Object.assign(Object.create(null), { // SHA-2 was published after 2000/09/xmldsig was locked, so sha256/sha512 live under 2001/04/xmlenc. 'sha1': 'http://www.w3.org/2000/09/xmldsig#sha1', 'sha256': 'http://www.w3.org/2001/04/xmlenc#sha256', 'sha512': 'http://www.w3.org/2001/04/xmlenc#sha512' -}; +}); -const MGF_ALGORITHMS = { +const MGF_ALGORITHMS = Object.assign(Object.create(null), { 'sha1': 'http://www.w3.org/2009/xmlenc11#mgf1sha1', 'sha224': 'http://www.w3.org/2009/xmlenc11#mgf1sha224', 'sha256': 'http://www.w3.org/2009/xmlenc11#mgf1sha256', 'sha384': 'http://www.w3.org/2009/xmlenc11#mgf1sha384', 'sha512': 'http://www.w3.org/2009/xmlenc11#mgf1sha512' -}; +}); -module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest, keyEncryptionMgf }) => { +module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest, keyEncryptionMgf, keyEncryptionOaepParams }) => { const digestUri = DIGEST_ALGORITHMS[keyEncryptionDigest] || keyEncryptionDigest; // RSA-1.5 doesn't hash the key, so it has no digest or DigestMethod. RSA-OAEP does. @@ -28,6 +28,7 @@ module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, key + ${isOAEP && keyEncryptionOaepParams ? `${escapehtml(keyEncryptionOaepParams)}` : ''} ${isOAEP11 && mgfUri ? `` : ''} ${isOAEP ? `` : ''} diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 84db49e..f8d7d03 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -17,14 +17,15 @@ const insecureAlgorithms = [ // XML-Enc 1.1 5.5.2. The normative list uses the xmlenc11#mgf1* URIs; the // xmlenc#MGF1withSHA1 spelling is accepted on decrypt because Example 33 in // that same section uses it and implementations copied it. -const MGF_ALGORITHMS = { +const MGF_ALGORITHMS = Object.assign(Object.create(null), { 'http://www.w3.org/2009/xmlenc11#mgf1sha1': 'sha1', 'http://www.w3.org/2009/xmlenc11#mgf1sha224': 'sha224', 'http://www.w3.org/2009/xmlenc11#mgf1sha256': 'sha256', 'http://www.w3.org/2009/xmlenc11#mgf1sha384': 'sha384', 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512', 'http://www.w3.org/2001/04/xmlenc#MGF1withSHA1': 'sha1' -}; +}); +const MGF_SHORT_NAMES = Object.values(MGF_ALGORITHMS); function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, callback) { const symmetricKeyBuffer = Buffer.isBuffer(symmetricKey) ? symmetricKey : Buffer.from(symmetricKey, 'utf-8'); @@ -32,15 +33,21 @@ function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, call try { const isOAEP = padding == crypto.constants.RSA_PKCS1_OAEP_PADDING; const oaepHash = isOAEP ? options.keyEncryptionDigest : undefined; + const oaepLabel = options.keyEncryptionOaepParams + ? (Buffer.isBuffer(options.keyEncryptionOaepParams) + ? options.keyEncryptionOaepParams + : Buffer.from(options.keyEncryptionOaepParams, 'base64')) + : Buffer.alloc(0); if (isOAEP && !mgf1Hash) { return callback(new Error('mgf1Hash is required for OAEP padding')); } let encrypted; - if (isOAEP && mgf1Hash !== oaepHash) { - // Node cannot set MGF1 separately from the OAEP digest. + if (isOAEP && (mgf1Hash !== oaepHash || oaepLabel.length > 0)) { + // Node cannot set MGF1 separately from the OAEP digest, and has no label option. encrypted = oaep.publicEncryptOaep(options.rsa_pub, symmetricKeyBuffer, { oaepHash: oaepHash, - mgf1Hash: mgf1Hash + mgf1Hash: mgf1Hash, + oaepLabel: oaepLabel }); } else { encrypted = crypto.publicEncrypt({ @@ -57,6 +64,7 @@ function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, call keyEncryptionMethod: options.keyEncryptionAlgorithm, keyEncryptionDigest: options.keyEncryptionDigest, keyEncryptionMgf: mgf1Hash, + keyEncryptionOaepParams: oaepLabel.length ? oaepLabel.toString('base64') : null, }; var result = utils.renderTemplate('keyinfo', params); @@ -99,7 +107,7 @@ function encryptKeyInfo(symmetricKey, options, callback) { if (MGF_ALGORITHMS[mgf1Hash]) { // It's a full URI, map to short name. mgf1Hash = MGF_ALGORITHMS[mgf1Hash]; - } else if (!['sha1', 'sha224', 'sha256', 'sha384', 'sha512'].includes(mgf1Hash)) { + } else if (!MGF_SHORT_NAMES.includes(mgf1Hash)) { // It's neither a known URI nor a valid short name. return callback(new Error('keyEncryptionMgf value ' + mgf1Hash + ' is not supported')); } @@ -329,7 +337,7 @@ function decryptKeyInfo(doc, options) { } var keyEncryptionAlgorithm = keyEncryptionMethod.getAttribute('Algorithm'); - if (options.disallowDecryptionWithInsecureAlgorithm !== false + if (options.disallowDecryptionWithInsecureAlgorithm !== false && insecureAlgorithms.indexOf(keyEncryptionAlgorithm) >= 0) { throw new Error('encryption algorithm ' + keyEncryptionAlgorithm + ' is not secure, fail to decrypt'); } @@ -344,6 +352,10 @@ function decryptKeyInfo(doc, options) { throw new Error('cant find encrypted key'); } + // Read the OAEP label from the optional OAEPparams element (XML-Enc 1.1 5.5.2). + const oaepParams = xpath.select("./*[local-name(.)='OAEPparams']", keyEncryptionMethod)[0]; + const oaepLabel = oaepParams ? Buffer.from(oaepParams.textContent, 'base64') : Buffer.alloc(0); + switch (keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': // The identifier fixes MGF1 to SHA-1 (XML-Enc 1.1 5.5.2); DigestMethod @@ -351,7 +363,7 @@ function decryptKeyInfo(doc, options) { if (xpath.select("./*[local-name(.)='MGF']", keyEncryptionMethod)[0]) { throw new Error('MGF element must not be present with ' + keyEncryptionAlgorithm); } - return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, 'sha1'); + return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, 'sha1', oaepLabel); case 'http://www.w3.org/2009/xmlenc11#rsa-oaep': { // MGF1 comes from the optional xenc11:MGF child; default MGF1-SHA1. @@ -364,7 +376,7 @@ function decryptKeyInfo(doc, options) { throw new Error('mask generation function ' + mgfAlgorithm + ' not supported'); } } - return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, mgf1Hash); + return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, mgf1Hash, oaepLabel); } case 'http://www.w3.org/2001/04/xmlenc#rsa-1_5': @@ -375,18 +387,20 @@ function decryptKeyInfo(doc, options) { } } -function decryptKeyInfoWithScheme(encryptedKey, options, padding, oaepHash, mgf1Hash) { +function decryptKeyInfoWithScheme(encryptedKey, options, padding, oaepHash, mgf1Hash, oaepLabel) { const key = Buffer.from(encryptedKey.textContent, 'base64'); - // Node cannot set the MGF1 digest separately, so only fall back to the JS - // implementation when it actually differs. Everything else stays in OpenSSL. + const label = oaepLabel || Buffer.alloc(0); if (padding === crypto.constants.RSA_PKCS1_OAEP_PADDING && !mgf1Hash) { throw new Error('mgf1Hash is required for OAEP padding'); } - if (padding !== crypto.constants.RSA_PKCS1_OAEP_PADDING || mgf1Hash === oaepHash) { + // Node's privateDecrypt has no label option, so a non-empty label also needs the shim. + const needsShim = padding === crypto.constants.RSA_PKCS1_OAEP_PADDING + && (mgf1Hash !== oaepHash || label.length > 0); + if (!needsShim) { const decrypted = crypto.privateDecrypt({ key: options.key, padding, oaepHash }, key); return Buffer.from(decrypted, 'binary'); } - return oaep.privateDecryptOaep(options.key, key, { oaepHash, mgf1Hash }); + return oaep.privateDecryptOaep(options.key, key, { oaepHash, mgf1Hash, oaepLabel: label }); } function encryptWithAlgorithm(algorithm, symmetricKey, ivLength, content, encoding, callback) { diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 959cbbd..4109ab7 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -442,4 +442,77 @@ describe('xmlenc11#rsa-oaep with explicit MGF', function () { done(); }); }); + + it('rejects MGF with "constructor" to avoid prototype pollution', function (done) { + xmlenc.encrypt('x', { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP_11, + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'constructor' + }, function (err) { + assert(err, 'expected an error'); + assert(/keyEncryptionMgf/.test(err.message)); + done(); + }); + }); + + it('rejects on decrypt', function () { + var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); + var wrapped = oaep.publicEncryptOaep(pub, Buffer.alloc(32), { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + var keyInfo = '' + + '' + + '' + + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + assert.throws(function () { + xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + }, /mask generation function/); + }); +}); + +describe('OAEPparams', function () { + var oaep = require('../lib/oaep'); + var crypto = require('crypto'); + + it('decrypts a key wrapped with a non-empty OAEP label', function () { + var label = Buffer.from('MYLABEL'); + var sym = crypto.randomBytes(32); + var wrapped = oaep.publicEncryptOaep(fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), sym, { + oaepHash: 'sha256', mgf1Hash: 'sha1', oaepLabel: label + }); + var keyInfo = '' + + '' + + '' + + '' + label.toString('base64') + '' + + '' + + '' + + '' + wrapped.toString('base64') + '' + + ''; + var recovered = xmlenc.decryptKeyInfo(keyInfo, { key: fs.readFileSync(__dirname + '/test-auth0.key') }); + assert.equal(Buffer.compare(Buffer.from(recovered), sym), 0); + }); + + it('round trips keyEncryptionOaepParams through encrypt and decrypt', function (done) { + xmlenc.encrypt('labelled content', { + rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), + pem: fs.readFileSync(__dirname + '/test-auth0.pem'), + encryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#aes256-gcm', + keyEncryptionAlgorithm: RSA_OAEP, + keyEncryptionDigest: 'sha256', + keyEncryptionOaepParams: Buffer.from('9lWu3Q==', 'base64') + }, function (err, result) { + if (err) return done(err); + assert(result.includes('9lWu3Q==')); + xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { + if (err2) return done(err2); + assert.equal(decrypted, 'labelled content'); + done(); + }); + }); + }); }); From f2aad8dab72510ace8838355586f508161cbad45 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 10:23:43 -0400 Subject: [PATCH 09/14] docs: document MGF1 semantics and the xmlenc11#rsa-oaep identifier Co-Authored-By: Claude Opus 5 --- README.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/README.md b/README.md index aec498a..e5c95af 100644 --- a/README.md +++ b/README.md @@ -75,6 +75,7 @@ Currently the library supports: * EncryptedKey to transport symmetric key using: * http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p + * http://www.w3.org/2009/xmlenc11#rsa-oaep * http://www.w3.org/2001/04/xmlenc#rsa-1_5 (Insecure Algorithm) * EncryptedData using: @@ -92,6 +93,30 @@ We recommend usage of AES-256-GCM (Galois/Counter Mode) for the strongest securi Note that `xml-encryption` versions prior to 4.0 supported AES-128-CBC and AES-256-CBC as secure algorithms. In version 4.0 onwards, these are treated as insecure because they use the Cipher Block Chaining (CBC) mode of encryption, which does not provide integrity guarantees. To continue using AES128-CBC and AES256-CBC, enable support for insecure algorithms via `disallowEncryptionWithInsecureAlgorithm/disallowDecryptionWithInsecureAlgorithm`. +### RSA-OAEP mask generation (MGF1) + +`http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p` fixes the mask generation function to **MGF1 with SHA-1**, per [XML Encryption 1.1 §5.5.2][xmlenc-oaep]. `keyEncryptionDigest` selects only the OAEP message digest, so `keyEncryptionDigest: 'sha256'` means OAEP-SHA256 with MGF1-SHA1. + +To use a different MGF1 digest, use the XML Encryption 1.1 identifier, which carries an explicit `` element: + +~~~js +var options = { + keyEncryptionAlgorithm: 'http://www.w3.org/2009/xmlenc11#rsa-oaep', + keyEncryptionDigest: 'sha256', + keyEncryptionMgf: 'sha256' // sha1 | sha224 | sha256 | sha384 | sha512, default sha1 +}; +~~~ + +`keyEncryptionMgf` accepts either a short digest name (`sha1`, `sha224`, `sha256`, `sha384`, or `sha512`) or a full `http://www.w3.org/2009/xmlenc11#mgf1*` URI. It is rejected with `rsa-oaep-mgf1p`, which has no valid MGF other than SHA-1. + +An optional OAEP label may be supplied as `keyEncryptionOaepParams` (a Buffer or a base64 string); it is emitted as `` and honoured on decrypt. + +Note: for the digest/MGF1 combinations Node's `crypto` cannot express, the OAEP padding is computed in JavaScript over the raw RSA primitive. That code path cannot offer the constant-time guarantees of OpenSSL's C implementation. It is used only when the MGF1 digest differs from the message digest or a label is present; all other combinations go through `crypto.privateDecrypt` unchanged. + +**Breaking change:** in versions 3.1.0 through 5.0.0, `rsa-oaep-mgf1p` with `keyEncryptionDigest: 'sha256'` or `'sha512'` produced ciphertext using MGF1-SHA256 or MGF1-SHA512, which was never compliant with the W3C specification. Starting in 5.1.0, `rsa-oaep-mgf1p` correctly produces MGF1-SHA1 ciphertext regardless of `keyEncryptionDigest`. Documents encrypted with the non-compliant behaviour will not decrypt with the current version. Such documents were never interoperable with Java xmlsec, .NET System.Security.Cryptography.Xml, or other spec-compliant peers. Callers who genuinely need MGF1-SHA256 or MGF1-SHA512 should use `http://www.w3.org/2009/xmlenc11#rsa-oaep` with the `keyEncryptionMgf` option. + +[xmlenc-oaep]: https://www.w3.org/TR/xmlenc-core1/#sec-RSA-OAEP + ### Allow listing specific algorithms when decrypting If decrypting with `disallowEncryptionWithInsecureAlgorithm: true`, you may wish to only support a subset of insecure algorithms (for example, supporting AES-256-CBC only). This can be achieved by extracting the encryption algorithm using the following code and applying validation as required. From ceb91be34da119f2132ebc39cbe5e3e1ac215154 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 10:25:05 -0400 Subject: [PATCH 10/14] docs: drop invented version number from the MGF1 breaking-change note semantic-release derives the version from the commit history; naming 5.1.0 in prose was both unverifiable and wrong for a breaking change. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e5c95af..6ea22ed 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ An optional OAEP label may be supplied as `keyEncryptionOaepParams` (a Buffer or Note: for the digest/MGF1 combinations Node's `crypto` cannot express, the OAEP padding is computed in JavaScript over the raw RSA primitive. That code path cannot offer the constant-time guarantees of OpenSSL's C implementation. It is used only when the MGF1 digest differs from the message digest or a label is present; all other combinations go through `crypto.privateDecrypt` unchanged. -**Breaking change:** in versions 3.1.0 through 5.0.0, `rsa-oaep-mgf1p` with `keyEncryptionDigest: 'sha256'` or `'sha512'` produced ciphertext using MGF1-SHA256 or MGF1-SHA512, which was never compliant with the W3C specification. Starting in 5.1.0, `rsa-oaep-mgf1p` correctly produces MGF1-SHA1 ciphertext regardless of `keyEncryptionDigest`. Documents encrypted with the non-compliant behaviour will not decrypt with the current version. Such documents were never interoperable with Java xmlsec, .NET System.Security.Cryptography.Xml, or other spec-compliant peers. Callers who genuinely need MGF1-SHA256 or MGF1-SHA512 should use `http://www.w3.org/2009/xmlenc11#rsa-oaep` with the `keyEncryptionMgf` option. +**Breaking change:** in versions 3.1.0 through 5.0.0, `rsa-oaep-mgf1p` with `keyEncryptionDigest: 'sha256'` or `'sha512'` produced ciphertext using MGF1-SHA256 or MGF1-SHA512, which was never compliant with the W3C specification. `rsa-oaep-mgf1p` now produces MGF1-SHA1 ciphertext regardless of `keyEncryptionDigest`. Documents encrypted with the earlier behaviour will not decrypt with the current version; they were never interoperable with Java xmlsec, .NET `System.Security.Cryptography.Xml`, or other spec-compliant peers. Callers who genuinely need MGF1-SHA256 or MGF1-SHA512 should use `http://www.w3.org/2009/xmlenc11#rsa-oaep` with the `keyEncryptionMgf` option. [xmlenc-oaep]: https://www.w3.org/TR/xmlenc-core1/#sec-RSA-OAEP From 79473f5d7fa87a0d1a44ceaa919650af0815776d Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 11:26:40 -0400 Subject: [PATCH 11/14] fix: correct OAEPparams namespace and derive MGF map single-source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - OAEPparams now emits as e:OAEPparams (xenc namespace), not bare OAEPparams (inherited xmldsig namespace). Schema-validating peers require it in xenc per EncryptionMethodType. - Test updated to assert namespaceURI and element ordering via DOM. - MGF_ALGORITHMS now defined once in lib/mgf-algorithms.js. Template derives short→URI map, ensuring sha1 emits xmlenc11#mgf1sha1 (not the legacy alias). Drift now caught: unmapped short names throw instead of emitting bare Algorithm="name". - Removed || keyEncryptionMgf fallback that silently serialized non-URIs. Unmapped values now fail loudly. Co-Authored-By: Claude Opus 5 --- lib/mgf-algorithms.js | 27 +++++++++++++++++++++++++++ lib/templates/keyinfo.tpl.xml.js | 16 ++++++---------- lib/xmlenc.js | 14 +------------- test/xmlenc.digest.js | 15 ++++++++++++++- 4 files changed, 48 insertions(+), 24 deletions(-) create mode 100644 lib/mgf-algorithms.js diff --git a/lib/mgf-algorithms.js b/lib/mgf-algorithms.js new file mode 100644 index 0000000..790517a --- /dev/null +++ b/lib/mgf-algorithms.js @@ -0,0 +1,27 @@ +// Canonical MGF URI → short-name map. XML-Enc 1.1 5.5.2. The normative list +// uses the xmlenc11#mgf1* URIs; the xmlenc#MGF1withSHA1 spelling is accepted +// on decrypt because Example 33 in that same section uses it and implementations +// copied it. +const MGF_ALGORITHMS = Object.assign(Object.create(null), { + 'http://www.w3.org/2009/xmlenc11#mgf1sha1': 'sha1', + 'http://www.w3.org/2009/xmlenc11#mgf1sha224': 'sha224', + 'http://www.w3.org/2009/xmlenc11#mgf1sha256': 'sha256', + 'http://www.w3.org/2009/xmlenc11#mgf1sha384': 'sha384', + 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512', + 'http://www.w3.org/2001/04/xmlenc#MGF1withSHA1': 'sha1' +}); + +const MGF_SHORT_NAMES = Object.values(MGF_ALGORITHMS); + +// Derive short-name → URI map for emit, excluding the legacy alias. +// xmlenc11#mgf1sha1 must win over xmlenc#MGF1withSHA1 for sha1. +const MGF_URI_FOR_EMIT = Object.assign(Object.create(null), {}); +for (const [uri, shortName] of Object.entries(MGF_ALGORITHMS)) { + // Only set if not already present (first occurrence wins). + // The xmlenc11#mgf1sha1 entry comes before the legacy entry, so it wins. + if (!MGF_URI_FOR_EMIT[shortName]) { + MGF_URI_FOR_EMIT[shortName] = uri; + } +} + +module.exports = { MGF_ALGORITHMS, MGF_SHORT_NAMES, MGF_URI_FOR_EMIT }; diff --git a/lib/templates/keyinfo.tpl.xml.js b/lib/templates/keyinfo.tpl.xml.js index 8d8172d..44ab87f 100644 --- a/lib/templates/keyinfo.tpl.xml.js +++ b/lib/templates/keyinfo.tpl.xml.js @@ -1,4 +1,5 @@ var escapehtml = require('escape-html'); +var { MGF_URI_FOR_EMIT } = require('../mgf-algorithms'); const DIGEST_ALGORITHMS = Object.assign(Object.create(null), { // SHA-2 was published after 2000/09/xmldsig was locked, so sha256/sha512 live under 2001/04/xmlenc. @@ -7,14 +8,6 @@ const DIGEST_ALGORITHMS = Object.assign(Object.create(null), { 'sha512': 'http://www.w3.org/2001/04/xmlenc#sha512' }); -const MGF_ALGORITHMS = Object.assign(Object.create(null), { - 'sha1': 'http://www.w3.org/2009/xmlenc11#mgf1sha1', - 'sha224': 'http://www.w3.org/2009/xmlenc11#mgf1sha224', - 'sha256': 'http://www.w3.org/2009/xmlenc11#mgf1sha256', - 'sha384': 'http://www.w3.org/2009/xmlenc11#mgf1sha384', - 'sha512': 'http://www.w3.org/2009/xmlenc11#mgf1sha512' -}); - module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, keyEncryptionDigest, keyEncryptionMgf, keyEncryptionOaepParams }) => { const digestUri = DIGEST_ALGORITHMS[keyEncryptionDigest] || keyEncryptionDigest; @@ -23,12 +16,15 @@ module.exports = ({ encryptionPublicCert, encryptedKey, keyEncryptionMethod, key // Only xmlenc11#rsa-oaep carries an MGF element. For rsa-oaep-mgf1p the MGF // is fixed to SHA-1 and the element MUST NOT be present (XML-Enc 1.1 5.5.2). const isOAEP11 = keyEncryptionMethod === 'http://www.w3.org/2009/xmlenc11#rsa-oaep'; - const mgfUri = MGF_ALGORITHMS[keyEncryptionMgf] || keyEncryptionMgf; + const mgfUri = MGF_URI_FOR_EMIT[keyEncryptionMgf]; + if (isOAEP11 && keyEncryptionMgf && !mgfUri) { + throw new Error('keyEncryptionMgf value ' + keyEncryptionMgf + ' is not a known short name'); + } return ` - ${isOAEP && keyEncryptionOaepParams ? `${escapehtml(keyEncryptionOaepParams)}` : ''} + ${isOAEP && keyEncryptionOaepParams ? `${escapehtml(keyEncryptionOaepParams)}` : ''} ${isOAEP11 && mgfUri ? `` : ''} ${isOAEP ? `` : ''} diff --git a/lib/xmlenc.js b/lib/xmlenc.js index f8d7d03..300ceb1 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -3,6 +3,7 @@ var xmldom = require('@xmldom/xmldom'); var dom = require('./dom-select'); var utils = require('./utils'); var oaep = require('./oaep'); +var { MGF_ALGORITHMS, MGF_SHORT_NAMES } = require('./mgf-algorithms'); const insecureAlgorithms = [ //https://www.w3.org/TR/xmlenc-core1/#rsav15note @@ -14,19 +15,6 @@ const insecureAlgorithms = [ 'http://www.w3.org/2001/04/xmlenc#aes128-cbc', ]; -// XML-Enc 1.1 5.5.2. The normative list uses the xmlenc11#mgf1* URIs; the -// xmlenc#MGF1withSHA1 spelling is accepted on decrypt because Example 33 in -// that same section uses it and implementations copied it. -const MGF_ALGORITHMS = Object.assign(Object.create(null), { - 'http://www.w3.org/2009/xmlenc11#mgf1sha1': 'sha1', - 'http://www.w3.org/2009/xmlenc11#mgf1sha224': 'sha224', - 'http://www.w3.org/2009/xmlenc11#mgf1sha256': 'sha256', - 'http://www.w3.org/2009/xmlenc11#mgf1sha384': 'sha384', - 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512', - 'http://www.w3.org/2001/04/xmlenc#MGF1withSHA1': 'sha1' -}); -const MGF_SHORT_NAMES = Object.values(MGF_ALGORITHMS); - function encryptKeyInfoWithScheme(symmetricKey, options, padding, mgf1Hash, callback) { const symmetricKeyBuffer = Buffer.isBuffer(symmetricKey) ? symmetricKey : Buffer.from(symmetricKey, 'utf-8'); diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index 4109ab7..ceea821 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -498,6 +498,8 @@ describe('OAEPparams', function () { }); it('round trips keyEncryptionOaepParams through encrypt and decrypt', function (done) { + var xmldom = require('@xmldom/xmldom'); + var xpath = require('xpath'); xmlenc.encrypt('labelled content', { rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), pem: fs.readFileSync(__dirname + '/test-auth0.pem'), @@ -507,7 +509,18 @@ describe('OAEPparams', function () { keyEncryptionOaepParams: Buffer.from('9lWu3Q==', 'base64') }, function (err, result) { if (err) return done(err); - assert(result.includes('9lWu3Q==')); + // OAEPparams must be in the xenc namespace, not xmldsig, and appear before MGF and DigestMethod. + var doc = new xmldom.DOMParser().parseFromString(result); + var encMethod = xpath.select("//*[local-name(.)='EncryptedKey']/*[local-name(.)='EncryptionMethod']", doc)[0]; + var params = xpath.select("*[local-name(.)='OAEPparams']", encMethod); + assert.equal(params.length, 1, 'OAEPparams element must be present'); + assert.equal(params[0].namespaceURI, 'http://www.w3.org/2001/04/xmlenc#', 'OAEPparams must be in xenc namespace'); + assert.equal(params[0].textContent, '9lWu3Q==', 'OAEPparams value must match'); + // Verify element ordering: OAEPparams comes before DigestMethod + var children = Array.from(encMethod.childNodes).filter(function (n) { return n.nodeType === 1; }); + var oaepIdx = children.findIndex(function (n) { return n.localName === 'OAEPparams'; }); + var digestIdx = children.findIndex(function (n) { return n.localName === 'DigestMethod'; }); + assert(oaepIdx >= 0 && digestIdx >= 0 && oaepIdx < digestIdx, 'OAEPparams must come before DigestMethod'); xmlenc.decrypt(result, { key: fs.readFileSync(__dirname + '/test-auth0.key') }, function (err2, decrypted) { if (err2) return done(err2); assert.equal(decrypted, 'labelled content'); From 149539323ee18eb69586acb40493eaefdbb0b42b Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 11:27:45 -0400 Subject: [PATCH 12/14] test: pin OAEP seed randomness property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two tests to verify publicEncryptOaep randomizes the OAEP seed: 1. Twenty encryptions of identical plaintext produce distinct ciphertexts. 2. The maskedSeed itself varies (extracted via RSA_NO_PADDING decrypt). This pins IND-CPA security — a constant seed would make OAEP deterministic and leak plaintext equality. The mutation test confirms: replacing crypto.randomBytes(hLen) with Buffer.alloc(hLen) drops the count from 100 passing to 98 passing (2 failing), and restoring it returns to 100. Co-Authored-By: Claude Opus 5 --- test/oaep.js | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/oaep.js b/test/oaep.js index 4941f46..a259d4a 100644 --- a/test/oaep.js +++ b/test/oaep.js @@ -205,5 +205,40 @@ describe('oaep', function () { oaep.privateDecryptOaep(key, ct, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); }, /oaep decoding error/); }); + + it('produces distinct ciphertexts for identical plaintexts (randomized seed)', function () { + var msg = Buffer.from('same message'); + var ciphertexts = []; + for (var i = 0; i < 20; i++) { + ciphertexts.push(oaep.publicEncryptOaep(pub, msg, { oaepHash: 'sha256', mgf1Hash: 'sha1' })); + } + // All ciphertexts should be distinct. + for (var j = 0; j < ciphertexts.length; j++) { + for (var k = j + 1; k < ciphertexts.length; k++) { + assert.notEqual(Buffer.compare(ciphertexts[j], ciphertexts[k]), 0, + 'ciphertext ' + j + ' and ' + k + ' should differ'); + } + } + }); + + it('randomizes the padding seed itself (not just RSA randomness)', function () { + var msg = Buffer.from('test'); + var seeds = []; + var hLen = crypto.createHash('sha256').digest().length; + for (var i = 0; i < 20; i++) { + var ct = oaep.publicEncryptOaep(pub, msg, { oaepHash: 'sha256', mgf1Hash: 'sha1' }); + // Recover EM = 0x00 || maskedSeed || maskedDB via raw RSA decrypt (no padding). + var em = crypto.privateDecrypt({ key: key, padding: crypto.constants.RSA_NO_PADDING }, ct); + var maskedSeed = em.subarray(1, 1 + hLen); + seeds.push(maskedSeed); + } + // All maskedSeed values should be distinct. + for (var j = 0; j < seeds.length; j++) { + for (var k = j + 1; k < seeds.length; k++) { + assert.notEqual(Buffer.compare(seeds[j], seeds[k]), 0, + 'maskedSeed ' + j + ' and ' + k + ' should differ'); + } + } + }); }); }); From 0483662fc4148dd7f502456dbca7657702e78fe4 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 11:40:11 -0400 Subject: [PATCH 13/14] refactor: separate the legacy MGF alias from the canonical list MGF_URI_FOR_EMIT was derived from a map that also held the decrypt-only xmlenc#MGF1withSHA1 alias, with first-occurrence-wins deciding which URI sha1 emits. Reordering the literal would have silently started emitting a non-normative URI with every test still green. The emit map now derives from the canonical 5.5.2 list alone, and a test pins that every emitted value is an xmlenc11#mgf1* URI. Co-Authored-By: Claude Opus 5 --- lib/mgf-algorithms.js | 39 +++++++++++++++++++++++---------------- test/xmlenc.digest.js | 13 +++++++++++++ 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/lib/mgf-algorithms.js b/lib/mgf-algorithms.js index 790517a..32a21bb 100644 --- a/lib/mgf-algorithms.js +++ b/lib/mgf-algorithms.js @@ -1,27 +1,34 @@ -// Canonical MGF URI → short-name map. XML-Enc 1.1 5.5.2. The normative list -// uses the xmlenc11#mgf1* URIs; the xmlenc#MGF1withSHA1 spelling is accepted -// on decrypt because Example 33 in that same section uses it and implementations -// copied it. -const MGF_ALGORITHMS = Object.assign(Object.create(null), { +// MGF URI ↔ short-name maps. XML-Enc 1.1 5.5.2. +// +// MGF_CANONICAL is the normative list, and the only source for what we emit. +// MGF_LEGACY_ALIASES is accepted on decrypt only: the xmlenc#MGF1withSHA1 +// spelling appears in Example 33 of that same section and implementations +// copied it, but it is not in the normative list and must never be emitted. +// Keeping the two separate means reordering either literal cannot change what +// we emit. +const MGF_CANONICAL = Object.assign(Object.create(null), { 'http://www.w3.org/2009/xmlenc11#mgf1sha1': 'sha1', 'http://www.w3.org/2009/xmlenc11#mgf1sha224': 'sha224', 'http://www.w3.org/2009/xmlenc11#mgf1sha256': 'sha256', 'http://www.w3.org/2009/xmlenc11#mgf1sha384': 'sha384', - 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512', + 'http://www.w3.org/2009/xmlenc11#mgf1sha512': 'sha512' +}); + +const MGF_LEGACY_ALIASES = Object.assign(Object.create(null), { 'http://www.w3.org/2001/04/xmlenc#MGF1withSHA1': 'sha1' }); -const MGF_SHORT_NAMES = Object.values(MGF_ALGORITHMS); +// URI → short name, for decrypt. Accepts the legacy alias. +const MGF_ALGORITHMS = Object.assign(Object.create(null), MGF_CANONICAL, MGF_LEGACY_ALIASES); -// Derive short-name → URI map for emit, excluding the legacy alias. -// xmlenc11#mgf1sha1 must win over xmlenc#MGF1withSHA1 for sha1. -const MGF_URI_FOR_EMIT = Object.assign(Object.create(null), {}); -for (const [uri, shortName] of Object.entries(MGF_ALGORITHMS)) { - // Only set if not already present (first occurrence wins). - // The xmlenc11#mgf1sha1 entry comes before the legacy entry, so it wins. - if (!MGF_URI_FOR_EMIT[shortName]) { - MGF_URI_FOR_EMIT[shortName] = uri; - } +// Short name → URI, for encrypt. Derived from the canonical map alone, so a +// short name we accept on decrypt is either emittable as a normative URI or +// not emittable at all — never emittable as the legacy alias. +const MGF_URI_FOR_EMIT = Object.create(null); +for (const uri of Object.keys(MGF_CANONICAL)) { + MGF_URI_FOR_EMIT[MGF_CANONICAL[uri]] = uri; } +const MGF_SHORT_NAMES = Object.keys(MGF_URI_FOR_EMIT); + module.exports = { MGF_ALGORITHMS, MGF_SHORT_NAMES, MGF_URI_FOR_EMIT }; diff --git a/test/xmlenc.digest.js b/test/xmlenc.digest.js index ceea821..bb25c57 100644 --- a/test/xmlenc.digest.js +++ b/test/xmlenc.digest.js @@ -392,6 +392,19 @@ describe('xmlenc11#rsa-oaep with explicit MGF', function () { assert.equal(Buffer.compare(Buffer.from(recovered), sym), 0); }); + it('never emits the MGF1withSHA1 alias it accepts on decrypt', function () { + // The alias is decrypt-only: it is not in 5.5.2's normative list, so emitting + // it would send a non-normative URI to peers. The emit map is derived from the + // canonical list alone, which is what makes this hold. + var mgf = require('../lib/mgf-algorithms'); + assert.equal(mgf.MGF_URI_FOR_EMIT['sha1'], 'http://www.w3.org/2009/xmlenc11#mgf1sha1'); + assert.equal(mgf.MGF_ALGORITHMS['http://www.w3.org/2001/04/xmlenc#MGF1withSHA1'], 'sha1'); + Object.keys(mgf.MGF_URI_FOR_EMIT).forEach(function (shortName) { + assert(mgf.MGF_URI_FOR_EMIT[shortName].indexOf('http://www.w3.org/2009/xmlenc11#mgf1') === 0, + shortName + ' must emit a normative xmlenc11 URI, got ' + mgf.MGF_URI_FOR_EMIT[shortName]); + }); + }); + it('rejects keyEncryptionMgf under mgf1p instead of silently ignoring it', function (done) { xmlenc.encrypt('x', { rsa_pub: fs.readFileSync(__dirname + '/test-auth0_rsa.pub'), From 52a395d8f488adcb3106aebbbca8ceebfdc2e270 Mon Sep 17 00:00:00 2001 From: Yamil Asusta Date: Fri, 31 Jul 2026 16:04:42 -0400 Subject: [PATCH 14/14] fix: read OAEPparams and MGF through the DOM helpers These three lookups were added on top of the XPath implementation and are the last remaining callers. Route them through dom.child so the OAEP additions use the same scoped resolution as the rest of decryptKeyInfo. test/dom-select.js wraps its fixtures with MGF1-SHA1 here, since rsa-oaep-mgf1p fixes the mask generation function regardless of DigestMethod and crypto.publicEncrypt cannot set the two digests independently. 121 passing. Co-Authored-By: Claude Opus 5 --- lib/xmlenc.js | 6 +++--- test/dom-select.js | 12 +++++------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/xmlenc.js b/lib/xmlenc.js index 300ceb1..8aec1e4 100644 --- a/lib/xmlenc.js +++ b/lib/xmlenc.js @@ -341,21 +341,21 @@ function decryptKeyInfo(doc, options) { } // Read the OAEP label from the optional OAEPparams element (XML-Enc 1.1 5.5.2). - const oaepParams = xpath.select("./*[local-name(.)='OAEPparams']", keyEncryptionMethod)[0]; + const oaepParams = dom.child(keyEncryptionMethod, 'OAEPparams'); const oaepLabel = oaepParams ? Buffer.from(oaepParams.textContent, 'base64') : Buffer.alloc(0); switch (keyEncryptionAlgorithm) { case 'http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p': // The identifier fixes MGF1 to SHA-1 (XML-Enc 1.1 5.5.2); DigestMethod // selects only the OAEP message digest. An xenc11:MGF child is a MUST NOT. - if (xpath.select("./*[local-name(.)='MGF']", keyEncryptionMethod)[0]) { + if (dom.child(keyEncryptionMethod, 'MGF')) { throw new Error('MGF element must not be present with ' + keyEncryptionAlgorithm); } return decryptKeyInfoWithScheme(encryptedKey, options, crypto.constants.RSA_PKCS1_OAEP_PADDING, oaepHash, 'sha1', oaepLabel); case 'http://www.w3.org/2009/xmlenc11#rsa-oaep': { // MGF1 comes from the optional xenc11:MGF child; default MGF1-SHA1. - const mgfElement = xpath.select("./*[local-name(.)='MGF']", keyEncryptionMethod)[0]; + const mgfElement = dom.child(keyEncryptionMethod, 'MGF'); let mgf1Hash = 'sha1'; if (mgfElement) { const mgfAlgorithm = mgfElement.getAttribute('Algorithm'); diff --git a/test/dom-select.js b/test/dom-select.js index 1d84829..478babb 100644 --- a/test/dom-select.js +++ b/test/dom-select.js @@ -1,9 +1,9 @@ var assert = require('assert'); -var crypto = require('crypto'); var fs = require('fs'); var xmldom = require('@xmldom/xmldom'); var xpath = require('xpath'); var dom = require('../lib/dom-select'); +var oaep = require('../lib/oaep'); var xmlenc = require('../lib/xmlenc'); var XENC = 'http://www.w3.org/2001/04/xmlenc#'; @@ -187,13 +187,11 @@ describe('decryptKeyInfo element resolution', function () { var pub = fs.readFileSync(__dirname + '/test-auth0_rsa.pub'); var key = fs.readFileSync(__dirname + '/test-auth0.key'); - // master's decrypt path uses crypto directly; oaepHash follows DigestMethod. + // rsa-oaep-mgf1p fixes MGF1 to SHA-1, so these fixtures must be wrapped that + // way whatever the DigestMethod says. crypto.publicEncrypt cannot express the + // two digests independently. function wrap(symmetricKey, oaepHash) { - return crypto.publicEncrypt({ - key: pub, - padding: crypto.constants.RSA_PKCS1_OAEP_PADDING, - oaepHash: oaepHash - }, symmetricKey); + return oaep.publicEncryptOaep(pub, symmetricKey, { oaepHash: oaepHash, mgf1Hash: 'sha1' }); } it('pairs DigestMethod with the EncryptedKey actually in use', function () {