diff --git a/.gitignore b/.gitignore index 23916c9dc..7bc2eaf72 100644 --- a/.gitignore +++ b/.gitignore @@ -85,11 +85,10 @@ keys/*-ossh-*cert.pub random-test.txt random-test-result.txt test.dat -# Scratch file the api tests write in the working directory. Removed on a -# clean run, left behind when one aborts. +# Scratch files the api and regression tests write in the working +# directory. Removed on a clean run, left behind when one aborts. ossh-cert-line.tmp -# Same for the regression tests' known_hosts fixtures, named by pid. -wolfssh_kh_*.tmp +wolfssh_*.tmp regress_known_hosts*.tmp # test output diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 42eca199f..85a98adb5 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -9775,6 +9775,9 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, WS_SFTP_GET_STATE* state = NULL; int sz; int ret = WS_SUCCESS; +#ifdef USE_WINDOWS_API + DWORD creationDisp; +#endif WLOG(WS_LOG_SFTP, "Entering wolfSSH_SFTP_Get()"); if (ssh == NULL || from == NULL || to == NULL) @@ -9853,6 +9856,36 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, /* if resuming then check for saved offset */ if (resume) { wolfSSH_SFTP_GetOfst(ssh, from, to, state->gOfst); + + /* A saved offset is only usable while the remote file + * still holds bytes past it. */ + if ((state->gOfst[0] > 0 || state->gOfst[1] > 0) + && (state->attrib.flags & WOLFSSH_FILEATRB_SIZE) + && (state->attrib.sz[1] < state->gOfst[1] + || (state->attrib.sz[1] == state->gOfst[1] + && state->attrib.sz[0] + <= state->gOfst[0]))) { + WLOG(WS_LOG_SFTP, "Remote file too short"); + state->gOfst[0] = 0; + state->gOfst[1] = 0; + } + + /* a saved offset is only usable if the local file still + * holds exactly that many bytes */ + if (state->gOfst[0] > 0 || state->gOfst[1] > 0) { + WMEMSET(&state->attrib, 0, sizeof(state->attrib)); + if (SFTP_GetAttributes(ssh->fs, to, &state->attrib, 1, + ssh->ctx->heap) != WS_SUCCESS + || (state->attrib.flags + & WOLFSSH_FILEATRB_SIZE) == 0 + || state->attrib.sz[0] != state->gOfst[0] + || state->attrib.sz[1] != state->gOfst[1]) { + WLOG(WS_LOG_SFTP, + "Size does not match, starting over"); + state->gOfst[0] = 0; + state->gOfst[1] = 0; + } + } } state->state = STATE_GET_OPEN_LOCAL; FALL_THROUGH; @@ -9865,20 +9898,23 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, else ret = WFOPEN(ssh->fs, &state->fl, to, WOLFSSH_O_WRONLY); #elif defined(USE_WINDOWS_API) - { - DWORD desiredAccess = GENERIC_WRITE; - if (state->gOfst[0] > 0 || state->gOfst[1] > 0) - desiredAccess |= FILE_APPEND_DATA; - state->fileHandle = WS_CreateFileA(to, desiredAccess, - (FILE_SHARE_DELETE | FILE_SHARE_READ | - FILE_SHARE_WRITE), CREATE_ALWAYS, - FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); - } - if (resume) { - WMEMSET(&state->offset, 0, sizeof(OVERLAPPED)); - state->offset.OffsetHigh = state->gOfst[1]; - state->offset.Offset = state->gOfst[0]; + creationDisp = CREATE_ALWAYS; + if (state->gOfst[0] > 0 || state->gOfst[1] > 0) + creationDisp = OPEN_EXISTING; + state->fileHandle = WS_CreateFileA(to, GENERIC_WRITE, + (FILE_SHARE_DELETE | FILE_SHARE_READ | + FILE_SHARE_WRITE), creationDisp, + FILE_ATTRIBUTE_NORMAL, ssh->ctx->heap); + if (state->fileHandle == INVALID_HANDLE_VALUE) { + WLOG(WS_LOG_SFTP, "Unable to open output file"); + ssh->error = WS_BAD_FILE_E; + ret = WS_FATAL_ERROR; + state->state = STATE_GET_CLEANUP; + continue; } + WMEMSET(&state->offset, 0, sizeof(OVERLAPPED)); + state->offset.OffsetHigh = state->gOfst[1]; + state->offset.Offset = state->gOfst[0]; #else if (state->gOfst[0] > 0 || state->gOfst[1] > 0) ret = WFOPEN(ssh->fs, &state->fl, to, "ab"); diff --git a/tests/api.c b/tests/api.c index 7f9cc47ca..b23df300d 100644 --- a/tests/api.c +++ b/tests/api.c @@ -5901,6 +5901,24 @@ static int sftpPutToCompletion(WOLFSSH* ssh, char* from, char* to, byte resume) return ret; } + + +/* Drives one wolfSSH_SFTP_Get() to a terminal result. */ +static int sftpGetToCompletion(WOLFSSH* ssh, char* from, char* to, byte resume) +{ + int ret = WS_FATAL_ERROR; + int tries; + + for (tries = 0; tries < SFTP_MAX_RETRY_TRIES; tries++) { + ret = wolfSSH_SFTP_Get(ssh, from, to, resume, NULL); + if (ret == WS_SUCCESS || + !sftp_error_is_notice(wolfSSH_get_error(ssh))) { + break; + } + } + + return ret; +} #endif /* !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM && !WOLFSSH_ZEPHYR */ @@ -6031,9 +6049,141 @@ static void test_wolfSSH_SFTP_PutResume(void) wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); -#ifdef WOLFSSH_ZEPHYR - k_sleep(Z_TIMEOUT_TICKS(100)); + ThreadJoin(serThread); + FreeTcpReady(&ready); +#endif /* !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM && !WOLFSSH_ZEPHYR */ +} + + +/* A resumed get must keep the bytes already at the local destination, and + * must only resume onto one whose size matches the saved offset. The + * echoserver shares this process, so the "remote" file is local. */ +static void test_wolfSSH_SFTP_GetResume(void) +{ +/* staging both files needs the hosted fopen()/getcwd() wrappers */ +#if !defined(NO_FILESYSTEM) && !defined(WOLFSSH_USER_FILESYSTEM) && \ + !defined(WOLFSSH_ZEPHYR) + func_args ser; + tcp_ready ready; + int argsCount; + WS_SOCKET_T clientFd; + + const char* args[10]; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + + THREAD_TYPE serThread; + + byte src[SFTP_PUT_RESUME_SZ]; + byte stale[SFTP_PUT_RESUME_SZ + SFTP_PUT_RESUME_OFST]; + byte expect[SFTP_PUT_RESUME_SZ]; + word32 ofst[2]; + char srcName[] = "wolfssh_12547_src.tmp"; + char dstName[] = "wolfssh_12547_dst.tmp"; + + WMEMSET(&ser, 0, sizeof(func_args)); + + argsCount = 0; + args[argsCount++] = "."; + args[argsCount++] = "-1"; + args[argsCount++] = "-p"; + args[argsCount++] = "0"; + ser.argv = (char**)args; + ser.argc = argsCount; + ser.signal = &ready; + InitTcpReady(ser.signal); + ThreadStart(echoserver_test, (void*)&ser, &serThread); + WaitTcpReady(&ready); + + sftp_client_connect(&ctx, &ssh, ready.port); + AssertNotNull(ctx); + AssertNotNull(ssh); + + sftpPutFillPattern(src, (word32)sizeof(src)); + WMEMSET(stale, 0xFF, sizeof(stale)); + AssertIntEQ(sftpPutWriteFile(srcName, src, (word32)sizeof(src)), 0); + + /* the saved offset matches the destination, so the download picks up + * where it left off. Staging a prefix unlike the source, and expecting it + * back untouched, is what separates a resume from a full re-download. */ + AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST), 0); + WMEMCPY(expect, stale, SFTP_PUT_RESUME_OFST); + WMEMCPY(expect + SFTP_PUT_RESUME_OFST, src + SFTP_PUT_RESUME_OFST, + sizeof(expect) - SFTP_PUT_RESUME_OFST); + ofst[0] = SFTP_PUT_RESUME_OFST; + ofst[1] = 0; + AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst), + WS_SUCCESS); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, expect, (word32)sizeof(expect)), + 0); + + /* the destination is gone, so the whole file is fetched again */ + WREMOVE(NULL, dstName); + ofst[0] = SFTP_PUT_RESUME_OFST; + ofst[1] = 0; + AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst), + WS_SUCCESS); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0); + + /* the destination is shorter than the saved offset, so the whole file is + * fetched again */ + AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST / 2), 0); + ofst[0] = SFTP_PUT_RESUME_OFST; + ofst[1] = 0; + AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst), + WS_SUCCESS); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0); + + /* the destination is longer than the saved offset, so the whole file is + * fetched again */ + AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST * 2), 0); + ofst[0] = SFTP_PUT_RESUME_OFST; + ofst[1] = 0; + AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst), + WS_SUCCESS); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0); + + /* a plain get still truncates a longer destination */ + AssertIntEQ(sftpPutWriteFile(dstName, stale, (word32)sizeof(stale)), 0); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 0), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, src, (word32)sizeof(src)), 0); + + /* the source has nothing left past the saved offset, so the whole file + * is fetched again. This case shortens the source, so it runs last. */ + AssertIntEQ(sftpPutWriteFile(dstName, stale, SFTP_PUT_RESUME_OFST), 0); + AssertIntEQ(sftpPutWriteFile(srcName, src, SFTP_PUT_RESUME_OFST / 2), 0); + ofst[0] = SFTP_PUT_RESUME_OFST; + ofst[1] = 0; + AssertIntEQ(wolfSSH_SFTP_SaveOfst(ssh, srcName, dstName, ofst), + WS_SUCCESS); + AssertIntEQ(sftpGetToCompletion(ssh, srcName, dstName, 1), WS_SUCCESS); + AssertIntEQ(sftpPutFileMatches(dstName, src, SFTP_PUT_RESUME_OFST / 2), 0); + + WREMOVE(NULL, dstName); + WREMOVE(NULL, srcName); + + /* take care of re-keying state before shutdown call */ + while (wolfSSH_get_error(ssh) == WS_REKEYING) { + wolfSSH_worker(ssh, NULL); + } + + argsCount = AbsorbBenignReset(ssh, wolfSSH_shutdown(ssh)); +#if DEFAULT_HIGHWATER_MARK < 8000 + if (argsCount == WS_REKEYING) { + argsCount = WS_SUCCESS; + } #endif + AssertIntEQ(argsCount, WS_SUCCESS); + + clientFd = wolfSSH_get_fd(ssh); + WCLOSESOCKET(clientFd); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); ThreadJoin(serThread); FreeTcpReady(&ready); #endif /* !NO_FILESYSTEM && !WOLFSSH_USER_FILESYSTEM && !WOLFSSH_ZEPHYR */ @@ -6051,6 +6201,7 @@ static void test_wolfSSH_SFTP_SetConfinePath(void) { ; } static void test_wolfSSH_SFTP_SetDefaultPath(void) { ; } static void test_wolfSSH_SFTP_SaveOfst(void) { ; } static void test_wolfSSH_SFTP_PutResume(void) { ; } +static void test_wolfSSH_SFTP_GetResume(void) { ; } #endif /* WOLFSSH_SFTP && !NO_WOLFSSH_CLIENT && !SINGLE_THREADED */ @@ -7999,6 +8150,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_wolfSSH_SFTP_SetDefaultPath(); test_wolfSSH_SFTP_SaveOfst(); test_wolfSSH_SFTP_PutResume(); + test_wolfSSH_SFTP_GetResume(); /* Either SCP or SFTP */ test_wolfSSH_RealPath();