From 562cdd4df1b2c741fec90bc80954d76da7a33266 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Fri, 21 Sep 2018 18:00:07 +0100 Subject: [PATCH] Fix mishandling of refusal to compress in SSH-1. I've just noticed that we call ssh1_bpp_start_compression even if the server responded to our compression request with SSH1_SMSG_FAILURE! Also, while I'm here, there's a potential race condition if the server were to send an unrelated message (such as SSH1_MSG_IGNORE) immediately after the SSH1_SMSG_SUCCESS that indicates compression being enabled - the BPP would try to decode the compressed IGNORE message before the SUCCESS got to the higher layer that would tell the BPP it should have enabled compression. Fixed that by changing the method by which we tell the BPP what's going on. --- ssh.c | 7 ++++--- ssh1bpp.c | 23 +++++++++++++++++------ sshbpp.h | 6 +++++- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/ssh.c b/ssh.c index 38640387..1e22cb8f 100644 --- a/ssh.c +++ b/ssh.c @@ -3200,6 +3200,7 @@ static void do_ssh1_connection(void *vctx) pkt = ssh_bpp_new_pktout(ssh->bpp, SSH1_CMSG_REQUEST_COMPRESSION); put_uint32(pkt, 6); /* gzip compression level */ ssh_pkt_write(ssh, pkt); + ssh1_bpp_requested_compression(ssh->bpp); crMaybeWaitUntilV((pktin = pq_pop(&ssh->pq_ssh1_connection)) != NULL); if (pktin->type != SSH1_SMSG_SUCCESS && pktin->type != SSH1_SMSG_FAILURE) { @@ -3207,9 +3208,9 @@ static void do_ssh1_connection(void *vctx) crStopV; } else if (pktin->type == SSH1_SMSG_FAILURE) { c_write_str(ssh, "Server refused to compress\r\n"); - } - logevent("Started zlib (RFC1950) compression"); - ssh1_bpp_start_compression(ssh->bpp); + } else { + logevent("Started zlib (RFC1950) compression"); + } } /* diff --git a/ssh1bpp.c b/ssh1bpp.c index a1fd78c9..702ffb8b 100644 --- a/ssh1bpp.c +++ b/ssh1bpp.c @@ -21,6 +21,7 @@ struct ssh1_bpp_state { struct crcda_ctx *crcda_ctx; + int pending_compression_request; ssh_compressor *compctx; ssh_decompressor *decompctx; @@ -82,17 +83,13 @@ void ssh1_bpp_new_cipher(BinaryPacketProtocol *bpp, } } -void ssh1_bpp_start_compression(BinaryPacketProtocol *bpp) +void ssh1_bpp_requested_compression(BinaryPacketProtocol *bpp) { struct ssh1_bpp_state *s; assert(bpp->vt == &ssh1_bpp_vtable); s = FROMFIELD(bpp, struct ssh1_bpp_state, bpp); - assert(!s->compctx); - assert(!s->decompctx); - - s->compctx = ssh_compressor_new(&ssh_zlib); - s->decompctx = ssh_decompressor_new(&ssh_zlib); + s->pending_compression_request = TRUE; } static void ssh1_bpp_handle_input(BinaryPacketProtocol *bpp) @@ -210,6 +207,20 @@ static void ssh1_bpp_handle_input(BinaryPacketProtocol *bpp) if (type == SSH1_MSG_DISCONNECT) s->bpp.seen_disconnect = TRUE; + + if (type == SSH1_SMSG_SUCCESS && s->pending_compression_request) { + assert(!s->compctx); + assert(!s->decompctx); + + s->compctx = ssh_compressor_new(&ssh_zlib); + s->decompctx = ssh_decompressor_new(&ssh_zlib); + + s->pending_compression_request = FALSE; + } + + if (type == SSH1_SMSG_FAILURE && s->pending_compression_request) { + s->pending_compression_request = FALSE; + } } } crFinishV; diff --git a/sshbpp.h b/sshbpp.h index a1c9d0ac..8fc02960 100644 --- a/sshbpp.h +++ b/sshbpp.h @@ -34,7 +34,11 @@ BinaryPacketProtocol *ssh1_bpp_new(void); void ssh1_bpp_new_cipher(BinaryPacketProtocol *bpp, const struct ssh1_cipheralg *cipher, const void *session_key); -void ssh1_bpp_start_compression(BinaryPacketProtocol *bpp); +/* requested_compression() notifies the SSH-1 BPP that we've just sent + * a request to enable compression, which means that on receiving the + * next SSH1_SMSG_SUCCESS or SSH1_SMSG_FAILURE message, it should set + * up zlib compression if it was SUCCESS. */ +void ssh1_bpp_requested_compression(BinaryPacketProtocol *bpp); /* * Structure that tracks how much data is sent and received, for