Clamp buffer size to maxPktSz * 8

[falemagn]

Clamp buffer size to maxPktSz * 8, we don't want to allocate the entire 4GB window for each session. However, grow the buffer within reason during rekeying, to buffer the incoming packets until rekeying is done.

[wolfSSL-Bot]

Can one of the admins verify this patch?

[JacobBarthelmeh]

ok to test

[JacobBarthelmeh]

if the upper bounds here of (1024*1024*100) is arbitrary then it should be a definable macro, or maybe related to the max window size

[ejohnstown]

I believe this PR is wrong. A channel's receive buffer is clamped to the size WOLFSSH_DEFAULT_WINDOW_SZ which can be changed at compile time. It shouldn't change size. Also, the peer knows how much it is allowed to send into the channel, it is supposed to be keeping track balancing the number of bytes sent, the peer's channel window adjust messages, and the initial window size. The channel's receive buffer should only be 128kB.

What is the problem you are trying to fix with this?

[codesquid]

Our use case is bulk transfer using SFTP over long fat pipes, networks with a high bandwidth-delay product. For example transferring a huge file between Europe and the US west coast at multi-gigabit/s speeds.

SSH's Connection Protocol implements flow control on top on TCP's own flow control. We have noticed that the interactions of these two mechanisms can negatively impact throughput. At the same time, the most common use-case for SFTP is utilizing SSH sessions with just a single channel. In this scenario, SSH's additional flow control is redundant. To maximize throughput, we thus make the channel window as large as possible by passing 2^32-1 as window size to wolfSSH_CTX_SetWindowPacketSize.

Obviously we don't want a giant receive buffer to be neeedlessly allocated for the entire window, which would be 4GiB for each single-channel SSH session. In practice, if each packet is immediatley procesed upon reception, the input buffer can be very small, technically it only needs to be large enough to hold a single packet of maximum allowed packet length (35000 bytes by default). The unfortunate exception to this though is rekeying. Once rekeying has started, any non-keying packets cannot be sent until rekeying has finished.

As per specifications, after starting a new key exchange by sending out SSH_MSG_KEXINIT, implementations must be prepared for an arbitrary number of incoming packets before receiving the corresponding SSH_MSG_KEXINIT from the peer. On the assumption that every received SSH_MSG_CHANNEL_DATA packet must be replied to, as is the case for SFTP servers, re-keying thus implies that for the duration the re-keying, either incoming packets, or newly generated outgoing packets need to be buffered.

As memory is not unlimited, we chose to put large but finite cap on the size the input buffer may grow to during re-keying, to guard against malicious peers that intentionally refuse to reply to the SSH_MSG_KEXINIT packet. We do not mind if you decide to change it to a configurable limit. Ideally it would be a dynamic policy that also takes the re-keying states of other sessions into account and the amounts of data buffered in those sessions, but that discussion is not material to this PR.

[ejohnstown]

We cannot accept this due to the effects on other users.

As far as rekeying goes, the endpoint sends its KEXINIT message and should hold off sending any new messages, agreed. Those new send messages should be backed up into the application. The endpoint's peer knows how much more data the endpoint can receive on each channel; it shouldn't oversend.

Adding the API wolfSSH_CHANNEL_IncreaseWindow() to increase a channel's receive buffer size and adding a callback hook for the start and end of keying I think would achieve the your goal. I am logging this as a feature request but need the time to work on it cleared by management.