From: Dr. Stephen Henson <steve@openssl.org>
Date: Wed, 4 Apr 2012 14:41:01 +0000 (+0000)
Subject: Tidy up EC parameter check code: instead of accessing internal structures
X-Git-Tag: master-post-reformat~1880
X-Git-Url: https://git.openssl.org/gitweb/?p=openssl.git;a=commitdiff_plain;h=fd2b65ce5385a09b99c9be5e8ebbb56fb0167226

Tidy up EC parameter check code: instead of accessing internal structures
add utility functions to t1_lib.c to check if EC certificates and parameters
are consistent with peer.
---

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index f680d35233..1ff5e9db55 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -152,11 +152,6 @@
 #include <openssl/objects.h>
 #include "ssl_locl.h"
 #include "kssl_lcl.h"
-#ifndef OPENSSL_NO_TLSEXT
-#ifndef OPENSSL_NO_EC
-#include "../crypto/ec/ec_lcl.h"
-#endif /* OPENSSL_NO_EC */
-#endif /* OPENSSL_NO_TLSEXT */
 #include <openssl/md5.h>
 #ifndef OPENSSL_NO_DH
 #include <openssl/dh.h>
@@ -3844,11 +3839,6 @@ SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
 	SSL_CIPHER *c,*ret=NULL;
 	STACK_OF(SSL_CIPHER) *prio, *allow;
 	int i,ii,ok;
-#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_EC)
-	unsigned int j;
-	int ec_ok, ec_nid;
-	unsigned char ec_search1 = 0, ec_search2 = 0;
-#endif
 	CERT *cert;
 	unsigned long alg_k,alg_a,mask_k,mask_a,emask_k,emask_a;
 
@@ -3949,155 +3939,14 @@ SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt,
 
 #ifndef OPENSSL_NO_TLSEXT
 #ifndef OPENSSL_NO_EC
-		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 */
-			(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 we are considering an ECC cipher suite that uses an ephemeral EC key */
-			(alg_k & SSL_kEECDH)
-			/* 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 we are considering an ECC cipher suite that uses our
+		 * certificate check it */
+		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 check it */
+		if (alg_k & SSL_kEECDH)
+			ok = ok && tls1_check_ec_tmp_key(s);
 #endif /* OPENSSL_NO_EC */
 #endif /* OPENSSL_NO_TLSEXT */
 
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 91169da523..1b646515a0 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1107,6 +1107,8 @@ int tls1_set_curves(unsigned char **pext, size_t *pextlen,
 			int *curves, size_t ncurves);
 int tls1_set_curves_list(unsigned char **pext, size_t *pextlen, 
 				const char *str);
+int tls1_check_ec_server_key(SSL *s);
+int tls1_check_ec_tmp_key(SSL *s);
 #endif /* OPENSSL_NO_EC */
 
 #ifndef OPENSSL_NO_TLSEXT
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 7a8dc31203..ffc78c9bae 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -307,6 +307,30 @@ int tls1_ec_nid2curve_id(int nid)
 		return 0;
 		}
 	}
+/* Get curves list, if "sess" is set return client curves otherwise
+ * preferred list
+ */
+static void tls1_get_curvelist(SSL *s, int sess,
+					const unsigned char **pcurves,
+					size_t *pcurveslen)
+	{
+	if (sess)
+		{
+		*pcurves = s->session->tlsext_ellipticcurvelist;
+		*pcurveslen = s->session->tlsext_ellipticcurvelist_length;
+		}
+	else
+		{
+		*pcurves = s->tlsext_ellipticcurvelist;
+		*pcurveslen = s->tlsext_ellipticcurvelist_length;
+		}
+	/* If not set use default: for now static structure */
+	if (!*pcurves)
+		{
+		*pcurves = eccurves_default;
+		*pcurveslen = sizeof(eccurves_default);
+		}
+	}
 
 /* Return any common values from two lists. One list is used as a 
  * preference list where we return the most preferred match.
@@ -362,28 +386,10 @@ int tls1_shared_curve(SSL *s, int nmatch)
 	/* Can't do anything on client side */
 	if (s->server == 0)
 		return -1;
-	/* Use our preferred curve list, if not set use default */
-	if (s->tlsext_ellipticcurvelist)
-		{
-		l1 = s->tlsext_ellipticcurvelist;
-		l1len = s->tlsext_ellipticcurvelist_length;
-		}
-	else
-		{
-		l1 = eccurves_default;
-		l1len = sizeof(eccurves_default);
-		}
-	/* Use peer preferred curve list, if not set use default */
-	if(s->session->tlsext_ellipticcurvelist)
-		{
-		l2 = s->session->tlsext_ellipticcurvelist;
-		l2len =s->session->tlsext_ellipticcurvelist_length;
-		}
-	else
-		{
-		l2 = eccurves_default;
-		l2len = sizeof(eccurves_default);
-		}
+	/* Get supported curves */
+	tls1_get_curvelist(s, 0, &l1, &l1len);
+	tls1_get_curvelist(s, 1, &l2, &l2len);
+
 	id = tls1_shared_list(s, l1, l1len, l2, l2len, nmatch);
 	if (nmatch == -1)
 		return id;
