Use appropriate internal types for EC curves and formats, rather than

storing and processing in wire encoded form.

Inspired by boringssl.

ok miod@

3 files changed, 124 insertions, 111 deletions

diff --git a/lib/libssl/ssl.h b/lib/libssl/ssl.h
index 2416b46d46f..2b14116e4cd 100644
--- a/lib/libssl/ssl.h
+++ b/lib/libssl/ssl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl.h,v 1.72 2014/11/18 05:33:43 miod Exp $ */
+/* $OpenBSD: ssl.h,v 1.73 2014/12/06 13:51:06 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -500,9 +500,10 @@ struct ssl_session_st {
 	struct ssl_session_st *prev, *next;
 	char *tlsext_hostname;
 	size_t tlsext_ecpointformatlist_length;
-	unsigned char *tlsext_ecpointformatlist; /* peer's list */
+	uint8_t *tlsext_ecpointformatlist; /* peer's list */
 	size_t tlsext_ellipticcurvelist_length;
-	unsigned char *tlsext_ellipticcurvelist; /* peer's list */
+	uint16_t *tlsext_ellipticcurvelist; /* peer's list */
+
 	/* RFC4507 info */
 	unsigned char *tlsext_tick;	/* Session ticket */
 	size_t tlsext_ticklen;		/* Session ticket length */
@@ -1142,9 +1143,9 @@ struct ssl_st {
 	/* RFC4507 session ticket expected to be received or sent */
 	int tlsext_ticket_expected;
 	size_t tlsext_ecpointformatlist_length;
-	unsigned char *tlsext_ecpointformatlist; /* our list */
+	uint8_t *tlsext_ecpointformatlist; /* our list */
 	size_t tlsext_ellipticcurvelist_length;
-	unsigned char *tlsext_ellipticcurvelist; /* our list */
+	uint16_t *tlsext_ellipticcurvelist; /* our list */
 
 	/* TLS Session Ticket extension override */
 	TLS_SESSION_TICKET_EXT *tlsext_session_ticket;
diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 74cacd4eec3..c425f67a5a5 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.77 2014/11/18 05:33:43 miod Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.78 2014/12/06 13:51:06 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -800,8 +800,8 @@ int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s);
 
 SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n);
 
-int tls1_ec_curve_id2nid(int curve_id);
-int tls1_ec_nid2curve_id(int nid);
+int tls1_ec_curve_id2nid(uint16_t curve_id);
+uint16_t tls1_ec_nid2curve_id(int nid);
 int tls1_check_curve(SSL *s, const unsigned char *p, size_t len);
 int tls1_get_shared_curve(SSL *s);
 
diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 5a6c0ddba0d..8a7553e3e73 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.70 2014/12/06 13:28:56 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.71 2014/12/06 13:51:06 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -240,45 +240,45 @@ static int nid_list[] = {
 	NID_brainpoolP512r1	/* brainpoolP512r1 (28) */
 };
 
