From 66455ab0efeb088045447da1e8904dccafee18a9 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sat, 12 Jul 2025 23:09:48 +0200 Subject: [PATCH] feat(gateway): translate TimeExceeded ICMP messages (#9812) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the DNS resource NAT table, we track parts of the layer 4 protocol of the connection in order to map packets back to the correct proxy IP in case multiple DNS names resolve to the same real IP. The involvement of layer 4 means we need to perform some packet inspection in case we receive ICMP errors from an upstream router. Presently, the only ICMP error we handle here is destination unreachable. Those are generated e.g. when we are trying to contact an IPv6 address but we don't have an IPv6 egress interface. An additional error that we want to handle here is "time exceeded": Time exceeded is sent when the TTL of a packet reaches 0. Typically, TTLs are set high enough such that the packet makes it to its destination. When using tools such as `tracepath` however, the TTL is specifically only incremented one-by-one in order to resolve the exact hops a packet is taking to a destination. Without handling the time exceeded ICMP error, using `tracepath` through Firezone is broken because the packets get dropped at the DNS resource NAT. With this PR, we generalise the functionality of detecting destination unreachable ICMP errors to also handle time-exceeded errors, allowing tools such as `tracepath` to somewhat work: ``` ❯ sudo docker compose exec --env RUST_LOG=info -it client /bin/sh -c 'tracepath -b example.com' 1?: [LOCALHOST] pmtu 1280 1: 100.82.110.64 (100.82.110.64) 0.795ms 1: 100.82.110.64 (100.82.110.64) 0.593ms 2: example.com (100.96.0.1) 0.696ms asymm 45 3: example.com (100.96.0.1) 5.788ms asymm 45 4: example.com (100.96.0.1) 7.787ms asymm 45 5: example.com (100.96.0.1) 8.412ms asymm 45 6: example.com (100.96.0.1) 9.545ms asymm 45 7: example.com (100.96.0.1) 7.312ms asymm 45 8: example.com (100.96.0.1) 8.779ms asymm 45 9: example.com (100.96.0.1) 9.455ms asymm 45 10: example.com (100.96.0.1) 14.410ms asymm 45 11: example.com (100.96.0.1) 24.244ms asymm 45 12: example.com (100.96.0.1) 31.286ms asymm 45 13: no reply 14: example.com (100.96.0.1) 303.860ms asymm 45 15: no reply 16: example.com (100.96.0.1) 135.616ms (This broken router returned corrupted payload) asymm 45 17: no reply 18: example.com (100.96.0.1) 161.647ms asymm 45 19: no reply 20: no reply 21: no reply 22: example.com (100.96.0.1) 238.066ms reached Resume: pmtu 1280 hops 22 back 45 ``` We say "somewhat work" because due to the NAT that is in place for DNS resources, the output does not disclose the intermediary hops beyond the Gateway. Co-authored-by: Antoine Labarussias --------- Co-authored-by: Antoine Labarussias --- .github/workflows/_rust.yml | 5 ++- ...icmp_dest_unreachable.rs => icmp_error.rs} | 39 ++++++++++-------- rust/connlib/ip-packet/src/lib.rs | 40 +++++++++---------- rust/connlib/ip-packet/src/make.rs | 10 +---- .../tunnel/proptest-regressions/tests.txt | 6 +++ rust/connlib/tunnel/src/peer.rs | 11 ++--- rust/connlib/tunnel/src/peer/nat_table.rs | 38 +++++++++--------- rust/connlib/tunnel/src/tests.rs | 2 +- rust/connlib/tunnel/src/tests/assertions.rs | 2 +- ...reachable_hosts.rs => icmp_error_hosts.rs} | 22 +++++----- rust/connlib/tunnel/src/tests/reference.rs | 20 +++++----- rust/connlib/tunnel/src/tests/sim_client.rs | 2 +- rust/connlib/tunnel/src/tests/sim_gateway.rs | 36 +++++++++++------ rust/connlib/tunnel/src/tests/strategies.rs | 6 +-- rust/connlib/tunnel/src/tests/sut.rs | 8 ++-- rust/connlib/tunnel/src/tests/tcp.rs | 2 +- website/src/components/Changelog/Gateway.tsx | 4 ++ 17 files changed, 135 insertions(+), 118 deletions(-) rename rust/connlib/ip-packet/src/{icmp_dest_unreachable.rs => icmp_error.rs} (83%) rename rust/connlib/tunnel/src/tests/{unreachable_hosts.rs => icmp_error_hosts.rs} (77%) 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. +