From 05002f727738ee445b4a9367f51474995302c6a6 Mon Sep 17 00:00:00 2001
From: jsing <>
Date: Thu, 3 Nov 2016 13:20:35 +0000
Subject: Split ssl3_get_key_exchange() into separate functions for DHE/ECDHE.

ok beck@ (who was struggling to keep lunch down while reviewing the diff)
---
 src/lib/libssl/s3_clnt.c | 461 ++++++++++++++++++++++++++---------------------
 1 file changed, 256 insertions(+), 205 deletions(-)

(limited to 'src')

diff --git a/src/lib/libssl/s3_clnt.c b/src/lib/libssl/s3_clnt.c
index d7cd37dec8..e9db8e27c2 100644
--- a/src/lib/libssl/s3_clnt.c
+++ b/src/lib/libssl/s3_clnt.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_clnt.c,v 1.139 2016/10/19 16:38:40 jsing Exp $ */
+/* $OpenBSD: s3_clnt.c,v 1.140 2016/11/03 13:20:35 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1091,6 +1091,246 @@ err:
 	return (ret);
 }
 
+static int
+ssl3_get_server_kex_dhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
+{
+	BN_CTX *bn_ctx = NULL;
+	SESS_CERT *sc = NULL;
+	DH *dh = NULL;
+	int al, i, param_len;
+	unsigned char *p;
+	long alg_a, n;
+
+	alg_a = s->s3->tmp.new_cipher->algorithm_auth;
+	n = *nn;
+	p = *pp;
+	sc = s->session->sess_cert;
+
+	if ((dh = DH_new()) == NULL) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_DH_LIB);
+		goto err;
+	}
+	if (2 > n)
+		goto truncated;
+	n2s(p, i);
+	param_len = i + 2;
+	if (param_len > n) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_P_LENGTH);
+		goto f_err;
+	}
+	if (!(dh->p = BN_bin2bn(p, i, NULL))) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_BN_LIB);
+		goto err;
+	}
+	p += i;
+
+	if (param_len + 2 > n)
+		goto truncated;
+	n2s(p, i);
+	param_len += i + 2;
+	if (param_len > n) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_G_LENGTH);
+		goto f_err;
+	}
+	if (!(dh->g = BN_bin2bn(p, i, NULL))) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_BN_LIB);
+		goto err;
+	}
+	p += i;
+
+	if (param_len + 2 > n)
+		goto truncated;
+	n2s(p, i);
+	param_len += i + 2;
+	if (param_len > n) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
+		    SSL_R_BAD_DH_PUB_KEY_LENGTH);
+		goto f_err;
+	}
+	if (!(dh->pub_key = BN_bin2bn(p, i, NULL))) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_BN_LIB);
+		goto err;
+	}
+	p += i;
+	n -= param_len;
+
+	/*
+	 * Check the strength of the DH key just constructed.
+	 * Discard keys weaker than 1024 bits.
+	 */
+	if (DH_size(dh) < 1024 / 8) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_P_LENGTH);
+		goto err;
+	}
+
+	if (alg_a & SSL_aRSA)
+		*pkey = X509_get_pubkey(sc->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
+	else if (alg_a & SSL_aDSS)
+		*pkey = X509_get_pubkey(sc->peer_pkeys[SSL_PKEY_DSA_SIGN].x509);
+	else 
+		/* XXX - Anonymous DH, so no certificate or pkey. */
+		*pkey = NULL;
+
+	sc->peer_dh_tmp = dh;
+
+	*nn = n;
+	*pp = p;
+
+	return (1);
+
+ truncated:
+	al = SSL_AD_DECODE_ERROR;
+	SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_PACKET_LENGTH);
+
+ f_err:
+	ssl3_send_alert(s, SSL3_AL_FATAL, al);
+
+ err:
+	DH_free(dh);
+	BN_CTX_free(bn_ctx);
+
+	return (-1);
+}
+
+static int
+ssl3_get_server_kex_ecdhe(SSL *s, EVP_PKEY **pkey, unsigned char **pp, long *nn)
+{
+	EC_POINT *srvr_ecpoint = NULL;
+	EC_KEY *ecdh = NULL;
+	BN_CTX *bn_ctx = NULL;
+	const EC_GROUP *group;
+	EC_GROUP *ngroup;
+	SESS_CERT *sc;
+	unsigned char *p;
+	int al, param_len;
+	long alg_a, n;
+	int curve_nid = 0;
+	int encoded_pt_len = 0;
+
+	alg_a = s->s3->tmp.new_cipher->algorithm_auth;
+	n = *nn;
+	p = *pp;
+	sc = s->session->sess_cert;
+
+	if ((ecdh = EC_KEY_new()) == NULL) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
+		goto err;
+	}
+
+	/*
+	 * Extract elliptic curve parameters and the server's ephemeral ECDH
+	 * public key. Keep accumulating lengths of various components in
+	 * param_len and make sure it never exceeds n.
+	 */
+
+	/*
+	 * XXX: For now we only support named (not generic) curves
+	 * and the ECParameters in this case is just three bytes.
+	 */
+	param_len = 3;
+	if (param_len > n) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT);
+		goto f_err;
+	}
+
+	/*
+	 * Check curve is one of our preferences, if not server has
+	 * sent an invalid curve.
+	 */
+	if (tls1_check_curve(s, p, param_len) != 1) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_WRONG_CURVE);
+		goto f_err;
+	}
+
+	if ((curve_nid = tls1_ec_curve_id2nid(*(p + 2))) == 0) {
+		al = SSL_AD_INTERNAL_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
+		    SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
+		goto f_err;
+	}
+
+	ngroup = EC_GROUP_new_by_curve_name(curve_nid);
+	if (ngroup == NULL) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_EC_LIB);
+		goto err;
+	}
+	if (EC_KEY_set_group(ecdh, ngroup) == 0) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_EC_LIB);
+		goto err;
+	}
+	EC_GROUP_free(ngroup);
+
+	group = EC_KEY_get0_group(ecdh);
+
+	p += 3;
+
+	/* Next, get the encoded ECPoint */
+	if (((srvr_ecpoint = EC_POINT_new(group)) == NULL) ||
+	    ((bn_ctx = BN_CTX_new()) == NULL)) {
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_MALLOC_FAILURE);
+		goto err;
+	}
+
+	if (param_len + 1 > n)
+		goto truncated;
+	encoded_pt_len = *p;
+	/* length of encoded point */
+	p += 1;
+	param_len += (1 + encoded_pt_len);
+	if ((param_len > n) || (EC_POINT_oct2point(group, srvr_ecpoint,
+	    p, encoded_pt_len, bn_ctx) == 0)) {
+		al = SSL_AD_DECODE_ERROR;
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_ECPOINT);
+		goto f_err;
+	}
+
+	n -= param_len;
+	p += encoded_pt_len;
+
+	/*
+	 * The ECC/TLS specification does not mention the use of DSA to sign
+	 * ECParameters in the server key exchange message. We do support RSA
+	 * and ECDSA.
+	 */
+	if (alg_a & SSL_aRSA)
+		*pkey = X509_get_pubkey(sc->peer_pkeys[SSL_PKEY_RSA_ENC].x509);
+	else if (alg_a & SSL_aECDSA)
+		*pkey = X509_get_pubkey(sc->peer_pkeys[SSL_PKEY_ECC].x509);
+	else
+		/* XXX - Anonymous ECDH, so no certificate or pkey. */
+		*pkey = NULL;
+
+	EC_KEY_set_public_key(ecdh, srvr_ecpoint);
+	sc->peer_ecdh_tmp = ecdh;
+
+	BN_CTX_free(bn_ctx);
+	EC_POINT_free(srvr_ecpoint);
+
+	*nn = n;
+	*pp = p;
+
+	return (1);
+
+ truncated:
+	al = SSL_AD_DECODE_ERROR;
+	SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_PACKET_LENGTH);
+
+ f_err:
+	ssl3_send_alert(s, SSL3_AL_FATAL, al);
+
+ err:
+	BN_CTX_free(bn_ctx);
+	EC_POINT_free(srvr_ecpoint);
+	EC_KEY_free(ecdh);
+
+	return (-1);
+}
+
 int
 ssl3_get_key_exchange(SSL *s)
 {
@@ -1102,12 +1342,6 @@ ssl3_get_key_exchange(SSL *s)
 	EVP_PKEY	*pkey = NULL;
 	const		 EVP_MD *md = NULL;
 	RSA		*rsa = NULL;
-	DH		*dh = NULL;
-	EC_KEY		*ecdh = NULL;
-	BN_CTX		*bn_ctx = NULL;
-	EC_POINT	*srvr_ecpoint = NULL;
-	int		 curve_nid = 0;
-	int		 encoded_pt_len = 0;
 
 	alg_k = s->s3->tmp.new_cipher->algorithm_mkey;
 	alg_a = s->s3->tmp.new_cipher->algorithm_auth;
@@ -1153,206 +1387,21 @@ ssl3_get_key_exchange(SSL *s)
 	}
 
 	param = p = (unsigned char *)s->init_msg;
