Skip to content

Commit ec05cc8

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
wolfsshd: reject unsafe ownership/symlink on trust-anchor file loads
1 parent ba09b58 commit ec05cc8

3 files changed

Lines changed: 276 additions & 7 deletions

File tree

‎apps/wolfsshd/test/run_all_sshd_tests.sh‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,127 @@ else
8383
fi
8484
fi
8585

86+
# Self-contained check for the ownership and symlink gate in getBufferFromFile().
87+
# The host key, host certificate, and user CA all load through the same
88+
# getBufferFromFile(..., 1) call, so exercising the gate via the host key covers
89+
# the identical code for the other two trust anchors. Starts a private wolfSSHd
90+
# with substituted host keys and asserts startup is refused for a symlink, a
91+
# group/world-writable file, and (when run as a non-root user with sudo) a file
92+
# owned by another user, and accepted for a proper mode-600 regular file. Does
93+
# not use the shared daemon, so it runs the same whether or not one was started.
94+
run_hostkey_perm_check() {
95+
printf "host key ownership/symlink gate ... "
96+
TOTAL=$((TOTAL+1))
97+
98+
HK_SSHD=../wolfsshd
99+
HK_KEY=../../../keys/server-key.pem
100+
HK_PORT=22399
101+
if [ ! -x "$HK_SSHD" ] || [ ! -f "$HK_KEY" ]; then
102+
printf "SKIPPED\n"
103+
SKIPPED=$((SKIPPED+1))
104+
return
105+
fi
106+
107+
HK_WORK=`mktemp -d 2>/dev/null` || HK_WORK=`mktemp -d -t sshdperm`
108+
cp "$HK_KEY" "$HK_WORK/hostkey.pem"
109+
chmod 600 "$HK_WORK/hostkey.pem"
110+
touch "$HK_WORK/authorized_keys"
111+
112+
hk_cfg() {
113+
cat > "$HK_WORK/cfg" <<EOF
114+
Port $HK_PORT
115+
Protocol 2
116+
PermitRootLogin yes
117+
PasswordAuthentication yes
118+
UsePrivilegeSeparation no
119+
UseDNS no
120+
HostKey $1
121+
AuthorizedKeysFile $HK_WORK/authorized_keys
122+
EOF
123+
}
124+
125+
# Load happens during startup before the listener; start, poll the log
126+
# rather than sleeping a fixed time, then stop. Both the host key load and
127+
# the listener emit a line, so stop as soon as either appears (max ~15s).
128+
# $1 (optional): "sudo" to launch the daemon as root for the owner branch.
129+
hk_run() {
130+
HK_PRE="$1"
131+
$HK_PRE "$HK_SSHD" -D -d -f "$HK_WORK/cfg" -p $HK_PORT > "$HK_WORK/log.txt" 2>&1 &
132+
HK_PID=$!
133+
i=0
134+
while [ $i -lt 15 ]; do
135+
if grep -qE "Listening on port|Refusing to load" "$HK_WORK/log.txt" 2>/dev/null; then
136+
break
137+
fi
138+
sleep 1
139+
i=$((i+1))
140+
done
141+
# When launched via sudo, $HK_PID is the sudo pid, not the daemon, and
142+
# sudo does not reliably forward the signal, so match the daemon by port.
143+
# This guarantees a regression cannot leave a root daemon bound to it.
144+
if [ -n "$HK_PRE" ]; then
145+
$HK_PRE pkill -f "$HK_SSHD.*$HK_PORT" 2>/dev/null
146+
else
147+
kill $HK_PID 2>/dev/null
148+
fi
149+
wait $HK_PID 2>/dev/null
150+
}
151+
152+
hk_fail() {
153+
printf "FAILED!\n%s\n" "$1"
154+
cat "$HK_WORK/log.txt"
155+
rm -rf "$HK_WORK"
156+
if [ "$USING_LOCAL_HOST" == 1 ]; then
157+
printf "Shutting down test wolfSSHd\n"
158+
stop_wolfsshd
159+
fi
160+
exit 1
161+
}
162+
163+
# proper mode-600 regular file must load. The only gate failure here is the
164+
# daemon refusing a properly-owned key; any other reason the daemon does not
165+
# reach the listener (port in use, environment cannot run the daemon) is
166+
# unrelated to the gate, so skip rather than fail the whole suite.
167+
hk_cfg "$HK_WORK/hostkey.pem"; hk_run
168+
if grep -q "Refusing to load" "$HK_WORK/log.txt"; then
169+
hk_fail "valid host key was refused"
170+
fi
171+
if ! grep -q "Listening on port" "$HK_WORK/log.txt"; then
172+
printf "SKIPPED (daemon could not listen)\n"
173+
SKIPPED=$((SKIPPED+1))
174+
rm -rf "$HK_WORK"
175+
return
176+
fi
177+
178+
# symlink must be refused
179+
ln -s "$HK_WORK/hostkey.pem" "$HK_WORK/link.pem"
180+
hk_cfg "$HK_WORK/link.pem"; hk_run
181+
grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "symlinked host key was not refused"
182+
183+
# non-regular file (FIFO) must be refused. Skip where mkfifo is unavailable.
184+
if mkfifo "$HK_WORK/fifo.pem" 2>/dev/null; then
185+
hk_cfg "$HK_WORK/fifo.pem"; hk_run
186+
grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "FIFO host key was not refused"
187+
fi
188+
189+
# group/world-writable file must be refused
190+
cp "$HK_KEY" "$HK_WORK/ww.pem"; chmod 666 "$HK_WORK/ww.pem"
191+
hk_cfg "$HK_WORK/ww.pem"; hk_run
192+
grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "world-writable host key was not refused"
193+
194+
# Owner-rejection branch (st_uid != 0 && st_uid != geteuid()): the primary
195+
# substitution vector. The mode-600 host key is owned by the invoking user,
196+
# so launching the daemon as root (euid 0) must refuse it. Needs a non-root
197+
# invoker and non-interactive sudo; skip the sub-case otherwise.
198+
if [ "`id -u`" -ne 0 ] && sudo -n true 2>/dev/null; then
199+
hk_cfg "$HK_WORK/hostkey.pem"; hk_run sudo
200+
grep -q "Refusing to load" "$HK_WORK/log.txt" || hk_fail "non-root-owned host key was not refused under root daemon"
201+
fi
202+
203+
rm -rf "$HK_WORK"
204+
printf "PASSED\n"
205+
}
206+
86207
run_test() {
87208
printf "$1 ... "
88209
./"$1" "$TEST_HOST" "$TEST_PORT" "$USER" &> stdout.txt
@@ -137,6 +258,8 @@ else
137258
# add additional tests here, check on var USING_LOCAL_HOST if can make sshd
138259
# server start/restart with changes
139260

261+
run_hostkey_perm_check
262+
140263
if [ "$USING_LOCAL_HOST" == 1 ]; then
141264
printf "Shutting down test wolfSSHd\n"
142265
stop_wolfsshd

‎apps/wolfsshd/test/start_sshd.sh‎

Lines changed: 66 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,68 @@
11
#!/bin/bash
22

3+
# Holds the per-daemon temp dir used for root-owned trust-anchor copies, so
4+
# stop_wolfsshd can remove it. Empty when no copies were made.
5+
SSHD_KEYDIR=""
6+
37
# starts up a sshd session, takes in the sshd_config file as an argument
48
start_wolfsshd() {
59
CURRENT_PIDS=`ps -e | grep wolfsshd | grep -oE "[0-9]+"`
10+
11+
ORIGCFG="$1"
12+
CONFIG="$ORIGCFG"
13+
# Reset so each invocation is self-contained regardless of call ordering.
14+
SSHD_KEYDIR=""
15+
16+
# wolfSSHd refuses to load a trust-anchor file that is not owned by the
17+
# daemon's user. This shared daemon is launched with sudo (euid 0) while the
18+
# repository key files are owned by the checkout user, so copy each
19+
# configured host key, host cert, and user CA into a private dir, make the
20+
# copies root-owned, and emit a temp config pointing at them. The
21+
# version-controlled files are left untouched so the suite stays re-runnable.
22+
if grep -qE '^(HostKey|HostCertificate|TrustedUserCAKeys)[[:space:]]' "$ORIGCFG"; then
23+
SSHD_KEYDIR=`mktemp -d 2>/dev/null` || SSHD_KEYDIR=`mktemp -d -t sshdkeys`
24+
CONFIG="$SSHD_KEYDIR/sshd_config"
25+
: > "$CONFIG"
26+
n=0
27+
# Rewrite the config line by line. For each trust-anchor directive copy
28+
# the file to a counter-named destination (so distinct directories with
29+
# the same basename do not collide) and emit the directive pointing at
30+
# the copy. Paths are built by string assembly, not sed, so a checkout
31+
# path containing regex or glob metacharacters cannot corrupt the
32+
# rewrite. The directive keyword is the first field and the path is the
33+
# remainder, so a path containing spaces is preserved.
34+
while IFS= read -r line; do
35+
read -r key src <<EOF
36+
$line
37+
EOF
38+
case "$key" in
39+
HostKey|HostCertificate|TrustedUserCAKeys)
40+
if [ -n "$src" ] && [ -e "$src" ]; then
41+
n=`expr $n + 1`
42+
dst="$SSHD_KEYDIR/anchor$n.pem"
43+
if ! cp "$src" "$dst"; then
44+
printf "WARNING: could not copy %s; using original path\n" "$src" >&2
45+
printf '%s\n' "$line" >> "$CONFIG"
46+
continue
47+
fi
48+
chmod go-w "$dst"
49+
if ! sudo chown 0 "$dst"; then
50+
printf "WARNING: could not chown %s to root; daemon may refuse to load it\n" "$src" >&2
51+
fi
52+
printf '%s %s\n' "$key" "$dst" >> "$CONFIG"
53+
else
54+
printf '%s\n' "$line" >> "$CONFIG"
55+
fi
56+
;;
57+
*)
58+
printf '%s\n' "$line" >> "$CONFIG"
59+
;;
60+
esac
61+
done < "$ORIGCFG"
62+
fi
63+
664
# find a port
7-
sudo ../wolfsshd -d -E ./log.txt -f $1
65+
sudo ../wolfsshd -d -E ./log.txt -f "$CONFIG"
866

