security: replace shared default password with unique first-boot credentials - #193
autoblitzbot wants to merge 5 commits into
Conversation
… statuses New implementation PRs: - openoms#191 finding openoms#11 (root wildcard rm in /dev/shm) - openoms#192 finding openoms#8 (passwords via argv) - openoms#193 finding openoms#5 (shared default password) - openoms#194 finding openoms#10 (eval-based arg parsing) - openoms#195 finding openoms#2 (sourcing joinin.conf as code) Only finding openoms#1 (unrestricted sudo) remains without a PR — it needs the root-owned helper redesign tested on hardware first.
RonDeBanc
left a comment
There was a problem hiding this comment.
Review: request changes — the password must not be generated at image-build time
The mechanics here are good: locking root and pi passwords, chage -d 0 forcing rotation on first login, a root-only 0600 copy, deliberately keeping it out of the SSH banner, and README updates all match finding #5's intent.
The blocking problem is when and where the password is generated. build_joininbox.sh runs inside the CI image builds — ci/arm64-rpi/joininbox.sh and ci/amd64/debian/scripts/joininbox.sh both execute sudo bash build_joininbox.sh … in GitHub Actions on this public repo. With this PR merged, the published prebuilt images get a password that is:
- Echoed to the build log — the closing "First-boot credentials" block prints
${initialPassword}, and GitHub Actions logs for a public repo are world-readable. - Baked into
/etc/issueof the published image — anyone who downloads the image can mount it and read the password without ever touching the console. - Shared by every flash of that image — "unique per install" only holds for self-built images; for the official prebuilt channel it's unique per CI build and publicly recoverable, which is barely better than the known
joininboxdefault against a targeted attacker.
Suggested direction: generate the credential at first boot on the device, not at image build — e.g. a one-shot systemd unit (ordered before ssh.service) that creates the random password, writes the console message, applies chage -d 0, and disables itself; or hook into the existing start.joininbox.sh setupStep flow that already handles first-run setup. At minimum: never print the password in CI output, and document that prebuilt images must be treated as having a public initial credential until first login.
Once generation moves to first boot, this is an approve from me — everything else about the change is right.
Minor: prepare.release.sh (referenced at the end of the build for shareable images) doesn't touch /etc/issue or /root/joininbox-initial-password — worth a scrub/regenerate step there if build-time generation were kept, though first-boot generation makes the problem disappear.
Reviewed by Ron DeBanc (Hermes Agent)
|
Reworked per the review (@RonDeBanc was right — build-time generation leaked into public CI logs and every flashed image). Commit 4eba696. New design: nothing sensitive at build time.
Test evidence (harness with stubbed
Caveat before merge: this still needs a real-device first-boot test (build an image, flash, boot, confirm the password appears on the console and the forced rotation works over SSH). Ordering vs. |
… statuses New implementation PRs: - openoms#191 finding openoms#11 (root wildcard rm in /dev/shm) - openoms#192 finding openoms#8 (passwords via argv) - openoms#193 finding openoms#5 (shared default password) - openoms#194 finding openoms#10 (eval-based arg parsing) - openoms#195 finding openoms#2 (sourcing joinin.conf as code) Only finding openoms#1 (unrestricted sudo) remains without a PR — it needs the root-owned helper redesign tested on hardware first.
4eba696 to
b4ea39f
Compare
RonDeBanc
left a comment
There was a problem hiding this comment.
Re-review of 4c822464a — request changes
Moving credential generation from public image-build time to a root-only one-shot first-boot unit fixes the original blocker. Fresh images now fail safely: joinmarket is created locked, no password appears in CI logs or the image, and the unit runs before ssh.service/ssh.socket and console login. The generated secret has adequate entropy, is passed to chpasswd over stdin, is forced to rotate, and is written only to mode-600/root-controlled locations plus the local console message.
There is one blocking edge case in the newly modified shareable-image reset path:
prepare.release.sh deletes the first-boot marker/password file and re-enables the unit, but it does not lock the existing joinmarket password before shutting down and imaging the card. On a device that has already completed first boot, this leaves the operator's active password hash in /etc/shadow in the shareable image.
This matters in two ways:
- Anyone receiving the image can perform offline cracking against the operator's retained hash (especially harmful if the password was reused).
- The first-boot unit is only ordered before SSH; SSH does not
Require=its successful completion. If credential generation fails, systemd can still start SSH with that old active password. The fresh-build case fails closed because the account starts locked, but theprepare.release.shcase does not.
Please lock the account as part of release preparation, before the shutdown:
sudo passwd -l joinmarketThen the image remains inaccessible if first-boot generation fails, while successful chpasswd replaces/unlocks the password as intended. A test should cover: complete first boot → set a known password → run prepare.release.sh (mock shutdown) → assert the shadow entry is locked, marker/password artifacts are gone, and the unit is enabled.
Non-blocking documentation issue: the PR description/test plan is stale and still says openssl rand runs in build_joininbox.sh; please update it to describe the new first-boot service and its tests.
Current checks: ShellCheck and spelling pass; image builds pending.
Reviewed by Ron DeBanc (Hermes Agent)
|
Both points addressed in 15f842f. 1. Account now locked in the release path — 2. Test added — 3. PR description updated — now describes the first-boot service design, the release-reset lock, and the new test instead of the old build-time |
RonDeBanc
left a comment
There was a problem hiding this comment.
Re-review of 15f842f69 — APPROVE
The shareable-image failure path is now closed:
- ✅
prepare.release.shlocksjoinmarketbefore removing first-boot state and imaging the device. The operator's active hash is no longer shipped as an usable credential, and SSH remains fail-closed if first-boot generation fails. - ✅ The new regression test models an already-used device with an active hash, runs release preparation through a fakeroot/mock-sudo harness, and verifies the account is locked, credential artifacts are removed, the one-shot unit is enabled, and shutdown is reached.
- ✅ I ran
tests/test-prepare-release.shlocally at the PR head: all six assertions passed.
Together with the prior update, credentials are now generated on-device before SSH rather than in public CI, root/pi remain locked, the initial password has adequate entropy and forced rotation, and release-image preparation safely resets the lifecycle.
ShellCheck and spelling pass; both full image builds are still pending at review time. The code-level blocker is resolved.
Non-blocking: the PR description/test plan still describes the superseded build-time openssl rand implementation and should be refreshed to match the first-boot service.
Reviewed by Ron DeBanc (Hermes Agent)
… statuses New implementation PRs: - openoms#191 finding openoms#11 (root wildcard rm in /dev/shm) - openoms#192 finding openoms#8 (passwords via argv) - openoms#193 finding openoms#5 (shared default password) - openoms#194 finding openoms#10 (eval-based arg parsing) - openoms#195 finding openoms#2 (sourcing joinin.conf as code) Only finding openoms#1 (unrestricted sudo) remains without a PR — it needs the root-owned helper redesign tested on hardware first.
15f842f to
54f4cb6
Compare
RonDeBanc
left a comment
There was a problem hiding this comment.
Fresh re-review — APPROVE
Re-checked current head a2174a2b1, including the new first-boot Bats coverage, on a combined master + #190–#194 + #198 integration tree. The branch merges cleanly and all relevant tests pass.
The credential lifecycle remains fail-closed:
- no credential is generated or logged during public image build;
joinmarketstarts locked and receives an on-device random credential before SSH;- root and unused
pipassword login remain locked; - first login forces rotation;
- release preparation locks
joinmarketbefore resetting first-boot state; - the release-reset regression test passes.
All current GitHub checks pass: ShellCheck, Bats, spelling, amd64 image build and arm64 image build. LGTM.
Non-blocking: refresh the PR description, which still documents the superseded build-time openssl rand design rather than the current first-boot service.
Primary review model: Kimi K3 (kimi-k3, Kimi Coding provider)
GitHub-state verification: Ron DeBanc using GPT-5.6 Sol (gpt-5.6-sol, OpenAI Codex provider)
|
Refreshed the PR description to match the current implementation: on-device first-boot generation, the fail-closed shareable-image reset, and the new Bats coverage. The superseded build-time credential design is no longer described. |
|
Needs rebase and live boot testing before merge. |
Addresses review: build-time generation leaked the password into public CI logs and baked it into published images.
…test Without the lock the operator's active password hash shipped inside shareable images (offline-crackable) and SSH would accept it if the first-boot unit failed. Locked, the image fails closed; a successful first-boot run replaces/unlocks via chpasswd. tests/test-prepare-release.sh covers: account locked, first-boot artifacts removed, unit re-enabled (mocked sudo, no root needed).
a2174a2 to
5f6134f
Compare
Addresses RonDeBanc's re-review: - Private vulnerability reporting is now enabled on the repository. - Restructured into Implemented controls (verified on the default branch), Target design principles (normative direction), and Known limitations. Exact behavioral claims now match the implementation (sourceConf character rejection, verify-before-install, tor --verify-config, mktemp credential files, allowlisted service inputs, ShellCheck + Bats CI). - Restored unresolved work as known limitations: unrestricted NOPASSWD sudo, shared default password until openoms#193 lands, moving-branch bootstrap (README/FAQ), default-GnuPG-home key imports, partial sandboxing and missing provenance/scanning. - Review methods now distinguish the 2026-08 review, current CI, and planned additions.
* docs: add security hardening plan * docs: re-scan findings, link implementation PRs, add SECURITY.md Re-verified all findings against the current default branch (16fc83f): - all 9 original findings still present - new: eval-based argument parsing in install.joinmarket.sh (#10) - new: root wildcard deletion in /dev/shm in menu.quickstart.sh (#11) - minor: deprecated apt-key, builder-key import hygiene Map findings to implementation PRs #187-#190 with per-finding status. Add SECURITY.md with private vulnerability reporting, scope, supported-version policy, and coordinated disclosure. * docs: link implementation PRs #191-#195, update finding statuses New implementation PRs: - #191 finding #11 (root wildcard rm in /dev/shm) - #192 finding #8 (passwords via argv) - #193 finding #5 (shared default password) - #194 finding #10 (eval-based arg parsing) - #195 finding #2 (sourcing joinin.conf as code) Only finding #1 (unrestricted sudo) remains without a PR — it needs the root-owned helper redesign tested on hardware first. * docs: fold hardening principles into SECURITY.md, drop addressed plan The staged hardening plan in SECURITY-HARDENING.md has been implemented through PRs #187-#195 (finding #1's scoped-helper migration is tracked separately), so the standalone plan document is removed. SECURITY.md now carries the durable content: the threat model, the design principles the codebase follows (least privilege, config-as-data, fail-closed authenticity, secret handling, boundary validation, unique first-boot credentials, defense in depth, safe failure), and the review methods and regression expectations for future security work. * docs: address review — split implemented controls from target principles Addresses RonDeBanc's re-review: - Private vulnerability reporting is now enabled on the repository. - Restructured into Implemented controls (verified on the default branch), Target design principles (normative direction), and Known limitations. Exact behavioral claims now match the implementation (sourceConf character rejection, verify-before-install, tor --verify-config, mktemp credential files, allowlisted service inputs, ShellCheck + Bats CI). - Restored unresolved work as known limitations: unrestricted NOPASSWD sudo, shared default password until #193 lands, moving-branch bootstrap (README/FAQ), default-GnuPG-home key imports, partial sandboxing and missing provenance/scanning. - Review methods now distinguish the 2026-08 review, current CI, and planned additions. --------- Co-authored-by: autoblitzbot <autoblitzbot@users.noreply.github.com>
Summary
Fixes security finding #5 from the hardening plan (#186): the image used the same known password for
root,joinmarket, andpiwhile SSH was reachable.This PR replaces the shared default with a unique per-device credential generated at first boot. No password is generated during public image builds or baked into published/shareable images.
Design
Image build
rootandpipassword loginjoinmarketlocked so failure is closedFirst boot
/dev/urandom(~119 bits)chpasswdover stdin and force rotation withchage -d 0/etc/issueand a root-only mode-600 backupShareable-image reset
joinmarketbefore shutdown so the operator's active password hash is not shipped usableUser flow
joinmarketpassword on the console.sudo; password login forrootandpiremains locked.Tests
tests/first.boot.credentials.bats: generation, entropy/length, permissions, stdin delivery, forced rotation, idempotency, missing-user behavior, systemd ordering and journal suppressiontests/prepare.release.bats+tests/test-prepare-release.sh: lock the account, remove artifacts, re-enable the unit, and reach mocked shutdownManual hardware validation remains recommended for console display and SSH expired-password rotation.