From 5a9f0102345643e45f616096094008c7b390ab4d Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 08:49:40 -0700 Subject: [PATCH 01/21] hack: fix DNS on IPv6-only kind clusters On a fresh IP_FAMILY=ipv6 cluster nothing resolves from inside a pod and no actor boots: CoreDNS inherits the node's IPv4 resolver, which a v6-only pod cannot reach, and "kind-registry" NXDOMAINs in atelet's own netns. Point the forward at an IPv6 upstream, overridable with IPV6_DNS_UPSTREAM, and give the registry its own server block, so it is asked for nothing but its own name. IPv4 and dual-stack clusters are unchanged, and atenet-egress still crashloops on v6-only for an unrelated Envoy bind bug. Asking once was not enough to prove that: about half of fresh clusters do not answer the first query, and a pod that goes unanswered stays unanswered, so the check re-asks with a new pod and prints what the pod saw when it gives up. It lives in hack/verify-ipv6-dns.sh rather than inline, because the registry block records an address the registry can move off and there was no way to re-check a cluster without rebuilding it. --- hack/create-kind-cluster.sh | 58 ++++++++++++++++++++ hack/verify-ipv6-dns.sh | 106 ++++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) create mode 100755 hack/verify-ipv6-dns.sh diff --git a/hack/create-kind-cluster.sh b/hack/create-kind-cluster.sh index f413e5c95..c2ffdab58 100755 --- a/hack/create-kind-cluster.sh +++ b/hack/create-kind-cluster.sh @@ -21,6 +21,7 @@ KIND_CLUSTER_NAME="${KIND_CLUSTER_NAME:-kind}" KUBECTL_CONTEXT="kind-${KIND_CLUSTER_NAME}" reg_name="kind-registry" reg_port="${KIND_REGISTRY_PORT:-5001}" +IPV6_DNS_UPSTREAM="${IPV6_DNS_UPSTREAM:-2001:4860:4860::8888 2001:4860:4860::8844}" if [[ $# -gt 0 ]]; then case "$1" in @@ -31,6 +32,8 @@ if [[ $# -gt 0 ]]; then echo "Configured through the environment:" echo " KIND_CLUSTER_NAME Name of the cluster to create (default: kind)." echo " IP_FAMILY Address families for pods and Services: ipv4, ipv6 or dual (default: ipv4)." + echo " IPV6_DNS_UPSTREAM Space-separated IPv6 resolvers CoreDNS forwards to when IP_FAMILY=ipv6" + echo " (default: Google Public DNS). Override where those are unreachable." exit 0 ;; esac @@ -196,6 +199,61 @@ if [ "$(docker inspect -f='{{json .NetworkSettings.Networks.kind}}' "${reg_name} docker network connect "kind" "${reg_name}" fi +# 4.5. Give CoreDNS an IPv6 forwarder and a registry entry +# +# CoreDNS runs dnsPolicy: Default, inheriting the node's IPv4 resolver, which +# no pod here can reach, so external lookups SERVFAIL. Step 3's registry +# wiring is node-side, so it misses atelet too: that pull runs in atelet's own +# netns, where "kind-registry" NXDOMAINs. +if [[ "${IP_FAMILY}" == "ipv6" ]]; then + echo "Repointing CoreDNS at an IPv6 resolver and teaching it '${reg_name}'..." + reg_v6="$(docker inspect "${reg_name}" \ + --format '{{.NetworkSettings.Networks.kind.GlobalIPv6Address}}')" + if [[ -z "${reg_v6}" ]]; then + echo "error: '${reg_name}' has no IPv6 address on the 'kind' network" >&2 + exit 1 + fi + + corefile="$(kubectl --context="${KUBECTL_CONTEXT}" -n kube-system get cm coredns \ + -o jsonpath='{.data.Corefile}')" + search="forward . /etc/resolv.conf" + replace="forward . ${IPV6_DNS_UPSTREAM}" + # $search unquoted: bash 3.2 splices the quotes in literally. Replacing just + # the target leaves kind's trailing "{ max_concurrent 1000 }" in place. + patched="${corefile/$search/$replace}" + if [[ "${patched}" == "${corefile}" ]]; then + echo "error: '${search}' not found in the CoreDNS Corefile" >&2 + echo " the Corefile layout changed upstream; update this block" >&2 + exit 1 + fi + + # Its own server block, not a hosts entry in .:53. A query is served by the + # one block whose zone is its longest suffix, so only "${reg_name}" arrives + # here -- which is why this hosts needs no fallthrough to avoid NXDOMAINing + # every other name. + patched="${patched} +${reg_name}:53 { + errors + hosts { + ${reg_v6} ${reg_name} + } +}" + + # A YAML patch file avoids escaping the Corefile's newlines into JSON. + { printf 'data:\n Corefile: |\n'; printf '%s\n' "${patched}" | sed 's/^/ /'; } \ + > "${ROOT}/bin/coredns-patch.yaml" + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system patch cm coredns \ + --type=merge --patch-file "${ROOT}/bin/coredns-patch.yaml" + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system rollout restart deploy/coredns + kubectl --context="${KUBECTL_CONTEXT}" -n kube-system rollout status deploy/coredns \ + --timeout=120s + + # Its own script so it can be re-run against a live cluster: the hosts entry + # above is a snapshot of an address the registry can move off (#1049). + KUBECTL_CONTEXT="${KUBECTL_CONTEXT}" REG_NAME="${reg_name}" \ + IPV6_DNS_UPSTREAM="${IPV6_DNS_UPSTREAM}" "${ROOT}"/hack/verify-ipv6-dns.sh +fi + # 5. Document the local registry in kube-public ConfigMap echo "Documenting local registry in cluster..." cat <&2 + exit 1 + ;; + esac +fi + +# Best-effort: only used to make the registry failure message actionable. +reg_v6="$(docker inspect "${REG_NAME}" \ + --format '{{.NetworkSettings.Networks.kind.GlobalIPv6Address}}' 2>/dev/null || true)" +reg_at="${reg_v6:+ at [${reg_v6}]:5000}" + +echo "Verifying DNS from a pod..." +# Probe from a pod, not the node: the node is dual-stack and passes either way. +# The registry leg fetches rather than resolves -- the hosts entry is AAAA-only, +# which fails nslookup's A query but satisfies getaddrinfo. +# +# --attach gives one stream and only the last leg's exit status, so each leg +# reports a marker on stdout and no failure message may contain one; PROBE_RAN +# separates a failed leg from a pod that never ran. Retry the pod, not the +# query: one that asks before CoreDNS settles stays broken for ~30s, while a +# fresh pod 10s later resolves first try. +probe="" +probe_max=4 +for ((probe_attempt = 1; probe_attempt <= probe_max; probe_attempt++)); do + attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" run "coredns-probe-$$-${probe_attempt}" \ + --rm --attach --quiet --restart=Never --image=busybox:1.36 --command -- \ + sh -c "echo PROBE_RAN + if out=\$(nslookup storage.googleapis.com 2>&1); then + echo RESOLVE_OK + else + echo \"resolve failed: \$(echo \"\$out\" | tail -2 | tr '\n' ' ')\" + fi + if out=\$(wget -T10 -O/dev/null http://${REG_NAME}:5000/v2/ 2>&1); then + echo REGISTRY_OK + else + echo \"registry fetch failed: \$(echo \"\$out\" | tail -1)\" + fi")" || true + # A pod that never started must not bury an earlier one's real failure. + if [[ "${attempt_out}" == *PROBE_RAN* ]]; then probe="${attempt_out}"; fi + # Only the resolve leg is a settling race; a down registry will not fix itself. + [[ "${probe}" == *RESOLVE_OK* ]] && break + if ((probe_attempt < probe_max)); then + echo " the cluster is not resolving yet; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." + sleep 10 + fi +done +if [[ "${probe}" != *RESOLVE_OK* || "${probe}" != *REGISTRY_OK* ]]; then + if [[ "${probe}" != *PROBE_RAN* ]]; then + echo "error: the probe pod never ran, so CoreDNS is unverified" >&2 + echo " check that it scheduled and that 'busybox:1.36' pulled" >&2 + elif [[ "${probe}" != *RESOLVE_OK* ]]; then + echo "error: a pod cannot resolve an external name" >&2 + echo " IPV6_DNS_UPSTREAM is '${IPV6_DNS_UPSTREAM}'; set it to a reachable resolver" >&2 + else + echo "error: DNS works but a pod cannot reach '${REG_NAME}'${reg_at}" >&2 + echo " check the registry container is up and on the 'kind' network" >&2 + fi + if [[ -n "${probe}" ]]; then + echo " probe output was:" >&2 + printf '%s\n' "${probe}" | sed 's/^/ /' >&2 + fi + exit 1 +fi From 74ad449ffd55539e80827b3a0aceaea87241abec Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Sat, 8 Aug 2026 10:58:33 -0700 Subject: [PATCH 02/21] atenet/router: bind the Envoy ingress listeners dual-stack The HTTP and HTTPS ingress listeners bound 0.0.0.0 only, so on a dual-stack cluster Envoy answered on the router Service's IPv4 ClusterIP and on nothing at all for IPv6. Each primary socket now carries an additional "::" address on the same port. Ipv4Compat stays false on the additional address: clearing IPV6_V6ONLY would collide with the primary already bound to that port. Leaving the primary alone is what keeps an IPv4-only cluster unchanged -- with the caveat that a node lacking AF_INET6 entirely could not bind "::" and the listener would not come up. First of three commits binding atenet's gateways dual-stack. (cherry picked from commit 501991d285a1efeb9a146a029136e2eb140001f4) --- cmd/atenet/internal/router/xds.go | 23 ++++++++++++++ cmd/atenet/internal/router/xds_test.go | 44 +++++++++++++++++++------- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index e2d76d8bf..8a02f4e23 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -1108,6 +1108,27 @@ func (x *XdsServer) buildTracing() *hcmv3.HttpConnectionManager_Tracing { } } +// dualStackAdditionalAddresses returns the IPv6 half of a dual-stack ingress +// listener, to pair with a primary 0.0.0.0 socket on the same port. Ipv4Compat +// stays false: clearing IPV6_V6ONLY would collide with that primary. +func dualStackAdditionalAddresses(port uint32) []*listenerv3.AdditionalAddress { + return []*listenerv3.AdditionalAddress{ + { + Address: &corev3.Address{ + Address: &corev3.Address_SocketAddress{ + SocketAddress: &corev3.SocketAddress{ + Address: "::", + Ipv4Compat: false, + PortSpecifier: &corev3.SocketAddress_PortValue{ + PortValue: port, + }, + }, + }, + }, + }, + } +} + func (x *XdsServer) buildListener() *listenerv3.Listener { hcm := x.buildHcm("ingress_http", true) @@ -1123,6 +1144,7 @@ func (x *XdsServer) buildListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.ingressPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1182,6 +1204,7 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.httpsPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 6fa5c428b..6a1569275 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -48,6 +48,36 @@ import ( "github.com/agent-substrate/substrate/internal/atunnel" ) +// assertDualStackIngress checks an ingress listener keeps its 0.0.0.0 primary +// and gains exactly one "::" socket on the same port. +func assertDualStackIngress(t *testing.T, l *listenerv3.Listener, wantPort uint32) { + t.Helper() + + sa := l.GetAddress().GetSocketAddress() + if sa.GetAddress() != "0.0.0.0" { + t.Errorf("Expected address '0.0.0.0', got %s", sa.GetAddress()) + } + if sa.GetPortValue() != wantPort { + t.Errorf("Expected port %d, got %d", wantPort, sa.GetPortValue()) + } + + addrs := l.GetAdditionalAddresses() + if len(addrs) != 1 { + t.Fatalf("Expected 1 additional address on %s, got %d", l.GetName(), len(addrs)) + } + + asa := addrs[0].GetAddress().GetSocketAddress() + if asa.GetAddress() != "::" { + t.Errorf("Expected additional address '::', got %s", asa.GetAddress()) + } + if asa.GetIpv4Compat() { + t.Error("Expected additional address Ipv4Compat to be false") + } + if asa.GetPortValue() != wantPort { + t.Errorf("Expected additional port %d, got %d", wantPort, asa.GetPortValue()) + } +} + func TestXdsServer_UpdateSnapshot(t *testing.T) { server := NewXdsServer(18000) server.SetConfig(8081, 50052, "10.0.0.1") @@ -150,14 +180,7 @@ func TestXdsServer_UpdateSnapshot(t *testing.T) { if raw, exists := listenersMap[IngressHTTPListener]; !exists { t.Errorf("Listener name '%s' is missing from snapshot listeners", IngressHTTPListener) } else { - l := raw.(*listenerv3.Listener) - sa := l.GetAddress().GetSocketAddress() - if sa.GetPortValue() != 8081 { - t.Errorf("Expected port 8081, got %d", sa.GetPortValue()) - } - if sa.GetAddress() != "0.0.0.0" { - t.Errorf("Expected address '0.0.0.0', got %s", sa.GetAddress()) - } + assertDualStackIngress(t, raw.(*listenerv3.Listener), 8081) } } @@ -192,10 +215,7 @@ func TestXdsServer_UpdateSnapshot_WithHttps(t *testing.T) { t.Errorf("Listener name '%s' is missing from snapshot listeners", IngressHTTPSListener) } else { l := raw.(*listenerv3.Listener) - sa := l.GetAddress().GetSocketAddress() - if sa.GetPortValue() != 8443 { - t.Errorf("Expected port 8443, got %d", sa.GetPortValue()) - } + assertDualStackIngress(t, l, 8443) // Verify the TLS config references the serving cert via SDS rather // than embedding it: inline filename DataSources are read only once From 9f6bcdf18b044d4e7256e7fd289e0f773b643266 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 12 Aug 2026 21:41:31 -0700 Subject: [PATCH 03/21] atenet/router: bind the admin socket and Service dual-stack The Envoy admin socket bound 0.0.0.0, and the atenet-router Service carried no ipFamilyPolicy -- which the API server defaults to SingleStack, one IPv4 ClusterIP and nothing else. Between them the router had no IPv6 address to answer on. The socket now binds "::" with ipv4_compat, one socket for both families, and the Service asks for PreferDualStack. bootstrap.v3.Admin takes a single address and has no additional_addresses, so the ingress listeners' shape is not available here; ipv4_compat is what makes the one socket serve both families. It is load-bearing: dataplane.go health-checks the admin listener over http://127.0.0.1:9901/ready, so a bare "::" would report the dataplane component of /statusz unhealthy. Prefer, not Require, keeps the Service valid on a single-stack cluster; spec.ipFamilies is left alone because the primary family is immutable and the API server appends the secondary itself. (cherry picked from commit 2a21292a3262d4a642c723550881d253737e410d) --- cmd/atenet/internal/router/dataplane.go | 2 ++ manifests/ate-install/atenet-router.yaml | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cmd/atenet/internal/router/dataplane.go b/cmd/atenet/internal/router/dataplane.go index bd9f2abfc..ff886dcaa 100644 --- a/cmd/atenet/internal/router/dataplane.go +++ b/cmd/atenet/internal/router/dataplane.go @@ -36,6 +36,8 @@ type dataplaneHealthCheck struct { // untouched, so atunnel always authorizes by the actor's own DNS name -- // ingress.New needs no per-dataplane routing mode. +// healthCheck dials IPv4 loopback, which is why the admin socket in +// manifests/ate-install/atenet-router.yaml needs ipv4_compat. func (r atenetRouter) healthCheck() dataplaneHealthCheck { switch r { case atenetRouterEnvoy: diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index e05e06efb..462e2a8ad 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -86,7 +86,9 @@ data: admin: address: socket_address: - address: 0.0.0.0 + # ipv4_compat is load-bearing: dataplane.go probes /ready over IPv4 loopback. + address: "::" + ipv4_compat: true port_value: 9901 node: @@ -354,6 +356,8 @@ metadata: namespace: ate-system spec: type: ClusterIP + # Prefer, not Require: Require fails Service creation on a single-stack cluster. + ipFamilyPolicy: PreferDualStack selector: app: atenet-router ports: From 908499f83c9052dc56e82925cc4ab6d6a166e2a1 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 13 Aug 2026 07:20:18 -0700 Subject: [PATCH 04/21] atenet/egress: bind the Envoy sockets and Service dual-stack The gateway's admin and :443 sockets bound 0.0.0.0, so on an IPv6-primary cluster the kubelet probed the pod on its only address and atenet-egress crashlooped -- Envoy started fine and logged "admin address: 0.0.0.0:15000" -- while an actor's CONNECT had no v6 path in. Both sockets now bind "::" with ipv4_compat, and the Service asks for PreferDualStack so a dual-stack cluster hands out an IPv6 ClusterIP to reach them on. One socket here rather than the ingress listeners' pair: IPv4 peers then arrive as ::ffff: addresses, and nothing on this path reads the peer -- actor identity comes from the client certificate and the access log records the cert SAN. ipv4_compat also has to stay on the admin socket, because the ext-proc sidecar's drainer dials 127.0.0.1:15000 and envoydrain.go reads a refusal there as "Envoy already exited", skipping the drain silently. Last of three. (cherry picked from commit 2549657bc13650207b28ae49a80b7e2e5e790c5e) --- manifests/ate-install/atenet-egress.yaml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/manifests/ate-install/atenet-egress.yaml b/manifests/ate-install/atenet-egress.yaml index 591ef31fc..a655a3759 100644 --- a/manifests/ate-install/atenet-egress.yaml +++ b/manifests/ate-install/atenet-egress.yaml @@ -37,12 +37,15 @@ data: envoy.yaml: | admin: address: - socket_address: { address: 0.0.0.0, port_value: 15000 } + # ipv4_compat is load-bearing: see --envoy-admin-address below. + socket_address: { address: "::", ipv4_compat: true, port_value: 15000 } static_resources: listeners: - name: egress address: - socket_address: { address: 0.0.0.0, port_value: 443 } + # ipv4_compat rather than a second socket: IPv4 peers arrive as + # ::ffff: and nothing here reads the peer -- identity is the cert. + socket_address: { address: "::", ipv4_compat: true, port_value: 443 } filter_chains: # Named so ext_proc can read it back as xds.filter_chain_name. Must # match EgressFilterChainName in @@ -379,6 +382,8 @@ metadata: namespace: ate-system spec: type: ClusterIP + # Prefer, not Require: Require fails Service creation on a single-stack cluster. + ipFamilyPolicy: PreferDualStack selector: app: atenet-egress ports: From ac9ee2dc6006d11df5552fc716611da1838cf6f4 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 20:39:32 -0700 Subject: [PATCH 05/21] hack/verify: keep the gateway Envoy admin sockets dual-stack Both gateway admin sockets bind "::" with ipv4_compat, and the flag is what keeps their in-pod callers working: dataplane.go health-checks the router's over IPv4 loopback, and envoydrain.go dials the egress one the same way and reads a refusal as "Envoy already exited", skipping the drain without reporting an error. No Go test, golden file, or verify script read either manifest, so dropping the flag would have failed silently. make verify now rejects an admin socket that binds "::" without it. (cherry picked from commit 4b478a0ead243a0f8f77af9e2ed969996874d2cd) --- hack/verify/atenet-admin-bind.sh | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100755 hack/verify/atenet-admin-bind.sh diff --git a/hack/verify/atenet-admin-bind.sh b/hack/verify/atenet-admin-bind.sh new file mode 100755 index 000000000..86090c016 --- /dev/null +++ b/hack/verify/atenet-admin-bind.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash + +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Dropping ipv4_compat from a gateway's Envoy admin socket fails silently: the +# drain sequence reads the refused IPv4 loopback dial as "Envoy already exited" +# and reports a drain it never performed. No Go test reads these manifests. + +set -o errexit -o nounset -o pipefail + +ROOT="$(git rev-parse --show-toplevel)" +cd "${ROOT}" + +rc=0 +for f in manifests/ate-install/atenet-router.yaml manifests/ate-install/atenet-egress.yaml; do + block="$(grep -A 6 -E '^ *admin:$' "${f}" || true)" + if [[ -z "${block}" ]]; then + echo "${f}: no Envoy admin block found; this check needs updating" >&2 + rc=1 + elif ! grep -q '"::"' <<<"${block}"; then + echo "${f}: Envoy admin socket does not bind \"::\"; an IPv6-primary pod cannot be probed" >&2 + rc=1 + elif ! grep -q 'ipv4_compat: true' <<<"${block}"; then + echo "${f}: Envoy admin socket binds \"::\" without ipv4_compat; IPv4 loopback dials will be refused" >&2 + rc=1 + fi +done + +exit "${rc}" From f047e731bdc368cb47fcc9d237f56bb1fd466d93 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 10:24:50 -0700 Subject: [PATCH 06/21] atenet/router: bind the CONNECT listeners dual-stack too The CONNECT-terminating listeners landed after the first commit of this series, so they kept a bare 0.0.0.0 socket while ingress HTTP and HTTPS gained their "::" pair. Give them the same additional address, so all four of the router's socket listeners answer on both families. Both are port-gated and no e2e suite configures them yet, which is why nothing caught this; the internal main_internal listener has no socket and needs nothing. (cherry picked from commit 54c727d22843556dfcbd06a4910087efe2114ba2) --- cmd/atenet/internal/router/xds.go | 2 ++ cmd/atenet/internal/router/xds_test.go | 8 +++----- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 8a02f4e23..903ac1fff 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -1236,6 +1236,7 @@ func (x *XdsServer) buildConnectTerminateListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.connectPlainTextPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ @@ -1269,6 +1270,7 @@ func (x *XdsServer) buildConnectTerminateTLSListener() *listenerv3.Listener { }, }, }, + AdditionalAddresses: dualStackAdditionalAddresses(uint32(x.connectTLSPort)), FilterChains: []*listenerv3.FilterChain{ { Filters: []*listenerv3.Filter{ diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 6a1569275..ea39fb7c4 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -374,16 +374,14 @@ func TestXdsServer_UpdateSnapshot_WithConnect(t *testing.T) { } if raw, exists := listenersMap["connect_terminate"]; !exists { t.Error("connect_terminate listener missing") - } else if sa := raw.(*listenerv3.Listener).GetAddress().GetSocketAddress(); sa.GetPortValue() != 8081 { - t.Errorf("Expected connect_terminate port 8081, got %d", sa.GetPortValue()) + } else { + assertDualStackIngress(t, raw.(*listenerv3.Listener), 8081) } if raw, exists := listenersMap["connect_terminate_tls"]; !exists { t.Error("connect_terminate_tls listener missing") } else { l := raw.(*listenerv3.Listener) - if sa := l.GetAddress().GetSocketAddress(); sa.GetPortValue() != 8444 { - t.Errorf("Expected connect_terminate_tls port 8444, got %d", sa.GetPortValue()) - } + assertDualStackIngress(t, l, 8444) ts := l.GetFilterChains()[0].GetTransportSocket() if ts.GetName() != "envoy.transport_sockets.tls" { t.Errorf("Expected connect_terminate_tls to be TLS-wrapped, got transport socket %q", ts.GetName()) From 5ad28ad0a9f7b5962766e9fdb8fd14dd35857dd8 Mon Sep 17 00:00:00 2001 From: lubingtan Date: Wed, 5 Aug 2026 20:58:54 +0800 Subject: [PATCH 07/21] atunnel: support IPv6 original destination lookup TCPOriginalDestination read only the IPv4 SOL_IP/SO_ORIGINAL_DST, so an actor's IPv6 connection redirected into the transparent egress listener had no destination to dial and the proxy failed it. Read IP6T_SO_ORIGINAL_DST too, falling back to it only when the IPv4 lookup returns ENOENT, so unrelated IPv4 failures keep their own error. One step towards dual-stack actor networking; the actor veth and its nftables rules are still IPv4-only. Co-authored-by: Yuan Gao (cherry picked from commit d8527b5e8ea24e588f694da726ee76784d11c41b) --- internal/atunnel/original_dst_linux.go | 68 +++- internal/atunnel/original_dst_linux_test.go | 395 ++++++++++++++++++++ 2 files changed, 444 insertions(+), 19 deletions(-) create mode 100644 internal/atunnel/original_dst_linux_test.go diff --git a/internal/atunnel/original_dst_linux.go b/internal/atunnel/original_dst_linux.go index 07dd0f934..8b4309119 100644 --- a/internal/atunnel/original_dst_linux.go +++ b/internal/atunnel/original_dst_linux.go @@ -18,6 +18,7 @@ package atunnel import ( "encoding/binary" + "errors" "fmt" "net" "strconv" @@ -26,10 +27,12 @@ import ( "golang.org/x/sys/unix" ) -// TCPOriginalDestination reads the IPv4 destination preserved by a Linux -// REDIRECT rule. Actor networking is currently IPv4-only. -// TODO(liorlieberman) add the IPv6 IP6T_SO_ORIGINAL_DST variant -// when actor veth setup gains dual-stack support. +// IP6T_SO_ORIGINAL_DST is not generated by golang.org/x/sys/unix. It is +// defined as 80 in linux/netfilter_ipv6/ip6_tables.h. +const ip6tSOOriginalDst = 80 + +// TCPOriginalDestination reads the IPv4 or IPv6 destination preserved by a +// Linux REDIRECT rule. func TCPOriginalDestination(conn net.Conn) (string, error) { tcpConn, ok := conn.(*net.TCPConn) if !ok { @@ -40,21 +43,15 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { return "", fmt.Errorf("atunnel: acquiring TCP syscall connection: %w", err) } - var addr unix.RawSockaddrInet4 var sockoptErr error + var destination string if err := rawConn.Control(func(fd uintptr) { - size := uint32(unsafe.Sizeof(addr)) - _, _, errno := unix.Syscall6( - unix.SYS_GETSOCKOPT, - fd, - unix.SOL_IP, - unix.SO_ORIGINAL_DST, - uintptr(unsafe.Pointer(&addr)), - uintptr(unsafe.Pointer(&size)), - 0, - ) - if errno != 0 { - sockoptErr = errno + destination, sockoptErr = originalIPv4Destination(fd) + // Linux returns ENOENT when the IPv4 original-destination option is + // queried on a redirected IPv6 connection. Only then try the IPv6 + // equivalent, so unrelated IPv4 failures retain their original error. + if errors.Is(sockoptErr, unix.ENOENT) { + destination, sockoptErr = originalIPv6Destination(fd) } }); err != nil { return "", fmt.Errorf("atunnel: accessing TCP socket: %w", err) @@ -62,11 +59,44 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { if sockoptErr != nil { return "", fmt.Errorf("atunnel: reading original TCP destination: %w", sockoptErr) } + return destination, nil +} + +func originalIPv4Destination(fd uintptr) (string, error) { + var addr unix.RawSockaddrInet4 + if errno := getOriginalDestination(fd, unix.SOL_IP, unix.SO_ORIGINAL_DST, unsafe.Pointer(&addr), unsafe.Sizeof(addr)); errno != 0 { + return "", errno + } + return formatOriginalDestination(addr.Addr[:], addr.Port) +} + +func originalIPv6Destination(fd uintptr) (string, error) { + var addr unix.RawSockaddrInet6 + if errno := getOriginalDestination(fd, unix.SOL_IPV6, ip6tSOOriginalDst, unsafe.Pointer(&addr), unsafe.Sizeof(addr)); errno != 0 { + return "", errno + } + return formatOriginalDestination(addr.Addr[:], addr.Port) +} + +func getOriginalDestination(fd uintptr, level, option int, addr unsafe.Pointer, addrSize uintptr) unix.Errno { + size := uint32(addrSize) + _, _, errno := unix.Syscall6( + unix.SYS_GETSOCKOPT, + fd, + uintptr(level), + uintptr(option), + uintptr(addr), + uintptr(unsafe.Pointer(&size)), + 0, + ) + return errno +} - portBytes := (*[2]byte)(unsafe.Pointer(&addr.Port)) +func formatOriginalDestination(ip []byte, rawPort uint16) (string, error) { + portBytes := (*[2]byte)(unsafe.Pointer(&rawPort)) port := binary.BigEndian.Uint16(portBytes[:]) if port == 0 { return "", fmt.Errorf("atunnel: original TCP destination has port zero") } - return net.JoinHostPort(net.IP(addr.Addr[:]).String(), strconv.Itoa(int(port))), nil + return net.JoinHostPort(net.IP(ip).String(), strconv.Itoa(int(port))), nil } diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go new file mode 100644 index 000000000..4f26d9805 --- /dev/null +++ b/internal/atunnel/original_dst_linux_test.go @@ -0,0 +1,395 @@ +//go:build linux + +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package atunnel + +import ( + "context" + "errors" + "fmt" + "net" + "os" + "strings" + "testing" + "time" + + "github.com/google/nftables" + "github.com/google/nftables/binaryutil" + "github.com/google/nftables/expr" + "github.com/vishvananda/netlink" + "github.com/vishvananda/netns" + "golang.org/x/sys/unix" + + "github.com/agent-substrate/substrate/internal/ateomnet" + "github.com/agent-substrate/substrate/internal/roottest" +) + +func TestTCPOriginalDestination(t *testing.T) { + roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") + + // Model the production path rather than redirecting a locally generated + // connection through OUTPUT. Actor egress enters the worker netns through a + // veth and is redirected in PREROUTING; that is the path on which Linux + // preserves SO_ORIGINAL_DST for atunnel. + actorNS := newTestNetNS(t) + actorIP, hostIP := setupTestVeth(t, actorNS) + // targetListener reserves the port the actor intends to reach. The NAT rule + // below must prevent connections from reaching it. + // + // redirectListener represents atunnel's local egress listener. It receives + // the redirected connection and is therefore the connection on which we ask + // Linux for the original destination. + redirectListener := listenTCP(t, hostIP) + defer redirectListener.Close() + targetListener := listenTCP(t, hostIP) + defer targetListener.Close() + targetPort := targetListener.Addr().(*net.TCPAddr).Port + + table := &nftables.Table{Family: nftables.TableFamilyIPv4, Name: fmt.Sprintf("atunnel_original_dst_test_%d", os.Getpid())} + installOriginalDstRedirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + + clientDone := make(chan error, 1) + go func() { + // From the actor's perspective this is an ordinary connection to + // hostIP:targetPort. The worker's PREROUTING rule redirects it before + // it reaches the host network stack's local delivery path. + clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + if err == nil { + _ = conn.Close() + } + return err + }) + }() + + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + t.Fatal(err) + } + redirected, err := redirectListener.Accept() + if err != nil { + t.Fatalf("accepting redirected connection: %v", err) + } + defer redirected.Close() + + // The accepted socket is addressed to redirectListener, but the kernel's + // SO_ORIGINAL_DST record must still contain the destination chosen by the + // actor before nftables rewrote it. + got, err := TCPOriginalDestination(redirected) + if err != nil { + t.Fatalf("TCPOriginalDestination: %v", err) + } + want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) + if got != want { + t.Errorf("original destination = %q, want %q", got, want) + } + if err := <-clientDone; err != nil { + t.Fatalf("dialing redirected connection: %v", err) + } +} + +func TestTCPOriginalDestinationIPv6(t *testing.T) { + roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") + + actorNS := newTestNetNS(t) + actorIP, hostIP := setupTestIPv6Veth(t, actorNS) + redirectListener := listenTCP6(t, hostIP) + defer redirectListener.Close() + targetListener := listenTCP6(t, hostIP) + defer targetListener.Close() + targetPort := targetListener.Addr().(*net.TCPAddr).Port + + table := &nftables.Table{Family: nftables.TableFamilyIPv6, Name: fmt.Sprintf("atunnel_original_dst_ipv6_test_%d", os.Getpid())} + installOriginalDstIPv6Redirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + + clientDone := make(chan error, 1) + go func() { + clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + if err == nil { + _ = conn.Close() + } + return err + }) + }() + + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + t.Fatal(err) + } + redirected, err := redirectListener.Accept() + if err != nil { + t.Fatalf("accepting redirected IPv6 connection: %v", err) + } + defer redirected.Close() + + // This assertion captures the IPv6 behavior required by #686. + got, err := TCPOriginalDestination(redirected) + if err != nil { + t.Fatalf("TCPOriginalDestination: %v", err) + } + want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) + if got != want { + t.Errorf("original IPv6 destination = %q, want %q", got, want) + } + if err := <-clientDone; err != nil { + t.Fatalf("dialing redirected IPv6 connection: %v", err) + } +} + +func newTestNetNS(t *testing.T) netns.NsHandle { + t.Helper() + name := fmt.Sprintf("atunnel-original-dst-%d", os.Getpid()) + ns, err := ateomnet.CreateNetNSWithoutSwitching(name) + if err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_SYS_ADMIN to create network namespace: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + _ = ns.Close() + if err := netns.DeleteNamed(name); err != nil { + t.Errorf("deleting test network namespace: %v", err) + } + }) + return ns +} + +func setupTestVeth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { + t.Helper() + hostName := fmt.Sprintf("atod%d", os.Getpid()) + peerName := fmt.Sprintf("atop%d", os.Getpid()) + if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + if link, err := netlink.LinkByName(hostName); err == nil { + if err := netlink.LinkDel(link); err != nil { + t.Errorf("deleting test veth: %v", err) + } + } + }) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + t.Fatal(err) + } + // Allocate one of the /30s in 198.18.0.0/16 from the PID so concurrent + // test processes do not try to use the same host-side address. + network := uint16(os.Getpid() % (1 << 14)) + thirdOctet := byte(network >> 6) + fourthOctet := byte(network&0x3f) << 2 + hostIP = net.IPv4(198, 18, thirdOctet, fourthOctet+1) + actorIP = net.IPv4(198, 18, thirdOctet, fourthOctet+2) + if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(30, 32)}}); err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetUp(hostLink); err != nil { + t.Fatal(err) + } + peer, err := netlink.LinkByName(peerName) + if err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { + t.Fatal(err) + } + // Complete the actor end of the point-to-point link inside its own netns. + if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + lo, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(lo); err != nil { + return err + } + link, err := netlink.LinkByName(peerName) + if err != nil { + return err + } + if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(30, 32)}}); err != nil { + return err + } + return netlink.LinkSetUp(link) + }); err != nil { + t.Fatal(err) + } + return actorIP, hostIP +} + +func listenTCP(t *testing.T, hostIP net.IP) net.Listener { + t.Helper() + listener, err := net.ListenTCP("tcp4", &net.TCPAddr{IP: hostIP, Port: 0}) + if err != nil { + t.Fatal(err) + } + return listener +} + +func setupTestIPv6Veth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { + t.Helper() + hostName := fmt.Sprintf("atod6%d", os.Getpid()) + peerName := fmt.Sprintf("atop6%d", os.Getpid()) + if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) + } + t.Fatal(err) + } + t.Cleanup(func() { + if link, err := netlink.LinkByName(hostName); err == nil { + if err := netlink.LinkDel(link); err != nil { + t.Errorf("deleting test IPv6 veth: %v", err) + } + } + }) + hostLink, err := netlink.LinkByName(hostName) + if err != nil { + t.Fatal(err) + } + prefix := uint16(os.Getpid()) + hostIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::1", prefix)) + actorIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::2", prefix)) + // This isolated veth has no competing IPv6 peers. Suppress DAD so the + // address can be bound immediately instead of remaining tentative while + // the test is trying to start its listener. + if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetUp(hostLink); err != nil { + t.Fatal(err) + } + peer, err := netlink.LinkByName(peerName) + if err != nil { + t.Fatal(err) + } + if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { + t.Fatal(err) + } + if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + lo, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(lo); err != nil { + return err + } + link, err := netlink.LinkByName(peerName) + if err != nil { + return err + } + if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { + return err + } + return netlink.LinkSetUp(link) + }); err != nil { + t.Fatal(err) + } + return actorIP, hostIP +} + +func listenTCP6(t *testing.T, hostIP net.IP) net.Listener { + t.Helper() + listener, err := net.ListenTCP("tcp6", &net.TCPAddr{IP: hostIP, Port: 0}) + if err != nil { + t.Fatal(err) + } + return listener +} + +func installOriginalDstRedirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { + t.Helper() + c := &nftables.Conn{} + c.AddTable(table) + chain := c.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, + // Restrict the rule to this test's actor so the temporary table cannot + // affect unrelated local TCP traffic. + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To4()}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, + &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, + &expr.Redir{RegisterProtoMin: 1}, + }, + }) + if err := c.Flush(); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to install nftables rule: %v", err) + } + t.Fatalf("installing nftables redirect: %v", err) + } + t.Cleanup(func() { + cleanup := &nftables.Conn{} + cleanup.DelTable(table) + if err := cleanup.Flush(); err != nil { + t.Errorf("removing nftables redirect: %v", err) + } + }) +} + +func installOriginalDstIPv6Redirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { + t.Helper() + c := &nftables.Conn{} + c.AddTable(table) + chain := c.AddChain(&nftables.Chain{ + Name: "prerouting", + Table: table, + Type: nftables.ChainTypeNAT, + Hooknum: nftables.ChainHookPrerouting, + Priority: nftables.ChainPriorityNATDest, + }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: chain, + Exprs: []expr.Any{ + &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, + // An IPv6 source address begins eight bytes into the IPv6 header. + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 8, Len: 16}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To16()}, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, + &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, + &expr.Redir{RegisterProtoMin: 1}, + }, + }) + if err := c.Flush(); err != nil { + if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { + t.Skipf("needs CAP_NET_ADMIN to install IPv6 nftables rule: %v", err) + } + t.Fatalf("installing IPv6 nftables redirect: %v", err) + } + t.Cleanup(func() { + cleanup := &nftables.Conn{} + cleanup.DelTable(table) + if err := cleanup.Flush(); err != nil { + t.Errorf("removing IPv6 nftables redirect: %v", err) + } + }) +} From cb7d66e2d648482d3e2c31891d3721c954d5750a Mon Sep 17 00:00:00 2001 From: lubingtan Date: Fri, 21 Aug 2026 11:46:21 +0800 Subject: [PATCH 08/21] atunnel: preserve IPv4 original destination errors --- internal/atunnel/original_dst_linux.go | 16 +++++-- internal/atunnel/original_dst_linux_test.go | 47 +++++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/internal/atunnel/original_dst_linux.go b/internal/atunnel/original_dst_linux.go index 8b4309119..b9d171747 100644 --- a/internal/atunnel/original_dst_linux.go +++ b/internal/atunnel/original_dst_linux.go @@ -42,15 +42,23 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { if err != nil { return "", fmt.Errorf("atunnel: acquiring TCP syscall connection: %w", err) } + // The IPv6 option is only meaningful on an AF_INET6 socket: on AF_INET the + // kernel returns EOPNOTSUPP, which would mask the real IPv4 error. A + // v4-mapped local address still means an IPv4 flow, so To4 is the test. + local, ok := tcpConn.LocalAddr().(*net.TCPAddr) + if !ok { + return "", fmt.Errorf("atunnel: original destination requires a TCP local address, got %T", tcpConn.LocalAddr()) + } + isIPv6 := local.IP.To4() == nil var sockoptErr error var destination string if err := rawConn.Control(func(fd uintptr) { destination, sockoptErr = originalIPv4Destination(fd) - // Linux returns ENOENT when the IPv4 original-destination option is - // queried on a redirected IPv6 connection. Only then try the IPv6 - // equivalent, so unrelated IPv4 failures retain their original error. - if errors.Is(sockoptErr, unix.ENOENT) { + // A pure-IPv6 socket leaves the inet addresses zeroed, so the IPv4 + // conntrack lookup always misses with ENOENT. That is the redirected + // IPv6 connection, and the only case worth retrying. + if isIPv6 && errors.Is(sockoptErr, unix.ENOENT) { destination, sockoptErr = originalIPv6Destination(fd) } }); err != nil { diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go index 4f26d9805..ef647c331 100644 --- a/internal/atunnel/original_dst_linux_test.go +++ b/internal/atunnel/original_dst_linux_test.go @@ -37,6 +37,53 @@ import ( "github.com/agent-substrate/substrate/internal/roottest" ) +// TestTCPOriginalDestinationPreservesErrno covers the failure path on an +// ordinary connection that no REDIRECT rule touched. The IPv4 lookup misses +// and reports ENOENT; that error must reach the caller. Retrying the IPv6 +// option on an AF_INET socket would replace it with EOPNOTSUPP, which says +// nothing about why the lookup failed. +// +// It runs in a fresh namespace because conntrack tracks loopback in any +// namespace that has nftables rules — including the one Docker runs in — and a +// tracked connection returns its real destination instead of missing. +func TestTCPOriginalDestinationPreservesErrno(t *testing.T) { + roottest.Require(t, "CAP_SYS_ADMIN for a network namespace with no conntrack hooks") + + ns := newTestNetNS(t) + if err := ateomnet.NetNSDo(context.Background(), ns, func(context.Context) error { + loopback, err := netlink.LinkByName("lo") + if err != nil { + return err + } + if err := netlink.LinkSetUp(loopback); err != nil { + return err + } + + listener, err := net.Listen("tcp4", "127.0.0.1:0") + if err != nil { + return err + } + defer listener.Close() + client, err := net.DialTimeout("tcp4", listener.Addr().String(), time.Second) + if err != nil { + return err + } + defer client.Close() + server, err := listener.Accept() + if err != nil { + return err + } + defer server.Close() + + if _, err := TCPOriginalDestination(server); !errors.Is(err, unix.ENOENT) { + return fmt.Errorf("want the IPv4 lookup's ENOENT, got %w", err) + } + return nil + }); err != nil { + t.Fatal(err) + } +} + func TestTCPOriginalDestination(t *testing.T) { roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") From 583a7b87f5aa3895216b01c0d44e89e1d7b31747 Mon Sep 17 00:00:00 2001 From: lubingtan Date: Fri, 21 Aug 2026 11:50:58 +0800 Subject: [PATCH 09/21] atunnel: stabilize original destination tests --- internal/atunnel/original_dst_linux_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go index ef647c331..3a9dbfca2 100644 --- a/internal/atunnel/original_dst_linux_test.go +++ b/internal/atunnel/original_dst_linux_test.go @@ -114,7 +114,7 @@ func TestTCPOriginalDestination(t *testing.T) { // hostIP:targetPort. The worker's PREROUTING rule redirects it before // it reaches the host network stack's local delivery path. clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) if err == nil { _ = conn.Close() } @@ -122,7 +122,7 @@ func TestTCPOriginalDestination(t *testing.T) { }) }() - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { t.Fatal(err) } redirected, err := redirectListener.Accept() @@ -164,7 +164,7 @@ func TestTCPOriginalDestinationIPv6(t *testing.T) { clientDone := make(chan error, 1) go func() { clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), time.Second) + conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) if err == nil { _ = conn.Close() } @@ -172,7 +172,7 @@ func TestTCPOriginalDestinationIPv6(t *testing.T) { }) }() - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(time.Second)); err != nil { + if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { t.Fatal(err) } redirected, err := redirectListener.Accept() From deb103ff673f4baf2eac4a59904c3bd8f34d85b0 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 10:22:32 -0700 Subject: [PATCH 10/21] ateom: drop the family from the atunnel ingress listen defaults Both ateom herders defaulted the actor ingress flags to "0.0.0.0:443" and "0.0.0.0:444", which reads as IPv4-only. It never was: Go treats an unspecified address as a wildcard and binds it dual-stack, so the sockets already served both families. Spell the defaults ":443" and ":444" so the flag says what it does, and note why in a comment. Part of the dual-stack actor networking series; no behavior change. (cherry picked from commit 51b2cbef37e1e2c6699fb7d8eee64e968f222be9) --- cmd/ateom-gvisor/main.go | 7 +++++-- cmd/ateom-microvm/main.go | 6 ++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/ateom-gvisor/main.go b/cmd/ateom-gvisor/main.go index 0620c09e3..9e77aae2f 100644 --- a/cmd/ateom-gvisor/main.go +++ b/cmd/ateom-gvisor/main.go @@ -65,8 +65,11 @@ var ( podUID = pflag.String("pod-uid", "", "The UID of the current pod") // TODO(liorlieberman) have a sub package for all atunnel releated things like that - atunnelListenAddress = pflag.String("atunnel-listen-address", "0.0.0.0:443", "Address for actor ingress HTTPS") - atunnelConnectListenAddress = pflag.String("atunnel-connect-listen-address", "0.0.0.0:444", "Address for actor ingress mTLS CONNECT") + // + // Every listen address here is an unspecified wildcard, which Go binds as a + // dual-stack socket. + atunnelListenAddress = pflag.String("atunnel-listen-address", ":443", "Address for actor ingress HTTPS") + atunnelConnectListenAddress = pflag.String("atunnel-connect-listen-address", ":444", "Address for actor ingress mTLS CONNECT") workerCredentialBundle = pflag.String("atunnel-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "Worker Pod credential bundle used by atunnel for inbound serving and outbound mTLS") podIdentityTrustBundle = pflag.String("atunnel-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "Pod identity trust bundle used for router clients and the node-local atelet") atunnelClientIdentity = pflag.String("atunnel-client-identity", "spiffe://cluster.local/ns/ate-system/sa/atenet-router", "SPIFFE identity allowed to call actor ingress HTTPS") diff --git a/cmd/ateom-microvm/main.go b/cmd/ateom-microvm/main.go index 6613fcd9b..d4dde4680 100644 --- a/cmd/ateom-microvm/main.go +++ b/cmd/ateom-microvm/main.go @@ -71,8 +71,10 @@ var ( otlpRelaySocket = flag.String("otlp-relay-socket", ateompath.AteletOTLPSocketPath(), "Unix socket of atelet's OTLP relay to export telemetry through, keeping it off the pod network. Empty, or absent at startup, exports directly to OTEL_EXPORTER_OTLP_ENDPOINT instead.") - atunnelListenAddress = flag.String("atunnel-listen-address", "0.0.0.0:443", "Address for actor ingress HTTPS") - atunnelConnectListenAddress = flag.String("atunnel-connect-listen-address", "0.0.0.0:444", "Address for actor ingress mTLS CONNECT") + // Every listen address here is an unspecified wildcard, which Go binds as a + // dual-stack socket. + atunnelListenAddress = flag.String("atunnel-listen-address", ":443", "Address for actor ingress HTTPS") + atunnelConnectListenAddress = flag.String("atunnel-connect-listen-address", ":444", "Address for actor ingress mTLS CONNECT") workerCredentialBundle = flag.String("atunnel-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "Worker Pod credential bundle used by atunnel for inbound serving and outbound mTLS") podIdentityTrustBundle = flag.String("atunnel-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "Pod identity trust bundle used for router clients and the node-local atelet") atunnelClientIdentity = flag.String("atunnel-client-identity", "spiffe://cluster.local/ns/ate-system/sa/atenet-router", "SPIFFE identity allowed to call actor ingress HTTPS") From 05f0be0f5c468ffcd767dd164a2bb7f893542faf Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Sat, 15 Aug 2026 12:57:09 +0000 Subject: [PATCH 11/21] ateomnet: enable IPv6 forwarding in worker pod netns EnableIPv4Forwarding now also writes /proc/sys/net/ipv6/conf/all/forwarding so actor IPv6 traffic (including DNS queries) is routed between the actor veth and pod eth0 instead of being dropped by ip6_forward() on dual-stack / IPv6-only clusters. Factor the sysctl write into writeSysctlIfUnset preserving the original read-only remount/restore behavior, and add unit coverage for its fast paths. Fixes: agent-substrate/substrate#945 --- internal/ateomnet/net.go | 28 ++++++++- internal/ateomnet/write_sysctl_test.go | 84 ++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 internal/ateomnet/write_sysctl_test.go diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e0..2d3daa473 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -193,6 +193,9 @@ func PodIPv4() (net.IP, error) { } // EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. +// It also enables IPv6 forwarding so actor IPv6 traffic (including DNS queries +// on IPv6-capable clusters) is routed between the veth and eth0 instead of +// being dropped by ip6_forward(). func EnableIPv4Forwarding() error { // Forwarding is required because actor packets now enter the worker pod via // the host-side veth and then leave through the pod's eth0. Without this, the @@ -203,20 +206,41 @@ func EnableIPv4Forwarding() error { // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag // is not locked: clear it, write the sysctl, restore ro. const path = "/proc/sys/net/ipv4/ip_forward" + if err := writeSysctlIfUnset(path); err != nil { + return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + } + // IPv6 forwarding: actor packets that arrive on the veth and leave via eth0 + // are IPv6 on dual-stack / IPv6-only clusters. Without + // net.ipv6.conf.all.forwarding the kernel drops every IPv6 packet in + // ip6_forward(), including the actor's DNS queries. conf.all.forwarding=1 + // also implies the per-interface default, so a single write covers the veth + // and eth0. + const v6path = "/proc/sys/net/ipv6/conf/all/forwarding" + if err := writeSysctlIfUnset(v6path); err != nil { + return fmt.Errorf("while enabling IPv6 forwarding in worker pod netns: %w", err) + } + return nil +} + +// writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". +func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil } if err := os.WriteFile(path, []byte("1\n"), 0o644); err == nil { return nil } + // Without privileged, the container runtime bind-mounts /proc/sys read-only. + // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag + // is not locked: clear it, write the sysctl, restore ro. if err := unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT, ""); err != nil { - return fmt.Errorf("while remounting /proc/sys read-write to enable IPv4 forwarding: %w", err) + return fmt.Errorf("while remounting /proc/sys read-write to enable forwarding: %w", err) } defer func() { _ = unix.Mount("none", "/proc/sys", "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, "") }() if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { - return fmt.Errorf("while enabling IPv4 forwarding in worker pod netns: %w", err) + return fmt.Errorf("while writing %s: %w", path, err) } return nil } diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go new file mode 100644 index 000000000..b4cb1db16 --- /dev/null +++ b/internal/ateomnet/write_sysctl_test.go @@ -0,0 +1,84 @@ +//go:build linux + +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ateomnet + +import ( + "os" + "path/filepath" + "testing" +) + +// TestWriteSysctlIfUnset verifies writeSysctlIfUnset's fast paths against a +// temp file standing in for a /proc/sys node: it must not rewrite a value +// that already reads "1", and it must write "1\n" when the value is missing +// or unset. The privileged bind-remount path is covered by the netns +// integration tests (withTestNetNS), which require root. +func TestWriteSysctlIfUnset(t *testing.T) { + dir := t.TempDir() + + t.Run("already_set", func(t *testing.T) { + p := filepath.Join(dir, "already") + // Sentinel content: if writeSysctlIfUnset rewrote the file, the value + // would change to "1\n" and this assertion would fail. Keeping the + // file larger than the helper's output makes a silent rewrite + // detectable. + if err := os.WriteFile(p, []byte("1 other-content\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if string(b) != "1 other-content\n" { + t.Fatalf("already-set file was rewritten: %q", b) + } + }) + + t.Run("unset_written", func(t *testing.T) { + p := filepath.Join(dir, "unset") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) + + t.Run("zero_is_rewritten", func(t *testing.T) { + p := filepath.Join(dir, "zero") + if err := os.WriteFile(p, []byte("0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset: %v", err) + } + b, err := os.ReadFile(p) + if err != nil { + t.Fatal(err) + } + if len(b) < 1 || b[0] != '1' { + t.Fatalf("expected '1' written, got %q", b) + } + }) +} From 47ca7484f77ee4c9f0ba8266af871ac4f023e666 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Sun, 16 Aug 2026 19:12:06 +0000 Subject: [PATCH 12/21] ateomnet: return nil when sysctl path missing in writeSysctlIfUnset IPv6 sysctls are absent on kernels with IPv6 disabled (e.g. some containers set net.ipv6.conf.* only when IPv6 is enabled). Treat a missing path as 'nothing to enable' instead of forcing a remount and failing, matching the documented behavior. --- internal/ateomnet/net.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 2d3daa473..a2a950178 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -223,6 +223,8 @@ func EnableIPv4Forwarding() error { } // writeSysctlIfUnset writes "1\n" to a sysctl path unless it already reads "1". +// If the path does not exist (e.g. IPv6 sysctls on a kernel with IPv6 disabled), +// it returns nil — IPv6 forwarding is simply unavailable, not an error. func writeSysctlIfUnset(path string) error { if b, err := os.ReadFile(path); err == nil && len(b) > 0 && b[0] == '1' { return nil @@ -230,6 +232,10 @@ func writeSysctlIfUnset(path string) error { if err := os.WriteFile(path, []byte("1\n"), 0o644); err == nil { return nil } + if _, err := os.Stat(path); os.IsNotExist(err) { + // Path absent (e.g. IPv6 disabled in kernel): nothing to enable. + return nil + } // Without privileged, the container runtime bind-mounts /proc/sys read-only. // The worker holds CAP_SYS_ADMIN and uses no user namespace, so the ro flag // is not locked: clear it, write the sysctl, restore ro. From a08ed2e8938f0a3385236d348149333c1307c4db Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Fri, 21 Aug 2026 13:04:19 +0000 Subject: [PATCH 13/21] ateomnet: rename EnableIPv4Forwarding to EnableForwarding The helper has enabled both address families since IPv6 forwarding was added; the name now says so. The single call site in SetupActorNetwork is updated along with the doc comment. --- internal/ateomnet/net.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index a2a950178..1f0fbc92a 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -192,11 +192,11 @@ func PodIPv4() (net.IP, error) { return nil, fmt.Errorf("pod eth0 has no IPv4 address") } -// EnableIPv4Forwarding enables IPv4 forwarding in the current network namespace. -// It also enables IPv6 forwarding so actor IPv6 traffic (including DNS queries -// on IPv6-capable clusters) is routed between the veth and eth0 instead of -// being dropped by ip6_forward(). -func EnableIPv4Forwarding() error { +// EnableForwarding enables IPv4 and IPv6 forwarding in the current network +// namespace, so actor traffic (including DNS queries on IPv6-capable clusters) +// is routed between the veth and eth0 instead of being dropped by ip_forward() +// or ip6_forward(). +func EnableForwarding() error { // Forwarding is required because actor packets now enter the worker pod via // the host-side veth and then leave through the pod's eth0. Without this, the // kernel would not route traffic between those interfaces even though both @@ -595,7 +595,7 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { return fmt.Errorf("while configuring actor veth in interior netns: %w", err) } - if err := EnableIPv4Forwarding(); err != nil { + if err := EnableForwarding(); err != nil { return err } if err := InstallActorNftablesRules(cfg.EgressRedirectPort); err != nil { From 44b82b30ab53c0941ba4ec1532a1791a4e9c5d88 Mon Sep 17 00:00:00 2001 From: Suraj Kumar Date: Fri, 21 Aug 2026 13:04:19 +0000 Subject: [PATCH 14/21] ateomnet: cover writeSysctlIfUnset's missing-path branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The os.Stat/IsNotExist fallback was never executed by the unit tests: every existing subtest's temp path could be created, so each returned at the os.WriteFile fast path. Point the new subtest at a node under a directory that does not exist — what procfs always does in production — and assert the file stays absent. --- internal/ateomnet/write_sysctl_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/ateomnet/write_sysctl_test.go b/internal/ateomnet/write_sysctl_test.go index b4cb1db16..d687932d8 100644 --- a/internal/ateomnet/write_sysctl_test.go +++ b/internal/ateomnet/write_sysctl_test.go @@ -65,6 +65,21 @@ func TestWriteSysctlIfUnset(t *testing.T) { } }) + t.Run("missing_path_is_noop", func(t *testing.T) { + // A node under a directory that does not exist stands in for + // /proc/sys/net/ipv6/... on a kernel with IPv6 disabled. The other + // subtests' paths can be created, so they return at the os.WriteFile + // fast path; this is the only one that reaches the os.Stat branch, + // which is what procfs always does in production. + p := filepath.Join(dir, "no-such-dir", "forwarding") + if err := writeSysctlIfUnset(p); err != nil { + t.Fatalf("writeSysctlIfUnset on a missing path: %v", err) + } + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("expected %s to stay absent, stat err = %v", p, err) + } + }) + t.Run("zero_is_rewritten", func(t *testing.T) { p := filepath.Join(dir, "zero") if err := os.WriteFile(p, []byte("0\n"), 0o644); err != nil { From 424325937c0269900278ff9ee1983b70ddfc3b32 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 18 Aug 2026 22:19:45 -0700 Subject: [PATCH 15/21] atenet/egress: resolve upstream names on both address families The egress Envoy pinned dns_lookup_family to V4_ONLY, so it asked only for A records. On an IPv6-only cluster no upstream name resolves and no actor can reach the internet. AUTO tries AAAA and falls back to A, so IPv4-only clusters behave as before. One step of the IPv6 egress work, and not the one that unblocks it -- actor egress still stops earlier, in atunnel's original-destination lookup. (cherry picked from commit de81578a5f9387adc7f4d626986426c632f3be85) --- manifests/ate-install/atenet-egress-with-sdsmint.yaml | 8 ++++---- manifests/ate-install/atenet-egress.yaml | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/manifests/ate-install/atenet-egress-with-sdsmint.yaml b/manifests/ate-install/atenet-egress-with-sdsmint.yaml index 72b3e1979..f283a0c34 100644 --- a/manifests/ate-install/atenet-egress-with-sdsmint.yaml +++ b/manifests/ate-install/atenet-egress-with-sdsmint.yaml @@ -236,7 +236,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -323,7 +323,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -410,7 +410,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO # The MITM must not weaken upstream authentication. Envoy decrypted the # actor's TLS with a leaf of its own; it still sends the real SNI here # and still validates the real origin's certificate against the public @@ -453,7 +453,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO # Envoy refuses to build a dynamic forward proxy cluster without # auto_sni and auto_san_validation unless this is set, because for # the usual TLS case resolving the host from a header and then not diff --git a/manifests/ate-install/atenet-egress.yaml b/manifests/ate-install/atenet-egress.yaml index a655a3759..64153ea6f 100644 --- a/manifests/ate-install/atenet-egress.yaml +++ b/manifests/ate-install/atenet-egress.yaml @@ -145,7 +145,7 @@ data: "@type": type.googleapis.com/envoy.extensions.filters.http.dynamic_forward_proxy.v3.FilterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO - name: envoy.filters.http.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router @@ -182,7 +182,7 @@ data: "@type": type.googleapis.com/envoy.extensions.clusters.dynamic_forward_proxy.v3.ClusterConfig dns_cache_config: name: egress_dns_cache - dns_lookup_family: V4_ONLY + dns_lookup_family: AUTO --- apiVersion: apps/v1 kind: Deployment From e2531ca79d5bf7af36cf28402892cde207425823 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 14 Aug 2026 11:03:44 -0700 Subject: [PATCH 16/21] ci: add an IPv6-only kind e2e job Runs the full install plus the demo and networking e2e suites against a single-stack IPv6-only kind cluster, and asserts the cluster really is v6-only so a green run cannot quietly become a second IPv4 run. It stays out of the e2e-test merge gate, so it reports IPv6 status without being able to block a PR, and it runs on every PR for now so the results are visible; the TODO on the trigger records the intended ci/ipv6 label gate. ubuntu-latest has no IPv6 egress, so the job stands up tayga for NAT64 and points CoreDNS at an upstream resolver through the well-known prefix. DNS64 is scoped to a catch-all server block: synthesizing AAAA over the cluster zones destroys the v6-only ClusterIP answers and the control plane never comes up. (cherry picked from commit 748e8412183ab0f85f4beb7bbaa8f39a5dcb2494) --- .github/workflows/e2e-ipv6.yaml | 366 ++++++++++++++++++++++++++++++++ 1 file changed, 366 insertions(+) create mode 100644 .github/workflows/e2e-ipv6.yaml diff --git a/.github/workflows/e2e-ipv6.yaml b/.github/workflows/e2e-ipv6.yaml new file mode 100644 index 000000000..c7a0e9a51 --- /dev/null +++ b/.github/workflows/e2e-ipv6.yaml @@ -0,0 +1,366 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: e2e-ipv6 +# Separate from pr-workflow.yaml so this can be gated independently -- and so a +# 40-minute IPv6 run never delays that workflow's merge-gating jobs. +# +# TODO(246): gate this before merging. It runs on every PR today so the IPv6-only +# results are visible without a maintainer having to act first; the intended +# steady state is a `ci/ipv6` label, which needs the label created upstream: +# +# on: {pull_request: {types: [labeled, opened, synchronize, reopened]}} +# if: contains(github.event.pull_request.labels.*.name, 'ci/ipv6') +# +# Either way this job stays out of the `e2e-test` gate, so it never blocks a PR. +on: + pull_request: +permissions: + contents: read +jobs: + e2e-test-ipv6: + runs-on: ubuntu-latest + # Nothing else in this workflow sets a timeout, so jobs inherit GitHub's + # 6-hour default. A broken IPv6 cluster does not crash, it misses + # 10-minute ActorTemplate deadlines, so an uncapped job burns hours. + timeout-minutes: 40 + env: + # Non-default name so these steps can be replayed locally without + # touching an existing cluster. install-ate-kind.sh does not derive + # KUBECTL_CONTEXT from the cluster name the way run-e2e-kind.sh does, + # so both have to be set here. + KIND_CLUSTER_NAME: ate-ipv6 + KUBECTL_CONTEXT: kind-ate-ipv6 + # 8.8.8.8 reached through the well-known NAT64 prefix. CoreDNS is a + # v6-only pod on a runner with no IPv6 egress of its own, so this is the + # only shape of upstream resolver it can reach. See "Set up NAT64". + IPV6_DNS_UPSTREAM: 64:ff9b::808:808 + steps: + - name: Checkout + uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 # v5.1.0 + - name: Setup Go + uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5.6.0 + with: + go-version-file: 'go.mod' + - name: Free disk space + # kind node image + control-plane images + snapshots are tight on the + # ~14GB runner disk even without the micro-VM assets. + run: | + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc /opt/hostedtoolcache/CodeQL + df -h / + - name: Enable IPv6 in the Docker daemon + # ubuntu-latest ships dockerd with IPv6 off, so kind would create its + # network v4-only and create-kind-cluster.sh would reject the cluster. + # Merge the two keys into whatever daemon.json the runner image ships + # rather than replacing the file. + run: | + sudo mkdir -p /etc/docker + [ -s /etc/docker/daemon.json ] || echo '{}' | sudo tee /etc/docker/daemon.json >/dev/null + sudo cat /etc/docker/daemon.json \ + | jq '. + {"ipv6": true, "ip6tables": true}' \ + | sudo tee /etc/docker/daemon.json.new >/dev/null + sudo mv /etc/docker/daemon.json.new /etc/docker/daemon.json + sudo systemctl restart docker + docker network inspect bridge --format 'bridge EnableIPv6={{.EnableIPv6}}' + - name: Set up NAT64 on the runner + # ubuntu-latest has no IPv6 egress whatsoever -- measured, not assumed: + # every curl -6 fails in ~2ms. A v6-only cluster still has to reach real + # v4 destinations (atelet fetches the gVisor tarball from GCS, + # TestActorEgress fetches example.com), so the runner translates for it. + # Ordered after the dockerd restart, which rebuilds the iptables chains + # these rules live in. + run: | + sudo apt-get update -qq + sudo apt-get install -y -qq tayga dnsutils + # tayga answers to .1/::1; the tun holds .2/::2 so host-originated + # traffic is not sourced from tayga's own address, which is + # self-addressed rather than translatable. The pool avoids both. + sudo tee /etc/tayga.conf >/dev/null <<'EOF' + tun-device nat64 + ipv4-addr 192.168.255.1 + # tayga refuses the well-known prefix with an RFC1918 pool unless it + # also holds a v6 address of its own, outside that prefix. + ipv6-addr 2001:db8:64::1 + prefix 64:ff9b::/96 + dynamic-pool 192.168.255.128/25 + data-dir /var/spool/tayga + EOF + sudo mkdir -p /var/spool/tayga + sudo tayga --mktun + sudo ip link set nat64 up + sudo ip addr add 192.168.255.2/24 dev nat64 + sudo ip -6 addr add 2001:db8:64::2/128 dev nat64 + sudo ip -6 route add 64:ff9b::/96 dev nat64 src 2001:db8:64::2 + sudo sysctl -qw net.ipv4.ip_forward=1 + sudo sysctl -qw net.ipv6.conf.all.forwarding=1 + sudo iptables -t nat -A POSTROUTING -s 192.168.255.0/24 -j MASQUERADE + # Insert, not append: docker sets the FORWARD policy to DROP. + sudo iptables -I FORWARD 1 -i nat64 -j ACCEPT + sudo iptables -I FORWARD 1 -o nat64 -j ACCEPT + sudo ip6tables -I FORWARD 1 -i nat64 -j ACCEPT + sudo ip6tables -I FORWARD 1 -o nat64 -j ACCEPT + # -d keeps tayga in the foreground and logs every dropped packet with a + # reason; detaching hides exactly the failures worth diagnosing. + sudo sh -c 'nohup tayga -d --config /etc/tayga.conf >/tmp/tayga.log 2>&1 &' + sleep 3 + pgrep -a tayga || { + echo "::error::tayga is not running"; sudo cat /tmp/tayga.log; exit 1; + } + - name: Verify NAT64 before building anything on it + # Hard gate. The cluster takes ~4 minutes and every step after it depends + # on translation working, so a broken translator should fail here with + # one clear message rather than as a rollout timeout ten minutes later. + run: | + # Map a live A record rather than hardcoding one: example.com's old + # 93.184.216.34 is retired and would fail for the wrong reason. + v4=$(getent ahostsv4 storage.googleapis.com | awk 'NR==1{print $1}') + # shellcheck disable=SC2086 + set -- ${v4//./ } + v6=$(printf '64:ff9b::%02x%02x:%02x%02x' "$1" "$2" "$3" "$4") + echo "NAT64 maps ${v4} -> ${v6}" + dig +timeout=5 +tries=1 @"${IPV6_DNS_UPSTREAM}" storage.googleapis.com A +short + code=$(curl -6 -sS -m 15 -o /dev/null -w '%{http_code}' \ + --resolve "storage.googleapis.com:443:[${v6}]" \ + https://storage.googleapis.com/ || echo 000) + echo "NAT64 HTTPS probe returned ${code}" + case "${code}" in + # Any HTTP status proves the translator carried a TCP stream; GCS + # answers a bare / with 400. ICMP is separately blocked, so a ping + # test here would report a failure that does not matter. + 2*|3*|4*) ;; + *) echo "::error::NAT64 is not translating; the cluster cannot egress" + sudo cat /tmp/tayga.log || true + exit 1 ;; + esac + - name: Create cluster + env: + IP_FAMILY: ipv6 + run: hack/create-kind-cluster.sh + - name: Assert the cluster is single-stack IPv6 + # This job is worthless if the cluster is not actually v6-only, and a + # green run leaves no evidence either way -- the diagnostics dump below + # only runs on failure. A kind default change or an IP_FAMILY regression + # would otherwise turn this into a second IPv4 run that reports success. + # Checked here rather than later so it fails as itself. + # + # PreferDualStack Services resolving to a single clusterIP is the + # positive signal: on a dual-stack cluster they would get two. + run: | + k() { kubectl --context="$KUBECTL_CONTEXT" "$@"; } + pod_cidrs=$(k get nodes -o jsonpath='{.items[*].spec.podCIDRs[*]}') + svc_ips=$(k -n default get svc kubernetes -o jsonpath='{.spec.clusterIPs[*]}') + node_ips=$(k get nodes -o jsonpath='{.items[*].status.addresses[?(@.type=="InternalIP")].address}') + for pair in "podCIDRs=${pod_cidrs}" "kubernetes.clusterIPs=${svc_ips}" "node.InternalIP=${node_ips}"; do + case "${pair#*=}" in + *.*) echo "::error::not single-stack IPv6 -- ${pair}"; exit 1 ;; + "") echo "::error::empty, cannot confirm IP family -- ${pair}"; exit 1 ;; + esac + echo " ${pair}" + done + echo "single-stack IPv6 confirmed" + - name: Apply DNS64 to external names only + # create-kind-cluster.sh already points CoreDNS at IPV6_DNS_UPSTREAM, so + # names resolve -- but the answers are unusable. Plain DNS64 synthesizes + # only for names with no AAAA, and the external names this job needs + # (storage.googleapis.com, example.com) do have AAAA records, pointing at + # real IPv6 addresses the runner cannot reach. Only translate_all forces + # them through the prefix. + # + # translate_all cannot go in the same server block as the cluster zones. + # dns64 wraps the whole plugin chain below it, and it answers a AAAA query + # by synthesizing from A -- so for an AAAA-only name it synthesizes from + # nothing and returns an empty answer. Every ClusterIP on a v6-only + # cluster is AAAA-only, so a single-block Corefile takes out all + # in-cluster service discovery: ate-api-server cannot find + # valkey-cluster.ate-system.svc and the install times out. + # + # So: cluster zones keep the chain kind shipped, the registry keeps the + # block create-kind-cluster.sh gave it, and dns64 sits in the catch-all + # with the forwarder. + run: | + kubectl --context="$KUBECTL_CONTEXT" -n kube-system get cm coredns \ + -o jsonpath='{.data.Corefile}' > /tmp/Corefile + # Re-zone the block kind shipped and lift out its forwarder, which moves + # to the catch-all below; health/ready/kubernetes/cache stay as-is. The + # rules are gated on "first" so they stop at that block's closing brace + # and leave the registry's own block untouched. + awk ' + NR == 1 && /^\.:53[[:space:]]*\{/ { + print "cluster.local:53 in-addr.arpa:53 ip6.arpa:53 {"; first = 1; next + } + first && /^ forward([[:space:]].*)?\{$/ { skip = 1; next } + first && skip && /^ \}$/ { skip = 0; next } + first && skip { next } + first && /^\}$/ { first = 0 } + { print } + ' /tmp/Corefile > /tmp/Corefile.new + if ! grep -q '^cluster.local:53' /tmp/Corefile.new; then + echo "::error::Corefile did not start with the .:53 block kind ships" + cat /tmp/Corefile; exit 1 + fi + # create-kind-cluster.sh owns this block. If it ever goes back to a + # hosts entry inside .:53, the split above silently drops the registry. + if ! grep -q '^kind-registry:53' /tmp/Corefile.new; then + echo "::error::no kind-registry server block in the Corefile" + cat /tmp/Corefile; exit 1 + fi + if grep -q 'forward' /tmp/Corefile.new; then + echo "::error::the forward block survived the split" + cat /tmp/Corefile.new; exit 1 + fi + cat >>/tmp/Corefile.new < /tmp/coredns-dns64.yaml + kubectl --context="$KUBECTL_CONTEXT" -n kube-system patch cm coredns \ + --type=merge --patch-file /tmp/coredns-dns64.yaml + kubectl --context="$KUBECTL_CONTEXT" -n kube-system rollout restart deploy/coredns + kubectl --context="$KUBECTL_CONTEXT" -n kube-system rollout status deploy/coredns --timeout=120s + cat /tmp/Corefile.new + - name: Verify cluster DNS answers both internal and external names + # The install is the next step and it takes ten minutes to fail. A DNS + # regression is the failure this Corefile is most likely to cause, so + # assert all three cases here where the message is unambiguous. + run: | + set -o pipefail + kubectl --context="$KUBECTL_CONTEXT" run dnscheck --rm --attach --quiet \ + --restart=Never --image=busybox:1.36 --command -- \ + sh -c ' + nslookup kubernetes.default.svc.cluster.local >/dev/null 2>&1 \ + || { echo "FAIL: an in-cluster Service does not resolve"; exit 1; } + nslookup storage.googleapis.com 2>/dev/null | grep -q "64:ff9b" \ + || { echo "FAIL: external names are not synthesized through NAT64"; exit 1; } + # Informational. Nothing downstream resolves this from a pod -- + # containerd pulls images on the node, and create-kind-cluster.sh + # runs its own registry probe before DNS64 is applied -- so a miss + # here is not a reason to fail the job. Printed because a change + # here would still be worth seeing. + echo "--- kind-registry, informational" + nslookup kind-registry 2>&1 | tail -4 + echo "DNS-OK" + ' | tee /tmp/dnscheck.log + grep -q DNS-OK /tmp/dnscheck.log + - name: Install Agent Substrate + run: hack/install-ate-kind.sh --deploy-ate-system + - name: Assert the control plane is up + # install-ate.sh runs under pipefail, so a failed apply does propagate. + # What it would not catch is a Deployment that rolls out and then + # crash-loops. Re-check everything deploy_ate_system waits on -- + # atenet-egress included, since a non-dual-stack Envoy listener fails + # there first, by way of a readiness probe the kubelet cannot reach. + run: | + for r in deployment/ate-api-server deployment/ate-controller \ + deployment/atenet-router deployment/atenet-egress \ + statefulset/valkey-cluster daemonset/atelet; do + kubectl --context="$KUBECTL_CONTEXT" -n ate-system rollout status "$r" --timeout=120s + done + kubectl --context="$KUBECTL_CONTEXT" -n podcertificate-controller-system \ + rollout status deployment/podcertificate-controller --timeout=120s + if kubectl --context="$KUBECTL_CONTEXT" -n ate-system get pods \ + -o jsonpath='{.items[*].status.containerStatuses[*].state.waiting.reason}' \ + | grep -q CrashLoopBackOff; then + echo "::error::a pod in ate-system is in CrashLoopBackOff" + exit 1 + fi + - name: Deploy gVisor counter demo + run: hack/install-ate-kind.sh --deploy-demo-counter + - name: Deploy egress demo + run: hack/install-ate-kind.sh --deploy-demo-egress + - name: Assert the demo fixtures exist + # A failed demo deploy exits 0: install-ate.sh dispatches demos through + # `if "${demo}_cmdline" "$1"`, which suspends errexit, and _cmdline ends + # in an unconditional `return 0`. Without this the suites fail later with + # "ActorTemplate not found", pointing at the tests instead of the install. + run: | + for ns_tmpl in ate-demo-counter/counter ate-demo-egress/egress; do + ns=${ns_tmpl%/*}; tmpl=${ns_tmpl#*/} + kubectl --context="$KUBECTL_CONTEXT" -n "${ns}" get actortemplate "${tmpl}" \ + || { echo "::error::${ns_tmpl} was not created -- the demo deploy failed silently"; exit 1; } + done + # One suite per step: run-e2e.sh takes exactly one target path, and this + # way a failure names the suite that produced it. + - name: Run E2E tests (demo) + id: e2e-demo + run: | + set -o pipefail + hack/run-e2e-kind.sh ./internal/e2e/suites/demo -v -args --no-color 2>&1 \ + | tee /tmp/e2e-demo.log + - name: Run E2E tests (networking) + # Runs even when demo failed -- networking is the half most likely to + # expose a single-family bug -- but stays skipped when an earlier step + # left no cluster to test against. always() is required, not decorative: + # an if: without a status function is implicitly ANDed with success(), + # which skips this step on exactly the failure it is meant to survive. + if: always() && steps.e2e-demo.outcome != 'skipped' + run: | + set -o pipefail + hack/run-e2e-kind.sh ./internal/e2e/suites/networking -v -args --no-color 2>&1 \ + | tee /tmp/e2e-networking.log + - name: Guard against a vacuously green run + # A suite that gates on a dual-stack Service and skips itself on v6-only + # exits 0, so a suite that only skipped would otherwise read as a pass. + # The bar is one real PASS per suite, not zero skips: demo legitimately + # skips the micro-VM-only Golden resume and the CSI volume tests on any + # family. Skips are printed so a growing list gets noticed. + # Skipped when the suites never ran: with no logs to count, this step + # would otherwise report a reassuring zero on a job that failed earlier. + if: always() && steps.e2e-demo.outcome != 'skipped' + run: | + for f in /tmp/e2e-demo.log /tmp/e2e-networking.log; do + [ -s "$f" ] || { echo "::error::${f} is missing or empty"; exit 1; } + passed=$(grep -c -- '--- PASS' "$f" || true) + echo "${f}: ${passed} passed, $(grep -c -- '--- SKIP' "$f" || true) skipped" + grep -h -- '--- SKIP' "$f" || true + if [ "${passed}" -eq 0 ]; then + echo "::error::${f} has no passing tests -- a suite that only skips proves nothing" + exit 1 + fi + done + - name: Dump diagnostics on failure + if: failure() + run: | + kubectl --context="$KUBECTL_CONTEXT" get actortemplate,workerpool,pods -A -o wide || true + dump() { + echo "=== logs: $1/$2 ===" + kubectl --context="$KUBECTL_CONTEXT" logs -n "$1" "$2" --all-containers --tail=300 2>/dev/null || true + } + for p in $(kubectl --context="$KUBECTL_CONTEXT" get pods -n ate-system -o name 2>/dev/null); do + dump ate-system "$p" + done + # Every worker pod in any namespace: the demo pools plus the e2e suites' + # randomly-named per-test namespaces, which the suites keep on failure. + kubectl --context="$KUBECTL_CONTEXT" get pods -A -l ate.dev/worker-pool \ + -o 'custom-columns=:.metadata.namespace,:.metadata.name' --no-headers 2>/dev/null \ + | while read -r ns name; do dump "$ns" "$name"; done + # IPv6-specific: the rewritten Corefile, and what each Service actually + # got assigned, are the two things that differ from the IPv4 job. + kubectl --context="$KUBECTL_CONTEXT" -n kube-system logs -l k8s-app=kube-dns --tail=100 || true + kubectl --context="$KUBECTL_CONTEXT" -n kube-system get cm coredns -o jsonpath='{.data.Corefile}' || true + kubectl --context="$KUBECTL_CONTEXT" get svc -A \ + -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,POLICY:.spec.ipFamilyPolicy,IPS:.spec.clusterIPs || true + # tayga logs a reason for every packet it declines to translate, which + # is the only view of an egress failure that is not a bare timeout. + echo "=== tayga ===" + sudo tail -100 /tmp/tayga.log || true From 7249571bd1eded4facd2913ae6d1c05303c61945 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Wed, 19 Aug 2026 12:23:57 -0700 Subject: [PATCH 17/21] hack: read the IPv6 DNS probe from the pod log The probe attached to the pod to collect its markers, and an attach can end before the last write arrives. A CI run lost the registry marker that way, so the check reported a registry it could not reach -- and then refused to re-probe, because only the resolve leg was treated as a settling race. Wait for the pod to terminate and read its log instead, and close the probe with a PROBE_DONE marker so a short read is re-probed rather than read as a failed fetch. A registry that really is down still fails on the first attempt. --- hack/verify-ipv6-dns.sh | 42 +++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/hack/verify-ipv6-dns.sh b/hack/verify-ipv6-dns.sh index 2afd300ae..7f21c041c 100755 --- a/hack/verify-ipv6-dns.sh +++ b/hack/verify-ipv6-dns.sh @@ -57,16 +57,20 @@ echo "Verifying DNS from a pod..." # The registry leg fetches rather than resolves -- the hosts entry is AAAA-only, # which fails nslookup's A query but satisfies getaddrinfo. # -# --attach gives one stream and only the last leg's exit status, so each leg +# One stream carries every leg and only the last one's exit status, so each leg # reports a marker on stdout and no failure message may contain one; PROBE_RAN -# separates a failed leg from a pod that never ran. Retry the pod, not the -# query: one that asks before CoreDNS settles stays broken for ~30s, while a -# fresh pod 10s later resolves first try. +# and PROBE_DONE bracket the run so a short read is told apart from a leg that +# failed. Read the log once the pod has terminated rather than attaching to it: +# an attach can drop the tail, and a lost registry marker then reads as an +# unreachable registry. Retry the pod, not the query: one that asks before +# CoreDNS settles stays broken for ~30s, while a fresh pod 10s later resolves +# first try. probe="" probe_max=4 for ((probe_attempt = 1; probe_attempt <= probe_max; probe_attempt++)); do - attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" run "coredns-probe-$$-${probe_attempt}" \ - --rm --attach --quiet --restart=Never --image=busybox:1.36 --command -- \ + probe_pod="coredns-probe-$$-${probe_attempt}" + kubectl --context="${KUBECTL_CONTEXT}" run "${probe_pod}" \ + --restart=Never --image=busybox:1.36 --command -- \ sh -c "echo PROBE_RAN if out=\$(nslookup storage.googleapis.com 2>&1); then echo RESOLVE_OK @@ -77,13 +81,28 @@ for ((probe_attempt = 1; probe_attempt <= probe_max; probe_attempt++)); do echo REGISTRY_OK else echo \"registry fetch failed: \$(echo \"\$out\" | tail -1)\" - fi")" || true + fi + echo PROBE_DONE" >/dev/null || true + for ((probe_wait = 0; probe_wait < 120; probe_wait++)); do + phase="$(kubectl --context="${KUBECTL_CONTEXT}" get pod "${probe_pod}" \ + -o jsonpath='{.status.phase}' 2>/dev/null || true)" + [[ "${phase}" == "Succeeded" || "${phase}" == "Failed" ]] && break + sleep 1 + done + attempt_out="$(kubectl --context="${KUBECTL_CONTEXT}" logs "${probe_pod}" 2>/dev/null || true)" + kubectl --context="${KUBECTL_CONTEXT}" delete pod "${probe_pod}" \ + --now --ignore-not-found --wait=false >/dev/null 2>&1 || true # A pod that never started must not bury an earlier one's real failure. if [[ "${attempt_out}" == *PROBE_RAN* ]]; then probe="${attempt_out}"; fi - # Only the resolve leg is a settling race; a down registry will not fix itself. - [[ "${probe}" == *RESOLVE_OK* ]] && break + # Only the resolve leg is a settling race; a down registry will not fix + # itself, so a finished probe is a verdict either way. An unfinished one + # reported no registry result at all, which is not the same as a failure. + if [[ "${probe}" == *RESOLVE_OK* ]] && + [[ "${probe}" == *REGISTRY_OK* || "${probe}" == *PROBE_DONE* ]]; then + break + fi if ((probe_attempt < probe_max)); then - echo " the cluster is not resolving yet; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." + echo " the probe did not come back clean; re-probing (attempt $((probe_attempt + 1)) of ${probe_max})..." sleep 10 fi done @@ -94,6 +113,9 @@ if [[ "${probe}" != *RESOLVE_OK* || "${probe}" != *REGISTRY_OK* ]]; then elif [[ "${probe}" != *RESOLVE_OK* ]]; then echo "error: a pod cannot resolve an external name" >&2 echo " IPV6_DNS_UPSTREAM is '${IPV6_DNS_UPSTREAM}'; set it to a reachable resolver" >&2 + elif [[ "${probe}" != *PROBE_DONE* ]]; then + echo "error: the probe stopped early, so the registry leg is unverified" >&2 + echo " re-run this script; DNS itself answered" >&2 else echo "error: DNS works but a pod cannot reach '${REG_NAME}'${reg_at}" >&2 echo " check the registry container is up and on the 'kind' network" >&2 From 1157ed1a0e62227a77f2cd158ffd13530d71658b Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 14:48:33 -0700 Subject: [PATCH 18/21] ateomnet: move the actor nftables table to the inet family The actor's NAT and filter rules lived in an ip table, which can only ever carry IPv4. They are now in an inet table, so one table can hold both address families when the actor veth becomes dual-stack. A bare payload match is ambiguous in an inet table, so every match now opens with an NFPROTO comparison and behaves exactly as it did before. NAT in the inet family needs Linux 4.18 or later. Teardown sweeps ip as well as inet. A table name is unique per family, so the ip table an earlier ateom left behind is invisible to an inet-only cleanup: the dump comes back empty, the "already clean" path reports success, and the stale table keeps redirecting alongside the new one. Part of #246 --- internal/ateomnet/net.go | 40 +++++---- internal/ateomnet/net_linux_test.go | 124 +++++++++++++++++++++++----- 2 files changed, 128 insertions(+), 36 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 1f0fbc92a..0583edb97 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -259,8 +259,9 @@ func InstallActorNftablesRules(egressPort uint16) error { // rules in an ateom-owned table makes cleanup simple and avoids mutating // Kubernetes or CNI-managed chains directly. // - // TODO: Add IPv6 veth addressing, forwarding, and nftables rules once actor - // networking supports dual-stack pods. The current actor network is IPv4-only. + // The table is in the inet family so one table can carry both address + // families once the actor veth is dual-stack. That makes a bare payload + // match ambiguous, so every match opens with an NFPROTO comparison. // // The rules do three things: // @@ -278,7 +279,7 @@ func InstallActorNftablesRules(egressPort uint16) error { c := &nftables.Conn{} table := &nftables.Table{ - Family: nftables.TableFamilyIPv4, + Family: nftables.TableFamilyINet, Name: ActorNftTableName, } c.AddTable(table) @@ -335,20 +336,25 @@ func RemoveActorNftablesRules() error { // Delete the whole ateom nftables table if it exists. The table is // per-worker and currently per-active-actor because this worker path runs at // most one actor at a time. Missing tables are treated as already clean. + // + // Both families are swept, not just the inet one this installs into: a table + // name is unique per family, so an ip table left by an earlier ateom would + // survive every later cleanup and keep redirecting alongside the new one. c := &nftables.Conn{} - tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) - if err != nil { - return fmt.Errorf("while listing nftables tables: %w", err) - } - for _, table := range tables { - if table.Name != ActorNftTableName { - continue + for _, family := range []nftables.TableFamily{nftables.TableFamilyINet, nftables.TableFamilyIPv4} { + tables, err := c.ListTablesOfFamily(family) + if err != nil { + return fmt.Errorf("while listing nftables tables: %w", err) } - c.DelTable(table) - if err := c.Flush(); err != nil { - return fmt.Errorf("while deleting actor nftables table: %w", err) + for _, table := range tables { + if table.Name != ActorNftTableName { + continue + } + c.DelTable(table) + if err := c.Flush(); err != nil { + return fmt.Errorf("while deleting actor nftables table: %w", err) + } } - return nil } return nil } @@ -359,6 +365,12 @@ func IPSourceEqual(ip string) []expr.Any { func IPPayloadEqual(offset uint32, ip string) []expr.Any { return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.NFPROTO_IPV4}, + }, &expr.Payload{ DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index b9c8ac45f..436efc32c 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -19,6 +19,7 @@ package ateomnet import ( "context" "errors" + "net" "runtime" "testing" @@ -86,6 +87,26 @@ func requireNftables(t *testing.T) { } } +// actorNftTableExists reports whether the actor table is present in the family +// InstallActorNftablesRules creates it in. The family is load-bearing: +// ListTablesOfFamily puts it in the netlink dump header, so the kernel filters +// the dump and a query for the wrong family comes back empty rather than +// erroring. +func actorNftTableExists(t *testing.T) bool { + t.Helper() + c := &nftables.Conn{} + tables, err := c.ListTablesOfFamily(nftables.TableFamilyINet) + if err != nil { + t.Fatalf("listing inet nftables tables: %v", err) + } + for _, table := range tables { + if table.Name == ActorNftTableName { + return true + } + } + return false +} + // linkByName returns the link, or nil when it does not exist. func linkByName(t *testing.T, name string) netlink.Link { t.Helper() @@ -115,6 +136,39 @@ func hasAddr(t *testing.T, link netlink.Link, cidr string) bool { return false } +// assertDefaultRoute requires link to carry -- or, when want is false, to not +// carry -- a default route via gw in the given family. +func assertDefaultRoute(t *testing.T, link netlink.Link, family int, gw net.IP, want bool) { + t.Helper() + + dst := "0.0.0.0/0" + if family == netlink.FAMILY_V6 { + dst = "::/0" + } + routes, err := netlink.RouteList(link, family) + if err != nil { + t.Fatalf("listing %s routes of %q: %v", dst, link.Attrs().Name, err) + } + var got bool + for _, route := range routes { + // A default route reports its destination either as nil or as an + // explicit zero-length mask, depending on how the kernel rendered it. + ones := 0 + if route.Dst != nil { + ones, _ = route.Dst.Mask.Size() + } + if ones == 0 && route.Gw.Equal(gw) { + got = true + } + } + switch { + case want && !got: + t.Errorf("%q has no %s route via %s, got %v", link.Attrs().Name, dst, gw, routes) + case !want && got: + t.Errorf("%q has a %s route via %s, want none, got %v", link.Attrs().Name, dst, gw, routes) + } +} + // TestSetupActorNetworkFinalState pins the namespace state gVisor and the // micro-VM guest read after an activation: what links exist, where, with which // addresses and routes. It deliberately asserts the end state rather than the @@ -169,28 +223,7 @@ func TestSetupActorNetworkFinalState(t *testing.T) { t.Error("interior loopback is not up") } - routes, err := netlink.RouteList(actor, netlink.FAMILY_V4) - if err != nil { - t.Fatalf("listing interior routes: %v", err) - } - // A default route reports its destination either as nil or as an - // explicit 0.0.0.0/0, depending on how the kernel rendered it. - isDefault := func(route netlink.Route) bool { - if route.Dst == nil { - return true - } - ones, _ := route.Dst.Mask.Size() - return ones == 0 - } - var haveDefault bool - for _, route := range routes { - if isDefault(route) && route.Gw.Equal(ActorVethGwIP) { - haveDefault = true - } - } - if !haveDefault { - t.Errorf("interior netns has no default route via %s, got %v", ActorVethGateway, routes) - } + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) return nil }); err != nil { t.Fatalf("inspecting interior netns: %v", err) @@ -215,9 +248,20 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { if linkByName(t, HostVethName) == nil { t.Fatalf("host veth %q missing after activation %d", HostVethName, i) } + if !actorNftTableExists(t) { + t.Fatalf("nftables table %q missing after activation %d", ActorNftTableName, i) + } if err := CleanupActorNetwork(ctx, interior); err != nil { t.Fatalf("CleanupActorNetwork (activation %d): %v", i, err) } + // Install and teardown have to name the same family. When they do not, + // teardown's dump comes back empty, its "missing tables are already + // clean" path reports success, and the table survives -- so the next + // activation stacks another copy of every chain and rule onto it and + // the leak is invisible to every other assertion here. + if actorNftTableExists(t) { + t.Fatalf("nftables table %q survived cleanup after activation %d", ActorNftTableName, i) + } } // Cleanup is idempotent: the extra call after the loop's last one must @@ -228,6 +272,9 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { if stray := linkByName(t, HostVethName); stray != nil { t.Errorf("host veth %q survived cleanup", HostVethName) } + if actorNftTableExists(t) { + t.Errorf("nftables table %q survived a repeated cleanup", ActorNftTableName) + } if err := NetNSDo(ctx, interior, func(context.Context) error { if stray := linkByName(t, ActorVethName); stray != nil { t.Errorf("actor veth %q survived cleanup", ActorVethName) @@ -239,6 +286,39 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { }) } +// TestRemoveActorNftablesRulesSweepsIPv4Family covers the upgrade case: a +// worker whose previous ateom created the actor table in the ip family. Table +// names are unique per family, so an inet-only cleanup could never see that +// table, and it would have kept redirecting alongside the inet one installed +// next to it. +func TestRemoveActorNftablesRulesSweepsIPv4Family(t *testing.T) { + roottest.Require(t, "creating network namespaces and nftables rules") + + withTestNetNS(t, func(netns.NsHandle) { + requireNftables(t) + + c := &nftables.Conn{} + c.AddTable(&nftables.Table{Family: nftables.TableFamilyIPv4, Name: ActorNftTableName}) + if err := c.Flush(); err != nil { + t.Fatalf("creating the stand-in ip actor table: %v", err) + } + + if err := RemoveActorNftablesRules(); err != nil { + t.Fatalf("RemoveActorNftablesRules: %v", err) + } + + tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) + if err != nil { + t.Fatalf("listing ip nftables tables: %v", err) + } + for _, table := range tables { + if table.Name == ActorNftTableName { + t.Fatal("the ip actor table survived cleanup") + } + } + }) +} + // TestSetupActorNetworkHostVethHWAddr covers the micro-VM requirement: a CH // snapshot freezes the guest's ARP entry for the gateway, so the worker-side // veth MAC has to be exactly the one the caller asked for, on every pod. From dc1bb16b9ecb72df98a3f7f5647f6c6bf34de301 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Thu, 20 Aug 2026 14:56:11 -0700 Subject: [PATCH 19/21] ateomnet: give the actor an IPv6 address when the pod has one Actor networking was IPv4-only, so an actor on a dual-stack worker pod could not reach an IPv6-only destination at all. SetupActorNetwork now assigns the fd00:169:254::/126 counterparts of the existing point-to-point pair to both ends of the actor veth, installs an IPv6 default route in the interior netns, and adds the matching rules to the inet-family actor table. Whether the actor gets IPv6 is decided once in the worker pod netns and carried into the interior one, which is created fresh and so always reports IPv6 available whatever the cluster's families are. Both halves have to hold: the pod needs a global IPv6 address of its own, and the veth has to accept an IPv6 address -- IPv4-only GKE sets disable_ipv6 and netlink then rejects the assignment with EPERM. Addresses carry IFA_F_NODAD rather than the accept_dad sysctl, which the unprivileged ateom container cannot write. Part of #246 --- cmd/ateom-microvm/run.go | 4 + internal/ateomnet/net.go | 189 ++++++++++++++++++++++++++-- internal/ateomnet/net_linux_test.go | 186 +++++++++++++++++++++++++++ 3 files changed, 369 insertions(+), 10 deletions(-) diff --git a/cmd/ateom-microvm/run.go b/cmd/ateom-microvm/run.go index 3015fed3b..329a79293 100644 --- a/cmd/ateom-microvm/run.go +++ b/cmd/ateom-microvm/run.go @@ -1065,6 +1065,10 @@ func tailString(s string, n int) string { // agent: configure eth0 (IP/MAC/MTU), install the connected + default routes, and // pin the gateway's ARP entry to its fixed MAC (so a restored guest's frozen // neighbor entry stays valid). +// +// TODO(#246): the guest is configured IPv4-only, so a micro-VM actor sees no +// IPv6 even on a dual-stack pod where the host veth has one. gVisor reads the +// interior netns and picks the address up; this path has to be told. func (s *AteomService) configureGuestNetwork(ctx context.Context, ac *kata.AgentClient, mtu uint64) error { if err := ac.UpdateInterface(ctx, &agentpb.Interface{ Device: ateomnet.ActorVethName, diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 0583edb97..9ad699700 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -43,6 +43,22 @@ const ( ActorVethIP = "169.254.17.2" ActorNftTableName = "ateom_actor" + // podPrimaryIfaceName is the worker pod's own interface, the one the CNI + // gave it. It is not ActorVethName despite the identical value: that one + // names the actor's end of the veth, which lives in the interior netns. + podPrimaryIfaceName = "eth0" + + // The IPv6 counterparts of the point-to-point pair above, chosen to echo the + // v4 addresses digit for digit. A fixed ULA rather than an RFC 4193 random + // prefix so the pair stays as readable in a packet dump as 169.254.17.x, and + // not fe80::/10 because a link-local source would need a scope id everywhere + // it is used. It must not overlap the cluster's pod CIDR; kind's dual-stack + // default is fd00:10:244::/56. + HostVethIPv6CIDR = "fd00:169:254::1/126" + ActorVethIPv6CIDR = "fd00:169:254::2/126" + ActorVethIPv6Gateway = "fd00:169:254::1" + ActorVethIPv6IP = "fd00:169:254::2" + // ActorVethSubnet is the point-to-point /30 the actor veth lives on. ActorVethSubnet = "169.254.17.0/30" ) @@ -51,6 +67,10 @@ var ( HostVethAddr = MustParseAddr(HostVethCIDR) ActorVethAddr = MustParseAddr(ActorVethCIDR) ActorVethGwIP = MustParseIP(ActorVethGateway) + + HostVethIPv6Addr = mustParseNoDADAddr(HostVethIPv6CIDR) + ActorVethIPv6Addr = mustParseNoDADAddr(ActorVethIPv6CIDR) + ActorVethIPv6GwIP = MustParseIPv6(ActorVethIPv6Gateway) ) // MustParseAddr parses a CIDR string into a netlink.Addr, panicking on error. @@ -62,6 +82,18 @@ func MustParseAddr(cidr string) *netlink.Addr { return a } +// mustParseNoDADAddr parses a CIDR into an address flagged IFA_F_NODAD. +// +// Per-address flag rather than the interface-wide accept_dad sysctl because the +// ateom container is unprivileged, so containerd mounts /proc/sys read-only. +// DAD is pointless on a point-to-point veth nobody else can reach, and it would +// otherwise hold the address tentative for ~1s on every resume. +func mustParseNoDADAddr(cidr string) *netlink.Addr { + a := MustParseAddr(cidr) + a.Flags = unix.IFA_F_NODAD + return a +} + // MustParseIP parses an IPv4 string into a net.IP, panicking on error. func MustParseIP(s string) net.IP { ip := net.ParseIP(s).To4() @@ -71,6 +103,57 @@ func MustParseIP(s string) net.IP { return ip } +// MustParseIPv6 parses an IPv6 string into a net.IP, panicking on error. An +// IPv4 string is an error: net.IP holds it as a 16-byte v4-mapped address, so +// it would pass a length check and then compare against no IPv6 header. +func MustParseIPv6(s string) net.IP { + ip := net.ParseIP(s) + if ip == nil || ip.To4() != nil { + panic(fmt.Sprintf("parsing constant IPv6 %q", s)) + } + return ip.To16() +} + +// linkIPv6Enabled reports whether IPv6 addresses can be assigned to the named +// link in the current netns. It answers a kernel capability question, not a +// cluster one: IPv4-only GKE leaves disable_ipv6=1 and netlink then rejects +// every IPv6 address with EPERM, but IPv4-only kind leaves it at 0 because the +// node kernel has IPv6 compiled in. A kernel built without IPv6 has no sysctl +// at all. Pair it with linkHasGlobalIPv6 to decide whether the actor gets IPv6; +// on its own it says yes on clusters that have no IPv6 anywhere. +func linkIPv6Enabled(name string) bool { + b, err := os.ReadFile("/proc/sys/net/ipv6/conf/" + name + "/disable_ipv6") + if err != nil { + return false + } + return len(b) > 0 && b[0] == '0' +} + +// linkHasGlobalIPv6 reports whether link carries a global IPv6 address. Called +// on the worker pod's own interface, that is what decides the families the +// actor can egress on. +// +// It answers whether the pod has an address to egress from, not whether that +// address routes anywhere: IsGlobalUnicast is true for a ULA, and dual-stack +// kind hands pods a ULA with no path off the host. Reachability is the +// cluster's problem, not something this can decide from inside the netns. +func linkHasGlobalIPv6(ctx context.Context, link netlink.Link) bool { + // netlink can report ErrDumpInterrupted alongside a valid partial answer. + // Trust a positive result either way: reporting false on a dual-stack pod + // silently strands the actor on IPv4, which is the costlier mistake. + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + slog.WarnContext(ctx, "listing IPv6 addresses of the worker pod interface", + "link", link.Attrs().Name, "error", err, "addressesRead", len(addrs)) + } + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() { + return true + } + } + return false +} + // MustParseMAC parses a MAC address string into a net.HardwareAddr, panicking on error. func MustParseMAC(s string) net.HardwareAddr { m, err := net.ParseMAC(s) @@ -82,7 +165,9 @@ func MustParseMAC(s string) net.HardwareAddr { // ConfigureActorVeth configures the actor veth inside the interior netns. // It assumes it is already running inside the target network namespace. -func ConfigureActorVeth(ctx context.Context) error { +// ipv6 comes from SetupActorNetwork, which decides it in the worker pod netns; +// this namespace cannot answer the question for itself. +func ConfigureActorVeth(ctx context.Context, ipv6 bool) error { // Run inside the gVisor interior netns. SetupActorNetwork has already created // the veth peer here, under its final name, so this only has to address it. // gVisor reads link names, addresses, and routes from this namespace when the @@ -107,6 +192,12 @@ func ConfigureActorVeth(ctx context.Context) error { if err := netlink.AddrReplace(actorLink, ActorVethAddr); err != nil { return fmt.Errorf("while assigning actor veth address: %w", err) } + if ipv6 { + if err := netlink.AddrReplace(actorLink, ActorVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning actor veth ipv6 address: %w", err) + } + } + if err := netlink.LinkSetUp(actorLink); err != nil { return fmt.Errorf("while bringing up actor veth: %w", err) } @@ -117,6 +208,15 @@ func ConfigureActorVeth(ctx context.Context) error { }); err != nil { return fmt.Errorf("while installing actor default route: %w", err) } + if ipv6 { + if err := netlink.RouteReplace(&netlink.Route{ + LinkIndex: actorLink.Attrs().Index, + Gw: ActorVethIPv6GwIP, + Dst: &net.IPNet{IP: net.ParseIP("::"), Mask: net.CIDRMask(0, 128)}, + }); err != nil { + return fmt.Errorf("while installing actor default ipv6 route: %w", err) + } + } return nil } @@ -173,7 +273,7 @@ func PodIPv4() (net.IP, error) { // Resolve the worker pod IPv4 address from the pod namespace's real eth0. // Because eth0 now stays in the pod namespace, this IP remains available for // both normal worker connectivity and the temporary inbound DNAT rule. - eth0Link, err := netlink.LinkByName("eth0") + eth0Link, err := netlink.LinkByName(podPrimaryIfaceName) if err != nil { return nil, fmt.Errorf("while getting pod eth0: %w", err) } @@ -259,10 +359,6 @@ func InstallActorNftablesRules(egressPort uint16) error { // rules in an ateom-owned table makes cleanup simple and avoids mutating // Kubernetes or CNI-managed chains directly. // - // The table is in the inet family so one table can carry both address - // families once the actor veth is dual-stack. That makes a bare payload - // match ambiguous, so every match opens with an NFPROTO comparison. - // // The rules do three things: // // * prerouting: redirect new actor TCP connections to atunnel's local @@ -294,6 +390,9 @@ func InstallActorNftablesRules(egressPort uint16) error { if redirectRule := ActorEgressRedirectRule(table, prerouting, egressPort); redirectRule != nil { c.AddRule(redirectRule) } + if redirectRuleIPv6 := ActorIPv6EgressRedirectRule(table, prerouting, egressPort); redirectRuleIPv6 != nil { + c.AddRule(redirectRuleIPv6) + } postrouting := c.AddChain(&nftables.Chain{ Name: "postrouting", @@ -307,6 +406,11 @@ func InstallActorNftablesRules(egressPort uint16) error { Chain: postrouting, Exprs: append(IPSourceEqual(ActorVethIP), &expr.Masq{}), }) + c.AddRule(&nftables.Rule{ + Table: table, + Chain: postrouting, + Exprs: append(IPv6SourceEqual(ActorVethIPv6IP), &expr.Masq{}), + }) acceptPolicy := nftables.ChainPolicyAccept forward := c.AddChain(&nftables.Chain{ @@ -337,9 +441,10 @@ func RemoveActorNftablesRules() error { // per-worker and currently per-active-actor because this worker path runs at // most one actor at a time. Missing tables are treated as already clean. // - // Both families are swept, not just the inet one this installs into: a table - // name is unique per family, so an ip table left by an earlier ateom would - // survive every later cleanup and keep redirecting alongside the new one. + // Both families are swept, not just the inet one this now installs into: a + // table name is unique per family, so an ip table left by an earlier ateom + // would survive every later cleanup and keep redirecting alongside the new + // one. c := &nftables.Conn{} for _, family := range []nftables.TableFamily{nftables.TableFamilyINet, nftables.TableFamilyIPv4} { tables, err := c.ListTablesOfFamily(family) @@ -385,6 +490,32 @@ func IPPayloadEqual(offset uint32, ip string) []expr.Any { } } +func IPv6SourceEqual(ip string) []expr.Any { + return IPv6PayloadEqual(8, ip) +} + +func IPv6PayloadEqual(offset uint32, ip string) []expr.Any { + return []expr.Any{ + &expr.Meta{Key: expr.MetaKeyNFPROTO, Register: 1}, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: []byte{unix.NFPROTO_IPV6}, + }, + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseNetworkHeader, + Offset: offset, + Len: 16, + }, + &expr.Cmp{ + Op: expr.CmpOpEq, + Register: 1, + Data: MustParseIPv6(ip), + }, + } +} + func TCPProtocol() []expr.Any { return []expr.Any{ &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, @@ -414,6 +545,24 @@ func ActorEgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port return &nftables.Rule{Table: table, Chain: chain, Exprs: exprs} } +// ActorIPv6EgressRedirectRule is ActorEgressRedirectRule for the actor's IPv6 +// source address. Both rules live in the same inet table, so each carries its +// own NFPROTO match to keep it off the other family's packets. +func ActorIPv6EgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port uint16) *nftables.Rule { + if port == 0 { + return nil + } + exprs := append(IPv6SourceEqual(ActorVethIPv6IP), TCPProtocol()...) + exprs = append(exprs, + &expr.Immediate{ + Register: 1, + Data: binaryutil.BigEndian.PutUint16(port), + }, + &expr.Redir{RegisterProtoMin: 1}, + ) + return &nftables.Rule{Table: table, Chain: chain, Exprs: exprs} +} + // CreateNetNSWithoutSwitching creates a named netns and returns its handle, // restoring the caller's current netns before returning. func CreateNetNSWithoutSwitching(name string) (netns.NsHandle, error) { @@ -599,11 +748,31 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { if err := netlink.AddrReplace(hostLink, HostVethAddr); err != nil { return fmt.Errorf("while assigning host veth address: %w", err) } + // Decided once, here in the worker pod netns, and carried into the interior + // netns below. Probing separately on each side would let them disagree: the + // interior netns is freshly created, so its sysctl is always the permissive + // kernel default whatever the pod's families are. + var podIPv6 bool + if podLink, err := netlink.LinkByName(podPrimaryIfaceName); err == nil { + podIPv6 = linkHasGlobalIPv6(ctx, podLink) + } + vethIPv6 := linkIPv6Enabled(HostVethName) + actorIPv6 := podIPv6 && vethIPv6 + if actorIPv6 { + if err := netlink.AddrReplace(hostLink, HostVethIPv6Addr); err != nil { + return fmt.Errorf("while assigning host veth ipv6 address: %w", err) + } + } else { + slog.InfoContext(ctx, "actor networking is IPv4-only", + "link", HostVethName, "podHasGlobalIPv6", podIPv6, "vethIPv6Enabled", vethIPv6) + } if err := netlink.LinkSetUp(hostLink); err != nil { return fmt.Errorf("while bringing up host veth: %w", err) } - if err := NetNSDo(ctx, cfg.InteriorNetNS, ConfigureActorVeth); err != nil { + if err := NetNSDo(ctx, cfg.InteriorNetNS, func(ctx context.Context) error { + return ConfigureActorVeth(ctx, actorIPv6) + }); err != nil { return fmt.Errorf("while configuring actor veth in interior netns: %w", err) } diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index 436efc32c..bbaddc78e 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -20,6 +20,7 @@ import ( "context" "errors" "net" + "os" "runtime" "testing" @@ -27,6 +28,7 @@ import ( "github.com/google/nftables" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" + "golang.org/x/sys/unix" ) // withTestNetNS runs fn with the calling thread inside a throwaway netns @@ -319,6 +321,190 @@ func TestRemoveActorNftablesRulesSweepsIPv4Family(t *testing.T) { }) } +// addPodEth0 plants a dummy link carrying cidrs in the current netns, standing +// in for the worker pod's own primary interface. withTestNetNS hands out a bare +// namespace, and the families on that interface are what SetupActorNetwork reads +// to decide the families the actor gets. +// +// The name has to be exactly podPrimaryIfaceName: the probe is link-scoped, so +// under any other name it answers false and the test asserts the opposite of +// what it means to. +func addPodEth0(t *testing.T, cidrs ...string) { + t.Helper() + + link := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: podPrimaryIfaceName}} + if err := netlink.LinkAdd(link); err != nil { + t.Fatalf("creating the stand-in pod %s: %v", podPrimaryIfaceName, err) + } + if err := netlink.LinkSetUp(link); err != nil { + t.Fatalf("bringing up the stand-in pod %s: %v", podPrimaryIfaceName, err) + } + for _, cidr := range cidrs { + addr := MustParseAddr(cidr) + addr.Flags |= unix.IFA_F_NODAD // else an IPv6 address stays tentative + if err := netlink.AddrAdd(link, addr); err != nil { + t.Fatalf("assigning %s to the stand-in pod %s: %v", cidr, podPrimaryIfaceName, err) + } + } +} + +// writeSysctl turns an IPv6 knob off in the current netns. "all" flushes the +// addresses already assigned; "default" only reaches links created afterwards. +func writeSysctl(t *testing.T, knob string) { + t.Helper() + path := "/proc/sys/net/ipv6/conf/" + knob + "/disable_ipv6" + if err := os.WriteFile(path, []byte("1\n"), 0o644); err != nil { + t.Fatalf("disabling IPv6 via %s: %v", path, err) + } +} + +// assertIPv6AddrNoDAD requires cidr to be present on link and to carry +// IFA_F_NODAD. +// +// The flag is the whole point: the ateom container is unprivileged, so the +// accept_dad sysctl this replaced could not be written and setup failed outright +// on a real worker. It passes as root, where /proc/sys is writable either way, +// so nothing else here would catch a regression back to the sysctl. +func assertIPv6AddrNoDAD(t *testing.T, link netlink.Link, cidr string) { + t.Helper() + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + t.Fatalf("listing IPv6 addresses of %q: %v", link.Attrs().Name, err) + } + want := MustParseAddr(cidr) + for _, addr := range addrs { + if addr.IPNet == nil || addr.IPNet.String() != want.IPNet.String() { + continue + } + if addr.Flags&unix.IFA_F_NODAD == 0 { + t.Errorf("%s on %q has flags %#x, want IFA_F_NODAD (%#x) set", cidr, link.Attrs().Name, addr.Flags, unix.IFA_F_NODAD) + } + return + } + t.Errorf("%q does not carry %s, got %v", link.Attrs().Name, cidr, addrs) +} + +// assertNoGlobalIPv6Addr requires link to carry no IPv6 address beyond the +// fe80::/64 the kernel gives every up link wherever IPv6 is enabled at all. +// That link-local is not what strands an actor -- the routable address is. +func assertNoGlobalIPv6Addr(t *testing.T, link netlink.Link) { + t.Helper() + addrs, err := netlink.AddrList(link, netlink.FAMILY_V6) + if err != nil { + t.Fatalf("listing IPv6 addresses of %q: %v", link.Attrs().Name, err) + } + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() { + t.Errorf("%q carries global IPv6 address %s, want none", link.Attrs().Name, addr) + } + } +} + +// TestSetupActorNetworkIPv6Gate is the truth table for who gets actor IPv6. +// Both halves have to hold: the worker pod needs a global IPv6 address of its +// own, or the actor prefers the AAAA of a dual-stack destination and the +// connection dies with nowhere to go; and the veth has to accept an IPv6 +// address at all, or the assignment fails with EPERM on the path of every +// SetupActorNetwork call and the actor never starts. +// +// The IPv4 half must come out identical in every case. +func TestSetupActorNetworkIPv6Gate(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + for _, tc := range []struct { + name string + // podAddrs go on the stand-in pod interface before setup runs. + podAddrs []string + // disable, when set, runs in the pod netns after podAddrs are assigned. + disable func(*testing.T) + wantIPv6 bool + }{{ + name: "dual-stack pod", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + wantIPv6: true, + }, { + // A probe that reads the wrong link or the wrong scope fails closed, + // which every IPv4 case here would happily accept. This one notices. + name: "IPv6-only pod", + podAddrs: []string{"fd00:10:244::7/64"}, + wantIPv6: true, + }, { + // An IPv4-only cluster whose kernel still has IPv6 compiled in, so every + // capability probe says yes. This is the case that turned the IPv4 e2e + // job red. + name: "pod without IPv6", + podAddrs: []string{"10.244.0.7/24"}, + }, { + // The default on IPv4-only GKE. Writing "all" also flushes podAddrs, so + // both halves of the gate are false here. + name: "IPv6 disabled for the whole netns", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + disable: func(t *testing.T) { + writeSysctl(t, "all") + writeSysctl(t, "default") + }, + }, { + // The one case the capability half is there for: the pod keeps its + // address, but the veth created next inherits disable_ipv6=1. + name: "IPv6 disabled per link", + podAddrs: []string{"10.244.0.7/24", "fd00:10:244::7/64"}, + disable: func(t *testing.T) { writeSysctl(t, "default") }, + }} { + t.Run(tc.name, func(t *testing.T) { + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + addPodEth0(t, tc.podAddrs...) + if tc.disable != nil { + tc.disable(t) + } + + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork: %v", err) + } + + host := linkByName(t, HostVethName) + if host == nil { + t.Fatalf("host veth %q missing from the pod netns", HostVethName) + } + if !hasAddr(t, host, HostVethCIDR) { + t.Errorf("host veth %q does not carry %s", HostVethName, HostVethCIDR) + } + if tc.wantIPv6 { + assertIPv6AddrNoDAD(t, host, HostVethIPv6CIDR) + } else { + assertNoGlobalIPv6Addr(t, host) + } + + if err := NetNSDo(ctx, interior, func(context.Context) error { + actor := linkByName(t, ActorVethName) + if actor == nil { + t.Fatalf("actor veth %q missing from the interior netns", ActorVethName) + } + if !hasAddr(t, actor, ActorVethCIDR) { + t.Errorf("actor veth %q does not carry %s", ActorVethName, ActorVethCIDR) + } + assertDefaultRoute(t, actor, netlink.FAMILY_V4, ActorVethGwIP, true) + + // The interior netns is created fresh, so its own sysctls always + // say IPv6 is available whatever the pod's families are. Only a + // decision carried across from the pod netns gets this right. + if tc.wantIPv6 { + assertIPv6AddrNoDAD(t, actor, ActorVethIPv6CIDR) + } else { + assertNoGlobalIPv6Addr(t, actor) + } + assertDefaultRoute(t, actor, netlink.FAMILY_V6, ActorVethIPv6GwIP, tc.wantIPv6) + return nil + }); err != nil { + t.Fatalf("inspecting interior netns: %v", err) + } + }) + }) + } +} + // TestSetupActorNetworkHostVethHWAddr covers the micro-VM requirement: a CH // snapshot freezes the guest's ARP entry for the gateway, so the worker-side // veth MAC has to be exactly the one the caller asked for, on every pod. From 13a1e4b714a6ccd7fef112dbb1aab514b50ad651 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 21 Aug 2026 08:26:45 -0700 Subject: [PATCH 20/21] atunnel: dispatch the original destination lookup by family The IPv6 lookup ran only as a fallback, after an IPv4 lookup that a pure-IPv6 socket can never satisfy: its inet addresses are zeroed, so the conntrack tuple is all zeros and the query always misses with ENOENT. That made every IPv6 egress connection pay a guaranteed-to-fail syscall whose result was discarded, and left the code depending on that kernel detail holding. The connection's local address already says which family the flow is, so ask for that family's option directly. The failure message now names the family it asked for. atunnel logs it at warn level and it is the only signal an operator gets when a redirected connection cannot be resolved. --- internal/atunnel/original_dst_linux.go | 28 +++++++++++++++----------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/internal/atunnel/original_dst_linux.go b/internal/atunnel/original_dst_linux.go index b9d171747..4a5cb33b3 100644 --- a/internal/atunnel/original_dst_linux.go +++ b/internal/atunnel/original_dst_linux.go @@ -18,7 +18,6 @@ package atunnel import ( "encoding/binary" - "errors" "fmt" "net" "strconv" @@ -27,8 +26,10 @@ import ( "golang.org/x/sys/unix" ) -// IP6T_SO_ORIGINAL_DST is not generated by golang.org/x/sys/unix. It is -// defined as 80 in linux/netfilter_ipv6/ip6_tables.h. +// IP6T_SO_ORIGINAL_DST is not generated by golang.org/x/sys/unix. It is defined +// as 80 in linux/netfilter_ipv6/ip6_tables.h — the same number as +// unix.SO_ORIGINAL_DST by coincidence, not by definition, since the two are +// options of different levels. const ip6tSOOriginalDst = 80 // TCPOriginalDestination reads the IPv4 or IPv6 destination preserved by a @@ -42,9 +43,10 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { if err != nil { return "", fmt.Errorf("atunnel: acquiring TCP syscall connection: %w", err) } - // The IPv6 option is only meaningful on an AF_INET6 socket: on AF_INET the - // kernel returns EOPNOTSUPP, which would mask the real IPv4 error. A - // v4-mapped local address still means an IPv4 flow, so To4 is the test. + // Each family keeps its original destination under its own socket option + // level, and querying the other one answers EOPNOTSUPP rather than anything + // about the flow. A v4-mapped local address still means an IPv4 flow, so To4 + // is the test, not the socket domain. local, ok := tcpConn.LocalAddr().(*net.TCPAddr) if !ok { return "", fmt.Errorf("atunnel: original destination requires a TCP local address, got %T", tcpConn.LocalAddr()) @@ -54,18 +56,20 @@ func TCPOriginalDestination(conn net.Conn) (string, error) { var sockoptErr error var destination string if err := rawConn.Control(func(fd uintptr) { - destination, sockoptErr = originalIPv4Destination(fd) - // A pure-IPv6 socket leaves the inet addresses zeroed, so the IPv4 - // conntrack lookup always misses with ENOENT. That is the redirected - // IPv6 connection, and the only case worth retrying. - if isIPv6 && errors.Is(sockoptErr, unix.ENOENT) { + if isIPv6 { destination, sockoptErr = originalIPv6Destination(fd) + return } + destination, sockoptErr = originalIPv4Destination(fd) }); err != nil { return "", fmt.Errorf("atunnel: accessing TCP socket: %w", err) } if sockoptErr != nil { - return "", fmt.Errorf("atunnel: reading original TCP destination: %w", sockoptErr) + family := "IPv4" + if isIPv6 { + family = "IPv6" + } + return "", fmt.Errorf("atunnel: reading original %s TCP destination: %w", family, sockoptErr) } return destination, nil } From 561cd0ea2e498bdb266a89802a78e7be34a7c49b Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Fri, 21 Aug 2026 09:56:24 -0700 Subject: [PATCH 21/21] atunnel: rework the original-destination tests The redirect tests built their veth, their nftables NAT table and both listeners in the host network namespace, so a machine with a default-deny INPUT policy dropped the redirected SYN and the test failed rather than skipped. Both sides now live in throwaway namespaces, which also removes the PID-derived addresses and interface names that could truncate or collide, and leaves no nftables state behind on the host. Coverage grew three ways: the IPv4 and IPv6 cases are one table built from the same ateomnet matchers production uses; that table gained the dual-stack socket the worker actually listens on, where an IPv4 actor arrives with a v4-mapped local address and is still only readable through the IPv4 socket option; and the address formatting, which was reachable only from root-gated tests, now has an ordinary one. Both lookups report a miss as ENOENT, so the failure-path cases assert the family named in the error rather than the errno alone. --- .../atunnel/original_dst_format_linux_test.go | 107 +++ internal/atunnel/original_dst_linux_test.go | 639 +++++++++--------- 2 files changed, 410 insertions(+), 336 deletions(-) create mode 100644 internal/atunnel/original_dst_format_linux_test.go diff --git a/internal/atunnel/original_dst_format_linux_test.go b/internal/atunnel/original_dst_format_linux_test.go new file mode 100644 index 000000000..0558af183 --- /dev/null +++ b/internal/atunnel/original_dst_format_linux_test.go @@ -0,0 +1,107 @@ +//go:build linux + +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package atunnel + +import ( + "encoding/binary" + "net" + "strings" + "testing" +) + +// networkOrderPort produces the raw field value the kernel leaves in +// RawSockaddrInet4.Port and RawSockaddrInet6.Port: a uint16 whose in-memory +// bytes are the port in network order, which on a little-endian host is not +// the port's numeric value. +func networkOrderPort(port uint16) uint16 { + return binary.NativeEndian.Uint16(binary.BigEndian.AppendUint16(nil, port)) +} + +func TestFormatOriginalDestination(t *testing.T) { + tests := []struct { + name string + ip []byte + port uint16 + want string + wantErr bool + }{ + { + name: "IPv4", + ip: []byte{198, 18, 0, 1}, + port: 443, + want: "198.18.0.1:443", + }, + { + name: "IPv6 is bracketed", + ip: net.ParseIP("fd00:198:18::1").To16(), + port: 443, + // SplitHostPort in the atunnel client needs the brackets. + want: "[fd00:198:18::1]:443", + }, + { + name: "v4-mapped IPv6 renders as IPv4", + ip: net.ParseIP("::ffff:198.18.0.1").To16(), + port: 8080, + want: "198.18.0.1:8080", + }, + { + name: "high port is not sign-extended", + ip: []byte{198, 18, 0, 1}, + port: 65535, + want: "198.18.0.1:65535", + }, + { + // A zero port means the lookup answered without a real destination, + // which would otherwise become a dial to port 0. + name: "port zero is rejected", + ip: []byte{198, 18, 0, 1}, + port: 0, + wantErr: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, err := formatOriginalDestination(test.ip, networkOrderPort(test.port)) + if test.wantErr { + if err == nil { + t.Fatalf("formatOriginalDestination() = %q, want an error", got) + } + return + } + if err != nil { + t.Fatalf("formatOriginalDestination() error = %v", err) + } + if got != test.want { + t.Errorf("formatOriginalDestination() = %q, want %q", got, test.want) + } + }) + } +} + +func TestTCPOriginalDestinationRejectsNonTCPConn(t *testing.T) { + client, server := net.Pipe() + t.Cleanup(func() { _ = client.Close() }) + t.Cleanup(func() { _ = server.Close() }) + + got, err := TCPOriginalDestination(client) + if err == nil { + t.Fatalf("TCPOriginalDestination() = %q, want an error on a non-TCP connection", got) + } + if !strings.Contains(err.Error(), "requires a TCP connection") { + t.Errorf("TCPOriginalDestination() error = %v, want it to name the unsupported connection type", err) + } +} diff --git a/internal/atunnel/original_dst_linux_test.go b/internal/atunnel/original_dst_linux_test.go index 3a9dbfca2..3e0ae61d1 100644 --- a/internal/atunnel/original_dst_linux_test.go +++ b/internal/atunnel/original_dst_linux_test.go @@ -21,7 +21,8 @@ import ( "errors" "fmt" "net" - "os" + "runtime" + "strconv" "strings" "testing" "time" @@ -37,373 +38,353 @@ import ( "github.com/agent-substrate/substrate/internal/roottest" ) -// TestTCPOriginalDestinationPreservesErrno covers the failure path on an -// ordinary connection that no REDIRECT rule touched. The IPv4 lookup misses -// and reports ENOENT; that error must reach the caller. Retrying the IPv6 -// option on an AF_INET socket would replace it with EOPNOTSUPP, which says -// nothing about why the lookup failed. -// -// It runs in a fresh namespace because conntrack tracks loopback in any -// namespace that has nftables rules — including the one Docker runs in — and a -// tracked connection returns its real destination instead of missing. -func TestTCPOriginalDestinationPreservesErrno(t *testing.T) { - roottest.Require(t, "CAP_SYS_ADMIN for a network namespace with no conntrack hooks") +// originalDstFamily parameterizes the redirect test. The wiring is the same in +// every case; only the addresses, the nftables table family, the source-address +// matcher and the sockets on either end differ. +type originalDstFamily struct { + name string + // listenNetwork and listenIP describe the worker's listeners; an empty + // listenIP binds the unspecified address. dialNetwork is what the actor + // dials with, which is not always the same family the listener was opened + // as. + listenNetwork string + listenIP string + dialNetwork string + nftFamily nftables.TableFamily + workerIP string + actorIP string + mask net.IPMask + addrFlags int + sourceEqual func(string) []expr.Any +} - ns := newTestNetNS(t) - if err := ateomnet.NetNSDo(context.Background(), ns, func(context.Context) error { - loopback, err := netlink.LinkByName("lo") - if err != nil { - return err - } - if err := netlink.LinkSetUp(loopback); err != nil { - return err - } +// Fixed addresses are safe because each test builds its own namespaces. +var originalDstFamilies = []originalDstFamily{ + { + name: "IPv4", + listenNetwork: "tcp4", + listenIP: "198.18.0.1", + dialNetwork: "tcp4", + nftFamily: nftables.TableFamilyIPv4, + workerIP: "198.18.0.1", + actorIP: "198.18.0.2", + mask: net.CIDRMask(30, 32), + sourceEqual: ateomnet.IPSourceEqual, + }, + { + name: "IPv6", + listenNetwork: "tcp6", + listenIP: "fd00:198:18::1", + dialNetwork: "tcp6", + nftFamily: nftables.TableFamilyIPv6, + workerIP: "fd00:198:18::1", + actorIP: "fd00:198:18::2", + mask: net.CIDRMask(64, 128), + // The veth is alone in a throwaway namespace, so nothing can collide + // with it. Skipping DAD lets the listener bind straight away instead of + // waiting out the tentative period. + addrFlags: unix.IFA_F_NODAD, + sourceEqual: ateomnet.IPv6SourceEqual, + }, + { + // atunnel listens on an unspecified address, which Go opens as one + // dual-stack AF_INET6 socket, so an IPv4 actor arrives there with a + // v4-mapped local address and its original destination is still only + // readable through the IPv4 socket option. This is the shape production + // runs in and the one the family check exists for. + name: "DualStackV4Mapped", + listenNetwork: "tcp", + dialNetwork: "tcp4", + nftFamily: nftables.TableFamilyIPv4, + workerIP: "198.18.0.1", + actorIP: "198.18.0.2", + mask: net.CIDRMask(30, 32), + sourceEqual: ateomnet.IPSourceEqual, + }, +} - listener, err := net.Listen("tcp4", "127.0.0.1:0") - if err != nil { - return err - } - defer listener.Close() - client, err := net.DialTimeout("tcp4", listener.Addr().String(), time.Second) - if err != nil { - return err - } - defer client.Close() - server, err := listener.Accept() - if err != nil { - return err - } - defer server.Close() +// TestTCPOriginalDestinationRedirect models the production egress path: actor +// traffic enters the worker namespace over a veth, an nftables PREROUTING rule +// redirects it to a local listener, and that listener asks the kernel what the +// actor originally dialed. Redirecting a locally generated connection through +// OUTPUT would not exercise the same path. +// +// The worker side gets its own namespace rather than borrowing the host's. That +// keeps the test clear of any local firewall policy — a default-deny INPUT +// chain would otherwise drop the redirected SYN — and the veth and nftables +// table go away with the namespace instead of needing to be swept up. +func TestTCPOriginalDestinationRedirect(t *testing.T) { + roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for network namespaces and an nftables REDIRECT rule") - if _, err := TCPOriginalDestination(server); !errors.Is(err, unix.ENOENT) { - return fmt.Errorf("want the IPv4 lookup's ENOENT, got %w", err) - } - return nil - }); err != nil { - t.Fatal(err) - } -} + for _, family := range originalDstFamilies { + t.Run(family.name, func(t *testing.T) { + workerNS := newTestNetNS(t) + actorNS := newTestNetNS(t) + requireNftables(t, workerNS) + setupTestVeth(t, family, workerNS, actorNS) -func TestTCPOriginalDestination(t *testing.T) { - roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") + // targetListener holds the port the actor means to reach, so the + // assertion below cannot pass by the connection simply arriving + // where it was aimed. redirectListener stands in for atunnel's own + // egress listener and is where the redirect must land instead. + redirectListener := listenInNetNS(t, workerNS, family) + targetListener := listenInNetNS(t, workerNS, family) + targetPort := targetListener.Addr().(*net.TCPAddr).Port + redirectPort := redirectListener.Addr().(*net.TCPAddr).Port + installOriginalDstRedirect(t, family, workerNS, targetPort, redirectPort) - // Model the production path rather than redirecting a locally generated - // connection through OUTPUT. Actor egress enters the worker netns through a - // veth and is redirected in PREROUTING; that is the path on which Linux - // preserves SO_ORIGINAL_DST for atunnel. - actorNS := newTestNetNS(t) - actorIP, hostIP := setupTestVeth(t, actorNS) - // targetListener reserves the port the actor intends to reach. The NAT rule - // below must prevent connections from reaching it. - // - // redirectListener represents atunnel's local egress listener. It receives - // the redirected connection and is therefore the connection on which we ask - // Linux for the original destination. - redirectListener := listenTCP(t, hostIP) - defer redirectListener.Close() - targetListener := listenTCP(t, hostIP) - defer targetListener.Close() - targetPort := targetListener.Addr().(*net.TCPAddr).Port + target := net.JoinHostPort(family.workerIP, strconv.Itoa(targetPort)) + clientDone := make(chan error, 1) + go func() { + // From the actor's side this is an ordinary connection to the + // worker's address; PREROUTING rewrites it on the way in. + clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + conn, err := net.DialTimeout(family.dialNetwork, target, 10*time.Second) + if err != nil { + return err + } + return conn.Close() + }) + }() - table := &nftables.Table{Family: nftables.TableFamilyIPv4, Name: fmt.Sprintf("atunnel_original_dst_test_%d", os.Getpid())} - installOriginalDstRedirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + if err := redirectListener.SetDeadline(time.Now().Add(10 * time.Second)); err != nil { + t.Fatalf("setting the accept deadline: %v", err) + } + redirected, err := redirectListener.Accept() + if err != nil { + t.Fatalf("accepting the redirected connection: %v", err) + } + defer redirected.Close() - clientDone := make(chan error, 1) - go func() { - // From the actor's perspective this is an ordinary connection to - // hostIP:targetPort. The worker's PREROUTING rule redirects it before - // it reaches the host network stack's local delivery path. - clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - conn, err := net.DialTimeout("tcp4", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) - if err == nil { - _ = conn.Close() + // The accepted socket is addressed to redirectListener; the kernel's + // conntrack record must still hold what the actor dialed. + got, err := TCPOriginalDestination(redirected) + if err != nil { + t.Fatalf("TCPOriginalDestination: %v", err) + } + if got != target { + t.Errorf("original destination = %q, want %q", got, target) + } + if err := <-clientDone; err != nil { + t.Fatalf("dialing through the redirect: %v", err) } - return err }) - }() - - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { - t.Fatal(err) - } - redirected, err := redirectListener.Accept() - if err != nil { - t.Fatalf("accepting redirected connection: %v", err) - } - defer redirected.Close() - - // The accepted socket is addressed to redirectListener, but the kernel's - // SO_ORIGINAL_DST record must still contain the destination chosen by the - // actor before nftables rewrote it. - got, err := TCPOriginalDestination(redirected) - if err != nil { - t.Fatalf("TCPOriginalDestination: %v", err) - } - want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) - if got != want { - t.Errorf("original destination = %q, want %q", got, want) - } - if err := <-clientDone; err != nil { - t.Fatalf("dialing redirected connection: %v", err) } } -func TestTCPOriginalDestinationIPv6(t *testing.T) { - roottest.Require(t, "CAP_NET_ADMIN + CAP_SYS_ADMIN for an actor-like network namespace and nftables REDIRECT rule") - - actorNS := newTestNetNS(t) - actorIP, hostIP := setupTestIPv6Veth(t, actorNS) - redirectListener := listenTCP6(t, hostIP) - defer redirectListener.Close() - targetListener := listenTCP6(t, hostIP) - defer targetListener.Close() - targetPort := targetListener.Addr().(*net.TCPAddr).Port +// TestTCPOriginalDestinationPreservesErrno covers the failure path on an +// ordinary connection that no REDIRECT rule touched. Each family's lookup +// misses and reports ENOENT, and that error must reach the caller rather than +// the EOPNOTSUPP a single-family socket answers for the other family's option. +// A dual-stack socket answers both with ENOENT, so the errno alone does not +// say which lookup ran; the family named in the message does. +// +// The dual-stack case is the shape production actually sees. atunnel listens +// on an unspecified address, which Go opens as an AF_INET6 socket with +// IPV6_V6ONLY off, so every IPv4 actor connection arrives with a v4-mapped +// local address and must still take the IPv4 lookup. +// +// These run in a fresh namespace because conntrack tracks loopback in any +// namespace that has nftables rules — including the one Docker runs in — and a +// tracked connection returns its real destination instead of missing. +func TestTCPOriginalDestinationPreservesErrno(t *testing.T) { + roottest.Require(t, "CAP_SYS_ADMIN for a network namespace with no conntrack hooks") - table := &nftables.Table{Family: nftables.TableFamilyIPv6, Name: fmt.Sprintf("atunnel_original_dst_ipv6_test_%d", os.Getpid())} - installOriginalDstIPv6Redirect(t, table, actorIP, targetPort, redirectListener.Addr().(*net.TCPAddr).Port) + tests := []struct { + name string + listenNetwork string + listenAddress string + dialNetwork string + dialHost string + // wantFamily is the family the error has to name. Both lookups miss + // with the same errno, so this is the only thing that distinguishes + // the one that ran from the one that should have. + wantFamily string + }{ + {name: "IPv4", listenNetwork: "tcp4", listenAddress: "127.0.0.1:0", dialNetwork: "tcp4", dialHost: "127.0.0.1", wantFamily: "IPv4"}, + {name: "IPv6", listenNetwork: "tcp6", listenAddress: "[::1]:0", dialNetwork: "tcp6", dialHost: "::1", wantFamily: "IPv6"}, + {name: "DualStackV4Mapped", listenNetwork: "tcp", listenAddress: ":0", dialNetwork: "tcp4", dialHost: "127.0.0.1", wantFamily: "IPv4"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ns := newTestNetNS(t) + // Recorded rather than asserted in place: t.Skipf unwinds the + // goroutine, and NetNSDo has the thread switched into another + // namespace at that point. + var lookupErr error + if err := ateomnet.NetNSDo(context.Background(), ns, func(context.Context) error { + listener, err := net.Listen(test.listenNetwork, test.listenAddress) + if err != nil { + return err + } + defer listener.Close() + _, port, err := net.SplitHostPort(listener.Addr().String()) + if err != nil { + return err + } + client, err := net.DialTimeout(test.dialNetwork, net.JoinHostPort(test.dialHost, port), 10*time.Second) + if err != nil { + return err + } + defer client.Close() + server, err := listener.Accept() + if err != nil { + return err + } + defer server.Close() - clientDone := make(chan error, 1) - go func() { - clientDone <- ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - conn, err := net.DialTimeout("tcp6", net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)), 10*time.Second) - if err == nil { - _ = conn.Close() + _, lookupErr = TCPOriginalDestination(server) + return nil + }); err != nil { + t.Fatal(err) + } + if errors.Is(lookupErr, unix.ENOPROTOOPT) { + // A kernel built without the conntrack socket option handler + // cannot answer either family, so there is nothing to assert. + t.Skipf("the kernel does not serve SO_ORIGINAL_DST: %v", lookupErr) + } + if !errors.Is(lookupErr, unix.ENOENT) { + t.Errorf("want a lookup miss reported as ENOENT, got %v", lookupErr) + } + if want := "original " + test.wantFamily + " TCP destination"; !strings.Contains(lookupErr.Error(), want) { + t.Errorf("want the error to report %q, got %v", want, lookupErr) } - return err }) - }() - - if err := redirectListener.(*net.TCPListener).SetDeadline(time.Now().Add(10 * time.Second)); err != nil { - t.Fatal(err) - } - redirected, err := redirectListener.Accept() - if err != nil { - t.Fatalf("accepting redirected IPv6 connection: %v", err) - } - defer redirected.Close() - - // This assertion captures the IPv6 behavior required by #686. - got, err := TCPOriginalDestination(redirected) - if err != nil { - t.Fatalf("TCPOriginalDestination: %v", err) - } - want := net.JoinHostPort(hostIP.String(), fmt.Sprint(targetPort)) - if got != want { - t.Errorf("original IPv6 destination = %q, want %q", got, want) - } - if err := <-clientDone; err != nil { - t.Fatalf("dialing redirected IPv6 connection: %v", err) } } +// newTestNetNS returns a throwaway network namespace with its loopback up. It +// is anonymous rather than named: there is no /var/run/netns bind mount to +// collide with a concurrent run or to leak if the process is killed, and +// closing the handle takes every link and nftables table in it away. func newTestNetNS(t *testing.T) netns.NsHandle { t.Helper() - name := fmt.Sprintf("atunnel-original-dst-%d", os.Getpid()) - ns, err := ateomnet.CreateNetNSWithoutSwitching(name) + // A namespace is a property of the thread, and netns.New switches the + // caller into the one it creates, so the thread has to be pinned until we + // have switched back. + runtime.LockOSThread() + defer runtime.UnlockOSThread() + current, err := netns.Get() if err != nil { - if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { - t.Skipf("needs CAP_SYS_ADMIN to create network namespace: %v", err) - } - t.Fatal(err) + t.Fatalf("getting the current netns: %v", err) } - t.Cleanup(func() { - _ = ns.Close() - if err := netns.DeleteNamed(name); err != nil { - t.Errorf("deleting test network namespace: %v", err) + defer current.Close() + // Registered before the namespace below so it runs after it is created. + defer func() { + if err := netns.Set(current); err != nil { + t.Errorf("restoring the original netns: %v", err) } - }) - return ns -} + }() -func setupTestVeth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { - t.Helper() - hostName := fmt.Sprintf("atod%d", os.Getpid()) - peerName := fmt.Sprintf("atop%d", os.Getpid()) - if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { - if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { - t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) - } - t.Fatal(err) - } - t.Cleanup(func() { - if link, err := netlink.LinkByName(hostName); err == nil { - if err := netlink.LinkDel(link); err != nil { - t.Errorf("deleting test veth: %v", err) - } - } - }) - hostLink, err := netlink.LinkByName(hostName) + ns, err := netns.New() if err != nil { - t.Fatal(err) - } - // Allocate one of the /30s in 198.18.0.0/16 from the PID so concurrent - // test processes do not try to use the same host-side address. - network := uint16(os.Getpid() % (1 << 14)) - thirdOctet := byte(network >> 6) - fourthOctet := byte(network&0x3f) << 2 - hostIP = net.IPv4(198, 18, thirdOctet, fourthOctet+1) - actorIP = net.IPv4(198, 18, thirdOctet, fourthOctet+2) - if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(30, 32)}}); err != nil { - t.Fatal(err) - } - if err := netlink.LinkSetUp(hostLink); err != nil { - t.Fatal(err) + if errors.Is(err, unix.EPERM) { + t.Skipf("needs CAP_SYS_ADMIN to create a network namespace: %v", err) + } + t.Fatalf("creating a test netns: %v", err) } - peer, err := netlink.LinkByName(peerName) + t.Cleanup(func() { _ = ns.Close() }) + + loopback, err := netlink.LinkByName("lo") if err != nil { - t.Fatal(err) - } - if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { - t.Fatal(err) + t.Fatalf("looking up lo in the test netns: %v", err) } - // Complete the actor end of the point-to-point link inside its own netns. - if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - lo, err := netlink.LinkByName("lo") - if err != nil { - return err - } - if err := netlink.LinkSetUp(lo); err != nil { - return err - } - link, err := netlink.LinkByName(peerName) - if err != nil { - return err - } - if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(30, 32)}}); err != nil { - return err - } - return netlink.LinkSetUp(link) - }); err != nil { - t.Fatal(err) + if err := netlink.LinkSetUp(loopback); err != nil { + t.Fatalf("bringing lo up in the test netns: %v", err) } - return actorIP, hostIP + return ns } -func listenTCP(t *testing.T, hostIP net.IP) net.Listener { +// requireNftables skips when the kernel in this environment cannot serve the +// nftables netlink API at all, which is a property of the machine rather than +// of the code under test. +func requireNftables(t *testing.T, ns netns.NsHandle) { t.Helper() - listener, err := net.ListenTCP("tcp4", &net.TCPAddr{IP: hostIP, Port: 0}) + c, err := nftables.New(nftables.WithNetNSFd(int(ns))) + if err == nil { + _, err = c.ListTablesOfFamily(nftables.TableFamilyIPv4) + } if err != nil { - t.Fatal(err) + t.Skipf("nftables unavailable in this environment: %v", err) } - return listener } -func setupTestIPv6Veth(t *testing.T, actorNS netns.NsHandle) (actorIP, hostIP net.IP) { +// setupTestVeth joins the two namespaces with an addressed point-to-point veth. +func setupTestVeth(t *testing.T, family originalDstFamily, workerNS, actorNS netns.NsHandle) { t.Helper() - hostName := fmt.Sprintf("atod6%d", os.Getpid()) - peerName := fmt.Sprintf("atop6%d", os.Getpid()) - if err := netlink.LinkAdd(&netlink.Veth{LinkAttrs: netlink.LinkAttrs{Name: hostName}, PeerName: peerName}); err != nil { - if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { - t.Skipf("needs CAP_NET_ADMIN to create veth: %v", err) - } - t.Fatal(err) - } - t.Cleanup(func() { - if link, err := netlink.LinkByName(hostName); err == nil { - if err := netlink.LinkDel(link); err != nil { - t.Errorf("deleting test IPv6 veth: %v", err) - } + const workerEnd, actorEnd = "atodw", "atoda" + if err := ateomnet.NetNSDo(context.Background(), workerNS, func(context.Context) error { + if err := netlink.LinkAdd(&netlink.Veth{ + LinkAttrs: netlink.LinkAttrs{Name: workerEnd}, + PeerName: actorEnd, + }); err != nil { + return fmt.Errorf("creating the veth: %w", err) } - }) - hostLink, err := netlink.LinkByName(hostName) - if err != nil { - t.Fatal(err) - } - prefix := uint16(os.Getpid()) - hostIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::1", prefix)) - actorIP = net.ParseIP(fmt.Sprintf("fd00:198:18:%x::2", prefix)) - // This isolated veth has no competing IPv6 peers. Suppress DAD so the - // address can be bound immediately instead of remaining tentative while - // the test is trying to start its listener. - if err := netlink.AddrAdd(hostLink, &netlink.Addr{IPNet: &net.IPNet{IP: hostIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { - t.Fatal(err) - } - if err := netlink.LinkSetUp(hostLink); err != nil { - t.Fatal(err) - } - peer, err := netlink.LinkByName(peerName) - if err != nil { - t.Fatal(err) - } - if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { - t.Fatal(err) - } - if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { - lo, err := netlink.LinkByName("lo") + peer, err := netlink.LinkByName(actorEnd) if err != nil { return err } - if err := netlink.LinkSetUp(lo); err != nil { - return err + if err := netlink.LinkSetNsFd(peer, int(actorNS)); err != nil { + return fmt.Errorf("moving the actor end into its namespace: %w", err) } - link, err := netlink.LinkByName(peerName) - if err != nil { - return err - } - if err := netlink.AddrAdd(link, &netlink.Addr{IPNet: &net.IPNet{IP: actorIP, Mask: net.CIDRMask(64, 128)}, Flags: unix.IFA_F_NODAD}); err != nil { - return err + return configureVethEnd(family, workerEnd, family.workerIP) + }); err != nil { + if errors.Is(err, unix.EPERM) { + t.Skipf("needs CAP_NET_ADMIN to create a veth: %v", err) } - return netlink.LinkSetUp(link) + t.Fatalf("wiring the worker end of the veth: %v", err) + } + if err := ateomnet.NetNSDo(context.Background(), actorNS, func(context.Context) error { + return configureVethEnd(family, actorEnd, family.actorIP) }); err != nil { - t.Fatal(err) + t.Fatalf("wiring the actor end of the veth: %v", err) } - return actorIP, hostIP } -func listenTCP6(t *testing.T, hostIP net.IP) net.Listener { - t.Helper() - listener, err := net.ListenTCP("tcp6", &net.TCPAddr{IP: hostIP, Port: 0}) +func configureVethEnd(family originalDstFamily, name, ip string) error { + link, err := netlink.LinkByName(name) if err != nil { - t.Fatal(err) + return err } - return listener + addr := &netlink.Addr{ + IPNet: &net.IPNet{IP: net.ParseIP(ip), Mask: family.mask}, + Flags: family.addrFlags, + } + if err := netlink.AddrAdd(link, addr); err != nil { + return fmt.Errorf("adding %s to %s: %w", ip, name, err) + } + return netlink.LinkSetUp(link) } -func installOriginalDstRedirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { +// listenInNetNS opens a listener inside ns. The socket stays bound to that +// namespace once created, so the caller can accept on it from wherever it +// happens to be running. +func listenInNetNS(t *testing.T, ns netns.NsHandle, family originalDstFamily) *net.TCPListener { t.Helper() - c := &nftables.Conn{} - c.AddTable(table) - chain := c.AddChain(&nftables.Chain{ - Name: "prerouting", - Table: table, - Type: nftables.ChainTypeNAT, - Hooknum: nftables.ChainHookPrerouting, - Priority: nftables.ChainPriorityNATDest, - }) - c.AddRule(&nftables.Rule{ - Table: table, - Chain: chain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, - // Restrict the rule to this test's actor so the temporary table cannot - // affect unrelated local TCP traffic. - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 12, Len: 4}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To4()}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, - &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, - &expr.Redir{RegisterProtoMin: 1}, - }, - }) - if err := c.Flush(); err != nil { - if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { - t.Skipf("needs CAP_NET_ADMIN to install nftables rule: %v", err) + var listener *net.TCPListener + if err := ateomnet.NetNSDo(context.Background(), ns, func(context.Context) error { + l, err := net.ListenTCP(family.listenNetwork, &net.TCPAddr{IP: net.ParseIP(family.listenIP)}) + if err != nil { + return err } - t.Fatalf("installing nftables redirect: %v", err) + listener = l + return nil + }); err != nil { + t.Fatalf("listening on %s %q: %v", family.listenNetwork, family.listenIP, err) } - t.Cleanup(func() { - cleanup := &nftables.Conn{} - cleanup.DelTable(table) - if err := cleanup.Flush(); err != nil { - t.Errorf("removing nftables redirect: %v", err) - } - }) + t.Cleanup(func() { _ = listener.Close() }) + return listener } -func installOriginalDstIPv6Redirect(t *testing.T, table *nftables.Table, actorIP net.IP, targetPort, redirectPort int) { +// installOriginalDstRedirect sends the actor's connections to targetPort on to +// redirectPort instead, the way a worker sends actor egress to atunnel. +func installOriginalDstRedirect(t *testing.T, family originalDstFamily, ns netns.NsHandle, targetPort, redirectPort int) { t.Helper() - c := &nftables.Conn{} - c.AddTable(table) + c, err := nftables.New(nftables.WithNetNSFd(int(ns))) + if err != nil { + t.Fatalf("opening nftables in the worker namespace: %v", err) + } + table := c.AddTable(&nftables.Table{Family: family.nftFamily, Name: "atunnel_original_dst_test"}) chain := c.AddChain(&nftables.Chain{ Name: "prerouting", Table: table, @@ -411,32 +392,18 @@ func installOriginalDstIPv6Redirect(t *testing.T, table *nftables.Table, actorIP Hooknum: nftables.ChainHookPrerouting, Priority: nftables.ChainPriorityNATDest, }) - c.AddRule(&nftables.Rule{ - Table: table, - Chain: chain, - Exprs: []expr.Any{ - &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: []byte{unix.IPPROTO_TCP}}, - // An IPv6 source address begins eight bytes into the IPv6 header. - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseNetworkHeader, Offset: 8, Len: 16}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: actorIP.To16()}, - &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, - &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, - &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, - &expr.Redir{RegisterProtoMin: 1}, - }, - }) + // Built from the same matchers as the production rule in + // ateomnet.ActorEgressRedirectRule, with a destination-port match added so + // the rule cannot fire again on the connection it just rewrote. + exprs := append(family.sourceEqual(family.actorIP), ateomnet.TCPProtocol()...) + exprs = append(exprs, + &expr.Payload{DestRegister: 1, Base: expr.PayloadBaseTransportHeader, Offset: 2, Len: 2}, + &expr.Cmp{Op: expr.CmpOpEq, Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(targetPort))}, + &expr.Immediate{Register: 1, Data: binaryutil.BigEndian.PutUint16(uint16(redirectPort))}, + &expr.Redir{RegisterProtoMin: 1}, + ) + c.AddRule(&nftables.Rule{Table: table, Chain: chain, Exprs: exprs}) if err := c.Flush(); err != nil { - if errors.Is(err, unix.EPERM) || strings.Contains(err.Error(), "operation not permitted") { - t.Skipf("needs CAP_NET_ADMIN to install IPv6 nftables rule: %v", err) - } - t.Fatalf("installing IPv6 nftables redirect: %v", err) + t.Fatalf("installing the %s redirect: %v", family.name, err) } - t.Cleanup(func() { - cleanup := &nftables.Conn{} - cleanup.DelTable(table) - if err := cleanup.Flush(); err != nil { - t.Errorf("removing IPv6 nftables redirect: %v", err) - } - }) }