Bug#1091820: openssh-server: two bugs in sshd with "hostkey none"
Package: openssh-server
Version: 1:8.4p1-5+deb11u3
Severity: normal
Tags: patch
X-Debbugs-Cc: res@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-v00@openssh.com" :
"ext-info-c,kex-strict-c-v00@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-v00@openssh.com" :
"ext-info-c,kex-strict-c-v00@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
Reply to: