From 75b4dc15efa339795ab956eae55f09f7f3c9f426 Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Tue, 16 Jun 2026 12:08:12 +0900 Subject: [PATCH] Bind SCP file timestamps to open descriptor --- configure.ac | 2 +- src/wolfscp.c | 74 ++++++++++++++++++++---- tests/unit.c | 153 +++++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/port.h | 20 +++++++ 4 files changed, 238 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index 4aab10756..ea4fa4902 100644 --- a/configure.ac +++ b/configure.ac @@ -87,7 +87,7 @@ AC_ARG_WITH(wolfssl, ) AC_CHECK_LIB([wolfssl],[wolfCrypt_Init],,[AC_MSG_ERROR([libwolfssl is required for ${PACKAGE}. It can be obtained from https://www.wolfssl.com/download.html/ .])]) -AC_CHECK_FUNCS([gethostbyname getaddrinfo gettimeofday inet_ntoa memset socket wc_ecc_set_rng]) +AC_CHECK_FUNCS([gethostbyname getaddrinfo gettimeofday inet_ntoa memset socket wc_ecc_set_rng futimens]) AC_CHECK_DECLS([[pread],[pwrite]],,[unistd.h]) # DEBUG diff --git a/src/wolfscp.c b/src/wolfscp.c index 62591596e..5e25462a5 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -1857,14 +1857,34 @@ static INLINE int wolfSSH_LastError(void) } +/* WOLFSSH_SCP_FD_UTIMES is defined for platforms that can set file timestamps + * on the still-open descriptor, binding the update to the inode. On those the + * timestamp is applied before the file is closed. Other platforms fall back to + * applying the timestamp by path after the file is closed. */ +#if defined(USE_WINDOWS_API) || defined(WFUTIMES) + #define WOLFSSH_SCP_FD_UTIMES +#endif + /* set file access and modification times + * + * On descriptor-capable platforms (fp != NULL and WOLFSSH_SCP_FD_UTIMES) the + * timestamps are applied to the open descriptor so the update is bound to the + * inode. This avoids a race where the path could be replaced by a symlink + * between closing the file and a path-based time update, which would let a + * peer-supplied timestamp be applied to an arbitrary target. Buffered file + * data is flushed first so the later close does not write and overwrite the + * modification time. On the POSIX path-based fallback, fp is NULL because the + * file has already been closed and the timestamp is applied by path. The + * Windows branch always requires an open fp and rejects fp == NULL with + * WS_BAD_ARGUMENT (there is no path-based fallback there). + * * Returns WS_SUCCESS on success, or negative upon error */ -static int SetTimestampInfo(const char* fileName, word64 mTime, word64 aTime) +static int SetTimestampInfo(WFILE* fp, const char* fileName, + word64 mTime, word64 aTime) { int ret = WS_SUCCESS; #ifdef USE_WINDOWS_API struct _utimbuf tmp; - int fd; #else struct timeval tmp[2]; #endif @@ -1874,18 +1894,33 @@ static int SetTimestampInfo(const char* fileName, word64 mTime, word64 aTime) if (ret == WS_SUCCESS) { #ifdef USE_WINDOWS_API - tmp.actime = aTime; - tmp.modtime = mTime; - _sopen_s(&fd, fileName, _O_RDWR, _SH_DENYNO, 0); - _futime(fd, &tmp); - _close(fd); + if (fp == NULL) { + ret = WS_BAD_ARGUMENT; + } + else { + tmp.actime = aTime; + tmp.modtime = mTime; + if (WFFLUSH(fp) != 0 || _futime(_fileno(fp), &tmp) != 0) + ret = WS_FATAL_ERROR; + } #else tmp[0].tv_sec = (time_t)aTime; tmp[0].tv_usec = 0; tmp[1].tv_sec = (time_t)mTime; tmp[1].tv_usec = 0; - ret = WUTIMES(fileName, tmp); + #ifdef WFUTIMES + if (fp != NULL) { + if (WFFLUSH(fp) != 0 || WFUTIMES(fileno(fp), tmp) != 0) + ret = WS_FATAL_ERROR; + } + else + #endif + { + (void)fp; + if (WUTIMES(fileName, tmp) != 0) + ret = WS_FATAL_ERROR; + } #endif } @@ -2093,15 +2128,34 @@ int wsScpRecvCallback(WOLFSSH* ssh, int state, const char* basePath, (void)WFFLUSH(fp); (void)fsync(fileno(fp)); flush_bytes = 0; +#endif +#ifdef WOLFSSH_SCP_FD_UTIMES + /* set timestamp info on the open file, before closing, so the + * update is bound to the inode and cannot be redirected to a + * symlink swapped in over the path */ + if (mTime != 0 || aTime != 0) + ret = SetTimestampInfo(fp, fileName, mTime, aTime); #endif WFCLOSE(ssh->fs, fp); fp = NULL; } +#ifdef WOLFSSH_SCP_FD_UTIMES + else if (mTime != 0 || aTime != 0) { + /* a descriptor-based update was required but the file was never + * opened; do not fall back to a path-based update that could + * follow a swapped symlink, fail so the result handling below + * aborts */ + ret = WS_FATAL_ERROR; + } +#endif /* set timestamp info */ if (mTime != 0 || aTime != 0) { - ret = SetTimestampInfo(fileName, mTime, aTime); - +#ifndef WOLFSSH_SCP_FD_UTIMES + /* no descriptor-based update available, set by path now that + * the file is closed so the close cannot overwrite the time */ + ret = SetTimestampInfo(NULL, fileName, mTime, aTime); +#endif if (ret == WS_SUCCESS) { ret = WS_SCP_CONTINUE; } else { diff --git a/tests/unit.c b/tests/unit.c index 882c704ef..4f3533271 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -3932,6 +3932,154 @@ static int test_ScpRecvCallback_NewDirChdirFail(void) return result; } +/* Drive the default SCP receive callback through a full single-file receive + * and confirm the peer-supplied modification/access times end up on the + * written file. + * + * Which code path applies the timestamp is decided at build time, not by this + * test: when futimens is detected (HAVE_FUTIMENS) the callback sets it on the + * open descriptor before the closing flush, otherwise it sets it by path after + * close. Both must yield the peer-supplied times, which is what this end-to-end + * test asserts. When the descriptor path is compiled in, this also guards its + * ordering relative to the closing flush: applying the timestamp before the + * buffered data was flushed would leave the modification time at the current + * time rather than the peer value. */ +static int test_ScpRecvCallback_Timestamp(void) +{ + char tmpDir[] = "/tmp/wolfssh_scptsXXXXXX"; + char filePath[PATH_MAX]; + char origCwd[PATH_MAX]; + const char data[] = "wolfssh scp timestamp regression\n"; + const word64 mTime = 1234567890; /* 2009-02-13 23:31:30 UTC */ + const word64 aTime = 1000000000; /* 2001-09-09 01:46:40 UTC */ + struct stat st; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + char* basePath = NULL; + int origCwdSaved = 0; + int baseReady = 0; + int ret; + int result = 0; + + filePath[0] = '\0'; + + if (getcwd(origCwd, sizeof(origCwd)) == NULL) + return -850; + origCwdSaved = 1; + + if (mkdtemp(tmpDir) == NULL) + return -851; + baseReady = 1; + + basePath = realpath(tmpDir, NULL); + if (basePath == NULL) { + result = -852; + goto cleanup; + } + + ret = snprintf(filePath, sizeof(filePath), "%s/ts_file.txt", basePath); + if (!scpTestSnprintfOk(ret, sizeof(filePath))) { + result = -853; + goto cleanup; + } + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + if (ctx == NULL) { + result = -854; + goto cleanup; + } + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -855; + goto cleanup; + } + + /* enter the destination directory */ + ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_NEW_REQUEST, basePath, + NULL, 0, 0, 0, 0, NULL, 0, 0, NULL); + if (ret != WS_SCP_CONTINUE) { + result = -856; + goto cleanup; + } + + /* open the destination file */ + ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_NEW_FILE, basePath, + "ts_file.txt", 0644, mTime, aTime, sizeof(data) - 1, NULL, 0, 0, + NULL); + if (ret != WS_SCP_CONTINUE) { + result = -857; + goto cleanup; + } + + /* write the file contents */ + ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_FILE_PART, basePath, + "ts_file.txt", 0644, mTime, aTime, sizeof(data) - 1, + (byte*)data, sizeof(data) - 1, 0, wolfSSH_GetScpRecvCtx(ssh)); + if (ret != WS_SCP_CONTINUE) { + result = -858; + goto cleanup; + } + + /* close the file and apply the peer-supplied timestamps */ + ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_FILE_DONE, basePath, + "ts_file.txt", 0644, mTime, aTime, sizeof(data) - 1, NULL, 0, 0, + wolfSSH_GetScpRecvCtx(ssh)); + if (ret != WS_SCP_CONTINUE) { + result = -859; + goto cleanup; + } + + /* the written file must carry the peer-supplied timestamps */ + if (stat(filePath, &st) != 0) { + result = -860; + goto cleanup; + } + if ((word64)st.st_mtime != mTime) { + result = -861; + goto cleanup; + } + if ((word64)st.st_atime != aTime) { + result = -862; + goto cleanup; + } + +#ifdef HAVE_FUTIMENS + /* Descriptor build only: a FILE_DONE carrying timestamps but no open file + * (NULL ctx) must abort rather than silently fall back to a path-based + * update that could follow a swapped symlink. Use distinct times and + * confirm the existing file's modification time is left untouched. */ + ret = wsScpRecvCallback(ssh, WOLFSSH_SCP_FILE_DONE, basePath, + "ts_file.txt", 0644, mTime + 100, aTime + 100, 0, NULL, 0, 0, + NULL); + if (ret != WS_SCP_ABORT) { + result = -864; + goto cleanup; + } + if (stat(filePath, &st) != 0) { + result = -865; + goto cleanup; + } + if ((word64)st.st_mtime != mTime) { + result = -866; + goto cleanup; + } +#endif + +cleanup: + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + if (filePath[0] != '\0') + (void)remove(filePath); + free(basePath); + if (origCwdSaved && chdir(origCwd) != 0 && result == 0) + result = -863; + if (baseReady) + (void)rmdir(tmpDir); + return result; +} + #endif /* WOLFSSH_SCP recv callback depth guard test */ @@ -4552,6 +4700,11 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("ScpRecvCallback_NewDirChdirFail: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_ScpRecvCallback_Timestamp(); + printf("ScpRecvCallback_Timestamp: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif #ifdef WOLFSSH_TEST_CAPTURING_ALLOCATOR diff --git a/wolfssh/port.h b/wolfssh/port.h index e327c12e7..7be23aba0 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -461,6 +461,26 @@ extern "C" { #include #else #define WUTIMES(f,t) utimes((f),(t)) + /* Bind timestamp updates to the open descriptor with futimens() + * (POSIX.1-2008) when the build detected it, immune to a symlink + * swapped in over the path. The SCP code passes a timeval pair + * (matching WUTIMES), so convert it to the timespec pair futimens() + * expects. Falls back to the path-based WUTIMES when futimens() is not + * present. */ + #ifdef HAVE_FUTIMENS + #include + #include + static inline int wFutimes(int fd, const struct timeval t[2]) + { + struct timespec ts[2]; + ts[0].tv_sec = t[0].tv_sec; + ts[0].tv_nsec = (long)t[0].tv_usec * 1000; + ts[1].tv_sec = t[1].tv_sec; + ts[1].tv_nsec = (long)t[1].tv_usec * 1000; + return futimens(fd, ts); + } + #define WFUTIMES(fd,t) wFutimes((fd),(t)) + #endif #endif #ifndef USE_WINDOWS_API