Skip to content

Commit 25bd7c1

Browse files
wolfsshd: add StrictModes permission/ownership checks for authorized_keys and host key
1 parent 2d0ef5a commit 25bd7c1

8 files changed

Lines changed: 460 additions & 4 deletions

File tree

apps/wolfsshd/auth.c

Lines changed: 164 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969

7070
#ifndef _WIN32
7171
#include <sys/types.h>
72+
#include <sys/stat.h>
7273
#include <pwd.h>
7374
#include <grp.h>
7475
#include <errno.h>
@@ -526,8 +527,153 @@ static int ResolveAuthKeysPath(const char* homeDir, const char* pattern,
526527
return ret;
527528
}
528529

530+
/* OpenSSH "StrictModes" style permission/ownership check. Verifies that 'path'
531+
* is not group or world writable. When 'checkOwner' is set the file (and any
532+
* walked directory) must also be owned by 'uid' or by root. When 'checkChain'
533+
* is set, each parent directory up to and including 'homeDir' is checked the
534+
* same way (a writable ancestor directory would let an attacker substitute the
535+
* file). When 'noReadOthers' is set (used for secret files such as the host
536+
* private key) the file is also rejected if it is group or world readable.
537+
*
538+
* Public keys are not secret, so for authorized_keys we guard against write
539+
* access (key injection) and enforce ownership by the user. For the host
540+
* private key we guard against disclosure; ownership is not enforced there
541+
* because the server may run privileged (e.g. via sudo) against a key owned by
542+
* an unprivileged service account.
543+
*
544+
* Returns WS_SUCCESS when the path is considered safe. On platforms without
545+
* POSIX ownership/mode semantics this is a no-op returning WS_SUCCESS. */
546+
int wolfSSHD_CheckFilePermissions(const char* path, const char* homeDir,
547+
WUID_T uid, int checkOwner, int checkChain, int noReadOthers)
548+
{
549+
#ifndef _WIN32
550+
int ret = WS_SUCCESS;
551+
struct stat s;
552+
char pathBuf[MAX_PATH_SZ];
553+
char* slash;
554+
word32 pathSz;
555+
word32 i;
556+
557+
if (path == NULL) {
558+
ret = WS_BAD_ARGUMENT;
559+
}
560+
561+
/* check the target file itself. stat() (not lstat()) is used so symlinked
562+
* key paths resolve to their target and are validated like OpenSSH, rather
563+
* than being rejected outright. */
564+
if (ret == WS_SUCCESS) {
565+
if (stat(path, &s) != 0) {
566+
wolfSSH_Log(WS_LOG_ERROR,
567+
"[SSHD] Unable to stat %s for permission check", path);
568+
ret = WS_BAD_FILE_E;
569+
}
570+
else if (!S_ISREG(s.st_mode)) {
571+
wolfSSH_Log(WS_LOG_ERROR,
572+
"[SSHD] %s is not a regular file", path);
573+
ret = WS_BAD_FILE_E;
574+
}
575+
}
576+
if (ret == WS_SUCCESS && checkOwner) {
577+
if (s.st_uid != uid && s.st_uid != 0) {
578+
wolfSSH_Log(WS_LOG_ERROR,
579+
"[SSHD] Bad owner on %s, must be owned by the user or root",
580+
path);
581+
ret = WS_BAD_FILE_E;
582+
}
583+
}
584+
if (ret == WS_SUCCESS) {
585+
if ((s.st_mode & (S_IWGRP | S_IWOTH)) != 0) {
586+
wolfSSH_Log(WS_LOG_ERROR,
587+
"[SSHD] %s is group or world writable", path);
588+
ret = WS_BAD_FILE_E;
589+
}
590+
}
591+
if (ret == WS_SUCCESS && noReadOthers) {
592+
if ((s.st_mode & (S_IRGRP | S_IROTH)) != 0) {
593+
wolfSSH_Log(WS_LOG_ERROR,
594+
"[SSHD] %s is group or world readable", path);
595+
ret = WS_BAD_FILE_E;
596+
}
597+
}
598+
599+
/* walk parent directories up to and including homeDir */
600+
if (ret == WS_SUCCESS && checkChain && homeDir != NULL) {
601+
pathSz = (word32)WSTRLEN(path);
602+
if (pathSz >= MAX_PATH_SZ) {
603+
ret = WS_BAD_FILE_E;
604+
}
605+
else {
606+
WMEMCPY(pathBuf, path, pathSz);
607+
pathBuf[pathSz] = '\0';
608+
609+
while (ret == WS_SUCCESS) {
610+
/* trim the last path component to move up one directory */
611+
slash = NULL;
612+
for (i = 0; pathBuf[i] != '\0'; i++) {
613+
if (pathBuf[i] == '/') {
614+
slash = &pathBuf[i];
615+
}
616+
}
617+
if (slash == NULL) {
618+
break; /* no more parent directories */
619+
}
620+
if (slash == pathBuf) {
621+
pathBuf[1] = '\0'; /* parent is root "/" */
622+
}
623+
else {
624+
*slash = '\0';
625+
}
626+
627+
if (stat(pathBuf, &s) != 0) {
628+
wolfSSH_Log(WS_LOG_ERROR,
629+
"[SSHD] Unable to stat directory %s", pathBuf);
630+
ret = WS_BAD_FILE_E;
631+
break;
632+
}
633+
if (!S_ISDIR(s.st_mode)) {
634+
wolfSSH_Log(WS_LOG_ERROR,
635+
"[SSHD] %s is not a directory", pathBuf);
636+
ret = WS_BAD_FILE_E;
637+
break;
638+
}
639+
if (checkOwner && s.st_uid != uid && s.st_uid != 0) {
640+
wolfSSH_Log(WS_LOG_ERROR,
641+
"[SSHD] Bad owner on directory %s", pathBuf);
642+
ret = WS_BAD_FILE_E;
643+
break;
644+
}
645+
if ((s.st_mode & (S_IWGRP | S_IWOTH)) != 0) {
646+
wolfSSH_Log(WS_LOG_ERROR,
647+
"[SSHD] Directory %s is group or world writable",
648+
pathBuf);
649+
ret = WS_BAD_FILE_E;
650+
break;
651+
}
652+
653+
/* stop after checking the home directory or filesystem root */
654+
if (WSTRCMP(pathBuf, homeDir) == 0 ||
655+
WSTRCMP(pathBuf, "/") == 0) {
656+
break;
657+
}
658+
}
659+
}
660+
}
661+
662+
return ret;
663+
#else
664+
WOLFSSH_UNUSED(path);
665+
WOLFSSH_UNUSED(homeDir);
666+
WOLFSSH_UNUSED(uid);
667+
WOLFSSH_UNUSED(checkOwner);
668+
WOLFSSH_UNUSED(checkChain);
669+
WOLFSSH_UNUSED(noReadOthers);
670+
return WS_SUCCESS;
671+
#endif
672+
}
673+
529674
static int SearchForPubKey(const char* path, const char* authKeysFile,
530-
const WS_UserAuthData_PublicKey* pubKeyCtx)
675+
const WS_UserAuthData_PublicKey* pubKeyCtx,
676+
WUID_T uid, int strictModes)
531677
{
532678
int ret = WSSHD_AUTH_SUCCESS;
533679
char authKeysPath[MAX_PATH_SZ];
@@ -546,6 +692,19 @@ static int SearchForPubKey(const char* path, const char* authKeysFile,
546692
ret = rc;
547693
}
548694

695+
/* When StrictModes is enabled, refuse the authorized keys file if it or any
696+
* parent directory up to the home directory is owned by another user or is
697+
* group/world writable, mirroring OpenSSH. */
698+
if (ret == WSSHD_AUTH_SUCCESS && strictModes) {
699+
if (wolfSSHD_CheckFilePermissions(authKeysPath, path, uid, 1, 1, 0)
700+
!= WS_SUCCESS) {
701+
wolfSSH_Log(WS_LOG_ERROR,
702+
"[SSHD] Authorized keys file %s failed StrictModes check",
703+
authKeysPath);
704+
ret = WSSHD_AUTH_FAILURE;
705+
}
706+
}
707+
549708
if (ret == WSSHD_AUTH_SUCCESS) {
550709
if (WFOPEN(NULL, &f, authKeysPath, "rb") != 0) {
551710
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s",
@@ -705,7 +864,8 @@ static int CheckPublicKeyUnix(const char* name,
705864
}
706865

707866
if (ret == WSSHD_AUTH_SUCCESS) {
708-
ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, pubKeyCtx);
867+
ret = SearchForPubKey(pwInfo->pw_dir, authorizedKeysFile, pubKeyCtx,
868+
pwInfo->pw_uid, wolfSSHD_ConfigGetStrictModes(authCtx->conf));
709869
}
710870
}
711871

@@ -1049,7 +1209,8 @@ static int CheckPublicKeyWIN(const char* usr,
10491209
if (ret == WSSHD_AUTH_SUCCESS) {
10501210
r[rSz-1] = L'\0';
10511211

1052-
ret = SearchForPubKey(r, authorizedKeysFile, pubKeyCtx);
1212+
ret = SearchForPubKey(r, authorizedKeysFile, pubKeyCtx, 0,
1213+
wolfSSHD_ConfigGetStrictModes(authCtx->conf));
10531214
if (ret != WSSHD_AUTH_SUCCESS) {
10541215
wolfSSH_Log(WS_LOG_ERROR,
10551216
"[SSHD] Failed to find public key for user %s", usr);

apps/wolfsshd/auth.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ HANDLE wolfSSHD_GetAuthToken(const WOLFSSHD_AUTH* auth);
8080
int wolfSSHD_GetHomeDirectory(WOLFSSHD_AUTH* auth, WOLFSSH* ssh, WCHAR* out, int outSz);
8181
#endif
8282

83+
int wolfSSHD_CheckFilePermissions(const char* path, const char* homeDir,
84+
WUID_T uid, int checkOwner, int checkChain, int noReadOthers);
85+
8386
#ifdef WOLFSSHD_UNIT_TEST
8487
#ifndef _WIN32
8588
extern int (*wsshd_setregid_cb)(WGID_T, WGID_T);

apps/wolfsshd/configuration.c

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ struct WOLFSSHD_CONFIG {
8989
byte permitRootLogin:1;
9090
byte permitEmptyPasswords:1;
9191
byte authKeysFileSet:1; /* if not set then no explicit authorized keys */
92+
byte strictModes:1; /* enforce file permission/ownership checks */
9293
};
9394

9495
int CountWhitespace(const char* in, int inSz, byte inv);
@@ -218,6 +219,7 @@ WOLFSSHD_CONFIG* wolfSSHD_ConfigNew(void* heap)
218219
ret->port = 22;
219220
ret->passwordAuth = 1;
220221
ret->loginTimer = 120;
222+
ret->strictModes = 1; /* on by default, matching OpenSSH */
221223
}
222224
return ret;
223225

@@ -315,6 +317,7 @@ static WOLFSSHD_CONFIG* wolfSSHD_ConfigCopy(WOLFSSHD_CONFIG* conf)
315317
newConf->permitRootLogin = conf->permitRootLogin;
316318
newConf->permitEmptyPasswords = conf->permitEmptyPasswords;
317319
newConf->authKeysFileSet = conf->authKeysFileSet;
320+
newConf->strictModes = conf->strictModes;
318321
}
319322
else {
320323
wolfSSHD_ConfigFree(newConf);
@@ -388,9 +391,10 @@ enum {
388391
OPT_TRUSTED_USER_CA_KEYS = 21,
389392
OPT_PIDFILE = 22,
390393
OPT_BANNER = 23,
394+
OPT_STRICT_MODES = 24,
391395
};
392396
enum {
393-
NUM_OPTIONS = 24
397+
NUM_OPTIONS = 25
394398
};
395399

396400
static const CONFIG_OPTION options[NUM_OPTIONS] = {
@@ -418,6 +422,7 @@ static const CONFIG_OPTION options[NUM_OPTIONS] = {
418422
{OPT_TRUSTED_USER_CA_KEYS, "TrustedUserCAKeys"},
419423
{OPT_PIDFILE, "PidFile"},
420424
{OPT_BANNER, "Banner"},
425+
{OPT_STRICT_MODES, "StrictModes"},
421426
};
422427

423428
/* returns WS_SUCCESS on success */
@@ -553,6 +558,31 @@ static int HandlePwAuth(WOLFSSHD_CONFIG* conf, const char* value)
553558
return ret;
554559
}
555560

561+
/* returns WS_SUCCESS on success */
562+
static int HandleStrictModes(WOLFSSHD_CONFIG* conf, const char* value)
563+
{
564+
int ret = WS_SUCCESS;
565+
566+
if (conf == NULL || value == NULL) {
567+
ret = WS_BAD_ARGUMENT;
568+
}
569+
570+
if (ret == WS_SUCCESS) {
571+
if (WSTRCMP(value, "no") == 0) {
572+
wolfSSH_Log(WS_LOG_INFO, "[SSHD] StrictModes disabled");
573+
conf->strictModes = 0;
574+
}
575+
else if (WSTRCMP(value, "yes") == 0) {
576+
conf->strictModes = 1;
577+
}
578+
else {
579+
ret = WS_BAD_ARGUMENT;
580+
}
581+
}
582+
583+
return ret;
584+
}
585+
556586
#define WOLFSSH_PROTOCOL_VERSION 2
557587

558588
/* returns WS_SUCCESS on success */
@@ -1065,6 +1095,9 @@ static int HandleConfigOption(WOLFSSHD_CONFIG** conf, int opt,
10651095
case OPT_BANNER:
10661096
ret = SetFileString(&(*conf)->banner, value, (*conf)->heap);
10671097
break;
1098+
case OPT_STRICT_MODES:
1099+
ret = HandleStrictModes(*conf, value);
1100+
break;
10681101
default:
10691102
break;
10701103
}
@@ -1278,6 +1311,19 @@ int wolfSSHD_ConfigGetAuthKeysFileSet(const WOLFSSHD_CONFIG* conf)
12781311
return ret;
12791312
}
12801313

1314+
/* returns 1 if StrictModes is enabled and 0 if not. Defaults to enabled (fail
1315+
* safe) when conf is NULL. */
1316+
int wolfSSHD_ConfigGetStrictModes(const WOLFSSHD_CONFIG* conf)
1317+
{
1318+
int ret = 1;
1319+
1320+
if (conf != NULL) {
1321+
ret = conf->strictModes;
1322+
}
1323+
1324+
return ret;
1325+
}
1326+
12811327
int wolfSSHD_ConfigSetAuthKeysFile(WOLFSSHD_CONFIG* conf, const char* file)
12821328
{
12831329
int ret = WS_SUCCESS;

apps/wolfsshd/configuration.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ word16 wolfSSHD_ConfigGetPort(const WOLFSSHD_CONFIG* conf);
4747
char* wolfSSHD_ConfigGetAuthKeysFile(const WOLFSSHD_CONFIG* conf);
4848
int wolfSSHD_ConfigGetAuthKeysFileSet(const WOLFSSHD_CONFIG* conf);
4949
int wolfSSHD_ConfigSetAuthKeysFile(WOLFSSHD_CONFIG* conf, const char* file);
50+
int wolfSSHD_ConfigGetStrictModes(const WOLFSSHD_CONFIG* conf);
5051
byte wolfSSHD_ConfigGetPermitEmptyPw(const WOLFSSHD_CONFIG* conf);
5152
byte wolfSSHD_ConfigGetPermitRoot(const WOLFSSHD_CONFIG* conf);
5253
byte wolfSSHD_ConfigGetPrivilegeSeparation(const WOLFSSHD_CONFIG* conf);

apps/wolfsshd/test/run_all_sshd_tests.sh

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,40 @@ run_test() {
104104
fi
105105
}
106106

107+
# Negative StrictModes check: a group/world readable host private key must make
108+
# wolfSSHd refuse to start when StrictModes is enabled (guards the wiring
109+
# between SetupCTX and wolfSSHD_CheckFilePermissions). Runs without sudo:
110+
# privilege separation is off and a high port is used, so no root is needed.
111+
run_strictmodes_negative_test() {
112+
printf "StrictModes negative host key test ... "
113+
# Use a relative host key path: wolfSSH log lines are capped at 120 chars,
114+
# so a long absolute path would truncate the "failed StrictModes check"
115+
# message this test greps for.
116+
cp ../../../keys/server-key.pem strictmodes_hostkey.pem
117+
chmod 644 strictmodes_hostkey.pem
118+
cat <<EOF > sshd_config_test_strictmodes
119+
Port 22622
120+
StrictModes yes
121+
UsePrivilegeSeparation no
122+
HostKey strictmodes_hostkey.pem
123+
EOF
124+
rm -f strictmodes_log.txt
125+
# -D keeps wolfSSHd in the foreground; a StrictModes failure makes it exit
126+
# rather than serve, so this returns on its own.
127+
../wolfsshd -D -d -f sshd_config_test_strictmodes -E strictmodes_log.txt
128+
TOTAL=$((TOTAL+1))
129+
if grep -q "failed StrictModes check" strictmodes_log.txt; then
130+
printf "PASSED\n"
131+
else
132+
printf "FAILED!\n"
133+
cat strictmodes_log.txt
134+
rm -f strictmodes_hostkey.pem sshd_config_test_strictmodes strictmodes_log.txt
135+
stop_wolfsshd
136+
exit 1
137+
fi
138+
rm -f strictmodes_hostkey.pem sshd_config_test_strictmodes strictmodes_log.txt
139+
}
140+
107141
# Run the tests
108142
if [[ -n "$MATCH" ]]; then
109143
if [[ " ${test_cases[*]} " =~ " $MATCH " ]]; then
@@ -147,6 +181,7 @@ else
147181
run_test "sshd_forcedcmd_test.sh"
148182
run_test "sshd_window_full_test.sh"
149183
run_test "sshd_empty_password_test.sh"
184+
run_strictmodes_negative_test
150185
else
151186
printf "Skipping tests that need to setup local SSHD\n"
152187
SKIPPED=$((SKIPPED+3))

apps/wolfsshd/test/start_sshd.sh

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,15 @@
33
# starts up a sshd session, takes in the sshd_config file as an argument
44
start_wolfsshd() {
55
CURRENT_PIDS=`ps -e | grep wolfsshd | grep -oE "[0-9]+"`
6+
7+
# git cannot persist a 0600 file mode, so tighten any configured host key
8+
# before launching. wolfSSHd refuses a group/world readable private key.
9+
while read -r HOSTKEY; do
10+
if [ -n "$HOSTKEY" ] && [ -f "$HOSTKEY" ]; then
11+
chmod 600 "$HOSTKEY" || { echo "chmod 600 failed for host key: $HOSTKEY" >&2; return 1; }
12+
fi
13+
done < <(grep -E '^[[:space:]]*HostKey[[:space:]]+' "$1" | awk '{print $2}')
14+
615
# find a port
716
sudo ../wolfsshd -d -E ./log.txt -f $1
817

0 commit comments

Comments
 (0)