Skip to content

tls,quic: commonize TLS cert handling between tls, dtls & quic - #64711

Merged
avivkeller merged 2 commits into
nodejs:mainfrom
pimterry:shared-tls-certs-impl
Aug 9, 2026
Merged

tls,quic: commonize TLS cert handling between tls, dtls & quic#64711
avivkeller merged 2 commits into
nodejs:mainfrom
pimterry:shared-tls-certs-impl

Conversation

@pimterry

@pimterry pimterry commented Jul 24, 2026

Copy link
Copy Markdown
Member

We are currently maintaining 3 different layers over OpenSSL's TLS API (node:tls/quic/dtls), which all reimplement the same things in subtly different ways, and will require any fixes to be applied 3x.

This is a first step towards commonizing them to share logic & options directly. Medium term, I want to end up with one API shape for TLS configuration (very close to current node:tls, maybe some additions from QUIC/DTLS) with a 95% shared implementation (some small exceptions for real protocol configuration divergences - but the exposed set of these is small). Consider this a first step and demo to discuss if people are on board. CC @nodejs/crypto.

This PR focuses on cert management:

  • Extracts node:tls CA & cert store logic into separate helpers
  • Uses them in node:quic and node:dtls, dropping the custom equivalents

This notably fixes a few bugs en route:

  • node:quic ran SSL_CTX_set_default_verify_paths every time it initialized a context, doing IO every time, then throwing the configuration away with the subsequent SSL_CTX_set_cert_store call.
  • node:dtls ignored the configured Node CA settings.
  • node:dtls didn't list trusted client CAs in the server handshake response for mtls.

These are all fixed by reusing the existing logic. The helpers are direct moves, so this shouldn't change node:tls behaviour at all - it just changes the other modules to use them. Behaviour of DTLS & QUIC will change in small ways, but only to align with node:tls.

@pimterry
pimterry requested review from jasnell and joyeecheung July 24, 2026 12:39
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/gyp
  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jul 24, 2026
Signed-off-by: Tim Perry <pimterry@gmail.com>
@pimterry
pimterry force-pushed the shared-tls-certs-impl branch from 87f1206 to b9d8979 Compare July 24, 2026 12:44
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.33333% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.14%. Comparing base (fe5037b) to head (fd023cb).
⚠️ Report is 268 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_tls_certificates.cc 71.15% 4 Missing and 11 partials ⚠️
src/crypto/crypto_context.cc 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #64711      +/-   ##
==========================================
- Coverage   90.14%   90.14%   -0.01%     
==========================================
  Files         741      744       +3     
  Lines      242246   242334      +88     
  Branches    45621    45628       +7     
==========================================
+ Hits       218381   218460      +79     
+ Misses      15370    15365       -5     
- Partials     8495     8509      +14     
Files with missing lines Coverage Δ
src/crypto/crypto_context.h 100.00% <ø> (ø)
src/crypto/crypto_context.cc 72.14% <87.50%> (-0.13%) ⬇️
src/crypto/crypto_tls_certificates.cc 71.15% <71.15%> (ø)

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

Renamed to _certificates - this will make the split clearer with the
other incoming future helper files.
@jasnell jasnell added tls Issues and PRs related to the tls subsystem. crypto Issues and PRs related to the crypto subsystem. quic Issues and PRs related to the QUIC implementation / HTTP/3. dtls labels Aug 8, 2026
@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@jasnell jasnell added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Aug 8, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@pimterry pimterry added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 9, 2026
@avivkeller
avivkeller merged commit e79c6b3 into nodejs:main Aug 9, 2026
101 of 102 checks passed
@avivkeller

Copy link
Copy Markdown
Member

Landed in e79c6b3

@avivkeller avivkeller removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 9, 2026
@aduh95 aduh95 added dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. labels Aug 13, 2026
@aduh95

aduh95 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Adding the dont-land labels since this is based on #63182

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. c++ Issues and PRs that require attention from people who are familiar with C++. 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. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. dont-land-on-v24.x PRs that should not land on the v24.x-staging branch and should not be released in v24.x. dont-land-on-v26.x PRs that should not land on the v26.x-staging branch and should not be released in v26.x. dtls lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. quic Issues and PRs related to the QUIC implementation / HTTP/3. tls Issues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants