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@

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index f6e922e..d2a99af 100644 (file)
--- a/lib/libssl/ssl_locl.h
+++ b/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/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 5d741cd..6450623 100644 (file)
--- a/lib/libssl/ssl_srvr.c
+++ b/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/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index d0764af..0e3ef7d 100644 (file)
--- a/lib/libssl/ssl_tlsext.c
+++ b/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/lib/libssl/ssl_tlsext.h b/lib/libssl/ssl_tlsext.h
index 7c6250a..1af2e6c 100644 (file)
--- a/lib/libssl/ssl_tlsext.h
+++ b/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/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 1cef08d..fbd7943 100644 (file)
--- a/lib/libssl/t1_lib.c
+++ b/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.
  *
@@ -661,75 +661,6 @@ tls12_get_req_sig_algs(SSL *s, unsigned char **sigalgs, size_t *sigalgs_len)
        *sigalgs_len = sizeof(tls12_sigalgs);
 }
 
-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)
 {