From b2526b9d62911d538b965e072147184f1f54fcc7 Mon Sep 17 00:00:00 2001 From: Andreas Wrede Date: Wed, 23 Sep 2026 22:36:17 -0400 Subject: [PATCH] deploy: harden mosquitto password_file guards (non-root, runuser errors, symlinks, traversal, all lines) Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KqhjP7nud5gdBg25zCiRk9 --- deploy.sh | 72 +++++++++++++++++++++++++++++++--------- tests/check-templates.sh | 48 ++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 16 deletions(-) diff --git a/deploy.sh b/deploy.sh index 70b92be..55b73b2 100755 --- a/deploy.sh +++ b/deploy.sh @@ -266,13 +266,41 @@ mosquitto_conf() { # path of mosquitto.conf; the test knob is honoured only whe echo "$c" } -mosquitto_pwfile_problem() { # [mosquitto.conf] — prints the password_file path when it exists but the mosquitto user cannot read it; else nothing - local conf=${1:-$(mosquitto_conf)} user=mosquitto pw +mosquitto_pwfile_paths() { # [mosquitto.conf] — every password_file value (mosquitto.conf + conf.d beside it), comments ignored, quotes stripped, absolute paths only + local conf=${1:-$(mosquitto_conf)} p + while IFS= read -r p; do + p=${p#\"}; p=${p%\"} + case "$p" in /*) echo "$p" ;; esac + done < <({ cat "$conf" "$(dirname "$conf")"/conf.d/*.conf 2>/dev/null || true; } | awk '/^[[:space:]]*password_file[[:space:]]/ { print $2 }') +} + +mosquitto_can_probe() { # runuser needs root; a test knob (VANLINK_MOSQUITTO_CONF/_USER) allows a stub runuser as non-root + [ "$(id -u)" = 0 ] || [ -n "${VANLINK_MOSQUITTO_CONF:-}${VANLINK_MOSQUITTO_USER:-}" ] +} + +mosquitto_pwfile_scan() { # [mosquitto.conf] — one line per problem: "unreadable " | "notraverse ". Silent when it cannot check. + local conf=${1:-$(mosquitto_conf)} user=mosquitto p f d rc + mosquitto_can_probe || return 0 [ "$(id -u)" = 0 ] || user=${VANLINK_MOSQUITTO_USER:-$user} - pw=$({ cat "$conf" "$(dirname "$conf")"/conf.d/*.conf 2>/dev/null || true; } | awk '/^[[:space:]]*password_file[[:space:]]/ { print $2 }' | tail -n1) || pw= - [ -n "$pw" ] && [ -e "$pw" ] || return 0 id "$user" >/dev/null 2>&1 && command -v runuser >/dev/null 2>&1 || return 0 - runuser -u "$user" -- test -r "$pw" >/dev/null 2>&1 || echo "$pw" + runuser -u "$user" -- true >/dev/null 2>&1 || return 0 # PAM/session trouble: cannot judge + while IFS= read -r p; do + f=$(readlink -f -- "$p" 2>/dev/null) || continue + [ -f "$f" ] || continue + d=$(dirname "$f") + rc=0; runuser -u "$user" -- test -x "$d" >/dev/null 2>&1 || rc=$? + if [ "$rc" != 0 ]; then + [ "$rc" -ge 125 ] && [ "$rc" -le 127 ] || echo "notraverse $d" + continue + fi + rc=0; runuser -u "$user" -- test -r "$f" >/dev/null 2>&1 || rc=$? + if [ "$rc" != 0 ] && { [ "$rc" -lt 125 ] || [ "$rc" -gt 127 ]; }; then echo "unreadable $f"; fi + done < <(mosquitto_pwfile_paths "$conf") + return 0 +} + +mosquitto_pwfile_problem() { # [mosquitto.conf] — prints each password_file (resolved) the mosquitto user cannot read; else nothing + mosquitto_pwfile_scan "$@" | sed -n 's/^unreadable //p' return 0 } @@ -363,14 +391,22 @@ post_check() { # read-only live checks; every probe is guarded, warns instead o connected) echo " ok van-router-mqtt: MQTT connection ok" ;; esac # g2b. mosquitto (the base router-status broker): unit active, password file readable by the broker's user - local mu mpw + local mu mpw mk mv mu=$(systemctl show -p ActiveState --value mosquitto 2>/dev/null) || mu= if [ "$mu" = active ]; then echo " ok unit mosquitto active" else warn "post-deploy: unit mosquitto is not active (ActiveState=${mu:-?})"; fi - mpw=$(mosquitto_pwfile_problem) - if [ -n "$mpw" ]; then - warn "post-deploy: mosquitto password file $mpw is not readable by the mosquitto user — the broker will fail to start on its next restart (fix: chown mosquitto:mosquitto $mpw; chmod 0600 $mpw)" - else echo " ok mosquitto password file readable (or none configured)"; fi + if ! mosquitto_can_probe; then + [ -z "$(mosquitto_pwfile_paths)" ] || echo " ok mosquitto password file: cannot verify as non-root (run with sudo)" + else + mpw=$(mosquitto_pwfile_scan) + if [ -z "$mpw" ]; then echo " ok mosquitto password file readable (or none configured)"; fi + while read -r mk mv; do + case "$mk" in + unreadable) warn "post-deploy: mosquitto password file $mv is not readable by the mosquitto user — the broker will fail to start on its next restart (fix: chown mosquitto:mosquitto $mv; chmod 0600 $mv)" ;; + notraverse) warn "post-deploy: mosquitto cannot traverse $mv to reach its password file" ;; + esac + done <<<"$mpw" + fi if [ "$ENABLE_LEDS" = 1 ]; then # g3. front LEDs (van-leds): each configured LED exists; the static ones carry the expected trigger. # The WAN LED follows the uplink, so its state is only reported, never warned about. @@ -1266,11 +1302,17 @@ systemctl daemon-reexec # reboot), deliberately not forced here (reconnecting a WAN flaps it). systemctl restart systemd-resolved || warn "systemd-resolved failed to restart (journalctl -xeu systemd-resolved)" # A password_file the mosquitto user cannot read (seen: root:root 0700) makes the restart fail and the broker stay down. -mq_pw=$(mosquitto_pwfile_problem /etc/mosquitto/mosquitto.conf) -if [ -n "$mq_pw" ]; then - chown mosquitto:mosquitto "$mq_pw" && chmod 0600 "$mq_pw" - warn "mosquitto password file $mq_pw was not readable by the mosquitto user — fixed ownership (mosquitto:mosquitto 0600)" -fi +while read -r mq_k mq_pw; do + case "$mq_k" in + unreadable) + if chown mosquitto:mosquitto "$mq_pw" && chmod 0600 "$mq_pw"; then + warn "mosquitto password file $mq_pw was not readable by the mosquitto user — fixed ownership (mosquitto:mosquitto 0600)" + else + warn "mosquitto password file $mq_pw was not readable by the mosquitto user and could NOT be fixed (do it manually: chown mosquitto:mosquitto $mq_pw; chmod 0600 $mq_pw)" + fi ;; + notraverse) warn "mosquitto cannot traverse $mq_pw to reach its password file" ;; + esac +done < <(mosquitto_pwfile_scan /etc/mosquitto/mosquitto.conf) systemctl restart mosquitto || warn "mosquitto failed to restart (journalctl -xeu mosquitto) — MQTT (Home Assistant, Li3, router status) is down" # networkd here owns only the AP radios + bridge + wired LAN port (no real uplink); # its wait-online would just stall network-online.target. NM-wait-online covers WANs. diff --git a/tests/check-templates.sh b/tests/check-templates.sh index d8dd2bf..0ba4c26 100755 --- a/tests/check-templates.sh +++ b/tests/check-templates.sh @@ -1055,7 +1055,18 @@ if awk '/^set_units\(\)/,0' deploy.sh | sed -n '/^echo "== apply =="/,/^post_che t23=$(mktemp -d); mkdir -p "$t23/etc/conf.d" "$t23/stub" "$t23/nostub" echo secret > "$t23/pw" printf '# password_file /nope\nlistener 1883\npassword_file %s\n' "$t23/pw" > "$t23/etc/mosquitto.conf" -stubru() { printf '#!/bin/sh\n# runuser -u USER -- test -r FILE\nexit %s\n' "$1" > "$t23/stub/runuser"; chmod +x "$t23/stub/runuser"; } +stubru() { # stubru [rc of `true`] [rc of test -x] [only this file fails test -r (default: all)] + cat > "$t23/stub/runuser" <&1; } stubru 1; o=$(mq "$t23/etc/mosquitto.conf") [ "$o" = "$t23/pw" ] && ok "23b: unreadable password file is reported (path only)" || bad "23b: unreadable: got '$o'" @@ -1082,6 +1093,41 @@ stubru 0; o=$(pcm) printf 'password_file %s\n' "$t23/pw" > "$t23/etc/mosquitto.conf" o=$(PATH="/usr/bin:/bin" VANLINK_MOSQUITTO_CONF="$t23/etc/mosquitto.conf" VANLINK_MOSQUITTO_USER=root POSTCHECK_WAIT=0 ./deploy.sh --post-check hosts/van.conf 2>&1 || true) ! echo "$o" | grep -q 'is not readable by the mosquitto user' && ok "23c: runuser missing -> no warn" || bad "23c: runuser missing warned" +# I3: several / quoted / symlinked password_file lines; runuser trouble reports nothing +echo secret2 > "$t23/pw2"; ln -sf "$t23/pw2" "$t23/link"; ln -sf "$t23/nowhere" "$t23/dangling"; mkdir -p "$t23/adir" +printf 'password_file %s\npassword_file %s\n' "$t23/pw" "$t23/pw2" > "$t23/etc/mosquitto.conf" +stubru 1 0 0 "$t23/pw"; o=$(mq "$t23/etc/mosquitto.conf") +[ "$o" = "$t23/pw" ] && ok "23b: two password_file lines, only the first unreadable -> that one" || bad "23b: two lines: got '$o'" +stubru 1 0 0 "$t23/pw2"; o=$(mq "$t23/etc/mosquitto.conf") +[ "$o" = "$t23/pw2" ] && ok "23b: the last line unreadable is found too" || bad "23b: last line: got '$o'" +printf 'password_file "%s"\n' "$t23/pw" > "$t23/etc/mosquitto.conf"; stubru 1; o=$(mq "$t23/etc/mosquitto.conf") +[ "$o" = "$t23/pw" ] && ok "23b: quoted path" || bad "23b: quoted: got '$o'" +printf 'password_file %s\n' "$t23/link" > "$t23/etc/mosquitto.conf"; o=$(mq "$t23/etc/mosquitto.conf") +[ "$o" = "$t23/pw2" ] && ok "23b: symlink -> resolved path reported" || bad "23b: symlink: got '$o'" +for pf in "$t23/dangling" "$t23/adir" "relative/pw"; do + printf 'password_file %s\n' "$pf" > "$t23/etc/mosquitto.conf"; o=$(mq "$t23/etc/mosquitto.conf") + [ -z "$o" ] && ok "23b: $(basename "$pf") (dangling/dir/relative) -> nothing" || bad "23b: $pf: got '$o'" +done +printf 'password_file %s\n' "$t23/pw" > "$t23/etc/mosquitto.conf" +stubru 1 125; o=$(mq "$t23/etc/mosquitto.conf"); [ -z "$o" ] && ok "23b: runuser -- true failing (125) -> nothing" || bad "23b: true fails: got '$o'" +stubru 126; o=$(mq "$t23/etc/mosquitto.conf"); [ -z "$o" ] && ok "23b: test -r exiting 126 -> nothing" || bad "23b: rc 126: got '$o'" +stubru 1 0 1; o=$(mq "$t23/etc/mosquitto.conf"); [ -z "$o" ] && ok "23b: untraversable dir -> not in the problem list (no chown)" || bad "23b: notraverse: got '$o'" +pcm() { PATH="$t23/stub:/usr/bin:/bin" VANLINK_MOSQUITTO_CONF="$t23/etc/mosquitto.conf" VANLINK_MOSQUITTO_USER=root POSTCHECK_WAIT=0 ./deploy.sh --post-check hosts/van.conf 2>&1 || true; } +o=$(pcm) +echo "$o" | grep -qF -- "-> post-deploy: mosquitto cannot traverse $t23 to reach its password file" && ok "23c: untraversable dir -> distinct warn" || bad "23c: no notraverse warn" +echo "$o" | grep -q 'is not readable by the mosquitto user' && bad "23c: notraverse also warned unreadable" || ok "23c: notraverse is not reported as unreadable" +# non-root without a knob: nothing, and the neutral line +stubru 1; o=$(PATH="$t23/stub:/usr/bin:/bin" ./deploy.sh --mosquitto-pwfile-check "$t23/etc/mosquitto.conf" 2>&1) +[ -z "$o" ] && ok "23b: non-root without a test knob -> nothing" || bad "23b: non-root: got '$o'" +o=$(PATH="$t23/stub:/usr/bin:/bin" POSTCHECK_WAIT=0 ./deploy.sh --post-check hosts/van.conf 2>&1 || true) +echo "$o" | grep -q 'is not readable by the mosquitto user' && bad "23c: non-root real run warned about the pw file" || ok "23c: non-root, no knob -> no pw warning" +o=$(bash -c '. <(sed -n "/^mosquitto_conf()/,/^post_check()/p" deploy.sh | sed "\$d"); mosquitto_conf() { echo "$1"; }; if mosquitto_can_probe; then echo probe; else [ -z "$(mosquitto_pwfile_paths "$1")" ] || echo NEUTRAL; fi' _ "$t23/etc/mosquitto.conf") +[ "$o" = NEUTRAL ] && ok "23c: non-root branch with a configured password_file takes the neutral path" || bad "23c: neutral branch: got '$o'" +grep -qF 'ok mosquitto password file: cannot verify as non-root (run with sudo)' deploy.sh && ok "23c: neutral line text present" || bad "23c: neutral line text missing" +# I2: fix path shape and order +if grep -qE '^ +if chown mosquitto:mosquitto "\$mq_pw" && chmod 0600 "\$mq_pw"; then' deploy.sh && grep -qF 'could NOT be fixed (do it manually' deploy.sh; then ok "23d: fix path is if/then/else with a NOT-fixed warn"; else bad "23d: fix path shape"; fi +c=$(grep -n '^ *if chown mosquitto:mosquitto' deploy.sh | cut -d: -f1); r=$(grep -n '^systemctl restart mosquitto' deploy.sh | cut -d: -f1) +[ -n "$c" ] && [ -n "$r" ] && [ "$c" -lt "$r" ] && ok "23d: chown block precedes the mosquitto restart" || bad "23d: chown/restart order ($c/$r)" rm -rf "$t23" exit "$fail"