New bug workaround: KEXINIT filtering.

We've occasionally had reports of SSH servers disconnecting as soon as
they receive PuTTY's KEXINIT. I think all such reports have involved
the kind of simple ROM-based SSH server software you find in small
embedded devices.

I've never been able to prove it, but I've always suspected that one
possible cause of this is simply that PuTTY's KEXINIT is _too long_,
either in number of algorithms listed or in total length (especially
given all the ones that end in @very.long.domain.name suffixes).

If I'm right about either of those being the cause, then it's just
become even more likely to happen, because of all the extra
Diffie-Hellman groups and GSSAPI algorithms we just threw into our
already-long list in the previous few commits.

A workaround I've had in mind for ages is to wait for the server's
KEXINIT, and then filter our own down to just the algorithms the
server also mentioned. Then our KEXINIT is no longer than that of the
server, and hence, presumably fits in whatever buffer it has. So I've
implemented that workaround, in anticipation of it being needed in the
near future.

(Well ... it's not _quite_ true that our KEXINIT is at most the same
length as the server. In fact I had to leave in one KEXINIT item that
won't match anything in the server's list, namely "ext-info-c" which
gates access to SHA-2 based RSA. So if we turn out to support
absolutely everything on all the server's lists, then our KEXINIT
would be a few bytes longer than the server's, even with this
workaround. But that would only cause trouble if the server's outgoing
KEXINIT was skating very close to whatever buffer size it has for the
incoming one, and I'm guessing that's not very likely.)

((Another possible cause of this kind of disconnection would be a
server that simply objects to seeing any KEXINIT string it doesn't
know how to speak. But _surely_ no such server would have survived
initial testing against any full-featured client at all!))

config.c

@@ -3154,6 +3154,10 @@ void setup_config_box(struct controlbox *b, bool midsession,
                           HELPCTX(ssh_bugs_dropstart),
                           sshbug_handler_manual_only,
                           I(CONF_sshbug_dropstart));
+            ctrl_droplist(s, "Chokes on PuTTY's full KEXINIT", 'p', 20,
+                          HELPCTX(ssh_bugs_filter_kexinit),
+                          sshbug_handler_manual_only,
+                          I(CONF_sshbug_filter_kexinit));
 
             ctrl_settitle(b, "Connection/SSH/More bugs",
                           "Further workarounds for SSH server bugs");

doc/config.but

@@ -3573,6 +3573,32 @@ auto-detection relies on the version string in the server's greeting,
 and PuTTY has to decide whether to expect this bug \e{before} it sees
 the server's greeting. So this is a manual workaround only.
 
