From 9641753e0fd146204d57b2a4165f552a81afade4 Mon Sep 17 00:00:00 2001
From: "dtucker@openbsd.org" <dtucker@openbsd.org>
Date: Mon, 6 Mar 2023 12:14:48 +0000
Subject: [PATCH] upstream: Refactor creation of KEX proposal.

This adds kex_proposal_populate_entries (and corresponding free) which
populates the KEX proposal array with dynamically allocated strings.
This replaces the previous mix of static and dynamic that has been the
source of previous leaks and bugs.  Remove unused compat functions.
With & ok djm@.

OpenBSD-Commit-ID: f2f99da4aae2233cb18bf9c749320c5e040a9c7b

Reference:https://github.com/openssh/openssh-portable/commit/9641753e0fd146204d57b2a4165f552a81afade4
Conflict:Remove now-unused compat bit SSH_BUG_RSASIGMD5 
         gssapi-keyex
---
 compat.c      | 35 ++------------------------
 compat.h      |  6 ++---
 kex.c         | 58 ++++++++++++++++++++++++++++++++++++++++++-
 kex.h         |  5 +++-
 sshconnect2.c | 68 +++++++++++++++++++--------------------------------
 sshd.c        | 37 +++++++++-------------------
 6 files changed, 102 insertions(+), 107 deletions(-)

diff --git a/compat.c b/compat.c
index 555a372..4f8a1fb 100644
--- a/compat.c
+++ b/compat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.c,v 1.121 2023/02/02 12:10:05 djm Exp $ */
+/* $OpenBSD: compat.c,v 1.126 2023/03/06 12:14:48 dtucker Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -36,7 +36,6 @@
 #include "compat.h"
 #include "log.h"
 #include "match.h"
-#include "kex.h"
 
 /* determine bug flags from SSH protocol banner */
 void
@@ -158,37 +157,7 @@ compat_banner(struct ssh *ssh, const char *version)
 
 /* Always returns pointer to allocated memory, caller must free. */
 char *
