Skip to content

crypto: add mgf1Hash for RSA-OAEP - #65073

Open
adamjmcgrath wants to merge 3 commits into
nodejs:mainfrom
adamjmcgrath:add-mgf1hash
Open

crypto: add mgf1Hash for RSA-OAEP#65073
adamjmcgrath wants to merge 3 commits into
nodejs:mainfrom
adamjmcgrath:add-mgf1hash

Conversation

@adamjmcgrath

@adamjmcgrath adamjmcgrath commented Aug 6, 2026

Copy link
Copy Markdown

crypto.publicEncrypt() and crypto.privateDecrypt() expose only the oaepHash option, which sets both the OAEP message digest and the MGF1 hash.

As a result, Node cannot decrypt (or produce) an RSA-OAEP ciphertext where the OAEP digest and the MGF1 hash differ — e.g. OAEP digest = SHA-256, MGF1 = SHA-1.

OpenSSL exposes the two digests independently via EVP_PKEY_CTX_set_rsa_oaep_md and EVP_PKEY_CTX_set_rsa_mgf1_md so this PR adds an mgf1Hash option that, when set, calls EVP_PKEY_CTX_set_rsa_mgf1_md independently of oaepHash.

This is required to implement http://www.w3.org/2001/04/xmlenc#rsa-oaep-mgf1p where the message digest can be configurable (e.g. SHA-256) but the mask generation function must be fixed to SHA-1.

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 6, 2026
@adamjmcgrath
adamjmcgrath marked this pull request as ready for review August 6, 2026 09:56
@panva panva added crypto Issues and PRs related to the crypto subsystem. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lts-watch-v24.x PRs that may need to be released in v24.x labels Aug 6, 2026

@panva panva left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Adam

@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 6, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 6, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Signed-off-by: Adam Mcgrath <adam.mcgrath@okta.com>
Signed-off-by: Adam Mcgrath <adam.mcgrath@okta.com>
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (27d6cfa) to head (0a04e0b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_cipher.cc 55.55% 0 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65073      +/-   ##
==========================================
- Coverage   90.31%   90.31%   -0.01%     
==========================================
  Files         759      759              
  Lines      248258   248269      +11     
  Branches    46854    46856       +2     
==========================================
- Hits       224225   224220       -5     
+ Misses      15465    15461       -4     
- Partials     8568     8588      +20     
Files with missing lines Coverage Δ
lib/internal/crypto/cipher.js 98.01% <100.00%> (+0.01%) ⬆️
src/crypto/crypto_cipher.h 68.47% <ø> (ø)
src/crypto/crypto_rsa.cc 64.21% <100.00%> (+0.12%) ⬆️
src/crypto/crypto_cipher.cc 76.51% <55.55%> (-0.41%) ⬇️

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. crypto Issues and PRs related to the crypto subsystem. lts-watch-v24.x PRs that may need to be released in v24.x needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants