nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Code Issues Projects Releases Wiki Activity

Restructure SSH-1 compression again.

Browse Source 
Having redesigned it a few days ago in commit 562cdd4df, I'm changing
it again, this time to fix a potential race condition on the _output_
side: the last change was intended to cope with a server sending an
asynchronous message like IGNORE immediately after enabling
compression, and this one fixes the case in which _we_ happen to
decide to send an IGNORE while a compression request is still pending.

I couldn't fix this until after the BPP was reorganised to have an
explicit output queue of packets, but now it does, I can simply defer
processing that queue on to the output raw-data bufchain if we're
waiting for a compression request to be answered. Once it is answered,
the BPP can release any pending packets.
This commit is contained in:
Simon Tatham 2018-09-22 11:09:15 +01:00
parent 3074440040
commit 344ec3aec5
2 changed files with 59 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ssh.c
View File

@ -3125,7 +3125,6 @@ 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) {
@ -3134,6 +3133,14 @@ static void do_ssh1_connection(void *vctx)
} else if (pktin->type == SSH1_SMSG_FAILURE) {
c_write_str(ssh, "Server refused to compress\r\n");
} else {
/*
* We don't have to actually do anything here: the SSH-1
* BPP will take care of automatically starting the
* compression, by recognising our outgoing request packet
* and the success response. (Horrible, but it's the
* easiest way to avoid race conditions if other packets
* cross in transit.)
*/
logevent("Started zlib (RFC1950) compression");
}
}

71
ssh1bpp.c
View File

@ -86,15 +86,6 @@ void ssh1_bpp_new_cipher(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);
s->pending_compression_request = TRUE;
}
#define BPP_READ(ptr, len) do \
{ \
crMaybeWaitUntilV(bufchain_try_fetch_consume( \
@ -212,21 +203,39 @@ static void ssh1_bpp_handle_input(BinaryPacketProtocol *bpp)
int type = s->pktin->type;
s->pktin = NULL;
if (type == SSH1_MSG_DISCONNECT)
switch (type) {
case SSH1_MSG_DISCONNECT:
s->bpp.seen_disconnect = TRUE;
break;
if (type == SSH1_SMSG_SUCCESS && s->pending_compression_request) {
assert(!s->compctx);
assert(!s->decompctx);
case SSH1_SMSG_SUCCESS:
case SSH1_SMSG_FAILURE:
if (s->pending_compression_request) {
/*
* This is the response to
* SSH1_CMSG_REQUEST_COMPRESSION.
*/
if (type == SSH1_SMSG_SUCCESS) {
/*
* If the response was positive, start
* compression.
*/
assert(!s->compctx);
assert(!s->decompctx);
s->compctx = ssh_compressor_new(&ssh_zlib);
s->decompctx = ssh_decompressor_new(&ssh_zlib);
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;
/*
* Either way, cancel the pending flag, and
* schedule a run of our output side in case we
* had any packets queued up in the meantime.
*/
s->pending_compression_request = FALSE;
queue_idempotent_callback(&s->bpp.ic_out_pq);
}
break;
}
}
}
@ -300,9 +309,31 @@ static void ssh1_bpp_handle_output(BinaryPacketProtocol *bpp)
struct ssh1_bpp_state *s = FROMFIELD(bpp, struct ssh1_bpp_state, bpp);
PktOut *pkt;
if (s->pending_compression_request) {
/*
* Don't send any output packets while we're awaiting a
* response to SSH1_CMSG_REQUEST_COMPRESSION, because if they
* cross over in transit with the responding SSH1_CMSG_SUCCESS
* then the other end could decode them with the wrong
* compression settings.
*/
return;
}
while ((pkt = pq_pop(&s->bpp.out_pq)) != NULL) {
int type = pkt->type;
ssh1_bpp_format_packet(s, pkt);
ssh_free_pktout(pkt);
if (type == SSH1_CMSG_REQUEST_COMPRESSION) {
/*
* When we see the actual compression request go past, set
* the pending flag, and stop processing packets this
* time.
*/
s->pending_compression_request = TRUE;
break;
}
}
}