From 6051dfe58625a7a23e2feb15f33bee9a42dd317f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 09:09:24 +0000 Subject: [PATCH 1/9] fix: no account of a built image can be authenticated before its first boot Three things the build leaves behind were more permissive than an appliance needs between the end of the build and the end of its first boot. conf/turnkey.d/rootpass, for a build given no ROOT_PASS, wrote a password field that a submitted password can match. It now writes one that no password matches. The value is deliberately not '!': shadow reads a field of exactly '!' as a locked account, and 'passwd --unlock root', which turnkey-sudoadmin runs on every first boot, refuses to unlock it, exits 3 and leaves the rest of that hook undone. Measured in a scratch root with 'unshare -r passwd --root'. The first boot writes over the field with chpasswd as it always did, which replaces it whatever it held. The same script expanded ROOT_PASS against the build directory before handing it to chpasswd, so a password holding a glob character was not the password that was set. conf/turnkey.d/webmin-pam is new and takes 'nullok' out of the PAM stack the webmin package installs. The option lets an account whose password is not set be authenticated without one; no appliance account is meant to be reachable that way and nothing in an appliance asked for it. The script fails rather than report success if the stack is absent or if the option survives. conf/turnkey.d/webmin-enable holds the web interface back until the first boot scripts have run. webmin.service gains a condition on the marker inithooks writes when its boot scripts are done, and a path unit starts it when that marker appears. The conditions are systemd's triggering conditions, an OR, so an image carrying no inithooks at all is not left without a web interface; the path unit triggers a one shot that stays active, so it fires once and does not restart Webmin behind an administrator who stopped it. Tests, 36 bats over the four measured conf scripts, assert behaviour and not configuration. Whether a password gets in is answered by the real crypt(3), the function pam_unix compares with, inside the two rules Linux-PAM applies around it; those rules were confirmed against unix_chkpwd, the helper pam_unix runs, with a scratch shadow file bind mounted over /etc/shadow under 'unshare -r -m'. Whether the web interface would start is answered by 'systemd-analyze condition', and the units it writes are handed to 'systemd-analyze verify'. The shadow tools are stubbed because usermod, chpasswd and passwd chroot into the root they are given and cannot be aimed at a scratch tree by an ordinary user; each stub carries the behaviour it reproduces and the command that measured it. tests/before-firstboot.bats runs the three scripts over one image in the order a build runs them and asks the whole question of the result, because no single one of them owns the answer. tests/coverage.sh measures a list of files rather than one. rootpass moved from /bin/sh to /bin/bash so that kcov can measure it, and uses no bash construct. Closes #11. --- conf/turnkey.d/rootpass | 18 +++-- conf/turnkey.d/webmin-enable | 51 ++++++++++++- conf/turnkey.d/webmin-pam | 24 +++++++ tests/before-firstboot.bats | 84 ++++++++++++++++++++++ tests/coverage.sh | 68 ++++++++++++++---- tests/fixtures/pam.d-webmin | 4 ++ tests/helpers.bash | 135 +++++++++++++++++++++++++++++++++++ tests/rootpass.bats | 97 +++++++++++++++++++++++++ tests/stubs/chpasswd | 16 +++++ tests/stubs/passwd | 46 ++++++++++++ tests/stubs/usermod | 23 ++++++ tests/webmin-enable.bats | 85 ++++++++++++++++++++++ tests/webmin-pam.bats | 90 +++++++++++++++++++++++ 13 files changed, 721 insertions(+), 20 deletions(-) create mode 100755 conf/turnkey.d/webmin-pam create mode 100644 tests/before-firstboot.bats create mode 100644 tests/fixtures/pam.d-webmin create mode 100644 tests/helpers.bash create mode 100644 tests/rootpass.bats create mode 100755 tests/stubs/chpasswd create mode 100755 tests/stubs/passwd create mode 100755 tests/stubs/usermod create mode 100644 tests/webmin-enable.bats create mode 100644 tests/webmin-pam.bats diff --git a/conf/turnkey.d/rootpass b/conf/turnkey.d/rootpass index d9a0f2ff..306a6422 100755 --- a/conf/turnkey.d/rootpass +++ b/conf/turnkey.d/rootpass @@ -1,12 +1,22 @@ -#!/bin/sh -e +#!/bin/bash -e + +# The password field written into an image the build gives no root password +# to. crypt() returns no such string for any input, so nothing a caller can +# submit matches it and the account cannot be authenticated until the first +# boot sets a password over it with chpasswd. +# +# Deliberately not '!': shadow reads a field of exactly '!' as a locked +# account, and 'passwd --unlock root', which turnkey-sudoadmin runs on every +# first boot, then refuses it ("unlocking the password would result in a +# passwordless account"), exits 3 and leaves the rest of that hook undone. +NO_PASSWORD='*' # chroot only environments don't need a root password if [ ! "$CHROOT_ONLY" ]; then if [ -n "$ROOT_PASS" ]; then - echo root:$ROOT_PASS | chpasswd + echo "root:$ROOT_PASS" | chpasswd else - # this is a blank password - usermod -p U6aMy0wojraho root + usermod -p "$NO_PASSWORD" root fi else passwd --lock root diff --git a/conf/turnkey.d/webmin-enable b/conf/turnkey.d/webmin-enable index d8563ecb..22964e33 100755 --- a/conf/turnkey.d/webmin-enable +++ b/conf/turnkey.d/webmin-enable @@ -1,3 +1,52 @@ -#!/bin/sh -e +#!/bin/bash -e + +# An appliance is configured by its first boot scripts, and a web interface +# is not something it needs before then. So the unit does not start with the +# rest of the boot: it starts once inithooks reports that its boot scripts +# have run. +# +# The two conditions are triggering conditions, which systemd reads as an +# OR: the marker is there, or the image carries no inithooks at all and +# there is nothing to wait for. The path unit starts Webmin when the marker +# appears; it triggers a one shot that stays active afterwards, so it fires +# once and does not restart Webmin behind an administrator who stopped it. +# +# SYSTEMD_DIR, INITHOOKS_MARKER and INITHOOKS_RUN are test hooks; a build +# leaves them unset. + +SYSTEMD_DIR=${SYSTEMD_DIR:-/etc/systemd/system} +INITHOOKS_MARKER=${INITHOOKS_MARKER:-/run/inithooks-complete} +INITHOOKS_RUN=${INITHOOKS_RUN:-/usr/lib/inithooks/run} + +mkdir -p "$SYSTEMD_DIR/webmin.service.d" + +cat > "$SYSTEMD_DIR/webmin.service.d/after-firstboot.conf" < "$SYSTEMD_DIR/webmin-after-firstboot.path" < "$SYSTEMD_DIR/webmin-after-firstboot.service" <&2 + exit 1 +} + +[[ -f "$PAM_WEBMIN" ]] || fatal "$PAM_WEBMIN does not exist" + +sed -i -E 's/[[:space:]]+nullok(_secure)?\b//g' "$PAM_WEBMIN" + +if grep -qE '\bnullok' "$PAM_WEBMIN"; then + fatal "nullok survived in $PAM_WEBMIN" +fi diff --git a/tests/before-firstboot.bats b/tests/before-firstboot.bats new file mode 100644 index 00000000..d8f7d020 --- /dev/null +++ b/tests/before-firstboot.bats @@ -0,0 +1,84 @@ +#!/usr/bin/env bats +# What a freshly built image lets through between the end of the build and +# the end of the first boot, when no password has been set on it yet. +# +# The conf scripts that decide it are run over one scratch image in the +# order the build runs them, and the image is then asked the only question +# worth asking of it: can anybody get in. +# +# No single script owns the answer. rootpass decides what the account +# carries, webmin-pam decides what the stack makes of it and webmin-enable +# decides whether the interface is listening at all, so they are tested +# together as well as apart. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + CONF_DIR="$TESTS_DIR/../conf/turnkey.d" + scratch_image + + export INITHOOKS_MARKER=$IMAGE/run/inithooks-complete + export INITHOOKS_RUN=$IMAGE/usr/lib/inithooks/run + mkdir -p "$IMAGE/run" "$IMAGE/usr/lib/inithooks" + touch "$INITHOOKS_RUN" + + DROPIN=$SYSTEMD_DIR/webmin.service.d/after-firstboot.conf + unset CHROOT_ONLY ROOT_PASS +} + +# build_image +# Runs the conf scripts of this change in the order fab runs a conf +# directory, which is the order their names sort in. +build_image() { + local script + for script in $(printf '%s\n' rootpass webmin-enable webmin-pam | sort); do + "$CONF_DIR/$script" + done +} + +@test "no password gets in before the first boot has set one" { + build_image + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "and the interface is not up to be asked" { + build_image + run ! unit_would_start "$DROPIN" +} + +@test "and it stays that way for as long as the first boot takes" { + build_image + run ! unit_would_start "$DROPIN" + run nothing_authenticates + [ "$status" -eq 0 ] + + passwd --unlock root + run nothing_authenticates + [ "$status" -eq 0 ] + run ! unit_would_start "$DROPIN" +} + +@test "and opens when the first boot has set a password" { + build_image + echo 'root:hunter2' | chpasswd + touch "$INITHOOKS_MARKER" + + run unit_would_start "$DROPIN" + [ "$status" -eq 0 ] + run authenticates "hunter2" + [ "$status" -eq 0 ] + run ! authenticates "" +} + +@test "a build given a root password is held shut the same way" { + export ROOT_PASS=s3cret + build_image + run ! unit_would_start "$DROPIN" + run authenticates "s3cret" + [ "$status" -eq 0 ] + run ! authenticates "" +} diff --git a/tests/coverage.sh b/tests/coverage.sh index e6a880b2..4194477c 100755 --- a/tests/coverage.sh +++ b/tests/coverage.sh @@ -1,7 +1,7 @@ #!/bin/bash -# Line coverage of conf/turnkey.d/postfix-local under the bats suite -# in this directory, measured with kcov. Exits 1 when the covered share of -# the script is below the threshold (default 95), 2 when a tool is missing. +# Line coverage of the conf scripts this repository has tests for, measured +# with kcov over the bats suite in this directory. Exits 1 when any measured +# file is below the threshold (default 95), 2 when a tool is missing. # # tests/coverage.sh [THRESHOLD] (or COVERAGE_THRESHOLD in the environment) # @@ -10,9 +10,18 @@ set -euo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -target="$(cd "$here/.." && pwd)/conf/turnkey.d/postfix-local" +repo="$(cd "$here/.." && pwd)" threshold="${1:-${COVERAGE_THRESHOLD:-95}}" +# Every file the suite is expected to cover. A file that is not listed is +# not measured, so a new script arrives here with its tests. +measured=( + conf/turnkey.d/postfix-local + conf/turnkey.d/rootpass + conf/turnkey.d/webmin-enable + conf/turnkey.d/webmin-pam +) + for tool in kcov bats; do if ! command -v "$tool" >/dev/null; then echo "$tool not found (apt-get install $tool)" >&2 @@ -20,18 +29,47 @@ for tool in kcov bats; do fi done -report="${COVERAGE_DIR:-$(mktemp -d)}" -kcov --include-path="$target" "$report" bats "$here" +include="" +for file in "${measured[@]}"; do + if [[ ! -f "$repo/$file" ]]; then + echo "measured file is missing: $file" >&2 + exit 2 + fi + include="${include:+$include,}$repo/$file" +done -# kcov names the sub directory after the command; with --include-path the -# report holds one file, so its first entry is ours -json="$(ls -t "$report"/*/coverage.json | grep -v /kcov-merged/ | head -1)" -percent="$(grep -o '"percent_covered": "[0-9.]*"' "$json" | head -1 | grep -o '[0-9.]*')" -covered="$(grep -o '"covered_lines": "[0-9]*"' "$json" | head -1 | grep -o '[0-9]*')" -total="$(grep -o '"total_lines": "[0-9]*"' "$json" | head -1 | grep -o '[0-9]*')" +report="${COVERAGE_DIR:-$(mktemp -d)}" +kcov --include-path="$include" "$report" bats "$here" -echo "postfix-local: $percent percent ($covered of $total lines) covered, threshold $threshold" -if ! awk -v p="$percent" -v t="$threshold" 'BEGIN { exit !(p + 0 >= t + 0) }'; then - echo "coverage below threshold (report: $report)" >&2 +json="$(find "$report" -name coverage.json -not -path '*/kcov-merged/*' | head -1)" +if [[ -z "$json" ]]; then + echo "coverage.sh: no coverage.json under $report" >&2 exit 1 fi + +# kcov writes one line per measured file: +# {"file": "PATH", "percent_covered": "P", "covered_lines": "C", ...}, +echo +echo "kcov line coverage (threshold $threshold percent):" +awk -F'"' -v threshold="$threshold" -v repo="$repo/" -v want="${#measured[@]}" ' + /^ *\{"file":/ { + file = $4 + sub(repo, "", file) + percent = $8 + 0 + seen++ + mark = (percent >= threshold) ? "ok" : "BELOW THRESHOLD" + if (percent < threshold) { + low = 1 + } + printf "%7.2f %4s/%-4s %-32s %s\n", percent, $12, $16, file, mark + } + /^ "percent_covered":/ { + printf "%7.2f total\n", $4 + 0 + } + END { + if (seen != want) { + printf "measured %d files, expected %d\n", seen, want > "/dev/stderr" + exit 1 + } + exit low + }' "$json" diff --git a/tests/fixtures/pam.d-webmin b/tests/fixtures/pam.d-webmin new file mode 100644 index 00000000..4bb7cb8a --- /dev/null +++ b/tests/fixtures/pam.d-webmin @@ -0,0 +1,4 @@ +#%PAM-1.0 +auth required pam_unix.so nullok +account required pam_unix.so +session required pam_unix.so diff --git a/tests/helpers.bash b/tests/helpers.bash new file mode 100644 index 00000000..3191615b --- /dev/null +++ b/tests/helpers.bash @@ -0,0 +1,135 @@ +# Shared helpers for the conf scripts that decide whether an account of a +# freshly built image can be authenticated. +# +# The verdict these tests take is the one pam_unix takes, and it is taken +# the way pam_unix takes it: +# +# crypt() is the real one, reached through perl. It is the function +# pam_unix compares a submitted password with. +# +# Around it, two rules out of Linux-PAM. _unix_blankpasswd() answers that +# an account has no password when its stored field is empty or when +# crypt() returns that field for the empty string, and it answers so only +# while nullok is in effect; _unix_verify_password() then either accepts +# without looking at what was submitted, or compares. The rules were +# confirmed against the real helper, which is the code pam_unix runs: +# +# printf '%s' "$password" | /usr/sbin/unix_chkpwd root nullok|nonull +# +# with the stored field bind mounted over /etc/shadow under +# 'unshare -r -m'. Accepts and refusals both reproduced, for the fields +# '', 'U6aMy0wojraho', '*' and '!'. +# +# The shadow tools are stubs, because usermod, chpasswd and passwd chroot +# into the root they are given and so cannot touch a scratch tree as an +# ordinary user. Each stub carries the behaviour it reproduces, and the +# command that measured it, at the top of the file. + +HELPERS_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +FIXTURES="$HELPERS_DIR/fixtures" + +# scratch_image +# The part of an image under construction that these scripts touch: the +# account databases, the PAM stack the webmin package installs and an empty +# systemd tree. Puts the stubs first on PATH and points every test hook at +# the scratch tree. +# +# root starts out carrying the field an earlier build wrote, so a script +# that writes nothing at all fails these tests instead of passing on the +# state it was handed. +scratch_image() { + IMAGE=$BATS_TEST_TMPDIR/image + mkdir -p "$IMAGE/etc/pam.d" "$IMAGE/etc/systemd/system" + printf 'root:x:0:0:root:/root:/bin/bash\n' > "$IMAGE/etc/passwd" + printf 'root:U6aMy0wojraho:20718:0:99999:7:::\n' > "$IMAGE/etc/shadow" + cp "$FIXTURES/pam.d-webmin" "$IMAGE/etc/pam.d/webmin" + + export PATH="$HELPERS_DIR/stubs:$PATH" + export STUB_LOG=$BATS_TEST_TMPDIR/calls.log + : > "$STUB_LOG" + + export SHADOW_FILE=$IMAGE/etc/shadow + export PAM_WEBMIN=$IMAGE/etc/pam.d/webmin + export SYSTEMD_DIR=$IMAGE/etc/systemd/system +} + +# crypt_of PASSWORD SETTING +# The real crypt(3). Prints what it returns, which is never the setting +# itself for a setting no hash can be built from. +crypt_of() { + perl -e 'print crypt($ARGV[0], $ARGV[1]) // ""' "$1" "$2" +} + +# field_of USER +# The password field USER carries in the scratch image. +field_of() { + awk -F: -v user="${1:-root}" '$1 == user { print $2 }' "${SHADOW_FILE:?}" +} + +# stack_allows_blank [PAM_FILE] +# Whether nullok is in effect for pam_unix on an auth line of the stack, +# which is what decides if a password that is not set authenticates. +stack_allows_blank() { + grep -E '^[[:space:]]*auth[[:space:]]' "${1:-$PAM_WEBMIN}" \ + | grep -qE '\bnullok(_secure)?\b' +} + +# blank_equivalent FIELD +# Linux-PAM's _unix_blankpasswd: a field that is empty, or that crypt() +# returns for the empty string, is an account whose password is not set. +blank_equivalent() { + if [[ -z "$1" ]]; then + return 0 + fi + [[ "$(crypt_of "" "$1")" == "$1" ]] +} + +# authenticates PASSWORD [USER] [PAM_FILE] +# Succeeds when pam_unix would let PASSWORD in for USER through that stack, +# fails when it would not. +authenticates() { + local password=$1 + local user=${2:-root} + local pam_file=${3:-$PAM_WEBMIN} + local field + field=$(field_of "$user") + + if stack_allows_blank "$pam_file" && blank_equivalent "$field"; then + return 0 + fi + if [[ -z "$field" ]]; then + return 1 + fi + [[ "$(crypt_of "$password" "$field")" == "$field" ]] +} + +# nothing_authenticates [USER] +# Succeeds when no password at all gets USER in: the empty one, the ones +# worth trying first, and a right looking one. +nothing_authenticates() { + local user=${1:-root} + local password + for password in "" " " "root" "toor" "password" "turnkey" "*" "!" \ + "U6aMy0wojraho" "hunter2"; do + if authenticates "$password" "$user"; then + echo "authenticated with '$password' against field '$(field_of "$user")'" >&2 + return 1 + fi + done +} + +# conditions_of UNIT_FILE +# The Condition lines of a unit file, as arguments for systemd-analyze. +conditions_of() { + grep -E '^Condition' "$1" +} + +# unit_would_start UNIT_FILE +# systemd's own verdict on whether a unit with those conditions would run, +# taken from systemd-analyze, not from reading the file. +unit_would_start() { + local -a conditions + readarray -t conditions < <(conditions_of "$1") + [[ ${#conditions[@]} -gt 0 ]] || return 0 + systemd-analyze condition "${conditions[@]}" >/dev/null +} diff --git a/tests/rootpass.bats b/tests/rootpass.bats new file mode 100644 index 00000000..f97205bf --- /dev/null +++ b/tests/rootpass.bats @@ -0,0 +1,97 @@ +#!/usr/bin/env bats +# Tests for conf/turnkey.d/rootpass: what a caller can get in with, in the +# image the script leaves behind. +# +# The verdict is pam_unix's, over the real crypt(); the shadow tools are +# stubs. See helpers.bash. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + SCRIPT="$TESTS_DIR/../conf/turnkey.d/rootpass" + scratch_image + unset CHROOT_ONLY ROOT_PASS +} + +@test "no password gets root in when the build sets none" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "the empty password is refused although the stack still allows a blank one" { + stack_allows_blank + run "$SCRIPT" + [ "$status" -eq 0 ] + run ! authenticates "" +} + +@test "the field left behind is one passwd --unlock accepts" { + run "$SCRIPT" + [ "$status" -eq 0 ] + before="$(field_of root)" + run passwd --unlock root + [ "$status" -eq 0 ] + [ "$(field_of root)" = "$before" ] + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "a field of '!' is the one passwd --unlock refuses, which is why it is not used" { + printf 'root:!:20718:0:99999:7:::\n' > "$SHADOW_FILE" + run passwd --unlock root + [ "$status" -eq 3 ] + [[ "$output" == *"passwordless account"* ]] + [ "$(field_of root)" = '!' ] +} + +@test "the password the first boot sets over it works" { + run "$SCRIPT" + [ "$status" -eq 0 ] + echo 'root:hunter2' | chpasswd + run authenticates "hunter2" + [ "$status" -eq 0 ] + run ! authenticates "" + run ! authenticates "hunter3" +} + +@test "a build time ROOT_PASS is the password, and nothing else is" { + export ROOT_PASS=s3cret + run "$SCRIPT" + [ "$status" -eq 0 ] + run authenticates "s3cret" + [ "$status" -eq 0 ] + run ! authenticates "" +} + +@test "a ROOT_PASS holding a glob character is set as written" { + mkdir -p "$IMAGE/build" + touch "$IMAGE/build/root:pass" + cd "$IMAGE/build" + export ROOT_PASS='p*ss' + run "$SCRIPT" + [ "$status" -eq 0 ] + run authenticates 'p*ss' + [ "$status" -eq 0 ] + run ! authenticates 'pass' +} + +@test "a chroot only build locks the account instead" { + export CHROOT_ONLY=y + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(cat "$STUB_LOG")" = "passwd --lock root" ] + [[ "$(field_of root)" == '!'* ]] + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "the account database is touched once, with usermod" { + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(cat "$STUB_LOG")" = "usermod -p * root" ] +} diff --git a/tests/stubs/chpasswd b/tests/stubs/chpasswd new file mode 100755 index 00000000..1902faa7 --- /dev/null +++ b/tests/stubs/chpasswd @@ -0,0 +1,16 @@ +#!/bin/bash +# Stand-in for chpasswd: records the call in STUB_LOG, reads 'user:password' +# lines on standard input and replaces the whole password field of +# SHADOW_FILE with a hash of the password, which is what chpasswd does to a +# field whatever that field held before. +echo "chpasswd $*" >> "${STUB_LOG:?}" + +while IFS= read -r line; do + user=${line%%:*} + password=${line#*:} + hash=$(perl -e 'print crypt($ARGV[0], $ARGV[1])' "$password" '$6$keelstub$') + awk -F: -v user="$user" -v hash="$hash" 'BEGIN { OFS=":" } + $1 == user { $2 = hash } + { print }' "${SHADOW_FILE:?}" > "$SHADOW_FILE.new" + mv "$SHADOW_FILE.new" "$SHADOW_FILE" +done diff --git a/tests/stubs/passwd b/tests/stubs/passwd new file mode 100755 index 00000000..dec3975f --- /dev/null +++ b/tests/stubs/passwd @@ -0,0 +1,46 @@ +#!/bin/bash +# Stand-in for passwd --lock and passwd --unlock, with the behaviour of +# shadow 4.17 measured in a scratch root: +# +# for f in '!' '*' 'U6aMy0wojraho' '!U6aMy0wojraho' ''; do ... \ +# unshare -r passwd --root "$dir" -u root; done +# +# field '!' -u exit 3, field unchanged, "unlocking the +# password would result in a passwordless +# account" +# field '*' -u exit 0, field unchanged +# field '!' -u exit 0, field '' +# any field -l exit 0, '!' prefixed unless already there +echo "passwd $*" >> "${STUB_LOG:?}" + +action= +user= +while [[ $# -gt 0 ]]; do + case $1 in + -l|--lock) action=lock; shift ;; + -u|--unlock) action=unlock; shift ;; + -*) shift ;; + *) user=$1; shift ;; + esac +done + +field=$(awk -F: -v user="${user:?}" '$1 == user { print $2 }' "${SHADOW_FILE:?}") + +case $action in + lock) + [[ "$field" == '!'* ]] || field="!$field" + ;; + unlock) + if [[ "$field" == '!' ]]; then + echo "passwd: unlocking the password would result in a passwordless account." >&2 + exit 3 + fi + field=${field#!} + ;; + *) exit 1 ;; +esac + +awk -F: -v user="$user" -v field="$field" 'BEGIN { OFS=":" } + $1 == user { $2 = field } + { print }' "$SHADOW_FILE" > "$SHADOW_FILE.new" +mv "$SHADOW_FILE.new" "$SHADOW_FILE" diff --git a/tests/stubs/usermod b/tests/stubs/usermod new file mode 100755 index 00000000..baa66044 --- /dev/null +++ b/tests/stubs/usermod @@ -0,0 +1,23 @@ +#!/bin/bash +# Stand-in for usermod: records the call in STUB_LOG and, for '-p FIELD +# USER', writes FIELD into the password field of SHADOW_FILE exactly as +# usermod does, without interpreting it. +echo "usermod $*" >> "${STUB_LOG:?}" + +field= +user= +while [[ $# -gt 0 ]]; do + case $1 in + -p) field=$2; shift 2 ;; + -*) shift ;; + *) user=$1; shift ;; + esac +done + +[[ -n "$user" ]] || exit 1 +[[ -n "$field" ]] || exit 0 + +awk -F: -v user="$user" -v field="$field" 'BEGIN { OFS=":" } + $1 == user { $2 = field } + { print }' "${SHADOW_FILE:?}" > "$SHADOW_FILE.new" +mv "$SHADOW_FILE.new" "$SHADOW_FILE" diff --git a/tests/webmin-enable.bats b/tests/webmin-enable.bats new file mode 100644 index 00000000..e425bd1d --- /dev/null +++ b/tests/webmin-enable.bats @@ -0,0 +1,85 @@ +#!/usr/bin/env bats +# Tests for conf/turnkey.d/webmin-enable: when the web interface of a +# freshly built image is allowed to start. +# +# The verdict on the conditions is systemd's own, taken from +# 'systemd-analyze condition'; the unit files are handed to +# 'systemd-analyze verify' so that systemd, and not a reader, says they +# parse. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + SCRIPT="$TESTS_DIR/../conf/turnkey.d/webmin-enable" + scratch_image + + export INITHOOKS_MARKER=$IMAGE/run/inithooks-complete + export INITHOOKS_RUN=$IMAGE/usr/lib/inithooks/run + mkdir -p "$IMAGE/run" "$IMAGE/usr/lib/inithooks" + touch "$INITHOOKS_RUN" + + DROPIN=$SYSTEMD_DIR/webmin.service.d/after-firstboot.conf +} + +@test "webmin does not start while the first boot scripts have not run" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run ! unit_would_start "$DROPIN" +} + +@test "webmin starts once they report they have" { + run "$SCRIPT" + [ "$status" -eq 0 ] + touch "$INITHOOKS_MARKER" + run unit_would_start "$DROPIN" + [ "$status" -eq 0 ] +} + +@test "an image with no inithooks is not left without a web interface" { + rm "$INITHOOKS_RUN" + run "$SCRIPT" + [ "$status" -eq 0 ] + run unit_would_start "$DROPIN" + [ "$status" -eq 0 ] +} + +@test "systemd accepts the units the script writes" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run systemd-analyze verify \ + "$SYSTEMD_DIR/webmin-after-firstboot.path" \ + "$SYSTEMD_DIR/webmin-after-firstboot.service" + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +@test "the marker is what the path unit waits for, and it fires once" { + run "$SCRIPT" + [ "$status" -eq 0 ] + unit=$SYSTEMD_DIR/webmin-after-firstboot.path + grep -qx "PathExists=$INITHOOKS_MARKER" "$unit" + grep -qx "Unit=webmin-after-firstboot.service" "$unit" + grep -qx "RemainAfterExit=yes" \ + "$SYSTEMD_DIR/webmin-after-firstboot.service" +} + +@test "webmin and the path unit are the two units enabled" { + run "$SCRIPT" + [ "$status" -eq 0 ] + diff - "$STUB_LOG" <<'EXPECTED' +systemctl enable webmin +systemctl enable webmin-after-firstboot.path +EXPECTED +} + +@test "running it again leaves the same image" { + run "$SCRIPT" + [ "$status" -eq 0 ] + first="$(cat "$DROPIN" "$SYSTEMD_DIR"/webmin-after-firstboot.*)" + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(cat "$DROPIN" "$SYSTEMD_DIR"/webmin-after-firstboot.*)" = "$first" ] +} diff --git a/tests/webmin-pam.bats b/tests/webmin-pam.bats new file mode 100644 index 00000000..e59b81da --- /dev/null +++ b/tests/webmin-pam.bats @@ -0,0 +1,90 @@ +#!/usr/bin/env bats +# Tests for conf/turnkey.d/webmin-pam: what the PAM stack webmin uses lets +# through, before and after the script has run over it. +# +# tests/fixtures/pam.d-webmin is the file the webmin package installs, as +# it stands in a built core layer. The scratch image account has no password +# set on it, which is the case the option under test decides. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + SCRIPT="$TESTS_DIR/../conf/turnkey.d/webmin-pam" + scratch_image +} + +@test "a wrong password stops getting in" { + run authenticates "anything at all" + [ "$status" -eq 0 ] + + run "$SCRIPT" + [ "$status" -eq 0 ] + + run ! authenticates "anything at all" +} + +@test "what is left is the field itself, which is rootpass's half" { + run "$SCRIPT" + [ "$status" -eq 0 ] + + run authenticates "" + [ "$status" -eq 0 ] + + printf 'root:*:20718:0:99999:7:::\n' > "$SHADOW_FILE" + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "the right password still gets in" { + run "$SCRIPT" + [ "$status" -eq 0 ] + echo 'root:hunter2' | chpasswd + run authenticates "hunter2" + [ "$status" -eq 0 ] +} + +@test "the option goes whatever separates it from the module" { + printf '#%%PAM-1.0\nauth required pam_unix.so nullok try_first_pass\n' \ + > "$PAM_WEBMIN" + run "$SCRIPT" + [ "$status" -eq 0 ] + run ! stack_allows_blank + [ "$(sed -n '2p' "$PAM_WEBMIN")" = "auth required pam_unix.so try_first_pass" ] +} + +@test "nothing else in the stack is touched" { + run "$SCRIPT" + [ "$status" -eq 0 ] + diff - "$PAM_WEBMIN" <<'EXPECTED' +#%PAM-1.0 +auth required pam_unix.so +account required pam_unix.so +session required pam_unix.so +EXPECTED +} + +@test "running it again changes nothing" { + run "$SCRIPT" + [ "$status" -eq 0 ] + first="$(cat "$PAM_WEBMIN")" + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(cat "$PAM_WEBMIN")" = "$first" ] +} + +@test "fails when the stack is not there" { + rm "$PAM_WEBMIN" + run "$SCRIPT" + [ "$status" -eq 1 ] + [ "$output" = "FATAL [webmin-pam]: $PAM_WEBMIN does not exist" ] +} + +@test "fails rather than report success when the option survives" { + printf '#%%PAM-1.0\nnullok\n' > "$PAM_WEBMIN" + run "$SCRIPT" + [ "$status" -eq 1 ] + [ "$output" = "FATAL [webmin-pam]: nullok survived in $PAM_WEBMIN" ] +} From d9dcbea1f05cbf23c4ab62a2842b8bcfec0228ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 09:09:25 +0000 Subject: [PATCH 2/9] docs: changelog and coverage for the first boot account hardening The core changelog gains a bullet per change: the password field a build with no ROOT_PASS leaves, the glob expansion of a build time ROOT_PASS, the nullok removal and the wait on the first boot scripts. COVERAGE.md records the branch: 100 percent over four measured files, 39 of 39 lines under kcov 43, 36 bats, with what is measured for real and what is stubbed spelled out. The gate stays at 100. --- COVERAGE.md | 42 +++++++++++++++++++++++++++++++++++++++ changes/turnkey.changelog | 23 +++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/COVERAGE.md b/COVERAGE.md index 0895e226..5908ea93 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,6 +4,48 @@ Measured on 2026-09-24 against upstream 19.x (b60dd23), following the project decision 0003 (90 percent floor per repository, 95 percent for every file our changes touch). +## Branch fix/webmin-auth-hardening: 100 percent, four files (2026-09-28) + +`tests/coverage.sh` now measures a list of files rather than one, and every +file this branch touches is on it. Measured with kcov 43 over 36 bats: + +| File | Lines | Covered | Percent | +|------|-------|---------|---------| +| conf/turnkey.d/postfix-local | 17 | 17 | 100 | +| conf/turnkey.d/rootpass | 6 | 6 | 100 | +| conf/turnkey.d/webmin-enable | 9 | 9 | 100 | +| conf/turnkey.d/webmin-pam | 7 | 7 | 100 | +| Total | 39 | 39 | 100 | + +`conf/turnkey.d/rootpass` and `conf/turnkey.d/webmin-enable` changed shebang +from `/bin/sh` to `/bin/bash`, because kcov measures bash and not dash and +decision 0003 gives no exemption for a file a change touches. rootpass uses +no bash construct; webmin-enable is new code in a file that was one line. + +The gate in `.github/workflows/tests.yml` stays at 100, the measured number. + +Every exit path is covered: `rootpass` in its three branches (a build +password, no build password, chroot only), `webmin-pam` on both of its +`fatal` paths and the happy one, `webmin-enable` on its single path with the +three verdicts systemd gives its conditions. + +The suite asserts behaviour and not configuration. The question "can this +password get in" is answered by the real `crypt(3)` through perl, which is +the function pam_unix compares with, wrapped in the two rules Linux-PAM +applies around it; those rules were confirmed against `unix_chkpwd`, the +helper pam_unix itself runs, with a scratch shadow file bind mounted over +`/etc/shadow` under `unshare -r -m`. The verdict on whether the web +interface would start is `systemd-analyze condition`, systemd's own +evaluator, and the units are handed to `systemd-analyze verify`. The shadow +tools are the one thing stubbed, because `usermod`, `chpasswd` and `passwd` +chroot into the root they are given and cannot be pointed at a scratch tree +by an ordinary user; each stub records the behaviour it reproduces and the +command that measured it. + +`tests/before-firstboot.bats` runs the three conf scripts over one scratch +image in the order a build runs them and asks the whole question of the +result, because no single script owns the answer. + ## Measured baseline on the default branch: 100 percent (2026-09-26) Pull request #2 merged on 2026-09-26 (merge commit 5a0a381) and brought diff --git a/changes/turnkey.changelog b/changes/turnkey.changelog index 20fcef6f..10886318 100644 --- a/changes/turnkey.changelog +++ b/changes/turnkey.changelog @@ -1,5 +1,28 @@ turnkey-core-19.0 (1) turnkey; urgency=low + * An appliance no longer carries an account that can be authenticated + before its first boot has set a password. A build given no 'ROOT_PASS' + wrote a password field into the root account that a submitted password + could match; it now writes one that no password matches, and the first + boot writes over it as before. The field is deliberately not '!', which + shadow reads as a locked account: 'passwd --unlock root', which + turnkey-sudoadmin runs on every first boot, refuses to unlock a field of + exactly '!', exits 3 and leaves the rest of that hook undone. + + * A build time 'ROOT_PASS' holding a glob character is now set as written. + It was expanded against the build directory before reaching chpasswd. + + * 'nullok' is removed from the PAM stack the webmin package installs, in + the new 'webmin-pam' conf script. It allowed an account whose password + is not set to be authenticated without one, which no appliance account + is meant to be, and nothing in an appliance asked for it. + + * Webmin waits for the first boot scripts before it starts. Its unit gains + a condition on the marker inithooks writes when its boot scripts have + run, and a path unit starts it when that marker appears, so the web + interface is not exposed before the appliance has been configured. An + image carrying no inithooks at all is unaffected. + * Upgraded base distribution to Debian 13.x/Trixie. * Replace TurnKey custom Debian-Installer based 'di-live' with new custom From 7ba662a813fb995d16a2923666d61e6e076f7da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:23:09 +0000 Subject: [PATCH 3/9] test: ask the real pam_unix whether a password gets in The helpers answered "would this password get in" with a hand written model of pam_unix: crypt() through perl wrapped in two rules. The model was missing rules the module has, and reported at least one acceptance the module does not make: with a blank equivalent field and no nullok it let the empty password in, and pam_unix refuses it. tests/pam-authenticate now asks Linux-PAM itself. It loads libpam.so.0 through ctypes, starts the scratch image's own stack with pam_start_confdir and calls pam_authenticate, in a user and mount namespace where the scratch shadow and passwd files are bind mounted over /etc/shadow and /etc/passwd. pam_unix runs in that process as root of the namespace, which is the path Webmin's miniserv takes; unix_chkpwd, the helper for unprivileged callers, is not the same path and does not give the same answers (with nullok and a blank equivalent field it refuses a non-empty password, and the in process module accepts any password without asking for one). The fail delay is handed to the application and ignored, which changes the time and not the verdict. The verdict is read from the PAM return code the client prints, never from an exit status, because unshare and mount exit 1 when they fail and a sandbox that does not work must not read as a refusal. Refutations use a new 'refuses' helper for the same reason, never '! authenticates'. Mutation checked: with an unshare that exits 1, every test that asks PAM fails. tests/pam-unix.bats pins the facts the other suites rest on, measured on libpam 1.7.0-5: field '' or 'U6aMy0wojraho', nullok any password, no prompt same fields, no nullok every password refused field starting '*' or '!', either every password refused One test did not survive the real module, and it was the claim that removing nullok leaves the empty password to the field. It does not: on the webmin stack, without nullok, the old placeholder refuses every password. What the field is for is every stack that keeps nullok, and Debian's common-auth does; tests/fixtures/pam.d-common-auth is that file as a built core carries it, and the rewritten test shows the old field letting anything in through it and '*' refusing everything. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- tests/before-firstboot.bats | 6 +- tests/fixtures/pam.d-common-auth | 3 + tests/helpers.bash | 126 ++++++++++++++++++------------- tests/pam-authenticate | 125 ++++++++++++++++++++++++++++++ tests/pam-unix.bats | 71 +++++++++++++++++ tests/rootpass.bats | 15 ++-- tests/webmin-pam.bats | 20 ++++- 7 files changed, 301 insertions(+), 65 deletions(-) create mode 100644 tests/fixtures/pam.d-common-auth create mode 100755 tests/pam-authenticate create mode 100644 tests/pam-unix.bats diff --git a/tests/before-firstboot.bats b/tests/before-firstboot.bats index d8f7d020..6774fba8 100644 --- a/tests/before-firstboot.bats +++ b/tests/before-firstboot.bats @@ -71,7 +71,8 @@ build_image() { [ "$status" -eq 0 ] run authenticates "hunter2" [ "$status" -eq 0 ] - run ! authenticates "" + run refuses "" + [ "$status" -eq 0 ] } @test "a build given a root password is held shut the same way" { @@ -80,5 +81,6 @@ build_image() { run ! unit_would_start "$DROPIN" run authenticates "s3cret" [ "$status" -eq 0 ] - run ! authenticates "" + run refuses "" + [ "$status" -eq 0 ] } diff --git a/tests/fixtures/pam.d-common-auth b/tests/fixtures/pam.d-common-auth new file mode 100644 index 00000000..25f30c24 --- /dev/null +++ b/tests/fixtures/pam.d-common-auth @@ -0,0 +1,3 @@ +auth [success=1 default=ignore] pam_unix.so nullok +auth requisite pam_deny.so +auth required pam_permit.so diff --git a/tests/helpers.bash b/tests/helpers.bash index 3191615b..7b1c1f77 100644 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -1,24 +1,21 @@ # Shared helpers for the conf scripts that decide whether an account of a # freshly built image can be authenticated. # -# The verdict these tests take is the one pam_unix takes, and it is taken -# the way pam_unix takes it: +# The verdict these tests take is the one pam_unix takes, because it is +# pam_unix that takes it: tests/pam-authenticate loads the real libpam, +# starts the scratch image's own stack and calls pam_authenticate, in a user +# and mount namespace where the scratch shadow and passwd files are bind +# mounted over /etc/shadow and /etc/passwd. pam_unix runs in that process as +# root of the namespace, which is the path Webmin's miniserv takes: it reads +# the shadow file itself and does not go through unix_chkpwd. # -# crypt() is the real one, reached through perl. It is the function -# pam_unix compares a submitted password with. -# -# Around it, two rules out of Linux-PAM. _unix_blankpasswd() answers that -# an account has no password when its stored field is empty or when -# crypt() returns that field for the empty string, and it answers so only -# while nullok is in effect; _unix_verify_password() then either accepts -# without looking at what was submitted, or compares. The rules were -# confirmed against the real helper, which is the code pam_unix runs: -# -# printf '%s' "$password" | /usr/sbin/unix_chkpwd root nullok|nonull -# -# with the stored field bind mounted over /etc/shadow under -# 'unshare -r -m'. Accepts and refusals both reproduced, for the fields -# '', 'U6aMy0wojraho', '*' and '!'. +# That matters, because the two paths disagree. Measured on libpam 1.7.0-5 +# with a blank equivalent field ('' or 'U6aMy0wojraho', which crypt() +# returns for the empty string) and nullok in the stack, pam_unix in process +# accepts any password without asking for one, while unix_chkpwd refuses a +# non-empty one. An earlier version of these helpers modelled pam_unix by +# hand and reported acceptances the module does not make; nothing here is a +# model any more. # # The shadow tools are stubs, because usermod, chpasswd and passwd chroot # into the root they are given and so cannot touch a scratch tree as an @@ -30,8 +27,8 @@ FIXTURES="$HELPERS_DIR/fixtures" # scratch_image # The part of an image under construction that these scripts touch: the -# account databases, the PAM stack the webmin package installs and an empty -# systemd tree. Puts the stubs first on PATH and points every test hook at +# account databases, the PAM stack the webmin package installs, Debian's +# common-auth as a built core carries it, and an empty systemd tree. Puts the stubs first on PATH and points every test hook at # the scratch tree. # # root starts out carrying the field an earlier build wrote, so a script @@ -43,6 +40,7 @@ scratch_image() { printf 'root:x:0:0:root:/root:/bin/bash\n' > "$IMAGE/etc/passwd" printf 'root:U6aMy0wojraho:20718:0:99999:7:::\n' > "$IMAGE/etc/shadow" cp "$FIXTURES/pam.d-webmin" "$IMAGE/etc/pam.d/webmin" + cp "$FIXTURES/pam.d-common-auth" "$IMAGE/etc/pam.d/common-auth" export PATH="$HELPERS_DIR/stubs:$PATH" export STUB_LOG=$BATS_TEST_TMPDIR/calls.log @@ -50,16 +48,10 @@ scratch_image() { export SHADOW_FILE=$IMAGE/etc/shadow export PAM_WEBMIN=$IMAGE/etc/pam.d/webmin + export PAM_COMMON_AUTH=$IMAGE/etc/pam.d/common-auth export SYSTEMD_DIR=$IMAGE/etc/systemd/system } -# crypt_of PASSWORD SETTING -# The real crypt(3). Prints what it returns, which is never the setting -# itself for a setting no hash can be built from. -crypt_of() { - perl -e 'print crypt($ARGV[0], $ARGV[1]) // ""' "$1" "$2" -} - # field_of USER # The password field USER carries in the scratch image. field_of() { @@ -67,52 +59,78 @@ field_of() { } # stack_allows_blank [PAM_FILE] -# Whether nullok is in effect for pam_unix on an auth line of the stack, -# which is what decides if a password that is not set authenticates. +# Whether nullok is written on an auth line of the stack. Configuration, +# not behaviour: what it does is for pam_verdict to say. stack_allows_blank() { grep -E '^[[:space:]]*auth[[:space:]]' "${1:-$PAM_WEBMIN}" \ | grep -qE '\bnullok(_secure)?\b' } -# blank_equivalent FIELD -# Linux-PAM's _unix_blankpasswd: a field that is empty, or that crypt() -# returns for the empty string, is an account whose password is not set. -blank_equivalent() { - if [[ -z "$1" ]]; then - return 0 - fi - [[ "$(crypt_of "" "$1")" == "$1" ]] +# pam_verdict PASSWORD [USER] [PAM_FILE] +# The real pam_authenticate over PAM_FILE (the scratch image's webmin stack +# by default) for USER with PASSWORD, against the scratch image's accounts. +# Returns 0 authenticated, 1 refused, and 3 or more when the question could +# not be asked. +# +# The verdict is read from the return code pam-authenticate prints, never +# from an exit status: unshare and mount exit 1 when they fail, and a +# sandbox that does not work must not be read as a refusal. +pam_verdict() { + local password=$1 + local user=${2:-root} + local pam_file=${3:-$PAM_WEBMIN} + local output + output=$(unshare --user --map-root-user --mount -- /bin/bash -c ' + mount --bind "$1" /etc/shadow || exit 4 + mount --bind "$2" /etc/passwd || exit 4 + exec "$3" "$4" "$5" "$6" "$7" + ' pam_verdict "${SHADOW_FILE:?}" "$IMAGE/etc/passwd" \ + "$HELPERS_DIR/pam-authenticate" \ + "$(dirname "$pam_file")" "$(basename "$pam_file")" \ + "$user" "$password") + echo "$output" + case $output in + "pam=0 "*) return 0 ;; + "pam=7 "*) return 1 ;; + "pam="*) return 3 ;; + *) echo "pam_verdict: the sandbox did not answer" >&2 + return 4 ;; + esac } # authenticates PASSWORD [USER] [PAM_FILE] -# Succeeds when pam_unix would let PASSWORD in for USER through that stack, -# fails when it would not. +# Succeeds when pam_unix lets PASSWORD in, fails otherwise, including when +# the question could not be asked. authenticates() { - local password=$1 - local user=${2:-root} - local pam_file=${3:-$PAM_WEBMIN} - local field - field=$(field_of "$user") + local rc=0 + pam_verdict "$@" || rc=$? + [[ $rc -eq 0 ]] +} - if stack_allows_blank "$pam_file" && blank_equivalent "$field"; then - return 0 - fi - if [[ -z "$field" ]]; then - return 1 +# refuses PASSWORD [USER] [PAM_FILE] +# Succeeds only when pam_unix refuses PASSWORD. A refutation is written with +# this and never as '! authenticates', which a sandbox that does not work +# would satisfy. +refuses() { + local rc=0 + pam_verdict "$@" || rc=$? + if [[ $rc -gt 1 ]]; then + echo "pam_verdict could not ask (status $rc)" >&2 fi - [[ "$(crypt_of "$password" "$field")" == "$field" ]] + [[ $rc -eq 1 ]] } -# nothing_authenticates [USER] -# Succeeds when no password at all gets USER in: the empty one, the ones -# worth trying first, and a right looking one. +# nothing_authenticates [USER] [PAM_FILE] +# Succeeds when pam_unix refuses every password tried: the empty one, the +# ones worth trying first, and a right looking one. nothing_authenticates() { local user=${1:-root} + local pam_file=${2:-$PAM_WEBMIN} local password for password in "" " " "root" "toor" "password" "turnkey" "*" "!" \ "U6aMy0wojraho" "hunter2"; do - if authenticates "$password" "$user"; then - echo "authenticated with '$password' against field '$(field_of "$user")'" >&2 + if ! refuses "$password" "$user" "$pam_file"; then + echo "not refused: '$password' against field '$(field_of "$user")'" >&2 return 1 fi done diff --git a/tests/pam-authenticate b/tests/pam-authenticate new file mode 100755 index 00000000..a177032a --- /dev/null +++ b/tests/pam-authenticate @@ -0,0 +1,125 @@ +#!/usr/bin/python3 +"""Ask the real Linux-PAM whether a password gets a user in. + + pam-authenticate CONFDIR SERVICE USER PASSWORD + +Loads libpam.so.0, starts SERVICE with its stack read from CONFDIR +(pam_start_confdir, Linux-PAM 1.4 and later) and calls pam_authenticate, +answering every prompt with PASSWORD. The modules in the stack are the ones +installed on this machine, pam_unix included, running in this process, +which is how Webmin's miniserv runs them: as root, reading the shadow file +itself rather than through unix_chkpwd. + +Run it as root of a user namespace with a scratch shadow file bind mounted +over /etc/shadow (helpers.bash does), never as the real root. + +Exit status: 0 authenticated, 1 refused (PAM_AUTH_ERR), 3 anything else, +so that a sandbox or stack that does not work is never read as a refusal. +Prints one line: the PAM return code and whether the stack asked for a +password at all. +""" + +import ctypes +import ctypes.util +import sys + +PAM_SUCCESS = 0 +PAM_AUTH_ERR = 7 +PAM_PROMPT_ECHO_OFF = 1 +PAM_PROMPT_ECHO_ON = 2 +PAM_FAIL_DELAY = 10 + + +class PamMessage(ctypes.Structure): + _fields_ = [("msg_style", ctypes.c_int), ("msg", ctypes.c_char_p)] + + +class PamResponse(ctypes.Structure): + _fields_ = [("resp", ctypes.c_void_p), ("resp_retcode", ctypes.c_int)] + + +CONV_FUNC = ctypes.CFUNCTYPE( + ctypes.c_int, + ctypes.c_int, + ctypes.POINTER(ctypes.POINTER(PamMessage)), + ctypes.POINTER(ctypes.POINTER(PamResponse)), + ctypes.c_void_p, +) + + +# void (*)(int retval, unsigned usec_delay, void *appdata_ptr) +DELAY_FUNC = ctypes.CFUNCTYPE(None, ctypes.c_int, ctypes.c_uint, + ctypes.c_void_p) + + +class PamConv(ctypes.Structure): + _fields_ = [("conv", CONV_FUNC), ("appdata_ptr", ctypes.c_void_p)] + + +def main(argv): + if len(argv) != 5: + print(__doc__.strip().splitlines()[2].strip(), file=sys.stderr) + return 3 + confdir, service, user, password = argv[1:] + + libpam = ctypes.CDLL("libpam.so.0") + libc = ctypes.CDLL(ctypes.util.find_library("c")) + libc.calloc.restype = ctypes.c_void_p + libc.calloc.argtypes = [ctypes.c_size_t, ctypes.c_size_t] + libc.strdup.restype = ctypes.c_void_p + libc.strdup.argtypes = [ctypes.c_char_p] + libpam.pam_start_confdir.argtypes = [ + ctypes.c_char_p, ctypes.c_char_p, ctypes.POINTER(PamConv), + ctypes.c_char_p, ctypes.POINTER(ctypes.c_void_p)] + libpam.pam_authenticate.argtypes = [ctypes.c_void_p, ctypes.c_int] + libpam.pam_end.argtypes = [ctypes.c_void_p, ctypes.c_int] + libpam.pam_set_item.argtypes = [ctypes.c_void_p, ctypes.c_int, + ctypes.c_void_p] + + prompted = [] + + # libpam frees the responses, so they are allocated with its allocator. + def converse(count, messages, responses, _appdata): + array = libc.calloc(count, ctypes.sizeof(PamResponse)) + if not array: + return 5 + reply = ctypes.cast(array, ctypes.POINTER(PamResponse)) + for i in range(count): + style = messages[i].contents.msg_style + if style in (PAM_PROMPT_ECHO_OFF, PAM_PROMPT_ECHO_ON): + prompted.append(style) + reply[i].resp = libc.strdup(password.encode()) + responses[0] = reply + return PAM_SUCCESS + + conv = PamConv(CONV_FUNC(converse), None) + handle = ctypes.c_void_p() + rc = libpam.pam_start_confdir(service.encode(), user.encode(), + ctypes.byref(conv), confdir.encode(), + ctypes.byref(handle)) + if rc != PAM_SUCCESS: + print("pam_start_confdir failed: %d" % rc, file=sys.stderr) + return 3 + # pam_unix asks for a delay of about two seconds after a refusal. An + # application that sets PAM_FAIL_DELAY is handed the delay instead of + # libpam sleeping it; this one ignores it. The verdict is unchanged. + no_delay = DELAY_FUNC(lambda _rc, _usec, _appdata: None) + rc = libpam.pam_set_item(handle, PAM_FAIL_DELAY, + ctypes.cast(no_delay, ctypes.c_void_p)) + if rc != PAM_SUCCESS: + print("pam_set_item(PAM_FAIL_DELAY) failed: %d" % rc, file=sys.stderr) + libpam.pam_end(handle, rc) + return 3 + rc = libpam.pam_authenticate(handle, 0) + libpam.pam_end(handle, rc) + + print("pam=%d prompted=%s" % (rc, "yes" if prompted else "no")) + if rc == PAM_SUCCESS: + return 0 + if rc == PAM_AUTH_ERR: + return 1 + return 3 + + +if __name__ == "__main__": + sys.exit(main(sys.argv)) diff --git a/tests/pam-unix.bats b/tests/pam-unix.bats new file mode 100644 index 00000000..cfc7d401 --- /dev/null +++ b/tests/pam-unix.bats @@ -0,0 +1,71 @@ +#!/usr/bin/env bats +# What pam_unix itself does with the password fields and the option these +# conf scripts decide, asked of the real module (tests/pam-authenticate). +# +# These are not tests of this repository's scripts. They pin the facts the +# other suites rest on, so that a libpam that changes them, or a sandbox +# that stops asking the module at all, fails here by name instead of +# turning another suite quietly green. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + scratch_image + NO_NULLOK=$IMAGE/etc/pam.d/no-nullok + printf 'auth\trequired\tpam_unix.so\n' > "$NO_NULLOK" + HASH=$(perl -e 'print crypt($ARGV[0], $ARGV[1])' s3cret '$6$keelpins$') +} + +set_field() { + printf 'root:%s:20718:0:99999:7:::\n' "$1" > "$SHADOW_FILE" +} + +@test "the sandbox asks the module: a set password gets in and a wrong one does not" { + set_field "$HASH" + run authenticates s3cret + [ "$status" -eq 0 ] + [[ "$output" == *"prompted=yes"* ]] + run refuses wrong + [ "$status" -eq 0 ] +} + +@test "a user the module cannot find is not read as a refusal" { + run pam_verdict anything nobody-here + [ "$status" -ge 3 ] + run refuses anything nobody-here + [ "$status" -ne 0 ] +} + +@test "with nullok, a blank equivalent field lets any password in without asking" { + local field + for field in '' 'U6aMy0wojraho'; do + set_field "$field" + run pam_verdict "anything at all" + [ "$status" -eq 0 ] + [[ "$output" == *"prompted=no"* ]] + done +} + +@test "without nullok, the same fields refuse every password, the empty one included" { + local field + for field in '' 'U6aMy0wojraho'; do + set_field "$field" + run nothing_authenticates root "$NO_NULLOK" + [ "$status" -eq 0 ] + done +} + +@test "a field starting with '*' or '!' refuses everything, whatever the option" { + local field stack + for field in '*' '!' "!$HASH" "*$HASH"; do + set_field "$field" + for stack in "$PAM_WEBMIN" "$NO_NULLOK"; do + run nothing_authenticates root "$stack" + [ "$status" -eq 0 ] + run refuses s3cret root "$stack" + [ "$status" -eq 0 ] + done + done +} diff --git a/tests/rootpass.bats b/tests/rootpass.bats index f97205bf..c2c131f2 100644 --- a/tests/rootpass.bats +++ b/tests/rootpass.bats @@ -27,7 +27,8 @@ setup() { stack_allows_blank run "$SCRIPT" [ "$status" -eq 0 ] - run ! authenticates "" + run refuses "" + [ "$status" -eq 0 ] } @test "the field left behind is one passwd --unlock accepts" { @@ -55,8 +56,10 @@ setup() { echo 'root:hunter2' | chpasswd run authenticates "hunter2" [ "$status" -eq 0 ] - run ! authenticates "" - run ! authenticates "hunter3" + run refuses "" + [ "$status" -eq 0 ] + run refuses "hunter3" + [ "$status" -eq 0 ] } @test "a build time ROOT_PASS is the password, and nothing else is" { @@ -65,7 +68,8 @@ setup() { [ "$status" -eq 0 ] run authenticates "s3cret" [ "$status" -eq 0 ] - run ! authenticates "" + run refuses "" + [ "$status" -eq 0 ] } @test "a ROOT_PASS holding a glob character is set as written" { @@ -77,7 +81,8 @@ setup() { [ "$status" -eq 0 ] run authenticates 'p*ss' [ "$status" -eq 0 ] - run ! authenticates 'pass' + run refuses 'pass' + [ "$status" -eq 0 ] } @test "a chroot only build locks the account instead" { diff --git a/tests/webmin-pam.bats b/tests/webmin-pam.bats index e59b81da..2cdc8c12 100644 --- a/tests/webmin-pam.bats +++ b/tests/webmin-pam.bats @@ -23,18 +23,30 @@ setup() { run "$SCRIPT" [ "$status" -eq 0 ] - run ! authenticates "anything at all" + run refuses "anything at all" + [ "$status" -eq 0 ] } -@test "what is left is the field itself, which is rootpass's half" { +@test "the empty password stops getting in as well, through this stack" { + run authenticates "" + [ "$status" -eq 0 ] + run "$SCRIPT" [ "$status" -eq 0 ] - run authenticates "" + run nothing_authenticates + [ "$status" -eq 0 ] +} + +@test "a stack that keeps the option still lets anything in, which is what the field is for" { + run "$SCRIPT" + [ "$status" -eq 0 ] + + run authenticates "anything at all" root "$PAM_COMMON_AUTH" [ "$status" -eq 0 ] printf 'root:*:20718:0:99999:7:::\n' > "$SHADOW_FILE" - run nothing_authenticates + run nothing_authenticates root "$PAM_COMMON_AUTH" [ "$status" -eq 0 ] } From 10e558efce10e1e942855e89b17363f53c313369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:28:19 +0000 Subject: [PATCH 4/9] test: ask pam_unix through sudo where a user namespace cannot mount The previous commit's sandbox needs a user namespace that may mount. The Ubuntu runner the CI uses makes the namespace but its AppArmor policy leaves it without the right to mount, and every test that asks PAM failed there, loudly, which is what the verdict parsing is for. sandbox_mount_ns tries the unprivileged namespace first and falls back to 'sudo --non-interactive unshare --mount' where sudo needs no password, which the runner has. Either way the bind mounts live in a private mount namespace and are never visible outside it, and the module asked is the same one. Where neither is available it says so and the tests fail; they do not skip. Both paths measured locally: 42 of 42, and no shadow mount left behind. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- tests/helpers.bash | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/helpers.bash b/tests/helpers.bash index 7b1c1f77..c9af3fed 100644 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -3,10 +3,10 @@ # # The verdict these tests take is the one pam_unix takes, because it is # pam_unix that takes it: tests/pam-authenticate loads the real libpam, -# starts the scratch image's own stack and calls pam_authenticate, in a user -# and mount namespace where the scratch shadow and passwd files are bind -# mounted over /etc/shadow and /etc/passwd. pam_unix runs in that process as -# root of the namespace, which is the path Webmin's miniserv takes: it reads +# starts the scratch image's own stack and calls pam_authenticate, in a +# private mount namespace where the scratch shadow and passwd files are bind +# mounted over /etc/shadow and /etc/passwd (sandbox_mount_ns). pam_unix runs +# in that process as root, which is the path Webmin's miniserv takes: it reads # the shadow file itself and does not go through unix_chkpwd. # # That matters, because the two paths disagree. Measured on libpam 1.7.0-5 @@ -66,6 +66,26 @@ stack_allows_blank() { | grep -qE '\bnullok(_secure)?\b' } +# sandbox_mount_ns COMMAND... +# Runs COMMAND as root of a private mount namespace: an unprivileged user +# namespace where the kernel allows one to mount (Debian), otherwise +# 'sudo -n unshare --mount' where sudo needs no password (the Ubuntu CI +# runner, whose AppArmor policy leaves an unprivileged user namespace +# without the right to mount). Either way nothing mounted is visible +# outside the namespace. Exits 77 when neither is available. +sandbox_mount_ns() { + if unshare --user --map-root-user --mount -- \ + /bin/sh -c 'mount -t tmpfs none /mnt' 2>/dev/null; then + unshare --user --map-root-user --mount -- "$@" + elif sudo -n unshare --mount -- \ + /bin/sh -c 'mount -t tmpfs none /mnt' 2>/dev/null; then + sudo -n unshare --mount -- "$@" + else + echo "sandbox_mount_ns: no mount namespace available here" >&2 + return 77 + fi +} + # pam_verdict PASSWORD [USER] [PAM_FILE] # The real pam_authenticate over PAM_FILE (the scratch image's webmin stack # by default) for USER with PASSWORD, against the scratch image's accounts. @@ -80,7 +100,7 @@ pam_verdict() { local user=${2:-root} local pam_file=${3:-$PAM_WEBMIN} local output - output=$(unshare --user --map-root-user --mount -- /bin/bash -c ' + output=$(sandbox_mount_ns /bin/bash -c ' mount --bind "$1" /etc/shadow || exit 4 mount --bind "$2" /etc/passwd || exit 4 exec "$3" "$4" "$5" "$6" "$7" From ede92d9eacfda7a87fa690e746c25ca592cca609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:32:05 +0000 Subject: [PATCH 5/9] fix: a later boot does not wait for the first boot's scripts again Webmin's drop-in waited for /run/inithooks-complete, which inithooks writes at the end of the everyboot pass on every boot, on a tmpfs. So the hold did not gate one boot: it gated every boot for the life of the appliance behind inithooks.service reaching that line, and a boot where it did not (a hook blocked on a console nobody is attached to, docs/traps.md entry 1) had no web interface, while 'systemctl start webmin' exited 0 and started nothing, because an unmet condition is a skipped, successful start. The one shot the path unit triggers now records that the interface was opened, in /var/lib/webmin-after-firstboot/opened, before it starts Webmin, and the drop-in accepts that record as a third triggering condition. The first boot is held as before; every boot after one that got to the end of its scripts starts Webmin with the rest of the boot. The build writes no record, and a test says so. The script's comment now says what the marker does not mean (scripts run, not scripts succeeded; rootpass's field is what holds an account shut on a first boot that failed to set a password) and gives the recovery for a first boot that never finishes: 'systemctl start webmin-after-firstboot.service', which opens it for that boot and every one after. Tests, written first and failing on the previous script for the two later boot cases and the recovery: - the unit and its drop-in are handed to systemd together: tests/fixtures/webmin.service is the unit webmin 2.660.turnkey0 installs (read from a built container), 'systemd-analyze verify' reads it with the drop-in in place, and the verdict on the conditions is taken over both files, so a plain Condition in the packaged unit, which would AND with these, is seen; - the one shot's ExecStart lines are run in order, with systemctl stubbed and mkdir and touch real, and the next boot is a boot with an empty /run. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- conf/turnkey.d/webmin-enable | 44 +++++++++++++---- tests/fixtures/webmin.service | 16 ++++++ tests/helpers.bash | 28 +++++++++-- tests/webmin-enable.bats | 91 +++++++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 14 deletions(-) create mode 100644 tests/fixtures/webmin.service diff --git a/conf/turnkey.d/webmin-enable b/conf/turnkey.d/webmin-enable index 22964e33..5f0ca4c0 100755 --- a/conf/turnkey.d/webmin-enable +++ b/conf/turnkey.d/webmin-enable @@ -2,26 +2,50 @@ # An appliance is configured by its first boot scripts, and a web interface # is not something it needs before then. So the unit does not start with the -# rest of the boot: it starts once inithooks reports that its boot scripts -# have run. +# rest of the boot until the first boot scripts have run once. # -# The two conditions are triggering conditions, which systemd reads as an -# OR: the marker is there, or the image carries no inithooks at all and -# there is nothing to wait for. The path unit starts Webmin when the marker -# appears; it triggers a one shot that stays active afterwards, so it fires -# once and does not restart Webmin behind an administrator who stopped it. +# The three conditions are triggering conditions, which systemd reads as an +# OR: # -# SYSTEMD_DIR, INITHOOKS_MARKER and INITHOOKS_RUN are test hooks; a build -# leaves them unset. +# - WEBMIN_OPENED exists: a first boot got to the end of its scripts on +# this machine once, and this boot is not waiting for it. The one shot +# below writes it; the build does not, so no image carries it. +# - the marker inithooks writes when its boot scripts are done, on the +# boot that has not got that far yet. +# - the image carries no inithooks at all, and there is nothing to wait +# for. +# +# The path unit starts Webmin when the marker appears; it triggers a one +# shot that stays active afterwards, so it fires once per boot and does not +# restart Webmin behind an administrator who stopped it. +# +# The marker is written when the scripts have run, not when they succeeded: +# inithooks logs a failing script and carries on. What keeps an account +# shut on a first boot that failed to set a password is rootpass (a field +# no password matches), not this. +# +# A first boot that never finishes keeps the interface shut on every boot +# until one does. To open it by hand, from a console: +# +# systemctl start webmin-after-firstboot.service +# +# which starts Webmin and records that it has been opened, for every boot +# after. 'systemctl start webmin' alone exits 0 and starts nothing while the +# conditions are unmet. +# +# SYSTEMD_DIR, INITHOOKS_MARKER, INITHOOKS_RUN and WEBMIN_OPENED are test +# hooks; a build leaves them unset. SYSTEMD_DIR=${SYSTEMD_DIR:-/etc/systemd/system} INITHOOKS_MARKER=${INITHOOKS_MARKER:-/run/inithooks-complete} INITHOOKS_RUN=${INITHOOKS_RUN:-/usr/lib/inithooks/run} +WEBMIN_OPENED=${WEBMIN_OPENED:-/var/lib/webmin-after-firstboot/opened} mkdir -p "$SYSTEMD_DIR/webmin.service.d" cat > "$SYSTEMD_DIR/webmin.service.d/after-firstboot.conf" </dev/null } + +# run_exec_start UNIT_FILE +# Runs the ExecStart lines of a oneshot unit in order, stopping at the +# first that fails, as systemd does. Each command is looked up by its name +# on PATH, so a stub stands in for systemctl and touch and mkdir are the +# real ones. +run_exec_start() { + local line + local -a argv + while IFS= read -r line; do + read -r -a argv <<< "${line#ExecStart=}" + argv[0]=$(basename "${argv[0]}") + "${argv[@]}" || return + done < <(grep -E '^ExecStart=' "$1") +} diff --git a/tests/webmin-enable.bats b/tests/webmin-enable.bats index e425bd1d..fc429a0f 100644 --- a/tests/webmin-enable.bats +++ b/tests/webmin-enable.bats @@ -21,7 +21,23 @@ setup() { mkdir -p "$IMAGE/run" "$IMAGE/usr/lib/inithooks" touch "$INITHOOKS_RUN" + export WEBMIN_OPENED=$IMAGE/var/lib/webmin-after-firstboot/opened DROPIN=$SYSTEMD_DIR/webmin.service.d/after-firstboot.conf + ONESHOT=$SYSTEMD_DIR/webmin-after-firstboot.service + + # webmin.service as the webmin package installs it, beside the drop-in + # the way it is in an image, so that systemd reads the two together. + # Only the program is swapped for one that exists here, which + # 'systemd-analyze verify' checks and nothing else depends on. + sed 's|^ExecStart=/usr/share/webmin/miniserv.pl|ExecStart=/bin/true|' \ + "$FIXTURES/webmin.service" > "$SYSTEMD_DIR/webmin.service" + UNIT=$SYSTEMD_DIR/webmin.service +} + +# next_boot +# What a reboot does to the state these units read: /run is emptied. +next_boot() { + rm -f "$INITHOOKS_MARKER" } @test "webmin does not start while the first boot scripts have not run" { @@ -56,6 +72,80 @@ setup() { [ -z "$output" ] } +@test "systemd accepts webmin.service with the drop-in in place" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run systemd-analyze verify --man=no "$UNIT" + [ "$status" -eq 0 ] + [ -z "$output" ] +} + +@test "the verdict is the merged unit's, the packaged conditions included" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run ! unit_would_start "$UNIT" "$DROPIN" + touch "$INITHOOKS_MARKER" + run unit_would_start "$UNIT" "$DROPIN" + [ "$status" -eq 0 ] + + # a plain condition in the packaged unit would AND with the drop-in's + printf '[Unit]\nConditionPathExists=%s\n' "$IMAGE/nowhere" >> "$UNIT" + run ! unit_would_start "$UNIT" "$DROPIN" +} + +@test "the first boot opening the interface leaves nothing for later boots to wait on" { + run "$SCRIPT" + [ "$status" -eq 0 ] + touch "$INITHOOKS_MARKER" + run run_exec_start "$ONESHOT" + [ "$status" -eq 0 ] + grep -qx "systemctl --no-block start webmin.service" "$STUB_LOG" + + next_boot + run unit_would_start "$UNIT" "$DROPIN" + [ "$status" -eq 0 ] +} + +@test "a boot whose inithooks never finish still has the interface, after one that did" { + run "$SCRIPT" + [ "$status" -eq 0 ] + touch "$INITHOOKS_MARKER" + run_exec_start "$ONESHOT" + next_boot + next_boot + [ ! -e "$INITHOOKS_MARKER" ] + run unit_would_start "$UNIT" "$DROPIN" + [ "$status" -eq 0 ] +} + +@test "the build itself opens nothing: no record of a first boot is left in the image" { + run "$SCRIPT" + [ "$status" -eq 0 ] + [ ! -e "$WEBMIN_OPENED" ] + next_boot + run ! unit_would_start "$UNIT" "$DROPIN" +} + +@test "a first boot that never finishes keeps it shut on the boots after it" { + run "$SCRIPT" + [ "$status" -eq 0 ] + next_boot + run ! unit_would_start "$UNIT" "$DROPIN" + next_boot + run ! unit_would_start "$UNIT" "$DROPIN" +} + +@test "the recovery the script's comment names opens it" { + run "$SCRIPT" + [ "$status" -eq 0 ] + grep -q "systemctl start webmin-after-firstboot.service" "$SCRIPT" + run run_exec_start "$ONESHOT" + [ "$status" -eq 0 ] + next_boot + run unit_would_start "$UNIT" "$DROPIN" + [ "$status" -eq 0 ] +} + @test "the marker is what the path unit waits for, and it fires once" { run "$SCRIPT" [ "$status" -eq 0 ] @@ -82,4 +172,5 @@ EXPECTED run "$SCRIPT" [ "$status" -eq 0 ] [ "$(cat "$DROPIN" "$SYSTEMD_DIR"/webmin-after-firstboot.*)" = "$first" ] + [ ! -e "$WEBMIN_OPENED" ] } From 7ccbfe84ed04a65a824cc210a9b05959d9b3448a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:32:05 +0000 Subject: [PATCH 6/9] fix: samba-rootpass writes the field rootpass writes, and keeps the password out of the log conf/samba-rootpass carried both of the defects this branch fixes in conf/turnkey.d/rootpass, and a third of its own: - with no ROOT_PASS it wrote 'U6aMy0wojraho', the crypt() of the empty string, and it runs after rootpass (mk/turnkey/fileserver.mk lists it in COMMON_CONF), so a Samba appliance had rootpass's '*' written back over. Through a stack carrying nullok, Debian's common-auth among them, the real pam_unix lets any password in against that field; - ROOT_PASS was expanded unquoted, against the build directory; - '#!/bin/sh -ex' traced 'echo root:$ROOT_PASS' and the smbpasswd feed into the build log, with the password in them. It now writes '*', quotes the password and does not trace. The shebang is bash, for kcov, as rootpass's is. 'smbpasswd -a -n' is unchanged: a null Samba password is refused unless smb.conf sets 'null passwords = yes', and nothing in this tree does. tests/samba-rootpass.bats, 6 tests, 4 of them failing on the previous script: nothing authenticates through the webmin stack or common-auth, the unlock runs first and the field it leaves is one unlock accepts, a glob is set as written, and the password is in neither output stream. smbpasswd is a stub that records its arguments and what it was fed. The file joins the measured list at 100 percent. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- conf/samba-rootpass | 24 +++++++++---- tests/coverage.sh | 1 + tests/samba-rootpass.bats | 73 +++++++++++++++++++++++++++++++++++++++ tests/stubs/smbpasswd | 7 ++++ 4 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 tests/samba-rootpass.bats create mode 100755 tests/stubs/smbpasswd diff --git a/conf/samba-rootpass b/conf/samba-rootpass index 470c379e..c4c166ff 100755 --- a/conf/samba-rootpass +++ b/conf/samba-rootpass @@ -1,18 +1,30 @@ -#!/bin/sh -ex +#!/bin/bash -e # set samba root password +# +# The same account field as conf/turnkey.d/rootpass writes, for the same +# reason: '*' is a field no password matches, through any PAM stack, until +# the first boot sets a password over it. The old value, 'U6aMy0wojraho', +# is what crypt() returns for the empty string, and a stack carrying nullok +# (Debian's common-auth does) let any password in against it. +# +# No xtrace: it printed ROOT_PASS into the build log. +# +# 'smbpasswd -n' gives the Samba account a null password, which Samba +# refuses to authenticate unless 'null passwords = yes' is set in smb.conf; +# nothing in this tree sets it. That call is unchanged. +NO_PASSWORD='*' + passwd --unlock root # useful when testing in chroot if [ -n "$ROOT_PASS" ]; then - echo root:$ROOT_PASS | chpasswd + echo "root:$ROOT_PASS" | chpasswd else - # this is a blank password - usermod -p U6aMy0wojraho root + usermod -p "$NO_PASSWORD" root fi -if [ "$ROOT_PASS" ]; then +if [ -n "$ROOT_PASS" ]; then (echo "$ROOT_PASS" ; echo "$ROOT_PASS" ) | smbpasswd -a -s root else smbpasswd -a -n root fi - diff --git a/tests/coverage.sh b/tests/coverage.sh index 4194477c..8a6cbac6 100755 --- a/tests/coverage.sh +++ b/tests/coverage.sh @@ -16,6 +16,7 @@ threshold="${1:-${COVERAGE_THRESHOLD:-95}}" # Every file the suite is expected to cover. A file that is not listed is # not measured, so a new script arrives here with its tests. measured=( + conf/samba-rootpass conf/turnkey.d/postfix-local conf/turnkey.d/rootpass conf/turnkey.d/webmin-enable diff --git a/tests/samba-rootpass.bats b/tests/samba-rootpass.bats new file mode 100644 index 00000000..9473db38 --- /dev/null +++ b/tests/samba-rootpass.bats @@ -0,0 +1,73 @@ +#!/usr/bin/env bats +# Tests for conf/samba-rootpass: the same question as rootpass.bats, for the +# copy of it the Samba appliances run, and what it hands smbpasswd. +# +# The verdict is the real pam_unix's (helpers.bash); the shadow tools and +# smbpasswd are stubs. + +bats_require_minimum_version 1.5.0 + +load helpers + +setup() { + TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" + SCRIPT="$TESTS_DIR/../conf/samba-rootpass" + scratch_image + unset ROOT_PASS +} + +@test "no password gets root in when the build sets none, through any stack" { + run "$SCRIPT" + [ "$status" -eq 0 ] + run nothing_authenticates root "$PAM_WEBMIN" + [ "$status" -eq 0 ] + run nothing_authenticates root "$PAM_COMMON_AUTH" + [ "$status" -eq 0 ] +} + +@test "the account is unlocked first, and the field left is one unlock accepts" { + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(head -1 "$STUB_LOG")" = "passwd --unlock root" ] + [ "$(field_of root)" = '*' ] + run passwd --unlock root + [ "$status" -eq 0 ] +} + +@test "the Samba account is added with no password when the build sets none" { + run "$SCRIPT" + [ "$status" -eq 0 ] + [ "$(tail -1 "$STUB_LOG")" = "smbpasswd -a -n root" ] +} + +@test "a build time ROOT_PASS is the password for both, and nothing else is" { + export ROOT_PASS=s3cret + run "$SCRIPT" + [ "$status" -eq 0 ] + run authenticates s3cret + [ "$status" -eq 0 ] + run refuses "" + [ "$status" -eq 0 ] + [ "$(tail -1 "$STUB_LOG")" = "smbpasswd -a -s root" ] + [ "$(cat "$STUB_LOG.stdin")" = "$(printf 's3cret\ns3cret')" ] +} + +@test "a ROOT_PASS holding a glob character is set as written" { + mkdir -p "$IMAGE/build" + touch "$IMAGE/build/root:pass" + cd "$IMAGE/build" + export ROOT_PASS='p*ss' + run "$SCRIPT" + [ "$status" -eq 0 ] + run authenticates 'p*ss' + [ "$status" -eq 0 ] + run refuses 'pass' + [ "$status" -eq 0 ] +} + +@test "the build password is not written to the build log" { + export ROOT_PASS='n0t-in-the-log' + run --separate-stderr "$SCRIPT" + [ "$status" -eq 0 ] + [[ "$output$stderr" != *"n0t-in-the-log"* ]] +} diff --git a/tests/stubs/smbpasswd b/tests/stubs/smbpasswd new file mode 100755 index 00000000..081ab365 --- /dev/null +++ b/tests/stubs/smbpasswd @@ -0,0 +1,7 @@ +#!/bin/bash +# Stand-in for smbpasswd: records the call in STUB_LOG, and what it was given +# on standard input in STUB_LOG.stdin, one line per line read. +echo "smbpasswd $*" >> "${STUB_LOG:?}" +if [[ " $* " == *" -s "* ]]; then + cat >> "$STUB_LOG.stdin" +fi From c4e7c3e678b1ed0958cb0983154b4cbc8cd9afb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:35:52 +0000 Subject: [PATCH 7/9] test: what pam_unix makes of a blank equivalent field depends on the libpam The CI runner (Ubuntu 24.04, libpam 1.5.3) failed five tests that pass on Debian 13 (libpam 1.7.0-5), all of them about the old placeholder 'U6aMy0wojraho', the crypt() of the empty string. The module is not the same module: - pam_unix runs unix_chkpwd for every shadowed account, as root too (get_account_info returns PAM_UNIX_RUN_HELPER outside the helper). The first commit of this series said the in process path does not; it does, and the helpers and tests/pam-authenticate now say so. - Before asking for a password, _unix_blankpasswd() hands the helper an empty one, with nullok when the stack has it. If the helper accepts, the account is authenticated without being asked. Measured by bind mounting a helper that always refuses over unix_chkpwd in the sandbox: the "any password, no prompt" verdict turns into a prompt and a refusal. - In 1.7, verify_pwd_hash() refuses an empty password without nullok and otherwise compares it; under nullok the empty password matches 'U6aMy0wojraho', so any password gets in. In 1.5 it refuses the empty password under nullok unless the field is empty, and compares it without, so there the same field lets the empty password in when nullok is gone. (Linux-PAM v1.5.3 and v1.7.0, modules/pam_unix/passverify.c.) So on the libpam an appliance runs, removing nullok alone closed the old field, and on 1.5 it did not; '*' is refused by both, through any stack. require_measured_libpam skips a test whose verdict is version dependent, naming both versions, when libpam-modules here is not 1.7. Five tests call it; the facts that hold on both (an empty field, '*' and '!', a real hash) and every test of the scripts themselves do not. Mutation checked by setting the measured version to 1.5: exactly those five skip. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- tests/helpers.bash | 40 ++++++++++++++++++++++++++++---------- tests/pam-authenticate | 10 +++++----- tests/pam-unix.bats | 44 ++++++++++++++++++++++++++++-------------- tests/webmin-pam.bats | 3 +++ 4 files changed, 67 insertions(+), 30 deletions(-) diff --git a/tests/helpers.bash b/tests/helpers.bash index a626e480..780392f7 100644 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -5,17 +5,19 @@ # pam_unix that takes it: tests/pam-authenticate loads the real libpam, # starts the scratch image's own stack and calls pam_authenticate, in a # private mount namespace where the scratch shadow and passwd files are bind -# mounted over /etc/shadow and /etc/passwd (sandbox_mount_ns). pam_unix runs -# in that process as root, which is the path Webmin's miniserv takes: it reads -# the shadow file itself and does not go through unix_chkpwd. +# mounted over /etc/shadow and /etc/passwd (sandbox_mount_ns). Everything +# pam_unix does is the real thing, including running unix_chkpwd, which it +# does for every shadowed account even as root. # -# That matters, because the two paths disagree. Measured on libpam 1.7.0-5 -# with a blank equivalent field ('' or 'U6aMy0wojraho', which crypt() -# returns for the empty string) and nullok in the stack, pam_unix in process -# accepts any password without asking for one, while unix_chkpwd refuses a -# non-empty one. An earlier version of these helpers modelled pam_unix by -# hand and reported acceptances the module does not make; nothing here is a -# model any more. +# It runs it twice, and that is what a hand model of it missed. Before it +# asks for a password at all, _unix_blankpasswd() hands the helper an empty +# one, with nullok when the stack carries it; if the helper accepts, the +# account "has a blank password" and is authenticated without being asked. +# On libpam 1.7 the helper accepts the empty password against any field +# crypt() returns for it, so with nullok a field of 'U6aMy0wojraho' lets any +# password in. Libpam 1.5 refuses that same empty password under nullok, +# and accepts it without. Which one a verdict holds for is therefore a +# property of the libpam that gave it: see require_measured_libpam. # # The shadow tools are stubs, because usermod, chpasswd and passwd chroot # into the root they are given and so cannot touch a scratch tree as an @@ -52,6 +54,24 @@ scratch_image() { export SYSTEMD_DIR=$IMAGE/etc/systemd/system } +# MEASURED_LIBPAM +# The libpam the version dependent facts in these suites were measured on: +# Debian 13's, which is the one an appliance runs. +MEASURED_LIBPAM=1.7 + +# require_measured_libpam +# Skips the calling test, naming both versions, when the libpam-modules +# installed here is not MEASURED_LIBPAM. For a test whose verdict differs +# between libpam versions; a test that holds on every version does not call +# it. +require_measured_libpam() { + local here + here=$(dpkg-query -W -f '${Version}' libpam-modules 2>/dev/null || true) + if [[ "$here" != "$MEASURED_LIBPAM".* ]]; then + skip "libpam-modules ${here:-unknown} here; measured on $MEASURED_LIBPAM" + fi +} + # field_of USER # The password field USER carries in the scratch image. field_of() { diff --git a/tests/pam-authenticate b/tests/pam-authenticate index a177032a..6647160a 100755 --- a/tests/pam-authenticate +++ b/tests/pam-authenticate @@ -6,12 +6,12 @@ Loads libpam.so.0, starts SERVICE with its stack read from CONFDIR (pam_start_confdir, Linux-PAM 1.4 and later) and calls pam_authenticate, answering every prompt with PASSWORD. The modules in the stack are the ones -installed on this machine, pam_unix included, running in this process, -which is how Webmin's miniserv runs them: as root, reading the shadow file -itself rather than through unix_chkpwd. +installed on this machine, and they do what they do in any application, +Webmin's miniserv included: pam_unix runs unix_chkpwd for a shadowed +account, as root or not. -Run it as root of a user namespace with a scratch shadow file bind mounted -over /etc/shadow (helpers.bash does), never as the real root. +Run it inside a private mount namespace with a scratch shadow file bind +mounted over /etc/shadow (helpers.bash does), never against the real one. Exit status: 0 authenticated, 1 refused (PAM_AUTH_ERR), 3 anything else, so that a sandbox or stack that does not work is never read as a refusal. diff --git a/tests/pam-unix.bats b/tests/pam-unix.bats index cfc7d401..e3150282 100644 --- a/tests/pam-unix.bats +++ b/tests/pam-unix.bats @@ -6,6 +6,11 @@ # other suites rest on, so that a libpam that changes them, or a sandbox # that stops asking the module at all, fails here by name instead of # turning another suite quietly green. +# +# What pam_unix makes of 'U6aMy0wojraho', the crypt() of the empty string, +# changed between libpam 1.5 and 1.7 (helpers.bash has how), so those two +# facts are pinned for the libpam an appliance runs and skipped, by name, +# on any other. bats_require_minimum_version 1.5.0 @@ -38,23 +43,32 @@ set_field() { [ "$status" -ne 0 ] } -@test "with nullok, a blank equivalent field lets any password in without asking" { - local field - for field in '' 'U6aMy0wojraho'; do - set_field "$field" - run pam_verdict "anything at all" - [ "$status" -eq 0 ] - [[ "$output" == *"prompted=no"* ]] - done +@test "with nullok, an empty field lets any password in without asking" { + set_field '' + run pam_verdict "anything at all" + [ "$status" -eq 0 ] + [[ "$output" == *"prompted=no"* ]] } -@test "without nullok, the same fields refuse every password, the empty one included" { - local field - for field in '' 'U6aMy0wojraho'; do - set_field "$field" - run nothing_authenticates root "$NO_NULLOK" - [ "$status" -eq 0 ] - done +@test "without nullok, an empty field refuses every password" { + set_field '' + run nothing_authenticates root "$NO_NULLOK" + [ "$status" -eq 0 ] +} + +@test "libpam 1.7, with nullok: the crypt of the empty string lets any password in without asking" { + require_measured_libpam + set_field 'U6aMy0wojraho' + run pam_verdict "anything at all" + [ "$status" -eq 0 ] + [[ "$output" == *"prompted=no"* ]] +} + +@test "libpam 1.7, without nullok: the crypt of the empty string refuses every password" { + require_measured_libpam + set_field 'U6aMy0wojraho' + run nothing_authenticates root "$NO_NULLOK" + [ "$status" -eq 0 ] } @test "a field starting with '*' or '!' refuses everything, whatever the option" { diff --git a/tests/webmin-pam.bats b/tests/webmin-pam.bats index 2cdc8c12..2174250e 100644 --- a/tests/webmin-pam.bats +++ b/tests/webmin-pam.bats @@ -17,6 +17,7 @@ setup() { } @test "a wrong password stops getting in" { + require_measured_libpam run authenticates "anything at all" [ "$status" -eq 0 ] @@ -28,6 +29,7 @@ setup() { } @test "the empty password stops getting in as well, through this stack" { + require_measured_libpam run authenticates "" [ "$status" -eq 0 ] @@ -39,6 +41,7 @@ setup() { } @test "a stack that keeps the option still lets anything in, which is what the field is for" { + require_measured_libpam run "$SCRIPT" [ "$status" -eq 0 ] From 4fb88c8b18aa481ecac6f9b66059b558cdf6a69f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:37:41 +0000 Subject: [PATCH 8/9] docs: changelog, coverage and comments re-derived from the real pam_unix The changelog said removing nullok stops a wrong password and the field stops the empty one. Asked of the real module, on the libpam an appliance runs, the old field with nullok let any password in, the field '*' closes it through every stack, and removing nullok from Webmin's stack narrows what reaches it. The rootpass entry now says it is the change that holds the account shut, and why; the webmin-pam entry says what it does and that common-auth keeps the option (Keel-Linux/common#13, opened for it, with why it needs a pam-auth-update profile and not an edit). The webmin-enable entry gains the later boots and the recovery, and samba-rootpass gets its own. COVERAGE.md: five files, 48 of 48 lines, 57 bats, and the paragraph on what is real rewritten: libpam through pam_authenticate, the version dependence and the five tests the CI runner skips because of it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- COVERAGE.md | 49 ++++++++++++++++++++------------------- changes/turnkey.changelog | 37 +++++++++++++++++++---------- conf/turnkey.d/webmin-pam | 4 +++- 3 files changed, 53 insertions(+), 37 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index 5908ea93..5f82f8c0 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -4,43 +4,44 @@ Measured on 2026-09-24 against upstream 19.x (b60dd23), following the project decision 0003 (90 percent floor per repository, 95 percent for every file our changes touch). -## Branch fix/webmin-auth-hardening: 100 percent, four files (2026-09-28) +## Branch fix/webmin-auth-hardening: 100 percent, five files (2026-09-29) `tests/coverage.sh` now measures a list of files rather than one, and every -file this branch touches is on it. Measured with kcov 43 over 36 bats: +file this branch touches is on it. Measured with kcov 43 over 57 bats on +Debian 13: | File | Lines | Covered | Percent | |------|-------|---------|---------| +| conf/samba-rootpass | 8 | 8 | 100 | | conf/turnkey.d/postfix-local | 17 | 17 | 100 | | conf/turnkey.d/rootpass | 6 | 6 | 100 | -| conf/turnkey.d/webmin-enable | 9 | 9 | 100 | +| conf/turnkey.d/webmin-enable | 10 | 10 | 100 | | conf/turnkey.d/webmin-pam | 7 | 7 | 100 | -| Total | 39 | 39 | 100 | +| Total | 48 | 48 | 100 | -`conf/turnkey.d/rootpass` and `conf/turnkey.d/webmin-enable` changed shebang -from `/bin/sh` to `/bin/bash`, because kcov measures bash and not dash and -decision 0003 gives no exemption for a file a change touches. rootpass uses -no bash construct; webmin-enable is new code in a file that was one line. +`conf/turnkey.d/rootpass`, `conf/turnkey.d/webmin-enable` and +`conf/samba-rootpass` changed shebang from `/bin/sh` to `/bin/bash`, because +kcov measures bash and not dash and decision 0003 gives no exemption for a +file a change touches. The gate in `.github/workflows/tests.yml` stays at 100, the measured number. -Every exit path is covered: `rootpass` in its three branches (a build -password, no build password, chroot only), `webmin-pam` on both of its -`fatal` paths and the happy one, `webmin-enable` on its single path with the -three verdicts systemd gives its conditions. - The suite asserts behaviour and not configuration. The question "can this -password get in" is answered by the real `crypt(3)` through perl, which is -the function pam_unix compares with, wrapped in the two rules Linux-PAM -applies around it; those rules were confirmed against `unix_chkpwd`, the -helper pam_unix itself runs, with a scratch shadow file bind mounted over -`/etc/shadow` under `unshare -r -m`. The verdict on whether the web -interface would start is `systemd-analyze condition`, systemd's own -evaluator, and the units are handed to `systemd-analyze verify`. The shadow -tools are the one thing stubbed, because `usermod`, `chpasswd` and `passwd` -chroot into the root they are given and cannot be pointed at a scratch tree -by an ordinary user; each stub records the behaviour it reproduces and the -command that measured it. +password get in" is put to the real Linux-PAM: `tests/pam-authenticate` +calls `pam_authenticate` through libpam over the scratch image's own stack, +in a private mount namespace with the scratch shadow and passwd files bind +mounted over the real ones, so pam_unix and its unix_chkpwd helper are the +installed ones. An earlier version of the suite modelled pam_unix by hand +and reported an acceptance the module does not make; nothing is modelled +now. What pam_unix makes of the crypt() of the empty string differs between +libpam 1.5 and 1.7, so the five tests that depend on it run on 1.7, the +appliance's, and skip by name elsewhere: the CI runner has 1.5.3 and skips +them. The verdict on whether the web interface would start is +`systemd-analyze condition` over the packaged `webmin.service` and the +drop-in together, both handed to `systemd-analyze verify`. The shadow tools +and `smbpasswd` are the things stubbed, because they chroot into the root +they are given or talk to a daemon; each stub records the behaviour it +reproduces. `tests/before-firstboot.bats` runs the three conf scripts over one scratch image in the order a build runs them and asks the whole question of the diff --git a/changes/turnkey.changelog b/changes/turnkey.changelog index 10886318..6e594e1f 100644 --- a/changes/turnkey.changelog +++ b/changes/turnkey.changelog @@ -2,26 +2,39 @@ turnkey-core-19.0 (1) turnkey; urgency=low * An appliance no longer carries an account that can be authenticated before its first boot has set a password. A build given no 'ROOT_PASS' - wrote a password field into the root account that a submitted password - could match; it now writes one that no password matches, and the first - boot writes over it as before. The field is deliberately not '!', which - shadow reads as a locked account: 'passwd --unlock root', which - turnkey-sudoadmin runs on every first boot, refuses to unlock a field of - exactly '!', exits 3 and leaves the rest of that hook undone. + wrote 'U6aMy0wojraho', the crypt() of the empty string, into the root + account. On libpam 1.7 (Debian 13), through any PAM stack carrying + 'nullok', Debian's common-auth among them, pam_unix authenticates that + field with any password without asking for one. It now writes '*', + which pam_unix refuses whatever is submitted and whatever the stack + says, and the first boot writes over it as before. This is the change + that holds the account shut; the two below narrow what reaches it. + The field is deliberately not '!', which shadow reads as a locked + account: 'passwd --unlock root', which turnkey-sudoadmin runs on every + first boot, refuses to unlock a field of exactly '!', exits 3 and + leaves the rest of that hook undone. * A build time 'ROOT_PASS' holding a glob character is now set as written. It was expanded against the build directory before reaching chpasswd. + * 'conf/samba-rootpass', which runs after rootpass on a Samba appliance, + wrote the old field back, expanded 'ROOT_PASS' the same way and traced + it into the build log. It now writes '*', quotes the password and does + not trace. + * 'nullok' is removed from the PAM stack the webmin package installs, in - the new 'webmin-pam' conf script. It allowed an account whose password - is not set to be authenticated without one, which no appliance account - is meant to be, and nothing in an appliance asked for it. + the new 'webmin-pam' conf script, so that an account whose password is + not set cannot be authenticated through Webmin without one. Nothing in + an appliance asked for it. 'common-auth' keeps it for now (#13). * Webmin waits for the first boot scripts before it starts. Its unit gains a condition on the marker inithooks writes when its boot scripts have - run, and a path unit starts it when that marker appears, so the web - interface is not exposed before the appliance has been configured. An - image carrying no inithooks at all is unaffected. + run, and a path unit starts it when that marker appears and records + that it has, so every later boot starts Webmin with the rest of the + boot. The marker means the scripts ran, not that they succeeded. A + first boot that never finishes keeps Webmin shut; 'systemctl start + webmin-after-firstboot.service' opens it by hand, for good. An image + carrying no inithooks at all is unaffected. * Upgraded base distribution to Debian 13.x/Trixie. diff --git a/conf/turnkey.d/webmin-pam b/conf/turnkey.d/webmin-pam index eee03d98..ef4ad189 100755 --- a/conf/turnkey.d/webmin-pam +++ b/conf/turnkey.d/webmin-pam @@ -4,7 +4,9 @@ # carries 'nullok' on the pam_unix line, which tells pam_unix that an # account whose password is not set may be authenticated without one. No # appliance account is meant to be reachable that way, and nothing in an -# appliance asks for the option, so it goes. +# appliance asks for the option, so it goes. Debian's common-auth carries it +# too and keeps it for now: pam-auth-update regenerates that file, so it +# needs a profile rather than an edit (Keel-Linux/common#13). # # PAM_WEBMIN is a test hook; a build leaves it unset. From 680d6da5a50901079982677b26ae6f5babfbba0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Tue, 29 Sep 2026 03:46:09 +0000 Subject: [PATCH 9/9] test: the whole question includes the stack everything else uses tests/before-firstboot.bats claims to ask a built image the only question worth asking, and asked it through Webmin's stack only. Measured against the previous rootpass with this branch's webmin-pam, on libpam 1.7, all its password tests passed: removing nullok from that one stack is enough to close the old field there. So nothing in the file would notice rootpass going back to 'U6aMy0wojraho'. The new test asks through common-auth as a built core carries it, which keeps nullok (#13), and fails against the previous rootpass. With it the file fails 4 of its 6 tests against the previous rootpass and webmin-enable, and 1 (this one) against the previous rootpass alone. COVERAGE.md counts 58 bats. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01SfWQScmDZ94KCS5DMYrfe6 --- COVERAGE.md | 2 +- tests/before-firstboot.bats | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/COVERAGE.md b/COVERAGE.md index 5f82f8c0..fa83cdf7 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -7,7 +7,7 @@ file our changes touch). ## Branch fix/webmin-auth-hardening: 100 percent, five files (2026-09-29) `tests/coverage.sh` now measures a list of files rather than one, and every -file this branch touches is on it. Measured with kcov 43 over 57 bats on +file this branch touches is on it. Measured with kcov 43 over 58 bats on Debian 13: | File | Lines | Covered | Percent | diff --git a/tests/before-firstboot.bats b/tests/before-firstboot.bats index 6774fba8..4cffe790 100644 --- a/tests/before-firstboot.bats +++ b/tests/before-firstboot.bats @@ -45,6 +45,16 @@ build_image() { [ "$status" -eq 0 ] } +# Webmin is one way in. The console login, su and everything else that +# includes common-auth are the others, and common-auth keeps nullok +# (Keel-Linux/common#13), so this is the question only the account field +# answers. +@test "and none gets in through the stack everything else uses either" { + build_image + run nothing_authenticates root "$PAM_COMMON_AUTH" + [ "$status" -eq 0 ] +} + @test "and the interface is not up to be asked" { build_image run ! unit_would_start "$DROPIN"