967
# set the PID of started sshd
1068
NEW_PID=`ps -e | grep wolfsshd | grep -oE "[0-9]+"`
@@ -16,4 +74,11 @@ start_wolfsshd() {
1674
stop_wolfsshd() {
1775
printf "Stopping SSHD, killing pid $PID\n"
1876
sudo kill $PID
77+
78+
# The temp dir is owned by the invoking user, so its root-owned key copies
79+
# can be removed without sudo.
80+
if [ -n "$SSHD_KEYDIR" ]; then
81+
rm -rf "$SSHD_KEYDIR"
82+
SSHD_KEYDIR=""
83+
fi
1984
}

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 87 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,19 @@
4646

4747
#include <signal.h>
4848

49+
#ifndef _WIN32
50+
/* Used by getBufferFromFile() to load security-critical files without
51+
* following an attacker-supplied symlink or trusting an unsafe owner. */
52+
#include <fcntl.h>
53+
#include <sys/stat.h>
54+
#include <unistd.h>
55+
#ifndef O_NOFOLLOW
56+
/* Older platforms lack O_NOFOLLOW; the lstat() pre-check still
57+
* rejects a symlinked leaf there. */
58+
#define O_NOFOLLOW 0
59+
#endif
60+
#endif
61+
4962
#ifdef NO_INLINE
5063
#include <wolfssh/misc.h>
5164
#else
@@ -237,20 +250,88 @@ static void freeBufferFromFile(byte* buf, void* heap)
237250
}
238251