+\S{config-ssh-bug-filter-kexinit} \q{Chokes on PuTTY's full \cw{KEXINIT}}
+
+At the start of an SSH connection, the client and server exchange long
+messages of type \cw{SSH_MSG_KEXINIT}, containing lists of all the
+cryptographic algorithms they're prepared to use. This is used to
+negotiate a set of algorithms that both ends can speak.
+
+Occasionally, a badly written server might have a length limit on the
+list it's prepared to receive, and refuse to make a connection simply
+because PuTTY is giving it too many choices.
+
+A workaround is to enable this flag, which will make PuTTY wait to
+send \cw{KEXINIT} until after it receives the one from the server, and
+then filter its own \cw{KEXINIT} to leave out any algorithm the server
+doesn't also announce support for. This will generally make PuTTY's
+\cw{KEXINIT} at most the size of the server's, and will otherwise make
+no difference to the algorithm negotiation.
+
+This flag is a minor violation of the SSH protocol, because both sides
+are supposed to send \cw{KEXINIT} proactively. It still works provided
+\e{one} side sends its \cw{KEXINIT} without waiting, but if both
+client and server waited for the other one to speak first, the
+connection would deadlock. We don't know of any servers that do this,
+but if there is one, then this flag will make PuTTY unable to speak to
+them at all.
+
 \S{config-ssh-bug-sig} \q{Requires padding on SSH-2 \i{RSA} \i{signatures}}
 
 Versions below 3.3 of \i{OpenSSH} require SSH-2 RSA signatures to be

putty.h

@@ -2022,6 +2022,7 @@ NORETURN void cleanup_exit(int);
     X(INT, NONE, sshbug_winadj) \
     X(INT, NONE, sshbug_chanreq) \
     X(INT, NONE, sshbug_dropstart) \
+    X(INT, NONE, sshbug_filter_kexinit) \
     /*                                                                \
      * ssh_simple means that we promise never to open any channel     \
      * other than the main one, which means it can safely use a very  \

settings.c

@@ -778,6 +778,7 @@ void save_open_settings(settings_w *sesskey, Conf *conf)
     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, "BugDropStart", 2-conf_get_int(conf, CONF_sshbug_dropstart));
+    write_setting_i(sesskey, "BugFilterKexinit", 2-conf_get_int(conf, CONF_sshbug_filter_kexinit));
     write_setting_b(sesskey, "StampUtmp", conf_get_bool(conf, CONF_stamp_utmp));
     write_setting_b(sesskey, "LoginShell", conf_get_bool(conf, CONF_login_shell));
     write_setting_b(sesskey, "ScrollbarOnLeft", conf_get_bool(conf, CONF_scrollbar_on_left));
@@ -1257,6 +1258,7 @@ void load_open_settings(settings_r *sesskey, Conf *conf)
     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);
     i = gppi_raw(sesskey, "BugDropStart", 1); conf_set_int(conf, CONF_sshbug_dropstart, 2-i);
+    i = gppi_raw(sesskey, "BugFilterKexinit", 1); conf_set_int(conf, CONF_sshbug_filter_kexinit, 2-i);
     conf_set_bool(conf, CONF_ssh_simple, false);
     gppb(sesskey, "StampUtmp", true, conf, CONF_stamp_utmp);
     gppb(sesskey, "LoginShell", true, conf, CONF_login_shell);

ssh.h

@@ -1880,6 +1880,7 @@ void old_keyfile_warning(void);
     X(BUG_CHOKES_ON_WINADJ)                     \
     X(BUG_SENDS_LATE_REQUEST_REPLY)             \
     X(BUG_SSH2_OLDGEX)                          \
+    X(BUG_REQUIRES_FILTERED_KEXINIT)            \
     /* end of list */
 #define TMP_DECLARE_LOG2_ENUM(thing) log2_##thing,
 enum { SSH_IMPL_BUG_LIST(TMP_DECLARE_LOG2_ENUM) };

ssh/transport2.c

@@ -1196,6 +1196,75 @@ static bool ssh2_scan_kexinits(
     return true;
 }
 
+static strbuf *write_filtered_kexinit(struct ssh2_transport_state *s)
+{
+    strbuf *pktout = strbuf_new();
+    BinarySource osrc[1], isrc[1];
+    BinarySource_BARE_INIT(
+        osrc, s->outgoing_kexinit->u, s->outgoing_kexinit->len);
+    BinarySource_BARE_INIT(
+        isrc, s->incoming_kexinit->u, s->incoming_kexinit->len);
+
+    /* Skip the packet type bytes from both packets */
+    get_byte(osrc);
+    get_byte(isrc);
+
+    /* Copy our cookie into the real output packet; skip their cookie */
+    put_datapl(pktout, get_data(osrc, 16));
+    get_data(isrc, 16);
+
+    /*
+     * Now we expect NKEXLIST+2 name-lists. We write into the outgoing
+     * packet a subset of our intended outgoing one, containing only
+     * names mentioned in the incoming out.
+     *
+     * NKEXLIST+2 because for this purpose we treat the 'languages'
+     * lists the same as the rest. In the rest of this code base we
+     * ignore those.
+     */
+    strbuf *out = strbuf_new();
+    for (size_t i = 0; i < NKEXLIST+2; i++) {
+        strbuf_clear(out);
+        ptrlen olist = get_string(osrc), ilist = get_string(isrc);
+        for (ptrlen oword; get_commasep_word(&olist, &oword) ;) {
+            ptrlen ilist_copy = ilist;
+            bool add = false;
+            for (ptrlen iword; get_commasep_word(&ilist_copy, &iword) ;) {
+                if (ptrlen_eq_ptrlen(oword, iword)) {
+                    /* Found this word in the incoming list. */
+                    add = true;
+                    break;
+                }
+            }
+
+            if (i == KEXLIST_KEX && ptrlen_eq_string(oword, "ext-info-c")) {
+                /* Special case: this will _never_ match anything from the
+                 * server, and we need it to enable SHA-2 based RSA.
+                 *
+                 * If this ever turns out to confuse any server all by
+                 * itself then I suppose we'll need an even more
+                 * draconian bug flag to exclude that too. (Obv, such
+                 * a server wouldn't be able to speak SHA-2 RSA
+                 * anyway.) */
+                add = true;
+            }
+
+            if (add)
+                add_to_commasep_pl(out, oword);
+        }
+        put_stringpl(pktout, ptrlen_from_strbuf(out));
+    }
+    strbuf_free(out);
+
+    /*
+     * Finally, copy the remaining parts of our intended KEXINIT.
+     */
+    put_bool(pktout, get_bool(osrc));  /* first-kex-packet-follows */
+    put_uint32(pktout, get_uint32(osrc)); /* reserved word */
+
+    return pktout;
+}
+
 void ssh2transport_finalise_exhash(struct ssh2_transport_state *s)
 {
     put_datapl(s->exhash, ptrlen_from_strbuf(s->kex_shared_secret));
@@ -1304,12 +1373,14 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
     put_uint32(s->outgoing_kexinit, 0);             /* reserved */
 
     /*
-     * Send our KEXINIT.
+     * Send our KEXINIT (in the normal case).
      */
-    pktout = ssh_bpp_new_pktout(s->ppl.bpp, SSH2_MSG_KEXINIT);
-    put_data(pktout, s->outgoing_kexinit->u + 1,
-             s->outgoing_kexinit->len - 1); /* omit initial packet type byte */
-    pq_push(s->ppl.out_pq, pktout);
+    if (!(s->ppl.remote_bugs & BUG_REQUIRES_FILTERED_KEXINIT)) {
+        pktout = ssh_bpp_new_pktout(s->ppl.bpp, SSH2_MSG_KEXINIT);
+        put_data(pktout, s->outgoing_kexinit->u + 1,
+                 s->outgoing_kexinit->len - 1); /* omit type byte */
+        pq_push(s->ppl.out_pq, pktout);
+    }
 
     /*
      * Flag that KEX is in progress.
@@ -1331,6 +1402,27 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
     put_byte(s->incoming_kexinit, SSH2_MSG_KEXINIT);
     put_data(s->incoming_kexinit, get_ptr(pktin), get_avail(pktin));
 
+    /*
+     * If we've delayed sending our KEXINIT so as to filter it down to
+     * only things the server won't choke on, send ours now.
+     */
+    if (s->ppl.remote_bugs & BUG_REQUIRES_FILTERED_KEXINIT) {
+        strbuf *sb = write_filtered_kexinit(s);
+
+        /* Send that data as a packet */
+        pktout = ssh_bpp_new_pktout(s->ppl.bpp, SSH2_MSG_KEXINIT);
+        put_datapl(pktout, ptrlen_from_strbuf(sb));
+        pq_push(s->ppl.out_pq, pktout);
+
+        /* And also replace our previous outgoing KEXINIT, since the
+         * host key signature will be validated against this reduced
+         * one. */
+        strbuf_shrink_to(s->outgoing_kexinit, 1); /* keep the type byte */
+        put_datapl(s->outgoing_kexinit, ptrlen_from_strbuf(sb));
+
+        strbuf_free(sb);
+    }
+
     /*
      * Work through the two KEXINIT packets in parallel to find the
      * selected algorithm identifiers.

ssh/verstring.c

@@ -606,6 +606,12 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s)
         bpp_logevent("We believe remote version has SSH-2 "
                      "channel request bug");
     }
+
+    if (conf_get_int(s->conf, CONF_sshbug_filter_kexinit) == FORCE_ON) {
+        s->remote_bugs |= BUG_REQUIRES_FILTERED_KEXINIT;
+        bpp_logevent("We believe remote version requires us to "
+                     "filter our KEXINIT");
+    }
 }
 
 const char *ssh_verstring_get_remote(BinaryPacketProtocol *bpp)

windows/help.h

@@ -170,6 +170,7 @@ typedef const char *HelpCtx;
 #define WINHELP_CTX_ssh_bugs_chanreq "config-ssh-bug-chanreq"
 #define WINHELP_CTX_ssh_bugs_oldgex2 "config-ssh-bug-oldgex2"
 #define WINHELP_CTX_ssh_bugs_dropstart "config-ssh-bug-dropstart"
+#define WINHELP_CTX_ssh_bugs_filter_kexinit "config-ssh-bug-filter-kexinit"
 #define WINHELP_CTX_serial_line "config-serial-line"
 #define WINHELP_CTX_serial_speed "config-serial-speed"
 #define WINHELP_CTX_serial_databits "config-serial-databits"