github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 18:18:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

test(connlib): don't route packets from IPs other than the client's (#5161)

Browse Source 
Currently, the gateway has a piece of functionality to ensure we only
ever route packets that actually originate from the client. This is
important because a gateway connects to multiple clients and thus -
without this check - client A could send a packet through the tunnel
that gets interpreted as traffic from client B by mangling the source IP
of their packets.

The portal assigns these source IPs when the clients sign in and passes
them to the gateway whenever a client connects. We can thus drop all
traffic on the gateway side from IPs that we don't recognise.

Currently, a client will still trigger a connection intent for an IP
packet, even if it doesn't have the tunnel's source IP set. We may want
to consider changing this behaviour in the future.
This commit is contained in:
Thomas Eizinger
2024-05-30 11:41:55 +10:00
committed by GitHub
parent cb9fe34437
commit 20cfcac7da
4 changed files with 98 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
rust/connlib/tunnel/proptest-regressions/tests.txt
View File

@@ -17,3 +17,4 @@ cc 4855625684cfe850e8b965097038ac8fed979cd21abd5284eb25e48136fcbb3e
cc bbc56c960f18a6115b3c114f2c5280a813fa2a12f21af383ad4154863fb944bc
cc 71165314a457a9f39928d882ffcc2ae02aedc2a35ce55b387c990b1bc5022a38 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 18548, tv_nsec: 387490499 }, utc_now: 2024-05-22T05:00:47.375132041Z, client: SimNode { id: ClientId(a89380c1-5379-5a2b-c5da-47fdeb956f75), state: PrivateKey("d348d4c5fa2ad05dfc686f289fb7084a16a095e208000844bc0ef7566b25addb"), ip4_socket: Some(127.0.0.1:41241), ip6_socket: Some([6e67:9b39:4a3f:a896:9e02:dc56:353e:65b3]:58740), tunnel_ip4: 100.111.145.90, tunnel_ip6: ::ffff:63.59.65.126 }, gateway: SimNode { id: GatewayId(d680e99a-31a7-9d8b-1052-6b85edfe42bc), state: PrivateKey("d39211badb6305b5b583574ee303b6a112b66311c38b4c22d68d69cb8d917bae"), ip4_socket: Some(82.215.208.98:18290), ip6_socket: Some([f122:b9a6:b3fb:bf35:319d:f921:d911:d012]:57327), tunnel_ip4: 100.81.19.26, tunnel_ip6: 3bfa:312f:c388:91b9:f6ce:95a6:a07:8784 }, relay: SimRelay { id: RelayId(a908052e-8823-7892-b1e2-e2c64deab06a), ip_stack: Dual { ip4: 75.190.170.204, ip6: ::ffff:0.235.25.126 }, allocations: {} }, client_cidr_resources: {}, connected_resources: {}, gateway_received_icmp_packets: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 28 }), name: "aaaa", address_description: "bmuf", sites: [Site { name: "zvkyrxzs", id: SiteId(67566938-ef2b-b442-64e0-f0f6fd05634d) }, Site { name: "hvzhiban", id: SiteId(4b7a5400-0fb2-2e42-444a-bc94fbaeed2e) }, Site { name: "fjmyjoh", id: SiteId(13340020-2717-880f-ba45-c823725c2234) }, Site { name: "fgrhxuvi", id: SiteId(8c260658-d7fb-c49c-c9f0-769cd8ced648) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000001), address: V4(Ipv4Network { network_address: 0.0.0.0, netmask: 28 }), name: "aaaa", address_description: "clkriftb", sites: [Site { name: "fgfg", id: SiteId(6b8d5470-3636-4879-2a32-94790962e6fd) }] }), SendICMPPacketToIp4Resource { r_idx: Index(11791212112709260679) }], None)
cc c9b812a8526dad4b00e366ea6b6d60054a4dd64c58849f1244f6fb2ba9d3686b # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 20170, tv_nsec: 301610515 }, utc_now: 2024-05-22T05:27:49.998877399Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: Some(127.0.0.1:1), ip6_socket: None, tunnel_ip4: 100.64.0.1, tunnel_ip6: ::ffff:0.0.0.0 }, gateway: SimNode { id: GatewayId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000001"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:12607), tunnel_ip4: 100.66.167.161, tunnel_ip6: ::ffff:155.93.248.155 }, relay: SimRelay { id: RelayId(c4e3332a-6f75-3a59-3ed8-15fbf26de134), ip_stack: Dual { ip4: 115.115.97.13, ip6: ::ffff:196.17.152.128 }, allocations: {} }, client_cidr_resources: {}, connected_resources: {}, gateway_received_icmp_packets: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V4(Ipv4Network { network_address: 127.0.0.1, netmask: 32 }), name: "aaaa", address_description: "mgnx", sites: [Site { name: "njmblcn", id: SiteId(b6f995cd-2c8e-a35d-117f-1e7affe3bc7b) }, Site { name: "angxwaw", id: SiteId(c1772dc0-60c6-ebfc-bcb5-978b288bbabf) }, Site { name: "vblitlg", id: SiteId(f41e2663-0254-e054-2353-0855dabea706) }, Site { name: "plcfnotvz", id: SiteId(72f3a8cc-8015-bf96-adcd-b40df5f25383) }, Site { name: "lflx", id: SiteId(214a47c8-5ce9-3e33-4738-da1a36d79d68) }, Site { name: "shfamjx", id: SiteId(1f2c00bf-3437-9ac9-c9d3-e66ebf3a1558) }, Site { name: "offgdto", id: SiteId(edb8c103-1dda-01f7-d648-47ccfad1eb97) }, Site { name: "gzss", id: SiteId(e2058be3-2fe7-5459-b8b3-eaee2c434828) }, Site { name: "ovougquk", id: SiteId(1410820a-7236-ac4d-e649-f276f3418e51) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000001), address: V4(Ipv4Network { network_address: 127.0.0.1, netmask: 32 }), name: "aaaa", address_description: "llgt", sites: [Site { name: "jmcxcphun", id: SiteId(190023c3-f08f-0b3c-ca68-ac5d45127341) }, Site { name: "mxofshkeo", id: SiteId(c39f0553-11d6-ddb0-1bc2-57e841e038fc) }, Site { name: "kaasxisn", id: SiteId(7c30df2b-95f9-0a6c-ebc7-92fd0a131376) }, Site { name: "hhitinvik", id: SiteId(39c252f6-4848-e5b7-1fe3-a4b886258e9a) }] }), SendICMPPacketToIp4Resource { r_idx: Index(0) }, SendICMPPacketToRandomIp { dst: 127.0.0.1 }], None)
cc d4d65c807696fb5a36f012090ef5bbcf3cc1917ddbbbe41d273387173b7b9044 # shrinks to (initial_state, transitions, seen_counter) = (ReferenceState { now: Instant { tv_sec: 4234, tv_nsec: 954499181 }, utc_now: 2024-05-30T01:02:43.851812081Z, client: SimNode { id: ClientId(00000000-0000-0000-0000-000000000000), state: PrivateKey("0000000000000000000000000000000000000000000000000000000000000000"), ip4_socket: None, ip6_socket: Some([::ffff:0.0.0.0]:1), tunnel_ip4: 100.64.0.1, tunnel_ip6: ::ffff:0.0.0.0 }, gateway: SimNode { id: GatewayId(b77d4d5f-3cf6-1b07-2957-bd465e5c2d9d), state: PrivateKey("498b2d67afe3cf56af8c010ec13ba4e185368077090cfbbb694c7a5cf03fb687"), ip4_socket: None, ip6_socket: Some([::ffff:85.37.17.81]:4098), tunnel_ip4: 100.109.173.208, tunnel_ip6: ::ffff:158.40.153.102 }, relay: SimRelay { id: RelayId(dcc26607-f113-9649-8ea5-2cd6366168cb), ip_stack: Dual { ip4: 127.0.0.1, ip6: ::ffff:217.253.220.229 }, allocations: {} }, system_dns_resolvers: [127.0.0.1, 199.164.14.248, 141.37.3.107], upstream_dns_resolvers: [IpPort(IpDnsServer { address: [::ffff:127.0.0.1]:53 }), IpPort(IpDnsServer { address: [cb46:599c:647e:cea8:ec5a:2437:92ac:d449]:53 })], client_cidr_resources: {}, connected_resources: {}, expected_icmp_handshakes: [] }, [AddCidrResource(ResourceDescriptionCidr { id: ResourceId(00000000-0000-0000-0000-000000000000), address: V6(Ipv6Network { network_address: ::ffff:0.0.0.0, netmask: 128 }), name: "aaaa", address_description: "xzthwhrin", sites: [Site { name: "ryjnkomlk", id: SiteId(cf128543-edc0-205e-06de-4c002a19df60) }, Site { name: "mvuvzltgn", id: SiteId(aafd15f6-eeb2-1396-81be-bc106f25bcec) }] }), SendICMPPacketToCidrResource { r_idx: Index(0), seq: 0, identifier: 0, src: Some(::ffff:0.0.0.0) }, SendICMPPacketToCidrResource { r_idx: Index(0), seq: 0, identifier: 0, src: Some(::ffff:127.0.0.1) }], None)

