From e555d5cad5afae7d5ef2bbc02ca591178fe16fed Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Fri, 16 Dec 2022 03:40:03 +0000 Subject: [PATCH] upstream: add a -X option to both scp(1) and sftp(1) to allow control over some SFTP protocol knobs: the copy buffer length and the number of inflight requests, both of which are used during upload/download. Previously these could be controlled in sftp(1) using the -b/-R options. This makes them available in both SFTP protocol clients using the same option character sequence. ok dtucker@ OpenBSD-Commit-ID: 27502bffc589776f5da1f31df8cb51abe9a15f1c --- scp.1 | 18 ++++++++++++++++-- scp.c | 42 ++++++++++++++++++++++++++++++++++++++---- sftp-client.c | 6 +++--- sftp.1 | 18 ++++++++++++++++-- sftp.c | 31 ++++++++++++++++++++++++++++--- 5 files changed, 101 insertions(+), 14 deletions(-) diff --git a/scp.1 b/scp.1 index cd23f9795..61e496333 100644 --- a/scp.1 +++ b/scp.1 @@ -8,9 +8,9 @@ .\" .\" Created: Sun May 7 00:14:37 1995 ylo .\" -.\" $OpenBSD: scp.1,v 1.110 2022/09/19 21:39:16 djm Exp $ +.\" $OpenBSD: scp.1,v 1.111 2022/12/16 03:40:03 djm Exp $ .\" -.Dd $Mdocdate: September 19 2022 $ +.Dd $Mdocdate: December 16 2022 $ .Dt SCP 1 .Os .Sh NAME @@ -28,6 +28,7 @@ .Op Fl o Ar ssh_option .Op Fl P Ar port .Op Fl S Ar program +.Op Fl X Ar sftp_option .Ar source ... target .Sh DESCRIPTION .Nm @@ -278,6 +279,19 @@ and to print debugging messages about their progress. This is helpful in debugging connection, authentication, and configuration problems. +.It Fl X Ar sftp_option +Specify an option that controls aspects of SFTP protocol behaviour. +The valid options are: +.Bl -tag -width Ds +.It Cm nrequests Ns = Ns Ar value +Controls how many concurrent SFTP read or write requests may be in progress +at any point in time during a download or upload. +By default 64 requests may be active concurrently. +.It Cm buffer Ns = Ns Ar value +Controls the maximum buffer size for a single SFTP read/write operation used +during download or upload. +By default a 32KB buffer is used. +.El .El .Sh EXIT STATUS .Ex -std scp diff --git a/scp.c b/scp.c index 73bfe2f56..6b2df8636 100644 --- a/scp.c +++ b/scp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: scp.c,v 1.249 2022/10/24 21:51:55 djm Exp $ */ +/* $OpenBSD: scp.c,v 1.250 2022/12/16 03:40:03 djm Exp $ */ /* * scp - secure remote copy. This is basically patched BSD rcp which * uses ssh to do the data transfer (instead of using rcmd). @@ -106,6 +106,9 @@ #include #endif #include +#ifdef HAVE_UTIL_H +# include +#endif #include #include #include @@ -176,6 +179,10 @@ char *ssh_program = _PATH_SSH_PROGRAM; pid_t do_cmd_pid = -1; pid_t do_cmd_pid2 = -1; +/* SFTP copy parameters */ +size_t sftp_copy_buflen; +size_t sftp_nrequests; + /* Needed for sftp */ volatile sig_atomic_t interrupted = 0; @@ -444,13 +451,14 @@ void throughlocal_sftp(struct sftp_conn *, struct sftp_conn *, int main(int argc, char **argv) { - int ch, fflag, tflag, status, n; + int ch, fflag, tflag, status, r, n; char **newargv, *argv0; const char *errstr; extern char *optarg; extern int optind; enum scp_mode_e mode = MODE_SFTP; char *sftp_direct = NULL; + long long llv; /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ sanitise_stdfd(); @@ -480,7 +488,7 @@ main(int argc, char **argv) fflag = Tflag = tflag = 0; while ((ch = getopt(argc, argv, - "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:")) != -1) { + "12346ABCTdfOpqRrstvD:F:J:M:P:S:c:i:l:o:X:")) != -1) { switch (ch) { /* User-visible flags. */ case '1': @@ -561,6 +569,31 @@ main(int argc, char **argv) addargs(&remote_remote_args, "-q"); showprogress = 0; break; + case 'X': + /* Please keep in sync with sftp.c -X */ + if (strncmp(optarg, "buffer=", 7) == 0) { + r = scan_scaled(optarg + 7, &llv); + if (r == 0 && (llv <= 0 || llv > 256 * 1024)) { + r = -1; + errno = EINVAL; + } + if (r == -1) { + fatal("Invalid buffer size \"%s\": %s", + optarg + 7, strerror(errno)); + } + sftp_copy_buflen = (size_t)llv; + } else if (strncmp(optarg, "nrequests=", 10) == 0) { + llv = strtonum(optarg + 10, 1, 256 * 1024, + &errstr); + if (errstr != NULL) { + fatal("Invalid number of requests " + "\"%s\": %s", optarg + 10, errstr); + } + sftp_nrequests = (size_t)llv; + } else { + fatal("Invalid -X option"); + } + break; /* Server options. */ case 'd': @@ -972,7 +1005,8 @@ do_sftp_connect(char *host, char *user, int port, char *sftp_direct, reminp, remoutp, pidp) < 0) return NULL; } - return do_init(*reminp, *remoutp, 32768, 64, limit_kbps); + return do_init(*reminp, *remoutp, + sftp_copy_buflen, sftp_nrequests, limit_kbps); } void diff --git a/sftp-client.c b/sftp-client.c index 35be53d69..6c97bfa9f 100644 --- a/sftp-client.c +++ b/sftp-client.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sftp-client.c,v 1.165 2022/09/19 10:43:12 djm Exp $ */ +/* $OpenBSD: sftp-client.c,v 1.166 2022/12/16 03:40:03 djm Exp $ */ /* * Copyright (c) 2001-2004 Damien Miller * @@ -68,10 +68,10 @@ extern volatile sig_atomic_t interrupted; extern int showprogress; -/* Default size of buffer for up/download */ +/* Default size of buffer for up/download (fix sftp.1 scp.1 if changed) */ #define DEFAULT_COPY_BUFLEN 32768 -/* Default number of concurrent outstanding requests */ +/* Default number of concurrent xfer requests (fix sftp.1 scp.1 if changed) */ #define DEFAULT_NUM_REQUESTS 64 /* Minimum amount of data to read at a time */ diff --git a/sftp.1 b/sftp.1 index 3b3f2c5a7..68923ae20 100644 --- a/sftp.1 +++ b/sftp.1 @@ -1,4 +1,4 @@ -.\" $OpenBSD: sftp.1,v 1.142 2022/09/19 21:39:16 djm Exp $ +.\" $OpenBSD: sftp.1,v 1.143 2022/12/16 03:40:03 djm Exp $ .\" .\" Copyright (c) 2001 Damien Miller. All rights reserved. .\" @@ -22,7 +22,7 @@ .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. .\" -.Dd $Mdocdate: September 19 2022 $ +.Dd $Mdocdate: December 16 2022 $ .Dt SFTP 1 .Os .Sh NAME @@ -44,6 +44,7 @@ .Op Fl R Ar num_requests .Op Fl S Ar program .Op Fl s Ar subsystem | sftp_server +.Op Fl X Ar sftp_option .Ar destination .Sh DESCRIPTION .Nm @@ -320,6 +321,19 @@ does not have an sftp subsystem configured. .It Fl v Raise logging level. This option is also passed to ssh. +.It Fl X Ar sftp_option +Specify an option that controls aspects of SFTP protocol behaviour. +The valid options are: +.Bl -tag -width Ds +.It Cm nrequests Ns = Ns Ar value +Controls how many concurrent SFTP read or write requests may be in progress +at any point in time during a download or upload. +By default 64 requests may be active concurrently. +.It Cm buffer Ns = Ns Ar value +Controls the maximum buffer size for a single SFTP read/write operation used +during download or upload. +By default a 32KB buffer is used. +.El .El .Sh INTERACTIVE COMMANDS Once in interactive mode, diff --git a/sftp.c b/sftp.c index c3c347e08..8f45b474f 100644 --- a/sftp.c +++ b/sftp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: sftp.c,v 1.222 2022/09/19 10:46:00 djm Exp $ */ +/* $OpenBSD: sftp.c,v 1.223 2022/12/16 03:40:03 djm Exp $ */ /* * Copyright (c) 2001-2004 Damien Miller * @@ -2431,7 +2431,7 @@ main(int argc, char **argv) struct sftp_conn *conn; size_t copy_buffer_len = 0; size_t num_requests = 0; - long long limit_kbps = 0; + long long llv, limit_kbps = 0; /* Ensure that fds 0, 1 and 2 are open or directed to /dev/null */ sanitise_stdfd(); @@ -2449,7 +2449,7 @@ main(int argc, char **argv) infile = stdin; while ((ch = getopt(argc, argv, - "1246AafhNpqrvCc:D:i:l:o:s:S:b:B:F:J:P:R:")) != -1) { + "1246AafhNpqrvCc:D:i:l:o:s:S:b:B:F:J:P:R:X:")) != -1) { switch (ch) { /* Passed through to ssh(1) */ case 'A': @@ -2546,6 +2546,31 @@ main(int argc, char **argv) ssh_program = optarg; replacearg(&args, 0, "%s", ssh_program); break; + case 'X': + /* Please keep in sync with ssh.c -X */ + if (strncmp(optarg, "buffer=", 7) == 0) { + r = scan_scaled(optarg + 7, &llv); + if (r == 0 && (llv <= 0 || llv > 256 * 1024)) { + r = -1; + errno = EINVAL; + } + if (r == -1) { + fatal("Invalid buffer size \"%s\": %s", + optarg + 7, strerror(errno)); + } + copy_buffer_len = (size_t)llv; + } else if (strncmp(optarg, "nrequests=", 10) == 0) { + llv = strtonum(optarg + 10, 1, 256 * 1024, + &errstr); + if (errstr != NULL) { + fatal("Invalid number of requests " + "\"%s\": %s", optarg + 10, errstr); + } + num_requests = (size_t)llv; + } else { + fatal("Invalid -X option"); + } + break; case 'h': default: usage();