Start rewriting TLS extension handling.

Introduce a TLS extension handling framework that has per-extension type
functions to determine if an extension is needed, to build the extension
data and parse the extension data. This is somewhat analogous to BoringSSL,
however these build and parse functions are intentionally symetrical. The
framework is hooked into the existing TLS handling code in such a way that
we can gradual convert the extension handling code.

Convert the TLS Server Name Indication extension to the new framework,
while rewriting it to use CBB/CBS and be more strict in the process.

Discussed with beck@

ok inoguchi@

4 files changed, 328 insertions, 141 deletions

diff --git a/src/lib/libssl/Makefile b/src/lib/libssl/Makefile
index 809851d251..f004507060 100644
--- a/src/lib/libssl/Makefile
+++ b/src/lib/libssl/Makefile
@@ -1,4 +1,4 @@
-# $OpenBSD: Makefile,v 1.35 2017/04/30 04:44:58 jsing Exp $
+# $OpenBSD: Makefile,v 1.36 2017/07/16 18:14:37 jsing Exp $
 
 .include <bsd.own.mk>
 .ifndef NOMAN
@@ -33,7 +33,7 @@ SRCS= \
         ssl_ciph.c ssl_stat.c ssl_rsa.c \
         ssl_asn1.c ssl_txt.c ssl_algs.c \
         bio_ssl.c ssl_err.c t1_reneg.c \
-        ssl_packet.c ssl_versions.c pqueue.c
+        ssl_packet.c ssl_tlsext.c ssl_versions.c pqueue.c
 SRCS+=  s3_cbc.c
 SRCS+=  bs_ber.c bs_cbb.c bs_cbs.c
 
diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
new file mode 100644
index 0000000000..18ac98103a
--- /dev/null
+++ b/src/lib/libssl/ssl_tlsext.c
@@ -0,0 +1,261 @@
+/* $OpenBSD: ssl_tlsext.c,v 1.1 2017/07/16 18:14:37 jsing Exp $ */
+/*
+ * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include "ssl_locl.h"
+
+#include "bytestring.h"
+#include "ssl_tlsext.h"
+
+/*
+ * Server Name Indication - RFC 6066, section 3.
+ */
+int
+tlsext_sni_clienthello_needs(SSL *s)
+{
+        return (s->tlsext_hostname != NULL);
+}
+
+int
+tlsext_sni_clienthello_build(SSL *s, CBB *cbb)
+{
+        CBB server_name_list, host_name;
+
+        if (!CBB_add_u16_length_prefixed(cbb, &server_name_list))
+                return 0;
+        if (!CBB_add_u8(&server_name_list, TLSEXT_NAMETYPE_host_name))
+                return 0;
+        if (!CBB_add_u16_length_prefixed(&server_name_list, &host_name))
+                return 0;
+        if (!CBB_add_bytes(&host_name, (const uint8_t *)s->tlsext_hostname,
+            strlen(s->tlsext_hostname)))
+                return 0;
+        if (!CBB_flush(cbb))
+                return 0;
+
+        return 1;
+}
+
+int
+tlsext_sni_clienthello_parse(SSL *s, CBS *cbs, int *alert)
+{
+        CBS server_name_list, host_name;
+        uint8_t name_type;
+
+        if (!CBS_get_u16_length_prefixed(cbs, &server_name_list))
+                goto err;
+
+        /*
+         * RFC 6066 section 3 forbids multiple host names with the same type.
+         * Additionally, only one type (host_name) is specified.
+         */
+        if (!CBS_get_u8(&server_name_list, &name_type))
+                goto err;
+        if (name_type != TLSEXT_NAMETYPE_host_name)
+                goto err;
+
+        if (!CBS_get_u16_length_prefixed(&server_name_list, &host_name))
+                goto err;
+        if (CBS_len(&host_name) == 0 ||
+            CBS_len(&host_name) > TLSEXT_MAXLEN_host_name ||
+            CBS_contains_zero_byte(&host_name)) {
+                *alert = TLS1_AD_UNRECOGNIZED_NAME;
+                return 0;
+        }
+
+        if (s->internal->hit) {
+                if (s->session->tlsext_hostname == NULL) {
+                        *alert = TLS1_AD_UNRECOGNIZED_NAME;
+                        return 0;
+                }
+                if (!CBS_mem_equal(&host_name, s->session->tlsext_hostname,
+                    strlen(s->session->tlsext_hostname))) {
+                        *alert = TLS1_AD_UNRECOGNIZED_NAME;
+                        return 0;
+                }
+        } else {
+                if (s->session->tlsext_hostname != NULL)
+                        goto err;
+                if (!CBS_strdup(&host_name, &s->session->tlsext_hostname)) {
+                        *alert = TLS1_AD_INTERNAL_ERROR;
+                        return 0;
+                }
+        }
+
+        if (CBS_len(&server_name_list) != 0)
+                goto err;
+        if (CBS_len(cbs) != 0)
+                goto err;
+
+        return 1;
+
+ err:
+        *alert = SSL_AD_DECODE_ERROR;
+        return 0;
+}
+
+int
+tlsext_sni_serverhello_needs(SSL *s)
+{
+        return (s->session->tlsext_hostname != NULL);
+}
+
+int
+tlsext_sni_serverhello_build(SSL *s, CBB *cbb)
+{
+        return 1;
+}
+
+int
+tlsext_sni_serverhello_parse(SSL *s, CBS *cbs, int *alert)
+{
+        if (s->tlsext_hostname == NULL || CBS_len(cbs) != 0) {
+                *alert = TLS1_AD_UNRECOGNIZED_NAME;
+                return 0;
+        }
+
+        return 1;
+}
+
+struct tls_extension {
+        uint16_t type;
+        int (*clienthello_needs)(SSL *s);
+        int (*clienthello_build)(SSL *s, CBB *cbb);
+        int (*clienthello_parse)(SSL *s, CBS *cbs, int *alert);
+        int (*serverhello_needs)(SSL *s);
+        int (*serverhello_build)(SSL *s, CBB *cbb);
+        int (*serverhello_parse)(SSL *s, CBS *cbs, int *alert);
+};
+
+static struct tls_extension tls_extensions[] = {
+        {
+                .type = TLSEXT_TYPE_server_name,
+                .clienthello_needs = tlsext_sni_clienthello_needs,
+                .clienthello_build = tlsext_sni_clienthello_build,
+                .clienthello_parse = tlsext_sni_clienthello_parse,
+                .serverhello_needs = tlsext_sni_serverhello_needs,
+                .serverhello_build = tlsext_sni_serverhello_build,
+                .serverhello_parse = tlsext_sni_serverhello_parse,
+        },
+};
+
+#define N_TLS_EXTENSIONS (sizeof(tls_extensions) / sizeof(*tls_extensions))
+
+int
+tlsext_clienthello_build(SSL *s, CBB *cbb)
+{
+        struct tls_extension *tlsext;
+        CBB extension_data;
+        size_t i;
+
+        memset(&extension_data, 0, sizeof(extension_data));
+
+        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
+                tlsext = &tls_extensions[i];
+
+                if (!tlsext->clienthello_needs(s))
+                        continue;
+                
+                if (!CBB_add_u16(cbb, tlsext->type))
+                        return 0;
+                if (!CBB_add_u16_length_prefixed(cbb, &extension_data))
+                        return 0;
+                if (!tls_extensions[i].clienthello_build(s, &extension_data))
+                        return 0;
+                if (!CBB_flush(cbb))
+                        return 0;
+        }
+
+        return 1;
+}
+
+int
+tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t type, int *alert)
+{
+        struct tls_extension *tlsext;
+        size_t i;
+
+        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
+                tlsext = &tls_extensions[i];
+
+                if (tlsext->type != type)
+                        continue;
+                if (!tlsext->clienthello_parse(s, cbs, alert))
+                        return 0;
+                if (CBS_len(cbs) != 0) {
+                        *alert = SSL_AD_DECODE_ERROR;
+                        return 0;
+                }
+
+                return 1;
+        }
+
+        /* Not found. */
+        return 2;
+}
+
+int
+tlsext_serverhello_build(SSL *s, CBB *cbb)
+{
+        struct tls_extension *tlsext;
+        CBB extension_data;
+        size_t i;
+
+        memset(&extension_data, 0, sizeof(extension_data));
+
+        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
+                tlsext = &tls_extensions[i];
+
+                if (!tlsext->serverhello_needs(s))
+                        continue;
+                
+                if (!CBB_add_u16(cbb, tlsext->type))
+                        return 0;
+                if (!CBB_add_u16_length_prefixed(cbb, &extension_data))
+                        return 0;
+                if (!tlsext->serverhello_build(s, &extension_data))
+                        return 0;
+                if (!CBB_flush(cbb))
+                        return 0;
+        }
+
+        return 1;
+}
+
+int
+tlsext_serverhello_parse_one(SSL *s, CBS *cbs, uint16_t type, int *alert)
+{
+        struct tls_extension *tlsext;
+        size_t i;
+
+        for (i = 0; i < N_TLS_EXTENSIONS; i++) {
+                tlsext = &tls_extensions[i];
+
+                if (tlsext->type != type)
+                        continue;
+                if (!tlsext->serverhello_parse(s, cbs, alert))
+                        return 0;
+                if (CBS_len(cbs) != 0) {
+                        *alert = SSL_AD_DECODE_ERROR;
+                        return 0;
+                }
+
+                return 1;
+        }
+
+        /* Not found. */
+        return 2;
+}
diff --git a/src/lib/libssl/ssl_tlsext.h b/src/lib/libssl/ssl_tlsext.h
new file mode 100644
index 0000000000..a8e478d9c0
--- /dev/null
+++ b/src/lib/libssl/ssl_tlsext.h
@@ -0,0 +1,31 @@
+/* $OpenBSD: ssl_tlsext.h,v 1.1 2017/07/16 18:14:37 jsing Exp $ */
+/*
+ * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+int tlsext_sni_clienthello_needs(SSL *s);
+int tlsext_sni_clienthello_build(SSL *s, CBB *cbb);
+int tlsext_sni_clienthello_parse(SSL *s, CBS *cbs, int *alert);
+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_clienthello_build(SSL *s, CBB *cbb);
+int tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t tlsext_type,
+    int *alert);
+
+int tlsext_serverhello_build(SSL *s, CBB *cbb);
+int tlsext_serverhello_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 eb2314ac26..e3046fec09 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.117 2017/05/07 04:22:24 beck Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.118 2017/07/16 18:14:37 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -117,7 +117,9 @@
 #include <openssl/ocsp.h>
 
 #include "ssl_locl.h"
+
 #include "bytestring.h"
+#include "ssl_tlsext.h"
 
 static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen,
     const unsigned char *sess_id, int sesslen,
@@ -678,6 +680,8 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
         int extdatalen = 0;
         unsigned char *ret = p;
         int using_ecc = 0;
+        size_t len;
+        CBB cbb;
 
         /* See if we support any ECC ciphersuites. */
         if (s->version != DTLS1_VERSION && s->version >= TLS1_VERSION) {
@@ -699,43 +703,21 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
         }
 
         ret += 2;