4
rust/connlib/tunnel/src/client.rs
View File

@@ -418,7 +418,7 @@ impl ClientState {
self.node.public_key()
}
#[tracing::instrument(level = "trace", skip_all, fields(dest))]
#[tracing::instrument(level = "trace", skip_all, fields(dst))]
pub(crate) fn encapsulate<'s>(
&'s mut self,
packet: MutableIpPacket<'_>,
@@ -432,7 +432,7 @@ impl ClientState {
Err(non_dns_packet) => non_dns_packet,
};
tracing::Span::current().record("dest", tracing::field::display(dest));
tracing::Span::current().record("dst", tracing::field::display(dest));
if is_definitely_not_a_resource(dest) {
return None;

6
rust/connlib/tunnel/src/gateway.rs
View File

@@ -175,6 +175,7 @@ impl GatewayState {
Some(transmit)
}
#[tracing::instrument(level = "trace", skip_all, fields(src, dst))]
pub(crate) fn decapsulate<'b>(
&mut self,
local: SocketAddr,
@@ -193,6 +194,9 @@ impl GatewayState {
.inspect_err(|e| tracing::warn!(%local, %from, num_bytes = %packet.len(), "Failed to decapsulate incoming packet: {e}"))
.ok()??;
tracing::Span::current().record("src", tracing::field::display(packet.source()));
tracing::Span::current().record("dst", tracing::field::display(packet.destination()));
let Some(peer) = self.peers.get_mut(&conn_id) else {
tracing::error!(%conn_id, %local, %from, "Couldn't find connection");
@@ -207,6 +211,8 @@ impl GatewayState {
return None;
}
tracing::trace!("Decapsulated packet");
Some(packet.into_immutable())
}

167
rust/connlib/tunnel/src/tests.rs
View File

@@ -106,17 +106,17 @@ enum Transition {
seq: u16,
identifier: u16,
},
/// Send a ICMP packet to an IPv4 resource.
SendICMPPacketToIp4Resource {
r_idx: sample::Index,
seq: u16,
identifier: u16,
},
/// Send a ICMP packet to an IPv6 resource.
SendICMPPacketToIp6Resource {
/// Send an ICMP packet to a CIDR resource.
SendICMPPacketToCidrResource {
r_idx: sample::Index,
seq: u16,
identifier: u16,
/// An optional source address.
///
/// Packets are only allowed to come from the IP assigned by the portal.
/// All others MUST be dropped by the gateway.
src: Option<IpAddr>,
},
/// The system's DNS servers changed.
UpdateSystemDnsServers { servers: Vec<IpAddr> },
@@ -245,14 +245,15 @@ impl StateMachineTest for TunnelTest {
buffered_transmits.extend(state.send_ip_packet_client_to_gateway(packet));
}
Transition::SendICMPPacketToIp4Resource {
Transition::SendICMPPacketToCidrResource {
r_idx,
seq,
identifier,
src,
} => {
let dst = ref_state.sample_ipv4_cidr_resource_dst(&r_idx);
let dst = ref_state.sample_cidr_resource_dst(&r_idx);
let packet = ip_packet::make::icmp_request_packet(
state.client.tunnel_ip(dst),
src.unwrap_or(state.client.tunnel_ip(dst)),
dst,
seq,
identifier,
@@ -260,21 +261,6 @@ impl StateMachineTest for TunnelTest {
buffered_transmits.extend(state.send_ip_packet_client_to_gateway(packet));
}
Transition::SendICMPPacketToIp6Resource {
r_idx,
seq,
identifier,
} => {
let dst = ref_state.sample_ipv6_cidr_resource_dst(&r_idx);
let packet = ip_packet::make::icmp_request_packet(
state.client.tunnel_ip(dst),
dst,
seq,
identifier,
);
buffered_transmits.extend(state.send_ip_packet_client_to_gateway(packet))
}
Transition::Tick { millis } => {
state.now += Duration::from_millis(millis);
}
@@ -290,20 +276,21 @@ impl StateMachineTest for TunnelTest {
}
};
state.advance(ref_state, &mut buffered_transmits);
assert!(buffered_transmits.is_empty()); // Sanity check to ensure we handled all packets.
// Assert our properties: Check that our actual state is equivalent to our expectation (the reference state).
for (resource_dst, seq, identifier) in ref_state.expected_icmp_handshakes.iter() {
let client_sent_request = &state
.client_sent_icmp_requests
.get(&(*seq, *identifier))
.remove(&(*seq, *identifier))
.unwrap();
let client_received_reply = &state
.client_received_icmp_replies
.get(&(*seq, *identifier))
.remove(&(*seq, *identifier))
.unwrap();
let gateway_received_request = &state
.gateway_received_requests
.get(&(*seq, *identifier))
.remove(&(*seq, *identifier))
.unwrap();
assert_eq!(
@@ -329,14 +316,23 @@ impl StateMachineTest for TunnelTest {
"ICMP request source == ICMP reply destination"
);
}
assert_eq!(
state.gateway_received_requests,
HashMap::new(),
"Unexpected ICMP requests on gateway"
);
assert_eq!(
state.client_received_icmp_replies,
HashMap::new(),
"Unexpected ICMP replies on client"
);
assert_eq!(
state.effective_dns_servers(),
ref_state.expected_dns_servers(),
"Effective DNS servers should match either system or upstream DNS"
);
assert!(buffered_transmits.is_empty()); // Sanity check to ensure we handled all packets.
state
}
}
@@ -422,8 +418,6 @@ impl ReferenceStateMachine for ReferenceState {
let set_upstream_dns_servers = upstream_dns_servers()
.prop_map(|servers| Transition::UpdateUpstreamDnsServers { servers });
let (num_ip4_resources, num_ip6_resources) = state.client_cidr_resources.len();
let mut strategies = vec![
(1, add_cidr_resource.boxed()),
(1, tick.boxed()),
@@ -432,12 +426,12 @@ impl ReferenceStateMachine for ReferenceState {
(1, icmp_to_random_ip().boxed()),
];
if num_ip4_resources > 0 {
strategies.push((3, icmp_to_ipv4_cidr_resource().boxed()));
}
if !state.client_cidr_resources.is_empty() {
strategies.push((10, icmp_to_cidr_resource_from_client().boxed()));
if num_ip6_resources > 0 {
strategies.push((3, icmp_to_ipv6_cidr_resource().boxed()));
// Packets from random source addresses are tested less frequently.
// Those are dropped by the gateway so this transition only ensures we have this safe-guard.
strategies.push((1, icmp_to_cidr_resource_from_random_src().boxed()));
}
Union::new_weighted(strategies).boxed()
@@ -447,6 +441,8 @@ impl ReferenceStateMachine for ReferenceState {
///
/// Here is where we implement the "expected" logic.
fn apply(mut state: Self::State, transition: &Self::Transition) -> Self::State {
state.expected_icmp_handshakes.clear();
match transition {
Transition::AddCidrResource(r) => {
state.client_cidr_resources.insert(r.address, r.clone());
@@ -456,23 +452,16 @@ impl ReferenceStateMachine for ReferenceState {
seq,
identifier,
} => {
state.on_icmp_packet(*dst, *seq, *identifier);
state.on_icmp_packet(None, *dst, *seq, *identifier);
}
Transition::SendICMPPacketToIp4Resource {
Transition::SendICMPPacketToCidrResource {
r_idx,
seq,
identifier,
src,
} => {
let dst = state.sample_ipv4_cidr_resource_dst(r_idx);
state.on_icmp_packet(dst, *seq, *identifier);
}
Transition::SendICMPPacketToIp6Resource {
r_idx,
seq,
identifier,
} => {
let dst = state.sample_ipv6_cidr_resource_dst(r_idx);
state.on_icmp_packet(dst, *seq, *identifier);
let dst = state.sample_cidr_resource_dst(r_idx);
state.on_icmp_packet(*src, dst, *seq, *identifier);
}
Transition::Tick { millis } => state.now += Duration::from_millis(*millis),
Transition::UpdateSystemDnsServers { servers } => {
@@ -508,31 +497,21 @@ impl ReferenceStateMachine for ReferenceState {
seq,
identifier,
} => state.is_valid_icmp_packet(dst, seq, identifier),
Transition::SendICMPPacketToIp4Resource {
Transition::SendICMPPacketToCidrResource {
r_idx,
seq,
identifier,
src,
} => {
if state.client_cidr_resources.len().0 == 0 {
if state.client_cidr_resources.is_empty() {
return false;
}
let dst = state.sample_ipv4_cidr_resource_dst(r_idx);
let dst = state.sample_cidr_resource_dst(r_idx);
let is_valid_icmp_packet = state.is_valid_icmp_packet(&dst, seq, identifier);
state.is_valid_icmp_packet(&dst.into(), seq, identifier)
}
Transition::SendICMPPacketToIp6Resource {
r_idx,
seq,
identifier,
} => {
if state.client_cidr_resources.len().1 == 0 {
return false;
}
let dst = state.sample_ipv6_cidr_resource_dst(r_idx);
state.is_valid_icmp_packet(&dst.into(), seq, identifier)
is_valid_icmp_packet
&& (src.is_none() || src.is_some_and(|s| s.is_ipv4() == dst.is_ipv4()))
}
Transition::UpdateSystemDnsServers { .. } => true,
Transition::UpdateUpstreamDnsServers { .. } => true,
@@ -972,8 +951,16 @@ impl TunnelTest {
/// Several helper functions to make the reference state more readable.
impl ReferenceState {
#[tracing::instrument(level = "debug", skip_all, fields(dst, resource))]
fn on_icmp_packet(&mut self, dst: impl Into<IpAddr>, seq: u16, identifier: u16) {
fn on_icmp_packet(
&mut self,
src: Option<IpAddr>,
dst: impl Into<IpAddr>,
seq: u16,
identifier: u16,
) {
let dst = dst.into();
let packet_originates_from_client =
src.is_none() || src.is_some_and(|src| src == self.client.tunnel_ip(src)); // Packets from IPs other than the client's are dropped by the gateway. We still create connections for them though.
tracing::Span::current().record("dst", tracing::field::display(dst));
// Second, if we are not yet connected, check if we have a resource for this IP.
@@ -982,7 +969,7 @@ impl ReferenceState {
return;
};
if self.connected_resources.contains(&resource.id) {
if self.connected_resources.contains(&resource.id) && packet_originates_from_client {
tracing::debug!("Connected to resource, expecting packet to be routed");
self.expected_icmp_handshakes
.push_back((dst, seq, identifier));
@@ -994,6 +981,22 @@ impl ReferenceState {
self.connected_resources.insert(resource.id);
}
fn sample_cidr_resource_dst(&self, idx: &sample::Index) -> IpAddr {
let (num_ip4_resources, num_ip6_resources) = self.client_cidr_resources.len();
let mut ips = Vec::new();
if num_ip4_resources > 0 {
ips.push(self.sample_ipv4_cidr_resource_dst(idx).into())
}
if num_ip6_resources > 0 {
ips.push(self.sample_ipv6_cidr_resource_dst(idx).into())
}
*idx.get(&ips)
}
/// Samples an [`Ipv4Addr`] from _any_ of our IPv4 CIDR resources.
fn sample_ipv4_cidr_resource_dst(&self, idx: &sample::Index) -> Ipv4Addr {
let num_ip4_resources = self.client_cidr_resources.len().0;
@@ -1435,24 +1438,32 @@ fn icmp_to_random_ip() -> impl Strategy<Value = Transition> {
})
}
fn icmp_to_ipv4_cidr_resource() -> impl Strategy<Value = Transition> {
fn icmp_to_cidr_resource_from_client() -> impl Strategy<Value = Transition> {
(any::<sample::Index>(), any::<u16>(), any::<u16>()).prop_map(
move |(r_idx, seq, identifier)| Transition::SendICMPPacketToIp4Resource {
move |(r_idx, seq, identifier)| Transition::SendICMPPacketToCidrResource {
r_idx,
seq,
identifier,
src: None,
},
)
}
fn icmp_to_ipv6_cidr_resource() -> impl Strategy<Value = Transition> {
(any::<sample::Index>(), any::<u16>(), any::<u16>()).prop_map(
move |(r_idx, seq, identifier)| Transition::SendICMPPacketToIp6Resource {
r_idx,
seq,
identifier,
},
fn icmp_to_cidr_resource_from_random_src() -> impl Strategy<Value = Transition> {
(
any::<sample::Index>(),
any::<u16>(),
any::<u16>(),
any::<IpAddr>(),
)
.prop_map(move |(r_idx, seq, identifier, src)| {
Transition::SendICMPPacketToCidrResource {
r_idx,
seq,
identifier,
src: Some(src),
}
})
}
fn client_id() -> impl Strategy<Value = ClientId> {