x/crypto/ssh: should avoid window adjustment on every Channel.Read

[willmo]

What version of Go are you using (go version)?

$ go version
go version go1.18.4 linux/amd64

golang.org/x/crypto v0.0.0-20220829220503-c86fa9a7ed90

Does this issue reproduce with the latest release?

Yes, channel.go is unchanged since 2017.

What did you do?

Used tcpdump/Wireshark to observe packets on a long-lived SSH connection where the only application-level activity is sporadic small writes or a long stream of writes in the same direction, or other easy-to-understand patterns.

What did you expect to see?

TCP segments with data generally correspond to application-level writes, with only occasional window adjustments sent in the other direction (and rarely a rekeying, etc.).

What did you see instead?

A window adjustment is sent after every Channel.Read on the receiving end, which wastes bandwidth.

Solution

OpenSSH has logic for deferring window adjustments, which I think usually waits until about 5% of the window has been consumed. It seemingly hasn't changed since 2007, so I assume it works well enough for them.

I added similar logic to channel.adjustWindow and it seems to work as expected in local testing. If this sounds like a reasonable change, I'll see about signing the CLA and submitting a patch.

[dr2chase]

@golang/security
if this is a bug, it has a proposed fix.

[rolandshoemaker]

Seems like a reasonable optimization, I'd be happy to take a look if you'd like to send a CL.

[gopherbot]

Change https://go.dev/cl/459915 mentions this issue: ssh: defer channel window adjustment