Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(relay): actually expire channels which allows re-binding them #4094

Merged
merged 3 commits into from
Mar 12, 2024
Merged

fix(relay): actually expire channels which allows re-binding them #4094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc446d2
Actually expiry channels and allow rebinding
thomaseizinger Mar 12, 2024
99476e2
Fix ping-pong tests
thomaseizinger Mar 12, 2024
9045747
Merge branch 'main' into fix/relay/actually-expire-channels
thomaseizinger Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 20 additions & 4 deletions rust/relay/src/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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<Attribute>, recipient: ClientSocket) {
Expand Down Expand Up @@ -1073,7 +1089,7 @@ impl Allocation {
}
}

#[derive(PartialEq)]
#[derive(PartialEq, Debug)]
enum TimedAction {
ExpireAllocation(AllocationId),
UnbindChannel((ClientSocket, u16)),
Expand Down
3 changes: 3 additions & 0 deletions rust/relay/src/time_events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<A> {
events: Vec<TimeEvent<A>>,
}
Expand All @@ -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 {
Expand Down Expand Up @@ -85,6 +87,7 @@ impl<A> PartialOrd for TimeEvent<A> {
}

#[cfg(test)]
#[allow(unused_must_use)]
mod tests {
use super::*;
use std::time::Duration;
Expand Down
135 changes: 125 additions & 10 deletions rust/relay/tests/regression.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand All @@ -376,12 +376,117 @@ 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(); // Lifetime longer than channel expiry

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,
#[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,
Expand All @@ -398,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(
Expand Down Expand Up @@ -442,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);
Expand Down Expand Up @@ -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 },
Expand Down