Fix scp clearing file when src and dest are the same

Resolves: rhbz#2056884

Signed-off-by: Zoltan Fridrich <zfridric@redhat.com>
This commit is contained in:
Zoltan Fridrich 2022-06-30 10:56:19 +02:00
parent d0bf0e31d9
commit c958ea0a38
3 changed files with 309 additions and 8 deletions

View File

@ -392,10 +392,9 @@ diff -up openssh-8.5p1/session.c.coverity openssh-8.5p1/session.c
}
/* SSH_CLIENT deprecated */
diff -up openssh-7.4p1/sftp.c.coverity openssh-7.4p1/sftp.c
--- openssh-7.4p1/sftp.c.coverity 2016-12-19 05:59:41.000000000 +0100
+++ openssh-7.4p1/sftp.c 2016-12-23 16:40:26.903788691 +0100
@@ -224,7 +224,7 @@ killchild(int signo)
--- a/sftp.c 2022-06-30 10:43:13.914058913 +0200
+++ b/sftp.c 2022-06-30 10:48:17.243997888 +0200
@@ -222,7 +222,7 @@ killchild(int signo)
pid = sshpid;
if (pid > 1) {
kill(pid, SIGTERM);
@ -404,8 +403,8 @@ diff -up openssh-7.4p1/sftp.c.coverity openssh-7.4p1/sftp.c
}
_exit(1);
@@ -762,6 +762,8 @@ process_put(struct sftp_conn *conn, cons
fflag || global_fflag) == -1)
@@ -768,6 +768,8 @@ process_put(struct sftp_conn *conn, cons
fflag || global_fflag, 0) == -1)
err = -1;
}
+ free(abs_dst);
@ -413,7 +412,7 @@ diff -up openssh-7.4p1/sftp.c.coverity openssh-7.4p1/sftp.c
}
out:
@@ -985,6 +987,7 @@ do_globbed_ls(struct sftp_conn *conn, co
@@ -991,6 +993,7 @@ do_globbed_ls(struct sftp_conn *conn, co
if (lflag & LS_LONG_VIEW) {
if (g.gl_statv[i] == NULL) {
error("no stat information for %s", fname);

View File

@ -0,0 +1,293 @@
diff --color -ru a/scp.c b/scp.c
--- a/scp.c 2022-06-30 09:47:39.529662574 +0200
+++ b/scp.c 2022-06-30 09:51:05.740719310 +0200
@@ -1324,12 +1324,12 @@
if (src_is_dir && iamrecursive) {
if (upload_dir(conn, src, abs_dst, pflag,
- SFTP_PROGRESS_ONLY, 0, 0, 1, 1) != 0) {
+ SFTP_PROGRESS_ONLY, 0, 0, 1, 1, 1) != 0) {
error("failed to upload directory %s to %s",
src, abs_dst);
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, abs_dst);
errs = 1;
}
@@ -1566,11 +1566,11 @@
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);
diff --color -ru a/sftp.c b/sftp.c
--- a/sftp.c 2022-06-30 09:47:39.530662594 +0200
+++ b/sftp.c 2022-06-30 09:52:05.118887441 +0200
@@ -666,12 +666,12 @@
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);
@@ -760,12 +760,12 @@
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, 0) == -1)
+ fflag || global_fflag, 0, 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;
}
}
diff --color -ru a/sftp-client.c b/sftp-client.c
--- a/sftp-client.c 2022-06-30 09:47:39.530662594 +0200
+++ b/sftp-client.c 2022-06-30 10:03:36.608473570 +0200
@@ -1454,7 +1454,7 @@
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;
@@ -1661,8 +1661,11 @@
/* Sanity check */
if (TAILQ_FIRST(&requests) != NULL)
fatal("Transfer complete, but requests still in queue");
- /* Truncate at highest contiguous point to avoid holes on interrupt */
- if (read_error || write_error || interrupted) {
+ /*
+ * Truncate at highest contiguous point to avoid holes on interrupt,
+ * or unconditionally if writing in place.
+ */
+ if (inplace_flag || read_error || write_error || interrupted) {
if (reordered && resume_flag) {
error("Unable to resume download of \"%s\": "
"server reordered requests", local_path);
@@ -1724,7 +1727,7 @@
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;
@@ -1781,7 +1784,7 @@
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))) {
@@ -1793,7 +1796,8 @@
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;
@@ -1831,7 +1835,7 @@
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;
@@ -1843,26 +1847,25 @@
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;
@@ -1913,10 +1916,15 @@
}
}
+ 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;
}
@@ -1999,6 +2007,12 @@
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;
@@ -2017,6 +2031,14 @@
status = SSH2_FX_FAILURE;
}
+ if (inplace_flag || (resume && (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, &t);
+ }
+
if (close(local_fd) == -1) {
error("Couldn't close local file \"%s\": %s", local_path,
strerror(errno));
@@ -2041,7 +2063,7 @@
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;
@@ -2119,12 +2141,13 @@
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("Uploading of file %s to %s failed!",
new_src, new_dst);
ret = -1;
@@ -2144,7 +2167,7 @@
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 create_dir)
+ int follow_link_flag, int create_dir, int inplace_flag)
{
char *dst_canon;
int ret;
@@ -2155,7 +2178,7 @@
}
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;
diff --color -ru a/sftp-client.h b/sftp-client.h
--- a/sftp-client.h 2022-06-30 09:47:39.530662594 +0200
+++ b/sftp-client.h 2022-06-30 10:05:50.835107759 +0200
@@ -138,28 +138,29 @@
* 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);
+int upload_dir(struct sftp_conn *, const char *, const char *,
+ int, int, int, int, int, int, int);
/*
* Download a 'from_path' from the 'from' connection and upload it to

View File

@ -51,7 +51,7 @@
# Do not forget to bump pam_ssh_agent_auth release if you rewind the main package release to 1
%global openssh_ver 8.7p1
%global openssh_rel 10
%global openssh_rel 11
%global pam_ssh_agent_ver 0.10.4
%global pam_ssh_agent_rel 4
@ -222,6 +222,10 @@ Patch983: openssh-8.7p1-evpgenkey.patch
# 291721bc7c840d113a49518f3fca70e86248b8e8
# 0fa33683223c76289470a954404047bc762be84c
Patch1000: openssh-8.7p1-minimize-sha1-use.patch
# Fix for scp clearing file when src and dest are the same (#2056884)
# upstream commits:
# 7b1cbcb7599d9f6a3bbad79d412604aa1203b5ee
Patch1001: openssh-8.7p1-scp-clears-file.patch
License: BSD
Requires: /sbin/nologin
@ -411,6 +415,7 @@ popd
%patch700 -p1 -b .fips
%patch1000 -p1 -b .minimize-sha1-use
%patch1001 -p1 -b .scp-clears-file
%patch100 -p1 -b .coverity
@ -691,6 +696,10 @@ test -f %{sysconfig_anaconda} && \
%endif
%changelog
* Mon Jul 11 2022 Zoltan Fridrich <zfridric@redhat.com> - 8.7p1-11
- Fix scp clearing file when src and dest are the same
Resolves: rhbz#2056884
* Wed Jun 29 2022 Dmitry Belyavskiy <dbelyavs@redhat.com> - 8.7p1-10
- Set minimal value of RSA key length via configuration option
Related: rhbz#2066882