From 956bbbfd9173f03a1e6d392a3d62360b086043ff Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Sun, 22 Dec 2024 13:09:14 +0100 Subject: [PATCH] fix(gateway): translate ICMPv6's `PacketTooBig` error (#7567) IPv6 treats fragmentation and MTU errors differently than IPv4. Rather than requiring fragmentation on each hop of a routing path, fragmentation needs to happen at the packet source and failure to route a packet triggers an ICMPv6 `PacketTooBig` error. These need to be translated back through our NAT64 implementation of the Gateway. Due to the size difference in the headers of IPv4 and IPv6, the available MTU to the IPv4 packet is 20 bytes _less_ than the MTU reported by the ICMP error. IPv6 headers are always 40 bytes, meaning if the MTU is reported as e.g. 1200 on the IPv6 side, we need to only offer 1180 to the IPv4 end of the application. Once the new MTU is then honored, the packets translated by our NAT64 implementation will still conform to the required MTU of 1200, despite the overhead introduced by the translation. Resolves: #7515. --- .../tunnel/proptest-regressions/tests.txt | 1 + rust/connlib/tunnel/src/tests/sim_gateway.rs | 22 ++++++++++--- .../tunnel/src/tests/unreachable_hosts.rs | 2 ++ rust/ip-packet/src/icmp_dest_unreachable.rs | 11 +++++-- rust/ip-packet/src/lib.rs | 14 ++++++-- rust/ip-packet/src/nat46.rs | 6 ++-- rust/ip-packet/src/nat64.rs | 33 ++++++++++++------- website/src/components/Changelog/Gateway.tsx | 7 +++- 8 files changed, 69 insertions(+), 27 deletions(-) diff --git a/rust/connlib/tunnel/proptest-regressions/tests.txt b/rust/connlib/tunnel/proptest-regressions/tests.txt index 9a8d29360..a3a3cf758 100644 --- a/rust/connlib/tunnel/proptest-regressions/tests.txt +++ b/rust/connlib/tunnel/proptest-regressions/tests.txt @@ -138,3 +138,4 @@ cc e60fe97280614a96052e1af1c8d3e4661ec2fead4d22106edf6fb5caf330b6a5 cc c50c783f60cd7126453f38d2ae37bea3120ebb588c68f99ad5ad4a659f331338 cc 606dacf44d60870087c7c65fb404f090c52762167e01e534dee1df0557d70304 cc e6fcc44d59815c5d62b58f9f12ceb4ea67be9de8421bae651c3d5ef8cecea8aa +cc eaa345674d5ae8799ae3c739184177b52e5912e6ff2b0925e07fd95f4e992c2a diff --git a/rust/connlib/tunnel/src/tests/sim_gateway.rs b/rust/connlib/tunnel/src/tests/sim_gateway.rs index 5e7d9751d..14e78625f 100644 --- a/rust/connlib/tunnel/src/tests/sim_gateway.rs +++ b/rust/connlib/tunnel/src/tests/sim_gateway.rs @@ -292,6 +292,11 @@ fn icmp_error_reply(packet: &IpPacket, error: IcmpError) -> Result { IcmpError::Network => icmpv4::DestUnreachableHeader::Network, IcmpError::Host => icmpv4::DestUnreachableHeader::Host, IcmpError::Port => icmpv4::DestUnreachableHeader::Port, + IcmpError::PacketTooBig { mtu } => { + icmpv4::DestUnreachableHeader::FragmentationNeeded { + next_hop_mtu: u16::try_from(mtu).unwrap_or(u16::MAX), + } + } }), ); @@ -299,11 +304,18 @@ fn icmp_error_reply(packet: &IpPacket, error: IcmpError) -> Result { } (IpAddr::V6(src), IpAddr::V6(dst)) => { let icmpv6 = ip_packet::PacketBuilder::ipv6(src.octets(), dst.octets(), 20).icmpv6( - Icmpv6Type::DestinationUnreachable(match error { - IcmpError::Network => icmpv6::DestUnreachableCode::NoRoute, - IcmpError::Host => icmpv6::DestUnreachableCode::NoRoute, - IcmpError::Port => icmpv6::DestUnreachableCode::Port, - }), + match error { + IcmpError::Network => { + Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::NoRoute) + } + IcmpError::Host => { + Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::NoRoute) + } + IcmpError::Port => { + Icmpv6Type::DestinationUnreachable(icmpv6::DestUnreachableCode::Port) + } + IcmpError::PacketTooBig { mtu } => Icmpv6Type::PacketTooBig { mtu }, + }, ); ip_packet::build!(icmpv6, payload) diff --git a/rust/connlib/tunnel/src/tests/unreachable_hosts.rs b/rust/connlib/tunnel/src/tests/unreachable_hosts.rs index b3fb8794d..2fb8ce3c9 100644 --- a/rust/connlib/tunnel/src/tests/unreachable_hosts.rs +++ b/rust/connlib/tunnel/src/tests/unreachable_hosts.rs @@ -47,6 +47,7 @@ fn icmp_error() -> impl Strategy { Just(IcmpError::Network), Just(IcmpError::Host), Just(IcmpError::Port), + any::().prop_map(|mtu| IcmpError::PacketTooBig { mtu }) ] } @@ -56,4 +57,5 @@ pub(crate) enum IcmpError { Network, Host, Port, + PacketTooBig { mtu: u32 }, } diff --git a/rust/ip-packet/src/icmp_dest_unreachable.rs b/rust/ip-packet/src/icmp_dest_unreachable.rs index b7080a14f..c60835af3 100644 --- a/rust/ip-packet/src/icmp_dest_unreachable.rs +++ b/rust/ip-packet/src/icmp_dest_unreachable.rs @@ -11,15 +11,19 @@ pub enum DestUnreachable { header: icmpv4::DestUnreachableHeader, total_length: u16, }, - V6(icmpv6::DestUnreachableCode), + V6Unreachable(icmpv6::DestUnreachableCode), + V6PacketTooBig { + mtu: u32, + }, } impl DestUnreachable { pub fn into_icmp_v4_type(self) -> Result { let header = match self { DestUnreachable::V4 { header, .. } => header, - DestUnreachable::V6(code) => nat64::translate_dest_unreachable(code) + DestUnreachable::V6Unreachable(code) => nat64::translate_dest_unreachable(code) .with_context(|| format!("Cannot translate {code:?} to ICMPv4"))?, + DestUnreachable::V6PacketTooBig { mtu } => nat64::translate_packet_too_big(mtu), }; Ok(Icmpv4Type::DestinationUnreachable(header)) @@ -37,7 +41,8 @@ impl DestUnreachable { Ok(icmpv6_type) } - DestUnreachable::V6(code) => Ok(Icmpv6Type::DestinationUnreachable(code)), + DestUnreachable::V6Unreachable(code) => Ok(Icmpv6Type::DestinationUnreachable(code)), + DestUnreachable::V6PacketTooBig { mtu } => Ok(Icmpv6Type::PacketTooBig { mtu }), } } } diff --git a/rust/ip-packet/src/lib.rs b/rust/ip-packet/src/lib.rs index ca80935ee..674818a85 100644 --- a/rust/ip-packet/src/lib.rs +++ b/rust/ip-packet/src/lib.rs @@ -677,8 +677,16 @@ impl IpPacket { return Ok(None); } - let Icmpv6Type::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 two 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 (ipv6, _) = LaxIpv6Slice::from_slice(icmpv6.payload()) @@ -697,7 +705,7 @@ impl IpPacket { l4_proto, raw: icmpv6.payload().to_vec(), }, - DestUnreachable::V6(error), + dest_unreachable, ))); } diff --git a/rust/ip-packet/src/nat46.rs b/rust/ip-packet/src/nat46.rs index b0fbff955..5a407e39e 100644 --- a/rust/ip-packet/src/nat46.rs +++ b/rust/ip-packet/src/nat46.rs @@ -260,9 +260,9 @@ pub fn translate_icmp_unreachable( Icmpv6Type::PacketTooBig { mtu: mtu as u32 } } - FragmentationNeeded { .. } => { - return None; // FIXME: We don't know our IPv4 / IPv6 MTU here so cannot currently implement this. - } + FragmentationNeeded { next_hop_mtu } => Icmpv6Type::PacketTooBig { + mtu: next_hop_mtu as u32 + 20, + }, // Code 5 (Source Route Failed): Set the Code to 0 (No route // to destination). Note that this error is unlikely since diff --git a/rust/ip-packet/src/nat64.rs b/rust/ip-packet/src/nat64.rs index f350e5b0e..c9f483564 100644 --- a/rust/ip-packet/src/nat64.rs +++ b/rust/ip-packet/src/nat64.rs @@ -218,18 +218,8 @@ fn translate_icmpv6_header( Icmpv6Type::DestinationUnreachable(i) => { Icmpv4Type::DestinationUnreachable(translate_dest_unreachable(i)?) } - // Packet Too Big (Type 2): Translate to an ICMPv4 Destination - // Unreachable (Type 3) with Code 4, and adjust the ICMPv4 - // checksum both to take the type change into account and to - // exclude the ICMPv6 pseudo-header. The MTU field MUST be - // adjusted for the difference between the IPv4 and IPv6 header - // sizes, taking into account whether or not the packet in error - // includes a Fragment Header, i.e., minimum(advertised MTU-20, - // MTU_of_IPv4_nexthop, (MTU_of_IPv6_nexthop)-20). - // - // See also the requirements in Section 6. - Icmpv6Type::PacketTooBig { .. } => { - return None; // FIXME + Icmpv6Type::PacketTooBig { mtu } => { + Icmpv4Type::DestinationUnreachable(translate_packet_too_big(mtu)) } // Time Exceeded (Type 3): Set the Type to 11, and adjust the ICMPv4 // checksum both to take the type change into account and to @@ -287,6 +277,25 @@ fn translate_icmpv6_header( Some(Icmpv4Header::new(icmpv4_type)) } +pub fn translate_packet_too_big(mtu: u32) -> etherparse::icmpv4::DestUnreachableHeader { + // Packet Too Big (Type 2): Translate to an ICMPv4 Destination + // Unreachable (Type 3) with Code 4, and adjust the ICMPv4 + // checksum both to take the type change into account and to + // exclude the ICMPv6 pseudo-header. The MTU field MUST be + // adjusted for the difference between the IPv4 and IPv6 header + // sizes, taking into account whether or not the packet in error + // includes a Fragment Header, i.e., minimum(advertised MTU-20, + // MTU_of_IPv4_nexthop, (MTU_of_IPv6_nexthop)-20). + // + // See also the requirements in Section 6. + + let mtu = u16::try_from(mtu).unwrap_or(u16::MAX); // Unlikely but necessary fallback. + + etherparse::icmpv4::DestUnreachableHeader::FragmentationNeeded { + next_hop_mtu: mtu - 20, // We don't know the next-hop MTUs here so we just subtract 20 bytes. + } +} + pub fn translate_dest_unreachable( code: etherparse::icmpv6::DestUnreachableCode, ) -> Option { diff --git a/website/src/components/Changelog/Gateway.tsx b/website/src/components/Changelog/Gateway.tsx index 1a7c1e11a..500c681c3 100644 --- a/website/src/components/Changelog/Gateway.tsx +++ b/website/src/components/Changelog/Gateway.tsx @@ -10,7 +10,12 @@ export default function Gateway() { return ( - + + + Fixes an issue where ICMPv6's `PacketTooBig' errors were not correctly + translated by the NAT64 module. + + Adds support for GSO (Generic Segmentation Offload), delivering