Skip to content

Commit db65d64

Browse files
yosuke-wolfsslyosuke-wolfssl
committed
wolfsshd: open PID file with O_NOFOLLOW and a fixed mode
1 parent 422f69d commit db65d64

3 files changed

Lines changed: 326 additions & 22 deletions

File tree

‎apps/wolfsshd/configuration.c‎

Lines changed: 110 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@
2929
#endif
3030

3131
#ifdef WOLFSSH_SSHD
32+
33+
/* Define POSIX feature-test macros before any system header (as auth.c does) so
34+
* fdopen/ftruncate/lstat/S_ISLNK are declared under strict -std= builds. */
35+
#ifdef __linux__
36+
#ifndef _XOPEN_SOURCE
37+
#define _XOPEN_SOURCE
38+
#endif
39+
#ifndef _GNU_SOURCE
40+
#define _GNU_SOURCE
41+
#endif
42+
#endif
43+
3244
/* functions for parsing out options from a config file and for handling loading
3345
* key/certs using the env. filesystem */
3446

@@ -51,10 +63,13 @@
5163

5264
#include "configuration.h"
5365

54-
#ifndef WIN32
66+
#ifndef _WIN32
5567
#include <dirent.h>
68+
#include <fcntl.h>
69+
#include <unistd.h>
70+
#include <sys/stat.h>
5671
#endif
57-
#ifdef WIN32
72+
#ifdef _WIN32
5873
#include <process.h>
5974
#endif
6075
#ifdef HAVE_LIMITS_H
@@ -1779,18 +1794,105 @@ void wolfSSHD_ConfigSavePID(const WOLFSSHD_CONFIG* conf)
17791794
{
17801795
FILE* f;
17811796
char buf[12]; /* large enough to hold 'int' type with null terminator */
1797+
word32 pidLen;
1798+
int writeOk;
1799+
#ifndef _WIN32
1800+
int fd;
1801+
int ok = 1;
1802+
struct stat st;
1803+
#endif
17821804

17831805
if (conf->pidFile != NULL) {
17841806
WMEMSET(buf, 0, sizeof(buf));
1807+
/* Reached only on real POSIX (wolfsshd requires fork/getpwnam), so raw
1808+
* open/fstat/ftruncate/fdopen are safe; the O_NOFOLLOW hardening needs
1809+
* a real fd that WFOPEN does not expose. */
1810+
#ifndef _WIN32
1811+
#if defined(WOLFSSH_HAVE_SYMLINK) && WOLFSSH_O_NOFOLLOW == 0
1812+
/* Symlinks are a concern here but O_NOFOLLOW is unavailable, so reject a
1813+
* symlinked leaf with a best-effort lstat pre-check (racy, the closest
1814+
* guard without the atomic primitive). */
1815+
if (lstat(conf->pidFile, &st) == 0 && S_ISLNK(st.st_mode)) {
1816+
wolfSSH_Log(WS_LOG_ERROR,
1817+
"[SSHD] Refusing symlinked PID file %s", conf->pidFile);
1818+
ok = 0;
1819+
}
1820+
#endif
1821+
/* Open with O_NOFOLLOW and an explicit 0644 mode so a pre-positioned
1822+
* symlink at the PID file path cannot redirect the write to another
1823+
* file, and the result is never world-writable under a relaxed umask. */
1824+
if (ok) {
1825+
/* No O_TRUNC (truncate only after the fstat check below) and
1826+
* O_NONBLOCK so a planted FIFO fails fast instead of stalling the
1827+
* open before that check runs. */
1828+
fd = open(conf->pidFile,
1829+
O_WRONLY | O_CREAT | O_NONBLOCK | WOLFSSH_O_NOFOLLOW,
1830+
0644);
1831+
if (fd < 0) {
1832+
/* covers a refused symlink leaf (O_NOFOLLOW) as well as any
1833+
* other open failure. */
1834+
wolfSSH_Log(WS_LOG_ERROR,
1835+
"[SSHD] Unable to open PID file %s", conf->pidFile);
1836+
}
1837+
/* O_NOFOLLOW blocks a symlink but not a hard link onto a root-owned
1838+
* file; require a single-link regular file before truncating. */
1839+
else if (fstat(fd, &st) != 0 || !S_ISREG(st.st_mode) ||
1840+
st.st_nlink != 1) {
1841+
wolfSSH_Log(WS_LOG_ERROR,
1842+
"[SSHD] Refusing unsafe PID file %s", conf->pidFile);
1843+
close(fd);
1844+
}
1845+
else if (ftruncate(fd, 0) != 0) {
1846+
wolfSSH_Log(WS_LOG_ERROR,
1847+
"[SSHD] Unable to truncate PID file %s", conf->pidFile);
1848+
close(fd);
1849+
}
1850+
else {
1851+
/* O_NONBLOCK left set is a no-op on a regular-file write. */
1852+
f = fdopen(fd, "wb");
1853+
if (f == NULL) {
1854+
wolfSSH_Log(WS_LOG_ERROR,
1855+
"[SSHD] Unable to open PID file stream %s",
1856+
conf->pidFile);
1857+
close(fd);
1858+
/* the file was created or truncated above; remove the empty
1859+
* leftover so no reader parses a zero-length PID. */
1860+
unlink(conf->pidFile);
1861+
}
1862+
else {
1863+
WSNPRINTF(buf, sizeof(buf), "%d", getpid());
1864+
pidLen = (word32)WSTRLEN(buf);
1865+
writeOk = ((word32)WFWRITE(NULL, buf, 1, pidLen, f) == pidLen);
1866+
/* always close, then drop a short-written or unflushed PID
1867+
* file so no reader parses a truncated value. */
1868+
if (WFCLOSE(NULL, f) != 0) {
1869+
writeOk = 0;
1870+
}
1871+
if (!writeOk) {
1872+
wolfSSH_Log(WS_LOG_ERROR,
1873+
"[SSHD] Failed to write PID file %s", conf->pidFile);
1874+
unlink(conf->pidFile);
1875+
}
1876+
}
1877+
}
1878+
}
1879+
#else
17851880
if (WFOPEN(NULL, &f, conf->pidFile, "wb") == 0) {
1786-
#ifndef WIN32
1787-
WSNPRINTF(buf, sizeof(buf), "%d", getpid());
1788-
#else
17891881
WSNPRINTF(buf, sizeof(buf), "%d", _getpid());
1790-
#endif
1791-
WFWRITE(NULL, buf, 1, WSTRLEN(buf), f);
1792-
WFCLOSE(NULL, f);
1882+
pidLen = (word32)WSTRLEN(buf);
1883+
writeOk = ((word32)WFWRITE(NULL, buf, 1, pidLen, f) == pidLen);
1884+
/* always close, then drop a short-written or unflushed PID file so
1885+
* no reader parses a truncated value. */
1886+
if (WFCLOSE(NULL, f) != 0) {
1887+
writeOk = 0;
1888+
}
1889+
if (!writeOk) {
1890+
wolfSSH_Log(WS_LOG_ERROR,
1891+
"[SSHD] Failed to write PID file %s", conf->pidFile);
1892+
WREMOVE(NULL, conf->pidFile);
1893+
}
17931894
}
1895+
#endif
17941896
}
17951897
}
17961898

