crypto: add crypto.parsePKCS12() by bmuenzenmeyer · Pull Request #65627 · nodejs/node

bmuenzenmeyer · 2026-08-28T20:00:53Z

Return the private key, end-entity certificate, and other certificates from a PKCS#12 (.p12/.pfx) bundle as a KeyObject and X509Certificate instances.

Reading a .p12 / .pfx bundle from JavaScript today means shelling out to the openssl pkcs12 CLI or taking a userland dependency such as node-forge. In talking to a colleague about this unfortunate missing method in core, I (with Claude) noticed Node.js already parses this internally.

This PR exposes those internals to end users. Our use case is loading identity files supplied by our environment, to be forwarded during MCP tool calls. This allows us to use real identity instead of service account.

Note

This is my first significant contribution to core that touches the internals. I am still getting my bearings with regard to the module mechanics, bindings, and c++. I'm committed to shaping this, but learning.

SecureContext::LoadPKCS12 has backed tls's pfx option for years, but its results are loaded straight into an SSL_CTX and never reach JavaScript.

const { privateKey, certificate } = parsePKCS12(
  readFileSync('bundle.p12'),
  { passphrase: 'secret' },
);

Returns { privateKey: KeyObject|null, certificate: X509Certificate|null, additionalCertificates: X509Certificate[] }.

nodejs-github-bot · 2026-08-28T20:00:59Z

Review requested:

@nodejs/crypto
@nodejs/gyp

panva

The direction looks alright. I noted a few issues to work through.

panva · 2026-08-28T21:27:26Z

+// OpenSSL treats NULL and "" differently for both MAC verification and bag
+// decryption, so callers must not collapse them. PKCS12_parse() verifies the
+// MAC itself when one is present, and tolerates the NULL / "" ambiguity.
+PKCS12ParseResult ParseBundle(const BIOPointer& bio, const char* pass) {


Could this direct parser be shared with SecureContext::LoadPKCS12() instead of maintaining two implementations of the same decoding? TLS can consume the shared parse result and then apply its stricter requirement that both a key and certificate are present.

would you be open to a follow up PR to do this? I'd considered the same pathway you suggest here for DRYness sake, but opted to keep the scope smaller and reviewable especially as I am learning the codebase

I dunno, is it a big shift?

if you are open to it, i'll look :) just trying to guard everyone's time

codecov · 2026-08-28T21:38:40Z

Codecov Report

❌ Patch coverage is 86.66667% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (f8d6d75) to head (06b6a3b).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
src/crypto/crypto_pkcs12.cc	77.31%	8 Missing and 14 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #65627      +/-   ##
==========================================
+ Coverage   90.07%   90.12%   +0.05%     
==========================================
  Files         769      770       +1     
  Lines      261396   261613     +217     
  Branches    49629    49681      +52     
==========================================
+ Hits       235451   235787     +336     
+ Misses      16965    16840     -125     
- Partials     8980     8986       +6

Files with missing lines	Coverage Δ
lib/crypto.js	`93.71% <100.00%> (+0.03%)`	⬆️
lib/internal/crypto/keys.js	`98.14% <100.00%> (+0.08%)`	⬆️
src/node_crypto.cc	`81.81% <ø> (ø)`
src/crypto/crypto_pkcs12.cc	`77.31% <77.31%> (ø)`

... and 69 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.

Return the private key, end-entity certificate, and any other non-matching certificates from a PKCS#12 (.p12/.pfx) bundle as a KeyObject and X509Certificate instances. Node.js already parses PKCS#12 in SecureContext::LoadPKCS12, which backs tls's `pfx` option, but the results are consumed directly into an SSL_CTX and never reach JavaScript. Callers who need the key or the certificates for anything other than an immediate TLS connection have to shell out to `openssl pkcs12` or take a userland dependency. The binding wraps d2i_PKCS12_bio() and PKCS12_parse() and follows their semantics, matching the existing TLS path: the first private key is returned, the end-entity certificate is the one associated with that key, and any remaining certificates are returned through `additionalCertificates`. A bundle containing no private key reports `certificate` as null and returns its certificates through `additionalCertificates`. Absent and empty passphrases are kept distinct, since OpenSSL treats them differently. Bundles that require OpenSSL's legacy provider throw ERR_CRYPTO_UNSUPPORTED_OPERATION, reusing the error added for the TLS path. Signed-off-by: bmuenzenmeyer <[email protected]>

jasnell · 2026-09-04T15:51:38Z

+  if (!d2i_PKCS12_bio(bio.get(), &p12_ptr) || p12_ptr == nullptr) {
+    return PKCS12ParseResult(PKCS12ParseError::NOT_RECOGNIZED,
+                             static_cast<int>(ERR_get_error()));
+  }


Doesn't need to be done now, but it probably makes sense to move this into ncrypto

yeah i picked up on that from some of my earliest research (agent-informed). i am happy to explore the work, but it feels like a follow-up

nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Aug 28, 2026

panva self-requested a review August 28, 2026 20:57

panva reviewed Aug 28, 2026

View reviewed changes

panva added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Aug 29, 2026

bmuenzenmeyer force-pushed the pkcs12 branch from 2d6065c to 1bc0b49 Compare September 4, 2026 10:43

panva requested a review from pimterry September 4, 2026 10:45

This comment was marked as outdated.

Sign in to view

bmuenzenmeyer marked this pull request as draft September 4, 2026 10:48

bmuenzenmeyer force-pushed the pkcs12 branch 3 times, most recently from dc7a151 to dbcba0a Compare September 4, 2026 13:01

bmuenzenmeyer marked this pull request as ready for review September 4, 2026 13:11

bmuenzenmeyer force-pushed the pkcs12 branch from dbcba0a to 3a8e383 Compare September 4, 2026 15:14

bmuenzenmeyer force-pushed the pkcs12 branch from 3a8e383 to 06b6a3b Compare September 4, 2026 15:23

jasnell reviewed Sep 4, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

bmuenzenmeyer commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Aug 28, 2026

Uh oh!

panva left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

panva Aug 28, 2026

Choose a reason for hiding this comment

Uh oh!

bmuenzenmeyer Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

panva Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

bmuenzenmeyer Aug 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

jasnell Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

bmuenzenmeyer Sep 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bmuenzenmeyer commented Aug 28, 2026 •

edited

Loading

codecov Bot commented Aug 28, 2026 •

edited

Loading