-static const unsigned char ecformats_default[] = {
+static const uint8_t ecformats_default[] = {
 	TLSEXT_ECPOINTFORMAT_uncompressed,
 	TLSEXT_ECPOINTFORMAT_ansiX962_compressed_prime,
 	TLSEXT_ECPOINTFORMAT_ansiX962_compressed_char2
 };
 
-static const unsigned char eccurves_default[] = {
-	0,14,			/* sect571r1 (14) */
-	0,13,			/* sect571k1 (13) */
-	0,25,			/* secp521r1 (25) */
-	0,28,			/* brainpool512r1 (28) */
-	0,11,			/* sect409k1 (11) */
-	0,12,			/* sect409r1 (12) */
-	0,27,			/* brainpoolP384r1 (27) */
-	0,24,			/* secp384r1 (24) */
-	0,9,			/* sect283k1 (9) */
-	0,10,			/* sect283r1 (10) */
-	0,26,			/* brainpoolP256r1 (26) */
-	0,22,			/* secp256k1 (22) */
-	0,23,			/* secp256r1 (23) */
-	0,8,			/* sect239k1 (8) */
-	0,6,			/* sect233k1 (6) */
-	0,7,			/* sect233r1 (7) */
-	0,20,			/* secp224k1 (20) */
-	0,21,			/* secp224r1 (21) */
-	0,4,			/* sect193r1 (4) */
-	0,5,			/* sect193r2 (5) */
-	0,18,			/* secp192k1 (18) */
-	0,19,			/* secp192r1 (19) */
-	0,1,			/* sect163k1 (1) */
-	0,2,			/* sect163r1 (2) */
-	0,3,			/* sect163r2 (3) */
-	0,15,			/* secp160k1 (15) */
-	0,16,			/* secp160r1 (16) */
-	0,17,			/* secp160r2 (17) */
+static const uint16_t eccurves_default[] = {
+	14,			/* sect571r1 (14) */
+	13,			/* sect571k1 (13) */
+	25,			/* secp521r1 (25) */
+	28,			/* brainpool512r1 (28) */
+	11,			/* sect409k1 (11) */
+	12,			/* sect409r1 (12) */
+	27,			/* brainpoolP384r1 (27) */
+	24,			/* secp384r1 (24) */
+	9,			/* sect283k1 (9) */
+	10,			/* sect283r1 (10) */
+	26,			/* brainpoolP256r1 (26) */
+	22,			/* secp256k1 (22) */
+	23,			/* secp256r1 (23) */
+	8,			/* sect239k1 (8) */
+	6,			/* sect233k1 (6) */
+	7,			/* sect233r1 (7) */
+	20,			/* secp224k1 (20) */
+	21,			/* secp224r1 (21) */
+	4,			/* sect193r1 (4) */
+	5,			/* sect193r2 (5) */
+	18,			/* secp192k1 (18) */
+	19,			/* secp192r1 (19) */
+	1,			/* sect163k1 (1) */
+	2,			/* sect163r1 (2) */
+	3,			/* sect163r2 (3) */
+	15,			/* secp160k1 (15) */
+	16,			/* secp160r1 (16) */
+	17,			/* secp160r2 (17) */
 };
 
 int
-tls1_ec_curve_id2nid(int curve_id)
+tls1_ec_curve_id2nid(uint16_t curve_id)
 {
 	/* ECC curves from draft-ietf-tls-ecc-12.txt (Oct. 17, 2005) */
 	if ((curve_id < 1) ||
@@ -287,7 +287,7 @@ tls1_ec_curve_id2nid(int curve_id)
 	return nid_list[curve_id - 1];
 }
 
-int
+uint16_t
 tls1_ec_nid2curve_id(int nid)
 {
 	/* ECC curves from draft-ietf-tls-ecc-12.txt (Oct. 17, 2005) */
@@ -359,7 +359,7 @@ tls1_ec_nid2curve_id(int nid)
  * exists, or the default formats if a custom list has not been specified.
  */
 static void
-tls1_get_formatlist(SSL *s, int client_formats, const unsigned char **pformats,
+tls1_get_formatlist(SSL *s, int client_formats, const uint8_t **pformats,
     size_t *pformatslen)
 {
 	if (client_formats != 0) {
@@ -382,7 +382,7 @@ tls1_get_formatlist(SSL *s, int client_formats, const unsigned char **pformats,
  * exists, or the default curves if a custom list has not been specified.
  */
 static void
-tls1_get_curvelist(SSL *s, int client_curves, const unsigned char **pcurves,
+tls1_get_curvelist(SSL *s, int client_curves, const uint16_t **pcurves,
     size_t *pcurveslen)
 {
 	if (client_curves != 0) {
@@ -403,17 +403,20 @@ tls1_get_curvelist(SSL *s, int client_curves, const unsigned char **pcurves,
 int
 tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
 {
-	const unsigned char *curves;
+	const uint16_t *curves;
 	size_t curveslen, i;
+	uint16_t cid;
 
 	/* Only named curves are supported. */
 	if (len != 3 || p[0] != NAMED_CURVE_TYPE)
 		return (0);
 
+	cid = (p[1] << 8) | p[2];
+
 	tls1_get_curvelist(s, 0, &curves, &curveslen);
 
-	for (i = 0; i < curveslen; i += 2, curves += 2) {
-		if (p[1] == curves[0] && p[2] == curves[1])
+	for (i = 0; i < curveslen; i++) {
+		if (curves[i] == cid)
 			return (1);
 	}
 	return (0);
@@ -422,10 +425,9 @@ tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
 int
 tls1_get_shared_curve(SSL *s)
 {
-	const unsigned char *pref, *supp, *tsupp;
 	size_t preflen, supplen, i, j;
+	const uint16_t *pref, *supp;
 	unsigned long server_pref;
-	int id;
 
 	/* Cannot do anything on the client side. */
 	if (s->server == 0)
@@ -436,13 +438,10 @@ tls1_get_shared_curve(SSL *s)
 	tls1_get_curvelist(s, (server_pref == 0), &pref, &preflen);
 	tls1_get_curvelist(s, (server_pref != 0), &supp, &supplen);
 
-	for (i = 0; i < preflen; i += 2, pref += 2) {
-		tsupp = supp;
-		for (j = 0; j < supplen; j += 2, tsupp += 2) {
-			if (pref[0] == tsupp[0] && pref[1] == tsupp[1]) {
-				id = (pref[0] << 8) | pref[1];
-				return (tls1_ec_curve_id2nid(id));
-			}
+	for (i = 0; i < preflen; i++) {
+		for (j = 0; j < supplen; j++) {
+			if (pref[i] == supp[j])
+				return (tls1_ec_curve_id2nid(pref[i]));
 		}
 	}
 	return (NID_undef);
@@ -450,7 +449,7 @@ tls1_get_shared_curve(SSL *s)
 
 /* 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)
+tls1_set_ec_id(uint16_t *curve_id, uint8_t *comp_id, EC_KEY *ec)
 {
 	const EC_GROUP *grp;
 	const EC_METHOD *meth;
@@ -473,13 +472,10 @@ tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, EC_KEY *ec)
 	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;
-	}
+	if (id != 0)
+		*curve_id = id;
+	else
+		*curve_id = is_prime ? 0xff01 : 0xff02;
 
 	/* Specify the compression identifier. */
 	if (comp_id != NULL) {
@@ -499,10 +495,11 @@ tls1_set_ec_id(unsigned char *curve_id, unsigned char *comp_id, EC_KEY *ec)
 
 /* 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)
+tls1_check_ec_key(SSL *s, const uint16_t *curve_id, const uint8_t *comp_id)
 {
-	const unsigned char *curves, *formats;
 	size_t curveslen, formatslen, i;
+	const uint16_t *curves;
+	const uint8_t *formats;
 
 	/*
 	 * Check point formats extension if present, otherwise everything
@@ -510,8 +507,8 @@ tls1_check_ec_key(SSL *s, unsigned char *curve_id, unsigned char *comp_id)
 	 */
 	tls1_get_formatlist(s, 1, &formats, &formatslen);
 	if (comp_id != NULL && formats != NULL) {
-		for (i = 0; i < formatslen; i++, formats++) {
-			if (*comp_id == *formats)
+		for (i = 0; i < formatslen; i++) {
+			if (formats[i] == *comp_id)
 				break;
 		}
 		if (i == formatslen)
@@ -522,10 +519,9 @@ tls1_check_ec_key(SSL *s, unsigned char *curve_id, unsigned char *comp_id)
 	 * Check curve list if present, otherwise everything is supported.
 	 */
 	tls1_get_curvelist(s, 1, &curves, &curveslen);
-	if (curves != NULL) {
-		for (i = 0; i < curveslen; i += 2, curves += 2) {
-			if (curves[0] == curve_id[0] &&
-			    curves[1] == curve_id[1])
+	if (curve_id != NULL && curves != NULL) {
+		for (i = 0; i < curveslen; i++) {
+			if (curves[i] == *curve_id)
 				break;
 		}
 		if (i == curveslen)
@@ -540,7 +536,8 @@ int
 tls1_check_ec_server_key(SSL *s)
 {
 	CERT_PKEY *cpk = s->cert->pkeys + SSL_PKEY_ECC;
-	unsigned char comp_id, curve_id[2];
+	uint16_t curve_id;
+	uint8_t comp_id;
 	EVP_PKEY *pkey;
 	int rv;
 
@@ -548,12 +545,12 @@ tls1_check_ec_server_key(SSL *s)
 		return (0);
 	if ((pkey = X509_get_pubkey(cpk->x509)) == NULL)
 		return (0);
-	rv = tls1_set_ec_id(curve_id, &comp_id, pkey->pkey.ec);
+	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);
+	return tls1_check_ec_key(s, &curve_id, &comp_id);
 }
 
 /* Check EC temporary key is compatible with client extensions. */
@@ -561,7 +558,7 @@ int
 tls1_check_ec_tmp_key(SSL *s)
 {
 	EC_KEY *ec = s->cert->ecdh_tmp;
-	unsigned char curve_id[2];
+	uint16_t curve_id;
 
 	if (s->cert->ecdh_tmp_auto != 0) {
 		/* Need a shared curve. */
@@ -575,10 +572,10 @@ tls1_check_ec_tmp_key(SSL *s)
 			return (1);
 		return (0);
 	}
-	if (tls1_set_ec_id(curve_id, NULL, ec) != 1)
+	if (tls1_set_ec_id(&curve_id, NULL, ec) != 1)
 		return (0);
 
-	return tls1_check_ec_key(s, curve_id, NULL);
+	return tls1_check_ec_key(s, &curve_id, NULL);
 }
 
 /*
@@ -723,8 +720,10 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 	}
 
 	if (using_ecc) {
-		const unsigned char *curves, *formats;
 		size_t curveslen, formatslen, lenmax;
+		const uint16_t *curves;
+		const uint8_t *formats;
+		int i;
 
 		/*
 		 * Add TLS extension ECPointFormats to the ClientHello message.
@@ -767,16 +766,16 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 		}
 
 		s2n(TLSEXT_TYPE_elliptic_curves, ret);
-		s2n(curveslen + 2, ret);
+		s2n((curveslen * 2) + 2, ret);
 
 		/* NB: draft-ietf-tls-ecc-12.txt uses a one-byte prefix for
 		 * elliptic_curve_list, but the examples use two bytes.
 		 * http://www1.ietf.org/mail-archive/web/tls/current/msg00538.html
 		 * resolves this to two bytes.
 		 */
-		s2n(curveslen, ret);
-		memcpy(ret, curves, curveslen);
-		ret += curveslen;
+		s2n(curveslen * 2, ret);
+		for (i = 0; i < curveslen; i++)
+			s2n(curves[i], ret);
 	}
 
 	if (!(SSL_get_options(s) & SSL_OP_NO_TICKET)) {
@@ -1334,60 +1333,66 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
 		else if (type == TLSEXT_TYPE_ec_point_formats &&
 		    s->version != DTLS1_VERSION) {
 			unsigned char *sdata = data;
-			int ecpointformatlist_length;
+			size_t formatslen;
+			uint8_t *formats;
 
 			if (size < 1) {
-				*al = SSL_AD_DECODE_ERROR;
+				*al = TLS1_AD_DECODE_ERROR;
 				return 0;
 			}
-			ecpointformatlist_length = *(sdata++);
-
-			if (ecpointformatlist_length != size - 1) {
+			formatslen = *(sdata++);
+			if (formatslen != size - 1) {
 				*al = TLS1_AD_DECODE_ERROR;
 				return 0;
 			}
+
 			if (!s->hit) {
 				free(s->session->tlsext_ecpointformatlist);
+				s->session->tlsext_ecpointformatlist = NULL;
 				s->session->tlsext_ecpointformatlist_length = 0;
-				if ((s->session->tlsext_ecpointformatlist =
-				    malloc(ecpointformatlist_length)) == NULL) {
+
+				if ((formats = reallocarray(NULL, formatslen,
+				    sizeof(uint8_t))) == NULL) {
 					*al = TLS1_AD_INTERNAL_ERROR;
 					return 0;
 				}
-				s->session->tlsext_ecpointformatlist_length = ecpointformatlist_length;
-				memcpy(s->session->tlsext_ecpointformatlist, sdata, ecpointformatlist_length);
+				memcpy(formats, sdata, formatslen);
+				s->session->tlsext_ecpointformatlist = formats;
+				s->session->tlsext_ecpointformatlist_length =
+				    formatslen;
 			}
 		} else if (type == TLSEXT_TYPE_elliptic_curves &&
 		    s->version != DTLS1_VERSION) {
 			unsigned char *sdata = data;
-			int ellipticcurvelist_length;
+			size_t curveslen, i;
+			uint16_t *curves;
 
 			if (size < 2) {
-				*al = SSL_AD_DECODE_ERROR;
+				*al = TLS1_AD_DECODE_ERROR;
 				return 0;
 			}
-			ellipticcurvelist_length = (*(sdata++) << 8);
-			ellipticcurvelist_length += (*(sdata++));
-
-			if (ellipticcurvelist_length != size - 2 ||
-			    ellipticcurvelist_length < 1 ||
-			    ellipticcurvelist_length % 2 != 0) {
+			n2s(sdata, curveslen);
+			if (curveslen != size - 2 || curveslen % 2 != 0) {
 				*al = TLS1_AD_DECODE_ERROR;
 				return 0;
 			}
+			curveslen /= 2;
+
 			if (!s->hit) {
 				if (s->session->tlsext_ellipticcurvelist) {
 					*al = TLS1_AD_DECODE_ERROR;
 					return 0;
 				}
 				s->session->tlsext_ellipticcurvelist_length = 0;
-				if ((s->session->tlsext_ellipticcurvelist =
-				    malloc(ellipticcurvelist_length)) == NULL) {
+				if ((curves = reallocarray(NULL, curveslen,
+				    sizeof(uint16_t))) == NULL) {
 					*al = TLS1_AD_INTERNAL_ERROR;
 					return 0;
 				}
-				s->session->tlsext_ellipticcurvelist_length = ellipticcurvelist_length;
-				memcpy(s->session->tlsext_ellipticcurvelist, sdata, ellipticcurvelist_length);
+				for (i = 0; i < curveslen; i++)
+					n2s(sdata, curves[i]);
+				s->session->tlsext_ellipticcurvelist = curves;
+				s->session->tlsext_ellipticcurvelist_length = curveslen;
 			}
 		}
 		else if (type == TLSEXT_TYPE_session_ticket) {
@@ -1628,26 +1633,33 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
 		else if (type == TLSEXT_TYPE_ec_point_formats &&
 		    s->version != DTLS1_VERSION) {
 			unsigned char *sdata = data;
-			int ecpointformatlist_length = *(sdata++);
+			size_t formatslen;
+			uint8_t *formats;
 
-			if (ecpointformatlist_length != size - 1 ||
-			    ecpointformatlist_length < 1) {
+			if (size < 1) {
 				*al = TLS1_AD_DECODE_ERROR;
 				return 0;
 			}
+			formatslen = *(sdata++);
+			if (formatslen != size - 1) {
+				*al = TLS1_AD_DECODE_ERROR;
+				return 0;
+			}
+
 			if (!s->hit) {
 				free(s->session->tlsext_ecpointformatlist);
+				s->session->tlsext_ecpointformatlist = NULL;
 				s->session->tlsext_ecpointformatlist_length = 0;
 
-				if ((s->session->tlsext_ecpointformatlist =
-				    malloc(ecpointformatlist_length)) == NULL) {
+				if ((formats = reallocarray(NULL, formatslen,
+				    sizeof(uint8_t))) == NULL) {
 					*al = TLS1_AD_INTERNAL_ERROR;
 					return 0;
 				}
+				memcpy(formats, sdata, formatslen);
+				s->session->tlsext_ecpointformatlist = formats;
 				s->session->tlsext_ecpointformatlist_length =
-				    ecpointformatlist_length;
-				memcpy(s->session->tlsext_ecpointformatlist,
-				    sdata, ecpointformatlist_length);
+				    formatslen;
 			}
 		}
 		else if (type == TLSEXT_TYPE_session_ticket) {