Skip to content

crypto: fix public PKCS8 export error - #65609

Open
koreahghg wants to merge 1 commit into
nodejs:mainfrom
koreahghg:crypto-fix-pkcs8-export-invalidaccesserror
Open

crypto: fix public PKCS8 export error#65609
koreahghg wants to merge 1 commit into
nodejs:mainfrom
koreahghg:crypto-fix-pkcs8-export-invalidaccesserror

Conversation

@koreahghg

Copy link
Copy Markdown

Summary

exportKeySpki() already rejects exporting a private key as 'spki'
with an InvalidAccessError, per the Web Crypto export key algorithm
steps for RSA, EC, CFRG, ML-DSA, and ML-KEM. exportKeyPkcs8() was
missing the symmetric check: exporting a public key as 'pkcs8'
fell through to the generic NotSupportedError ("Unable to export
... key using pkcs8 format") instead of the spec-mandated
InvalidAccessError.

This adds the same key-type check to exportKeyPkcs8(), mirroring
exportKeySpki(), and drops the now-redundant type guard around its
call site in exportKeySync(). This also fixes wrapKey(), which
delegates to the same export path (subtle.wrapKey('pkcs8', publicKey, ...)
previously threw the wrong error type too).

Test plan

  • Added subtle.exportKey('pkcs8', publicKey) rejection coverage
    in test/parallel/test-webcrypto-export-import-ec.js
  • Added subtle.wrapKey('pkcs8', publicKey, ...) rejection coverage
    in test/parallel/test-webcrypto-wrap-unwrap.js
  • node --check on all changed files
  • Full make test (please run in CI; no local build environment was
    available when preparing this patch)

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Aug 28, 2026
@panva
panva self-requested a review August 28, 2026 05:51

@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.

@koreahghg thank you, please change the first commit message line to

crypto: fix public PKCS8 export error

mirroring the other fix done in 3615979

exportKeySpki() already rejects exporting a private key as 'spki'
with an InvalidAccessError, per the Web Crypto export key algorithm
steps for each of RSA, EC, CFRG, ML-DSA, and ML-KEM. exportKeyPkcs8()
was missing the symmetric check: exporting a public key as 'pkcs8'
fell through to the generic "Unable to export ... key using pkcs8
format" NotSupportedError instead of the spec-mandated
InvalidAccessError.

Add the same key-type check to exportKeyPkcs8(), mirroring
exportKeySpki(), and drop the now-redundant type guard around its
call site in exportKeySync(). This also fixes wrapKey(), which
delegates to the same export path.

Add test coverage for both subtle.exportKey('pkcs8', publicKey) and
subtle.wrapKey('pkcs8', publicKey, ...).

Signed-off-by: koreahghg <koreahghg@gmail.com>
@koreahghg
koreahghg force-pushed the crypto-fix-pkcs8-export-invalidaccesserror branch from 298b584 to 1a4fc6f Compare August 28, 2026 06:24
@koreahghg

Copy link
Copy Markdown
Author

Updated the commit message per review, thanks!

@panva panva changed the title crypto: reject pkcs8 export of public keys crypto: fix public PKCS8 export error Aug 28, 2026
@panva panva added author ready PRs with CI started, the required approvals, and no outstanding review comments. request-ci Add this label to start a Jenkins CI on a PR. labels Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (9f04fcd) to head (1a4fc6f).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65609      +/-   ##
==========================================
- Coverage   90.07%   90.06%   -0.01%     
==========================================
  Files         751      751              
  Lines      254916   254920       +4     
  Branches    48133    48125       -8     
==========================================
- Hits       229605   229588      -17     
  Misses      16496    16496              
- Partials     8815     8836      +21     
Files with missing lines Coverage Δ
lib/internal/crypto/webcrypto.js 97.77% <100.00%> (+<0.01%) ⬆️

... and 34 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 has been minimized.

@panva panva removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 28, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the webcrypto Issues and PRs related to the Web Crypto API. label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. webcrypto Issues and PRs related to the Web Crypto API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants