Fix/rsa oaep mgf1 - #140
Open
elbuo8 wants to merge 14 commits into
Open
Conversation
elbuo8
commented
Jul 31, 2026
| // Resolve DigestMethod relative to the EncryptionMethod we already located, | ||
| // not by an absolute path: with EncryptedData/KeyInfo/RetrievalMethod the | ||
| // EncryptedKey lives outside KeyInfo and an anchored XPath finds nothing. | ||
| const keyDigestMethod = xpath.select("./*[local-name(.)='DigestMethod']", keyEncryptionMethod)[0]; |
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…sZero/isOne 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 <noreply@anthropic.com>
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.
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.
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
semantic-release derives the version from the commit history; naming 5.1.0 in prose was both unverifiable and wrong for a breaking change.
- 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
elbuo8
force-pushed
the
fix/rsa-oaep-mgf1
branch
from
July 31, 2026 20:20
11da941 to
52a395d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.
Description
References
Testing
Checklist