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.

[originally from svn r10039]
This commit is contained in:
Simon Tatham 2013-09-08 13:20:49 +00:00
Родитель 8e7b0d0e4b
Коммит 043a762b5f
1 изменённых файлов: 71 добавлений и 15 удалений

86
ssh.c
Просмотреть файл

@ -4322,6 +4322,7 @@ void sshfwd_unclean_close(struct ssh_channel *c, const char *err)
break; break;
} }
c->type = CHAN_ZOMBIE; c->type = CHAN_ZOMBIE;
c->pending_eof = FALSE; /* this will confuse a zombie channel */
ssh2_channel_check_close(c); ssh2_channel_check_close(c);
} }
@ -7018,6 +7019,15 @@ static void ssh2_channel_check_close(struct ssh_channel *c)
Ssh ssh = c->ssh; Ssh ssh = c->ssh;
struct Packet *pktout; 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) || if ((!((CLOSES_SENT_EOF | CLOSES_RCVD_EOF) & ~c->closes) ||
c->type == CHAN_ZOMBIE) && c->type == CHAN_ZOMBIE) &&
!c->v.v2.chanreq_head && !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); c = ssh2_channel_msg(ssh, pktin);
if (!c) if (!c)
return; return;
if (c->type != CHAN_SOCKDATA_DORMANT) assert(c->halfopen); /* ssh2_channel_msg will have enforced this */
return; /* dunno why they're confirming this */
c->remoteid = ssh_pkt_getuint32(pktin); c->remoteid = ssh_pkt_getuint32(pktin);
c->halfopen = FALSE; c->halfopen = FALSE;
c->type = CHAN_SOCKDATA;
c->v.v2.remwindow = ssh_pkt_getuint32(pktin); c->v.v2.remwindow = ssh_pkt_getuint32(pktin);
c->v.v2.remmaxpkt = 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) 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) 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; char *reason_string;
int reason_length; int reason_length;
struct ssh_channel *c; struct ssh_channel *c;
c = ssh2_channel_msg(ssh, pktin); c = ssh2_channel_msg(ssh, pktin);
if (!c) if (!c)
return; return;
if (c->type != CHAN_SOCKDATA_DORMANT) assert(c->halfopen); /* ssh2_channel_msg will have enforced this */
return; /* dunno why they're failing this */
reason_code = ssh_pkt_getuint32(pktin); if (c->type == CHAN_SOCKDATA_DORMANT) {
if (reason_code >= lenof(reasons)) reason_code = ssh_pkt_getuint32(pktin);
reason_code = 0; /* ensure reasons[reason_code] in range */ if (reason_code >= lenof(reasons))
ssh_pkt_getstring(pktin, &reason_string, &reason_length); reason_code = 0; /* ensure reasons[reason_code] in range */
logeventf(ssh, "Forwarded connection refused by server: %s [%.*s]", ssh_pkt_getstring(pktin, &reason_string, &reason_length);
reasons[reason_code], reason_length, reason_string); 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); del234(ssh->channels, c);
sfree(c); sfree(c);