Skip to content

Commit 4b05c4e

Browse files
ejohnstownphilljj
authored andcommitted
sftp: report missing and refused paths by errno
STAT, LSTAT, OPEN, REMOVE, RENAME and OPENDIR take the status for a failed call from errno, as OpenSSH's sftp-server does: ENOENT and ENOTDIR answer SSH_FX_NO_SUCH_FILE, EACCES and EPERM SSH_FX_PERMISSION_DENIED, anything else SSH_FX_FAILURE. Only the default POSIX file system sets errno; other ports keep their old status. - OPENDIR on POSIX no longer reports every failure as NO_SUCH_FILE Issue: F-16081
1 parent 8cdfc3e commit 4b05c4e

2 files changed

Lines changed: 202 additions & 8 deletions

File tree

‎src/wolfsftp.c‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,6 +2138,33 @@ static int SFTP_SendStatus(WOLFSSH* ssh, byte type, int reqId, const char* msg)
21382138
}
21392139

21402140

2141+
/* Status code for a file system call that just failed, from errno where the
2142+
* file system sets it: NO_SUCH_FILE for a missing path, PERMISSION_DENIED for
2143+
* a refused one, FAILURE otherwise. Without errno it is fallback. Call it
2144+
* before anything else, a log line included, can change errno. */
2145+
static byte SFTP_FailStatus(byte fallback)
2146+
{
2147+
byte type = fallback;
2148+
2149+
#ifdef WOLFSSH_SFTP_ERRNO
2150+
switch (errno) {
2151+
case ENOENT:
2152+
case ENOTDIR:
2153+
type = WOLFSSH_FTP_NOFILE;
2154+
break;
2155+
case EACCES:
2156+
case EPERM:
2157+
type = WOLFSSH_FTP_PERMISSION;
2158+
break;
2159+
default:
2160+
type = WOLFSSH_FTP_FAILURE;
2161+
break;
2162+
}
2163+
#endif
2164+
return type;
2165+
}
2166+
2167+
21412168
/* Handles packet to remove a directory
21422169
*
21432170
* returns WS_SUCCESS on success
@@ -2472,6 +2499,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
24722499
char naf[] = "Not A File";
24732500
char per[] = "Permission denied";
24742501
char tmf[] = "Too Many Open File Handles";
2502+
byte type = WOLFSSH_FTP_FAILURE;
24752503
int handleStored = 0;
24762504

24772505
if (ssh == NULL) {
@@ -2601,9 +2629,10 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
26012629
fd = WOPEN(ssh->fs, dir, m, WOLFSSH_SFTP_SAFE_MODE(atr.per));
26022630
#endif
26032631
if (fd < 0) {
2632+
type = SFTP_FailStatus(WOLFSSH_FTP_FAILURE);
26042633
WLOG(WS_LOG_SFTP, "Error opening file %s", dir);
26052634
res = oer;
2606-
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
2635+
if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res,
26072636
"English", NULL, &outSz) != WS_SIZE_ONLY) {
26082637
ret = WS_FATAL_ERROR;
26092638
goto cleanup;
@@ -2651,7 +2680,7 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
26512680
}
26522681
}
26532682
else {
2654-
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
2683+
if (wolfSSH_SFTP_CreateStatus(ssh, type, reqId, res,
26552684
"English", out, &outSz) != WS_SUCCESS) {
26562685
ret = WS_FATAL_ERROR;
26572686
goto cleanup;
@@ -2962,9 +2991,10 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
29622991
}
29632992

29642993
if (WOPENDIR(ssh->fs, ssh->ctx->heap, &ctx, dir) != 0) {
2994+
byte type = SFTP_FailStatus(WOLFSSH_FTP_NOFILE);
2995+
29652996
WLOG(WS_LOG_SFTP, "Error with opening directory: %s", dir);
2966-
rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_NOFILE, reqId,
2967-
"Unable To Open Directory");
2997+
rc = SFTP_SendStatus(ssh, type, reqId, "Unable To Open Directory");
29682998
return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc;
29692999
}
29703000

@@ -4981,6 +5011,7 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
49815011
if (WS_DeleteFileA(name, ssh->ctx->heap) == 0)
49825012
#endif /* USE_WINDOWS_API */
49835013
{
5014+
type = SFTP_FailStatus(WOLFSSH_FTP_FAILURE);
49845015
WLOG(WS_LOG_SFTP, "Error removing file");
49855016
#if defined(WOLFSSL_NUCLEUS) && defined(DEBUG_WOLFSSH)
49865017
if (ret == NUF_ACCES)
@@ -5007,7 +5038,9 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
50075038
}
50085039
else if (ret != WS_SUCCESS) {
50095040
res = err;
5010-
type = WOLFSSH_FTP_FAILURE;
5041+
if (type == WOLFSSH_FTP_OK) {
5042+
type = WOLFSSH_FTP_FAILURE;
5043+
}
50115044
}
50125045

