From a493fa5584bb5976dd090ae7813df73adb7d7ffa Mon Sep 17 00:00:00 2001 From: gibbyb Date: Thu, 17 Sep 2026 14:36:10 -0400 Subject: [PATCH] Open what is shut, and never widen what is already open setup-server queried --query-port, saw "not open", and blanket-opened 80, 443 and 81. On a machine reaching those ports through rich rules -- one CDN's address sets, say -- neither appears in --list-ports, so the step read a deliberate, narrow grant as "closed" and replaced it with a public one. It did this during `panama update`, which asks nothing and runs unattended, so there was no prompt and no diff for anyone to read. A production server lost its firewall posture that way, port 81 and the proxy's admin portal included. Each port is now checked for existing coverage first -- an open port, an enabled service, or an accepting rich rule, ranges included -- and left alone when it has some. The bias stays where it was: only a rule that positively reads as a grant counts, and anything unrecognised still falls through to opening the port, because a fresh server that is silently unreachable is the worse failure. A rule that DROPS a port is not coverage either; reading a block as a grant would leave the proxy unreachable on a machine nobody thought to re-check. A machine that manages its own firewall opts out once, with `echo manual > ~/.local/state/panama/firewall`, and Panama never touches firewalld on it again. A file rather than an environment variable, because the run that needs to respect it is the unattended one. tests/setup/server-firewall-contract pins all of it, and fails against the previous code on exactly the cases that cost the posture. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 4 +- server/README.md | 8 +- setup/packages/server-packages | 6 +- setup/scripts/setup-server | 72 ++++++++++- tests/contracts.manifest | 1 + tests/setup/server-firewall-contract | 184 +++++++++++++++++++++++++++ 6 files changed, 268 insertions(+), 7 deletions(-) create mode 100755 tests/setup/server-firewall-contract diff --git a/README.md b/README.md index b30975c..15632b3 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ in order, without stopping again: | `link-vicinae-scripts` | Publishes the Vicinae script commands | | `setup-identity` | git config, `gh auth login`, an SSH key — whichever were asked for | | `install-hardware` | NVIDIA, Secure Boot enrollment, Fedora's extras, firmware — each only if it was asked for. Last, because enrollment and firmware are consumed at the next boot | -| `setup-server` | Server role only: linger, unprivileged ports from 80, firewalld (80/443/81), the `nginx-bridge` network, and the nightly image-update timer | +| `setup-server` | Server role only: linger, unprivileged ports from 80, firewalld (80/443/81, opened only where nothing already grants them), the `nginx-bridge` network, and the nightly image-update timer | | `link-server` | Server role only: creates `~/Server` and refreshes the links of whatever services this machine has enabled | A server runs the shared stages plus its own two, and skips the rest — @@ -250,7 +250,7 @@ docs/ Settings reference, and the design specs behind the work ## Tests -190 of them, under `tests/`. `tests/contracts.manifest` classifies every +191 of them, under `tests/`. `tests/contracts.manifest` classifies every contract by the capabilities it needs. Run the hermetic set, or grant a specific external capability when automation needs it: diff --git a/server/README.md b/server/README.md index 3f9b45d..9f39e05 100644 --- a/server/README.md +++ b/server/README.md @@ -40,7 +40,13 @@ is what `panama server enable` refuses to start on. the volume path they were running with until their cutover, because the repo copy must render identically to the live one — renames happen with the service stopped, not in git. -- Only 80, 443 and 81 are open (setup-server's doing). Everything else is +- Only 80, 443 and 81 are open, and `setup-server` opens each one only where + nothing already grants it. A port reachable through an enabled service or an + accepting rich rule — a CDN-scoped grant, say — is left exactly as it is, + because turning a narrow grant into a public one during an unattended + `panama update` is not a decision an installer gets to make. A machine that + manages its own firewall opts out for good with + `echo manual > ~/.local/state/panama/firewall`. Everything else is reverse-proxied over the `nginx-bridge` network by container name, with no published ports of its own. A service that truly needs another host port says so in a README in its directory, and the port is opened by hand. diff --git a/setup/packages/server-packages b/setup/packages/server-packages index 118964d..dccd721 100644 --- a/setup/packages/server-packages +++ b/setup/packages/server-packages @@ -2,9 +2,9 @@ # is the shell environment plus containers, and almost everything it needs is # already core. # -# setup-server opens 80/443/81 through it. Fedora Server ships it; a minimal -# cloud image may not, and a server whose firewall step silently no-ops is a -# server somebody believes is firewalled. +# setup-server opens 80/443/81 through it, where nothing already grants them. +# Fedora Server ships it; a minimal cloud image may not, and a server whose +# firewall step silently no-ops is a server somebody believes is firewalled. firewalld # Node comes through nvm here for the same reason as on the desktop -- # config/bash/shell switches versions per project from .nvmrc -- and because diff --git a/setup/scripts/setup-server b/setup/scripts/setup-server index d6e419a..231f25d 100755 --- a/setup/scripts/setup-server +++ b/setup/scripts/setup-server @@ -43,11 +43,81 @@ fi # documents that in its own folder and it is opened by hand, because a list of # per-service firewall holes maintained by an installer is a list nobody # audits. -if systemctl is-active firewalld >/dev/null 2>&1; then +# +# "Open" means reachable, not literally listed by --list-ports. A machine can +# already reach 80 through a rich rule admitting one CDN's address set, or +# through an enabled service. On such a machine `--add-port` is not the no-op +# it looks like: it replaces a deliberate, narrow grant with a blanket one, +# and does it during a routine `panama update` that asked nobody anything. +# So each port is checked for existing coverage and left alone when it has +# some. Panama opens what is shut; it does not widen what is already open. +# +# The bias is deliberate. Only a rule that positively reads as covering the +# port counts, and anything unrecognised falls through to opening it, because +# a fresh server that silently ends up unreachable is the worse failure -- +# see the firewalld comment in setup/packages/server-packages. For the same +# reason only `accept` rules count: a rich rule that drops a port is not +# coverage, it is the opposite, and treating it as coverage would leave the +# reverse proxy unreachable on a machine nobody thought to check. +# +# A machine that manages its own firewall opts out once: +# +# echo manual > ~/.local/state/panama/firewall +# +# after which Panama never touches firewalld on it again. A file rather than +# an environment variable because `panama update` runs unattended and asks +# nothing; a decision that lives only as long as somebody exports a variable +# is a decision that quietly lapses on the next run. +STATE_DIR="${XDG_STATE_HOME:-$HOME/.local/state}/panama" +FIREWALL_DECISION="$STATE_DIR/firewall" + +# Every port an enabled permanent service already grants, one / +# per line. Read back out of firewalld rather than from a hardcoded http=80 +# table, so a machine's own service definition counts for exactly as much as +# a shipped one. +service_ports() { + local service ports + for service in $(sudo firewall-cmd --permanent --list-services 2>/dev/null || true); do + ports="$(sudo firewall-cmd --permanent --info-service="$service" 2>/dev/null \ + | sed -n 's/^[[:space:]]*ports:[[:space:]]*//p')" || true + [[ -n "$ports" ]] || continue + printf '%s\n' $ports + done +} + +# Does an accepting rich rule already admit this tcp port? Ranges count, so a +# rule granting 8000-8100 covers 8080. +rich_rule_covers() { + local port="$1" rule value low high + while IFS= read -r rule; do + [[ "$rule" == *accept* ]] || continue + [[ "$rule" == *'protocol="tcp"'* ]] || continue + value="${rule#*port port=\"}" + [[ "$value" != "$rule" ]] || continue + value="${value%%\"*}" + if [[ "$value" == *-* ]]; then + low="${value%%-*}"; high="${value##*-}" + [[ "$low" =~ ^[0-9]+$ && "$high" =~ ^[0-9]+$ ]] || continue + (( port >= low && port <= high )) && return 0 + else + [[ "$value" == "$port" ]] && return 0 + fi + done < <(sudo firewall-cmd --permanent --list-rich-rules 2>/dev/null || true) + return 1 +} + +if [[ "$(cat "$FIREWALL_DECISION" 2>/dev/null || true)" == manual ]]; then + log "Firewall left to this machine ($FIREWALL_DECISION says manual)" +elif systemctl is-active firewalld >/dev/null 2>&1; then + granted_ports="$(service_ports)" reload_needed=0 for port in 80 443 81; do if sudo firewall-cmd --permanent --query-port="${port}/tcp" >/dev/null 2>&1; then log "Port ${port}/tcp already open" + elif grep -qxF "${port}/tcp" <<<"$granted_ports"; then + log "Port ${port}/tcp already granted by an enabled service; leaving it alone" + elif rich_rule_covers "$port"; then + log "Port ${port}/tcp already granted by a rich rule; leaving it alone" else log "Opening port ${port}/tcp" sudo firewall-cmd --permanent --add-port="${port}/tcp" >/dev/null diff --git a/tests/contracts.manifest b/tests/contracts.manifest index 2551e98..e9b60fe 100644 --- a/tests/contracts.manifest +++ b/tests/contracts.manifest @@ -278,6 +278,7 @@ hermetic tests/setup/role-contract # Root bootstrap uses a temporary filesystem, PATH adapters, real public-key # parsing, and a user namespace for the actual-root fixture guard. hermetic tests/setup/root-server-bootstrap-contract +hermetic tests/setup/server-firewall-contract hermetic tests/setup/skills-contract hermetic tests/setup/test-runner-contract hermetic tests/setup/update-command-contract diff --git a/tests/setup/server-firewall-contract b/tests/setup/server-firewall-contract new file mode 100755 index 0000000..91595ec --- /dev/null +++ b/tests/setup/server-firewall-contract @@ -0,0 +1,184 @@ +#!/usr/bin/env bash + +# setup-server's firewall step: what it opens, and what it refuses to widen. +# +# The bug this pins cost a production VPS its firewall posture. That machine +# reached 80 and 443 only from one CDN's address sets, through rich rules; +# neither port appeared in --list-ports. The old step queried --query-port, +# saw "not open", and blanket-opened 80, 443 and 81 -- turning a narrow grant +# into a public one, and port 81 into a publicly reachable admin portal. It +# did it during `panama update`, which asks nothing and runs unattended, so +# there was no prompt and no diff for anyone to read. +# +# What is pinned here, and why each half matters: +# +# * A port already granted -- by an open port, an enabled service, or an +# accepting rich rule -- is left alone. Panama opens what is shut; it +# never widens what is already open. +# * A port granted by nothing is still opened. The opposite failure is a +# fresh server that is silently unreachable, which is worse: see the +# firewalld comment in setup/packages/server-packages. +# * A rich rule that DROPS a port is not coverage. Reading a block as a +# grant would leave the reverse proxy unreachable on a machine nobody +# thought to re-check. +# * `echo manual > $XDG_STATE_HOME/panama/firewall` stops Panama touching +# firewalld at all, and that opt-out is a file so it survives the next +# unattended update. + +set -uo pipefail + +repo_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +stage="$repo_dir/setup/scripts/setup-server" + +findings=() +note() { findings+=("$1"); } + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +# ── Stubs ──────────────────────────────────────────────────────────────────── +# Everything setup-server reaches for outside the firewall block is answered +# "already done", so the only behaviour under test is which ports it decides +# to open. firewall-cmd is the one stub that reports: it appends each mutating +# call to $FW_LOG, and answers queries from the scenario in the environment. + +bin="$work/bin" +mkdir -p "$bin" + +cat >"$bin/firewall-cmd" <<'STUB' +#!/usr/bin/env bash +for arg in "$@"; do + case "$arg" in + --query-port=*) + want="${arg#--query-port=}"; want="${want%/tcp}" + for open in ${FW_OPEN:-}; do [[ "$open" == "$want" ]] && exit 0; done + exit 1 ;; + --add-port=*) printf 'add %s\n' "${arg#--add-port=}" >>"$FW_LOG"; exit 0 ;; + --reload) printf 'reload\n' >>"$FW_LOG"; exit 0 ;; + --list-services) printf '%s\n' "${FW_SERVICES:-}"; exit 0 ;; + --info-service=*) + name="${arg#--info-service=}" + for pair in ${FW_SERVICE_PORTS:-}; do + [[ "${pair%%:*}" == "$name" ]] && printf ' ports: %s\n' "${pair#*:}" + done + exit 0 ;; + --list-rich-rules) printf '%s\n' "${FW_RICH:-}"; exit 0 ;; + esac +done +exit 0 +STUB + +# Forwards firewall-cmd so the stub above sees the call; swallows everything +# else, stdin included, so `printf | sudo tee` cannot die of SIGPIPE here. +cat >"$bin/sudo" <<'STUB' +#!/usr/bin/env bash +case "${1:-}" in + firewall-cmd) shift; exec firewall-cmd "$@" ;; + *) cat >/dev/null 2>&1; exit 0 ;; +esac +STUB + +cat >"$bin/systemctl" <<'STUB' +#!/usr/bin/env bash +[[ "$*" == "is-active firewalld" ]] && exit "${FW_INACTIVE:-0}" +exit 0 +STUB + +cat >"$bin/loginctl" <<'STUB' +#!/usr/bin/env bash +printf 'Linger=yes\n' +STUB + +cat >"$bin/podman" <<'STUB' +#!/usr/bin/env bash +exit 0 +STUB + +chmod +x "$bin"/* + +# Runs the stage in a scenario and prints the firewall actions it took, in +# order, as a single space-separated line so a whole run compares in one go. +run_stage() { + local log="$work/fw.log" home="$work/home" state="$work/state" + rm -rf "$home" "$state"; mkdir -p "$home" "$state/panama" + : >"$log" + [[ -n "${DECISION:-}" ]] && printf '%s\n' "$DECISION" >"$state/panama/firewall" + env PATH="$bin:$PATH" HOME="$home" XDG_STATE_HOME="$state" \ + PANAMA_PATH="$repo_dir" FW_LOG="$log" \ + FW_OPEN="${FW_OPEN:-}" FW_SERVICES="${FW_SERVICES:-}" \ + FW_SERVICE_PORTS="${FW_SERVICE_PORTS:-}" FW_RICH="${FW_RICH:-}" \ + FW_INACTIVE="${FW_INACTIVE:-0}" \ + bash "$stage" "$work/out" 2>&1 + tr '\n' ' ' <"$log" | sed 's/ *$//' +} + +# Each scenario is one call: clear the environment it reads, set what it needs. +scenario() { unset FW_OPEN FW_SERVICES FW_SERVICE_PORTS FW_RICH FW_INACTIVE DECISION; } + +cloudflare_rules='rule family="ipv4" source ipset="cloudflare4" port port="80" protocol="tcp" accept +rule family="ipv4" source ipset="cloudflare4" port port="443" protocol="tcp" accept' + +# ── A machine with nothing granted gets all three ──────────────────────────── +scenario +got="$(run_stage)" +[[ "$got" == "add 80/tcp add 443/tcp add 81/tcp reload" ]] \ + || note "a bare machine did not get all three ports opened; got: $got" + +# ── Rich rules granting 80/443 are not widened ─────────────────────────────── +scenario; FW_RICH="$cloudflare_rules" +got="$(run_stage)" +[[ "$got" == "add 81/tcp reload" ]] \ + || note "a CDN-only rich rule on 80/443 was widened to a public port; got: $got" + +# ── An enabled service granting 80/443 is not widened ──────────────────────── +scenario; FW_SERVICES="http https"; FW_SERVICE_PORTS="http:80/tcp https:443/tcp" +got="$(run_stage)" +[[ "$got" == "add 81/tcp reload" ]] \ + || note "ports granted by an enabled service were opened again; got: $got" + +# ── Already-open ports are left alone, and nothing reloads ─────────────────── +scenario; FW_OPEN="80 443 81" +got="$(run_stage)" +[[ -z "$got" ]] \ + || note "a fully configured firewall was touched anyway; got: $got" + +# ── A drop rule is not a grant ─────────────────────────────────────────────── +scenario; FW_RICH='rule family="ipv4" port port="80" protocol="tcp" drop' +got="$(run_stage)" +[[ "$got" == "add 80/tcp add 443/tcp add 81/tcp reload" ]] \ + || note "a rich rule DROPPING 80 was read as granting it; got: $got" + +# ── A port range grants every port inside it ───────────────────────────────── +scenario; FW_RICH='rule family="ipv4" port port="79-82" protocol="tcp" accept' +got="$(run_stage)" +[[ "$got" == "add 443/tcp reload" ]] \ + || note "a rich rule granting 79-82 did not cover 80 and 81; got: $got" + +# ── A udp grant is not a tcp grant ─────────────────────────────────────────── +scenario; FW_RICH='rule family="ipv4" port port="80" protocol="udp" accept' +got="$(run_stage)" +[[ "$got" == "add 80/tcp add 443/tcp add 81/tcp reload" ]] \ + || note "a udp rich rule was read as granting tcp 80; got: $got" + +# ── The opt-out ────────────────────────────────────────────────────────────── +scenario; DECISION=manual +got="$(run_stage)" +[[ -z "$got" ]] \ + || note "the manual opt-out did not stop Panama touching firewalld; got: $got" + +grep -q 'Firewall left to this machine' "$work/out" \ + || note 'the manual opt-out does not say why it skipped the firewall' + +# ── An inactive firewalld is not an error ──────────────────────────────────── +scenario; FW_INACTIVE=1 +got="$(run_stage)" +[[ -z "$got" ]] \ + || note "firewalld reported inactive but ports were still touched; got: $got" + +# ── Report ─────────────────────────────────────────────────────────────────── +if (( ${#findings[@]} > 0 )); then + printf 'setup-server firewall contract FAILED:\n' >&2 + printf ' - %s\n' "${findings[@]}" >&2 + exit 1 +fi +printf 'setup-server firewall contract passed\n'