Defer X509_NAME canonicalization until first use - #3383
Draft
jakemas wants to merge 2 commits into
Draft
Conversation
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.
Contributor
|
🔒 Security Review — View 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
force-pushed
the
x509-defer-name-canon
branch
from
July 29, 2026 22:34
feeac83 to
9b84766
Compare
jakemas
force-pushed
the
x509-defer-name-canon
branch
from
July 29, 2026 22:39
9b84766 to
108912e
Compare
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
Contributor
Author
|
hmm still not fast enough.. open to ideas |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
d2i_X509eagerly canonicalizes both the issuer and subjectX509_NAMEduring parsing. Profiling shows this dominates parse cost — on an RSA-2048 leaf it accounts for ~40% ofd2i_X509and 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_canonlazily 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:
x509_name_ex_d2inow 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.X509_NAMEis frequently shared read-only across threads (e.g. a CA certificate cached in anX509_STOREand 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_canonserializes 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 thei2d_X509_NAMEmodified path — go through this one locked helper, keeping the locking discipline uniform. The parse-only fast path never takes the lock.Testing
All
crypto_testcases pass. The change is clean under ThreadSanitizer, including a stress test that concurrently runsX509_NAME_cmp,X509_NAME_hash, andi2d_X509_NAMEagainst a single shared parsed name.Local microbenchmark:
d2i_X509on 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.