Skip to content

crypto: add mgf1Hash for RSA-OAEP - #65073

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

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
Contributor

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 50 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

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 8, 2026
@avivkeller avivkeller added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 9, 2026
@avivkeller

Copy link
Copy Markdown
Member
Commit Queue failed
- Loading data for nodejs/node/pull/65073
✔  Done loading data for nodejs/node/pull/65073
----------------------------------- PR info ------------------------------------
Title      crypto: add mgf1Hash for RSA-OAEP (#65073)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     adamjmcgrath:add-mgf1hash -> nodejs:main
Labels     crypto, test, author ready, needs-ci, commit-queue, commit-queue-squash, lts-watch-v24.x
Commits    3
 - crypto: add mgf1Hash for RSA-OAEP
 - crypto: fix lint and tests
 - test: update test
Committers 1
 - Adam Mcgrath <adam.mcgrath@okta.com>
PR-URL: https://github.com/nodejs/node/pull/65073
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/65073
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 06 Aug 2026 08:23:40 GMT
   ✔  Approvals: 1
   ✔  - Filip Skokan (@panva) (TSC): https://github.com/nodejs/node/pull/65073#pullrequestreview-4875510214
   ✘  This PR needs to wait 82 more hours to land (or 0 minutes if there is one more approval)
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-08-06T17:55:09Z: https://ci.nodejs.org/job/node-test-pull-request/75581/
- Querying data for job/node-test-pull-request/75581/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /Users/avivkeller/Documents/projects/nodejs/node/.ncu
/nodejs/node/actions/runs/

@avivkeller avivkeller removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Aug 9, 2026
avivkeller pushed a commit that referenced this pull request Aug 9, 2026
Signed-off-by: Adam Mcgrath <adam.mcgrath@okta.com>
PR-URL: #65073
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
@avivkeller

Copy link
Copy Markdown
Member

Landed in 11423b1

@avivkeller avivkeller closed this Aug 9, 2026
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.

4 participants