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

[marcos-mendez]

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]

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]

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.