codec and x.509 hardening#2323
Draft
tob-scott-a wants to merge 4 commits intoRustCrypto:masterfrom
Draft
Conversation
…leak
The previous implementation used `match input.split_last()` and broke out
of the loop on the first non-`=` byte, so the iteration count revealed
which trailing position held the first non-`=`. For well-formed Base32
this leaks only `len % 5` (already public via the documented threat
model), but for malformed inputs an attacker can distinguish "first
trailing char is `=`" vs not by timing.
Replace it with a fixed six-byte fold over the trailing window. The
sticky `all_pad` mask collapses to 0 the first time a non-`=` byte is
seen, contributing nothing to `pad_count` for the rest of the loop,
while every iteration runs unconditionally. Same set of valid/invalid
inputs are still accepted (the chunk loop's `err |= (src_rem.len() ==
N)` checks continue to reject pad-count `{2,5}` indirectly).
All existing tests pass — 2 unit + 5 proptest + 5 vector cases — and
the change leaves the behavioral contract unchanged. Closes the timing
half of the maintainer TODO at the old line 246.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a research-preview gate that compiles wrapper shims around the public encode/decode APIs of `base16ct`, `base32ct`, and `base64ct` with fixed-length, `core::hint::black_box`-anchored buffers, dumps the emitted assembly via `rustc --emit=asm`, and reports per-wrapper conditional-branch counts. The CT-critical inner functions (`decode_nibble`, `decode_5bits`, `decode_6bits`, `is_pad_ct`) are `#[inline(always)]` and not exported. What matters for CT correctness is what they look like *after* LLVM has inlined them into the public path real callers use — so the wrappers exist to anchor that path so the asm scanner sees the actual machinery. Snapshotted baseline (`aarch64-apple-darwin`, rustc 1.85): five `base16ct` wrappers and two each of `base32ct`/`base64ct` are fully branch-free; remaining counts are length-conditional or panic-trampoline branches that LLVM didn't fold (not CT bugs). The intended use is `./check.sh --baseline | diff baseline.<arch>.txt` as a regression-detection gate — any nonzero diff in a `branches=0` row is a CT-violation candidate worth manual review. Known limitations are documented in the README: panic-target filtering is not yet implemented, conditional moves (`cmov`/`csel`) are not flagged, memory-access-pattern leaks aren't caught, and not all `base64ct` variants are separately wrapped (they share machinery via the `Alphabet` trait). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the C2SP x509-limbo certificate corpus into `x509-cert/tests/`.
The harness loads `limbo.json`, walks every testcase, decodes every
embedded certificate via `Certificate::from_der`, and exercises the
`BasicConstraints` / `KeyUsage` / `ExtendedKeyUsage` / `SubjectAltName`
/ `NameConstraints` field decoders. Two assertions guard the suite:
1. Every `expected_result == SUCCESS` testcase's certs MUST decode.
2. Every `expected_result == FAILURE` testcase tagged as
`malformed-cert` MUST be rejected by `Certificate::from_der`.
Chain-validation tests (CRL handling, path-length, EKU policy, name
constraints, signature verification) are out of scope — `x509-cert` is
a decoder, not a validator. Those testcases are counted but not
asserted; the harness's gap report (`tests/limbo/README.md`) classifies
each limbo feature as EXERCISED / PUNTED / OUT-OF-SCOPE.
Results against `C2SP/x509-limbo@086b0da8b83d78ed0f491d6df6672b2673406500`
(9,773 testcases, 39,497 individual certificates):
expected-pass decoded: 928 / 928 (100%)
expected-fail rejected at from_der: 1 / 8,845
malformed-cert decoded silently: 0 (assertion holds)
The 8,844 expected-fail testcases that decode without complaint are
chain-validation-layer failures (e.g. `crl::*`, `pathlen::*`,
`invalid::invalid-issuer-key`); not bugs in `x509-cert`.
The 39MB `limbo.json` fixture is gitignored. `tests/limbo/fetch.sh`
shallow-clones a pinned upstream SHA via `gh repo clone` + `git fetch`,
copies the fixture into place, and verifies the SHA before fetching.
The harness skips cleanly when the fixture is absent so offline builds
and consumers who don't run `fetch.sh` are unaffected.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the `Certificate` re-export ahead of the `der` and `ext` modules
in the `use x509_cert { ... }` group so `cargo fmt --all -- --check`
passes. Pure formatting; no behavior change.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tarcieri
reviewed
Apr 29, 2026
| @@ -0,0 +1,119 @@ | |||
| # ct-disasm — constant-time disassembly check (research preview) | |||
Member
There was a problem hiding this comment.
For something like this it would probably be better to put it in https://github.com/rustcrypto/utils though it does look interesting
tarcieri
reviewed
Apr 29, 2026
Comment on lines
+3
to
+4
| A regression-detection tool for the constant-time hot paths in `base16ct`, | ||
| `base32ct`, and `base64ct`. Compiles a small wrapper crate that drives each |
Member
There was a problem hiding this comment.
Oh hmm, if it's specialized to these I guess it can go here
Member
|
My understanding is that x509-limbo is more geared towards validation logic. x509-cert does not handle validation. @carl-wallace made a validator (which I recommend and use) over in https://github.com/carl-wallace/rust-pki/tree/main/certval I'd love if this could be adopted here at some point. I'd rather see this dropped from here for now. |
baloo
reviewed
May 2, 2026
| continue | ||
| fi | ||
|
|
||
| COUNT=$(printf '%s\n' "$BODY" | grep -cE "(^|[[:space:]])($BRANCH_REGEX)([[:space:]]|$)" || true) |
Member
There was a problem hiding this comment.
Suggested change
| COUNT=$(printf '%s\n' "$BODY" | grep -cE "(^|[[:space:]])($BRANCH_REGEX)([[:space:]]|$)" || true) | |
| COUNT=$(printf '%s\n' "$BODY" | grep -cE "^[[:space:]]*($BRANCH_REGEX)[[:space:]]*$" || true) |
Am I reading this regexp wrong?
Surely there is a better way to read assembly than a regex anyway? That seems like it's looking for false-positives.
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
3 participants
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.
Note
This is all being submitted as one branch for discussion. If you prefer each to be individual branches / pull requests, I will decompose this into multiple. I'm submitting this as a draft PR for now. If you're happy with all 3 changes and want them in one go, I'm also happy to flip it into a ready-to-merge PR.
During the course of testing internal Claude skills I'm developing, I tasked them with finding/mitigating side-channels across RustCrypto. It yielded a lot of false positives and not much worth writing about.
Some follow-up work:
remove_paddinghad a TODO; 4e5982c aspires to address it