Rewrite EllipticCurves TLS extension handling using CBB/CBS and the new

extension framework.

input + ok jsing@

5 files changed, 472 insertions, 77 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 84bb6879b01..52e4b6c5e9b 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.186 2017/08/11 17:54:41 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.187 2017/08/11 20:14:13 doug Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1382,6 +1382,8 @@ 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);
+void tls1_get_curvelist(SSL *s, int client_curves, const uint16_t **pcurves,
+    size_t *pcurveslen);
 
 __END_HIDDEN_DECLS
 
diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index ad30f43389f..c050224c708 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.5 2017/08/11 06:30:41 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.6 2017/08/11 20:14:13 doug Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -21,6 +21,116 @@
 #include "bytestring.h"
 #include "ssl_tlsext.h"
 
+
+/*
+ * Supported Elliptic Curves - RFC 4492 section 5.1.1
+ */
+int
+tlsext_ec_clienthello_needs(SSL *s)
+{
+	return ssl_has_ecc_ciphers(s);
+}
+
+int
+tlsext_ec_clienthello_build(SSL *s, CBB *cbb)
+{
+	CBB curvelist;
+	size_t curves_len;
+	int i;
+	const uint16_t *curves;
+
+	tls1_get_curvelist(s, 0, &curves, &curves_len);
+
+	if (curves_len == 0) {
+		SSLerror(s, ERR_R_INTERNAL_ERROR);
+		return 0;
+	}
+
+	if (!CBB_add_u16_length_prefixed(cbb, &curvelist))
+		return 0;
+
+	for (i = 0; i < curves_len; i++) {
+		if (!CBB_add_u16(&curvelist, curves[i]))
+			return 0;
+	}
+
+	if (!CBB_flush(cbb))
+		return 0;
+
+	return 1;
+}
+
+int
+tlsext_ec_clienthello_parse(SSL *s, CBS *cbs, int *alert)
+{
+	CBS curvelist;
+	size_t curves_len;
+
+	if (!CBS_get_u16_length_prefixed(cbs, &curvelist))
+		goto err;
+	if (CBS_len(cbs) != 0)
+		goto err;
+
+	curves_len = CBS_len(&curvelist);
+	if (curves_len == 0 || curves_len % 2 != 0)
+		goto err;
+	curves_len /= 2;
+
+	if (!s->internal->hit) {
+		int i;
+		uint16_t *curves;
+
+		if (SSI(s)->tlsext_supportedgroups != NULL)
+			goto err;
+
+		if ((curves = reallocarray(NULL, curves_len,
+		    sizeof(uint16_t))) == NULL) {
+			*alert = TLS1_AD_INTERNAL_ERROR;
+			return 0;
+		}
+
+		for (i = 0; i < curves_len; i++) {
+			if (!CBS_get_u16(&curvelist, &curves[i])) {
+				free(curves);
+				goto err;
+			}
+		}
+
+		if (CBS_len(&curvelist) != 0) {
+			free(curves);
+			goto err;
+		}
+
+		SSI(s)->tlsext_supportedgroups = curves;
+		SSI(s)->tlsext_supportedgroups_length = curves_len;
+	}
+
+	return 1;
+
+ err:
+	*alert = TLS1_AD_DECODE_ERROR;
+	return 0;
+}
+
+/* This extension is never used by the server. */
+int
+tlsext_ec_serverhello_needs(SSL *s)
+{
+	return 0;
+}
+
+int
+tlsext_ec_serverhello_build(SSL *s, CBB *cbb)
+{
+	return 0;
+}
+
+int
+tlsext_ec_serverhello_parse(SSL *s, CBS *cbs, int *alert)
+{
+	return 0;
+}
+
 /*
  * Supported Point Formats Extension - RFC 4492 section 5.1.2
  */
@@ -420,6 +530,15 @@ static struct tls_extension tls_extensions[] = {
 		.serverhello_build = tlsext_ecpf_serverhello_build,
 		.serverhello_parse = tlsext_ecpf_serverhello_parse,
 	},
