Bug 2949 - "limits@openssh.com" extension to SFTP to query various transfer limits
Summary: "limits@openssh.com" extension to SFTP to query various transfer limits
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: sftp-server (show other bugs)
Version: -current
Hardware: All All
: P5 enhancement
Assignee: Mike Frysinger
URL: https://lists.mindrot.org/pipermail/o...
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-29 21:19 AEDT by Mike Frysinger
Modified: 2021-04-23 15:10 AEST (History)
1 user (show)

See Also:


Attachments
PROTOCOL v1 (1.48 KB, text/plain)
2018-12-29 21:19 AEDT, Mike Frysinger
no flags Details
PROTOCOL v1 (1.47 KB, text/plain)
2019-02-08 08:56 AEDT, Mike Frysinger
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Frysinger 2018-12-29 21:19:31 AEDT
Created attachment 3218 [details]
PROTOCOL v1

i haven't been able to find any prior art in this space.  if anyone is familiar with a server/client implementing a relevant extension, feel free to highlight it.

`sftp` has a -B option to set the transfer buffer size.  this applies to both read & write SFTP packets.

when doing a read of a really large size, OpenSSH will respond with short reads.  e.g. if you request 1MiB, the server will just respond with 64KiB (sftp-server.c:process_read hardcodes buf[64*1024]).  this leaves the client with expecting to chunk things up at one size, but ends up having to backfill things dynamically.  the client is able to recover though, so that's good.

when doing a write of a really large size, OpenSSH will just close the connection as soon as it sees the header with the large write.  the server hardcodes 256KiB (SFTP_MAX_MSG_LENGTH) and any attempt to write anything larger than that is immediately rejected.  this is not friendly and clients basically have to default to 32KiB all the time and force users to manually select a size that they happen to know the server they're connecting to supports.

if the server supports an extension to dynamically query the exact limits the server imposes, the client can start with the 32KiB default, and then automatically increase to something with better throughput.
Comment 1 Damien Miller 2019-01-22 21:35:23 AEDT
That seems reasonable, though some of these are only implicitly defined. E.g. there is no internal limit to the max number of file handles, sftp-server will just allocate up to the fd/heap rlimits.

So IMO there should be some way to express "unspecified" or "unlimited" (internally at least) limits. Maybe 0 for a limit?

Also, the values might as well be 64 bits.
Comment 2 Mike Frysinger 2019-02-08 08:50:51 AEDT
i tried to account for that here:
> If the server doesn't enforce a specific limit, then the field may be set to 0.
> This implies the server relies on the OS to enforce limits (e.g. available
> memory or file handles), and such limits might be dynamic.  The client SHOULD
> take care to not try to exceed reasonable limits.

i think that covers it ?

wrt 32bit vs 64bit, i couldn't imagine a situation where a single packet would hit that, but i guess if we're laying groundwork for a protocol that will stick around for decades, might as well plan for then rather than regret it.
Comment 3 Mike Frysinger 2019-02-08 08:56:23 AEDT
Created attachment 3238 [details]
PROTOCOL v1

change all the fields from uint32 to uint64
Comment 4 Mike Frysinger 2020-01-01 21:34:12 AEDT
was there anything other feedback ?
Comment 5 Damien Miller 2020-08-28 13:52:01 AEST
sorry for stalling. This looks fine to me
Comment 6 Mike Frysinger 2020-11-30 16:06:41 AEDT
i've posted a patch series then implementing things:
https://lists.mindrot.org/pipermail/openssh-unix-dev/2020-November/038977.html

hopefully it'll be easy to get through since the proposal is simple :).
Comment 7 Mike Frysinger 2021-01-27 18:31:04 AEDT
anything i can do to help with reviewing the posted patches ?
Comment 8 Mike Frysinger 2021-03-01 20:02:15 AEDT
looks like the PROTOCOL & server support have been merged so far. whoo!
Comment 9 Damien Miller 2021-03-01 20:26:16 AEDT
yes, I'll try to do the rest shortly after the 8.5 release (this week)
Comment 10 Mike Frysinger 2021-04-04 23:15:28 AEST
everything should be merged now for the next release (8.6).
thanks Damien!
Comment 11 Damien Miller 2021-04-23 15:10:42 AEST
closing resolved bugs as of 8.6p1 release
Comment 12 Damien Miller 2021-04-23 15:10:44 AEST
closing resolved bugs as of 8.6p1 release