diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 1b7a74155..17920faad 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1270,6 +1270,80 @@ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid, #endif } +/* Open the host private key. See auth.h for the contract. */ +int wolfSSHD_OpenHostKeyFile(const char* path, WFILE** out) +{ +#ifndef _WIN32 + struct stat st; + WFILE* f; + int fd; + int flags; + + if (path == NULL || out == NULL) { + return WS_BAD_ARGUMENT; + } + *out = WBADFILE; + + /* O_NONBLOCK so a FIFO cannot stall the open */ + fd = open(path, O_RDONLY | O_NONBLOCK); + if (fd < 0) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", path); + return WS_BAD_FILE_E; + } + + if (fstat(fd, &st) != 0) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to stat %s", path); + close(fd); + return WS_BAD_FILE_E; + } + + /* a FIFO or device would misbehave on read */ + if (!S_ISREG(st.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load (not a regular file): %s", path); + close(fd); + return WS_BAD_FILE_E; + } + + /* sshd polices only a key the daemon owns; loaded before any + * privilege drop, so geteuid() is the invoking user. */ + if (st.st_uid == geteuid() && (st.st_mode & 077) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Permissions %04o are too open: %s", + (unsigned int)(st.st_mode & 07777), path); + close(fd); + return WS_BAD_FILE_E; + } + + /* restore blocking reads */ + flags = fcntl(fd, F_GETFL); + if (flags != -1) { + (void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK); + } + + f = fdopen(fd, "rb"); + if (f == NULL) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Unable to open stream for %s", path); + close(fd); + return WS_BAD_FILE_E; + } + + *out = f; + return WS_SUCCESS; +#else + if (path == NULL || out == NULL) { + return WS_BAD_ARGUMENT; + } + *out = WBADFILE; + if (WFOPEN(NULL, out, path, "rb") != 0) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", path); + return WS_BAD_FILE_E; + } + return WS_SUCCESS; +#endif +} + /* Scan a resolved keys file (authorized_keys or TrustedUserCAKeys) for * (key, keySz). Fails closed with WSSHD_AUTH_FAILURE when no line matches. * strictModes opens through the secure gate; the file must be owned by uid. */ @@ -1780,11 +1854,11 @@ static int CheckPublicKeyUnix(const char* name, } } - /* The signing CA must be listed in TrustedUserCAKeys. A daemon trust - * anchor, so it is always secure-gated, regardless of StrictModes. */ + /* The signing CA must be listed in TrustedUserCAKeys, a file sshd + * applies no permission policy to. */ if (ret == WSSHD_AUTH_SUCCESS) { ret = SearchKeysFile(usrCaKeysFile, pubKeyCtx->caKey, - pubKeyCtx->caKeySz, geteuid(), 1 /* strictModes */); + pubKeyCtx->caKeySz, geteuid(), 0 /* strictModes */); } /* Bind the certificate to the requested user via its principals. */ diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index 637cdda7b..3cece1d8f 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -102,12 +102,22 @@ HANDLE wolfSSHD_GetAuthToken(const WOLFSSHD_AUTH* auth); int wolfSSHD_GetHomeDirectory(WOLFSSHD_AUTH* auth, WOLFSSH* ssh, WCHAR* out, int outSz); #endif -/* Secure open for trusted files, shared by the authorized_keys path (auth.c) - * and the trust-anchor loads in wolfsshd.c (host key, host cert, user CA keys). - * See the definition in auth.c for the meaning of each argument. */ +/* Secure open for the authorized_keys path and the shadow file lookup, both + * in auth.c. See the definition there for the meaning of each argument. */ int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid, int rejectReadable, void* heap, WFILE** out); +/* Open the host private key, returning a stream ready for reading. + * + * Matches sshd: refuse the key only when the daemon owns it and it is group or + * world accessible. Owner, directories and symlinks on the path are left to + * the administrator. A non-regular file is refused too, which sshd does not + * do, so a FIFO cannot stall the daemon at startup. + * + * Returns WS_SUCCESS and sets *out, else logs the reason. On _WIN32 the file + * is opened directly, relying on filesystem ACLs. */ +int wolfSSHD_OpenHostKeyFile(const char* path, WFILE** out); + /* classifies a loaded host private key buffer as OpenSSH or ASN1/DER. * *keyDer is a WMALLOC'd (heap, DYNTYPE_SSHD) buffer to WS_FORCEZERO + * WFREE on a PEM decode, else NULL. */ diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index 7f34ee009..fcf048cf1 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -124,34 +124,29 @@ run_test() { fi } -# Negative trust-anchor check: a group/world readable host private key must make -# wolfSSHd refuse to start. The host private key is a secret loaded through the -# secure gate (getBufferFromFile/wolfSSHD_OpenSecureFile in SetupCTX), which is -# always enforced and independent of StrictModes, so this config sets -# "StrictModes no" to prove the gate still rejects. Runs without sudo: privilege -# separation is off and a high port is used, so no root is needed. -run_strictmodes_negative_test() { - printf "Host key trust-anchor negative test ... " +# A group/world accessible host key owned by the daemon's user must make +# wolfSSHd refuse to start, as sshd does. No sudo, so the key copy is owned by +# the invoking user, which is what arms the check, and a high port needs no root. +run_hostkey_mode_negative_test() { + printf "Host key mode negative test ... " # A local copy of the host key, made group/world readable. cp ../../../keys/server-key.pem strictmodes_hostkey.pem chmod 644 strictmodes_hostkey.pem cat < sshd_config_test_strictmodes Port 22622 -StrictModes no UsePrivilegeSeparation no HostKey strictmodes_hostkey.pem EOF rm -f strictmodes_log.txt - # -D keeps wolfSSHd in the foreground; a StrictModes failure makes it exit - # rather than serve, so this returns on its own. Wrap in 'timeout' when - # available so a regression fails the test instead of hanging the runner. + # -D keeps wolfSSHd in the foreground; the rejection makes it exit, so this + # returns on its own. 'timeout' keeps a regression from hanging the runner. TIMEOUT="" if command -v timeout >/dev/null 2>&1; then TIMEOUT="timeout 30" fi $TIMEOUT ../wolfsshd -D -d -f sshd_config_test_strictmodes -E strictmodes_log.txt TOTAL=$((TOTAL+1)) - if grep -q "group or world readable" strictmodes_log.txt; then + if grep -q "are too open" strictmodes_log.txt; then printf "PASSED\n" else printf "FAILED!\n" @@ -212,18 +207,13 @@ run_strictmodes_authkeys_negative_test() { fi } -# Self-contained check for the ownership and symlink gate in getBufferFromFile(). -# The host key, host certificate, and user CA all load through the same -# getBufferFromFile(..., WOLFSSHD_LOAD_SECRET/WOLFSSHD_LOAD_TRUST) call, which -# delegates to wolfSSHD_OpenSecureFile(), so exercising the gate via the host -# key covers the identical code for the other two trust anchors. Starts a -# private wolfSSHd with substituted host keys and asserts startup is refused for -# a symlink, a group/world-writable file, and (when run as a non-root user with -# sudo) a file owned by another user, and accepted for a proper mode-600 regular -# file. Does not use the shared daemon, so it runs the same whether or not one -# was started. +# Host key checks, matching sshd: refuse a group/world accessible key the daemon +# owns, do not police one owned by someone else, and inspect neither the +# directories leading to the key nor a symlinked path. A non-regular file is +# refused so a FIFO cannot stall startup. Uses a private wolfSSHd with +# substituted host keys, not the shared daemon. run_hostkey_perm_check() { - printf "host key ownership/symlink gate ... " + printf "host key permission check ... " TOTAL=$((TOTAL+1)) HK_SSHD=../wolfsshd @@ -263,9 +253,9 @@ AuthorizedKeysFile $HK_WORK/authorized_keys EOF } - # Load happens during startup before the listener; start, poll the log - # rather than sleeping a fixed time, then stop. Both the host key load and - # the listener emit a line, so stop as soon as either appears (max ~15s). + # The key loads before the listener; poll the log rather than sleeping a + # fixed time. Both a rejection and the listener emit a line, so stop as soon + # as either appears (max ~15s). # $1 (optional): "sudo" to launch the daemon as root for the owner branch. hk_run() { HK_PRE="$1" @@ -273,7 +263,7 @@ EOF HK_PID=$! i=0 while [ $i -lt 15 ]; do - if grep -qE "Listening on port|Refusing to load" "$HK_WORK/log.txt" 2>/dev/null; then + if grep -qE "Listening on port|Refusing to load|are too open" "$HK_WORK/log.txt" 2>/dev/null; then break fi sleep 1 @@ -301,12 +291,10 @@ EOF exit 1 } - # proper mode-600 regular file must load. The only gate failure here is the - # daemon refusing a properly-owned key; any other reason the daemon does not - # reach the listener (port in use, environment cannot run the daemon) is - # unrelated to the gate, so skip rather than fail the whole suite. + # proper mode-600 regular file must load. Anything else that keeps the daemon + # from the listener (port in use, cannot run here) is unrelated, so skip. hk_cfg "$HK_WORK/hostkey.pem"; hk_run - if grep -q "Refusing to load" "$HK_WORK/log.txt"; then + if grep -qE "Refusing to load|are too open" "$HK_WORK/log.txt"; then hk_fail "valid host key was refused" fi if ! grep -q "Listening on port" "$HK_WORK/log.txt"; then @@ -316,29 +304,36 @@ EOF return fi - # symlink must be refused + # a symlink to a good key must load; sshd does not inspect the path ln -s "$HK_WORK/hostkey.pem" "$HK_WORK/link.pem" hk_cfg "$HK_WORK/link.pem"; hk_run - grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "symlinked host key was not refused" + grep -q "Listening on port" "$HK_WORK/log.txt" || hk_fail "symlinked host key was refused" - # non-regular file (FIFO) must be refused. Skip where mkfifo is unavailable. + # a key in a world-writable directory must load; sshd walks no parent chain + mkdir -p "$HK_WORK/wwdir" && chmod 777 "$HK_WORK/wwdir" + cp "$HK_WORK/hostkey.pem" "$HK_WORK/wwdir/hostkey.pem" + chmod 600 "$HK_WORK/wwdir/hostkey.pem" + hk_cfg "$HK_WORK/wwdir/hostkey.pem"; hk_run + grep -q "Listening on port" "$HK_WORK/log.txt" || hk_fail "host key under a world-writable dir was refused" + + # non-regular file (FIFO) must be refused rather than stall the daemon. + # Skip where mkfifo is unavailable. if mkfifo "$HK_WORK/fifo.pem" 2>/dev/null; then hk_cfg "$HK_WORK/fifo.pem"; hk_run grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "FIFO host key was not refused" fi - # group/world-writable file must be refused + # group/world-writable file owned by the daemon must be refused cp "$HK_KEY" "$HK_WORK/ww.pem"; chmod 666 "$HK_WORK/ww.pem" hk_cfg "$HK_WORK/ww.pem"; hk_run - grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "world-writable host key was not refused" + grep -q "are too open" "$HK_WORK/log.txt" || hk_fail "world-writable host key was not refused" - # Owner-rejection branch (st_uid != 0 && st_uid != geteuid()): the primary - # substitution vector. The mode-600 host key is owned by the invoking user, - # so launching the daemon as root (euid 0) must refuse it. Needs a non-root - # invoker and non-interactive sudo; skip the sub-case otherwise. + # sshd does not police a key owned by another user: the mode-600 key belongs + # to the invoking user, so a root daemon (euid 0) must still load it. Needs a + # non-root invoker and non-interactive sudo. if [ "`id -u`" -ne 0 ] && sudo -n true 2>/dev/null; then hk_cfg "$HK_WORK/hostkey.pem"; hk_run sudo - grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "non-root-owned host key was not refused under root daemon" + grep -q "Listening on port" "$HK_WORK/log.txt" || hk_fail "non-root-owned host key was refused under root daemon" fi rm -rf "$HK_WORK" @@ -396,7 +391,7 @@ else run_test "sshd_permitroot_test.sh" run_test "sshd_permitroot_prohibit_password.sh" run_test "sshd_permitroot_forced_cmd.sh" - run_strictmodes_negative_test + run_hostkey_mode_negative_test run_test "sshd_login_grace_test.sh" run_test "sshd_privdrop_fail_test.sh" else diff --git a/apps/wolfsshd/test/sshd_ossh_cert_test.sh b/apps/wolfsshd/test/sshd_ossh_cert_test.sh index f78a29604..4feedcbb9 100755 --- a/apps/wolfsshd/test/sshd_ossh_cert_test.sh +++ b/apps/wolfsshd/test/sshd_ossh_cert_test.sh @@ -58,8 +58,8 @@ mkdir -p "$MARKERDIR" chown "$LOGINUSER" "$MARKERDIR" 2>/dev/null chmod 700 "$MARKERDIR" -# The host private key is a secret loaded through the secure gate, which refuses -# a group/world readable file. The committed key is 644, so use a 600 copy. +# wolfSSHd refuses a group/world accessible host key it owns, and the committed +# key is 644, so use a 600 copy. HOSTKEY="$WORK/hostkey.pem" cp "$ROOT/keys/server-key.pem" "$HOSTKEY" chmod 600 "$HOSTKEY" diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 1c553c07a..89b1ac699 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -1,85 +1,10 @@ #!/bin/bash -# Holds the per-daemon temp dir used for root-owned trust-anchor copies, so -# stop_wolfsshd can remove it. Empty when no copies were made. -SSHD_KEYDIR="" - # starts up a sshd session, takes in the sshd_config file as an argument start_wolfsshd() { CURRENT_PIDS=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` - ORIGCFG="$1" - CONFIG="$ORIGCFG" - # Reset so each invocation is self-contained regardless of call ordering. - SSHD_KEYDIR="" - - # wolfSSHd loads each trust anchor (host key, host cert, user CA) through the - # secure gate, which refuses a file not owned by the daemon's user or root - # and, for the secret host key, a group/world readable one. This shared - # daemon is launched with sudo (euid 0) while the repository key files are - # owned by the checkout user, so copy each configured trust anchor into a - # private dir, make the copies root-owned and mode 0600, and emit a temp - # config pointing at them. The version-controlled files are left untouched so - # the suite stays re-runnable. - if grep -qE '^[[:space:]]*(HostKey|HostCertificate|TrustedUserCAKeys)[[:space:]]' "$ORIGCFG"; then - SSHD_KEYDIR=$(mktemp -d 2>/dev/null) || SSHD_KEYDIR=$(mktemp -d -t sshdkeys) - if [ -z "$SSHD_KEYDIR" ] || [ ! -d "$SSHD_KEYDIR" ]; then - printf "WARNING: could not create temp dir for trust-anchor copies; using original config\n" >&2 - SSHD_KEYDIR="" - else - CONFIG="$SSHD_KEYDIR/sshd_config" - : > "$CONFIG" || { printf "WARNING: could not write %s; using original config\n" "$CONFIG" >&2; CONFIG="$ORIGCFG"; rm -rf "$SSHD_KEYDIR"; SSHD_KEYDIR=""; } - fi - # Only rewrite when the temp config was set up. On any fallback above - # SSHD_KEYDIR is empty and CONFIG still points at ORIGCFG; running the - # loop then would read from and append to the same file, never reaching - # EOF (runaway append) and would also operate on "/anchorN.pem" at the - # filesystem root. Skipping it leaves the original config untouched. - if [ -n "$SSHD_KEYDIR" ]; then - n=0 - # Rewrite the config line by line. For each trust-anchor directive - # copy the file to a counter-named destination (so distinct - # directories with the same basename do not collide) and emit the - # directive pointing at the copy. Paths are built by string assembly, - # not sed, so a checkout path containing regex or glob metacharacters - # cannot corrupt the rewrite. The directive keyword is the first - # field and the path is the remainder, so a path containing spaces is - # preserved. The "|| [ -n "$line" ]" keeps a final line lacking a - # trailing newline from being dropped. - while IFS= read -r line || [ -n "$line" ]; do - read -r key src <&2 - printf '%s\n' "$line" >> "$CONFIG" - continue - fi - # Owner-only: satisfies the writable check for every - # trust anchor and the no-group/world-readable check for - # the secret host key. The daemon runs as root and reads - # via the owner bits. - chmod 600 "$dst" - if ! sudo chown 0 "$dst"; then - printf "WARNING: could not chown %s to root; daemon may refuse to load it\n" "$src" >&2 - fi - printf '%s %s\n' "$key" "$dst" >> "$CONFIG" - else - printf '%s\n' "$line" >> "$CONFIG" - fi - ;; - *) - printf '%s\n' "$line" >> "$CONFIG" - ;; - esac - done < "$ORIGCFG" - fi - fi + CONFIG="$1" # SSHD_BIN picks the binary; SSHD_ENV passes env (e.g. LD_PRELOAD) that plain # sudo would strip. SSHD_ENV is unquoted to split NAME=VALUE, so no spaces. @@ -103,11 +28,4 @@ stop_wolfsshd() { sudo kill -0 $PID 2>/dev/null || break sleep 0.1 done - - # The temp dir is owned by the invoking user, so its root-owned key copies - # can be removed without sudo. - if [ -n "$SSHD_KEYDIR" ]; then - rm -rf "$SSHD_KEYDIR" - SSHD_KEYDIR="" - fi } diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 5b023955c..ace0c6eac 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -247,15 +247,11 @@ static void freeBufferFromFile(byte* buf, void* heap) } -/* Load class for getBufferFromFile(). NORMAL files (e.g. the banner) are opened - * directly. TRUST and SECRET files are trust anchors loaded through the secure - * gate (no symlink, owned by root or the daemon, no group/world writable path - * component); SECRET additionally rejects a group/world readable file, used for - * the host private key. */ +/* Load class for getBufferFromFile(). NORMAL is a plain open, as sshd polices + * none of the paths root configured. HOSTKEY adds sshd's host key check. */ enum { - WOLFSSHD_LOAD_NORMAL = 0, - WOLFSSHD_LOAD_TRUST = 1, - WOLFSSHD_LOAD_SECRET = 2 + WOLFSSHD_LOAD_NORMAL = 0, + WOLFSSHD_LOAD_HOSTKEY = 1 }; /* set bufSz to size wanted if too small and buf is null */ @@ -271,25 +267,12 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap, if (fileName == NULL) return NULL; - if (loadClass == WOLFSSHD_LOAD_NORMAL) { - if (WFOPEN(NULL, &file, fileName, "rb") != 0) + if (loadClass == WOLFSSHD_LOAD_HOSTKEY) { + if (wolfSSHD_OpenHostKeyFile(fileName, &file) != WS_SUCCESS) return NULL; } - else { - /* Trust anchors always go through the secure gate, regardless of - * StrictModes. The owner is the daemon's effective user (or root), and - * the host private key (SECRET) is also refused if group/world - * readable. */ - if (wolfSSHD_OpenSecureFile(fileName, -#ifndef _WIN32 - geteuid(), -#else - 0, -#endif - loadClass == WOLFSSHD_LOAD_SECRET /* rejectReadable */, - heap, &file) != WS_SUCCESS) { - return NULL; - } + else if (WFOPEN(NULL, &file, fileName, "rb") != 0) { + return NULL; } if (!WFSEEK_SUCCESS(WFSEEK(NULL, file, 0, WSEEK_END))) { @@ -399,16 +382,11 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, byte* data; word32 dataSz = 0; - /* The host private key is a secret trust anchor: refuse a symlink, - * an unsafe owner or path, or a group/world readable/writable - * file. */ data = getBufferFromFile(hostKey, &dataSz, heap, - WOLFSSHD_LOAD_SECRET); + WOLFSSHD_LOAD_HOSTKEY); if (data == NULL) { - /* NULL means the secure gate rejected the file (bad owner, - * symlink, group/world writable/readable; reason already - * logged) or the read failed, so report a file error rather - * than a memory error. */ + /* NULL is a rejected key (logged) or a read failure, not a + * memory error. */ wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error reading host key file."); ret = WS_BAD_FILE_E; @@ -498,9 +476,9 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, #endif data = getBufferFromFile(hostCert, &dataSz, heap, - WOLFSSHD_LOAD_TRUST); + WOLFSSHD_LOAD_NORMAL); if (data == NULL) { - /* secure-gate rejection or read failure, not memory */ + /* read failure, not memory */ wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error reading host certificate file."); ret = WS_BAD_FILE_E; @@ -515,7 +493,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, ret = WS_BAD_FILE_E; } else { - /* Already read through the secure gate, so not _file. */ + /* Already read into a buffer, so not _file. */ ret = wolfSSH_ReadCert_buffer(data, dataSz, &der, &derSz, &type, &typeSz, &flavor, heap); if (ret == WS_SUCCESS @@ -559,9 +537,9 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, wolfSSH_Log(WS_LOG_INFO, "[SSHD] Using CA keys file %s", caCert); data = getBufferFromFile(caCert, &dataSz, heap, - WOLFSSHD_LOAD_TRUST); + WOLFSSHD_LOAD_NORMAL); if (data == NULL) { - /* secure-gate rejection or read failure, not memory */ + /* read failure, not memory */ wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error reading CA cert file."); ret = WS_BAD_FILE_E;