diff --git a/configure.ac b/configure.ac index 9100e4097..c5c5e5212 100644 --- a/configure.ac +++ b/configure.ac @@ -199,6 +199,15 @@ AC_LINK_IFELSE( AC_MSG_RESULT([yes])], [AC_MSG_RESULT([no])]) +AC_MSG_CHECKING([for linkat]) +AC_LINK_IFELSE( + [AC_LANG_PROGRAM([[#include +#include ]], + [[(void)linkat(AT_FDCWD, "x", AT_FDCWD, "y", 0);]])], + [AC_DEFINE([HAVE_LINKAT], [1], [linkat() available and declared]) + AC_MSG_RESULT([yes])], + [AC_MSG_RESULT([no])]) + AC_CHECK_DECLS([[pread],[pwrite]],,[unistd.h]) # DEBUG diff --git a/src/wolfsftp.c b/src/wolfsftp.c index a5659c457..2e260f93d 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -4981,6 +4981,9 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) char name[WOLFSSH_MAX_FILENAME]; word32 idx = 0; int ret = WS_SUCCESS; +#ifndef USE_WINDOWS_API + WS_SFTP_FILEATRB atr; +#endif byte type = WOLFSSH_FTP_OK; int rc; @@ -5006,7 +5009,20 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) if (ret == WS_SUCCESS) { #ifndef USE_WINDOWS_API - if (WREMOVE(ssh->fs, name) < 0) + /* REMOVE never takes a directory (draft-ietf-secsh-filexfer-02 6.5) */ + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, name, &atr, 1, ssh->ctx->heap) + == WS_SUCCESS + && (atr.per & FILEATRB_PER_MASK_TYPE) == FILEATRB_PER_DIR) { + WLOG(WS_LOG_SFTP, "Not removing a directory"); + ret = WS_BAD_FILE_E; + } + #ifdef WUNLINK + /* unlink() refuses a directory swapped in after the check */ + else if (WUNLINK(ssh->fs, name) < 0) + #else + else if (WREMOVE(ssh->fs, name) < 0) + #endif #else /* USE_WINDOWS_API */ if (WS_DeleteFileA(name, ssh->ctx->heap) == 0) #endif /* USE_WINDOWS_API */ @@ -5053,6 +5069,65 @@ int wolfSSH_SFTP_RecvRemove(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) } +#ifndef USE_WINDOWS_API +/* Rename without replacing an existing name (draft-ietf-secsh-filexfer-02 + * 6.5). linkat() refuses one atomically; other types, builds without linkat(), + * and filesystems without hard links check first and race a concurrent + * create. + * + * returns WS_SUCCESS on success + */ +static int SFTP_RenameNoReplace(WOLFSSH* ssh, char* old, char* name) +{ + WS_SFTP_FILEATRB atr; + + /* a missing or unreachable oldpath is reported first, with its errno */ + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, old, &atr, 1, ssh->ctx->heap) + != WS_SUCCESS) { + return WS_BAD_FILE_E; + } + +#ifdef WLINK + if ((atr.per & FILEATRB_PER_MASK_TYPE) == FILEATRB_PER_FILE) { + if (WLINK(ssh->fs, old, name) == 0) { + int err; + + if (WUNLINK(ssh->fs, old) == 0) { + return WS_SUCCESS; + } + /* undo the link, keeping the unlink errno */ + err = errno; + (void)WUNLINK(ssh->fs, name); + errno = err; + return WS_BAD_FILE_E; + } + if (errno == EEXIST) { + WLOG(WS_LOG_SFTP, "Rename target exists"); + errno = EEXIST; + return WS_BAD_FILE_E; + } + } +#endif + + WMEMSET(&atr, 0, sizeof(atr)); + if (SFTP_GetAttributes(ssh->fs, name, &atr, 1, ssh->ctx->heap) + == WS_SUCCESS) { + WLOG(WS_LOG_SFTP, "Rename target exists"); + #ifdef WOLFSSH_SFTP_ERRNO + /* the reply status comes from errno, which the lstat left stale */ + errno = EEXIST; + #endif + return WS_BAD_FILE_E; + } + if (WRENAME(ssh->fs, old, name) < 0) { + return WS_BAD_FILE_E; + } + return WS_SUCCESS; +} +#endif /* USE_WINDOWS_API */ + + /* Handles packet to rename a file * * returns WS_SUCCESS on success @@ -5101,8 +5176,9 @@ int wolfSSH_SFTP_RecvRename(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz) if (ret == WS_SUCCESS) { #ifndef USE_WINDOWS_API - if (WRENAME(ssh->fs, old, name) < 0) + if (SFTP_RenameNoReplace(ssh, old, name) != WS_SUCCESS) #else /* USE_WINDOWS_API */ + /* MoveFile never replaces */ if (WS_MoveFileA(old, name, ssh->ctx->heap) == 0) #endif /* USE_WINDOWS_API */ { @@ -5218,8 +5294,10 @@ static int SFTP_GetAttributes(void* fs, const char* fileName, return WS_SUCCESS; } - /* handle case of "/" */ - if (sz < 3 && fileName[0] == WS_DELIM && ret == NUF_NOFILE) { + /* handle case of "/" or "/.", not a root-level name such as "/a" */ + if (fileName[0] == WS_DELIM + && (sz == 1 || (sz == 2 && fileName[1] == '.')) + && ret == NUF_NOFILE) { atr->flags |= WOLFSSH_FILEATRB_PERM; atr->per |= 040000; return WS_SUCCESS; @@ -5603,8 +5681,10 @@ static int SFTP_GetAttributes(void* fs, const char* fileName, } /* handle case of "/" */ - /* Calling f_stat for "/" returns FR_INVALID_NAME. So we simulate the result. */ - if (sz < 3 && fileName[0] == WS_DELIM) { + /* Calling f_stat for "/" returns FR_INVALID_NAME. So we simulate the result. + * Only "/" and "/." though; "/a" is a root-level name for f_stat. */ + if (fileName[0] == WS_DELIM + && (sz == 1 || (sz == 2 && fileName[1] == '.'))) { atr->flags |= WOLFSSH_FILEATRB_PERM; atr->per |= 040000; return WS_SUCCESS; diff --git a/tests/regress.c b/tests/regress.c index 658405cfb..4f7594604 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -16395,6 +16395,207 @@ static void TestSftpAppendWritesAtEof(void) wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); } + +/* FXP_REMOVE, FXP_STAT, FXP_LSTAT or FXP_OPENDIR path, by type. */ +static int SftpPathRequest(WOLFSSH* ssh, byte type, int reqId, + const char* path) +{ + byte pkt[WOLFSSH_MAX_FILENAME + UINT32_SZ]; + word32 idx = 0; + word32 pathSz = (word32)WSTRLEN(path); + + SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; + + switch (type) { + case WOLFSSH_FTP_REMOVE: + return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx); + case WOLFSSH_FTP_STAT: + return wolfSSH_SFTP_RecvSTAT(ssh, reqId, pkt, idx); + case WOLFSSH_FTP_LSTAT: + return wolfSSH_SFTP_RecvLSTAT(ssh, reqId, pkt, idx); + #ifndef NO_WOLFSSH_DIR + case WOLFSSH_FTP_OPENDIR: + return wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx); + #endif + default: + return WS_BAD_ARGUMENT; + } +} + +/* FXP_RENAME from to. */ +static int SftpRenamePath(WOLFSSH* ssh, int reqId, const char* from, + const char* to) +{ + byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)]; + word32 idx = 0; + word32 sz; + + sz = (word32)WSTRLEN(from); + SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, from, sz); idx += sz; + sz = (word32)WSTRLEN(to); + SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; + WMEMCPY(pkt + idx, to, sz); idx += sz; + + return wolfSSH_SFTP_RecvRename(ssh, reqId, pkt, idx); +} + +static void SftpWriteTestFile(const char* path, const char* content) +{ + WFILE* file; + + AssertIntEQ(WFOPEN(NULL, &file, path, "wb"), 0); + AssertTrue(file != WBADFILE); + AssertIntEQ((int)WFWRITE(NULL, content, 1, WSTRLEN(content), file), + (int)WSTRLEN(content)); + WFCLOSE(NULL, file); +} + +/* REMOVE refuses a directory, RENAME an existing newpath + * (draft-ietf-secsh-filexfer-02 6.5). */ +static void TestSftpRemoveRenameSemantics(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + int rid = 700; + WSTAT_T st; + char cwd[WOLFSSH_MAX_FILENAME]; + char fileA[64], fileB[64], fileC[64]; + char dirA[64], dirB[64], dirC[64]; +#if defined(WOLFSSH_HAVE_SYMLINK) && !defined(USE_WINDOWS_API) + char lnk[64]; +#endif +#ifndef USE_WINDOWS_API + char roDir[64], roFile[128], roDest[128]; + /* the refused unlink or rename reports why */ +#ifdef WOLFSSH_SFTP_ERRNO + word32 roStatus = WOLFSSH_FTP_PERMISSION; +#else + word32 roStatus = WOLFSSH_FTP_FAILURE; +#endif +#endif + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS); + + WSNPRINTF(fileA, sizeof(fileA), "wolfssh_rm_a_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(fileB, sizeof(fileB), "wolfssh_rm_b_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(fileC, sizeof(fileC), "wolfssh_rm_c_%lu.tmp", SFTP_TEST_PID()); + WSNPRINTF(dirA, sizeof(dirA), "wolfssh_rm_da_%lu", SFTP_TEST_PID()); + WSNPRINTF(dirB, sizeof(dirB), "wolfssh_rm_db_%lu", SFTP_TEST_PID()); + WSNPRINTF(dirC, sizeof(dirC), "wolfssh_rm_dc_%lu", SFTP_TEST_PID()); + WMEMSET(cwd, 0, sizeof(cwd)); + AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1)); + AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS); + + SftpWriteTestFile(fileA, "aaaa"); + SftpWriteTestFile(fileB, "bb"); + (void)WREMOVE(ssh->fs, fileC); + AssertIntEQ(WMKDIR(ssh->fs, dirA, 0755), 0); + AssertIntEQ(WMKDIR(ssh->fs, dirB, 0755), 0); + (void)WRMDIR(ssh->fs, dirC); + + /* REMOVE refuses an empty directory */ + AssertIntEQ(SftpPathRequest(ssh, WOLFSSH_FTP_REMOVE, rid, dirA), + WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WSTAT(ssh->fs, dirA, &st), 0); + + /* RENAME refuses an existing file; neither changes */ + AssertIntEQ(SftpRenamePath(ssh, rid, fileA, fileB), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertSftpFileContents(fileA, "aaaa", 4); + AssertSftpFileContents(fileB, "bb", 2); + + /* RENAME refuses an existing empty directory, as FAILURE whatever an + * earlier call left in errno */ +#ifdef WOLFSSH_SFTP_ERRNO + errno = ENOENT; +#endif + AssertIntEQ(SftpRenamePath(ssh, rid, dirA, dirB), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WSTAT(ssh->fs, dirA, &st), 0); + AssertIntEQ(WSTAT(ssh->fs, dirB, &st), 0); + +#if defined(WOLFSSH_HAVE_SYMLINK) && !defined(USE_WINDOWS_API) + /* a dangling symlink is an existing newpath too */ + WSNPRINTF(lnk, sizeof(lnk), "wolfssh_rm_ln_%lu", SFTP_TEST_PID()); + (void)WREMOVE(ssh->fs, lnk); + AssertIntEQ(symlink(fileC, lnk), 0); + AssertIntEQ(SftpRenamePath(ssh, rid, fileA, lnk), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_FAILURE); + AssertIntEQ(WLSTAT(ssh->fs, lnk, &st), 0); + AssertTrue(S_ISLNK(st.st_mode)); + AssertSftpFileContents(fileA, "aaaa", 4); + /* REMOVE takes the link, not its target */ + AssertIntEQ(SftpPathRequest(ssh, WOLFSSH_FTP_REMOVE, rid, lnk), + WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WLSTAT(ssh->fs, lnk, &st) != 0); + /* a link to a directory is not a directory to REMOVE */ + AssertIntEQ(symlink(dirB, lnk), 0); + AssertIntEQ(SftpPathRequest(ssh, WOLFSSH_FTP_REMOVE, rid, lnk), + WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WLSTAT(ssh->fs, lnk, &st) != 0); + AssertIntEQ(WSTAT(ssh->fs, dirB, &st), 0); +#endif + +#ifndef USE_WINDOWS_API + /* a failed move leaves both names as they were: out of a read-only + * directory takes the link rollback, into one the link fallback */ + if (geteuid() != 0) { + WSNPRINTF(roDir, sizeof(roDir), "wolfssh_rm_ro_%lu", + SFTP_TEST_PID()); + WSNPRINTF(roFile, sizeof(roFile), "%s/%s", roDir, fileC); + WSNPRINTF(roDest, sizeof(roDest), "%s/%s", roDir, fileB); + AssertIntEQ(WMKDIR(ssh->fs, roDir, 0755), 0); + SftpWriteTestFile(roFile, "cc"); + AssertIntEQ(chmod(roDir, 0555), 0); + + AssertIntEQ(SftpRenamePath(ssh, rid, roFile, fileC), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, roStatus); + AssertSftpFileContents(roFile, "cc", 2); + AssertTrue(WLSTAT(ssh->fs, fileC, &st) != 0); + + AssertIntEQ(SftpRenamePath(ssh, rid, fileB, roDest), WS_BAD_FILE_E); + AssertSftpStatusReply(ssh, rid++, roStatus); + AssertSftpFileContents(fileB, "bb", 2); + AssertTrue(WLSTAT(ssh->fs, roDest, &st) != 0); + + AssertIntEQ(chmod(roDir, 0755), 0); + AssertIntEQ(WREMOVE(ssh->fs, roFile), 0); + AssertIntEQ(WRMDIR(ssh->fs, roDir), 0); + } +#endif + + /* RENAME to a free name moves a file and a directory */ + AssertIntEQ(SftpRenamePath(ssh, rid, fileA, fileC), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, fileA, &st) != 0); + AssertSftpFileContents(fileC, "aaaa", 4); + AssertIntEQ(SftpRenamePath(ssh, rid, dirA, dirC), WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, dirA, &st) != 0); + AssertIntEQ(WSTAT(ssh->fs, dirC, &st), 0); + + /* REMOVE still removes a file */ + AssertIntEQ(SftpPathRequest(ssh, WOLFSSH_FTP_REMOVE, rid, fileC), + WS_SUCCESS); + AssertSftpStatusReply(ssh, rid++, WOLFSSH_FTP_OK); + AssertTrue(WSTAT(ssh->fs, fileC, &st) != 0); + + (void)WREMOVE(ssh->fs, fileB); + (void)WRMDIR(ssh->fs, dirB); + (void)WRMDIR(ssh->fs, dirC); + wolfSSH_SFTP_TestRecvStateFree(ssh); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} #endif /* !NO_WOLFSSH_SERVER && !NO_FILESYSTEM */ #if !defined(NO_WOLFSSH_SERVER) && !defined(USE_WINDOWS_API) && \ @@ -17460,51 +17661,6 @@ static void TestSftpOpenDirAllocFailure(void) #endif /* NO_WOLFSSH_DIR */ #ifdef WOLFSSH_SFTP_ERRNO -/* FXP_REMOVE, FXP_STAT, FXP_LSTAT or FXP_OPENDIR path, by type. */ -static int SftpPathRequest(WOLFSSH* ssh, byte type, int reqId, - const char* path) -{ - byte pkt[WOLFSSH_MAX_FILENAME + UINT32_SZ]; - word32 idx = 0; - word32 pathSz = (word32)WSTRLEN(path); - - SftpPutU32(pathSz, pkt + idx); idx += UINT32_SZ; - WMEMCPY(pkt + idx, path, pathSz); idx += pathSz; - - switch (type) { - case WOLFSSH_FTP_REMOVE: - return wolfSSH_SFTP_RecvRemove(ssh, reqId, pkt, idx); - case WOLFSSH_FTP_STAT: - return wolfSSH_SFTP_RecvSTAT(ssh, reqId, pkt, idx); - case WOLFSSH_FTP_LSTAT: - return wolfSSH_SFTP_RecvLSTAT(ssh, reqId, pkt, idx); - #ifndef NO_WOLFSSH_DIR - case WOLFSSH_FTP_OPENDIR: - return wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx); - #endif - default: - return WS_BAD_ARGUMENT; - } -} - -/* FXP_RENAME from to. */ -static int SftpRenamePath(WOLFSSH* ssh, int reqId, const char* from, - const char* to) -{ - byte pkt[2 * (WOLFSSH_MAX_FILENAME + UINT32_SZ)]; - word32 idx = 0; - word32 sz; - - sz = (word32)WSTRLEN(from); - SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; - WMEMCPY(pkt + idx, from, sz); idx += sz; - sz = (word32)WSTRLEN(to); - SftpPutU32(sz, pkt + idx); idx += UINT32_SZ; - WMEMCPY(pkt + idx, to, sz); idx += sz; - - return wolfSSH_SFTP_RecvRename(ssh, reqId, pkt, idx); -} - /* A request naming a file that does not exist answers NO_SUCH_FILE, one the * server may not reach answers PERMISSION_DENIED, and anything else stays * FAILURE (draft-ietf-secsh-filexfer-02 Section 7). The cause comes from @@ -21878,6 +22034,8 @@ int main(int argc, char** argv) #if !defined(NO_WOLFSSH_SERVER) && !defined(NO_FILESYSTEM) /* FXF_APPEND writes land at EOF regardless of the request offset */ TestSftpAppendWritesAtEof(); + /* REMOVE refuses directories, RENAME refuses an existing newpath */ + TestSftpRemoveRenameSemantics(); #endif #if !defined(NO_WOLFSSH_SERVER) && defined(USE_WINDOWS_API) && \ !defined(NO_FILESYSTEM) diff --git a/wolfssh/port.h b/wolfssh/port.h index 4e1464141..85d4cb445 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -1613,7 +1613,7 @@ extern "C" { #else - #include /* used for rmdir */ + #include /* used for rmdir, linkat, and unlink */ #include /* used for mkdir, stat, and lstat */ #include /* used for remove and rename */ #include /* used for why a call failed */ @@ -1626,6 +1626,15 @@ extern "C" { #define WSTAT(fs,p,b) stat((p),(b)) #ifndef USE_OSE_API #define WLSTAT(fs,p,b) lstat((p),(b)) + #define WUNLINK(fs,p) unlink((p)) + #ifdef HAVE_LINKAT + /* SFTP no-replace rename; it reads errno. linkat() without + * AT_SYMLINK_FOLLOW links a symlink itself, where link() may + * follow it. */ + #include + #include + #define WLINK(fs,o,n) linkat(AT_FDCWD, (o), AT_FDCWD, (n), 0) + #endif #else #define WLSTAT(fs,p,b) stat((p),(b)) #endif