Clean up EC cipher handling in ssl3_choose_cipher().

The existing code reaches around into various internals of EC, which it
should not know anything about. Replace this with a set of functions that
that can correctly extract the necessary details and handle the
comparisions.

Based on a commit to OpenSSL, with some inspiration from boringssl.

ok miod@

3 files changed, 152 insertions, 145 deletions

diff --git a/lib/libssl/s3_lib.c b/lib/libssl/s3_lib.c
index d8b923afd4a..246aa6f23df 100644
--- a/lib/libssl/s3_lib.c
+++ b/lib/libssl/s3_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: s3_lib.c,v 1.80 2014/09/07 12:16:23 jsing Exp $ */
+/* $OpenBSD: s3_lib.c,v 1.81 2014/09/30 15:40:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -149,11 +149,12 @@
  */
 
 #include <stdio.h>
+
+#include <openssl/dh.h>
+#include <openssl/md5.h>
 #include <openssl/objects.h>
+
 #include "ssl_locl.h"
-#include "../crypto/ec/ec_lcl.h"
-#include <openssl/md5.h>
-#include <openssl/dh.h>
 
 #define SSL3_NUM_CIPHERS	(sizeof(ssl3_ciphers) / sizeof(SSL_CIPHER))
 
@@ -2394,14 +2395,11 @@ ssl3_ctx_callback_ctrl(SSL_CTX *ctx, int cmd, void (*fp)(void))
 SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
     STACK_OF(SSL_CIPHER) *srvr)
 {
-	SSL_CIPHER		*c, *ret = NULL;
-	STACK_OF(SSL_CIPHER)	*prio, *allow;
-	int			 i, ii, ok;
-	unsigned int j;
-	int ec_ok, ec_nid;
-	unsigned char ec_search1 = 0, ec_search2 = 0;
-	CERT *cert;
 	unsigned long alg_k, alg_a, mask_k, mask_a;
+	STACK_OF(SSL_CIPHER) *prio, *allow;
+	SSL_CIPHER *c, *ret = NULL;
+	int i, ii, ok;
+	CERT *cert;
 
 	/* Let's see which ciphers we can support */
 	cert = s->cert;
@@ -2439,141 +2437,18 @@ SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
 
 		ok = (alg_k & mask_k) && (alg_a & mask_a);
 
-		if (
-		/*
-		 * if we are considering an ECC cipher suite that uses our
-		 * certificate
-		 */
-		(alg_a & SSL_aECDSA || alg_a & SSL_aECDH)
-		/* and we have an ECC certificate */
-		&& (s->cert->pkeys[SSL_PKEY_ECC].x509 != NULL)
-		/*
-		 * and the client specified a Supported Point Formats
-		 * extension
-		 */
-		&& ((s->session->tlsext_ecpointformatlist_length > 0) &&
-		(s->session->tlsext_ecpointformatlist != NULL))
-		/* and our certificate's point is compressed */
-		&& (
-		(s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info != NULL)
-		&& (s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info->key != NULL)
-		&& (s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info->key->public_key != NULL)
-		&& (s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info->key->public_key->data != NULL)
-		&& (
-		(*(s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info->key->public_key->data) == POINT_CONVERSION_COMPRESSED)
-		|| (*(s->cert->pkeys[SSL_PKEY_ECC].x509->cert_info->key->public_key->data) == POINT_CONVERSION_COMPRESSED + 1)
-		)
-		)
-		) {
-			ec_ok = 0;
-			/*
-			 * If our certificate's curve is over a field type
-			 * that the client does not support then do not allow
-			 * this cipher suite to be negotiated
-			 */
-			if (
-			(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec != NULL)
-			&& (s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group != NULL)
-			&& (s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth != NULL)
-			&& (EC_METHOD_get_field_type(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth) == NID_X9_62_prime_field)
-			) {
-				for (j = 0; j < s->session->tlsext_ecpointformatlist_length; j++) {
-					if (s->session->tlsext_ecpointformatlist[j] == TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime) {
-						ec_ok = 1;
-						break;
-					}
-				}
-			} else if (EC_METHOD_get_field_type(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth) == NID_X9_62_characteristic_two_field) {
-				for (j = 0; j < s->session->tlsext_ecpointformatlist_length; j++) {
-					if (s->session->tlsext_ecpointformatlist[j] == TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2) {
-						ec_ok = 1;
-						break;
-					}
-				}
-			}
-			ok = ok && ec_ok;
-		}
-		if (
 		/*
 		 * If we are considering an ECC cipher suite that uses our
-		 * certificate
+		 * certificate check it.
 		 */
-		(alg_a & SSL_aECDSA || alg_a & SSL_aECDH)
-		/* and we have an ECC certificate */
-		&& (s->cert->pkeys[SSL_PKEY_ECC].x509 != NULL)
-		/* and the client specified an EllipticCurves extension */
-		&& ((s->session->tlsext_ellipticcurvelist_length > 0) && (s->session->tlsext_ellipticcurvelist != NULL))
-		) {
-			ec_ok = 0;
-			if (
-			(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec != NULL)
-			&& (s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group != NULL)
-			) {
-				ec_nid = EC_GROUP_get_curve_name(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group);
-				if ((ec_nid == 0)
-				&& (s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth != NULL)
-				) {
-					if (EC_METHOD_get_field_type(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth) == NID_X9_62_prime_field) {
-						ec_search1 = 0xFF;
-						ec_search2 = 0x01;
-					} else if (EC_METHOD_get_field_type(s->cert->pkeys[SSL_PKEY_ECC].privatekey->pkey.ec->group->meth) == NID_X9_62_characteristic_two_field) {
-						ec_search1 = 0xFF;
-						ec_search2 = 0x02;
-					}
-				} else {
-					ec_search1 = 0x00;
-					ec_search2 = tls1_ec_nid2curve_id(ec_nid);
-				}
-				if ((ec_search1 != 0) || (ec_search2 != 0)) {
-					for (j = 0; j < s->session->tlsext_ellipticcurvelist_length / 2; j++) {
-						if ((s->session->tlsext_ellipticcurvelist[2*j] == ec_search1) && (s->session->tlsext_ellipticcurvelist[2*j + 1] == ec_search2)) {
-							ec_ok = 1;
-							break;
-						}
-					}
-				}
-			}
-			ok = ok && ec_ok;
-		}
-		if (
+		if (alg_a & (SSL_aECDSA|SSL_aECDH))
+			ok = ok && tls1_check_ec_server_key(s);
 		/*
-		 * if we are considering an ECC cipher suite that uses an
-		 * ephemeral EC key
+		 * If we are considering an ECC cipher suite that uses
+		 * an ephemeral EC key check it.
 		 */
-		(alg_k & SSL_kECDHE)
-		/* and we have an ephemeral EC key */
-		&& (s->cert->ecdh_tmp != NULL)
-		/* and the client specified an EllipticCurves extension */
-		&& ((s->session->tlsext_ellipticcurvelist_length > 0) && (s->session->tlsext_ellipticcurvelist != NULL))
-		) {
-			ec_ok = 0;
-			if (s->cert->ecdh_tmp->group != NULL) {
-				ec_nid = EC_GROUP_get_curve_name(s->cert->ecdh_tmp->group);
-				if ((ec_nid == 0)
-				&& (s->cert->ecdh_tmp->group->meth != NULL)
-				) {
-					if (EC_METHOD_get_field_type(s->cert->ecdh_tmp->group->meth) == NID_X9_62_prime_field) {
-						ec_search1 = 0xFF;
-						ec_search2 = 0x01;
-					} else if (EC_METHOD_get_field_type(s->cert->ecdh_tmp->group->meth) == NID_X9_62_characteristic_two_field) {
-						ec_search1 = 0xFF;
-						ec_search2 = 0x02;
-					}
-				} else {
-					ec_search1 = 0x00;
-					ec_search2 = tls1_ec_nid2curve_id(ec_nid);
-				}
-				if ((ec_search1 != 0) || (ec_search2 != 0)) {
-					for (j = 0; j < s->session->tlsext_ellipticcurvelist_length / 2; j++) {
-						if ((s->session->tlsext_ellipticcurvelist[2*j] == ec_search1) && (s->session->tlsext_ellipticcurvelist[2*j + 1] == ec_search2)) {
-							ec_ok = 1;
-							break;
-						}
-					}
-				}
-			}
-			ok = ok && ec_ok;
-		}
+		if (alg_k & SSL_kECDHE)
+			ok = ok && tls1_check_ec_tmp_key(s);
 
 		if (!ok)
 			continue;
diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 3eee18cbd60..8ec4c69d5be 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.69 2014/09/27 11:01:06 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.70 2014/09/30 15:40:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -839,6 +839,9 @@ long ssl_get_algorithm2(SSL *s);
 int tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize);
 int tls12_get_req_sig_algs(SSL *s, unsigned char *p);
 