-	param_len = 0;
+	param_len = n;
 
 	if (alg_k & SSL_kDHE) {
-		if ((dh = DH_new()) == NULL) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_DH_LIB);
-			goto err;
-		}
-		if (2 > n)
-			goto truncated;
-		n2s(p, i);
-		param_len = i + 2;
-		if (param_len > n) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_BAD_DH_P_LENGTH);
-			goto f_err;
-		}
-		if (!(dh->p = BN_bin2bn(p, i, NULL))) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_BN_LIB);
-			goto err;
-		}
-		p += i;
-
-		if (param_len + 2 > n)
-			goto truncated;
-		n2s(p, i);
-		param_len += i + 2;
-		if (param_len > n) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_BAD_DH_G_LENGTH);
-			goto f_err;
-		}
-		if (!(dh->g = BN_bin2bn(p, i, NULL))) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_BN_LIB);
+		if (ssl3_get_server_kex_dhe(s, &pkey, &p, &n) != 1)
 			goto err;
-		}
-		p += i;
-
-		if (param_len + 2 > n)
-			goto truncated;
-		n2s(p, i);
-		param_len += i + 2;
-		if (param_len > n) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_BAD_DH_PUB_KEY_LENGTH);
-			goto f_err;
-		}
-		if (!(dh->pub_key = BN_bin2bn(p, i, NULL))) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_BN_LIB);
-			goto err;
-		}
-		p += i;
-		n -= param_len;
-
-		/*
-		 * Check the strength of the DH key just constructed.
-		 * Discard keys weaker than 1024 bits.
-		 */
-
-		if (DH_size(dh) < 1024 / 8) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_BAD_DH_P_LENGTH);
-			goto err;
-		}
-
-		if (alg_a & SSL_aRSA)
-			pkey = X509_get_pubkey(
-			    s->session->sess_cert->peer_pkeys[
-			    SSL_PKEY_RSA_ENC].x509);
-		else if (alg_a & SSL_aDSS)
-			pkey = X509_get_pubkey(
-			    s->session->sess_cert->peer_pkeys[
-			    SSL_PKEY_DSA_SIGN].x509);
-		/* else anonymous DH, so no certificate or pkey. */
-
-		s->session->sess_cert->peer_dh_tmp = dh;
-		dh = NULL;
 	} else if (alg_k & SSL_kECDHE) {
-		const EC_GROUP *group;
-		EC_GROUP *ngroup;
-
-		if ((ecdh = EC_KEY_new()) == NULL) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_MALLOC_FAILURE);
-			goto err;
-		}
-
-		/*
-		 * Extract elliptic curve parameters and the
-		 * server's ephemeral ECDH public key.
-		 * Keep accumulating lengths of various components in
-		 * param_len and make sure it never exceeds n.
-		 */
-
-		/*
-		 * XXX: For now we only support named (not generic) curves
-		 * and the ECParameters in this case is just three bytes.
-		 */
-		param_len = 3;
-		if (param_len > n) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_LENGTH_TOO_SHORT);
-			goto f_err;
-		}
-
-		/*
-		 * Check curve is one of our preferences, if not server has
-		 * sent an invalid curve.
-		 */
-		if (tls1_check_curve(s, p, param_len) != 1) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_WRONG_CURVE);
-			goto f_err;
-		}
-
-		if ((curve_nid = tls1_ec_curve_id2nid(*(p + 2))) == 0) {
-			al = SSL_AD_INTERNAL_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_UNABLE_TO_FIND_ECDH_PARAMETERS);
-			goto f_err;
-		}
-
-		ngroup = EC_GROUP_new_by_curve_name(curve_nid);
-		if (ngroup == NULL) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_EC_LIB);
-			goto err;
-		}
-		if (EC_KEY_set_group(ecdh, ngroup) == 0) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_EC_LIB);
-			goto err;
-		}
-		EC_GROUP_free(ngroup);
-
-		group = EC_KEY_get0_group(ecdh);
-
-		p += 3;
-
-		/* Next, get the encoded ECPoint */
-		if (((srvr_ecpoint = EC_POINT_new(group)) == NULL) ||
-		    ((bn_ctx = BN_CTX_new()) == NULL)) {
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    ERR_R_MALLOC_FAILURE);
+		if (ssl3_get_server_kex_ecdhe(s, &pkey, &p, &n) != 1)
 			goto err;
