arrange for scp, when in sftp mode, to not ftruncate(3) files early
authordjm <djm@openbsd.org>
Fri, 13 May 2022 06:31:50 +0000 (06:31 +0000)
committerdjm <djm@openbsd.org>
Fri, 13 May 2022 06:31:50 +0000 (06:31 +0000)
previous behavious of unconditionally truncating the destination file
would cause "scp ~/foo localhost:" and "scp localhost:foo ~/" to
delete all the contents of their destination.

spotted by solene@ sthen@, also bz3431; ok dtucker@

usr.bin/ssh/scp.c
usr.bin/ssh/sftp-client.c
usr.bin/ssh/sftp-client.h
usr.bin/ssh/sftp.c

index fc5d99e..582e232 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: scp.c,v 1.247 2022/03/20 08:52:17 djm Exp $ */
+/* $OpenBSD: scp.c,v 1.248 2022/05/13 06:31:50 djm Exp $ */
 /*
  * scp - secure remote copy.  This is basically patched BSD rcp which
  * uses ssh to do the data transfer (instead of using rcmd).
@@ -1278,11 +1278,11 @@ source_sftp(int argc, char *src, char *targ, struct sftp_conn *conn)
 
        if (src_is_dir && iamrecursive) {
                if (upload_dir(conn, src, abs_dst, pflag,
-                   SFTP_PROGRESS_ONLY, 0, 0, 1) != 0) {
+                   SFTP_PROGRESS_ONLY, 0, 0, 1, 1) != 0) {
                        error("failed to upload directory %s to %s", src, targ);
                        errs = 1;
                }
-       } else if (do_upload(conn, src, abs_dst, pflag, 0, 0) != 0) {
+       } else if (do_upload(conn, src, abs_dst, pflag, 0, 0, 1) != 0) {
                error("failed to upload file %s to %s", src, targ);
                errs = 1;
        }
@@ -1519,11 +1519,11 @@ sink_sftp(int argc, char *dst, const char *src, struct sftp_conn *conn)
                debug("Fetching %s to %s\n", g.gl_pathv[i], abs_dst);
                if (globpath_is_dir(g.gl_pathv[i]) && iamrecursive) {
                        if (download_dir(conn, g.gl_pathv[i], abs_dst, NULL,
-                           pflag, SFTP_PROGRESS_ONLY, 0, 0, 1) == -1)
+                           pflag, SFTP_PROGRESS_ONLY, 0, 0, 1, 1) == -1)
                                err = -1;
                } else {
                        if (do_download(conn, g.gl_pathv[i], abs_dst, NULL,
-                           pflag, 0, 0) == -1)
+                           pflag, 0, 0, 1) == -1)
                                err = -1;
                }
                free(abs_dst);
index 403a2a8..78a64c9 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.c,v 1.162 2022/03/31 03:07:03 djm Exp $ */
+/* $OpenBSD: sftp-client.c,v 1.163 2022/05/13 06:31:50 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -1560,7 +1560,7 @@ progress_meter_path(const char *path)
 int
 do_download(struct sftp_conn *conn, const char *remote_path,
     const char *local_path, Attrib *a, int preserve_flag, int resume_flag,
-    int fsync_flag)
+    int fsync_flag, int inplace_flag)
 {
        struct sshbuf *msg;
        u_char *handle;
@@ -1607,8 +1607,8 @@ do_download(struct sftp_conn *conn, const char *remote_path,
            &handle, &handle_len) != 0)
                return -1;
 
-       local_fd = open(local_path,
-           O_WRONLY | O_CREAT | (resume_flag ? 0 : O_TRUNC), mode | S_IWUSR);
+       local_fd = open(local_path, O_WRONLY | O_CREAT |
+       ((resume_flag || inplace_flag) ? 0 : O_TRUNC), mode | S_IWUSR);
        if (local_fd == -1) {
                error("open local \"%s\": %s", local_path, strerror(errno));
                goto fail;
@@ -1827,7 +1827,7 @@ do_download(struct sftp_conn *conn, const char *remote_path,
 static int
 download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
     int depth, Attrib *dirattrib, int preserve_flag, int print_flag,
-    int resume_flag, int fsync_flag, int follow_link_flag)
+    int resume_flag, int fsync_flag, int follow_link_flag, int inplace_flag)
 {
        int i, ret = 0;
        SFTP_DIRENT **dir_entries;
@@ -1886,7 +1886,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
                        if (download_dir_internal(conn, new_src, new_dst,
                            depth + 1, &(dir_entries[i]->a), preserve_flag,
                            print_flag, resume_flag,
-                           fsync_flag, follow_link_flag) == -1)
+                           fsync_flag, follow_link_flag, inplace_flag) == -1)
                                ret = -1;
                } else if (S_ISREG(dir_entries[i]->a.perm) ||
                    (follow_link_flag && S_ISLNK(dir_entries[i]->a.perm))) {
@@ -1898,7 +1898,8 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
                        if (do_download(conn, new_src, new_dst,
                            S_ISLNK(dir_entries[i]->a.perm) ? NULL :
                            &(dir_entries[i]->a),
-                           preserve_flag, resume_flag, fsync_flag) == -1) {
+                           preserve_flag, resume_flag, fsync_flag,
+                           inplace_flag) == -1) {
                                error("Download of file %s to %s failed",
                                    new_src, new_dst);
                                ret = -1;
@@ -1936,7 +1937,7 @@ download_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 int
 download_dir(struct sftp_conn *conn, const char *src, const char *dst,
     Attrib *dirattrib, int preserve_flag, int print_flag, int resume_flag,
-    int fsync_flag, int follow_link_flag)
+    int fsync_flag, int follow_link_flag, int inplace_flag)
 {
        char *src_canon;
        int ret;
@@ -1948,26 +1949,25 @@ download_dir(struct sftp_conn *conn, const char *src, const char *dst,
 
        ret = download_dir_internal(conn, src_canon, dst, 0,
            dirattrib, preserve_flag, print_flag, resume_flag, fsync_flag,
-           follow_link_flag);
+           follow_link_flag, inplace_flag);
        free(src_canon);
        return ret;
 }
 
 int
 do_upload(struct sftp_conn *conn, const char *local_path,
-    const char *remote_path, int preserve_flag, int resume, int fsync_flag)
+    const char *remote_path, int preserve_flag, int resume,
+    int fsync_flag, int inplace_flag)
 {
        int r, local_fd;
-       u_int status = SSH2_FX_OK;
-       u_int id;
-       u_char type;
+       u_int openmode, id, status = SSH2_FX_OK, reordered = 0;
        off_t offset, progress_counter;
-       u_char *handle, *data;
+       u_char type, *handle, *data;
        struct sshbuf *msg;
        struct stat sb;
-       Attrib a, *c = NULL;
-       u_int32_t startid;
-       u_int32_t ackid;
+       Attrib a, t, *c = NULL;
+       u_int32_t startid, ackid;
+       u_int64_t highwater = 0;
        struct request *ack = NULL;
        struct requests acks;
        size_t handle_len;
@@ -2019,10 +2019,15 @@ do_upload(struct sftp_conn *conn, const char *local_path,
                }
        }
 
+       openmode = SSH2_FXF_WRITE|SSH2_FXF_CREAT;
+       if (resume)
+               openmode |= SSH2_FXF_APPEND;
+       else if (!inplace_flag)
+               openmode |= SSH2_FXF_TRUNC;
+
        /* Send open request */
