From 0b29fa1e6395a21567ccbcf38ec3c9be76d0e8af Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 17 Sep 2026 11:47:31 -0500 Subject: [PATCH 1/4] ssh: authorized_keys moves to /media/fat/config, migrating on first boot The FAT-partition authorized_keys path shipped as /media/fat/linux/... in v2026.09.11-beta, chosen for local consistency with ssh.ext4 and wpa_supplicant.conf. The community had already settled this five years earlier: security_fixes.sh (MiSTer-devel/Scripts_MiSTer) has read /media/fat/config/authorized_keys since v2.1, 2021-12-17. Kreeblah raised it on the forum and asked, reasonably, for one location rather than two -- #183. config/ is also the right directory on its own terms: it holds user configuration, while linux/ holds the boot payload an update rewrites. Everything that made the FAT path work is unchanged -- same partition, same initramfs mount options, so StrictModes stays satisfied. sshd_config now lists .ssh/authorized_keys plus the config/ path, and NOT the old one: two live locations is the confusion #183 exists to end. What makes dropping it safe is a one-time migration in S50sshd, before sshd starts. It merges rather than clobbers when both files exist, verifies the destination holds every key the old file had, and only then removes the old file -- copy, verify, delete, never mv, because that file may be the only way into the box. If the move cannot be completed, sshd starts with the legacy path appended via -o AuthorizedKeysFile for that boot and warns on the console; a failed migration must not cost someone their key login. scripts/test-authorized-keys-migration.sh covers 14 cases and runs each twice, once with the host's GNU coreutils and once with BusyBox applets. That is not belt-and-braces: `grep -F -x -v -f` against an EMPTY pattern file matches nothing under GNU grep and everything under BusyBox, and an empty config/authorized_keys therefore made the merge silently produce an empty file -- destroying the key it was migrating -- while passing a naive verification. Case 12 is that bug; it cannot fail under GNU grep. lint.yml installs busybox-static if the runner lacks it. ci-tests.sh asserts, on the shipped artifacts: the new path is listed, the old one is not, StrictModes is untouched, and S50sshd still carries the migration. Verified against the real ARM userland (BusyBox 1.38.0, bash 5.2.37) in a chroot: 28/28 test cases, both start() paths, and OpenSSH 10.5p1 reporting `AuthorizedKeysFile .ssh/authorized_keys /media/fat/config/authorized_keys` with `StrictModes yes` under sshd -T. Not yet re-run on the rig. Closes #183 Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/lint.yml | 22 ++ .../rootfs-overlay/etc/init.d/S50sshd | 123 +++++++- .../rootfs-overlay/etc/ssh/sshd_config | 17 +- scripts/ci-tests.sh | 42 ++- scripts/test-authorized-keys-migration.sh | 264 ++++++++++++++++++ 5 files changed, 460 insertions(+), 8 deletions(-) create mode 100755 scripts/test-authorized-keys-migration.sh diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index d826b1e6..d1374597 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -18,6 +18,7 @@ # shellcheck -s sh -x package/azcopy/azcopy-profile.sh # scripts/shellcheck-composite-actions.sh # .github/actions/*/action.yml run: bodies # scripts/test-installer-splash.sh # SD-installer first-boot splash (ADR 0020 §6) +# scripts/test-authorized-keys-migration.sh # S50sshd's authorized_keys move (#183) # ################################################################################ @@ -371,3 +372,24 @@ jobs: run: | set -euo pipefail scripts/test-installer-splash.sh + + # Same shape again, for S50sshd's one-time authorized_keys migration + # (issue #183). It deletes the user's old key file once it has copied it, + # so a bug there does not mean "the migration did not run", it means "the + # key is gone" on a box whose password nobody remembers. + # + # BusyBox is installed if the runner image lacks it, because the sharpest + # edge in that function -- `grep -F -x -v -f` against an EMPTY pattern + # file -- behaves the OPPOSITE way under BusyBox and GNU grep, and only + # the BusyBox reading is the one the image ships. Without it, case 12 + # cannot fail. The test reports a loud SKIP rather than a pass if the + # install does not work out, so a broken package name degrades the check + # visibly instead of silently. + - name: authorized_keys migration unit test (scripts/test-authorized-keys-migration.sh) + run: | + set -euo pipefail + if ! command -v busybox >/dev/null 2>&1; then + sudo apt-get update -qq && sudo apt-get install -y -qq busybox-static \ + || echo "::warning::could not install busybox -- the BusyBox pass of the authorized_keys migration test will be SKIPPED" + fi + scripts/test-authorized-keys-migration.sh diff --git a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd index 6cd512ee..4e831ec4 100755 --- a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd +++ b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd @@ -16,12 +16,122 @@ KEYIMG=/media/fat/linux/ssh.ext4 KEYDIR=/etc/ssh_keys +# A USER's key, as opposed to the host keys above: sshd_config's second +# AuthorizedKeysFile path, on the exFAT partition an update never touches. +# AUTHKEYS_LEGACY is where we put it in v2026.09.11-beta, before issue #183 +# established that the community had already standardised on /media/fat/config +# -- security_fixes.sh (MiSTer-devel/Scripts_MiSTer) has read it from there +# since v2.1, 2021-12-17. migrate_authorized_keys() below moves it, once. +AUTHKEYS=/media/fat/config/authorized_keys +AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys + # Make sure the ssh-keygen program exists [ -f /usr/bin/ssh-keygen ] || exit 0 umask 077 +# Move a user's authorized_keys from the pre-#183 location to the standard one. +# A no-op on every boot after the first, and on every box that never used the +# old path (the -f test below is the whole cost in that case). +# +# This runs in init rather than being left to the user because the alternative +# is silent: sshd would simply stop reading the file someone put on the card +# after following our own FAQ, and key login would fail with nothing anywhere +# saying why. The old path is NOT kept as a second AuthorizedKeysFile entry -- +# one location is the point of #183 -- so this migration is what makes dropping +# it safe. +# +# COPY, VERIFY, THEN DELETE. Never mv: while this function runs, the legacy +# file may be the user's only way into the box, so it is removed only once the +# standard file provably contains every key it held. +# +# Returns 0 if there is nothing to do or the migration completed; 1 if the +# legacy file is still the only copy, which start() answers by keeping sshd +# reading it for this boot. +migrate_authorized_keys() { + [ -f "$AUTHKEYS_LEGACY" ] || return 0 + + echo "Migrating $AUTHKEYS_LEGACY -> $AUTHKEYS" + + # Intermediates on tmpfs, not on the card: a half-written merge must not be + # left behind next to the real file if the power goes at the wrong moment. + legacy_lines=/run/authorized_keys.lines.$$ + legacy_keys=/run/authorized_keys.keys.$$ + dest_lines=/run/authorized_keys.dest.$$ + merged=/run/authorized_keys.merged.$$ + rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" + + # Two views of the old file. Every non-blank line is what gets carried over + # -- a "# work laptop" comment is part of the user's file and should survive + # the move -- while the key lines alone answer "is there anything here worth + # keeping?" and give the count reported below. + grep -v '^[[:space:]]*$' "$AUTHKEYS_LEGACY" > "$legacy_lines" + grep -v '^[[:space:]]*#' "$legacy_lines" > "$legacy_keys" + + if [ ! -s "$legacy_keys" ]; then + rm -f "$AUTHKEYS_LEGACY" "$legacy_lines" "$legacy_keys" "$dest_lines" + echo " old file held no keys -- removed" + return 0 + fi + + # An EMPTY pattern file is the trap here: BusyBox grep (what this image + # ships) treats `-f ` as matching EVERY line, where GNU grep matches + # none -- so `grep -F -x -v -f` against a zero-byte $AUTHKEYS silently + # selects nothing and the merge below would produce an empty file, throwing + # the user's key away. Testing $dest_lines for content, rather than testing + # $AUTHKEYS for existence, is what keeps the merge branch off an empty + # pattern file. (Found by scripts/test-authorized-keys-migration.sh case 12, + # running the real BusyBox; it is not reproducible with GNU grep.) + : > "$dest_lines" + [ -f "$AUTHKEYS" ] && grep -v '^[[:space:]]*$' "$AUTHKEYS" > "$dest_lines" + + if [ -s "$dest_lines" ]; then + # Both files have content -- MERGE, never clobber. Someone can + # legitimately have a security_fixes.sh-era key in config/ and a + # different one in linux/ (two PCs, two keys), and either one being + # dropped here would lock a machine out. -F -x -v -f appends only the + # lines config/ does not already have, so re-running cannot duplicate + # a key either. + cat "$AUTHKEYS" > "$merged" + grep -F -x -v -f "$dest_lines" "$legacy_lines" >> "$merged" + else + mkdir -p "${AUTHKEYS%/*}" + cat "$legacy_lines" > "$merged" + fi + + # Written under a temporary name and renamed, so a reader (sshd on a later + # start, or a concurrent one) never sees a partial file at $AUTHKEYS. The + # check is against what actually landed ON THE CARD, not against $merged. + # `[ -s ]` before the grep for the same empty-pattern-file reason as above: + # an empty result file can never be a correct migration of a non-empty key + # list, and must not be allowed to pass the check by matching everything. + if cp "$merged" "$AUTHKEYS.new" && mv "$AUTHKEYS.new" "$AUTHKEYS" && + [ -s "$AUTHKEYS" ] && + ! grep -F -x -v -f "$AUTHKEYS" "$legacy_lines" | grep -q .; then + rm -f "$AUTHKEYS_LEGACY" + sync + echo " OK -- $(wc -l < "$legacy_keys") key(s) migrated" + rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" + return 0 + fi + + echo "WARNING: could not migrate $AUTHKEYS_LEGACY to $AUTHKEYS." >&2 + echo "WARNING: sshd will keep reading the OLD path for this boot, so key" >&2 + echo "WARNING: login still works; move the file by hand to make it stick." >&2 + # Best-effort cleanup, and quiet: whatever made the migration fail (a + # read-only card, a non-directory in the way) tends to make these fail + # too, and their errors would only bury the three lines above. + rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" "$AUTHKEYS.new" 2>/dev/null + return 1 +} + start() { + if migrate_authorized_keys; then + authkeys_fallback= + else + authkeys_fallback=yes + fi + echo 'Checking for SSH host key storage' if [ ! -f "$KEYIMG" ]; then @@ -61,10 +171,19 @@ start() { # -o HostKey overrides sshd_config's compiled paths, so the tmpfs # fallback above is actually honoured (otherwise sshd would read the # persistent /etc/ssh_keys paths from the config and find nothing). - /usr/sbin/sshd \ - -o "HostKey=$KEYDIR/ssh_host_rsa_key" \ + set -- -o "HostKey=$KEYDIR/ssh_host_rsa_key" \ -o "HostKey=$KEYDIR/ssh_host_ecdsa_key" \ -o "HostKey=$KEYDIR/ssh_host_ed25519_key" + + # Migration failed and the user's only key is still at the old path. An + # -o AuthorizedKeysFile REPLACES the config's list, so the two shipped + # paths have to be restated alongside it. Same principle as the host-key + # fallback above: a degraded outcome is fine, no way in is not. + if [ -n "$authkeys_fallback" ]; then + set -- "$@" -o "AuthorizedKeysFile=.ssh/authorized_keys $AUTHKEYS $AUTHKEYS_LEGACY" + fi + + /usr/sbin/sshd "$@" touch /var/lock/sshd echo "OK" } diff --git a/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config b/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config index 91252738..4bcb8f23 100644 --- a/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config +++ b/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config @@ -46,12 +46,25 @@ PermitRootLogin yes # The default is to check both .ssh/authorized_keys and .ssh/authorized_keys2 # but this is overridden so installations will only check .ssh/authorized_keys # -# SECOND PATH ADDED: /media/fat/linux/authorized_keys -- a user's key that +# SECOND PATH ADDED: /media/fat/config/authorized_keys -- a user's key that # SURVIVES AN IMAGE UPDATE. An update replaces linux.img wholesale, so anything # under /root/.ssh -- which lives inside that file -- is destroyed by it. That # is the durable reason for this second path, and it holds regardless of how # the root filesystem happens to be mounted at the time. # +# WHY /media/fat/config AND NOT /media/fat/linux: this is the location the +# community already standardised on. security_fixes.sh in +# MiSTer-devel/Scripts_MiSTer has read /media/fat/config/authorized_keys since +# v2.1 (2021-12-17), copying it to /root/.ssh/authorized_keys when run. It also +# matches what each directory is FOR: /media/fat/config holds user +# configuration (device.bin, core .cfg files), while /media/fat/linux holds the +# boot payload an update rewrites (linux.img, u-boot.img, the DTB). We shipped +# /media/fat/linux/authorized_keys first (v2026.09.11-beta) and moved here after +# Kreeblah pointed the prior art out on the forum -- issue #183. S50sshd +# migrates the old file on first boot, so the change costs existing users +# nothing; the old path is deliberately NOT kept listed here, because one +# location is the entire point. +# # Getting a key into the first path is awkward besides: / is mounted READ-ONLY # at boot (`ro` on the cmdline; inittab's remount-rw line is deliberately left # commented, ADR 0011) and a fresh image ships NO /root/.ssh at all. It does @@ -84,7 +97,7 @@ PermitRootLogin yes # # ORDER MATTERS ONLY FOR PRECEDENCE, NOT FUNCTION: sshd tries every listed # file, so a key in either location works. -AuthorizedKeysFile .ssh/authorized_keys /media/fat/linux/authorized_keys +AuthorizedKeysFile .ssh/authorized_keys /media/fat/config/authorized_keys #AuthorizedPrincipalsFile none diff --git a/scripts/ci-tests.sh b/scripts/ci-tests.sh index d2df288c..55940a7a 100755 --- a/scripts/ci-tests.sh +++ b/scripts/ci-tests.sh @@ -1835,20 +1835,35 @@ require_present "etc/init.d/S50sshd" "S50sshd" # read-only at boot and a fresh image ships no /root/.ssh at all; it becomes # writable only via /etc/profile's remount on interactive login, so putting a # key there by hand needs a login first -- circular when the key IS the login -# method.) /media/fat/linux/authorized_keys is the only location a user can +# method.) /media/fat/config/authorized_keys is the only location a user can # write from an ordinary PC (card reader, any OS) that the update process does # not touch. Dropping this line would silently return every user to "your key # is gone after each update", with nothing else failing. +# +# WHY /media/fat/config AND NOT /media/fat/linux: it is where the community +# already puts this file -- security_fixes.sh (MiSTer-devel/Scripts_MiSTer) has +# read it from there since v2.1, 2021-12-17. We shipped the linux/ path first +# and moved in issue #183; S50sshd migrates the old file once, on boot. The +# checks below pin BOTH halves of that: the new path must be listed, and the +# old one must NOT be (two live locations is the confusion #183 exists to end), +# and the migration must still be present in the shipped init script. if tar_has "etc/ssh/sshd_config"; then sshd_conf="$WORKDIR/sshd_config" tar xOf "$ROOTFS_TAR" ./etc/ssh/sshd_config > "$sshd_conf" 2>/dev/null - if grep -qE '^AuthorizedKeysFile[[:space:]].*[[:space:]]/media/fat/linux/authorized_keys[[:space:]]*$' "$sshd_conf"; then - pass "sshd_config: AuthorizedKeysFile includes /media/fat/linux/authorized_keys (key survives an image update)" + if grep -qE '^AuthorizedKeysFile[[:space:]].*[[:space:]]/media/fat/config/authorized_keys[[:space:]]*$' "$sshd_conf"; then + pass "sshd_config: AuthorizedKeysFile includes /media/fat/config/authorized_keys (key survives an image update)" else - fail "sshd_config: AuthorizedKeysFile includes /media/fat/linux/authorized_keys" \ + fail "sshd_config: AuthorizedKeysFile includes /media/fat/config/authorized_keys" \ "absent -- a user key placed on the FAT partition would be ignored, so SSH key access would be lost on every image update. Actual: $(grep -E '^AuthorizedKeysFile' "$sshd_conf" || echo '')" fi + if grep -qE '^AuthorizedKeysFile[[:space:]].*/media/fat/linux/authorized_keys' "$sshd_conf"; then + fail "sshd_config: the pre-#183 /media/fat/linux path is not listed" \ + "still present -- #183 standardised on /media/fat/config, and keeping both live recreates exactly the 'which file does my key go in?' confusion the change removes. S50sshd migrates the old file; it must not also be read." + else + pass "sshd_config: the pre-#183 /media/fat/linux path is gone (one FAT location, not two)" + fi + # StrictModes must stay at its default (yes). The FAT path above satisfies it # only because the initramfs mounts with fmask=0022,dmask=0022 # (board/mister/common/initramfs-overlay/init); an explicit 'StrictModes no' @@ -1863,6 +1878,25 @@ else fail "sshd_config present" "etc/ssh/sshd_config not in rootfs.tar" fi +# The other half of #183: the shipped S50sshd must still carry the one-time +# move of a pre-#183 key. Without it, anyone who followed the old FAQ silently +# loses key login on the update that lands this change -- sshd would simply +# stop reading the file they put on the card, with nothing to say why. +# Behaviour is unit-tested by scripts/test-authorized-keys-migration.sh; this +# only asserts the code reached the image. +if tar_has "etc/init.d/S50sshd"; then + s50="$WORKDIR/S50sshd" + tar xOf "$ROOTFS_TAR" ./etc/init.d/S50sshd > "$s50" 2>/dev/null + if grep -q 'migrate_authorized_keys' "$s50" && + grep -q '^AUTHKEYS=/media/fat/config/authorized_keys' "$s50" && + grep -q '^AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys' "$s50"; then + pass "S50sshd: carries the one-time /media/fat/linux -> /media/fat/config authorized_keys migration" + else + fail "S50sshd: carries the one-time authorized_keys migration" \ + "migrate_authorized_keys and/or its AUTHKEYS/AUTHKEYS_LEGACY paths are missing from the shipped init script -- existing users' keys would not be moved, and sshd no longer reads the old location" + fi +fi + # ============================================================================= section "P3.8 — MIDI / MT-32 parity" # ============================================================================= diff --git a/scripts/test-authorized-keys-migration.sh b/scripts/test-authorized-keys-migration.sh new file mode 100755 index 00000000..e231deab --- /dev/null +++ b/scripts/test-authorized-keys-migration.sh @@ -0,0 +1,264 @@ +#!/usr/bin/env bash +# +# Unit test for the one-time authorized_keys migration in +# board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd (issue #183). +# +# WHY A SEPARATE TEST. The function moves the user's SSH key from the +# pre-#183 location (/media/fat/linux/authorized_keys) to the standard one +# (/media/fat/config/authorized_keys) and then DELETES the old file. On a box +# whose root password nobody remembers, that file is the only way in -- so a +# bug here is not "the migration did not happen", it is "the key is gone". +# The rest of the SSH plumbing has coverage (scripts/ci-tests.sh asserts the +# shipped sshd_config, the rig proves key auth end to end), but neither of +# those ever runs this function. This does, in about a second, with no build +# artifacts, no QEMU and no privilege. +# +# WHY IT RUNS EVERYTHING TWICE. The target runs BusyBox applets, not GNU +# coreutils, and the two disagree on exactly the edge this function leans on: +# +# grep -F -x -v -f +# GNU grep -> the empty pattern set matches nothing, every line passes +# BusyBox grep -> the empty pattern set matches everything, no line passes +# +# A zero-byte /media/fat/config/authorized_keys (an easy thing for a user to +# leave behind) therefore took a GNU-clean merge and produced an EMPTY result +# under BusyBox -- which also silently satisfied a naive "is anything missing?" +# check, so the old file was deleted. Case 12 below is that bug. It cannot be +# reproduced with GNU grep, which is why the BusyBox pass is not optional +# garnish: it is the pass that matters. +# +# WHAT IT CANNOT TELL YOU. It exercises the function in isolation against a +# sandbox directory, not against exFAT, and not in the context of start(). That +# sshd then actually accepts a key from the migrated file is the rig's job +# (docs/ssh-ftp-parity.md §1.3). +# +# Usage: scripts/test-authorized-keys-migration.sh [path/to/S50sshd] +set -euo pipefail + +ROOT="$(cd -- "$(dirname -- "$0")/.." && pwd)" +S50="${1:-$ROOT/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd}" + +# The shell the target runs for init scripts is bash (/bin/sh -> bash on this +# image, PR #145), but the function is written to POSIX sh; dash is the +# stricter check and is what CI runners ship. Fall back to sh. +SH="$(command -v dash || command -v sh)" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT + +log() { printf '[test-authkeys] %s\n' "$*"; } +die() { printf '[test-authkeys] FATAL: %s\n' "$*" >&2; exit 2; } + +[ -f "$S50" ] || die "no S50sshd at $S50" + +log "script = $S50" +log "shell = $SH" + +# ---------------------------------------------------------------- extraction +# From the AUTHKEYS constants through the end of migrate_authorized_keys() -- +# the first `}` in column 1 closes that function. Taking the constants too +# means the test uses the SHIPPED paths rather than a copy of them that could +# drift. +sed -n '/^AUTHKEYS=/,/^}/p' "$S50" > "$WORK/migrate.sh" +grep -q '^migrate_authorized_keys()' "$WORK/migrate.sh" || die "migrate_authorized_keys() not found in $S50" +grep -q '^}$' "$WORK/migrate.sh" || die "extraction did not reach the end of the function" +grep -q '^start()' "$WORK/migrate.sh" && die "extraction ran past the function into start()" +log "extracted $(wc -l < "$WORK/migrate.sh") lines" + +# Retarget the three absolute paths at the sandbox. Each rewrite is asserted, +# so a rename in S50sshd fails the test loudly instead of quietly testing a +# function that still points at the real /media/fat. +rewrite() { # + local before after + before="$(md5sum < "$WORK/migrate.sh")" + sed -i "$1" "$WORK/migrate.sh" + after="$(md5sum < "$WORK/migrate.sh")" + [ "$before" != "$after" ] || die "path rewrite matched nothing ($2) -- S50sshd changed shape" +} +rewrite "s#^AUTHKEYS=/media/fat/config/authorized_keys#AUTHKEYS=$WORK/card/config/authorized_keys#" "AUTHKEYS" +rewrite "s#^AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys#AUTHKEYS_LEGACY=$WORK/card/linux/authorized_keys#" "AUTHKEYS_LEGACY" +rewrite "s#/run/authorized_keys\.#$WORK/run/authorized_keys.#g" "/run intermediates" + +# ------------------------------------------------------------------- harness +K1='ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1 one@pc' +K2='ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB2 two@laptop' + +L="$WORK/card/linux/authorized_keys" +C="$WORK/card/config/authorized_keys" + +failures=0 +pass_label="" +desc="" +out="" +rc=0 + +reset() { + rm -rf "$WORK/card" "$WORK/run" + mkdir -p "$WORK/card/linux" "$WORK/run" +} + +# Each case runs the function in a fresh shell, so a stray variable cannot leak +# from one case into the next. +migrate() { "$SH" -c ". '$WORK/migrate.sh'; migrate_authorized_keys" 2>&1; } + +# rc is captured, never fatal: a non-zero return is a RESULT here (case 8 +# expects one), and `out=$(...)` under `set -e` would otherwise abort the run. +run() { + out="" + rc=0 + out="$(migrate)" || rc=$? +} + +pass() { printf ' %-58s PASS\n' "$desc"; } +fail() { + printf ' %-58s FAIL\n' "$desc" + printf ' rc=%s out=%s\n' "$rc" "${out:-}" >&2 + printf ' config: %s\n' "$(cat "$C" 2>&1 || true)" >&2 + printf ' legacy: %s\n' "$(cat "$L" 2>&1 || true)" >&2 + failures=$((failures + 1)) +} + +# Non-blank line count, which is what "how many keys landed" means here. +keycount() { grep -c . "$1" 2>/dev/null || true; } + +# --------------------------------------------------------------------- cases +run_suite() { + log "=== pass: $pass_label ===" + + desc="1 legacy only, config dir absent" + reset; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ "$(cat "$C")" = "$K1" ] + then pass; else fail; fi + + desc="2 legacy only, config dir exists, no key file" + reset; mkdir -p "$WORK/card/config"; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ "$(cat "$C")" = "$K1" ] + then pass; else fail; fi + + desc="3 both exist, different keys -> merged, nothing lost" + reset; mkdir -p "$WORK/card/config" + printf '%s\n' "$K2" > "$C"; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && + grep -qxF "$K1" "$C" && grep -qxF "$K2" "$C" && [ "$(keycount "$C")" = 2 ] + then pass; else fail; fi + + desc="4 both exist, same key -> no duplicate" + reset; mkdir -p "$WORK/card/config" + printf '%s\n' "$K1" > "$C"; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ "$(keycount "$C")" = 1 ] + then pass; else fail; fi + + desc="5 legacy holds no keys -> retired, no config file created" + reset; printf '# just a comment\n\n \n' > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ ! -e "$C" ] + then pass; else fail; fi + + desc="6 no legacy file -> silent no-op" + reset; mkdir -p "$WORK/card/config"; printf '%s\n' "$K1" > "$C"; run + if [ "$rc" -eq 0 ] && [ -z "$out" ] && [ "$(cat "$C")" = "$K1" ] + then pass; else fail; fi + + desc="7 comments carried over alongside keys" + reset; printf '# my key\n%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && + grep -qxF "$K1" "$C" && grep -qxF '# my key' "$C" + then pass; else fail; fi + + # A regular file where the config DIRECTORY should be: mkdir -p and the + # copy both fail, which is the "migration impossible" branch. What matters + # is that the user's key is still where sshd's -o fallback will look for + # it, and that the console says so. + desc="8 destination unusable -> rc=1, legacy KEPT, warns" + reset; printf '%s\n' "$K1" > "$L"; : > "$WORK/card/config"; run + if [ "$rc" -eq 1 ] && [ "$(cat "$L")" = "$K1" ] && + printf '%s' "$out" | grep -q WARNING + then pass; else fail; fi + + desc="9 idempotent: second boot is a silent no-op" + reset; printf '%s\n' "$K1" > "$L"; migrate >/dev/null 2>&1 || true; run + if [ "$rc" -eq 0 ] && [ -z "$out" ] && [ "$(cat "$C")" = "$K1" ] + then pass; else fail; fi + + desc="10 no card mounted at all -> silent no-op" + reset; rm -rf "$WORK/card"; run + if [ "$rc" -eq 0 ] && [ -z "$out" ] + then pass; else fail; fi + + desc="11 no /run intermediates left behind" + reset; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ -z "$(ls -A "$WORK/run" 2>/dev/null || true)" ] + then pass; else fail; fi + + # THE ONE THAT ONLY FAILS UNDER BUSYBOX -- see the header. An empty + # destination file must not swallow the key being migrated into it. + desc="12 empty config file present -> legacy key still lands" + reset; mkdir -p "$WORK/card/config"; : > "$C"; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && grep -qxF "$K1" "$C" + then pass; else fail; fi + + # Written from Windows with Notepad. The line keeps its CR either way -- + # the point is that it is not dropped, duplicated or mangled. + desc="13 CRLF legacy file -> the line still arrives, once" + reset; printf '%s\r\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ "$(keycount "$C")" = 1 ] + then pass; else fail; fi + + desc="14 several legacy keys, one already present -> all arrive once" + reset; mkdir -p "$WORK/card/config" + printf '%s\n' "$K2" > "$C"; printf '%s\n%s\n' "$K1" "$K2" > "$L"; run + if [ "$rc" -eq 0 ] && [ "$(keycount "$C")" = 2 ] && + grep -qxF "$K1" "$C" && grep -qxF "$K2" "$C" + then pass; else fail; fi +} + +# ------------------------------------------------------- pass 1: host applets +pass_label="host ($(grep --version 2>/dev/null | head -1 || echo 'unknown grep'))" +run_suite + +# ---------------------------------------------------- pass 2: BusyBox applets +# The applets the image actually ships, on the front of PATH, so the function's +# bare `grep`/`cp`/`mv`/... calls land on BusyBox without the function knowing. +# Each shim names its applet explicitly rather than relying on argv[0], so +# $BUSYBOX may be a wrapper as well as the binary itself -- which is how this +# pass can be pointed at the ARM BusyBox the image really ships: +# +# printf '#!/bin/sh\nexec qemu-arm -L output/target output/target/usr/bin/busybox "$@"\n' > /tmp/bb +# chmod +x /tmp/bb && BUSYBOX=/tmp/bb scripts/test-authorized-keys-migration.sh +BUSYBOX="${BUSYBOX:-$(command -v busybox || true)}" +if [ -n "$BUSYBOX" ]; then + mkdir -p "$WORK/bb" + for applet in grep cat cp mv rm mkdir wc sync ls; do + printf '#!/bin/sh\nexec %s %s "$@"\n' "$BUSYBOX" "$applet" > "$WORK/bb/$applet" + chmod +x "$WORK/bb/$applet" + done + + # Assert the divergence this pass exists for, so that if a future BusyBox + # ever adopts GNU's reading, this test says so out loud instead of just + # going quietly green. (`sed -n 1p`, not `head -1`, throughout this + # section: BusyBox prints a long applet list, and head closing the pipe + # early turns into a SIGPIPE that `set -o pipefail` would treat as a + # failure of the whole script.) + : > "$WORK/empty-pattern" + printf 'a-key-line\n' > "$WORK/one-line" + if "$WORK/bb/grep" -F -x -v -f "$WORK/empty-pattern" "$WORK/one-line" >/dev/null 2>&1; then + log "NOTE: this BusyBox treats an empty -f pattern file the GNU way" + log " (matches nothing), so case 12 is not BusyBox-specific here." + else + log "confirmed: BusyBox 'grep -f ' matches every line (GNU: none)" + fi + + pass_label="busybox ($("$BUSYBOX" 2>&1 | sed -n '1s/.*\(BusyBox v[0-9.]*\).*/\1/p' || true))" + PATH="$WORK/bb:$PATH" run_suite +else + log "SKIP: no busybox found -- the BusyBox pass did not run, and case 12" + log " cannot fail under GNU grep. Install busybox-static, or point" + log " \$BUSYBOX at one, to get it." +fi + +echo +if [ "$failures" -eq 0 ]; then + log "all cases passed" + exit 0 +fi +log "$failures case(s) FAILED" +exit 1 From dee6afdc59a2dcb9c7b229d5c574dab947537207 Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 17 Sep 2026 11:47:31 -0500 Subject: [PATCH 2/4] docs: the FAT authorized_keys lives in /media/fat/config (#183) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit README, the FAQ, ssh-ftp-parity §1.3 and the table row, init-parity's S50sshd row, ci.md's lint.yml overview, the hardening plan's S1 and ADR 0031's Tier 1 all named the pre-#183 linux/ path. §1.3 gains the reasoning for the directory and the migration's design; the FAQ gains a note for users whose key is in the old place (they need do nothing); ci.md's "one non-shell job" paragraph was already stale and now lists all three. The hardening plan and ADR 0031 are still proposals -- only their path references move, no decision in either changes. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 8 ++- docs/ci.md | 21 ++++++-- .../0031-secure-by-default-network-posture.md | 5 +- docs/init-parity.md | 2 +- docs/security-hardening-plan.md | 3 +- docs/ssh-ftp-parity.md | 54 +++++++++++++++---- docs/user/faq.md | 21 ++++++-- 7 files changed, 91 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index 787e8b31..f8d9a57a 100644 --- a/README.md +++ b/README.md @@ -415,11 +415,15 @@ Full write-up with the reasoning for each: [`docs/patch-provenance.md` §10](doc update discards it. (The root filesystem is read-only at boot and only becomes writable once you log in, so putting a key there also means logging in first, which is circular when the key *is* the login method.) `sshd` here also - reads **`/media/fat/linux/authorized_keys`**, on the exFAT partition an update never + reads **`/media/fat/config/authorized_keys`**, on the exFAT partition an update never touches: drop your `.pub` file there from any PC with a card reader and key login keeps working across every future update. No shell access, no script to edit, and `StrictModes` stays on — the initramfs mounts that partition `fmask=0022,dmask=0022`, - which is exactly what `sshd` requires. ([FAQ](docs/user/faq.md#ssh-key-persist)) + which is exactly what `sshd` requires. That is the same file + [`security_fixes.sh`](https://github.com/MiSTer-devel/Scripts_MiSTer) has read since + 2021, so a key set up for stock already works here — the difference is that stock + copies it *into* `linux.img` and so needs the script re-run after every update, while + this image reads it in place. ([FAQ](docs/user/faq.md#ssh-key-persist)) - **OpenSSH 8.6p1 → 10.5p1**, **Samba ~4.14 → 4.24.6**, **BlueZ → 5.86**, **wpa_supplicant 2.9 → 2.12** — the network-facing surface, several release cycles of hardening each. diff --git a/docs/ci.md b/docs/ci.md index 25b331a5..a01a8368 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -2191,11 +2191,22 @@ line — which, for a `runs-on:` typo or an unquoted glob in a rarely-hit branch, can be months. Cheap (a two-binary download, no build) and scoped by `paths:` so it never fires on the 3-hour image build's pushes. -It has since picked up one non-shell job for the same reason: the last step -runs `scripts/sbom-to-dependency-snapshot.py --self-test`. That script is -pure logic over a CSV that `release.yml` only exercises on a tag, so this is -the only place a regression in it surfaces on the PR that caused it. See -[`#dependency-graph-submission`](#dependency-graph-submission). +It has since picked up three jobs that are not linting at all, for the same +reason — each is pure logic that nothing else reaches on a PR, and each is +fast enough to belong in the cheap workflow rather than the 3-hour one: + +- `scripts/sbom-to-dependency-snapshot.py --self-test` — logic over a CSV that + `release.yml` only exercises on a tag. See + [`#dependency-graph-submission`](#dependency-graph-submission). +- `scripts/test-installer-splash.sh` — the SD-installer's first-boot splash, + whose only other coverage needs a built `sdcard.img` and two QEMU boots. +- `scripts/test-authorized-keys-migration.sh` — `S50sshd`'s one-time move of a + user's `authorized_keys` to `/media/fat/config` (issue #183). The step + installs `busybox-static` if the runner image has no BusyBox, because the + migration's sharpest edge (`grep -f` against an empty pattern file) behaves + the *opposite* way under GNU grep, and the BusyBox reading is the one the + image ships. The test prints a loud SKIP rather than a pass if BusyBox is + unavailable. ### push/pull_request split, applied even though this job is cheap diff --git a/docs/decisions/0031-secure-by-default-network-posture.md b/docs/decisions/0031-secure-by-default-network-posture.md index 2cddbdea..6631adc1 100644 --- a/docs/decisions/0031-secure-by-default-network-posture.md +++ b/docs/decisions/0031-secure-by-default-network-posture.md @@ -89,7 +89,8 @@ Everything below was verified on 2026-09-11 against the HIL rig (beta `260904`, the MiSTer update ecosystem; that is a parity fact, not a gap this project can close alone. - `authorized_keys` on the FAT partition already survives updates, satisfies `StrictModes`, - and is CI-asserted (`docs/ssh-ftp-parity.md` §1.3). + and is CI-asserted (`docs/ssh-ftp-parity.md` §1.3). Since issue #183 it lives at + `/media/fat/config/authorized_keys`, the location `security_fixes.sh` already used. ### The tension @@ -110,7 +111,7 @@ the owner's answer to Q1 below; Tier 3 is recorded so it is not re-discovered. ### Tier 1 — invisible to a stock-style user -1. **Key present ⇒ password auth off.** If `/media/fat/linux/authorized_keys` (or +1. **Key present ⇒ password auth off.** If `/media/fat/config/authorized_keys` (or `/root/.ssh/authorized_keys`) is non-empty at sshd start, `S50sshd` passes `-o PasswordAuthentication=no -o KbdInteractiveAuthentication=no`. Implemented in the init script, not `sshd_config`, because a `Match` block cannot test for a file. Lockout diff --git a/docs/init-parity.md b/docs/init-parity.md index 57892210..47d1fc3e 100644 --- a/docs/init-parity.md +++ b/docs/init-parity.md @@ -70,7 +70,7 @@ an equivalent the package set already installs. | `S45bluetooth` | **adapted** (mechanism reproduced, package default neutralized) | Stock's real file is a **symlink** to `/bin/bluetoothd`, which does the ext4-image persistence trick for `/var/lib/bluetooth` (BT pairing keys) that ADR 0015 explicitly mirrors for SSH host keys. Reproduced **byte-identical** (`diff` exit 0) at `bin/bluetoothd`, with `etc/init.d/S45bluetooth` a symlink to it — exactly stock's shape. **Problem found and fixed:** `BR2_PACKAGE_BLUEZ5_UTILS` installs its own `S40bluetoothd`, which starts `bluetoothd` directly with **no** persistence step — on our read-only `/`, `/var/lib/bluetooth` (not in fstab, so not tmpfs) would be unwritable, and running it would race the real `S45bluetooth` over the D-Bus name and the HCI socket. `etc/init.d/S40bluetoothd` is overlaid to a documented no-op stub so bluetoothd starts exactly once, correctly. | | `S49ntp` | **identical** (overlaid to fix a real bug) | Byte-identical to stock's script (`ntpd -g`, runs as root). **Problem found and fixed:** the package's own default `S49ntp` runs `ntpd -u ntp:ntp -g` — dropping privileges to an `ntp` user that **does not exist** in this build's `/etc/passwd` (verified: `grep '^ntp:' output/target/etc/passwd` → no match). Left as the package default, `ntpd` would fail to start on every boot, silently breaking time sync forever. Reverted to stock's root-run form via the overlay. | | `S50proftpd` | **identical** | Byte-for-byte identical to stock (`diff` exit 0). Not overlaid. | -| `S50sshd` | **adapted** (ADR 0015) | Stock's simple shape (`ssh-keygen -A`; bare `/usr/sbin/sshd`; `touch /var/lock/sshd`) is kept, but `ssh-keygen -A` is replaced with the ADR 0015 per-device mechanism: create/mount `/media/fat/linux/ssh.ext4` at `/etc/ssh_keys` (mirrors `bin/bluetoothd`'s own ext4-image idiom almost line for line), then generate the three key types individually into it if missing. See "SSH host keys" below for the full mechanism and why. | +| `S50sshd` | **adapted** (ADR 0015) | Stock's simple shape (`ssh-keygen -A`; bare `/usr/sbin/sshd`; `touch /var/lock/sshd`) is kept, but `ssh-keygen -A` is replaced with the ADR 0015 per-device mechanism: create/mount `/media/fat/linux/ssh.ext4` at `/etc/ssh_keys` (mirrors `bin/bluetoothd`'s own ext4-image idiom almost line for line), then generate the three key types individually into it if missing. Since issue #183 it also performs one further step before starting sshd: a **one-time move** of a user's `authorized_keys` from the pre-#183 `/media/fat/linux/` to `/media/fat/config/`, the location the community's `security_fixes.sh` already used. See "SSH host keys" below for the host-key mechanism and `docs/ssh-ftp-parity.md` §1.3 for the user-key one. | | `S91smb` | **identical** (overlaid to restore stock's opt-in gate) | Byte-identical to stock. **Problem found and fixed:** the package's own default `S91smb` only guards on `/etc/samba/smb.conf` existing; stock has a **second** guard, `[ -f /media/fat/linux/samba.sh ] \|\| exit 0`. Without it, shipping `/etc/samba/smb.conf` (done for config parity, see below) would make Samba **auto-start on every boot** — stock's actual behavior is opt-in (Samba only starts once the user/Downloader drops `samba.sh` onto the FAT partition). Reverted to stock's double-guard form, plus its extra `mkdir -p` calls and the `samba.sh` trailer call. | | `S99user` | **identical** | Not present as a package default (no package provides a MiSTer-specific user hook). Added byte-identical to stock: calls `/media/fat/linux/user-startup.sh` if present. | diff --git a/docs/security-hardening-plan.md b/docs/security-hardening-plan.md index f2579d3b..cd28e4a6 100644 --- a/docs/security-hardening-plan.md +++ b/docs/security-hardening-plan.md @@ -29,7 +29,8 @@ Re-run its three login tests before starting; if any result differs, update the ### S1 — Key present ⇒ password auth off — Size S — Depends: none -`S50sshd`: before starting sshd, if `/media/fat/linux/authorized_keys` or +`S50sshd`: before starting sshd, if `/media/fat/config/authorized_keys` (the standard +location since issue #183; `S50sshd` migrates the old `linux/` one) or `/root/.ssh/authorized_keys` exists and contains at least one non-comment line, and `/media/fat/linux/sshd_allow_password` does **not** exist, append `-o PasswordAuthentication=no -o KbdInteractiveAuthentication=no` to the sshd invocation. diff --git a/docs/ssh-ftp-parity.md b/docs/ssh-ftp-parity.md index f568a321..47a66f2c 100644 --- a/docs/ssh-ftp-parity.md +++ b/docs/ssh-ftp-parity.md @@ -91,7 +91,7 @@ version (`$OpenBSD: sshd_config,v 1.105` header, OpenSSH 10.2p1 per |---|---|---|---| | `PermitRootLogin` | `yes` (uncommented) | `yes` (uncommented, comment added explaining why) | **kept, parity preserved** | | `UsePAM` | `yes` | `yes` | **kept, parity preserved** | -| `AuthorizedKeysFile` | `.ssh/authorized_keys` | `.ssh/authorized_keys` **+ `/media/fat/linux/authorized_keys`** | **intentional divergence, added 2026-09-05** — see §1.3 | +| `AuthorizedKeysFile` | `.ssh/authorized_keys` | `.ssh/authorized_keys` **+ `/media/fat/config/authorized_keys`** | **intentional divergence, added 2026-09-05; the FAT path moved from `linux/` to `config/` on 2026-09-17 (issue #183)** — see §1.3 | | `PermitUserEnvironment` | `yes` | `yes` (comment added: MiSTer scripts rely on it) | identical | | `Subsystem sftp` | `/usr/libexec/sftp-server` | same | identical | | `HostKey` lines | commented defaults (`/etc/ssh/ssh_host_{rsa,dsa,ecdsa,ed25519}_key`) | uncommented, repointed at `/etc/ssh_keys/...`, **no DSA entry** | intentional divergence — ADR 0015, not new | @@ -141,9 +141,40 @@ when the key *is* the login method, and it still does not survive the next updat > only the reasoning needed to be right. **The fix.** `sshd` accepts multiple `AuthorizedKeysFile` paths and tries each in turn, so -the shipped config now lists the stock path *plus* `/media/fat/linux/authorized_keys`. -Nothing else changes: no init script, no bind-mount, no `user-startup.sh` hook, no new -persistence image. +the shipped config now lists the stock path *plus* `/media/fat/config/authorized_keys`. +Nothing else changes: no bind-mount, no `user-startup.sh` hook, no new persistence image. + +**Which FAT directory — `config/`, not `linux/` (issue #183, 2026-09-17).** This shipped +first as `/media/fat/linux/authorized_keys`, chosen for local consistency: `ssh.ext4` +(ADR 0015), `wpa_supplicant.conf` and the boot payload all live in `linux/`. That was +decided without checking the community's prior art, and the prior art is five years +older: `security_fixes.sh` in `MiSTer-devel/Scripts_MiSTer` has read +`/media/fat/config/authorized_keys` since **v2.1 (2021-12-17)**, copying it to +`/root/.ssh/authorized_keys` when run. Raised on the forum by Kreeblah, who reasonably +asked that there be *one* location rather than two. `config/` also matches what the +directories are for: `config/` is user configuration (`device.bin`, core `.cfg` files), +`linux/` is the boot payload an update rewrites. Everything in this section holds +identically for either directory — same partition, same mount options, same +`StrictModes` argument — so only the path moved. + +Note what stock's script does with that file: it *copies* it into `/root/.ssh`, i.e. +into `linux.img`, so on stock it has to be re-run after every OS update. We read it in +place, so the same file needs no script at all. Sharing the location means a user who +set up keys for stock is already set up here. + +**Migration.** `S50sshd` moves an existing `/media/fat/linux/authorized_keys` to the new +path before starting sshd, once: it merges rather than clobbers if both files exist, +verifies the destination holds every key the old file had, and only then deletes the old +file. The old path is deliberately **not** kept as a third `AuthorizedKeysFile` entry — +one location is the point — so the migration is what makes dropping it safe. If the move +cannot be completed (a card mounted read-only, no free space), sshd starts with the +legacy path appended via `-o AuthorizedKeysFile` for that boot and warns on the console; +a failed migration must never be the reason someone cannot log in. Unit-tested by +`scripts/test-authorized-keys-migration.sh`, which runs every case twice — once with the +host's GNU coreutils and once with BusyBox applets, because `grep -F -x -v -f` against an +*empty* pattern file matches nothing under GNU and everything under BusyBox, and only the +BusyBox reading is the one the image ships. That divergence was a real key-destroying bug +caught by the test (case 12), not a hypothetical. **Why not reuse ADR 0015's `ssh.ext4`?** It was considered and rejected. That mechanism is right for *host* keys because the **device** writes them: an ext4 image inside a file on @@ -164,13 +195,18 @@ this cannot be invalidated by a card mounted differently elsewhere. Disabling **Verified on hardware**, not reasoned about: a second `sshd` on port 2223 configured with *only* the FAT path and `StrictModes yes` accepted a key login (OpenSSH 10.5p1, exFAT, -real board). The shipped config additionally passes `sshd -t` and reports both paths under +real board). That test predates #183 and used `linux/`; the directory change does not +affect what it proved (same partition, same mount options), but a rig re-run against +`config/` is owed and is listed as such in the PR. The shipped config additionally passes `sshd -t` and reports both paths under `sshd -T` on the device. -**CI:** `scripts/ci-tests.sh` asserts the FAT path is present in the **shipped** -`sshd_config` (not the overlay source) and that `StrictModes no` is absent — dropping -either would otherwise return every user to "your key is gone after each update" with -nothing failing. User-facing instructions are in +**CI:** `scripts/ci-tests.sh` asserts, against the **shipped** artifacts rather than the +overlay sources, that `sshd_config` lists `/media/fat/config/authorized_keys`, that it no +longer lists the pre-#183 `linux/` path (two live locations is the confusion #183 exists +to end), that `StrictModes no` is absent, and that `S50sshd` still carries the migration. +Dropping any of those would otherwise return some set of users to "your key is gone" with +nothing failing. `scripts/test-authorized-keys-migration.sh` covers the migration's +behaviour and runs on every PR from `lint.yml`. User-facing instructions are in [the FAQ](user/faq.md#ssh-key-persist). diff --git a/docs/user/faq.md b/docs/user/faq.md index c43b1647..a6630b7d 100644 --- a/docs/user/faq.md +++ b/docs/user/faq.md @@ -93,13 +93,13 @@ running this image. ## How do I log in with an SSH key, and make it survive image updates? -Put your **public** key in a file called `authorized_keys` in the `linux` folder on the +Put your **public** key in a file called `authorized_keys` in the `config` folder on the card's main (exFAT) partition — the same partition you see when you put the card in your PC: ``` -/media/fat/linux/authorized_keys # on the box -\linux\authorized_keys # from Windows/macOS with a card reader +/media/fat/config/authorized_keys # on the box +\config\authorized_keys # from Windows/macOS with a card reader ``` Paste in the contents of your **`.pub`** file (e.g. `~/.ssh/id_ed25519.pub`) — one key @@ -119,6 +119,21 @@ is gone again after the next update.) The exFAT partition is never reflashed, so kept there is picked up again after every update. `sshd` reads both locations, so you do not have to choose. +It is also the file the community's `security_fixes.sh` script has used since 2021, so a +key you already set up for stock MiSTer works here as-is — with one difference in your +favour: that script *copies* the key into `linux.img` and therefore has to be re-run +after every OS update, while this image reads the card file directly and never needs it +re-run. + +> **This location moved.** Beta releases up to and including `v2026.09.16-beta` read +> `/media/fat/`**`linux`**`/authorized_keys` instead. If that is where your key is, you do +> not have to do anything: the first boot after updating moves it to +> `/media/fat/config/authorized_keys` for you (merging it in if you already had keys +> there), prints a line to the console saying so, and removes the old file. Nothing is +> deleted until the new file is confirmed to hold every key the old one had, and if the +> move cannot be completed, that boot keeps reading the old location so key login still +> works. + This is the same principle as the per-device host keys above: anything that must outlive an update lives on the data partition, not in the image. From 34e570a37f771e7ee3acea9cce063b06b8dc2eef Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 17 Sep 2026 12:57:13 -0500 Subject: [PATCH 3/4] ssh: fix three migration defects found in review of #184 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three are in migrate_authorized_keys(); two were reproduced before fixing, and each now has a test case that fails against the pre-fix code. 1. HIGH -- a destination file with no trailing newline lost a key. The merge base was `cat "$AUTHKEYS"`, copied byte for byte, while the lines appended to it come from grep and are always newline-terminated. Notepad does not write a final newline, and putting the key on the card from Windows is exactly what the FAQ describes, so an existing config/ key and the first migrated one were spliced into a single invalid line: "…AAAA2222 two@laptopssh-ed25519 AAAA1111 one@pc" The mv had already landed by then, so the verification correctly failed and kept the legacy file -- but the pre-existing key was gone. This is the one case where copy-verify-delete did not protect the file being merged INTO. The base is now $dest_lines (grep output), which is newline-terminated by construction. Case 15. 2. MEDIUM -- an unwritable /run read as "this file holds no keys", and that is the one branch that deletes. Neither grep's redirect was checked, so a failed write and a genuine empty result were indistinguishable; the legacy file was removed and rc=0 meant start() did not even arm the -o fallback. /run is now proven writable by creating a scratch DIRECTORY up front, and the greps go through filter(), which treats status 1 (no match) as success and anything higher as a failed migration. Same guard covers the $AUTHKEYS read, which could otherwise route a non-empty destination into the clobber branch. Case 16. 3. LOW -- sync ran after the unlink rather than between the copy and it. exFAT is not mounted sync here, so a power cut in that window could commit the deletion while the new file was still only in page cache -- precisely the ordering the design exists to rule out. filter() itself needed the explicit `else` to be correct: after a bare `if cmd; then ...; fi`, `$?` is the IF STATEMENT's status (zero when the condition was false), not the command's. Caught by cases 5 and 12 going red. Verified: 16/16 cases in both passes (GNU and the image's real ARM BusyBox 1.38.0 under qemu), each new case red against a mutant with its guard removed; start() re-checked end to end in the chroot; sshd -T unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../rootfs-overlay/etc/init.d/S50sshd | 99 +++++++++++++++---- scripts/test-authorized-keys-migration.sh | 41 ++++++-- 2 files changed, 112 insertions(+), 28 deletions(-) diff --git a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd index 4e831ec4..564b05c7 100755 --- a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd +++ b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd @@ -16,6 +16,12 @@ KEYIMG=/media/fat/linux/ssh.ext4 KEYDIR=/etc/ssh_keys +# Make sure the ssh-keygen program exists +[ -f /usr/bin/ssh-keygen ] || exit 0 + +umask 077 + +# -------------------------------------------------------------------------- # A USER's key, as opposed to the host keys above: sshd_config's second # AuthorizedKeysFile path, on the exFAT partition an update never touches. # AUTHKEYS_LEGACY is where we put it in v2026.09.11-beta, before issue #183 @@ -25,10 +31,37 @@ KEYDIR=/etc/ssh_keys AUTHKEYS=/media/fat/config/authorized_keys AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys -# Make sure the ssh-keygen program exists -[ -f /usr/bin/ssh-keygen ] || exit 0 +# The three lines every failure path prints. A failed migration is survivable +# by design -- sshd still starts, and start() keeps the old path in its +# AuthorizedKeysFile list for this boot -- so it costs a console warning, not +# a login. +migrate_warn() { + echo "WARNING: could not migrate $AUTHKEYS_LEGACY to $AUTHKEYS." >&2 + echo "WARNING: sshd will keep reading the OLD path for this boot, so key" >&2 + echo "WARNING: login still works; move the file by hand to make it stick." >&2 +} -umask 077 +# grep into a file, distinguishing "matched nothing" from "went wrong". +# +# Status 1 is "no lines matched", a legitimate result here (an old file of +# nothing but blank lines). Status 2 and up -- a read error, or the shell +# failing to create the output file -- must NOT reach the caller as an empty +# file, because "empty" is what its delete branch keys off: an unwritable +# /run would otherwise read as "this file holds no keys" and take the user's +# key with it. +filter() { # + _out=$1 + shift + # The `else` is load-bearing: with a bare `if ... ; fi`, `$?` afterwards is + # the IF STATEMENT's status -- zero when the condition was false and there + # was no branch to run -- not grep's. Inside the else it is still grep's. + if grep "$@" > "$_out"; then + return 0 + else + _st=$? + [ "$_st" -eq 1 ] + fi +} # Move a user's authorized_keys from the pre-#183 location to the standard one. # A no-op on every boot after the first, and on every box that never used the @@ -55,21 +88,33 @@ migrate_authorized_keys() { # Intermediates on tmpfs, not on the card: a half-written merge must not be # left behind next to the real file if the power goes at the wrong moment. - legacy_lines=/run/authorized_keys.lines.$$ - legacy_keys=/run/authorized_keys.keys.$$ - dest_lines=/run/authorized_keys.dest.$$ - merged=/run/authorized_keys.merged.$$ - rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" + # + # A DIRECTORY, created first, because creating it is what proves /run is + # writable BEFORE anything below trusts an empty file -- see filter(). + scratch=/run/authorized_keys.$$ + if ! mkdir "$scratch"; then + migrate_warn + return 1 + fi + legacy_lines="$scratch/legacy-lines" + legacy_keys="$scratch/legacy-keys" + dest_lines="$scratch/dest-lines" + merged="$scratch/merged" # Two views of the old file. Every non-blank line is what gets carried over # -- a "# work laptop" comment is part of the user's file and should survive # the move -- while the key lines alone answer "is there anything here worth # keeping?" and give the count reported below. - grep -v '^[[:space:]]*$' "$AUTHKEYS_LEGACY" > "$legacy_lines" - grep -v '^[[:space:]]*#' "$legacy_lines" > "$legacy_keys" + if ! filter "$legacy_lines" -v '^[[:space:]]*$' "$AUTHKEYS_LEGACY" || + ! filter "$legacy_keys" -v '^[[:space:]]*#' "$legacy_lines"; then + migrate_warn + rm -rf "$scratch" + return 1 + fi if [ ! -s "$legacy_keys" ]; then - rm -f "$AUTHKEYS_LEGACY" "$legacy_lines" "$legacy_keys" "$dest_lines" + rm -f "$AUTHKEYS_LEGACY" + rm -rf "$scratch" echo " old file held no keys -- removed" return 0 fi @@ -83,7 +128,12 @@ migrate_authorized_keys() { # pattern file. (Found by scripts/test-authorized-keys-migration.sh case 12, # running the real BusyBox; it is not reproducible with GNU grep.) : > "$dest_lines" - [ -f "$AUTHKEYS" ] && grep -v '^[[:space:]]*$' "$AUTHKEYS" > "$dest_lines" + if [ -f "$AUTHKEYS" ] && + ! filter "$dest_lines" -v '^[[:space:]]*$' "$AUTHKEYS"; then + migrate_warn + rm -rf "$scratch" + return 1 + fi if [ -s "$dest_lines" ]; then # Both files have content -- MERGE, never clobber. Someone can @@ -92,7 +142,13 @@ migrate_authorized_keys() { # dropped here would lock a machine out. -F -x -v -f appends only the # lines config/ does not already have, so re-running cannot duplicate # a key either. - cat "$AUTHKEYS" > "$merged" + # + # The base is $dest_lines, NOT `cat "$AUTHKEYS"`: grep output is always + # newline-terminated, while the file on the card need not be -- Notepad + # does not add a final newline, and that is the exact workflow the FAQ + # describes. Copying it raw would splice the first migrated key onto + # the end of the user's existing one, destroying both. + cat "$dest_lines" > "$merged" grep -F -x -v -f "$dest_lines" "$legacy_lines" >> "$merged" else mkdir -p "${AUTHKEYS%/*}" @@ -108,20 +164,25 @@ migrate_authorized_keys() { if cp "$merged" "$AUTHKEYS.new" && mv "$AUTHKEYS.new" "$AUTHKEYS" && [ -s "$AUTHKEYS" ] && ! grep -F -x -v -f "$AUTHKEYS" "$legacy_lines" | grep -q .; then - rm -f "$AUTHKEYS_LEGACY" + # sync BEFORE the delete, not after. exFAT here is mounted without + # `sync` (unlike the host-key image above, which asks for it + # explicitly), so a power cut between these two lines could otherwise + # commit the unlink while the new file is still only in page cache -- + # the one ordering this whole copy-verify-delete dance exists to rule + # out. sync + rm -f "$AUTHKEYS_LEGACY" echo " OK -- $(wc -l < "$legacy_keys") key(s) migrated" - rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" + rm -rf "$scratch" return 0 fi - echo "WARNING: could not migrate $AUTHKEYS_LEGACY to $AUTHKEYS." >&2 - echo "WARNING: sshd will keep reading the OLD path for this boot, so key" >&2 - echo "WARNING: login still works; move the file by hand to make it stick." >&2 + migrate_warn # Best-effort cleanup, and quiet: whatever made the migration fail (a # read-only card, a non-directory in the way) tends to make these fail # too, and their errors would only bury the three lines above. - rm -f "$legacy_lines" "$legacy_keys" "$dest_lines" "$merged" "$AUTHKEYS.new" 2>/dev/null + rm -rf "$scratch" 2>/dev/null + rm -f "$AUTHKEYS.new" 2>/dev/null return 1 } diff --git a/scripts/test-authorized-keys-migration.sh b/scripts/test-authorized-keys-migration.sh index e231deab..780e109f 100755 --- a/scripts/test-authorized-keys-migration.sh +++ b/scripts/test-authorized-keys-migration.sh @@ -55,14 +55,15 @@ log "script = $S50" log "shell = $SH" # ---------------------------------------------------------------- extraction -# From the AUTHKEYS constants through the end of migrate_authorized_keys() -- -# the first `}` in column 1 closes that function. Taking the constants too -# means the test uses the SHIPPED paths rather than a copy of them that could -# drift. -sed -n '/^AUTHKEYS=/,/^}/p' "$S50" > "$WORK/migrate.sh" -grep -q '^migrate_authorized_keys()' "$WORK/migrate.sh" || die "migrate_authorized_keys() not found in $S50" -grep -q '^}$' "$WORK/migrate.sh" || die "extraction did not reach the end of the function" -grep -q '^start()' "$WORK/migrate.sh" && die "extraction ran past the function into start()" +# The AUTHKEYS constants through the last helper, stopping at start(). Taking +# the constants too means the test uses the SHIPPED paths rather than a copy of +# them that could drift; stopping at start() rather than at the first `}` is +# what lets the migration be split across helper functions. +sed -n '/^AUTHKEYS=/,/^start()/p' "$S50" | sed '$d' > "$WORK/migrate.sh" +for want in '^AUTHKEYS=' '^migrate_authorized_keys()' '^migrate_warn()' '^filter()'; do + grep -q "$want" "$WORK/migrate.sh" || die "$want not found in the extracted section of $S50" +done +grep -q '^start()' "$WORK/migrate.sh" && die "extraction ran past the helpers into start()" log "extracted $(wc -l < "$WORK/migrate.sh") lines" # Retarget the three absolute paths at the sandbox. Each rewrite is asserted, @@ -77,7 +78,7 @@ rewrite() { # } rewrite "s#^AUTHKEYS=/media/fat/config/authorized_keys#AUTHKEYS=$WORK/card/config/authorized_keys#" "AUTHKEYS" rewrite "s#^AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys#AUTHKEYS_LEGACY=$WORK/card/linux/authorized_keys#" "AUTHKEYS_LEGACY" -rewrite "s#/run/authorized_keys\.#$WORK/run/authorized_keys.#g" "/run intermediates" +rewrite "s#/run/authorized_keys\.#$WORK/run/authorized_keys.#g" "/run scratch dir" # ------------------------------------------------------------------- harness K1='ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1 one@pc' @@ -209,6 +210,28 @@ run_suite() { if [ "$rc" -eq 0 ] && [ "$(keycount "$C")" = 2 ] && grep -qxF "$K1" "$C" && grep -qxF "$K2" "$C" then pass; else fail; fi + # The destination file written by Notepad: no final newline. `cat`ing it + # as the merge base spliced the first migrated key onto the end of the + # existing one -- destroying BOTH keys, and doing it before the + # verification could veto anything, since the copy had already landed. + desc="15 destination without a trailing newline -> no spliced line" + reset; mkdir -p "$WORK/card/config" + printf '%s' "$K2" > "$C"; printf '%s\n' "$K1" > "$L"; run + if [ "$rc" -eq 0 ] && [ ! -e "$L" ] && [ "$(keycount "$C")" = 2 ] && + grep -qxF "$K1" "$C" && grep -qxF "$K2" "$C" + then pass; else fail; fi + + # /run unusable. A grep that cannot write its output produces an empty + # file, which used to be indistinguishable from "this file holds no keys" + # -- the one branch that deletes. A regular file where the scratch + # directory must go reproduces it regardless of privilege. + desc="16 scratch dir unusable -> rc=1, legacy KEPT, nothing deleted" + reset; printf '%s\n' "$K1" > "$L"; rm -rf "$WORK/run"; : > "$WORK/run"; run + if [ "$rc" -eq 1 ] && [ "$(cat "$L")" = "$K1" ] && + printf '%s' "$out" | grep -q WARNING + then pass; else fail; fi + rm -f "$WORK/run" + } # ------------------------------------------------------- pass 1: host applets From bf1051a2ac6972e544345201aa6ab259fd3a8df0 Mon Sep 17 00:00:00 2001 From: "Michael C. Ferguson" Date: Thu, 17 Sep 2026 20:16:48 -0500 Subject: [PATCH 4/4] ssh: trim the shipped overlay comments, move the reasoning to docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit S50sshd and sshd_config are copied to every device; they should not carry essay-length rationale. Comment blocks in both are now at most two lines, and what they used to spell out moved into the docs that already cover these files: - docs/ssh-ftp-parity.md §1.3 gains an "Implementation notes" list -- the /run scratch-directory probe and filter()'s status rule, filter()'s load-bearing `else`, the empty -f pattern file behaving oppositely under BusyBox and GNU grep, why the merge base is grep output rather than `cat` of the card file, why sync sits between the copy and the delete, and why the fallback restates all three AuthorizedKeysFile paths. - docs/init-parity.md gains the host-key tmpfs fallback as item 8: why sshd must start even when the ext4 mount fails, and why that only works because the host keys are passed with -o. S50sshd: 277 -> 197 lines, code byte-identical (verified by diffing both versions with comments stripped). sshd_config: our added comment lines 71 -> 19; stock's own template comments are untouched, since trimming those would be gratuitous divergence from the file the parity audit compares against. Re-verified after the trim: 16/16 cases in both passes (GNU and the image's real ARM BusyBox 1.38.0), the three ci-tests gates, and OpenSSH 10.5p1 in the chroot still reporting PermitRootLogin/StrictModes/AuthorizedKeysFile/ PermitUserEnvironment unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../rootfs-overlay/etc/init.d/S50sshd | 148 ++++-------------- .../rootfs-overlay/etc/ssh/sshd_config | 60 +------ docs/init-parity.md | 9 ++ docs/ssh-ftp-parity.md | 32 ++++ 4 files changed, 79 insertions(+), 170 deletions(-) diff --git a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd index 564b05c7..57c25d76 100755 --- a/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd +++ b/board/mister/de10nano/rootfs-overlay/etc/init.d/S50sshd @@ -2,15 +2,8 @@ # # sshd Starts sshd. # -# P2.3 / ADR 0015: SSH host keys are NOT shipped in this image. Every stock -# MiSTer bakes the SAME ssh_host_* private keys into a read-only rootfs (all -# dated 2016-12-31) -- a real vulnerability: trivial host impersonation, -# and no host-key-mismatch warning to tip a user off, since every box's key -# "matches". We generate a unique key set per device, on first boot, and -# persist it to a small ext4 image on the FAT data partition -- mirroring -# stock's OWN mechanism for exactly this shape of problem (Bluetooth -# pairing keys also need real Unix permissions on a permission-less FAT -# card): see bin/bluetoothd and docs/decisions/0015-per-device-ssh-host-keys.md. +# Per-device host keys (ADR 0015) + the one-time user authorized_keys move +# (#183). Full reasoning: docs/init-parity.md, docs/ssh-ftp-parity.md §1.3. # KEYIMG=/media/fat/linux/ssh.ext4 @@ -21,40 +14,26 @@ KEYDIR=/etc/ssh_keys umask 077 -# -------------------------------------------------------------------------- -# A USER's key, as opposed to the host keys above: sshd_config's second -# AuthorizedKeysFile path, on the exFAT partition an update never touches. -# AUTHKEYS_LEGACY is where we put it in v2026.09.11-beta, before issue #183 -# established that the community had already standardised on /media/fat/config -# -- security_fixes.sh (MiSTer-devel/Scripts_MiSTer) has read it from there -# since v2.1, 2021-12-17. migrate_authorized_keys() below moves it, once. +# A USER's key, on the exFAT partition an update never touches. AUTHKEYS_LEGACY +# is the pre-#183 location, migrated below. See docs/ssh-ftp-parity.md §1.3. AUTHKEYS=/media/fat/config/authorized_keys AUTHKEYS_LEGACY=/media/fat/linux/authorized_keys -# The three lines every failure path prints. A failed migration is survivable -# by design -- sshd still starts, and start() keeps the old path in its -# AuthorizedKeysFile list for this boot -- so it costs a console warning, not -# a login. +# A failed migration is survivable: sshd still starts, and start() keeps the old +# path in its AuthorizedKeysFile list for this boot. migrate_warn() { echo "WARNING: could not migrate $AUTHKEYS_LEGACY to $AUTHKEYS." >&2 echo "WARNING: sshd will keep reading the OLD path for this boot, so key" >&2 echo "WARNING: login still works; move the file by hand to make it stick." >&2 } -# grep into a file, distinguishing "matched nothing" from "went wrong". -# -# Status 1 is "no lines matched", a legitimate result here (an old file of -# nothing but blank lines). Status 2 and up -- a read error, or the shell -# failing to create the output file -- must NOT reach the caller as an empty -# file, because "empty" is what its delete branch keys off: an unwritable -# /run would otherwise read as "this file holds no keys" and take the user's -# key with it. +# grep into a file, distinguishing "matched nothing" (status 1, fine) from "went +# wrong" (2+), which must never reach the caller as an empty file -- see §1.3. filter() { # _out=$1 shift - # The `else` is load-bearing: with a bare `if ... ; fi`, `$?` afterwards is - # the IF STATEMENT's status -- zero when the condition was false and there - # was no branch to run -- not grep's. Inside the else it is still grep's. + # The `else` is load-bearing: after a bare `if ...; fi`, `$?` is the IF + # STATEMENT's status, not grep's. Inside the else it is still grep's. if grep "$@" > "$_out"; then return 0 else @@ -63,34 +42,15 @@ filter() { # fi } -# Move a user's authorized_keys from the pre-#183 location to the standard one. -# A no-op on every boot after the first, and on every box that never used the -# old path (the -f test below is the whole cost in that case). -# -# This runs in init rather than being left to the user because the alternative -# is silent: sshd would simply stop reading the file someone put on the card -# after following our own FAQ, and key login would fail with nothing anywhere -# saying why. The old path is NOT kept as a second AuthorizedKeysFile entry -- -# one location is the point of #183 -- so this migration is what makes dropping -# it safe. -# -# COPY, VERIFY, THEN DELETE. Never mv: while this function runs, the legacy -# file may be the user's only way into the box, so it is removed only once the -# standard file provably contains every key it held. -# -# Returns 0 if there is nothing to do or the migration completed; 1 if the -# legacy file is still the only copy, which start() answers by keeping sshd -# reading it for this boot. +# Move a user's authorized_keys to the #183 location, once. COPY, VERIFY, THEN +# DELETE -- never mv: that file may be the only way into the box. migrate_authorized_keys() { [ -f "$AUTHKEYS_LEGACY" ] || return 0 echo "Migrating $AUTHKEYS_LEGACY -> $AUTHKEYS" - # Intermediates on tmpfs, not on the card: a half-written merge must not be - # left behind next to the real file if the power goes at the wrong moment. - # - # A DIRECTORY, created first, because creating it is what proves /run is - # writable BEFORE anything below trusts an empty file -- see filter(). + # Scratch on tmpfs, as a DIRECTORY: creating it proves /run is writable + # before anything below trusts an empty file. scratch=/run/authorized_keys.$$ if ! mkdir "$scratch"; then migrate_warn @@ -101,10 +61,8 @@ migrate_authorized_keys() { dest_lines="$scratch/dest-lines" merged="$scratch/merged" - # Two views of the old file. Every non-blank line is what gets carried over - # -- a "# work laptop" comment is part of the user's file and should survive - # the move -- while the key lines alone answer "is there anything here worth - # keeping?" and give the count reported below. + # Non-blank lines are what gets carried over (a user's comments included); + # the key lines alone decide whether there is anything worth keeping. if ! filter "$legacy_lines" -v '^[[:space:]]*$' "$AUTHKEYS_LEGACY" || ! filter "$legacy_keys" -v '^[[:space:]]*#' "$legacy_lines"; then migrate_warn @@ -119,14 +77,8 @@ migrate_authorized_keys() { return 0 fi - # An EMPTY pattern file is the trap here: BusyBox grep (what this image - # ships) treats `-f ` as matching EVERY line, where GNU grep matches - # none -- so `grep -F -x -v -f` against a zero-byte $AUTHKEYS silently - # selects nothing and the merge below would produce an empty file, throwing - # the user's key away. Testing $dest_lines for content, rather than testing - # $AUTHKEYS for existence, is what keeps the merge branch off an empty - # pattern file. (Found by scripts/test-authorized-keys-migration.sh case 12, - # running the real BusyBox; it is not reproducible with GNU grep.) + # Test $dest_lines for content, not $AUTHKEYS for existence: an EMPTY + # pattern file matches every line under BusyBox grep and none under GNU. : > "$dest_lines" if [ -f "$AUTHKEYS" ] && ! filter "$dest_lines" -v '^[[:space:]]*$' "$AUTHKEYS"; then @@ -136,18 +88,8 @@ migrate_authorized_keys() { fi if [ -s "$dest_lines" ]; then - # Both files have content -- MERGE, never clobber. Someone can - # legitimately have a security_fixes.sh-era key in config/ and a - # different one in linux/ (two PCs, two keys), and either one being - # dropped here would lock a machine out. -F -x -v -f appends only the - # lines config/ does not already have, so re-running cannot duplicate - # a key either. - # - # The base is $dest_lines, NOT `cat "$AUTHKEYS"`: grep output is always - # newline-terminated, while the file on the card need not be -- Notepad - # does not add a final newline, and that is the exact workflow the FAQ - # describes. Copying it raw would splice the first migrated key onto - # the end of the user's existing one, destroying both. + # MERGE, never clobber: two PCs can mean two keys. Base is $dest_lines, + # not `cat "$AUTHKEYS"`, which splices when the card file lacks a \n. cat "$dest_lines" > "$merged" grep -F -x -v -f "$dest_lines" "$legacy_lines" >> "$merged" else @@ -155,21 +97,13 @@ migrate_authorized_keys() { cat "$legacy_lines" > "$merged" fi - # Written under a temporary name and renamed, so a reader (sshd on a later - # start, or a concurrent one) never sees a partial file at $AUTHKEYS. The - # check is against what actually landed ON THE CARD, not against $merged. - # `[ -s ]` before the grep for the same empty-pattern-file reason as above: - # an empty result file can never be a correct migration of a non-empty key - # list, and must not be allowed to pass the check by matching everything. + # Rename so no reader sees a partial file, then verify what landed ON THE + # CARD. `-s` guards the empty-pattern-file trap described above. if cp "$merged" "$AUTHKEYS.new" && mv "$AUTHKEYS.new" "$AUTHKEYS" && [ -s "$AUTHKEYS" ] && ! grep -F -x -v -f "$AUTHKEYS" "$legacy_lines" | grep -q .; then - # sync BEFORE the delete, not after. exFAT here is mounted without - # `sync` (unlike the host-key image above, which asks for it - # explicitly), so a power cut between these two lines could otherwise - # commit the unlink while the new file is still only in page cache -- - # the one ordering this whole copy-verify-delete dance exists to rule - # out. + # sync BEFORE the delete: exFAT is not mounted sync, so the new data + # must reach the card before the old file's unlink can. sync rm -f "$AUTHKEYS_LEGACY" echo " OK -- $(wc -l < "$legacy_keys") key(s) migrated" @@ -178,9 +112,8 @@ migrate_authorized_keys() { fi migrate_warn - # Best-effort cleanup, and quiet: whatever made the migration fail (a - # read-only card, a non-directory in the way) tends to make these fail - # too, and their errors would only bury the three lines above. + # Quiet: whatever failed the migration tends to fail these too, and the + # errors would only bury the warning above. rm -rf "$scratch" 2>/dev/null rm -f "$AUTHKEYS.new" 2>/dev/null return 1 @@ -202,15 +135,8 @@ start() { mkdir -p "$KEYDIR" mount -o sync,dirsync,noatime,nodiratime "$KEYIMG" "$KEYDIR" - # Persistence is best-effort; sshd starting is NOT. If the ext4 mount did - # not take (missing /media/fat, corrupt image, no free loop device), then - # $KEYDIR is the READ-ONLY rootfs and ssh-keygen below cannot write there - # -- sshd would then come up with no host key and refuse every connection. - # That is the one outcome we must never ship, because serial is then the - # only way back in. Fall back to a tmpfs keydir so sshd ALWAYS starts; - # keys are non-persistent in that mode (regenerated each boot), which is a - # tolerable degradation, and we say so loudly on the console so it is - # diagnosable rather than silent. + # Persistence is best-effort; sshd starting is NOT. No mount means $KEYDIR + # is the read-only rootfs, so fall back to tmpfs and say so loudly. if ! mountpoint -q "$KEYDIR"; then echo "WARNING: could not mount $KEYIMG at $KEYDIR -- SSH host keys" >&2 echo "WARNING: will be EPHEMERAL this boot (regenerated, not persisted)." >&2 @@ -218,28 +144,22 @@ start() { mkdir -p "$KEYDIR" fi - # Create any missing keys -- first boot only, each type generated - # individually so it lands FLAT in $KEYDIR (ssh-keygen -A -f instead - # uses its argument as a prefix ahead of the whole compiled-in - # /etc/ssh/... path, not a replacement for it, which would not match - # the flat HostKey paths below in sshd_config). + # First boot only, per type: `ssh-keygen -A -f` treats its argument as a + # PREFIX, so keys would not land flat in $KEYDIR (docs/init-parity.md). for kt in rsa ecdsa ed25519; do key="$KEYDIR/ssh_host_${kt}_key" [ -f "$key" ] || ssh-keygen -q -t "$kt" -N '' -f "$key" done printf "Starting sshd: " - # -o HostKey overrides sshd_config's compiled paths, so the tmpfs - # fallback above is actually honoured (otherwise sshd would read the - # persistent /etc/ssh_keys paths from the config and find nothing). + # -o HostKey overrides sshd_config's paths, so the tmpfs fallback above is + # actually honoured. set -- -o "HostKey=$KEYDIR/ssh_host_rsa_key" \ -o "HostKey=$KEYDIR/ssh_host_ecdsa_key" \ -o "HostKey=$KEYDIR/ssh_host_ed25519_key" - # Migration failed and the user's only key is still at the old path. An - # -o AuthorizedKeysFile REPLACES the config's list, so the two shipped - # paths have to be restated alongside it. Same principle as the host-key - # fallback above: a degraded outcome is fine, no way in is not. + # Migration failed, so keep reading the old path this boot. -o REPLACES the + # config's list, hence both shipped paths are restated. if [ -n "$authkeys_fallback" ]; then set -- "$@" -o "AuthorizedKeysFile=.ssh/authorized_keys $AUTHKEYS $AUTHKEYS_LEGACY" fi diff --git a/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config b/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config index 4bcb8f23..b8ca6a82 100644 --- a/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config +++ b/board/mister/de10nano/rootfs-overlay/etc/ssh/sshd_config @@ -15,11 +15,8 @@ #ListenAddress 0.0.0.0 #ListenAddress :: -# P2.3 / ADR 0015: host keys are per-device, generated on first boot by -# /etc/init.d/S50sshd into a persistent ext4 image mounted at /etc/ssh_keys -# (NOT the compiled-in /etc/ssh/ssh_host_* default -- "/" is read-only and -# these must survive reboots/reflashes). The image ships NO ssh_host_* -# private keys anywhere -- see docs/decisions/0015-per-device-ssh-host-keys.md. +# ADR 0015: per-device host keys, generated on first boot by S50sshd into an +# ext4 image mounted here. No ssh_host_* private key ships in this image. HostKey /etc/ssh_keys/ssh_host_rsa_key HostKey /etc/ssh_keys/ssh_host_ecdsa_key HostKey /etc/ssh_keys/ssh_host_ed25519_key @@ -46,57 +43,8 @@ PermitRootLogin yes # The default is to check both .ssh/authorized_keys and .ssh/authorized_keys2 # but this is overridden so installations will only check .ssh/authorized_keys # -# SECOND PATH ADDED: /media/fat/config/authorized_keys -- a user's key that -# SURVIVES AN IMAGE UPDATE. An update replaces linux.img wholesale, so anything -# under /root/.ssh -- which lives inside that file -- is destroyed by it. That -# is the durable reason for this second path, and it holds regardless of how -# the root filesystem happens to be mounted at the time. -# -# WHY /media/fat/config AND NOT /media/fat/linux: this is the location the -# community already standardised on. security_fixes.sh in -# MiSTer-devel/Scripts_MiSTer has read /media/fat/config/authorized_keys since -# v2.1 (2021-12-17), copying it to /root/.ssh/authorized_keys when run. It also -# matches what each directory is FOR: /media/fat/config holds user -# configuration (device.bin, core .cfg files), while /media/fat/linux holds the -# boot payload an update rewrites (linux.img, u-boot.img, the DTB). We shipped -# /media/fat/linux/authorized_keys first (v2026.09.11-beta) and moved here after -# Kreeblah pointed the prior art out on the forum -- issue #183. S50sshd -# migrates the old file on first boot, so the change costs existing users -# nothing; the old path is deliberately NOT kept listed here, because one -# location is the entire point. -# -# Getting a key into the first path is awkward besides: / is mounted READ-ONLY -# at boot (`ro` on the cmdline; inittab's remount-rw line is deliberately left -# commented, ADR 0011) and a fresh image ships NO /root/.ssh at all. It does -# become writable later -- /etc/profile ends with `mount -o remount,rw /` on -# interactive login, which is how / ever becomes writable at all (stock parity, -# docs/init-parity.md) -- so a key CAN be placed there by hand. It just needs a -# login first, which is circular when the key IS the login method, and it does -# not survive the next update either way. The FAT partition is not reflashed, -# so a key here persists across every update. This -# is the same "persist it on /media/fat" principle as ADR 0015's host keys, -# and sshd's native multi-path support means it needs no init script, no -# bind-mount and no user-startup.sh hook. -# -# WHY NOT ssh.ext4 (ADR 0015's mechanism)? Considered and rejected. That is an -# ext4 image INSIDE a file, right for HOST keys because the DEVICE writes them -# -- but an authorized_keys file is written by the USER, and an ext4-in-a-file -# cannot be opened from Windows or macOS with a card reader, while editing it -# on the box needs the very shell access the key is meant to grant. The split -# is who writes the file: machine-written state in ssh.ext4, user-supplied -# state on exFAT. -# -# StrictModes STAYS ON (default yes) and this path satisfies it: the initramfs -# mounts the FAT partition with fmask=0022,dmask=0022 and no uid/gid options -# (board/mister/common/initramfs-overlay/init:27), so the file lands -# root-owned 0755 and its parents 0755 -- owner-writable only, which is what -# sshd requires. Those mount options are OURS and fixed, not user-tunable, so -# this cannot be silently invalidated by a differently-mounted card. Verified -# end-to-end on hardware (exFAT, StrictModes=yes, key auth from this path -# alone). -# -# ORDER MATTERS ONLY FOR PRECEDENCE, NOT FUNCTION: sshd tries every listed -# file, so a key in either location works. +# Second path added: a user's key on the exFAT partition, which an image update +# does not touch. Why there, and the migration: docs/ssh-ftp-parity.md §1.3. AuthorizedKeysFile .ssh/authorized_keys /media/fat/config/authorized_keys #AuthorizedPrincipalsFile none diff --git a/docs/init-parity.md b/docs/init-parity.md index 47d1fc3e..b2533121 100644 --- a/docs/init-parity.md +++ b/docs/init-parity.md @@ -135,6 +135,15 @@ both shapes; folding avoids a second file and keeps the ordering trivial to read uses marker files for exactly this). 7. Verified **zero** `ssh_host_*` files anywhere in the built and extracted image (see the report's Check 3). +8. **Persistence is best-effort; sshd starting is not.** If the `mount` does not take + (no `/media/fat`, corrupt image, no free loop device), `$KEYDIR` is still the + *read-only* rootfs, `ssh-keygen` cannot write there, and sshd would come up with no + host key and refuse every connection — with serial the only way back in. So + `S50sshd` falls back to a tmpfs `$KEYDIR` and says so loudly on the console: keys + are then regenerated each boot, which is a tolerable degradation where "no way in" + is not. The fallback only works because sshd is invoked with `-o HostKey=…` for + each type: an `-o` overrides `sshd_config`'s paths, which otherwise still point at + `/etc/ssh_keys` and would find nothing. On CRNG timing: not re-verified on this build (that requires hardware, P2.9's job); ADR 0015 cites a hardware-measured `crng init done` at ~2.17 s on this same kernel, diff --git a/docs/ssh-ftp-parity.md b/docs/ssh-ftp-parity.md index 47a66f2c..a5654039 100644 --- a/docs/ssh-ftp-parity.md +++ b/docs/ssh-ftp-parity.md @@ -200,6 +200,38 @@ affect what it proved (same partition, same mount options), but a rig re-run aga `config/` is owed and is listed as such in the PR. The shipped config additionally passes `sshd -t` and reports both paths under `sshd -T` on the device. +**Implementation notes.** `S50sshd` is a file that ships to every device, so it carries +two-line comments and points here instead. What its migration is actually defending +against, in the order the code meets it: + +- **Order in `AuthorizedKeysFile` is precedence, not function.** sshd tries every listed + file, so a key in either the `.ssh/` or the FAT path works. +- **The scratch area is a directory under `/run`, created up front.** Creating it is what + proves `/run` is writable *before* anything trusts an empty intermediate file. A grep + that cannot write its output produces nothing, which is indistinguishable from a grep + that found nothing — and "found nothing" is the one branch that deletes the user's + file. For the same reason the greps go through `filter()`, which treats status 1 (no + match) as success and 2+ (read error, failed redirect) as a failed migration. +- **`filter()` needs its explicit `else`.** After a bare `if cmd; then …; fi`, `$?` is the + *if statement's* status — zero when the condition was false — not the command's. Inside + an `else` it is still the command's. Getting this wrong made every "no match" look like + an error. +- **An empty `-f` pattern file behaves oppositely under the two greps.** BusyBox (what + this image ships) matches *every* line; GNU matches none. So a zero-byte + `config/authorized_keys` turned `grep -F -x -v -f` into "select nothing", produced an + empty merge, and satisfied a naive "is anything missing?" check — deleting the key it + was migrating. Both the merge branch and the verification therefore test for *content* + (`[ -s ]`), never for existence. +- **The merge base is grep output, never `cat` of the card file.** Notepad does not write + a final newline, and appending newline-terminated lines to a file that lacks one splices + the first migrated key onto the end of the existing one, destroying both. +- **`sync` goes between the copy and the delete.** exFAT is not mounted `sync` here, so a + power cut after the unlink could otherwise commit the deletion while the new file is + still only in page cache. +- **The failure path restates all three paths.** `-o AuthorizedKeysFile` *replaces* the + config's list rather than adding to it, so the fallback passes `.ssh/authorized_keys` + and both FAT paths. + **CI:** `scripts/ci-tests.sh` asserts, against the **shipped** artifacts rather than the overlay sources, that `sshd_config` lists `/media/fat/config/authorized_keys`, that it no longer lists the pre-#183 `linux/` path (two live locations is the confusion #183 exists