Skip to content

crypto: harden X509Certificate state - #65518

Open
panva wants to merge 3 commits into
nodejs:mainfrom
panva:crypto-harden-x509-state
Open

crypto: harden X509Certificate state#65518
panva wants to merge 3 commits into
nodejs:mainfrom
panva:crypto-harden-x509-state

Conversation

@panva

@panva panva commented Aug 24, 2026

Copy link
Copy Markdown
Member

This follows the prior KeyObject and CryptoKey hardening done in #63111 and applies the same native/private-slot model to X509Certificate.

Construct X509Certificate instances directly on the native X.509 wrapper instead of storing the wrapper and lazy cache behind observable symbols. Cached values remain in private state, receiver validation uses a native brand check.

Furthermore, given the learnings from hardening X509Certificate without losing performance, cache constructor-known KeyObject types and normal CryptoKey slots while retaining the native fallback for transferred keys.

The added benchmark comparison shows no statistically significant regressions. Only improvements in construction and property access across the three hardened classes.

                                               confidence  improvement  (*)     (**)    (***)
KeyObject construction                                    -0.42%       ±2.70%  ±3.60%  ±4.72%
KeyObject symmetricKeySize first                           0.42%       ±1.50%  ±2.00%  ±2.60%
KeyObject type first                            ***       55.39%       ±1.34%  ±1.79%  ±2.35%
KeyObject type                                  ***       11.63%       ±0.85%  ±1.13%  ±1.47%
KeyObject equals                                          -0.44%       ±0.92%  ±1.22%  ±1.59%

CryptoKey construction                          ***       76.95%       ±3.80%  ±5.07%  ±6.63%
CryptoKey algorithm first                       ***       63.99%       ±1.62%  ±2.16%  ±2.81%
CryptoKey extractable first                     ***       83.19%       ±3.40%  ±4.54%  ±5.92%
CryptoKey type first                            ***       82.60%       ±4.11%  ±5.48%  ±7.15%
CryptoKey usages first                          ***       84.15%       ±3.21%  ±4.28%  ±5.58%
CryptoKey type                                             0.72%       ±0.97%  ±1.29%  ±1.68%
CryptoKey to KeyObject                                     0.48%       ±1.62%  ±2.16%  ±2.83%

X509Certificate construction                               0.83%       ±1.84%  ±2.45%  ±3.21%
X509Certificate checkHost                                 -0.35%       ±1.13%  ±1.51%  ±1.96%
X509Certificate publicKey first                  **        3.21%       ±1.98%  ±2.65%  ±3.46%
X509Certificate publicKey                       ***       34.26%       ±0.89%  ±1.18%  ±1.54%
X509Certificate subject first                   ***        2.98%       ±1.59%  ±2.11%  ±2.75%
X509Certificate subject                         ***       33.24%       ±1.44%  ±1.93%  ±2.53%

panva added 3 commits August 24, 2026 17:09
Measure construction and low-cost method paths for crypto classes whose
state is stored in native or private slots.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Cache constructor-known KeyObject types in the existing private slot
until a native handle is needed. Prime normal CryptoKey slot arrays
from constructor arguments while retaining the native fallback for
transferred keys.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Construct X509Certificate instances directly on the existing native
X.509 wrapper and keep lazy cached values in private state. Use a
non-throwing native brand check while preserving the existing
structured-clone behavior.

Preserve derived-constructor prototype semantics without rereading
new.target.prototype.

Add coverage for receiver validation, hidden state, prototype edge
cases, subclassing, and structured cloning through workers and message
ports.

Replace the key-only instanceof lint rule with a module-aware crypto
class guard and extend it to X509Certificate. Resolve references by
lexical binding so shadowed constructor names remain valid.

Signed-off-by: Filip Skokan <panva.ip@gmail.com>
@panva panva added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label Aug 24, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/performance

@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 Aug 24, 2026
@panva panva added crypto Issues and PRs related to the crypto subsystem. performance Issues and PRs related to the performance of Node.js. labels Aug 24, 2026
@nodejs-github-bot

This comment was marked as outdated.

@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.54808% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.15%. Comparing base (f509cf1) to head (46c6571).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_x509.cc 72.50% 7 Missing and 15 partials ⚠️
lib/internal/crypto/x509.js 96.91% 8 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #65518    +/-   ##
========================================
  Coverage   90.14%   90.15%            
========================================
  Files         751      751            
  Lines      253585   253707   +122     
  Branches    47772    47808    +36     
========================================
+ Hits       228596   228719   +123     
+ Misses      16228    16220     -8     
- Partials     8761     8768     +7     
Files with missing lines Coverage Δ
lib/internal/crypto/keys.js 98.07% <100.00%> (+0.03%) ⬆️
src/crypto/crypto_x509.h 52.94% <ø> (ø)
lib/internal/crypto/x509.js 96.54% <96.91%> (+4.83%) ⬆️
src/crypto/crypto_x509.cc 73.06% <72.50%> (-1.24%) ⬇️

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

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

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. crypto Issues and PRs related to the crypto subsystem. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. performance Issues and PRs related to the performance of Node.js.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants