diff --git a/docs/plans/404-egress-return-icmpv6-handling.md b/docs/plans/404-egress-return-icmpv6-handling.md new file mode 100644 index 0000000..d8b0aa4 --- /dev/null +++ b/docs/plans/404-egress-return-icmpv6-handling.md @@ -0,0 +1,459 @@ +# Implementation Plan — Translate ICMPv6 Egress Replies Instead of Dropping Them + +- **Issue:** [datum-cloud/galactic#404](https://github.com/datum-cloud/galactic/issues/404) — "Egress + replies that are not TCP or UDP are dropped, including path MTU discovery." +- **Applies to:** `internal/plumbing/ebpf/edgeprog/edgenat.c`'s egress (masquerade) datapath, added by + the still-open, still-unmerged `feat/865-egress-phase-b` branch (galactic#381) — **not yet on + `main`**. See §7 for why this changes where the fix should land. +- **Status:** planning only — no implementation started. + +## 1. Issue recap + +`handle_egress_return` (the branch that handles internet-originated replies addressed to a gateway +node's public `masq_addr`) claims that address and drops anything that isn't TCP or UDP: + +```c +if (ip6->nexthdr != EDGE_IPPROTO_TCP && ip6->nexthdr != EDGE_IPPROTO_UDP) { + count_drop(DROP_REASON_MALFORMED_EGRESS_RETURN); + return XDP_DROP; +} +``` + +`masq_addr` is reachable from the entire internet, and the internet routinely sends ICMPv6 to it: +Destination Unreachable, Packet Too Big (the PMTUD message), Time Exceeded, and Echo Reply all arrive +this way, and all of them are dropped today. Per the issue and the author's own review comment (#381), +this was a known, called-out deferral — `handle_egress_forward` (the tenant-outbound side) has the +identical restriction and the same review comment flags it as "the tenant-outbound side of the same +restriction" — but nothing tracked closing either half. + +Two consequences called out as the reason this matters: + +- **Packet Too Big being dropped breaks PMTUD.** A tenant connection crossing a smaller-MTU link + anywhere on the path stalls instead of adapting, on large transfers only, intermittently — one of the + hardest failure classes to attribute back to its actual cause. +- **Echo Reply being dropped breaks the simplest reachability check a tenant can run** (`ping` from + inside their own workload), which reads as "the network is broken" long before ansyone suspects the + gateway. + +The review comment also flags two smaller items to fold in: `DROP_REASON_MALFORMED_EGRESS_RETURN` is +the wrong reason name for a well-formed ICMPv6 packet (it's a protocol-policy decision, not a parse +failure), and translating ICMP errors back to the originating tenant requires parsing the embedded +original datagram to recover the masqueraded port, since ICMPv6 has no ports of its own to key +`egress_conn_table` on. + +## 2. Current behavior (read from `feat/865-egress-phase-b`) + +`handle_egress_return` (`edgenat.c`, currently ~line 1227) unconditionally requires +`ip6->nexthdr == EDGE_IPPROTO_TCP || EDGE_IPPROTO_UDP` before doing anything else, counting +`DROP_REASON_MALFORMED_EGRESS_RETURN` and dropping otherwise. `handle_egress_forward` (currently +~line 1074) has the mirrored restriction on the inner (post-decap) packet, counting +`DROP_REASON_MALFORMED_EGRESS_FORWARD`. Both reach the top-level `edge_nat()` dispatcher unconditionally +for their respective claimed addresses (`masq_addr`, `egress_sid`) — there is no protocol filtering +before either function is called, only inside them. + +`egress_conn_table`'s existing reverse-direction key — `(proto, dest_addr:dest_port → +masq_addr:masq_port)` — is exactly what a TCP/UDP reply is looked up by (§3.2/§3.3 of +`docs/plans/865-edge-gateway-nat66-egress.md`). This plan's core insight is that both new ICMPv6 cases +can reuse that same key shape without any map or struct change: + +- An **ICMPv6 error message** (Destination Unreachable/Packet Too Big/Time Exceeded/Parameter Problem) + embeds the IPv6 header and (per RFC 4443) at least the first 8 bytes of the transport header of the + packet that triggered it — which, for a packet this program itself SNAT'd on the way out, is + `masq_addr:masq_port → dest_addr:dest_port`, read one layer deeper than a direct TCP/UDP reply. +- An **ICMPv6 Echo Reply** has no ports at all, but its Identifier field plays the same role a + port does for every other conntrack implementation (Linux's `nf_conntrack` ICMP tracker does the + same) — so `handle_egress_forward` needs a matching change on the way out: mask the Identifier the + same way it already masks the source port for TCP/UDP. + +## 3. Fix + +### 3.1 New wire constants and header structs (`edgenat.c`) + +Alongside `EDGE_IPPROTO_TCP`/`EDGE_IPPROTO_UDP`: + +```c +#define EDGE_IPPROTO_ICMPV6 58 + +#define EDGE_ICMPV6_DEST_UNREACH 1 +#define EDGE_ICMPV6_PACKET_TOO_BIG 2 +#define EDGE_ICMPV6_TIME_EXCEEDED 3 +#define EDGE_ICMPV6_PARAM_PROBLEM 4 +#define EDGE_ICMPV6_ECHO_REQUEST 128 +#define EDGE_ICMPV6_ECHO_REPLY 129 +``` + +Alongside `edge_tcphdr`/`edge_udphdr` (packet-parsing structs, never map key/values — no `bpf2go +-type` exposure needed, same as the existing two): + +```c +// Common 4-byte prefix shared by every ICMPv6 message type this program +// reads (RFC 4443 §2.1). +struct edge_icmp6hdr { + __u8 type; + __u8 code; + __be16 check; +} __attribute__((packed)); + +// Destination Unreachable/Packet Too Big/Time Exceeded/Parameter Problem +// (RFC 4443 §3) share this 8-byte header shape -- the 4 bytes after +// checksum vary by type (unused for 1/3, MTU for 2, pointer for 4) and +// this program never reads them. What follows is "as much of the +// invoking packet as possible," guaranteed to include at least the +// embedded IPv6 header's first 48 bytes (RFC 4443 §2.4(c)) -- the full +// 40-byte IPv6 header plus the first 8 bytes of whatever transport +// header follows, which is where both TCP and UDP keep their two 16-bit +// port fields. +struct edge_icmp6_error_hdr { + __u8 type; + __u8 code; + __be16 check; + __u8 unused[4]; +} __attribute__((packed)); + +// Echo Request/Reply (RFC 4443 §4). identifier stands in for the port +// egress_conn_table is keyed by, for both this program's PAT-style +// re-mapping and the tenant's own kernel matching a reply back to the +// socket that sent the request -- see handle_egress_forward_icmp6 and +// handle_egress_return_icmp6_echo. +struct edge_icmp6_echo_hdr { + __u8 type; + __u8 code; + __be16 check; + __be16 identifier; + __be16 sequence; +} __attribute__((packed)); +``` + +### 3.2 New drop reasons, appended (safe — nothing on this branch has shipped) + +```c +enum edge_drop_reason { + ... // unchanged, 0-14 + DROP_REASON_MALFORMED_EGRESS_ICMP = 15, + DROP_REASON_NO_EGRESS_ICMP_CONN = 16, + DROP_REASON_COUNT = 17, +}; +``` + +This directly answers the review comment's "a distinct reason would pay for itself" note: an operator +reading drop counters can now tell a genuinely malformed ICMPv6 message (`MALFORMED_EGRESS_ICMP`) apart +from a well-formed one with no matching flow (`NO_EGRESS_ICMP_CONN`) apart from the pre-existing +TCP/UDP-specific `MALFORMED_EGRESS_RETURN`/`NO_EGRESS_RETURN_CONN`. No reuse of +`DROP_REASON_NO_EGRESS_CONN_NOT_SYN` for the ICMP forward-allocation path — every Echo Request may +start a new flow the same way "any UDP packet may start a new flow" already does, so that check never +applies to ICMP and no new reason is needed there. `DROP_REASON_EGRESS_PAT_EXHAUSTED` is reused as-is +for identifier-claim exhaustion (§3.4) — it's the same exhausted-probe condition regardless of which +field is being re-mapped. + +Mirror in `internal/plumbing/ebpf/edgeprog/dropreason.go` (hand-kept in sync, per that file's own doc +comment): add `DropReasonMalformedEgressICMP uint32 = 15`, `DropReasonNoEgressICMPConn uint32 = 16`, +bump `DropReasonCount` to `17`, and add both to `DropReasonNames` (`"malformed_egress_icmp"`, +`"no_egress_icmp_conn"`). + +No `go:generate`/`bpf2go -type` change needed — `edge_icmp6hdr`/`edge_icmp6_error_hdr`/ +`edge_icmp6_echo_hdr` are packet-parsing structs, not map key/value types, the same category +`edge_tcphdr`/`edge_udphdr` already fall into. Re-run `task ebpf:generate` after editing `edgenat.c` so +the compiled object picks up the widened `drop_reasons` `PERCPU_ARRAY` (`DROP_REASON_COUNT` grew). + +### 3.3 `handle_egress_return`: dispatch on protocol instead of gating on it + +```c +static EDGE_ALWAYS_INLINE int handle_egress_return(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + if (ip6->nexthdr == EDGE_IPPROTO_ICMPV6) + return handle_egress_return_icmp6(ctx, ip6, data_end); + + if (ip6->nexthdr != EDGE_IPPROTO_TCP && ip6->nexthdr != EDGE_IPPROTO_UDP) + // Some other protocol addressed to masq_addr -- e.g. Neighbor + // Discovery, or an Echo Request targeting this node's own + // public address directly rather than replying to a tenant + // flow. Not this program's to translate; hand it to the + // normal kernel stack instead of dropping it (mirrors step 1's + // "can't fully parse/match -> XDP_PASS", just decided + // per-protocol here since this address is otherwise claimed). + return XDP_PASS; + + /* ... existing TCP/UDP body, unchanged ... */ +} +``` + +`handle_egress_return_icmp6` reads just the shared 4-byte prefix, bounds-checks it, and dispatches +again by ICMPv6 type: + +```c +static EDGE_ALWAYS_INLINE int handle_egress_return_icmp6(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + struct edge_icmp6hdr *icmp6 = (void *) (ip6 + 1); + if ((void *) (icmp6 + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + if (icmp6->type == EDGE_ICMPV6_ECHO_REPLY) + return handle_egress_return_icmp6_echo(ctx, ip6, data_end); + + if (icmp6->type == EDGE_ICMPV6_DEST_UNREACH || icmp6->type == EDGE_ICMPV6_PACKET_TOO_BIG || + icmp6->type == EDGE_ICMPV6_TIME_EXCEEDED || icmp6->type == EDGE_ICMPV6_PARAM_PROBLEM) + return handle_egress_return_icmp6_error(ctx, ip6, data_end); + + // Router Advertisement, Neighbor Solicitation/Advertisement, an Echo + // Request targeting masq_addr directly, ... -- not a reply to any + // tenant flow this program tracks. XDP_PASS, not XDP_DROP. + return XDP_PASS; +} +``` + +### 3.4 Echo Reply: identifier as the pseudo-port, both directions + +**Forward (tenant → internet), `handle_egress_forward`:** currently the inner (post-decap) packet is +rejected outright unless `nexthdr` is TCP/UDP. Split the existing TCP/UDP body into +`handle_egress_forward_l4` (unchanged logic, just factored out) and add a sibling +`handle_egress_forward_icmp6`, dispatched the same way `handle_egress_return` now is: + +```c +if (inner->nexthdr == EDGE_IPPROTO_ICMPV6) + return handle_egress_forward_icmp6(ctx, eth, inner, tenant_arg, backend_usid, data_end); +if (inner->nexthdr != EDGE_IPPROTO_TCP && inner->nexthdr != EDGE_IPPROTO_UDP) { + count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); + return XDP_DROP; +} +return handle_egress_forward_l4(ctx, eth, inner, tenant_arg, backend_usid, data_end); +``` + +`handle_egress_forward_icmp6` accepts only `EDGE_ICMPV6_ECHO_REQUEST` (anything else from a tenant +backend — Echo Reply, Router Solicitation, Neighbor Discovery — has no defined masquerade behavior and +is dropped, `DROP_REASON_MALFORMED_EGRESS_ICMP`, same "this address is claimed" reasoning as everywhere +else in this file). On a miss, it allocates exactly like `handle_egress_forward_l4`'s SNAT-port claim, +just keyed by `identifier` in both the forward key's `sport`/`dport` slots and the reverse key's, with +the same bounded linear-probe/`BPF_NOEXIST` claim over `egress_conn_table` — no new map, no new probe +technique, just the field being re-mapped is an identifier instead of a port. +`egress_conn_value.backend_port`/`dest_port`/`masq_port` are reused to hold the identifier for +`proto == EDGE_IPPROTO_ICMPV6` flows rather than adding dedicated fields — call this out with an +explicit comment on `struct egress_conn_value` (same reasoning the file already applies elsewhere: +"passing the same old/new value ... contributes zero diff," `fix_l4_checksum` doesn't care what a field +means, just that old/new pairs line up). + +The rewrite masquerades **both** the source address and the identifier (mirroring the SNAT-port +rewrite exactly, with identifier standing in for port), fixing the ICMPv6 checksum via the same +`fix_l4_checksum` helper (an address+word-pair checksum-diff, not a Full-NAT-specific one — any field +held equal old/new contributes zero delta, so passing `0` for the unused word-slot is safe, same +technique `handle_egress_forward_l4`'s own SNAT-only rewrite already uses). + +**Return (internet → tenant), `handle_egress_return_icmp6_echo`:** looks up `egress_conn_table` by the +reverse key built from `ip6->saddr`/`ip6->daddr`/`echo->identifier` (identifier in both the `sport` and +`dport` slots, matching what the forward allocation wrote) — a miss counts +`DROP_REASON_NO_EGRESS_ICMP_CONN` and drops. A hit un-masquerades **both** fields DNAT-style: rewrite +`ip6->daddr` from `masq_addr` to `cv->backend_addr`, and rewrite `echo->identifier` from the masqueraded +value back to `cv->backend_port` (the tenant's own original identifier, captured at allocation time, +before masquerading) — this is the piece easy to get wrong: leaving the identifier untouched would let +the destination-address rewrite succeed while the tenant's own ping process still doesn't recognize the +reply, because the identifier it sees would be the masqueraded one, not the one it originally sent. Fix +the checksum with the same `fix_l4_checksum` reuse, then `push_outer_header` toward +`cv->backend_usid` exactly like the existing TCP/UDP return path — no different tail shape. + +### 3.5 ICMPv6 errors: recover the flow from the embedded datagram + +`handle_egress_return_icmp6_error` is the piece with actual teeth (PMTUD). It must **not** reuse +`parse_l4` against the embedded transport header — `parse_l4` bounds-checks a full `struct edge_tcphdr` +(20 bytes), but RFC 4443 only guarantees the first 8 bytes of the invoking packet's transport header, and +a minimally-sized ICMPv6 error message legitimately won't have the rest. Both TCP's and UDP's source/dest +port fields sit in the first 4 bytes of either header shape, well within that guaranteed minimum, so add +a narrower helper that reads only those two fields: + +```c +// parse_embedded_ports reads the two 16-bit port fields both edge_tcphdr +// and edge_udphdr start with, bounds-checking only those 4 bytes -- +// deliberately not parse_l4, whose full-struct bounds check would reject +// a validly-minimal ICMPv6 error message's embedded TCP header (RFC 4443 +// guarantees only the first 8 bytes of the invoking transport header). +static EDGE_ALWAYS_INLINE int parse_embedded_ports(void *l4, void *data_end, __be16 *sport, __be16 *dport) +{ + __be16 *ports = l4; + if ((void *) (ports + 2) > data_end) + return -1; + *sport = ports[0]; + *dport = ports[1]; + return 0; +} +``` + +Then: + +```c +static EDGE_ALWAYS_INLINE int handle_egress_return_icmp6_error(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + struct edge_icmp6_error_hdr *err = (void *) (ip6 + 1); + struct edge_ip6hdr *embedded = (void *) (err + 1); + if ((void *) (embedded + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + if (embedded->nexthdr != EDGE_IPPROTO_TCP && embedded->nexthdr != EDGE_IPPROTO_UDP) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + __be16 embedded_sport, embedded_dport; + if (parse_embedded_ports((void *) (embedded + 1), data_end, &embedded_sport, &embedded_dport) != 0) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + // The embedded packet is masq_addr:masq_port -> dest_addr:dest_port + // -- exactly the packet handle_egress_forward last sent -- so this + // is the *same reverse key* a direct TCP/UDP reply is looked up by, + // just read one layer deeper. + struct egress_conn_key rev_key; + __builtin_memset(&rev_key, 0, sizeof(rev_key)); + rev_key.proto = embedded->nexthdr; + __builtin_memcpy(rev_key.saddr, embedded->daddr, 16); + rev_key.sport = embedded_dport; + __builtin_memcpy(rev_key.daddr, embedded->saddr, 16); + rev_key.dport = embedded_sport; + + struct egress_conn_value *cv = bpf_map_lookup_elem(&egress_conn_table, &rev_key); + if (!cv) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + // Two rewrites land on the *same* checksum (ICMPv6's own, which + // covers the whole message including the embedded bytes verbatim -- + // the embedded packet's own stale L4 checksum is untouched and never + // independently re-validated by anyone downstream): the outer + // packet's destination (masq_addr -> backend_addr, so it routes to + // the right worker node) and the embedded packet's own source + // address/port (masq_addr:masq_port -> backend_addr:backend_port), + // so the tenant's IP stack recognizes this error as belonging to a + // socket it actually opened. Both old values are masq_addr/masq_port + // by construction (this program's own earlier SNAT), so this is + // genuinely two separate memory locations converging on one value + // change apiece -- fix_l4_checksum's four word-slots don't have to + // mean "one address's source/dest" here, just "two old values, two + // new values, diffed together" (same generic reuse its own doc + // comment already licenses). + __u8 old_outer_daddr[16], old_embedded_saddr[16]; + __builtin_memcpy(old_outer_daddr, ip6->daddr, 16); + __builtin_memcpy(old_embedded_saddr, embedded->saddr, 16); + + fix_l4_checksum(&err->check, old_outer_daddr, old_embedded_saddr, 0, embedded_sport, + cv->backend_addr, cv->backend_addr, 0, cv->backend_port); + + __builtin_memcpy(ip6->daddr, cv->backend_addr, 16); + __builtin_memcpy(embedded->saddr, cv->backend_addr, 16); + __be16 *embedded_ports = (void *) (embedded + 1); + embedded_ports[0] = cv->backend_port; + + __u32 cfg_key = 0; + struct gw_config *cfg = bpf_map_lookup_elem(&gw_config_table, &cfg_key); + if (!cfg) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + __be16 inner_payload_len = ip6->payload_len; + if (push_outer_header(ctx, cfg->gw_addr, cv->backend_usid, inner_payload_len) != 0) + return XDP_DROP; + + return XDP_TX; +} +``` + +Note this reuses `struct egress_conn_key`/`egress_conn_value` and `egress_conn_table` completely +unmodified — no new map, matching the review comment's own framing ("that is the standard NAT +approach"). `Parameter Problem` (type 4) is folded into the same generic handler as the other three +error types even though the issue text only names Destination Unreachable/PTB/Time Exceeded by name — +it carries an embedded datagram the identical way and costs nothing extra to cover. + +## 4. Explicitly out of scope + +- **The FIB-lookup PMTUD gap** (`count_fib_drop`'s `DROP_REASON_FIB_FRAG_NEEDED` case, documented in + `docs/agents/ARCHITECTURE-GATEWAY.md`'s Known Constraints and shared with + `internal/plumbing/ebpf/prog/usid.c`) is a different problem — *generating* a fresh ICMPv6 Packet Too + Big when this gateway's own uplink route can't carry a packet, rather than *translating* one some + other router already generated. This plan only does the latter. Not touched here. +- **Anti-spoofing on the embedded datagram.** `handle_egress_return_icmp6_error` trusts the embedded + original packet's addresses/ports unconditionally once `egress_conn_table` confirms a matching flow + exists — consistent with this datapath's existing trust model (design plan §7 item 4 already flags a + dedicated security review of the broader trust boundary as a prerequisite before any gateway node runs + this against real traffic; this plan doesn't reopen that review, just doesn't make it any wider). +- **ICMPv6 arriving at `gw_addr`** (the ingress return address) is unchanged and intentionally so — + `gw_addr` is fabric-internal and only ever sees traffic this gateway itself sourced (design plan §3.1), + a materially different risk/reward trade than an internet-facing address. + +## 5. Testing + +`internal/plumbing/ebpf/edgeprog/edgenat_test.go`, root-required, `BPF_PROG_TEST_RUN`-based (mirroring +the existing `TestEdgeNat_ReturnPacketUnNATsEndToEnd`/`TestEdgeNat_ReturnWithNoConnDrops` shape). Note +`feat/865-egress-phase-b` currently has **no** egress-specific tests at all yet (`grep -n "Egress" +edgenat_test.go` on that branch is empty) — design plan §6 calls for base coverage +(`TestEdgeNat_EgressForward...`/`TestEdgeNat_EgressReturn...`) that hasn't landed yet either. This plan's +tests assume that base coverage exists (add it first if it still doesn't by the time this is picked up) +and add these on top: + +- `TestEdgeNat_EgressReturnICMPDestUnreachableTranslatesToTenant` — pre-seed `egress_conn_table`'s + reverse row via a real forward-direction packet (or a direct map write mirroring one), then send an + ICMPv6 Destination Unreachable with an embedded `masq_addr:masq_port → dest_addr:dest_port` TCP + segment; assert `XDP_TX`, outer daddr rewritten to `backend_addr`, embedded saddr/sport rewritten to + `backend_addr:backend_port`, valid ICMPv6 checksum, SRv6 push toward `backend_usid`. +- `TestEdgeNat_EgressReturnICMPPacketTooBigTranslatesToTenant` — same shape, type 2 — this is the PMTUD + case the issue calls "the one with teeth." +- `TestEdgeNat_EgressReturnICMPTimeExceededTranslatesToTenant` — type 3, same assertions. +- `TestEdgeNat_EgressReturnICMPUnknownConnDrops` — an ICMPv6 error whose embedded tuple matches no + `egress_conn_table` row; assert `XDP_DROP` and `DROP_REASON_NO_EGRESS_ICMP_CONN` (not + `MALFORMED_EGRESS_ICMP` — the naming distinction the review comment asked for). +- `TestEdgeNat_EgressPingRoundTrip` — send an Echo Request through `handle_egress_forward`, assert + identifier masqueraded and SNAT applied; feed the resulting masqueraded identifier back through + `handle_egress_return` as an Echo Reply, assert the identifier and destination address are restored to + the tenant's original values and the packet reaches the right `backend_usid`. +- `TestEdgeNat_EgressForwardICMPNonEchoRequestDrops` — an Echo Reply or other ICMPv6 type arriving from + a tenant backend via `egress_sid`; assert `XDP_DROP`/`MALFORMED_EGRESS_ICMP`, not silently accepted. +- `TestEdgeNat_EgressReturnUnhandledICMPPassesThrough` — an ICMPv6 type that is none of the handled + cases (e.g. a Router Advertisement, if constructible in the test harness, or any other type value) + arriving addressed to `masq_addr`; assert `XDP_PASS`, not `XDP_DROP` — the actual behavior change this + issue asks for on the "pass or handle the rest" half of its desired outcome. + +`internal/plumbing/ebpf/edgeprog/dropreason_test.go` (if one exists on this branch) or an inline check: +`DropReasonNames` has an entry for every index up to `DropReasonCount - 1`, so the two new reasons don't +silently fall back to a blank Prometheus label. + +## 6. Documentation + +- `edgenat.c`'s own file-header comment (point 5, "EGRESS RETURN BRANCH") currently says: "this includes + any non-TCP/UDP protocol arriving addressed to masq_addr, e.g. ICMPv6, which this program does not + special-case and drops rather than XDP_PASS." Rewrite to describe the new ICMPv6 dispatch (error + translation, Echo Reply translation, XDP_PASS for anything else) instead of the old blanket-drop + behavior. Point 4 ("EGRESS FORWARD BRANCH") needs the equivalent update for Echo Request. +- `docs/agents/ARCHITECTURE-GATEWAY.md`'s Known Constraints section currently still describes egress as + "planned, not implemented" (stale relative to `feat/865-egress-phase-b`'s actual code — a pre-existing + gap in that stack, not something this plan should try to fix in isolation). Whichever phase finally + updates that section to describe the real, implemented egress datapath should fold in a line noting + the ICMPv6 handling decision this plan makes (translate errors + Echo Reply, pass through everything + else), so the doc doesn't ship describing the pre-fix blanket-drop behavior as current. + +## 7. Rollout: this belongs on `feat/865-egress-phase-b`, not a follow-up PR + +Per [[project_865_egress_implementation_stack]], the entire egress feature (galactic#380/381/383/385/386) +is still open and unmerged as of this writing — `handle_egress_forward`/`handle_egress_return` have +never shipped. That makes this the right moment to fold the fix directly into **galactic#381** (the PR +that owns `edgenat.c`'s egress datapath) before it merges, rather than shipping the known-bad blanket-drop +behavior first and filing a separate fix afterward. Concretely: rebase/amend commits on +`feat/865-egress-phase-b` rather than branching from `main` (main doesn't have this code at all yet). +The three PRs stacked on top (383/385/386) rebase automatically when 381 gains commits, the same as any +other change to a stacked branch — no separate coordination needed beyond the review already in +progress. No `config/`/CRD/rollout changes of any kind — this is exclusively a datapath + drop-reason +change, entirely internal to `edgenat.c`'s own claimed addresses. + +## 8. Open questions for review + +- Should `handle_egress_forward_icmp6`'s identifier-claim probe share `EDGE_PAT_PORT_BASE`/ + `EDGE_PAT_PORT_RANGE` with the TCP/UDP port-claim probe (as sketched above, since both draw from the + same `masq_addr` and the same `egress_conn_table`), or does sharing the numeric range risk a + higher-than-expected collision rate between real SNAT ports and masqueraded ICMP identifiers under + heavy ping traffic? Leaning toward sharing (simplicity, and `BPF_NOEXIST` already makes any collision + self-resolving via the next probe attempt) but flagging since it's a capacity trade-off, not a + correctness one. +- Is folding `Parameter Problem` (type 4) into the same generic error handler as the three issue-named + types the right call, or should it stay unhandled (`XDP_PASS`) until a concrete need for it surfaces? + This plan includes it since the marginal cost is effectively zero, but it's not explicitly requested by + the issue. diff --git a/internal/plumbing/ebpf/edgeprog/dropreason.go b/internal/plumbing/ebpf/edgeprog/dropreason.go index 14c6d5b..b2d291a 100644 --- a/internal/plumbing/ebpf/edgeprog/dropreason.go +++ b/internal/plumbing/ebpf/edgeprog/dropreason.go @@ -33,7 +33,17 @@ const ( DropReasonMalformedEgressReturn uint32 = 13 DropReasonNoEgressReturnConn uint32 = 14 - DropReasonCount uint32 = 15 + // ICMPv6 egress drop reasons (galactic#404) -- see edgenat.c's + // handle_egress_forward_icmp6/handle_egress_return_icmp6. Kept + // distinct from the TCP/UDP-specific reasons above (and from each + // other) so an operator reading drop counters can tell "this ICMPv6 + // message didn't parse" apart from "it parsed fine but matched no + // flow" -- see edgenat.c's enum edge_drop_reason for the full + // rationale, including the #381 review comment this closes. + DropReasonMalformedEgressICMP uint32 = 15 + DropReasonNoEgressICMPConn uint32 = 16 + + DropReasonCount uint32 = 17 ) // DropReasonNames maps each DropReason* index to a short, stable, @@ -55,4 +65,6 @@ var DropReasonNames = map[uint32]string{ DropReasonEgressPATExhausted: "egress_pat_exhausted", DropReasonMalformedEgressReturn: "malformed_egress_return", DropReasonNoEgressReturnConn: "no_egress_return_conn", + DropReasonMalformedEgressICMP: "malformed_egress_icmp", + DropReasonNoEgressICMPConn: "no_egress_icmp_conn", } diff --git a/internal/plumbing/ebpf/edgeprog/edgenat.c b/internal/plumbing/ebpf/edgeprog/edgenat.c index 83524e3..d60693a 100644 --- a/internal/plumbing/ebpf/edgeprog/edgenat.c +++ b/internal/plumbing/ebpf/edgeprog/edgenat.c @@ -129,45 +129,72 @@ // Outer next header must be 41 (the same plain IPv6-in-IPv6 wire // format every other cross-node SRv6 packet in this codebase uses -- // a tenant VRF's default route needs zero new encap format). Strip -// the outer header (reuse strip_outer_header verbatim), look up -// egress_conn_table by the forward key (proto, tenant_arg, -// backend_addr:backend_port -> dest_addr:dest_port). A miss on a TCP -// SYN or any UDP packet allocates a masq_port via the same +// the outer header (reuse strip_outer_header verbatim), then dispatch +// on the *inner* packet's own next header: TCP/UDP goes to +// handle_egress_forward_l4 (the original SYN/any-UDP allocation logic, +// unchanged); an ICMPv6 Echo Request goes to handle_egress_forward_icmp6 +// (galactic#404) -- the Identifier field stands in for the port +// egress_conn_table is keyed by, the same technique Linux's own +// nf_conntrack ICMP tracker uses, since ICMPv6 echo has no real ports. +// Anything else is dropped (DROP_REASON_MALFORMED_EGRESS_FORWARD) -- +// this address is claimed the same as every other branch here. +// +// Both l4 and icmp6 sub-branches share the same allocation shape: a +// miss on a fresh flow (a TCP SYN, any UDP packet, or any Echo +// Request) allocates a masq_port/masq_identifier via the same // linear-probe/BPF_NOEXIST technique handle_forward's own SNAT-port // claim uses, against the reverse key (proto, dest_addr:dest_port -> // masq_addr:masq_port) -- tenant_arg is fixed at 0 in that reverse // row, since masq_addr:masq_port is already globally unique by // construction (the claim itself guarantees it) and needs no tenant -// dimension. SNAT saddr to masq_addr:masq_port, fix the L4 checksum, -// and XDP_TX the *inner* packet back out this same interface -// unwrapped -- no outer header pushed. This is the one genuinely new -// tail shape in this file: every other branch either pushes an outer -// header (handle_forward) or has already stripped one before -// rewriting (handle_return); this one strips one and sends the -// revealed inner packet on as a plain IPv6 frame toward the real -// internet. +// dimension. SNAT saddr (and, for ICMPv6, the Identifier) to +// masq_addr:masq_port, fix the checksum, and XDP_TX the *inner* +// packet back out this same interface unwrapped -- no outer header +// pushed. This is the one genuinely new tail shape in this file: +// every other branch either pushes an outer header (handle_forward) +// or has already stripped one before rewriting (handle_return); this +// one strips one and sends the revealed inner packet on as a plain +// IPv6 frame toward the real internet. // // 5. EGRESS RETURN BRANCH (#865): if the outer destination matches this // node's own configured masq_addr (egress_config_table) -- a plain // address compare, no nexthdr==41 requirement, since this arrives as // an ordinary internet-originated IPv6 packet, not an SRv6-encapsulated -// one -- parse the L4 header and look up egress_conn_table by the -// reverse key (proto, dest_addr:dest_port -> masq_addr:masq_port); no -// tenant_arg needed in this direction, masq_addr:masq_port is already -// unique per flow by construction. A miss drops (claimed address, no -// pass-through -- same fail-closed convention every other -// claimed-address branch in this file already uses; this includes any -// non-TCP/UDP protocol arriving addressed to masq_addr, e.g. ICMPv6, -// which this program does not special-case and drops rather than -// XDP_PASS, the same choice already made for gw_addr's own return -// branch). A hit DNATs daddr to backend_addr:backend_port (source -// address/port untouched -- a destination-only rewrite, unlike -// Full-NAT's four-field rewrite), fixes the checksum, and pushes a -// fresh 40-byte outer SRv6 header (reusing push_outer_header verbatim) -// sourced from this node's own gw_addr (gw_config_table -- the same -// "this node, as an SRv6 speaker" identity handle_forward's own push -// already uses) and addressed to backend_usid, then XDP_TX -- the -// return-trip mirror of handle_egress_forward(). +// one -- dispatch on next header. TCP/UDP looks up egress_conn_table +// by the reverse key (proto, dest_addr:dest_port -> masq_addr:masq_port) +// as before; no tenant_arg needed in this direction, masq_addr:masq_port +// is already unique per flow by construction. A miss drops (claimed +// address, no pass-through -- same fail-closed convention every other +// claimed-address branch in this file already uses). +// +// ICMPv6 (galactic#404) is no longer a blanket drop: an Echo Reply is +// looked up the same way an Echo Request allocated its flow (Identifier +// as the reverse key's pseudo-port) and un-masqueraded DNAT-style, +// restoring both the destination address and the original Identifier +// the tenant itself sent. Destination Unreachable/Packet Too Big/Time +// Exceeded/Parameter Problem (RFC 4443 error messages) embed the IPv6 +// header and at least the first 8 bytes of the transport header of the +// packet that triggered them -- for a packet this program itself SNAT'd, +// that is masq_addr:masq_port -> dest_addr:dest_port, exactly +// egress_conn_table's existing reverse key read one layer deeper. Path +// MTU discovery rides on Packet Too Big specifically: dropping these +// (the pre-#404 behavior) is what stalled large transfers instead of +// letting them adapt. A recognized ICMPv6 message with no matching +// egress_conn_table row drops (DROP_REASON_NO_EGRESS_ICMP_CONN, kept +// distinct from the TCP/UDP path's own DROP_REASON_NO_EGRESS_RETURN_CONN +// so operators can tell them apart from drop counters alone). Any other +// ICMPv6 type, or any other protocol entirely (Neighbor Discovery, an +// Echo Request targeting masq_addr directly, ...) is not a reply to any +// tenant flow this program tracks -- XDP_PASS, not XDP_DROP, handing it +// to the normal kernel stack instead of claiming and dropping it. +// +// Every translated case (TCP/UDP, Echo Reply, and each ICMPv6 error +// type) fixes its checksum and pushes a fresh 40-byte outer SRv6 header +// (reusing push_outer_header verbatim) sourced from this node's own +// gw_addr (gw_config_table -- the same "this node, as an SRv6 speaker" +// identity handle_forward's own push already uses) and addressed to +// backend_usid, then XDP_TX -- the return-trip mirror of the forward +// branch above. // // A real eBPF verifier gotcha carried over from gwprog's own header // comment: the backend-selection index (hash % backend_count, then @@ -242,8 +269,23 @@ static __u64 (*bpf_ktime_get_ns)(void) = (void *) BPF_FUNC_ktime_get_ns; // pushed packets with zero changes on that end. #define EDGE_IPPROTO_IPV6 41 +// EDGE_IPPROTO_ICMPV6 (58) is the next-header value for every ICMPv6 +// message the egress return/forward branches special-case (galactic#404) -- +// see struct edge_icmp6hdr/edge_icmp6_error_hdr/edge_icmp6_echo_hdr below. +#define EDGE_IPPROTO_ICMPV6 58 + #define EDGE_TCP_FLAG_SYN 0x02 +// ICMPv6 message types this program reads (RFC 4443). The four error +// types (1-4) share struct edge_icmp6_error_hdr's shape; the two echo +// types share struct edge_icmp6_echo_hdr's. +#define EDGE_ICMPV6_DEST_UNREACH 1 +#define EDGE_ICMPV6_PACKET_TOO_BIG 2 +#define EDGE_ICMPV6_TIME_EXCEEDED 3 +#define EDGE_ICMPV6_PARAM_PROBLEM 4 +#define EDGE_ICMPV6_ECHO_REQUEST 128 +#define EDGE_ICMPV6_ECHO_REPLY 129 + // --------------------------------------------------------------------- // Minimal, self-contained header structs -- byte-exact to the wire // formats, hand-rolled so this file has exactly one external header @@ -292,6 +334,46 @@ struct edge_udphdr { __be16 check; } __attribute__((packed)); +// struct edge_icmp6hdr is the common 4-byte prefix every ICMPv6 message +// starts with (RFC 4443 §2.1) -- used only to read type/code before +// dispatching to one of the two more specific shapes below (galactic#404). +struct edge_icmp6hdr { + __u8 type; + __u8 code; + __be16 check; +} __attribute__((packed)); + +// struct edge_icmp6_error_hdr is the 8-byte header shape Destination +// Unreachable/Packet Too Big/Time Exceeded/Parameter Problem (RFC 4443 §3, +// types 1-4) all share: type, code, checksum, then 4 bytes whose meaning +// varies by type (unused for 1/3, MTU for 2, pointer for 4) that this +// program never reads. What follows is "as much of the invoking packet as +// possible," guaranteed to include at least the embedded IPv6 header's +// first 48 bytes (RFC 4443 §2.4(c)) -- the full 40-byte IPv6 header plus +// the first 8 bytes of whatever transport header follows, which is where +// both TCP and UDP keep their two 16-bit port fields (see +// parse_embedded_ports, deliberately not parse_l4 -- that guarantee falls +// short of a full struct edge_tcphdr). +struct edge_icmp6_error_hdr { + __u8 type; + __u8 code; + __be16 check; + __u8 unused[4]; +} __attribute__((packed)); + +// struct edge_icmp6_echo_hdr is the Echo Request/Reply header (RFC 4443 +// §4). identifier stands in for the port egress_conn_table is keyed by -- +// the standard NAT66/NAT64 technique for a protocol with no real ports +// (Linux's own nf_conntrack ICMP tracker does the same) -- see +// handle_egress_forward_icmp6/handle_egress_return_icmp6_echo. +struct edge_icmp6_echo_hdr { + __u8 type; + __u8 code; + __be16 check; + __be16 identifier; + __be16 sequence; +} __attribute__((packed)); + // --------------------------------------------------------------------- // Map key/value types. // --------------------------------------------------------------------- @@ -438,6 +520,13 @@ struct egress_conn_key { // arbitrary internet destination, and the masquerade address. tenant_arg // is carried here too (alongside the reverse row, where it is always 0) // so both directions share one struct shape. +// +// For an ICMPv6 Echo flow (proto == EDGE_IPPROTO_ICMPV6, galactic#404), +// backend_port/dest_port/masq_port hold the Echo Identifier instead of a +// real port -- a deliberate reuse rather than dedicated identifier fields, +// the same "a field held equal old/new contributes zero diff" generic +// reuse fix_l4_checksum's own call sites already lean on elsewhere in this +// file (see handle_egress_forward_icmp6/handle_egress_return_icmp6_echo). struct egress_conn_value { __u16 tenant_arg; __u8 backend_addr[16]; @@ -483,7 +572,19 @@ enum edge_drop_reason { DROP_REASON_EGRESS_PAT_EXHAUSTED = 12, DROP_REASON_MALFORMED_EGRESS_RETURN = 13, DROP_REASON_NO_EGRESS_RETURN_CONN = 14, - DROP_REASON_COUNT = 15, + // ICMPv6 egress drop reasons (galactic#404) -- see + // handle_egress_forward_icmp6/handle_egress_return_icmp6. Kept distinct + // from the TCP/UDP-specific reasons above rather than reused: an + // operator reading drop counters should be able to tell "this ICMPv6 + // message didn't parse" apart from "it parsed fine but matched no + // flow," and apart from the TCP/UDP path's own equivalents -- the + // original review comment on #381 flagged exactly this ambiguity + // (DROP_REASON_MALFORMED_EGRESS_RETURN previously double-booked as + // both "malformed" and "well-formed but not TCP/UDP," a protocol-policy + // decision mislabeled as a parse failure). + DROP_REASON_MALFORMED_EGRESS_ICMP = 15, + DROP_REASON_NO_EGRESS_ICMP_CONN = 16, + DROP_REASON_COUNT = 17, }; // --------------------------------------------------------------------- @@ -701,6 +802,24 @@ static EDGE_ALWAYS_INLINE int parse_l4(__u8 proto, void *l4, void *data_end, str return -1; } +// parse_embedded_ports reads the two 16-bit port fields both edge_tcphdr +// and edge_udphdr start with, bounds-checking only those 4 bytes -- +// deliberately not parse_l4, whose full-struct bounds check (a complete +// struct edge_tcphdr, 20 bytes) would reject a validly-minimal ICMPv6 +// error message's embedded TCP header: RFC 4443 guarantees only the first +// 8 bytes of the invoking transport header, and both TCP's and UDP's +// source/dest port fields sit in the first 4 of those, well within that +// minimum (galactic#404's handle_egress_return_icmp6_error). +static EDGE_ALWAYS_INLINE int parse_embedded_ports(void *l4, void *data_end, __be16 *sport, __be16 *dport) +{ + __be16 *ports = l4; + if ((void *) (ports + 2) > data_end) + return -1; + *sport = ports[0]; + *dport = ports[1]; + return 0; +} + // fix_l4_checksum applies the combined address+port checksum delta for a // Full-NAT rewrite (both addresses and both ports changed) to *check_ptr // -- already resolved to the correct field by parse_l4, so this function @@ -1064,60 +1183,15 @@ static EDGE_ALWAYS_INLINE int handle_return(struct xdp_md *ctx, struct edge_ip6h // Egress branch (masquerade) (datum-cloud/enhancements#865). // --------------------------------------------------------------------- -// handle_egress_forward is triggered when the outer destination's locator -// matches this node's own configured egress_sid and outer nexthdr == 41 -- -// a fresh (or already-established) outbound flow from a tenant VPC backend -// Pod toward an arbitrary internet destination. tenant_arg is the uFMT -// Argument value already extracted from the packet's own destination -// address by the caller (edge_nat), before this function strips the outer -// header that address lives on. -static EDGE_ALWAYS_INLINE int handle_egress_forward(struct xdp_md *ctx, struct edge_ip6hdr *outer, - __u16 tenant_arg, void *data_end) +// handle_egress_forward_l4 handles the TCP/UDP shape of a fresh (or +// already-established) egress flow -- factored out of handle_egress_forward +// unchanged (galactic#404 split this out to make room for +// handle_egress_forward_icmp6 as a sibling, not to change this path's own +// behavior). +static EDGE_ALWAYS_INLINE int handle_egress_forward_l4(struct xdp_md *ctx, struct edge_ethhdr *eth, + struct edge_ip6hdr *inner, __u16 tenant_arg, + const __u8 backend_usid[16], void *data_end) { - if (outer->nexthdr != EDGE_IPPROTO_IPV6) { - count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); - return XDP_DROP; - } - - // The outer source is the originating worker node's own SRv6 address - // -- the same node that encapsulated this packet via its tenant VRF's - // default route toward egress_sid (internal/plumbing/srv6. - // RouteEgressAdd's SEG6 encap route, design plan §4.4). Captured here, - // before strip_outer_header discards the outer header entirely, and - // remembered in egress_conn_value.backend_usid so the eventual reply - // (handle_egress_return) knows which node to push a return SRv6 - // header toward -- there is no rule_table-equivalent policy entry for - // egress (design plan §3.2), so this wire-derived value is the only - // source of that address Phase B has. - // - // ASSUMPTION FLAGGED FOR REVIEW: this relies on the kernel's SEG6 - // encap route always selecting the node's own uSID address as the - // pushed outer source, the same way it does for every other cross- - // node SRv6 packet in this codebase. That has not been independently - // verified against RouteEgressAdd's actual netlink-level source- - // address-selection behavior as part of this phase -- worth - // confirming before Phase D's e2e proof relies on it. - __u8 backend_usid[16]; - __builtin_memcpy(backend_usid, outer->saddr, 16); - - struct edge_ethhdr *eth; - if (strip_outer_header(ctx, ð) != 0) { - count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); - return XDP_DROP; - } - - data_end = (void *) (long) ctx->data_end; - - struct edge_ip6hdr *inner = (void *) (eth + 1); - if ((void *) (inner + 1) > data_end) { - count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); - return XDP_DROP; - } - if (inner->nexthdr != EDGE_IPPROTO_TCP && inner->nexthdr != EDGE_IPPROTO_UDP) { - count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); - return XDP_DROP; - } - struct l4_view l4v; if (parse_l4(inner->nexthdr, (void *) (inner + 1), data_end, &l4v) != 0) { count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); @@ -1220,22 +1294,204 @@ static EDGE_ALWAYS_INLINE int handle_egress_forward(struct xdp_md *ctx, struct e return XDP_TX; } -// handle_egress_return is triggered when the outer destination matches this -// node's own configured masq_addr -- an ordinary internet-originated IPv6 -// packet (no SRv6 encapsulation), the reply half of a flow -// handle_egress_forward already established. -static EDGE_ALWAYS_INLINE int handle_egress_return(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +// handle_egress_forward_icmp6 handles a tenant backend's own ICMPv6 Echo +// Request leaving via egress_sid -- the forward half of the ping round +// trip handle_egress_return_icmp6_echo completes on the way back +// (galactic#404). Any other ICMPv6 type from a tenant backend (Echo +// Reply, Router Solicitation, Neighbor Discovery, ...) has no defined +// masquerade behavior in this design and is dropped, not passed through -- +// this address (egress_sid) is claimed the same as every other branch in +// this file. +static EDGE_ALWAYS_INLINE int handle_egress_forward_icmp6(struct xdp_md *ctx, struct edge_ethhdr *eth, + struct edge_ip6hdr *inner, __u16 tenant_arg, + const __u8 backend_usid[16], void *data_end) { - // Claimed address past this point (masq_addr): any protocol other - // than TCP/UDP -- e.g. ICMPv6 -- is dropped rather than passed - // through to the normal kernel stack, the same fail-closed choice - // already made for gw_addr's own return branch (this file's header - // comment, point 2). - if (ip6->nexthdr != EDGE_IPPROTO_TCP && ip6->nexthdr != EDGE_IPPROTO_UDP) { - count_drop(DROP_REASON_MALFORMED_EGRESS_RETURN); + struct edge_icmp6_echo_hdr *echo = (void *) (inner + 1); + if ((void *) (echo + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + if (echo->type != EDGE_ICMPV6_ECHO_REQUEST) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + // Forward key: identifier stands in for both sport/dport (struct + // egress_conn_value's own comment) -- everything else mirrors + // handle_egress_forward_l4's forward key exactly. + struct egress_conn_key fwd_key; + __builtin_memset(&fwd_key, 0, sizeof(fwd_key)); + fwd_key.proto = EDGE_IPPROTO_ICMPV6; + __builtin_memcpy(fwd_key.saddr, inner->saddr, 16); + fwd_key.sport = echo->identifier; + __builtin_memcpy(fwd_key.daddr, inner->daddr, 16); + fwd_key.dport = echo->identifier; + fwd_key.tenant_arg = tenant_arg; + + struct egress_conn_value *existing = bpf_map_lookup_elem(&egress_conn_table, &fwd_key); + struct egress_conn_value cv; + + if (existing) { + __builtin_memcpy(&cv, existing, sizeof(cv)); + } else { + // Every Echo Request may start a new flow -- there is no + // SYN-equivalent concept for ICMP, the same "any UDP packet + // may start a new flow" reasoning handle_egress_forward_l4 + // already applies to UDP. + __u32 cfg_key = 0; + struct egress_config *ecfg = bpf_map_lookup_elem(&egress_config_table, &cfg_key); + if (!ecfg) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + __builtin_memset(&cv, 0, sizeof(cv)); + cv.tenant_arg = tenant_arg; + __builtin_memcpy(cv.backend_addr, inner->saddr, 16); + cv.backend_port = echo->identifier; // the tenant's own original identifier + __builtin_memcpy(cv.backend_usid, backend_usid, 16); + __builtin_memcpy(cv.dest_addr, inner->daddr, 16); + cv.dest_port = echo->identifier; + __builtin_memcpy(cv.masq_addr, ecfg->masq_addr, 16); + cv.proto = EDGE_IPPROTO_ICMPV6; + + // Identifier-claim probe: the same bounded linear-probe/ + // BPF_NOEXIST technique handle_egress_forward_l4's masq_port + // claim uses, over the same numeric range, just keyed by + // identifier instead of port -- two different backend Pods + // (or the same Pod's two concurrent pings) can legitimately + // pick the same identifier value, and masq_addr has only one + // address to share, so the identifier must be re-mapped + // exactly like a SNAT port would be. + __u32 base = fnv1a_flow(inner->saddr, echo->identifier); + int claimed = 0; + + #pragma unroll + for (int i = 0; i < EDGE_PAT_PROBE_LIMIT; i++) { + __u16 candidate = EDGE_PAT_PORT_BASE + ((base + (__u32) i) % EDGE_PAT_PORT_RANGE); + cv.masq_port = __builtin_bswap16(candidate); + + struct egress_conn_key rev_key; + __builtin_memset(&rev_key, 0, sizeof(rev_key)); + rev_key.proto = EDGE_IPPROTO_ICMPV6; + __builtin_memcpy(rev_key.saddr, inner->daddr, 16); + rev_key.sport = cv.masq_port; + __builtin_memcpy(rev_key.daddr, ecfg->masq_addr, 16); + rev_key.dport = cv.masq_port; + // tenant_arg left at 0 in the reverse row -- see struct + // egress_conn_key's comment. + + if (bpf_map_update_elem(&egress_conn_table, &rev_key, &cv, BPF_NOEXIST) == 0) { + claimed = 1; + break; + } + } + + if (!claimed) { + count_drop(DROP_REASON_EGRESS_PAT_EXHAUSTED); + return XDP_DROP; + } + + bpf_map_update_elem(&egress_conn_table, &fwd_key, &cv, BPF_ANY); + } + + // Masquerade both the source address and the identifier -- mirroring + // handle_egress_forward_l4's SNAT-only rewrite exactly, with + // identifier standing in for port throughout. fix_l4_checksum is a + // generic address+word-pair checksum-diff helper, not a Full-NAT- + // specific one, so passing 0 for the unused dport-shaped word slot on + // both sides (contributing zero diff) is safe -- the same technique + // handle_egress_forward_l4's own SNAT-only comment documents. + __u8 old_saddr[16]; + __builtin_memcpy(old_saddr, inner->saddr, 16); + __be16 old_identifier = echo->identifier; + + fix_l4_checksum(&echo->check, old_saddr, inner->daddr, old_identifier, 0, + cv.masq_addr, inner->daddr, cv.masq_port, 0); + + __builtin_memcpy(inner->saddr, cv.masq_addr, 16); + echo->identifier = cv.masq_port; + + long fib_rc = resolve_fib_and_write_eth(ctx, ctx->ingress_ifindex, cv.masq_addr, cv.dest_addr, + __builtin_bswap16(inner->payload_len) + (__u16) sizeof(*inner), eth); + if (fib_rc != BPF_FIB_LKUP_RET_SUCCESS) { + count_fib_drop(fib_rc); + return XDP_DROP; + } + + return XDP_TX; +} + +// handle_egress_forward is triggered when the outer destination's locator +// matches this node's own configured egress_sid and outer nexthdr == 41 -- +// a fresh (or already-established) outbound flow from a tenant VPC backend +// Pod toward an arbitrary internet destination. tenant_arg is the uFMT +// Argument value already extracted from the packet's own destination +// address by the caller (edge_nat), before this function strips the outer +// header that address lives on. Strips the outer header, resolves the +// inner packet's own next header, and dispatches to handle_egress_forward_l4 +// (TCP/UDP, the original logic) or handle_egress_forward_icmp6 (Echo +// Request, galactic#404) -- anything else drops, this address is claimed. +static EDGE_ALWAYS_INLINE int handle_egress_forward(struct xdp_md *ctx, struct edge_ip6hdr *outer, + __u16 tenant_arg, void *data_end) +{ + if (outer->nexthdr != EDGE_IPPROTO_IPV6) { + count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); + return XDP_DROP; + } + + // The outer source is the originating worker node's own SRv6 address + // -- the same node that encapsulated this packet via its tenant VRF's + // default route toward egress_sid (internal/plumbing/srv6. + // RouteEgressAdd's SEG6 encap route, design plan §4.4). Captured here, + // before strip_outer_header discards the outer header entirely, and + // remembered in egress_conn_value.backend_usid so the eventual reply + // (handle_egress_return) knows which node to push a return SRv6 + // header toward -- there is no rule_table-equivalent policy entry for + // egress (design plan §3.2), so this wire-derived value is the only + // source of that address Phase B has. + // + // ASSUMPTION FLAGGED FOR REVIEW: this relies on the kernel's SEG6 + // encap route always selecting the node's own uSID address as the + // pushed outer source, the same way it does for every other cross- + // node SRv6 packet in this codebase. That has not been independently + // verified against RouteEgressAdd's actual netlink-level source- + // address-selection behavior as part of this phase -- worth + // confirming before Phase D's e2e proof relies on it. + __u8 backend_usid[16]; + __builtin_memcpy(backend_usid, outer->saddr, 16); + + struct edge_ethhdr *eth; + if (strip_outer_header(ctx, ð) != 0) { + count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); + return XDP_DROP; + } + + data_end = (void *) (long) ctx->data_end; + + struct edge_ip6hdr *inner = (void *) (eth + 1); + if ((void *) (inner + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); + return XDP_DROP; + } + + if (inner->nexthdr == EDGE_IPPROTO_ICMPV6) + return handle_egress_forward_icmp6(ctx, eth, inner, tenant_arg, backend_usid, data_end); + + if (inner->nexthdr != EDGE_IPPROTO_TCP && inner->nexthdr != EDGE_IPPROTO_UDP) { + count_drop(DROP_REASON_MALFORMED_EGRESS_FORWARD); return XDP_DROP; } + return handle_egress_forward_l4(ctx, eth, inner, tenant_arg, backend_usid, data_end); +} + +// handle_egress_return_l4 handles the TCP/UDP shape of an egress reply -- +// factored out of handle_egress_return unchanged (galactic#404 split this +// out to make room for the ICMPv6 siblings below, not to change this +// path's own behavior). +static EDGE_ALWAYS_INLINE int handle_egress_return_l4(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ struct l4_view l4v; if (parse_l4(ip6->nexthdr, (void *) (ip6 + 1), data_end, &l4v) != 0) { count_drop(DROP_REASON_MALFORMED_EGRESS_RETURN); @@ -1285,6 +1541,208 @@ static EDGE_ALWAYS_INLINE int handle_egress_return(struct xdp_md *ctx, struct ed return XDP_TX; } +// handle_egress_return_icmp6_echo handles an ICMPv6 Echo Reply addressed +// to masq_addr -- the reply half of the ping round trip +// handle_egress_forward_icmp6 started (galactic#404). Looks up +// egress_conn_table by identifier (the same pseudo-port key the forward +// allocation wrote) and un-masquerades both the destination address and +// the identifier DNAT-style -- restoring the identifier is the part easy +// to miss: leaving it at the masqueraded value would let the address +// rewrite succeed while the tenant's own ping process still doesn't +// recognize the reply, since the identifier it observes would not be the +// one it originally sent. +static EDGE_ALWAYS_INLINE int handle_egress_return_icmp6_echo(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + struct edge_icmp6_echo_hdr *echo = (void *) (ip6 + 1); + if ((void *) (echo + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + struct egress_conn_key rev_key; + __builtin_memset(&rev_key, 0, sizeof(rev_key)); + rev_key.proto = EDGE_IPPROTO_ICMPV6; + __builtin_memcpy(rev_key.saddr, ip6->saddr, 16); + rev_key.sport = echo->identifier; + __builtin_memcpy(rev_key.daddr, ip6->daddr, 16); + rev_key.dport = echo->identifier; + + struct egress_conn_value *cv = bpf_map_lookup_elem(&egress_conn_table, &rev_key); + if (!cv) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + // Two fields change: destination address (masq_addr -> backend_addr) + // and identifier (the masqueraded value -> cv->backend_port, the + // tenant's own original identifier) -- source address is untouched, + // the same DNAT-only shape handle_egress_return_l4 applies to ports, + // just for ICMP's identifier field instead. + __u8 old_daddr[16]; + __builtin_memcpy(old_daddr, ip6->daddr, 16); + __be16 old_identifier = echo->identifier; + + fix_l4_checksum(&echo->check, ip6->saddr, old_daddr, old_identifier, 0, + ip6->saddr, cv->backend_addr, cv->backend_port, 0); + + __builtin_memcpy(ip6->daddr, cv->backend_addr, 16); + echo->identifier = cv->backend_port; + + __u32 cfg_key = 0; + struct gw_config *cfg = bpf_map_lookup_elem(&gw_config_table, &cfg_key); + if (!cfg) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + __be16 inner_payload_len = ip6->payload_len; + + if (push_outer_header(ctx, cfg->gw_addr, cv->backend_usid, inner_payload_len) != 0) + return XDP_DROP; + + return XDP_TX; +} + +// handle_egress_return_icmp6_error handles Destination Unreachable/Packet +// Too Big/Time Exceeded/Parameter Problem addressed to masq_addr +// (galactic#404) -- the piece with actual teeth, since Packet Too Big is +// how path MTU discovery reaches a tenant. The embedded original datagram +// -- masq_addr:masq_port -> dest_addr:dest_port, exactly the packet +// handle_egress_forward_l4 last sent -- carries everything needed to key +// egress_conn_table's existing reverse row; no new map, no new key shape. +// +// Deliberately uses parse_embedded_ports, not parse_l4: RFC 4443 +// guarantees only the first 8 bytes of the invoking transport header, and +// parse_l4's full-struct bounds check (20 bytes for TCP) would reject a +// validly-minimal error message the port-only read here does not need to +// reject. +static EDGE_ALWAYS_INLINE int handle_egress_return_icmp6_error(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + struct edge_icmp6_error_hdr *err = (void *) (ip6 + 1); + struct edge_ip6hdr *embedded = (void *) (err + 1); + if ((void *) (embedded + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + if (embedded->nexthdr != EDGE_IPPROTO_TCP && embedded->nexthdr != EDGE_IPPROTO_UDP) { + // The invoking packet wasn't one this program itself sent + // (handle_egress_forward only ever emits TCP/UDP or ICMPv6 + // Echo Request) -- not attributable to a tenant flow. + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + __be16 embedded_sport, embedded_dport; + if (parse_embedded_ports((void *) (embedded + 1), data_end, &embedded_sport, &embedded_dport) != 0) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + // The embedded packet is masq_addr:masq_port -> dest_addr:dest_port -- + // exactly the packet handle_egress_forward_l4 last sent -- so this is + // the *same reverse key* a direct TCP/UDP reply is looked up by, just + // read one layer deeper. + struct egress_conn_key rev_key; + __builtin_memset(&rev_key, 0, sizeof(rev_key)); + rev_key.proto = embedded->nexthdr; + __builtin_memcpy(rev_key.saddr, embedded->daddr, 16); + rev_key.sport = embedded_dport; + __builtin_memcpy(rev_key.daddr, embedded->saddr, 16); + rev_key.dport = embedded_sport; + + struct egress_conn_value *cv = bpf_map_lookup_elem(&egress_conn_table, &rev_key); + if (!cv) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + // Two rewrites land on the same checksum (ICMPv6's own, which covers + // the whole message including the embedded bytes verbatim -- the + // embedded packet's own stale L4 checksum is untouched and never + // independently re-validated by anyone downstream): the outer + // packet's destination (masq_addr -> backend_addr, so it routes to + // the right worker node) and the embedded packet's own source + // address/port (masq_addr:masq_port -> backend_addr:backend_port), + // so the tenant's IP stack recognizes this error as belonging to a + // socket it actually opened. Both old values are masq_addr/masq_port + // by construction (this program's own earlier SNAT), so this is + // genuinely two separate memory locations converging on one value + // change apiece -- fix_l4_checksum's four word-slots don't have to + // mean "one address's source/dest" here, just "two old values, two + // new values, diffed together" (the same generic reuse its other + // call sites in this file already lean on). + __u8 old_outer_daddr[16], old_embedded_saddr[16]; + __builtin_memcpy(old_outer_daddr, ip6->daddr, 16); + __builtin_memcpy(old_embedded_saddr, embedded->saddr, 16); + + fix_l4_checksum(&err->check, old_outer_daddr, old_embedded_saddr, 0, embedded_sport, + cv->backend_addr, cv->backend_addr, 0, cv->backend_port); + + __builtin_memcpy(ip6->daddr, cv->backend_addr, 16); + __builtin_memcpy(embedded->saddr, cv->backend_addr, 16); + __be16 *embedded_ports = (void *) (embedded + 1); + embedded_ports[0] = cv->backend_port; + + __u32 cfg_key = 0; + struct gw_config *cfg = bpf_map_lookup_elem(&gw_config_table, &cfg_key); + if (!cfg) { + count_drop(DROP_REASON_NO_EGRESS_ICMP_CONN); + return XDP_DROP; + } + + __be16 inner_payload_len = ip6->payload_len; + + if (push_outer_header(ctx, cfg->gw_addr, cv->backend_usid, inner_payload_len) != 0) + return XDP_DROP; + + return XDP_TX; +} + +// handle_egress_return_icmp6 reads the common 4-byte ICMPv6 prefix and +// dispatches by type (galactic#404): Echo Reply and the four RFC 4443 +// error types translate back to the originating tenant; anything else +// (Router Advertisement, Neighbor Solicitation/Advertisement, an Echo +// Request targeting masq_addr directly, ...) is not a reply to any tenant +// flow this program tracks -- XDP_PASS, not XDP_DROP, handing it to the +// normal kernel stack instead of claiming and dropping it. +static EDGE_ALWAYS_INLINE int handle_egress_return_icmp6(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + struct edge_icmp6hdr *icmp6 = (void *) (ip6 + 1); + if ((void *) (icmp6 + 1) > data_end) { + count_drop(DROP_REASON_MALFORMED_EGRESS_ICMP); + return XDP_DROP; + } + + if (icmp6->type == EDGE_ICMPV6_ECHO_REPLY) + return handle_egress_return_icmp6_echo(ctx, ip6, data_end); + + if (icmp6->type == EDGE_ICMPV6_DEST_UNREACH || icmp6->type == EDGE_ICMPV6_PACKET_TOO_BIG || + icmp6->type == EDGE_ICMPV6_TIME_EXCEEDED || icmp6->type == EDGE_ICMPV6_PARAM_PROBLEM) + return handle_egress_return_icmp6_error(ctx, ip6, data_end); + + return XDP_PASS; +} + +// handle_egress_return is triggered when the outer destination matches this +// node's own configured masq_addr -- an ordinary internet-originated IPv6 +// packet (no SRv6 encapsulation), the reply half of a flow +// handle_egress_forward already established. Dispatches on next header: +// TCP/UDP to handle_egress_return_l4 (the original logic), ICMPv6 to +// handle_egress_return_icmp6 (galactic#404); any other protocol is not +// this program's to translate -- XDP_PASS, mirroring step 1's own +// can't-fully-parse-or-match fallthrough, just decided per-protocol here +// since this address is otherwise claimed. +static EDGE_ALWAYS_INLINE int handle_egress_return(struct xdp_md *ctx, struct edge_ip6hdr *ip6, void *data_end) +{ + if (ip6->nexthdr == EDGE_IPPROTO_ICMPV6) + return handle_egress_return_icmp6(ctx, ip6, data_end); + + if (ip6->nexthdr != EDGE_IPPROTO_TCP && ip6->nexthdr != EDGE_IPPROTO_UDP) + return XDP_PASS; + + return handle_egress_return_l4(ctx, ip6, data_end); +} + // --------------------------------------------------------------------- // Entry point. // --------------------------------------------------------------------- diff --git a/internal/plumbing/ebpf/edgeprog/edgenat_egress_icmp_test.go b/internal/plumbing/ebpf/edgeprog/edgenat_egress_icmp_test.go new file mode 100644 index 0000000..49896fc --- /dev/null +++ b/internal/plumbing/ebpf/edgeprog/edgenat_egress_icmp_test.go @@ -0,0 +1,438 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package edgeprog + +import ( + "encoding/binary" + "net/netip" + "testing" +) + +// ICMPv6 message types/lengths used by the tests below (galactic#404). +// Kept local to this file rather than added to edgenat.c's own constants +// -- these mirror RFC 4443, not anything this program's C code needs a +// Go-side name for. +const ( + icmpv6DestUnreachable = uint8(1) + icmpv6PacketTooBig = uint8(2) + icmpv6TimeExceeded = uint8(3) + icmpv6RouterSolicitation = uint8(133) // an arbitrary "not handled" type + icmpv6EchoRequest = uint8(128) + icmpv6EchoReply = uint8(129) + icmp6EchoHdrLen = 8 // type+code+check+identifier+sequence + icmp6ErrHdrLen = 8 // type+code+check+4 type-specific bytes + icmp6EmbeddedPortsMinBytes = 8 // RFC 4443's guaranteed minimum embedded-transport-header length +) + +// buildICMPv6EchoPacket returns a full Ethernet+IPv6+ICMPv6 Echo +// Request/Reply frame (identifier+sequence, no payload) with a correct +// checksum. +func buildICMPv6EchoPacket(src, dst netip.Addr, icmpType uint8, identifier, sequence uint16) []byte { + pkt := make([]byte, ethHdrLen+ip6HdrLen+icmp6EchoHdrLen) + binary.BigEndian.PutUint16(pkt[12:14], 0x86DD) + + ip6 := pkt[ethHdrLen:] + ip6[0] = 0x60 + binary.BigEndian.PutUint16(ip6[4:6], icmp6EchoHdrLen) + ip6[6] = 58 // ICMPv6 + ip6[7] = 64 + sb, db := src.As16(), dst.As16() + copy(ip6[8:24], sb[:]) + copy(ip6[24:40], db[:]) + + icmp := ip6[ip6HdrLen:] + icmp[0] = icmpType + icmp[1] = 0 + binary.BigEndian.PutUint16(icmp[4:6], identifier) + binary.BigEndian.PutUint16(icmp[6:8], sequence) + binary.BigEndian.PutUint16(icmp[2:4], 0) + + csum := ipv6L4Checksum(src, dst, 58, icmp) + binary.BigEndian.PutUint16(icmp[2:4], csum) + + return pkt +} + +// buildEncappedICMPv6EchoPacket wraps buildICMPv6EchoPacket's inner frame +// in an outer IPv6-in-IPv6 (nexthdr=41) header, mirroring +// buildEncappedTCPPacket -- the wire shape of a tenant backend's own Echo +// Request arriving SRv6-encapsulated and addressed to egress_sid. +func buildEncappedICMPv6EchoPacket( + outerSrc, outerDst, innerSrc, innerDst netip.Addr, icmpType uint8, identifier, sequence uint16, +) []byte { + inner := buildICMPv6EchoPacket(innerSrc, innerDst, icmpType, identifier, sequence)[ethHdrLen:] + + pkt := make([]byte, ethHdrLen+ip6HdrLen+len(inner)) + binary.BigEndian.PutUint16(pkt[12:14], 0x86DD) + + outer := pkt[ethHdrLen:] + outer[0] = 0x60 + binary.BigEndian.PutUint16(outer[4:6], uint16(len(inner))) + outer[6] = 41 + outer[7] = 64 + sb, db := outerSrc.As16(), outerDst.As16() + copy(outer[8:24], sb[:]) + copy(outer[24:40], db[:]) + + copy(pkt[ethHdrLen+ip6HdrLen:], inner) + return pkt +} + +// buildICMPv6ErrorPacket returns a full Ethernet+IPv6+ICMPv6-error frame, +// as if generated by an intermediate router (routerAddr) in response to a +// packet this program itself sent -- masqAddr:masqPort -> destAddr:destPort, +// embeddedProto -- truncated to embeddedLen bytes of that packet's own +// transport header. RFC 4443 guarantees only the first 8 bytes, so a +// caller passing icmp6EmbeddedPortsMinBytes exercises the minimal, +// worst-case shape parse_embedded_ports (not parse_l4) is specifically +// written to survive -- a full 20-byte TCP header is not guaranteed to be +// present, only its first 8 bytes are, and both port fields live there. +func buildICMPv6ErrorPacket( + routerAddr, masqAddr, destAddr netip.Addr, + icmpType, embeddedProto uint8, masqPort, destPort uint16, embeddedLen int, +) []byte { + embedded := make([]byte, ip6HdrLen+embeddedLen) + embedded[0] = 0x60 + binary.BigEndian.PutUint16(embedded[4:6], uint16(embeddedLen)) + embedded[6] = embeddedProto + embedded[7] = 64 + ma, da := masqAddr.As16(), destAddr.As16() + copy(embedded[8:24], ma[:]) + copy(embedded[24:40], da[:]) + if embeddedLen >= 4 { + binary.BigEndian.PutUint16(embedded[40:42], masqPort) + binary.BigEndian.PutUint16(embedded[42:44], destPort) + } + + pkt := make([]byte, ethHdrLen+ip6HdrLen+icmp6ErrHdrLen+len(embedded)) + binary.BigEndian.PutUint16(pkt[12:14], 0x86DD) + + ip6 := pkt[ethHdrLen:] + ip6[0] = 0x60 + binary.BigEndian.PutUint16(ip6[4:6], uint16(icmp6ErrHdrLen+len(embedded))) + ip6[6] = 58 // ICMPv6 + ip6[7] = 64 + ra := routerAddr.As16() + copy(ip6[8:24], ra[:]) + copy(ip6[24:40], ma[:]) // dst = masqAddr, the original packet's own source + + icmp := ip6[ip6HdrLen:] + icmp[0] = icmpType + icmp[1] = 0 + // icmp[4:8] (the type-specific 4 bytes) left zero -- never read. + copy(icmp[icmp6ErrHdrLen:], embedded) + binary.BigEndian.PutUint16(icmp[2:4], 0) + + csum := ipv6L4Checksum(routerAddr, masqAddr, 58, icmp) + binary.BigEndian.PutUint16(icmp[2:4], csum) + + return pkt +} + +// testEgressReturnICMPErrorTranslatesToTenant is the shared body for the +// three RFC 4443 error-type tests below: a pre-seeded egress_conn_table +// reverse row plus an ICMPv6 error whose embedded datagram matches it must +// translate back to the originating tenant, not drop -- the issue's own +// "path MTU discovery" case (Packet Too Big) is the one with actual teeth, +// but Destination Unreachable and Time Exceeded share the identical fix. +func testEgressReturnICMPErrorTranslatesToTenant(t *testing.T, icmpType uint8) { + t.Helper() + + backendAddr := mustAddr(t, testEgressBackendAddr) + destAddr := mustAddr(t, testEgressDest) + masqAddr := mustAddr(t, testEgressMasqAddr) + workerUsid := mustAddr(t, testWorkerUsid1) + gwAddr := mustAddr(t, testGWAddr) + routerAddr := mustAddr(t, "2001:db8:ff::1") // an intermediate router; never asserted on + const masqPort = uint16(45001) + + env, cleanup := setupTestEnv(t, []netip.Addr{workerUsid}) + defer cleanup() + + objs := loadObjects(t) + installEgressConfig(t, objs) + if err := objs.GwConfigTable.Put(uint32(0), EdgenatGwConfig{GwAddr: gwAddr.As16()}); err != nil { + t.Fatalf("populate gw_config_table: %v", err) + } + + rev := EdgenatEgressConnKey{ + Proto: 6, // TCP -- the flow this error message reports on + Saddr: destAddr.As16(), Sport: htons(testEgressDestPort), + Daddr: masqAddr.As16(), Dport: htons(masqPort), + } + cv := EdgenatEgressConnValue{ + TenantArg: testTenantArg1, + BackendAddr: backendAddr.As16(), + BackendPort: htons(testEgressBackendPort), + BackendUsid: workerUsid.As16(), + DestAddr: destAddr.As16(), + DestPort: htons(testEgressDestPort), + MasqAddr: masqAddr.As16(), + MasqPort: htons(masqPort), + Proto: 6, + } + if err := objs.EgressConnTable.Put(rev, cv); err != nil { + t.Fatalf("populate egress_conn_table reverse row: %v", err) + } + + // embeddedLen == icmp6EmbeddedPortsMinBytes deliberately -- the RFC + // 4443 guaranteed minimum, well short of a full 20-byte TCP header. + pkt := buildICMPv6ErrorPacket(routerAddr, masqAddr, destAddr, icmpType, 6, + masqPort, testEgressDestPort, icmp6EmbeddedPortsMinBytes) + + ret, out := runXDP(t, objs.EdgeNat, pkt, env.ifindex) + if ret != xdpTx { + t.Fatalf("verdict = %d, want XDP_TX (%d)", ret, xdpTx) + } + + wantLen := len(pkt) + 40 // push_outer_header grows the packet by exactly 40 bytes + out = out[:wantLen] + parseEth(t, out) + + outer := out[ethHdrLen:] + if got := outer[6]; got != 41 { + t.Errorf("outer nexthdr = %d, want 41 (IPv6-in-IPv6)", got) + } + if got := netip.AddrFrom16([16]byte(outer[8:24])); got != gwAddr { + t.Errorf("outer saddr = %s, want this gateway's own address %s", got, gwAddr) + } + if got := netip.AddrFrom16([16]byte(outer[24:40])); got != workerUsid { + t.Errorf("outer daddr = %s, want the originating worker node's uSID %s", got, workerUsid) + } + + inner := outer[ip6HdrLen:] + if got := netip.AddrFrom16([16]byte(inner[24:40])); got != backendAddr { + t.Errorf("inner (ICMPv6 packet's own) daddr = %s, want backend address %s (un-masqueraded)", got, backendAddr) + } + + icmp := inner[ip6HdrLen:] + if got := icmp[0]; got != icmpType { + t.Errorf("icmp type = %d, want unchanged %d", got, icmpType) + } + embedded := icmp[icmp6ErrHdrLen:] + if got := netip.AddrFrom16([16]byte(embedded[8:24])); got != backendAddr { + t.Errorf("embedded saddr = %s, want backend address %s (un-masqueraded)", got, backendAddr) + } + if got := netip.AddrFrom16([16]byte(embedded[24:40])); got != destAddr { + t.Errorf("embedded daddr = %s, want unchanged internet destination %s", got, destAddr) + } + if got := binary.BigEndian.Uint16(embedded[40:42]); got != testEgressBackendPort { + t.Errorf("embedded source port = %d, want backend port %d (un-masqueraded)", got, testEgressBackendPort) + } + if got := binary.BigEndian.Uint16(embedded[42:44]); got != testEgressDestPort { + t.Errorf("embedded dest port = %d, want unchanged %d", got, testEgressDestPort) + } + + icmpZeroed := make([]byte, len(icmp)) + copy(icmpZeroed, icmp) + binary.BigEndian.PutUint16(icmpZeroed[2:4], 0) + wantCsum := ipv6L4Checksum(routerAddr, backendAddr, 58, icmpZeroed) + if gotCsum := binary.BigEndian.Uint16(icmp[2:4]); gotCsum != wantCsum { + t.Errorf("ICMPv6 checksum = %#04x, want %#04x (independently recomputed)", gotCsum, wantCsum) + } +} + +func TestEdgeNat_EgressReturnICMPDestUnreachableTranslatesToTenant(t *testing.T) { + testEgressReturnICMPErrorTranslatesToTenant(t, icmpv6DestUnreachable) +} + +// TestEdgeNat_EgressReturnICMPPacketTooBigTranslatesToTenant is the issue's +// own headline case: Packet Too Big is how path MTU discovery reaches a +// tenant. Dropping it (the pre-#404 behavior) is what stalled large +// transfers on smaller-MTU paths instead of letting them adapt. +func TestEdgeNat_EgressReturnICMPPacketTooBigTranslatesToTenant(t *testing.T) { + testEgressReturnICMPErrorTranslatesToTenant(t, icmpv6PacketTooBig) +} + +func TestEdgeNat_EgressReturnICMPTimeExceededTranslatesToTenant(t *testing.T) { + testEgressReturnICMPErrorTranslatesToTenant(t, icmpv6TimeExceeded) +} + +// TestEdgeNat_EgressReturnICMPUnknownConnDrops covers an ICMPv6 error +// message whose embedded tuple matches no egress_conn_table row -- the +// address is claimed, so this must drop, with a reason distinct from the +// TCP/UDP path's own DROP_REASON_NO_EGRESS_RETURN_CONN (the review comment +// on #381 asked for exactly this distinction). +func TestEdgeNat_EgressReturnICMPUnknownConnDrops(t *testing.T) { + requireRoot(t) + objs := loadObjects(t) + installEgressConfig(t, objs) + if err := objs.GwConfigTable.Put(uint32(0), EdgenatGwConfig{GwAddr: mustAddr(t, testGWAddr).As16()}); err != nil { + t.Fatalf("populate gw_config_table: %v", err) + } + + pkt := buildICMPv6ErrorPacket( + mustAddr(t, "2001:db8:ff::1"), mustAddr(t, testEgressMasqAddr), mustAddr(t, testEgressDest), + icmpv6DestUnreachable, 6, 45999, testEgressDestPort, icmp6EmbeddedPortsMinBytes, + ) + ret, _ := runXDP(t, objs.EdgeNat, pkt, 1) + if ret != xdpDrop { + t.Fatalf("verdict = %d, want XDP_DROP (%d)", ret, xdpDrop) + } + + got := sumPerCPU(t, objs.DropReasons, DropReasonNoEgressICMPConn) + if got != 1 { + t.Errorf("drop_reasons[no_egress_icmp_conn] = %d, want 1", got) + } +} + +// TestEdgeNat_EgressReturnUnhandledICMPPassesThrough covers an ICMPv6 type +// that is neither Echo Reply nor a recognized error type (a Router +// Solicitation, here) arriving addressed to masq_addr -- this must +// XDP_PASS, not XDP_DROP, the actual "pass or handle the rest" behavior +// change #404 asks for, handing it to the normal kernel stack instead of +// claiming and dropping it. +func TestEdgeNat_EgressReturnUnhandledICMPPassesThrough(t *testing.T) { + requireRoot(t) + objs := loadObjects(t) + installEgressConfig(t, objs) + + pkt := buildICMPv6EchoPacket(mustAddr(t, testEgressDest), mustAddr(t, testEgressMasqAddr), + icmpv6RouterSolicitation, 0, 0) + ret, out := runXDP(t, objs.EdgeNat, pkt, 1) + if ret != xdpPass { + t.Fatalf("verdict = %d, want XDP_PASS (%d)", ret, xdpPass) + } + if string(out[:len(pkt)]) != string(pkt) { + t.Error("XDP_PASS packet was modified, want byte-for-byte untouched") + } +} + +// TestEdgeNat_EgressForwardICMPNonEchoRequestDrops covers a tenant backend +// sending some other ICMPv6 type (an Echo Reply, here) out via egress_sid +// -- there is no defined masquerade behavior for it, so this must drop, +// not pass through (egress_sid is claimed). +func TestEdgeNat_EgressForwardICMPNonEchoRequestDrops(t *testing.T) { + requireRoot(t) + objs := loadObjects(t) + installEgressConfig(t, objs) + + pkt := buildEncappedICMPv6EchoPacket( + mustAddr(t, testWorkerUsid1), egressSIDAddr(t, testTenantArg1), + mustAddr(t, testEgressBackendAddr), mustAddr(t, testEgressDest), + icmpv6EchoReply, 0x1234, 1, + ) + ret, _ := runXDP(t, objs.EdgeNat, pkt, 1) + if ret != xdpDrop { + t.Fatalf("verdict = %d, want XDP_DROP (%d)", ret, xdpDrop) + } + + got := sumPerCPU(t, objs.DropReasons, DropReasonMalformedEgressICMP) + if got != 1 { + t.Errorf("drop_reasons[malformed_egress_icmp] = %d, want 1", got) + } +} + +// TestEdgeNat_EgressPingRoundTrip covers the full ping round trip #404 +// asks for: a tenant backend's Echo Request must masquerade both the +// source address and the identifier on the way out, and the internet +// peer's Echo Reply (which always echoes the identifier it was sent +// unchanged) must restore both the destination address and the tenant's +// own original identifier on the way back -- the identifier restoration +// is the part easy to get wrong; getting only the address right would +// still leave the tenant's own ping process unable to recognize the reply. +func TestEdgeNat_EgressPingRoundTrip(t *testing.T) { + backendAddr := mustAddr(t, testEgressBackendAddr) + destAddr := mustAddr(t, testEgressDest) + masqAddr := mustAddr(t, testEgressMasqAddr) + workerUsid := mustAddr(t, testWorkerUsid1) + gwAddr := mustAddr(t, testGWAddr) + const identifier = uint16(0xabcd) + const sequence = uint16(1) + + env, cleanup := setupTestEnv(t, []netip.Addr{destAddr, workerUsid}) + defer cleanup() + + objs := loadObjects(t) + installEgressConfig(t, objs) + if err := objs.GwConfigTable.Put(uint32(0), EdgenatGwConfig{GwAddr: gwAddr.As16()}); err != nil { + t.Fatalf("populate gw_config_table: %v", err) + } + + // Forward: the tenant backend's Echo Request leaves via egress_sid. + fwdPkt := buildEncappedICMPv6EchoPacket( + workerUsid, egressSIDAddr(t, testTenantArg1), + backendAddr, destAddr, + icmpv6EchoRequest, identifier, sequence, + ) + fwdRet, fwdOut := runXDP(t, objs.EdgeNat, fwdPkt, env.ifindex) + if fwdRet != xdpTx { + t.Fatalf("forward verdict = %d, want XDP_TX (%d)", fwdRet, xdpTx) + } + + fwdWantLen := ethHdrLen + ip6HdrLen + icmp6EchoHdrLen + fwdOut = fwdOut[:fwdWantLen] + fwdIP6 := fwdOut[ethHdrLen:] + if got := netip.AddrFrom16([16]byte(fwdIP6[8:24])); got != masqAddr { + t.Fatalf("forward saddr (SNAT) = %s, want masq_addr %s", got, masqAddr) + } + if got := netip.AddrFrom16([16]byte(fwdIP6[24:40])); got != destAddr { + t.Fatalf("forward daddr = %s, want unchanged internet destination %s", got, destAddr) + } + fwdICMP := fwdIP6[ip6HdrLen:] + gotMasqIdentifier := binary.BigEndian.Uint16(fwdICMP[4:6]) + if gotMasqIdentifier == identifier { + t.Fatalf("masqueraded identifier = original identifier %#04x, want a re-mapped value", identifier) + } + + fwdICMPZeroed := make([]byte, len(fwdICMP)) + copy(fwdICMPZeroed, fwdICMP) + binary.BigEndian.PutUint16(fwdICMPZeroed[2:4], 0) + wantFwdCsum := ipv6L4Checksum(masqAddr, destAddr, 58, fwdICMPZeroed) + if gotCsum := binary.BigEndian.Uint16(fwdICMP[2:4]); gotCsum != wantFwdCsum { + t.Errorf("forward ICMPv6 checksum = %#04x, want %#04x (independently recomputed)", gotCsum, wantFwdCsum) + } + + // Return: the internet peer echoes the request back unchanged, still + // addressed using the masqueraded identifier it actually received -- + // the real-world behavior an Echo Reply always has. + retPkt := buildICMPv6EchoPacket(destAddr, masqAddr, icmpv6EchoReply, gotMasqIdentifier, sequence) + retRet, retOut := runXDP(t, objs.EdgeNat, retPkt, env.ifindex) + if retRet != xdpTx { + t.Fatalf("return verdict = %d, want XDP_TX (%d)", retRet, xdpTx) + } + + retWantLen := len(retPkt) + 40 + retOut = retOut[:retWantLen] + parseEth(t, retOut) + + outer := retOut[ethHdrLen:] + if got := outer[6]; got != 41 { + t.Errorf("outer nexthdr = %d, want 41 (IPv6-in-IPv6)", got) + } + if got := netip.AddrFrom16([16]byte(outer[8:24])); got != gwAddr { + t.Errorf("outer saddr = %s, want this gateway's own address %s", got, gwAddr) + } + if got := netip.AddrFrom16([16]byte(outer[24:40])); got != workerUsid { + t.Errorf("outer daddr = %s, want the originating worker node's uSID %s", got, workerUsid) + } + + inner := outer[ip6HdrLen:] + if got := netip.AddrFrom16([16]byte(inner[8:24])); got != destAddr { + t.Errorf("inner saddr = %s, want unchanged internet peer address %s", got, destAddr) + } + if got := netip.AddrFrom16([16]byte(inner[24:40])); got != backendAddr { + t.Errorf("inner daddr (DNAT) = %s, want backend address %s", got, backendAddr) + } + + innerICMP := inner[ip6HdrLen:] + if got := innerICMP[0]; got != icmpv6EchoReply { + t.Errorf("icmp type = %d, want unchanged Echo Reply (%d)", got, icmpv6EchoReply) + } + if got := binary.BigEndian.Uint16(innerICMP[4:6]); got != identifier { + t.Errorf("restored identifier = %#04x, want tenant's original %#04x", got, identifier) + } + if got := binary.BigEndian.Uint16(innerICMP[6:8]); got != sequence { + t.Errorf("sequence = %d, want unchanged %d", got, sequence) + } + + innerICMPZeroed := make([]byte, len(innerICMP)) + copy(innerICMPZeroed, innerICMP) + binary.BigEndian.PutUint16(innerICMPZeroed[2:4], 0) + wantRetCsum := ipv6L4Checksum(destAddr, backendAddr, 58, innerICMPZeroed) + if gotCsum := binary.BigEndian.Uint16(innerICMP[2:4]); gotCsum != wantRetCsum { + t.Errorf("return ICMPv6 checksum = %#04x, want %#04x (independently recomputed)", gotCsum, wantRetCsum) + } +}