From a086e9a7c928e56c03136c0aba0094ae25913efa Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 00:48:45 -0600 Subject: [PATCH 01/22] fix(nat): Keep an ICMP error from deleting the Query session it reports on RFC 5508 REQ-6, found by tracking the specification the ICMP handler was already arguing from in prose. An ICMP error is trivially spoofable and masquerade releases the public address and port along with the flow, so honouring every Destination Unreachable handed an off-path attacker both a way to break a ping it cannot observe and a way to churn the port pool. The interlock's remaining survivor on this citation is the IPv6 arm, which no test reaches: masquerade carries a v6 pool and the nat suite has no IPv6 case at all. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- .duvet/config.toml | 3 + .../rfc/rfc5508/section-10.toml | 12 + .../rfc/rfc5508/section-3.1.toml | 59 + .../rfc/rfc5508/section-3.2.toml | 30 + .../rfc/rfc5508/section-4.1.toml | 85 + .../rfc/rfc5508/section-4.2.1.toml | 45 + .../rfc/rfc5508/section-4.2.2.toml | 62 + .../rfc/rfc5508/section-4.3.toml | 21 + .../rfc/rfc5508/section-5.toml | 51 + .../rfc/rfc5508/section-6.toml | 25 + .../rfc/rfc5508/section-7.1.1.toml | 37 + .../rfc/rfc5508/section-7.1.2.toml | 12 + .../rfc/rfc5508/section-7.2.toml | 19 + .../rfc/rfc5508/section-7.3.toml | 20 + .../rfc/rfc5508/section-7.4.toml | 17 + .../rfc/rfc5508/section-7.5.toml | 17 + .../rfc/rfc5508/section-7.6.toml | 18 + .../rfc/rfc5508/section-7.7.toml | 18 + .../rfc/rfc5508/section-7.toml | 35 + .../rfc/rfc5508/section-8.toml | 17 + .../rfc/rfc5508/section-9.toml | 193 ++ .duvet/snapshot.txt | 313 ++++ .../www.rfc-editor.org/rfc/rfc5508.txt | 1627 +++++++++++++++++ nat/src/icmp_handler/nf.rs | 18 +- nat/src/masquerade/test.rs | 124 ++ scripts/spec-interlock.ts | 14 + 26 files changed, 2890 insertions(+), 2 deletions(-) create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-10.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.1.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.2.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.1.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.1.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.2.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.3.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-5.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-6.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.1.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.2.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.2.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.3.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.4.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.5.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.6.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.7.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-8.toml create mode 100644 .duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-9.toml create mode 100644 .duvet/specifications/www.rfc-editor.org/rfc/rfc5508.txt diff --git a/.duvet/config.toml b/.duvet/config.toml index 53f123a632..ae1cfefba6 100644 --- a/.duvet/config.toml +++ b/.duvet/config.toml @@ -17,3 +17,6 @@ enabled = true [[specification]] source = "https://www.rfc-editor.org/rfc/rfc4787" + +[[specification]] +source = "https://www.rfc-editor.org/rfc/rfc5508" diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-10.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-10.toml new file mode 100644 index 0000000000..d7c1968d35 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-10.toml @@ -0,0 +1,12 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-10" + + +[[spec]] +level = "SHOULD" +quote = ''' +Blocking such ICMP messages is +known to break some protocol features (most notably path MTU +Discovery) and some applications (e.g., ping, traceroute), and such +blocking is NOT RECOMMENDED. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.1.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.1.toml new file mode 100644 index 0000000000..518989622f --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.1.toml @@ -0,0 +1,59 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-3.1" + + +[[spec]] +level = "MUST" +quote = ''' +Unless explicitly overridden by local policy, a NAT device MUST +permit ICMP Queries and their associated responses, when the Query is +initiated from a private host to the external hosts. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +NAT mapping of ICMP Query Identifiers SHOULD be external-host +independent. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +If host +A reused the Query Id X to send ICMP Queries to the same or different +external host, the NAT device SHOULD reuse the same Query Id mapping +(i.e., map the private host's Query Id X to Query Id X' on NAT's +public IP address) instead of assigning a different mapping. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +Below is justification for making the endpoint-independent mapping +for ICMP Query Id a SHOULD [RFC2119] requirement. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +Given the dichotomy between legacy applications not +requiring endpoint-independent mapping and future applications that +might require it, the requirement level is kept at SHOULD [RFC2119]. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-1: Unless explicitly overridden by local policy, a NAT device +MUST permit ICMP Queries and their associated responses, when +the Query is initiated from a private host to the external +hosts. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) NAT mapping of ICMP Query Identifiers SHOULD be external- +host independent. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.2.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.2.toml new file mode 100644 index 0000000000..f8c14c65f4 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-3.2.toml @@ -0,0 +1,30 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-3.2" + + +[[spec]] +level = "MUST" +quote = ''' +An ICMP Query session timer MUST NOT expire in less than 60 seconds. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +It is RECOMMENDED that the ICMP Query session timer be made +configurable. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 +seconds. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) It is RECOMMENDED that the ICMP Query session timer be made +configurable. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.1.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.1.toml new file mode 100644 index 0000000000..da118f8716 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.1.toml @@ -0,0 +1,85 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-4.1" + + +[[spec]] +level = "SHOULD" +quote = ''' +When an ICMP Error packet is received, if the +ICMP checksum fails to validate, the NAT SHOULD silently drop the +ICMP Error packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +For this reason, if the IP checksum of the embedded +packet within an ICMP Error message fails to validate, the NAT SHOULD +silently drop the Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +Specifically, if the embedded packet includes +IP options, the NAT device MUST traverse past the IP options to +locate the start of transport header for the embedded packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +A NAT device SHOULD NOT validate the transport checksum of the +embedded packet within an ICMP Error message, even when it is +possible to do so. +''' + +[[spec]] +level = "MUST" +quote = ''' +In the case that the ICMP Error payload includes ICMP extensions +[ICMP-EXT], the NAT device MUST exclude the optional zero-padding and +the ICMP extensions when evaluating transport checksum for the +embedded packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-3: When an ICMP Error packet is received, if the ICMP checksum +fails to validate, the NAT SHOULD silently drop the ICMP Error +packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) If the IP checksum of the embedded packet fails to +validate, the NAT SHOULD silently drop the Error packet; +and +''' + +[[spec]] +level = "MUST" +quote = ''' +b) If the embedded packet includes IP options, the NAT device +MUST traverse past the IP options to locate the start of +the transport header for the embedded packet; and +''' + +[[spec]] +level = "SHOULD" +quote = ''' +c) The NAT device SHOULD NOT validate the transport checksum +of the embedded packet within an ICMP Error message, even +when it is possible to do so; and +''' + +[[spec]] +level = "MUST" +quote = ''' +d) If the ICMP Error payload contains ICMP extensions +[ICMP-EXT], the NAT device MUST exclude the optional zero- +padding and the ICMP extensions when evaluating transport +checksum for the embedded packet. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.1.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.1.toml new file mode 100644 index 0000000000..e1ec7e2607 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.1.toml @@ -0,0 +1,45 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-4.2.1" + + +[[spec]] +level = "SHOULD" +quote = ''' +If the NAT device does not have an +active mapping for the embedded packet, the NAT SHOULD silently drop +the ICMP Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +Otherwise, the NAT device MUST use the +matching NAT Session to translate the embedded packet; that is, +translate the source IP address of the embedded packet (e.g., Host-y' +-> Host-y) and transport headers. +''' + +[[spec]] +level = "MUST" +quote = ''' +The NAT device MUST also use the matching NAT Session to translate +the destination IP address in the outer IP header. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-4: If a NAT device receives an ICMP Error packet from an external +realm, and the NAT device does not have an active mapping for +the embedded payload, the NAT SHOULD silently drop the ICMP +Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +If the NAT has active mapping for the embedded +payload, then the NAT MUST do the following prior to +forwarding the packet, unless explicitly overridden by local +policy: +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.2.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.2.toml new file mode 100644 index 0000000000..46995cdebb --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.2.2.toml @@ -0,0 +1,62 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-4.2.2" + + +[[spec]] +level = "MUST" +quote = ''' +When the NAT device receives the ICMP Error packet, the NAT device +MUST use the packet embedded within the ICMP Error message (i.e., the +IP packet from Host-x to Host-y) to look up the NAT Session to which +the embedded packet belongs. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +If the NAT device does not have an +active mapping for the embedded packet, the NAT SHOULD silently drop +the ICMP Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +Otherwise, the NAT device MUST use the +matching NAT Session to translate the embedded packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +So, if the NAT device has active mapping for the +IP address of the intermediate node Router-y, the NAT device MUST +translate the source IP address of the ICMP Error packet with the +public IP address in the mapping. +''' + +[[spec]] +level = "MUST" +quote = ''' +In all other cases, the NAT device +MUST simply use its own IP address in the external domain to +translate the source IP address. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-5: If a NAT device receives an ICMP Error packet from the private +realm, and the NAT does not have an active mapping for the +embedded payload, the NAT SHOULD silently drop the ICMP Error +packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +If the NAT has active mapping for the embedded +payload, then the NAT MUST do the following prior to +forwarding the packet, unless explicitly overridden by local +policy: +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.3.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.3.toml new file mode 100644 index 0000000000..1aeac8a1c2 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-4.3.toml @@ -0,0 +1,21 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-4.3" + + +[[spec]] +level = "MUST" +quote = ''' +While processing an ICMP Error packet pertaining to an ICMP Query or +Query response message, a NAT device MUST NOT refresh or delete the +NAT Session that pertains to the embedded payload within the ICMP +Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-6: While processing an ICMP Error packet pertaining to an ICMP +Query or Query response message, a NAT device MUST NOT refresh +or delete the NAT Session that pertains to the embedded +payload within the ICMP Error packet. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-5.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-5.toml new file mode 100644 index 0000000000..41f0f08aff --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-5.toml @@ -0,0 +1,51 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-5" + + +[[spec]] +level = "MUST" +quote = ''' +Specifically, +NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the traversal +of hairpinned ICMP Query sessions. +''' + +[[spec]] +level = "MUST" +quote = ''' +All NAT devices (i.e., Basic NAT as well as NAPT +devices) MUST support the traversal of hairpinned ICMP Error +messages. +''' + +[[spec]] +level = "MUST" +quote = ''' +In addition, the NAT device MUST translate the destination +IP address of the outer IP header to be same as the source IP address +of the embedded IP packet after the translation. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-7: NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the +traversal of hairpinned ICMP Query sessions. +''' + +[[spec]] +level = "MUST" +quote = ''' +All NAT devices +(i.e., Basic NAT as well as NAPT devices) MUST support the +traversal of hairpinned ICMP Error messages: +''' + +[[spec]] +level = "MUST" +quote = ''' +a) When forwarding a hairpinned ICMP Error message, the NAT +device MUST translate the destination IP address of the +outer IP header to be same as the source IP address of the +embedded IP packet after the translation. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-6.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-6.toml new file mode 100644 index 0000000000..e4aea4ce9a --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-6.toml @@ -0,0 +1,25 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-6" + + +[[spec]] +level = "SHOULD" +quote = ''' +When a NAT device is unable to establish a NAT Session for a new +transport-layer (TCP, UDP, ICMP, etc.) flow due to resource +constraints or administrative restrictions, the NAT device SHOULD +send an ICMP destination unreachable message, with a code of 13 +(Communication administratively prohibited) to the sender, and drop +the original packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-8: When a NAT device is unable to establish a NAT Session for a +new transport-layer (TCP, UDP, ICMP, etc.) flow due to resource +constraints or administrative restrictions, the NAT device SHOULD +send an ICMP destination unreachable message, with a code of 13 +(Communication administratively prohibited) to the sender, and drop +the original packet. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.1.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.1.toml new file mode 100644 index 0000000000..44c8c6e046 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.1.toml @@ -0,0 +1,37 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.1.1" + + +[[spec]] +level = "MUST" +quote = ''' +Further, [PMTU] states +that the router MUST include the MTU of that next-hop network in the +low-order 16 bits of the ICMP header field that is labeled "unused" +in the ICMP specification [ICMP]. +''' + +[[spec]] +level = "MUST" +quote = ''' +A NAT device MUST honor the DF bit in the IP header of the packets +that transit the device. +''' + +[[spec]] +level = "MUST" +quote = ''' +If +the DF bit is set on a transit IP packet and the NAT device cannot +forward the packet without fragmentation, the NAT device MUST send a +"Packet Too Big" ICMP message (ICMP type 3, code 4) with the next-hop +MTU back to the sender and drop the original IP packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +If the DF bit is not set and the MTU on the forwarding interface of +the NAT device mandates fragmentation, the NAT device MUST fragment +the packet and forward the fragments [RFC1812]. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.2.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.2.toml new file mode 100644 index 0000000000..3382d63358 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.1.2.toml @@ -0,0 +1,12 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.1.2" + + +[[spec]] +level = "MUST" +quote = ''' +When the NAT device is the recipient of a "Packet Too Big" ICMP +message from the network, the NAT device MUST forward the ICMP +message back to the intended recipient, pursuant to the previously +stated requirements (REQ-3, REQ-4, and REQ-5). +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.2.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.2.toml new file mode 100644 index 0000000000..ed23a583f3 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.2.toml @@ -0,0 +1,19 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.2" + + +[[spec]] +level = "MUST" +quote = ''' +A NAT device MUST generate a "Time Exceeded" ICMP Error message when +it discards a packet due to an expired Time to Live (TTL) field. +''' + +[[spec]] +level = "MUST" +quote = ''' +A +NAT device MAY have a per-interface option to disable origination of +these messages on that interface, but that option MUST default to +allowing the messages to be originated. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.3.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.3.toml new file mode 100644 index 0000000000..d4f8d2b567 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.3.toml @@ -0,0 +1,20 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.3" + + +[[spec]] +level = "MAY" +quote = ''' +A NAT device MAY support modifying IP addresses in the source route +option so the IP addresses in the source route option are realm +relevant. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +If a NAT device does not support forwarding packets with +the source route option, the NAT device SHOULD NOT forward outbound +ICMP messages that contain the source route option in the outer or +inner IP header. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.4.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.4.toml new file mode 100644 index 0000000000..881931832f --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.4.toml @@ -0,0 +1,17 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.4" + + +[[spec]] +level = "MUST" +quote = ''' +Section 4.3.3.9 of [RFC1812] says an IP router MUST implement support +for receiving ICMP Address Mask Request messages and responding with +ICMP Address Mask Reply messages. +''' + +[[spec]] +level = "MAY" +quote = ''' +A NAT device MAY support address mask request/reply messages. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.5.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.5.toml new file mode 100644 index 0000000000..0f4a786c74 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.5.toml @@ -0,0 +1,17 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.5" + + +[[spec]] +level = "MUST" +quote = ''' +Section 4.3.3.5 of [RFC1812] says an IP router MUST generate a +Parameter Problem message for any error not specifically covered by +another ICMP message. +''' + +[[spec]] +level = "MAY" +quote = ''' +A NAT device MAY support parameter problem messages. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.6.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.6.toml new file mode 100644 index 0000000000..a2baaed875 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.6.toml @@ -0,0 +1,18 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.6" + + +[[spec]] +level = "MUST" +quote = ''' +Section 4.3.3.10 of [RFC1812] says an IP router MUST support the +router part of the ICMP Router Discovery Protocol on all connected +networks on which the router supports either IP multicast or IP +broadcast addressing. +''' + +[[spec]] +level = "MAY" +quote = ''' +A NAT device MAY support Router Advertisement and Solicitations. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.7.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.7.toml new file mode 100644 index 0000000000..f7c9ca5f5b --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.7.toml @@ -0,0 +1,18 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7.7" + + +[[spec]] +level = "SHOULD" +quote = ''' +When generating an ICMP message, a NAT device SHOULD copy the +diffserv class of the message that causes the sending of the ICMP +error message. +''' + +[[spec]] +level = "MAY" +quote = ''' +A NAT device MAY allow configuration of the diffserv +class to be used for the different types of ICMP messages. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.toml new file mode 100644 index 0000000000..8abdbfb370 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-7.toml @@ -0,0 +1,35 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-7" + + +[[spec]] +level = "MAY" +quote = ''' +REQ-9: A NAT device MAY implement a policy control that prevents ICMP +messages being generated toward certain interface(s). +''' + +[[spec]] +level = "MUST" +quote = ''' +MUST support: +''' + +[[spec]] +level = "MAY" +quote = ''' +MAY support: +''' + +[[spec]] +level = "SHOULD" +quote = ''' +SHOULD NOT support: +''' + +[[spec]] +level = "SHOULD" +quote = ''' +In addition, a NAT device is RECOMMENDED to conform to the +following implementation considerations: +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-8.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-8.toml new file mode 100644 index 0000000000..458147e915 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-8.toml @@ -0,0 +1,17 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-8" + + +[[spec]] +level = "MAY" +quote = ''' +A NAT MAY drop or appropriately handle Non- +QueryError ICMP messages. +''' + +[[spec]] +level = "MAY" +quote = ''' +REQ-11: A NAT MAY drop or appropriately handle Non-QueryError +ICMP messages. +''' + diff --git a/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-9.toml b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-9.toml new file mode 100644 index 0000000000..348b1cee01 --- /dev/null +++ b/.duvet/requirements/www.rfc-editor.org/rfc/rfc5508/section-9.toml @@ -0,0 +1,193 @@ +target = "https://www.rfc-editor.org/rfc/rfc5508#section-9" + + +[[spec]] +level = "MUST" +quote = ''' +REQ-1: Unless explicitly overridden by local policy, a NAT device +MUST permit ICMP Queries and their associated responses, when +the Query is initiated from a private host to the external +hosts. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) NAT mapping of ICMP Query Identifiers SHOULD be external +host independent. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 +seconds. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) It is RECOMMENDED that the ICMP Query session timer be made +configurable. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-3: When an ICMP Error packet is received, if the ICMP checksum +fails to validate, the NAT SHOULD silently drop the ICMP Error +packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +a) If the IP checksum of the embedded packet fails to +validate, the NAT SHOULD silently drop the Error packet; +and +''' + +[[spec]] +level = "MUST" +quote = ''' +b) If the embedded packet includes IP options, the NAT device +MUST traverse past the IP options to locate the start of +the transport header for the embedded packet; and +''' + +[[spec]] +level = "SHOULD" +quote = ''' +c) The NAT device SHOULD NOT validate the transport checksum +of the embedded packet within an ICMP Error message, even +when it is possible to do so; and +''' + +[[spec]] +level = "MUST" +quote = ''' +d) If the ICMP Error payload contains ICMP extensions +[ICMP-EXT], the NAT device MUST exclude the optional zero- +padding and the ICMP extensions when evaluating transport +checksum for the embedded packet. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-4: If a NAT device receives an ICMP Error packet from an external +realm, and the NAT device does not have an active mapping for +the embedded payload, the NAT SHOULD silently drop the ICMP +Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +If the NAT has active mapping for the embedded +payload, then the NAT MUST do the following prior to +forwarding the packet, unless explicitly overridden by local +policy: +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-5: If a NAT device receives an ICMP Error packet from the private +realm, and the NAT does not have an active mapping for the +embedded payload, the NAT SHOULD silently drop the ICMP Error +packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +If the NAT has active mapping for the embedded +payload, then the NAT MUST do the following prior to +forwarding the packet, unless explicitly overridden by local +policy. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-6: While processing an ICMP Error packet pertaining to an ICMP +Query or Query response message, a NAT device MUST NOT refresh +or delete the NAT Session that pertains to the embedded +payload within the ICMP Error packet. +''' + +[[spec]] +level = "MUST" +quote = ''' +REQ-7: NAT devices enforcing Basic NAT ([NAT-TRAD]) MUST support the +traversal of hairpinned ICMP Query sessions. +''' + +[[spec]] +level = "MUST" +quote = ''' +All NAT devices +(i.e., Basic NAT as well as NAPT devices) MUST support the +traversal of hairpinned ICMP Error messages. +''' + +[[spec]] +level = "MUST" +quote = ''' +a) When forwarding a hairpinned ICMP Error message, the NAT +device MUST translate the destination IP address of the +outer IP header to be same as the source IP address of the +embedded IP packet after the translation. +''' + +[[spec]] +level = "SHOULD" +quote = ''' +REQ-8: When a NAT device is unable to establish a NAT Session for a +new transport-layer (TCP, UDP, ICMP, etc.) flow due to +resource constraints or administrative restrictions, the NAT +device SHOULD send an ICMP destination unreachable message, +with a code of 13 (Communication administratively prohibited) +to the sender, and drop the original packet. +''' + +[[spec]] +level = "MAY" +quote = ''' +REQ-9: A NAT device MAY implement a policy control that prevents ICMP +messages being generated toward certain interface(s). +''' + +[[spec]] +level = "MUST" +quote = ''' +MUST support: +''' + +[[spec]] +level = "MAY" +quote = ''' +MAY support: +''' + +[[spec]] +level = "SHOULD" +quote = ''' +SHOULD NOT support: +''' + +[[spec]] +level = "SHOULD" +quote = ''' +In addition, a NAT device is RECOMMENDED to conform to the +following implementation considerations: +''' + +[[spec]] +level = "MAY" +quote = ''' +REQ-11: A NAT MAY drop or appropriately handle Non-QueryError ICMP +messages. +''' + diff --git a/.duvet/snapshot.txt b/.duvet/snapshot.txt index 5c12884204..763eb05945 100644 --- a/.duvet/snapshot.txt +++ b/.duvet/snapshot.txt @@ -354,3 +354,316 @@ SPECIFICATION: https://www.rfc-editor.org/rfc/rfc5382 TEXT[!SHOULD]: Unreachable (Type 3) messages. TEXT[!MUST,implementation,test]: REQ-10: Receipt of any sort of ICMP message MUST NOT terminate the TEXT[!MUST,implementation,test]: NAT mapping or TCP connection for which the ICMP was generated. + +SPECIFICATION: https://www.rfc-editor.org/rfc/rfc5508 + SECTION: [ICMP Query Mapping](#section-3.1) + TEXT[!MUST]: Unless explicitly overridden by local policy, a NAT device MUST + TEXT[!MUST]: permit ICMP Queries and their associated responses, when the Query is + TEXT[!MUST]: initiated from a private host to the external hosts. + TEXT[!SHOULD]: NAT mapping of ICMP Query Identifiers SHOULD be external-host + TEXT[!SHOULD]: independent. + TEXT[!SHOULD]: If host + TEXT[!SHOULD]: A reused the Query Id X to send ICMP Queries to the same or different + TEXT[!SHOULD]: external host, the NAT device SHOULD reuse the same Query Id mapping + TEXT[!SHOULD]: (i.e., map the private host's Query Id X to Query Id X' on NAT's + TEXT[!SHOULD]: public IP address) instead of assigning a different mapping. + TEXT[!SHOULD]: Below is justification for making the endpoint-independent mapping + TEXT[!SHOULD]: for ICMP Query Id a SHOULD [RFC2119] requirement. + TEXT[!SHOULD]: Given the dichotomy between legacy applications not + TEXT[!SHOULD]: requiring endpoint-independent mapping and future applications that + TEXT[!SHOULD]: might require it, the requirement level is kept at SHOULD [RFC2119]. + TEXT[!MUST]: REQ-1: Unless explicitly overridden by local policy, a NAT device + TEXT[!MUST]: MUST permit ICMP Queries and their associated responses, when + TEXT[!MUST]: the Query is initiated from a private host to the external + TEXT[!MUST]: hosts. + TEXT[!SHOULD]: a) NAT mapping of ICMP Query Identifiers SHOULD be external- + TEXT[!SHOULD]: host independent. + + SECTION: [ICMP Query Session Timeouts](#section-3.2) + TEXT[!MUST]: An ICMP Query session timer MUST NOT expire in less than 60 seconds. + TEXT[!SHOULD]: It is RECOMMENDED that the ICMP Query session timer be made + TEXT[!SHOULD]: configurable. + TEXT[!MUST]: REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 + TEXT[!MUST]: seconds. + TEXT[!SHOULD]: a) It is RECOMMENDED that the ICMP Query session timer be made + TEXT[!SHOULD]: configurable. + + SECTION: [ICMP Error Payload Validation](#section-4.1) + TEXT[!SHOULD]: When an ICMP Error packet is received, if the + TEXT[!SHOULD]: ICMP checksum fails to validate, the NAT SHOULD silently drop the + TEXT[!SHOULD]: ICMP Error packet. + TEXT[!SHOULD]: For this reason, if the IP checksum of the embedded + TEXT[!SHOULD]: packet within an ICMP Error message fails to validate, the NAT SHOULD + TEXT[!SHOULD]: silently drop the Error packet. + TEXT[!MUST]: Specifically, if the embedded packet includes + TEXT[!MUST]: IP options, the NAT device MUST traverse past the IP options to + TEXT[!MUST]: locate the start of transport header for the embedded packet. + TEXT[!SHOULD]: A NAT device SHOULD NOT validate the transport checksum of the + TEXT[!SHOULD]: embedded packet within an ICMP Error message, even when it is + TEXT[!SHOULD]: possible to do so. + TEXT[!MUST]: In the case that the ICMP Error payload includes ICMP extensions + TEXT[!MUST]: [ICMP-EXT], the NAT device MUST exclude the optional zero-padding and + TEXT[!MUST]: the ICMP extensions when evaluating transport checksum for the + TEXT[!MUST]: embedded packet. + TEXT[!SHOULD]: REQ-3: When an ICMP Error packet is received, if the ICMP checksum + TEXT[!SHOULD]: fails to validate, the NAT SHOULD silently drop the ICMP Error + TEXT[!SHOULD]: packet. + TEXT[!SHOULD]: a) If the IP checksum of the embedded packet fails to + TEXT[!SHOULD]: validate, the NAT SHOULD silently drop the Error packet; + TEXT[!SHOULD]: and + TEXT[!MUST]: b) If the embedded packet includes IP options, the NAT device + TEXT[!MUST]: MUST traverse past the IP options to locate the start of + TEXT[!MUST]: the transport header for the embedded packet; and + TEXT[!SHOULD]: c) The NAT device SHOULD NOT validate the transport checksum + TEXT[!SHOULD]: of the embedded packet within an ICMP Error message, even + TEXT[!SHOULD]: when it is possible to do so; and + TEXT[!MUST]: d) If the ICMP Error payload contains ICMP extensions + TEXT[!MUST]: [ICMP-EXT], the NAT device MUST exclude the optional zero- + TEXT[!MUST]: padding and the ICMP extensions when evaluating transport + TEXT[!MUST]: checksum for the embedded packet. + + SECTION: [ICMP Error Packet Received from the External Realm](#section-4.2.1) + TEXT[!SHOULD]: If the NAT device does not have an + TEXT[!SHOULD]: active mapping for the embedded packet, the NAT SHOULD silently drop + TEXT[!SHOULD]: the ICMP Error packet. + TEXT[!MUST]: Otherwise, the NAT device MUST use the + TEXT[!MUST]: matching NAT Session to translate the embedded packet; that is, + TEXT[!MUST]: translate the source IP address of the embedded packet (e.g., Host-y' + TEXT[!MUST]: -> Host-y) and transport headers. + TEXT[!MUST]: The NAT device MUST also use the matching NAT Session to translate + TEXT[!MUST]: the destination IP address in the outer IP header. + TEXT[!SHOULD]: REQ-4: If a NAT device receives an ICMP Error packet from an external + TEXT[!SHOULD]: realm, and the NAT device does not have an active mapping for + TEXT[!SHOULD]: the embedded payload, the NAT SHOULD silently drop the ICMP + TEXT[!SHOULD]: Error packet. + TEXT[!MUST]: If the NAT has active mapping for the embedded + TEXT[!MUST]: payload, then the NAT MUST do the following prior to + TEXT[!MUST]: forwarding the packet, unless explicitly overridden by local + TEXT[!MUST]: policy: + + SECTION: [ICMP Error Packet Received from the Private Realm](#section-4.2.2) + TEXT[!MUST]: When the NAT device receives the ICMP Error packet, the NAT device + TEXT[!MUST]: MUST use the packet embedded within the ICMP Error message (i.e., the + TEXT[!MUST]: IP packet from Host-x to Host-y) to look up the NAT Session to which + TEXT[!MUST]: the embedded packet belongs. + TEXT[!SHOULD]: If the NAT device does not have an + TEXT[!SHOULD]: active mapping for the embedded packet, the NAT SHOULD silently drop + TEXT[!SHOULD]: the ICMP Error packet. + TEXT[!MUST]: Otherwise, the NAT device MUST use the + TEXT[!MUST]: matching NAT Session to translate the embedded packet. + TEXT[!MUST]: So, if the NAT device has active mapping for the + TEXT[!MUST]: IP address of the intermediate node Router-y, the NAT device MUST + TEXT[!MUST]: translate the source IP address of the ICMP Error packet with the + TEXT[!MUST]: public IP address in the mapping. + TEXT[!MUST]: In all other cases, the NAT device + TEXT[!MUST]: MUST simply use its own IP address in the external domain to + TEXT[!MUST]: translate the source IP address. + TEXT[!SHOULD]: REQ-5: If a NAT device receives an ICMP Error packet from the private + TEXT[!SHOULD]: realm, and the NAT does not have an active mapping for the + TEXT[!SHOULD]: embedded payload, the NAT SHOULD silently drop the ICMP Error + TEXT[!SHOULD]: packet. + TEXT[!MUST]: If the NAT has active mapping for the embedded + TEXT[!MUST]: payload, then the NAT MUST do the following prior to + TEXT[!MUST]: forwarding the packet, unless explicitly overridden by local + TEXT[!MUST]: policy: + + SECTION: [NAT Sessions Pertaining to ICMP Error Payload](#section-4.3) + TEXT[!MUST]: While processing an ICMP Error packet pertaining to an ICMP Query or + TEXT[!MUST]: Query response message, a NAT device MUST NOT refresh or delete the + TEXT[!MUST]: NAT Session that pertains to the embedded payload within the ICMP + TEXT[!MUST]: Error packet. + TEXT[!MUST,implementation,test]: REQ-6: While processing an ICMP Error packet pertaining to an ICMP + TEXT[!MUST,implementation,test]: Query or Query response message, a NAT device MUST NOT refresh + TEXT[!MUST,implementation,test]: or delete the NAT Session that pertains to the embedded + TEXT[!MUST,implementation,test]: payload within the ICMP Error packet. + + SECTION: [Hairpinning Support for ICMP Packets](#section-5) + TEXT[!MUST]: Specifically, + TEXT[!MUST]: NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the traversal + TEXT[!MUST]: of hairpinned ICMP Query sessions. + TEXT[!MUST]: All NAT devices (i.e., Basic NAT as well as NAPT + TEXT[!MUST]: devices) MUST support the traversal of hairpinned ICMP Error + TEXT[!MUST]: messages. + TEXT[!MUST]: In addition, the NAT device MUST translate the destination + TEXT[!MUST]: IP address of the outer IP header to be same as the source IP address + TEXT[!MUST]: of the embedded IP packet after the translation. + TEXT[!MUST]: REQ-7: NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the + TEXT[!MUST]: traversal of hairpinned ICMP Query sessions. + TEXT[!MUST]: All NAT devices + TEXT[!MUST]: (i.e., Basic NAT as well as NAPT devices) MUST support the + TEXT[!MUST]: traversal of hairpinned ICMP Error messages: + TEXT[!MUST]: a) When forwarding a hairpinned ICMP Error message, the NAT + TEXT[!MUST]: device MUST translate the destination IP address of the + TEXT[!MUST]: outer IP header to be same as the source IP address of the + TEXT[!MUST]: embedded IP packet after the translation. + + SECTION: [Rejection of Outbound Flows Disallowed by NAT](#section-6) + TEXT[!SHOULD]: When a NAT device is unable to establish a NAT Session for a new + TEXT[!SHOULD]: transport-layer (TCP, UDP, ICMP, etc.) flow due to resource + TEXT[!SHOULD]: constraints or administrative restrictions, the NAT device SHOULD + TEXT[!SHOULD]: send an ICMP destination unreachable message, with a code of 13 + TEXT[!SHOULD]: (Communication administratively prohibited) to the sender, and drop + TEXT[!SHOULD]: the original packet. + TEXT[!SHOULD]: REQ-8: When a NAT device is unable to establish a NAT Session for a + TEXT[!SHOULD]: new transport-layer (TCP, UDP, ICMP, etc.) flow due to resource + TEXT[!SHOULD]: constraints or administrative restrictions, the NAT device SHOULD + TEXT[!SHOULD]: send an ICMP destination unreachable message, with a code of 13 + TEXT[!SHOULD]: (Communication administratively prohibited) to the sender, and drop + TEXT[!SHOULD]: the original packet. + + SECTION: [Conformance to RFC 1812](#section-7) + TEXT[!MAY]: REQ-9: A NAT device MAY implement a policy control that prevents ICMP + TEXT[!MAY]: messages being generated toward certain interface(s). + TEXT[!MUST]: MUST support: + TEXT[!MAY]: MAY support: + TEXT[!SHOULD]: SHOULD NOT support: + TEXT[!SHOULD]: In addition, a NAT device is RECOMMENDED to conform to the + TEXT[!SHOULD]: following implementation considerations: + + SECTION: [Generating "Packet Too Big" ICMP Error Message](#section-7.1.1) + TEXT[!MUST]: Further, [PMTU] states + TEXT[!MUST]: that the router MUST include the MTU of that next-hop network in the + TEXT[!MUST]: low-order 16 bits of the ICMP header field that is labeled "unused" + TEXT[!MUST]: in the ICMP specification [ICMP]. + TEXT[!MUST]: A NAT device MUST honor the DF bit in the IP header of the packets + TEXT[!MUST]: that transit the device. + TEXT[!MUST]: If + TEXT[!MUST]: the DF bit is set on a transit IP packet and the NAT device cannot + TEXT[!MUST]: forward the packet without fragmentation, the NAT device MUST send a + TEXT[!MUST]: "Packet Too Big" ICMP message (ICMP type 3, code 4) with the next-hop + TEXT[!MUST]: MTU back to the sender and drop the original IP packet. + TEXT[!MUST]: If the DF bit is not set and the MTU on the forwarding interface of + TEXT[!MUST]: the NAT device mandates fragmentation, the NAT device MUST fragment + TEXT[!MUST]: the packet and forward the fragments [RFC1812]. + + SECTION: [Forwarding "Packet Too Big" ICMP Error Message](#section-7.1.2) + TEXT[!MUST]: When the NAT device is the recipient of a "Packet Too Big" ICMP + TEXT[!MUST]: message from the network, the NAT device MUST forward the ICMP + TEXT[!MUST]: message back to the intended recipient, pursuant to the previously + TEXT[!MUST]: stated requirements (REQ-3, REQ-4, and REQ-5). + + SECTION: [Time Exceeded Message](#section-7.2) + TEXT[!MUST]: A NAT device MUST generate a "Time Exceeded" ICMP Error message when + TEXT[!MUST]: it discards a packet due to an expired Time to Live (TTL) field. + TEXT[!MUST]: NAT device MAY have a per-interface option to disable origination of + TEXT[!MUST]: these messages on that interface, but that option MUST default to + TEXT[!MUST]: allowing the messages to be originated. + + SECTION: [Source Route Options](#section-7.3) + TEXT[!MAY]: A NAT device MAY support modifying IP addresses in the source route + TEXT[!MAY]: option so the IP addresses in the source route option are realm + TEXT[!MAY]: relevant. + TEXT[!SHOULD]: If a NAT device does not support forwarding packets with + TEXT[!SHOULD]: the source route option, the NAT device SHOULD NOT forward outbound + TEXT[!SHOULD]: ICMP messages that contain the source route option in the outer or + TEXT[!SHOULD]: inner IP header. + + SECTION: [Address Mask Request/Reply Messages](#section-7.4) + TEXT[!MUST]: Section 4.3.3.9 of [RFC1812] says an IP router MUST implement support + TEXT[!MUST]: for receiving ICMP Address Mask Request messages and responding with + TEXT[!MUST]: ICMP Address Mask Reply messages. + TEXT[!MAY]: A NAT device MAY support address mask request/reply messages. + + SECTION: [Parameter Problem Message](#section-7.5) + TEXT[!MUST]: Section 4.3.3.5 of [RFC1812] says an IP router MUST generate a + TEXT[!MUST]: Parameter Problem message for any error not specifically covered by + TEXT[!MUST]: another ICMP message. + TEXT[!MAY]: A NAT device MAY support parameter problem messages. + + SECTION: [Router Advertisement and Solicitations](#section-7.6) + TEXT[!MUST]: Section 4.3.3.10 of [RFC1812] says an IP router MUST support the + TEXT[!MUST]: router part of the ICMP Router Discovery Protocol on all connected + TEXT[!MUST]: networks on which the router supports either IP multicast or IP + TEXT[!MUST]: broadcast addressing. + TEXT[!MAY]: A NAT device MAY support Router Advertisement and Solicitations. + + SECTION: [DS Field Usage](#section-7.7) + TEXT[!SHOULD]: When generating an ICMP message, a NAT device SHOULD copy the + TEXT[!SHOULD]: diffserv class of the message that causes the sending of the ICMP + TEXT[!SHOULD]: error message. + TEXT[!MAY]: A NAT device MAY allow configuration of the diffserv + TEXT[!MAY]: class to be used for the different types of ICMP messages. + + SECTION: [Non-QueryError ICMP Messages](#section-8) + TEXT[!MAY]: A NAT MAY drop or appropriately handle Non- + TEXT[!MAY]: QueryError ICMP messages. + TEXT[!MAY]: REQ-11: A NAT MAY drop or appropriately handle Non-QueryError + TEXT[!MAY]: ICMP messages. + + SECTION: [Summary of Requirements](#section-9) + TEXT[!MUST]: REQ-1: Unless explicitly overridden by local policy, a NAT device + TEXT[!MUST]: MUST permit ICMP Queries and their associated responses, when + TEXT[!MUST]: the Query is initiated from a private host to the external + TEXT[!MUST]: hosts. + TEXT[!SHOULD]: a) NAT mapping of ICMP Query Identifiers SHOULD be external + TEXT[!SHOULD]: host independent. + TEXT[!MUST]: REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 + TEXT[!MUST]: seconds. + TEXT[!SHOULD]: a) It is RECOMMENDED that the ICMP Query session timer be made + TEXT[!SHOULD]: configurable. + TEXT[!SHOULD]: REQ-3: When an ICMP Error packet is received, if the ICMP checksum + TEXT[!SHOULD]: fails to validate, the NAT SHOULD silently drop the ICMP Error + TEXT[!SHOULD]: packet. + TEXT[!SHOULD]: a) If the IP checksum of the embedded packet fails to + TEXT[!SHOULD]: validate, the NAT SHOULD silently drop the Error packet; + TEXT[!SHOULD]: and + TEXT[!MUST]: b) If the embedded packet includes IP options, the NAT device + TEXT[!MUST]: MUST traverse past the IP options to locate the start of + TEXT[!MUST]: the transport header for the embedded packet; and + TEXT[!SHOULD]: c) The NAT device SHOULD NOT validate the transport checksum + TEXT[!SHOULD]: of the embedded packet within an ICMP Error message, even + TEXT[!SHOULD]: when it is possible to do so; and + TEXT[!MUST]: d) If the ICMP Error payload contains ICMP extensions + TEXT[!MUST]: [ICMP-EXT], the NAT device MUST exclude the optional zero- + TEXT[!MUST]: padding and the ICMP extensions when evaluating transport + TEXT[!MUST]: checksum for the embedded packet. + TEXT[!SHOULD]: REQ-4: If a NAT device receives an ICMP Error packet from an external + TEXT[!SHOULD]: realm, and the NAT device does not have an active mapping for + TEXT[!SHOULD]: the embedded payload, the NAT SHOULD silently drop the ICMP + TEXT[!SHOULD]: Error packet. + TEXT[!MUST]: If the NAT has active mapping for the embedded + TEXT[!MUST]: payload, then the NAT MUST do the following prior to + TEXT[!MUST]: forwarding the packet, unless explicitly overridden by local + TEXT[!MUST]: policy: + TEXT[!SHOULD]: REQ-5: If a NAT device receives an ICMP Error packet from the private + TEXT[!SHOULD]: realm, and the NAT does not have an active mapping for the + TEXT[!SHOULD]: embedded payload, the NAT SHOULD silently drop the ICMP Error + TEXT[!SHOULD]: packet. + TEXT[!MUST]: If the NAT has active mapping for the embedded + TEXT[!MUST]: payload, then the NAT MUST do the following prior to + TEXT[!MUST]: forwarding the packet, unless explicitly overridden by local + TEXT[!MUST]: policy. + TEXT[!MUST]: REQ-6: While processing an ICMP Error packet pertaining to an ICMP + TEXT[!MUST]: Query or Query response message, a NAT device MUST NOT refresh + TEXT[!MUST]: or delete the NAT Session that pertains to the embedded + TEXT[!MUST]: payload within the ICMP Error packet. + TEXT[!MUST]: REQ-7: NAT devices enforcing Basic NAT ([NAT-TRAD]) MUST support the + TEXT[!MUST]: traversal of hairpinned ICMP Query sessions. + TEXT[!MUST]: All NAT devices + TEXT[!MUST]: (i.e., Basic NAT as well as NAPT devices) MUST support the + TEXT[!MUST]: traversal of hairpinned ICMP Error messages. + TEXT[!MUST]: a) When forwarding a hairpinned ICMP Error message, the NAT + TEXT[!MUST]: device MUST translate the destination IP address of the + TEXT[!MUST]: outer IP header to be same as the source IP address of the + TEXT[!MUST]: embedded IP packet after the translation. + TEXT[!SHOULD]: REQ-8: When a NAT device is unable to establish a NAT Session for a + TEXT[!SHOULD]: new transport-layer (TCP, UDP, ICMP, etc.) flow due to + TEXT[!SHOULD]: resource constraints or administrative restrictions, the NAT + TEXT[!SHOULD]: device SHOULD send an ICMP destination unreachable message, + TEXT[!SHOULD]: with a code of 13 (Communication administratively prohibited) + TEXT[!SHOULD]: to the sender, and drop the original packet. + TEXT[!MAY]: REQ-9: A NAT device MAY implement a policy control that prevents ICMP + TEXT[!MAY]: messages being generated toward certain interface(s). + TEXT[!MUST]: MUST support: + TEXT[!MAY]: MAY support: + TEXT[!SHOULD]: SHOULD NOT support: + TEXT[!SHOULD]: In addition, a NAT device is RECOMMENDED to conform to the + TEXT[!SHOULD]: following implementation considerations: + TEXT[!MAY]: REQ-11: A NAT MAY drop or appropriately handle Non-QueryError ICMP + TEXT[!MAY]: messages. + + SECTION: [Security Considerations](#section-10) + TEXT[!SHOULD]: Blocking such ICMP messages is + TEXT[!SHOULD]: known to break some protocol features (most notably path MTU + TEXT[!SHOULD]: Discovery) and some applications (e.g., ping, traceroute), and such + TEXT[!SHOULD]: blocking is NOT RECOMMENDED. diff --git a/.duvet/specifications/www.rfc-editor.org/rfc/rfc5508.txt b/.duvet/specifications/www.rfc-editor.org/rfc/rfc5508.txt new file mode 100644 index 0000000000..d1d5a78939 --- /dev/null +++ b/.duvet/specifications/www.rfc-editor.org/rfc/rfc5508.txt @@ -0,0 +1,1627 @@ + + + + + + +Network Working Group P. Srisuresh +Request for Comments: 5508 Kazeon Systems +BCP: 148 B. Ford +Category: Best Current Practice MPI-SWS + S. Sivakumar + Cisco Systems + S. Guha + Cornell U. + April 2009 + + + NAT Behavioral Requirements for ICMP + +Status of This Memo + + This document specifies an Internet Best Current Practices for the + Internet Community, and requests discussion and suggestions for + improvements. Distribution of this memo is unlimited. + +Copyright Notice + + Copyright (c) 2009 IETF Trust and the persons identified as the + document authors. All rights reserved. + + This document is subject to BCP 78 and the IETF Trust's Legal + Provisions Relating to IETF Documents in effect on the date of + publication of this document (http://trustee.ietf.org/license-info). + Please review these documents carefully, as they describe your rights + and restrictions with respect to this document. + + This document may contain material from IETF Documents or IETF + Contributions published or made publicly available before November + 10, 2008. The person(s) controlling the copyright in some of this + material may not have granted the IETF Trust the right to allow + modifications of such material outside the IETF Standards Process. + Without obtaining an adequate license from the person(s) controlling + the copyright in such materials, this document may not be modified + outside the IETF Standards Process, and derivative works of it may + not be created outside the IETF Standards Process, except to format + it for publication as an RFC or to translate it into languages other + than English. + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 1] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +Abstract + + This document specifies the behavioral properties required of the + Network Address Translator (NAT) devices in conjunction with the + Internet Control Message Protocol (ICMP). The objective of this memo + is to make NAT devices more predictable and compatible with diverse + application protocols that traverse the devices. Companion documents + provide behavioral recommendations specific to TCP, UDP, and other + protocols. + +Table of Contents + + 1. Introduction and Scope ..........................................3 + 2. Terminology .....................................................4 + 3. ICMP Query Handling .............................................6 + 3.1. ICMP Query Mapping .........................................6 + 3.2. ICMP Query Session Timeouts ................................7 + 4. ICMP Error Forwarding ...........................................8 + 4.1. ICMP Error Payload Validation ..............................8 + 4.2. ICMP Error Packet Translation .............................10 + 4.2.1. ICMP Error Packet Received from the External Realm .11 + 4.2.2. ICMP Error Packet Received from the Private Realm ..13 + 4.3. NAT Sessions Pertaining to ICMP Error Payload .............15 + 5. Hairpinning Support for ICMP Packets ...........................16 + 6. Rejection of Outbound Flows Disallowed by NAT ..................17 + 7. Conformance to RFC 1812 ........................................17 + 7.1. IP Packet Fragmentation ...................................19 + 7.1.1. Generating "Packet Too Big" ICMP Error Message ....19 + 7.1.2. Forwarding "Packet Too Big" ICMP Error Message ....20 + 7.2. Time Exceeded Message .....................................20 + 7.3. Source Route Options ......................................20 + 7.4. Address Mask Request/Reply Messages .......................20 + 7.5. Parameter Problem Message .................................21 + 7.6. Router Advertisement and Solicitations ....................21 + 7.7. DS Field Usage ............................................21 + 8. Non-QueryError ICMP Messages ...................................22 + 9. Summary of Requirements ........................................22 + 10. Security Considerations .......................................25 + 11. Acknowledgements ..............................................26 + 12. References ....................................................27 + 12.1. Normative References .....................................27 + 12.2. Informative References ...................................27 + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 2] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +1. Introduction and Scope + + As pointed out in RFC 3424 [UNSAF], NAT implementations vary widely + in terms of how they handle different traffic. The purpose of this + document is to define a specific set of requirements for NAT behavior + with regard to ICMP messages. The objective is to reduce the + unpredictability and brittleness the NAT devices (NATs) introduce. + This document is an adjunct to [BEH-UDP], [BEH-TCP], and other + protocol-specific BEHAVE document(s) in the future that define + requirements for NATs when handling protocol-specific traffic. + + The requirements of this specification apply to traditional NATs as + described in [NAT-TRAD]. A traditional NAT has two variations, + namely Basic NAT and Network Address Port Translator (NAPT). Of + these, NAPT is by far the most commonly deployed NAT device. NAPT + allows multiple private hosts to share a single public IP address + simultaneously. + + This document only covers the ICMP aspects of NAT traversal, + specifically the traversal of ICMP Query messages and ICMP Error + messages. Traditional NAT inherently mandates firewall-like + filtering behavior [BEH-UDP]. However, firewall functionality in + general or any other middlebox functionality is out of the scope of + this document. + + In some cases, ICMP message traversal behavior on a NAT device may be + overridden by local administrative policies. Some administrators may + choose to entirely prohibit forwarding of ICMP Error messages across + a NAT device. Some others may choose to prohibit ICMP-Query-based + applications across a NAT device. These are local policies and not + within the scope of this document. For this reason, some of the ICMP + requirements listed in the document are preceded with a constraint of + local policy permitting. + + This document focuses strictly on the behavior of the NAT device, and + not on the behavior of applications that traverse NATs. Application + designers may refer to [BEH-APP] and [ICE] for recommendations and + guidelines on how to make applications work robustly over NATs that + follow the requirements specified here and the adjunct protocol- + specific BEHAVE documents. + + Per [RFC1812], ICMP is a control protocol that is considered to be an + integral part of IP, although it is architecturally layered upon IP + -- it uses IP to carry its data end-to-end. As such, many of the + ICMP behavioral requirements discussed in this document apply to all + IP protocols. + + + + + +Srisuresh, et al. Best Current Practice [Page 3] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + In case a requirement in this document conflicts with protocol- + specific BEHAVE requirement(s), protocol-specific BEHAVE documents + will take precedence. The authors are not aware of any conflicts + between this and any other IETF document at the time of this writing. + + Section 2 describes the terminology used throughout the document. + Section 3 is focused on requirements concerning ICMP-Query-based + applications traversing a NAT device. Sections 4 and 5 describe + requirements concerning ICMP Error messages traversing a NAT device. + Sections 6 describes requirements concerning ICMP Error messages + generated by a NAT device. Section 7 reviews RFC 1812 conformance + requirements and applicability to NATs when handling ICMP messages. + Section 8 reviews a requirement for ICMP messages that are neither + ICMP Query nor ICMP Error kind. Section 9 summarizes all the + requirements in one place. Section 10 has a discussion on security + considerations. + +2. Terminology + + Definitions for the majority of the NAT terms used throughout the + document may be found in [NAT-TERM] and [BEH-UDP]. + + The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", + "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this + document are to be interpreted as described in RFC 2119 [RFC2119]. + + The term "Realm" is adapted from [NAT-TERM] and is defined as + follows. "Realm" is often interchanged for "network domain" or + simply "network" throughout the document. + + Address realm or Realm - An address realm is a network domain in + which the network addresses are uniquely assigned to entities such + that datagrams can be routed to them. Routing protocols used within + the network domain are responsible for finding routes to entities + given their network addresses. Note that this document is limited to + describing NAT in the IPv4 environment and does not address the use + of NAT in other types of environments (e.g., the IPV6 environment). + + The term "NAT Session" is adapted from [NAT-MIB] and is defined as + follows: + + NAT Session - A NAT session is an association between a session as + seen in the private realm and a session as seen in the public realm, + by virtue of NAT translation. If a session in the private realm were + to be represented as (PrivateSrcAddr, PrivateDstAddr, + TransportProtocol, PrivateSrcPort, PrivateDstPort) and the same + session in the public realm were to be represented as (PublicSrcAddr, + PublicDstAddr, TransportProtocol, PublicSrcPort, PublicDstPort), the + + + +Srisuresh, et al. Best Current Practice [Page 4] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + NAT session would provide the translation glue between the two + session representations. NAT sessions in the document are restricted + to sessions based on TCP, UDP, and ICMP. In the future, NAT sessions + may be extended to be based on other transport protocols such as + Stream Control Transmission Protocol (SCTP), UDP-lite, and Datagram + Congestion Control Protocol (DCCP). + + ICMP Message Classification - Section 3.2.2 of [RFC1122] and Section + 4.3.1 of [RFC1812] broadly group ICMP messages into two main + categories, namely "ICMP Query" messages and "ICMP Error" messages. + All ICMP Error messages listed in RFC 1122 and RFC 1812 contain part + of the Internet datagram that elicited the ICMP error. All the ICMP + Query messages listed in RFC 1122 and RFC 1812 contain an + "Identifier" field, which is referred to in this document as the + "Query Identifier". There are however ICMP messages that do not fall + into either of these two categories. We refer to them as "Non- + QueryError ICMP Messages". All three ICMP message classes are + described as follows: + + o ICMP Query Messages - ICMP Query messages are characterized by an + Identifier field in the ICMP header. The Identifier field used by + the ICMP Query messages is also referred to as "Query Identifier" + or "Query Id", for short throughout the document. A Query Id is + used by Query senders and responders as the equivalent of a TCP/UDP + port to identify an ICMP Query session. ICMP Query messages + include ICMP messages defined after RFC 1122 or RFC 1812 (for + example, Domain Name Request/Reply ICMP messages defined in RFC + 1788), as they include request/response pairs and contain an + "Identifier" field. + + o ICMP Error Messages - ICMP Error messages provide signaling for IP. + All ICMP Error messages are characterized by the fact that they + embed the original datagram that triggered the ICMP Error message. + The original datagram embedded within the ICMP Error payload is + also referred to as the "Embedded packet" throughout the document. + Unlike ICMP Query messages, ICMP Error messages do not have a Query + Id in the ICMP header. + + o Non-QueryError ICMP Messages - ICMP messages that do not fall under + either of the above two classes are referred to as "Non-QueryError + ICMP Messages" throughout the document. For example, Router + Discovery ICMP messages [RFC1256] are "request/response" type ICMP + messages. However, they are not characterized as ICMP Query + messages in this document as they do not have an "Identifier" field + within the messages. Likewise, there are other ICMP messages + defined in [RFC4065] that do not fall in either of the ICMP Query + or ICMP Error message categories, but will be referred to as Non- + QueryError ICMP messages. + + + +Srisuresh, et al. Best Current Practice [Page 5] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + The reason for categorizing ICMP messages for NAT behavioral + properties is that each category has different characteristics used + for mapping (i.e., the Query Id and the Embedded datagram), which + leaves the Non-QueryError ICMP messages in a separate, distinctive + group. + +3. ICMP Query Handling + + This section lists the behavioral requirements for a NAT device when + processing ICMP Query packets. The following subsections discuss + requirements specific to ICMP Query handling in detail. + +3.1. ICMP Query Mapping + + Unless explicitly overridden by local policy, a NAT device MUST + permit ICMP Queries and their associated responses, when the Query is + initiated from a private host to the external hosts. ICMP Query + mapping by NAT devices is necessary for current ICMP-Query-based + applications to work. This entails a NAT device to transparently + forward ICMP Query packets initiated from the nodes behind NAT, and + the responses to these Query packets in the opposite direction. As + specified in [NAT-TRAD], this requires translating the IP header. A + NAPT device further translates the ICMP Query Id and the associated + checksum in the ICMP header prior to forwarding. + + NAT mapping of ICMP Query Identifiers SHOULD be external-host + independent. Say, an internal host A sent an ICMP Query out to an + external host B using Query Id X. And, say, the NAT assigned this an + external mapping of Query Id X' on the NAT's public address. If host + A reused the Query Id X to send ICMP Queries to the same or different + external host, the NAT device SHOULD reuse the same Query Id mapping + (i.e., map the private host's Query Id X to Query Id X' on NAT's + public IP address) instead of assigning a different mapping. This is + similar to the "endpoint independent mapping" requirement specified + in the TCP and UDP requirement documents [BEH-UDP], [BEH-TCP]. + + Below is justification for making the endpoint-independent mapping + for ICMP Query Id a SHOULD [RFC2119] requirement. ICMP Ping + [RFC1470] and ICMP traceroute [MS-TRCRT] are two most commonly known + legacy applications built on top of ICMP Query messages. Neither of + these applications require the ICMP Query Id to be retained across + different sessions with external hosts. But, that may not be the + case with future applications. In the future, when an end host + application reuses the same Query Identifier in sessions with + different target hosts, the end host application might require that + the endpoint identity (i.e., the tuple of IP address and Query + Identifier) appears the same across all its target hosts. In an IP + network without NAT requirements, such a requirement will be valid. + + + +Srisuresh, et al. Best Current Practice [Page 6] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + In a world with NAT devices, the above assumption will be valid when + NAT devices enforce endpoint mapping that is external-host + independent. Given the dichotomy between legacy applications not + requiring endpoint-independent mapping and future applications that + might require it, the requirement level is kept at SHOULD [RFC2119]. + + REQ-1: Unless explicitly overridden by local policy, a NAT device + MUST permit ICMP Queries and their associated responses, when + the Query is initiated from a private host to the external + hosts. + + a) NAT mapping of ICMP Query Identifiers SHOULD be external- + host independent. + +3.2. ICMP Query Session Timeouts + + NATs maintain a mapping timeout for the ICMP Queries that traverse + them. The mapping timeout is the time a mapping will stay active + without packets traversing the NAT. There is great variation in the + values used by different NATs. The ICMP Query session timeout + requirement is necessary for current ICMP Query applications to work. + Query response times can vary. ICMP-Query-based applications are + primarily request/response driven. + + Ideally, the timeout should be set to Maximum Round Trip Time + (Maximum RTT). For the purposes of constraining the maximum RTT, the + Maximum Segment Lifetime (MSL), defined in [RFC793], could be + considered a guideline to set packet lifetime. Per [RFC793], MSL is + the maximum amount of time a TCP segment can exist in a network + before being delivered to the intended recipient. This is the + maximum duration an IP packet can be assumed to take to reach the + intended destination node before declaring that the packet will no + longer be delivered. For an application initiating an ICMP Query + message and waiting for a response for the Query, the Maximum RTT + could in practice be constrained to be the sum total of MSL for the + Query message and MSL for the response message. In other words, + Maximum RTT could be constrained to no more than 2x MSL. The + recommended value for MSL in [RFC793] is 120 seconds, even though + several implementations set this to 60 seconds or 30 seconds. When + MSL is 120 seconds, the Maximum RTT (2x MSL) would be 240 seconds. + + In practice, ICMP Ping [RFC1470] and ICMP traceroute [MS-TRCRT], the + two most commonly known legacy applications built on top of ICMP + Query messages, take less than 10 seconds to complete a round trip + when the target node is operational on the network. + + + + + + +Srisuresh, et al. Best Current Practice [Page 7] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + Setting the ICMP NAT session timeout to a very large duration (say, + 240 seconds) could potentially tie up precious NAT resources such as + Query mappings and NAT Sessions for the whole duration. On the other + hand, setting the timeout very low can result in premature freeing of + NAT resources and applications failing to complete gracefully. The + ICMP Query session timeout needs to be a balance between the two + extremes. A 60-second timeout is a balance between the two extremes. + An ICMP Query session timer MUST NOT expire in less than 60 seconds. + It is RECOMMENDED that the ICMP Query session timer be made + configurable. + + REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 + seconds. + + a) It is RECOMMENDED that the ICMP Query session timer be made + configurable. + +4. ICMP Error Forwarding + + Many applications make use of ICMP Error messages from end hosts and + intermediate devices to shorten application timeouts. Some + applications will not operate correctly without the receipt of ICMP + Error messages. The following sub-sections discuss the requirements + a NAT device must conform to in order to ensure reliable forwarding. + +4.1. ICMP Error Payload Validation + + An ICMP Error message checksum covers the entire ICMP message, + including the payload. When an ICMP Error packet is received, if the + ICMP checksum fails to validate, the NAT SHOULD silently drop the + ICMP Error packet. This is because NAT uses the embedded IP and + transport headers for forwarding and translating the ICMP Error + message (described in Section 4.2). When the ICMP checksum is + invalid, the embedded IP and transport headers, which are covered by + the ICMP checksum, are also suspect. + + [RFC1812] and [RFC1122] require a router or an end host that receives + an IP packet with an invalid IP header checksum to silently drop the + IP packet. As such, end hosts and routers do not generate an ICMP + Error message in response to IP packets with invalid IP header + checksums. For this reason, if the IP checksum of the embedded + packet within an ICMP Error message fails to validate, the NAT SHOULD + silently drop the Error packet. + + When the IP packet embedded within the ICMP Error message includes IP + options, the NAT device must not assume that the transport header of + the embedded packet is at a fixed offset (as would be the case when + there are no IP options associated with the packet) from the start of + + + +Srisuresh, et al. Best Current Practice [Page 8] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + the embedded packet. Specifically, if the embedded packet includes + IP options, the NAT device MUST traverse past the IP options to + locate the start of transport header for the embedded packet. + + It is possible to compute the transport checksum of the embedded + packet within an ICMP Error message when the ICMP Error message + contains the entire transport segment. However, ICMP Error messages + do not contain the entire transport segment in many cases. This is + because [ICMP] stipulates that an ICMP Error message should embed an + IP header and only a minimum of 64 bits of the IP payload. Even + though Section 4.3.2.3 of [RFC1812] recommends an ICMP Error + originator include as much of the original packet as possible in the + payload, the length of the resulting ICMP datagram cannot exceed 576 + bytes. ICMP Error originators truncate IP packets that do not fit + within the stipulations. + + A NAT device SHOULD NOT validate the transport checksum of the + embedded packet within an ICMP Error message, even when it is + possible to do so. This is because a NAT dropping an ICMP Error + message due to an invalid transport checksum will make it harder for + end hosts to receive error reporting for certain types of corruption. + End-to-end validation of ICMP Error messages is best left to end + hosts. Many newer revision end host TCP/IP stacks implement the + improvements in [TCP-SOFT] and do not accept ICMP Error messages with + a mismatched IP or TCP checksum in the embedded packet, if the + embedded datagram contains a full IP packet and the TCP checksum can + be calculated. + + In the case that the ICMP Error payload includes ICMP extensions + [ICMP-EXT], the NAT device MUST exclude the optional zero-padding and + the ICMP extensions when evaluating transport checksum for the + embedded packet. Readers are urged to refer to [ICMP-EXT] for + information on identifying the presence of ICMP extensions in an ICMP + message. + + REQ-3: When an ICMP Error packet is received, if the ICMP checksum + fails to validate, the NAT SHOULD silently drop the ICMP Error + packet. If the ICMP checksum is valid, do the following: + + a) If the IP checksum of the embedded packet fails to + validate, the NAT SHOULD silently drop the Error packet; + and + + b) If the embedded packet includes IP options, the NAT device + MUST traverse past the IP options to locate the start of + the transport header for the embedded packet; and + + + + + +Srisuresh, et al. Best Current Practice [Page 9] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + c) The NAT device SHOULD NOT validate the transport checksum + of the embedded packet within an ICMP Error message, even + when it is possible to do so; and + + d) If the ICMP Error payload contains ICMP extensions + [ICMP-EXT], the NAT device MUST exclude the optional zero- + padding and the ICMP extensions when evaluating transport + checksum for the embedded packet. + +4.2. ICMP Error Packet Translation + + Section 4.3 of [NAT-TRAD] describes the fields of an ICMP Error + message that a NAT device translates. In this section, we describe + the requirements a NAT device must conform to while performing the + translations. Requirements identified in this section are necessary + for the current applications to work correctly. + + Consider the following scenario in Figure 1. Say, NAT-xy is a NAT + device connecting hosts in private and external networks. Router-x + and Host-x are in the external network. Router-y and Host-y are in + the private network. The subnets in the external network are + routable from the private as well as the external domains. By + contrast, the subnets in the private network are only routable within + the private domain. When Host-y initiated a session to Host-x, let + us say that the NAT device mapped the endpoint on Host-y into Host-y' + in the external network. The following subsections describe the + processing of ICMP Error messages on the NAT device(NAT-xy) when the + NAT device receives an ICMP Error message in response to a packet + pertaining to this session. + + + + + + + + + + + + + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 10] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + Host-x + | + ---------------+------------------- + | + +-------------+ + | Router-x | + +-------------+ + External Network | + --------------------+--------+------------------- + | ^ + | | (Host-y', Host-x) + | | + +-------------+ + | NAT-xy | + +-------------+ + | + Private Network | + ----------------+------------+---------------- + | + +-------------+ + | Router-y | + +-------------+ + | + ----------------+-------+-------- + | ^ + | | (Host-y, Host-x) + | | + Host-y + + Figure 1. A Session from a Private Host Traversing a NAT Device + +4.2.1. ICMP Error Packet Received from the External Realm + + Say, a packet from Host-y to Host-x triggered an ICMP Error message + from one of Router-x or Host-x (both of which are in the external + domain). Such an ICMP Error packet will have one of Router-x or + Host-x as the source IP address and Host-y' as the destination IP + address as described in Figure 2 below. + + + + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 11] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + Host-x + | + ---------------+------------------- + | + +-------------+ + | Router-x | + +-------------+ + External Network | + --------------------+--------+------------------- + | + | | ICMP Error Packet to Host-y' + | v + +-------------+ + | NAT-xy | + +-------------+ + Private Network | + ----------------+------------+---------------- + | + +-------------+ + | Router-y | + +-------------+ + | + ----------------+-------+-------- + | + Host-y + + Figure 2. ICMP Error Packet Received from External Network + + When the NAT device receives the ICMP Error packet, the NAT device + uses the packet embedded within the ICMP Error message (i.e., the IP + packet from Host-y' to Host-x) to look up the NAT Session to which + the embedded packet belongs. If the NAT device does not have an + active mapping for the embedded packet, the NAT SHOULD silently drop + the ICMP Error packet. Otherwise, the NAT device MUST use the + matching NAT Session to translate the embedded packet; that is, + translate the source IP address of the embedded packet (e.g., Host-y' + -> Host-y) and transport headers. + + The ICMP Error payload may contain ICMP extension objects [ICMP-EXT]. + NATs are encouraged to support ICMP extension objects. At the time + of this writing, the authors are not aware of any standard ICMP + extension objects containing realm-specific information. + + The NAT device MUST also use the matching NAT Session to translate + the destination IP address in the outer IP header. In the outer + header, the source IP address will remain unchanged because the + originator of the ICMP Error message (Host-x or Router-x) is in an + external domain and is routable from the private domain. + + + +Srisuresh, et al. Best Current Practice [Page 12] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + REQ-4: If a NAT device receives an ICMP Error packet from an external + realm, and the NAT device does not have an active mapping for + the embedded payload, the NAT SHOULD silently drop the ICMP + Error packet. If the NAT has active mapping for the embedded + payload, then the NAT MUST do the following prior to + forwarding the packet, unless explicitly overridden by local + policy: + + a) Revert the IP and transport headers of the embedded IP + packet to their original form, using the matching mapping; + and + + b) Leave the ICMP Error type and code unchanged; and + + c) Modify the destination IP address of the outer IP header to + be the same as the source IP address of the embedded packet + after translation. + +4.2.2. ICMP Error Packet Received from the Private Realm + + Now, say, a packet from Host-x to Host-y triggered an ICMP Error + message from one of Router-y or Host-y (both of which are in the + private domain). Such an ICMP Error packet will have one of Router-y + or Host-y as the source IP address and Host-x as the destination IP + address as specified in Figure 3 below. + + + + + + + + + + + + + + + + + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 13] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + Host-x + | + ---------------+------------------- + | + +-------------+ + | Router-x | + +-------------+ + External Network | + --------------------+--------+------------------- + | + | + +-------------+ + | NAT-xy | + +-------------+ + | ^ + | | ICMP Error Packet to Host-x + Private Network | + ----------------+------------+---------------- + | + +-------------+ + | Router-y | + +-------------+ + | + ----------------+-------+-------- + | + Host-y + + Figure 3. ICMP Error Packet Received from Private Network + + When the NAT device receives the ICMP Error packet, the NAT device + MUST use the packet embedded within the ICMP Error message (i.e., the + IP packet from Host-x to Host-y) to look up the NAT Session to which + the embedded packet belongs. If the NAT device does not have an + active mapping for the embedded packet, the NAT SHOULD silently drop + the ICMP Error packet. Otherwise, the NAT device MUST use the + matching NAT Session to translate the embedded packet. + + The ICMP Error payload may contain ICMP extension objects [ICMP-EXT]. + NATs are encouraged to support ICMP extension objects. At the time + of this writing, the authors are not aware of any standard ICMP + extension objects containing realm-specific information. + + In the outer header, the destination IP address will remain + unchanged, as the IP address for Host-x is already in the external + domain. If the ICMP Error message is generated by Host-y, the NAT + device must simply use the NAT Session to translate the source IP + address Host-y to Host-y'. If the ICMP Error message is originated + by the intermediate node Router-y, translation of the source IP + + + +Srisuresh, et al. Best Current Practice [Page 14] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + address varies depending on whether the Basic NAT or NAPT function + [NAT-TRAD] is enforced by the NAT device. A NAT device enforcing the + Basic NAT function has a pool of public IP addresses and enforces + address mapping (which is different from the endpoint mapping + enforced by NAPT) when a private node initiates an outgoing session + via the NAT device. So, if the NAT device has active mapping for the + IP address of the intermediate node Router-y, the NAT device MUST + translate the source IP address of the ICMP Error packet with the + public IP address in the mapping. In all other cases, the NAT device + MUST simply use its own IP address in the external domain to + translate the source IP address. + + REQ-5: If a NAT device receives an ICMP Error packet from the private + realm, and the NAT does not have an active mapping for the + embedded payload, the NAT SHOULD silently drop the ICMP Error + packet. If the NAT has active mapping for the embedded + payload, then the NAT MUST do the following prior to + forwarding the packet, unless explicitly overridden by local + policy: + + a) Revert the IP and transport headers of the embedded IP + packet to their original form, using the matching mapping; + and + + b) Leave the ICMP Error type and code unchanged; and + + c) If the NAT enforces Basic NAT function ([NAT-TRAD]), and + the NAT has active mapping for the IP address that sent the + ICMP Error, translate the source IP address of the ICMP + Error packet with the public IP address in the mapping. In + all other cases, translate the source IP address of the + ICMP Error packet with its own public IP address. + +4.3. NAT Sessions Pertaining to ICMP Error Payload + + While processing an ICMP Error packet pertaining to an ICMP Query or + Query response message, a NAT device MUST NOT refresh or delete the + NAT Session that pertains to the embedded payload within the ICMP + Error packet. This is in spite of the fact that the NAT device uses + the NAT Session to translate the embedded payload. This ensures that + the NAT Session will not be modified if someone is able to spoof ICMP + Error messages for the session. [ICMP-ATK] lists a number of + potential ICMP attacks that may be attempted by malicious users on + the network. This requirement is necessary for current applications + to work correctly. + + + + + + +Srisuresh, et al. Best Current Practice [Page 15] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + REQ-6: While processing an ICMP Error packet pertaining to an ICMP + Query or Query response message, a NAT device MUST NOT refresh + or delete the NAT Session that pertains to the embedded + payload within the ICMP Error packet. + +5. Hairpinning Support for ICMP Packets + + [BEH-UDP] and [BEH-TCP] mandate support for hairpinning for UDP and + TCP sessions, respectively, on NAT devices. A NAT device needs to + support hairpinning for ICMP Query sessions as well. Specifically, + NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the traversal + of hairpinned ICMP Query sessions. Say, for example, individual + private hosts register their NAT assigned external IP address with a + rendezvous server. Other hosts that wish to initiate ICMP Query + sessions to the registered hosts might do so using the public address + registered with the rendezvous server. For this reason, Basic NAT + devices are required to support the traversal of hairpinned ICMP + Query sessions. This requirement is necessary for current + applications to work correctly. + + Packets belonging to any of the hairpinned sessions could, in turn, + trigger ICMP Error messages directed to the source of hairpinned IP + packets. Such hairpinned ICMP Error messages will traverse the NAT + devices en route. All NAT devices (i.e., Basic NAT as well as NAPT + devices) MUST support the traversal of hairpinned ICMP Error + messages. Specifically, the NAT device must translate not only the + embedded hairpinned packet, but also the outer IP header that is + hairpinned. This requirement is necessary for current applications + to work correctly. + + A hairpinned ICMP Error message is received from a node in a private + network. As such, the ICMP Error processing requirement specified in + Req-5 is applicable in its entirety in processing the ICMP Error + message. In addition, the NAT device MUST translate the destination + IP address of the outer IP header to be same as the source IP address + of the embedded IP packet after the translation. + + REQ-7: NAT devices enforcing Basic NAT [NAT-TRAD] MUST support the + traversal of hairpinned ICMP Query sessions. All NAT devices + (i.e., Basic NAT as well as NAPT devices) MUST support the + traversal of hairpinned ICMP Error messages: + + a) When forwarding a hairpinned ICMP Error message, the NAT + device MUST translate the destination IP address of the + outer IP header to be same as the source IP address of the + embedded IP packet after the translation. + + + + + +Srisuresh, et al. Best Current Practice [Page 16] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +6. Rejection of Outbound Flows Disallowed by NAT + + A NAT device typically permits all outbound sessions. However, a NAT + device may disallow some outbound sessions due to resource + constraints or administration considerations. For example, a NAT + device may not permit the first packet of a new outbound session if + the NAT device is out of resources (out of addresses or TCP/UDP + ports, or NAT Session resources) to set up a state for the session, + or, if the specific session is administratively restricted by the NAT + device. + + When a NAT device is unable to establish a NAT Session for a new + transport-layer (TCP, UDP, ICMP, etc.) flow due to resource + constraints or administrative restrictions, the NAT device SHOULD + send an ICMP destination unreachable message, with a code of 13 + (Communication administratively prohibited) to the sender, and drop + the original packet. This requirement is meant primarily for future + use. Current applications do not require this for them to work + correctly. The justification for using ICMP code 13 in the ICMP + Error message is as follows: Section 5.2.7.1 of [RFC1812] recommends + routers use ICMP code 13 (Communication administratively prohibited) + when they administratively filter packets. ICMP code 13 is a soft + error and is on par with other soft error codes generated in response + to transient events such as "network unreachable" (ICMP type=3, + code=0). + + Some NAT designers opt to never reject an outbound flow. When a NAT + runs short of resources, they prefer to steal a resource from an + existing NAT Session rather than reject the outbound flow. Such a + design choice may appear conformant to REQ-8 below. However, the + design choice is in violation of the spirit of both REQ-8 and REQ-2. + Such a design choice is strongly discouraged. + + REQ-8: When a NAT device is unable to establish a NAT Session for a + new transport-layer (TCP, UDP, ICMP, etc.) flow due to resource + constraints or administrative restrictions, the NAT device SHOULD + send an ICMP destination unreachable message, with a code of 13 + (Communication administratively prohibited) to the sender, and drop + the original packet. + +7. Conformance to RFC 1812 + + This document specifies NATs to have a behavior that is consistent + with the way routers handle ICMP messages, as specified in Section + 4.3 of [RFC1812]. However, since the publication of [RFC1812], some + of its requirements are no longer best current practices. Thus, the + following requirements are derived from [RFC1812] and apply to NATs + compliant with this specification: + + + +Srisuresh, et al. Best Current Practice [Page 17] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + REQ-9: A NAT device MAY implement a policy control that prevents ICMP + messages being generated toward certain interface(s). + Implementation of such a policy control overrides the MUSTs + and SHOULDs in REQ-10. + + REQ-10: Unless overridden by REQ-9's policy, a NAT device needs to + support ICMP messages as below, some conforming to Section + 4.3 of [RFC1812] and some superseding the requirements of + Section 4.3 of [RFC1812]: + + a. MUST support: + + 1. Destination Unreachable Message, as described in Section + 7.1 of this document. + + 2. Time Exceeded Message, as described in Section 7.2 of + this document. + + 3. Echo Request/Reply Messages, as described in REQ-1. + + b. MAY support: + + 1. Redirect Message, as described in Section 4.3.3.2 of + [RFC1812]. + + 2. Timestamp and Timestamp Reply Messages, as described in + Section 4.3.3.8 of [RFC1812]. + + 3. Source Route Options, as described in Section 7.3 of + this document. + + 4. Address Mask Request/Reply Message, as described in + Section 7.4 of this document. + + 5. Parameter Problem Message, as described in Section 7.5 + of this document. + + 6. Router Advertisement and Solicitations, as described in + Section 7.6 of this document. + + c. SHOULD NOT support: + + 1. Source Quench Message, as described in Section 4.3.3.3 + of [RFC1812]. + + 2. Information Request/reply, as described in Section + 4.3.3.7 of [RFC1812]. + + + + +Srisuresh, et al. Best Current Practice [Page 18] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + In addition, a NAT device is RECOMMENDED to conform to the + following implementation considerations: + + d. DS Field Usage, as described in Section 7.7 of this + document. + + e. When Not to Send ICMP Errors, as described in Section + 4.3.2.7 of [RFC1812]. + + f. Rate Limiting, as described in Section 4.3.2.8 of + [RFC1812]. + +7.1. IP Packet Fragmentation + + Many networking applications (which include TCP- as well as UDP-based + applications) depend on ICMP Error messages from the network to + perform end-to-end path MTU discovery [PMTU]. Once the path MTU is + discovered, an application that chooses to avoid fragmentation may do + so by originating IP packets that fit within the path MTU en route + and setting the DF (Don't Fragment) bit in the IP header, so the + intermediate nodes en route do not fragment the IP packets. The + following sub-sections discuss the need for NAT devices to honor the + DF bit in the IP header and be able to generate "Packet Too Big" ICMP + Error message when they cannot forward the IP packet without + fragmentation. Also discussed is the need to seamlessly forward ICMP + Error messages generated by other intermediate devices. + +7.1.1. Generating "Packet Too Big" ICMP Error Message + + When a router is unable to forward a datagram because it exceeds the + MTU of the next-hop network and its Don't Fragment (DF) bit is set, + the router is required by [RFC1812] to return an ICMP Destination + Unreachable message to the source of the datagram, with the code + indicating "fragmentation needed and DF set". Further, [PMTU] states + that the router MUST include the MTU of that next-hop network in the + low-order 16 bits of the ICMP header field that is labeled "unused" + in the ICMP specification [ICMP]. + + A NAT device MUST honor the DF bit in the IP header of the packets + that transit the device. The NAT device may not be able to forward + an IP packet without fragmentation if the MTU on the forwarding + interface of the NAT device is not adequate for the IP packet. If + the DF bit is set on a transit IP packet and the NAT device cannot + forward the packet without fragmentation, the NAT device MUST send a + "Packet Too Big" ICMP message (ICMP type 3, code 4) with the next-hop + MTU back to the sender and drop the original IP packet. The sender + will usually resend after taking the appropriate corrective action. + + + + +Srisuresh, et al. Best Current Practice [Page 19] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + If the DF bit is not set and the MTU on the forwarding interface of + the NAT device mandates fragmentation, the NAT device MUST fragment + the packet and forward the fragments [RFC1812]. + +7.1.2. Forwarding "Packet Too Big" ICMP Error Message + + This is the flip side of the argument for the above section. By + virtue of the address translation NAT performs, NAT may end up being + the recipient of "Packet Too Big" messages. + + When the NAT device is the recipient of a "Packet Too Big" ICMP + message from the network, the NAT device MUST forward the ICMP + message back to the intended recipient, pursuant to the previously + stated requirements (REQ-3, REQ-4, and REQ-5). + +7.2. Time Exceeded Message + + A NAT device MUST generate a "Time Exceeded" ICMP Error message when + it discards a packet due to an expired Time to Live (TTL) field. A + NAT device MAY have a per-interface option to disable origination of + these messages on that interface, but that option MUST default to + allowing the messages to be originated. + + When a NAT device conforms to the above requirement, it ensures that + legacy applications such as Traceroute [RFC1470], [MS-TRCRT], which + depend upon the "Time Exceeded" ICMP Error message, will continue to + operate even as NAT devices are en route. + +7.3. Source Route Options + + A NAT device MAY support modifying IP addresses in the source route + option so the IP addresses in the source route option are realm + relevant. If a NAT device does not support forwarding packets with + the source route option, the NAT device SHOULD NOT forward outbound + ICMP messages that contain the source route option in the outer or + inner IP header. This is because such messages could reveal private + IP addresses to the external realm. + +7.4. Address Mask Request/Reply Messages + + Section 4.3.3.9 of [RFC1812] says an IP router MUST implement support + for receiving ICMP Address Mask Request messages and responding with + ICMP Address Mask Reply messages. However, several years (more than + 13 years at the time of this document) have elapsed since the text in + RFC 1812 was written. In the intervening time, DHCP [DHCP] has + replaced the use of address mask request/reply. At the current time, + + + + + +Srisuresh, et al. Best Current Practice [Page 20] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + there is rarely any host that does not meet host requirements + [RFC1122] and needs a NAT device to support address mask + request/reply. + + For this reason, a NAT device is not required to support this ICMP + message. + + A NAT device MAY support address mask request/reply messages. + +7.5. Parameter Problem Message + + Section 4.3.3.5 of [RFC1812] says an IP router MUST generate a + Parameter Problem message for any error not specifically covered by + another ICMP message. However, this message is rarely used in + practice in networks where IPv4 NATs are deployed. + + For this reason, a NAT device is not required to support this ICMP + message. + + A NAT device MAY support parameter problem messages. + +7.6. Router Advertisement and Solicitations + + Section 4.3.3.10 of [RFC1812] says an IP router MUST support the + router part of the ICMP Router Discovery Protocol on all connected + networks on which the router supports either IP multicast or IP + broadcast addressing. However, this message is rarely used in + practice in networks where IPv4 NATs are deployed. + + For this reason, a NAT device is not required to support this ICMP + message. + + A NAT device MAY support Router Advertisement and Solicitations. + +7.7. DS Field Usage + + [RFC1812] refers to the Type of Service (TOS) octet in the IP header, + which contains the TOS and IP precedence fields. However, the TOS + and IP precedence fields are no longer in use today. [RFC2474] + renamed the TOS octet as the DS field and defined diffserv classes + within the DS field. + + When generating an ICMP message, a NAT device SHOULD copy the + diffserv class of the message that causes the sending of the ICMP + error message. A NAT device MAY allow configuration of the diffserv + class to be used for the different types of ICMP messages. + + + + + +Srisuresh, et al. Best Current Practice [Page 21] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +8. Non-QueryError ICMP Messages + + In the preceding sections, ICMP requirements were identified for NAT + devices, with a primary focus on ICMP Query and ICMP Error messages, + as defined in the Terminology Section (see Section 2). This document + provides no guidance on the handling of Non-QueryError ICMP messages + by the NAT devices. A NAT MAY drop or appropriately handle Non- + QueryError ICMP messages. + + REQ-11: A NAT MAY drop or appropriately handle Non-QueryError + ICMP messages. The semantics of Non-QueryError ICMP messages + is defined in Section 2. + +9. Summary of Requirements + + Below is a summary of all the requirements. + + REQ-1: Unless explicitly overridden by local policy, a NAT device + MUST permit ICMP Queries and their associated responses, when + the Query is initiated from a private host to the external + hosts. + + a) NAT mapping of ICMP Query Identifiers SHOULD be external + host independent. + + REQ-2: An ICMP Query session timer MUST NOT expire in less than 60 + seconds. + + a) It is RECOMMENDED that the ICMP Query session timer be made + configurable. + + REQ-3: When an ICMP Error packet is received, if the ICMP checksum + fails to validate, the NAT SHOULD silently drop the ICMP Error + packet. If the ICMP checksum is valid, do the following: + + a) If the IP checksum of the embedded packet fails to + validate, the NAT SHOULD silently drop the Error packet; + and + + b) If the embedded packet includes IP options, the NAT device + MUST traverse past the IP options to locate the start of + the transport header for the embedded packet; and + + c) The NAT device SHOULD NOT validate the transport checksum + of the embedded packet within an ICMP Error message, even + when it is possible to do so; and + + + + + +Srisuresh, et al. Best Current Practice [Page 22] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + d) If the ICMP Error payload contains ICMP extensions + [ICMP-EXT], the NAT device MUST exclude the optional zero- + padding and the ICMP extensions when evaluating transport + checksum for the embedded packet. + + REQ-4: If a NAT device receives an ICMP Error packet from an external + realm, and the NAT device does not have an active mapping for + the embedded payload, the NAT SHOULD silently drop the ICMP + Error packet. If the NAT has active mapping for the embedded + payload, then the NAT MUST do the following prior to + forwarding the packet, unless explicitly overridden by local + policy: + + a) Revert the IP and transport headers of the embedded IP + packet to their original form, using the matching mapping; + and + + b) Leave the ICMP Error type and code unchanged; and + + c) Modify the destination IP address of the outer IP header to + be same as the source IP address of the embedded packet + after translation. + + REQ-5: If a NAT device receives an ICMP Error packet from the private + realm, and the NAT does not have an active mapping for the + embedded payload, the NAT SHOULD silently drop the ICMP Error + packet. If the NAT has active mapping for the embedded + payload, then the NAT MUST do the following prior to + forwarding the packet, unless explicitly overridden by local + policy. + + a) Revert the IP and transport headers of the embedded IP + packet to their original form, using the matching mapping; + and + + b) Leave the ICMP Error type and code unchanged; and + + c) If the NAT enforces Basic NAT function [NAT-TRAD], and the + NAT has active mapping for the IP address that sent the + ICMP Error, translate the source IP address of the ICMP + Error packet with the public IP address in the mapping. In + all other cases, translate the source IP address of the + ICMP Error packet with its own public IP address. + + REQ-6: While processing an ICMP Error packet pertaining to an ICMP + Query or Query response message, a NAT device MUST NOT refresh + or delete the NAT Session that pertains to the embedded + payload within the ICMP Error packet. + + + +Srisuresh, et al. Best Current Practice [Page 23] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + REQ-7: NAT devices enforcing Basic NAT ([NAT-TRAD]) MUST support the + traversal of hairpinned ICMP Query sessions. All NAT devices + (i.e., Basic NAT as well as NAPT devices) MUST support the + traversal of hairpinned ICMP Error messages. + + a) When forwarding a hairpinned ICMP Error message, the NAT + device MUST translate the destination IP address of the + outer IP header to be same as the source IP address of the + embedded IP packet after the translation. + + REQ-8: When a NAT device is unable to establish a NAT Session for a + new transport-layer (TCP, UDP, ICMP, etc.) flow due to + resource constraints or administrative restrictions, the NAT + device SHOULD send an ICMP destination unreachable message, + with a code of 13 (Communication administratively prohibited) + to the sender, and drop the original packet. + + REQ-9: A NAT device MAY implement a policy control that prevents ICMP + messages being generated toward certain interface(s). + Implementation of such a policy control overrides the MUSTs + and SHOULDs in REQ-10. + + REQ-10: Unless overridden by REQ-9's policy, a NAT device needs to + support ICMP messages as below, some conforming to Section + 4.3 of [RFC1812] and some superseding the requirements of + Section 4.3 of [RFC1812]: + + a. MUST support: + + 1. Destination Unreachable Message, as described in Section + 7.1 of this document. + + 2. Time Exceeded Message, as described in Section 7.2 of + this document. + + 3. Echo Request/Reply Messages, as described in REQ-1. + + b. MAY support: + + 1. Redirect Message, as described in Section 4.3.3.2 of + [RFC1812]. + + 2. Timestamp and Timestamp Reply Messages, as described in + Section 4.3.3.8 of [RFC1812]. + + 3. Source Route Options, as described in Section 7.3 of + this document. + + + + +Srisuresh, et al. Best Current Practice [Page 24] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + 4. Address Mask Request/Reply Message, as described in + Section 7.4 of this document. + + 5. Parameter Problem Message, as described in Section 7.5 + of this document. + + 6. Router Advertisement and Solicitations, as described in + Section 7.6 of this document. + + c. SHOULD NOT support: + + 1. Source Quench Message, as described in Section 4.3.3.3 + of [RFC1812]. + + 2. Information Request/reply, as described in Section + 4.3.3.7 of [RFC1812]. + + In addition, a NAT device is RECOMMENDED to conform to the + following implementation considerations: + + d. DS Field Usage, as described in Section 7.7 of this + document. + + e. When Not to Send ICMP Errors, as described in Section + 4.3.2.7 of [RFC1812]. + + f. Rate Limiting, as described in Section 4.3.2.8 of + [RFC1812]. + + REQ-11: A NAT MAY drop or appropriately handle Non-QueryError ICMP + messages. The semantics of Non-QueryError ICMP messages is + defined in Section 2. + +10. Security Considerations + + This document does not introduce any new security concerns related to + ICMP message handling in the NAT devices. However, the requirements + in the document do mitigate some security concerns known to exist + with ICMP messages. + + [ICMP-ATK] lists a number of ICMP attacks that can be directed + against end host TCP stacks. For example, a rogue entity could + bombard the NAT device with a large number of ICMP Errors. If the + NAT device did not validate the legitimacy of the ICMP Error packets, + the ICMP Errors would be forwarded directly to the end nodes. End + hosts not capable of defending themselves against such bogus ICMP + Error attacks could be adversely impacted by such attacks. Req-3 + recommends validating the ICMP checksum and the IP checksum of the + + + +Srisuresh, et al. Best Current Practice [Page 25] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + embedded payload prior to forwarding. These checksum validations by + themselves do not protect end hosts from attacks. However, checksum + validation mitigates end hosts from malformed ICMP Error attacks. + Req-4 and Req-5 further mandate that when a NAT device does not find + a mapping selection for the embedded payload, the NAT should drop the + ICMP Error packets, without forwarding. + + A rogue source could also try to send bogus ICMP Error messages for + the active NAT sessions, with intent to destroy the sessions. Req-6 + averts such an attack by ensuring that an ICMP Error message does not + affect the state of a session on the NAT device. + + Req-8 recommends a NAT device sending an ICMP Error message when the + NAT device is unable to create a NAT session due to lack of + resources. Some administrators may choose not to have the NAT device + send an ICMP Error message, as doing so could confirm to a malicious + attacker that the attack has succeeded. For this reason, sending of + the specific ICMP Error message stated in REQ-8 is left to the + discretion of the NAT device administrator. + + Unfortunately, ICMP messages are sometimes blocked at network + boundaries due to local security policy. Thus, some of the + requirements in this document allow local policy to override the + recommendations of this document. Blocking such ICMP messages is + known to break some protocol features (most notably path MTU + Discovery) and some applications (e.g., ping, traceroute), and such + blocking is NOT RECOMMENDED. + +11. Acknowledgements + + The authors wish to thank Fernando Gont, Dan Wing, Carlos Pignataro, + Philip Matthews, and members of the BEHAVE working group for doing a + thorough review of early versions of the document and providing + valuable input and offering generous amounts of their time in shaping + the ICMP requirements. Their valuable feedback made this document a + better read. Dan Wing and Fernando Gont were a steady source of + encouragement. Fernando Gont spent many hours preparing slides and + presenting the document in an IETF meeting on behalf of the authors. + The authors wish to thank Carlos Pignataro and Dan Tappan, authors of + the [ICMP-EXT] document, for their feedback concerning ICMP + extensions. The authors wish to thank Philip Matthews for agreeing + to be a technical reviewer for the document. Lastly, the authors + highly appreciate the rigorous feedback from the IESG members. + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 26] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +12. References + +12.1. Normative References + + [BEH-UDP] Audet, F., Ed., and C. Jennings, "Network Address + Translation (NAT) Behavioral Requirements for Unicast + UDP", BCP 127, RFC 4787, January 2007. + + [ICMP] Postel, J., "Internet Control Message Protocol", STD 5, + RFC 792, September 1981. + + [ICMP-EXT] Bonica, R., Gan, D., Tappan, D., and C. Pignataro, + "Extended ICMP to Support Multi-Part Messages", RFC 4884, + April 2007. + + [NAT-TRAD] Srisuresh, P. and K. Egevang, "Traditional IP Network + Address Translator (Traditional NAT)", RFC 3022, January + 2001. + + [RFC793] Postel, J., "Transmission Control Protocol", STD 7, RFC + 793, September 1981. + + [RFC1812] Baker, F., Ed., "Requirements for IP Version 4 Routers", + RFC 1812, June 1995. + + [RFC2119] Bradner, S., "Key words for use in RFCs to Indicate + Requirement Levels", BCP 14, RFC 2119, March 1997. + +12.2. Informative References + + [BEH-APP] Ford, B., Srisuresh, P., and D. Kegel, "Application Design + Guidelines for Traversal through Network Address + Translators", Work in Progress, March 2007. + + [BEH-TCP] Guha, S., Ed., Biswas, K., Ford, B., Sivakumar, S., and P. + Srisuresh, "NAT Behavioral Requirements for TCP", BCP 142, + RFC 5382, October 2008. + + [DHCP] Droms, R., "Dynamic Host Configuration Protocol", RFC + 2131, March 1997. + + [ICE] Rosenberg, J., "Interactive Connectivity Establishment + (ICE): A Protocol for Network Address Translator (NAT) + Traversal for Offer/Answer Protocols", Work in Progress, + October 2007. + + [ICMP-ATK] Gont, F., "ICMP Attacks against TCP", Work in Progress, + October 2008. + + + +Srisuresh, et al. Best Current Practice [Page 27] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + + [MS-TRCRT] Microsoft Support, "How to use the Tracert command-line + utility to troubleshoot TCP/IP problems in Windows", + http://support.microsoft.com/kb/162326, October, 2006. + + [NAT-MIB] Rohit, R., Srisuresh, P., Raghunarayan, R., Pai, N., and + C. Wang, "Definitions of Managed Objects for Network + Address Translators (NAT)", RFC 4008, March 2005. + + [NAT-TERM] Srisuresh, P. and M. Holdrege, "IP Network Address + Translator (NAT) Terminology and Considerations", RFC + 2663, August 1999. + + [PMTU] Mogul, J. and S. Deering, "Path MTU discovery", RFC 1191, + November 1990. + + [RFC1122] Braden, R., Ed., "Requirements for Internet Hosts - + Communication Layers", STD 3, RFC 1122, October 1989. + + [RFC1256] Deering, S., Ed., "ICMP Router Discovery Messages", RFC + 1256, September 1991. + + [RFC1470] Enger, R. and J. Reynolds, "FYI on a Network Management + Tool Catalog: Tools for Monitoring and Debugging TCP/IP + Internets and Interconnected Devices", FYI 2, RFC 1470, + June 1993. + + [RFC2474] Nichols, K., Blake, S., Baker, F., and D. Black, + "Definition of the Differentiated Services Field (DS + Field) in the IPv4 and IPv6 Headers", RFC 2474, December + 1998. + + [RFC4065] Kempf, J., "Instructions for Seamoby and Experimental + Mobility Protocol IANA Allocations", RFC 4065, July 2005. + + [TCP-SOFT] Gont, F., "TCP's Reaction to Soft Errors", RFC 5461, + February 2009. + + [UNSAF] Daigle, L., Ed., and IAB, "IAB Considerations for + UNilateral Self-Address Fixing (UNSAF) Across Network + Address Translation", RFC 3424, November 2002. + + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 28] + +RFC 5508 NAT Behavioral Requirements for ICMP April 2009 + + +Authors' Addresses + + Pyda Srisuresh + Kazeon Systems, Inc. + 1161 San Antonio Rd. + Mountain View, CA 94043 + U.S.A. + + Phone: +1 408 836 4773 + EMail: srisuresh@yahoo.com + + + Bryan Ford + Max Planck Institute for Software Systems + Campus Building E1 4 + D-66123 Saarbruecken + Germany + + Phone: +49-681-9325657 + EMail: baford@mpi-sws.org + + + Senthil Sivakumar + Cisco Systems, Inc. + 7100-8 Kit Creek Road + PO Box 14987 + Research Triangle Park, NC 27709-4987 + U.S.A. + + Phone: +1 919 392 5158 + EMail: ssenthil@cisco.com + + + Saikat Guha + Cornell University + 331 Upson Hall + Ithaca, NY 14853 + U.S.A. + + Phone: +1 607 255 1008 + EMail: saikat@cs.cornell.edu + + + + + + + + + + +Srisuresh, et al. Best Current Practice [Page 29] + diff --git a/nat/src/icmp_handler/nf.rs b/nat/src/icmp_handler/nf.rs index c75170fe8b..7b7576bfe0 100644 --- a/nat/src/icmp_handler/nf.rs +++ b/nat/src/icmp_handler/nf.rs @@ -6,7 +6,7 @@ use flow_entry::flow_table::FlowTable; use net::buffer::PacketBufferMut; -use net::headers::TryIcmpAny; +use net::headers::{EmbeddedTransport, TryEmbeddedTransport, TryIcmpAny}; use net::icmp_any::IcmpAny; use net::icmp4::{Icmp4DestUnreachable, Icmp4Type}; use net::icmp6::Icmp6Type; @@ -59,6 +59,19 @@ fn is_icmp_unrecoverable(packet: &mut Packet) -> (boo } } +//= https://www.rfc-editor.org/rfc/rfc5508#section-4.3 +//# REQ-6: While processing an ICMP Error packet pertaining to an ICMP +//# Query or Query response message, a NAT device MUST NOT refresh +//# or delete the NAT Session that pertains to the embedded +//# payload within the ICMP Error packet. +fn embeds_icmp_query(packet: &Packet) -> bool { + match packet.try_embedded_transport() { + Some(EmbeddedTransport::Icmp4(icmp4)) => icmp4.is_query_message(), + Some(EmbeddedTransport::Icmp6(icmp6)) => icmp6.is_query_message(), + _ => false, + } +} + impl IcmpErrorHandler { fn handle_icmp_error_msg(&self, packet: &mut Packet) { let Some(icmp_error_packet) = IcmpErrorPacket::new(packet) else { @@ -164,9 +177,10 @@ impl IcmpErrorHandler { // if the problem is hardly recoverable. This expedites removing those flows, which would probably // be never hit again and, in case of masquerading, releases the allocated ports sooner. // This optimization is only applied if the `NatFlowStatus` is one-way. + let embeds_query = embeds_icmp_query(packet); let (unrecoverable, reason) = is_icmp_unrecoverable(packet); let reason = reason.unwrap_or("unspecified"); - if unrecoverable && status == NatFlowStatus::OneWay { + if unrecoverable && status == NatFlowStatus::OneWay && !embeds_query { debug!("Invalidating flows due to ICMP error (reason={reason} flow-status={status})"); flow.invalidate_pair(); } else { diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index d14cd5476b..5a01d594e5 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -2288,3 +2288,127 @@ async fn test_recheck_flow_when_allocator_is_kept() { tokio::time::sleep(Duration::from_secs(1)).await; assert_eq!(flow_table.active_len(), Some(2)); } + +fn icmp_echo_through( + pipeline: &mut DynPipeline, + src_vni: Vni, + src_ip: Ipv4Addr, + dst_ip: Ipv4Addr, + direction: IcmpEchoDirection, + identifier: u16, +) -> Packet { + let mut packet: Packet = + build_test_icmp4_echo(src_ip, dst_ip, identifier, direction).unwrap(); + packet.meta_mut().set_overlay(true); + packet.meta_mut().set_masquerade(true); + packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(src_vni)); + + let packets_out: Vec<_> = pipeline.process(std::iter::once(packet)).collect(); + packets_out.into_iter().next().unwrap() +} + +//= https://www.rfc-editor.org/rfc/rfc5508#section-4.3 +//= type=test +//# REQ-6: While processing an ICMP Error packet pertaining to an ICMP +//# Query or Query response message, a NAT device MUST NOT refresh +//# or delete the NAT Session that pertains to the embedded +//# payload within the ICMP Error packet. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn an_icmp_error_does_not_tear_down_the_query_session_it_reports_on() { + let (flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs()); + + let (host, target) = (addr_v4("1.1.2.3"), addr_v4("3.3.3.3")); + let identifier = 1337; + + let echo = icmp_echo_through( + &mut pipeline, + vni(100), + host, + target, + IcmpEchoDirection::Request, + identifier, + ); + let public = echo.try_ipv4().unwrap().source().inner(); + let translated_identifier = echo.try_icmp4().unwrap().identifier().unwrap(); + + let echo = icmp_echo_through( + &mut pipeline, + vni(100), + host, + target, + IcmpEchoDirection::Request, + identifier, + ); + let flow = echo + .meta() + .flow_info + .clone() + .expect("no flow for the Query"); + assert_eq!(flow_status(&echo), Some(FlowStatus::Active)); + + let (_, _, _, _, _, _, done_reason) = check_packet_icmp_error( + &mut pipeline, + vni(200), + vni(100), + addr_v4("1.2.2.18"), + public, + public, + target, + NextHeader::ICMP, + translated_identifier, + 0, + ); + assert_eq!(done_reason, None); + + assert!( + flow.is_active(), + "the Query session was torn down by the Error message reporting on it" + ); + let related = flow.related.as_ref().and_then(std::sync::Weak::upgrade); + assert!( + related.is_none_or(|related| related.is_active()), + "the Query session's reverse flow was torn down by the Error message" + ); + assert_eq!(flow_table.active_len(), Some(2)); +} + +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn an_icmp_error_about_a_tcp_flow_still_tears_it_down() { + let (flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs()); + + let mut syn = tcp_packet_to_masquerade(); + syn.try_tcp_mut().unwrap().set_syn(true); + let out = process_packet(&mut pipeline, syn); + assert!(!out.is_done()); + let (public, public_port) = translation(&out); + let target = out.try_ipv4().unwrap().destination(); + + let mut syn = tcp_packet_to_masquerade(); + syn.try_tcp_mut().unwrap().set_syn(true); + let out = process_packet(&mut pipeline, syn); + let flow = out.meta().flow_info.clone().expect("no flow for the SYN"); + assert!(flow.is_active()); + assert_eq!(nat_flow_status(&out), Some(NatFlowStatus::OneWay)); + + let (_, _, _, _, _, _, done_reason) = check_packet_icmp_error( + &mut pipeline, + vni(200), + vni(100), + addr_v4("1.2.2.18"), + public, + public, + target, + NextHeader::TCP, + public_port, + 80, + ); + assert_eq!(done_reason, None); + + assert!( + !flow.is_active(), + "an ICMP error about a one-way TCP flow no longer invalidates it" + ); + assert_eq!(flow_table.active_len(), Some(0)); +} diff --git a/scripts/spec-interlock.ts b/scripts/spec-interlock.ts index 3aeb65762e..23fee254e0 100755 --- a/scripts/spec-interlock.ts +++ b/scripts/spec-interlock.ts @@ -50,6 +50,20 @@ const ACCEPTED: Accepted[] = [ "half is caught, and correctly: a block with the first half full and the second free is " + "ordinary, so `ones == 128` is reachable there and `1u128 << 128` overflows.", })), + { + requirement: "https://www.rfc-editor.org/rfc/rfc5508#section-4.3", + mutant: + "nat/src/icmp_handler/nf.rs: replace embeds_icmp_query -> bool with true", + reason: + "Equivalent with respect to REQ-6, and only with respect to REQ-6. The requirement is " + + "one-sided -- it forbids deleting a session whose embedded payload is a Query, and asks " + + "nothing of any other payload -- so a NAT that deleted nothing would conform. A predicate " + + "forced to `true` is exactly that NAT. What it loses is the invalidation optimization, " + + "which is ours rather than the RFC's, and which the crate does test: " + + "`an_icmp_error_about_a_tcp_flow_still_tears_it_down` fails with the mutant applied by " + + "hand, the rest of the 213-test nat suite passes. Deliberately not cited as a REQ-6 test, " + + "because it checks the behaviour the requirement declines to constrain.", + }, ]; function stableName(mutant: string): string { From 1ad9a9d8bce66811090f4eea3a751d88d76135a6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 00:52:49 -0600 Subject: [PATCH 02/22] test(net): Generate IPv4 options, and stop asserting they cannot happen The `TypeGenerator` contract is that a generator eventually reaches every legal value; this one documented that it did not reach `Ipv4::options`, so no property in the workspace has ever seen a variable-length IPv4 header. `parse_back` is why it stayed that way: it sized its buffer to `MIN_LEN` and asserted the deparse wrote exactly that, which is the generator's limitation restated as a requirement. The whole workspace passes once both are fixed. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- net/src/ipv4/mod.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/net/src/ipv4/mod.rs b/net/src/ipv4/mod.rs index ca5f8520f4..0f4667d053 100644 --- a/net/src/ipv4/mod.rs +++ b/net/src/ipv4/mod.rs @@ -47,6 +47,13 @@ pub struct Ipv4LengthError { max: usize, } +#[derive(Debug, thiserror::Error, PartialEq, Eq, Clone, Copy)] +#[error("invalid IPv4 options length {len}: must be a multiple of 4 and at most 40 bytes")] +#[allow(missing_docs)] +pub struct Ipv4OptionsLenError { + len: usize, +} + impl Ipv4 { /// The minimum length of an IPv4 header (i.e., a header with no options) #[allow(clippy::unwrap_used)] // const-eval and trivially safe @@ -82,6 +89,14 @@ impl Ipv4 { self.0.options.as_slice() } + #[allow(missing_docs)] + pub fn set_options(&mut self, data: &[u8]) -> Result<&mut Self, Ipv4OptionsLenError> { + self.0 + .set_options(data) + .map_err(|_| Ipv4OptionsLenError { len: data.len() })?; + Ok(self) + } + // TODO: proper wrapper type for [`IpNumber`] (low priority) /// Get the next layer protocol which follows this header. #[must_use] @@ -488,6 +503,13 @@ mod contract { /// Generates an arbitrary [`Ipv4`] header with the [`NextHeader`] specified in `self`. fn generate(&self, u: &mut D) -> Option { let mut header = Ipv4(Ipv4Header::default()); + let option_words = u8::gen_bounded(u, Bound::Included(&0), Bound::Included(&10))?; + let mut options = [0u8; (Ipv4::MAX_LEN.get() - Ipv4::MIN_LEN.get()) as usize]; + let options = &mut options[..(option_words as usize) * 4]; + for byte in options.iter_mut() { + *byte = u.produce()?; + } + header.set_options(options).ok()?; header.set_source(u.produce()?); header.set_destination(Ipv4Addr::from(u.produce::()?)); header.set_next_header(self.0); @@ -520,7 +542,6 @@ mod contract { /// reach the set of all [`Ipv4`] (as should be true with any implementation of /// [`TypeGenerator`]). /// - /// Unfortunately, the current implementation does not cover [`Ipv4::options`]. fn generate(u: &mut D) -> Option { GenWithNextHeader(u.produce()?).generate(u) } @@ -539,13 +560,15 @@ mod test { #[test] fn parse_back() { bolero::check!().with_type().for_each(|header: &Ipv4| { - let mut buffer = [0u8; MIN_LEN_USIZE]; + let mut buffer = [0u8; MAX_LEN_USIZE]; let bytes_written = header .deparse(&mut buffer) .unwrap_or_else(|e| unreachable!("{e:?}")); - assert_eq!(bytes_written, Ipv4::MIN_LEN); + assert_eq!(bytes_written.get() as usize, header.header_len()); + assert!(bytes_written >= Ipv4::MIN_LEN && bytes_written <= Ipv4::MAX_LEN); let (parse_back, bytes_read) = Ipv4::parse(&buffer[..(bytes_written.get() as usize)]) .unwrap_or_else(|e| unreachable!("{e:?}")); + assert_eq!(header.options(), parse_back.options()); assert_eq!(header.source(), parse_back.source()); assert_eq!(header.destination(), parse_back.destination()); assert_eq!(header.protocol(), parse_back.protocol()); From 6f6eaca390b73634fe19bb883df6418bad91882e Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 01:00:32 -0600 Subject: [PATCH 03/22] test(nat): Pin the recoverable Destination Unreachable code `is_icmp_unrecoverable` decides whether an ICMP error costs the flow it reports on, and no test said anything about it: the whole function could be replaced by `(true, None)` or `(false, None)` and the suite passed. The distinction it draws is the one that matters for availability -- Fragmentation Needed is Path MTU Discovery, and the sender is about to retry the flow that tearing it down would strand. The fixture builder took the Destination Unreachable code as a constant, so the recoverable case could not be built at all. Ten survivors in this file become three, all of them IPv6. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/masquerade/test.rs | 88 +++++++++++++++++++++++++++++++++++- net/src/packet/test_utils.rs | 39 +++++++++++++++- 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index 5a01d594e5..1b18398141 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -27,12 +27,13 @@ use net::headers::TryTcpMut; use net::headers::{ EmbeddedTransport, TryEmbeddedTransport as _, TryIcmp4, TryInnerIpv4, TryIpv4, TryUdp, }; -use net::icmp4::Icmp4Type; use net::icmp4::TruncatedIcmp4; +use net::icmp4::{Icmp4DestUnreachable, Icmp4Type}; use net::ip::NextHeader; use net::packet::test_utils::build_test_tcp_ipv4_packet; use net::packet::test_utils::{ - IcmpEchoDirection, build_test_icmp4_destination_unreachable_packet, build_test_icmp4_echo, + IcmpEchoDirection, IcmpErrorAddrs, build_test_icmp4_destination_unreachable_packet, + build_test_icmp4_destination_unreachable_packet_with_code, build_test_icmp4_echo, build_test_udp_ipv4_frame, build_test_udp_ipv4_packet, }; use net::packet::{DoneReason, Packet, VpcDiscriminant}; @@ -1779,6 +1780,89 @@ async fn test_masquerade_tcp_reset() { assert_eq!(flow_table.active_len(), Some(0)); } +fn one_way_tcp_flow( + pipeline: &mut DynPipeline, +) -> (Arc, Ipv4Addr, u16, Ipv4Addr) { + let mut syn = tcp_packet_to_masquerade(); + syn.try_tcp_mut().unwrap().set_syn(true); + let out = process_packet(pipeline, syn); + assert!(!out.is_done()); + let (public, public_port) = translation(&out); + let target = out.try_ipv4().unwrap().destination(); + + let mut syn = tcp_packet_to_masquerade(); + syn.try_tcp_mut().unwrap().set_syn(true); + let out = process_packet(pipeline, syn); + let flow = out.meta().flow_info.clone().expect("no flow for the SYN"); + assert!(flow.is_active()); + assert_eq!(nat_flow_status(&out), Some(NatFlowStatus::OneWay)); + + (flow, public, public_port, target) +} + +fn icmp_unreachable_about_tcp( + pipeline: &mut DynPipeline, + unreachable: Icmp4DestUnreachable, + public: Ipv4Addr, + public_port: u16, + target: Ipv4Addr, +) -> Option { + let mut packet = build_test_icmp4_destination_unreachable_packet_with_code( + unreachable, + IcmpErrorAddrs { + outer_src: addr_v4("1.2.2.18"), + outer_dst: public, + inner_src: public, + inner_dst: target, + }, + NextHeader::TCP, + public_port, + 80, + ) + .unwrap(); + packet.meta_mut().set_overlay(true); + packet.meta_mut().set_masquerade(false); + packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(vni(200))); + + let out: Vec<_> = pipeline.process(std::iter::once(packet)).collect(); + out[0].get_done() +} + +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn path_mtu_discovery_does_not_tear_down_the_flow_that_triggered_it() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs()); + let (flow, public, public_port, target) = one_way_tcp_flow(&mut pipeline); + + let done = icmp_unreachable_about_tcp( + &mut pipeline, + Icmp4DestUnreachable::FragmentationNeeded { + next_hop_mtu: Some(1400.try_into().unwrap()), + }, + public, + public_port, + target, + ); + assert_eq!(done, None); + assert!( + flow.is_active(), + "Fragmentation Needed tore down the flow that is about to retry" + ); + + let done = icmp_unreachable_about_tcp( + &mut pipeline, + Icmp4DestUnreachable::Network, + public, + public_port, + target, + ); + assert_eq!(done, None); + assert!( + !flow.is_active(), + "Network Unreachable no longer tears the flow down" + ); +} + // Walk the client-initiated graceful-close states. #[tokio::test] #[cfg_attr(not(emulated), traced_test)] diff --git a/net/src/packet/test_utils.rs b/net/src/packet/test_utils.rs index a0e551e699..97fcf427d7 100644 --- a/net/src/packet/test_utils.rs +++ b/net/src/packet/test_utils.rs @@ -17,7 +17,7 @@ use crate::eth::Eth; use crate::eth::ethtype::EthType; use crate::eth::mac::{DestinationMac, Mac, SourceMac}; use crate::headers::{EmbeddedHeadersBuilder, EmbeddedTransport, HeadersBuilder, Net, Transport}; -use crate::icmp4::{Icmp4, TruncatedIcmp4}; +use crate::icmp4::{Icmp4, Icmp4DestUnreachable, TruncatedIcmp4}; use crate::ip::NextHeader; use crate::ipv4::Ipv4; use crate::ipv4::addr::UnicastIpv4Addr; @@ -320,6 +320,41 @@ pub fn build_test_icmp4_destination_unreachable_packet( inner_param_1: u16, inner_param_2: u16, ) -> Result, InvalidPacket> { + build_test_icmp4_destination_unreachable_packet_with_code( + Icmp4DestUnreachable::Network, + IcmpErrorAddrs { + outer_src: outer_src_ip, + outer_dst: outer_dst_ip, + inner_src: inner_src_ip, + inner_dst: inner_dst_ip, + }, + next_header, + inner_param_1, + inner_param_2, + ) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct IcmpErrorAddrs { + pub outer_src: Ipv4Addr, + pub outer_dst: Ipv4Addr, + pub inner_src: Ipv4Addr, + pub inner_dst: Ipv4Addr, +} + +pub fn build_test_icmp4_destination_unreachable_packet_with_code( + unreachable: Icmp4DestUnreachable, + addrs: IcmpErrorAddrs, + next_header: NextHeader, + inner_param_1: u16, + inner_param_2: u16, +) -> Result, InvalidPacket> { + let IcmpErrorAddrs { + outer_src: outer_src_ip, + outer_dst: outer_dst_ip, + inner_src: inner_src_ip, + inner_dst: inner_dst_ip, + } = addrs; let mut headers = HeadersBuilder::default(); // Ethernet @@ -365,7 +400,7 @@ pub fn build_test_icmp4_destination_unreachable_packet( // ICMP let icmp = Icmp4(Icmpv4Header::new(Icmpv4Type::DestinationUnreachable( - DestUnreachableHeader::Network, + DestUnreachableHeader::from(unreachable), ))); // Outer IPv4 From 83b2e704738726dfd895c85526372f6efff877de Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 01:02:40 -0600 Subject: [PATCH 04/22] test(nat): Measure the IPv6 side of masquerade instead of arguing it The pools, the allocator and the stage are generic over the address family and `build_pool66` mirrors `build_pool44` exactly, which is the argument that kept the IPv6 instantiation of all three untested: the nat crate had no IPv6 case at all. It translates and untranslates correctly. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/masquerade/test.rs | 97 +++++++++++++++++++++++++++++++++++++- net/src/ipv4/mod.rs | 4 +- 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index 1b18398141..a1579510c4 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -30,6 +30,7 @@ use net::headers::{ use net::icmp4::TruncatedIcmp4; use net::icmp4::{Icmp4DestUnreachable, Icmp4Type}; use net::ip::NextHeader; +use net::packet::test_utils::build_test_ipv6_packet_with_transport; use net::packet::test_utils::build_test_tcp_ipv4_packet; use net::packet::test_utils::{ IcmpEchoDirection, IcmpErrorAddrs, build_test_icmp4_destination_unreachable_packet, @@ -43,7 +44,7 @@ use net::vxlan::Vni; use net::{FlowKey, IpProtoKey, UdpProtoKey}; use pipeline::DynPipeline; use pipeline::NetworkFunction; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use std::str::FromStr; use std::time::Duration; use tracectl::get_trace_ctl; @@ -315,6 +316,29 @@ fn build_overlay_2vpcs() -> Overlay { } // identical to build_overlay_2vpcs() but masquerading with 4.4.0.0/16 +fn build_overlay_2vpcs_v6() -> Overlay { + let mut vpc_table = VpcTable::new(); + let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); + let _ = vpc_table.add(Vpc::new("VPC-2", "BBBBB", 200).expect("Failed to add VPC")); + + let expose121 = VpcExpose::empty() + .make_masquerade(None) + .unwrap() + .ip("2001:db8:1::/48".into()) + .as_range("2001:db8:2::/48".into()) + .unwrap(); + let expose211 = VpcExpose::empty().ip("2001:db8:3::/48".into()); + + let manifest12 = VpcManifest::new("VPC-1").exposing(expose121); + let manifest21 = VpcManifest::new("VPC-2").exposing(expose211); + let peering12 = VpcPeering::with_default_group("VPC-1--VPC-2", manifest12, manifest21); + + let mut peering_table = VpcPeeringTable::new(); + peering_table.add(peering12).expect("Failed to add peering"); + + Overlay::new(vpc_table, peering_table) +} + fn build_overlay_2vpcs_modified() -> Overlay { let mut vpc_table = VpcTable::new(); let _ = vpc_table.add(Vpc::new("VPC-1", "AAAAA", 100).expect("Failed to add VPC")); @@ -2496,3 +2520,74 @@ async fn an_icmp_error_about_a_tcp_flow_still_tears_it_down() { ); assert_eq!(flow_table.active_len(), Some(0)); } + +fn addr_v6(addr: &str) -> Ipv6Addr { + Ipv6Addr::from_str(addr).expect("Failed to create IPv6 address") +} + +fn tcp_v6_to_masquerade( + src: Ipv6Addr, + dst: Ipv6Addr, + sport: u16, + dport: u16, +) -> Packet { + let mut packet = + build_test_ipv6_packet_with_transport(64, Some(NextHeader::TCP)).expect("bad fixture"); + packet + .set_ip_source(IpAddr::V6(src).try_into().unwrap()) + .unwrap(); + packet.set_ip_destination(IpAddr::V6(dst)).unwrap(); + packet.set_source_port(sport.try_into().unwrap()).unwrap(); + packet + .set_destination_port(dport.try_into().unwrap()) + .unwrap(); + + let tcp = packet.try_tcp_mut().unwrap(); + tcp.set_syn(true); + tcp.set_ack(false); + tcp.set_fin(false); + tcp.set_rst(false); + + packet.meta_mut().set_overlay(true); + packet.meta_mut().src_vpcd = Some(vpcd(100)); + packet.meta_mut().set_masquerade(true); + packet +} + +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn masquerade_translates_ipv6() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs_v6()); + + let (host, target) = (addr_v6("2001:db8:1::1"), addr_v6("2001:db8:3::1")); + let public_range = "2001:db8:2::".parse::().unwrap(); + + let out = process_packet(&mut pipeline, tcp_v6_to_masquerade(host, target, 4321, 80)); + assert!( + !out.is_done(), + "the packet was dropped: {:?}", + out.get_done() + ); + + let IpAddr::V6(src) = out.ip_source().unwrap() else { + panic!("the packet came out IPv4"); + }; + let IpAddr::V6(dst) = out.ip_destination().unwrap() else { + panic!("the packet came out IPv4"); + }; + + assert_ne!(src, host, "the source was not translated"); + assert_eq!( + src.segments()[..3], + public_range.segments()[..3], + "the source was translated outside the configured public range" + ); + assert_eq!(dst, target, "the destination was translated"); + + let reply = build_reply(&out); + let back = process_packet(&mut pipeline, reply); + assert!(!back.is_done()); + assert_eq!(back.ip_destination().unwrap(), IpAddr::V6(host)); + assert_eq!(back.ip_source().unwrap(), IpAddr::V6(target)); + assert_eq!(nat_flow_status(&back), Some(NatFlowStatus::TwoWay)); +} diff --git a/net/src/ipv4/mod.rs b/net/src/ipv4/mod.rs index 0f4667d053..4115611205 100644 --- a/net/src/ipv4/mod.rs +++ b/net/src/ipv4/mod.rs @@ -91,8 +91,8 @@ impl Ipv4 { #[allow(missing_docs)] pub fn set_options(&mut self, data: &[u8]) -> Result<&mut Self, Ipv4OptionsLenError> { - self.0 - .set_options(data) + self.0.options = data + .try_into() .map_err(|_| Ipv4OptionsLenError { len: data.len() })?; Ok(self) } From a29aa040288877d12cf041dbf98cade5b129f9f9 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 01:23:12 -0600 Subject: [PATCH 05/22] test(nat): State what the allocator's port ranges mean Turning a usage bitmap into port ranges is the allocator's answer to "what is allocated", and 28 of the crate's surviving mutants were in it: the arithmetic that turns a bit offset into a port number, the join between a block's two halves, and the fold across blocks were all unstated. Nothing said a range had to be maximal, or that folding blocks in a different order gave the same answer -- which is the order the caller actually uses, since it iterates a `HashMap`. The two bounds in `merge_ranges` guard additions next to the edges of `u16`, and a property confined to low ports reads them as no-ops; drawing blocks from the whole port space is what makes them die. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/masquerade/apalloc/port_alloc.rs | 148 +++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/nat/src/masquerade/apalloc/port_alloc.rs b/nat/src/masquerade/apalloc/port_alloc.rs index 9a1ec74382..05ec0c65bb 100644 --- a/nat/src/masquerade/apalloc/port_alloc.rs +++ b/nat/src/masquerade/apalloc/port_alloc.rs @@ -975,6 +975,7 @@ mod tests { use super::*; use bolero::{Driver, TypeGenerator}; use lpm::prefix::PortRange; + use std::collections::BTreeMap; use std::net::Ipv4Addr; // set_bitmap_value(), through the two operations built on it @@ -1293,6 +1294,153 @@ mod tests { assert!(!port_is_used(&bitmap, 128)); } + fn assert_covers_exactly( + ranges: &BTreeSet, + covered: impl Fn(u16) -> bool, + span: u16, + ) { + let mut times = vec![0usize; usize::from(span)]; + for range in ranges { + assert!(range.end() < span, "range past the space: {range:?}"); + for port in range.start()..=range.end() { + times[usize::from(port)] += 1; + } + } + for port in 0..span { + let times = times[usize::from(port)]; + assert_eq!( + times, + usize::from(covered(port)), + "port {port} is covered {times} times: {ranges:?}" + ); + } + } + + fn assert_maximal_and_ordered(ranges: &BTreeSet) { + let mut previous_end: Option = None; + for range in ranges { + assert!(range.start() <= range.end(), "inverted range: {range:?}"); + if let Some(previous_end) = previous_end { + assert!( + range.start() > previous_end + 1, + "adjacent ranges were not merged: {ranges:?}" + ); + } + previous_end = Some(range.end()); + } + } + + #[test] + fn a_bitmap_half_reports_exactly_the_ports_its_bits_mark() { + bolero::check!() + .with_type() + .for_each(|&(bitmap, second_half): &(u128, bool)| { + let base: u16 = if second_half { 128 } else { 0 }; + let ranges = collect_ranges_from_u128_bitmap(bitmap, base); + assert_maximal_and_ordered(&ranges); + assert_covers_exactly( + &ranges, + |port| { + port >= base && port < base + 128 && bitmap & (1u128 << (port - base)) != 0 + }, + 256, + ); + }); + } + + #[test] + fn a_block_reports_exactly_the_ports_its_bitmap_marks() { + bolero::check!() + .with_type() + .for_each(|&(first_half, second_half): &(u128, u128)| { + let bitmap = Bitmap256 { + first_half, + second_half, + }; + let ranges = bitmap.allocated_port_ranges(); + assert_maximal_and_ordered(&ranges); + assert_covers_exactly( + &ranges, + |port| match u8::try_from(port) { + Ok(offset) => port_is_used(&bitmap, offset), + Err(_) => false, + }, + 512, + ); + }); + } + + #[test] + fn folding_blocks_in_any_order_describes_the_same_ports() { + bolero::check!() + .with_type() + .for_each(|blocks: &[(u8, u128, u128); 4]| { + let mut bitmaps = BTreeMap::new(); + let mut order = Vec::new(); + for &(index, first_half, second_half) in blocks { + if bitmaps + .insert( + index, + Bitmap256 { + first_half, + second_half, + }, + ) + .is_none() + { + order.push(index); + } + } + + let mut folded = BTreeSet::new(); + for index in &order { + let base = u16::from(*index) * 256; + let bitmap = &bitmaps[index]; + let shifted: BTreeSet = bitmap + .allocated_port_ranges() + .iter() + .map(|range| { + PortRange::new(range.start() + base, range.end() + base) + .unwrap_or_else(|_| unreachable!()) + }) + .collect(); + merge_ranges(&mut folded, shifted); + } + + assert_maximal_and_ordered(&folded); + + for range in &folded { + for block in (range.start() / 256)..=(range.end() / 256) { + assert!( + bitmaps.contains_key( + &u8::try_from(block).unwrap_or_else(|_| unreachable!()) + ), + "{range:?} covers block {block}, which was not supplied" + ); + } + } + + let mut times = BTreeMap::::new(); + for range in &folded { + for port in range.start()..=range.end() { + *times.entry(port).or_default() += 1; + } + } + for (index, bitmap) in &bitmaps { + let base = u16::from(*index) * 256; + for offset in 0..=u8::MAX { + let port = base + u16::from(offset); + let seen = times.get(&port).copied().unwrap_or(0); + assert_eq!( + seen, + usize::from(port_is_used(bitmap, offset)), + "port {port} is covered {seen} times: {folded:?}" + ); + } + } + }); + } + #[test] fn port_zero_is_reserved_in_the_first_block_alone() { assert!(port_is_used(&Bitmap256::for_block(0, &[], true), 0)); From a1a2bb93b199bad55b702f205796867ad9254b71 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 01:33:47 -0600 Subject: [PATCH 06/22] test(nat): Reach the ICMPv6 side of the ICMP error handler The RFC 5508 REQ-6 citation was decorative on IPv6: the arm of the predicate that recognises an embedded ICMPv6 Query could be deleted outright and the suite passed, because no ICMPv6 fixture existed. The interlock reported this as `unreached` rather than as a survivor, which is what said the fix was to change what the test feeds. Path MTU Discovery needed its own IPv6 test rather than a parameter: v4 spells the recoverable case as a Destination Unreachable code and v6 as a separate type, so one test cannot reach both arms. REQ-6 now holds under `just spec-interlock`. What survives in this file is the Packet Too Big guard, which decides nothing the fallback would not; the reason is recorded next to it. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/masquerade/test.rs | 159 ++++++++++++++++++++++++++++++++++- net/src/packet/test_utils.rs | 135 ++++++++++++++++++++++++++++- 2 files changed, 290 insertions(+), 4 deletions(-) diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index a1579510c4..4124198538 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -23,19 +23,22 @@ use net::buffer::{PacketBufferMut, TestBuffer}; use net::eth::mac::Mac; use net::flows::flow_info_item::ExtractRef; use net::flows::{FlowInfo, FlowStatus}; -use net::headers::TryTcpMut; use net::headers::{ EmbeddedTransport, TryEmbeddedTransport as _, TryIcmp4, TryInnerIpv4, TryIpv4, TryUdp, }; +use net::headers::{TryIcmp6, TryTcpMut}; use net::icmp4::TruncatedIcmp4; use net::icmp4::{Icmp4DestUnreachable, Icmp4Type}; +use net::icmp6::{Icmp6DestUnreachable, Icmp6PacketTooBig, Icmp6Type}; use net::ip::NextHeader; use net::packet::test_utils::build_test_ipv6_packet_with_transport; use net::packet::test_utils::build_test_tcp_ipv4_packet; use net::packet::test_utils::{ - IcmpEchoDirection, IcmpErrorAddrs, build_test_icmp4_destination_unreachable_packet, + Icmp6ErrorAddrs, IcmpEchoDirection, IcmpErrorAddrs, + build_test_icmp4_destination_unreachable_packet, build_test_icmp4_destination_unreachable_packet_with_code, build_test_icmp4_echo, - build_test_udp_ipv4_frame, build_test_udp_ipv4_packet, + build_test_icmp6_echo, build_test_icmp6_error_packet, build_test_udp_ipv4_frame, + build_test_udp_ipv4_packet, }; use net::packet::{DoneReason, Packet, VpcDiscriminant}; use net::tcp::TruncatedTcp; @@ -2591,3 +2594,153 @@ async fn masquerade_translates_ipv6() { assert_eq!(back.ip_source().unwrap(), IpAddr::V6(target)); assert_eq!(nat_flow_status(&back), Some(NatFlowStatus::TwoWay)); } + +fn icmp6_echo_through( + pipeline: &mut DynPipeline, + src_vni: Vni, + src_ip: Ipv6Addr, + dst_ip: Ipv6Addr, + identifier: u16, +) -> Packet { + let mut packet: Packet = + build_test_icmp6_echo(src_ip, dst_ip, identifier, IcmpEchoDirection::Request).unwrap(); + packet.meta_mut().set_overlay(true); + packet.meta_mut().set_masquerade(true); + packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(src_vni)); + pipeline + .process(std::iter::once(packet)) + .next() + .unwrap_or_else(|| unreachable!()) +} + +fn icmp6_error_through( + pipeline: &mut DynPipeline, + src_vni: Vni, + icmp_type: Icmp6Type, + addrs: Icmp6ErrorAddrs, + next_header: NextHeader, + inner_param_1: u16, + inner_param_2: u16, +) -> Packet { + let mut packet = + build_test_icmp6_error_packet(icmp_type, addrs, next_header, inner_param_1, inner_param_2) + .unwrap(); + packet.meta_mut().set_overlay(true); + packet.meta_mut().set_masquerade(false); + packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(src_vni)); + pipeline + .process(std::iter::once(packet)) + .next() + .unwrap_or_else(|| unreachable!()) +} + +//= https://www.rfc-editor.org/rfc/rfc5508#section-4.3 +//= type=test +//# REQ-6: While processing an ICMP Error packet pertaining to an ICMP +//# Query or Query response message, a NAT device MUST NOT refresh +//# or delete the NAT Session that pertains to the embedded +//# payload within the ICMP Error packet. +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn an_icmp6_error_does_not_tear_down_the_query_session_it_reports_on() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs_v6()); + + let (host, target) = (addr_v6("2001:db8:1::1"), addr_v6("2001:db8:3::1")); + let identifier = 1337; + + let echo = icmp6_echo_through(&mut pipeline, vni(100), host, target, identifier); + assert!( + !echo.is_done(), + "the echo was dropped: {:?}", + echo.get_done() + ); + let IpAddr::V6(public) = echo.ip_source().unwrap() else { + panic!("the echo came out IPv4"); + }; + let translated_identifier = echo.try_icmp6().unwrap().identifier().unwrap(); + + let echo = icmp6_echo_through(&mut pipeline, vni(100), host, target, identifier); + let flow = echo + .meta() + .flow_info + .clone() + .expect("no flow for the Query"); + assert!(flow.is_active()); + + let out = icmp6_error_through( + &mut pipeline, + vni(200), + Icmp6Type::DestUnreachable(Icmp6DestUnreachable::Address), + Icmp6ErrorAddrs { + outer_src: addr_v6("2001:db8:3::fe"), + outer_dst: public, + inner_src: public, + inner_dst: target, + }, + NextHeader::ICMP6, + translated_identifier, + 0, + ); + assert_eq!(out.get_done(), None, "the error was dropped"); + + assert!( + flow.is_active(), + "the Query session was torn down by the Error message reporting on it" + ); +} + +#[tokio::test] +#[cfg_attr(not(emulated), traced_test)] +async fn ipv6_path_mtu_discovery_does_not_tear_down_the_flow_that_triggered_it() { + let (_flow_table, mut pipeline, _allocw) = test_setup(1, &build_overlay_2vpcs_v6()); + + let (host, target) = (addr_v6("2001:db8:1::1"), addr_v6("2001:db8:3::1")); + let out = process_packet(&mut pipeline, tcp_v6_to_masquerade(host, target, 4321, 80)); + assert!(!out.is_done()); + let IpAddr::V6(public) = out.ip_source().unwrap() else { + panic!("the packet came out IPv4"); + }; + let public_port: u16 = out.transport_src_port().unwrap().into(); + + let out = process_packet(&mut pipeline, tcp_v6_to_masquerade(host, target, 4321, 80)); + let flow = out.meta().flow_info.clone().expect("no flow for the SYN"); + assert!(flow.is_active()); + assert_eq!(nat_flow_status(&out), Some(NatFlowStatus::OneWay)); + + let addrs = Icmp6ErrorAddrs { + outer_src: addr_v6("2001:db8:3::fe"), + outer_dst: public, + inner_src: public, + inner_dst: target, + }; + + let out = icmp6_error_through( + &mut pipeline, + vni(200), + Icmp6Type::PacketTooBig(Icmp6PacketTooBig::new(1400).unwrap()), + addrs, + NextHeader::TCP, + public_port, + 80, + ); + assert_eq!(out.get_done(), None); + assert!( + flow.is_active(), + "Packet Too Big tore down the flow that is about to retry" + ); + + let out = icmp6_error_through( + &mut pipeline, + vni(200), + Icmp6Type::DestUnreachable(Icmp6DestUnreachable::Address), + addrs, + NextHeader::TCP, + public_port, + 80, + ); + assert_eq!(out.get_done(), None); + assert!( + !flow.is_active(), + "Address Unreachable no longer tears the flow down" + ); +} diff --git a/net/src/packet/test_utils.rs b/net/src/packet/test_utils.rs index 97fcf427d7..1b53f61f87 100644 --- a/net/src/packet/test_utils.rs +++ b/net/src/packet/test_utils.rs @@ -18,6 +18,7 @@ use crate::eth::ethtype::EthType; use crate::eth::mac::{DestinationMac, Mac, SourceMac}; use crate::headers::{EmbeddedHeadersBuilder, EmbeddedTransport, HeadersBuilder, Net, Transport}; use crate::icmp4::{Icmp4, Icmp4DestUnreachable, TruncatedIcmp4}; +use crate::icmp6::{Icmp6, Icmp6Type, TruncatedIcmp6}; use crate::ip::NextHeader; use crate::ipv4::Ipv4; use crate::ipv4::addr::UnicastIpv4Addr; @@ -30,7 +31,7 @@ use crate::tcp::{Tcp, TcpChecksumPayload, TruncatedTcp}; use crate::udp::port::UdpPort; use crate::udp::{TruncatedUdp, Udp, UdpChecksum, UdpChecksumPayload, UdpEncap}; use etherparse::icmpv4::DestUnreachableHeader; -use etherparse::{IcmpEchoHeader, Icmpv4Header, Icmpv4Type}; +use etherparse::{IcmpEchoHeader, Icmpv4Header, Icmpv4Type, Icmpv6Header, Icmpv6Type}; use std::default::Default; use std::net::{Ipv4Addr, Ipv6Addr}; use std::str::FromStr; @@ -642,3 +643,135 @@ pub fn build_test_vxlan_ipv6_packet_with_outer_qos( Packet::new(TestBuffer::from_raw_data(&data)) } + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Icmp6ErrorAddrs { + pub outer_src: Ipv6Addr, + pub outer_dst: Ipv6Addr, + pub inner_src: Ipv6Addr, + pub inner_dst: Ipv6Addr, +} + +#[must_use] +pub fn build_test_icmp6_error_packet( + icmp_type: Icmp6Type, + addrs: Icmp6ErrorAddrs, + next_header: NextHeader, + inner_param_1: u16, + inner_param_2: u16, +) -> Result, InvalidPacket> { + let mut headers = HeadersBuilder::default(); + headers.eth(Some(make_default_for_eth(EthType::IPV6))); + + let mut inner_transport = match next_header { + NextHeader::TCP => EmbeddedTransport::Tcp(TruncatedTcp::FullHeader(Tcp::new( + inner_param_1.try_into().unwrap(), + inner_param_2.try_into().unwrap(), + ))), + NextHeader::UDP => EmbeddedTransport::Udp(TruncatedUdp::FullHeader(Udp::new( + inner_param_1.try_into().unwrap(), + inner_param_2.try_into().unwrap(), + ))), + NextHeader::ICMP6 => EmbeddedTransport::Icmp6(TruncatedIcmp6::FullHeader(Icmp6( + Icmpv6Header::new(Icmpv6Type::EchoRequest(IcmpEchoHeader { + id: inner_param_1, + seq: inner_param_2, + })), + ))), + _ => panic!("Unsupported next header: {next_header:?}"), + }; + + let mut inner_ipv6 = Ipv6::default(); + inner_ipv6.set_source(UnicastIpv6Addr::new(addrs.inner_src).unwrap()); + inner_ipv6.set_destination(addrs.inner_dst); + inner_ipv6.set_hop_limit(4); + inner_ipv6.set_next_header(next_header); + inner_ipv6.set_payload_length(inner_transport.size().get()); + let inner_net = Net::Ipv6(inner_ipv6); + + let described = format!("{icmp_type:?}"); + let icmp = Icmp6(Icmpv6Header::new(icmp_type.into())); + assert!( + icmp.is_error_message(), + "not an ICMPv6 error message: {described}" + ); + + let mut outer_ipv6 = Ipv6::default(); + outer_ipv6.set_source(UnicastIpv6Addr::new(addrs.outer_src).unwrap()); + outer_ipv6.set_destination(addrs.outer_dst); + outer_ipv6.set_hop_limit(8); + outer_ipv6.set_next_header(NextHeader::ICMP6); + outer_ipv6.set_payload_length( + icmp.size().get() + inner_net.size().get() + inner_transport.size().get(), + ); + let outer_net = Net::Ipv6(outer_ipv6); + + match &mut inner_transport { + EmbeddedTransport::Tcp(TruncatedTcp::FullHeader(tcp)) => { + tcp.update_checksum(&TcpChecksumPayload::new(&inner_net, &[])) + .unwrap(); + } + EmbeddedTransport::Udp(TruncatedUdp::FullHeader(udp)) => { + udp.update_checksum(&UdpChecksumPayload::new(&inner_net, &[])) + .unwrap(); + } + _ => {} + } + + let mut embedded_headers = EmbeddedHeadersBuilder::default(); + embedded_headers.net(Some(inner_net)); + embedded_headers.transport(Some(inner_transport)); + let embedded_headers = embedded_headers.build().unwrap(); + + let mut icmp_transport = Transport::Icmp6(icmp); + icmp_transport.update_checksum(&outer_net, Some(&embedded_headers), []); + + headers.net(Some(outer_net)); + headers.transport(Some(icmp_transport)); + headers.embedded_ip(Some(embedded_headers)); + let headers = headers.build().unwrap(); + + let data = vec![0u8; headers.size().get() as usize]; + let mut buffer = TestBuffer::from_raw_data(&data); + headers.deparse(buffer.as_mut()).unwrap(); + Packet::new(buffer) +} + +#[must_use] +pub fn build_test_icmp6_echo( + src_ip: Ipv6Addr, + dst_ip: Ipv6Addr, + identifier: u16, + direction: IcmpEchoDirection, +) -> Result, InvalidPacket> { + let mut headers = HeadersBuilder::default(); + headers.eth(Some(make_default_for_eth(EthType::IPV6))); + + let echo_header = IcmpEchoHeader { + id: identifier, + seq: 0, + }; + let icmp = Icmp6(Icmpv6Header::new(match direction { + IcmpEchoDirection::Request => Icmpv6Type::EchoRequest(echo_header), + IcmpEchoDirection::Reply => Icmpv6Type::EchoReply(echo_header), + })); + + let mut ipv6 = Ipv6::default(); + ipv6.set_source(UnicastIpv6Addr::new(src_ip).unwrap()); + ipv6.set_destination(dst_ip); + ipv6.set_hop_limit(8); + ipv6.set_next_header(NextHeader::ICMP6); + ipv6.set_payload_length(icmp.size().get()); + let net = Net::Ipv6(ipv6); + + let mut icmp_transport = Transport::Icmp6(icmp); + icmp_transport.update_checksum(&net, None, []); + + headers.net(Some(net)); + headers.transport(Some(icmp_transport)); + let headers = headers.build().unwrap(); + + let mut buffer: TestBuffer = TestBuffer::new(); + headers.deparse(buffer.as_mut()).unwrap(); + Packet::new(buffer) +} From ade85149cd175ab10481e9416ea12c9cb0b65668 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 01:45:12 -0600 Subject: [PATCH 07/22] test(routing): State that ECMP spreads, and how a FibEntry classifies itself Three things the forwarding path decides that nothing asserted. Nothing said ECMP spreads. The entry selector could be replaced by "always entry zero" and the suite passed -- a router forwarding every flow down one of its paths, which looks healthy until a link saturates. Stability is asserted alongside it, because a flow whose packets take different paths reorders itself and splits in two for anything downstream that keys on the path. `is_iplocal` and `is_vxlan_with_vni` are read on the forwarding path and had no test at all. Both are stated against a second reading of the same fact rather than against a restatement of their bodies. `> 1` in the selector is equivalent to `>= 1` and the reason is recorded next to it; `== 1` and `< 1` are the ones that matter, and now die. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- routing/src/fib/fibobjects.rs | 43 +++++++++++++++++++++++++++++++- routing/src/fib/test.rs | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/routing/src/fib/fibobjects.rs b/routing/src/fib/fibobjects.rs index 3afde62a45..fdceabd4dd 100644 --- a/routing/src/fib/fibobjects.rs +++ b/routing/src/fib/fibobjects.rs @@ -295,7 +295,7 @@ mod squash_properties { } #[derive(Debug, Clone, Copy, Default)] - struct Entry; + pub(super) struct Entry; impl ValueGenerator for Entry { type Output = FibEntry; @@ -407,3 +407,44 @@ mod squash_properties { }); } } + +#[cfg(test)] +mod classification_properties { + use super::squash_properties::Entry; + use super::*; + + #[test] + fn an_entry_is_ip_local_exactly_when_delivering_locally_is_all_it_does() { + bolero::check!() + .with_generator(Entry) + .cloned() + .for_each(|entry: FibEntry| { + let locals = entry + .iter() + .filter(|inst| matches!(inst, PktInstruction::Local(_))) + .count(); + assert_eq!( + entry.is_iplocal(), + locals == 1 && entry.iter().count() == 1, + "for {entry:?}" + ); + }); + } + + #[test] + fn asking_about_one_vni_agrees_with_asking_which_vni() { + bolero::check!() + .with_generator(Entry) + .cloned() + .for_each(|entry: FibEntry| { + for raw in 1..=4u32 { + let vni = Vni::new_checked(raw).unwrap_or_else(|_| unreachable!()); + assert_eq!( + entry.is_vxlan_with_vni(vni), + entry.is_vxlan() == Some(vni), + "vni {raw} for {entry:?}" + ); + } + }); + } +} diff --git a/routing/src/fib/test.rs b/routing/src/fib/test.rs index f981804db8..99a0562ba9 100644 --- a/routing/src/fib/test.rs +++ b/routing/src/fib/test.rs @@ -399,6 +399,52 @@ mod tests { } // Tests fib reader utilities returning guards + #[test] + fn ecmp_uses_the_whole_group_and_picks_the_same_entry_for_a_packet() { + let (mut fibw, fibr) = FibWriter::new(0); + + let prefix = Prefix::from("192.168.1.0/24"); + let nhkey = NhopKey::with_address(&IpAddr::from_str("7.0.0.1").unwrap()); + let entries: Vec = (1..=5) + .map(|i| build_fib_entry_egress(i, &format!("10.0.{i}.1"))) + .collect(); + let fibgroup = build_fibgroup(&entries); + fibw.register_fibgroup(&nhkey, &fibgroup, false); + fibw.add_fibroute(prefix, vec![nhkey.clone()], false); + fibw.publish(); + + let mut chosen: HashMap = HashMap::new(); + for port in 1024u16..1224 { + let mut packet = test_packet(); + packet + .set_udp_destination_port(UdpPort::new_checked(port).unwrap()) + .unwrap(); + let (matched, entry) = fibr.lpm_entry_prefix(&packet).unwrap(); + assert_eq!(matched, prefix); + assert!( + entries.contains(&entry), + "the selected entry is not in the group" + ); + chosen.insert(port, (*entry).clone()); + } + + let distinct: HashSet<&FibEntry> = chosen.values().collect(); + assert!( + distinct.len() > 1, + "every one of 200 flows took the same one of {} paths", + entries.len() + ); + + for (port, expected) in &chosen { + let mut packet = test_packet(); + packet + .set_udp_destination_port(UdpPort::new_checked(*port).unwrap()) + .unwrap(); + let (_, entry) = fibr.lpm_entry_prefix(&packet).unwrap(); + assert_eq!(&*entry, expected, "the entry chosen for port {port} moved"); + } + } + #[test] fn test_fib_guards() { // create fib From e4e1efaedb2b6b60b3a4483d0d1b2b72eb89e97c Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 02:17:51 -0600 Subject: [PATCH 08/22] test(net): Say which checksums decide an ICMP error, and which does not RFC 5508 REQ-3 and its (a) and (c) clauses. The code already did the right thing and the comment already named the RFC; what was missing was a test that could tell a validator checking too much from one checking too little. The existing test breaks every checksum at once, so it cannot. Both sub-clauses need the ICMP checksum recomputed after the field under test is broken, because it covers them. That is not a trick to make the test pass: without it the outer check fires first and neither clause is ever reached, and with it the packet is the one a real reporter produces. Cited in `net` rather than on the existing `nat` test, because the interlock declines a citation whose implementation and test are in different crates. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- .duvet/snapshot.txt | 18 +++--- net/src/packet/icmp_err.rs | 120 +++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 9 deletions(-) diff --git a/.duvet/snapshot.txt b/.duvet/snapshot.txt index 763eb05945..40cacb3672 100644 --- a/.duvet/snapshot.txt +++ b/.duvet/snapshot.txt @@ -405,18 +405,18 @@ SPECIFICATION: https://www.rfc-editor.org/rfc/rfc5508 TEXT[!MUST]: [ICMP-EXT], the NAT device MUST exclude the optional zero-padding and TEXT[!MUST]: the ICMP extensions when evaluating transport checksum for the TEXT[!MUST]: embedded packet. - TEXT[!SHOULD]: REQ-3: When an ICMP Error packet is received, if the ICMP checksum - TEXT[!SHOULD]: fails to validate, the NAT SHOULD silently drop the ICMP Error - TEXT[!SHOULD]: packet. - TEXT[!SHOULD]: a) If the IP checksum of the embedded packet fails to - TEXT[!SHOULD]: validate, the NAT SHOULD silently drop the Error packet; - TEXT[!SHOULD]: and + TEXT[!SHOULD,implementation,test]: REQ-3: When an ICMP Error packet is received, if the ICMP checksum + TEXT[!SHOULD,implementation,test]: fails to validate, the NAT SHOULD silently drop the ICMP Error + TEXT[!SHOULD,implementation,test]: packet. + TEXT[!SHOULD,implementation,test]: a) If the IP checksum of the embedded packet fails to + TEXT[!SHOULD,implementation,test]: validate, the NAT SHOULD silently drop the Error packet; + TEXT[!SHOULD,implementation,test]: and TEXT[!MUST]: b) If the embedded packet includes IP options, the NAT device TEXT[!MUST]: MUST traverse past the IP options to locate the start of TEXT[!MUST]: the transport header for the embedded packet; and - TEXT[!SHOULD]: c) The NAT device SHOULD NOT validate the transport checksum - TEXT[!SHOULD]: of the embedded packet within an ICMP Error message, even - TEXT[!SHOULD]: when it is possible to do so; and + TEXT[!SHOULD,implementation,test]: c) The NAT device SHOULD NOT validate the transport checksum + TEXT[!SHOULD,implementation,test]: of the embedded packet within an ICMP Error message, even + TEXT[!SHOULD,implementation,test]: when it is possible to do so; and TEXT[!MUST]: d) If the ICMP Error payload contains ICMP extensions TEXT[!MUST]: [ICMP-EXT], the NAT device MUST exclude the optional zero- TEXT[!MUST]: padding and the ICMP extensions when evaluating transport diff --git a/net/src/packet/icmp_err.rs b/net/src/packet/icmp_err.rs index 44556a1027..2ee12a7ccf 100644 --- a/net/src/packet/icmp_err.rs +++ b/net/src/packet/icmp_err.rs @@ -74,6 +74,18 @@ impl<'a> IcmpErrorPacket<'a> { /// - If the ICMP checksum is not valid, returns `IcmpErrorPacketError::BadChecksumIcmp`. /// - If the inner IPv4 checksum is not valid, returns /// `IcmpErrorPacketError::BadChecksumInnerIpv4`. + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //# REQ-3: When an ICMP Error packet is received, if the ICMP checksum + //# fails to validate, the NAT SHOULD silently drop the ICMP Error + //# packet. + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //# a) If the IP checksum of the embedded packet fails to + //# validate, the NAT SHOULD silently drop the Error packet; + //# and + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //# c) The NAT device SHOULD NOT validate the transport checksum + //# of the embedded packet within an ICMP Error message, even + //# when it is possible to do so; and pub fn validate_checksums(&self) -> Result<(), IcmpErrorPacketError> { self.icmp .validate_checksum(&self.checksum_payload()) @@ -301,3 +313,111 @@ mod tests { icmp_error_packet.validate_checksums().unwrap(); } } + +#[cfg(test)] +mod req3_properties { + use super::*; + use crate::buffer::TestBuffer; + use crate::checksum::Checksum; + use crate::headers::{TryEmbeddedTransportMut, TryIcmpAny, TryIcmpAnyMut, TryInnerIpv4Mut}; + use crate::icmp_any::IcmpAnyChecksum; + use crate::ipv4::Ipv4Checksum; + use crate::packet::{IcmpErrorMsg, Packet}; + + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //= type=test + //# REQ-3: When an ICMP Error packet is received, if the ICMP checksum + //# fails to validate, the NAT SHOULD silently drop the ICMP Error + //# packet. + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //= type=test + //# a) If the IP checksum of the embedded packet fails to + //# validate, the NAT SHOULD silently drop the Error packet; + //# and + //= https://www.rfc-editor.org/rfc/rfc5508#section-4.1 + //= type=test + //# c) The NAT device SHOULD NOT validate the transport checksum + //# of the embedded packet within an ICMP Error message, even + //# when it is possible to do so; and + #[test] + fn only_the_icmp_and_embedded_ip_checksums_decide_an_icmp_error() { + bolero::check!().with_generator(IcmpErrorMsg {}).for_each( + |generated: &Packet| { + let mut good = generated.clone(); + good.update_checksums(); + if IcmpErrorPacket::new(&good).is_none() { + return; + } + assert!( + IcmpErrorPacket::new(&good) + .unwrap_or_else(|| unreachable!()) + .validate_checksums() + .is_ok(), + "a packet with every checksum set does not validate" + ); + + let mut transport_broken = good.clone(); + if let Some(transport) = transport_broken.try_embedded_transport_mut() + && let Some(current) = transport.checksum() + { + transport.update_checksum(current, 0, 1); + transport_broken.update_checksums(); + assert!( + IcmpErrorPacket::new(&transport_broken) + .unwrap_or_else(|| unreachable!()) + .validate_checksums() + .is_ok(), + "the embedded transport checksum was consulted" + ); + } + + let mut icmp_broken = good.clone(); + let current = u16::from( + icmp_broken + .try_icmp_any() + .unwrap_or_else(|| unreachable!()) + .checksum() + .unwrap_or_else(|| unreachable!()), + ); + let _ = icmp_broken + .try_icmp_any_mut() + .unwrap_or_else(|| unreachable!()) + .set_checksum(IcmpAnyChecksum::new(current ^ 1)); + assert!( + matches!( + IcmpErrorPacket::new(&icmp_broken) + .unwrap_or_else(|| unreachable!()) + .validate_checksums(), + Err(IcmpErrorPacketError::BadChecksumIcmp(_)) + ), + "a wrong ICMP checksum was accepted" + ); + + let mut inner_broken = good.clone(); + if let Some(inner) = inner_broken.try_inner_ipv4_mut() + && let Some(current) = inner.checksum().map(u16::from) + { + let _ = inner.set_checksum(Ipv4Checksum::new(current ^ 1)); + let payload: Vec = inner_broken.payload.as_ref().to_vec(); + let headers = &mut inner_broken.headers; + let net = headers.net.clone().unwrap_or_else(|| unreachable!()); + let embedded = headers.embedded_ip.clone(); + headers + .transport + .as_mut() + .unwrap_or_else(|| unreachable!()) + .update_checksum(&net, embedded.as_ref(), payload.as_slice()); + assert!( + matches!( + IcmpErrorPacket::new(&inner_broken) + .unwrap_or_else(|| unreachable!()) + .validate_checksums(), + Err(IcmpErrorPacketError::BadChecksumInnerIpv4(_)) + ), + "a wrong embedded IP checksum was accepted" + ); + } + }, + ); + } +} From 92986def29e8325d499abbb950769ef15d913780 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 02:31:25 -0600 Subject: [PATCH 09/22] fix(acl-filter,flow-filter): Match on the protocol the packet carries RFC 8200 puts extension headers between the IP header and the transport, so for IPv6 the IP header's next-header field names the first extension header rather than what the packet carries. Both filters matched their rules against that field. In `acl-filter` this is a bypass: a Deny rule naming TCP stops applying to any TCP packet with one Hop-by-Hop header in front of it, and the sender chooses whether there is one. In `flow-filter` the same mistake fails the other way, dropping traffic a protocol-restricted expose was configured to carry -- which had been written down as a characterization test rather than recognised as the same defect. `upper_layer_proto` returns `None` when the chain runs past MAX_NET_EXTENSIONS rather than guessing, because a filter has to decide what to do about a chain nobody finished reading. Both callers drop. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- acl-filter/src/lib.rs | 5 +++- acl-filter/src/tests.rs | 52 ++++++++++++++++++++++++++++++++++++++++ flow-filter/src/lib.rs | 8 ++++++- flow-filter/src/tests.rs | 18 ++++++++------ net/src/headers/mod.rs | 14 +++++++++++ net/src/ip/mod.rs | 6 +++++ net/src/packet/utils.rs | 8 +++++-- 7 files changed, 100 insertions(+), 11 deletions(-) diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index 1e0440c481..c251dfcb06 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -186,7 +186,10 @@ impl TryFrom<&Packet> for PacketSummary { let src_ip = net.src_addr(); let dst_ip = net.dst_addr(); - let proto = net.next_header(); + let Some(proto) = packet.upper_layer_proto() else { + debug!("Could not determine the upper-layer protocol, dropping packet"); + return Err(DoneReason::Malformed); + }; let ports = packet .try_transport() .and_then(|t| t.src_port().zip(t.dst_port())); diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 24ab8ad172..6c5a6bf46c 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -263,6 +263,27 @@ fn build_tcp_packet_v6(src: Ipv6Addr, dst: Ipv6Addr, sport: u16, dport: u16) -> .unwrap() } +fn build_tcp_packet_v6_with_hop_by_hop( + src: Ipv6Addr, + dst: Ipv6Addr, + sport: u16, + dport: u16, +) -> Headers { + HeaderStack::new() + .eth(|_| {}) + .ipv6(|ip| { + ip.set_source(UnicastIpv6Addr::new(src).unwrap()); + ip.set_destination(dst); + }) + .hop_by_hop(|_| {}) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(sport).unwrap()); + tcp.set_destination(TcpPort::try_from(dport).unwrap()); + }) + .build_headers() + .unwrap() +} + // ICMP (IP protocol 1): a non-TCP/UDP protocol, used to exercise the `Other(n)` and `Any` tables. fn build_icmp_packet(src: Ipv4Addr, dst: Ipv4Addr) -> Headers { HeaderStack::new() @@ -1214,3 +1235,34 @@ mod dpdk_backend { }); } } + +#[test] +fn an_extension_header_does_not_evade_a_protocol_rule() { + let acl = Acl::new( + AclAction::Allow, + vec![rule( + "deny-tcp", + AclAction::Deny, + AclScope::Packet, + pattern(&[V1_IPS_V6], &[V2_IPS_V6], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter(V1_IPS_V6, V2_IPS_V6, Some(acl)); + + let plain = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet_v6(v6("2001:db8::5"), v6("2001:db9::5"), 1234, 80), + ); + assert!(is_denied(&run(&mut filter, plain))); + + let stuffed = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet_v6_with_hop_by_hop(v6("2001:db8::5"), v6("2001:db9::5"), 1234, 80), + ); + assert!( + is_denied(&run(&mut filter, stuffed)), + "a Hop-by-Hop header carried TCP past a rule denying TCP" + ); +} diff --git a/flow-filter/src/lib.rs b/flow-filter/src/lib.rs index 39f24731bc..ad150fdcf7 100644 --- a/flow-filter/src/lib.rs +++ b/flow-filter/src/lib.rs @@ -140,12 +140,18 @@ impl FlowFilter { return Classification::Drop; }; + let Some(proto) = packet.upper_layer_proto() else { + debug!("{nfi}: Could not determine the upper-layer protocol, dropping packet"); + packet.done(DoneReason::Malformed); + return Classification::Drop; + }; + let input = LookupInput { src_vpcd, dst_vpcd: revalidation_dst_vpcd, src_ip: net.src_addr(), dst_ip: net.dst_addr(), - proto: net.next_header(), + proto, ports: packet .try_transport() .and_then(|t| t.src_port().zip(t.dst_port())), diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index b3a90928de..856128e278 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -1953,10 +1953,8 @@ mod adversarial_headers { // Protocol, port, and flow-generation edge cases. -/// IPv6 extension headers occupy `Net::next_header()`, while `try_transport()` still finds the TCP -/// ports. Protocol-restricted exposes therefore do not match TCP behind an extension header. #[test] -fn ipv6_extension_header_masks_the_transport_protocol() { +fn ipv6_extension_header_does_not_mask_the_transport_protocol() { use net::headers::builder::HeaderStack; use net::headers::{TryIp, TryTransport}; use net::ipv6::UnicastIpv6Addr; @@ -1986,6 +1984,11 @@ fn ipv6_extension_header_masks_the_transport_protocol() { net::ip::NextHeader::new(0), "an extension header should occupy the next-header field", ); + assert_eq!( + probe_packet.upper_layer_proto(), + Some(net::ip::NextHeader::TCP), + "the protocol the packet carries is TCP, whatever the IP header's field says", + ); assert_eq!( probe_packet .try_transport() @@ -2015,11 +2018,12 @@ fn ipv6_extension_header_masks_the_transport_protocol() { ); let (mut flow_filter, _writer) = make_flow_filter(tcp_only); let out = run(&mut flow_filter, packet(Some(vpcd(100)), with_hop_by_hop())); - assert_eq!( - out.get_done(), - Some(DoneReason::Filtered), - "a TCP-restricted expose does not see this packet as TCP, so nothing covers it", + assert!( + !out.is_done(), + "a TCP-restricted expose did not see this packet as TCP: {:?}", + out.get_done() ); + assert_eq!(out.meta().dst_vpcd, Some(vpcd(200))); // An unrestricted expose confirms that the address remains routable. let any_proto = context( diff --git a/net/src/headers/mod.rs b/net/src/headers/mod.rs index d81a84b950..8a113905ce 100644 --- a/net/src/headers/mod.rs +++ b/net/src/headers/mod.rs @@ -756,6 +756,20 @@ impl Headers { &self.net_ext } + #[must_use] + pub fn upper_layer_proto(&self) -> Option { + let next = match self.net_ext.last() { + Some(NetExt::HopByHop(h)) => h.next_header(), + Some(NetExt::DestOpts(h)) => h.next_header(), + Some(NetExt::Routing(h)) => h.next_header(), + Some(NetExt::Fragment(h)) => h.next_header(), + Some(NetExt::Ipv4Auth(h)) => h.next_header(), + Some(NetExt::Ipv6Auth(h)) => h.next_header(), + None => self.net.as_ref()?.next_header(), + }; + (!next.is_ipv6_extension()).then_some(next) + } + /// Get a reference to the transport header, if present. #[must_use] pub fn transport(&self) -> Option<&Transport> { diff --git a/net/src/ip/mod.rs b/net/src/ip/mod.rs index ae5dcbce5c..cc6b6ddcc2 100644 --- a/net/src/ip/mod.rs +++ b/net/src/ip/mod.rs @@ -64,6 +64,12 @@ impl NextHeader { /// IP Authentication Header (RFC 4302) pub const AUTH: NextHeader = NextHeader(IpNumber::AUTHENTICATION_HEADER); + #[must_use] + #[allow(missing_docs)] + pub const fn is_ipv6_extension(self) -> bool { + matches!(self.as_u8(), 0 | 43 | 44 | 51 | 60) + } + /// Generate a new [`NextHeader`] /// /// `const` so callers can build associated constants from it (see this type's `MaskBits` diff --git a/net/src/packet/utils.rs b/net/src/packet/utils.rs index 6e333bce3a..9d9af3f3ed 100644 --- a/net/src/packet/utils.rs +++ b/net/src/packet/utils.rs @@ -145,8 +145,12 @@ impl Packet { .map_err(|_| PacketUtilError::IpVersionMismatch(ip)) } - /// Get the Ip protocol / next-header of an IPv4 / IPv6 [`Packet`] - /// Returns None if the packet does not have an IP header + #[must_use] + #[allow(missing_docs)] + pub fn upper_layer_proto(&self) -> Option { + self.headers.upper_layer_proto() + } + #[allow(missing_docs)] pub fn ip_proto(&self) -> Option { self.try_ip().map(|net| match net { From cd707253a8de9f9dbde66cb42a8b79b52ce2709b Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 02:34:39 -0600 Subject: [PATCH 10/22] test(acl-filter): Prove the over-limit chain is dropped, not guessed The case only exists on the wire -- the header builder caps at MAX_NET_EXTENSIONS -- so the fixture is bytes. That is also the only place an attacker writes. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- acl-filter/src/tests.rs | 61 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 6c5a6bf46c..6f90b94fba 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -1266,3 +1266,64 @@ fn an_extension_header_does_not_evade_a_protocol_rule() { "a Hop-by-Hop header carried TCP past a rule denying TCP" ); } + +#[test] +fn a_chain_past_the_parser_limit_is_dropped_rather_than_guessed() { + const HOP_BY_HOP: u8 = 0; + const TCP: u8 = 6; + + let mut chain = Vec::new(); + for i in 0..4 { + let next = if i == 3 { TCP } else { HOP_BY_HOP }; + chain.extend_from_slice(&[next, 0, 1, 4, 0, 0, 0, 0]); + } + + let mut tcp = vec![0u8; 20]; + tcp[0..2].copy_from_slice(&1234u16.to_be_bytes()); + tcp[2..4].copy_from_slice(&80u16.to_be_bytes()); + tcp[12] = 0x50; + tcp[13] = 0x02; + + let mut bytes = Vec::new(); + bytes.extend_from_slice(&[0x02, 0, 0, 0, 0, 2]); + bytes.extend_from_slice(&[0x02, 0, 0, 0, 0, 1]); + bytes.extend_from_slice(&0x86DDu16.to_be_bytes()); + bytes.extend_from_slice(&[0x60, 0, 0, 0]); + #[allow(clippy::cast_possible_truncation)] + bytes.extend_from_slice(&((chain.len() + tcp.len()) as u16).to_be_bytes()); + bytes.push(HOP_BY_HOP); + bytes.push(64); + bytes.extend_from_slice(&v6("2001:db8::5").octets()); + bytes.extend_from_slice(&v6("2001:db9::5").octets()); + bytes.extend_from_slice(&chain); + bytes.extend_from_slice(&tcp); + + let mut buffer = TestBuffer::from_raw_data(&bytes); + let mut over_limit = Packet::new(buffer.clone()).unwrap(); + assert_eq!( + over_limit.upper_layer_proto(), + None, + "the parser stopped mid-chain but a protocol was reported anyway" + ); + let _ = &mut buffer; + + over_limit.meta_mut().set_overlay(true); + over_limit.meta_mut().src_vpcd = Some(vpcd(VNI1)); + over_limit.meta_mut().dst_vpcd = Some(vpcd(VNI2)); + + let acl = Acl::new( + AclAction::Allow, + vec![rule( + "allow-tcp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS_V6], &[V2_IPS_V6], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter(V1_IPS_V6, V2_IPS_V6, Some(acl)); + let out = run(&mut filter, over_limit); + assert!( + out.is_done(), + "a packet whose header chain was never fully read went through" + ); +} From dd890303470cbdf8a3eccc7d956e10b9cf0787f2 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 02:49:50 -0600 Subject: [PATCH 11/22] fix(net): Take ICMPv6 Parameter Problem off the extension-structure list RFC 4884 section 4.6 gives one list per address family and they differ: ICMPv4 Parameter Problem may carry an extension structure and ICMPv6 Parameter Problem may not, because its bytes 4..8 are the Pointer and there is nowhere to put a length attribute. The IPv6 list was the IPv4 list. The consequence is a pointer read as a length: any ICMPv6 Parameter Problem whose pointer exceeds 255 announces an "original datagram" of up to 2040 octets, and `check_full_payload` decides whether the embedded packet is complete against that number. Same shape as the RFC 4884 defect this branch opened with -- a near-duplicate where the specification is not symmetric -- so both lists are now stated, and the citation needs both tests. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- .duvet/snapshot.txt | 4 +-- net/src/icmp4/mod.rs | 75 +++++++++++++++++++++++++++++++++++++++++++- net/src/icmp6/mod.rs | 64 +++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 3 deletions(-) diff --git a/.duvet/snapshot.txt b/.duvet/snapshot.txt index 40cacb3672..98408d2550 100644 --- a/.duvet/snapshot.txt +++ b/.duvet/snapshot.txt @@ -210,8 +210,8 @@ SPECIFICATION: https://www.rfc-editor.org/rfc/rfc4884 SECTION: [ICMP Messages That Can Be Extended](#section-4.6) TEXT[!MAY]: The ICMP Extension Structure MAY be appended to messages of the TEXT[!MAY]: following types: - TEXT[!MUST]: The ICMP Extension Structure MUST NOT be appended to any of the other - TEXT[!MUST]: ICMP messages mentioned in Section 4. + TEXT[!MUST,implementation,test]: The ICMP Extension Structure MUST NOT be appended to any of the other + TEXT[!MUST,implementation,test]: ICMP messages mentioned in Section 4. SECTION: [Compliant Application Receives ICMP Message with No Extensions](#section-5.4) TEXT[!MUST]: If the length attribute is zero, the compliant application diff --git a/net/src/icmp4/mod.rs b/net/src/icmp4/mod.rs index 8c94855edb..925e29b4f9 100644 --- a/net/src/icmp4/mod.rs +++ b/net/src/icmp4/mod.rs @@ -606,9 +606,11 @@ impl Icmp4 { }) } + //= https://www.rfc-editor.org/rfc/rfc4884#section-4.6 + //# The ICMP Extension Structure MUST NOT be appended to any of the other + //# ICMP messages mentioned in Section 4. #[must_use] pub(crate) fn supports_extensions(&self) -> bool { - // See RFC 4884. Redirect does not get an optional length field. matches!( self.icmp_type(), Icmp4Type::DestUnreachable(_) | Icmp4Type::TimeExceeded(_) | Icmp4Type::ParamProblem(_) @@ -1142,6 +1144,77 @@ mod test { use crate::parse::{DeParse, IntoNonZeroUSize, Parse}; use etherparse::Icmpv4Header; + //= https://www.rfc-editor.org/rfc/rfc4884#section-4.6 + //= type=test + //# The ICMP Extension Structure MUST NOT be appended to any of the other + //# ICMP messages mentioned in Section 4. + #[test] + fn only_three_icmpv4_types_can_carry_an_extension_structure() { + use etherparse::{Icmpv4Type, icmpv4}; + + fn icmp4(icmp_type: Icmpv4Type) -> Icmp4 { + Icmp4(Icmpv4Header { + icmp_type, + checksum: 0, + }) + } + + assert!( + icmp4(Icmpv4Type::DestinationUnreachable( + icmpv4::DestUnreachableHeader::Network + )) + .supports_extensions() + ); + assert!( + icmp4(Icmpv4Type::TimeExceeded( + icmpv4::TimeExceededCode::TtlExceededInTransit + )) + .supports_extensions() + ); + assert!( + icmp4(Icmpv4Type::ParameterProblem( + icmpv4::ParameterProblemHeader::PointerIndicatesError(4) + )) + .supports_extensions(), + "unlike ICMPv6, ICMPv4 Parameter Problem does carry a length attribute" + ); + + assert!( + !icmp4(Icmpv4Type::EchoRequest(etherparse::IcmpEchoHeader { + id: 1, + seq: 1 + })) + .supports_extensions() + ); + assert!( + !icmp4(Icmpv4Type::Redirect(icmpv4::RedirectHeader { + code: icmpv4::RedirectCode::RedirectForNetwork, + gateway_internet_address: [10, 0, 0, 1], + })) + .supports_extensions() + ); + } + + #[test] + fn the_icmpv4_length_attribute_counts_32_bit_words() { + use etherparse::{Icmpv4Type, icmpv4}; + + let unreachable = Icmp4(Icmpv4Header { + icmp_type: Icmpv4Type::DestinationUnreachable(icmpv4::DestUnreachableHeader::Network), + checksum: 0, + }); + let mut buf = [0u8; 8]; + buf[4] = 200; + buf[5] = 17; + assert_eq!(unreachable.payload_length(&buf), 17 * 4); + + let echo = Icmp4(Icmpv4Header { + icmp_type: Icmpv4Type::EchoRequest(etherparse::IcmpEchoHeader { id: 1, seq: 1 }), + checksum: 0, + }); + assert_eq!(echo.payload_length(&buf), 0); + } + /// A redirect with a multicast gateway should be treated as unknown /// ICMP, since RFC 1122 section 3.2.2.2 requires unicast. #[test] diff --git a/net/src/icmp6/mod.rs b/net/src/icmp6/mod.rs index eb7bf8ac2b..5a51cf3b15 100644 --- a/net/src/icmp6/mod.rs +++ b/net/src/icmp6/mod.rs @@ -1199,6 +1199,70 @@ mod test { use crate::icmp6::{Icmp6, Icmp6Type}; use crate::parse::{DeParse, Parse}; + use etherparse::{Icmpv6Header, Icmpv6Type}; + + fn icmp6(icmp_type: Icmpv6Type) -> Icmp6 { + Icmp6(Icmpv6Header { + icmp_type, + checksum: 0, + }) + } + + //= https://www.rfc-editor.org/rfc/rfc4884#section-4.6 + //= type=test + //# The ICMP Extension Structure MUST NOT be appended to any of the other + //# ICMP messages mentioned in Section 4. + #[test] + fn only_two_icmpv6_types_can_carry_an_extension_structure() { + assert!( + icmp6(Icmpv6Type::DestinationUnreachable( + etherparse::icmpv6::DestUnreachableCode::NoRoute + )) + .supports_extensions() + ); + assert!( + icmp6(Icmpv6Type::TimeExceeded( + etherparse::icmpv6::TimeExceededCode::HopLimitExceeded + )) + .supports_extensions() + ); + + assert!( + !icmp6(Icmpv6Type::ParameterProblem( + etherparse::icmpv6::ParameterProblemHeader { + code: etherparse::icmpv6::ParameterProblemCode::ErroneousHeaderField, + pointer: 0x0100, + } + )) + .supports_extensions(), + "ICMPv6 Parameter Problem has no room for a length attribute; its bytes 4..8 are the \ + Pointer, and reading a length out of them makes a large pointer look like a claim \ + about how much of the offending datagram was included" + ); + assert!(!icmp6(Icmpv6Type::PacketTooBig { mtu: 1500 }).supports_extensions()); + assert!( + !icmp6(Icmpv6Type::EchoRequest(etherparse::IcmpEchoHeader { + id: 1, + seq: 1 + })) + .supports_extensions() + ); + } + + #[test] + fn the_icmpv6_length_attribute_counts_64_bit_words() { + let unreachable = icmp6(Icmpv6Type::DestinationUnreachable( + etherparse::icmpv6::DestUnreachableCode::NoRoute, + )); + let mut buf = [0u8; 8]; + buf[4] = 17; + buf[5] = 200; + assert_eq!(unreachable.payload_length(&buf), 17 * 8); + + let too_big = icmp6(Icmpv6Type::PacketTooBig { mtu: 1500 }); + assert_eq!(too_big.payload_length(&buf), 0); + } + /// A Packet Too Big with MTU below 1280 should be treated as unknown /// `ICMPv6`, since RFC 8200 section 5 sets 1280 as the IPv6 minimum. #[test] From f229823eadee8e7f07534e056ca021e36e4a8222 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 11:20:20 -0600 Subject: [PATCH 12/22] fix(acl-filter,flow-filter): Refuse a header chain the stage cannot read Both stages reached into the headers for the two layers they care about, which means every layer they do not name is silently along for the ride. `pat` was built to make that impossible -- it fails the match when an unnamed layer is present, and VLAN tags are strict there because they decide forwarding -- and both stages bypassed it. A VLAN tag is not an evasion: it sits ahead of the IP header, so the protocol and ports a rule matches on were always right. The problem is the other side. Nothing after these stages reads `headers.vlan` -- `Egress` rewrites the MACs and leaves it, VXLAN re-encapsulation puts the outer headers in front of it -- so a tag chosen by whoever built the inner frame of a tunnelled packet would be forwarded onto whatever segment it names, decided by nobody. There is no configuration that expresses an opinion about one. The cost is one `ArrayVec::len` comparison: `opt_eth` and `opt_transport` are `Option` maps over fields both stages already read, `ext_gap_ok` for `Net` is a constant `true`, and `step` is `#[inline]`. Everything else is the type-level accumulator. `upper_layer_proto` stays, and the two are complementary rather than redundant: `pat` skips extension headers at the network position on purpose, so a chain that ran past MAX_NET_EXTENSIONS still matches and only `upper_layer_proto` reports that the transport was never reached. The flow-filter oracle read the protocol out of the same field the classifier did, so correcting one required correcting the other -- and that emptied the suite's "non-transport protocol" bucket, which had been filled entirely by extension-header stacks being misread. `V4ExoticProto` supplies a real one. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- acl-filter/src/lib.rs | 23 +++++++++++++------ acl-filter/src/tests.rs | 49 ++++++++++++++++++++++++++++++++++++++++ flow-filter/src/lib.rs | 24 ++++++++++++++------ flow-filter/src/tests.rs | 23 ++++++++++++++++--- 4 files changed, 102 insertions(+), 17 deletions(-) diff --git a/acl-filter/src/lib.rs b/acl-filter/src/lib.rs index c251dfcb06..472fad4b8b 100644 --- a/acl-filter/src/lib.rs +++ b/acl-filter/src/lib.rs @@ -6,7 +6,7 @@ use config::external::overlay::acl::{AclAction, AclScope}; use net::buffer::PacketBufferMut; use net::flows::FlowInfo; use net::flows::FlowStatus; -use net::headers::{TryIp, TryTransport}; +use net::headers::{TryHeaders, TryIp}; use net::ip::NextHeader; use net::packet::{DoneReason, Packet, PacketMeta, VpcDiscriminant}; use net::vxlan::Vni; @@ -179,9 +179,20 @@ impl TryFrom<&Packet> for PacketSummary { let VpcDiscriminant::VNI(src_vni) = src_vpcd; let VpcDiscriminant::VNI(dst_vni) = dst_vpcd; - let Some(net) = packet.try_ip() else { - debug!("No IP headers found, dropping packet"); - return Err(DoneReason::NotIp); + let Some((_eth, net, transport)) = packet + .headers() + .pat() + .opt_eth() + .net() + .opt_transport() + .done() + else { + if packet.try_ip().is_none() { + debug!("No IP headers found, dropping packet"); + return Err(DoneReason::NotIp); + } + debug!("Header chain carries a layer this stage cannot account for, dropping packet"); + return Err(DoneReason::Unhandled); }; let src_ip = net.src_addr(); @@ -190,9 +201,7 @@ impl TryFrom<&Packet> for PacketSummary { debug!("Could not determine the upper-layer protocol, dropping packet"); return Err(DoneReason::Malformed); }; - let ports = packet - .try_transport() - .and_then(|t| t.src_port().zip(t.dst_port())); + let ports = transport.and_then(|t| t.src_port().zip(t.dst_port())); Ok(Self { src_vni, diff --git a/acl-filter/src/tests.rs b/acl-filter/src/tests.rs index 6f90b94fba..4f242d344b 100644 --- a/acl-filter/src/tests.rs +++ b/acl-filter/src/tests.rs @@ -1236,6 +1236,55 @@ mod dpdk_backend { } } +#[test] +fn a_vlan_tagged_overlay_frame_is_refused() { + use net::vlan::Vid; + + let acl = Acl::new( + AclAction::Deny, + vec![rule( + "allow-tcp", + AclAction::Allow, + AclScope::Packet, + pattern(&[V1_IPS], &[V2_IPS], AclProtoMatch::Tcp), + )], + ); + let mut filter = build_filter_v4(Some(acl)); + + let plain = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + build_tcp_packet(v4("10.0.0.5"), v4("20.0.0.5"), 1234, 80), + ); + assert!(is_allowed(&run(&mut filter, plain))); + + let tagged = packet( + vpcd(VNI1), + Some(vpcd(VNI2)), + HeaderStack::new() + .eth(|_| {}) + .vlan(|v| { + v.set_vid(Vid::new(4000).unwrap()); + }) + .ipv4(|ip| { + ip.set_source(UnicastIpv4Addr::new(v4("10.0.0.5")).unwrap()); + ip.set_destination(v4("20.0.0.5")); + }) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(1234u16).unwrap()); + tcp.set_destination(TcpPort::try_from(80u16).unwrap()); + }) + .build_headers() + .unwrap(), + ); + let out = run(&mut filter, tagged); + assert_eq!( + out.get_done(), + Some(DoneReason::Unhandled), + "a VLAN-tagged overlay frame was forwarded on the strength of its inner headers" + ); +} + #[test] fn an_extension_header_does_not_evade_a_protocol_rule() { let acl = Acl::new( diff --git a/flow-filter/src/lib.rs b/flow-filter/src/lib.rs index ad150fdcf7..2fc7ae78f0 100644 --- a/flow-filter/src/lib.rs +++ b/flow-filter/src/lib.rs @@ -8,7 +8,7 @@ use config::external::overlay::vpcpeering::{ValidatedExpose, VpcExposeNatConfig} use net::FlowKey; use net::buffer::PacketBufferMut; use net::flows::{FlowInfo, FlowStatus}; -use net::headers::{TryIp, TryTransport}; +use net::headers::{TryHeaders, TryIp}; use net::packet::{DoneReason, Packet, PacketMeta, VpcDiscriminant}; use pipeline::{NetworkFunction, PipelineData}; use tracectl::trace_target; @@ -129,9 +129,21 @@ impl FlowFilter { self.flow_revalidation_data(flow_summary, genid); } - let Some(net) = packet.try_ip() else { - debug!("{nfi}: No IP headers found, dropping packet"); - packet.done(DoneReason::NotIp); + let Some((_eth, net, transport)) = packet + .headers() + .pat() + .opt_eth() + .net() + .opt_transport() + .done() + else { + if packet.try_ip().is_none() { + debug!("{nfi}: No IP headers found, dropping packet"); + packet.done(DoneReason::NotIp); + } else { + debug!("{nfi}: Header chain carries a layer this stage cannot account for"); + packet.done(DoneReason::Unhandled); + } return Classification::Drop; }; let Some(src_vpcd) = packet.meta().src_vpcd else { @@ -152,9 +164,7 @@ impl FlowFilter { src_ip: net.src_addr(), dst_ip: net.dst_addr(), proto, - ports: packet - .try_transport() - .and_then(|t| t.src_port().zip(t.dst_port())), + ports: transport.and_then(|t| t.src_port().zip(t.dst_port())), gate: revalidation_gate, }; Classification::Lookup { diff --git a/flow-filter/src/tests.rs b/flow-filter/src/tests.rs index 856128e278..033b1dc80c 100644 --- a/flow-filter/src/tests.rs +++ b/flow-filter/src/tests.rs @@ -1460,13 +1460,18 @@ fn probe_from_packet(pkt: &Packet, src_vpcd: VpcDiscriminant) -> Opt gate: SourceGate::Ungated, src_ip: net.src_addr(), dst_ip: net.dst_addr(), - proto: net.next_header(), + proto: pkt.upper_layer_proto()?, ports: pkt .try_transport() .and_then(|t| t.src_port().zip(t.dst_port())), }) } +fn carries_unaccounted_layers(pkt: &Packet) -> bool { + use net::headers::TryHeaders; + !pkt.headers().vlan().is_empty() +} + fn observed_outcome(pkt: &Packet) -> NfOutcome { if pkt.is_done() { return NfOutcome::Dropped(pkt.get_done()); @@ -1624,7 +1629,8 @@ fn nf_metadata_matches_config_oracle() { mod adversarial_headers { use super::{ - NfOutcome, expected_outcome, make_flow_filter, observed_outcome, probe_from_packet, + NfOutcome, carries_unaccounted_layers, expected_outcome, make_flow_filter, + observed_outcome, probe_from_packet, }; use crate::context::FlowFilterContext; use crate::context::fuzz::oracle_lookup; @@ -1735,6 +1741,7 @@ mod adversarial_headers { V4Icmp, /// A VLAN tag between the Ethernet and IP layers. VlanV4Tcp, + V4ExoticProto, /// An IPv4 authentication header ahead of the transport. V4AuthTcp, V6Tcp, @@ -1746,12 +1753,13 @@ mod adversarial_headers { impl Shape { /// Every shape, in selector and counter order. - const ALL: [Shape; 10] = [ + const ALL: [Shape; 11] = [ Shape::NoIp, Shape::V4Tcp, Shape::V4Udp, Shape::V4Icmp, Shape::VlanV4Tcp, + Shape::V4ExoticProto, Shape::V4AuthTcp, Shape::V6Tcp, Shape::V6Udp, @@ -1793,6 +1801,13 @@ mod adversarial_headers { .ipv4(pin_v4) .tcp(|_| {}) .generate(driver), + Shape::V4ExoticProto => ChainBase::new() + .eth(|_| {}) + .ipv4(|ip| { + pin_v4(ip); + ip.set_next_header(net::ip::NextHeader::new(132)); + }) + .generate(driver), Shape::V4AuthTcp => ChainBase::new() .eth(|_| {}) .ipv4(pin_v4) @@ -1867,6 +1882,7 @@ mod adversarial_headers { // Extract the key before the NF consumes the packet. let probe = probe_from_packet(&packet, src_vpcd()); + let unaccounted = carries_unaccounted_layers(&packet); if let Some(probe) = probe.as_ref() { if probe.ports.is_none() { PORTLESS.fetch_add(1, Ordering::Relaxed); @@ -1889,6 +1905,7 @@ mod adversarial_headers { let expected = match probe.as_ref() { // No IP layer: dropped before any table is consulted. None => NfOutcome::Dropped(Some(DoneReason::NotIp)), + Some(_) if unaccounted => NfOutcome::Dropped(Some(DoneReason::Unhandled)), Some(probe) => expected_outcome(oracle_lookup(&overlay, probe)), }; assert_eq!( From e3931e0baec87991ad56dd6976dc8dc3a517e307 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 11:27:56 -0600 Subject: [PATCH 13/22] fix(net): Stop writing the VXLAN header twice in the test fixture `Headers::size` counts the VXLAN header, because it is one of them and `headers.deparse` writes it. Both fixtures added it again by hand and sized the buffer for both copies, so eight stray octets sat in front of every inner frame. Decapsulation read them as the start of an Ethernet header and stopped there, which the existing tests could not see: they decapsulate and re-encap to compare *outer* DSCP and ECN, and never look at what came out. Found by writing the first test that does look, which is also what the overlay's refusal of VLAN-tagged frames rests on -- if decapsulation stripped tags rather than handing them on, the check in `IpForwarder` would be dead code. The refusal goes at the decapsulation boundary rather than at the first stage that happens to look, so a tag never becomes an overlay packet: `IcmpErrorHandler` and `FlowLookup` both run before the filters that would otherwise refuse it. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/ipforward.rs | 12 +++-- net/src/packet/mod.rs | 52 +++++++++++++++++++++ net/src/packet/test_utils.rs | 28 +++++------ 3 files changed, 73 insertions(+), 19 deletions(-) diff --git a/dataplane/src/packet_processor/ipforward.rs b/dataplane/src/packet_processor/ipforward.rs index 7863f267df..c13f7e849a 100644 --- a/dataplane/src/packet_processor/ipforward.rs +++ b/dataplane/src/packet_processor/ipforward.rs @@ -5,7 +5,7 @@ #![allow(clippy::similar_names)] -use net::headers::{TryHeadersMut, TryIpv4Mut, TryIpv6Mut}; +use net::headers::{TryHeaders, TryHeadersMut, TryIpv4Mut, TryIpv6Mut}; use net::packet::{DoneReason, Packet}; use net::{buffer::PacketBufferMut, checksum::Checksum}; use pipeline::NetworkFunction; @@ -154,8 +154,14 @@ impl IpForwarder { debug!("Next fib/vrf is {next_vrf}"); /* At this point decapsulation has already happened and `Packet` refers to - the innner packet. Annotate the incoming vni and the corresponding vrf to - make lookups from */ + the innner packet. */ + + if !packet.headers().vlan().is_empty() { + debug!("{nfi}: Decapsulated frame carries a VLAN tag; not supported"); + packet.done(DoneReason::Unhandled); + return; + } + packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(vni)); packet.meta_mut().vrf = Some(next_vrf); packet.meta_mut().set_overlay(true); diff --git a/net/src/packet/mod.rs b/net/src/packet/mod.rs index 003206faa4..3f0de6f68d 100644 --- a/net/src/packet/mod.rs +++ b/net/src/packet/mod.rs @@ -849,6 +849,58 @@ mod qos_roundtrip_tests { VxlanEncap::new(headers).unwrap() } + #[test] + fn decapsulation_hands_on_a_vlan_tag_it_was_given() { + use crate::buffer::TestBuffer; + use crate::eth::ethtype::EthType; + use crate::headers::TryHeaders; + use crate::headers::builder::HeaderStack; + use crate::ipv4::UnicastIpv4Addr; + use crate::packet::Packet; + use crate::packet::test_utils::build_test_vxlan_ipv4_packet_carrying; + use crate::parse::DeParse; + use crate::vlan::Vid; + use std::net::Ipv4Addr; + + let tagged = HeaderStack::new() + .eth(|eth| { + eth.set_ether_type(EthType::VLAN); + }) + .vlan(|v| { + v.set_vid(Vid::new(4000).unwrap()); + }) + .ipv4(|ip| { + ip.set_source(UnicastIpv4Addr::new(Ipv4Addr::new(10, 0, 0, 5)).unwrap()); + ip.set_destination(Ipv4Addr::new(20, 0, 0, 5)); + ip.set_ttl(64); + }) + .build_headers() + .unwrap(); + let mut buffer = TestBuffer::new(); + tagged.deparse(buffer.as_mut()).unwrap(); + let inner = Packet::new(buffer).unwrap(); + let inner_buf = inner.serialize().unwrap(); + + let mut outer = build_test_vxlan_ipv4_packet_carrying( + Dscp::new(0).unwrap(), + Ecn::new(0).unwrap(), + inner_buf.as_ref(), + ) + .unwrap(); + assert!( + outer.headers().vlan().is_empty(), + "the tunnel's own frame is untagged" + ); + + outer.vxlan_decap().unwrap().unwrap(); + assert_eq!( + outer.headers().vlan().len(), + 1, + "the inner frame's tag did not survive decapsulation, so nothing downstream can refuse it" + ); + assert_eq!(outer.headers().vlan()[0].vid(), Vid::new(4000).unwrap()); + } + #[test] fn vxlan_decap_then_encap_preserves_outer_qos_ipv4_underlay() { let in_dscp = Dscp::new(46).unwrap(); diff --git a/net/src/packet/test_utils.rs b/net/src/packet/test_utils.rs index 1b53f61f87..deb7d4bb6a 100644 --- a/net/src/packet/test_utils.rs +++ b/net/src/packet/test_utils.rs @@ -526,11 +526,17 @@ pub fn build_test_vxlan_ipv4_packet_with_outer_qos( dscp: Dscp, ecn: Ecn, ) -> Result, InvalidPacket> { - // Inner ethernet frame bytes let inner = build_test_ipv4_packet(64).unwrap(); let inner_buf = inner.serialize().unwrap(); - let inner_bytes = inner_buf.as_ref(); + build_test_vxlan_ipv4_packet_carrying(dscp, ecn, inner_buf.as_ref()) +} +#[must_use] +pub fn build_test_vxlan_ipv4_packet_carrying( + dscp: Dscp, + ecn: Ecn, + inner_bytes: &[u8], +) -> Result, InvalidPacket> { // VXLAN header bytes let vni = Vni::new_checked(100).unwrap(); let vxlan = Vxlan::new(vni); @@ -567,17 +573,12 @@ pub fn build_test_vxlan_ipv4_packet_with_outer_qos( let headers = headers.build().unwrap(); // Buffer: outer headers + vxlan bytes + inner bytes - let total_len = headers.size().get() as usize + udp_payload_len; + let total_len = headers.size().get() as usize + inner_bytes.len(); let mut data = vec![0u8; total_len]; headers.deparse(data.as_mut()).unwrap(); - let hdr_off = headers.size().get() as usize; - vxlan - .deparse(&mut data[hdr_off..hdr_off + vxlan_len]) - .unwrap(); - - let inner_off = hdr_off + vxlan_len; + let inner_off = headers.size().get() as usize; data[inner_off..inner_off + inner_bytes.len()].copy_from_slice(inner_bytes); Packet::new(TestBuffer::from_raw_data(&data)) @@ -628,17 +629,12 @@ pub fn build_test_vxlan_ipv6_packet_with_outer_qos( headers.udp_encap(Some(UdpEncap::Vxlan(vxlan))); let headers = headers.build().unwrap(); - let total_len = headers.size().get() as usize + udp_payload_len; + let total_len = headers.size().get() as usize + inner_bytes.len(); let mut data = vec![0u8; total_len]; headers.deparse(data.as_mut()).unwrap(); - let hdr_off = headers.size().get() as usize; - vxlan - .deparse(&mut data[hdr_off..hdr_off + vxlan_len]) - .unwrap(); - - let inner_off = hdr_off + vxlan_len; + let inner_off = headers.size().get() as usize; data[inner_off..inner_off + inner_bytes.len()].copy_from_slice(inner_bytes); Packet::new(TestBuffer::from_raw_data(&data)) From 15535d1088cf2621286a0e3a8561fd9d1d5dc92e Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Sat, 22 Aug 2026 11:51:47 -0600 Subject: [PATCH 14/22] docs(code): Separate the rule from the gap it currently produces The rule is that a stage matches the shape of the chain it acts on rather than reaching in for the fields it wants, so that a layer nobody considered cannot be answered over. Two defects the same week made the case, failing in opposite directions: a field read as something it was not, and a field not read at all. The overlay's refusal of VLAN-tagged frames is a consequence of the rule and not an architectural position. VLAN inside VXLAN is legitimate traffic; nothing here is equipped to carry it, which is a different claim and a smaller one. Written down with what changing the answer would take, because the refusal was phrased in a way that read as permanent -- and the same phrasing would have been wrong about MPLS, or about whatever comes next. Naming the layer in the pattern is how a stage says it has been taught. That the untaught stages keep refusing without anybody listing them is the point. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- dataplane/src/packet_processor/ipforward.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dataplane/src/packet_processor/ipforward.rs b/dataplane/src/packet_processor/ipforward.rs index c13f7e849a..483882e416 100644 --- a/dataplane/src/packet_processor/ipforward.rs +++ b/dataplane/src/packet_processor/ipforward.rs @@ -157,7 +157,9 @@ impl IpForwarder { the innner packet. */ if !packet.headers().vlan().is_empty() { - debug!("{nfi}: Decapsulated frame carries a VLAN tag; not supported"); + debug!( + "{nfi}: Decapsulated frame carries a VLAN tag, which nothing downstream is equipped to carry" + ); packet.done(DoneReason::Unhandled); return; } From 0464e3ac394b8321f0fce37383e7689458275e5a Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 26 Aug 2026 12:49:14 -0600 Subject: [PATCH 15/22] fix(net): Leave a disabled IPv4 UDP quote checksum alone Storing the incremental update made these writes real for the first time, and with them the write-side rules the old no-op never had to obey. A UDP datagram over IPv4 may carry a zero checksum to say the sender computed none. There is no sum there to fold a delta into, so translating a port or an address of such a quote turned the marker into a checksum for a sum nobody took. Zero is spoken for on the other side too: over IPv6 the field is mandatory, so a fold that lands on zero goes out as the other spelling instead. Which rule applies depends on the IP version of the packet the header was quoted from, which `EmbeddedTransport` cannot see, hence the new argument. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/icmp_handler/icmp_error_msg.rs | 168 +++++++++++++++++++++++-- net/src/headers/embedded.rs | 30 ++++- net/src/packet/icmp_err.rs | 29 +++-- 3 files changed, 206 insertions(+), 21 deletions(-) diff --git a/nat/src/icmp_handler/icmp_error_msg.rs b/nat/src/icmp_handler/icmp_error_msg.rs index 06688e1daa..4098e67c46 100644 --- a/nat/src/icmp_handler/icmp_error_msg.rs +++ b/nat/src/icmp_handler/icmp_error_msg.rs @@ -9,7 +9,8 @@ use crate::NatTranslationData; use net::buffer::PacketBufferMut; use net::checksum::Checksum; use net::headers::{ - EmbeddedTransport, TryEmbeddedHeadersMut, TryEmbeddedTransportMut, TryInnerIpMut, + EmbeddedIpVersion, EmbeddedTransport, TryEmbeddedHeadersMut, TryEmbeddedTransportMut, + TryInnerIpMut, }; use net::icmp_any::TruncatedIcmpAny; use net::packet::{DoneReason, Packet}; @@ -73,9 +74,11 @@ pub(crate) fn nat_translate_icmp_inner_src( .embedded_headers_mut() .ok_or(IcmpErrorMsgError::NoEmbeddedHeaders)?; - embedded_headers + let inner_ip = embedded_headers .try_inner_ip_mut() - .ok_or(IcmpErrorMsgError::NoInnerIpHeader)? + .ok_or(IcmpErrorMsgError::NoInnerIpHeader)?; + let old_addr = inner_ip.src_addr(); + inner_ip .try_set_source( target_addr .try_into() @@ -94,7 +97,7 @@ pub(crate) fn nat_translate_icmp_inner_src( match transport { EmbeddedTransport::Tcp(_) | EmbeddedTransport::Udp(_) => { - translate_inner_tcp_udp_src(transport, target_port)?; + translate_inner_tcp_udp_src(transport, quoted_version(old_addr), target_port)?; } EmbeddedTransport::Icmp4(icmp4) => { translate_inner_icmp(icmp4, target_port); @@ -106,6 +109,14 @@ pub(crate) fn nat_translate_icmp_inner_src( Ok(()) } +fn quoted_version(addr: IpAddr) -> EmbeddedIpVersion { + if addr.is_ipv4() { + EmbeddedIpVersion::Ipv4 + } else { + EmbeddedIpVersion::Ipv6 + } +} + pub(crate) fn nat_translate_icmp_inner_dst( packet: &mut Packet, target_addr: IpAddr, @@ -115,9 +126,11 @@ pub(crate) fn nat_translate_icmp_inner_dst( .embedded_headers_mut() .ok_or(IcmpErrorMsgError::NoEmbeddedHeaders)?; - embedded_headers + let inner_ip = embedded_headers .try_inner_ip_mut() - .ok_or(IcmpErrorMsgError::NoInnerIpHeader)? + .ok_or(IcmpErrorMsgError::NoInnerIpHeader)?; + let old_addr = inner_ip.dst_addr(); + inner_ip .try_set_destination(target_addr) .map_err(|_| IcmpErrorMsgError::InvalidIpVersion)?; @@ -132,7 +145,7 @@ pub(crate) fn nat_translate_icmp_inner_dst( match transport { EmbeddedTransport::Tcp(_) | EmbeddedTransport::Udp(_) => { - translate_inner_tcp_udp_dst(transport, target_port) + translate_inner_tcp_udp_dst(transport, quoted_version(old_addr), target_port) } _ => Ok(()), // ICMP is dealt with when dealing with the source port } @@ -168,6 +181,7 @@ where fn translate_inner_tcp_udp_src( transport: &mut EmbeddedTransport, + quoted: EmbeddedIpVersion, target_port: NatPort, ) -> Result<(), IcmpErrorMsgError> { // Assume we have TCP or UDP, with source port always present @@ -185,13 +199,14 @@ fn translate_inner_tcp_udp_src( // transport checksum update is to do an unconditional, incremental update here. Note // that this checksum will not be updated again when deparsing the packet. if let Some(current_checksum) = transport.checksum() { - transport.update_checksum(current_checksum, old_port, new_port.get()); + transport.update_checksum(quoted, current_checksum, old_port, new_port.get()); } Ok(()) } fn translate_inner_tcp_udp_dst( transport: &mut EmbeddedTransport, + quoted: EmbeddedIpVersion, target_port: NatPort, ) -> Result<(), IcmpErrorMsgError> { // Assume we have TCP or UDP, with destination port always present @@ -209,7 +224,7 @@ fn translate_inner_tcp_udp_dst( .set_destination(new_port) .unwrap_or_else(|_| unreachable!()); if let Some(current_checksum) = transport.checksum() { - transport.update_checksum(current_checksum, old_port, new_port.get()); + transport.update_checksum(quoted, current_checksum, old_port, new_port.get()); } Ok(()) } @@ -440,3 +455,138 @@ mod bolero_tests { ); } } + +#[cfg(test)] +mod quoted_transport_checksum { + use super::*; + use net::buffer::TestBuffer; + use net::checksum::Checksum; + use net::headers::{TryEmbeddedTransport, TryEmbeddedTransportMut}; + use net::icmp6::{Icmp6DestUnreachable, Icmp6Type}; + use net::ip::NextHeader; + use net::packet::test_utils::{ + Icmp6ErrorAddrs, build_test_icmp4_destination_unreachable_packet, + build_test_icmp6_error_packet, + }; + use net::udp::UdpChecksum; + use std::net::{Ipv4Addr, Ipv6Addr}; + + const OUTER_SRC: Ipv4Addr = Ipv4Addr::new(192, 0, 2, 1); + const OUTER_DST: Ipv4Addr = Ipv4Addr::new(192, 0, 2, 2); + const INNER_SRC: Ipv4Addr = Ipv4Addr::new(198, 51, 100, 7); + const INNER_DST: Ipv4Addr = Ipv4Addr::new(203, 0, 113, 9); + const NAT_SRC: Ipv4Addr = Ipv4Addr::new(198, 51, 100, 200); + const NAT_DST: Ipv4Addr = Ipv4Addr::new(203, 0, 113, 250); + + const OUTER_SRC_V6: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1); + const OUTER_DST_V6: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 2); + const INNER_SRC_V6: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 1, 0, 0, 0, 0, 7); + const INNER_DST_V6: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 2, 0, 0, 0, 0, 9); + const NAT_SRC_V6: Ipv6Addr = Ipv6Addr::new(0x2001, 0xdb8, 0xfeed, 0xf00d, 0, 0, 0, 0xc8); + + const OLD_PORT: u16 = 1234; + const PEER_PORT: u16 = 5678; + const NEW_PORT: u16 = 4321; + const OLD_ID: u16 = 0x1111; + const NEW_ID: u16 = 0x7f7f; + + fn quote_v4( + inner_src: Ipv4Addr, + inner_dst: Ipv4Addr, + next_header: NextHeader, + param_1: u16, + param_2: u16, + ) -> Packet { + build_test_icmp4_destination_unreachable_packet( + OUTER_SRC, + OUTER_DST, + inner_src, + inner_dst, + next_header, + param_1, + param_2, + ) + .unwrap_or_else(|_| unreachable!()) + } + + fn quote_v6( + inner_src: Ipv6Addr, + inner_dst: Ipv6Addr, + next_header: NextHeader, + param_1: u16, + param_2: u16, + ) -> Packet { + build_test_icmp6_error_packet( + Icmp6Type::DestUnreachable(Icmp6DestUnreachable::NoRoute), + Icmp6ErrorAddrs { + outer_src: OUTER_SRC_V6, + outer_dst: OUTER_DST_V6, + inner_src, + inner_dst, + }, + next_header, + param_1, + param_2, + ) + .unwrap_or_else(|_| unreachable!()) + } + + fn quoted_checksum(packet: &Packet) -> u16 { + packet + .try_embedded_transport() + .and_then(EmbeddedTransport::checksum) + .unwrap_or_else(|| unreachable!()) + } + + fn port(value: u16) -> NatPort { + NatPort::new_port(NonZero::new(value).unwrap_or_else(|| unreachable!())) + } + + #[test] + fn a_disabled_ipv4_udp_quote_checksum_stays_disabled() { + let mut packet = quote_v4(INNER_SRC, INNER_DST, NextHeader::UDP, OLD_PORT, PEER_PORT); + match packet.try_embedded_transport_mut() { + Some(EmbeddedTransport::Udp(udp)) => { + udp.set_checksum(UdpChecksum::new(0)) + .unwrap_or_else(|_| unreachable!()); + } + _ => unreachable!(), + } + + nat_translate_icmp_inner_src(&mut packet, IpAddr::V4(NAT_SRC), Some(port(NEW_PORT))) + .unwrap_or_else(|_| unreachable!()); + + assert_eq!( + quoted_checksum(&packet), + 0, + "a quote that carried no checksum came out carrying one" + ); + } + + #[test] + fn a_zero_ipv6_udp_quote_checksum_is_folded_into() { + let mut packet = quote_v6( + INNER_SRC_V6, + INNER_DST_V6, + NextHeader::UDP, + OLD_PORT, + PEER_PORT, + ); + match packet.try_embedded_transport_mut() { + Some(EmbeddedTransport::Udp(udp)) => { + udp.set_checksum(UdpChecksum::new(0)) + .unwrap_or_else(|_| unreachable!()); + } + _ => unreachable!(), + } + + nat_translate_icmp_inner_src(&mut packet, IpAddr::V6(NAT_SRC_V6), Some(port(NEW_PORT))) + .unwrap_or_else(|_| unreachable!()); + + assert_ne!( + quoted_checksum(&packet), + 0, + "an IPv6 quote was left with a checksum IPv6 forbids" + ); + } +} diff --git a/net/src/headers/embedded.rs b/net/src/headers/embedded.rs index 7e12f9f918..0a89be5dc1 100644 --- a/net/src/headers/embedded.rs +++ b/net/src/headers/embedded.rs @@ -28,6 +28,7 @@ use std::num::NonZero; #[cfg(any(test, feature = "bolero"))] pub use contract::*; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum EmbeddedIpVersion { Ipv4, Ipv6, @@ -624,7 +625,22 @@ impl EmbeddedTransport { } } - pub fn update_checksum(&mut self, current_checksum: u16, old_value: u16, new_value: u16) { + fn udp_checksum_disabled(&self, quoted: EmbeddedIpVersion, current: u16) -> bool { + matches!(self, EmbeddedTransport::Udp(_)) + && quoted == EmbeddedIpVersion::Ipv4 + && current == 0 + } + + pub fn update_checksum( + &mut self, + quoted: EmbeddedIpVersion, + current_checksum: u16, + old_value: u16, + new_value: u16, + ) { + if self.udp_checksum_disabled(quoted, current_checksum) { + return; + } match self { EmbeddedTransport::Tcp(tcp) => { let updated = tcp.increment_update_checksum( @@ -640,6 +656,11 @@ impl EmbeddedTransport { old_value, new_value, ); + let updated = if u16::from(updated) == 0 { + UdpChecksum::new(u16::MAX) + } else { + updated + }; let _ = udp.set_checksum(updated); } EmbeddedTransport::Icmp4(icmp) => { @@ -668,11 +689,16 @@ impl EmbeddedTransport { if old.is_ipv4() != new.is_ipv4() { return; } + let quoted = if old.is_ipv4() { + EmbeddedIpVersion::Ipv4 + } else { + EmbeddedIpVersion::Ipv6 + }; for (old_word, new_word) in address_words(old).into_iter().zip(address_words(new)) { let Some(current) = self.checksum() else { return; }; - self.update_checksum(current, old_word, new_word); + self.update_checksum(quoted, current, old_word, new_word); } } } diff --git a/net/src/packet/icmp_err.rs b/net/src/packet/icmp_err.rs index 2ee12a7ccf..e29991bd31 100644 --- a/net/src/packet/icmp_err.rs +++ b/net/src/packet/icmp_err.rs @@ -319,7 +319,9 @@ mod req3_properties { use super::*; use crate::buffer::TestBuffer; use crate::checksum::Checksum; - use crate::headers::{TryEmbeddedTransportMut, TryIcmpAny, TryIcmpAnyMut, TryInnerIpv4Mut}; + use crate::headers::{ + EmbeddedIpVersion, TryEmbeddedTransportMut, TryIcmpAny, TryIcmpAnyMut, TryInnerIpv4Mut, + }; use crate::icmp_any::IcmpAnyChecksum; use crate::ipv4::Ipv4Checksum; use crate::packet::{IcmpErrorMsg, Packet}; @@ -357,18 +359,25 @@ mod req3_properties { ); let mut transport_broken = good.clone(); + let quoted = match transport_broken.try_inner_ip() { + None => unreachable!(), + Some(Net::Ipv4(_)) => EmbeddedIpVersion::Ipv4, + Some(Net::Ipv6(_)) => EmbeddedIpVersion::Ipv6, + }; if let Some(transport) = transport_broken.try_embedded_transport_mut() && let Some(current) = transport.checksum() { - transport.update_checksum(current, 0, 1); - transport_broken.update_checksums(); - assert!( - IcmpErrorPacket::new(&transport_broken) - .unwrap_or_else(|| unreachable!()) - .validate_checksums() - .is_ok(), - "the embedded transport checksum was consulted" - ); + transport.update_checksum(quoted, current, 0, 1); + if transport.checksum() != Some(current) { + transport_broken.update_checksums(); + assert!( + IcmpErrorPacket::new(&transport_broken) + .unwrap_or_else(|| unreachable!()) + .validate_checksums() + .is_ok(), + "the embedded transport checksum was consulted" + ); + } } let mut icmp_broken = good.clone(); From 2ef8be6547fd7c52e2b8af5305586984ccf12ddc Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 26 Aug 2026 12:49:41 -0600 Subject: [PATCH 16/22] fix(nat): Fold a quoted address rewrite into the quoted transport checksum `update_checksum_for_address` was added with no caller. Wiring it is the point: TCP, UDP and ICMPv6 are checksummed over a pseudo-header built from the quoted packet's addresses, so rewriting one and stopping there leaves the quote describing an address that is no longer in it. The fold is its own step rather than part of port translation because a mapping that moves only an address never reaches the transport header otherwise -- the port path returns first when there is no port to move. Nothing on the wire catches this. RFC 5508 REQ-3(c) tells a NAT not to validate the quoted transport checksum of an error it receives, so a wrong one survives to the end host that finally reads the quote. Hence an oracle built from scratch: the tests compare against a fixture that computes these checksums rather than folding deltas into them. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/icmp_handler/icmp_error_msg.rs | 69 ++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/nat/src/icmp_handler/icmp_error_msg.rs b/nat/src/icmp_handler/icmp_error_msg.rs index 4098e67c46..52ef7fd486 100644 --- a/nat/src/icmp_handler/icmp_error_msg.rs +++ b/nat/src/icmp_handler/icmp_error_msg.rs @@ -86,6 +86,8 @@ pub(crate) fn nat_translate_icmp_inner_src( ) .map_err(|_| IcmpErrorMsgError::InvalidIpVersion)?; + fold_inner_address(embedded_headers, old_addr, target_addr); + let Some(target_port) = target_port else { // No port to translate, we're done return Ok(()); @@ -117,6 +119,19 @@ fn quoted_version(addr: IpAddr) -> EmbeddedIpVersion { } } +fn fold_inner_address(embedded_headers: &mut H, old_addr: IpAddr, new_addr: IpAddr) +where + H: TryEmbeddedTransportMut + ?Sized, +{ + if old_addr == new_addr { + return; + } + let Some(transport) = embedded_headers.try_embedded_transport_mut() else { + return; + }; + transport.update_checksum_for_address(old_addr, new_addr); +} + pub(crate) fn nat_translate_icmp_inner_dst( packet: &mut Packet, target_addr: IpAddr, @@ -134,6 +149,8 @@ pub(crate) fn nat_translate_icmp_inner_dst( .try_set_destination(target_addr) .map_err(|_| IcmpErrorMsgError::InvalidIpVersion)?; + fold_inner_address(embedded_headers, old_addr, target_addr); + let Some(target_port) = target_port else { // No port to translate, we're done return Ok(()); @@ -542,6 +559,58 @@ mod quoted_transport_checksum { NatPort::new_port(NonZero::new(value).unwrap_or_else(|| unreachable!())) } + #[test] + fn an_address_only_rewrite_reaches_a_quoted_tcp_checksum() { + let mut translated = quote_v4(INNER_SRC, INNER_DST, NextHeader::TCP, OLD_PORT, PEER_PORT); + nat_translate_icmp_inner_src(&mut translated, IpAddr::V4(NAT_SRC), None) + .unwrap_or_else(|_| unreachable!()); + + let built = quote_v4(NAT_SRC, INNER_DST, NextHeader::TCP, OLD_PORT, PEER_PORT); + assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); + } + + #[test] + fn an_address_and_port_rewrite_reaches_a_quoted_udp_checksum() { + let mut translated = quote_v4(INNER_SRC, INNER_DST, NextHeader::UDP, OLD_PORT, PEER_PORT); + nat_translate_icmp_inner_src(&mut translated, IpAddr::V4(NAT_SRC), Some(port(NEW_PORT))) + .unwrap_or_else(|_| unreachable!()); + + let built = quote_v4(NAT_SRC, INNER_DST, NextHeader::UDP, NEW_PORT, PEER_PORT); + assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); + } + + #[test] + fn a_destination_rewrite_reaches_a_quoted_tcp_checksum() { + let mut translated = quote_v4(INNER_SRC, INNER_DST, NextHeader::TCP, OLD_PORT, PEER_PORT); + nat_translate_icmp_inner_dst(&mut translated, IpAddr::V4(NAT_DST), Some(port(NEW_PORT))) + .unwrap_or_else(|_| unreachable!()); + + let built = quote_v4(INNER_SRC, NAT_DST, NextHeader::TCP, OLD_PORT, NEW_PORT); + assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); + } + + #[test] + fn an_ipv6_address_rewrite_reaches_a_quoted_tcp_checksum() { + let mut translated = quote_v6( + INNER_SRC_V6, + INNER_DST_V6, + NextHeader::TCP, + OLD_PORT, + PEER_PORT, + ); + nat_translate_icmp_inner_src(&mut translated, IpAddr::V6(NAT_SRC_V6), None) + .unwrap_or_else(|_| unreachable!()); + + let built = quote_v6( + NAT_SRC_V6, + INNER_DST_V6, + NextHeader::TCP, + OLD_PORT, + PEER_PORT, + ); + assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); + } + #[test] fn a_disabled_ipv4_udp_quote_checksum_stays_disabled() { let mut packet = quote_v4(INNER_SRC, INNER_DST, NextHeader::UDP, OLD_PORT, PEER_PORT); From 02a2974706c3bf51768348dd1268957cd4a3f138 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 26 Aug 2026 12:49:44 -0600 Subject: [PATCH 17/22] fix(nat): Store the checksum a quoted identifier translation computes The same dropped return value as `EmbeddedTransport::update_checksum`, at a site that path does not run through: an ICMP quote's identifier is translated on the header directly, so it kept its own copy of the bug after the shared one was fixed. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/icmp_handler/icmp_error_msg.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/nat/src/icmp_handler/icmp_error_msg.rs b/nat/src/icmp_handler/icmp_error_msg.rs index 52ef7fd486..4ee358404b 100644 --- a/nat/src/icmp_handler/icmp_error_msg.rs +++ b/nat/src/icmp_handler/icmp_error_msg.rs @@ -189,11 +189,12 @@ where // No checksum to update, we're done return; }; - let _ = icmp.increment_update_checksum( + let updated = icmp.increment_update_checksum( T::Checksum::from(current_checksum), old_identifier, new_identifier, ); + let _ = icmp.set_checksum(updated); } fn translate_inner_tcp_udp_src( @@ -611,6 +612,20 @@ mod quoted_transport_checksum { assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); } + #[test] + fn an_identifier_rewrite_reaches_a_quoted_icmp_checksum() { + let mut translated = quote_v4(INNER_SRC, INNER_DST, NextHeader::ICMP, OLD_ID, PEER_PORT); + nat_translate_icmp_inner_src( + &mut translated, + IpAddr::V4(NAT_SRC), + Some(NatPort::Identifier(NEW_ID)), + ) + .unwrap_or_else(|_| unreachable!()); + + let built = quote_v4(NAT_SRC, INNER_DST, NextHeader::ICMP, NEW_ID, PEER_PORT); + assert_eq!(quoted_checksum(&translated), quoted_checksum(&built)); + } + #[test] fn a_disabled_ipv4_udp_quote_checksum_stays_disabled() { let mut packet = quote_v4(INNER_SRC, INNER_DST, NextHeader::UDP, OLD_PORT, PEER_PORT); From 7524397b2f65d1eeede074382cc3cdf1586c0989 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 26 Aug 2026 13:00:23 -0600 Subject: [PATCH 18/22] fix(nat): Arbitrate the race to create a masquerade flow `insert_if_absent` was added with no caller, so the race it describes was still open: two packets of one new flow, handled at the same moment on two cores, each allocated a public tuple and each installed a pair. The second displaced the first's forward half, and the first's reverse half -- keyed on an allocation nothing else can collide with -- stayed live, translating for a tuple that goes back to the pool with the half that was displaced and is then handed to somebody else. The loser keeps nothing. Its reverse half is never inserted, and dropping the pair it built releases the allocation through `AllocatedPort`, so there is no hand-written release path to keep in step. It masquerades with the winner's flow, which is the answer the packet after it would have got anyway. Re-checking and teardown stay with whoever installed the flow. A flow won by another packet was built from an allocator handle of its own, and judging it by this packet's could invalidate a sound flow over an allocator it never used. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/masquerade/nf.rs | 97 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/nat/src/masquerade/nf.rs b/nat/src/masquerade/nf.rs index ce08ddedc3..57540d8d32 100644 --- a/nat/src/masquerade/nf.rs +++ b/nat/src/masquerade/nf.rs @@ -732,3 +732,100 @@ mod tests { assert_eq!(udp.destination(), UdpPort::try_from(1234).unwrap()); } } + +#[cfg(test)] +mod race { + use super::*; + use crate::masquerade::probe::Fabric; + use crate::static_nat::probe::{build, vni}; + use config::external::overlay::vpcpeering::VpcExpose; + use config::external::overlay::vpcpeering::contract::{LOCAL_VNI, REMOTE_VNI}; + use lpm::prefix::PrefixWithOptionalPorts; + use net::buffer::TestBuffer; + + fn prefix(spec: &str) -> PrefixWithOptionalPorts { + PrefixWithOptionalPorts::new(spec.parse().unwrap_or_else(|_| unreachable!()), None) + } + + fn fabric() -> Fabric { + let exposes = vec![ + VpcExpose::empty() + .make_masquerade(None) + .unwrap_or_else(|e| unreachable!("{e}")) + .ip(prefix("10.0.0.0/24")) + .as_range(prefix("172.16.0.0/24")) + .unwrap_or_else(|e| unreachable!("{e}")), + ]; + Fabric::build(&exposes).unwrap_or_else(|| unreachable!("a fixed expose builds")) + } + + #[tokio::test] + async fn the_loser_of_a_race_masquerades_with_the_winners_flow() { + let fabric = fabric(); + let (_lookup, masq) = fabric.stages(); + let allocator = masq + .allocator + .get() + .unwrap_or_else(|| unreachable!("the fabric installed an allocator")); + + let source: IpAddr = "10.0.0.1".parse().unwrap_or_else(|_| unreachable!()); + let destination: IpAddr = "3.3.3.1".parse().unwrap_or_else(|_| unreachable!()); + let mut packet: Packet = build(source, destination, false, 4000, 80); + let meta = packet.meta_mut(); + meta.set_overlay(true); + meta.set_masquerade(true); + meta.src_vpcd = Some(VpcDiscriminant::from_vni(vni(LOCAL_VNI))); + meta.dst_vpcd = Some(VpcDiscriminant::from_vni(vni(REMOTE_VNI))); + + let key = FlowKey::try_from(&packet).unwrap_or_else(|_| unreachable!("the probe keys")); + let (src_vpcd, dst_vpcd) = + Masquerade::discriminants(&packet).unwrap_or_else(|_| unreachable!()); + let genid = allocator.genid(); + + let allocate = || { + allocator + .allocate(src_vpcd, dst_vpcd, key.src_ip(), key.proto()) + .unwrap_or_else(|e| unreachable!("the pool has room: {e}")) + }; + let winning = allocate(); + let losing = allocate(); + let losing_tuple = (losing.allocation.ip(), losing.allocation.port()); + assert_ne!( + (winning.allocation.ip(), winning.allocation.port()), + losing_tuple, + "the two racers drew the same tuple, so this races nothing" + ); + let losing_reverse = Masquerade::new_reverse_session(&key, &losing, dst_vpcd) + .unwrap_or_else(|e| unreachable!("{e}")); + + let winner = masq + .create_flow_pair(&mut packet, &key, &key, winning, genid) + .unwrap_or_else(|e| unreachable!("{e}")); + let MasqueradeFlow::Installed(winner) = winner else { + unreachable!("the first packet did not install the flow"); + }; + + let outcome = masq + .create_flow_pair(&mut packet, &key, &key, losing, genid) + .unwrap_or_else(|e| unreachable!("{e}")); + let MasqueradeFlow::Held(held) = outcome else { + panic!("the second packet installed a pair of its own over a live flow"); + }; + assert!( + Arc::ptr_eq(&held, &winner), + "the loser was handed a flow that is not the one holding the key" + ); + + assert!( + masq.flow_table.lookup(&losing_reverse).is_none(), + "the loser's reverse half was left in the table, mapping a tuple about to be reused" + ); + + let next = allocate(); + assert_eq!( + (next.allocation.ip(), next.allocation.port()), + losing_tuple, + "the loser's allocation never went back to the pool" + ); + } +} From 753d139cc0a3ec2438f57b85a13fd66d76fbaf41 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Wed, 26 Aug 2026 13:03:31 -0600 Subject: [PATCH 19/22] fix(nat): Arbitrate the race to create a port-forwarding flow No concurrency is needed to reach this one. `process_packet` takes the slow path on the stamp alone -- `get_packet_port_fw_state` reads what `FlowLookup` attached and never falls back to the table, unlike masquerade -- and a burst is stamped before any of it is forwarded. Two packets of one new flow in a single burst therefore both build a pair, and the second tears the first's down. Sequentially that ends consistent, which is why it went unseen: the second pair carries the same translation, so a connection that has been rebuilt underneath its own first packets forwards exactly like one that was left alone. Two cores end it worse. Displacing a flow invalidates its partner, so a loser's reverse insert landing after the winner's strikes down the winner's forward half and then stays live under it -- a pair that answers replies it can no longer forward. Signed-off-by: Daniel Noland Co-Authored-By: Claude Opus 5 (1M context) --- nat/src/portfw/nf.rs | 84 +++++++++++++++++++++++++++++++++++++++++ nat/src/portfw/probe.rs | 4 ++ 2 files changed, 88 insertions(+) diff --git a/nat/src/portfw/nf.rs b/nat/src/portfw/nf.rs index 8c538cf38e..b231deb9be 100644 --- a/nat/src/portfw/nf.rs +++ b/nat/src/portfw/nf.rs @@ -410,3 +410,87 @@ impl NetworkFunction for PortForwarder { self.pipeline_data = data; } } + +#[cfg(test)] +mod race { + use super::*; + use crate::portfw::probe::{Arrival, Fabric}; + use crate::static_nat::probe::build; + use config::external::overlay::vpcpeering::VpcExpose; + use lpm::prefix::{L4Protocol, PortRange, PrefixWithOptionalPorts}; + use net::buffer::TestBuffer; + use pipeline::NetworkFunction; + use std::net::IpAddr; + + fn side(prefix: &str, first: u16, last: u16) -> PrefixWithOptionalPorts { + PrefixWithOptionalPorts::new( + prefix.parse().unwrap_or_else(|_| unreachable!()), + Some(PortRange::new(first, last).unwrap_or_else(|_| unreachable!())), + ) + } + + fn fabric() -> Fabric { + let expose = VpcExpose::empty() + .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .unwrap_or_else(|e| unreachable!("{e}")) + .ip(side("10.0.0.0/30", 9000, 9003)) + .as_range(side("172.16.0.0/30", 8000, 8003)) + .unwrap_or_else(|e| unreachable!("{e}")); + Fabric::build(&[expose]).unwrap_or_else(|| unreachable!("a fixed expose builds")) + } + + fn entries(fabric: &Fabric) -> Vec { + let mut ids: Vec = fabric + .flows() + .snapshot(|_, _| true) + .map(|flow| Arc::as_ptr(&flow) as usize) + .collect(); + ids.sort_unstable(); + ids + } + + #[tokio::test] + async fn a_second_packet_of_one_burst_keeps_the_first_packets_flow() { + let fabric = fabric(); + let (mut lookup, mut pfw) = fabric.stages(); + let arrival = Arrival::inbound(); + + let peer: IpAddr = "3.3.3.1".parse().unwrap_or_else(|_| unreachable!()); + let published: IpAddr = "172.16.0.1".parse().unwrap_or_else(|_| unreachable!()); + let packet = || { + let mut packet: Packet = build(peer, published, true, 1234, 8001); + arrival.stamp(&mut packet); + packet + }; + + let mut stamped = lookup.process(vec![packet(), packet()].into_iter()); + let mut first = stamped.next().unwrap_or_else(|| unreachable!()); + let mut second = stamped.next().unwrap_or_else(|| unreachable!()); + drop(stamped); + for packet in [&mut first, &mut second] { + packet.meta_mut().dst_vpcd = arrival.dst_vpcd.map(VpcDiscriminant::from_vni); + } + + pfw.process(std::iter::once(first)).for_each(drop); + let installed = entries(&fabric); + assert_eq!( + installed.len(), + 2, + "the first packet of the burst did not install a pair" + ); + + pfw.process(std::iter::once(second)).for_each(drop); + assert_eq!( + entries(&fabric), + installed, + "the second packet of the burst replaced the pair the first one installed" + ); + assert!( + fabric + .flows() + .snapshot(|_, _| true) + .all(|flow| flow.is_active()), + "the burst left a half of the pair no longer live" + ); + } +} diff --git a/nat/src/portfw/probe.rs b/nat/src/portfw/probe.rs index 60f88c9565..c98a3852f3 100644 --- a/nat/src/portfw/probe.rs +++ b/nat/src/portfw/probe.rs @@ -161,6 +161,10 @@ impl Fabric { !self.rules.is_empty() } + pub(crate) fn flows(&self) -> &Arc { + &self.flow_table + } + pub(crate) fn is_private(&self, addr: IpAddr, port: u16) -> bool { self.rules .iter() From 094c896d7753f07bf97f054ffbdb669e6f709e5b Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Thu, 27 Aug 2026 01:53:06 -0600 Subject: [PATCH 20/22] fix(nat): Match on the protocol the packet carries `fix(acl-filter,flow-filter): Match on the protocol the packet carries` converted the two filters and left `nat`. For IPv6 the IP header's next-header field names the first extension header rather than the transport, so: * `is_port_forwardable` is false for a TCP packet with one Hop-by-Hop header in front of it, and the packet is refused as unsupported traffic; * `PortFwKey` names the extension header, so the forward and reverse rule lookups find nothing. Between them that is a published service being unreachable over IPv6 from any sender that inserts one extension header -- and the sender chooses. It is the same failure the `flow-filter` half of that commit describes: dropping traffic a protocol-restricted expose was configured to carry. `next_flow_status` had the same read behind a different argument: the old comment defended it on the grounds that transport headers may be absent with fragmentation. `upper_layer_proto` walks the chain, fragment header included, so it answers that rather than trading it away; the fallback keeps the old answer for a chain that ran past MAX_NET_EXTENSIONS. `is_icmp` keeps its IP-version check alongside the protocol, so ICMPv4 in IPv6 stays unsupported rather than becoming translatable. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Daniel Noland --- nat/src/masquerade/protocol.rs | 7 +++---- nat/src/portfw/nf.rs | 4 ++-- nat/src/portfw/packet.rs | 24 +++++++++++++----------- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/nat/src/masquerade/protocol.rs b/nat/src/masquerade/protocol.rs index 800159e832..aa2dde30e0 100644 --- a/nat/src/masquerade/protocol.rs +++ b/nat/src/masquerade/protocol.rs @@ -120,12 +120,11 @@ pub(crate) fn next_flow_status( action: NatAction, // action of the flow hit status: NatFlowStatus, // current status ) -> NatFlowStatus { + let ip = packet.try_ip().unwrap_or_else(|| unreachable!()); let proto = packet - .try_ip() - .unwrap_or_else(|| unreachable!()) // packet without IP hdr should not make it here - .next_header(); + .upper_layer_proto() + .unwrap_or_else(|| ip.next_header()); - // match on next-header, instead of relying on headers, as those may not be present w/ fragmentation match proto { NextHeader::UDP => next_flow_status_udp(action, status).udp_status_patch(packet, action), NextHeader::ICMP | NextHeader::ICMP6 => next_flow_status_icmp(action, status), diff --git a/nat/src/portfw/nf.rs b/nat/src/portfw/nf.rs index b231deb9be..26450f3e7d 100644 --- a/nat/src/portfw/nf.rs +++ b/nat/src/portfw/nf.rs @@ -221,8 +221,8 @@ impl PortForwarder { ) -> Option> { // These could be retrieved from the FlowKey, but we don't have it :( ... let src_vpcd = packet.meta().src_vpcd?; + let proto = packet.upper_layer_proto()?; let net = packet.try_ip()?; - let proto = net.next_header(); let dst_ip = net.dst_addr(); let dst_port = packet.transport_dst_port()?; let key = PortFwKey::new(src_vpcd, proto); @@ -261,8 +261,8 @@ impl PortForwarder { ) -> Option> { // get required properties from packet let src_vpcd = packet.meta().src_vpcd?; + let proto = packet.upper_layer_proto()?; let net = packet.try_ip()?; - let proto = net.next_header(); let src_ip = net.src_addr(); let src_port = packet.transport_src_port()?; diff --git a/nat/src/portfw/packet.rs b/nat/src/portfw/packet.rs index 6d3e74a96f..f5292c2c97 100644 --- a/nat/src/portfw/packet.rs +++ b/nat/src/portfw/packet.rs @@ -27,16 +27,16 @@ pub(crate) enum NatPacketError { } #[inline] -fn is_port_forwardable(net: &Net) -> bool { - matches!(net.next_header(), NextHeader::UDP | NextHeader::TCP) +fn is_port_forwardable(proto: Option) -> bool { + matches!(proto, Some(NextHeader::UDP | NextHeader::TCP)) } #[inline] -fn is_icmp(net: &Net) -> bool { - match net { - Net::Ipv4(ipv4) => ipv4.next_header() == NextHeader::ICMP, - Net::Ipv6(ipv6) => ipv6.next_header() == NextHeader::ICMP6, - } +fn is_icmp(proto: Option, net: &Net) -> bool { + matches!( + (proto, net), + (Some(NextHeader::ICMP), Net::Ipv4(_)) | (Some(NextHeader::ICMP6), Net::Ipv6(_)) + ) } #[inline] @@ -52,6 +52,7 @@ fn snat_packet( ); let mut modified = false; + let proto = packet.upper_layer_proto(); match packet .headers_mut() .pat_mut() @@ -61,7 +62,7 @@ fn snat_packet( .done() { // traffic can be port forwarded: it's Ip + UDP/TCP - Some((_, ip, tp)) if is_port_forwardable(ip) => { + Some((_, ip, tp)) if is_port_forwardable(proto) => { if ip.src_addr() != new_src_ip.inner() { ip.try_set_source(new_src_ip)?; modified = true; @@ -74,7 +75,7 @@ fn snat_packet( } } // needed for ICMP error handling - Some((_, ip, Transport::Icmp4(_) | Transport::Icmp6(_))) if is_icmp(ip) => { + Some((_, ip, Transport::Icmp4(_) | Transport::Icmp6(_))) if is_icmp(proto, ip) => { if ip.src_addr() != new_src_ip.inner() { ip.try_set_source(new_src_ip)?; modified = true; @@ -104,6 +105,7 @@ fn dnat_packet( ); let mut modified = false; + let proto = packet.upper_layer_proto(); match packet .headers_mut() .pat_mut() @@ -112,7 +114,7 @@ fn dnat_packet( .transport() .done() { - Some((_, ip, tp)) if is_port_forwardable(ip) => { + Some((_, ip, tp)) if is_port_forwardable(proto) => { if ip.dst_addr() != new_dst_ip { ip.try_set_destination(new_dst_ip)?; modified = true; @@ -125,7 +127,7 @@ fn dnat_packet( } } // needed for ICMP error handling - Some((_, ip, Transport::Icmp4(_) | Transport::Icmp6(_))) if is_icmp(ip) => { + Some((_, ip, Transport::Icmp4(_) | Transport::Icmp6(_))) if is_icmp(proto, ip) => { if ip.dst_addr() != new_dst_ip { ip.try_set_destination(new_dst_ip)?; modified = true; From 824b6d60ed4d3f8240835ed60579d3c864c49b4b Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Thu, 27 Aug 2026 20:07:25 -0600 Subject: [PATCH 21/22] fix(nat): Key the fresh port-forwarding lookup on the carried protocol too `Match on the protocol the packet carries` converted `get_rule_from_pkt_fw_path` and `_rev_path` and left `can_be_port_forwarded`, which is the path a flow takes before it has any state -- so the failure that commit describes survived in the place it does the most damage. `.net().transport()` is permissive about extension headers, so an IPv6 TCP packet behind a Hop-by-Hop header reaches the qualifying arm and is keyed on Hop-by-Hop. No rule matches, and a published service is unreachable over IPv6 from any sender that inserts one extension header -- which the sender chooses. The regression test asserts on the flow pair rather than the translation, because the pair only exists if a rule was found. Signed-off-by: Daniel Noland --- nat/Cargo.toml | 2 +- nat/src/portfw/nf.rs | 75 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 3 deletions(-) diff --git a/nat/Cargo.toml b/nat/Cargo.toml index 59cf71c42c..be0cfdcd58 100644 --- a/nat/Cargo.toml +++ b/nat/Cargo.toml @@ -42,7 +42,7 @@ config = { workspace = true, features = ["bolero"] } fixin = { workspace = true } test-utils = { workspace = true } lpm = { workspace = true, features = ["testing"] } -net = { workspace = true, features = ["bolero"] } +net = { workspace = true, features = ["bolero", "builder"] } tokio = { workspace = true, features = ["macros", "rt", "test-util", "time"] } tracectl = { workspace = true } diff --git a/nat/src/portfw/nf.rs b/nat/src/portfw/nf.rs index 26450f3e7d..0e7f12f4b9 100644 --- a/nat/src/portfw/nf.rs +++ b/nat/src/portfw/nf.rs @@ -61,7 +61,12 @@ impl PortForwarder { return None; }; - if let Some((dst_ip, dst_port, proto)) = + let Some(proto) = packet.upper_layer_proto() else { + debug!("Ignoring packet: header chain was never walked to a transport"); + return None; + }; + + if let Some((dst_ip, dst_port)) = match packet.headers().pat().eth().net().transport().done() { Some((_, _net, Transport::Tcp(tcp))) if !tcp.is_first_segment() => { debug!("Ignoring TCP packet: it has no SYN and we have no state for it"); @@ -73,7 +78,7 @@ impl PortForwarder { { if let Ok(dst_ip) = UnicastIpAddr::try_from(dst_ip) { debug!("Packet qualifies for port-forwarding"); - Some((dst_ip, dst_port, net.next_header())) + Some((dst_ip, dst_port)) } else { debug!("Ignoring packet: destination IP is not unicast"); None @@ -439,6 +444,50 @@ mod race { Fabric::build(&[expose]).unwrap_or_else(|| unreachable!("a fixed expose builds")) } + fn v6_fabric() -> Fabric { + let expose = VpcExpose::empty() + .make_port_forwarding(None, Some(L4Protocol::Tcp)) + .unwrap_or_else(|e| unreachable!("{e}")) + .ip(side("2001:db8::/126", 9000, 9003)) + .as_range(side("2001:db8:1::/126", 8000, 8003)) + .unwrap_or_else(|e| unreachable!("{e}")); + Fabric::build(&[expose]).unwrap_or_else(|| unreachable!("a fixed expose builds")) + } + + fn v6_hop_by_hop_syn(src: &str, dst: &str, sport: u16, dport: u16) -> Packet { + use net::eth::ethtype::EthType; + use net::headers::builder::HeaderStack; + use net::ipv6::UnicastIpv6Addr; + use net::packet::test_utils::make_default_for_eth; + use net::parse::DeParse; + use net::tcp::port::TcpPort; + + let headers = HeaderStack::new() + .eth(|eth| *eth = make_default_for_eth(EthType::IPV6)) + .ipv6(|ip| { + ip.set_source( + UnicastIpv6Addr::new(src.parse().unwrap_or_else(|_| unreachable!())) + .unwrap_or_else(|_| unreachable!()), + ); + ip.set_destination(dst.parse().unwrap_or_else(|_| unreachable!())); + ip.set_hop_limit(64); + }) + .hop_by_hop(|_| {}) + .tcp(|tcp| { + tcp.set_source(TcpPort::try_from(sport).unwrap_or_else(|_| unreachable!())); + tcp.set_destination(TcpPort::try_from(dport).unwrap_or_else(|_| unreachable!())); + tcp.set_syn(true); + }) + .build_headers() + .unwrap_or_else(|e| unreachable!("{e:?}")); + + let mut buffer: TestBuffer = TestBuffer::new(); + headers + .deparse(buffer.as_mut()) + .unwrap_or_else(|e| unreachable!("{e:?}")); + Packet::new(buffer).unwrap_or_else(|_| unreachable!("the fixture parses")) + } + fn entries(fabric: &Fabric) -> Vec { let mut ids: Vec = fabric .flows() @@ -449,6 +498,28 @@ mod race { ids } + #[tokio::test] + async fn a_fresh_flow_behind_an_extension_header_is_forwarded() { + let fabric = v6_fabric(); + let (mut lookup, mut pfw) = fabric.stages(); + let arrival = Arrival::inbound(); + + let mut packet = v6_hop_by_hop_syn("2001:db8:ffff::1", "2001:db8:1::1", 1234, 8001); + arrival.stamp(&mut packet); + + let mut stamped = lookup.process(std::iter::once(packet)); + let mut packet = stamped.next().unwrap_or_else(|| unreachable!()); + drop(stamped); + packet.meta_mut().dst_vpcd = arrival.dst_vpcd.map(VpcDiscriminant::from_vni); + + pfw.process(std::iter::once(packet)).for_each(drop); + assert_eq!( + entries(&fabric).len(), + 2, + "a TCP SYN behind one Hop-by-Hop header did not match its port-forwarding rule" + ); + } + #[tokio::test] async fn a_second_packet_of_one_burst_keeps_the_first_packets_flow() { let fabric = fabric(); From b2926305eaa2f184e95c55f89dea0108488bf4a5 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Thu, 27 Aug 2026 20:08:29 -0600 Subject: [PATCH 22/22] style(nat): Reach Weak through the concurrency facade Signed-off-by: Daniel Noland --- nat/src/masquerade/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nat/src/masquerade/test.rs b/nat/src/masquerade/test.rs index 4124198538..7e3787bf2e 100644 --- a/nat/src/masquerade/test.rs +++ b/nat/src/masquerade/test.rs @@ -9,7 +9,7 @@ use crate::masquerade::{MasqueradeConfig, NatAllocatorWriter}; use crate::{IcmpErrorHandler, Masquerade, NatPort}; use ahash::HashMap; use common::cliprovider::Frame; -use concurrency::sync::Arc; +use concurrency::sync::{Arc, Weak}; use config::GenId; use config::external::overlay::Overlay; use config::external::overlay::vpc::{Vpc, VpcTable}; @@ -2476,7 +2476,7 @@ async fn an_icmp_error_does_not_tear_down_the_query_session_it_reports_on() { flow.is_active(), "the Query session was torn down by the Error message reporting on it" ); - let related = flow.related.as_ref().and_then(std::sync::Weak::upgrade); + let related = flow.related.as_ref().and_then(Weak::upgrade); assert!( related.is_none_or(|related| related.is_active()), "the Query session's reverse flow was torn down by the Error message"