Rewrite the ECPointFormats TLS extension handling using CBB/CBS and the

new extension framework.

input + ok jsing@

4 files changed, 125 insertions, 147 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 8ef2d014021..c11c5899e37 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.184 2017/08/10 17:18:38 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.185 2017/08/11 05:06:34 doug Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1380,6 +1380,9 @@ int SSL_state_func_code(int _state);
 #define SSLerrorx(r) ERR_PUT_error(ERR_LIB_SSL,(0xfff),(r),__FILE__,__LINE__)
 void SSL_error_internal(const SSL *s, int r, char *f, int l);
 
+void tls1_get_formatlist(SSL *s, int client_formats, const uint8_t **pformats,
+    size_t *pformatslen);
+
 __END_HIDDEN_DECLS
 
 #endif
diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index 400c69fa87e..e8723b502cd 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.3 2017/07/24 17:39:43 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.4 2017/08/11 05:06:34 doug Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  *
@@ -21,6 +21,103 @@
 #include "ssl_tlsext.h"
 
 /*
+ * Supported Point Formats Extension - RFC 4492 section 5.1.2
+ */
+static int
+tlsext_ecpf_build(SSL *s, CBB *cbb)
+{
+	CBB ecpf;
+	size_t formats_len;
+	const uint8_t *formats;
+
+	tls1_get_formatlist(s, 0, &formats, &formats_len);
+
+	if (formats_len == 0) {
+		SSLerror(s, ERR_R_INTERNAL_ERROR);
+		return 0;
+	}
+
+	if (!CBB_add_u8_length_prefixed(cbb, &ecpf))
+		return 0;
+	if (!CBB_add_bytes(&ecpf, formats, formats_len))
+		return 0;
+	if (!CBB_flush(cbb))
+		return 0;
+
+	return 1;
+}
+
+static int
+tlsext_ecpf_parse(SSL *s, CBS *cbs, int *alert)
+{
+	CBS ecpf;
+
+	if (!CBS_get_u8_length_prefixed(cbs, &ecpf))
+		goto err;
+	if (CBS_len(&ecpf) == 0)
+		goto err;
+	if (CBS_len(cbs) != 0)
+		goto err;
+
+	/* Must contain uncompressed (0) */
+	if (!CBS_contains_zero_byte(&ecpf)) {
+		SSLerror(s, SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST);
+		goto err;
+	}
+
+	if (!s->internal->hit) {
+		if (!CBS_stow(&ecpf, &(SSI(s)->tlsext_ecpointformatlist),
+		    &(SSI(s)->tlsext_ecpointformatlist_length)))
+			goto err;
+	}
+
+	return 1;
+
+ err:
+	*alert = TLS1_AD_INTERNAL_ERROR;
+	return 0;
+}
+
+int
+tlsext_ecpf_clienthello_needs(SSL *s)
+{
+	return ssl_has_ecc_ciphers(s);
+}
+
+int
+tlsext_ecpf_clienthello_build(SSL *s, CBB *cbb)
+{
+	return tlsext_ecpf_build(s, cbb);
+}
+
+int
+tlsext_ecpf_clienthello_parse(SSL *s, CBS *cbs, int *alert)
+{
+	return tlsext_ecpf_parse(s, cbs, alert);
+}
+
+int
+tlsext_ecpf_serverhello_needs(SSL *s)
+{
+	if (s->version == DTLS1_VERSION)
+		return 0;
+
+	return ssl_using_ecc_cipher(s);
+}
+
+int
+tlsext_ecpf_serverhello_build(SSL *s, CBB *cbb)
+{
+	return tlsext_ecpf_build(s, cbb);
+}
+
+int
+tlsext_ecpf_serverhello_parse(SSL *s, CBS *cbs, int *alert)
+{
+	return tlsext_ecpf_parse(s, cbs, alert);
+}
+
+/*
  * Renegotiation Indication - RFC 5746.
  */
 int
@@ -313,6 +410,15 @@ static struct tls_extension tls_extensions[] = {
 		.serverhello_build = tlsext_ri_serverhello_build,
 		.serverhello_parse = tlsext_ri_serverhello_parse,
 	},
+	{
+		.type = TLSEXT_TYPE_ec_point_formats,
+		.clienthello_needs = tlsext_ecpf_clienthello_needs,
+		.clienthello_build = tlsext_ecpf_clienthello_build,
+		.clienthello_parse = tlsext_ecpf_clienthello_parse,
+		.serverhello_needs = tlsext_ecpf_serverhello_needs,
+		.serverhello_build = tlsext_ecpf_serverhello_build,
+		.serverhello_parse = tlsext_ecpf_serverhello_parse,
+	},
 };
 
 #define N_TLS_EXTENSIONS (sizeof(tls_extensions) / sizeof(*tls_extensions))