@@ -466,6 +472,130 @@ int tls1_set_curves_list(unsigned char **pext, size_t *pextlen,
 		return 0;
 	return tls1_set_curves(pext, pextlen, ncb.nid_arr, ncb.nidcnt);
 	}
+/* 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)
+	{
+	int is_prime, id;
+	const EC_GROUP *grp;
+	const EC_POINT *pt;
+	const EC_METHOD *meth;
+	if (!ec)
+		return 0;
+	/* Determine if it is a prime field */
+	grp = EC_KEY_get0_group(ec);
+        pt = EC_KEY_get0_public_key(ec);
+	if (!grp || !pt)
+		return 0;
+        meth = EC_GROUP_method_of(grp);
+	if (!meth)
+		return 0;
+        if (EC_METHOD_get_field_type(meth) == NID_X9_62_prime_field)
+		is_prime = 1;
+	else
+		is_prime = 0;
+	/* Determine curve ID */
+	id = EC_GROUP_get_curve_name(grp);
+	id = tls1_ec_nid2curve_id(id);
+	/* If we have an ID set it, otherwise set arbitrary explicit curve */
+	if (id)
+		{
+		curve_id[0] = 0;
+		curve_id[1] = (unsigned char)id;
+		}
+	else
+		{
+		curve_id[0] = 0xff;
+		if (is_prime)
+			curve_id[1] = 0x01;
+		else
+			curve_id[1] = 0x02;
+		}
+	if (comp_id)
+		{
+		if (EC_KEY_get_conv_form(ec) == POINT_CONVERSION_COMPRESSED)
+			{
+			if (is_prime)
+				*comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime;
+			else
+				*comp_id = TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2;
+			}
+		else
+			*comp_id = TLSEXT_ECPOINTFORMAT_uncompressed;
+		}
+	return 1;
+	}
+/* Check 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;
+	/* If point formats extension present check it, otherwise everything
+	 * is supported (see RFC4492).
+	 */
+	if (comp_id && s->session->tlsext_ecpointformatlist)
+		{
+		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;
+		}
+	/* If curve list present check it, otherwise everything is
+	 * supported.
+	 */
+	if (s->session->tlsext_ellipticcurvelist)
+		{
+		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])
+				return 1;
+			}
+		return 0;
+		}
+	return 1;
+	}
+/* Check EC server key is compatible with client extensions */
+int tls1_check_ec_server_key(SSL *s)
+	{
+	int rv;
+	CERT_PKEY *cpk = s->cert->pkeys + SSL_PKEY_ECC;
+	EVP_PKEY *pkey;
+	unsigned char comp_id, curve_id[2];
+	if (!cpk->x509 || !cpk->privatekey)
+		return 0;
+	pkey = X509_get_pubkey(cpk->x509);
+	if (!pkey)
+		return 0;
+	rv = tls1_set_ec_id(curve_id, &comp_id, pkey->pkey.ec);
+	EVP_PKEY_free(pkey);
+	if (!rv)
+		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)
+	{
+	unsigned char curve_id[2];
+	EC_KEY *ec = s->cert->ecdh_tmp;
+	if (!ec)
+		{
+		if (s->cert->ecdh_tmp_cb)
+			return 1;
+		else
+			return 0;
+		}
+	if (!tls1_set_ec_id(curve_id, NULL, ec))
+		return 1;
+	return tls1_check_ec_key(s, curve_id, NULL);
+	}
 
 #endif /* OPENSSL_NO_EC */
 
@@ -685,15 +815,7 @@ unsigned char *ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned cha
 
 		/* Add TLS extension EllipticCurves to the ClientHello message */
 		plist = s->tlsext_ellipticcurvelist;
-		/* If we have a custom curve list use it otherwise
-		 * use default */
-		if (plist)
-			plistlen = s->tlsext_ellipticcurvelist_length;
-		else
-			{
-			plist = eccurves_default;
-			plistlen = sizeof(eccurves_default);
-			}
+		tls1_get_curvelist(s, 0, &plist, &plistlen);
 
 		if ((lenmax = limit - ret - 6) < 0) return NULL; 
 		if (plistlen > (size_t)lenmax) return NULL;