+int tls1_check_ec_server_key(SSL *s);
+int tls1_check_ec_tmp_key(SSL *s);
+
 int ssl_add_clienthello_use_srtp_ext(SSL *s, unsigned char *p,
     int *len, int maxlen);
 int ssl_parse_clienthello_use_srtp_ext(SSL *s, unsigned char *d,
diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 20f576e796e..d40768560cb 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.58 2014/09/27 11:01:06 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.59 2014/09/30 15:40:09 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -110,11 +110,13 @@
  */
 
 #include <stdio.h>
-#include <openssl/objects.h>
+
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
+#include <openssl/objects.h>
 #include <openssl/ocsp.h>
 #include <openssl/rand.h>
+
 #include "ssl_locl.h"
 
 static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen,
@@ -406,6 +408,134 @@ tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
 	return (0);
 }
 
+/* For an EC key set TLS ID and required compression based on parameters. */
+static int
+tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, EC_KEY *ec)
+{
+	const EC_GROUP *grp;
+	const EC_METHOD *meth;
+	int is_prime = 0;
+	int nid, id;
+
+	if (ec == NULL)
+		return (0);
+
+	if (EC_KEY_get0_public_key(ec) == NULL)
+		return (0);
+
+	/* Determine if it is a prime field. */
+	if ((grp = EC_KEY_get0_group(ec)) == NULL)
+		return (0);
+	if ((meth = EC_GROUP_method_of(grp)) == NULL)
+		return (0);
+	if (EC_METHOD_get_field_type(meth) == NID_X9_62_prime_field)
+		is_prime = 1;
+
+	/* Determine curve ID. */
+	nid = EC_GROUP_get_curve_name(grp);
+	id = tls1_ec_nid2curve_id(nid);
+
+	/* If we have an ID set it, otherwise set arbitrary explicit curve. */
+	if (id != 0) {
+		curve_id[0] = 0;
+		curve_id[1] = (unsigned char)id;
+	} else {
+		curve_id[0] = 0xff;
+		curve_id[1] = is_prime ? 0x01 : 0x02;
+	}
+
+	/* Specify the compression identifier. */
+	if (comp_id != NULL) {
+		if (EC_KEY_get_conv_form(ec) == POINT_CONVERSION_COMPRESSED) {
+			*comp_id = is_prime ?
+			    TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime :
+			    TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2;
+		} else {
+			*comp_id = TLSEXT_ECPOINTFORMAT_uncompressed;
+		}
+	}
+	return (1);
+}
+
+/* Check that an EC key is compatible with extensions. */
+static int
+tls1_check_ec_key(SSL *s, unsigned char *curve_id, unsigned char *comp_id)
+{
+	const unsigned char *p;
+	size_t plen, i;
+
+	/*
+	 * Check point formats extension if present, otherwise everything
+	 * is supported (see RFC4492).
+	 */
+	if (comp_id != NULL && s->session->tlsext_ecpointformatlist != NULL) {
+		p = s->session->tlsext_ecpointformatlist;
+		plen = s->session->tlsext_ecpointformatlist_length;
+		for (i = 0; i < plen; i++, p++) {
+			if (*comp_id == *p)
+				break;
+		}
+		if (i == plen)
+			return (0);
+	}
+
+	/*
+	 * Check curve list if present, otherwise everything is supported.
+	 */
+	if (s->session->tlsext_ellipticcurvelist != NULL) {
+		p = s->session->tlsext_ellipticcurvelist;
+		plen = s->session->tlsext_ellipticcurvelist_length;
+		for (i = 0; i < plen; i += 2, p += 2) {
+			if (p[0] == curve_id[0] && p[1] == curve_id[1])
+				break;
+		}
+		if (i == plen)
+			return (0);
+	}
+
+	return (1);
+}
+
+/* Check EC server key is compatible with client extensions. */
+int
+tls1_check_ec_server_key(SSL *s)
+{
+	CERT_PKEY *cpk = s->cert->pkeys + SSL_PKEY_ECC;
+	unsigned char comp_id, curve_id[2];
+	EVP_PKEY *pkey;
+	int rv;
+
+	if (cpk->x509 == NULL || cpk->privatekey == NULL)
+		return (0);
+	if ((pkey = X509_get_pubkey(cpk->x509)) == NULL)
+		return (0);
+	rv = tls1_set_ec_id(curve_id, &comp_id, pkey->pkey.ec);
+	EVP_PKEY_free(pkey);
+	if (rv != 1)
+		return (0);
+
+	return tls1_check_ec_key(s, curve_id, &comp_id);
+}
+
+/* Check EC temporary key is compatible with client extensions. */
+int
+tls1_check_ec_tmp_key(SSL *s)
+{
+	EC_KEY *ec = s->cert->ecdh_tmp;
+	unsigned char curve_id[2];
+
+	if (ec == NULL) {
+		if (s->cert->ecdh_tmp_cb != NULL)
+			return (1);
+		else
+			return (0);
+	}
+	if (tls1_set_ec_id(curve_id, NULL, ec) != 1)
+		return (0);
+
+	return tls1_check_ec_key(s, curve_id, NULL);
+}
+
 /*
  * List of supported signature algorithms and hashes. Should make this
  * customisable at some point, for now include everything we support.
@@ -2132,4 +2262,3 @@ tls1_process_sigalgs(SSL *s, const unsigned char *data, int dsize)
 		c->pkeys[SSL_PKEY_ECC].digest = EVP_sha1();
 	return 1;
 }
-