test(noise): add interop tests, listener binary, and criterion benchmarks for ML-KEM-768 HFS#1
Open
paschal533 wants to merge 12 commits into
Open
Conversation
Creates transports/noise/tests/interop_hfs.rs with: - SeededRng / SeededResolver: deterministic snow CryptoResolver using StdRng::seed_from_u64 so every handshake is bit-for-bit reproducible. - run_seeded_handshake(): drives a full Noise_XXhfs_25519+ML-KEM-768_ ChaChaPoly_SHA256 exchange (3 messages) and asserts both sides agree on the handshake hash. - print_vectors #[ignore] test: generates the pinned HANDSHAKE_HASH constant (msg1=1216 B, msg2=1200 B, msg3=64 B, seeds 1 and 2). - Pins HANDSHAKE_HASH from the generator run: [0xcd,0xc9,0x4b,0x9c,0xe2,0x30,0x37,0x84,...] Also adds use-chacha20poly1305 + use-sha2 to the snow dev-dep so DefaultResolver can fully resolve the ChaChaPoly_SHA256 pattern.
The previous implementation pinned a HANDSHAKE_HASH constant that was generated on one machine. The royzah/snow fork ML-KEM generate() calls system entropy regardless of SeededResolver, so the hash differs every run and every machine. Fix (option b — pragmatic): - Remove HANDSHAKE_HASH constant entirely - Change run_seeded_handshake to return both initiator and responder hashes so callers can assert intra-run agreement - Update print_vectors to assert init_hash == resp_hash (both sides agree within one run), assert hash != [0u8;32] (non-trivial), and assert message sizes match XXhfs geometry (1216/1200/64 bytes) - Fix msg1 comment: (token: e) -> (tokens: e, e1 — ephemeral KEM pubkey) - Remove three vec![0u8;65535] allocations; reuse buf via buf.resize() - Run cargo fmt (also reformats mlkem_hfs.rs line breaks)
Fix 1 (CRITICAL): Collapse nested if in parse_port() using 'if let' chain syntax
- Changed 'else if !args[i].starts_with("--") { if let Ok(...) }'
- To: 'if !args[i].starts_with("--") && let Ok(...)'
- Resolves: clippy::collapsible_if warning
Fix 2 (MEDIUM): Ensure consistent ERROR prefix on all error paths
- Convert main() from Result-based to explicit error handling
- All errors now print to stderr with "ERROR " prefix before exiting(1)
- Matches parse_port() error reporting style
- bench_xx_handshake and bench_xxhfs_handshake: Move Endpoint::pair allocation to setup closure so ring-buffer creation is excluded from timing measurements. - bench_xxhfs_transport_1kb: Move payload vec![0u8; 1024] to setup closure so each iteration reuses the same allocation, eliminating allocation noise from the measured send path. This ensures benchmarks measure only the cryptographic operations, not resource allocation.
- Rewrite pr-comment-draft.md: remove false claim about pinned HANDSHAKE_HASH constant; document that snow's ML-KEM generate() bypasses the seeded RNG so only intra-run hash agreement and message geometry can be asserted deterministically. Remove unsupported ~2-5ms overhead claim. - Fix msg2 token comment in interop_hfs.rs: remove spurious e1 token (e1 appears in msg1, not msg2). Correct tokens are e, ee, s, es, ekem1. - Strengthen print_vectors assertion: replace assert!(msg2.len() > 1000) with assert_eq!(msg2.len(), 1200) to match the named tests.
Author
Update: Full Rust ↔ JS ↔ Python triangle — 3/3 PASSWe migrated both js-libp2p-noise (PR #665) and py-libp2p (PR #1310) from X-Wing to raw ML-KEM-768 (matching this PR's protocol) and ran a live 3-way interop test:
All three runtimes (Rust/JS/Python) now speak the same wire protocol on Peer IDs from the live run: (Previously our JS and Python used X-Wing / |
This was referenced Jun 24, 2026
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.
Summary
Additive contribution on top of this PR's implementation — royzah's source files (
src/) are untouched.What's added
tests/interop_hfs.rs—SeededResolverinfrastructure + two structural compliance tests:handshake_structural_properties: verifies msg1=1216B, msg2=1200B, msg3=64B and both sides compute the same handshake hash within a single runsecond_run_produces_coherent_results: second independent run with different seedsprint_vectors(ignored): diagnostic helper that prints message hex for manual cross-language inspectionexamples/noise_hfs_listener.rs— standalone TCP listener binary for cross-language interop testing:benches/noise_hfs.rs— Criterion benchmarks comparing classical Noise XX vs XXhfs handshake latency, plus 1KB post-handshake transport throughput:Live Rust ↔ Python interop — confirmed ✓
I wrote a standalone Python dialer speaking
Noise_XXhfs_25519+ML-KEM-768_ChaChaPoly_SHA256directly (usingkyber-py), then dialed this PR's listener binary live:Both sides completed mutual authentication (identity signatures verified) and the three-message handshake succeeded end-to-end. Message geometry confirmed: msg1=1216B, msg2=1304B (includes libp2p identity payload), msg3=168B.
Note on X-Wing variant: Our existing JS and Python implementations use X-Wing (ML-KEM-768 + X25519 bundled, protocol
/noise-pq/1.0.0) — a valid but distinct approach from this PR's raw ML-KEM-768 pattern. The two are not wire-compatible, which is worth calling out for libp2p/specs#723.Finding: snow's ML-KEM entropy bypasses the seeded resolver
The snow fork's
generate()inDefaultResolvercalls system entropy directly, ignoring the injectedCryptoResolver. This prevents fully deterministic cross-language test vectors (relevant to specs#723 question 2 on KDF mixing order). Worth considering a seeded generation path in the fork for interop testing.Suggestion: RustCrypto
ml-kemswap (follow-up)Resolver::resolve_kem()delegates to snow'sDefaultResolver, whose ML-KEM backend is marked unaudited. The RustCryptoml-kemcrate (FIPS 203 compliant, audited by NCC Group) could replace it by implementingsnow::types::Kem— same pattern as the existingx25519-dalekDH wrapper. Happy to write that as a follow-up PR if you're open to it.Test plan
cargo test --features mlkem-hfs --test interop_hfs→ 2 passed, 1 ignoredcargo build --example noise_hfs_listener --features mlkem-hfs→ compilescargo bench --bench noise_hfs --features mlkem-hfs --no-run→ compilescargo clippy --features mlkem-hfs→ zero new warnings in noise cratecargo fmt --check→ passessrc/lib.rs,src/protocol.rs, orsrc/io/