-
-
Notifications
You must be signed in to change notification settings - Fork 175
Description
User-visible problem
If I compile and run the echoserver example program, and then attempt to connect to it using PuTTY with compression enabled, the connection hangs during setup. PuTTY's packet log shows that it sent its first encrypted packet, SSH_MSG_SERVICE_REQUEST("ssh-userauth"), and never received SSH_MSG_SERVICE_ACCEPT in reply.
I observed this when building from git commit be5c22e, the location of the v0.54.3 tag.
Background
There are two related compression methods (other than none) supported by both PuTTY and russh. They are zlib, the original one defined in RFC 4253, and [email protected], a modified version invented by OpenSSH.
They are not the same. In the original zlib, compression is initialised at the same time as encryption, in both directions: the first packet sent after SSH_MSG_NEWKEYS is the first compressed packet. But [email protected] delays enabling compression in the client-to-server direction until userauth is complete, so that unauthenticated users can't hammer on the server's zlib decoder looking for exploits.
OpenSSH's client uses [email protected] in preference to zlib. PuTTY puts them the other way round. The client's preference order determines which method is selected, so PuTTY ends up negotiating zlib compression while the OpenSSH client negotiates [email protected].
Diagnosis
It looks to me as if russh is treating zlib as an alias for [email protected], which is incorrect. When PuTTY and russh negotiate the zlib mode for client-to-server compression, PuTTY sends its SSH_MSG_SERVICE_REQUEST in compressed form, because zlib specifies that compression is enabled instantly at the same time as encryption. But recompiling russh with extra diagnostics, I see that when russh receives that encrypted packet, it calls Decompress::decompress() on a Decompress object which at that point still contains Decompress::None. So it misinterprets the SSH_MSG_SERVICE_REQUEST as a packet with the nonsense type 120, ignores it completely, and continues waiting for what it thinks is a SSH_MSG_SERVICE_REQUEST.
If I hack the source code of PuTTY so that it also treats zlib as specifying the same delayed compression as [email protected], then the connection is set up successfully.
Possible root cause
I also see in compression.rs that the internal identifiers for the two compression methods are confusingly the wrong way round, because the identifier containing the word LEGACY refers to the newer [email protected] and not the older zlib:
#[cfg(feature = "flate2")]
pub const ZLIB: Name = Name("zlib");
#[cfg(feature = "flate2")]
pub const ZLIB_LEGACY: Name = Name("[email protected]");This makes me wonder if the author of this part of the code might have misinterpreted the existence of the two names as meaning that the domain-qualified name came first, and was later standardised under a bare name. That is a common pattern in other cases (particularly new crypto), but in this case, it's not what happened. zlib came first, and [email protected] is not an alias for it but a modification of it.
Fix
I haven't tried to prepare a patch against the russh code, but the shape of the fix is that if zlib is selected, then init_decompress in the server should be called at the same time as encryption is set up. Only [email protected] should defer it until after userauth.
If russh is unwilling to take the risk of unauthenticated users being able to try to exploit the server's zlib decoder, then an alternative fix is to remove zlib from the option list completely, leaving only [email protected].