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

extension framework.

input + ok jsing@

4 files changed, 135 insertions, 76 deletions

diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 84bb6879b0..52e4b6c5e9 100644
--- a/src/lib/libssl/ssl_locl.h
+++ b/src/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/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index ad30f43389..c050224c70 100644
--- a/src/lib/libssl/ssl_tlsext.c
+++ b/src/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/src/lib/libssl/ssl_tlsext.h b/src/lib/libssl/ssl_tlsext.h
index 6f79755f81..38f8ffaa65 100644
--- a/src/lib/libssl/ssl_tlsext.h
+++ b/src/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/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index 2e90d3e9df..4983ad27fa 100644
--- a/src/lib/libssl/t1_lib.c
+++ b/src/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;