1
0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00

Fix check for "ext-info-s".

ssh2_scan_kexinits must check to see whether it's behaving as an SSH
client or server, in order to decide whether to look for "ext-info-s"
in the server's KEXINIT or "ext-info-c" in the client's, respectively.

This check was done by testing the pointer 'server_hostkeys' to see if
it was non-NULL. I think I must have imagined that a variable of that
name meant "the host keys we have available to offer the client, if we
are the server", as the similarly named parameter 'our_hostkeys' in
write_kexinit_lists in fact does mean. So I expected it to be non-NULL
for the server and NULL for the client, and coded accordingly.

But in fact it's used by the client: it collects host key types the
client has _seen_ from the server, in order to offer them as cross-
certification actions in the specials menu. Moreover, it's _always_
non-NULL, because in the server, it's easier to leave it present but
empty than to get rid of it.

So this code was always behaving as if it was the server, i.e. it was
looking for "ext-info-c" in the client KEXINIT. When it was in fact
the client, that test would always succeed, because we _sent_ that
KEXINIT ourselves!

But nobody ever noticed, because when we're the client, it doesn't
matter whether we saw "ext-info-c", because we don't have any reason
to send EXT_INFO from client to server. We're only concerned with
server-to-client EXT_INFO. So this embarrassing bug had no actual
effect.
This commit is contained in:
Simon Tatham 2023-11-24 19:20:43 +00:00
parent c6013e2969
commit 9e09915157

View File

@ -956,7 +956,7 @@ struct server_hostkeys {
};
static bool ssh2_scan_kexinits(
ptrlen client_kexinit, ptrlen server_kexinit,
ptrlen client_kexinit, ptrlen server_kexinit, bool we_are_server,
struct kexinit_algorithm_list kexlists[NKEXLIST],
const ssh_kex **kex_alg, const ssh_keyalg **hostkey_alg,
transport_direction *cs, transport_direction *sc,
@ -1167,9 +1167,9 @@ static bool ssh2_scan_kexinits(
*/
{
ptrlen extinfo_advert =
(server_hostkeys ? PTRLEN_LITERAL("ext-info-c") :
(we_are_server ? PTRLEN_LITERAL("ext-info-c") :
PTRLEN_LITERAL("ext-info-s"));
ptrlen list = (server_hostkeys ? clists[KEXLIST_KEX] :
ptrlen list = (we_are_server ? clists[KEXLIST_KEX] :
slists[KEXLIST_KEX]);
for (ptrlen word; get_commasep_word(&list, &word) ;)
if (ptrlen_eq_ptrlen(word, extinfo_advert))
@ -1463,7 +1463,7 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl)
if (!ssh2_scan_kexinits(
ptrlen_from_strbuf(s->client_kexinit),
ptrlen_from_strbuf(s->server_kexinit),
ptrlen_from_strbuf(s->server_kexinit), s->ssc != NULL,
s->kexlists, &s->kex_alg, &s->hostkey_alg, s->cstrans,
s->sctrans, &s->warn_kex, &s->warn_hk, &s->warn_cscipher,
&s->warn_sccipher, s->ppl.ssh, NULL, &s->ignorepkt, &hks,