50135046
/* keep the operation result on success; on a send failure propagate it so
@@ -5073,6 +5106,7 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
50735106
if (WS_MoveFileA(old, name, ssh->ctx->heap) == 0)
50745107
#endif /* USE_WINDOWS_API */
50755108
{
5109+
type = SFTP_FailStatus(WOLFSSH_FTP_FAILURE);
50765110
WLOG(WS_LOG_SFTP, "Error renaming file");
50775111
ret = WS_BAD_FILE_E;
50785112
}
@@ -5085,7 +5119,9 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
50855119
ret = WS_BAD_FILE_E;
50865120
}
50875121
else if (ret != WS_SUCCESS) {
5088-
type = WOLFSSH_FTP_FAILURE;
5122+
if (type == WOLFSSH_FTP_OK) {
5123+
type = WOLFSSH_FTP_FAILURE;
5124+
}
50895125
res = err;
50905126
}
50915127

@@ -6030,8 +6066,9 @@ int wolfSSH_SFTP_RecvSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
60306066
WMEMSET((byte*)&atr, 0, sizeof(WS_SFTP_FILEATRB));
60316067
if (SFTP_GetAttributes(ssh->fs, name, &atr, 0, ssh->ctx->heap)
60326068
!= WS_SUCCESS) {
6069+
statusType = SFTP_FailStatus(WOLFSSH_FTP_FAILURE);
60336070
WLOG(WS_LOG_SFTP, "Unable to get stat of file/directory");
6034-
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId,
6071+
if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId,
60356072
ser, "English", NULL, &outSz) != WS_SIZE_ONLY) {
60366073
return WS_FATAL_ERROR;
60376074
}
@@ -6124,8 +6161,9 @@ int wolfSSH_SFTP_RecvLSTAT(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
61246161
if ((ret = SFTP_GetAttributes(ssh->fs, name, &atr, 1, ssh->ctx->heap))
61256162
!= WS_SUCCESS) {
61266163
/* tell peer that was not ok */
6164+
statusType = SFTP_FailStatus(WOLFSSH_FTP_FAILURE);
61276165
WLOG(WS_LOG_SFTP, "Unable to get lstat of file/directory");
6128-
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId,
6166+
if (wolfSSH_SFTP_CreateStatus(ssh, statusType, reqId,
61296167
ser, "English", NULL, &outSz) != WS_SIZE_ONLY) {
61306168
return WS_FATAL_ERROR;
61316169
}

‎tests/regress.c‎

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17459,6 +17459,158 @@ static void TestSftpOpenDirAllocFailure(void)
1745917459
#endif /* !WOLFSSL_STATIC_MEMORY && !WOLFSSL_DEBUG_MEMORY */
1746017460
#endif /* NO_WOLFSSH_DIR */
1746117461

17462+
#ifdef WOLFSSH_SFTP_ERRNO
17463+
/* FXP_REMOVE, FXP_STAT, FXP_LSTAT or FXP_OPENDIR path, by type. */
17464+
static int SftpPathRequest(WOLFSSH* ssh, byte type, int reqId,
17465+
const char* path)
17466+
{
17467+
byte pkt[WOLFSSH_MAX_FILENAME + UINT32_SZ];
17468+
word32 idx = 0;
17469+
word32 pathSz = (word32)WSTRLEN(path);
17470+
17471+
SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ;
17472+
WMEMCPY(pkt + idx, path, pathSz); idx += pathSz;
17473+
17474+
switch (type) {
17475+
case WOLFSSH_FTP_REMOVE:
17476+
return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx);
17477+
case WOLFSSH_FTP_STAT:
17478+
return wolfSSH_SFTP_RecvSTAT(ssh, reqId, pkt, idx);
17479+
case WOLFSSH_FTP_LSTAT:
17480+
return wolfSSH_SFTP_RecvLSTAT(ssh, reqId, pkt, idx);
17481+
#ifndef NO_WOLFSSH_DIR
17482+
case WOLFSSH_FTP_OPENDIR:
17483+
return wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx);
17484+
#endif
17485+
default:
17486+
return WS_BAD_ARGUMENT;
17487+
}
17488+
}
17489+
17490+
/* FXP_RENAME from to. */
17491+
static int SftpRenamePath(WOLFSSH* ssh, int reqId, const char* from,
17492+
const char* to)
17493+
{
17494+
byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)];
17495+
word32 idx = 0;
17496+
word32 sz;
17497+
17498+
sz = (word32)WSTRLEN(from);
17499+
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
17500+
WMEMCPY(pkt + idx, from, sz); idx += sz;
17501+
sz = (word32)WSTRLEN(to);
17502+
SftpPutU32(sz, pkt + idx); idx += UINT32_SZ;
17503+
WMEMCPY(pkt + idx, to, sz); idx += sz;
17504+
17505+
return wolfSSH_SFTP_RecvRename(ssh, reqId, pkt, idx);
17506+
}
17507+
17508+
/* A request naming a file that does not exist answers NO_SUCH_FILE, one the
17509+
* server may not reach answers PERMISSION_DENIED, and anything else stays
17510+
* FAILURE (draft-ietf-secsh-filexfer-02 Section 7). The cause comes from
17511+
* errno, so this runs on the POSIX file system only. */
17512+
static void TestSftpMissingFileStatus(void)
17513+
{
17514+
static const byte pathTypes[] = {
17515+
WOLFSSH_FTP_STAT, WOLFSSH_FTP_LSTAT, WOLFSSH_FTP_REMOVE,
17516+
#ifndef NO_WOLFSSH_DIR
17517+
WOLFSSH_FTP_OPENDIR,
17518+
#endif
17519+
};
17520+
WOLFSSH_CTX* ctx;
17521+
WOLFSSH* ssh;
17522+
int rid = 1000;
17523+
int reqId;
17524+
word32 i;
17525+
byte handle[WOLFSSH_HANDLE_ID_SZ];
17526+
char cwd[WOLFSSH_MAX_FILENAME];
17527+
char dir[64];
17528+
char missing[96];
17529+
char locked[96];
17530+
char present[96];
17531+
char inLocked[128];
17532+
FILE* f;
17533+
17534+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
17535+
AssertNotNull(ctx);
17536+
ssh = wolfSSH_new(ctx);
17537+
AssertNotNull(ssh);
17538+
AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS);
17539+
17540+
WMEMSET(cwd, 0, sizeof(cwd));
17541+
AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1));
17542+
AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS);
17543+
AssertIntEQ(wolfSSH_SFTP_SetConfinePath(ssh, cwd), WS_SUCCESS);
17544+
17545+
WSNPRINTF(dir, sizeof(dir), "wolfssh_nofile_%d", (int)getpid());
17546+
WSNPRINTF(missing, sizeof(missing), "%s/missing", dir);
17547+
WSNPRINTF(present, sizeof(present), "%s/present", dir);
17548+
WSNPRINTF(locked, sizeof(locked), "%s/locked", dir);
17549+
WSNPRINTF(inLocked, sizeof(inLocked), "%s/locked/x", dir);
17550+
AssertIntEQ(mkdir(dir, 0700), 0);
17551+
AssertIntEQ(mkdir(locked, 0700), 0);
17552+
f = fopen(present, "wb");
17553+
AssertNotNull(f);
17554+
fclose(f);
17555+
17556+
/* missing file */
17557+
for (i = 0; i < sizeof(pathTypes); i++) {
17558+
reqId = rid++;
17559+
AssertTrue(SftpPathRequest(ssh, pathTypes[i], reqId, missing)
17560+
!= WS_SUCCESS);
17561+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_NOFILE);
17562+
}
17563+
reqId = rid++;
17564+
AssertTrue(SftpOpenPath(ssh, reqId, missing, WOLFSSH_FXF_READ, handle)
17565+
!= WS_SUCCESS);
17566+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_NOFILE);
17567+
reqId = rid++;
17568+
AssertTrue(SftpRenamePath(ssh, reqId, missing, present) != WS_SUCCESS);
17569+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_NOFILE);
17570+
17571+
#ifndef NO_WOLFSSH_DIR
17572+
/* ENOTDIR counts as missing, as in OpenSSH's sftp-server */
17573+
reqId = rid++;
17574+
AssertTrue(SftpPathRequest(ssh, WOLFSSH_FTP_OPENDIR, reqId, present)
17575+
!= WS_SUCCESS);
17576+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_NOFILE);
17577+
#endif
17578+
/* present but not removable, a non-empty directory: FAILURE */
17579+
reqId = rid++;
17580+
AssertTrue(SftpPathRequest(ssh, WOLFSSH_FTP_REMOVE, reqId, dir)
17581+
!= WS_SUCCESS);
17582+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_FAILURE);
17583+
17584+
/* a search-permission failure on the way: PERMISSION_DENIED. Root
17585+
* passes the permission check, so only an unprivileged run sees it. */
17586+
AssertIntEQ(chmod(locked, 0), 0);
17587+
if (geteuid() != 0) {
17588+
for (i = 0; i < sizeof(pathTypes); i++) {
17589+
reqId = rid++;
17590+
AssertTrue(SftpPathRequest(ssh, pathTypes[i], reqId, inLocked)
17591+
!= WS_SUCCESS);
17592+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_PERMISSION);
17593+
}
17594+
reqId = rid++;
17595+
AssertTrue(SftpOpenPath(ssh, reqId, inLocked, WOLFSSH_FXF_READ,
17596+
handle) != WS_SUCCESS);
17597+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_PERMISSION);
17598+
reqId = rid++;
17599+
AssertTrue(SftpRenamePath(ssh, reqId, inLocked, present)
17600+
!= WS_SUCCESS);
17601+
AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_PERMISSION);
17602+
}
17603+
AssertIntEQ(chmod(locked, 0700), 0);
17604+
17605+
(void)remove(present);
17606+
(void)rmdir(locked);
17607+
(void)rmdir(dir);
17608+
wolfSSH_SFTP_TestRecvStateFree(ssh);
17609+
wolfSSH_free(ssh);
17610+
wolfSSH_CTX_free(ctx);
17611+
}
17612+
#endif /* WOLFSSH_SFTP_ERRNO */
17613+
1746217614
/* A failed close() must still drop the handle from the session tracking list;
1746317615
* otherwise the stale descriptor lingers and is closed a second time when the
1746417616
* session is torn down. Open a file, invalidate its descriptor out of band so
@@ -21712,6 +21864,10 @@ int main(int argc, char** argv)
2171221864
TestSftpOpenDirAllocFailure();
2171321865
#endif
2171421866
#endif
21867+
#ifdef WOLFSSH_SFTP_ERRNO
21868+
/* a missing file answers NO_SUCH_FILE, a blocked one PERMISSION_DENIED */
21869+
TestSftpMissingFileStatus();
21870+
#endif
2171521871
/* a failed close still drops the handle from the tracking list */
2171621872
TestSftpCloseFailureRemovesHandle();
2171721873
/* confinement follows the confine root, not the start path */

0 commit comments

Comments
 (0)