Package: openssh-server Version: 1:8.4p1-5+deb11u3 Severity: normal Tags: patch X-Debbugs-Cc: r...@qoxp.net
The sshd "hostkey none" option -- part of the GSSAPI key-exchange patch maintained by Debian -- has two bugs: Bug #1) The Debian GSSAPI code drops all kex algorithms from the outgoing server proposal here, in the case of "hostkey none": [sshd.c] /* * If we don't have a host key, then there's no point advertising * the other key exchange algorithms */ if (strlen(myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS]) == 0) orig = NULL; This became buggy with the introduction of extension "signalling" via kex pseudo-algorithms, originally ext-info-*, and later kex-strict-*. The code now drops any such signals originated by kex_proposal_populate_entries(). This prevents ext-info negotiation, and with the addition of the strict-kex feature, it caused a worse bug: SSH connections using a MAC (non-AEAD bulk cipher) with a client that supports strict-kex die like so: $ ssh ... Bad packet length 1328530089. ssh_dispatch_run_fatal: Connection to 172.18.0.2 port 22: Connection corrupted This is because the upstream server code believes it has advertised strict-kex, so turns on the feature if it sees a matching advertisement from the client. But it never *actually* advertised it, since the GSSAPI patch strips the advertisement from the outgoing kex proposal, so the client never sees it. Now the server has strict-kex enabled while the client does not. As soon as the server turns on encryption with the NEWKEYS message, it resets its outgoing packet sequence number to zero -- but the client doesn't do this, so a MAC failure ensues. This only applies to connections using a MAC; the sequence number is unused in the case of the newer AEAD ciphers. Bug #2) sshd dies with an invalid free(): [strace -fe signal sshd -ddd] debug1: expecting SSH2_MSG_NEWKEYS [preauth] debug3: receive packet: type 21 [preauth] debug1: ssh_packet_read_poll2: resetting read seqnr 3 [preauth] free(): invalid pointer debug1: SSH2_MSG_NEWKEYS received [preauth] [pid 57] rt_sigprocmask(SIG_UNBLOCK, [ABRT], NULL, 8) = 0 debug2: ssh_set_newkeys: mode 0 [preauth] [pid 57] tgkill(57, 57, SIGABRT) = 234 This happens because in the case of "hostkey none", the GSSAPI patch resets the outgoing kex hostkey-algorithm list to contain just the "null" algorithm: [sshd.c] /* * If we've got GSSAPI mechanisms, then we've got the 'null' host * key alg, but we can't tell people about it unless its the only * host key algorithm we support */ if (gss && (strlen(myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS])) == 0) myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = "null"; This is fine, except that it uses a static string ("null") to do it, whereas the upstream kex_proposal_free_entries() expects all entries in a kex proposal to be dynamically allocated, and tries to free it. This can be fixed with an easy change to: myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = xstrdup("null"); I'm including a patch fixing both bugs, against 1:9.6p1-5. A patch against 'master' will need to apply to sshd-session.c instead of sshd.c with later changes from upstream, but looks otherwise to be identical. My patch proposes to fix bug #1 by altering the upstream kex_proposal_populate_entries(). The current Debian GSSAPI patch avoids doing that in favor of separate code that modifies the kex proposal later, which makes sense as a general strategy for maintaining the patch but seems problematic now. With the addition of kex signalling, that code has no way to know which kex algorithms are "real" and can be safely dropped, and which are signals that it must retain. It would need to hard-code a list of signals to look for and keep, and that list will go out of date whenever new signals appear upstream, easily causing another bug of the same kind. It seems better to fix it in the upstream code, where it will either continue to work correctly as new signals are added, or if there's a conflicting upstream change we'll be forced to examine it and make sure it still works. This change could in fact be proposed upstream, since by itself it does not rely on GSSAPI at all; it merely accommodates a case that only *arises* with the gss-kex patch. Alternatively, we could retain the same strategy if the upstream provided a global list of signal algorithms to refer to, but currently that list is just coded explicitly inside kex_proposal_populate_entries(): [kex.c] /* Append feature signalling to KexAlgorithms. */ if ((cp = kex_names_cat(kexalgos, ssh->kex->server ? "ext-info-s,kex-strict-s-...@openssh.com" : "ext-info-c,kex-strict-c-...@openssh.com")) == NULL) fatal_f("kex_names_cat"); Here is the patch against 1:9.6p1-5: ================================================================================ diff --git a/kex.c b/kex.c index 6f06a4f7..db6717e9 100644 --- a/kex.c +++ b/kex.c @@ -378,25 +378,14 @@ kex_proposal_populate_entries(struct ssh *ssh, char *prop[PROPOSAL_MAX], const char *defpropclient[PROPOSAL_MAX] = { KEX_CLIENT }; const char **defprop = ssh->kex->server ? defpropserver : defpropclient; u_int i; - char *cp, *hkalgs_prop; + char *cp; if (prop == NULL) fatal_f("proposal missing"); - /* our hostkey algorithm proposal */ - hkalgs_prop = xstrdup(hkalgs ? hkalgs : defprop[PROPOSAL_SERVER_HOST_KEY_ALGS]); - - /* - * If we don't have a hostkey (sshd_config "HostKey none" => - * hkalgs_prop list is empty), there's no point in including - * the default kex algorithms; start with the empty list - * instead. GSSAPI code will later add the dynamically - * determined gss-* algorithms. - */ + /* Append EXT_INFO signalling to KexAlgorithms */ if (kexalgos == NULL) - kexalgos = strlen(hkalgs_prop) == 0 ? "" : defprop[PROPOSAL_KEX_ALGS]; - - /* Append feature signalling to KexAlgorithms. */ + kexalgos = defprop[PROPOSAL_KEX_ALGS]; if ((cp = kex_names_cat(kexalgos, ssh->kex->server ? "ext-info-s,kex-strict-s-...@openssh.com" : "ext-info-c,kex-strict-c-...@openssh.com")) == NULL) @@ -420,7 +409,7 @@ kex_proposal_populate_entries(struct ssh *ssh, char *prop[PROPOSAL_MAX], prop[i] = xstrdup(comp ? comp : defprop[i]); break; case PROPOSAL_SERVER_HOST_KEY_ALGS: - prop[i] = hkalgs_prop; + prop[i] = xstrdup(hkalgs ? hkalgs : defprop[i]); break; default: prop[i] = xstrdup(defprop[i]); diff --git a/sshd.c b/sshd.c index 625c1f32..6dfa5fff 100644 --- a/sshd.c +++ b/sshd.c @@ -2505,6 +2505,14 @@ do_ssh2_kex(struct ssh *ssh) char *newstr = NULL; orig = myproposal[PROPOSAL_KEX_ALGS]; + /* + * If we don't have a host key, then there's no point advertising + * the other key exchange algorithms + */ + + if (strlen(myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS]) == 0) + orig = NULL; + if (options.gss_keyex) gss = ssh_gssapi_server_mechanisms(); else @@ -2523,7 +2531,7 @@ do_ssh2_kex(struct ssh *ssh) * host key algorithm we support */ if (gss && (strlen(myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS])) == 0) - myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = xstrdup("null"); + myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = "null"; if (newstr) myproposal[PROPOSAL_KEX_ALGS] = newstr; ================================================================================ -- System Information: Debian Release: 11.11 APT prefers oldstable-updates APT policy: (500, 'oldstable-updates'), (500, 'oldstable-security'), (500, 'oldstable') Architecture: amd64 (x86_64) Kernel: Linux 5.10.0-33-cloud-amd64 (SMP w/2 CPU threads) Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages openssh-server depends on: ii adduser 3.118+deb11u1 ii debconf [debconf-2.0] 1.5.77 ii dpkg 1.20.13 ii libaudit1 1:3.0-2 ii libc6 2.31-13+deb11u11 ii libcom-err2 1.46.2-2+deb11u1 ii libcrypt1 1:4.4.18-4 ii libgssapi-krb5-2 1.18.3-6+deb11u5 ii libkrb5-3 1.18.3-6+deb11u5 ii libpam-modules 1.4.0-9+deb11u1 ii libpam-runtime 1.4.0-9+deb11u1 ii libpam0g 1.4.0-9+deb11u1 ii libselinux1 3.1-3 ii libssl1.1 1.1.1w-0+deb11u2 ii libsystemd0 247.3-7+deb11u6 ii libwrap0 7.6.q-31 ii lsb-base 11.1.0 ii openssh-client 1:8.4p1-5+deb11u3 ii openssh-sftp-server 1:8.4p1-5+deb11u3 ii procps 2:3.3.17-5 ii runit-helper 2.10.3 ii ucf 3.0043 ii zlib1g 1:1.2.11.dfsg-2+deb11u2 Versions of packages openssh-server recommends: ii libpam-systemd [logind] 247.3-7+deb11u6 pn ncurses-term <none> ii xauth 1:1.1-1 Versions of packages openssh-server suggests: pn molly-guard <none> pn monkeysphere <none> pn ssh-askpass <none> pn ufw <none> -- debconf information excluded