diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e2744..60b118f 100644 --- a/CHANGELOG.md +++ b/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 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/` without ever asserting its absence, the one path its own absence block did not cover; and the stat now rides diff --git a/drill/doctor.sh b/drill/doctor.sh index 4fac72b..6a33185 100755 --- a/drill/doctor.sh +++ b/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 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. diff --git a/host/box-firewall.sh b/host/box-firewall.sh index bb96b50..d7959bf 100644 --- a/host/box-firewall.sh +++ b/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 -# persist across boots on their own, so the default only papers over the -# no-bridge-yet window at boot on a default-subnet host. +# bridge exists, so the live read is the truth at install time. At boot the +# bridge may not be addressed yet — then GW is EMPTY and the UFW carve-out +# 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 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 diff --git a/test/cli.sh b/test/cli.sh index b23b782..0c681ae 100644 --- a/test/cli.sh +++ b/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 [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