Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions rustls/src/crypto/emulation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ pub enum FingerprintSignatureAlgorithm {
// EdDSA algorithms
Ed25519,
Ed448,
// ML-DSA algorithms (draft-ietf-tls-mldsa)
MlDsa44,
MlDsa65,
MlDsa87,
// Legacy
EcdsaSha1Legacy,
}
Expand All @@ -274,6 +278,9 @@ impl FingerprintSignatureAlgorithm {
Self::RsaPkcs1Sha1 => SignatureScheme::RSA_PKCS1_SHA1,
Self::Ed25519 => SignatureScheme::ED25519,
Self::Ed448 => SignatureScheme::ED448,
Self::MlDsa44 => SignatureScheme::ML_DSA_44,
Self::MlDsa65 => SignatureScheme::ML_DSA_65,
Self::MlDsa87 => SignatureScheme::ML_DSA_87,
Self::EcdsaSha1Legacy => SignatureScheme::ECDSA_SHA1_Legacy,
}
}
Expand Down Expand Up @@ -416,6 +423,9 @@ impl FingerprintSignatureAlgorithm {
Self::Ed25519 => ED25519_ALGS,
// Ed448 is not supported by webpki, SHA1 legacy uses fallback in mapping
Self::Ed448 | Self::RsaPkcs1Sha1 | Self::EcdsaSha1Legacy => EMPTY,
// ML-DSA is advertised to match browsers that offer it, but webpki has
// no verifier for it, so it contributes nothing to certificate validation.
Self::MlDsa44 | Self::MlDsa65 | Self::MlDsa87 => EMPTY,
}
}

Expand Down Expand Up @@ -457,6 +467,21 @@ impl FingerprintSignatureAlgorithm {
static ECDSA_SHA1_FALLBACK: &[&dyn pki_types::SignatureVerificationAlgorithm] =
&[webpki_algs::ECDSA_P256_SHA256];
static ED25519: &[&dyn pki_types::SignatureVerificationAlgorithm] = &[webpki_algs::ED25519];
// ML-DSA is advertised for fingerprint accuracy but webpki has no verifier
// for it. The mapping doubles as the list of schemes we offer, so the entry
// has to exist; it must also be non-empty, because the TLS 1.3 verify path
// indexes the first element. This placeholder can never validate an ML-DSA
// signature, so a server that actually selects one fails the handshake
// cleanly instead of panicking.
//
// Ed25519 rather than a P-256 verifier: the placeholder does get run, so a
// server whose certificate key matches it could have a CertificateVerify
// mislabelled as ML-DSA accepted. Only the legitimate key holder can produce
// such a signature, but P-256 is the common case for publicly-trusted server
// certificates whereas Ed25519 is not issued by public CAs, so this keeps the
// window as small as the fallback approach allows.
static ML_DSA_FALLBACK: &[&dyn pki_types::SignatureVerificationAlgorithm] =
&[webpki_algs::ED25519];
Comment on lines +483 to +484

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reject ML-DSA rather than verifying it as Ed25519

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 CertificateVerify mislabeled as ML-DSA is accepted because verify_tls13_signature invokes 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 👍 / 👎.

Copy link
Copy Markdown
Author

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_scheme is the single choke point for all three verify paths (verify.rs:161, :198, :220):

.filter_map(|item| if item.0 == scheme { Some(item.1) } else { None })
.filter(|algs| !algs.is_empty())
.next()
.ok_or_else(|| PeerMisbehaved::SignedHandshakeWithUnadvertisedSigScheme.into())

with ML_DSA_FALLBACK replaced by the existing EMPTY.

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: SignedHandshakeWithUnadvertisedSigScheme is not quite the right name for "advertised, but no verifier available".


match self {
Self::EcdsaSecp256r1Sha256 => {
Expand All @@ -481,6 +506,9 @@ impl FingerprintSignatureAlgorithm {
Self::Ed25519 => Some((SignatureScheme::ED25519, ED25519)),
// Ed448 is not supported
Self::Ed448 => None,
Self::MlDsa44 => Some((SignatureScheme::ML_DSA_44, ML_DSA_FALLBACK)),
Self::MlDsa65 => Some((SignatureScheme::ML_DSA_65, ML_DSA_FALLBACK)),
Self::MlDsa87 => Some((SignatureScheme::ML_DSA_87, ML_DSA_FALLBACK)),
}
}
}
Expand Down
Loading