diff --git a/lib/libssl/ssl_tlsext.h b/lib/libssl/ssl_tlsext.h
index 4b0194861a0..67a9cfb6889 100644
--- a/lib/libssl/ssl_tlsext.h
+++ b/lib/libssl/ssl_tlsext.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.h,v 1.2 2017/07/24 17:10:31 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.h,v 1.3 2017/08/11 05:06:34 doug Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  *
@@ -29,6 +29,13 @@ int tlsext_sni_serverhello_needs(SSL *s);
 int tlsext_sni_serverhello_build(SSL *s, CBB *cbb);
 int tlsext_sni_serverhello_parse(SSL *s, CBS *cbs, int *alert);
 
+int tlsext_ecpf_clienthello_needs(SSL *s);
+int tlsext_ecpf_clienthello_build(SSL *s, CBB *cbb);
+int tlsext_ecpf_clienthello_parse(SSL *s, CBS *cbs, int *alert);
+int tlsext_ecpf_serverhello_needs(SSL *s);
+int tlsext_ecpf_serverhello_build(SSL *s, CBB *cbb);
+int tlsext_ecpf_serverhello_parse(SSL *s, CBS *cbs, int *alert);
+
 int tlsext_clienthello_build(SSL *s, CBB *cbb);
 int tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t tlsext_type,
     int *alert);
diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 42fd18fe2d6..2e90d3e9df1 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.124 2017/08/10 17:18:38 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.125 2017/08/11 05:06:34 doug Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -342,7 +342,7 @@ tls1_ec_nid2curve_id(int nid)
  * the client/session formats. Otherwise return the custom format list if one
  * exists, or the default formats if a custom list has not been specified.
  */