‎apps/wolfsshd/test/test_configuration.c‎

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,6 +1898,196 @@ static int test_OpenSecureFile(void)
18981898

18991899
return ret;
19001900
}
1901+
1902+
/* load a config whose PidFile is 'pidTarget' and run wolfSSHD_ConfigSavePID
1903+
* under umask(0), restoring the umask afterward so later tests are unaffected */
1904+
static int pidSave(const char* confPath, const char* pidTarget)
1905+
{
1906+
int ret;
1907+
mode_t old;
1908+
WOLFSSHD_CONFIG* cfg;
1909+
FILE* f;
1910+
1911+
f = fopen(confPath, "w");
1912+
if (f == NULL) {
1913+
return WS_FATAL_ERROR;
1914+
}
1915+
fprintf(f, "PidFile %s\n", pidTarget);
1916+
fclose(f);
1917+
1918+
cfg = wolfSSHD_ConfigNew(NULL);
1919+
if (cfg == NULL) {
1920+
return WS_MEMORY_E;
1921+
}
1922+
ret = wolfSSHD_ConfigLoad(cfg, confPath);
1923+
if (ret == WS_SUCCESS) {
1924+
/* deliberate worst case: prove the explicit 0644 mode holds even when
1925+
* the umask would not mask any bits */
1926+
old = umask(0);
1927+
wolfSSHD_ConfigSavePID(cfg);
1928+
umask(old);
1929+
}
1930+
wolfSSHD_ConfigFree(cfg);
1931+
return ret;
1932+
}
1933+
1934+
/* wolfSSHD_ConfigSavePID must refuse a symlink at the PID path so a planted
1935+
* link cannot truncate another file, and must create the PID file without
1936+
* group or world write even under a permissive umask. */
1937+
static int test_ConfigSavePID(void)
1938+
{
1939+
int ret = WS_SUCCESS;
1940+
int rd;
1941+
long pid = -1;
1942+
char base[] = "/tmp/wolfsshd_pidXXXXXX";
1943+
char conf[80] = "";
1944+
char pidPath[96] = "";
1945+
char victim[96] = "";
1946+
char linkPath[96] = "";
1947+
char hvictim[96] = "";
1948+
char hlink[96] = "";
1949+
char fifo[96] = "";
1950+
const char* secret = "VICTIM-CONTENTS\n";
1951+
FILE* f = NULL;
1952+
struct stat st;
1953+
char rbuf[64];
1954+
1955+
if (mkdtemp(base) == NULL) {
1956+
Log(" mkdtemp failed.\n");
1957+
ret = WS_FATAL_ERROR;
1958+
}
1959+
1960+
if (ret == WS_SUCCESS) {
1961+
snprintf(conf, sizeof(conf), "%s/sshd_config", base);
1962+
snprintf(pidPath, sizeof(pidPath), "%s/wolfsshd.pid", base);
1963+
snprintf(victim, sizeof(victim), "%s/victim", base);
1964+
snprintf(linkPath, sizeof(linkPath), "%s/link.pid", base);
1965+
snprintf(hvictim, sizeof(hvictim), "%s/hvictim", base);
1966+
snprintf(hlink, sizeof(hlink), "%s/hlink.pid", base);
1967+
snprintf(fifo, sizeof(fifo), "%s/fifo.pid", base);
1968+
}
1969+
1970+
/* Scenario 1: a normal path is written with our PID and is not group or
1971+
* world writable despite umask(0). */
1972+
if (ret == WS_SUCCESS) {
1973+
ret = pidSave(conf, pidPath);
1974+
}
1975+
if (ret == WS_SUCCESS) {
1976+
if (stat(pidPath, &st) != 0) {
1977+
ret = WS_FATAL_ERROR;
1978+
}
1979+
else {
1980+
f = fopen(pidPath, "r");
1981+
rd = (f != NULL) ? fscanf(f, "%ld", &pid) : 0;
1982+
if (f != NULL) {
1983+
fclose(f);
1984+
}
1985+
ret = smExpect("normal PID file written with our PID",
1986+
(rd == 1 && pid == (long)getpid()) ? WS_SUCCESS
1987+
: WS_FATAL_ERROR, 1);
1988+
if (ret == WS_SUCCESS) {
1989+
ret = smExpect("PID file not group or world writable",
1990+
(st.st_mode & (S_IWGRP | S_IWOTH)) ? WS_FATAL_ERROR
1991+
: WS_SUCCESS, 1);
1992+
}
1993+
}
1994+
}
1995+
1996+
/* Scenario 2: a symlink at the PID path is refused, link target untouched.
1997+
* The common build exercises the atomic O_NOFOLLOW path; the lstat fallback
1998+
* needs a symlink-capable platform without O_NOFOLLOW. */
1999+
if (ret == WS_SUCCESS) {
2000+
f = fopen(victim, "w");
2001+
if (f == NULL) {
2002+
ret = WS_FATAL_ERROR;
2003+
}
2004+
else {
2005+
fputs(secret, f);
2006+
fclose(f);
2007+
}
2008+
}
2009+
if (ret == WS_SUCCESS && symlink(victim, linkPath) != 0) {
2010+
ret = WS_FATAL_ERROR;
2011+
}
2012+
if (ret == WS_SUCCESS) {
2013+
ret = pidSave(conf, linkPath);
2014+
}
2015+
if (ret == WS_SUCCESS) {
2016+
f = fopen(victim, "r");
2017+
WMEMSET(rbuf, 0, sizeof(rbuf));
2018+
rd = (f != NULL) ? (int)fread(rbuf, 1, sizeof(rbuf) - 1, f) : -1;
2019+
if (f != NULL) {
2020+
fclose(f);
2021+
}
2022+
(void)rd;
2023+
/* A WOLFSSH_NO_SYMLINK_CHECK build does no symlink check by design, so
2024+
* only assert the target survived when the check is compiled in. */
2025+
#ifdef WOLFSSH_HAVE_SYMLINK
2026+
ret = smExpect("symlinked PID path not followed, target intact",
2027+
(WSTRCMP(rbuf, secret) == 0) ? WS_SUCCESS : WS_FATAL_ERROR, 1);
2028+
#else
2029+
(void)rbuf;
2030+
#endif
2031+
}
2032+
2033+
/* Scenario 3: a hard link at the PID path is refused (O_NOFOLLOW stops a
2034+
* symlink but not a hard link), leaving the link target untouched. The
2035+
* st_nlink check that enforces this is unconditional, so this always runs. */
2036+
if (ret == WS_SUCCESS) {
2037+
f = fopen(hvictim, "w");
2038+
if (f == NULL) {
2039+
ret = WS_FATAL_ERROR;
2040+
}
2041+
else {
2042+
fputs(secret, f);
2043+
fclose(f);
2044+
}
2045+
}
2046+
if (ret == WS_SUCCESS && link(hvictim, hlink) != 0) {
2047+
ret = WS_FATAL_ERROR;
2048+
}
2049+
if (ret == WS_SUCCESS) {
2050+
ret = pidSave(conf, hlink);
2051+
}
2052+
if (ret == WS_SUCCESS) {
2053+
f = fopen(hvictim, "r");
2054+
WMEMSET(rbuf, 0, sizeof(rbuf));
2055+
rd = (f != NULL) ? (int)fread(rbuf, 1, sizeof(rbuf) - 1, f) : -1;
2056+
if (f != NULL) {
2057+
fclose(f);
2058+
}
2059+
(void)rd;
2060+
ret = smExpect("hard-linked PID path refused, target intact",
2061+
(WSTRCMP(rbuf, secret) == 0) ? WS_SUCCESS : WS_FATAL_ERROR, 1);
2062+
}
2063+
2064+
/* Scenario 4: a FIFO at the PID path is rejected (O_NONBLOCK fast-fails the
2065+
* open and the S_ISREG check refuses it), and the FIFO is left in place
2066+
* rather than replaced by a regular PID file. */
2067+
if (ret == WS_SUCCESS && mkfifo(fifo, 0600) != 0) {
2068+
ret = WS_FATAL_ERROR;
2069+
}
2070+
if (ret == WS_SUCCESS) {
2071+
ret = pidSave(conf, fifo);
2072+
}
2073+
if (ret == WS_SUCCESS) {
2074+
ret = smExpect("FIFO PID path refused, still a FIFO",
2075+
(lstat(fifo, &st) == 0 && S_ISFIFO(st.st_mode)) ? WS_SUCCESS
2076+
: WS_FATAL_ERROR, 1);
2077+
}
2078+
2079+
/* cleanup */
2080+
unlink(fifo);
2081+
unlink(linkPath);
2082+
unlink(victim);
2083+
unlink(hlink);
2084+
unlink(hvictim);
2085+
unlink(pidPath);
2086+
unlink(conf);
2087+
rmdir(base);
2088+
2089+
return ret;
2090+
}
19012091
#endif /* !_WIN32 */
19022092

19032093
const TEST_CASE testCases[] = {
@@ -1920,6 +2110,7 @@ const TEST_CASE testCases[] = {
19202110
TEST_DECL(test_ConfigFree),
19212111
#ifndef _WIN32
19222112
TEST_DECL(test_OpenSecureFile),
2113+
TEST_DECL(test_ConfigSavePID),
19232114
#endif
19242115
#ifdef WOLFSSL_BASE64_ENCODE
19252116
TEST_DECL(test_CheckAuthKeysLine),

0 commit comments

Comments
 (0)