From fc446d20e5822dae20df3dee10715e1e9e82cbcd Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 12 Mar 2024 14:13:54 +1100 Subject: [PATCH 1/2] Actually expiry channels and allow rebinding --- rust/relay/src/server.rs | 24 +++++-- rust/relay/src/time_events.rs | 3 + rust/relay/tests/regression.rs | 115 +++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 4 deletions(-) diff --git a/rust/relay/src/server.rs b/rust/relay/src/server.rs index af3e89b154f..13f5b820502 100644 --- a/rust/relay/src/server.rs +++ b/rust/relay/src/server.rs @@ -417,10 +417,13 @@ where channel.bound = false; - self.time_events.add( + let wake_deadline = self.time_events.add( now + Duration::from_secs(5 * 60), TimedAction::DeleteChannel((client, chan)), ); + self.pending_commands.push_back(Command::Wake { + deadline: wake_deadline, + }); } } TimedAction::DeleteChannel((client, chan)) => { @@ -693,10 +696,13 @@ where tracing::info!(target: "relay", "Refreshed channel binding"); - self.time_events.add( + let wake_deadline = self.time_events.add( channel.expiry, TimedAction::UnbindChannel((sender, requested_channel)), ); + self.pending_commands.push_back(Command::Wake { + deadline: wake_deadline, + }); self.send_message( channel_bind_success_response(request.transaction_id()), sender, @@ -870,10 +876,12 @@ where id: AllocationId, now: SystemTime, ) { + let expiry = now + CHANNEL_BINDING_DURATION; + let existing = self.channels_by_client_and_number.insert( (client, requested_channel), Channel { - expiry: now + CHANNEL_BINDING_DURATION, + expiry, peer_address: peer, allocation: id, bound: true, @@ -886,6 +894,14 @@ where .insert((client, peer), requested_channel); debug_assert!(existing.is_none()); + + let wake_deadline = self.time_events.add( + expiry, + TimedAction::UnbindChannel((client, requested_channel)), + ); + self.pending_commands.push_back(Command::Wake { + deadline: wake_deadline, + }); } fn send_message(&mut self, message: Message, recipient: ClientSocket) { @@ -1073,7 +1089,7 @@ impl Allocation { } } -#[derive(PartialEq)] +#[derive(PartialEq, Debug)] enum TimedAction { ExpireAllocation(AllocationId), UnbindChannel((ClientSocket, u16)), diff --git a/rust/relay/src/time_events.rs b/rust/relay/src/time_events.rs index 3fb25efa2c3..160bff19460 100644 --- a/rust/relay/src/time_events.rs +++ b/rust/relay/src/time_events.rs @@ -7,6 +7,7 @@ use std::time::SystemTime; /// /// It is the caller's responsibility to keep track of actual time passing. /// They should call [`TimeEvents::next_trigger`] to find out when to next call [`TimeEvents::pending_actions`]. +#[derive(Debug)] pub struct TimeEvents { events: Vec>, } @@ -18,6 +19,7 @@ where /// Add an action to be executed at the specified time. /// /// Returns the new wake deadline for convenience. + #[must_use] pub fn add(&mut self, trigger: SystemTime, action: A) -> SystemTime { self.events.retain(|event| event.action != action); self.events.push(TimeEvent { @@ -85,6 +87,7 @@ impl PartialOrd for TimeEvent { } #[cfg(test)] +#[allow(unused_must_use)] mod tests { use super::*; use std::time::Duration; diff --git a/rust/relay/tests/regression.rs b/rust/relay/tests/regression.rs index fbb93011488..9bef25572f9 100644 --- a/rust/relay/tests/regression.rs +++ b/rust/relay/tests/regression.rs @@ -376,6 +376,112 @@ fn ping_pong_relay( ); } +#[proptest] +fn allows_rebind_channel_after_expiry( + #[strategy(firezone_relay::proptest::transaction_id())] allocate_transaction_id: TransactionId, + #[strategy(firezone_relay::proptest::transaction_id())] + channel_bind_transaction_id: TransactionId, + #[strategy(firezone_relay::proptest::transaction_id())] + channel_bind_2_transaction_id: TransactionId, + #[strategy(firezone_relay::proptest::username_salt())] username_salt: String, + #[strategy(firezone_relay::proptest::channel_number())] channel: ChannelNumber, + source: SocketAddrV4, + peer: SocketAddrV4, + peer2: SocketAddrV4, + public_relay_addr: Ipv4Addr, + #[strategy(firezone_relay::proptest::now())] now: SystemTime, + #[strategy(firezone_relay::proptest::nonce())] nonce: Uuid, +) { + let _ = env_logger::try_init(); + + let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); + let secret = server.auth_secret().to_owned(); + let lifetime = Lifetime::new(Duration::from_secs(60 * 60)).unwrap(); + + server.assert_commands( + from_client( + source, + Allocate::new_authenticated_udp_implicit_ip4( + allocate_transaction_id, + Some(lifetime.clone()), + valid_username(now, &username_salt), + &secret, + nonce, + ), + now, + ), + [ + Wake(now + lifetime.lifetime()), + CreateAllocation(49152, AddressFamily::V4), + send_message( + source, + allocate_response( + allocate_transaction_id, + public_relay_addr, + 49152, + source, + &lifetime, + ), + ), + ], + ); + + let now = now + Duration::from_secs(1); + + server.assert_commands( + from_client( + source, + ChannelBind::new( + channel_bind_transaction_id, + channel, + XorPeerAddress::new(peer.into()), + valid_username(now, &username_salt), + &secret, + nonce, + ), + now, + ), + [ + Wake(now + Duration::from_secs(60 * 10)), // For channel expiry + send_message(source, channel_bind_response(channel_bind_transaction_id)), + ], + ); + + let now = now + Duration::from_secs(60 * 10 + 1); + + server.assert_commands( + forward_time_to(now), + [ + Wake(now + Duration::from_secs(60 * 5)), // For channel deletion + ], + ); + + let now = now + Duration::from_secs(60 * 5 + 1); + + server.assert_commands(forward_time_to(now), []); + + let now = now + Duration::from_secs(1); + + server.assert_commands( + from_client( + source, + ChannelBind::new( + channel_bind_2_transaction_id, + channel, + XorPeerAddress::new(peer2.into()), + valid_username(now, &username_salt), + &secret, + nonce, + ), + now, + ), + [ + Wake(now + Duration::from_secs(60 * 10)), // For channel expiry + send_message(source, channel_bind_response(channel_bind_2_transaction_id)), + ], + ); +} + #[proptest] fn ping_pong_ip6_relay( #[strategy(firezone_relay::proptest::transaction_id())] allocate_transaction_id: TransactionId, @@ -580,6 +686,15 @@ impl TestServer { parse_message(&payload) ) } + (Output::SendMessage((_, message)), Command::Wake { deadline, .. }) => { + panic!( + "Expected `SendMessage({message:?})`, got `Wake({})`", + deadline + .duration_since(SystemTime::UNIX_EPOCH) + .unwrap() + .as_secs(), + ) + } ( Output::SendChannelData((peer, channeldata)), Command::SendMessage { recipient, payload }, From 99476e25406618ce0795f5bb433c039715c5ae58 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 12 Mar 2024 14:40:52 +1100 Subject: [PATCH 2/2] Fix ping-pong tests --- rust/relay/tests/regression.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/rust/relay/tests/regression.rs b/rust/relay/tests/regression.rs index 9bef25572f9..a1a35a13674 100644 --- a/rust/relay/tests/regression.rs +++ b/rust/relay/tests/regression.rs @@ -291,7 +291,6 @@ fn ping_pong_relay( #[strategy(firezone_relay::proptest::transaction_id())] allocate_transaction_id: TransactionId, #[strategy(firezone_relay::proptest::transaction_id())] channel_bind_transaction_id: TransactionId, - #[strategy(firezone_relay::proptest::allocation_lifetime())] lifetime: Lifetime, #[strategy(firezone_relay::proptest::username_salt())] username_salt: String, #[strategy(firezone_relay::proptest::channel_number())] channel: ChannelNumber, source: SocketAddrV4, @@ -306,6 +305,7 @@ fn ping_pong_relay( let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); let secret = server.auth_secret().to_owned(); + let lifetime = Lifetime::new(Duration::from_secs(60 * 60)).unwrap(); // Lifetime longer than channel expiry server.assert_commands( from_client( @@ -350,10 +350,10 @@ fn ping_pong_relay( ), now, ), - [send_message( - source, - channel_bind_response(channel_bind_transaction_id), - )], + [ + Wake(now + Duration::from_secs(60 * 10)), + send_message(source, channel_bind_response(channel_bind_transaction_id)), + ], ); let now = now + Duration::from_secs(1); @@ -396,7 +396,7 @@ fn allows_rebind_channel_after_expiry( let mut server = TestServer::new(public_relay_addr).with_nonce(nonce); let secret = server.auth_secret().to_owned(); - let lifetime = Lifetime::new(Duration::from_secs(60 * 60)).unwrap(); + let lifetime = Lifetime::new(Duration::from_secs(60 * 60)).unwrap(); // Lifetime longer than channel expiry server.assert_commands( from_client( @@ -487,7 +487,6 @@ fn ping_pong_ip6_relay( #[strategy(firezone_relay::proptest::transaction_id())] allocate_transaction_id: TransactionId, #[strategy(firezone_relay::proptest::transaction_id())] channel_bind_transaction_id: TransactionId, - #[strategy(firezone_relay::proptest::allocation_lifetime())] lifetime: Lifetime, #[strategy(firezone_relay::proptest::username_salt())] username_salt: String, #[strategy(firezone_relay::proptest::channel_number())] channel: ChannelNumber, source: SocketAddrV6, @@ -504,6 +503,7 @@ fn ping_pong_ip6_relay( let mut server = TestServer::new((public_relay_ip4_addr, public_relay_ip6_addr)).with_nonce(nonce); let secret = server.auth_secret().to_owned(); + let lifetime = Lifetime::new(Duration::from_secs(60 * 60)).unwrap(); // Lifetime longer than channel expiry server.assert_commands( from_client( @@ -548,10 +548,10 @@ fn ping_pong_ip6_relay( ), now, ), - [send_message( - source, - channel_bind_response(channel_bind_transaction_id), - )], + [ + Wake(now + Duration::from_secs(60 * 10)), + send_message(source, channel_bind_response(channel_bind_transaction_id)), + ], ); let now = now + Duration::from_secs(1);