Skip to content

Commit

Permalink
Remove caching of hmac control key
Browse files Browse the repository at this point in the history
This shrinks the Entry state by ~700 bytes.
  • Loading branch information
Mark-Simulacrum committed Sep 27, 2024
1 parent 603a84d commit 04cc58b
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 19 deletions.
4 changes: 2 additions & 2 deletions dc/s2n-quic-dc/src/path/secret/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ impl Map {

match packet {
control::Packet::StaleKey(packet) => {
let Some(packet) = packet.authenticate(key) else {
let Some(packet) = packet.authenticate(&key) else {
return;
};
state.mark_live(self.state.cleaner.epoch());
Expand All @@ -447,7 +447,7 @@ impl Map {
.fetch_add(1, Ordering::Relaxed);
}
control::Packet::ReplayDetected(packet) => {
let Some(_packet) = packet.authenticate(key) else {
let Some(_packet) = packet.authenticate(&key) else {
return;
};
self.state
Expand Down
3 changes: 2 additions & 1 deletion dc/s2n-quic-dc/src/path/secret/map/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,12 @@ fn check_invariants_no_overflow() {
}

#[test]
#[cfg(all(target_pointer_width = "64", target_os = "linux"))]
fn entry_size() {
// Don't run this outside of GHA -- it is likely to break on random changes (e.g., new Rust
// versions) and so is expected to be somewhat flaky. Ideally we'd have something like "Is this
// a personal/local build?" too but that seems also hard.
if std::env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") {
assert_eq!(fake_entry(0).size(), 334);
assert_eq!(fake_entry(0).size(), 330);
}
}
21 changes: 5 additions & 16 deletions dc/s2n-quic-dc/src/path/secret/sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use super::schedule;
use crate::{crypto::awslc::open, packet::secret_control};
use once_cell::sync::OnceCell;
use s2n_quic_core::varint::VarInt;
use std::sync::atomic::{AtomicU64, Ordering};

Expand All @@ -13,16 +12,6 @@ type StatelessReset = [u8; secret_control::TAG_LEN];
pub struct State {
current_id: AtomicU64,
pub(super) stateless_reset: StatelessReset,
control_secret: OnceCell<open::control::Secret>,
}

impl super::map::SizeOf for OnceCell<open::control::Secret> {
fn size(&self) -> usize {
// FIXME: OnceCell stores the value inline, but it also has a AtomicPtr to a list of
// waiters. That should be ~empty after init finishes. We should probably just initialize
// this eagerly when creating the secret rather than adding extra mutable state.
std::mem::size_of::<Self>()
}
}

impl super::map::SizeOf for StatelessReset {}
Expand All @@ -32,9 +21,8 @@ impl super::map::SizeOf for State {
let State {
current_id,
stateless_reset,
control_secret,
} = self;
current_id.size() + stateless_reset.size() + control_secret.size()
current_id.size() + stateless_reset.size()
}
}

Expand All @@ -43,7 +31,6 @@ impl State {
Self {
current_id: AtomicU64::new(0),
stateless_reset,
control_secret: Default::default(),
}
}

Expand All @@ -69,8 +56,10 @@ impl State {
}

#[inline]
pub fn control_secret(&self, secret: &schedule::Secret) -> &open::control::Secret {
self.control_secret.get_or_init(|| secret.control_opener())
pub fn control_secret(&self, secret: &schedule::Secret) -> open::control::Secret {
// We don't try to cache this, hmac init is cheap (~200-600ns depending on algorithm) and
// the space requirement is huge (700+ bytes)
secret.control_opener()
}

/// Update the sender for a received stale key packet.
Expand Down

0 comments on commit 04cc58b

Please sign in to comment.