diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 0f91e0dc7..491d64194 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -19,6 +19,13 @@ */ +#ifdef __linux__ + /* for O_PATH */ + #ifndef _GNU_SOURCE + #define _GNU_SOURCE + #endif +#endif + #ifdef HAVE_CONFIG_H #include #endif @@ -1555,6 +1562,115 @@ static byte* wolfSSH_SFTP_RecvGetData(WOLFSSH* ssh) } +/* Root length without trailing separators, "/" kept; over 1 is confined. */ +static word32 SFTP_ConfineLen(const char* root) +{ + word32 len = 0; + + if (root != NULL) { + len = (word32)WSTRLEN(root); + while (len > 1 && WOLFSSH_SFTP_IS_DELIM(root[len - 1])) { + len--; + } + } + return len; +} + + +/* Returns WS_PERMISSIONS if canonical path s is outside the confine root. */ +static int SFTP_CheckConfined(WOLFSSH* ssh, char* s) +{ + int ret = WS_SUCCESS; + const char* confinePath = ssh->sftpConfinePath; + word32 cpLen = SFTP_ConfineLen(confinePath); + word32 sLen = (word32)WSTRLEN(s); + + if (confinePath == NULL) { + return WS_SUCCESS; + } + + if (cpLen > 1) { + /* Constant time over the whole root. Windows folds ASCII case, as the + * root keeps GetCurrentDirectoryA()'s case. */ + if (sLen < cpLen) { + ret = WS_PERMISSIONS; + } + else { + int diff; +#ifdef USE_WINDOWS_API + word32 i; + byte a, b; + + diff = 0; + for (i = 0; i < cpLen; i++) { + a = (byte)s[i]; + b = (byte)confinePath[i]; + a = (byte)(a | (((byte)(a - 'A') < 26) << 5)); + b = (byte)(b | (((byte)(b - 'A') < 26) << 5)); + diff |= a ^ b; + } +#else + diff = wolfSSH_ConstantCompare((const byte*)s, + (const byte*)confinePath, cpLen); +#endif + diff |= (s[cpLen] != '\0' && !WOLFSSH_SFTP_IS_DELIM(s[cpLen])); + if (diff != 0) { + ret = WS_PERMISSIONS; + } + } + } + else { + /* root is "/", the whole filesystem: unconfined, but still only + * absolute paths are accepted */ + if (s[0] == '\0' || !WOLFSSH_SFTP_IS_DELIM(s[0])) { + ret = WS_PERMISSIONS; + } + } + +#ifdef WOLFSSH_HAVE_SYMLINK + if (ret == WS_SUCCESS && cpLen > 1) { + /* RealPath does not resolve links, so reject any link below the + * root. A missing leaf is allowed so creates work. */ + word32 i; + char saved; + + for (i = cpLen + 1; i <= sLen && ret == WS_SUCCESS; i++) { + if (i != sLen && !WOLFSSH_SFTP_IS_DELIM(s[i])) { + continue; + } + saved = s[i]; + s[i] = '\0'; + if (wIsSymlink(s)) { + ret = WS_PERMISSIONS; + } + s[i] = saved; + } + } +#endif /* WOLFSSH_HAVE_SYMLINK */ + + return ret; +} + + +/* What relative requests resolve against: the start path, or the root when + * a confined start path is outside it or goes through a link. */ +static const char* SFTP_StartPath(WOLFSSH* ssh) +{ + const char* start = ssh->sftpDefaultPath; + char s[WOLFSSH_MAX_FILENAME]; + + if (start != NULL && SFTP_ConfineLen(ssh->sftpConfinePath) > 1) { + WSTRNCPY(s, start, sizeof(s) - 1); + s[sizeof(s) - 1] = '\0'; + if (SFTP_CheckConfined(ssh, s) != WS_SUCCESS) { + start = ssh->sftpConfinePath; + } + } + + return start; +} + + /* returns WS_SUCCESS on success */ static int wolfSSH_SFTP_RecvRealPath(WOLFSSH* ssh, int reqId, byte* data, int maxSz) @@ -1636,7 +1752,7 @@ static int wolfSSH_SFTP_RecvRealPath(WOLFSSH* ssh, int reqId, byte* data, return ret; } - ret = wolfSSH_RealPath(ssh->sftpDefaultPath, r, s, sizeof s); + ret = wolfSSH_RealPath(SFTP_StartPath(ssh), r, s, sizeof s); if (ret != WS_SUCCESS) { return ret; } @@ -1644,6 +1760,29 @@ static int wolfSSH_SFTP_RecvRealPath(WOLFSSH* ssh, int reqId, byte* data, WLOG(WS_LOG_SFTP, "Real Path Directory = %s", s); + /* name only what the other requests would accept */ + if (SFTP_CheckConfined(ssh, s) != WS_SUCCESS) { + WLOG(WS_LOG_SFTP, "Real Path outside the confinement root"); + outSz = 0; + if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, + "Permission denied", "English", NULL, &outSz) + != WS_SIZE_ONLY) { + return WS_FATAL_ERROR; + } + out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER); + if (out == NULL) { + return WS_MEMORY_E; + } + if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_PERMISSION, reqId, + "Permission denied", "English", out, &outSz) + != WS_SUCCESS) { + WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER); + return WS_FATAL_ERROR; + } + wolfSSH_SFTP_RecvSetSend(ssh, out, outSz); + return WS_BAD_FILE_E; + } + /* send response */ outSz = WOLFSSH_SFTP_HEADER + (UINT32_SZ * 3) + (sSz * 2); WMEMSET(&atr, 0, sizeof(WS_SFTP_FILEATRB)); @@ -2043,8 +2182,6 @@ static int GetAndCleanPath(WOLFSSH* ssh, const byte* data, word32 sz, char* s, word32 sSz) { int ret; - word32 cpLen = 0; - const char* confinePath = ssh->sftpConfinePath; char r[WOLFSSH_MAX_FILENAME]; if (sz >= sizeof r) @@ -2052,71 +2189,113 @@ static int GetAndCleanPath(WOLFSSH* ssh, WMEMCPY(r, data, sz); r[sz] = '\0'; - ret = wolfSSH_RealPath(ssh->sftpDefaultPath, r, s, sSz); - if (ret == WS_SUCCESS && confinePath != NULL) { - /* both sides are canonical - the root by SetConfinePath, the request - * by the RealPath call above - so a prefix compare confines it */ - cpLen = (word32)WSTRLEN(confinePath); - /* strip trailing separator(s), but keep a lone "/" as-is */ - while (cpLen > 1 && WOLFSSH_SFTP_IS_DELIM(confinePath[cpLen - 1])) { - cpLen--; - } - if (cpLen > 1) { - /* the resolved path must be the root itself or sit under it. - * Windows paths are case-insensitive and the root keeps - * GetCurrentDirectoryA()'s case, so compare without case there - * rather than reject in-jail requests over case alone. */ -#ifdef USE_WINDOWS_API - if (WSTRNCASECMP(s, confinePath, cpLen) != 0 || -#else - if (WSTRNCMP(s, confinePath, cpLen) != 0 || + ret = wolfSSH_RealPath(SFTP_StartPath(ssh), r, s, sSz); + if (ret == WS_SUCCESS) { + ret = SFTP_CheckConfined(ssh, s); + } + + return ret; +} + + +/* Needs a search-only open: O_RDONLY would demand read permission on each + * directory. */ +#if defined(WOLFSSH_HAVE_DIRFD) && defined(WOLFSSH_HAVE_SYMLINK) && \ + defined(O_NOFOLLOW) && (defined(O_SEARCH) || defined(O_PATH)) +#define WOLFSSH_SFTP_OPEN_BENEATH + +/* Opens path walking from the confine root with O_NOFOLLOW, refusing a link + * swapped in after the check. Returns -1 with errno set on failure. */ +static WFD SFTP_OpenBeneath(WOLFSSH* ssh, const char* path, int flags, + int mode) +{ + char comp[WOLFSSH_MAX_FILENAME]; + const char* p; + const char* end; + word32 cpLen = SFTP_ConfineLen(ssh->sftpConfinePath); + WFD dfd; + WFD nfd; + WFD fd = -1; + int err; + + if (cpLen <= 1) { + return WOPEN(ssh->fs, path, flags, mode); + } +#ifdef WOLFSSH_TEST_INTERNAL + if (ssh->testSftpPreOpen != NULL) { + ssh->testSftpPreOpen(ssh->testSftpPreOpenCtx); + } #endif - (s[cpLen] != '\0' && !WOLFSSH_SFTP_IS_DELIM(s[cpLen]))) { - ret = WS_PERMISSIONS; - } + + /* the root is trusted, as in SFTP_CheckConfined */ + dfd = WOPEN(ssh->fs, ssh->sftpConfinePath, + WOLFSSH_O_SEARCH | WOLFSSH_O_DIRECTORY, 0); + p = path + cpLen; + while (dfd >= 0) { + while (WOLFSSH_SFTP_IS_DELIM(*p)) { + p++; + } + end = p; + while (*end != '\0' && !WOLFSSH_SFTP_IS_DELIM(*end)) { + end++; + } + if (end == p) { + /* path is the root itself */ + fd = WOPENAT(ssh->fs, dfd, ".", flags | WOLFSSH_O_NOFOLLOW, mode); + break; } - else { - /* root is "/", the whole filesystem: unconfined, but still only - * absolute paths are accepted */ - if (s[0] == '\0' || !WOLFSSH_SFTP_IS_DELIM(s[0])) { - ret = WS_PERMISSIONS; - } + WMEMCPY(comp, p, (word32)(end - p)); + comp[end - p] = '\0'; + p = end; + while (WOLFSSH_SFTP_IS_DELIM(*p)) { + p++; + } + if (*p == '\0') { + fd = WOPENAT(ssh->fs, dfd, comp, flags | WOLFSSH_O_NOFOLLOW, mode); + break; } + nfd = WOPENAT(ssh->fs, dfd, comp, WOLFSSH_O_SEARCH | + WOLFSSH_O_DIRECTORY | WOLFSSH_O_NOFOLLOW, 0); + err = errno; + WCLOSE(ssh->fs, dfd); + errno = err; + dfd = nfd; + } + if (dfd >= 0) { + err = errno; + WCLOSE(ssh->fs, dfd); + errno = err; } -#ifdef WOLFSSH_HAVE_SYMLINK - if (ret == WS_SUCCESS && confinePath != NULL && cpLen > 1) { - /* Defense in depth: the prefix check only proves the path string - * stays under the jail, and wolfSSH_RealPath does not resolve - * symlinks, so an in-jail link pointing out would pass it and then be - * followed. Reject any existing component below the root that is a - * link, in-jail targets included; a not-yet-created leaf is left to - * the operation so creates still work. */ - word32 i; - word32 sLen = (word32)WSTRLEN(s); - char saved; + return fd; +} - for (i = cpLen; i <= sLen && ret == WS_SUCCESS; i++) { - /* act only at a component boundary (a separator) or the leaf */ - if (i != sLen && !WOLFSSH_SFTP_IS_DELIM(s[i])) { - continue; - } - /* the jail root itself is trusted; only inspect inside the jail */ - if (i <= cpLen) { - continue; - } - saved = s[i]; - s[i] = '\0'; - if (wIsSymlink(s)) { - ret = WS_PERMISSIONS; - } - s[i] = saved; - } +#ifndef NO_WOLFSSH_DIR +/* SFTP_OpenBeneath for WOPENDIR; returns 0 on success. */ +static int SFTP_OpenDirBeneath(WOLFSSH* ssh, WDIR* dir, const char* path) +{ + WFD fd; + int err; + + if (SFTP_ConfineLen(ssh->sftpConfinePath) <= 1) { + return WOPENDIR(ssh->fs, ssh->ctx->heap, dir, path); } -#endif /* WOLFSSH_HAVE_SYMLINK */ - return ret; + fd = SFTP_OpenBeneath(ssh, path, WOLFSSH_O_RDONLY | WOLFSSH_O_DIRECTORY, + 0); + if (fd < 0) { + return 1; + } + if (WFDOPENDIR(ssh->fs, dir, fd) != 0) { + err = errno; + WCLOSE(ssh->fs, fd); + errno = err; + return 1; + } + return 0; } +#endif /* !NO_WOLFSSH_DIR */ +#endif /* WOLFSSH_SFTP_OPEN_BENEATH */ /* Builds a status packet and queues it for send. @@ -2634,6 +2813,8 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) fd = -1; } } + #elif defined(WOLFSSH_SFTP_OPEN_BENEATH) + fd = SFTP_OpenBeneath(ssh, dir, m, WOLFSSH_SFTP_SAFE_MODE(atr.per)); #else fd = WOPEN(ssh->fs, dir, m, WOLFSSH_SFTP_SAFE_MODE(atr.per)); #endif @@ -2999,7 +3180,11 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc; } +#ifdef WOLFSSH_SFTP_OPEN_BENEATH + if (SFTP_OpenDirBeneath(ssh, &ctx, dir) != 0) { +#else if (WOPENDIR(ssh->fs, ssh->ctx->heap, &ctx, dir) != 0) { +#endif byte type = SFTP_FailStatus(WOLFSSH_FTP_NOFILE); WLOG(WS_LOG_SFTP, "Error with opening directory: %s", dir); @@ -6714,6 +6899,25 @@ int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh) } #endif /* NO_WOLFSSH_DIR */ +/* Test hook run between the confinement check and the open walk. Returns + * WS_NOT_COMPILED on builds without the walk. */ +int wolfSSH_SFTP_TestSetPreOpen(WOLFSSH* ssh, void (*cb)(void* ctx), + void* ctx) +{ + if (ssh == NULL) { + return WS_BAD_ARGUMENT; + } +#ifdef WOLFSSH_SFTP_OPEN_BENEATH + ssh->testSftpPreOpen = cb; + ssh->testSftpPreOpenCtx = ctx; + return WS_SUCCESS; +#else + (void)cb; + (void)ctx; + return WS_NOT_COMPILED; +#endif +} + #ifndef USE_WINDOWS_API /* Close the underlying descriptor of the head tracked file handle out of band, * leaving the node in the list with a now-stale fd. The next RecvClose on that @@ -9460,6 +9664,14 @@ int wolfSSH_SFTP_SetConfinePath(WOLFSSH* ssh, const char* path) if (ret == WS_SUCCESS) { ret = StorePath(ssh, &ssh->sftpConfinePath, canon); } +#ifdef WOLFSSH_NO_SFTP_CONFINE + /* "/" confines nothing, so it is allowed. Any other root is still stored, + * so a caller ignoring the error keeps the lexical jail. */ + if (ret == WS_SUCCESS && WSTRSPN(canon, "/\\") != WSTRLEN(canon)) { + WLOG(WS_LOG_SFTP, "SFTP confinement unsupported, links not refused"); + ret = WS_NOT_COMPILED; + } +#endif return ret; } diff --git a/tests/api.c b/tests/api.c index b68c46f60..48155ed69 100644 --- a/tests/api.c +++ b/tests/api.c @@ -6001,6 +6001,7 @@ static void test_wolfSSH_SFTP_ReKey_NonBlock(void) sftp_rekey_test(1); } +#ifndef WOLFSSH_NO_SFTP_CONFINE static void test_wolfSSH_SFTP_Confinement(void) { func_args ser; @@ -6297,6 +6298,18 @@ static void test_wolfSSH_SFTP_Confinement(void) wolfSSH_SFTPNAME_list_free(ls); ls = NULL; + /* REALPATH refuses an escape but still names the root */ + ls = wolfSSH_SFTP_RealPath(ssh, escFile); + AssertNull(ls); +#if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) + ls = wolfSSH_SFTP_RealPath(ssh, escRel); + AssertNull(ls); +#endif + ls = wolfSSH_SFTP_RealPath(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; + #if !defined(WOLFSSH_ZEPHYR) && !defined(USE_WINDOWS_API) && \ defined(WOLFSSH_HAVE_SYMLINK) /* Symlink escape: an in-jail symlink to the out-of-jail tree resolves to @@ -6321,6 +6334,14 @@ static void test_wolfSSH_SFTP_Confinement(void) AssertNotNull(ls); wolfSSH_SFTPNAME_list_free(ls); ls = NULL; + + /* nor is a path through the link named by REALPATH */ + ls = wolfSSH_SFTP_RealPath(ssh, escSymThru); + AssertNull(ls); + ls = wolfSSH_SFTP_LS(ssh, curDir); + AssertNotNull(ls); + wolfSSH_SFTPNAME_list_free(ls); + ls = NULL; #endif /* Positive case: a relative write op that resolves inside the jail must be @@ -6379,6 +6400,10 @@ static void test_wolfSSH_SFTP_Confinement(void) } #endif } +#else +/* the echoserver's -D is refused on this build */ +static void test_wolfSSH_SFTP_Confinement(void) { ; } +#endif /* !WOLFSSH_NO_SFTP_CONFINE */ /* A session given only a start path is NOT confined to it: the wolfsshd @@ -6521,6 +6546,15 @@ static void test_wolfSSH_SFTP_SetConfinePath(void) AssertStrEQ(ssh->sftpDefaultPath, "/tmp/start"); AssertNull(ssh->sftpConfinePath); +#ifdef WOLFSSH_NO_SFTP_CONFINE + /* refused on this build, except "/", but still stored */ + AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, "/tmp/./jail"), + WS_NOT_COMPILED); + AssertStrEQ(ssh->sftpConfinePath, "/tmp/jail"); + AssertStrEQ(ssh->sftpDefaultPath, "/tmp/start"); + AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, "/"), WS_SUCCESS); + AssertStrEQ(ssh->sftpConfinePath, "/"); +#else /* a confinement root on its own does not move the start path */ AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, "/tmp/./jail"), WS_SUCCESS); AssertStrEQ(ssh->sftpConfinePath, "/tmp/jail"); @@ -6534,6 +6568,7 @@ static void test_wolfSSH_SFTP_SetConfinePath(void) AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, "/var/jail2"), WS_SUCCESS); AssertStrEQ(ssh->sftpConfinePath, "/var/jail2"); AssertStrEQ(ssh->sftpDefaultPath, "/tmp/jail/sub"); +#endif wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); diff --git a/tests/regress.c b/tests/regress.c index 07cc15ff5..7c4559a1e 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -17463,6 +17463,15 @@ static void TestSftpBufferSendPendingOutput(void) } #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) +/* Confines a fixture to root; lexical only where links are not refused. */ +#ifdef WOLFSSH_NO_SFTP_CONFINE + #define AssertSftpConfine(ssh, root) \ + AssertIntEQ(wolfSSH_SFTP_SetConfinePath((ssh), (root)), WS_NOT_COMPILED) +#else + #define AssertSftpConfine(ssh, root) \ + AssertIntEQ(wolfSSH_SFTP_SetConfinePath((ssh), (root)), WS_SUCCESS) +#endif + /* Write a big-endian uint32 (the SFTP wire encoding). */ static void SftpPutU32(word32 val, byte* out) { @@ -17925,7 +17934,7 @@ static void TestSftpForgedHandleRejected(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); /* ---- positive control: legitimately open a file over SFTP ---- * RecvOpen assigns the first handle the per-session id {0,0}. */ @@ -18111,7 +18120,7 @@ static void TestSftpHandleNamespaceIsolation(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); /* open a directory -> first id from the shared counter */ idx = 0; @@ -18244,7 +18253,7 @@ static void TestSftpHandleLimit(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); /* open the cap's worth of handles against one file; all must succeed */ for (i = 0; i < WOLFSSH_MAX_SFTP_HANDLES; i++) { @@ -18378,7 +18387,7 @@ static void TestSftpDirHandleLimit(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); /* open the cap's worth of handles on "."; all must succeed */ for (i = 0; i < WOLFSSH_MAX_SFTP_HANDLES; i++) { @@ -18631,7 +18640,7 @@ static void TestSftpReadDirBounded(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); SftpMakeDirFixture(dir, sizeof(dir), FILES); WMEMSET(seen, 0, sizeof(seen)); @@ -18686,7 +18695,7 @@ static void TestSftpReadDirLongName(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); SftpMakeDirFixture(dir, sizeof(dir), FILES); WSNPRINTF(longName, sizeof(longName), "f%0*d", LONG_SZ - 1, FILES); WSNPRINTF(path, sizeof(path), "%s/%s", dir, longName); @@ -18811,7 +18820,7 @@ static void TestSftpReadDirAllocFailure(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); SftpMakeDirFixture(dir, sizeof(dir), FILES); /* count the allocations of a clean READDIR */ @@ -18870,7 +18879,7 @@ static void TestSftpOpenDirAllocFailure(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); SftpPutU32(1, pkt + idx); idx += UINT32_SZ; pkt[idx++] = '.'; @@ -18938,7 +18947,7 @@ static void TestSftpMissingFileStatus(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); WSNPRINTF(dir, sizeof(dir), "wolfssh_nofile_%d", (int)getpid()); WSNPRINTF(missing, sizeof(missing), "%s/missing", dir); @@ -19040,7 +19049,7 @@ static void TestSftpCloseFailureRemovesHandle(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); idx = 0; SftpPutU32((word32)WSTRLEN(path), pkt + idx); idx += UINT32_SZ; @@ -19177,7 +19186,7 @@ static void TestSftpSetStatAttributes(void) WMEMSET(cwd, 0, sizeof(cwd)); AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); - AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); /* fixture: a file of known length owned by this process */ fd = WOPEN(ssh->fs, path, @@ -19348,6 +19357,7 @@ static void TestSftpSetStatAttributes(void) wolfSSH_CTX_free(ctx); } +#ifndef WOLFSSH_NO_SFTP_CONFINE /* Sends an FXP_STAT for path and returns the handler's return code. */ static int SftpStatPath(WOLFSSH* ssh, int reqId, const char* path) { @@ -19483,6 +19493,325 @@ static void TestSftpStartPathInsideConfineRoot(void) wolfSSH_CTX_free(ctx); } +/* A start path outside the root is replaced by the root, so relative + * requests resolve there instead of failing. */ +static void TestSftpStartPathOutsideConfineRoot(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + int rid = 550; + char cwd[WOLFSSH_MAX_FILENAME - 64]; + char root[WOLFSSH_MAX_FILENAME]; + char start[WOLFSSH_MAX_FILENAME]; + char rootFile[WOLFSSH_MAX_FILENAME]; + char startFile[WOLFSSH_MAX_FILENAME]; + WFILE* fp = NULL; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + + WMEMSET(cwd, 0, sizeof(cwd)); + if (WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1) == NULL) { + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return; + } + + #define CONFINE_OUT "%s/wolfssh_confout_%d" + WSNPRINTF(root, sizeof(root), CONFINE_OUT "_root", cwd, (int)getpid()); + WSNPRINTF(start, sizeof(start), CONFINE_OUT "_start", cwd, (int)getpid()); + WSNPRINTF(rootFile, sizeof(rootFile), CONFINE_OUT "_root/f", + cwd, (int)getpid()); + WSNPRINTF(startFile, sizeof(startFile), CONFINE_OUT "_start/f", + cwd, (int)getpid()); + #undef CONFINE_OUT + + AssertIntEQ(WMKDIR(ssh->fs, root, 0755), 0); + AssertIntEQ(WMKDIR(ssh->fs, start, 0755), 0); + AssertIntEQ(WFOPEN(ssh->fs, &fp, rootFile, "wb"), 0); + AssertNotNull(fp); + WFCLOSE(ssh->fs, fp); + AssertIntEQ(WFOPEN(ssh->fs, &fp, startFile, "wb"), 0); + AssertNotNull(fp); + WFCLOSE(ssh->fs, fp); + + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, start), WS_SUCCESS); + AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, root), WS_SUCCESS); + + /* relative names resolve under the root */ + AssertIntEQ(SftpStatPath(ssh, ++rid, "."), WS_SUCCESS); + AssertSftpAttrsReply(ssh, rid); + AssertIntEQ(SftpStatPath(ssh, ++rid, "f"), WS_SUCCESS); + AssertSftpAttrsReply(ssh, rid); + + /* the start directory itself stays out of bounds */ + AssertIntEQ(SftpStatPath(ssh, ++rid, startFile), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid, WOLFSSH_FTP_PERMISSION); + + (void)WREMOVE(ssh->fs, rootFile); + (void)WREMOVE(ssh->fs, startFile); + (void)WRMDIR(ssh->fs, root); + (void)WRMDIR(ssh->fs, start); + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +/* A confined session can create and read through mode 0300 directories. + * Skipped as root, which passes every permission check. */ +static void TestSftpOpenThroughSearchOnlyDir(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + int rid = 600; + int created; + int createdDeep; + int reread; + byte handle[WOLFSSH_HANDLE_ID_SZ]; + char cwd[WOLFSSH_MAX_FILENAME]; + char dir[64]; + char sub[96]; + char file[96]; + char deep[128]; + + if (geteuid() == 0) { + return; + } + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + + WMEMSET(cwd, 0, sizeof(cwd)); + AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); + AssertSftpConfine(ssh, cwd); + + WSNPRINTF(dir, sizeof(dir), "wolfssh_search_%d", (int)getpid()); + WSNPRINTF(sub, sizeof(sub), "%s/sub", dir); + WSNPRINTF(file, sizeof(file), "%s/up", dir); + WSNPRINTF(deep, sizeof(deep), "%s/sub/up", dir); + AssertIntEQ(mkdir(dir, 0700), 0); + AssertIntEQ(mkdir(sub, 0300), 0); + AssertIntEQ(chmod(dir, 0300), 0); + + /* assert after restoring the modes so cleanup can run */ + created = SftpOpenPath(ssh, ++rid, file, + WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT, handle); + if (created == WS_SUCCESS) { + (void)SftpCloseHandle(ssh, ++rid, handle); + } + createdDeep = SftpOpenPath(ssh, ++rid, deep, + WOLFSSH_FXF_WRITE | WOLFSSH_FXF_CREAT, handle); + if (createdDeep == WS_SUCCESS) { + (void)SftpCloseHandle(ssh, ++rid, handle); + } + reread = SftpOpenPath(ssh, ++rid, deep, WOLFSSH_FXF_READ, handle); + if (reread == WS_SUCCESS) { + (void)SftpCloseHandle(ssh, ++rid, handle); + } + + (void)chmod(dir, 0700); + (void)chmod(sub, 0700); + (void)remove(deep); + (void)remove(file); + (void)rmdir(sub); + (void)rmdir(dir); + AssertIntEQ(created, WS_SUCCESS); + AssertIntEQ(createdDeep, WS_SUCCESS); + AssertIntEQ(reread, WS_SUCCESS); + AssertIntEQ(wolfSSH_SFTP_TestFileHandleCount(ssh), 0); + + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + +/* Swaps victim for a link to target between the check and the open. */ +typedef struct SftpLinkSwap { + const char* victim; + const char* aside; + const char* target; + int swapped; +} SftpLinkSwap; + +static void SftpLinkSwapCb(void* ctx) +{ + SftpLinkSwap* s = (SftpLinkSwap*)ctx; + + if (s->victim != NULL && rename(s->victim, s->aside) == 0) { + if (symlink(s->target, s->victim) == 0) { + s->swapped = 1; + } + else { + (void)rename(s->aside, s->victim); + } + } +} + +static void SftpLinkSwapUndo(SftpLinkSwap* s) +{ + if (s->swapped) { + (void)unlink(s->victim); + (void)rename(s->aside, s->victim); + s->swapped = 0; + } +} + +/* Returns the lowest free descriptor, for spotting an fd leak. */ +static int SftpLowestFreeFd(void) +{ + int fd = dup(0); + + if (fd >= 0) { + close(fd); + } + return fd; +} + +/* OPEN and OPENDIR refuse a component swapped for a link after the check, + * at the leaf and partway down. */ +static void TestSftpOpenRefusesSwappedLink(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + SftpLinkSwap swap; + int rid = 700; + int ret; + int refused[3] = { 0, 1, 0 }; /* the OPENDIR case may be compiled out */ + int handles; + int freeFd; + int leakedFd; + byte handle[WOLFSSH_HANDLE_ID_SZ]; + /* leaves room for the fixture suffixes */ + char cwd[WOLFSSH_MAX_FILENAME - 64]; + char base[WOLFSSH_MAX_FILENAME]; + char root[WOLFSSH_MAX_FILENAME]; + char out[WOLFSSH_MAX_FILENAME]; + char outFile[WOLFSSH_MAX_FILENAME]; + char dir[WOLFSSH_MAX_FILENAME]; + char dirAside[WOLFSSH_MAX_FILENAME]; + char dirFile[WOLFSSH_MAX_FILENAME]; + char leaf[WOLFSSH_MAX_FILENAME]; + char leafAside[WOLFSSH_MAX_FILENAME]; + WFILE* fp = NULL; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + WMEMSET(&swap, 0, sizeof(swap)); + + WMEMSET(cwd, 0, sizeof(cwd)); + if (wolfSSH_SFTP_TestSetPreOpen(ssh, SftpLinkSwapCb, &swap) + != WS_SUCCESS || + WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1) == NULL) { + /* no walk on this build, or too deep a directory for the fixture */ + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return; + } + + #define SWAP_BASE "%s/wolfssh_swap_%d" + WSNPRINTF(base, sizeof(base), SWAP_BASE, cwd, (int)getpid()); + WSNPRINTF(root, sizeof(root), SWAP_BASE "/jail", cwd, (int)getpid()); + WSNPRINTF(out, sizeof(out), SWAP_BASE "/out", cwd, (int)getpid()); + WSNPRINTF(outFile, sizeof(outFile), SWAP_BASE "/out/f", + cwd, (int)getpid()); + WSNPRINTF(dir, sizeof(dir), SWAP_BASE "/jail/d", cwd, (int)getpid()); + WSNPRINTF(dirAside, sizeof(dirAside), SWAP_BASE "/jail/d.real", + cwd, (int)getpid()); + WSNPRINTF(dirFile, sizeof(dirFile), SWAP_BASE "/jail/d/f", + cwd, (int)getpid()); + WSNPRINTF(leaf, sizeof(leaf), SWAP_BASE "/jail/g", cwd, (int)getpid()); + WSNPRINTF(leafAside, sizeof(leafAside), SWAP_BASE "/jail/g.real", + cwd, (int)getpid()); + #undef SWAP_BASE + + AssertIntEQ(WMKDIR(ssh->fs, base, 0700), 0); + AssertIntEQ(WMKDIR(ssh->fs, root, 0700), 0); + AssertIntEQ(WMKDIR(ssh->fs, out, 0700), 0); + AssertIntEQ(WMKDIR(ssh->fs, dir, 0700), 0); + AssertIntEQ(WFOPEN(ssh->fs, &fp, outFile, "wb"), 0); + WFCLOSE(ssh->fs, fp); + AssertIntEQ(WFOPEN(ssh->fs, &fp, dirFile, "wb"), 0); + WFCLOSE(ssh->fs, fp); + AssertIntEQ(WFOPEN(ssh->fs, &fp, leaf, "wb"), 0); + WFCLOSE(ssh->fs, fp); + + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, root), WS_SUCCESS); + AssertSftpConfine(ssh, root); + freeFd = SftpLowestFreeFd(); + + /* positive control: no swap */ + AssertIntEQ(SftpOpenPath(ssh, ++rid, "d/f", WOLFSSH_FXF_READ, handle), + WS_SUCCESS); + AssertIntEQ(SftpCloseHandle(ssh, ++rid, handle), WS_SUCCESS); + AssertIntEQ(SftpOpenPath(ssh, ++rid, "g", WOLFSSH_FXF_READ, handle), + WS_SUCCESS); + AssertIntEQ(SftpCloseHandle(ssh, ++rid, handle), WS_SUCCESS); +#ifndef NO_WOLFSSH_DIR + SftpOpenDirPath(ssh, ++rid, "d", handle); + AssertIntEQ(SftpCloseHandle(ssh, ++rid, handle), WS_SUCCESS); +#endif + + /* asserts wait until the fixture is removed */ + /* a directory partway down swapped for a link to out/ */ + swap.victim = dir; + swap.aside = dirAside; + swap.target = out; + ret = SftpOpenPath(ssh, ++rid, "d/f", WOLFSSH_FXF_READ, handle); + refused[0] = swap.swapped && ret != WS_SUCCESS; + SftpLinkSwapUndo(&swap); + +#ifndef NO_WOLFSSH_DIR + /* the directory listed swapped for a link to out/ */ + ret = SftpPathRequest(ssh, WOLFSSH_FTP_OPENDIR, ++rid, "d"); + refused[1] = swap.swapped && ret != WS_SUCCESS; + SftpLinkSwapUndo(&swap); +#endif + + /* the file opened swapped for a link to out/f */ + swap.victim = leaf; + swap.aside = leafAside; + swap.target = outFile; + ret = SftpOpenPath(ssh, ++rid, "g", WOLFSSH_FXF_READ, handle); + refused[2] = swap.swapped && ret != WS_SUCCESS; + SftpLinkSwapUndo(&swap); + swap.victim = NULL; + + handles = wolfSSH_SFTP_TestFileHandleCount(ssh); +#ifndef NO_WOLFSSH_DIR + handles += wolfSSH_SFTP_TestDirHandleCount(ssh); +#endif + leakedFd = SftpLowestFreeFd() != freeFd; + + (void)WREMOVE(ssh->fs, dirFile); + (void)WREMOVE(ssh->fs, leaf); + (void)WREMOVE(ssh->fs, outFile); + (void)WRMDIR(ssh->fs, dir); + (void)WRMDIR(ssh->fs, out); + (void)WRMDIR(ssh->fs, root); + (void)WRMDIR(ssh->fs, base); + AssertIntEQ(refused[0], 1); + AssertIntEQ(refused[1], 1); + AssertIntEQ(refused[2], 1); + AssertIntEQ(handles, 0); + AssertIntEQ(leakedFd, 0); + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} +#endif /* !WOLFSSH_NO_SFTP_CONFINE */ + #endif /* !NO_WOLFSSH_SERVER && !USE_WINDOWS_API && !NO_FILESYSTEM */ #if !defined(NO_WOLFSSH_SERVER) && defined(USE_WINDOWS_API) && \ @@ -23507,8 +23836,15 @@ int main(int argc, char** argv) #endif /* a failed close still drops the handle from the tracking list */ TestSftpCloseFailureRemovesHandle(); + #ifndef WOLFSSH_NO_SFTP_CONFINE /* confinement follows the confine root, not the start path */ TestSftpStartPathInsideConfineRoot(); + TestSftpStartPathOutsideConfineRoot(); + /* a directory that may be searched but not read can be passed through */ + TestSftpOpenThroughSearchOnlyDir(); + /* a link swapped in after the check is refused by the open walk */ + TestSftpOpenRefusesSwappedLink(); + #endif /* SETSTAT/FSETSTAT apply the attributes they acknowledge */ TestSftpSetStatAttributes(); #endif diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 90afa378a..6ebfd3e9f 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1539,6 +1539,8 @@ struct WOLFSSH { #ifdef WOLFSSH_TEST_INTERNAL word32 testSftpSendCap; /* test hook: cap per-call SFTP buffer send */ word32 testSftpStallPending; /* test hook: force N flush-only resumes */ + void (*testSftpPreOpen)(void* ctx); /* test hook: before the open walk */ + void* testSftpPreOpenCtx; #endif byte userAuthSeen; /* a userauth request has bound the username */ word32 maxAuthAttempts; /* server cap on failed userauth attempts */ diff --git a/wolfssh/port.h b/wolfssh/port.h index 85d4cb445..a7fa0c64c 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1776,6 +1776,15 @@ extern "C" { #define WOLFSSH_O_NOFOLLOW 0 #endif +/* Builds where SFTP cannot refuse a link below a confine root; see + * wolfsftp.h. */ +#if !defined(WOLFSSH_NO_SFTP_CONFINE) && \ + (defined(WOLFSSH_NO_SYMLINK_CHECK) || defined(WOLFSSH_USER_FILESYSTEM) || \ + (defined(WOLFSSH_HAVE_SYMLINK) && !defined(USE_WINDOWS_API) && \ + !defined(O_NOFOLLOW))) + #define WOLFSSH_NO_SFTP_CONFINE +#endif + /* Catch-all so callers can test a seek result unconditionally. Ports whose * seek returns the new file position rather than a 0/-1 status define their * own WFSEEK_SUCCESS() in the port block above; see Nucleus and MPLAB diff --git a/wolfssh/wolfsftp.h b/wolfssh/wolfsftp.h index 29b9bc84f..0c109ca28 100644 --- a/wolfssh/wolfsftp.h +++ b/wolfssh/wolfsftp.h @@ -244,22 +244,23 @@ WOLFSSH_LOCAL int wolfSSH_SFTP_OpenDir(WOLFSSH* ssh, byte* buf, word32 bufSz); * Paths are resolved lexically, which cannot prove a link stays in-jail, so * confinement rejects ALL symbolic links below the root - including ones * pointing back inside it (e.g. "current -> releases/v3"). Serve trees without - * symlinks, or build with WOLFSSH_NO_SYMLINK_CHECK to drop the check, and the - * escape protection with it. + * symlinks. REALPATH answers are confined the same way. A start path outside + * the root, or through a link below it, is replaced by the root. * * The root itself is trusted and never checked, so a root reached through a * symbolic link is as wide as that link's target. Give a root the server * controls, with no symlink components. * - * That link check is defense in depth, not a boundary: it is a TOCTOU check, - * inspecting each component before the operation later acts on the path by - * name. A concurrent writer inside the jail, running as the user the server - * does file operations as, could swap a checked component for a symlink in - * that window; a session cannot race itself, since wolfSSH serves one - * session's requests serially. The check reliably blocks static in-jail - * symlinks, but closing the race portably needs *at/O_NOFOLLOW primitives some - * supported filesystems lack. Hostile multi-tenant deployments want an - * OS-level jail (chroot plus dropped privileges). + * Builds that cannot refuse a link (WOLFSSH_NO_SFTP_CONFINE: under + * WOLFSSH_NO_SYMLINK_CHECK, WOLFSSH_USER_FILESYSTEM, or POSIX without + * O_NOFOLLOW) return WS_NOT_COMPILED for any root but "/". The root is still + * stored and checked lexically, so links can escape it. + * + * A concurrent writer in the jail can swap in a link after the check. Where + * openat(), O_NOFOLLOW, and O_SEARCH or O_PATH exist, OPEN and OPENDIR walk + * from the root refusing links; other requests still act by name. Hostile + * multi-tenant deployments want an OS-level jail (chroot plus dropped + * privileges). */ WOLFSSH_API int wolfSSH_SFTP_SetDefaultPath(WOLFSSH* ssh, const char* path); WOLFSSH_API int wolfSSH_SFTP_SetConfinePath(WOLFSSH* ssh, const char* path); @@ -377,6 +378,8 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void); word32* sz); WOLFSSH_API void wolfSSH_SFTP_TestRecvStateFree(WOLFSSH* ssh); WOLFSSH_API int wolfSSH_SFTP_TestFileHandleCount(WOLFSSH* ssh); + WOLFSSH_API int wolfSSH_SFTP_TestSetPreOpen(WOLFSSH* ssh, + void (*cb)(void* ctx), void* ctx); #ifndef NO_WOLFSSH_DIR WOLFSSH_API int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh); #endif