diff --git a/.github/workflows/paramiko-sftp-test.yml b/.github/workflows/paramiko-sftp-test.yml index 50d042480..c7d4041c5 100644 --- a/.github/workflows/paramiko-sftp-test.yml +++ b/.github/workflows/paramiko-sftp-test.yml @@ -94,9 +94,14 @@ jobs: Subsystem sftp internal-sftp EOF - # Set proper permissions for keys + # Set proper permissions for keys. wolfSSHd loads the host key + # through the secure gate, which refuses a key not owned by root or + # the daemon's user, or one that is group/world readable or writable. + # The daemon is launched with sudo (euid 0) while the checkout key is + # owned by the runner, so make it root-owned and 0600. chmod 600 ./keys/server-key.pem - + sudo chown 0:0 ./keys/server-key.pem + # Print debug info echo "Contents of sshd_config.txt:" cat sshd_config.txt diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml index 45ccd992d..a59f6d280 100644 --- a/.github/workflows/sshd-test.yml +++ b/.github/workflows/sshd-test.yml @@ -107,6 +107,10 @@ jobs: sudo apt-get -y install valgrind touch sshd_config.txt ./configure --enable-all LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DWOLFSSH_NO_FPKI -DWOLFSSH_NO_SFTP_TIMEOUT -DWOLFSSH_MAX_SFTP_RW=4000000 -DMAX_PATH_SZ=120" --enable-static --disable-shared && make + # wolfSSHd loads the host key through the secure gate; the daemon runs + # under sudo (euid 0) so make the key root-owned and 0600. + sudo chmod 600 ./keys/server-key.pem + sudo chown 0:0 ./keys/server-key.pem sudo timeout --preserve-status -s 2 5 valgrind --error-exitcode=1 --leak-check=full ./apps/wolfsshd/wolfsshd -D -f sshd_config -h ./keys/server-key.pem -d -p 22222 # regression test, check that cat command does not hang @@ -119,6 +123,10 @@ jobs: cat ./keys/hansel-*.pub > authorized_keys_test sed -i.bak "s/hansel/$USER/" ./authorized_keys_test ./configure --enable-all LDFLAGS="-L${{ github.workspace }}/build-dir/lib" CPPFLAGS="-I${{ github.workspace }}/build-dir/include -DWOLFSSH_NO_FPKI -DWOLFSSH_NO_SFTP_TIMEOUT -DWOLFSSH_MAX_SFTP_RW=4000000 -DMAX_PATH_SZ=120" --enable-static --disable-shared && make + # Host key must be root-owned and 0600 for the sudo-launched daemon's + # secure gate to load it. + sudo chmod 600 ./keys/server-key.pem + sudo chown 0:0 ./keys/server-key.pem sudo ./apps/wolfsshd/wolfsshd -f sshd_config.txt -h ./keys/server-key.pem -p 22225 chmod 600 ./keys/hansel-key-rsa.pem tail -c 50000 /dev/urandom > test diff --git a/.github/workflows/x509-interop.yml b/.github/workflows/x509-interop.yml index a31a32b26..742d31b5e 100644 --- a/.github/workflows/x509-interop.yml +++ b/.github/workflows/x509-interop.yml @@ -169,6 +169,13 @@ jobs: - name: Start wolfSSHd working-directory: ./wolfssh/ run: | + # wolfSSHd loads the host key, host cert, and user CA through the + # secure gate. The daemon runs under sudo (euid 0), so make all three + # root-owned. The secret host key must also be 0600; the public cert + # and CA stay 0644 (not group/world writable, still runner readable). + sudo chmod 600 $PWD/keys/server-key.pem + sudo chown 0:0 $PWD/keys/server-key.pem $PWD/keys/server-cert.pem \ + $PWD/keys/ca-cert-ecc.pem sudo ./apps/wolfsshd/wolfsshd -f sshd_config -d \ -E $PWD/wolfsshd-log.txt & for i in $(seq 1 20); do diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index e5a833a0f..bf1e27181 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -69,9 +69,22 @@ #ifndef _WIN32 #include +#include #include #include #include +#include +#include +#include +#include +#ifndef O_NOFOLLOW + /* Older platforms lack O_NOFOLLOW; the lstat() pre-check and the post-open + * st_dev/st_ino comparison still reject a symlinked leaf there. */ + #define O_NOFOLLOW 0 +#endif +#ifndef PATH_MAX + #define PATH_MAX 4096 +#endif #endif #if !defined(_WIN32) && !(defined(__OSX__) || defined(__APPLE__)) @@ -526,8 +539,225 @@ static int ResolveAuthKeysPath(const char* homeDir, const char* pattern, return ret; } +/* Securely open a trusted file, failing closed on a symlink, bad ownership, or + * unsafe permissions, and hand back an open stream ready for reading. This is + * the single gate for every security-critical file wolfsshd loads: a user's + * authorized_keys, the host private key, the host certificate, and the user + * certificate-authority keys. + * + * path - file to open. + * ownerUid - the file itself must be owned by this user id or by root + * (0). authorized_keys uses the owning user's id; the + * daemon's trust anchors use the effective user id. Parent + * directories are checked for writability but not ownership, + * so a file may legitimately live under a directory owned by + * a third party (e.g. a key under a build checkout or a + * service account's tree). + * rejectReadable - when set, also refuse a file that is group or world + * readable. Used for secrets such as the host private key. + * heap - heap hint for the temporary path buffer. + * out - set to the open stream on success, WBADFILE otherwise. + * + * Returns WS_SUCCESS and sets *out on success; a specific reason is logged on + * failure. On platforms without POSIX ownership semantics (_WIN32) the checks + * are skipped and the file is opened directly, relying on filesystem ACLs. */ +int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid, + int rejectReadable, void* heap, WFILE** out) +{ +#ifndef _WIN32 + int ret = WS_SUCCESS; + int fd = -1; + int flags; + struct stat lst; + struct stat st; + WFILE* f; + char* resolved = NULL; + char* slash; + word32 i; + + if (path == NULL || out == NULL) { + return WS_BAD_ARGUMENT; + } + *out = WBADFILE; + + /* The leaf must be a real, regular file. lstat() (not stat()) is used so a + * symlinked leaf is rejected outright rather than silently followed to an + * attacker-chosen target. */ + if (lstat(path, &lst) != 0 || !S_ISREG(lst.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: missing, not a regular file, or a " + "symlink", path); + ret = WS_BAD_FILE_E; + } + + /* Canonicalize the path with realpath(), resolving any intermediate + * symlinks, then open and validate that canonical path so the file opened + * and the parent chain validated below are one and the same. */ + if (ret == WS_SUCCESS) { + resolved = (char*)WMALLOC(PATH_MAX, heap, DYNTYPE_BUFFER); + if (resolved == NULL) { + ret = WS_MEMORY_E; + } + } + if (ret == WS_SUCCESS) { + if (realpath(path, resolved) == NULL) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to resolve path %s", path); + ret = WS_BAD_FILE_E; + } + } + + /* Open the canonicalized path (not the original) so the directory chain + * validated below is exactly the chain open() traverses. realpath() already + * resolved every intermediate symlink; O_NOFOLLOW guards the + * already-verified non-symlink leaf, and O_NONBLOCK keeps the open from + * stalling on a FIFO swapped in after the lstat() and is cleared before the + * buffered reads. The original path is used only in log messages. */ + if (ret == WS_SUCCESS) { + fd = open(resolved, O_RDONLY | O_NOFOLLOW | O_NONBLOCK); + if (fd < 0) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", path); + ret = WS_BAD_FILE_E; + } + } + if (ret == WS_SUCCESS) { + if (fstat(fd, &st) != 0) { + wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to stat %s", path); + ret = WS_BAD_FILE_E; + } + } + /* The ownership and mode checks run on the opened descriptor so there is no + * window to swap the file after the check. Comparing st_dev/st_ino against + * the earlier lstat() closes the narrow swap window on platforms where + * O_NOFOLLOW is unavailable and compiles to 0. */ + if (ret == WS_SUCCESS) { + if (!S_ISREG(st.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: not a regular file", path); + ret = WS_BAD_FILE_E; + } + else if (st.st_uid != ownerUid && st.st_uid != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: not owned by the user or root", + path); + ret = WS_BAD_FILE_E; + } + else if ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: group or world writable", path); + ret = WS_BAD_FILE_E; + } + else if (rejectReadable && (st.st_mode & (S_IRGRP | S_IROTH)) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: group or world readable", path); + ret = WS_BAD_FILE_E; + } + else if (st.st_dev != lst.st_dev || st.st_ino != lst.st_ino) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Refusing to load %s: file changed during open", path); + ret = WS_BAD_FILE_E; + } + } + + /* Validate every parent directory of the canonicalized path up to the + * filesystem root: none may be group or world writable (unless sticky), + * which is what would let another user rename the file and swap it. Ancestor + * ownership is not enforced; the leaf owner check above is what stops a file + * owned by a third party from being loaded. Since realpath() resolved all + * intermediate symlinks, this is the same chain open() traversed. The walk + * trims components from 'resolved' in place, which is fine now that the file + * is already open. */ + while (ret == WS_SUCCESS) { + /* trim the last component to move up one directory */ + slash = NULL; + for (i = 0; resolved[i] != '\0'; i++) { + if (resolved[i] == '/') { + slash = &resolved[i]; + } + } + if (slash == NULL) { + break; /* no further parent (realpath always returns an absolute + * path, so this is not expected) */ + } + if (slash == resolved) { + resolved[1] = '\0'; /* parent is the root directory "/" */ + } + else { + *slash = '\0'; + } + + if (stat(resolved, &st) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Unable to stat directory %s", resolved); + ret = WS_BAD_FILE_E; + } + else if (!S_ISDIR(st.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %s is not a directory", resolved); + ret = WS_BAD_FILE_E; + } + else if ((st.st_mode & (S_IWGRP | S_IWOTH)) != 0 && + (st.st_mode & S_ISVTX) == 0) { + /* A world/group writable directory is unsafe unless it is sticky: + * the sticky bit stops a non-owner from renaming or deleting files + * they do not own, which is exactly the substitution this guards + * against (e.g. /tmp is mode 1777). */ + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Directory %s is group or world writable", resolved); + ret = WS_BAD_FILE_E; + } + + if (ret != WS_SUCCESS || WSTRCMP(resolved, "/") == 0) { + break; /* reached the filesystem root */ + } + } + + /* The target is a regular file, so restore blocking semantics for the + * buffered reads the caller will perform. */ + if (ret == WS_SUCCESS) { + 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); + ret = WS_BAD_FILE_E; + } + else { + fd = -1; /* ownership of the descriptor moved to the stream */ + *out = f; + } + } + + if (fd >= 0) { + close(fd); + } + if (resolved != NULL) { + WFREE(resolved, heap, DYNTYPE_BUFFER); + } + + return ret; +#else + WOLFSSH_UNUSED(ownerUid); + WOLFSSH_UNUSED(rejectReadable); + WOLFSSH_UNUSED(heap); + + 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 +} + static int SearchForPubKey(const char* path, const char* authKeysFile, - const WS_UserAuthData_PublicKey* pubKeyCtx) + const WS_UserAuthData_PublicKey* pubKeyCtx, + WUID_T uid, int strictModes) { int ret = WSSHD_AUTH_SUCCESS; char authKeysPath[MAX_PATH_SZ]; @@ -546,8 +776,21 @@ static int SearchForPubKey(const char* path, const char* authKeysFile, ret = rc; } + /* When StrictModes is enabled, open through the secure gate: the file must + * be a regular file (no symlink), owned by the user or root, with no + * group/world writable component in its path. When disabled, fall back to a + * plain open. */ if (ret == WSSHD_AUTH_SUCCESS) { - if (WFOPEN(NULL, &f, authKeysPath, "rb") != 0) { + if (strictModes) { + if (wolfSSHD_OpenSecureFile(authKeysPath, uid, + 0 /* rejectReadable */, NULL, &f) != WS_SUCCESS) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Authorized keys file %s failed StrictModes check", + authKeysPath); + ret = WSSHD_AUTH_FAILURE; + } + } + else if (WFOPEN(NULL, &f, authKeysPath, "rb") != 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", authKeysPath); ret = WS_BAD_FILE_E; @@ -593,6 +836,10 @@ static int SearchForPubKey(const char* path, const char* authKeysFile, WFCLOSE(NULL, f); } + if (lineBuf != NULL) { + WFREE(lineBuf, NULL, DYNTYPE_BUFFER); + } + if (ret == WSSHD_AUTH_SUCCESS && !foundKey) { ret = WSSHD_AUTH_FAILURE; } @@ -705,7 +952,8 @@ static int CheckPublicKeyUnix(const char* name, } if (ret == WSSHD_AUTH_SUCCESS) { - ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, pubKeyCtx); + ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, pubKeyCtx, + pwInfo->pw_uid, wolfSSHD_ConfigGetStrictModes(authCtx->conf)); } } @@ -1049,7 +1297,8 @@ static int CheckPublicKeyWIN(const char* usr, if (ret == WSSHD_AUTH_SUCCESS) { r[rSz-1] = L'\0'; - ret = SearchForPubKey(r, authorizedKeysFile, pubKeyCtx); + ret = SearchForPubKey(r, authorizedKeysFile, pubKeyCtx, 0, + wolfSSHD_ConfigGetStrictModes(authCtx->conf)); if (ret != WSSHD_AUTH_SUCCESS) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Failed to find public key for user %s", usr); diff --git a/apps/wolfsshd/auth.h b/apps/wolfsshd/auth.h index 6d3d706c0..9e430ecc0 100644 --- a/apps/wolfsshd/auth.h +++ b/apps/wolfsshd/auth.h @@ -80,6 +80,12 @@ 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. */ +int wolfSSHD_OpenSecureFile(const char* path, WUID_T ownerUid, + int rejectReadable, void* heap, WFILE** out); + #ifdef WOLFSSHD_UNIT_TEST #ifndef _WIN32 extern int (*wsshd_setregid_cb)(WGID_T, WGID_T); diff --git a/apps/wolfsshd/configuration.c b/apps/wolfsshd/configuration.c index 1c9f97fd9..686392a62 100644 --- a/apps/wolfsshd/configuration.c +++ b/apps/wolfsshd/configuration.c @@ -89,6 +89,7 @@ struct WOLFSSHD_CONFIG { byte permitRootLogin:1; byte permitEmptyPasswords:1; byte authKeysFileSet:1; /* if not set then no explicit authorized keys */ + byte strictModes:1; /* enforce file permission/ownership checks */ }; /* Maximum depth of nested Include directives. Bounds the recursion @@ -226,6 +227,7 @@ WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap) ret->port = 22; ret->passwordAuth = 1; ret->loginTimer = 120; + ret->strictModes = 1; /* on by default, matching OpenSSH */ } return ret; @@ -323,6 +325,7 @@ static WOLFSSHD_CONFIG* wolfSSHD_ConfigCopy(WOLFSSHD_CONFIG* conf) newConf->permitRootLogin = conf->permitRootLogin; newConf->permitEmptyPasswords = conf->permitEmptyPasswords; newConf->authKeysFileSet = conf->authKeysFileSet; + newConf->strictModes = conf->strictModes; } else { wolfSSHD_ConfigFree(newConf); @@ -396,9 +399,10 @@ enum { OPT_TRUSTED_USER_CA_KEYS = 21, OPT_PIDFILE = 22, OPT_BANNER = 23, + OPT_STRICT_MODES = 24, }; enum { - NUM_OPTIONS = 24 + NUM_OPTIONS = 25 }; static const CONFIG_OPTION options[NUM_OPTIONS] = { @@ -426,6 +430,7 @@ static const CONFIG_OPTION options[NUM_OPTIONS] = { {OPT_TRUSTED_USER_CA_KEYS, "TrustedUserCAKeys"}, {OPT_PIDFILE, "PidFile"}, {OPT_BANNER, "Banner"}, + {OPT_STRICT_MODES, "StrictModes"}, }; /* returns WS_SUCCESS on success */ @@ -561,6 +566,31 @@ static int HandlePwAuth(WOLFSSHD_CONFIG* conf, const char* value) return ret; } +/* returns WS_SUCCESS on success */ +static int HandleStrictModes(WOLFSSHD_CONFIG* conf, const char* value) +{ + int ret = WS_SUCCESS; + + if (conf == NULL || value == NULL) { + ret = WS_BAD_ARGUMENT; + } + + if (ret == WS_SUCCESS) { + if (WSTRCMP(value, "no") == 0) { + wolfSSH_Log(WS_LOG_INFO, "[SSHD] StrictModes disabled"); + conf->strictModes = 0; + } + else if (WSTRCMP(value, "yes") == 0) { + conf->strictModes = 1; + } + else { + ret = WS_BAD_ARGUMENT; + } + } + + return ret; +} + #define WOLFSSH_PROTOCOL_VERSION 2 /* returns WS_SUCCESS on success */ @@ -1148,6 +1178,9 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt, case OPT_BANNER: ret = SetFileString(&(*conf)->banner, value, (*conf)->heap); break; + case OPT_STRICT_MODES: + ret = HandleStrictModes(*conf, value); + break; default: break; } @@ -1378,6 +1411,19 @@ int wolfSSHD_ConfigGetAuthKeysFileSet(const WOLFSSHD_CONFIG* conf) return ret; } +/* returns 1 if StrictModes is enabled and 0 if not. Defaults to enabled (fail + * safe) when conf is NULL. */ +int wolfSSHD_ConfigGetStrictModes(const WOLFSSHD_CONFIG* conf) +{ + int ret = 1; + + if (conf != NULL) { + ret = conf->strictModes; + } + + return ret; +} + int wolfSSHD_ConfigSetAuthKeysFile(WOLFSSHD_CONFIG* conf, const char* file) { int ret = WS_SUCCESS; diff --git a/apps/wolfsshd/configuration.h b/apps/wolfsshd/configuration.h index d2b491109..c6253ba48 100644 --- a/apps/wolfsshd/configuration.h +++ b/apps/wolfsshd/configuration.h @@ -47,6 +47,7 @@ word16 wolfSSHD_ConfigGetPort(const WOLFSSHD_CONFIG* conf); char* wolfSSHD_ConfigGetAuthKeysFile(const WOLFSSHD_CONFIG* conf); int wolfSSHD_ConfigGetAuthKeysFileSet(const WOLFSSHD_CONFIG* conf); int wolfSSHD_ConfigSetAuthKeysFile(WOLFSSHD_CONFIG* conf, const char* file); +int wolfSSHD_ConfigGetStrictModes(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPermitEmptyPw(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPermitRoot(const WOLFSSHD_CONFIG* conf); byte wolfSSHD_ConfigGetPrivilegeSeparation(const WOLFSSHD_CONFIG* conf); diff --git a/apps/wolfsshd/test/run_all_sshd_tests.sh b/apps/wolfsshd/test/run_all_sshd_tests.sh index de9aba64f..51236726c 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -124,6 +124,229 @@ 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 ... " + # Use a relative host key path: wolfSSH log lines are capped at 120 chars, + # so a long absolute path would truncate the "group or world readable" + # message this test greps for. + 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. + 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 + printf "PASSED\n" + else + printf "FAILED!\n" + cat strictmodes_log.txt + rm -f strictmodes_hostkey.pem sshd_config_test_strictmodes strictmodes_log.txt + stop_wolfsshd + exit 1 + fi + rm -f strictmodes_hostkey.pem sshd_config_test_strictmodes strictmodes_log.txt +} + +# Negative authorized_keys StrictModes test: a group/world writable +# authorized_keys file must make public-key authentication fail (exercises the +# StrictModes branch in SearchForPubKey). Uses the already-running local sshd, +# whose AuthorizedKeysFile is ./authorized_keys_test and whose log is ./log.txt. +run_strictmodes_authkeys_negative_test() { + printf "StrictModes negative authorized_keys test ... " + local tmo="" + if command -v timeout >/dev/null 2>&1; then + tmo="timeout 30" + fi + TOTAL=$((TOTAL+1)) + + # Positive control: with safe 0644 perms the same client must succeed, so a + # non-zero exit below can be attributed to the permission change rather than + # an unrelated client/connection failure. + chmod 0644 authorized_keys_test + ( cd ../../.. && $tmo ./examples/client/client -c 'exit' -u "$USER" \ + -i ./keys/hansel-key-ecc.der -j ./keys/hansel-key-ecc.pub \ + -h "$TEST_HOST" -p "$TEST_PORT" ) > /dev/null 2>&1 + if [ "$?" != 0 ]; then + printf "FAILED! (public-key auth failed with safe 0644 authorized_keys; setup issue)\n" + stop_wolfsshd + exit 1 + fi + + # Negative: a world-writable authorized_keys must make public-key auth fail + # AND make the daemon log the StrictModes rejection, so the failure is for + # the right reason and not an unrelated client error. Count existing + # rejection lines first so a re-run is not confused by stale matches. + local before + before=$(grep -c "failed StrictModes check" log.txt 2>/dev/null || echo 0) + chmod 0666 authorized_keys_test + ( cd ../../.. && $tmo ./examples/client/client -c 'exit' -u "$USER" \ + -i ./keys/hansel-key-ecc.der -j ./keys/hansel-key-ecc.pub \ + -h "$TEST_HOST" -p "$TEST_PORT" ) > /dev/null 2>&1 + local result=$? + chmod 0644 authorized_keys_test + local after + after=$(grep -c "failed StrictModes check" log.txt 2>/dev/null || echo 0) + if [ "$result" != 0 ] && [ "$after" -gt "$before" ]; then + printf "PASSED\n" + else + printf "FAILED! (expected StrictModes rejection: client exit=%s, new log matches=%s)\n" \ + "$result" "$((after - before))" + stop_wolfsshd + exit 1 + 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. +run_hostkey_perm_check() { + printf "host key ownership/symlink gate ... " + TOTAL=$((TOTAL+1)) + + HK_SSHD=../wolfsshd + HK_KEY=../../../keys/server-key.pem + HK_PORT=22399 + if [ ! -x "$HK_SSHD" ] || [ ! -f "$HK_KEY" ]; then + printf "SKIPPED\n" + SKIPPED=$((SKIPPED+1)) + return + fi + + HK_WORK=$(mktemp -d 2>/dev/null) || HK_WORK=$(mktemp -d -t sshdperm) + if [ -z "$HK_WORK" ] || [ ! -d "$HK_WORK" ]; then + printf "SKIPPED (mktemp failed)\n" + SKIPPED=$((SKIPPED+1)) + return + fi + + cp "$HK_KEY" "$HK_WORK/hostkey.pem" || { + printf "SKIPPED (could not prepare hostkey)\n" + SKIPPED=$((SKIPPED+1)) + rm -rf "$HK_WORK" + return + } + chmod 600 "$HK_WORK/hostkey.pem" + touch "$HK_WORK/authorized_keys" + hk_cfg() { + cat > "$HK_WORK/cfg" < "$HK_WORK/log.txt" 2>&1 & + 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 + break + fi + sleep 1 + i=$((i+1)) + done + # When launched via sudo, $HK_PID is the sudo pid, not the daemon, and + # sudo does not reliably forward the signal, so match the daemon by port. + # This guarantees a regression cannot leave a root daemon bound to it. + if [ -n "$HK_PRE" ]; then + $HK_PRE pkill -f "$HK_SSHD.*$HK_PORT" 2>/dev/null + else + kill $HK_PID 2>/dev/null + fi + wait $HK_PID 2>/dev/null + } + + hk_fail() { + printf "FAILED!\n%s\n" "$1" + cat "$HK_WORK/log.txt" + rm -rf "$HK_WORK" + if [ "$USING_LOCAL_HOST" == 1 ]; then + printf "Shutting down test wolfSSHd\n" + stop_wolfsshd + fi + 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. + hk_cfg "$HK_WORK/hostkey.pem"; hk_run + if grep -q "Refusing to load" "$HK_WORK/log.txt"; then + hk_fail "valid host key was refused" + fi + if ! grep -q "Listening on port" "$HK_WORK/log.txt"; then + printf "SKIPPED (daemon could not listen)\n" + SKIPPED=$((SKIPPED+1)) + rm -rf "$HK_WORK" + return + fi + + # symlink must be refused + 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" + + # non-regular file (FIFO) must be refused. 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 + 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" + + # 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. + 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" + fi + + rm -rf "$HK_WORK" + printf "PASSED\n" +} + # Run the tests if [[ -n "$MATCH" ]]; then echo "Running test: $MATCH" @@ -152,6 +375,17 @@ else # add additional tests here, check on var USING_LOCAL_HOST if can make sshd # server start/restart with changes + # trust-anchor ownership/symlink gate (host key, host cert, user CA). Runs a + # private daemon, so it does not depend on the shared local sshd. + run_hostkey_perm_check + + # exercise the authorized_keys StrictModes path against the running sshd + if [ "$USING_LOCAL_HOST" == 1 ]; then + run_strictmodes_authkeys_negative_test + else + SKIPPED=$((SKIPPED+1)) + fi + if [ "$USING_LOCAL_HOST" == 1 ]; then printf "Shutting down test wolfSSHd\n" stop_wolfsshd @@ -162,9 +396,10 @@ else run_test "sshd_forcedcmd_test.sh" run_test "sshd_window_full_test.sh" run_test "sshd_empty_password_test.sh" + run_strictmodes_negative_test else printf "Skipping tests that need to setup local SSHD\n" - SKIPPED=$((SKIPPED+3)) + SKIPPED=$((SKIPPED+4)) fi # these tests run with X509 sshd-config loaded @@ -178,6 +413,15 @@ else fi fi +# Teardown safety net: the start/stop pairs above stop each daemon they start, +# but background test daemons survive across CI steps that share this runner, +# and a later step (the valgrind "memory after close down" check) binds the same +# port 22222. Make sure no test daemon lingers when this script exits so that +# step does not fail with "tcp bind failed". Harmless when nothing is running. +if [ "$USING_LOCAL_HOST" == 1 ]; then + sudo pkill -f "wolfsshd" 2>/dev/null || true +fi + printf "All tests ran, $TOTAL passed, $SKIPPED skipped\n" exit 0 diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 2e97649d3..11ab1db8c 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -1,10 +1,88 @@ #!/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 '^(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 + # find a port - sudo ../wolfsshd -d -E ./log.txt -f $1 + sudo ../wolfsshd -d -E ./log.txt -f "$CONFIG" # set the PID of started sshd NEW_PID=`ps -e | grep wolfsshd | grep -oE "[0-9]+"` @@ -16,4 +94,11 @@ start_wolfsshd() { stop_wolfsshd() { printf "Stopping SSHD, killing pid $PID\n" sudo kill $PID + + # 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/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index 5e01acef9..7c702b8b9 100644 --- a/apps/wolfsshd/test/test_configuration.c +++ b/apps/wolfsshd/test/test_configuration.c @@ -23,6 +23,13 @@ #include #include +#ifndef _WIN32 + #include + #include + #include + #include +#endif + #ifndef WOLFSSH_DEFAULT_LOG_WIDTH #define WOLFSSH_DEFAULT_LOG_WIDTH 120 #endif @@ -194,6 +201,11 @@ static int test_ConfigDefaults(void) if (wolfSSHD_ConfigGetPwAuth(conf) == 0) ret = WS_FATAL_ERROR; } + if (ret == WS_SUCCESS) { + /* StrictModes defaults to on */ + if (wolfSSHD_ConfigGetStrictModes(conf) != 1) + ret = WS_FATAL_ERROR; + } wolfSSHD_ConfigFree(conf); return ret; @@ -242,6 +254,11 @@ static int test_ParseConfigLine(void) {"Password auth yes", "PasswordAuthentication yes", 0}, {"Password auth invalid", "PasswordAuthentication wolfsshd", 1}, + /* StrictModes tests. */ + {"Strict modes no", "StrictModes no", 0}, + {"Strict modes yes", "StrictModes yes", 0}, + {"Strict modes invalid", "StrictModes wolfsshd", 1}, + /* Include files tests. */ {"Include file bad", "Include sshd_config.d/test.bad", 1}, {"Include file exists", "Include sshd_config.d/01-test.conf", 0}, @@ -315,6 +332,8 @@ static int test_ConfigCopy(void) if (ret == WS_SUCCESS) ret = PCL("PermitEmptyPasswords yes"); if (ret == WS_SUCCESS) ret = PCL("PermitRootLogin yes"); if (ret == WS_SUCCESS) ret = PCL("UsePrivilegeSeparation sandbox"); + /* set to non-default (default is on) so a dropped copy is detected */ + if (ret == WS_SUCCESS) ret = PCL("StrictModes no"); /* trigger ConfigCopy via Match; conf advances to the new node */ if (ret == WS_SUCCESS) ret = PCL("Match User testuser"); @@ -401,6 +420,11 @@ static int test_ConfigCopy(void) if (wolfSSHD_ConfigGetPrivilegeSeparation(match) != WOLFSSHD_PRIV_SANDBOX) ret = WS_FATAL_ERROR; } + if (ret == WS_SUCCESS) { + /* source set StrictModes off; the copy must carry it over */ + if (wolfSSHD_ConfigGetStrictModes(match) != 0) + ret = WS_FATAL_ERROR; + } wolfSSHD_ConfigFree(head); return ret; @@ -1005,6 +1029,221 @@ static int test_CAKeysFileDiffers(void) return ret; } +#ifndef _WIN32 +/* report a single secure-open scenario; returns WS_SUCCESS when the observed + * result matches expectation (wantOk != 0 means expect acceptance) */ +static int smExpect(const char* desc, int gotRet, int wantOk) +{ + int ok = wantOk ? (gotRet == WS_SUCCESS) : (gotRet != WS_SUCCESS); + + Log(" Testing scenario: %s. %s\n", desc, ok ? "PASSED" : "FAILED"); + return ok ? WS_SUCCESS : WS_FATAL_ERROR; +} + +/* establish a scenario precondition; returns WS_SUCCESS when the chmod + * succeeded so a failed setup does not masquerade as a passing scenario */ +static int smChmod(const char* path, mode_t mode) +{ + int ret = WS_SUCCESS; + + if (chmod(path, mode) != 0) { + Log(" chmod of %s failed.\n", path); + ret = WS_FATAL_ERROR; + } + return ret; +} + +/* open 'path' through the secure gate and immediately close it, returning the + * gate's verdict so a scenario can assert acceptance or rejection */ +static int smOpen(const char* path, WUID_T ownerUid, int rejectReadable) +{ + WFILE* f = WBADFILE; + int ret = wolfSSHD_OpenSecureFile(path, ownerUid, rejectReadable, NULL, &f); + + if (ret == WS_SUCCESS && f != WBADFILE) { + WFCLOSE(NULL, f); + } + return ret; +} + +static int test_OpenSecureFile(void) +{ + int ret = WS_SUCCESS; + char base[] = "/tmp/wolfsshd_smXXXXXX"; + /* initialized so the unconditional cleanup is harmless if mkdtemp fails */ + char ssh[64] = ""; + char keys[96] = ""; + char hostkey[96] = ""; + char linkKeys[96] = ""; + char wopen[80] = ""; + char wopenKeys[160] = ""; + WUID_T uid = getuid(); + FILE* f = NULL; + + if (mkdtemp(base) == NULL) { + Log(" mkdtemp failed.\n"); + ret = WS_FATAL_ERROR; + } + + if (ret == WS_SUCCESS) { + snprintf(ssh, sizeof(ssh), "%s/.ssh", base); + snprintf(keys, sizeof(keys), "%s/.ssh/authorized_keys", base); + snprintf(hostkey, sizeof(hostkey), "%s/host_key", base); + snprintf(linkKeys, sizeof(linkKeys), "%s/.ssh/link_keys", base); + snprintf(wopen, sizeof(wopen), "%s/wopen", base); + snprintf(wopenKeys, sizeof(wopenKeys), "%s/wopen/keys", base); + + if (mkdir(ssh, 0700) != 0) { + ret = WS_FATAL_ERROR; + } + } + if (ret == WS_SUCCESS) { + f = fopen(keys, "w"); + if (f == NULL) { + ret = WS_FATAL_ERROR; + } + else { + fputs("ssh-rsa AAAA test\n", f); + fclose(f); + } + } + if (ret == WS_SUCCESS) { + f = fopen(hostkey, "w"); + if (f == NULL) { + ret = WS_FATAL_ERROR; + } + else { + fputs("KEYDATA\n", f); + fclose(f); + } + } + + /* authorized_keys style: owner, writable path, and symlink checks. The temp + * tree lives under /tmp (mode 1777); the sticky bit makes that world + * writable ancestor safe, so a good 0600 file is still accepted. */ + if (ret == WS_SUCCESS) + ret = smChmod(base, 0700); + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0700); + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0600); + if (ret == WS_SUCCESS) { + ret = smExpect("good perms accepted", smOpen(keys, uid, 0), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0660); + if (ret == WS_SUCCESS) { + ret = smExpect("group-writable file rejected", smOpen(keys, uid, 0), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0606); + if (ret == WS_SUCCESS) { + ret = smExpect("world-writable file rejected", smOpen(keys, uid, 0), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0600); + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0770); + if (ret == WS_SUCCESS) { + ret = smExpect("group-writable parent dir rejected", + smOpen(keys, uid, 0), 0); + } + /* a sticky world-writable parent (as /tmp itself is) is tolerated, since + * the sticky bit blocks substitution by other users */ + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 01777); + if (ret == WS_SUCCESS) { + ret = smExpect("sticky world-writable parent dir accepted", + smOpen(keys, uid, 0), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0700); + /* The helper accepts root-owned files, so the wrong-owner assertion is only + * meaningful when the test is not running as root. */ + if (ret == WS_SUCCESS && uid != 0) { + ret = smExpect("wrong owner rejected", smOpen(keys, uid + 1, 0), 0); + } + /* a symlinked leaf is rejected outright; lstat() sees the link */ + if (ret == WS_SUCCESS) { + if (symlink(keys, linkKeys) != 0) { + Log(" symlink creation failed.\n"); + ret = WS_FATAL_ERROR; + } + } + if (ret == WS_SUCCESS) { + ret = smExpect("symlinked leaf rejected", smOpen(linkKeys, uid, 0), 0); + } + /* a non-sticky world-writable ancestor directory is rejected */ + if (ret == WS_SUCCESS) { + if (mkdir(wopen, 0700) != 0) { + ret = WS_FATAL_ERROR; + } + } + if (ret == WS_SUCCESS) { + f = fopen(wopenKeys, "w"); + if (f == NULL) { + ret = WS_FATAL_ERROR; + } + else { + fputs("ssh-rsa AAAA test\n", f); + fclose(f); + } + } + if (ret == WS_SUCCESS) + ret = smChmod(wopenKeys, 0600); + if (ret == WS_SUCCESS) + ret = smChmod(wopen, 0777); + if (ret == WS_SUCCESS) { + ret = smExpect("non-sticky world-writable ancestor rejected", + smOpen(wopenKeys, uid, 0), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(wopen, 0700); + /* a non-regular-file target (here a directory) is rejected */ + if (ret == WS_SUCCESS) { + ret = smExpect("directory target rejected (not a regular file)", + smOpen(ssh, uid, 0), 0); + } + + /* host key style: secret, reject group/world readable */ + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0600); + if (ret == WS_SUCCESS) { + ret = smExpect("host key 0600 accepted", smOpen(hostkey, uid, 1), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0640); + if (ret == WS_SUCCESS) { + ret = smExpect("group-readable host key rejected", + smOpen(hostkey, uid, 1), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0604); + if (ret == WS_SUCCESS) { + ret = smExpect("world-readable host key rejected", + smOpen(hostkey, uid, 1), 0); + } + if (ret == WS_SUCCESS) { + ret = smExpect("missing file rejected", + smOpen("/tmp/wolfsshd_sm_dne_xyz", uid, 0), 0); + } + if (ret == WS_SUCCESS) { + ret = smExpect("NULL path rejected", smOpen(NULL, uid, 0), 0); + } + + /* cleanup */ + unlink(linkKeys); + unlink(keys); + unlink(hostkey); + unlink(wopenKeys); + rmdir(wopen); + rmdir(ssh); + rmdir(base); + + return ret; +} +#endif /* !_WIN32 */ + const TEST_CASE testCases[] = { TEST_DECL(test_ConfigDefaults), TEST_DECL(test_ParseConfigLine), @@ -1014,6 +1253,9 @@ const TEST_CASE testCases[] = { TEST_DECL(test_CAKeysFileDiffers), TEST_DECL(test_IncludeRecursionBound), TEST_DECL(test_ConfigFree), +#ifndef _WIN32 + TEST_DECL(test_OpenSecureFile), +#endif #ifdef WOLFSSL_BASE64_ENCODE TEST_DECL(test_CheckAuthKeysLine), #endif diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 4389e2774..b25a3f2fb 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -235,8 +235,20 @@ 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. */ +enum { + WOLFSSHD_LOAD_NORMAL = 0, + WOLFSSHD_LOAD_TRUST = 1, + WOLFSSHD_LOAD_SECRET = 2 +}; + /* set bufSz to size wanted if too small and buf is null */ -static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap) +static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap, + int loadClass) { FILE* file; byte* buf = NULL; @@ -247,8 +259,26 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap) if (fileName == NULL) return NULL; - if (WFOPEN(NULL, &file, fileName, "rb") != 0) - return NULL; + if (loadClass == WOLFSSHD_LOAD_NORMAL) { + if (WFOPEN(NULL, &file, fileName, "rb") != 0) + 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; + } + } WFSEEK(NULL, file, 0, WSEEK_END); fileSz = WFTELL(NULL, file); if (fileSz < 0) { @@ -265,6 +295,10 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap) WFREE(buf, heap, DYNTYPE_SSHD); return NULL; } + /* NUL terminate so callers that treat the buffer as a C string (e.g. + * the banner via wolfSSH_CTX_SetBanner/WSTRLEN) do not read past the + * allocation. The extra byte was already accounted for above. */ + buf[readSz] = '\0'; if (bufSz) *bufSz = readSz; } @@ -329,7 +363,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, if (ret == WS_SUCCESS) { #ifndef NO_FILESYSTEM *banner = getBufferFromFile(wolfSSHD_ConfigGetBanner(conf), - NULL, heap); + NULL, heap, WOLFSSHD_LOAD_NORMAL); #endif if (*banner) { wolfSSH_CTX_SetBanner(*ctx, (char*)*banner); @@ -349,11 +383,19 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, byte* data; word32 dataSz = 0; - data = getBufferFromFile(hostKey, &dataSz, heap); + /* 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); 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. */ wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error reading host key file."); - ret = WS_MEMORY_E; + ret = WS_BAD_FILE_E; } @@ -393,11 +435,13 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, byte* data; word32 dataSz = 0; - data = getBufferFromFile(hostCert, &dataSz, heap); + data = getBufferFromFile(hostCert, &dataSz, heap, + WOLFSSHD_LOAD_TRUST); if (data == NULL) { + /* secure-gate rejection or read failure, not memory */ wolfSSH_Log(WS_LOG_ERROR, - "[SSHD] Error reading host key file."); - ret = WS_MEMORY_E; + "[SSHD] Error reading host certificate file."); + ret = WS_BAD_FILE_E; } @@ -439,11 +483,13 @@ 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); + data = getBufferFromFile(caCert, &dataSz, heap, + WOLFSSHD_LOAD_TRUST); if (data == NULL) { + /* secure-gate rejection or read failure, not memory */ wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Error reading CA cert file."); - ret = WS_MEMORY_E; + ret = WS_BAD_FILE_E; }