From 986962bb64078fb0faca9568bae5095edd8aa3d1 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sun, 30 Aug 2026 18:28:43 -0400 Subject: [PATCH] Keep the sshd hardening migration from locking users out Validate authorized_keys line by line with the question sshd actually asks: ssh-keygen -lf on the whole file also fingerprints a private key copied there by mistake, which sshd cannot use, so the migration would have disabled the only working login path. Tighten ~/.ssh and authorized_keys the way omarchy-setup-security-sshd does, and back off from a group-writable home directory: StrictModes makes sshd ignore the key either way, with the same lockout. Complete with a notice instead of failing on conditions the migration cannot repair (a broken or pre-Include sshd_config, an overriding admin rule, a failed reload of a valid config), so those machines keep passwords as they were without blocking every migration queued behind this one. Only missing privileges stay pending, since a terminal rerun fixes that. Co-Authored-By: Claude Fable 5 --- migrations/1788124236.sh | 67 +++++++++++---- test/shell.d/sshd-hardening-migration-test.sh | 81 ++++++++++++++++--- 2 files changed, 121 insertions(+), 27 deletions(-) diff --git a/migrations/1788124236.sh b/migrations/1788124236.sh index 824fc58f..0282b775 100644 --- a/migrations/1788124236.sh +++ b/migrations/1788124236.sh @@ -11,6 +11,15 @@ as_root() { fi } +# Passwords staying enabled is the state the machine has been living with, so a +# condition this migration cannot repair completes with a notice instead of +# failing and holding up every migration queued behind it. Only missing +# privileges stay pending below, because rerunning from a terminal fixes that. +skip() { + echo "$1 SSH password authentication remains enabled; run omarchy-setup-security-sshd to harden manually." + exit 0 +} + # The fixed setup command writes this file itself. Its presence is also the # machine-wide completion state, so migrations run by another account no-op. if [[ -e $config || -L $config ]]; then @@ -26,10 +35,39 @@ if ! systemctl is-enabled --quiet sshd.service 2>/dev/null && exit 0 fi +# sshd reads authorized_keys one entry per line, while ssh-keygen -lf +# fingerprints whole files in formats sshd does not accept there — a private +# key copied in by mistake passes the file-level check even though sshd finds +# no usable entry in it. Ask sshd's question instead: does any single line +# parse as a public key? +has_usable_key() { + local line + while IFS= read -r line || [[ -n $line ]]; do + if [[ $line =~ ^[[:space:]]*(#|$) ]]; then + continue + fi + if ssh-keygen -lf /dev/stdin <<<"$line" >/dev/null 2>&1; then + return 0 + fi + done <"$authorized_keys" + return 1 +} + if [[ ! -f $authorized_keys || -L $authorized_keys || ! -s $authorized_keys || ! -r $authorized_keys ]] || - ! ssh-keygen -lf "$authorized_keys" >/dev/null 2>&1; then - echo "Leaving SSH password authentication unchanged because $authorized_keys has no usable public key." - exit 0 + ! has_usable_key; then + skip "$authorized_keys has no usable public key." +fi + +# Under StrictModes, sshd's default, a group- or world-writable home directory, +# ~/.ssh, or authorized_keys makes sshd ignore the key that just validated, and +# passwords would then be the only way in. Tighten the two paths the setup +# command owns, exactly as it does; the home directory is not ours to change. +home_mode=$(stat -c '%a' "$HOME" 2>/dev/null) || skip "Could not inspect the permissions on $HOME." +if (( 8#$home_mode & 8#022 )); then + skip "$HOME is group- or world-writable, so sshd would ignore the authorized key." +fi +if ! chmod 700 "$HOME/.ssh" || ! chmod 600 "$authorized_keys"; then + skip "Could not tighten the permissions on $authorized_keys." fi echo "Disabling SSH password authentication on the existing key-based SSH setup..." @@ -44,26 +82,27 @@ then exit 1 fi -# Syntax alone is insufficient because sshd uses the first value it reads. If -# another administrator rule wins, remove our ineffective file and keep the -# migration pending rather than claiming the machine is protected. +# The drop-in itself is always valid, so a rejection means the configuration +# was already broken before it arrived — the administrator's to repair. if ! as_root sshd -t; then as_root rm -f -- "$config" || true - echo "sshd rejected the hardening config. Fix the SSH configuration and run omarchy-migrate again." >&2 - exit 1 + skip "sshd rejected its configuration." fi effective_config=$(as_root sshd -T) || { as_root rm -f -- "$config" || true - echo "Could not inspect sshd's effective configuration. Run omarchy-migrate again after fixing SSH." >&2 - exit 1 + skip "Could not inspect sshd's effective configuration." } +# Syntax alone is insufficient because sshd uses the first value it reads. An +# sshd_config predating the packaged sshd_config.d Include never reads the +# drop-in at all, and an earlier administrator rule overrides it. Either way +# the file is ineffective: remove it rather than claiming the machine is +# protected. if ! grep -qixF "passwordauthentication no" <<<"$effective_config" || ! grep -qixF "kbdinteractiveauthentication no" <<<"$effective_config"; then as_root rm -f -- "$config" || true - echo "Another SSH rule keeps password authentication enabled. Fix its ordering and run omarchy-migrate again." >&2 - exit 1 + skip "sshd does not apply the hardening drop-in, so an earlier rule or a config without the sshd_config.d include wins." fi # An enabled but deliberately stopped daemon picks the file up on its next @@ -71,7 +110,7 @@ fi # sessions survive while new ones get the hardened policy. if systemctl is-active --quiet sshd.service 2>/dev/null; then if ! as_root systemctl reload sshd.service; then - echo "The hardening config is valid but sshd could not reload it. Run omarchy-migrate again after fixing the service." >&2 - exit 1 + echo "The hardening config is installed and valid, but sshd did not reload; it takes effect when sshd next restarts." >&2 + exit 0 fi fi diff --git a/test/shell.d/sshd-hardening-migration-test.sh b/test/shell.d/sshd-hardening-migration-test.sh index c5659eb6..d48a374a 100755 --- a/test/shell.d/sshd-hardening-migration-test.sh +++ b/test/shell.d/sshd-hardening-migration-test.sh @@ -38,6 +38,9 @@ STUB cat >"$stub_bin/sudo" <<'STUB' #!/bin/bash printf 'sudo %s\n' "$*" >>"${CALL_LOG:?}" +if [[ ${SUDO_ALLOWED:-1} != 1 ]]; then + exit 1 +fi exec "$@" STUB @@ -53,11 +56,19 @@ run_migration() { local config="$root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf" mkdir -p "$home/.ssh" "${config%/*}" + chmod "${HOME_MODE:-755}" "$home" : >"$test_dir/$scenario.calls" - if [[ ${AUTHORIZED_KEY_STATE:-valid} == "valid" ]]; then - printf '%s\n' "$public_key" >"$home/.ssh/authorized_keys" - elif [[ $AUTHORIZED_KEY_STATE == "invalid" ]]; then - printf 'not a public key\n' >"$home/.ssh/authorized_keys" + case "${AUTHORIZED_KEY_STATE:-valid}" in + valid) printf '%s\n' "$public_key" >"$home/.ssh/authorized_keys" ;; + invalid) printf 'not a public key\n' >"$home/.ssh/authorized_keys" ;; + private) cat "$test_dir/key" >"$home/.ssh/authorized_keys" ;; + esac + if [[ ${LOOSE_SSH_PERMS:-0} == 1 ]]; then + chmod 755 "$home/.ssh" + chmod 644 "$home/.ssh/authorized_keys" + fi + if [[ ${ALREADY_HARDENED:-0} == 1 ]]; then + printf 'PasswordAuthentication no\n' >"$config" fi # Keep the privileged production destination fixed in the shipped migration. @@ -70,6 +81,7 @@ run_migration() { SSHD_PASSWORD_AUTH="${SSHD_PASSWORD_AUTH:-no}" \ SSHD_KBD_AUTH="${SSHD_KBD_AUTH:-no}" \ SSHD_RELOAD_VALID="${SSHD_RELOAD_VALID:-1}" \ + SUDO_ALLOWED="${SUDO_ALLOWED:-1}" \ bash -euo pipefail } @@ -79,6 +91,12 @@ SSHD_ENABLED=0 SSHD_ACTIVE=0 run_migration disabled ! grep -q '^sudo ' "$test_dir/disabled.calls" || fail "disabled SSH does not prompt for privileges" pass "SSH migration no-ops when sshd is not enabled or active" +ALREADY_HARDENED=1 SSHD_ENABLED=1 SSHD_ACTIVE=1 run_migration hardened >/dev/null +[[ ! -s $test_dir/hardened.calls ]] || fail "an already-hardened machine must not touch sshd or prompt" +grep -qxF "PasswordAuthentication no" "$test_dir/hardened/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf" || + fail "the existing hardening config is left alone" +pass "SSH migration no-ops when the hardening config already exists" + AUTHORIZED_KEY_STATE=missing SSHD_ENABLED=1 run_migration no-key >/dev/null [[ ! -e $test_dir/no-key/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || fail "SSH migration must not disable passwords without an authorized key" @@ -89,10 +107,30 @@ AUTHORIZED_KEY_STATE=invalid SSHD_ENABLED=1 run_migration invalid-key >/dev/null fail "SSH migration must not trust a malformed authorized_keys file" pass "SSH migration requires a usable authorized key before disabling passwords" -SSHD_ENABLED=1 SSHD_ACTIVE=1 run_migration active >/dev/null +# ssh-keygen -lf accepts a whole private-key file, so only a per-line check +# catches the classic `cp id_ed25519 authorized_keys` slip that sshd cannot use. +AUTHORIZED_KEY_STATE=private SSHD_ENABLED=1 run_migration private-key >/dev/null +[[ ! -e $test_dir/private-key/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || + fail "SSH migration must not treat a private key as an authorized key" +! grep -q '^sudo ' "$test_dir/private-key.calls" || fail "a private-key authorized_keys does not prompt for privileges" +pass "SSH migration rejects an authorized_keys holding a private key" + +# StrictModes makes sshd ignore authorized_keys under a group-writable home, +# so the key that validated would be unusable and passwords the only way in. +HOME_MODE=775 SSHD_ENABLED=1 run_migration loose-home >/dev/null +[[ ! -e $test_dir/loose-home/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || + fail "SSH migration must not disable passwords when sshd would ignore the key" +! grep -q '^sudo ' "$test_dir/loose-home.calls" || fail "a group-writable home does not prompt for privileges" +pass "SSH migration leaves a group-writable home directory alone" + +LOOSE_SSH_PERMS=1 SSHD_ENABLED=1 SSHD_ACTIVE=1 run_migration active >/dev/null config="$test_dir/active/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf" grep -qxF "PasswordAuthentication no" "$config" || fail "SSH migration disables password authentication" grep -qxF "KbdInteractiveAuthentication no" "$config" || fail "SSH migration disables keyboard-interactive authentication" +[[ $(stat -c '%a' "$test_dir/active/home/.ssh") == "700" ]] || + fail "SSH migration tightens ~/.ssh so StrictModes accepts the key" +[[ $(stat -c '%a' "$test_dir/active/home/.ssh/authorized_keys") == "600" ]] || + fail "SSH migration tightens authorized_keys so StrictModes accepts the key" grep -qxF "sudo sshd -t" "$test_dir/active.calls" || fail "SSH migration validates sshd syntax" grep -qxF "sudo sshd -T" "$test_dir/active.calls" || fail "SSH migration validates effective sshd settings" grep -qxF "sudo systemctl reload sshd.service" "$test_dir/active.calls" || fail "SSH migration reloads an active daemon" @@ -104,18 +142,35 @@ SSHD_ENABLED=1 SSHD_ACTIVE=0 run_migration stopped >/dev/null ! grep -qF 'reload sshd.service' "$test_dir/stopped.calls" || fail "SSH migration must not start or reload a stopped daemon" pass "SSH migration hardens an enabled daemon without starting it" -if SSHD_ENABLED=1 SSHD_ACTIVE=1 SSHD_PASSWORD_AUTH=yes run_migration ineffective >"$test_dir/ineffective.output" 2>&1; then - fail "SSH migration must fail when password authentication remains effective" -fi +# Conditions the migration cannot repair complete with a notice — leaving the +# machine as it was — so they never block the migrations queued behind this one. +SSHD_ENABLED=1 SSHD_ACTIVE=1 SSHD_PASSWORD_AUTH=yes run_migration ineffective >"$test_dir/ineffective.output" 2>&1 || + fail "an ineffective drop-in must complete without blocking later migrations" [[ ! -e $test_dir/ineffective/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || fail "SSH migration removes an ineffective config" ! grep -qF 'reload sshd.service' "$test_dir/ineffective.calls" || fail "SSH migration must not reload ineffective hardening" -pass "SSH migration stays pending when another rule keeps password authentication enabled" +pass "SSH migration backs off when another rule keeps password authentication enabled" -if SSHD_ENABLED=1 SSHD_ACTIVE=1 SSHD_SYNTAX_VALID=0 run_migration invalid-config >"$test_dir/invalid-config.output" 2>&1; then - fail "SSH migration must fail when sshd rejects its config" -fi +SSHD_ENABLED=1 SSHD_ACTIVE=1 SSHD_SYNTAX_VALID=0 run_migration invalid-config >"$test_dir/invalid-config.output" 2>&1 || + fail "a rejected config must complete without blocking later migrations" [[ ! -e $test_dir/invalid-config/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || fail "SSH migration removes a rejected config" ! grep -qF 'reload sshd.service' "$test_dir/invalid-config.calls" || fail "SSH migration must not reload rejected hardening" -pass "SSH migration fails safely when sshd rejects the config" +pass "SSH migration backs off when sshd rejects the config" + +# The installed config is valid, so a failed reload only delays it until the +# next sshd restart; keep it staged rather than failing or removing it. +SSHD_ENABLED=1 SSHD_ACTIVE=1 SSHD_RELOAD_VALID=0 run_migration reload-fail >"$test_dir/reload-fail.output" 2>&1 || + fail "a failed reload must complete without blocking later migrations" +[[ -e $test_dir/reload-fail/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || + fail "a failed reload keeps the valid hardening config staged" +pass "SSH migration keeps the hardening staged when sshd cannot reload" + +# Privileges are the one genuinely retryable failure: stay pending so the +# login notifier prompts for a terminal run. +if SUDO_ALLOWED=0 SSHD_ENABLED=1 SSHD_ACTIVE=1 run_migration no-sudo >"$test_dir/no-sudo.output" 2>&1; then + fail "SSH migration must stay pending when privileges are unavailable" +fi +[[ ! -e $test_dir/no-sudo/root/etc/ssh/sshd_config.d/10-omarchy-hardening.conf ]] || + fail "no hardening config is left behind without privileges" +pass "SSH migration stays pending until privileges are granted"