diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 062455d4d..a11543172 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -69,6 +69,7 @@ #ifndef _WIN32 #include +#include #include #include #include @@ -526,8 +527,176 @@ static int ResolveAuthKeysPath(const char* homeDir, const char* pattern, return ret; } +/* OpenSSH "StrictModes" style permission/ownership check. Verifies that 'path' + * is not group or world writable. When 'checkOwner' is set the file (and any + * walked directory) must also be owned by 'uid' or by root. When 'checkChain' + * is set, each parent directory up to and including 'homeDir' is checked the + * same way (a writable ancestor directory would let an attacker substitute the + * file). When 'noReadOthers' is set (used for secret files such as the host + * private key) the file is also rejected if it is group or world readable. + * + * Public keys are not secret, so for authorized_keys we guard against write + * access (key injection) and enforce ownership by the user. For the host + * private key we guard against disclosure; ownership is not enforced there + * because the server may run privileged (e.g. via sudo) against a key owned by + * an unprivileged service account. + * + * Returns WS_SUCCESS when the path is considered safe. On platforms without + * POSIX ownership/mode semantics this is a no-op returning WS_SUCCESS. */ +int wolfSSHD_CheckFilePermissions(const char* path, const char* homeDir, + WUID_T uid, int checkOwner, int checkChain, int noReadOthers) +{ +#ifndef _WIN32 + int ret = WS_SUCCESS; + int fileInHome; + struct stat s; + char pathBuf[MAX_PATH_SZ]; + char* slash; + word32 pathSz; + word32 homeLen; + word32 i; + + if (path == NULL) { + ret = WS_BAD_ARGUMENT; + } + + /* check the target file itself. stat() (not lstat()) is used so symlinked + * key paths resolve to their target and are validated, rather than being + * rejected outright. */ + if (ret == WS_SUCCESS) { + if (stat(path, &s) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Unable to stat %s for permission check", path); + ret = WS_BAD_FILE_E; + } + else if (!S_ISREG(s.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %s is not a regular file", path); + ret = WS_BAD_FILE_E; + } + } + if (ret == WS_SUCCESS && checkOwner) { + if (s.st_uid != uid && s.st_uid != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Bad owner on %s, must be owned by the user or root", + path); + ret = WS_BAD_FILE_E; + } + } + if (ret == WS_SUCCESS) { + if ((s.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %s is group or world writable", path); + ret = WS_BAD_FILE_E; + } + } + if (ret == WS_SUCCESS && noReadOthers) { + if ((s.st_mode & (S_IRGRP | S_IROTH)) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %s is group or world readable", path); + ret = WS_BAD_FILE_E; + } + } + + /* Walk parent directories checking that none is group or world writable, + * since a writable ancestor would let another user rename it and substitute + * the key file. When the file is inside the user's home directory the walk + * stops at the home directory and also enforces ownership (the user/root + * must own the file and each directory down from home). When the file is a + * custom AuthorizedKeysFile outside the home directory, the walk continues + * to the filesystem root but only checks writability, not ownership, since + * those ancestors are system-managed and not owned by the user. + * + * homeLen is normalized to ignore any trailing slash on homeDir so the + * subtree test does not depend on how pw_dir is formatted. */ + homeLen = (homeDir != NULL) ? (word32)WSTRLEN(homeDir) : 0; + while (homeLen > 0 && homeDir[homeLen - 1] == '/') { + homeLen--; + } + fileInHome = (homeDir != NULL && + WSTRNCMP(path, homeDir, homeLen) == 0 && + (path[homeLen] == '/' || path[homeLen] == '\0')); + if (ret == WS_SUCCESS && checkChain && homeDir != NULL) { + pathSz = (word32)WSTRLEN(path); + if (pathSz >= MAX_PATH_SZ) { + ret = WS_BAD_FILE_E; + } + else { + WMEMCPY(pathBuf, path, pathSz); + pathBuf[pathSz] = '\0'; + + while (ret == WS_SUCCESS) { + /* trim the last path component to move up one directory */ + slash = NULL; + for (i = 0; pathBuf[i] != '\0'; i++) { + if (pathBuf[i] == '/') { + slash = &pathBuf[i]; + } + } + if (slash == NULL) { + break; /* no more parent directories */ + } + if (slash == pathBuf) { + pathBuf[1] = '\0'; /* parent is root "/" */ + } + else { + *slash = '\0'; + } + + if (stat(pathBuf, &s) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Unable to stat directory %s", pathBuf); + ret = WS_BAD_FILE_E; + break; + } + if (!S_ISDIR(s.st_mode)) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] %s is not a directory", pathBuf); + ret = WS_BAD_FILE_E; + break; + } + if (checkOwner && fileInHome && + s.st_uid != uid && s.st_uid != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Bad owner on directory %s", pathBuf); + ret = WS_BAD_FILE_E; + break; + } + if ((s.st_mode & (S_IWGRP | S_IWOTH)) != 0) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Directory %s is group or world writable", + pathBuf); + ret = WS_BAD_FILE_E; + break; + } + + /* stop at the home directory (in-home files) or the + * filesystem root (out-of-home files) */ + if ((fileInHome && + WSTRNCMP(pathBuf, homeDir, homeLen) == 0 && + pathBuf[homeLen] == '\0') || + WSTRCMP(pathBuf, "/") == 0) { + break; + } + } + } + } + + return ret; +#else + WOLFSSH_UNUSED(path); + WOLFSSH_UNUSED(homeDir); + WOLFSSH_UNUSED(uid); + WOLFSSH_UNUSED(checkOwner); + WOLFSSH_UNUSED(checkChain); + WOLFSSH_UNUSED(noReadOthers); + 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,6 +715,20 @@ static int SearchForPubKey(const char* path, const char* authKeysFile, ret = rc; } + /* When StrictModes is enabled, refuse the authorized keys file if it or any + * parent directory up to the home directory is owned by another user or is + * group/world writable. */ + if (ret == WSSHD_AUTH_SUCCESS && strictModes) { + if (wolfSSHD_CheckFilePermissions(authKeysPath, path, uid, + 1 /* checkOwner */, 1 /* checkChain */, 0 /* noReadOthers */) + != WS_SUCCESS) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Authorized keys file %s failed StrictModes check", + authKeysPath); + ret = WSSHD_AUTH_FAILURE; + } + } + if (ret == WSSHD_AUTH_SUCCESS) { if (WFOPEN(NULL, &f, authKeysPath, "rb") != 0) { wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", @@ -705,7 +888,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 +1233,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..b12cae9ee 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 +/* StrictModes permission/ownership check, shared by the authorized_keys path + * (auth.c) and the host private key load (wolfsshd.c). See the definition in + * auth.c for the meaning of each flag. */ +int wolfSSHD_CheckFilePermissions(const char* path, const char* homeDir, + WUID_T uid, int checkOwner, int checkChain, int noReadOthers); + #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 91dd6480c..8d93d35b2 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 */ }; int CountWhitespace(const char* in, int inSz, byte inv); @@ -218,6 +219,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; @@ -315,6 +317,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); @@ -388,9 +391,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] = { @@ -418,6 +422,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 */ @@ -553,6 +558,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 */ @@ -1065,6 +1095,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; } @@ -1278,6 +1311,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 064db6bbb..e2445db68 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 ddb0d20d3..eb6488790 100755 --- a/apps/wolfsshd/test/run_all_sshd_tests.sh +++ b/apps/wolfsshd/test/run_all_sshd_tests.sh @@ -104,6 +104,72 @@ run_test() { fi } +# Negative StrictModes check: a group/world readable host private key must make +# wolfSSHd refuse to start when StrictModes is enabled (guards the wiring +# between SetupCTX and wolfSSHD_CheckFilePermissions). Runs without sudo: +# privilege separation is off and a high port is used, so no root is needed. +run_strictmodes_negative_test() { + printf "StrictModes negative host key test ... " + # Use a relative host key path: wolfSSH log lines are capped at 120 chars, + # so a long absolute path would truncate the "failed StrictModes check" + # 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 yes +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 "failed StrictModes check" 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. +run_strictmodes_authkeys_negative_test() { + printf "StrictModes negative authorized_keys test ... " + chmod 0666 authorized_keys_test + local tmo="" + if command -v timeout >/dev/null 2>&1; then + tmo="timeout 30" + fi + # public-key auth must be refused while authorized_keys is world writable + ( 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 + TOTAL=$((TOTAL+1)) + if [ "$result" != 0 ]; then + printf "PASSED\n" + else + printf "FAILED! (public-key auth succeeded with world-writable authorized_keys)\n" + stop_wolfsshd + exit 1 + fi +} + # Run the tests if [[ -n "$MATCH" ]]; then if [[ " ${test_cases[*]} " =~ " $MATCH " ]]; then @@ -137,6 +203,13 @@ else # add additional tests here, check on var USING_LOCAL_HOST if can make sshd # server start/restart with changes + # 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 @@ -147,9 +220,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 diff --git a/apps/wolfsshd/test/start_sshd.sh b/apps/wolfsshd/test/start_sshd.sh index 2e97649d3..38d7e2992 100755 --- a/apps/wolfsshd/test/start_sshd.sh +++ b/apps/wolfsshd/test/start_sshd.sh @@ -3,6 +3,15 @@ # 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]+"` + + # git cannot persist a 0600 file mode, so tighten any configured host key + # before launching. wolfSSHd refuses a group/world readable private key. + while read -r HOSTKEY; do + if [ -n "$HOSTKEY" ] && [ -f "$HOSTKEY" ]; then + chmod 600 "$HOSTKEY" || { echo "chmod 600 failed for host key: $HOSTKEY" >&2; return 1; } + fi + done < <(grep -E '^[[:space:]]*HostKey[[:space:]]+' "$1" | awk '{print $2}') + # find a port sudo ../wolfsshd -d -E ./log.txt -f $1 diff --git a/apps/wolfsshd/test/test_configuration.c b/apps/wolfsshd/test/test_configuration.c index e1a2b2f6d..c7f9f53e9 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; @@ -855,6 +879,200 @@ static int test_CAKeysFileDiffers(void) return ret; } +#ifndef _WIN32 +/* report a single CheckFilePermissions 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; +} + +static int test_CheckFilePermissions(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] = ""; + 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); + + 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 + write + chain */ + 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", + wolfSSHD_CheckFilePermissions(keys, base, uid, 1, 1, 0), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0660); + if (ret == WS_SUCCESS) { + ret = smExpect("group-writable file rejected", + wolfSSHD_CheckFilePermissions(keys, base, uid, 1, 1, 0), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(keys, 0606); + if (ret == WS_SUCCESS) { + ret = smExpect("world-writable file rejected", + wolfSSHD_CheckFilePermissions(keys, base, uid, 1, 1, 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", + wolfSSHD_CheckFilePermissions(keys, base, uid, 1, 1, 0), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0700); + /* The temp dir lives under the world-writable /tmp. An in-home lookup stops + * at the home directory, so /tmp above it is never checked (see "good perms + * accepted"). Treating the same file as outside the home directory makes the + * walk continue to the filesystem root, where the world-writable /tmp + * ancestor is rejected. Ownership is not enforced on out-of-home ancestors, + * only writability. */ + if (ret == WS_SUCCESS) { + ret = smExpect("out-of-home file walks to root, writable ancestor rejected", + wolfSSHD_CheckFilePermissions(keys, "/nonexistent_home", uid, + 1, 1, 0), 0); + } + /* 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", + wolfSSHD_CheckFilePermissions(keys, base, uid + 1, 1, 1, 0), 0); + } + if (ret == WS_SUCCESS) { + ret = smExpect("wrong owner ignored when checkOwner=0", + wolfSSHD_CheckFilePermissions(keys, base, uid + 1, 0, 1, 0), 1); + } + /* a symlinked authorized_keys pointing at a safe regular file is accepted: + * stat() follows the link and validates the target */ + 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 authorized_keys accepted", + wolfSSHD_CheckFilePermissions(linkKeys, base, uid, 1, 1, 0), 1); + } + /* with checkChain=0 the parent directories are intentionally not inspected, + * so a group-writable .ssh does not cause rejection */ + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0770); + if (ret == WS_SUCCESS) { + ret = smExpect("checkChain=0 skips parent dirs", + wolfSSHD_CheckFilePermissions(keys, base, uid, 1, 0, 0), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(ssh, 0700); + /* a non-regular-file target (here a directory) is rejected */ + if (ret == WS_SUCCESS) { + ret = smExpect("directory target rejected (not a regular file)", + wolfSSHD_CheckFilePermissions(ssh, base, uid, 1, 0, 0), 0); + } + + /* host key style: no owner check, reject group/world readable */ + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0600); + if (ret == WS_SUCCESS) { + ret = smExpect("host key 0600 accepted", + wolfSSHD_CheckFilePermissions(hostkey, NULL, uid, 0, 0, 1), 1); + } + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0640); + if (ret == WS_SUCCESS) { + ret = smExpect("group-readable host key rejected", + wolfSSHD_CheckFilePermissions(hostkey, NULL, uid, 0, 0, 1), 0); + } + if (ret == WS_SUCCESS) + ret = smChmod(hostkey, 0604); + if (ret == WS_SUCCESS) { + ret = smExpect("world-readable host key rejected", + wolfSSHD_CheckFilePermissions(hostkey, NULL, uid, 0, 0, 1), 0); + } + if (ret == WS_SUCCESS) { + ret = smExpect("missing file rejected", + wolfSSHD_CheckFilePermissions("/tmp/wolfsshd_sm_dne_xyz", NULL, + uid, 0, 0, 0), 0); + } + if (ret == WS_SUCCESS) { + ret = smExpect("NULL path rejected", + wolfSSHD_CheckFilePermissions(NULL, NULL, uid, 0, 0, 0), 0); + } + + /* cleanup */ + unlink(linkKeys); + unlink(keys); + unlink(hostkey); + rmdir(ssh); + rmdir(base); + + return ret; +} +#endif /* !_WIN32 */ + const TEST_CASE testCases[] = { TEST_DECL(test_ConfigDefaults), TEST_DECL(test_ParseConfigLine), @@ -862,6 +1080,9 @@ const TEST_CASE testCases[] = { TEST_DECL(test_GetUserConfMatchOverride), TEST_DECL(test_CAKeysFileDiffers), TEST_DECL(test_ConfigFree), +#ifndef _WIN32 + TEST_DECL(test_CheckFilePermissions), +#endif #ifdef WOLFSSL_BASE64_ENCODE TEST_DECL(test_CheckAuthKeysLine), #endif diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 0d1785d08..900cdd2d8 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -347,6 +347,20 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx, wolfSSH_Log(WS_LOG_ERROR, "[SSHD] No host private key set"); ret = WS_BAD_ARGUMENT; } +#ifndef _WIN32 + /* When StrictModes is enabled, refuse the host private key if it is + * group/world readable or writable (it is a secret). Ownership is not + * checked because the server may run privileged against a key owned by + * an unprivileged service account. */ + else if (wolfSSHD_ConfigGetStrictModes(conf) && + wolfSSHD_CheckFilePermissions(hostKey, NULL, getuid(), + 0 /* checkOwner */, 0 /* checkChain */, 1 /* noReadOthers */) + != WS_SUCCESS) { + wolfSSH_Log(WS_LOG_ERROR, + "[SSHD] Host private key %s failed StrictModes check", hostKey); + ret = WS_BAD_FILE_E; + } +#endif else { byte* data; word32 dataSz = 0;