feat(network): split the workload forward chain by family and enforce the block list on every path

[brunodam]

Description

The inet weaver-workload-policy forward chain was one flat list where every deny policy emitted four rules and every stamp policy emitted one per address family. On a dual-stack host running the canonical BN set, roughly half the expressions evaluated per forwarded packet were guaranteed non-matches.

The hooked chain is now a single meta nfproto dispatch into forward_ipv4 / forward_ipv6, with every rule moved into the family chain it can actually match. Sibling of #979, which did the same to the host firewall's input chain.

meta nfproto rather than meta protocol: in an inet table it reads the netfilter protocol family straight off the hook instead of depending on the skb's ethertype, which is not guaranteed for locally generated traffic or non-Ethernet interfaces.

Security-posture change — please review this deliberately

The chain policy flips from drop to accept, and both ct state matches, the terminal drop, and the unclassified-pod-egress tier all go away. The table stops being an enforcement point on the forward hook and becomes purely a traffic classifier. Workload isolation rests entirely on Cilium from here — including on a host whose Cilium datapath is degraded or not yet up, which previously still had weaver's policy drop as a backstop.

The issue asked for this to be signed off as a posture change rather than merged as a perf tweak. Flagging it accordingly rather than burying it in the diff.

What does not change:

• bn-restricted still quarantines, both directions and both families. It drops explicitly; it never relied on the chain policy.
• ct mark set/restore is retained. Tier 2 is what classifies asymmetric backfill replies — dropping conntrack from the table wholesale would leave them unclassified in the HTB default class.
• Unclassified traffic lands where it always did. The removed tier carried no meta priority, and neither does falling through to policy accept, so both end in the HTB default class. That equivalence is what makes the tier dead rather than load-bearing.

Why the reply restore is duplicated rather than hoisted

Tier 2 (ct direction reply ct mark …) matches conntrack only, so it is family-agnostic and looks like it belongs in the base chain above the dispatch. It does not. It ends in accept, which is terminal for the base chain — a quarantined peer's reply traffic would sail past the tier-1 deny drops that now sit below the dispatch.

It is emitted into both family chains instead: one extra line in the document, zero extra evaluations per packet, ordering preserved exactly. TestRender_FamilySplit pins it in both chains, and the reasoning is in the rendered comment so nobody "optimizes" it upward later.

Rationales this change invalidated

Three places asserted things that stop being true, each corrected here rather than left to rot:

