Complete the TLS extension handling rewrite for the server-side.

This removes ssl_parse_clienthello_tlsext() and allows the CBS to be
passed all the way through from ssl3_get_client_hello(). The renegotation
check gets pulled up into ssl3_get_client_hello() which is where other
such checks exist.

The TLS extension parsing now also ensures that we do not get duplicates
of any known extensions (the old pre-rewrite code only did this for some
extensions).

ok inoguchi@

5 files changed, 86 insertions, 98 deletions

diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index f6e922e99c..d2a99afaa4 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.201 2017/10/12 16:06:32 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.202 2018/01/27 15:30:05 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -164,6 +164,9 @@
 
 __BEGIN_HIDDEN_DECLS
 
+#define CTASSERT(x)     extern char  _ctassert[(x) ? 1 : -1 ]   \
+                            __attribute__((__unused__))
+
 #define l2n(l,c)        (*((c)++)=(unsigned char)(((l)>>24)&0xff), \
                          *((c)++)=(unsigned char)(((l)>>16)&0xff), \
                          *((c)++)=(unsigned char)(((l)>> 8)&0xff), \
@@ -1275,8 +1278,6 @@ uint16_t tls1_ec_nid2curve_id(const int nid);
 int tls1_check_curve(SSL *s, const uint16_t curve_id);
 int tls1_get_shared_curve(SSL *s);
 
-int ssl_parse_clienthello_tlsext(SSL *s, unsigned char **data,
-    unsigned char *d, int n, int *al);
 int ssl_parse_serverhello_tlsext(SSL *s, unsigned char **data,
     size_t n, int *al);
 int ssl_check_clienthello_tlsext_early(SSL *s);
diff --git a/src/lib/libssl/ssl_srvr.c b/src/lib/libssl/ssl_srvr.c
index 5d741cdc81..6450623d4a 100644
--- a/src/lib/libssl/ssl_srvr.c
+++ b/src/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.26 2017/10/12 15:52:50 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.27 2018/01/27 15:30:05 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -813,7 +813,6 @@ ssl3_get_client_hello(SSL *s)
         int i, j, ok, al, ret = -1, cookie_valid = 0;
         long n;
         unsigned long id;
-        unsigned char *p, *d;
         SSL_CIPHER *c;
         STACK_OF(SSL_CIPHER) *ciphers = NULL;
         unsigned long alg_k;
@@ -843,8 +842,7 @@ ssl3_get_client_hello(SSL *s)
         if (n < 0)
                 goto err;
 
-        d = p = (unsigned char *)s->internal->init_msg;
-        end = d + n;
+        end = (unsigned char *)s->internal->init_msg + n;
 
         CBS_init(&cbs, s->internal->init_msg, n);
 
@@ -1038,14 +1036,17 @@ ssl3_get_client_hello(SSL *s)
                 goto f_err;
         }
 
