Hold an image shut until its first boot has set a password #12

Merged

marcos-mendez merged 11 commits from fix/webmin-auth-hardening into 19.x

2026-09-29 01:36:26 -03:00

marcos-mendez commented

2026-09-28 06:06:19 -03:00

(Migrated from github.com)

Closes #11.

The problem

A built appliance has accounts before its first boot has set a password on
any of them, and three things in the build made that state more open than it
has to be.

conf/turnkey.d/rootpass, for a build given no ROOT_PASS, wrote a
password field that a submitted password can match, rather than one that
matches nothing until a password is set. The PAM stack the webmin package
installs carries nullok, which lets an account whose password is not set
be authenticated without one. And the web interface starts with the rest of
the boot, before the first boot scripts that configure the appliance have
run at all.

Established before changing anything

Why the placeholder existed, and why it cannot simply become !.
/etc/default/inithooks ships SUDOADMIN=false, so firstboot.d/29sudoadmin
runs turnkey-sudoadmin off --disable-setpass on every first boot, before
30rootpass. That path calls passwd --unlock root. Measured against
shadow in a scratch root:

for field in '!' '*' 'U6aMy0wojraho' '!<hash>' ''; do
    unshare -r passwd --root "$dir" -u root; done

field '!'       exit 3, unchanged, "unlocking the password would
                result in a passwordless account"
field '*'       exit 0, unchanged
field '!<hash>' exit 0, '<hash>'

turnkey-sudoadmin runs under bash -e, so exit 3 there ends the hook
before permitrootlogin_ssh, restart_sshd and set_status, and
/etc/sudoadmin/state is never written. Locking the account is therefore
the fix that breaks the first boot, which docs/traps.md puts at the top of
the list. * is what the account gets: crypt() returns it for no input,
passwd --unlock accepts it, and bin/setpass.py sets the real password
with chpasswd, which replaces the field whatever it held.

What the placeholder was for. Nothing in the first boot authenticates
against it. sshd ships PermitRootLogin yes but OpenSSH defaults
PermitEmptyPasswords no, so it was never an SSH login. The first boot
dialog runs on tty1 with Conflicts=getty@tty1.service, so there is no
login prompt to answer during it either. What it bought was a console login
on an image whose first boot has not run.

Why not allow= or a loopback bind in miniserv.conf. Both outlive the
first boot: nothing removes them, so an allow= written at build time locks
out remote administration for good, and a loopback bind has to be rewritten
and the service restarted by something later. Holding the unit back needs no
undoing, so that is what is here instead.

The change

conf/turnkey.d/rootpass writes *, with the reason ! is not used in a
comment. 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 got set; it is quoted now.
conf/turnkey.d/webmin-pam, new, takes nullok out of the stack and
fails rather than report success if the stack is absent or the option
survives.
conf/turnkey.d/webmin-enable holds the web interface back. webmin.service
gains a condition on the marker inithooks writes when its boot scripts are
done, and a path unit starts it when the marker appears. The conditions are
systemd triggering conditions, an OR, so an image with 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.
conf/samba-rootpass, which runs after rootpass on a Samba appliance and
wrote the old field back, writes * too, quotes the password and no
longer traces it into the build log.