+	{
+		.type = TLSEXT_TYPE_elliptic_curves,
+		.clienthello_needs = tlsext_ec_clienthello_needs,
+		.clienthello_build = tlsext_ec_clienthello_build,
+		.clienthello_parse = tlsext_ec_clienthello_parse,
+		.serverhello_needs = tlsext_ec_serverhello_needs,
+		.serverhello_build = tlsext_ec_serverhello_build,
+		.serverhello_parse = tlsext_ec_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 6f79755f81a..38f8ffaa658 100644
--- a/lib/libssl/ssl_tlsext.h
+++ b/lib/libssl/ssl_tlsext.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.h,v 1.4 2017/08/11 06:30:41 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.h,v 1.5 2017/08/11 20:14:13 doug Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -30,6 +30,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_ec_clienthello_needs(SSL *s);
+int tlsext_ec_clienthello_build(SSL *s, CBB *cbb);
+int tlsext_ec_clienthello_parse(SSL *s, CBS *cbs, int *alert);
+int tlsext_ec_serverhello_needs(SSL *s);
+int tlsext_ec_serverhello_build(SSL *s, CBB *cbb);
+int tlsext_ec_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);
@@ -37,6 +44,7 @@ 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 2e90d3e9df1..4983ad27fa7 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.125 2017/08/11 05:06:34 doug Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.126 2017/08/11 20:14:13 doug Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -365,7 +365,7 @@ tls1_get_formatlist(SSL *s, int client_formats, const uint8_t **pformats,
  * the client/session curves. Otherwise return the custom curve list if one
  * exists, or the default curves if a custom list has not been specified.
  */
-static void
+void
 tls1_get_curvelist(SSL *s, int client_curves, const uint16_t **pcurves,
     size_t *pcurveslen)
 {
@@ -674,12 +674,9 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 {
 	int extdatalen = 0;
 	unsigned char *ret = p;
-	int using_ecc;
 	size_t len;
 	CBB cbb;
 
-	using_ecc = ssl_has_ecc_ciphers(s);
-
 	ret += 2;
 	if (ret >= limit)
 		return NULL; /* this really never occurs, but ... */
@@ -698,40 +695,6 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 		return NULL;
 	ret += len;
 
-	if (using_ecc) {
-		size_t curveslen, lenmax;
-		const uint16_t *curves;
-		int i;
-
-		/*
-		 * Add TLS extension EllipticCurves to the ClientHello message.
-		 */
-		tls1_get_curvelist(s, 0, &curves, &curveslen);
-
-		if ((size_t)(limit - ret) < 6)
-			return NULL;
-
-		lenmax = limit - ret - 6;
-		if (curveslen * 2 > lenmax)
-			return NULL;
-		if (curveslen * 2 > 65532) {
-			SSLerror(s, ERR_R_INTERNAL_ERROR);
-			return NULL;
-		}
-
-		s2n(TLSEXT_TYPE_elliptic_curves, 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.
-		 * https://www1.ietf.org/mail-archive/web/tls/current/msg00538.html
-		 * resolves this to two bytes.
-		 */
-		s2n(curveslen * 2, ret);
-		for (i = 0; i < curveslen; i++)
-			s2n(curves[i], ret);
-	}
-
 	if (!(SSL_get_options(s) & SSL_OP_NO_TICKET)) {
 		int ticklen;
 		if (!s->internal->new_session && s->session && s->session->tlsext_tick)
@@ -1142,40 +1105,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_elliptic_curves &&
-		    s->version != DTLS1_VERSION) {
-			unsigned char *sdata = data;
-			size_t curveslen, i;
-			uint16_t *curves;
-
-			if (size < 2) {
-				*al = TLS1_AD_DECODE_ERROR;
-				return 0;
-			}
-			n2s(sdata, curveslen);
-			if (curveslen != size - 2 || curveslen % 2 != 0) {
-				*al = TLS1_AD_DECODE_ERROR;
-				return 0;
-			}
-			curveslen /= 2;
-
-			if (!s->internal->hit) {
-				if (SSI(s)->tlsext_supportedgroups) {
-					*al = TLS1_AD_DECODE_ERROR;
-					return 0;
-				}
-				SSI(s)->tlsext_supportedgroups_length = 0;
-				if ((curves = reallocarray(NULL, curveslen,
-				    sizeof(uint16_t))) == NULL) {
-					*al = TLS1_AD_INTERNAL_ERROR;
-					return 0;
-				}
-				for (i = 0; i < curveslen; i++)
-					n2s(sdata, curves[i]);
-				SSI(s)->tlsext_supportedgroups = curves;
-				SSI(s)->tlsext_supportedgroups_length = curveslen;
-			}
-		} 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;
diff --git a/regress/lib/libssl/tlsext/tlsexttest.c b/regress/lib/libssl/tlsext/tlsexttest.c
index 5a7a34c56c0..4f9e6e29e23 100644
--- a/regress/lib/libssl/tlsext/tlsexttest.c
+++ b/regress/lib/libssl/tlsext/tlsexttest.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: tlsexttest.c,v 1.4 2017/08/11 05:06:34 doug Exp $ */
+/* $OpenBSD: tlsexttest.c,v 1.5 2017/08/11 20:14:13 doug Exp $ */
 /*
  * Copyright (c) 2017 Joel Sing <jsing@openbsd.org>
  *
@@ -34,6 +34,17 @@ hexdump(const unsigned char *buf, size_t len)
 }
 
 static void
+hexdump2(const uint16_t *buf, size_t len)
+{
+	size_t i;
+
+	for (i = 1; i <= len / 2; i++)
+		fprintf(stderr, " 0x%04hx,%s", buf[i - 1], i % 8 ? "" : "\n");
+
+	fprintf(stderr, "\n");
+}
+
+static void
 compare_data(const uint8_t *recv, size_t recv_len, const uint8_t *expect,
     size_t expect_len)
 {
@@ -44,6 +55,17 @@ compare_data(const uint8_t *recv, size_t recv_len, const uint8_t *expect,
 	hexdump(expect, expect_len);
 }
 
+static void
+compare_data2(const uint16_t *recv, size_t recv_len, const uint16_t *expect,
+    size_t expect_len)
+{
+	fprintf(stderr, "received:\n");
+	hexdump2(recv, recv_len);
+
+	fprintf(stderr, "test data:\n");
+	hexdump2(expect, expect_len);
+}
+
 #define FAIL(msg, ...)						\
 do {								\
 	fprintf(stderr, "[%s:%d] FAIL: ", __FILE__, __LINE__);	\
@@ -988,6 +1010,317 @@ test_tlsext_ecpf_serverhello(void)
 	return (failure);
 }
 
+/*
+ * ellliptic_curves - RFC 4492 section 5.1.1 (Supported Elliptic Curves).
+ *
+ * This extension is only used by the client.
+ */
+
+static uint8_t tlsext_ec_clienthello_default[] = {
+	0x00, 0x06,
+	0x00, 0x1d,  /* X25519 (29) */
+	0x00, 0x17,  /* secp256r1 (23) */
+	0x00, 0x18   /* secp384r1 (24) */
+};
+
+static uint16_t tlsext_ec_clienthello_secp384r1_val[] = {
+	0x0018   /* tls1_ec_nid2curve_id(NID_secp384r1) */
+};
+static uint8_t tlsext_ec_clienthello_secp384r1[] = {
+	0x00, 0x02,
+	0x00, 0x18  /* secp384r1 (24) */
+};
+
+/* Example from RFC 4492 section 5.1.1 */
+static uint16_t tlsext_ec_clienthello_nistp192and224_val[] = {
+	0x0013,  /* tls1_ec_nid2curve_id(NID_X9_62_prime192v1) */
+	0x0015   /* tls1_ec_nid2curve_id(NID_secp224r1) */
+};
+static uint8_t tlsext_ec_clienthello_nistp192and224[] = {
+	0x00, 0x04,
+	0x00, 0x13, /* secp192r1 aka NIST P-192 */
+	0x00, 0x15  /* secp224r1 aka NIST P-224 */
+};
+
+static int
+test_tlsext_ec_clienthello(void)
+{
+	unsigned char *data = NULL;
+	SSL_CTX *ssl_ctx = NULL;
+	SSL *ssl = NULL;
+	size_t dlen;
+	int failure, alert;
+	CBB cbb;
+	CBS cbs;
+
+	failure = 1;
+
+	CBB_init(&cbb, 0);
+
+	if ((ssl_ctx = SSL_CTX_new(TLS_client_method())) == NULL)
+		errx(1, "failed to create SSL_CTX");
+	if ((ssl = SSL_new(ssl_ctx)) == NULL)
+		errx(1, "failed to create SSL");
+
+	/*
+	 * Default ciphers include EC so we need it by default.
+	 */
+	if (!tlsext_ec_clienthello_needs(ssl)) {
+		FAIL("clienthello should need Ellipticcurves for default "
+		    "ciphers\n");
+		goto err;
+	}
+
+	/*
+	 * Exclude cipher suites so we can test not including it.
+	 */
+	if (!SSL_set_cipher_list(ssl, "TLSv1.2:!ECDHE:!ECDSA")) {
+		FAIL("clienthello should be able to set cipher list\n");
+		goto err;
+	}
+	if (tlsext_ec_clienthello_needs(ssl)) {
+		FAIL("clienthello should not need Ellipticcurves\n");
+		goto err;
+	}
+
+	/*
+	 * Use libtls default for the rest of the testing
+	 */
+	if (!SSL_set_cipher_list(ssl, "TLSv1.2+AEAD+ECDHE")) {
+		FAIL("clienthello should be able to set cipher list\n");
+		goto err;
+	}
+	if (!tlsext_ec_clienthello_needs(ssl)) {
+		FAIL("clienthello should need Ellipticcurves\n");
+		goto err;
+	}
+
+	/*
+	 * Test with a session secp384r1.  The default is used instead.
+	 */
+	if ((ssl->session = SSL_SESSION_new()) == NULL)
+		errx(1, "failed to create session");
+
+	if ((SSI(ssl)->tlsext_supportedgroups = malloc(sizeof(uint16_t)))
+	    == NULL) {
+		FAIL("client could not malloc\n");
+		goto err;
+	}
+	SSI(ssl)->tlsext_supportedgroups[0] = tls1_ec_nid2curve_id(NID_secp384r1);
+	SSI(ssl)->tlsext_supportedgroups_length = 1;
+
+	if (!tlsext_ec_clienthello_needs(ssl)) {
+		FAIL("clienthello should need Ellipticcurves\n");
+		goto err;
+	}
+
+	if (!tlsext_ec_clienthello_build(ssl, &cbb)) {
+		FAIL("clienthello failed to build Ellipticcurves\n");
+		goto err;
+	}
+
+	if (!CBB_finish(&cbb, &data, &dlen))
+		errx(1, "failed to finish CBB");
+
+	if (dlen != sizeof(tlsext_ec_clienthello_default)) {
+		FAIL("got clienthello Ellipticcurves with length %zu, "
+		    "want length %zu\n", dlen,
+		    sizeof(tlsext_ec_clienthello_default));
+		compare_data(data, dlen, tlsext_ec_clienthello_default,
+		    sizeof(tlsext_ec_clienthello_default));
+		goto err;
+	}
+
+	if (memcmp(data, tlsext_ec_clienthello_default, dlen) != 0) {
+		FAIL("clienthello Ellipticcurves differs:\n");
+		compare_data(data, dlen, tlsext_ec_clienthello_default,
+		    sizeof(tlsext_ec_clienthello_default));
+		goto err;
+	}
+
+	/*
+	 * Test parsing secp384r1
+	 */
+	CBB_cleanup(&cbb);
+	CBB_init(&cbb, 0);
+	free(data);
+	data = NULL;
+
+	SSL_SESSION_free(ssl->session);
+	if ((ssl->session = SSL_SESSION_new()) == NULL)
+		errx(1, "failed to create session");
+
+	CBS_init(&cbs, tlsext_ec_clienthello_secp384r1,
+	    sizeof(tlsext_ec_clienthello_secp384r1));
+	if (!tlsext_ec_clienthello_parse(ssl, &cbs, &alert)) {
+		FAIL("failed to parse clienthello Ellipticcurves\n");
+		goto err;
+	}
+
+	if (SSI(ssl)->tlsext_supportedgroups_length !=
+	    sizeof(tlsext_ec_clienthello_secp384r1_val) / sizeof(uint16_t)) {
+		FAIL("no tlsext_ellipticcurves from clienthello "
+		    "Ellipticcurves\n");
+		goto err;
+	}
+
+	if (memcmp(SSI(ssl)->tlsext_supportedgroups,
+	    tlsext_ec_clienthello_secp384r1_val,
+	    sizeof(tlsext_ec_clienthello_secp384r1_val)) != 0) {
+		FAIL("clienthello had an incorrect Ellipticcurves "
+		    "entry\n");
+		compare_data2(SSI(ssl)->tlsext_supportedgroups,
+		    SSI(ssl)->tlsext_supportedgroups_length * 2,
+		    tlsext_ec_clienthello_secp384r1_val,
+		    sizeof(tlsext_ec_clienthello_secp384r1_val));
+		goto err;
+	}
+
+	/*
+	 * Use a custom order.
+	 */
+	CBB_cleanup(&cbb);
+	CBB_init(&cbb, 0);
+
+	SSL_SESSION_free(ssl->session);
+	if ((ssl->session = SSL_SESSION_new()) == NULL)
+		errx(1, "failed to create session");
+
+	if ((ssl->internal->tlsext_supportedgroups = malloc(sizeof(uint16_t) * 2)) == NULL) {
+		FAIL("client could not malloc\n");
+		goto err;
+	}
+	ssl->internal->tlsext_supportedgroups[0] = tls1_ec_nid2curve_id(NID_X9_62_prime192v1);
+	ssl->internal->tlsext_supportedgroups[1] = tls1_ec_nid2curve_id(NID_secp224r1);
+	ssl->internal->tlsext_supportedgroups_length = 2;
+
+	if (!tlsext_ec_clienthello_needs(ssl)) {
+		FAIL("clienthello should need Ellipticcurves\n");
+		goto err;
+	}
+
+	if (!tlsext_ec_clienthello_build(ssl, &cbb)) {
+		FAIL("clienthello failed to build Ellipticcurves\n");
+		goto err;
+	}
+
+	if (!CBB_finish(&cbb, &data, &dlen))
+		errx(1, "failed to finish CBB");
+
+	if (dlen != sizeof(tlsext_ec_clienthello_nistp192and224)) {
+		FAIL("got clienthello Ellipticcurves with length %zu, "
+		    "want length %zu\n", dlen,
+		    sizeof(tlsext_ec_clienthello_nistp192and224));
+		fprintf(stderr, "received:\n");
+		hexdump(data, dlen);
+		fprintf(stderr, "test data:\n");
+		hexdump(tlsext_ec_clienthello_nistp192and224,
+		    sizeof(tlsext_ec_clienthello_nistp192and224));
+		goto err;
+	}
+
+	if (memcmp(data, tlsext_ec_clienthello_nistp192and224, dlen) != 0) {
+		FAIL("clienthello Ellipticcurves differs:\n");
+		fprintf(stderr, "received:\n");
+		hexdump(data, dlen);
+		fprintf(stderr, "test data:\n");
+		hexdump(tlsext_ec_clienthello_nistp192and224,
+		    sizeof(tlsext_ec_clienthello_nistp192and224));
+		goto err;
+	}
+
+	/*
+	 * Parse non-default curves to session.
+	 */
+	CBB_cleanup(&cbb);
+	CBB_init(&cbb, 0);
+	free(data);
+	data = NULL;
+
+	SSL_SESSION_free(ssl->session);
+	if ((ssl->session = SSL_SESSION_new()) == NULL)
+		errx(1, "failed to create session");
+
+	/* Reset back to the default list. */
+	free(ssl->internal->tlsext_supportedgroups);
+	ssl->internal->tlsext_supportedgroups = NULL;
+	ssl->internal->tlsext_supportedgroups_length = 0;
+
+	CBS_init(&cbs, tlsext_ec_clienthello_nistp192and224,
+	    sizeof(tlsext_ec_clienthello_nistp192and224));
+	if (!tlsext_ec_clienthello_parse(ssl, &cbs, &alert)) {
+		FAIL("failed to parse clienthello Ellipticcurves\n");
+		goto err;
+	}
+
+	if (SSI(ssl)->tlsext_supportedgroups_length !=
+	    sizeof(tlsext_ec_clienthello_nistp192and224_val) / sizeof(uint16_t)) {
+		FAIL("no tlsext_ellipticcurves from clienthello "
+		    "Ellipticcurves\n");
+		goto err;
+	}
+
+	if (memcmp(SSI(ssl)->tlsext_supportedgroups,
+	    tlsext_ec_clienthello_nistp192and224_val,
+	    sizeof(tlsext_ec_clienthello_nistp192and224_val)) != 0) {
+		FAIL("clienthello had an incorrect Ellipticcurves entry\n");
+		compare_data2(SSI(ssl)->tlsext_supportedgroups,
+		    SSI(ssl)->tlsext_supportedgroups_length * 2,
+		    tlsext_ec_clienthello_nistp192and224_val,
+		    sizeof(tlsext_ec_clienthello_nistp192and224_val));
+		goto err;
+	}
+
+	failure = 0;
+
+ err:
+	CBB_cleanup(&cbb);
+	SSL_CTX_free(ssl_ctx);
+	SSL_free(ssl);
+	free(data);
+
+	return (failure);
+}
+
+
+/* elliptic_curves is only used by the client so this doesn't test much. */
+static int
+test_tlsext_ec_serverhello(void)
+{
+	SSL_CTX *ssl_ctx = NULL;
+	SSL *ssl = NULL;
+	int failure;
+
+	failure = 1;
+
+	if ((ssl_ctx = SSL_CTX_new(TLS_server_method())) == NULL)
+		errx(1, "failed to create SSL_CTX");
+	if ((ssl = SSL_new(ssl_ctx)) == NULL)
+		errx(1, "failed to create SSL");
+
+	if (tlsext_ec_serverhello_needs(ssl)) {
+		FAIL("serverhello should not need elliptic_curves\n");
+		goto err;
+	}
+
+	if ((ssl->session = SSL_SESSION_new()) == NULL)
+		errx(1, "failed to create session");
+
+	if (tlsext_ec_serverhello_needs(ssl)) {
+		FAIL("serverhello should not need elliptic_curves\n");
+		goto err;
+	}
+
+	failure = 0;
+
+ err:
+	SSL_CTX_free(ssl_ctx);
+	SSL_free(ssl);
+
+	return (failure);
+
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1004,5 +1337,8 @@ main(int argc, char **argv)
 	failed |= test_tlsext_ecpf_clienthello();
 	failed |= test_tlsext_ecpf_serverhello();
 
+	failed |= test_tlsext_ec_clienthello();
+	failed |= test_tlsext_ec_serverhello();
+
 	return (failed);
 }