diff --git a/.github/workflows/_rust.yml b/.github/workflows/_rust.yml index b95a57370..28b41364e 100644 --- a/.github/workflows/_rust.yml +++ b/.github/workflows/_rust.yml @@ -106,7 +106,10 @@ jobs: rg --count --no-ignore "Packet for CIDR resource" "$TESTCASES_DIR" rg --count --no-ignore "Packet for Internet resource" "$TESTCASES_DIR" rg --count --no-ignore "Truncating DNS response" "$TESTCASES_DIR" - rg --count --no-ignore "Destination is unreachable" "$TESTCASES_DIR" + rg --count --no-ignore "ICMP Error error=V4Unreachable" "$TESTCASES_DIR" + rg --count --no-ignore "ICMP Error error=V6Unreachable" "$TESTCASES_DIR" + rg --count --no-ignore "ICMP Error error=V4TimeExceeded" "$TESTCASES_DIR" + rg --count --no-ignore "ICMP Error error=V6TimeExceeded" "$TESTCASES_DIR" rg --count --no-ignore "Forwarding query for DNS resource to corresponding site" "$TESTCASES_DIR" env: diff --git a/rust/connlib/ip-packet/src/icmp_dest_unreachable.rs b/rust/connlib/ip-packet/src/icmp_error.rs similarity index 83% rename from rust/connlib/ip-packet/src/icmp_dest_unreachable.rs rename to rust/connlib/ip-packet/src/icmp_error.rs index ce574277c..202bd783f 100644 --- a/rust/connlib/ip-packet/src/icmp_dest_unreachable.rs +++ b/rust/connlib/ip-packet/src/icmp_error.rs @@ -6,39 +6,44 @@ use etherparse::{Icmpv4Type, Icmpv6Type, LaxIpv4Slice, LaxIpv6Slice, icmpv4, icm use crate::{Layer4Protocol, Protocol}; #[derive(Debug, Clone, PartialEq, Eq)] -pub enum DestUnreachable { - V4 { - header: icmpv4::DestUnreachableHeader, - total_length: u16, - }, +pub enum IcmpError { + V4Unreachable(icmpv4::DestUnreachableHeader), + V4TimeExceeded(icmpv4::TimeExceededCode), V6Unreachable(icmpv6::DestUnreachableCode), - V6PacketTooBig { - mtu: u32, - }, + V6PacketTooBig { mtu: u32 }, + V6TimeExceeded(icmpv6::TimeExceededCode), } -impl DestUnreachable { +impl IcmpError { pub fn into_icmp_v4_type(self) -> Result { - let header = match self { - DestUnreachable::V4 { header, .. } => header, - DestUnreachable::V6Unreachable(_) => { + let icmpv4_type = match self { + IcmpError::V4Unreachable(header) => Icmpv4Type::DestinationUnreachable(header), + IcmpError::V4TimeExceeded(code) => Icmpv4Type::TimeExceeded(code), + IcmpError::V6Unreachable(_) => { bail!("Cannot translate IPv6 unreachable to ICMPv4") } - DestUnreachable::V6PacketTooBig { .. } => { + IcmpError::V6PacketTooBig { .. } => { bail!("Cannot translate IPv6 packet too big to ICMPv4") } + IcmpError::V6TimeExceeded { .. } => { + bail!("Cannot translate IPv6 packet time exceeded to ICMPv4") + } }; - Ok(Icmpv4Type::DestinationUnreachable(header)) + Ok(icmpv4_type) } pub fn into_icmp_v6_type(self) -> Result { match self { - DestUnreachable::V4 { .. } => { + IcmpError::V4Unreachable { .. } => { bail!("Cannot translate IPv4 unreachable to ICMPv6") } - DestUnreachable::V6Unreachable(code) => Ok(Icmpv6Type::DestinationUnreachable(code)), - DestUnreachable::V6PacketTooBig { mtu } => Ok(Icmpv6Type::PacketTooBig { mtu }), + IcmpError::V4TimeExceeded { .. } => { + bail!("Cannot translate IPv4 time exceeded to ICMPv6") + } + IcmpError::V6Unreachable(code) => Ok(Icmpv6Type::DestinationUnreachable(code)), + IcmpError::V6PacketTooBig { mtu } => Ok(Icmpv6Type::PacketTooBig { mtu }), + IcmpError::V6TimeExceeded(code) => Ok(Icmpv6Type::TimeExceeded(code)), } } } diff --git a/rust/connlib/ip-packet/src/lib.rs b/rust/connlib/ip-packet/src/lib.rs index 9c976aead..68983253c 100644 --- a/rust/connlib/ip-packet/src/lib.rs +++ b/rust/connlib/ip-packet/src/lib.rs @@ -4,7 +4,7 @@ pub mod make; mod fz_p2p_control; mod fz_p2p_control_slice; -mod icmp_dest_unreachable; +mod icmp_error; #[cfg(feature = "proptest")] #[allow(clippy::unwrap_used)] @@ -14,7 +14,7 @@ use bufferpool::{Buffer, BufferPool}; pub use etherparse::*; pub use fz_p2p_control::EventType as FzP2pEventType; pub use fz_p2p_control_slice::FzP2pControlSlice; -pub use icmp_dest_unreachable::{DestUnreachable, FailedPacket}; +pub use icmp_error::{FailedPacket, IcmpError}; use anyhow::{Context as _, Result, bail}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; @@ -589,8 +589,8 @@ impl IpPacket { .ok() } - /// In case the packet is an ICMP unreachable error, parses the unroutable packet from the ICMP payload. - pub fn icmp_unreachable_destination(&self) -> Result> { + /// In case the packet is an ICMP error with a failed packet, parses the failed packet from the ICMP payload. + pub fn icmp_error(&self) -> Result> { if let Some(icmpv4) = self.as_icmpv4() { let icmp_type = icmpv4.icmp_type(); @@ -602,8 +602,14 @@ impl IpPacket { return Ok(None); } - let Icmpv4Type::DestinationUnreachable(error) = icmp_type else { - bail!("ICMP message is not `DestinationUnreachable` but {icmp_type:?}"); + #[expect( + clippy::wildcard_enum_match_arm, + reason = "We only want to match on these variants" + )] + let icmp_error = match icmp_type { + Icmpv4Type::DestinationUnreachable(error) => IcmpError::V4Unreachable(error), + Icmpv4Type::TimeExceeded(code) => IcmpError::V4TimeExceeded(code), + other => bail!("ICMP message {other:?} is not supported"), }; let (ipv4, _) = LaxIpv4Slice::from_slice(icmpv4.payload()) @@ -622,10 +628,7 @@ impl IpPacket { l4_proto, raw: icmpv4.payload().to_vec(), }, - DestUnreachable::V4 { - header: error, - total_length: header.total_len(), - }, + icmp_error, ))); } @@ -642,16 +645,13 @@ impl IpPacket { #[expect( clippy::wildcard_enum_match_arm, - reason = "We only want to match on these two variants" + reason = "We only want to match on these variants" )] - let dest_unreachable = match icmp_type { - Icmpv6Type::DestinationUnreachable(error) => DestUnreachable::V6Unreachable(error), - Icmpv6Type::PacketTooBig { mtu } => DestUnreachable::V6PacketTooBig { mtu }, - other => { - bail!( - "ICMP message is not `DestinationUnreachable` or `PacketTooBig` but {other:?}" - ); - } + let icmp_error = match icmp_type { + Icmpv6Type::DestinationUnreachable(error) => IcmpError::V6Unreachable(error), + Icmpv6Type::PacketTooBig { mtu } => IcmpError::V6PacketTooBig { mtu }, + Icmpv6Type::TimeExceeded(code) => IcmpError::V6TimeExceeded(code), + other => bail!("ICMPv6 message {other:?} is not supported"), }; let (ipv6, _) = LaxIpv6Slice::from_slice(icmpv6.payload()) @@ -670,7 +670,7 @@ impl IpPacket { l4_proto, raw: icmpv6.payload().to_vec(), }, - dest_unreachable, + icmp_error, ))); } diff --git a/rust/connlib/ip-packet/src/make.rs b/rust/connlib/ip-packet/src/make.rs index 659634dc1..6c570df93 100644 --- a/rust/connlib/ip-packet/src/make.rs +++ b/rust/connlib/ip-packet/src/make.rs @@ -283,10 +283,7 @@ mod tests { IpAddr::V4(Ipv4Addr::new(10, 0, 0, 1)) ); assert_eq!(icmp_error.source(), IpAddr::V4(Ipv4Addr::LOCALHOST)); - assert!(matches!( - icmp_error.icmp_unreachable_destination(), - Ok(Some(_)) - )); + assert!(matches!(icmp_error.icmp_error(), Ok(Some(_)))); } #[test_strategy::proptest()] @@ -314,10 +311,7 @@ mod tests { IpAddr::V6(Ipv6Addr::new(1, 0, 0, 0, 0, 0, 0, 1)) ); assert_eq!(icmp_error.source(), IpAddr::V6(Ipv6Addr::LOCALHOST)); - assert!(matches!( - icmp_error.icmp_unreachable_destination(), - Ok(Some(_)) - )); + assert!(matches!(icmp_error.icmp_error(), Ok(Some(_)))); } fn payload(max_size: usize) -> impl Strategy> { diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index 1282b3a6b..6af7700f2 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -176,4 +176,10 @@ cc 7ab081a00991a3265b2ca82f2203284759bc50ef2805e5514baa0c24c966a580 cc 9cac073e45583d9940fd8813b93c4cadea91c5d304c454ab8d050b44ba49dc13 cc 608f3ed9392aa067bc730538d75f3692edf2ad5c3fa98beb3e95b166e04f7b5f cc 57c9d6263fdae8b6bb51fbb7108372c7d695d1186163fcfcdce010a6666c3db5 +cc ac941192fcad26613cd6fdf71abf0ed13b5b8bde938b5cbfeb402aa9f6ad4cd3 +cc 3b93f4549f2d3d4b559ba8c331d270e99e124cb6ed61e199c90fef2676bf2fca +cc 788990b99a7cb43f54e7d82a3a66f3317a3d2ca39236db6fa8f6855ba86fe9f5 cc f90e2fe4827f91aba42bf4806b53d1a1e7df7d1fd912d3a2cf32774eb0006f8a +cc 964a23e1cc7b6b3ef5f65b48228ecb13c4fbb136f053f33ee254c21c8f404f6e +cc 589929cc32dca7f976c0b06d0f165ebaa35a4a8bf8dbcc0145041e36d0153b9c +cc b7f1a952ce4b23c0dfb2383e3fa245222e3093b2a35bc83569c934481993e5e3 diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index 48c9fd36e..0125c0dba 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -318,10 +318,7 @@ impl ClientOnGateway { self.ensure_client_ip(packet.destination())?; // Always allow ICMP errors to pass through, even in the presence of filters that don't allow ICMP. - if packet - .icmp_unreachable_destination() - .is_ok_and(|e| e.is_some()) - { + if packet.icmp_error().is_ok_and(|e| e.is_some()) { return Ok(Some(packet)); } @@ -409,12 +406,12 @@ impl ClientOnGateway { ) -> anyhow::Result> { let (proto, ip) = match self.nat_table.translate_incoming(&packet, now)? { TranslateIncomingResult::Ok { proto, src } => (proto, src), - TranslateIncomingResult::DestinationUnreachable(prototype) => { - tracing::debug!(dst = %prototype.outside_dst(), proxy_ip = %prototype.inside_dst(), error = ?prototype.error(), "Destination is unreachable"); + TranslateIncomingResult::IcmpError(prototype) => { + tracing::debug!(error = ?prototype.error(), dst = %prototype.outside_dst(), proxy_ip = %prototype.inside_dst(), "ICMP Error"); let icmp_error = prototype .into_packet(self.client_tun.v4, self.client_tun.v6) - .context("Failed to create `DestinationUnreachable` ICMP error")?; + .context("Failed to create ICMP error")?; return Ok(Some(icmp_error)); } diff --git a/rust/connlib/tunnel/src/peer/nat_table.rs b/rust/connlib/tunnel/src/peer/nat_table.rs index dc648e6ac..46edc06ad 100644 --- a/rust/connlib/tunnel/src/peer/nat_table.rs +++ b/rust/connlib/tunnel/src/peer/nat_table.rs @@ -1,7 +1,7 @@ //! A stateful symmetric NAT table that performs conversion between a client's picked proxy ip and the actual resource's IP. use anyhow::{Context, Result}; use bimap::BiMap; -use ip_packet::{DestUnreachable, FailedPacket, IpPacket, PacketBuilder, Protocol}; +use ip_packet::{FailedPacket, IcmpError, IpPacket, PacketBuilder, Protocol}; use std::collections::{BTreeMap, HashSet}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::time::{Duration, Instant}; @@ -107,18 +107,16 @@ impl NatTable { packet: &IpPacket, now: Instant, ) -> Result { - if let Some((failed_packet, icmp_error)) = packet.icmp_unreachable_destination()? { + if let Some((failed_packet, icmp_error)) = packet.icmp_error()? { let outside = (failed_packet.src_proto(), failed_packet.dst()); if let Some((inside_proto, inside_dst)) = self.translate_incoming_inner(&outside, now) { - return Ok(TranslateIncomingResult::DestinationUnreachable( - DestinationUnreachablePrototype { - inside_dst, - inside_proto, - failed_packet, - icmp_error, - }, - )); + return Ok(TranslateIncomingResult::IcmpError(IcmpErrorPrototype { + inside_dst, + inside_proto, + failed_packet, + icmp_error, + })); } if self.expired.contains(&outside) { @@ -168,27 +166,27 @@ impl NatTable { } } -/// A prototype for an ICMP "Destination unreachable" packet. +/// A prototype for an ICMP error packet. /// -/// A packet coming in from the "outside" of the NAT may be an ICMP "Destination unreachable" error. +/// A packet coming in from the "outside" of the NAT may be an ICMP error. /// In that case, our regular NAT lookup will fail as that one relies on Layer-4 protocol (TCP/UDP port or ICMP identifier). /// /// ICMP error messages contain a part of the original IP packet that could not be routed. /// In order for the NAT to be transparent, the IP and protocol layer within that original packet also need to be translated. #[derive(Debug, PartialEq, Eq)] -pub struct DestinationUnreachablePrototype { +pub struct IcmpErrorPrototype { /// The "original" destination IP that could not be reached. /// /// This is a "proxy IP" as generated by the Firezone client during DNS resolution. inside_dst: IpAddr, inside_proto: Protocol, - icmp_error: DestUnreachable, + icmp_error: IcmpError, failed_packet: FailedPacket, } -impl DestinationUnreachablePrototype { +impl IcmpErrorPrototype { /// Turns this prototype into an actual ICMP error IP packet, targeting the given IPv4/IPv6 address, depending on the original Resource address. pub fn into_packet(self, dst_v4: Ipv4Addr, dst_v6: Ipv6Addr) -> Result { // First, translate the failed packet as if it would have directly originated from the client (without our NAT applied). @@ -216,7 +214,7 @@ impl DestinationUnreachablePrototype { } } - pub fn error(&self) -> &DestUnreachable { + pub fn error(&self) -> &IcmpError { &self.icmp_error } @@ -232,7 +230,7 @@ impl DestinationUnreachablePrototype { #[derive(Debug, PartialEq)] pub enum TranslateIncomingResult { Ok { proto: Protocol, src: IpAddr }, - DestinationUnreachable(DestinationUnreachablePrototype), + IcmpError(IcmpErrorPrototype), ExpiredNatSession, NoNatSession, } @@ -341,7 +339,7 @@ mod tests { TranslateIncomingResult::Ok { proto, src } => (proto, src), TranslateIncomingResult::NoNatSession | TranslateIncomingResult::ExpiredNatSession - | TranslateIncomingResult::DestinationUnreachable(_) => panic!("Wrong result"), + | TranslateIncomingResult::IcmpError(_) => panic!("Wrong result"), } }); @@ -376,7 +374,7 @@ mod tests { TranslateIncomingResult::Ok { .. } => {} result @ (TranslateIncomingResult::NoNatSession | TranslateIncomingResult::ExpiredNatSession - | TranslateIncomingResult::DestinationUnreachable(_)) => { + | TranslateIncomingResult::IcmpError(_)) => { panic!("Wrong result: {result:?}") } }; @@ -389,7 +387,7 @@ mod tests { TranslateIncomingResult::ExpiredNatSession => {} result @ (TranslateIncomingResult::NoNatSession | TranslateIncomingResult::Ok { .. } - | TranslateIncomingResult::DestinationUnreachable(_)) => { + | TranslateIncomingResult::IcmpError(_)) => { panic!("Wrong result: {result:?}") } }; diff --git a/rust/connlib/tunnel/src/tests.rs b/rust/connlib/tunnel/src/tests.rs index 764202a29..8b686309b 100644 --- a/rust/connlib/tunnel/src/tests.rs +++ b/rust/connlib/tunnel/src/tests.rs @@ -19,6 +19,7 @@ mod composite_strategy; mod dns_records; mod dns_server_resource; mod flux_capacitor; +mod icmp_error_hosts; mod reference; mod sim_client; mod sim_gateway; @@ -29,7 +30,6 @@ mod stub_portal; mod sut; mod tcp; mod transition; -mod unreachable_hosts; type QueryId = u16; diff --git a/rust/connlib/tunnel/src/tests/assertions.rs b/rust/connlib/tunnel/src/tests/assertions.rs index 264e6bded..831397be7 100644 --- a/rust/connlib/tunnel/src/tests/assertions.rs +++ b/rust/connlib/tunnel/src/tests/assertions.rs @@ -208,7 +208,7 @@ fn assert_packets_properties( let Some(gateway_received_request) = received_requests.get(payload) else { if client_received_reply - .icmp_unreachable_destination() + .icmp_error() .ok() .is_some_and(|icmp| icmp.is_some()) { diff --git a/rust/connlib/tunnel/src/tests/unreachable_hosts.rs b/rust/connlib/tunnel/src/tests/icmp_error_hosts.rs similarity index 77% rename from rust/connlib/tunnel/src/tests/unreachable_hosts.rs rename to rust/connlib/tunnel/src/tests/icmp_error_hosts.rs index 562a20b2e..554059293 100644 --- a/rust/connlib/tunnel/src/tests/unreachable_hosts.rs +++ b/rust/connlib/tunnel/src/tests/icmp_error_hosts.rs @@ -8,24 +8,20 @@ use proptest::{prelude::*, sample}; use super::dns_records::DnsRecords; #[derive(Debug, Clone)] -pub(crate) struct UnreachableHosts { +pub(crate) struct IcmpErrorHosts { inner: BTreeMap, } -impl UnreachableHosts { +impl IcmpErrorHosts { pub(crate) fn icmp_error_for_ip(&self, ip: IpAddr) -> Option { self.inner.get(&ip).copied() } - - pub(crate) fn is_unreachable(&self, ip: IpAddr) -> bool { - self.inner.contains_key(&ip) - } } -/// Samples a subset of the provided DNS records which we will treat as "unreachable". -pub(crate) fn unreachable_hosts( +/// Samples a subset of the provided DNS records which we will generate ICMP errors. +pub(crate) fn icmp_error_hosts( dns_resource_records: DnsRecords, -) -> impl Strategy { +) -> impl Strategy { // First, deduplicate all IPs. let unique_ips = dns_resource_records.ips_iter().collect::>(); let ips = Vec::from_iter(unique_ips); @@ -35,7 +31,7 @@ pub(crate) fn unreachable_hosts( .prop_flat_map(|ips| { let num_ips = ips.len(); - sample::subsequence(ips, 0..num_ips) // Pick a subset of the unreachable IPs. + sample::subsequence(ips, 0..num_ips) // Pick a subset of IPs. }) .prop_flat_map(|ips| { ips.into_iter() @@ -43,7 +39,7 @@ pub(crate) fn unreachable_hosts( .collect::>() }) .prop_map(BTreeMap::from_iter) - .prop_map(|inner| UnreachableHosts { inner }) + .prop_map(|inner| IcmpErrorHosts { inner }) } fn icmp_error() -> impl Strategy { @@ -51,7 +47,8 @@ fn icmp_error() -> impl Strategy { Just(IcmpError::Network), Just(IcmpError::Host), Just(IcmpError::Port), - any::().prop_map(|mtu| IcmpError::PacketTooBig { mtu }) + any::().prop_map(|mtu| IcmpError::PacketTooBig { mtu }), + Just(IcmpError::TimeExceeded { code: 0 }) ] } @@ -62,4 +59,5 @@ pub(crate) enum IcmpError { Host, Port, PacketTooBig { mtu: u32 }, + TimeExceeded { code: u8 }, } diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index 7029ab7fb..e3c1e1109 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -1,5 +1,5 @@ use super::dns_records::DnsRecords; -use super::unreachable_hosts::{UnreachableHosts, unreachable_hosts}; +use super::icmp_error_hosts::{IcmpErrorHosts, icmp_error_hosts}; use super::{ composite_strategy::CompositeStrategy, sim_client::*, sim_gateway::*, sim_net::*, strategies::*, stub_portal::StubPortal, transition::*, @@ -41,7 +41,7 @@ pub(crate) struct ReferenceState { pub(crate) tcp_resources: BTreeMap>, /// A subset of all DNS resource records that have been selected to produce an ICMP error. - pub(crate) unreachable_hosts: UnreachableHosts, + pub(crate) icmp_error_hosts: IcmpErrorHosts, pub(crate) network: RoutingTable, } @@ -87,7 +87,7 @@ impl ReferenceState { Just(gateways), Just(portal), Just(dns_resource_records.clone()), - unreachable_hosts(dns_resource_records), + icmp_error_hosts(dns_resource_records), Just(relays), Just(global_dns), Just(drop_direct_client_traffic), @@ -100,7 +100,7 @@ impl ReferenceState { gateways, portal, dns_resource_records, - unreachable_hosts, + icmp_error_hosts, relays, global_dns, drop_direct_client_traffic, @@ -110,8 +110,8 @@ impl ReferenceState { Just(gateways), Just(portal), Just(dns_resource_records.clone()), - Just(unreachable_hosts.clone()), - tcp_resources(dns_resource_records, unreachable_hosts), + Just(icmp_error_hosts.clone()), + tcp_resources(dns_resource_records, icmp_error_hosts), Just(relays), Just(global_dns), Just(drop_direct_client_traffic), @@ -125,7 +125,7 @@ impl ReferenceState { gateways, portal, records, - unreachable_hosts, + icmp_error_hosts, tcp_resources, relays, mut global_dns, @@ -157,8 +157,8 @@ impl ReferenceState { relays, portal, global_dns, - unreachable_hosts, tcp_resources, + icmp_error_hosts, drop_direct_client_traffic, routing_table, )) @@ -183,8 +183,8 @@ impl ReferenceState { relays, portal, global_dns_records, - unreachable_hosts, tcp_resources, + icmp_error_hosts, drop_direct_client_traffic, network, )| { @@ -194,7 +194,7 @@ impl ReferenceState { relays, portal, global_dns_records, - unreachable_hosts, + icmp_error_hosts, network, drop_direct_client_traffic, tcp_resources, diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index e678658f3..39ea8e0a0 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -244,7 +244,7 @@ impl SimClient { /// Process an IP packet received on the client. pub(crate) fn on_received_packet(&mut self, packet: IpPacket) { - match packet.icmp_unreachable_destination() { + match packet.icmp_error() { Ok(Some((failed_packet, _))) => { match failed_packet.layer4_protocol() { Layer4Protocol::Udp { src, dst } => { diff --git a/rust/connlib/tunnel/src/tests/sim_gateway.rs b/rust/connlib/tunnel/src/tests/sim_gateway.rs index 5fd00d6e9..4f4d62e5a 100644 --- a/rust/connlib/tunnel/src/tests/sim_gateway.rs +++ b/rust/connlib/tunnel/src/tests/sim_gateway.rs @@ -1,11 +1,11 @@ use super::{ dns_records::DnsRecords, dns_server_resource::{TcpDnsServerResource, UdpDnsServerResource}, + icmp_error_hosts::{IcmpError, IcmpErrorHosts}, reference::{PrivateKey, private_key}, sim_net::{Host, dual_ip_stack, host}, sim_relay::{SimRelay, map_explode}, strategies::latency, - unreachable_hosts::{IcmpError, UnreachableHosts}, }; use crate::{GatewayState, IpConfig}; use anyhow::{Result, bail}; @@ -72,7 +72,7 @@ impl SimGateway { pub(crate) fn receive( &mut self, transmit: Transmit, - unreachable_hosts: &UnreachableHosts, + icmp_error_hosts: &IcmpErrorHosts, now: Instant, utc_now: DateTime, ) -> Option { @@ -87,7 +87,7 @@ impl SimGateway { return None; }; - self.on_received_packet(packet, unreachable_hosts, now) + self.on_received_packet(packet, icmp_error_hosts, now) } pub(crate) fn advance_resources( @@ -172,7 +172,7 @@ impl SimGateway { fn on_received_packet( &mut self, packet: IpPacket, - unreachable_hosts: &UnreachableHosts, + icmp_error_hosts: &IcmpErrorHosts, now: Instant, ) -> Option { // TODO: Instead of handling these things inline, here, should we dispatch them via `RoutingTable`? @@ -183,7 +183,7 @@ impl SimGateway { // If so, generate the error reply. // We still want to do all the book-keeping in terms of tracking which requests we received. // Therefore, pass the generated `icmp_error` to resulting `handle_` functions instead of sending it right away. - let icmp_error = unreachable_hosts + let icmp_error = icmp_error_hosts .icmp_error_for_ip(dst_ip) .map(|icmp_error| icmp_error_reply(&packet, icmp_error).unwrap()); @@ -368,16 +368,25 @@ fn icmp_error_reply(packet: &IpPacket, error: IcmpError) -> Result { match (src, dst) { (IpAddr::V4(src), IpAddr::V4(dst)) => { let icmpv4 = ip_packet::PacketBuilder::ipv4(src.octets(), dst.octets(), 20).icmpv4( - Icmpv4Type::DestinationUnreachable(match error { - IcmpError::Network => icmpv4::DestUnreachableHeader::Network, - IcmpError::Host => icmpv4::DestUnreachableHeader::Host, - IcmpError::Port => icmpv4::DestUnreachableHeader::Port, - IcmpError::PacketTooBig { mtu } => { + match error { + IcmpError::Network => { + Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Network) + } + IcmpError::Host => { + Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Host) + } + IcmpError::Port => { + Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Port) + } + IcmpError::PacketTooBig { mtu } => Icmpv4Type::DestinationUnreachable( icmpv4::DestUnreachableHeader::FragmentationNeeded { next_hop_mtu: u16::try_from(mtu).unwrap_or(u16::MAX), - } + }, + ), + IcmpError::TimeExceeded { code } => { + Icmpv4Type::TimeExceeded(icmpv4::TimeExceededCode::from_u8(code).unwrap()) } - }), + }, ); ip_packet::build!(icmpv4, payload) @@ -395,6 +404,9 @@ fn icmp_error_reply(packet: &IpPacket, error: IcmpError) -> Result { Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::Port) } IcmpError::PacketTooBig { mtu } => Icmpv6Type::PacketTooBig { mtu }, + IcmpError::TimeExceeded { code } => { + Icmpv6Type::TimeExceeded(icmpv6::TimeExceededCode::from_u8(code).unwrap()) + } }, ); diff --git a/rust/connlib/tunnel/src/tests/strategies.rs b/rust/connlib/tunnel/src/tests/strategies.rs index 3bb111e7b..aafe52d9a 100644 --- a/rust/connlib/tunnel/src/tests/strategies.rs +++ b/rust/connlib/tunnel/src/tests/strategies.rs @@ -1,5 +1,5 @@ use super::dns_records::DnsRecords; -use super::unreachable_hosts::UnreachableHosts; +use super::icmp_error_hosts::IcmpErrorHosts; use super::{sim_net::Host, sim_relay::ref_relay_host, stub_portal::StubPortal}; use crate::client::{ CidrResource, DNS_SENTINELS_V4, DNS_SENTINELS_V6, DnsResource, IPV4_RESOURCES, IPV6_RESOURCES, @@ -160,7 +160,7 @@ pub(crate) fn stub_portal() -> impl Strategy { /// The port is sampled together with domain. pub(crate) fn tcp_resources( dns_records: DnsRecords, - unreachable_hosts: UnreachableHosts, + imcp_error_hosts: IcmpErrorHosts, ) -> impl Strategy>> { let all_domains = dns_records.domains_iter().collect::>(); @@ -174,7 +174,7 @@ pub(crate) fn tcp_resources( .filter(|(domain, _)| { dns_records .domain_ips_iter(domain) - .all(|ip| !unreachable_hosts.is_unreachable(ip)) + .all(|ip| imcp_error_hosts.icmp_error_for_ip(ip).is_none()) }) .map({ let dns_records = dns_records.clone(); diff --git a/rust/connlib/tunnel/src/tests/sut.rs b/rust/connlib/tunnel/src/tests/sut.rs index cced4c19a..73ac03c84 100644 --- a/rust/connlib/tunnel/src/tests/sut.rs +++ b/rust/connlib/tunnel/src/tests/sut.rs @@ -1,5 +1,6 @@ use super::buffered_transmits::BufferedTransmits; use super::dns_records::DnsRecords; +use super::icmp_error_hosts::IcmpErrorHosts; use super::reference::ReferenceState; use super::sim_client::SimClient; use super::sim_gateway::SimGateway; @@ -7,7 +8,6 @@ use super::sim_net::{Host, HostId, RoutingTable}; use super::sim_relay::SimRelay; use super::stub_portal::StubPortal; use super::transition::{Destination, DnsQuery}; -use super::unreachable_hosts::UnreachableHosts; use crate::client::Resource; use crate::dns::is_subdomain; use crate::messages::{IceCredentials, Key, SecretKey}; @@ -388,7 +388,7 @@ impl TunnelTest { // `handle_timeout` needs to be called at the very top to advance state after we have made other modifications. self.handle_timeout( &ref_state.global_dns_records, - &ref_state.unreachable_hosts, + &ref_state.icmp_error_hosts, buffered_transmits, now, ); @@ -538,7 +538,7 @@ impl TunnelTest { fn handle_timeout( &mut self, global_dns_records: &DnsRecords, - unreachable_hosts: &UnreachableHosts, + icmp_error_hosts: &IcmpErrorHosts, buffered_transmits: &mut BufferedTransmits, now: Instant, ) { @@ -580,7 +580,7 @@ impl TunnelTest { while let Some(transmit) = gateway.poll_inbox(now) { let Some(reply) = gateway.exec_mut(|g| { - g.receive(transmit, unreachable_hosts, now, self.flux_capacitor.now()) + g.receive(transmit, icmp_error_hosts, now, self.flux_capacitor.now()) }) else { continue; }; diff --git a/rust/connlib/tunnel/src/tests/tcp.rs b/rust/connlib/tunnel/src/tests/tcp.rs index d3cd3affa..f4624f288 100644 --- a/rust/connlib/tunnel/src/tests/tcp.rs +++ b/rust/connlib/tunnel/src/tests/tcp.rs @@ -75,7 +75,7 @@ impl Client { pub fn handle_inbound(&mut self, packet: IpPacket) { // TODO: Upstream ICMP error handling to `smoltcp`. - if let Ok(Some((failed_packet, _))) = packet.icmp_unreachable_destination() { + if let Ok(Some((failed_packet, _))) = packet.icmp_error() { if let Layer4Protocol::Tcp { dst, .. } = failed_packet.layer4_protocol() { if let Some(handle) = self .sockets_by_remote diff --git a/website/src/components/Changelog/Gateway.tsx b/website/src/components/Changelog/Gateway.tsx index 1d1d2a2dd..75be45c86 100644 --- a/website/src/components/Changelog/Gateway.tsx +++ b/website/src/components/Changelog/Gateway.tsx @@ -30,6 +30,10 @@ export default function Gateway() { Responds with ICMP errors for filtered packets. + + Adds support for translating Time-Exceeded ICMP errors in the DNS + resource NAT, allowing `tracepath` to work through a Firezone tunnel. +