What each part does, asked of the real pam_unix on libpam 1.7.0-5 (the
appliance's): the old field let any password in, without a prompt,
through any stack carrying nullok, which includes Debian's common-auth.
* refuses everything through every stack, so rootpass is the change
that holds the account shut. Removing nullok from Webmin's stack also
closes the old field there on 1.7 (not on libpam 1.5, where the empty
password then gets in); common-auth keeps it, #13. The hold keeps the
interface down until the first boot's scripts have run, and the marker
means they ran, not that they succeeded.

Tests, and what is real in them

58 bats. The question "would this password get in" is put to Linux-PAM
itself: 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 bind mounted over the real ones, so pam_unix
and its unix_chkpwd helper are the installed ones. (The first version
modelled pam_unix by hand, and review found the model wrong.) 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 and skip by
name elsewhere; the CI runner has 1.5.3. Whether the interface would start
is answered by systemd-analyze condition over the packaged
webmin.service and the drop-in together, and both are handed to
systemd-analyze verify.

The shadow tools and smbpasswd are stubbed: usermod, chpasswd and
passwd chroot into the root they are given, so an ordinary user cannot
aim them at a scratch tree. 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, through
Webmin's stack and through common-auth. With this branch's webmin-pam
and the previous rootpass and webmin-enable, it fails 4 of its 6
tests; with only rootpass reverted, 1. tests/rootpass.bats fails 5 of
9 against the previous script, tests/samba-rootpass.bats 4 of 6.

Coverage: 100 percent, 48 of 48 lines over the five measured files, kcov 43.
tests/coverage.sh measures a list rather than one file. rootpass and
webmin-enable moved 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. The gate stays at 100.

What this does not do

Nothing is built, signed or published, and no build lock was taken, so no
appliance has been booted with this. The end to end verdict belongs to an
appliance boot test.

The marker the unit waits for is /run/inithooks-complete, which today's
inithooks already writes, so this stands on its own and needs no ordering
against Keel-Linux/inithooks#17 or #18. The first boot that reaches it
records that in /var/lib/webmin-after-firstboot/opened, and every boot
after starts Webmin without waiting. A first boot that never finishes keeps
Webmin shut until systemctl start webmin-after-firstboot.service is run
from a console.

common-auth still carries nullok: #13.

Closes #11. ## The problem A built appliance has accounts before its first boot has set a password on any of them, and three things in the build made that state more open than it has to be. `conf/turnkey.d/rootpass`, for a build given no `ROOT_PASS`, wrote a password field that a submitted password can match, rather than one that matches nothing until a password is set. The PAM stack the `webmin` package installs carries `nullok`, which lets an account whose password is not set be authenticated without one. And the web interface starts with the rest of the boot, before the first boot scripts that configure the appliance have run at all. ## Established before changing anything **Why the placeholder existed, and why it cannot simply become `!`.** `/etc/default/inithooks` ships `SUDOADMIN=false`, so `firstboot.d/29sudoadmin` runs `turnkey-sudoadmin off --disable-setpass` on every first boot, before `30rootpass`. That path calls `passwd --unlock root`. Measured against shadow in a scratch root: for field in '!' '*' 'U6aMy0wojraho' '!<hash>' ''; do unshare -r passwd --root "$dir" -u root; done field '!' exit 3, unchanged, "unlocking the password would result in a passwordless account" field '*' exit 0, unchanged field '!<hash>' exit 0, '<hash>' `turnkey-sudoadmin` runs under `bash -e`, so exit 3 there ends the hook before `permitrootlogin_ssh`, `restart_sshd` and `set_status`, and `/etc/sudoadmin/state` is never written. Locking the account is therefore the fix that breaks the first boot, which `docs/traps.md` puts at the top of the list. `*` is what the account gets: `crypt()` returns it for no input, `passwd --unlock` accepts it, and `bin/setpass.py` sets the real password with `chpasswd`, which replaces the field whatever it held. **What the placeholder was for.** Nothing in the first boot authenticates against it. `sshd` ships `PermitRootLogin yes` but OpenSSH defaults `PermitEmptyPasswords no`, so it was never an SSH login. The first boot dialog runs on tty1 with `Conflicts=getty@tty1.service`, so there is no login prompt to answer during it either. What it bought was a console login on an image whose first boot has not run. **Why not `allow=` or a loopback bind in `miniserv.conf`.** Both outlive the first boot: nothing removes them, so an `allow=` written at build time locks out remote administration for good, and a loopback bind has to be rewritten and the service restarted by something later. Holding the unit back needs no undoing, so that is what is here instead. ## The change - `conf/turnkey.d/rootpass` writes `*`, with the reason `!` is not used in a comment. 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 got set; it is quoted now. - `conf/turnkey.d/webmin-pam`, new, takes `nullok` out of the stack and fails rather than report success if the stack is absent or the option survives. - `conf/turnkey.d/webmin-enable` holds the web interface back. `webmin.service` gains a condition on the marker inithooks writes when its boot scripts are done, and a path unit starts it when the marker appears. The conditions are systemd triggering conditions, an OR, so an image with 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. - `conf/samba-rootpass`, which runs after rootpass on a Samba appliance and wrote the old field back, writes `*` too, quotes the password and no longer traces it into the build log. What each part does, asked of the real `pam_unix` on libpam 1.7.0-5 (the appliance's): the old field let *any* password in, without a prompt, through any stack carrying `nullok`, which includes Debian's `common-auth`. `*` refuses everything through every stack, so `rootpass` is the change that holds the account shut. Removing `nullok` from Webmin's stack also closes the old field there on 1.7 (not on libpam 1.5, where the empty password then gets in); `common-auth` keeps it, #13. The hold keeps the interface down until the first boot's scripts have run, and the marker means they ran, not that they succeeded. ## Tests, and what is real in them 58 bats. The question "would this password get in" is put to Linux-PAM itself: `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 bind mounted over the real ones, so `pam_unix` and its `unix_chkpwd` helper are the installed ones. (The first version modelled `pam_unix` by hand, and review found the model wrong.) 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 and skip by name elsewhere; the CI runner has 1.5.3. Whether the interface would start is answered by `systemd-analyze condition` over the packaged `webmin.service` and the drop-in together, and both are handed to `systemd-analyze verify`. The shadow tools and `smbpasswd` are stubbed: `usermod`, `chpasswd` and `passwd` chroot into the root they are given, so an ordinary user cannot aim them at a scratch tree. 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, through Webmin's stack and through `common-auth`. With this branch's `webmin-pam` and the previous `rootpass` and `webmin-enable`, it fails 4 of its 6 tests; with only `rootpass` reverted, 1. `tests/rootpass.bats` fails 5 of 9 against the previous script, `tests/samba-rootpass.bats` 4 of 6. Coverage: 100 percent, 48 of 48 lines over the five measured files, kcov 43. `tests/coverage.sh` measures a list rather than one file. `rootpass` and `webmin-enable` moved 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. The gate stays at 100. ## What this does not do Nothing is built, signed or published, and no build lock was taken, so no appliance has been booted with this. The end to end verdict belongs to an appliance boot test. The marker the unit waits for is `/run/inithooks-complete`, which today's inithooks already writes, so this stands on its own and needs no ordering against Keel-Linux/inithooks#17 or #18. The first boot that reaches it records that in `/var/lib/webmin-after-firstboot/opened`, and every boot after starts Webmin without waiting. A first boot that never finishes keeps Webmin shut until `systemctl start webmin-after-firstboot.service` is run from a console. `common-auth` still carries `nullok`: #13.

marcos-mendez commented

2026-09-28 07:03:50 -03:00

(Migrated from github.com)

Reviewed as a security change. I re-measured every claim rather than reading
them; where I disagree it is because a measurement disagreed, and the
commands are given so you can settle it either way.

Reproduced as stated: the passwd --unlock table (! exits 3 and leaves
the field, * and a hash exit 0 and leave the field, !<hash> exits 0 and
strips the !), measured with unshare -r passwd --root; that
turnkey-sudoadmin runs user_state root unlock under bash -e ahead of
restart_sshd and set_status, so the exit 3 really does end that hook;
that bin/setpass.py pipes to chpasswd and nothing in inithooks or
confconsole reads the field; crypt() returns *0 for * on every input I
tried; coverage 100.00 over the four files, 39/39, and tests/rootpass.bats
failing 5 of 9 against the previous script.

HIGH — tests/helpers.bash:83-104, and the prose derived from it.
blank_equivalent and authenticates are a hand-written model of
pam_unix, and the model is missing two rules the module has. In
verify_pwd_hash() (libpam 1.7.0-5, modules/pam_unix/passverify.c:80-92)
an empty submitted password is refused outright, before any comparison, when
the stack does not carry nullok; and a stored field whose first character
is * or ! is refused outright whatever was submitted. The helper models
neither, so it reports at least one acceptance the module does not, and the
suite is green against a pam_unix that does not exist. That is
docs/traps.md, "Asserting the configuration is not asserting the
behaviour", with the stub half of "the test stub has to be as impolite as
the program".

The instrument that would have caught it is sound; it was just fed wrong.
pam_read_passwords() (libpam/include/pam_inline.h) reads NUL-separated
strings, so

printf '%s' "$password" | /usr/sbin/unix_chkpwd root nullok

delivers nothing and the helper tests the empty password whatever you piped
in — which is also why that sandbox looked as though it "refused every real
comparison". It does not; the user namespace is not the problem. Add the
terminator and a correct password returns 0:

printf '%s\0' "$password" | /usr/sbin/unix_chkpwd root nullok|nonull

with a scratch shadow bind-mounted over /etc/shadow under unshare -r -m.
That gives a real table over both options and every field you care about.
Please correct the helper to carry both rules, re-run, and re-derive the
"each does part of it" sentence in the PR body and the wording in
changes/turnkey.changelog from the result — one half of it does not
survive the corrected model.

HIGH — conf/turnkey.d/webmin-enable:17-22: there is no way back, and
nothing says so. /run/inithooks-complete is written by inithooks' run
at the end of the everyboot pass, on every boot, and /run is a tmpfs. So
this does not gate one boot; it gates every boot for the life of the
appliance, behind inithooks.service reaching that line. If it ever does
not — docs/traps.md entry 1 is a first boot that blocks forever in one
hook, hit three times here — the appliance has no web interface and no
console, on every boot, and systemctl start webmin exits 0 and starts
nothing, because systemd counts an unmet condition as a skipped, successful
start. At minimum the changelog and the comment need the recovery line
(touch /run/inithooks-complete, or start
webmin-after-firstboot.service). Better would be a second thing the
drop-in accepts that means "this is not the first boot", so a later boot
does not inherit the first boot's gate.

MEDIUM — conf/turnkey.d/webmin-enable: the marker does not mean what the
hold wants it to mean. exec_scripts logs a failing script and carries on,
and the marker is touched afterwards regardless, so a boot in which
30rootpass exits non-zero still opens the interface. That is survivable
only because rootpass now writes a field nothing matches — which makes
rootpass, not the hold, the change carrying the weight. The PR body reads
the other way round; worth correcting, because it changes what a future
reader thinks is safe to weaken.

MEDIUM — conf/turnkey.d/webmin-pam:19: one stack out of the set.
/etc/pam.d/common-auth on a built core carries pam_unix.so nullok as
well — it is Debian's default — and it is the stack nearly everything else
includes. "No appliance account is meant to be reachable that way and
nothing in an appliance asked for the option" applies to it at least as
strongly as to the one file this script edits. Either widen the script or
open the follow-up and name it here.

MEDIUM — conf/samba-rootpass:7,10: both of this PR's defects, untouched.
It carries the same placeholder and the same unquoted $ROOT_PASS expansion
this PR fixes two files away. "No Keel appliance builds it" is a good reason
not to ask an appliance boot about it; it is not a reason to leave the
second copy in the tree while the first is being corrected, and the PR body
points straight at it. It is two lines plus a look at smbpasswd -n.

LOW — conf/turnkey.d/webmin-enable: the drop-in is never verified
against the unit it modifies. systemd-analyze verify is handed the two
new units; the drop-in is only ever read by systemd-analyze condition over
its own Condition lines. A packaged webmin.service that one day carries a
plain Condition*= of its own would AND with these two and never be
satisfied. Verifying webmin.service with the drop-in in place costs one
line.

LOW — tests/before-firstboot.bats: the "four of five" is not reproducible
as written. Against the previous scripts with webmin-pam absent
altogether, all five die at status 127 on the missing script. Keeping
webmin-pam and reverting only the other two gives exactly the four you
quote. Say which baseline the number is against.

LOW — tests/coverage.sh: a three-way conflict, and not a mechanical
one. #5 and #8 both rewrite this file into a targets=("file:suite") shape
with one kcov run per file; this branch rewrites it into one run over a
measured list. Whichever lands second has to be rewritten, not rebased.
This is the security change — land it first and let the other two adopt
whichever shape wins.

The change does what it sets out to do, and the three parts are the right
three. The findings above are about what the tests prove, what happens on
the boot after the one this was written for, and one file left half-done.
None of them is a reason to keep the current state.

Warning

Reviewed as a security change. I re-measured every claim rather than reading them; where I disagree it is because a measurement disagreed, and the commands are given so you can settle it either way. **Reproduced as stated:** the `passwd --unlock` table (`!` exits 3 and leaves the field, `*` and a hash exit 0 and leave the field, `!<hash>` exits 0 and strips the `!`), measured with `unshare -r passwd --root`; that `turnkey-sudoadmin` runs `user_state root unlock` under `bash -e` ahead of `restart_sshd` and `set_status`, so the exit 3 really does end that hook; that `bin/setpass.py` pipes to `chpasswd` and nothing in inithooks or confconsole reads the field; `crypt()` returns `*0` for `*` on every input I tried; coverage 100.00 over the four files, 39/39, and `tests/rootpass.bats` failing 5 of 9 against the previous script. --- **HIGH — `tests/helpers.bash:83-104`, and the prose derived from it.** `blank_equivalent` and `authenticates` are a hand-written model of `pam_unix`, and the model is missing two rules the module has. In `verify_pwd_hash()` (libpam 1.7.0-5, `modules/pam_unix/passverify.c:80-92`) an empty submitted password is refused outright, before any comparison, when the stack does not carry `nullok`; and a stored field whose first character is `*` or `!` is refused outright whatever was submitted. The helper models neither, so it reports at least one acceptance the module does not, and the suite is green against a `pam_unix` that does not exist. That is `docs/traps.md`, "Asserting the configuration is not asserting the behaviour", with the stub half of "the test stub has to be as impolite as the program". The instrument that would have caught it is sound; it was just fed wrong. `pam_read_passwords()` (`libpam/include/pam_inline.h`) reads NUL-separated strings, so printf '%s' "$password" | /usr/sbin/unix_chkpwd root nullok delivers nothing and the helper tests the empty password whatever you piped in — which is also why that sandbox looked as though it "refused every real comparison". It does not; the user namespace is not the problem. Add the terminator and a correct password returns 0: printf '%s\0' "$password" | /usr/sbin/unix_chkpwd root nullok|nonull with a scratch shadow bind-mounted over `/etc/shadow` under `unshare -r -m`. That gives a real table over both options and every field you care about. Please correct the helper to carry both rules, re-run, and re-derive the "each does part of it" sentence in the PR body and the wording in `changes/turnkey.changelog` from the result — one half of it does not survive the corrected model. **HIGH — `conf/turnkey.d/webmin-enable:17-22`: there is no way back, and nothing says so.** `/run/inithooks-complete` is written by inithooks' `run` at the end of the everyboot pass, on every boot, and `/run` is a tmpfs. So this does not gate one boot; it gates every boot for the life of the appliance, behind `inithooks.service` reaching that line. If it ever does not — `docs/traps.md` entry 1 is a first boot that blocks forever in one hook, hit three times here — the appliance has no web interface and no console, on every boot, and `systemctl start webmin` exits 0 and starts nothing, because systemd counts an unmet condition as a skipped, successful start. At minimum the changelog and the comment need the recovery line (`touch /run/inithooks-complete`, or start `webmin-after-firstboot.service`). Better would be a second thing the drop-in accepts that means "this is not the first boot", so a later boot does not inherit the first boot's gate. **MEDIUM — `conf/turnkey.d/webmin-enable`: the marker does not mean what the hold wants it to mean.** `exec_scripts` logs a failing script and carries on, and the marker is touched afterwards regardless, so a boot in which `30rootpass` exits non-zero still opens the interface. That is survivable only because `rootpass` now writes a field nothing matches — which makes `rootpass`, not the hold, the change carrying the weight. The PR body reads the other way round; worth correcting, because it changes what a future reader thinks is safe to weaken. **MEDIUM — `conf/turnkey.d/webmin-pam:19`: one stack out of the set.** `/etc/pam.d/common-auth` on a built core carries `pam_unix.so nullok` as well — it is Debian's default — and it is the stack nearly everything else includes. "No appliance account is meant to be reachable that way and nothing in an appliance asked for the option" applies to it at least as strongly as to the one file this script edits. Either widen the script or open the follow-up and name it here. **MEDIUM — `conf/samba-rootpass:7,10`: both of this PR's defects, untouched.** It carries the same placeholder and the same unquoted `$ROOT_PASS` expansion this PR fixes two files away. "No Keel appliance builds it" is a good reason not to ask an appliance boot about it; it is not a reason to leave the second copy in the tree while the first is being corrected, and the PR body points straight at it. It is two lines plus a look at `smbpasswd -n`. **LOW — `conf/turnkey.d/webmin-enable`: the drop-in is never verified against the unit it modifies.** `systemd-analyze verify` is handed the two new units; the drop-in is only ever read by `systemd-analyze condition` over its own Condition lines. A packaged `webmin.service` that one day carries a plain `Condition*=` of its own would AND with these two and never be satisfied. Verifying `webmin.service` with the drop-in in place costs one line. **LOW — `tests/before-firstboot.bats`: the "four of five" is not reproducible as written.** Against the previous scripts with `webmin-pam` absent altogether, all five die at status 127 on the missing script. Keeping `webmin-pam` and reverting only the other two gives exactly the four you quote. Say which baseline the number is against. **LOW — `tests/coverage.sh`: a three-way conflict, and not a mechanical one.** #5 and #8 both rewrite this file into a `targets=("file:suite")` shape with one kcov run per file; this branch rewrites it into one run over a `measured` list. Whichever lands second has to be rewritten, not rebased. This is the security change — land it first and let the other two adopt whichever shape wins. --- The change does what it sets out to do, and the three parts are the right three. The findings above are about what the tests prove, what happens on the boot after the one this was written for, and one file left half-done. None of them is a reason to keep the current state. **Warning**

marcos-mendez commented

2026-09-29 01:19:39 -03:00

(Migrated from github.com)

Review addressed in 7ba662a..7d4ef76, all new commits. The tests now call pam_authenticate through the real libpam over the scratch stack. On libpam 1.7 the old field let any password in without a prompt through any nullok stack, so * is what holds the account shut. The PR body and changelog now say that, and common-auth is #13. Webmin now opens for good once a first boot completes, with a recovery line, and samba-rootpass is fixed. CI runs libpam 1.5.3, so the 5 version-specific tests skip there by name.

Review addressed in 7ba662a..7d4ef76, all new commits. The tests now call `pam_authenticate` through the real libpam over the scratch stack. On libpam 1.7 the old field let any password in without a prompt through any `nullok` stack, so `*` is what holds the account shut. The PR body and changelog now say that, and `common-auth` is #13. Webmin now opens for good once a first boot completes, with a recovery line, and `samba-rootpass` is fixed. CI runs libpam 1.5.3, so the 5 version-specific tests skip there by name.