diff --git a/rust/connlib/ip-packet/src/make.rs b/rust/connlib/ip-packet/src/make.rs index b639d45dc..659634dc1 100644 --- a/rust/connlib/ip-packet/src/make.rs +++ b/rust/connlib/ip-packet/src/make.rs @@ -172,16 +172,20 @@ where } } -pub fn icmp_dst_unreachable(original_packet: &IpPacket) -> Result { +pub fn icmp_dest_unreachable( + original_packet: &IpPacket, + icmpv4: icmpv4::DestUnreachableHeader, + icmpv6: icmpv6::DestUnreachableCode, +) -> Result { let src = original_packet.source(); let dst = original_packet.destination(); let icmp_error = match (src, dst) { (IpAddr::V4(src), IpAddr::V4(dst)) => { - icmpv4_network_unreachable(dst, src, original_packet)? + icmpv4_unreachable(dst, src, original_packet, icmpv4)? } (IpAddr::V6(src), IpAddr::V6(dst)) => { - icmpv6_address_unreachable(dst, src, original_packet)? + icmpv6_unreachable(dst, src, original_packet, icmpv6)? } (IpAddr::V4(_), IpAddr::V6(_)) => { bail!("Invalid IP packet: Inconsistent IP address versions") @@ -194,14 +198,14 @@ pub fn icmp_dst_unreachable(original_packet: &IpPacket) -> Result { Ok(icmp_error) } -fn icmpv4_network_unreachable( +fn icmpv4_unreachable( src: Ipv4Addr, dst: Ipv4Addr, original_packet: &IpPacket, + code: icmpv4::DestUnreachableHeader, ) -> Result { - let builder = PacketBuilder::ipv4(src.octets(), dst.octets(), 20).icmpv4( - crate::Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Network), - ); + let builder = PacketBuilder::ipv4(src.octets(), dst.octets(), 20) + .icmpv4(crate::Icmpv4Type::DestinationUnreachable(code)); let payload = original_packet.packet(); let header_len = original_packet @@ -218,16 +222,16 @@ fn icmpv4_network_unreachable( Ok(ip_packet) } -fn icmpv6_address_unreachable( +fn icmpv6_unreachable( src: Ipv6Addr, dst: Ipv6Addr, original_packet: &IpPacket, + code: icmpv6::DestUnreachableCode, ) -> Result { const MAX_ICMP_ERROR_PAYLOAD_LEN: usize = MAX_IP_SIZE - Ipv6Header::LEN - Icmpv6Header::MAX_LEN; - let builder = PacketBuilder::ipv6(src.octets(), dst.octets(), 20).icmpv6( - crate::Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::Address), - ); + let builder = PacketBuilder::ipv6(src.octets(), dst.octets(), 20) + .icmpv6(crate::Icmpv6Type::DestinationUnreachable(code)); let payload = original_packet.packet(); let actual_payload_len = std::cmp::min(payload.len(), MAX_ICMP_ERROR_PAYLOAD_LEN); @@ -267,7 +271,12 @@ mod tests { ) .unwrap(); - let icmp_error = icmp_dst_unreachable(&unreachable_packet).unwrap(); + let icmp_error = icmp_dest_unreachable( + &unreachable_packet, + icmpv4::DestUnreachableHeader::Network, + icmpv6::DestUnreachableCode::Address, + ) + .unwrap(); assert_eq!( icmp_error.destination(), @@ -293,7 +302,12 @@ mod tests { ) .unwrap(); - let icmp_error = icmp_dst_unreachable(&unreachable_packet).unwrap(); + let icmp_error = icmp_dest_unreachable( + &unreachable_packet, + icmpv4::DestUnreachableHeader::Network, + icmpv6::DestUnreachableCode::Address, + ) + .unwrap(); assert_eq!( icmp_error.destination(), diff --git a/rust/connlib/tunnel/src/gateway.rs b/rust/connlib/tunnel/src/gateway.rs index 776c3478e..575595239 100644 --- a/rust/connlib/tunnel/src/gateway.rs +++ b/rust/connlib/tunnel/src/gateway.rs @@ -163,7 +163,8 @@ impl GatewayState { .context("Failed to translate outbound packet")? { TranslateOutboundResult::Send(ip_packet) => Ok(Some(ip_packet)), - TranslateOutboundResult::DestinationUnreachable(reply) => { + TranslateOutboundResult::DestinationUnreachable(reply) + | TranslateOutboundResult::Filtered(reply) => { let Some(transmit) = encrypt_packet(reply, cid, &mut self.node, now)? else { return Ok(None); }; @@ -172,7 +173,6 @@ impl GatewayState { Ok(None) } - TranslateOutboundResult::Filtered => Ok(None), } } diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index ed47b9a3d..1117c88e0 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -276,47 +276,6 @@ impl ClientOnGateway { } } - fn transform_network_to_tun( - &mut self, - packet: IpPacket, - now: Instant, - ) -> anyhow::Result { - let dst = packet.destination(); - - // Packets to the TUN interface don't get transformed. - if self.gateway_tun.is_ip(dst) { - return Ok(TranslateOutboundResult::Send(packet)); - } - - // Packets for CIDR resources / Internet resource are forwarded as is. - if !is_dns_addr(dst) { - return Ok(TranslateOutboundResult::Send(packet)); - } - - let Some(state) = self.permanent_translations.get_mut(&packet.destination()) else { - return Ok(TranslateOutboundResult::DestinationUnreachable( - ip_packet::make::icmp_dst_unreachable(&packet)?, - )); - }; - - if state.resolved_ip.is_ipv4() != dst.is_ipv4() { - return Ok(TranslateOutboundResult::DestinationUnreachable( - ip_packet::make::icmp_dst_unreachable(&packet)?, - )); - } - - let (source_protocol, real_ip) = - self.nat_table - .translate_outgoing(&packet, state.resolved_ip, now)?; - - let mut packet = packet - .translate_destination(source_protocol, real_ip) - .context("Failed to translate packet to new destination")?; - packet.update_checksum(); - - Ok(TranslateOutboundResult::Send(packet)) - } - pub fn translate_outbound( &mut self, packet: IpPacket, @@ -325,7 +284,13 @@ impl ClientOnGateway { // Filtering a packet is not an error. if let Err(e) = self.ensure_allowed_src_and_dst(&packet) { tracing::debug!(filtered_packet = ?packet, "{e:#}"); - return Ok(TranslateOutboundResult::Filtered); + return Ok(TranslateOutboundResult::Filtered( + ip_packet::make::icmp_dest_unreachable( + &packet, + ip_packet::icmpv4::DestUnreachableHeader::FilterProhibited, + ip_packet::icmpv6::DestUnreachableCode::Prohibited, + )?, + )); } // Failing to transform is an error we want to know about further up. @@ -380,6 +345,55 @@ impl ClientOnGateway { Ok(Some(packet)) } + fn transform_network_to_tun( + &mut self, + packet: IpPacket, + now: Instant, + ) -> anyhow::Result { + let dst = packet.destination(); + + // Packets to the TUN interface don't get transformed. + if self.gateway_tun.is_ip(dst) { + return Ok(TranslateOutboundResult::Send(packet)); + } + + // Packets for CIDR resources / Internet resource are forwarded as is. + if !is_dns_addr(dst) { + return Ok(TranslateOutboundResult::Send(packet)); + } + + let Some(state) = self.permanent_translations.get_mut(&packet.destination()) else { + return Ok(TranslateOutboundResult::DestinationUnreachable( + ip_packet::make::icmp_dest_unreachable( + &packet, + ip_packet::icmpv4::DestUnreachableHeader::Network, + ip_packet::icmpv6::DestUnreachableCode::Address, + )?, + )); + }; + + if state.resolved_ip.is_ipv4() != dst.is_ipv4() { + return Ok(TranslateOutboundResult::DestinationUnreachable( + ip_packet::make::icmp_dest_unreachable( + &packet, + ip_packet::icmpv4::DestUnreachableHeader::Network, + ip_packet::icmpv6::DestUnreachableCode::Address, + )?, + )); + } + + let (source_protocol, real_ip) = + self.nat_table + .translate_outgoing(&packet, state.resolved_ip, now)?; + + let mut packet = packet + .translate_destination(source_protocol, real_ip) + .context("Failed to translate packet to new destination")?; + packet.update_checksum(); + + Ok(TranslateOutboundResult::Send(packet)) + } + fn transform_tun_to_network( &mut self, packet: IpPacket, @@ -483,7 +497,7 @@ impl ClientOnGateway { pub enum TranslateOutboundResult { Send(IpPacket), DestinationUnreachable(IpPacket), - Filtered, + Filtered(IpPacket), } impl GatewayOnClient { @@ -874,7 +888,7 @@ mod tests { assert!(matches!( peer.translate_outbound(pkt, Instant::now()).unwrap(), - crate::peer::TranslateOutboundResult::Filtered + crate::peer::TranslateOutboundResult::Filtered(_) )); let pkt = ip_packet::make::udp_packet( @@ -888,7 +902,7 @@ mod tests { assert!(matches!( peer.translate_outbound(pkt, Instant::now()).unwrap(), - crate::peer::TranslateOutboundResult::Filtered + crate::peer::TranslateOutboundResult::Filtered(_) )); let pkt = ip_packet::make::udp_packet( @@ -938,7 +952,7 @@ mod tests { assert!(matches!( peer.translate_outbound(pkt, Instant::now()).unwrap(), - crate::peer::TranslateOutboundResult::Filtered + crate::peer::TranslateOutboundResult::Filtered(_) )); let pkt = ip_packet::make::udp_packet( diff --git a/rust/connlib/tunnel/src/tests/reference.rs b/rust/connlib/tunnel/src/tests/reference.rs index fbc80ba1c..4dfb44e3d 100644 --- a/rust/connlib/tunnel/src/tests/reference.rs +++ b/rust/connlib/tunnel/src/tests/reference.rs @@ -394,14 +394,13 @@ impl ReferenceState { } } Transition::SendIcmpPacket { - src, dst, seq, identifier, payload, + .. } => state.client.exec_mut(|client| { client.on_icmp_packet( - *src, dst.clone(), *seq, *identifier, @@ -411,15 +410,14 @@ impl ReferenceState { ) }), Transition::SendUdpPacket { - src, dst, sport, dport, payload, + .. } => { state.client.exec_mut(|client| { client.on_udp_packet( - *src, dst.clone(), *sport, *dport, @@ -430,15 +428,14 @@ impl ReferenceState { }); } Transition::SendTcpPayload { - src, dst, sport, dport, payload, + .. } => { state.client.exec_mut(|client| { client.on_tcp_packet( - *src, dst.clone(), *sport, *dport, diff --git a/rust/connlib/tunnel/src/tests/sim_client.rs b/rust/connlib/tunnel/src/tests/sim_client.rs index da865bdd2..97f91a429 100644 --- a/rust/connlib/tunnel/src/tests/sim_client.rs +++ b/rust/connlib/tunnel/src/tests/sim_client.rs @@ -597,13 +597,6 @@ impl RefClient { .collect() } - pub(crate) fn is_tunnel_ip(&self, ip: IpAddr) -> bool { - match ip { - IpAddr::V4(ip4) => self.tunnel_ip4 == ip4, - IpAddr::V6(ip6) => self.tunnel_ip6 == ip6, - } - } - pub(crate) fn tunnel_ip_for(&self, dst: IpAddr) -> IpAddr { match dst { IpAddr::V4(_) => self.tunnel_ip4.into(), @@ -611,10 +604,8 @@ impl RefClient { } } - #[expect(clippy::too_many_arguments, reason = "We don't care.")] pub(crate) fn on_icmp_packet( &mut self, - src: IpAddr, dst: Destination, seq: Seq, identifier: Identifier, @@ -623,7 +614,6 @@ impl RefClient { gateway_by_ip: impl Fn(IpAddr) -> Option, ) { self.on_packet( - src, dst.clone(), (dst, seq, identifier), |ref_client| &mut ref_client.expected_icmp_handshakes, @@ -633,10 +623,8 @@ impl RefClient { ); } - #[expect(clippy::too_many_arguments, reason = "We don't care.")] pub(crate) fn on_udp_packet( &mut self, - src: IpAddr, dst: Destination, sport: SPort, dport: DPort, @@ -645,7 +633,6 @@ impl RefClient { gateway_by_ip: impl Fn(IpAddr) -> Option, ) { self.on_packet( - src, dst.clone(), (dst, sport, dport), |ref_client| &mut ref_client.expected_udp_handshakes, @@ -655,10 +642,8 @@ impl RefClient { ); } - #[expect(clippy::too_many_arguments, reason = "We don't care.")] pub(crate) fn on_tcp_packet( &mut self, - src: IpAddr, dst: Destination, sport: SPort, dport: DPort, @@ -667,7 +652,6 @@ impl RefClient { gateway_by_ip: impl Fn(IpAddr) -> Option, ) { self.on_packet( - src, dst.clone(), (dst, sport, dport), |ref_client| &mut ref_client.expected_tcp_exchanges, @@ -677,11 +661,9 @@ impl RefClient { ); } - #[expect(clippy::too_many_arguments, reason = "We don't care.")] #[tracing::instrument(level = "debug", skip_all, fields(dst, resource, gateway))] fn on_packet( &mut self, - src: IpAddr, dst: Destination, packet_id: E, map: impl FnOnce(&mut Self) -> &mut BTreeMap>, @@ -718,10 +700,6 @@ impl RefClient { gateway }; - if !self.is_tunnel_ip(src) { - return; - } - tracing::debug!(%payload, "Sending packet"); map(self) diff --git a/website/src/components/Changelog/Gateway.tsx b/website/src/components/Changelog/Gateway.tsx index 87a64e8a3..1d1d2a2dd 100644 --- a/website/src/components/Changelog/Gateway.tsx +++ b/website/src/components/Changelog/Gateway.tsx @@ -27,6 +27,9 @@ export default function Gateway() { Excludes ICMP errors from the ICMP traffic filter. Those are now always routed back to the client. + + Responds with ICMP errors for filtered packets. +