forked from rustls/rustls
-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add ML-DSA signature algorithms to fingerprint emulation #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
barjin
merged 3 commits into
apify:impit-main
from
jackc625:feat/ml-dsa-fingerprint-sigalgs
Aug 5, 2026
+28
−0
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an emulated fingerprint advertises an ML-DSA scheme and a TLS 1.3 peer selects it while presenting an Ed25519 key, an ordinary Ed25519
CertificateVerifymislabeled as ML-DSA is accepted becauseverify_tls13_signatureinvokes the first algorithm in this mapping. This silently treats two distinct signature schemes as interchangeable; keep the scheme advertise-only but make verification fail closed, including handling an empty mapping without indexing it.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Real finding, and it is the tradeoff described in the PR body rather than something new — but the severity is lower than P1 implies, and there is a smaller fix than the one suggested.
Severity. Triggering this requires a valid Ed25519 signature over the handshake transcript, which only the legitimate key holder can produce. An attacker without the key cannot forge one, and an attacker who has it gains nothing that an ordinary handshake would not already give them. So this is a conformance defect — RFC 8446 §4.4.3, two distinct schemes treated as interchangeable — not an authentication bypass. Reachability is narrow as well: it needs a server that presents an Ed25519 certificate and deliberately mislabels its CertificateVerify, and no publicly-trusted CA issues Ed25519 TLS server certificates today.
Still worth fixing. Fail-closed is the right default here.
Fix. Instead of handling the empty case at each indexing site,
convert_schemeis the single choke point for all three verify paths (verify.rs:161,:198,:220):with
ML_DSA_FALLBACKreplaced by the existingEMPTY.That keeps ML-DSA advertised —
supported_schemes()reads only the mapping keys, so the ClientHello and the emulated JA4 are unchanged — while making both[0]indexing sites structurally unreachable rather than safe by convention. It is a no-op for current behaviour: all thirteen existing mapping entries are non-empty, so the filter never fires today.This is one line in
webpki/verify.rs, so whether the fork wants that delta against upstream is your call. Happy to push it, or to drop the placeholder approach some other way and keep the change confined to the emulation module. Either way one imprecision remains:SignedHandshakeWithUnadvertisedSigSchemeis not quite the right name for "advertised, but no verifier available".