239252

240-
/* set bufSz to size wanted if too small and buf is null */
241-
static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
253+
/* Reads a file into a newly allocated buffer. When secure is non-zero the file
254+
* holds a trust anchor (host key, host cert, or user CA) so the load is refused
255+
* unless it is a regular file, reached without a symlink, owned by root or the
256+
* daemon's effective user, and not group or world writable. This blocks a local
257+
* user from substituting the file via a symlink or a writable parent directory.
258+
* set bufSz to size wanted if too small and buf is null */
259+
static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap,
260+
int secure)
242261
{
243262
FILE* file;
244263
byte* buf = NULL;
245264
long fileSz;
246265
word32 readSz;
266+
#ifndef _WIN32
267+
struct stat lst;
268+
struct stat st;
269+
int fd;
270+
int flags;
271+
#endif
247272

248273
WOLFSSH_UNUSED(heap);
274+
#ifdef _WIN32
275+
WOLFSSH_UNUSED(secure);
276+
#endif
249277

250278
if (fileName == NULL) return NULL;
251279

280+
#ifndef _WIN32
281+
if (secure) {
282+
/* lstat() rejects a symlink or any non-regular leaf such as a FIFO or
283+
* device, O_NOFOLLOW keeps open() from following a symlink, and
284+
* O_NONBLOCK keeps open() from blocking on a FIFO swapped in after the
285+
* lstat(). The owner and mode checks run on the opened fd so there is
286+
* no window to swap the file after the check. */
287+
if (lstat(fileName, &lst) != 0 || !S_ISREG(lst.st_mode)) {
288+
wolfSSH_Log(WS_LOG_ERROR,
289+
"[SSHD] Refusing to load %s: missing or not a regular file",
290+
fileName);
291+
return NULL;
292+
}
293+
fd = open(fileName, O_RDONLY | O_NOFOLLOW | O_NONBLOCK);
294+
if (fd < 0) {
295+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to open %s", fileName);
296+
return NULL;
297+
}
298+
if (fstat(fd, &st) != 0) {
299+
wolfSSH_Log(WS_LOG_ERROR, "[SSHD] Unable to stat %s", fileName);
300+
close(fd);
301+
return NULL;
302+
}
303+
if (!S_ISREG(st.st_mode) ||
304+
(st.st_uid != 0 && st.st_uid != geteuid()) ||
305+
(st.st_mode & (S_IWGRP | S_IWOTH)) != 0) {
306+
wolfSSH_Log(WS_LOG_ERROR,
307+
"[SSHD] Refusing to load %s: must be a regular file owned by "
308+
"root or the daemon and not group or world writable", fileName);
309+
close(fd);
310+
return NULL;
311+
}
312+
/* The target is a regular file, so clear O_NONBLOCK before the
313+
* buffered reads below, which expect ordinary blocking semantics. */
314+
flags = fcntl(fd, F_GETFL);
315+
if (flags != -1)
316+
(void)fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
317+
file = fdopen(fd, "rb");
318+
if (file == NULL) {
319+
close(fd);
320+
return NULL;
321+
}
322+
}
323+
else {
324+
if (WFOPEN(NULL, &file, fileName, "rb") != 0)
325+
return NULL;
326+
}
327+
#else
328+
/* The secure ownership and symlink gate is POSIX only. Windows has no
329+
* comparable uid model and relies on filesystem ACLs to protect the
330+
* trust-anchor files, so the file is opened directly regardless of the
331+
* secure flag. */
252332
if (WFOPEN(NULL, &file, fileName, "rb") != 0)
253333
return NULL;
334+
#endif
254335
WFSEEK(NULL, file, 0, WSEEK_END);
255336
fileSz = WFTELL(NULL, file);
256337
if (fileSz < 0) {
@@ -331,7 +412,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx,
331412
if (ret == WS_SUCCESS) {
332413
#ifndef NO_FILESYSTEM
333414
*banner = getBufferFromFile(wolfSSHD_ConfigGetBanner(conf),
334-
NULL, heap);
415+
NULL, heap, 0);
335416
#endif
336417
if (*banner) {
337418
wolfSSH_CTX_SetBanner(*ctx, (char*)*banner);
@@ -351,7 +432,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx,
351432
byte* data;
352433
word32 dataSz = 0;
353434

354-
data = getBufferFromFile(hostKey, &dataSz, heap);
435+
data = getBufferFromFile(hostKey, &dataSz, heap, 1);
355436
if (data == NULL) {
356437
wolfSSH_Log(WS_LOG_ERROR,
357438
"[SSHD] Error reading host key file.");
@@ -395,7 +476,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx,
395476
byte* data;
396477
word32 dataSz = 0;
397478

398-
data = getBufferFromFile(hostCert, &dataSz, heap);
479+
data = getBufferFromFile(hostCert, &dataSz, heap, 1);
399480
if (data == NULL) {
400481
wolfSSH_Log(WS_LOG_ERROR,
401482
"[SSHD] Error reading host key file.");
@@ -441,7 +522,7 @@ static int SetupCTX(WOLFSSHD_CONFIG* conf, WOLFSSH_CTX** ctx,
441522

442523

443524
wolfSSH_Log(WS_LOG_INFO, "[SSHD] Using CA keys file %s", caCert);
444-
data = getBufferFromFile(caCert, &dataSz, heap);
525+
data = getBufferFromFile(caCert, &dataSz, heap, 1);
445526
if (data == NULL) {
446527
wolfSSH_Log(WS_LOG_ERROR,
447528
"[SSHD] Error reading CA cert file.");

0 commit comments

Comments
 (0)