-static void
+void
 tls1_get_formatlist(SSL *s, int client_formats, const uint8_t **pformats,
     size_t *pformatslen)
 {
@@ -699,34 +699,11 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 	ret += len;
 
 	if (using_ecc) {
-		size_t curveslen, formatslen, lenmax;
+		size_t curveslen, lenmax;
 		const uint16_t *curves;
-		const uint8_t *formats;
 		int i;
 
 		/*
-		 * Add TLS extension ECPointFormats to the ClientHello message.
-		 */
-		tls1_get_formatlist(s, 0, &formats, &formatslen);
-
-		if ((size_t)(limit - ret) < 5)
-			return NULL;
-
-		lenmax = limit - ret - 5;
-		if (formatslen > lenmax)
-			return NULL;
-		if (formatslen > 255) {
-			SSLerror(s, ERR_R_INTERNAL_ERROR);
-			return NULL;
-		}
-
-		s2n(TLSEXT_TYPE_ec_point_formats, ret);
-		s2n(formatslen + 1, ret);
-		*(ret++) = (unsigned char)formatslen;
-		memcpy(ret, formats, formatslen);
-		ret += formatslen;
-
-		/*
 		 * Add TLS extension EllipticCurves to the ClientHello message.
 		 */
 		tls1_get_curvelist(s, 0, &curves, &curveslen);
@@ -931,14 +908,12 @@ skip_ext:
 unsigned char *
 ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 {
-	int using_ecc, extdatalen = 0;
+	int extdatalen = 0;
 	unsigned char *ret = p;
 	int next_proto_neg_seen;
 	size_t len;
 	CBB cbb;
 
-	using_ecc = ssl_using_ecc_cipher(s);
-
 	ret += 2;
 	if (ret >= limit)
 		return NULL; /* this really never occurs, but ... */
@@ -957,33 +932,6 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 		return NULL;
 	ret += len;
 
-	if (using_ecc && s->version != DTLS1_VERSION) {
-		const unsigned char *formats;
-		size_t formatslen, lenmax;
-
-		/*
-		 * Add TLS extension ECPointFormats to the ServerHello message.
-		 */
-		tls1_get_formatlist(s, 0, &formats, &formatslen);
-
-		if ((size_t)(limit - ret) < 5)
-			return NULL;
-
-		lenmax = limit - ret - 5;
-		if (formatslen > lenmax)
-			return NULL;
-		if (formatslen > 255) {
-			SSLerror(s, ERR_R_INTERNAL_ERROR);
-			return NULL;
-		}
-
-		s2n(TLSEXT_TYPE_ec_point_formats, ret);
-		s2n(formatslen + 1, ret);
-		*(ret++) = (unsigned char)formatslen;
-		memcpy(ret, formats, formatslen);
-		ret += formatslen;
-	}
-
 	/*
 	 * Currently the server should not respond with a SupportedCurves
 	 * extension.
@@ -1194,38 +1142,7 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
 		if (!tlsext_clienthello_parse_one(s, &cbs, type, al))
 			return 0;
 
-		if (type == TLSEXT_TYPE_ec_point_formats &&
-		    s->version != DTLS1_VERSION) {
-			unsigned char *sdata = data;
-			size_t formatslen;
-			uint8_t *formats;
-
-			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->internal->hit) {
-				free(SSI(s)->tlsext_ecpointformatlist);
-				SSI(s)->tlsext_ecpointformatlist = NULL;
-				SSI(s)->tlsext_ecpointformatlist_length = 0;
-
-				if ((formats = reallocarray(NULL, formatslen,
-				    sizeof(uint8_t))) == NULL) {
-					*al = TLS1_AD_INTERNAL_ERROR;
-					return 0;
-				}
-				memcpy(formats, sdata, formatslen);
-				SSI(s)->tlsext_ecpointformatlist = formats;
-				SSI(s)->tlsext_ecpointformatlist_length =
-				    formatslen;
-			}
-		} else if (type == TLSEXT_TYPE_elliptic_curves &&
+		if (type == TLSEXT_TYPE_elliptic_curves &&
 		    s->version != DTLS1_VERSION) {
 			unsigned char *sdata = data;
 			size_t curveslen, i;
@@ -1510,39 +1427,7 @@ ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, size_t n, int *al)
 		if (!tlsext_serverhello_parse_one(s, &cbs, type, al))
 			return 0;
 
-		if (type == TLSEXT_TYPE_ec_point_formats &&
-		    s->version != DTLS1_VERSION) {
-			unsigned char *sdata = data;
-			size_t formatslen;
-			uint8_t *formats;
-
-			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->internal->hit) {
-				free(SSI(s)->tlsext_ecpointformatlist);
-				SSI(s)->tlsext_ecpointformatlist = NULL;
-				SSI(s)->tlsext_ecpointformatlist_length = 0;
-
-				if ((formats = reallocarray(NULL, formatslen,
-				    sizeof(uint8_t))) == NULL) {
-					*al = TLS1_AD_INTERNAL_ERROR;
-					return 0;
-				}
-				memcpy(formats, sdata, formatslen);
-				SSI(s)->tlsext_ecpointformatlist = formats;
-				SSI(s)->tlsext_ecpointformatlist_length =
-				    formatslen;
-			}
-		}
-		else if (type == TLSEXT_TYPE_session_ticket) {
+		if (type == TLSEXT_TYPE_session_ticket) {
 			if (s->internal->tls_session_ticket_ext_cb &&
 			    !s->internal->tls_session_ticket_ext_cb(s, data, size, s->internal->tls_session_ticket_ext_cb_arg)) {
 				*al = TLS1_AD_INTERNAL_ERROR;
@@ -1779,29 +1664,6 @@ ssl_check_serverhello_tlsext(SSL *s)
 	int ret = SSL_TLSEXT_ERR_NOACK;
 	int al = SSL_AD_UNRECOGNIZED_NAME;
 
-	/* If we are client and using an elliptic curve cryptography cipher
-	 * suite, then if server returns an EC point formats lists extension
-	 * it must contain uncompressed.
-	 */
-	if (ssl_using_ecc_cipher(s) &&
-	    s->internal->tlsext_ecpointformatlist != NULL &&
-	    s->internal->tlsext_ecpointformatlist_length > 0) {
-		/* we are using an ECC cipher */
-		size_t i;
-		unsigned char *list;
-		int found_uncompressed = 0;
-		list = SSI(s)->tlsext_ecpointformatlist;
-		for (i = 0; i < SSI(s)->tlsext_ecpointformatlist_length; i++) {
-			if (*(list++) == TLSEXT_ECPOINTFORMAT_uncompressed) {
-				found_uncompressed = 1;
-				break;
-			}
-		}
-		if (!found_uncompressed) {
-			SSLerror(s, SSL_R_TLS_INVALID_ECPOINTFORMAT_LIST);
-			return -1;
-		}
-	}
 	ret = SSL_TLSEXT_ERR_OK;
 
 	if (s->ctx != NULL && s->ctx->internal->tlsext_servername_callback != 0)