From 91707cc430710949f0e97356589b558a74a10bc1 Mon Sep 17 00:00:00 2001 From: "Jan Winkelmann (keks)" Date: Mon, 23 Jun 2025 15:29:04 +0200 Subject: [PATCH] Address feedback --- Cargo.lock | 1 - ciphers/Cargo.toml | 43 ++++++++++-------- ciphers/src/subtle/libcrux/mod.rs | 6 +-- ciphers/src/subtle/mod.rs | 7 ++- readme.md | 6 ++- rosenpass/benches/trace_handshake.rs | 18 ++++++-- rosenpass/src/protocol/protocol.rs | 67 ++++++++++------------------ util/Cargo.toml | 3 +- util/src/trace_bench.rs | 12 +++-- 9 files changed, 82 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b356525..cbe6f62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2172,7 +2172,6 @@ version = "0.1.0" dependencies = [ "anyhow", "base64ct", - "lazy_static", "libcrux-test-utils", "mio", "rustix", diff --git a/ciphers/Cargo.toml b/ciphers/Cargo.toml index 5a1c375..c20e338 100644 --- a/ciphers/Cargo.toml +++ b/ciphers/Cargo.toml @@ -11,27 +11,34 @@ readme = "readme.md" rust-version = "1.77.0" [features] -experiment_libcrux_all = [ - "experiment_libcrux_blake2", - "experiment_libcrux_chachapoly", - "experiment_libcrux_chachapoly_test", - "experiment_libcrux_kyber", -] -experiment_libcrux_blake2 = ["dep:libcrux-blake2", "dep:thiserror"] -experiment_libcrux_chachapoly = ["dep:libcrux-chacha20poly1305"] +# whether the types should be defined +experiment_libcrux_define_blake2 = ["dep:libcrux-blake2", "dep:thiserror"] +experiment_libcrux_define_kyber = ["dep:libcrux-ml-kem", "dep:rand"] +experiment_libcrux_define_chachapoly = ["dep:libcrux-chacha20poly1305"] + +# whether the types should be used by default +experiment_libcrux_blake2 = ["experiment_libcrux_define_blake2"] +experiment_libcrux_kyber = ["experiment_libcrux_define_kyber"] +experiment_libcrux_chachapoly = ["experiment_libcrux_define_chachapoly"] experiment_libcrux_chachapoly_test = [ - "experiment_libcrux_chachapoly", - "dep:libcrux", + "experiment_libcrux_define_chachapoly", + "dep:libcrux", ] -experiment_libcrux_kyber = ["dep:libcrux-ml-kem", "dep:rand"] -bench = [ - "dep:thiserror", - "dep:rand", - "dep:libcrux", - "dep:libcrux-blake2", - "dep:libcrux-ml-kem", - "dep:libcrux-chacha20poly1305", + +# shorthands +experiment_libcrux_define_all = [ + "experiment_libcrux_define_blake2", + "experiment_libcrux_define_chachapoly", + "experiment_libcrux_define_kyber", ] +experiment_libcrux_all = [ + "experiment_libcrux_blake2", + "experiment_libcrux_chachapoly", + "experiment_libcrux_chachapoly_test", + "experiment_libcrux_kyber", +] + +bench = ["experiment_libcrux_define_all"] [[bench]] name = "primitives" diff --git a/ciphers/src/subtle/libcrux/mod.rs b/ciphers/src/subtle/libcrux/mod.rs index 210bc61..f481e53 100644 --- a/ciphers/src/subtle/libcrux/mod.rs +++ b/ciphers/src/subtle/libcrux/mod.rs @@ -4,11 +4,11 @@ //! //! [Github](https://github.com/cryspen/libcrux) -#[cfg(any(feature = "experiment_libcrux_blake2", feature = "bench"))] +#[cfg(feature = "experiment_libcrux_define_blake2")] pub mod blake2b; -#[cfg(any(feature = "experiment_libcrux_chachapoly", feature = "bench"))] +#[cfg(feature = "experiment_libcrux_define_chachapoly")] pub mod chacha20poly1305_ietf; -#[cfg(any(feature = "experiment_libcrux_kyber", feature = "bench"))] +#[cfg(feature = "experiment_libcrux_define_kyber")] pub mod kyber512; diff --git a/ciphers/src/subtle/mod.rs b/ciphers/src/subtle/mod.rs index 6221324..6d3083b 100644 --- a/ciphers/src/subtle/mod.rs +++ b/ciphers/src/subtle/mod.rs @@ -9,9 +9,8 @@ pub mod custom; pub mod rust_crypto; #[cfg(any( - feature = "experiment_libcrux_blake2", - feature = "experiment_libcrux_chachapoly", - feature = "experiment_libcrux_kyber", - feature = "bench" + feature = "experiment_libcrux_define_blake2", + feature = "experiment_libcrux_define_chachapoly", + feature = "experiment_libcrux_define_kyber", ))] pub mod libcrux; diff --git a/readme.md b/readme.md index 655a563..6021361 100644 --- a/readme.md +++ b/readme.md @@ -94,13 +94,13 @@ written to a trace, which is then inspected after a run. Benchmarks are automatically run on CI. The measurements are visualized in the [Benchmark Dashboard]. -[Benchmark Dashboard]: https://rosenpass.github.io/benchmarks +[Benchmark Dashboard]: https://rosenpass.github.io/rosenpass/benchmarks ### Primitive Benchmarks There are benchmarks for the functions of the traits `Kem`, `Aead` and `KeyedHash`. They are run for all implementations in the `primitives` -benchmark of `rosenpass-ciphers`. Run the benchmarks using +benchmark of `rosenpass-ciphers`. Run the benchmarks and view their results using ``` cargo bench -p rosenpass-ciphers --bench primitives -F bench @@ -120,6 +120,8 @@ performs some minor statistical analysis of the trace can be run using cargo bench -p rosenpass --bench trace_handshake -F trace_bench ``` +This runs the benchmarks and prints the results in machine-readable JSON. + --- # Mirrors diff --git a/rosenpass/benches/trace_handshake.rs b/rosenpass/benches/trace_handshake.rs index 7710495..95a7bdb 100644 --- a/rosenpass/benches/trace_handshake.rs +++ b/rosenpass/benches/trace_handshake.rs @@ -12,7 +12,7 @@ use libcrux_test_utils::tracing::{EventType, Trace as _}; use rosenpass_cipher_traits::primitives::Kem; use rosenpass_ciphers::StaticKem; use rosenpass_secret_memory::secret_policy_try_use_memfd_secrets; -use rosenpass_util::trace_bench::{RpEventType, TRACE}; +use rosenpass_util::trace_bench::RpEventType; use rosenpass::protocol::{ CryptoServer, HandleMsgResult, MsgBuf, PeerPtr, ProtocolVersion, SPk, SSk, SymKey, @@ -20,6 +20,10 @@ use rosenpass::protocol::{ const ITERATIONS: usize = 100; +/// Performs a full protocol run by processing a message and recursing into handling that message, +/// until no further response is produced. Returns the keys produce by the two parties. +/// +/// Ensures that each party produces one of the two keys. fn handle( tx: &mut CryptoServer, msgb: &mut MsgBuf, @@ -46,6 +50,10 @@ fn handle( Ok((txk, rxk.or(xch))) } +/// Performs the full handshake by calling `handle` with the correct values, based on just two +/// `CryptoServer`s. +/// +/// Ensures that both parties compute the same keys. fn hs(ini: &mut CryptoServer, res: &mut CryptoServer) -> Result<()> { let (mut inib, mut resb) = (MsgBuf::zero(), MsgBuf::zero()); let sz = ini.initiate_handshake(PeerPtr(0), &mut *inib)?; @@ -54,12 +62,14 @@ fn hs(ini: &mut CryptoServer, res: &mut CryptoServer) -> Result<()> { Ok(()) } +/// Generates a new key pair. fn keygen() -> Result<(SSk, SPk)> { let (mut sk, mut pk) = (SSk::zero(), SPk::zero()); StaticKem.keygen(sk.secret_mut(), pk.deref_mut())?; Ok((sk, pk)) } +/// Creates two instanves of `CryptoServer`, generating key pairs for each. fn make_server_pair(protocol_version: ProtocolVersion) -> Result<(CryptoServer, CryptoServer)> { let psk = SymKey::random(); let ((ska, pka), (skb, pkb)) = (keygen()?, keygen()?); @@ -73,6 +83,8 @@ fn make_server_pair(protocol_version: ProtocolVersion) -> Result<(CryptoServer, } fn main() { + let trace = rosenpass_util::trace_bench::trace(); + // Attempt to use memfd_secrets for storing sensitive key material secret_policy_try_use_memfd_secrets(); @@ -83,7 +95,7 @@ fn main() { } // Emit a marker event to separate V02 and V03 trace sections - TRACE.emit_on_the_fly("start-hs-v03"); + trace.emit_on_the_fly("start-hs-v03"); // Run protocol for V03 let (mut a_v03, mut b_v03) = make_server_pair(ProtocolVersion::V03).unwrap(); @@ -92,7 +104,7 @@ fn main() { } // Collect the trace events generated during the handshakes - let trace: Vec<_> = TRACE.clone().report(); + let trace: Vec<_> = trace.clone().report(); // Split the trace into V02 and V03 sections based on the marker let (trace_v02, trace_v03) = { diff --git a/rosenpass/src/protocol/protocol.rs b/rosenpass/src/protocol/protocol.rs index dd95a5c..13df003 100644 --- a/rosenpass/src/protocol/protocol.rs +++ b/rosenpass/src/protocol/protocol.rs @@ -17,6 +17,9 @@ use std::{ use anyhow::{bail, ensure, Context, Result}; +#[cfg(feature = "trace_bench")] +use rosenpass_util::trace_bench::Trace as _; + use crate::{hash_domains, msgs::*, RosenpassError}; use memoffset::span_of; use rosenpass_cipher_traits::primitives::{ @@ -3551,9 +3554,9 @@ impl CryptoServer { /// trace, which allows reconstructing the run times of the individual sections for performance /// measurement. macro_rules! protocol_section { - ($label:expr, $body:tt) => {{ + ($label:expr, $body:block) => {{ #[cfg(feature = "trace_bench")] - let _span_raii_handle = rosenpass_util::trace_bench::TRACE.emit_span($label); + let _span_guard = rosenpass_util::trace_bench::trace().emit_span($label); #[allow(unused_braces)] $body @@ -3563,14 +3566,10 @@ macro_rules! protocol_section { impl CryptoServer { /// Core cryptographic protocol implementation: Kicks of the handshake /// on the initiator side, producing the InitHello message. - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_initiation", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_initiation(&mut self, peer: PeerPtr, ih: &mut InitHello) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_initiation"); + let mut hs = InitiatorHandshake::zero_with_timestamp( self, peer.get(self).protocol_version.keyed_hash(), @@ -3633,19 +3632,15 @@ impl CryptoServer { /// Core cryptographic protocol implementation: Parses an [InitHello] message and produces a /// [RespHello] message on the responder side. - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_init_hello", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_init_hello( &mut self, ih: &InitHello, rh: &mut RespHello, keyed_hash: KeyedHash, ) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_init_hello"); + let mut core = HandshakeState::zero(keyed_hash); core.sidi = SessionId::from_slice(&ih.sidi); @@ -3721,14 +3716,10 @@ impl CryptoServer { /// Core cryptographic protocol implementation: Parses an [RespHello] message and produces an /// [InitConf] message on the initiator side. - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_resp_hello", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_resp_hello(&mut self, rh: &RespHello, ic: &mut InitConf) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_resp_hello"); + // RHI2 let peer = self .lookup_handshake(SessionId::from_slice(&rh.sidi)) @@ -3844,19 +3835,15 @@ impl CryptoServer { /// /// This concludes the handshake on the cryptographic level; the [EmptyData] message is just /// an acknowledgement message telling the initiator to stop performing retransmissions. - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_init_conf", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_init_conf( &mut self, ic: &InitConf, rc: &mut EmptyData, keyed_hash: KeyedHash, ) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_init_conf"); + // (peer, bn) ← LoadBiscuit(InitConf.biscuit) // ICR1 let (peer, biscuit_no, mut core) = protocol_section!("ICR1", { @@ -3956,18 +3943,14 @@ impl CryptoServer { /// message then terminates the handshake. /// /// The EmptyData message is just there to tell the initiator to abort retransmissions. - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_resp_conf", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_resp_conf( &mut self, msg_in: &Ref<&[u8], Envelope>, seal_broken: String, ) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_resp_conf"); + let rc: &EmptyData = &msg_in.payload; let sid = SessionId::from_slice(&rc.sid); let hs = self @@ -4020,14 +4003,10 @@ impl CryptoServer { /// DOS mitigation features. /// /// See more on DOS mitigation in Rosenpass in the [whitepaper](https://rosenpass.eu/whitepaper.pdf). - #[cfg_attr( - feature = "trace_bench", - rosenpass_util::trace_bench::trace_span( - "handle_cookie_reply", - rosenpass_util::trace_bench::TRACE - ) - )] pub fn handle_cookie_reply(&mut self, cr: &CookieReply) -> Result { + #[cfg(feature = "trace_bench")] + let _span_guard = rosenpass_util::trace_bench::trace().emit_span("handle_cookie_reply"); + let peer_ptr: Option = self .lookup_session(Public::new(cr.inner.sid)) .map(|v| PeerPtr(v.0)) diff --git a/util/Cargo.toml b/util/Cargo.toml index 99d5baf..bdd1ddf 100644 --- a/util/Cargo.toml +++ b/util/Cargo.toml @@ -25,8 +25,7 @@ mio = { workspace = true } tempfile = { workspace = true } uds = { workspace = true, optional = true, features = ["mio_1xx"] } libcrux-test-utils = { workspace = true, optional = true } -lazy_static = { workspace = true, optional = true } [features] experiment_file_descriptor_passing = ["uds"] -trace_bench = ["dep:libcrux-test-utils", "dep:lazy_static"] +trace_bench = ["dep:libcrux-test-utils"] diff --git a/util/src/trace_bench.rs b/util/src/trace_bench.rs index 5f1528a..367efa6 100644 --- a/util/src/trace_bench.rs +++ b/util/src/trace_bench.rs @@ -1,11 +1,10 @@ +use std::sync::OnceLock; use std::time::Instant; use libcrux_test_utils::tracing; -lazy_static::lazy_static! { - /// The trace value used in all Rosepass crates. - pub static ref TRACE: RpTrace = RpTrace::default(); -} +/// The trace value used in all Rosepass crates. +static TRACE: OnceLock = OnceLock::new(); /// The trace type used to trace Rosenpass for performance measurement. pub type RpTrace = tracing::MutexTrace<&'static str, Instant>; @@ -17,3 +16,8 @@ pub type RpEventType = tracing::TraceEvent<&'static str, Instant>; // [`libcrux_test_utils`]. pub use libcrux_test_utils::tracing::trace_span; pub use tracing::Trace; + +/// Returns a reference to the trace and lazily initializes it. +pub fn trace() -> &'static tracing::MutexTrace<&'static str, Instant> { + TRACE.get_or_init(tracing::MutexTrace::default) +}