mirror of
https://git.tartarus.org/simon/putty.git
synced 2025-01-09 17:38:00 +00:00
Implement this year's consensus on CHANNEL_FAILURE vs CHANNEL_CLOSE.
We now expect that after the server has sent us CHANNEL_CLOSE, we should not expect to see any replies to our outstanding channel requests, and conversely after we have sent CHANNEL_CLOSE we avoid sending any reply to channel requests from the server. This was the consensus among implementors discussing the problem on ietf-ssh in April 2014. To cope with current OpenSSH's (and perhaps other servers we don't know about yet) willingness to send request replies after CHANNEL_CLOSE, I introduce a bug-compatibility flag which is detected for every OpenSSH version up to and including the current 6.6 - but not beyond, since https://bugzilla.mindrot.org/show_bug.cgi?id=1818 promises that 6.7 will also implement the new consensus behaviour. [originally from svn r10200]
This commit is contained in:
Simon Tatham
parent
3d337b49ef
commit
aaaf70a0fc
3
config.c
3
config.c
@ -2490,6 +2490,9 @@ void setup_config_box(struct controlbox *b, int midsession,
|
||||
ctrl_droplist(s, "Ignores SSH-2 maximum packet size", 'x', 20,
|
||||
HELPCTX(ssh_bugs_maxpkt2),
|
||||
sshbug_handler, I(CONF_sshbug_maxpkt2));
|
||||
ctrl_droplist(s, "Replies to channel requests after channel close",
|
||||
'q', 20, HELPCTX(ssh_bugs_chanreq),
|
||||
sshbug_handler, I(CONF_sshbug_chanreq));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -3294,6 +3294,31 @@ believes the server has this bug, it will never send its
|
||||
\cq{winadj@putty.projects.tartarus.org} request, and will make do
|
||||
without its timing data.
|
||||
|
||||
\S{config-ssh-bug-chanreq} \q{Replies to channel requests after channel close}
|
||||
|
||||
\cfg{winhelp-topic}{ssh.bugs.chanreq}
|
||||
|
||||
The SSH protocol as published in RFC 4254 has an ambiguity which
|
||||
arises if one side of a connection tries to close a channel, while the
|
||||
other side simultaneously sends a request within the channel and asks
|
||||
for a reply. RFC 4254 leaves it unclear whether the closing side
|
||||
should reply to the channel request after having announced its
|
||||
intention to close the channel.
|
||||
|
||||
Discussion on the \cw{ietf-ssh} mailing list in April 2014 formed a
|
||||
clear consensus that the right answer is no. However, because of the
|
||||
ambiguity in the specification, some SSH servers have implemented the
|
||||
other policy; for example,
|
||||
\W{https://bugzilla.mindrot.org/show_bug.cgi?id=1818}{OpenSSH used to}
|
||||
until it was fixed.
|
||||
|
||||
Because PuTTY sends channel requests with the \q{want reply} flag
|
||||
throughout channels' lifetime (see \k{config-ssh-bug-winadj}), it's
|
||||
possible that when connecting to such a server it might receive a
|
||||
reply to a request after it thinks the channel has entirely closed,
|
||||
and terminate with an error along the lines of \q{Received
|
||||
\cw{SSH2_MSG_CHANNEL_FAILURE} for nonexistent channel 256}.
|
||||
|
||||
\H{config-serial} The Serial panel
|
||||
|
||||
The \i{Serial} panel allows you to configure options that only apply
|
||||
|
||||
1
putty.h
1
putty.h
@ -838,6 +838,7 @@ void cleanup_exit(int);
|
||||
X(INT, NONE, sshbug_maxpkt2) \
|
||||
X(INT, NONE, sshbug_ignore2) \
|
||||
X(INT, NONE, sshbug_winadj) \
|
||||
X(INT, NONE, sshbug_chanreq) \
|
||||
/* \
|
||||
* ssh_simple means that we promise never to open any channel \
|
||||
* other than the main one, which means it can safely use a very \
|
||||
|
||||
@ -626,6 +626,7 @@ void save_open_settings(void *sesskey, Conf *conf)
|
||||
write_setting_i(sesskey, "BugRekey2", 2-conf_get_int(conf, CONF_sshbug_rekey2));
|
||||
write_setting_i(sesskey, "BugMaxPkt2", 2-conf_get_int(conf, CONF_sshbug_maxpkt2));
|
||||
write_setting_i(sesskey, "BugWinadj", 2-conf_get_int(conf, CONF_sshbug_winadj));
|
||||
write_setting_i(sesskey, "BugChanReq", 2-conf_get_int(conf, CONF_sshbug_chanreq));
|
||||
write_setting_i(sesskey, "StampUtmp", conf_get_int(conf, CONF_stamp_utmp));
|
||||
write_setting_i(sesskey, "LoginShell", conf_get_int(conf, CONF_login_shell));
|
||||
write_setting_i(sesskey, "ScrollbarOnLeft", conf_get_int(conf, CONF_scrollbar_on_left));
|
||||
@ -970,6 +971,7 @@ void load_open_settings(void *sesskey, Conf *conf)
|
||||
i = gppi_raw(sesskey, "BugRekey2", 0); conf_set_int(conf, CONF_sshbug_rekey2, 2-i);
|
||||
i = gppi_raw(sesskey, "BugMaxPkt2", 0); conf_set_int(conf, CONF_sshbug_maxpkt2, 2-i);
|
||||
i = gppi_raw(sesskey, "BugWinadj", 0); conf_set_int(conf, CONF_sshbug_winadj, 2-i);
|
||||
i = gppi_raw(sesskey, "BugChanReq", 0); conf_set_int(conf, CONF_sshbug_chanreq, 2-i);
|
||||
conf_set_int(conf, CONF_ssh_simple, FALSE);
|
||||
gppi(sesskey, "StampUtmp", 1, conf, CONF_stamp_utmp);
|
||||
gppi(sesskey, "LoginShell", 1, conf, CONF_login_shell);
|
||||
|
||||
51
ssh.c
51
ssh.c
@ -75,6 +75,7 @@ static const char *const ssh2_disconnect_reasons[] = {
|
||||
#define BUG_SSH2_MAXPKT 256
|
||||
#define BUG_CHOKES_ON_SSH2_IGNORE 512
|
||||
#define BUG_CHOKES_ON_WINADJ 1024
|
||||
#define BUG_SENDS_LATE_REQUEST_REPLY 2048
|
||||
|
||||
/*
|
||||
* Codes for terminal modes.
|
||||
@ -2812,6 +2813,19 @@ static void ssh_detect_bugs(Ssh ssh, char *vstring)
|
||||
ssh->remote_bugs |= BUG_CHOKES_ON_WINADJ;
|
||||
logevent("We believe remote version has winadj bug");
|
||||
}
|
||||
|
||||
if (conf_get_int(ssh->conf, CONF_sshbug_chanreq) == FORCE_ON ||
|
||||
(conf_get_int(ssh->conf, CONF_sshbug_chanreq) == AUTO &&
|
||||
(wc_match("OpenSSH_[2-5].*", imp) ||
|
||||
wc_match("OpenSSH_6.[0-6]*", imp)))) {
|
||||
/*
|
||||
* These versions have the SSH-2 channel request bug. 6.7 and
|
||||
* above do not:
|
||||
* https://bugzilla.mindrot.org/show_bug.cgi?id=1818
|
||||
*/
|
||||
ssh->remote_bugs |= BUG_SENDS_LATE_REQUEST_REPLY;
|
||||
logevent("We believe remote version has SSH-2 channel request bug");
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
@ -7119,10 +7133,11 @@ static void ssh2_queue_chanreq_handler(struct ssh_channel *c,
|
||||
* request-specific data added and be sent. Note that if a handler is
|
||||
* provided, it's essential that the request actually be sent.
|
||||
*
|
||||
* The handler will usually be passed the response packet in pktin.
|
||||
* If pktin is NULL, this means that no reply will ever be forthcoming
|
||||
* (e.g. because the entire connection is being destroyed) and the
|
||||
* handler should free any storage it's holding.
|
||||
* The handler will usually be passed the response packet in pktin. If
|
||||
* pktin is NULL, this means that no reply will ever be forthcoming
|
||||
* (e.g. because the entire connection is being destroyed, or because
|
||||
* the server initiated channel closure before we saw the response)
|
||||
* and the handler should free any storage it's holding.
|
||||
*/
|
||||
static struct Packet *ssh2_chanreq_init(struct ssh_channel *c, char *type,
|
||||
cchandler_fn_t handler, void *ctx)
|
||||
@ -7612,6 +7627,21 @@ static void ssh2_msg_channel_close(Ssh ssh, struct Packet *pktin)
|
||||
*/
|
||||
ssh2_channel_got_eof(c);
|
||||
|
||||
if (!(ssh->remote_bugs & BUG_SENDS_LATE_REQUEST_REPLY)) {
|
||||
/*
|
||||
* It also means we stop expecting to see replies to any
|
||||
* outstanding channel requests, so clean those up too.
|
||||
* (ssh_chanreq_init will enforce by assertion that we don't
|
||||
* subsequently put anything back on this list.)
|
||||
*/
|
||||
while (c->v.v2.chanreq_head) {
|
||||
struct outstanding_channel_request *ocr = c->v.v2.chanreq_head;
|
||||
ocr->handler(c, NULL, ocr->ctx);
|
||||
c->v.v2.chanreq_head = ocr->next;
|
||||
sfree(ocr);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* And we also send an outgoing EOF, if we haven't already, on the
|
||||
* assumption that CLOSE is a pretty forceful announcement that
|
||||
@ -7787,6 +7817,16 @@ static void ssh2_msg_channel_request(Ssh ssh, struct Packet *pktin)
|
||||
ssh_pkt_getstring(pktin, &type, &typelen);
|
||||
want_reply = ssh2_pkt_getbool(pktin);
|
||||
|
||||
if (c->closes & CLOSES_SENT_CLOSE) {
|
||||
/*
|
||||
* We don't reply to channel requests after we've sent
|
||||
* CHANNEL_CLOSE for the channel, because our reply might
|
||||
* cross in the network with the other side's CHANNEL_CLOSE
|
||||
* and arrive after they have wound the channel up completely.
|
||||
*/
|
||||
want_reply = FALSE;
|
||||
}
|
||||
|
||||
/*
|
||||
* Having got the channel number, we now look at
|
||||
* the request type string to see if it's something
|
||||
@ -8416,7 +8456,8 @@ static void ssh2_msg_authconn(Ssh ssh, struct Packet *pktin)
|
||||
static void ssh2_response_authconn(struct ssh_channel *c, struct Packet *pktin,
|
||||
void *ctx)
|
||||
{
|
||||
do_ssh2_authconn(c->ssh, NULL, 0, pktin);
|
||||
if (pktin)
|
||||
do_ssh2_authconn(c->ssh, NULL, 0, pktin);
|
||||
}
|
||||
|
||||
static void do_ssh2_authconn(Ssh ssh, unsigned char *in, int inlen,
|
||||
|
||||
@ -146,6 +146,7 @@
|
||||
#define WINHELP_CTX_ssh_bugs_rekey2 "ssh.bugs.rekey2:config-ssh-bug-rekey"
|
||||
#define WINHELP_CTX_ssh_bugs_maxpkt2 "ssh.bugs.maxpkt2:config-ssh-bug-maxpkt2"
|
||||
#define WINHELP_CTX_ssh_bugs_winadj "ssh.bugs.winadj:config-ssh-bug-winadj"
|
||||
#define WINHELP_CTX_ssh_bugs_chanreq "ssh.bugs.winadj:config-ssh-bug-chanreq"
|
||||
#define WINHELP_CTX_serial_line "serial.line:config-serial-line"
|
||||
#define WINHELP_CTX_serial_speed "serial.speed:config-serial-speed"
|
||||
#define WINHELP_CTX_serial_databits "serial.databits:config-serial-databits"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user