hashgraph / hashgraph/solo-weaver

feat(network/firewall): block statusz-derived restricted CIDRs on the host input path

Open
#978 1 comment 0 reactions 1 assignee Claimed by @Dosik13 View on GitHub
Dominant language
Go
Stars
3
Forks
0
Avg merge
3d 2h
Merged PRs (30d)
46

Description

## Problem

The block node reports a `restricted` category via statusz, which the traffic-shaper daemon reconciles into the `bn-restricted` set every poll tick. That set lives only in `inet weaver-workload-policy`, whose chain is on the `forward` hook — so restricted peers are blocked from reaching **pods**, but nothing stops them from reaching the **host** itself. Traffic to host-level services (SSH, the kubelet/API ports in `@in_cluster_ports`, ICMP) is filtered by `inet weaver-host-firewall` on the `input` hook, which has never heard of the restricted list.

A peer the block node has declared restricted should be blocked on both paths. Today it is blocked on one.

The obvious move — folding restricted CIDRs into the host firewall's `@blocked_addrs` — collides with an invariant that is deliberate and documented in two places. `internal/network/firewall/table.go:31-37`:

```go
// BlockedCIDRs is the operator-curated deny list (set @blocked_addrs). It is
// purely operator-managed for its whole lifecycle — nothing in this package
// or the daemon ever writes to it automatically. This is deliberately
// distinct from the BN workload plane's `bn-restricted` set (`inet weaver-workload-policy`),
// which the traffic-shaper daemon reconciles from the block node's statusz
// "restricted" category; an operator block list needs a home the daemon
// never overwrites.
```

and `internal/workflows/steps/step_network_policy.go:68-73` makes the same point from the other side ("A permanent, purely operator-managed block list lives instead on the host firewall … a different table entirely").

Merging the two into one set would mean two writers with different lifecycles on one set: the operator via `network firewall --blocked-cidrs`, which re-renders the whole table destructively (`add table` / `delete table` / `add table`, tmpl:1-3), and the daemon via incremental element ops every poll tick. An operator re-render would silently wipe the statusz-derived entries, and — because `internal/network/firewall/parse.go` recovers prior state by parsing elements back out of the on-disk file — daemon-added entries would either be lost or get promoted into the operator's persisted list. There would also be no provenance left to answer "which of these do I remove when statusz stops reporting it?"

## Proposed fix

Keep the invariant; add a second, daemon-owned set rather than merging.

Declare `restricted_addrs` / `restricted_addrs6` in the host firewall template with **no elements** — the same pattern the policy table already uses for daemon-managed port sets (`internal/network/policy/render.go:161`, `set %s { type inet_service; }`). Because the template never renders elements for them, `Parse` stays unaffected and the operator's `@blocked_addrs` round-trip is untouched. Add a drop rule for them adjacent to the existing blocklist drops, above the conntrack fast-path so that restricting a peer also tears down its already-open connections — matching the behavior `@blocked_addrs` already has (tmpl:18-21).

Two gaps to close on the way:

**The firewall package has no write path.** Its `Runner` is `List` / `Delete` / `Exists` only, and the interface comment at `internal/network/firewall/nft.go:15-19` states the reason: live application happens by writing the on-disk artifact and restarting the systemd oneshot, so there is no `Apply`. Filling a set at runtime needs element operations on this table — mirroring `AddElements` / `DeleteElements` / `SetElements` / `ListElements` in `internal/network/policy/nft.go:91-164`.

**Re-render wipes the set.** The host table's apply is destructive (`delete table` then `add table`), so any operator-triggered re-render clears daemon-populated elements. The policy package already solves exactly this with `snapshotMembership` / `restoreSet` around the re-apply (`internal/network/policy/manager.go:252-291`); the host firewall needs the equivalent, or an explicit decision that the set simply repopulates on the next poll tick and a window of non-enforcement is acceptable.

On the daemon side, the reconciler currently writes only through `r.applier.ApplySets` into the policy manager (`internal/blocknode/shaper/reconciler.go:161`). Feeding a second table means either a second applier or widening that seam to carry a destination. The `(Inbound, CategoryRestricted)` binding at `internal/blocknode/shaper/policy_map.go:81` is the source to fan out from.

This is additive to the existing `bn-restricted` deny in the workload policy table, not a replacement — different hooks, different traffic. Both should reflect the same statusz membership.

## Acceptance

- [ ] `@blocked_addrs` / `@blocked_addrs6` remain purely operator-managed; nothing in the daemon writes to them
- [ ] Host firewall renders daemon-owned `restricted_addrs` / `restricted_addrs6` sets, declared without elements
- [ ] A CIDR in the restricted set is dropped on the host `input` path, and the drop terminates already-established connections from it
- [ ] Restricted membership is reconciled from statusz on every poll tick, and entries are removed when statusz stops reporting them
- [ ] The workload policy table's `bn-restricted` deny continues to work unchanged
- [ ] `network firewall --blocked-cidrs` re-render does not permanently lose daemon-populated restricted entries
- [ ] `internal/network/firewall/parse.go` round-trip is unaffected — restricted elements never land in the operator's persisted list
- [ ] `internal/network/firewall/testdata/network-host.golden.nft` regenerated

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.