-compat_cipher_proposal(struct ssh *ssh, char *cipher_prop)
-{
-	if (!(ssh->compat & SSH_BUG_BIGENDIANAES))
-		return xstrdup(cipher_prop);
-	debug2_f("original cipher proposal: %s", cipher_prop);
-	if ((cipher_prop = match_filter_denylist(cipher_prop, "aes*")) == NULL)
-		fatal("match_filter_denylist failed");
-	debug2_f("compat cipher proposal: %s", cipher_prop);
-	if (*cipher_prop == '\0')
-		fatal("No supported ciphers found");
-	return cipher_prop;
-}
-
-/* Always returns pointer to allocated memory, caller must free. */
-char *
-compat_pkalg_proposal(struct ssh *ssh, char *pkalg_prop)
-{
-	if (!(ssh->compat & SSH_BUG_RSASIGMD5))
-		return xstrdup(pkalg_prop);
-	debug2_f("original public key proposal: %s", pkalg_prop);
-	if ((pkalg_prop = match_filter_denylist(pkalg_prop, "ssh-rsa")) == NULL)
-		fatal("match_filter_denylist failed");
-	debug2_f("compat public key proposal: %s", pkalg_prop);
-	if (*pkalg_prop == '\0')
-		fatal("No supported PK algorithms found");
-	return pkalg_prop;
-}
-
-/* Always returns pointer to allocated memory, caller must free. */
-char *
-compat_kex_proposal(struct ssh *ssh, char *p)
+compat_kex_proposal(struct ssh *ssh, const char *p)
 {
 	char *cp = NULL, *cp2 = NULL;
 
diff --git a/compat.h b/compat.h
index 167409b..013c855 100644
--- a/compat.h
+++ b/compat.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.h,v 1.57 2021/06/06 03:40:39 djm Exp $ */
+/* $OpenBSD: compat.h,v 1.62 2023/03/06 12:14:48 dtucker Exp $ */
 
 /*
  * Copyright (c) 1999, 2000, 2001 Markus Friedl.  All rights reserved.
@@ -61,7 +61,5 @@
 struct ssh;
 
 void    compat_banner(struct ssh *, const char *);
-char	*compat_cipher_proposal(struct ssh *, char *);
-char	*compat_pkalg_proposal(struct ssh *, char *);
-char	*compat_kex_proposal(struct ssh *, char *);
+char	*compat_kex_proposal(struct ssh *, const char *);
 #endif
diff --git a/kex.c b/kex.c
index e8c2741..b681c58 100644
--- a/kex.c
+++ b/kex.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.c,v 1.168 2021/04/03 06:18:40 djm Exp $ */
+/* $OpenBSD: kex.c,v 1.176 2023/03/06 12:14:48 dtucker Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  *
@@ -60,6 +60,7 @@
 #include "misc.h"
 #include "dispatch.h"
 #include "monitor.h"
+#include "myproposal.h"
 #include "xmalloc.h"
 
 #include "ssherr.h"
@@ -359,6 +360,61 @@ kex_assemble_names(char **listp, const char *def, const char *all)
 	return r;
 }
 
+/*
+ * Fill out a proposal array with dynamically allocated values, which may
+ * be modified as required for compatibility reasons.
+ * Any of the options may be NULL, in which case the default is used.
+ * Array contents must be freed by calling kex_proposal_free_entries.
+ */
+void
+kex_proposal_populate_entries(struct ssh *ssh, char *prop[PROPOSAL_MAX],
+    const char *kexalgos, const char *ciphers, const char *macs,
+    const char *comp, const char *hkalgs)
+{
+	const char *defpropserver[PROPOSAL_MAX] = { KEX_SERVER };
+	const char *defpropclient[PROPOSAL_MAX] = { KEX_CLIENT };
+	const char **defprop = ssh->kex->server ? defpropserver : defpropclient;
+	u_int i;
+
+	if (prop == NULL)
+		fatal_f("proposal missing");
+
+	for (i = 0; i < PROPOSAL_MAX; i++) {
+		switch(i) {
+		case PROPOSAL_KEX_ALGS:
+			prop[i] = compat_kex_proposal(ssh,
+			    kexalgos ? kexalgos : defprop[i]);
+			break;
+		case PROPOSAL_ENC_ALGS_CTOS:
+		case PROPOSAL_ENC_ALGS_STOC:
+			prop[i] = xstrdup(ciphers ? ciphers : defprop[i]);
+			break;
+		case PROPOSAL_MAC_ALGS_CTOS:
+		case PROPOSAL_MAC_ALGS_STOC:
+			prop[i]  = xstrdup(macs ? macs : defprop[i]);
+			break;
+		case PROPOSAL_COMP_ALGS_CTOS:
+		case PROPOSAL_COMP_ALGS_STOC:
+			prop[i] = xstrdup(comp ? comp : defprop[i]);
+			break;
+		case PROPOSAL_SERVER_HOST_KEY_ALGS:
+			prop[i] = xstrdup(hkalgs ? hkalgs : defprop[i]);
+			break;
+		default:
+			prop[i] = xstrdup(defprop[i]);
+		}
+	}
+}
+
+void
+kex_proposal_free_entries(char *prop[PROPOSAL_MAX])
+{
+	u_int i;
+
+	for (i = 0; i < PROPOSAL_MAX; i++)
+		free(prop[i]);
+}
+
 /* Validate GSS KEX method name list */
 int
 kex_gss_names_valid(const char *names)
diff --git a/kex.h b/kex.h
index 8b95227..87ba7c8 100644
--- a/kex.h
+++ b/kex.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.h,v 1.114 2021/01/31 22:55:29 djm Exp $ */
+/* $OpenBSD: kex.h,v 1.118 2023/03/06 12:14:48 dtucker Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -192,6 +192,9 @@ char	*kex_alg_list(char);
 char	*kex_gss_alg_list(char);
 char	*kex_names_cat(const char *, const char *);
 int	 kex_assemble_names(char **, const char *, const char *);
+void	 kex_proposal_populate_entries(struct ssh *, char *prop[PROPOSAL_MAX],
+    const char *, const char *, const char *, const char *, const char *);
+void	 kex_proposal_free_entries(char *prop[PROPOSAL_MAX]);
 int	 kex_gss_names_valid(const char *);
 
 int	 kex_exchange_identification(struct ssh *, int, const char *);
diff --git a/sshconnect2.c b/sshconnect2.c
index eb0df92..9267534 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshconnect2.c,v 1.359 2022/07/01 03:39:44 dtucker Exp $ */
+/* $OpenBSD: sshconnect2.c,v 1.364 2023/03/06 12:14:48 dtucker Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Damien Miller.  All rights reserved.
@@ -58,7 +58,6 @@
 #include "cipher.h"
 #include "sshkey.h"
 #include "kex.h"
-#include "myproposal.h"
 #include "sshconnect.h"
 #include "authfile.h"
 #include "dh.h"
@@ -216,11 +215,9 @@ void
 ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
     const struct ssh_conn_info *cinfo)
 {
-	char *myproposal[PROPOSAL_MAX] = { KEX_CLIENT };
-	char *s, *all_key;
-	char *prop_kex = NULL, *prop_enc = NULL, *prop_hostkey = NULL;
-	int r, use_known_hosts_order = 0;
-
+	char *myproposal[PROPOSAL_MAX];
+	char *s, *all_key, *hkalgs = NULL;
+	int r;
 #if defined(GSSAPI) && defined(WITH_OPENSSL)
 	char *orig = NULL, *gss = NULL;
 	char *gss_host = NULL;
@@ -230,15 +227,9 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 	xxx_hostaddr = hostaddr;
 	xxx_conn_info = cinfo;
 
-	/*
-	 * If the user has not specified HostkeyAlgorithms, or has only
-	 * appended or removed algorithms from that list then prefer algorithms
-	 * that are in the list that are supported by known_hosts keys.
-	 */
-	if (options.hostkeyalgorithms == NULL ||
-	    options.hostkeyalgorithms[0] == '-' ||
-	    options.hostkeyalgorithms[0] == '+')
-		use_known_hosts_order = 1;
+	if (options.rekey_limit || options.rekey_interval)
+		ssh_packet_set_rekey_limits(ssh, options.rekey_limit,
+		    options.rekey_interval);
 
 	/* Expand or fill in HostkeyAlgorithms */
 	all_key = sshkey_alg_list(0, 0, 1, ',');
@@ -249,25 +240,22 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 
 	if ((s = kex_names_cat(options.kex_algorithms, "ext-info-c")) == NULL)
 		fatal_f("kex_names_cat");
-	myproposal[PROPOSAL_KEX_ALGS] = prop_kex = compat_kex_proposal(ssh, s);
-	myproposal[PROPOSAL_ENC_ALGS_CTOS] =
-	    myproposal[PROPOSAL_ENC_ALGS_STOC] = prop_enc =
-	    compat_cipher_proposal(ssh, options.ciphers);
-	myproposal[PROPOSAL_COMP_ALGS_CTOS] =
-	    myproposal[PROPOSAL_COMP_ALGS_STOC] =
-	    (char *)compression_alg_list(options.compression);
-	myproposal[PROPOSAL_MAC_ALGS_CTOS] =
-	    myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs;
-	if (use_known_hosts_order) {
-		/* Query known_hosts and prefer algorithms that appear there */
-		myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = prop_hostkey =
-		    compat_pkalg_proposal(ssh,
-		    order_hostkeyalgs(host, hostaddr, port, cinfo));
-	} else {
-		/* Use specified HostkeyAlgorithms exactly */
-		myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = prop_hostkey =
-		    compat_pkalg_proposal(ssh, options.hostkeyalgorithms);
-	}
+
+	/*
+	 * If the user has not specified HostkeyAlgorithms, or has only
+	 * appended or removed algorithms from that list then prefer algorithms
+	 * that are in the list that are supported by known_hosts keys.
+	 */
+	if (options.hostkeyalgorithms == NULL ||
+	    options.hostkeyalgorithms[0] == '-' ||
+	    options.hostkeyalgorithms[0] == '+')
+		hkalgs = order_hostkeyalgs(host, hostaddr, port, cinfo);
+
+	kex_proposal_populate_entries(ssh, myproposal, s, options.ciphers,
+	    options.macs, compression_alg_list(options.compression),
+	    hkalgs ? hkalgs : options.hostkeyalgorithms);
+
+	free(hkalgs);
 
 #if defined(GSSAPI) && defined(WITH_OPENSSL)
 	if (options.gss_keyex) {
@@ -310,10 +298,6 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 	}
 #endif
 
-	if (options.rekey_limit || options.rekey_interval)
-		ssh_packet_set_rekey_limits(ssh, options.rekey_limit,
-		    options.rekey_interval);
-
 	/* start key exchange */
 	if ((r = kex_setup(ssh, myproposal)) != 0)
 		fatal_r(r, "kex_setup");
@@ -357,6 +341,7 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 	ssh_dispatch_run_fatal(ssh, DISPATCH_BLOCK, &ssh->kex->done);
 
 	/* remove ext-info from the KEX proposals for rekeying */
+	free(myproposal[PROPOSAL_KEX_ALGS]);
 	myproposal[PROPOSAL_KEX_ALGS] =
 	    compat_kex_proposal(ssh, options.kex_algorithms);
 #if defined(GSSAPI) && defined(WITH_OPENSSL)
@@ -380,10 +365,7 @@ ssh_kex2(struct ssh *ssh, char *host, struct sockaddr *hostaddr, u_short port,
 	    (r = ssh_packet_write_wait(ssh)) != 0)
 		fatal_fr(r, "send packet");
 #endif
-	/* Free only parts of proposal that were dynamically allocated here. */
-	free(prop_kex);
-	free(prop_enc);
-	free(prop_hostkey);
+	kex_proposal_free_entries(myproposal);
 }
 
 /*
diff --git a/sshd.c b/sshd.c
index dd7e1d4..aa3c3b7 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.589 2022/07/01 03:39:44 dtucker Exp $ */
+/* $OpenBSD: sshd.c,v 1.599 2023/03/06 12:14:48 dtucker Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -107,7 +107,6 @@
 #include "digest.h"
 #include "sshkey.h"
 #include "kex.h"
-#include "myproposal.h"
 #include "authfile.h"
 #include "pathnames.h"
 #include "atomicio.h"
@@ -2524,33 +2523,23 @@ sshd_hostkey_sign(struct ssh *ssh, struct sshkey *privkey,
 static void
 do_ssh2_kex(struct ssh *ssh)
 {
-	char *myproposal[PROPOSAL_MAX] = { KEX_SERVER };
+	char *hkalgs = NULL, *myproposal[PROPOSAL_MAX];
+	const char *compression = NULL;
 	struct kex *kex;
-	char *prop_kex = NULL, *prop_enc = NULL, *prop_hostkey = NULL;
 	int r;
 
-	myproposal[PROPOSAL_KEX_ALGS] = prop_kex = compat_kex_proposal(ssh,
-	    options.kex_algorithms);
-	myproposal[PROPOSAL_ENC_ALGS_CTOS] =
-		myproposal[PROPOSAL_ENC_ALGS_STOC] = prop_enc =
-		compat_cipher_proposal(ssh, options.ciphers);
-	myproposal[PROPOSAL_ENC_ALGS_STOC] = compat_cipher_proposal(ssh,
-	    options.ciphers);
-	myproposal[PROPOSAL_MAC_ALGS_CTOS] =
-	    myproposal[PROPOSAL_MAC_ALGS_STOC] = options.macs;
-
-	if (options.compression == COMP_NONE) {
-		myproposal[PROPOSAL_COMP_ALGS_CTOS] =
-		    myproposal[PROPOSAL_COMP_ALGS_STOC] = "none";
-	}
-
 	if (options.rekey_limit || options.rekey_interval)
 		ssh_packet_set_rekey_limits(ssh, options.rekey_limit,
 		    options.rekey_interval);
-	/* coverity[leaked_storage : FALSE]*/
-	myproposal[PROPOSAL_SERVER_HOST_KEY_ALGS] = prop_hostkey =
-		compat_pkalg_proposal(ssh, list_hostkey_types());
 
+	if (options.compression == COMP_NONE)
+		compression = "none";
+	hkalgs = list_hostkey_types();
+
+	kex_proposal_populate_entries(ssh, myproposal, options.kex_algorithms,
+	    options.ciphers, options.macs, compression, hkalgs);
+
+	free(hkalgs);
 #if defined(GSSAPI) && defined(WITH_OPENSSL)
 	{
 	char *orig;
@@ -2645,9 +2634,7 @@ do_ssh2_kex(struct ssh *ssh)
 	    (r = ssh_packet_write_wait(ssh)) != 0)
 		fatal_fr(r, "send test");
 #endif
-	free(prop_kex);
-	free(prop_enc);
-	free(prop_hostkey);
+	kex_proposal_free_entries(myproposal);
 	debug("KEX done");
 }
 
-- 
2.33.0

