diff --git a/crates/web-bot-auth/Cargo.toml b/crates/web-bot-auth/Cargo.toml index 2abf4c9..b9885f6 100644 --- a/crates/web-bot-auth/Cargo.toml +++ b/crates/web-bot-auth/Cargo.toml @@ -11,6 +11,9 @@ repository.workspace = true keywords.workspace = true categories.workspace = true +[[bench]] +name = "keyring_verify" +harness = false [dependencies] ed25519-dalek = { workspace = true } diff --git a/crates/web-bot-auth/benches/keyring_verify.rs b/crates/web-bot-auth/benches/keyring_verify.rs new file mode 100644 index 0000000..664ee80 --- /dev/null +++ b/crates/web-bot-auth/benches/keyring_verify.rs @@ -0,0 +1,146 @@ +//! Deterministic executable benchmark for `MessageVerifier::verify` against a +//! `KeyRing`. +//! +//! Run both modes with the default iteration count: +//! +//! ```text +//! cargo bench -p web-bot-auth --bench keyring_verify +//! ``` +//! +//! Select one mode or override the iteration count after `--`: +//! +//! ```text +//! cargo bench -p web-bot-auth --bench keyring_verify -- verify-only 100000 +//! ``` +//! +//! `verify-only` parses the message once and measures verification only; +//! `end-to-end` parses and verifies on every iteration. Both modes build the +//! keyring once, outside the measured loop. `cargo test --all-targets` runs a +//! short smoke check instead of a full measurement. + +use std::hint::black_box; +use std::time::Instant; + +use web_bot_auth::components::{CoveredComponent, DerivedComponent, HTTPField}; +use web_bot_auth::keyring::{Algorithm, KeyRing}; +use web_bot_auth::message_signatures::{MessageVerifier, SignedMessage}; + +/// Fixed test vector shared with the crate's unit tests. +const KEY_ID: &str = "poqkLGiymh_W0uP6PZFw-dvez3QJT5SolqXBCW38r0U"; +const PUBLIC_KEY: [u8; ed25519_dalek::PUBLIC_KEY_LENGTH] = [ + 0x26, 0xb4, 0x0b, 0x8f, 0x93, 0xff, 0xf3, 0xd8, 0x97, 0x11, 0x2f, 0x7e, 0xbc, 0x58, 0x2b, 0x23, + 0x2d, 0xbd, 0x72, 0x51, 0x7d, 0x08, 0x2f, 0xe8, 0x3c, 0xfb, 0x30, 0xdd, 0xce, 0x43, 0xd1, 0xbb, +]; + +struct StandardTestVector; + +impl SignedMessage for StandardTestVector { + fn lookup_component(&self, name: &CoveredComponent) -> Vec { + match name { + CoveredComponent::HTTP(HTTPField { name, .. }) => { + if name == "signature" { + return vec!["sig1=:uz2SAv+VIemw+Oo890bhYh6Xf5qZdLUgv6/PbiQfCFXcX/vt1A8Pf7OcgL2yUDUYXFtffNpkEr5W6dldqFrkDg==:".to_owned()]; + } + if name == "signature-input" { + return vec![r#"sig1=("@authority");created=1735689600;keyid="poqkLGiymh_W0uP6PZFw-dvez3QJT5SolqXBCW38r0U";alg="ed25519";expires=1735693200;nonce="gubxywVx7hzbYKatLgzuKDllDAIXAkz41PydU7aOY7vT+Mb3GJNxW0qD4zJ+IOQ1NVtg+BNbTCRUMt1Ojr5BgA==";tag="web-bot-auth""#.to_owned()]; + } + vec![] + } + CoveredComponent::Derived(DerivedComponent::Authority { .. }) => { + vec!["example.com".to_string()] + } + _ => vec![], + } + } +} + +fn main() { + let mut invoked_by_cargo_bench = false; + let mut mode = None; + let mut iteration_arg = None; + for arg in std::env::args().skip(1) { + if arg == "--bench" { + invoked_by_cargo_bench = true; + } else if arg.starts_with("--") { + continue; + } else if mode.is_none() { + mode = Some(arg); + } else if iteration_arg.is_none() { + iteration_arg = Some(arg); + } else { + eprintln!("unexpected argument: {arg}"); + std::process::exit(2); + } + } + + let iterations: u64 = iteration_arg + .map(|value| value.parse().expect("iterations must be an integer")) + .unwrap_or(if invoked_by_cargo_bench || mode.is_some() { + 30_000 + } else { + 100 + }); + assert!(iterations > 0, "iterations must be greater than zero"); + + let modes: &[&str] = match mode.as_deref() { + None => &["verify-only", "end-to-end"], + Some("verify-only") => &["verify-only"], + Some("end-to-end") => &["end-to-end"], + Some(other) => { + eprintln!("unknown mode: {other} (expected verify-only or end-to-end)"); + std::process::exit(2); + } + }; + + // Key import and keyring construction happen once, before measurement. + let mut keyring = KeyRing::default(); + keyring.import_raw(KEY_ID.to_string(), Algorithm::Ed25519, PUBLIC_KEY.to_vec()); + let message = StandardTestVector; + + for mode in modes { + run(mode, iterations, &keyring, &message); + } +} + +fn run(mode: &str, iterations: u64, keyring: &KeyRing, message: &StandardTestVector) { + let mut checksum: u64 = 0; + let elapsed = match mode { + "verify-only" => { + let verifier = MessageVerifier::parse(message, |(_, _)| true).unwrap(); + let start = Instant::now(); + for _ in 0..iterations { + // `verify` consumes the verifier, so clone the parsed message. + let timing = black_box(verifier.clone()) + .verify(black_box(keyring), None) + .unwrap(); + checksum = checksum + .wrapping_add(timing.generation.as_nanos() as u64) + .wrapping_add(timing.verification.as_nanos() as u64) + .wrapping_add(1); + } + start.elapsed() + } + "end-to-end" => { + let start = Instant::now(); + for _ in 0..iterations { + let verifier = MessageVerifier::parse(black_box(message), |(_, _)| true).unwrap(); + let timing = verifier.verify(black_box(keyring), None).unwrap(); + checksum = checksum + .wrapping_add(timing.generation.as_nanos() as u64) + .wrapping_add(timing.verification.as_nanos() as u64) + .wrapping_add(1); + } + start.elapsed() + } + _ => unreachable!("mode validated in main"), + }; + + // Every iteration contributes at least 1 to the checksum; assert so the + // loop's work cannot be optimized away. + black_box(checksum); + assert!(checksum >= iterations); + let nanos_per_iteration = elapsed.as_secs_f64() * 1_000_000_000.0 / iterations as f64; + println!( + "{mode}: {nanos_per_iteration:.1} ns/iteration ({iterations} iterations in {elapsed:?}, checksum={checksum})" + ); +} diff --git a/crates/web-bot-auth/src/keyring.rs b/crates/web-bot-auth/src/keyring.rs index 1f2162f..249b271 100644 --- a/crates/web-bot-auth/src/keyring.rs +++ b/crates/web-bot-auth/src/keyring.rs @@ -159,13 +159,45 @@ impl Thumbprintable { /// verifying keys for verificiation. #[derive(Default, Debug, Clone)] pub struct KeyRing { - ring: HashMap, + ring: HashMap, +} + +/// A keyring entry: the raw key material backing the public `get` API, plus +/// verification state prepared once at insertion time. +#[derive(Debug, Clone)] +struct KeyEntry { + raw: (Algorithm, PublicKey), + prepared: PreparedKey, +} + +/// Verification state prepared when an entry is created, so that verification +/// does not pay for key reconstruction on every request. +#[derive(Debug, Clone)] +pub(crate) enum PreparedKey { + /// `Some` when the raw bytes yielded a valid `VerifyingKey` at import time. + /// `None` preserves the historical behavior of deferring the + /// `InvalidKeyLength` error to verification. + Ed25519(Option), + /// No preparation is possible for algorithms we do not yet verify. + Unsupported, +} + +impl From<(Algorithm, PublicKey)> for KeyEntry { + fn from(raw: (Algorithm, PublicKey)) -> KeyEntry { + let prepared = match &raw { + (Algorithm::Ed25519, public_key) => { + PreparedKey::Ed25519(VerifyingKey::try_from(public_key.as_slice()).ok()) + } + _ => PreparedKey::Unsupported, + }; + KeyEntry { raw, prepared } + } } impl FromIterator<(String, (Algorithm, PublicKey))> for KeyRing { fn from_iter>(iter: T) -> KeyRing { KeyRing { - ring: HashMap::from_iter(iter), + ring: HashMap::from_iter(iter.into_iter().map(|(id, raw)| (id, raw.into()))), } } } @@ -182,7 +214,7 @@ impl KeyRing { !self.ring.contains_key(&identifier) && self .ring - .insert(identifier, (algorithm, public_key)) + .insert(identifier, (algorithm, public_key).into()) .is_none() } @@ -197,7 +229,15 @@ impl KeyRing { /// Retrieve a key. Semantics are identical to `HashMap::get`. pub fn get(&self, identifier: &String) -> Option<&(Algorithm, Vec)> { - self.ring.get(identifier) + self.ring.get(identifier).map(|entry| &entry.raw) + } + + /// Retrieve the algorithm and prepared verification state for a key. + /// Used by verification to avoid reconstructing verifying keys per request. + pub(crate) fn get_prepared(&self, identifier: &String) -> Option<(&Algorithm, &PreparedKey)> { + self.ring + .get(identifier) + .map(|entry| (&entry.raw.0, &entry.prepared)) } /// Import a single JSON Web Key. This method is fallible. diff --git a/crates/web-bot-auth/src/message_signatures.rs b/crates/web-bot-auth/src/message_signatures.rs index 83afbcc..67525bd 100644 --- a/crates/web-bot-auth/src/message_signatures.rs +++ b/crates/web-bot-auth/src/message_signatures.rs @@ -9,7 +9,7 @@ use time::UtcDateTime; use super::ImplementationError; use crate::components::{self, CoveredComponent, HTTPField}; -use crate::keyring::{Algorithm, KeyRing}; +use crate::keyring::{Algorithm, KeyRing, PreparedKey}; static OBSOLETE_LINE_FOLDING: LazyLock = LazyLock::new(|| Regex::new(r"\s*\r\n\s+").unwrap()); @@ -579,8 +579,8 @@ impl MessageVerifier { keyring: &KeyRing, key_id: Option, ) -> Result { - let keying_material = (match key_id { - Some(key) => keyring.get(&key), + let (algorithm, prepared) = (match key_id { + Some(key) => keyring.get_prepared(&key), None => self .parsed .base @@ -588,18 +588,16 @@ impl MessageVerifier { .details .keyid .as_ref() - .and_then(|key| keyring.get(key)), + .and_then(|key| keyring.get_prepared(key)), }) .ok_or(ImplementationError::NoSuchKey)?; let generation = UtcDateTime::now(); let (base_representation, _) = self.parsed.base.into_ascii()?; let generation = (UtcDateTime::now() - generation).unsigned_abs(); - match &keying_material.0 { - Algorithm::Ed25519 => { - use ed25519_dalek::{Signature, Verifier, VerifyingKey}; - let verifying_key = VerifyingKey::try_from(keying_material.1.as_slice()) - .map_err(|_| ImplementationError::InvalidKeyLength)?; + match (algorithm, prepared) { + (Algorithm::Ed25519, PreparedKey::Ed25519(Some(verifying_key))) => { + use ed25519_dalek::{Signature, Verifier}; let sig = Signature::try_from(self.parsed.signature.as_slice()) .map_err(|_| ImplementationError::InvalidSignatureLength)?; @@ -613,7 +611,10 @@ impl MessageVerifier { verification: (UtcDateTime::now() - verification).unsigned_abs(), }) } - other => Err(ImplementationError::UnsupportedAlgorithm(other.clone())), + (Algorithm::Ed25519, PreparedKey::Ed25519(None)) => { + Err(ImplementationError::InvalidKeyLength) + } + (other, _) => Err(ImplementationError::UnsupportedAlgorithm(other.clone())), } } } @@ -681,6 +682,123 @@ mod tests { assert!(timing.verification.as_nanos() > 0); } + const TEST_KEY_ID: &str = "poqkLGiymh_W0uP6PZFw-dvez3QJT5SolqXBCW38r0U"; + const TEST_PUBLIC_KEY: [u8; ed25519_dalek::PUBLIC_KEY_LENGTH] = [ + 0x26, 0xb4, 0x0b, 0x8f, 0x93, 0xff, 0xf3, 0xd8, 0x97, 0x11, 0x2f, 0x7e, 0xbc, 0x58, 0x2b, + 0x23, 0x2d, 0xbd, 0x72, 0x51, 0x7d, 0x08, 0x2f, 0xe8, 0x3c, 0xfb, 0x30, 0xdd, 0xce, 0x43, + 0xd1, 0xbb, + ]; + + fn keyring_with_test_key() -> KeyRing { + let mut keyring = KeyRing::default(); + keyring.import_raw( + TEST_KEY_ID.to_string(), + Algorithm::Ed25519, + TEST_PUBLIC_KEY.to_vec(), + ); + keyring + } + + #[test] + fn test_verifying_with_prepared_key() { + let keyring = keyring_with_test_key(); + // Verify repeatedly against the same keyring: the prepared key must be + // reusable across requests without being cloned or reconstructed. + for _ in 0..2 { + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + assert!(verifier.verify(&keyring, None).is_ok()); + } + } + + #[test] + fn test_importing_invalid_ed25519_key_material_defers_error() { + let mut keyring = KeyRing::default(); + // `import_raw` historically accepts raw bytes that cannot become a + // valid `VerifyingKey`; the error surfaces at verification time. + assert!(keyring.import_raw(TEST_KEY_ID.to_string(), Algorithm::Ed25519, vec![0x42; 7],)); + // The raw bytes remain retrievable through the public `get` API. + assert_eq!( + keyring.get(&TEST_KEY_ID.to_string()), + Some(&(Algorithm::Ed25519, vec![0x42; 7])) + ); + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + let err = verifier.verify(&keyring, None).unwrap_err(); + assert!(matches!(err, ImplementationError::InvalidKeyLength)); + } + + #[test] + fn test_verifying_invalid_signature_fails() { + struct TamperedSignature {} + impl SignedMessage for TamperedSignature { + fn lookup_component(&self, name: &CoveredComponent) -> Vec { + let mut values = (StandardTestVector {}).lookup_component(name); + if let CoveredComponent::HTTP(HTTPField { name, .. }) = name + && name == "signature" + { + // A well-formed but wrong signature for the test vector. + values = vec!["sig1=:uz2SAv+VIemw+Oo890bhYh6Xf5qZdLUgv6/PbiQfCFXcX/vt1A8Pf7OcgL2yUDUYXFtffNpkEr5W6dldqFrkDA==:".to_owned()]; + } + values + } + } + let keyring = keyring_with_test_key(); + let verifier = MessageVerifier::parse(&TamperedSignature {}, |(_, _)| true).unwrap(); + let err = verifier.verify(&keyring, None).unwrap_err(); + assert!(matches!(err, ImplementationError::FailedToVerify(_))); + } + + #[test] + fn test_verifying_unsupported_algorithm() { + let mut keyring = KeyRing::default(); + assert!(keyring.import_raw( + TEST_KEY_ID.to_string(), + Algorithm::HmacSha256, + TEST_PUBLIC_KEY.to_vec(), + )); + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + let err = verifier.verify(&keyring, None).unwrap_err(); + assert!(matches!( + err, + ImplementationError::UnsupportedAlgorithm(Algorithm::HmacSha256) + )); + } + + #[test] + fn test_duplicate_import_keeps_original_key() { + let mut keyring = keyring_with_test_key(); + // Duplicate imports are rejected and leave the original entry intact. + assert!(!keyring.import_raw(TEST_KEY_ID.to_string(), Algorithm::Ed25519, vec![0x42; 7],)); + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + assert!(verifier.verify(&keyring, None).is_ok()); + } + + #[test] + fn test_verifying_key_from_from_iterator() { + let keyring = KeyRing::from_iter([( + TEST_KEY_ID.to_string(), + (Algorithm::Ed25519, TEST_PUBLIC_KEY.to_vec()), + )]); + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + assert!(verifier.verify(&keyring, None).is_ok()); + } + + #[test] + fn test_verifying_renamed_key() { + let mut keyring = keyring_with_test_key(); + assert!(keyring.rename_key(TEST_KEY_ID.to_string(), "renamed".to_string())); + // The old identifier no longer resolves. + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + let err = verifier.verify(&keyring, None).unwrap_err(); + assert!(matches!(err, ImplementationError::NoSuchKey)); + // The renamed key verifies with its prepared state intact. + let verifier = MessageVerifier::parse(&StandardTestVector {}, |(_, _)| true).unwrap(); + assert!( + verifier + .verify(&keyring, Some("renamed".to_string())) + .is_ok() + ); + } + #[test] fn test_signing() { struct SigningTest {}