diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index 7ef595314..d13f82080 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -1,6 +1,6 @@ use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet, VecDeque, hash_map}; use std::iter; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; +use std::net::{IpAddr, SocketAddr}; use std::time::Instant; use crate::client::{IPV4_RESOURCES, IPV6_RESOURCES}; @@ -12,7 +12,7 @@ use dns_types::DomainName; use filter_engine::FilterEngine; use ip_network::{IpNetwork, Ipv4Network, Ipv6Network}; use ip_network_table::IpNetworkTable; -use ip_packet::{IpPacket, PacketBuilder, Protocol, UnsupportedProtocol, icmpv4, icmpv6}; +use ip_packet::{IpPacket, Protocol, UnsupportedProtocol}; use crate::utils::network_contains_network; use crate::{GatewayEvent, IpConfig, otel}; @@ -294,13 +294,25 @@ impl ClientOnGateway { } let Some(state) = self.permanent_translations.get_mut(&packet.destination()) else { - return self.dst_unreachable(&packet); + return Ok(TranslateOutboundResult::DestinationUnreachable( + ip_packet::make::icmp_dst_unreachable( + self.gateway_tun.v4, + self.gateway_tun.v6, + &packet, + )?, + )); }; #[expect(clippy::collapsible_if, reason = "We want the feature flag separate.")] if firezone_telemetry::feature_flags::icmp_unreachable_instead_of_nat64() { if state.resolved_ip.is_ipv4() != dst.is_ipv4() { - return self.dst_unreachable(&packet); + return Ok(TranslateOutboundResult::DestinationUnreachable( + ip_packet::make::icmp_dst_unreachable( + self.gateway_tun.v4, + self.gateway_tun.v6, + &packet, + )?, + )); } } @@ -473,47 +485,6 @@ impl ClientOnGateway { pub fn id(&self) -> ClientId { self.id } - - fn dst_unreachable(&self, packet: &IpPacket) -> Result { - let src = packet.source(); - - let icmp_error = match src { - IpAddr::V4(src) => icmpv4_network_unreachable(self.gateway_tun.v4, src, packet)?, - IpAddr::V6(src) => icmpv6_address_unreachable(self.gateway_tun.v6, src, packet)?, - }; - - Ok(TranslateOutboundResult::DestinationUnreachable(icmp_error)) - } -} - -fn icmpv4_network_unreachable( - src: Ipv4Addr, - dst: Ipv4Addr, - original_packet: &IpPacket, -) -> Result { - let builder = PacketBuilder::ipv4(src.octets(), dst.octets(), 20).icmpv4( - ip_packet::Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Network), - ); - let payload = original_packet.packet(); - - let ip_packet = ip_packet::build!(builder, payload)?; - - Ok(ip_packet) -} - -fn icmpv6_address_unreachable( - src: Ipv6Addr, - dst: Ipv6Addr, - original_packet: &IpPacket, -) -> Result { - let builder = PacketBuilder::ipv6(src.octets(), dst.octets(), 20).icmpv6( - ip_packet::Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::Address), - ); - let payload = original_packet.packet(); - - let ip_packet = ip_packet::build!(builder, payload)?; - - Ok(ip_packet) } #[derive(Debug, PartialEq)] diff --git a/rust/ip-packet/src/make.rs b/rust/ip-packet/src/make.rs index 57f61f190..54af34ccf 100644 --- a/rust/ip-packet/src/make.rs +++ b/rust/ip-packet/src/make.rs @@ -1,9 +1,9 @@ //! Factory module for making all kinds of packets. -use crate::{IpPacket, IpPacketBuf}; +use crate::{IpPacket, IpPacketBuf, MAX_IP_SIZE}; use anyhow::{Context as _, Result, bail}; -use etherparse::PacketBuilder; -use std::net::IpAddr; +use etherparse::{Icmpv6Header, Ipv6Header, PacketBuilder, icmpv4, icmpv6}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; /// Helper macro to turn a [`PacketBuilder`] into an [`IpPacket`]. #[macro_export] @@ -156,6 +156,121 @@ where } } +pub fn icmp_dst_unreachable( + ipv4_src: Ipv4Addr, + ipv6_src: Ipv6Addr, + original_packet: &IpPacket, +) -> Result { + let src = original_packet.source(); + + let icmp_error = match src { + IpAddr::V4(src) => icmpv4_network_unreachable(ipv4_src, src, original_packet)?, + IpAddr::V6(src) => icmpv6_address_unreachable(ipv6_src, src, original_packet)?, + }; + + Ok(icmp_error) +} + +fn icmpv4_network_unreachable( + src: Ipv4Addr, + dst: Ipv4Addr, + original_packet: &IpPacket, +) -> Result { + let builder = PacketBuilder::ipv4(src.octets(), dst.octets(), 20).icmpv4( + crate::Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Network), + ); + let payload = original_packet.packet(); + + let header_len = original_packet + .ipv4_header() + .context("Not an IPv4 packet")? + .header_len(); + let icmp_error_payload_len = header_len + 8; + + let actual_payload_len = std::cmp::min(payload.len(), icmp_error_payload_len); + let error_payload = &payload[..actual_payload_len]; + + let ip_packet = crate::build!(builder, error_payload)?; + + Ok(ip_packet) +} + +fn icmpv6_address_unreachable( + src: Ipv6Addr, + dst: Ipv6Addr, + original_packet: &IpPacket, +) -> 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 payload = original_packet.packet(); + + let actual_payload_len = std::cmp::min(payload.len(), MAX_ICMP_ERROR_PAYLOAD_LEN); + let error_payload = &payload[..actual_payload_len]; + + let ip_packet = crate::build!(builder, error_payload)?; + + Ok(ip_packet) +} + #[derive(thiserror::Error, Debug)] #[error("IPs must be of the same version")] pub struct IpVersionMismatch; + +#[cfg(all(test, feature = "proptest"))] +mod tests { + use etherparse::{Ipv4Header, Ipv6Header, UdpHeader}; + use proptest::{ + collection, + prelude::{Strategy, any}, + }; + + use crate::MAX_IP_SIZE; + + use super::*; + + #[test_strategy::proptest()] + fn ipv4_icmp_unreachable( + #[strategy(payload(MAX_IP_SIZE - Ipv4Header::MIN_LEN - UdpHeader::LEN))] payload: Vec, + ) { + let unreachable_packet = + udp_packet(Ipv4Addr::LOCALHOST, Ipv4Addr::LOCALHOST, 0, 0, payload).unwrap(); + + let icmp_error = + icmp_dst_unreachable(ERROR_SRC_IPV4, ERROR_SRC_IPV6, &unreachable_packet).unwrap(); + + assert_eq!(icmp_error.destination(), IpAddr::V4(Ipv4Addr::LOCALHOST)); + assert_eq!(icmp_error.source(), IpAddr::V4(ERROR_SRC_IPV4)); + assert!(matches!( + icmp_error.icmp_unreachable_destination(), + Ok(Some(_)) + )); + } + + #[test_strategy::proptest()] + fn ipv6_icmp_unreachable_max_payload( + #[strategy(payload(MAX_IP_SIZE - Ipv6Header::LEN - UdpHeader::LEN))] payload: Vec, + ) { + let unreachable_packet = + udp_packet(Ipv6Addr::LOCALHOST, Ipv6Addr::LOCALHOST, 0, 0, payload).unwrap(); + + let icmp_error = + icmp_dst_unreachable(ERROR_SRC_IPV4, ERROR_SRC_IPV6, &unreachable_packet).unwrap(); + + assert_eq!(icmp_error.destination(), IpAddr::V6(Ipv6Addr::LOCALHOST)); + assert_eq!(icmp_error.source(), IpAddr::V6(ERROR_SRC_IPV6)); + assert!(matches!( + icmp_error.icmp_unreachable_destination(), + Ok(Some(_)) + )); + } + + const ERROR_SRC_IPV4: Ipv4Addr = Ipv4Addr::new(1, 1, 1, 1); + const ERROR_SRC_IPV6: Ipv6Addr = Ipv6Addr::new(1, 1, 1, 1, 1, 1, 1, 1); + + fn payload(max_size: usize) -> impl Strategy> { + collection::vec(any::(), 0..=max_size) + } +} diff --git a/website/src/components/Changelog/Gateway.tsx b/website/src/components/Changelog/Gateway.tsx index 117ceacf6..26763aa6e 100644 --- a/website/src/components/Changelog/Gateway.tsx +++ b/website/src/components/Changelog/Gateway.tsx @@ -22,7 +22,12 @@ export default function Gateway() { return ( - + + + Fixes an issue where ICMP unreachable errors for large packets would + not be sent. + + Fixes an issue where ECN bits got erroneously cleared without updating