-
         if (ret >= limit)
                 return NULL; /* this really never occurs, but ... */
 
-        if (s->tlsext_hostname != NULL) {
-                /* Add TLS extension servername to the Client Hello message */
-                size_t size_str, lenmax;
-
-                /* check for enough space.
-                   4 for the servername type and extension length
-                   2 for servernamelist length
-                   1 for the hostname type
-                   2 for hostname length
-                   + hostname length
-                */
-
-                if ((size_t)(limit - ret) < 9)
-                        return NULL;
-
-                lenmax = limit - ret - 9;
-                if ((size_str = strlen(s->tlsext_hostname)) > lenmax)
-                        return NULL;
-
-                /* extension type and length */
-                s2n(TLSEXT_TYPE_server_name, ret);
-
-                s2n(size_str + 5, ret);
-
-                /* length of servername list */
-                s2n(size_str + 3, ret);
-
-                /* hostname type, length and hostname */
-                *(ret++) = (unsigned char) TLSEXT_NAMETYPE_host_name;
-                s2n(size_str, ret);
-                memcpy(ret, s->tlsext_hostname, size_str);
-                ret += size_str;
+        CBB_init_fixed(&cbb, ret, limit - ret);
+        if (!tlsext_clienthello_build(s, &cbb)) {
+                CBB_cleanup(&cbb);
+                return NULL;
+        }
+        if (!CBB_finish(&cbb, NULL, &len)) {
+                CBB_cleanup(&cbb);
+                return NULL;
         }
+        if (len > (limit - ret))
+                return NULL;
+        ret += len;
 
         /* Add RI if renegotiating */
         if (s->internal->renegotiate) {
@@ -997,6 +979,8 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
         unsigned long alg_a, alg_k;
         unsigned char *ret = p;
         int next_proto_neg_seen;
+        size_t len;
+        CBB cbb;
 
         alg_a = S3I(s)->hs.new_cipher->algorithm_auth;
         alg_k = S3I(s)->hs.new_cipher->algorithm_mkey;
@@ -1007,14 +991,18 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
         if (ret >= limit)
                 return NULL; /* this really never occurs, but ... */
 
-        if (!s->internal->hit && s->internal->servername_done == 1 &&
-            s->session->tlsext_hostname != NULL) {
-                if ((size_t)(limit - ret) < 4)
-                        return NULL;
-
-                s2n(TLSEXT_TYPE_server_name, ret);
-                s2n(0, ret);
+        CBB_init_fixed(&cbb, ret, limit - ret);
+        if (!tlsext_serverhello_build(s, &cbb)) {
+                CBB_cleanup(&cbb);
+                return NULL;
         }
+        if (!CBB_finish(&cbb, NULL, &len)) {
+                CBB_cleanup(&cbb);
+                return NULL;
+        }
+        if (len > (limit - ret))
+                return NULL;
+        ret += len;
 
         if (S3I(s)->send_connection_binding) {
                 int el;
@@ -1241,6 +1229,7 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
         unsigned char *end = d + n;
         int renegotiate_seen = 0;
         int sigalg_seen = 0;
+        CBS cbs;
 
         s->internal->servername_done = 0;
         s->tlsext_status_type = -1;
@@ -1269,106 +1258,12 @@ ssl_parse_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d,
                 if (s->internal->tlsext_debug_cb)
                         s->internal->tlsext_debug_cb(s, 0, type, data, size,
                             s->internal->tlsext_debug_arg);
-/* The servername extension is treated as follows:
-
-   - Only the hostname type is supported with a maximum length of 255.
-   - The servername is rejected if too long or if it contains zeros,
-     in which case an fatal alert is generated.
-   - The servername field is maintained together with the session cache.
-   - When a session is resumed, the servername call back invoked in order
-     to allow the application to position itself to the right context.
-   - The servername is acknowledged if it is new for a session or when
-     it is identical to a previously used for the same session.
-     Applications can control the behaviour.  They can at any time
-     set a 'desirable' servername for a new SSL object. This can be the
-     case for example with HTTPS when a Host: header field is received and
-     a renegotiation is requested. In this case, a possible servername
-     presented in the new client hello is only acknowledged if it matches
-     the value of the Host: field.
-   - Applications must  use SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
-     if they provide for changing an explicit servername context for the session,
-     i.e. when the session has been established with a servername extension.
-   - On session reconnect, the servername extension may be absent.
-
-*/
-
-                if (type == TLSEXT_TYPE_server_name) {
-                        unsigned char *sdata;
-                        int servname_type;
-                        int dsize;
 
-                        if (size < 2) {
-                                *al = SSL_AD_DECODE_ERROR;
-                                return 0;
-                        }
-                        n2s(data, dsize);
-
-                        size -= 2;
-                        if (dsize > size) {
-                                *al = SSL_AD_DECODE_ERROR;
-                                return 0;
-                        }
-
-                        sdata = data;
-                        while (dsize > 3) {
-                                servname_type = *(sdata++);
-
-                                n2s(sdata, len);
-                                dsize -= 3;
-
-                                if (len > dsize) {
-                                        *al = SSL_AD_DECODE_ERROR;
-                                        return 0;
-                                }
-                                if (s->internal->servername_done == 0)
-                                        switch (servname_type) {
-                                        case TLSEXT_NAMETYPE_host_name:
-                                                if (!s->internal->hit) {
-                                                        if (s->session->tlsext_hostname) {
-                                                                *al = SSL_AD_DECODE_ERROR;
-                                                                return 0;
-                                                        }
-                                                        if (len > TLSEXT_MAXLEN_host_name) {
-                                                                *al = TLS1_AD_UNRECOGNIZED_NAME;
-                                                                return 0;
-                                                        }
-                                                        if ((s->session->tlsext_hostname =
-                                                            malloc(len + 1)) == NULL) {
-                                                                *al = TLS1_AD_INTERNAL_ERROR;
-                                                                return 0;
-                                                        }
-                                                        memcpy(s->session->tlsext_hostname, sdata, len);
-                                                        s->session->tlsext_hostname[len] = '\0';
-                                                        if (strlen(s->session->tlsext_hostname) != len) {
-                                                                free(s->session->tlsext_hostname);
-                                                                s->session->tlsext_hostname = NULL;
-                                                                *al = TLS1_AD_UNRECOGNIZED_NAME;
-                                                                return 0;
-                                                        }
-                                                        s->internal->servername_done = 1;
-
-
-                                                } else {
-                                                        s->internal->servername_done = s->session->tlsext_hostname &&
-                                                            strlen(s->session->tlsext_hostname) == len &&
-                                                            strncmp(s->session->tlsext_hostname, (char *)sdata, len) == 0;
-                                                }
-                                                break;
-
-                                        default:
-                                                break;
-                                        }
-
-                                dsize -= len;
-                        }
-                        if (dsize != 0) {
-                                *al = SSL_AD_DECODE_ERROR;
-                                return 0;
-                        }
-
-                }
+                CBS_init(&cbs, data, size);
+                if (!tlsext_clienthello_parse_one(s, &cbs, type, al))
+                        return 0;
 
-                else if (type == TLSEXT_TYPE_ec_point_formats &&
+                if (type == TLSEXT_TYPE_ec_point_formats &&
                     s->version != DTLS1_VERSION) {
                         unsigned char *sdata = data;
                         size_t formatslen;