Skip to content

Defer X509_NAME canonicalization until first use - #3383

Draft
jakemas wants to merge 2 commits into
aws:mainfrom
jakemas:x509-defer-name-canon
Draft

Defer X509_NAME canonicalization until first use#3383
jakemas wants to merge 2 commits into
aws:mainfrom
jakemas:x509-defer-name-canon

Conversation

@jakemas

@jakemas jakemas commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description of changes

d2i_X509 eagerly canonicalizes both the issuer and subject X509_NAME during parsing. Profiling shows this dominates parse cost — on an RSA-2048 leaf it accounts for ~40% of d2i_X509 and scales with the number of RDNs — even though the canonical encoding is only consumed by name comparison (X509_NAME_cmp, X509_NAME_hash) and directoryName name-constraint checks. Most parsed certificates never have their names compared, so this work is wasted.

This change defers canonicalization until the encoding is actually needed. A new helper x509_name_ensure_canon lazily builds and caches it on first use; the comparison, hash, and name-constraint paths call it instead of assuming the encoding is present after parse.

Two properties of the previous behavior are preserved:

  • Input validation. Canonicalization implicitly validated each attribute value (e.g. rejecting malformed UTF-8). x509_name_ex_d2i now runs a validate-only pass over attribute values at parse time, so malformed names are still rejected on parse — without building or caching the full canonical encoding.
  • Thread safety. A parsed X509_NAME is frequently shared read-only across threads (e.g. a CA certificate cached in an X509_STORE and compared during concurrent verifies). Previously canonicalization happened at parse time and comparison was a pure read; naively deferring it would turn the first comparison into an unsynchronized write on a shared object. x509_name_ensure_canon serializes the lazy computation with a per-name mutex. All callers that may compute the canonical encoding — X509_NAME_cmp, X509_NAME_hash, nc_dn, and the i2d_X509_NAME modified path — go through this one locked helper, keeping the locking discipline uniform. The parse-only fast path never takes the lock.

Testing

All crypto_test cases pass. The change is clean under ThreadSanitizer, including a stress test that concurrently runs X509_NAME_cmp, X509_NAME_hash, and i2d_X509_NAME against a single shared parsed name.

Local microbenchmark: d2i_X509 on an RSA-2048 leaf improves ~29% (17.8µs → 12.8µs); RDN-heavy names benefit most.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

X509_NAME_ex_d2i eagerly computed the canonical encoding of every name
during parsing. For a certificate this canonicalizes both the issuer and
subject, re-encoding each RDN, and profiling shows this dominates the cost
of d2i_X509: on an RSA-2048 leaf it accounts for roughly 40% of parse time
and scales with the number of RDNs.

The canonical encoding is only consumed by name comparison (X509_NAME_cmp,
X509_NAME_hash) and directoryName name-constraint checks. Most parsed
certificates never have their names compared, so this work is wasted.

Defer canonicalization until the encoding is actually needed. A new
helper, x509_name_ensure_canon, lazily builds and caches the canonical
encoding on first use, and the comparison/hash/name-constraint paths call
it instead of relying on the encoding being present after parse.

Two properties of the old behavior are preserved:

  - Input validation. Canonicalization implicitly validated each attribute
    value (e.g. rejecting malformed UTF-8). To keep malformed names being
    rejected at parse time, x509_name_ex_d2i now runs a validate-only pass
    over the attribute values without building or caching the full
    canonical encoding.

  - Thread safety. A parsed X509_NAME is frequently shared read-only across
    threads (e.g. a CA certificate cached in an X509_STORE and compared
    during concurrent verifies). Previously canonicalization happened at
    parse time and comparison was a pure read. Deferring it would turn the
    first comparison into an unsynchronized write on a shared object, so
    x509_name_ensure_canon serializes the lazy computation with a per-name
    mutex. The parse-only fast path never takes the lock.

Reduces d2i_X509 on an RSA-2048 leaf by ~29% (17.8us -> 12.8us) in a local
microbenchmark. All crypto_test cases pass and the change is clean under
ThreadSanitizer.
@jakemas
jakemas requested a review from a team as a code owner July 29, 2026 22:03
@jakemas jakemas closed this Jul 29, 2026
@jakemas jakemas reopened this Jul 29, 2026
@jakemas
jakemas marked this pull request as draft July 29, 2026 22:10
@github-actions

Copy link
Copy Markdown
Contributor

🔒 Security ReviewView Report

Please review before merging.

x509_name_ex_i2d rebuilt the canonical encoding by calling x509_name_encode
and x509_name_canon directly, outside canon_lock. Since X509_NAME_cmp and
X509_NAME_hash now canonicalize under the lock, a concurrent i2d_X509_NAME on
a modified shared name could run x509_name_canon at the same time as a
comparison, and x509_name_canon frees and reallocates canon_enc — a
double-free / use-after-free.

Route the i2d modified path through x509_name_ensure_canon so every caller
that may compute the canonical encoding does so under the same per-name lock.
x509_name_ensure_canon performs the same encode-then-canon sequence, so the
subsequent read of bytes->length is unchanged.
@jakemas jakemas changed the title Defer X509_NAME canonicalization until first use X.509 parsing performance: defer name canonicalization; add zero-copy X509_skim Jul 29, 2026
github-actions[bot]

This comment was marked as outdated.

@jakemas
jakemas force-pushed the x509-defer-name-canon branch from feeac83 to 9b84766 Compare July 29, 2026 22:34
@jakemas
jakemas force-pushed the x509-defer-name-canon branch from 9b84766 to 108912e Compare July 29, 2026 22:39
@jakemas jakemas changed the title X.509 parsing performance: defer name canonicalization; add zero-copy X509_skim Defer X509_NAME canonicalization until first use Jul 29, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.22%. Comparing base (ea70f68) to head (108912e).

Files with missing lines Patch % Lines
crypto/x509/x_name.c 93.54% 2 Missing ⚠️
crypto/x509/v3_ncons.c 0.00% 1 Missing ⚠️
crypto/x509/x509_cmp.c 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3383      +/-   ##
==========================================
+ Coverage   78.20%   78.22%   +0.01%     
==========================================
  Files         695      695              
  Lines      124220   124240      +20     
  Branches    17261    17262       +1     
==========================================
+ Hits        97148    97186      +38     
+ Misses      26149    26132      -17     
+ Partials      923      922       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jakemas

jakemas commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

hmm still not fast enough.. open to ideas

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants