fix(gateway): truncate payload of ICMP errors (#9059)

When the Gateway is handed an IP packet for a DNS resource that it
cannot route, it sends back an ICMP unreachable error. According to RFC
792 [0] (for ICMPv4) and RFC 4443 [1] (for ICMPv6), parts of the
original packet should be included in the ICMP error payload to allow
the sending party to correlate, what could not be sent.

For ICMPv4, the RFC says:

```
Internet Header + 64 bits of Data Datagram

The internet header plus the first 64 bits of the original
datagram's data.  This data is used by the host to match the
message to the appropriate process.  If a higher level protocol
uses port numbers, they are assumed to be in the first 64 data
bits of the original datagram's data.
```

For ICMPv6, the RFC says:

```
As much of invoking packet as possible without the ICMPv6 packet exceeding the minimum IPv6 MTU
```

[0]: https://datatracker.ietf.org/doc/html/rfc792
[1]: https://datatracker.ietf.org/doc/html/rfc4443#section-3.1
This commit is contained in:
Thomas Eizinger
2025-05-09 11:08:31 +09:30
committed by GitHub
parent 18ec6c6860
commit 33d5c32f35
3 changed files with 140 additions and 49 deletions

View File

@@ -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<TranslateOutboundResult> {
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<IpPacket, anyhow::Error> {
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<IpPacket, anyhow::Error> {
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)]

View File

@@ -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<IpPacket> {
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<IpPacket, anyhow::Error> {
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<IpPacket, anyhow::Error> {
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<u8>,
) {
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<u8>,
) {
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<Value = Vec<u8>> {
collection::vec(any::<u8>(), 0..=max_size)
}
}

View File

@@ -22,7 +22,12 @@ export default function Gateway() {
return (
<Entries downloadLinks={downloadLinks} title="Gateway">
<Unreleased></Unreleased>
<Unreleased>
<ChangeItem pull="9059">
Fixes an issue where ICMP unreachable errors for large packets would
not be sent.
</ChangeItem>
</Unreleased>
<Entry version="1.4.8" date={new Date("2025-05-02")}>
<ChangeItem pull="9009">
Fixes an issue where ECN bits got erroneously cleared without updating