From e490de772914d536f0b77e0caf4bed3e26b5d79c Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Thu, 27 Nov 2025 15:19:54 +1100
Subject: [PATCH] fix(gateway): don't route outbound ICMP errors (#10989)

ICMP errors like "Destination unreachable" can be sent by both ends of a
network connection. For DNS resources, handling these packets requires
special care as we also need to translate the failed packet embedded
within these ICMP messages such that the recipient can correctly relate
them to the network socket that has sent the original packet.

We do this for inbound ICMP errors already to alert the Client of e.g.
unreachable paths such as unreachable IPv6 networks. Outbound ICMP
errors, that is, ICMP errors generated by the Client for a packet sent
by a resource are currently not handled and result in warnings such as:

> Failed to translate outbound packet: Unsupported ICMPv4 type:
DestinationUnreachable(Port)

Whilst it is possible to correctly handle and translate these packets,
doing so requires a fair amount of work and changes to a very critical
part of the Gateway. As such, we simply drop these packets for now as
"unroutable packets" which downgrades their log level to DEBUG.

Resolves: #10983
---
 .../tunnel/src/gateway/client_on_gateway.rs   | 33 +++++++++++++++++++
 .../tunnel/src/gateway/unroutable_packet.rs   | 10 ++++++
 2 files changed, 43 insertions(+)

diff --git a/rust/libs/connlib/tunnel/src/gateway/client_on_gateway.rs b/rust/libs/connlib/tunnel/src/gateway/client_on_gateway.rs
index c178ea9bb..7fa5ca624 100644
--- a/rust/libs/connlib/tunnel/src/gateway/client_on_gateway.rs
+++ b/rust/libs/connlib/tunnel/src/gateway/client_on_gateway.rs
@@ -300,6 +300,10 @@ impl ClientOnGateway {
         packet: IpPacket,
         now: Instant,
     ) -> anyhow::Result<TranslateOutboundResult> {
+        if packet.icmp_error().is_ok_and(|e| e.is_some()) {
+            bail!(UnroutablePacket::outbound_icmp_error(&packet))
+        }
+
         // Filtering a packet is not an error.
         if let Err(e) = self.ensure_allowed_outbound(&packet) {
             tracing::debug!(filtered_packet = ?packet, "{e:#}");
@@ -686,6 +690,7 @@ mod tests {
         time::Duration,
     };
 
+    use anyhow::ErrorExt;
     use ip_packet::make::TcpFlags;
 
     use crate::{
@@ -1210,6 +1215,34 @@ mod tests {
         assert!(peer.permanent_translations.contains_key(&proxy_ip6_2()));
     }
 
+    #[test]
+    fn no_translate_outbound_icmp_error() {
+        let _guard = logging::test("trace");
+
+        let now = Instant::now();
+
+        let mut peer = ClientOnGateway::new(
+            client_id(),
+            client_tun(),
+            gateway_tun(),
+            flow_tracker::ClientProperties::default(),
+        );
+
+        let icmp_unreachable = ip_packet::make::icmp_dest_unreachable_network(
+            &ip_packet::make::udp_packet(proxy_ip4_1(), client_tun_ipv4(), 443, 50000, vec![])
+                .unwrap(),
+        )
+        .unwrap();
+
+        let error = peer.translate_outbound(icmp_unreachable, now).unwrap_err();
+        let error = error.any_downcast_ref::<UnroutablePacket>().unwrap();
+
+        assert_eq!(error.to_string(), "Unroutable packet: OutboundIcmpError");
+        assert_eq!(error.source().to_string(), "unknown");
+        assert_eq!(error.destination().to_string(), "unknown");
+        assert_eq!(error.proto().to_string(), "unknown");
+    }
+
     fn foo_dns_resource() -> crate::messages::gateway::ResourceDescription {
         crate::messages::gateway::ResourceDescription::Dns(
             crate::messages::gateway::ResourceDescriptionDns {
diff --git a/rust/libs/connlib/tunnel/src/gateway/unroutable_packet.rs b/rust/libs/connlib/tunnel/src/gateway/unroutable_packet.rs
index 11a6bda6e..7ca0012a5 100644
--- a/rust/libs/connlib/tunnel/src/gateway/unroutable_packet.rs
+++ b/rust/libs/connlib/tunnel/src/gateway/unroutable_packet.rs
@@ -48,6 +48,13 @@ impl UnroutablePacket {
         }
     }
 
+    pub fn outbound_icmp_error(packet: &IpPacket) -> Self {
+        Self {
+            five_tuple: FiveTuple::for_packet(packet),
+            error: RoutingError::OutboundIcmpError,
+        }
+    }
+
     pub fn reason(&self) -> RoutingError {
         self.error
     }
@@ -138,6 +145,8 @@ pub enum RoutingError {
     NoPeerState,
     #[display("No connection")]
     NotConnected,
+    #[display("OutboundIcmpError")]
+    OutboundIcmpError,
     #[display("Other")]
     Other,
 }
@@ -150,6 +159,7 @@ impl From<RoutingError> for opentelemetry::Value {
             RoutingError::NotAPeer => opentelemetry::Value::from("NotAPeer"),
             RoutingError::NoPeerState => opentelemetry::Value::from("NoPeerState"),
             RoutingError::NotConnected => opentelemetry::Value::from("NotConnected"),
+            RoutingError::OutboundIcmpError => opentelemetry::Value::from("OutboundIcmpError"),
             RoutingError::Other => opentelemetry::Value::from("Other"),
         }
     }