fix(tls): avoid repeated certificate signature checks

[MavenRain]

Description

TLS 1.3 transcript verification currently repeats the certificate and libp2p identity-extension signature checks that rustls has already required through its certificate callback. Verify only the transcript signature at this boundary, reducing a successful full QUIC handshake from seven to five signature checks per endpoint. The three certificate parses remain.

The crate-private helper borrows the certificate supplied by rustls, returns no PeerId, and adds no shared verifier state. The public verified parser, expected-peer checks, and post-establishment QUIC identity extraction retain their contracts. Add regression tests, reproducible release-mode profiles, RSA fixtures, and the TLS 0.7.1 changelog/version metadata.

Related to #6633.

AI Assistance Disclosure

Tools used: OpenAI Codex, for implementation, profiling, tests, and PR preparation.

Attestation:

• I have read every line of this diff, understand what it does, and can explain it in review.

This personal attestation is left for the contributor to complete during draft review.

Notes & open questions

Local medians on an Apple M2 Pro, rustc 1.95.0, rustls 0.23.45, AWS-LC for TLS/key exchange, and ring for the custom certificate verification:

CPU measurement	Before	After
Default transcript callback	128.768 us	49.464 us
Full handshake, both endpoints	771.458 us	612.128 us
Resumed handshake, both endpoints	333.702 us	324.060 us

The full-handshake reduction is 20.7%. These are in-memory rustls QUIC handshakes including verified PeerId extraction, excluding UDP, packet encryption, and network scheduling. Profiles use 10 warmups and seven samples, with 100 handshakes or 200 component operations per sample. First-flight and resumed-handshake variation does not establish an improvement. The roughly 80 us saved per endpoint corresponds to about 1% of one core at 125 full handshakes/second, as a linear CPU-budget estimate.

Profiles cover the nine advertised signature schemes and concurrent handshakes. Reproduce with:

cargo test --release -p libp2p-tls --lib profile_handshakes -- --ignored --nocapture
cargo test --release -p libp2p-tls --lib profile_rsa_signatures -- --ignored --nocapture

For a baseline, restore the transcript callback's certificate::parse(cert)?.verify_signature(signature_scheme, message, signature)?; call and retain the same profile source, providers, and lockfile.

Validation:

• 40 TLS/QUIC tests passed; Clippy with warnings denied and workspace formatting passed.
• After the version bump, a locked, offline release build check passed for the full workspace and all targets with libp2p-quic/tokio enabled.
• Regression tests independently corrupt certificate, identity-extension, and transcript signatures in both directions, check expected PeerIds, and exercise concurrency, cancellation, and session isolation.
• An external local QUIC harness passed all nine combinations of patched TLS, published 0.7.0, and published 0.6.2 configurations using a common current QUIC engine. This is not a historical QUIC dependency or cross-language matrix.
• The workspace run covered 111 test binaries and initially failed five cases. Three passed isolated retries; IPv6 mDNS discovery and WebRTC CryptoProvider initialization still failed outside the changed verifier path.
• Existing RSA negotiation restrictions were reproduced with the original callback. Default successful RSA handshake coverage uses RSA-PSS-SHA512; component tests cover all six RSA schemes.

Change checklist

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works
• A changelog entry has been made in the appropriate crates

[grantkee]

Everything LGTM @MavenRain ! I flagged a few things for you to consider. Overall, I think this is a subtle but important optimization. Looking forward to reviewing any additional feedback from the libp2p team 🙏

[grantkee]

This assertion pins a pre-existing mislabel: a corrupted certificate self-signature surfaces as SignatureAlgorithmMismatch only because a map_err in certificate.rs discards the real InvalidSignatureForPublicKey error.

The code I'm actually asking about is transports/tls/src/certificate.rs:416-417, which this PR doesn't change. Commenting here because this is the assertion that locks the label in.

Tracing the InvalidProof::Certificate case: the test flips the last DER byte (:93), which sits inside the signature BIT STRING, so the certificate still parses. rustls runs verify_client_cert / verify_server_cert before verify_tls13_signature, so the failure happens in verify_presented_certs → certificate::parse → verify(). There signature_scheme() succeeds and verify_signature fails at certificate.rs:330-331 with webpki::Error::InvalidSignatureForPublicKey, and then

self.verify_signature(signature_scheme, raw_certificate, signature)
    .map_err(|_| Error::SignatureAlgorithmMismatch)?;

throws that away. From<ParseError> in verifier.rs:238-247 wraps the result as InvalidCertificate(Other(SignatureAlgorithmMismatch)). That map_err is the only source of the label on this path: :349 only fires when the scheme passed in differs from self.signature_scheme(), which can't happen inside verify(), and rustls 0.23.45 never produces it. The same map_err also relabels the UnsupportedSignatureAlgorithmContext errors for P-521 and Ed448 (:360, :368). webpki documents SignatureAlgorithmMismatch as "TBSCertificate signature field does not match the algorithm in the signature", which isn't this failure.

