Handle socket errors on half-open channels.

Anthony Ho reports that this can occur naturally in some situation
involving Windows 8 + IE 11 and dynamic port forwarding: apparently we
get through the SOCKS negotiation, send our CHANNEL_OPEN, and then
*immediately* suffer a local WSAECONNABORTED error before the server
has sent back its OPEN_CONFIRMATION or OPEN_FAILURE. In this situation
ssh2_channel_check_close was failing to notice that the channel didn't
yet have a valid server id, and sending out a CHANNEL_CLOSE anyway
containing 32 bits of uninitialised nonsense.

We now handle this by turning our half-open CHAN_SOCKDATA_DORMANT into
a half-open CHAN_ZOMBIE, which means in turn that our handler
functions for OPEN_CONFIRMATION and OPEN_FAILURE have to recognise and
handle that case, the former by immediately initiating channel closure
once we _do_ have the channel's server id to do it with.

git-svn-id: http://svn.tartarus.org/sgt/putty@10039 cda61777-01e9-0310-a592-d414129be87e

diff --git a/ssh.c b/ssh.c
index 7fabb8394bff3d6c59c3452e7408109031a2c624..cd672979b6f554afcd1cc0c5325b57c79db47cd4 100644 (file)
--- a/ssh.c
+++ b/ssh.c
@@ -4322,6 +4322,7 @@ void sshfwd_unclean_close(struct ssh_channel *c, const char *err)
         break;
     }
     c->type = CHAN_ZOMBIE;
+    c->pending_eof = FALSE;   /* this will confuse a zombie channel */
 
     ssh2_channel_check_close(c);
 }
@@ -7018,6 +7019,15 @@ static void ssh2_channel_check_close(struct ssh_channel *c)
     Ssh ssh = c->ssh;
     struct Packet *pktout;
 
+    if (c->halfopen) {
+        /*
+         * If we've sent out our own CHANNEL_OPEN but not yet seen
+         * either OPEN_CONFIRMATION or OPEN_FAILURE in response, then
+         * it's too early to be sending close messages of any kind.
+         */
+        return;
+    }
+
     if ((!((CLOSES_SENT_EOF | CLOSES_RCVD_EOF) & ~c->closes) ||
         c->type == CHAN_ZOMBIE) &&
        !c->v.v2.chanreq_head &&
@@ -7159,17 +7169,42 @@ static void ssh2_msg_channel_open_confirmation(Ssh ssh, struct Packet *pktin)
     c = ssh2_channel_msg(ssh, pktin);
     if (!c)
        return;
-    if (c->type != CHAN_SOCKDATA_DORMANT)
-       return;                        /* dunno why they're confirming this */
+    assert(c->halfopen); /* ssh2_channel_msg will have enforced this */
     c->remoteid = ssh_pkt_getuint32(pktin);
     c->halfopen = FALSE;
-    c->type = CHAN_SOCKDATA;
     c->v.v2.remwindow = ssh_pkt_getuint32(pktin);
     c->v.v2.remmaxpkt = ssh_pkt_getuint32(pktin);
-    if (c->u.pfd.s)
-       pfd_confirm(c->u.pfd.s);
+
+    if (c->type == CHAN_SOCKDATA_DORMANT) {
+        c->type = CHAN_SOCKDATA;
+        if (c->u.pfd.s)
+            pfd_confirm(c->u.pfd.s);
+    } else if (c->type == CHAN_ZOMBIE) {
+        /*
+         * This case can occur if a local socket error occurred
+         * between us sending out CHANNEL_OPEN and receiving
+         * OPEN_CONFIRMATION. In this case, all we can do is
+         * immediately initiate close proceedings now that we know the
+         * server's id to put in the close message.
+         */
+        ssh2_channel_check_close(c);
+    } else {
+        /*
+         * We never expect to receive OPEN_CONFIRMATION for any
+         * *other* channel type (since only local-to-remote port
+         * forwardings cause us to send CHANNEL_OPEN after the main
+         * channel is live - all other auxiliary channel types are
+         * initiated from the server end). It's safe to enforce this
+         * by assertion rather than by ssh_disconnect, because the
+         * real point is that we never constructed a half-open channel
+         * structure in the first place with any type other than the
+         * above.
+         */
+        assert(!"Funny channel type in ssh2_msg_channel_open_confirmation");
+    }
+
     if (c->pending_eof)
-        ssh_channel_try_eof(c);
+        ssh_channel_try_eof(c);        /* in case we had a pending EOF */
 }
 
 static void ssh2_msg_channel_open_failure(Ssh ssh, struct Packet *pktin)
@@ -7185,20 +7220,41 @@ static void ssh2_msg_channel_open_failure(Ssh ssh, struct Packet *pktin)
     char *reason_string;
     int reason_length;
     struct ssh_channel *c;
+
     c = ssh2_channel_msg(ssh, pktin);
     if (!c)
        return;
-    if (c->type != CHAN_SOCKDATA_DORMANT)
-       return;                        /* dunno why they're failing this */
+    assert(c->halfopen); /* ssh2_channel_msg will have enforced this */
 
-    reason_code = ssh_pkt_getuint32(pktin);
-    if (reason_code >= lenof(reasons))
-       reason_code = 0; /* ensure reasons[reason_code] in range */
-    ssh_pkt_getstring(pktin, &reason_string, &reason_length);
-    logeventf(ssh, "Forwarded connection refused by server: %s [%.*s]",
-             reasons[reason_code], reason_length, reason_string);
+    if (c->type == CHAN_SOCKDATA_DORMANT) {
+        reason_code = ssh_pkt_getuint32(pktin);
+        if (reason_code >= lenof(reasons))
+            reason_code = 0; /* ensure reasons[reason_code] in range */
+        ssh_pkt_getstring(pktin, &reason_string, &reason_length);
+        logeventf(ssh, "Forwarded connection refused by server: %s [%.*s]",
+                  reasons[reason_code], reason_length, reason_string);
 
-    pfd_close(c->u.pfd.s);
+        pfd_close(c->u.pfd.s);
+    } else if (c->type == CHAN_ZOMBIE) {
+        /*
+         * This case can occur if a local socket error occurred
+         * between us sending out CHANNEL_OPEN and receiving
+         * OPEN_FAILURE. In this case, we need do nothing except allow
+         * the code below to throw the half-open channel away.
+         */
+    } else {
+        /*
+         * We never expect to receive OPEN_FAILURE for any *other*
+         * channel type (since only local-to-remote port forwardings
+         * cause us to send CHANNEL_OPEN after the main channel is
+         * live - all other auxiliary channel types are initiated from
+         * the server end). It's safe to enforce this by assertion
+         * rather than by ssh_disconnect, because the real point is
+         * that we never constructed a half-open channel structure in
+         * the first place with any type other than the above.
+         */
+        assert(!"Funny channel type in ssh2_msg_channel_open_failure");
+    }
 
     del234(ssh->channels, c);
     sfree(c);