Drop actor UDP egress to every port but DNS

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.

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.