-		}
-
-		if (param_len + 1 > n)
-			goto truncated;
-		encoded_pt_len = *p;
-		/* length of encoded point */
-		p += 1;
-		param_len += (1 + encoded_pt_len);
-		if ((param_len > n) || (EC_POINT_oct2point(group, srvr_ecpoint,
-		    p, encoded_pt_len, bn_ctx) == 0)) {
-			al = SSL_AD_DECODE_ERROR;
-			SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-			    SSL_R_BAD_ECPOINT);
-			goto f_err;
-		}
-
-		n -= param_len;
-		p += encoded_pt_len;
-
-		/*
-		 * The ECC/TLS specification does not mention the use
-		 * of DSA to sign ECParameters in the server key
-		 * exchange message. We do support RSA and ECDSA.
-		 */
-		if (alg_a & SSL_aRSA)
-			pkey = X509_get_pubkey(
-			    s->session->sess_cert->peer_pkeys[
-			    SSL_PKEY_RSA_ENC].x509);
-		else if (alg_a & SSL_aECDSA)
-			pkey = X509_get_pubkey(
-			    s->session->sess_cert->peer_pkeys[
-			    SSL_PKEY_ECC].x509);
-		/* Else anonymous ECDH, so no certificate or pkey. */
-		EC_KEY_set_public_key(ecdh, srvr_ecpoint);
-		s->session->sess_cert->peer_ecdh_tmp = ecdh;
-		ecdh = NULL;
-		BN_CTX_free(bn_ctx);
-		bn_ctx = NULL;
-		EC_POINT_free(srvr_ecpoint);
-		srvr_ecpoint = NULL;
-	} else if (alg_k) {
+	} else if (alg_k != 0) {
 		al = SSL_AD_UNEXPECTED_MESSAGE;
-		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE,
-		    SSL_R_UNEXPECTED_MESSAGE);
+		SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_UNEXPECTED_MESSAGE);
 			goto f_err;
 	}
 
-	/* p points to the next byte, there are 'n' bytes left */
+	param_len = param_len - n;
 
 	/* if it was signed, check the signature */
 	if (pkey != NULL) {
@@ -1471,23 +1520,25 @@ ssl3_get_key_exchange(SSL *s)
 			goto f_err;
 		}
 	}
+
 	EVP_PKEY_free(pkey);
 	EVP_MD_CTX_cleanup(&md_ctx);
+
 	return (1);
-truncated:
+
+ truncated:
 	/* wrong packet length */
 	al = SSL_AD_DECODE_ERROR;
 	SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_PACKET_LENGTH);
-f_err:
+
+ f_err:
 	ssl3_send_alert(s, SSL3_AL_FATAL, al);
-err:
+
+ err:
 	EVP_PKEY_free(pkey);
 	RSA_free(rsa);
-	DH_free(dh);
-	BN_CTX_free(bn_ctx);
-	EC_POINT_free(srvr_ecpoint);
-	EC_KEY_free(ecdh);
 	EVP_MD_CTX_cleanup(&md_ctx);
+
 	return (-1);
 }
 
-- 
cgit v1.2.3-55-g6feb