Merge pull request #89 from dan-claude-bot/fix/ufw-subnet-converge

box-firewall: converge the UFW carve-out off the live bridge; fail closed at boot (#86 follow-up)

CHANGELOG.md

@@ -165,6 +165,30 @@ which records not just what changed but what each drill run proved.
 
 ### Fixed
 
+- **UFW's gateway carve-out converges with the bridge, and the doctor can
+  see it** (the #86 review's blind spot) — `box-firewall` gated its whole
+  UFW block behind "a `DENY on boxnet` rule exists", pinning every UFW host
+  to the gateway of the *first* run: a bridge remapped off a colliding
+  subnet (#80's escape hatch) kept its stale `allow … to <old-gw> port 53`
+  and never gained the live gateway's, so box→gateway DNS died at box's own
+  deny — while the doctor's carve-out check read only the incus ACL (which
+  setup-host converges) and called the host clean. The UFW allows now
+  converge off the live bridge address on every run (stale DNS allows
+  deleted, the live set ensured — ufw skips existing rules, so a fresh host
+  gets the identical rule set and a re-run is a no-op), and `box doctor`
+  reads UFW's own table wherever UFW is active, flagging a DNS allow that
+  does not match `boxnet`'s gateway (and stale allows left beside a live
+  one). The no-UFW nft carve-out never had this failure mode: it is
+  interface-scoped, no gateway address to go stale.
+- **The boot-time gateway fallback is gone — no rule beats a wrong one** —
+  with the bridge not yet addressed when `box-firewall.service` ran,
+  `box-firewall` guessed `GW=10.88.0.1`; on a `BOX_SUBNET` host that hit
+  that window the UFW carve-out was built for the wrong gateway, a latent
+  DNS drop (#86 review). It now fails closed: an unaddressed bridge leaves
+  the persisted UFW rules exactly as they are (they survive boots on their
+  own, and nothing else in the script needs the gateway) and says so on
+  stderr; the next setup-host run or service restart converges them once
+  the bridge is addressed.
 - **`revoke --purge` re-checks the incus-user state** — the purge removed
   `/var/lib/incus/users/<uid>` without ever asserting its absence, the one
   path its own absence block did not cover; and the stat now rides

drill/doctor.sh

@@ -62,6 +62,38 @@ gw_squat_signature() {
       print "duplicate connected routes for the uplink subnet " up " (" devs[up] ") — whichever link last gains carrier wins, and a nested bridge with carrier blackholes egress" }'
 }
 
+# --- The UFW half of the gateway carve-out (#86 review) ---------------------
+# setup-host converges the ACL's gateway allow and box-firewall converges
+# UFW's — but a doctor that reads only the ACL hands a remapped UFW host a
+# clean bill while a stale 'allow … to <old-gw> port 53' quietly drops
+# box→gateway DNS. This reads UFW's own table. Pure text in (`ufw status`
+# output, the network, the live gateway), findings out (one per line,
+# silence is agreement) — the gw_squat_signature seam, so test/cli.sh drives
+# it against canned tables. Judged only where UFW is active: the no-UFW nft
+# carve-out is interface-scoped (no gateway address to go stale).
+ufw_dns_findings() {
+  local status="$1" net="$2" gw="$3" allows stale
+  allows="$(printf '%s\n' "$status" | awk -v net="$net" '
+    $2 ~ /^53\// && $3 == "on" && $4 == net { print $1 }' | sort -u)"
+  if [ -z "$allows" ]; then
+    # No DNS allow at all is a drop only if OUR deny is there to do the
+    # dropping — a UFW host box-firewall never touched has nothing to judge.
+    printf '%s\n' "$status" | grep " on $net" | grep -q "DENY" \
+      && printf 'UFW denies in on %s with NO DNS allow at all — box DNS to the gateway is dropped\n' "$net"
+    return 0
+  fi
+  if ! printf '%s\n' "$allows" | grep -qxF "$gw"; then
+    printf "UFW's DNS allow points at %s — NOT %s's live gateway (%s): box DNS dies at UFW's deny\n" \
+      "$(printf '%s' "$allows" | tr '\n' ' ')" "$net" "$gw"
+    return 0
+  fi
+  stale="$(printf '%s\n' "$allows" | grep -vxF "$gw")" || true
+  [ -n "$stale" ] \
+    && printf "stale UFW DNS allow(s) for %s left beside the live gateway's — box-firewall converges these away now\n" \
+         "$(printf '%s' "$stale" | tr '\n' ' ')"
+  return 0
+}
+
 # The signature, probed INSIDE a box: its routes, read where they live. A
 # poisoned guest looks healthy from every host-side config check — the nested
 # bridge and the captured gateway exist only in the guest's kernel.
@@ -243,6 +275,26 @@ else
   inf "      (or: sudo systemctl restart box-firewall.service)"
 fi
 
+# The UFW blind spot: the ACL check further down compares the ACL's carve-out
+# to the live gateway, but on a UFW host the SAME stale-carve-out failure can
+# live in UFW's own table — and did, invisibly (#86 review). Judge it here,
+# from `ufw status`, wherever UFW is the active firewall and a bridge exists
+# to compare against (a fresh host has neither).
+if command -v ufw >/dev/null 2>&1; then
+  ufw_out="$(sudo ufw status 2>/dev/null)"
+  ufw_gw="$(incus network get boxnet ipv4.address 2>/dev/null | cut -d/ -f1)"
+  if printf '%s\n' "$ufw_out" | grep -q "Status: active" && [ -n "$ufw_gw" ]; then
+    findings="$(ufw_dns_findings "$ufw_out" boxnet "$ufw_gw")"
+    if [ -n "$findings" ]; then
+      while IFS= read -r line; do no "$line"; done <<<"$findings"
+      inf "the bridge moved (#80's escape hatch) and UFW did not follow"
+      inf "fix:  sudo /usr/local/sbin/box-firewall   (it converges the UFW allows off the live bridge now)"
+    else
+      ok "no stale UFW DNS carve-out — box DNS to the gateway ($ufw_gw) survives UFW"
+    fi
+  fi
+fi
+
 # box-net is the placement contract since the 0.4.0 rename; claude-dev is its
 # pre-rename ancestor and may linger while legacy boxes still reference it.
 # Check whichever exist — an unisolated NIC is a fault on either.

host/box-firewall.sh

@@ -9,15 +9,34 @@ NET=boxnet
 # The gateway is read off the live bridge, not hardcoded: the subnet is an
 # input now (BOX_SUBNET, setup-host.sh — #80), and a bridge moved off a
 # colliding subnet must keep its firewall. setup-host runs us after the
-# bridge exists, so the live read is the truth at install time; UFW rules
+# bridge exists, so the live read is the truth at install time. At boot the
-# persist across boots on their own, so the default only papers over the
+# bridge may not be addressed yet — then GW is EMPTY and the UFW carve-out
-# no-bridge-yet window at boot on a default-subnet host.
+# below is left alone rather than built for a guessed gateway: the old
+# GW=10.88.0.1 fallback was wrong on every BOX_SUBNET host that hit the
+# no-bridge window, a latent DNS drop (#86 review). Failing closed costs
+# nothing: UFW rules persist across boots on their own, and nothing else in
+# this script needs the gateway (the nft carve-out is interface-scoped).
 # ('|| true': under pipefail an absent bridge would kill the script here.)
 GW="$(ip -4 -o addr show dev "$NET" 2>/dev/null | awk '{ split($4, a, "/"); print a[1]; exit }' || true)"
-[ -n "$GW" ] || GW=10.88.0.1
 
 if command -v ufw >/dev/null && ufw status 2>/dev/null | grep -q "Status: active"; then
-  if ! ufw status | grep "on $NET" | grep -q "DENY"; then
+  if [ -z "$GW" ]; then
+    echo "box-firewall: $NET has no address yet — UFW DNS carve-out left as-is (no rule beats a wrong one; the persisted rules survive boots, and setup-host or a service restart converges them once the bridge is addressed)" >&2
+  else
+    # Converge, don't create-once (the ACL's own #86 lesson, applied to UFW):
+    # gating this block on "a DENY on boxnet exists" pinned every host to the
+    # gateway of the FIRST run — a bridge remapped off a colliding subnet
+    # (#80's escape hatch) kept its stale 'allow … to <old-gw> port 53' and
+    # never gained the live gateway's, so box→gateway DNS died at our own
+    # deny while every config looked right. Drop the DNS allows aimed
+    # anywhere else, then ensure the live set: ufw skips a rule that already
+    # exists, so the re-run is a no-op and a fresh host gets exactly the
+    # rules it always did.
+    for stale in $(ufw status | awk -v net="$NET" -v gw="$GW" '
+        $2 ~ /^53\// && $3 == "on" && $4 == net && $1 != gw { print $1 }' | sort -u); do
+      ufw delete allow in on "$NET" to "$stale" port 53 proto tcp || true
+      ufw delete allow in on "$NET" to "$stale" port 53 proto udp || true
+    done
     ufw insert 1 deny in on "$NET"
     ufw insert 1 allow in on "$NET" to "$GW" port 53 proto tcp
     ufw insert 1 allow in on "$NET" to "$GW" port 53 proto udp

test/cli.sh

@@ -847,6 +847,184 @@ check "doctor: the egress-broken-DNS-fine fingerprint is named on both tiers" 0
 check "doctor: the ACL carve-out is checked against the live gateway" 0 "" \
   grep -qF "does NOT match boxnet's gateway" "$ROOT/drill/doctor.sh"
 
+# ---------------------------------------------------------------------------
+# box-firewall's UFW converge and the fail-closed boot window (#86 review,
+# items 1–2). The whole script is DRIVEN under shims (the setup-host seam):
+# a fake ufw serves canned `ufw status` tables and logs every mutation, fake
+# nft/sysctl/iptables swallow the rest, and the shim ip answers the
+# live-bridge read. Stale gateway allows must converge to the live gateway,
+# a fresh UFW host must get exactly the rule set it always did, a no-UFW
+# host must keep its nft path, and the no-bridge-address boot window must
+# mutate NOTHING — the old GW=10.88.0.1 fallback built the carve-out for
+# the wrong gateway on every BOX_SUBNET host that hit it.
+# ---------------------------------------------------------------------------
+FWSHIM="$(mktemp -d)"; UFWSHIM="$(mktemp -d)"; WFW="$(mktemp -d)"
+cat > "$UFWSHIM/ufw" <<'SHIM'
+#!/usr/bin/env bash
+# Fake ufw: 'status' prints $FAKE_UFW_STATUS; every call is logged to
+# $FAKE_UFW_LOG. Mutations mutate nothing, of course.
+[ -n "${FAKE_UFW_LOG:-}" ] && printf 'ufw %s\n' "$*" >> "$FAKE_UFW_LOG"
+case "${1:-}" in status) printf '%s\n' "${FAKE_UFW_STATUS:-Status: inactive}" ;; esac
+exit 0
+SHIM
+cat > "$FWSHIM/nft" <<'SHIM'
+#!/usr/bin/env bash
+# Fake nft: logs to $FAKE_NFT_LOG. The bridge-table probe answers "absent"
+# so the creation path runs (and is logged) instead of being skipped.
+[ -n "${FAKE_NFT_LOG:-}" ] && printf 'nft %s\n' "$*" >> "$FAKE_NFT_LOG"
+case "$*" in "list table bridge box") exit 1 ;; esac
+exit 0
+SHIM
+cat > "$FWSHIM/sysctl" <<'SHIM'
+#!/usr/bin/env bash
+exit 0
+SHIM
+cat > "$FWSHIM/iptables" <<'SHIM'
+#!/usr/bin/env bash
+# Fake iptables: the DOCKER-USER probe answers "no such chain", so the
+# docker block is deterministically skipped whether or not this runner
+# happens to have docker.
+exit 1
+SHIM
+chmod +x "$UFWSHIM/ufw" "$FWSHIM/nft" "$FWSHIM/sysctl" "$FWSHIM/iptables"
+
+runfw() { # runfw <ufw|noufw> [VAR=val ...] — the real box-firewall, under shims
+  local mode="$1" p; shift
+  p="$FWSHIM:$SHIMDIR:$PATH"
+  [ "$mode" = ufw ] && p="$UFWSHIM:$p"
+  env PATH="$p" "$@" bash "$ROOT/host/box-firewall.sh"
+}
+
+# Canned `ufw status` tables, modeled on the real output shape.
+U_HDR='Status: active
+
+To                         Action      From
+--                         ------      ----
+22/tcp                     ALLOW       Anywhere'
+U_FRESH="$U_HDR"
+U_OLDGW="$U_HDR
+Anywhere on boxnet         DENY        Anywhere
+10.88.0.1 53/tcp on boxnet ALLOW       Anywhere
+10.88.0.1 53/udp on boxnet ALLOW       Anywhere
+67/udp on boxnet           ALLOW       Anywhere
+Anywhere on boxnet         ALLOW FWD   Anywhere"
+U_LIVEGW="$U_HDR
+Anywhere on boxnet         DENY        Anywhere
+10.89.0.1 53/tcp on boxnet ALLOW       Anywhere
+10.89.0.1 53/udp on boxnet ALLOW       Anywhere
+67/udp on boxnet           ALLOW       Anywhere
+Anywhere on boxnet         ALLOW FWD   Anywhere"
+BX88='5: boxnet    inet 10.88.0.1/24 scope global boxnet'
+BX89='5: boxnet    inet 10.89.0.1/24 scope global boxnet'
+
+# The remapped host (#80's escape hatch): bridge on 10.89, UFW still carrying
+# 10.88's carve-out — the stale allows go, the live gateway's land.
+check "box-firewall: a remapped bridge CONVERGES the UFW carve-out" 0 "" \
+  runfw ufw FAKE_IP4_BOXNET="$BX89" FAKE_UFW_STATUS="$U_OLDGW" FAKE_UFW_LOG="$WFW/remap.log"
+check "box-firewall: ...the stale tcp allow is deleted" 0 "" \
+  grep -qF 'ufw delete allow in on boxnet to 10.88.0.1 port 53 proto tcp' "$WFW/remap.log"
+check "box-firewall: ...and the stale udp allow" 0 "" \
+  grep -qF 'ufw delete allow in on boxnet to 10.88.0.1 port 53 proto udp' "$WFW/remap.log"
+check "box-firewall: ...the live gateway gains its tcp allow" 0 "" \
+  grep -qF 'ufw insert 1 allow in on boxnet to 10.89.0.1 port 53 proto tcp' "$WFW/remap.log"
+check "box-firewall: ...and its udp allow" 0 "" \
+  grep -qF 'ufw insert 1 allow in on boxnet to 10.89.0.1 port 53 proto udp' "$WFW/remap.log"
+check "box-firewall: ...the live gateway's rules are never deleted" 1 "" \
+  grep -qF 'delete allow in on boxnet to 10.89.0.1' "$WFW/remap.log"
+
+# The agreeing host: rules already match the live gateway — nothing deleted
+# (ufw itself skips the re-adds as existing rules).
+check "box-firewall: an agreeing UFW host deletes nothing" 0 "" \
+  runfw ufw FAKE_IP4_BOXNET="$BX89" FAKE_UFW_STATUS="$U_LIVEGW" FAKE_UFW_LOG="$WFW/agree.log"
+check "box-firewall: ...no delete was issued" 1 "" grep -qF ' delete ' "$WFW/agree.log"
+
+# The fresh host: no boxnet rules yet — exactly the five historical commands,
+# aimed at the live gateway, and nothing else (unchanged behavior).
+check "box-firewall: a fresh UFW host runs clean" 0 "" \
+  runfw ufw FAKE_IP4_BOXNET="$BX88" FAKE_UFW_STATUS="$U_FRESH" FAKE_UFW_LOG="$WFW/fresh.log"
+check "box-firewall: ...the deny lands" 0 "" \
+  grep -qF 'ufw insert 1 deny in on boxnet' "$WFW/fresh.log"
+check "box-firewall: ...the DNS allows aim at the live gateway" 0 "" \
+  grep -qF 'ufw insert 1 allow in on boxnet to 10.88.0.1 port 53 proto tcp' "$WFW/fresh.log"
+# shellcheck disable=SC2016  # $1 expands in the child shell, by design
+check "box-firewall: ...DHCP and the route allow land too" 0 "" bash -c '
+  grep -qF "ufw insert 1 allow in on boxnet to any port 67 proto udp" "$1" &&
+  grep -qF "ufw route allow in on boxnet" "$1"' _ "$WFW/fresh.log"
+# shellcheck disable=SC2016  # $1 expands in the child shell, by design
+check "box-firewall: ...exactly the five historical mutations, no more" 0 "" \
+  bash -c '[ "$(grep -vc "^ufw status" "$1")" -eq 5 ]' _ "$WFW/fresh.log"
+
+# The boot window (#86 review item 2): bridge not yet addressed → NO guessed
+# gateway, NO mutation at all — the persisted rules are left exactly as they
+# are, and the skip says so. (The old fallback built 10.88.0.1 rules on a
+# BOX_SUBNET host here — a latent DNS drop.)
+check "box-firewall: an unaddressed bridge FAILS CLOSED on a UFW host" 0 "left as-is" \
+  runfw ufw FAKE_IP4_BOXNET= FAKE_UFW_STATUS="$U_OLDGW" FAKE_UFW_LOG="$WFW/boot.log"
+# shellcheck disable=SC2016  # $1 expands in the child shell, by design
+check "box-firewall: ...not one ufw mutation was issued" 0 "" \
+  bash -c '[ "$(grep -vc "^ufw status" "$1")" -eq 0 ]' _ "$WFW/boot.log"
+check "box-firewall: the hardcoded gateway fallback is GONE (comments aside)" 1 "" \
+  grep -qE '^[^#]*GW=10' "$ROOT/host/box-firewall.sh"
+
+# The no-UFW host: untouched semantics — the nft input carve-out is
+# interface-scoped, so it needs no gateway and applies even in the boot
+# window where the UFW path now declines to guess.
+check "box-firewall: a no-UFW host keeps its nft path" 0 "" \
+  runfw noufw FAKE_IP4_BOXNET="$BX89" FAKE_NFT_LOG="$WFW/nft.log"
+check "box-firewall: ...the DNS/DHCP accept is interface-scoped" 0 "" \
+  grep -qF 'add rule inet box input iifname boxnet udp dport { 53, 67 } accept' "$WFW/nft.log"
+check "box-firewall: ...and the input drop lands" 0 "" \
+  grep -qF 'add rule inet box input iifname boxnet drop' "$WFW/nft.log"
+check "box-firewall: the nft path survives the boot window too" 0 "" \
+  runfw noufw FAKE_IP4_BOXNET= FAKE_NFT_LOG="$WFW/nftboot.log"
+check "box-firewall: ...with the same interface-scoped carve-out" 0 "" \
+  grep -qF 'add rule inet box input iifname boxnet udp dport { 53, 67 } accept' "$WFW/nftboot.log"
+
+# ---------------------------------------------------------------------------
+# The doctor's UFW blind spot (#86 review item 1, second half): the ACL
+# check alone gave a remapped UFW host a clean bill while the stale UFW
+# allow dropped box DNS. ufw_dns_findings is pure text → findings, the
+# gw_squat_signature seam: extracted and driven against canned tables.
+# ---------------------------------------------------------------------------
+UFWFN="$(mktemp)"
+awk '/^ufw_dns_findings\(\) \{/,/^\}/' "$ROOT/drill/doctor.sh" > "$UFWFN"
+check "ufw_dns_findings: extracted from doctor.sh (guards the awk)" 0 "DNS allow" cat "$UFWFN"
+check "ufw_dns_findings: the extracted function is valid bash" 0 "" bash -n "$UFWFN"
+ufwsig()   { bash -c ". '$UFWFN'; ufw_dns_findings \"\$1\" \"\$2\" \"\$3\"" _ "$1" "$2" "$3"; }
+noufwsig() { [ -z "$(ufwsig "$1" "$2" "$3")" ]; }
+
+check "ufw findings: agreement is SILENT" 0 "" noufwsig "$U_LIVEGW" boxnet 10.89.0.1
+check "ufw findings: a stale carve-out is flagged as NOT the live gateway" \
+  0 "NOT boxnet's live gateway" ufwsig "$U_OLDGW" boxnet 10.89.0.1
+check "ufw findings: ...naming the address it points at" \
+  0 "10.88.0.1" ufwsig "$U_OLDGW" boxnet 10.89.0.1
+# Our deny with no DNS allow at all is a drop — say so.
+U_DENYONLY="$U_HDR
+Anywhere on boxnet         DENY        Anywhere"
+check "ufw findings: a deny with NO DNS allow is a drop" \
+  0 "NO DNS allow" ufwsig "$U_DENYONLY" boxnet 10.89.0.1
+# A UFW host box-firewall never touched has nothing to judge — clean.
+check "ufw findings: an untouched UFW host is CLEAN" 0 "" noufwsig "$U_FRESH" boxnet 10.89.0.1
+# A stale allow left BESIDE the live one still gets named (residue, not a drop).
+U_BOTH="$U_LIVEGW
+10.88.0.1 53/tcp on boxnet ALLOW       Anywhere"
+check "ufw findings: a stale allow beside the live one is named" \
+  0 "stale UFW DNS allow" ufwsig "$U_BOTH" boxnet 10.89.0.1
+# Rules on OTHER interfaces are not boxnet's problem.
+U_OTHERIF="$U_LIVEGW
+10.88.0.1 53/tcp on eth0   ALLOW       Anywhere"
+check "ufw findings: another interface's DNS allow is ignored" 0 "" \
+  noufwsig "$U_OTHERIF" boxnet 10.89.0.1
+
+# The wiring: doctor judges UFW's own table where UFW is active, and the fix
+# points at the converging box-firewall.
+# shellcheck disable=SC2016  # the $-string is a literal in the target file
+check "doctor: reads UFW's table through ufw_dns_findings" 0 "" \
+  grep -qF 'ufw_dns_findings "$ufw_out"' "$ROOT/drill/doctor.sh"
+check "doctor: the UFW fix names the converge" 0 "" \
+  grep -qF 'converges the UFW allows' "$ROOT/drill/doctor.sh"
+rm -f "$UFWFN"; rm -rf "$FWSHIM" "$UFWSHIM" "$WFW"
+
 # The docs keep the new promises.
 check "help setup-host names BOX_SUBNET" 0 "BOX_SUBNET" "$BOX" help setup-host
 check "help setup-host names the refusal" 0 "REFUSES" "$BOX" help setup-host