From 8377adcb94ac44b5e27b3a94a39beef4f62a0aca Mon Sep 17 00:00:00 2001 From: Haiyan Meng Date: Fri, 11 Sep 2026 16:33:53 -0400 Subject: [PATCH] Drop actor UDP egress to every port but DNS Only TCP is redirected into atunnel, so UDP left the worker pod through the compatibility masquerade with no CONNECT authority, no access log, and no policy hook. QUIC on 443 is the case that matters: an actor could make its own HTTPS egress unintercepted just by speaking HTTP/3, with no timing trick needed. Add a forward-chain rule ahead of the catch-all accept that drops UDP from the actor veth to any destination port but 53, keeping the DNS exception the masquerade exists for. The rule counts what it drops, so a workload that legitimately needs UDP surfaces in `nft list table ip ateom_actor` rather than as an unexplained timeout. The rule is installed whether or not an egress gateway is configured: unlike the redirect it needs nothing to redirect to, and a deployment with no egress control is where unrestricted UDP is worst. The existing TODO narrows rather than goes away. The DNS exception is still any port-53 destination rather than the configured cluster resolver, and protocols other than TCP and UDP still reach the masquerade. --- internal/ateomnet/net.go | 49 ++++++++++--- internal/ateomnet/net_linux_test.go | 107 ++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 9 deletions(-) diff --git a/internal/ateomnet/net.go b/internal/ateomnet/net.go index 91203a8e04..ee0902fb88 100644 --- a/internal/ateomnet/net.go +++ b/internal/ateomnet/net.go @@ -238,10 +238,8 @@ func InstallActorNftablesRules(egressPort uint16) error { // listener. REDIRECT preserves SO_ORIGINAL_DST for the CONNECT authority. // * postrouting: masquerade traffic not handled by the TCP tunnel, notably // DNS over UDP, so hostname resolution continues to work. - // * forward: accept forwarded packets between the actor veth and pod eth0. - // - // TODO: Restrict the compatibility masquerade to DNS traffic sent to the - // configured cluster resolver and drop all other non-tunneled actor egress. + // * forward: drop actor UDP egress to any port but DNS, and accept the rest + // of the packets forwarded between the actor veth and pod eth0. if err := RemoveActorNftablesRules(); err != nil { return err } @@ -286,6 +284,9 @@ func InstallActorNftablesRules(egressPort uint16) error { Priority: nftables.ChainPriorityFilter, Policy: &acceptPolicy, }) + // Order matters: the accept below is a catch-all, so the drop has to precede + // it. + c.AddRule(actorNonDNSUDPDropRule(table, forward)) c.AddRule(&nftables.Rule{ Table: table, Chain: forward, @@ -343,13 +344,13 @@ func IPPayloadEqual(offset uint32, ip string) []expr.Any { } } -func TCPProtocol() []expr.Any { +func l4ProtocolEqual(proto byte) []expr.Any { return []expr.Any{ &expr.Meta{Key: expr.MetaKeyL4PROTO, Register: 1}, &expr.Cmp{ Op: expr.CmpOpEq, Register: 1, - Data: []byte{unix.IPPROTO_TCP}, + Data: []byte{proto}, }, } } @@ -361,7 +362,7 @@ func ActorEgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port if port == 0 { return nil } - exprs := append(IPSourceEqual(ActorVethIP), TCPProtocol()...) + exprs := append(IPSourceEqual(ActorVethIP), l4ProtocolEqual(unix.IPPROTO_TCP)...) exprs = append(exprs, &expr.Immediate{ Register: 1, @@ -372,6 +373,36 @@ func ActorEgressRedirectRule(table *nftables.Table, chain *nftables.Chain, port return &nftables.Rule{Table: table, Chain: chain, Exprs: exprs} } +// actorNonDNSUDPDropRule returns the forward-chain rule that drops actor UDP +// egress to every destination port but [dnsPort]. +// +// The rule counts what it drops: a workload that legitimately needs UDP shows +// up as a rising counter in `nft list table ip ateom_actor` rather than as an +// unexplained timeout. +func actorNonDNSUDPDropRule(table *nftables.Table, chain *nftables.Chain) *nftables.Rule { + // dnsPort is the only destination port on which actor UDP egress is forwarded. + const dnsPort = 53 + + exprs := append(IPSourceEqual(ActorVethIP), l4ProtocolEqual(unix.IPPROTO_UDP)...) + exprs = append(exprs, + // Destination port, at offset 2 of the UDP header. + &expr.Payload{ + DestRegister: 1, + Base: expr.PayloadBaseTransportHeader, + Offset: 2, + Len: 2, + }, + &expr.Cmp{ + Op: expr.CmpOpNeq, + Register: 1, + Data: binaryutil.BigEndian.PutUint16(dnsPort), + }, + &expr.Counter{}, + &expr.Verdict{Kind: expr.VerdictDrop}, + ) + 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) { @@ -491,8 +522,8 @@ func SetupActorNetwork(ctx context.Context, cfg NetworkConfig) (retErr error) { // Kubernetes-provided eth0 out of the worker pod. // // The nftables rules installed here redirect actor TCP egress to atunnel - // when configured and masquerade traffic the TCP tunnel does not handle - // (notably DNS over UDP). + // when configured, masquerade traffic the TCP tunnel does not handle + // (notably DNS over UDP), and drop actor UDP egress to any other port. // // Clean up stale state from a failed prior activation before creating the // next actor-side network. The worker currently runs one actor at a time. diff --git a/internal/ateomnet/net_linux_test.go b/internal/ateomnet/net_linux_test.go index b9c8ac45f4..36588db5b7 100644 --- a/internal/ateomnet/net_linux_test.go +++ b/internal/ateomnet/net_linux_test.go @@ -19,11 +19,13 @@ package ateomnet import ( "context" "errors" + "net" "runtime" "testing" "github.com/agent-substrate/substrate/internal/roottest" "github.com/google/nftables" + "github.com/google/nftables/expr" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" ) @@ -239,6 +241,111 @@ func TestSetupActorNetworkIsRepeatable(t *testing.T) { }) } +// addForwardingTarget gives the pod netns somewhere to forward actor packets +// to, standing in for the real pod's eth0 and default route. Without it a +// forwarded packet is dropped for want of a route before it ever reaches the +// forward hook the rules under test live on. The device is a dummy, so the +// packets go nowhere after that, which is all the assertions need. +func addForwardingTarget(t *testing.T, cidr string) { + t.Helper() + link := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "target0"}} + if err := netlink.LinkAdd(link); err != nil { + t.Fatalf("creating the forwarding target link: %v", err) + } + if err := netlink.AddrAdd(link, MustParseAddr(cidr)); err != nil { + t.Fatalf("addressing the forwarding target link: %v", err) + } + if err := netlink.LinkSetUp(link); err != nil { + t.Fatalf("bringing up the forwarding target link: %v", err) + } +} + +// droppedUDPPackets reads the packet count off the forward chain's counted +// rule, which [actorNonDNSUDPDropRule] is. +func droppedUDPPackets(t *testing.T) uint64 { + t.Helper() + c := &nftables.Conn{} + tables, err := c.ListTablesOfFamily(nftables.TableFamilyIPv4) + if err != nil { + t.Fatalf("listing nftables tables: %v", err) + } + for _, table := range tables { + if table.Name != ActorNftTableName { + continue + } + rules, err := c.GetRules(table, &nftables.Chain{Name: "forward", Table: table}) + if err != nil { + t.Fatalf("listing forward chain rules: %v", err) + } + for _, rule := range rules { + for _, e := range rule.Exprs { + if counter, ok := e.(*expr.Counter); ok { + return counter.Packets + } + } + } + t.Fatalf("forward chain has no counted rule, got %d rules", len(rules)) + } + t.Fatalf("nftables table %q is missing", ActorNftTableName) + return 0 +} + +// sendUDP sends one datagram to addr and reports whether the local send +// succeeded. UDP has no acknowledgement, so a successful send says nothing +// about delivery -- the drop is observed through the nftables counter instead. +func sendUDP(t *testing.T, addr string) { + t.Helper() + conn, err := net.Dial("udp4", addr) + if err != nil { + t.Fatalf("dialing %s: %v", addr, err) + } + defer conn.Close() + if _, err := conn.Write([]byte("probe")); err != nil { + t.Fatalf("sending a datagram to %s: %v", addr, err) + } +} + +// TestActorNonDNSUDPIsDropped covers the forward-chain rule behaviorally: only +// TCP is redirected into atunnel, so UDP on any port but 53 must not reach the +// masquerade, and DNS must still get through or the sandbox cannot resolve +// anything. +func TestActorNonDNSUDPIsDropped(t *testing.T) { + roottest.Require(t, "creating network namespaces, veth pairs, and nftables rules") + ctx := context.Background() + + withTestNetNS(t, func(interior netns.NsHandle) { + requireNftables(t) + + const target = "192.0.2.1" + addForwardingTarget(t, "192.0.2.254/24") + if err := SetupActorNetwork(ctx, NetworkConfig{InteriorNetNS: interior}); err != nil { + t.Fatalf("SetupActorNetwork: %v", err) + } + + before := droppedUDPPackets(t) + if err := NetNSDo(ctx, interior, func(context.Context) error { + sendUDP(t, net.JoinHostPort(target, "53")) + return nil + }); err != nil { + t.Fatalf("sending DNS from the interior netns: %v", err) + } + if got := droppedUDPPackets(t); got != before { + t.Errorf("DNS datagram was dropped: counter went from %d to %d", before, got) + } + + if err := NetNSDo(ctx, interior, func(context.Context) error { + sendUDP(t, net.JoinHostPort(target, "443")) + sendUDP(t, net.JoinHostPort(target, "9999")) + return nil + }); err != nil { + t.Fatalf("sending non-DNS UDP from the interior netns: %v", err) + } + if got := droppedUDPPackets(t); got != before+2 { + t.Errorf("dropped packets = %d, want %d: non-DNS UDP reached the masquerade", got, before+2) + } + }) +} + // 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.