The mapping predates this PR (it's certificate.rs:399 on master), so I'm not asking for it to be fixed here. But this is the first test that asserts on the label, and it does so through Debug text, which makes the eventual fix harder than it needs to be.

A structural check would still pin today's behaviour and would be a one-line update when the label is fixed. assert_eq! on the rustls::Error itself can't work because OtherError's PartialEq always returns false (rustls-0.23.45/src/error.rs:1050-1053), and matches!(error, InvalidCertificate(_)) is true for all three arms, so the inner webpki::Error has to be downcast:

fn webpki_cause(error: &rustls::Error) -> Option<&webpki::Error> {
    match error {
        rustls::Error::InvalidCertificate(CertificateError::Other(other)) => {
            other.0.downcast_ref::<webpki::Error>()
        }
        _ => None,
    }
}

and then here:

InvalidProof::Certificate => assert_eq!(
    webpki_cause(&error),
    // Pre-existing label from certificate.rs:417; tracked for follow-up.
    Some(&webpki::Error::SignatureAlgorithmMismatch),
    "{error:?}"
),
InvalidProof::Extension => assert_eq!(
    webpki_cause(&error),
    Some(&webpki::Error::UnknownIssuer),
    "{error:?}"
),

I ran this variant in a scratch copy: the test passes and cargo clippy -p libp2p-tls --all-targets -- -D warnings is clean.

For the follow-up, propagating the real error at :417 with .map_err(|VerificationError(e)| e)? keeps the wire alert unchanged (CertificateUnknown). Additionally mapping InvalidSignatureForPublicKey to CertificateError::BadSignature in From<ParseError> would give the most accurate label but changes the alert to DecryptError, so I'd keep both out of a perf PR.

[MavenRain]

Thanks, replaced the Debug-string checks with webpki_cause downcasts and direct webpki::Error comparisons. Added a comment explaining the existing SignatureAlgorithmMismatch label so a later error-mapping fix can update the assertion directly.

[grantkee]

The regression tests import their handshake helpers from the ignored profiling module, so the required tests depend on optional benchmark code.

All five non-ignored tests in this file (:129, :205, :220, :248, :316) reach certificate_for, client, server and rsa_fixtures through this import and the profile::rsa_fixtures() calls at :226 and :258. Those live in certificate_profile.rs at :39, :115, :127 and :255, next to the two #[ignore = "manual release-mode CPU profile"] tests, and the fixtures are named profile_rsa_*.der for the same reason. Anyone who later wants to prune or rework the harness has to touch the regression tests to do it.

I looked at whether the harness could move to a Criterion benches/ target, since that's the workspace convention (criterion = { version = "0.8" } in the root Cargo.toml, used by identity, swarm, muxers/mplex and misc/quick-protobuf-codec). It mostly can't: the per-step profile in profile_certificate (certificate_profile.rs:55-113) uses parse_unverified, signature_scheme(), the private P2pCertificate / P2pExtension fields, P2P_SIGNING_PREFIX, make_libp2p_extension and the new pub(crate) verify_tls13_signature, none of which a bench target can reach. So the before/after comparison that justifies this PR has a legitimate reason to stay a cfg(test) child of certificate. Only profile_quic_tls (:170-253) is public-API-only.

What I'd suggest instead is decoupling the helpers from the harness:

1. Add a test-only test_support child module of certificate. It has to be a child because certificate_for calls the private make_libp2p_extension.
2. Move certificate_for, client, server, rsa_fixtures and transfer there, keeping pub(super).
3. Here: use super::{test_support::{certificate_for, client, rsa_fixtures, server}, *}; and replace the two profile::rsa_fixtures() calls with rsa_fixtures().
4. In certificate_profile.rs: use super::test_support::{certificate_for, client, rsa_fixtures, server};
5. Optionally rename test_assets/profile_rsa_*.der to something like rsa_fixture_*.der (not rsa_*.der, which collides with the existing rsa_pkcs1_*.der / rsa_pss_sha384.der) and update gen_profile_rsa.py to match.

After that the profile can be kept as is, partly moved to a Criterion bench for the public-API parts, or dropped after merge as a one-off measurement, without touching the regression tests. I wouldn't expose internals (#[doc(hidden)] pub or a feature flag) on a published crate just to benchmark them.

[MavenRain]

Moved all five shared helpers into the test-only certificate::test_support module with pub(super) visibility. The regression tests and profiler now import from there. Also renamed the RSA fixtures and generator to rsa_fixture_*.der and gen_rsa_fixtures.py.

[grantkee]

This entry links the issue where the rest of the file links the PR.

Every other linked entry in this changelog uses a /pull/ link (the wording varies; :20 is See [6357](...)), and repo-wide it's about 1263 /pull/ links to 22 /issues/ links. There is precedent for issue-only entries (protocols/floodsub/CHANGELOG.md:4 uses the identical form, and older entries in noise, kad and multistream-select do too), and nothing in scripts/ensure-version-bump-and-changelog.sh, docs/release.md or CONTRIBUTING.md prescribes the format, so this is a nudge rather than a rule. A few crates cite both (swarm/CHANGELOG.md:135-136, core/CHANGELOG.md:63-64), which is what I'd do here:

Suggested change

	- Avoid repeating certificate and identity-extension signature checks in TLS 1.3 transcript verification.
	See [issue 6633](https://github.com/libp2p/rust-libp2p/issues/6633).
	- Avoid repeating certificate and identity-extension signature checks in TLS 1.3 transcript verification.
	See [issue 6633](https://github.com/libp2p/rust-libp2p/issues/6633) for details.
	See [PR 6634](https://github.com/libp2p/rust-libp2p/pull/6634).

[MavenRain]

Applied your suggested wording: the changelog now links both issue #6633 and PR #6634.

[grantkee]

The directory now has two fixture generators and a copied private key, and neither the reason for the second generator nor where rsa-2048.pk8 came from is written down anywhere.

gen.sh and openssl.cfg are untouched and don't mention this script, so someone regenerating fixtures later finds two toolchains with no pointer between them. I checked that the second generator is justified: it signs with a fixed RSA key the Rust tests load (certificate_profile.rs:292,311, certificate_handshake_tests.rs:223,266), it produces a SignedKey that actually verifies (from a throwaway Ed25519 host key, :27-33), and it covers RSA-PSS, which is still a TODO in gen.sh:29-34. The extension layout matches the existing fixtures (OID 1.3.6.1.4.1.53594.1.1, critical, SEQUENCE { OCTET STRING(36) protobuf Ed25519 pubkey, OCTET STRING(64) sig }). rsa-2048.pk8 is byte-identical to identity/src/test/rsa-2048.pk8, and copying it is fine: the repo has no cross-crate include_bytes!("../..") precedent, and exclude = ["src/test_assets"] already keeps it out of the published crate.

So this is documentation only. Two small additions would close the gap. First, in gen.sh near the RSA-PSS TODO at :29:

# RSA fixtures with a *valid* libp2p extension signature over a fixed key
# (profile_rsa_{pkcs1,pss}_{sha256,sha384,sha512}.der) are produced by
# gen_profile_rsa.py (run: python3 -I gen_profile_rsa.py).

Second, in this docstring: that rsa-2048.pk8 is a copy of identity/src/test/rsa-2048.pk8 kept here so tests can sign with a known RSA key, that the SignedKey is signed with a throwaway Ed25519 host key, and that the PSS fixtures use salt length = digest length.

Unrelated to this PR, but since I was in there: the gen.sh fixtures embed the static SignedKey from openssl.cfg:5-6, which doesn't verify against their freshly generated keys, and their validity window was 2021-12-15 to 2022-01-14. Worth a separate issue if anyone wants those fixtures to be more than parse-only.

[MavenRain]

Added the pointer and regeneration command in gen.sh. The renamed generator's docstring now documents the RSA key's origin in identity/src/test, the throwaway Ed25519 host key used to sign the extension, and digest-length PSS salts.

[grantkee]

Three nested for_each closures put the test body at 20 spaces of indent where plain for loops would read the same and flatten it.

Purely cosmetic, so feel free to ignore. The three levels are algorithm (:136-137), sender (:139-140) and proof (:146-147); the drive callback at :178-182 has to stay a closure because drive takes impl FnMut(Sender, &mut [u8]). The rewrite is behaviour-preserving: the bodies have no return, ?, break or continue, the loop variables are Copy, array into_iter() yields the same order, and the mutable borrow of corrupted by the callback ends before the read at :193. The two new files use for_each consistently (17 calls plus one try_for_each), so if you do change it, it's worth doing across both, including the two-level nesting at :270 / :275.

#[test]
fn independently_invalid_proofs_fail_in_both_directions() {
    for algorithm in [&rcgen::PKCS_ECDSA_P256_SHA256, &rcgen::PKCS_ECDSA_P384_SHA384, &rcgen::PKCS_ED25519] {
        for sender in [Sender::Client, Sender::Server] {
            for proof in [InvalidProof::Certificate, InvalidProof::Extension, InvalidProof::Transcript] {
                // body of :148-199 unchanged
            }
        }
    }
}

[MavenRain]

Flattened the nested case iteration using flat_map chains ending in a single for_each, in both the handshake tests and profiler. This reduces the indentation you flagged while keeping the existing iterator style.