feat(instance): per-instance identity + placeholder-next-hop guard + router-id defaulting #234

Merged
vxfemboy merged 4 commits from fix/nft-table-name-and-rtbh-nexthop-guard into main 2026-08-01 03:35:34 +00:00
Owner

Why

Two arming defects surfaced during the 2026-07-31 box work.

1. A second blackwalld instance can't coexist with the first. Covering a second ingress path (e.g. a separate IX-peering NIC) with its own daemon was blocked: both instances share three global kernel resources and clobber each other — the nft table (every apply does add+flush, so they flush-wipe each other's rules), the deception TPROXY fwmark, and its policy route table (teardown runs ip rule del + ip route flush, so one stopping rips out the other's plumbing).

2. Arming with a placeholder blackhole next-hop fails silently. An rtbh next-hop left at a documentation/discard template value (192.0.2.1, 100::1, …) never resolves to a real discard route, so BIRD leaves the /32 unreachable and never exports it — the daemon shows Established yet announces nothing.

What

instance=<name> (optional top-level directive) → blackwall_core::InstanceIds derives all three resources per-instance: table blackwall_<name>, and a fwmark + route-table id taken from independent 32-bit halves of a 64-bit hash of the name (mark bit31 set → never 0x1, never == route id; route id ≥ 0x4000_0000 → never 100). Absent = today's exact identity (blackwall / 0x1 / 100) — fully backward-compatible. Threaded through nft render + apply/teardown (now takes &Policy), plus a new blackwalld teardown --config subcommand so the runit crash backstop cleans up only its own instance.

Placeholder next-hop guardRtbhPolicy::placeholder_next_hops() flags RFC 5737 / 3849 / 6666 ranges; the flow daemon refuses to start armed (not-shadow) if any rtbh next-hop is one. Shadow is exempt.

Review

An independent review verified both Rust goals correct end-to-end (all three resources namespaced, stamp-mark == match-mark, guard on the live path only, range checks correct, all 14 mechanical instance: None insertions landed right) and caught two issues, both fixed in 33e004e:

  • CRITICAL: the runit finish crash script still hardcoded the default identity → a named instance's crash would wipe the default instance's live dataplane. Now delegates to blackwalld teardown.
  • MEDIUM: mark + route-table shared one 16-bit hash slot (~1/65536 silent collision) → now independent 64-bit halves.

Tests

InstanceIds derivation (defaults / namespacing / distinctness / stability), instance= parse (set/default/empty/duplicate), named-instance table rename, placeholder-next-hop detection. Workspace clippy -D warnings + fmt clean.

Still the operator's decision (not in this PR)

The real fleet blackhole next-hop value (#4) — this PR only stops the placeholder shipping silently. Router-id validation in bird-config (#232) is a noted fast-follow.

Also in this branch — #232 router-id

router-id is now optional and defaults to the IPv4 local-addr (blackwall's own address, distinct from the BIRD peer's router-id by construction); router-id == peer is rejected at parse. This is the code hardening for the Bad BGP identifier collision hit at arming — same fix applied live (router-id → container IP). Tests: default-from-local-addr, reject-equal-to-peer, error-when-undeterminable.

## Why Two arming defects surfaced during the 2026-07-31 box work. **1. A second `blackwalld` instance can't coexist with the first.** Covering a second ingress path (e.g. a separate IX-peering NIC) with its own daemon was blocked: both instances share three global kernel resources and clobber each other — the nft table (every apply does `add`+`flush`, so they flush-wipe each other's rules), the deception TPROXY fwmark, and its policy route table (teardown runs `ip rule del` + `ip route flush`, so one stopping rips out the other's plumbing). **2. Arming with a placeholder blackhole next-hop fails silently.** An `rtbh` next-hop left at a documentation/discard template value (`192.0.2.1`, `100::1`, …) never resolves to a real discard route, so BIRD leaves the `/32` `unreachable` and never exports it — the daemon shows `Established` yet announces nothing. ## What **`instance=<name>`** (optional top-level directive) → `blackwall_core::InstanceIds` derives all three resources per-instance: table `blackwall_<name>`, and a fwmark + route-table id taken from independent 32-bit halves of a 64-bit hash of the name (mark bit31 set → never `0x1`, never == route id; route id ≥ `0x4000_0000` → never `100`). **Absent = today's exact identity (`blackwall` / `0x1` / `100`)** — fully backward-compatible. Threaded through nft `render` + `apply`/`teardown` (now takes `&Policy`), plus a new `blackwalld teardown --config` subcommand so the runit crash backstop cleans up **only its own** instance. **Placeholder next-hop guard** — `RtbhPolicy::placeholder_next_hops()` flags RFC 5737 / 3849 / 6666 ranges; the flow daemon refuses to start armed (not-shadow) if any rtbh next-hop is one. Shadow is exempt. ## Review An independent review verified both Rust goals correct end-to-end (all three resources namespaced, stamp-mark == match-mark, guard on the live path only, range checks correct, all 14 mechanical `instance: None` insertions landed right) and caught two issues, both fixed in `33e004e`: - **CRITICAL**: the runit `finish` crash script still hardcoded the default identity → a named instance's crash would wipe the *default* instance's live dataplane. Now delegates to `blackwalld teardown`. - **MEDIUM**: mark + route-table shared one 16-bit hash slot (~1/65536 silent collision) → now independent 64-bit halves. ## Tests InstanceIds derivation (defaults / namespacing / distinctness / stability), `instance=` parse (set/default/empty/duplicate), named-instance table rename, placeholder-next-hop detection. Workspace `clippy -D warnings` + `fmt` clean. ## Still the operator's decision (not in this PR) The real fleet blackhole next-hop value (#4) — this PR only stops the placeholder shipping silently. Router-id validation in `bird-config` (#232) is a noted fast-follow. ## Also in this branch — #232 router-id `router-id` is now optional and defaults to the IPv4 `local-addr` (blackwall's own address, distinct from the BIRD peer's router-id by construction); `router-id == peer` is rejected at parse. This is the code hardening for the `Bad BGP identifier` collision hit at arming — same fix applied live (router-id → container IP). Tests: default-from-local-addr, reject-equal-to-peer, error-when-undeterminable.
Two arming defects surfaced on the box (2026-07-31 field work):

instance= — a second blackwalld on one box (to cover a separate IX-peering NIC)
collides with the first on three shared kernel resources: the nft table (every
apply does add+flush, so they flush-wipe each other), the deception TPROXY
fwmark, and its policy route table (teardown runs ip rule del + ip route flush,
so one stopping rips out the other's plumbing). New optional `instance=<name>`
directive derives all three per-instance via blackwall_core::InstanceIds
(table blackwall_<name>, distinct mark, distinct route table). Absent = today's
exact identity (blackwall / 0x1 / 100), fully backward-compatible. Threaded
through nft render + apply/teardown; teardown now takes &Policy.

next-hop guard — arming (flow without shadow) with an rtbh next-hop in a
documentation/discard range (RFC 5737/3849/6666 — the unreplaced template
values) now refuses to start, instead of showing Established while BIRD leaves
the blackhole unreachable and exports nothing. RtbhPolicy::placeholder_next_hops
+ a startup check on the live path.

Tests: InstanceIds derivation (defaults, namespacing, distinctness, stability),
instance= parse (set/default/empty/duplicate), named-instance table rename,
placeholder-next-hop detection. Workspace clippy -D warnings + fmt clean.
Addresses the review of the instance= feature:

- CRITICAL: the runit `finish` crash backstop hardcoded `nft delete table inet
  blackwall` + `fwmark 0x1 lookup 100`, so on a SIGKILL a named instance would
  wipe the DEFAULT instance's live table/route (and leak its own) — reintroducing
  the exact cross-instance collision via the crash path. Added a `blackwalld
  teardown --config` subcommand that reuses the instance-aware
  blackwall_nft::teardown(&policy); finish now delegates to it, so it removes only
  this instance's resources.

- MEDIUM: InstanceIds derived the mark and route-table from the same 16-bit hash
  slot, so two distinct names had a ~1/65536 chance of sharing both (silent
  teardown collision). Now they come from independent 32-bit halves of a 64-bit
  FNV-1a — mark has bit31 set (never 0x1, never == route id), route id ≥ 0x40000000
  (never 100) — so a collision is ~1/2^31 per resource.

- LOW: fixed a stale intra-doc link to the removed TPROXY_MARK const; documented
  that engine tproxy-port=/nfqueue-num= must also differ per instance (not covered
  by instance=) and to point a copied service dir's run+finish at the new config.

Workspace clippy -D warnings + fmt clean; InstanceIds tests still green.
fix(rtbh): default router-id to local-addr; reject router-id == peer (#232)
Some checks failed
CI / check (pull_request) Failing after 7m42s
CI / lab (pull_request) Has been skipped
0acfc5c787
Arming hit BIRD 'Bad BGP identifier' because the rtbh router-id was hand-set to
a value matching the peer's identity — a session that never establishes, masked
for weeks by shadow mode never opening it.

- router-id is now OPTIONAL and defaults to the IPv4 local-addr (blackwall's own
  source address), which is distinct from the BIRD peer's router-id by
  construction — exactly the fix applied live on the box.
- reject router-id == peer address at parse time (a common variant of the same
  mistake).
- errors clearly when router-id is omitted and there's no IPv4 local-addr to
  default from.

Tests: default-from-local-addr, reject-equal-to-peer, error-when-undeterminable.
Docs: router-id documented as optional in deployment.md.
Merge remote-tracking branch 'origin/main' into fix/nft-table-name-and-rtbh-nexthop-guard
Some checks failed
CI / check (pull_request) Successful in 18m26s
CI / lab (pull_request) Failing after 2s
d7bb97dcce
Sign in to join this conversation.
No description provided.