-       if (send_open(conn, remote_path, "dest", SSH2_FXF_WRITE|SSH2_FXF_CREAT|
-           (resume ? SSH2_FXF_APPEND : SSH2_FXF_TRUNC),
-           &a, &handle, &handle_len) != 0) {
+       if (send_open(conn, remote_path, "dest", openmode, &a,
+           &handle, &handle_len) != 0) {
                close(local_fd);
                return -1;
        }
@@ -2103,6 +2108,12 @@ do_upload(struct sftp_conn *conn, const char *local_path,
                            ack->id, ack->len, (unsigned long long)ack->offset);
                        ++ackid;
                        progress_counter += ack->len;
+                       if (!reordered && ack->offset <= highwater)
+                               highwater = ack->offset + ack->len;
+                       else if (!reordered && ack->offset > highwater) {
+                               debug3_f("server reordered ACKs");
+                               reordered = 1;
+                       }
                        free(ack);
                }
                offset += len;
@@ -2120,6 +2131,14 @@ do_upload(struct sftp_conn *conn, const char *local_path,
                status = SSH2_FX_FAILURE;
        }
 
+       if ((resume || inplace_flag) && (status != SSH2_FX_OK || interrupted)) {
+               debug("truncating at %llu", (unsigned long long)highwater);
+               attrib_clear(&t);
+               t.flags = SSH2_FILEXFER_ATTR_SIZE;
+               t.size = highwater;
+               do_fsetstat(conn, handle, handle_len, &a);
+       }
+
        if (close(local_fd) == -1) {
                error("close local \"%s\": %s", local_path, strerror(errno));
                status = SSH2_FX_FAILURE;
@@ -2143,7 +2162,7 @@ do_upload(struct sftp_conn *conn, const char *local_path,
 static int
 upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
     int depth, int preserve_flag, int print_flag, int resume, int fsync_flag,
-    int follow_link_flag)
+    int follow_link_flag, int inplace_flag)
 {
        int ret = 0;
        DIR *dirp;
@@ -2221,12 +2240,13 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 
                        if (upload_dir_internal(conn, new_src, new_dst,
                            depth + 1, preserve_flag, print_flag, resume,
-                           fsync_flag, follow_link_flag) == -1)
+                           fsync_flag, follow_link_flag, inplace_flag) == -1)
                                ret = -1;
                } else if (S_ISREG(sb.st_mode) ||
                    (follow_link_flag && S_ISLNK(sb.st_mode))) {
                        if (do_upload(conn, new_src, new_dst,
-                           preserve_flag, resume, fsync_flag) == -1) {
+                           preserve_flag, resume, fsync_flag,
+                           inplace_flag) == -1) {
                                error("upload \"%s\" to \"%s\" failed",
                                    new_src, new_dst);
                                ret = -1;
@@ -2246,7 +2266,7 @@ upload_dir_internal(struct sftp_conn *conn, const char *src, const char *dst,
 int
 upload_dir(struct sftp_conn *conn, const char *src, const char *dst,
     int preserve_flag, int print_flag, int resume, int fsync_flag,
-    int follow_link_flag)
+    int follow_link_flag, int inplace_flag)
 {
        char *dst_canon;
        int ret;
@@ -2257,7 +2277,7 @@ upload_dir(struct sftp_conn *conn, const char *src, const char *dst,
        }
 
        ret = upload_dir_internal(conn, src, dst_canon, 0, preserve_flag,
-           print_flag, resume, fsync_flag, follow_link_flag);
+           print_flag, resume, fsync_flag, follow_link_flag, inplace_flag);
 
        free(dst_canon);
        return ret;
index 6c62821..d26cb71 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp-client.h,v 1.36 2022/03/31 03:07:03 djm Exp $ */
+/* $OpenBSD: sftp-client.h,v 1.37 2022/05/13 06:31:50 djm Exp $ */
 
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
@@ -135,28 +135,29 @@ int do_fsync(struct sftp_conn *conn, u_char *, u_int);
  * Download 'remote_path' to 'local_path'. Preserve permissions and times
  * if 'pflag' is set
  */
-int do_download(struct sftp_conn *, const char *, const char *,
-    Attrib *, int, int, int);
+int do_download(struct sftp_conn *, const char *, const char *, Attrib *,
+    int, int, int, int);
 
 /*
  * Recursively download 'remote_directory' to 'local_directory'. Preserve
  * times if 'pflag' is set
  */
-int download_dir(struct sftp_conn *, const char *, const char *,
-    Attrib *, int, int, int, int, int);
+int download_dir(struct sftp_conn *, const char *, const char *, Attrib *,
+    int, int, int, int, int, int);
 
 /*
  * Upload 'local_path' to 'remote_path'. Preserve permissions and times
  * if 'pflag' is set
  */
-int do_upload(struct sftp_conn *, const char *, const char *, int, int, int);
+int do_upload(struct sftp_conn *, const char *, const char *,
+    int, int, int, int);
 
 /*
  * Recursively upload 'local_directory' to 'remote_directory'. Preserve
  * times if 'pflag' is set
  */
-int upload_dir(struct sftp_conn *, const char *, const char *, int, int, int,
-    int, int);
+int upload_dir(struct sftp_conn *, const char *, const char *,
+    int, int, int, int, int, int);
 
 /*
  * Download a 'from_path' from the 'from' connection and upload it to
index e47adf5..ee2890f 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: sftp.c,v 1.215 2022/05/08 22:32:36 djm Exp $ */
+/* $OpenBSD: sftp.c,v 1.216 2022/05/13 06:31:50 djm Exp $ */
 /*
  * Copyright (c) 2001-2004 Damien Miller <djm@openbsd.org>
  *
@@ -676,12 +676,12 @@ process_get(struct sftp_conn *conn, const char *src, const char *dst,
                if (globpath_is_dir(g.gl_pathv[i]) && (rflag || global_rflag)) {
                        if (download_dir(conn, g.gl_pathv[i], abs_dst, NULL,
                            pflag || global_pflag, 1, resume,
-                           fflag || global_fflag, 0) == -1)
+                           fflag || global_fflag, 0, 0) == -1)
                                err = -1;
                } else {
                        if (do_download(conn, g.gl_pathv[i], abs_dst, NULL,
                            pflag || global_pflag, resume,
-                           fflag || global_fflag) == -1)
+                           fflag || global_fflag, 0) == -1)
                                err = -1;
                }
                free(abs_dst);
@@ -770,12 +770,12 @@ process_put(struct sftp_conn *conn, const char *src, const char *dst,
                if (globpath_is_dir(g.gl_pathv[i]) && (rflag || global_rflag)) {
                        if (upload_dir(conn, g.gl_pathv[i], abs_dst,
                            pflag || global_pflag, 1, resume,
-                           fflag || global_fflag, 0) == -1)
+                           fflag || global_fflag, 0, 0) == -1)
                                err = -1;
                } else {
                        if (do_upload(conn, g.gl_pathv[i], abs_dst,
                            pflag || global_pflag, resume,
-                           fflag || global_fflag) == -1)
+                           fflag || global_fflag, 0) == -1)
                                err = -1;
                }
        }