• docs/dev/traffic-shaper.md (from docs(traffic-shaper): document coexistence with the host's existing nftables and tc stack #984) said "Both weaver chains are policy drop … That makes weaver the binding filter on the node." Now true on input only. The section is rewritten as a four-row table covering every hook weaver registers on, and says plainly that forward constrains nothing.
• The empty-registry file removal was justified as a blackhole guard (an empty policy drop chain would kill pod startup, image pulls, DNS). Under policy accept an empty chain is inert. The behavior is kept — never replaying stale classification is still right — but it is hygiene now, not safety. Same rationale appeared in four places.
• The Delete path's "an empty chain (policy drop, no rules) is applied" was already wrong before this PR: manager.go:726-743 tears the table down entirely. Fixed in the doc comment, the CLI help, and docs/quickstart.md (per the repo rule that CLI surface changes must update quickstart), because leaving a policy drop claim in operator-facing help after this change would be doubly misleading.

Accepted regression

ExtractPodCIDRs recovers the pod CIDR from inline ip saddr <CIDR> literals so a --deny create — which never supplies --pod-cidr — can re-render stamp siblings. The removed tier was the only rule putting an inline CIDR into a deny-only document, so this sequence loses recovery:

stamps exist → all stamps deleted → a new stamp is created without --pod-cidr

Post-change that create fails with the existing explicit "pod CIDR is required to render a --stamp policy" error, not silently, and every production caller supplies the value. Accepted rather than adding a recovery mechanism for a path that surfaces loudly.

Files changed

File	Change
internal/network/policy/render.go	Base chain reduced to policy accept + the nfproto dispatch; new renderFamilyChain and family; tier 5, both ct state rules and the terminal drop deleted
internal/network/policy/policy_test.go	chainBody helper; ordering assertions rescoped per chain; new TestRender_FamilySplit and single-stack empty-chain test
internal/network/policy/testdata/*.golden.nft	Regenerated (both)
internal/network/policy/manager.go	Delete doc comment and inline rationale
internal/network/policy/render_weaver.go	Empty-registry rationale
internal/network/policy/{render_weaver,manager_ops}_test.go	Same, in test comments
internal/workflows/steps/step_network_{nft_weaver,policy}.go	Same
cmd/cli/commands/network/policy/delete.go, docs/quickstart.md	CLI help fix + required quickstart counterpart
docs/dev/traffic-shaper.md	Coexistence section: input enforces, forward classifies

internal/network/policy/parse.go is deliberately untouched — its regexes scan the whole document rather than anchoring to chain position, so they survive the layout change.

Review guide

Document order is no longer evaluation order. The family chains are defined below the hooked chain but run before it falls through, via the dispatch. Any whole-document strings.Index ordering assertion is now meaningless — it compares positions in chains that never evaluate the same packet. Hence the chainBody helper (same shape as the one #979 added on the firewall side); scope every new ordering assertion to one chain body.

Invariants to verify, each of which fails silently — the ruleset still loads:

• The quarantine drops lead each family chain. Every rule below them ends in accept, so a deny that sorted lower would let a restricted peer's traffic be stamped and accepted.
• The reply restore precedes the classification tiers. On the reply the addresses are reversed, so a broad fallthrough rule would otherwise claim the packet and stamp it with the forward class instead of the reply class.
• The reply restore appears in both family chains and not in the hooked chain.
• The hooked chain holds only the policy line and the dispatch — anything that creeps in there is evaluated for every forwarded packet. TestRender_FamilySplit counts the rules.
• Specific classification still precedes fallthrough within each family.
• No rule in a family chain matches the other family.

Test commands

go test ./internal/network/policy/...

# Linux-only packages (internal/mount does not build on macOS)
GOOS=linux GOARCH=arm64 go test -c -tags='!integration' -o /tmp/steps.test ./internal/workflows/steps/
docker run --rm -v /tmp:/t debian:stable-slim /t/steps.test

nft validation against a real kernel. Unit tests only compare strings and cannot tell you whether nft accepts the ruleset, so both goldens — plus the single-stack case that renders an empty forward_ipv6 — were loaded on nftables v1.1.3. All three load, re-apply idempotently, and every jump resolves:

docker run --rm --privileged -v <dir>:/t debian:stable-slim sh -c \
  'apt-get update -qq && apt-get install -y -qq nftables && \
   nft -f /t/dual.nft && nft -f /t/dual.nft && nft list table inet weaver-workload-policy'

The kernel normalizes priority 0 → priority filter and prints meta priority 0x10020 back as 1:20; both are nft's listing behavior, not drift.

The empty-chain case is worth knowing about: stamp-only policies on a single-stack host leave the absent family with no rules at all, but the dispatch jumps to both families unconditionally and an unresolved jump fails the entire load. So the chain must still be emitted. It renders as chain forward_ipv6 { }, and TestRender_SingleStackEmitsAnEmptyChainForTheAbsentFamily pins it.

Manual UAT

Setup — one command. Builds both binaries, self-installs the provisioner on the BN VM, clones the peers VM with five distinct un-NAT'd source IPs, seeds the statusz roster from them, and deploys the block node with shaping and firewall enabled:

task network:all
task network:ip     # publisher / partner / public / backfill / restricted addresses
task vm:ssh         # BN VM
task network:ssh    # peers VM

1. The chains exist and the dispatch resolves

sudo nft list table inet weaver-workload-policy | grep -E 'chain |jump'

	chain forward {
		meta nfproto vmap { ipv4 : jump forward_ipv4, ipv6 : jump forward_ipv6 }
	chain forward_ipv4 {
	chain forward_ipv6 {

An unresolved jump fails the load outright, so reaching this output is itself the check. The hooked chain must hold exactly those two lines — no ct state, no trailing drop:

sudo nft -a list chain inet weaver-workload-policy forward

On a single-stack node the absent family must carry the deny tier and nothing else. A ct direction reply line there means the reply-restore gate regressed:

sudo nft list chain inet weaver-workload-policy forward_ipv6

2. Resolve the two shaped devices

The six classes are split across two devices, which is the easiest thing to get wrong when reading counters:

for d in $(ls /sys/class/net); do
  tc qdisc show dev "$d" 2>/dev/null | grep -q 'qdisc htb 1:' && echo "$d"
done

Expect two: the --egress-interface NIC (egress classes partner 1:40, public 1:50, reserve-egress 1:60) and the block node pod's host-side veth (ingress classes publisher 1:10, backfill-response 1:20, reserve-ingress 1:30). The ingress hierarchy is attached per-pod by the daemon, so it appears only once a block node pod is running. Looking for ingress classes on the physical NIC shows them permanently at zero and reads as a false regression.

3. Traffic lands in the class it should

Read the live port sets first — they are reconciled from statusz, not literals:

for s in bn-publisher bn-subscriber-in bn-partner-out bn-public-out bn-mgmt-in bn-mgmt-out bn-backfill; do
  printf '%s: ' "$s"
  sudo nft list set inet weaver-workload-policy "${s}_ports" 2>/dev/null \
    | sed -n 's/.*elements = {\(.*\)}.*/\1/p'
done

Watch the counters live on the BN VM, one terminal per direction:

solo-provisioner network shape watch --device egress  --iface enp0s1 --interval 1
solo-provisioner network shape watch --device ingress --iface <veth from step 2> --interval 1

Create a debug container sharing the block node pod's network namespace — classification matches the pod's addresses, so traffic must originate there:

kubectl debug pod/block-node-block-node-server-0 -n block-node \
  --image=networkstatic/iperf3 --target=block-node-server \
  --profile=general --attach=false -- sh -c 'sleep 3600'

kubectl exec -it block-node-block-node-server-0 -n block-node \
  -c debugger-<suffix> -- /bin/sh

Egress classes. Start a listener on the peer holding the matching role address, then drive the flow from the pod with --cport set to a port in that policy's set — the egress rules match tcp sport, so the client port is what selects the class:

# on the peers VM, bound to the role's address
iperf3 -s -B <role-ip>

# in the debug container
iperf3 -c <role-ip> -p 5201 --cport <port from that policy's set> -t 30 -i 1

Role address	Policy	--cport from	Expected class
$PARTNER_IP	bn-partner-out	bn-partner-out_ports	1:40
$PUBLIC_IP	bn-public-out	bn-public-out_ports	1:50
$BACKFILL_IP	bn-backfill	bn-backfill (compound set)	1:60 fwd, 1:20 on the reply
any unlisted address	—	any unlisted port	default class 1:60

Ingress classes. Drive from the peer toward the block node's real listening ports, with the source address selecting the policy:

# on the peers VM
iperf3 -c <bn-address> -p <port from bn-publisher_ports> -B $PUBLISHER_IP -t 30 -i 1

The block node owns those ports, so iperf3 will not complete a handshake — the byte volume is small and the check is that the counter moves on the expected class at all, not throughput. --class narrows the watch when a row is ambiguous:

solo-provisioner network shape watch --device ingress --iface <veth> --class publisher --interval 1

Each generator must move its own class and no other. Traffic collapsing into the default class means the stamp rules stopped matching — the most likely regression from moving rules into the family chains.

4. Quarantine still drops, both directions and both families

sudo solo-provisioner network policy add --name bn-restricted --cidr $RESTRICTED_IP/32

From the peers VM, iperf3 -c <bn-address> -B $RESTRICTED_IP must fail outright, and the drops must be the first rules in each family chain. An already-open connection from that peer dies immediately — there is no conntrack fast-path left to shield it.

5. Survives a reboot

sudo reboot
systemctl status solo-provisioner-network-nft.service
sudo nft list table inet weaver-workload-policy | grep -c 'chain '

Unit active (exited), chain count 3.

Membership sets come back empty and stay so until the daemon's first successful statusz poll — pre-existing behavior, not introduced here, tracked by #990.

Risks / rollback. The posture change is the risk; see above. No startup migration re-renders this template — the only re-render paths are step_network_policy.go and the network policy CLI verbs — so the new layout cannot leak silently into already-provisioned clusters, and old and new layouts coexist across the fleet until each node is explicitly re-rendered. That is safe because the artifact is replaced wholesale rather than patched. Rollback is a straight revert plus re-apply; there is no migration to undo.

Block list on every path (inet weaver-host-firewall)

Second commit, second table. @blocked_addrs was spelled once, on the input hook — which does not block a peer, it blocks the peer from initiating. Two paths stayed open:

• The host could still dial a blocked address. There was no output chain, so outbound traffic to a blocked CIDR left normally — and the replies came back in under the input chain's ct state established accept, because they are not a new connection. For any host-initiated connection, the block list had no effect in either direction.
• Forwarded, pod-bound traffic from a blocked source was never filtered. The host firewall has no forward chain, and the workload policy table only drops @bn-restricted.

Two base chains close both:

chain prerouting_blocklist {
    type filter hook prerouting priority -300; policy accept;
    ip saddr @blocked_addrs drop
    ip6 saddr @blocked_addrs6 drop
}

chain output {
    type filter hook output priority 0; policy accept;
    ip daddr @blocked_addrs drop
    ip6 daddr @blocked_addrs6 drop
}

-300 is the raw band, ahead of conntrack at -200, so a blocked source never gets a tuple lookup. That hook also covers the forward path, which is what closes the pod-bound gap.

The output chain is deliberately not an egress allowlist and must stay policy accept. Enumerating legitimate outbound traffic on a Kubernetes node — kubelet to the API server, etcd, DNS, NTP, image pulls from arbitrary registries, Cilium, Teleport — is large, brittle, and stranding the node is the failure mode. TestRender_BlockListReachesEveryPath asserts the chain carries block-list rules only, so it cannot drift into one.

Scope change — please read

@blocked_addrs now filters pod-bound forwarded traffic, which the host firewall has never done. The block list starts meaning "blocked on this node" rather than "blocked from the host's own services". That blurs the host/workload table split described as deliberate in internal/network/firewall/table.go:31-37 and internal/workflows/steps/step_network_policy.go:68-73; both comments, the --blocked-cidrs flag help, docs/quickstart.md, and docs/dev/traffic-shaper.md are reworded to match.

The input copy of the drop is kept. It is redundant with prerouting for anything arriving on a wire, but the ordering it encodes — block list ahead of the conntrack fast-path, so an added CIDR also kills already-open connections — is a tested property of the input chain and should not become contingent on a chain registered on another hook.

Verified against a real kernel, not by inspection

nftables v1.1.3. The table loads, re-applies idempotently, and all three base chains register at the expected hooks (nft normalizes -300 to raw):

type filter hook prerouting priority raw; policy accept;
type filter hook input priority filter; policy drop;
type filter hook output priority filter; policy accept;

The drops actually fire, which is the part string tests cannot reach:

# host dials a blocked destination
ip daddr @blocked_addrs counter packets 1 bytes 34 drop     # output chain

# packet from a blocked source arrives over a veth pair
ip saddr @blocked_addrs counter packets 1 bytes 34 drop     # prerouting_blocklist
ip saddr @blocked_addrs counter packets 0 bytes 0  drop     # input — never reached

That second pair is also the evidence for the redundancy note above: prerouting takes the packet, input never sees it.

Conntrack was checked separately, because the intuition that a drop leaves a stale entry is wrong: entries are only confirmed at the end of the pipeline, so a packet dropped at filter priority 0 never lands one. Same UDP payload to two destinations, counters proving both traversed the chain — accepted destination left 1 entry, dropped destination left 0. The prerouting move therefore saves per-packet CPU, not table growth.

Manual UAT — block list

# 1. Three base chains registered
sudo nft list table inet weaver-host-firewall | grep 'type filter hook'
# expect prerouting/raw (policy accept), input/filter (policy drop), output/filter (policy accept)

# 2. Block a peer you can reach
sudo solo-provisioner network firewall add --blocked-cidr <peer>/32

# 3. Outbound is now dead — this is the gap that was open
nc -zv -w5 <peer> 22          # expect timeout
ping -c3 <peer>               # expect 100% loss

# 4. An already-open connection to that peer dies immediately
# 5. From a pod, confirm the blocked peer is unreachable (forward path)
kubectl run uat --rm -it --restart=Never --image=nicolaka/netshoot -- nc -zv -w5 <peer> 443

# 6. Restore
sudo solo-provisioner network firewall remove --blocked-cidr <peer>/32

Step 3 is the regression check. Before this change the connection would establish, because only inbound initiation was blocked.

Test harness fix (taskfiles/network.yaml, scripts/network/provision-peers.sh)

Unrelated to the nft work, but it rides along because it is what makes the UAT above runnable at all.

network:up brought up no macvlan source IPs and no iperf3, yet still wrote a hosts.env listing five peer addresses — so every downstream task ran against addresses that did not exist, and a classification failure looked like a shaper bug rather than a harness that never armed.

The peers VM has no passwordless sudo. vm.yaml installs that drop-in only on the BN VM and the golden image does not carry one, so each sudo in the macvlan setup blocked on a password prompt non-interactive SSH cannot answer. The failures were swallowed twice: 2>/dev/null || true on the link add, and a trailing || echo on the whole block.

• provision-peers.sh installs the sudoers drop-in and verifies it with sudo -n over a real login — the same verify-by-effect the SSH-key step already uses, for the same reason.
• It also pins arp_ignore=1 / arp_announce=2 before any child exists. All six addresses share one /24, and at the default arp_ignore=0 any interface answers ARP for any local address; the upstream switch then binds the parent's management IP to a child's MAC and the peers VM drops off mid-run.
• network:up fails hard rather than warning: an unresolved NIC, a missing iperf3, or an address that did not land now stops the run before hosts.env is written. iperf3 installs once instead of five times inside the loop.

Verified on the UTM harness: all five macvlan source IPs come up and each reaches the BN VM from its own address. That also settles the macvlan-over-UTM question the file flagged as its load-bearing unknown — the segment does forward the child MACs, so the five-tiny-VMs fallback is not needed.

Follow-up worth doing separately: the macvlan layer exists to give each peer its own MAC, but nothing in the classification path matches a MAC — every rule matches ip saddr/ip daddr. Five secondary addresses on the primary NIC would give the same five source IPs and delete the child-MAC question, the denyinterfaces handling, and the ARP tuning outright.

Related Issues

• Closes perf(network/policy): split the forward chain by address family, default to accept, and drop the ct state matches #977
• Closes feat(network/firewall): enforce the operator block list on every path, not just inbound #991

Sibling of #979 (input chain). #978 builds on the host-firewall side and should land after #979.

[swirlds-automation]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[Copilot]

Pull request overview

This PR restructures the inet weaver-workload-policy nftables ruleset used on the forward hook, splitting evaluation by address family and changing the hooked chain from an enforcement point (policy drop) into a classifier (policy accept) that primarily stamps meta priority for the HTB hierarchy.

Changes:

• Replaces the single flat forward chain with a minimal hooked chain that dispatches via meta nfproto into forward_ipv4 / forward_ipv6, moving rules into their matching family chain.
• Removes conntrack state gating (ct state ...) and the terminal drop, eliminating the “default drop + escape hatches” structure and making forward classification-only (explicit bn-restricted drops remain).
• Updates tests, goldens, CLI help text, and operator docs to reflect the new chain layout and the deliberate security posture change.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
internal/workflows/steps/step_network_policy.go	Updates step comment to reflect that an empty registry means no persisted nft file.
internal/workflows/steps/step_network_nft_weaver.go	Updates rationale comment around removing stale persisted policy file on empty registry.
internal/network/policy/render.go	Implements family split (forward_ipv4/forward_ipv6), minimal base chain dispatch, and removes enforcement-era tiers (ct state, terminal drop, unclassified tier).
internal/network/policy/render_weaver.go	Updates empty-registry behavior rationale to “hygiene not blackhole guard” under policy accept.
internal/network/policy/render_weaver_test.go	Updates empty-registry test comments to match new semantics.
internal/network/policy/policy_test.go	Adds chainBody helper and new tests/assertions for family split structure, ordering, and single-stack empty-chain emission.
internal/network/policy/manager.go	Updates Delete doc/comments to reflect last-policy behavior (tear down table entirely).
internal/network/policy/manager_ops_test.go	Adjusts test comment for last-policy delete behavior under new semantics.
internal/network/policy/testdata/network-weaver-workload-policy.golden.nft	Regenerates golden to reflect base dispatch chain and per-family chains.
internal/network/policy/testdata/network-weaver-workload-policy-dualstack.golden.nft	Regenerates dual-stack golden with per-family chain separation and new policy semantics.
cmd/cli/commands/network/policy/delete.go	Updates CLI help text to match actual last-policy delete behavior (tear down table).
docs/quickstart.md	Updates operator-facing quickstart text for network policy delete to match new semantics.
docs/dev/traffic-shaper.md	Rewrites coexistence section to clearly distinguish input (enforcing) vs forward (classifying).

Suppressed comments (2)

internal/network/policy/render.go:400

• renderStampRule()’s comment says that when a family has no pod CIDR, “the other family's chain still carries the deny tier”. With the current implementation, the other family chain may also still carry tier-2 reply-restore rules (which are pod-CIDR-independent). Updating the comment to reflect that only stamp classification rules are suppressed would keep the docs accurate.

// renderStampRule renders one address family's classification rule for a stamp
// policy, honoring --from-entity world (no IP-set clause) and --reply-stamp
// (compound-key egress forward rule with a ct mark write). It returns "" when
// this family has no pod CIDR, so a single-stack deployment renders only its own
// family's rules while the other family's chain still carries the deny tier.
func renderStampRule(p *Policy, f family) (string, error) {
	if f.podCIDR == "" {
		return "", nil
	}

internal/network/policy/policy_test.go:195

• This test’s doc comment says the deny tier is “the only tier rendered for a family without a pod CIDR”, but tier 2 (reply-restore) is rendered independent of pod CIDR. The current fixture happens not to include reply-stamp policies, but the comment is inaccurate and could mislead future changes.

// TestRender_SingleStackEmitsAnEmptyChainForTheAbsentFamily covers the one input
// that produces a chain with no rules at all: stamp-only policies (no deny tier,
// which is the only tier rendered for a family without a pod CIDR) on a
// single-stack deployment. The chain must still exist, because the hooked chain's
// vmap jumps to it unconditionally and an unresolved jump fails the whole load.
func TestRender_SingleStackEmitsAnEmptyChainForTheAbsentFamily(t *testing.T) {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[alex-au]

Nice work!