-        p = (unsigned char *)CBS_data(&cbs);
-
-        /* TLS extensions*/
-        if (!ssl_parse_clienthello_tlsext(s, &p, d, n, &al)) {
-                /* 'al' set by ssl_parse_clienthello_tlsext */
+        if (!tlsext_clienthello_parse(s, &cbs, &al)) {
                 SSLerror(s, SSL_R_PARSE_TLSEXT);
                 goto f_err;
         }
+
+        if (!S3I(s)->renegotiate_seen && s->internal->renegotiate) {
+                al = SSL_AD_HANDSHAKE_FAILURE;
+                SSLerror(s, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+                goto f_err;
+        }
+
         if (ssl_check_clienthello_tlsext_early(s) <= 0) {
                 SSLerror(s, SSL_R_CLIENTHELLO_TLSEXT);
                 goto err;
diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index d0764af3c0..0e3ef7da15 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.19 2018/01/27 15:17:13 jsing Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.20 2018/01/27 15:30:05 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1292,6 +1292,35 @@ static struct tls_extension tls_extensions[] = {
 
 #define N_TLS_EXTENSIONS (sizeof(tls_extensions) / sizeof(*tls_extensions))
 
+/* Ensure that extensions fit in a uint32_t bitmask. */
+CTASSERT(N_TLS_EXTENSIONS <= (sizeof(uint32_t) * 8));
+
+static struct tls_extension *
+tls_extension_find(uint16_t type, size_t *tls_extensions_idx)
+{
+        size_t i;
+
+        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
+                if (tls_extensions[i].type == type) {
+                        *tls_extensions_idx = i;
+                        return &tls_extensions[i];
+                }
+        }
+
+        return NULL;
+}
+
+static void
+tlsext_clienthello_reset_state(SSL *s)
+{
+        s->internal->servername_done = 0;
+        s->tlsext_status_type = -1;
+        S3I(s)->renegotiate_seen = 0;
+        free(S3I(s)->alpn_selected);
+        S3I(s)->alpn_selected = NULL;
+        s->internal->srtp_profile = NULL;
+}
+
 int
 tlsext_clienthello_build(SSL *s, CBB *cbb)
 {
@@ -1329,28 +1358,55 @@ tlsext_clienthello_build(SSL *s, CBB *cbb)
 }
 
 int
-tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t type, int *alert)
+tlsext_clienthello_parse(SSL *s, CBS *cbs, int *alert)
 {
+        CBS extensions, extension_data;
         struct tls_extension *tlsext;
-        size_t i;
+        uint32_t extensions_seen = 0;
+        uint16_t type;
+        size_t idx;
 
-        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
-                tlsext = &tls_extensions[i];
+        /* XXX - this possibly should be done by the caller... */
+        tlsext_clienthello_reset_state(s);
 
-                if (tlsext->type != type)
+        /* An empty extensions block is valid. */
+        if (CBS_len(cbs) == 0)
+                return 1;
+
+        *alert = SSL_AD_DECODE_ERROR;
+
+        if (!CBS_get_u16_length_prefixed(cbs, &extensions))
+                return 0;
+
+        while (CBS_len(&extensions) > 0) {
+                if (!CBS_get_u16(&extensions, &type))
+                        return 0;
+                if (!CBS_get_u16_length_prefixed(&extensions, &extension_data))
+                        return 0;
+
+                if (s->internal->tlsext_debug_cb != NULL)
+                        s->internal->tlsext_debug_cb(s, 0, type,
+                            (unsigned char *)CBS_data(&extension_data),
+                            CBS_len(&extension_data),
+                            s->internal->tlsext_debug_arg);
+
+                /* Unknown extensions are ignored. */
+                if ((tlsext = tls_extension_find(type, &idx)) == NULL)
                         continue;
-                if (!tlsext->clienthello_parse(s, cbs, alert))
+
+                /* Check for duplicate extensions. */
+                if ((extensions_seen & (1 << idx)) != 0)
                         return 0;
-                if (CBS_len(cbs) != 0) {
-                        *alert = SSL_AD_DECODE_ERROR;
+                extensions_seen |= (1 << idx);
+
+                if (!tlsext->clienthello_parse(s, &extension_data, alert))
                         return 0;
-                }
 
-                return 1;
+                if (CBS_len(&extension_data) != 0)
+                        return 0;
         }
 
-        /* Not found. */
-        return 2;
+        return 1;
 }
 
 int
diff --git a/src/lib/libssl/ssl_tlsext.h b/src/lib/libssl/ssl_tlsext.h
index 7c6250a7f7..1af2e6cb3b 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.10 2017/08/27 02:58:04 doug Exp $ */
+/* $OpenBSD: ssl_tlsext.h,v 1.11 2018/01/27 15:30:05 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -82,8 +82,7 @@ int tlsext_srtp_serverhello_parse(SSL *s, CBS *cbs, int *alert);
 #endif
 
 int tlsext_clienthello_build(SSL *s, CBB *cbb);
-int tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t tlsext_type,
-    int *alert);
+int tlsext_clienthello_parse(SSL *s, CBS *cbs, int *alert);
 
 int tlsext_serverhello_build(SSL *s, CBB *cbb);
 int tlsext_serverhello_parse_one(SSL *s, CBS *cbs, uint16_t tlsext_type,
diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index 1cef08d094..fbd79431db 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.139 2017/10/11 17:35:00 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.140 2018/01/27 15:30:05 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -662,75 +662,6 @@ tls12_get_req_sig_algs(SSL *s, unsigned char **sigalgs, size_t *sigalgs_len)
 }
 
 int
-ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
-    int n, int *al)
-{
-        unsigned short type;
-        unsigned short size;
-        unsigned short len;
-        unsigned char *data = *p;
-        unsigned char *end = d + n;
-        CBS cbs;
-
-        s->internal->servername_done = 0;
-        s->tlsext_status_type = -1;
-        S3I(s)->renegotiate_seen = 0;
-        free(S3I(s)->alpn_selected);
-        S3I(s)->alpn_selected = NULL;
-        s->internal->srtp_profile = NULL;
-
-        if (data == end)
-                goto ri_check;
-
-        if (end - data < 2)
-                goto err;
-        n2s(data, len);
-
-        if (end - data != len)
-                goto err;
-
-        while (end - data >= 4) {
-                n2s(data, type);
-                n2s(data, size);
-
-                if (end - data < size)
-                        goto err;
-
-                if (s->internal->tlsext_debug_cb)
-                        s->internal->tlsext_debug_cb(s, 0, type, data, size,
-                            s->internal->tlsext_debug_arg);
-
-                CBS_init(&cbs, data, size);
-                if (!tlsext_clienthello_parse_one(s, &cbs, type, al))
-                        return 0;
-
-                data += size;
-        }
-
-        /* Spurious data on the end */
-        if (data != end)
-                goto err;
-
-        *p = data;
-
-ri_check:
-
-        /* Need RI if renegotiating */
-
-        if (!S3I(s)->renegotiate_seen && s->internal->renegotiate) {
-                *al = SSL_AD_HANDSHAKE_FAILURE;
-                SSLerror(s, SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
-                return 0;
-        }
-
-        return 1;
-
-err:
-        *al = SSL_AD_DECODE_ERROR;
-        return 0;
-}
-
-int
 ssl_parse_serverhello_tlsext(SSL *s, unsigned char **p, size_t n, int *al)
 {
         unsigned short type;