diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index d79d21508..12efaf4d5 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1322,21 +1322,34 @@ static int DoCheckUser(const char* usr, WOLFSSHD_AUTH* auth) { int ret = WOLFSSH_USERAUTH_SUCCESS; int rc; + int isRoot = 0; WOLFSSHD_CONFIG* usrConf; +#ifndef _WIN32 + struct passwd* pwInfo; +#endif wolfSSH_Log(WS_LOG_INFO, "[SSHD] Checking user name %s", usr); +#ifndef _WIN32 + /* PermitRootLogin covers every uid 0 account (so an alias like "toor" + * cannot bypass it) and the literal name "root", so a transient getpwnam + * failure cannot skip the check for root. */ + pwInfo = getpwnam(usr); + if ((pwInfo != NULL && pwInfo->pw_uid == 0) || XSTRCMP(usr, "root") == 0) { + isRoot = 1; + } +#else + /* No uid 0 on Windows and no logon token yet at this pre-auth stage, so + * fall back to the literal name; a token based Administrators membership + * check would belong after authentication. */ if (XSTRCMP(usr, "root") == 0) { - /* Resolve the per-user configuration so that a Match block override of - * PermitRootLogin is honored, rather than only consulting the global - * config node. The lookup is only needed for the root user, so it is - * scoped to this branch. - * - * A NULL return means the root user's configuration could not be - * resolved (e.g. group set enumeration failed inside - * wolfSSHD_AuthGetUserConf). Fail closed and reject the login in that - * case rather than falling back to the global node, since denying root - * on an unresolvable configuration is the safe choice. */ + isRoot = 1; + } +#endif + + if (isRoot == 1) { + /* Resolve per-user config so a Match override is honored; a NULL + * result is unresolvable, so fail closed and reject. */ usrConf = wolfSSHD_AuthGetUserConf(auth, usr, NULL, NULL, NULL, NULL, NULL); if (usrConf == NULL || wolfSSHD_ConfigGetPermitRoot(usrConf) == 0) { diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 51236726c..91cc4c2b1 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -396,10 +396,11 @@ else run_test "sshd_forcedcmd_test.sh" run_test "sshd_window_full_test.sh" run_test "sshd_empty_password_test.sh" + run_test "sshd_permitroot_test.sh" run_strictmodes_negative_test else printf "Skipping tests that need to setup local SSHD\n" - SKIPPED=$((SKIPPED+4)) + SKIPPED=$((SKIPPED+5)) fi # these tests run with X509 sshd-config loaded diff --git a/apps/wolfsshd/test/sshd_permitroot_test.sh b/apps/wolfsshd/test/sshd_permitroot_test.sh new file mode 100755 index 000000000..11ce55bb8 --- /dev/null +++ b/apps/wolfsshd/test/sshd_permitroot_test.sh @@ -0,0 +1,127 @@ +#!/bin/bash + +# Test for PermitRootLogin enforcement. +# +# DoCheckUser rejects a uid 0 login before any credential check, so these checks +# only need to connect and inspect the daemon log; no valid password is needed. +# Two layers are exercised: +# 1. the always-present "root" account (negative + positive control), and +# 2. a duplicate uid 0 alias account, which proves the check is uid based +# rather than name based. The alias layer creates a second uid 0 account so +# it only runs as root on Linux with useradd/userdel and is skipped +# otherwise. + +if [ -z "$1" ] || [ -z "$2" ]; then + echo "expecting host and port as arguments" + echo "$0 127.0.0.1 22222" + exit 1 +fi + +TEST_HOST="$1" +TEST_PORT="$2" +PWD=`pwd` + +source ./start_sshd.sh + +TEST_CLIENT="../../../examples/client/client" +REJECT_MSG="Login as root not permitted" +ALIAS_USER="" +ALIAS_CREATED="" + +cleanup() { + if [ -n "$ALIAS_CREATED" ]; then + sudo userdel "$ALIAS_USER" 2>/dev/null + fi + rm -f sshd_config_test_permitroot_no sshd_config_test_permitroot_yes +} +trap cleanup EXIT +# On a signal, exit so the EXIT trap above removes the uid 0 alias. SIGKILL +# cannot be trapped and is the one case a leftover account is possible. +trap 'exit 1' INT TERM HUP + +write_config() { + # $1 = destination file, $2 = PermitRootLogin value + cat < "$1" +Port $TEST_PORT +Protocol 2 +LoginGraceTime 600 +PermitRootLogin $2 +PasswordAuthentication yes +PermitEmptyPasswords no +UsePrivilegeSeparation no +UseDNS no +HostKey $PWD/../../../keys/server-key.pem +AuthorizedKeysFile $PWD/authorized_keys_test +EOF +} + +# Start the daemon with the given config, attempt a login as the given user with +# a dummy password, then stop the daemon. Leaves the run output in ./log.txt. +attempt_login() { + # $1 = config file, $2 = username + sudo rm -f ./log.txt + start_wolfsshd "$1" + if [ -z "$PID" ]; then + echo "FAIL: could not start wolfsshd" + exit 1 + fi + timeout 10 $TEST_CLIENT -u "$2" -P "wolfsshd-dummy-pw" -c 'true' \ + -h "$TEST_HOST" -p "$TEST_PORT" > /dev/null 2>&1 + # Let wolfsshd flush the log before we stop it. + sleep 1 + stop_wolfsshd +} + +write_config sshd_config_test_permitroot_no no +write_config sshd_config_test_permitroot_yes yes + +# Layer 1a (negative): root must be rejected when PermitRootLogin is no. +attempt_login sshd_config_test_permitroot_no root +if ! sudo grep -q "$REJECT_MSG" ./log.txt; then + echo "FAIL: root login was not rejected under PermitRootLogin no" + echo "----- log.txt -----" + sudo cat ./log.txt + exit 1 +fi + +# Layer 1b (positive control): with PermitRootLogin yes the gate must not fire, +# so the rejection message must be absent while the request still reached +# DoCheckUser. +attempt_login sshd_config_test_permitroot_yes root +if sudo grep -q "$REJECT_MSG" ./log.txt; then + echo "FAIL: root rejected even though PermitRootLogin is yes" + echo "----- log.txt -----" + sudo cat ./log.txt + exit 1 +fi +if ! sudo grep -q "Checking user name root" ./log.txt; then + echo "FAIL: positive control did not reach the user check" + echo "----- log.txt -----" + sudo cat ./log.txt + exit 1 +fi + +# Layer 2 (alias): only when a duplicate uid 0 account can be created and torn +# down safely. This is the case the fix targets: a non-"root" name that resolves +# to uid 0 must still be subject to PermitRootLogin. +if [ "$(uname -s)" = "Linux" ] && command -v useradd >/dev/null 2>&1 && \ + command -v userdel >/dev/null 2>&1 && sudo -n true 2>/dev/null; then + ALIAS_USER="wolfsshduid0$$" + if sudo useradd -o -u 0 -g 0 -M -s /usr/sbin/nologin "$ALIAS_USER" \ + 2>/dev/null; then + ALIAS_CREATED=1 + attempt_login sshd_config_test_permitroot_no "$ALIAS_USER" + if ! sudo grep -q "$REJECT_MSG" ./log.txt; then + echo "FAIL: uid 0 alias '$ALIAS_USER' bypassed PermitRootLogin no" + echo "----- log.txt -----" + sudo cat ./log.txt + exit 1 + fi + else + echo "SKIP alias layer: could not create uid 0 alias account" + fi +else + echo "SKIP alias layer: needs root on Linux with useradd/userdel" +fi + +exit 0 diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index aa5c410de..82db0addc 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -470,7 +470,10 @@ static int test_ConfigCopy(void) * The auth-boundary enforcement itself (a tightened Match node is honored, and * a NULL per-user config rejects rather than falls through to the global node) * is covered by manual/integration testing of wolfsshd against an sshd_config - * containing a Match block that disables password auth and PermitRootLogin. */ + * containing a Match block that disables password auth and PermitRootLogin. + * On Unix the PermitRootLogin check now resolves the account and gates on + * uid 0 (not the literal name "root"), so a non-"root" uid 0 alias is also + * rejected; that behavior is covered by sshd_permitroot_test.sh. */ static int test_GetUserConfMatchOverride(void) { int ret = WS_SUCCESS;