Move the full extension building into tlsext_{client,server}hello_build(),

leaving ssl_add_{client,server}hello_tlsext() as pointer to CBB wrappers.

ok doug@

2 files changed, 26 insertions, 47 deletions

diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index 340ebeda5c..abc012d3af 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.14 2017/08/29 19:20:13 doug Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.15 2017/08/30 16:44:37 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -1300,11 +1300,12 @@ static struct tls_extension tls_extensions[] = {
 int
 tlsext_clienthello_build(SSL *s, CBB *cbb)
 {
+        CBB extensions, extension_data;
         struct tls_extension *tlsext;
-        CBB extension_data;
         size_t i;
 
-        memset(&extension_data, 0, sizeof(extension_data));
+        if (!CBB_add_u16_length_prefixed(cbb, &extensions))
+                return 0;
 
         for (i = 0; i < N_TLS_EXTENSIONS; i++) {
                 tlsext = &tls_extensions[i];
@@ -1312,16 +1313,17 @@ tlsext_clienthello_build(SSL *s, CBB *cbb)
                 if (!tlsext->clienthello_needs(s))
                         continue;
 
-                if (!CBB_add_u16(cbb, tlsext->type))
+                if (!CBB_add_u16(&extensions, tlsext->type))
                         return 0;
-                if (!CBB_add_u16_length_prefixed(cbb, &extension_data))
+                if (!CBB_add_u16_length_prefixed(&extensions, &extension_data))
                         return 0;
                 if (!tls_extensions[i].clienthello_build(s, &extension_data))
                         return 0;
-                if (!CBB_flush(cbb))
-                        return 0;
         }
 
+        if (!CBB_flush(cbb))
+                return 0;
+
         return 1;
 }
 
@@ -1353,11 +1355,12 @@ tlsext_clienthello_parse_one(SSL *s, CBS *cbs, uint16_t type, int *alert)
 int
 tlsext_serverhello_build(SSL *s, CBB *cbb)
 {
+        CBB extensions, extension_data;
         struct tls_extension *tlsext;
-        CBB extension_data;
         size_t i;
 
-        memset(&extension_data, 0, sizeof(extension_data));
+        if (!CBB_add_u16_length_prefixed(cbb, &extensions))
+                return 0;
 
         for (i = 0; i < N_TLS_EXTENSIONS; i++) {
                 tlsext = &tls_extensions[i];
@@ -1365,16 +1368,17 @@ tlsext_serverhello_build(SSL *s, CBB *cbb)
                 if (!tlsext->serverhello_needs(s))
                         continue;
 
-                if (!CBB_add_u16(cbb, tlsext->type))
+                if (!CBB_add_u16(&extensions, tlsext->type))
                         return 0;
-                if (!CBB_add_u16_length_prefixed(cbb, &extension_data))
+                if (!CBB_add_u16_length_prefixed(&extensions, &extension_data))
                         return 0;
                 if (!tlsext->serverhello_build(s, &extension_data))
                         return 0;
-                if (!CBB_flush(cbb))
-                        return 0;
         }
 
+        if (!CBB_flush(cbb))
+                return 0;
+
         return 1;
 }
 
diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index a9f10166fe..0d03b45a97 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.136 2017/08/27 02:58:04 doug Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.137 2017/08/30 16:44:37 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -664,16 +664,13 @@ tls12_get_req_sig_algs(SSL *s, unsigned char **sigalgs, size_t *sigalgs_len)
 unsigned char *
 ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 {
-        int extdatalen = 0;
-        unsigned char *ret = p;
         size_t len;
         CBB cbb;
 
-        ret += 2;
-        if (ret >= limit)
-                return NULL; /* this really never occurs, but ... */
+        if (p >= limit)
+                return NULL;
 
-        if (!CBB_init_fixed(&cbb, ret, limit - ret))
+        if (!CBB_init_fixed(&cbb, p, limit - p))
                 return NULL;
         if (!tlsext_clienthello_build(s, &cbb)) {
                 CBB_cleanup(&cbb);
@@ -683,30 +680,20 @@ ssl_add_clienthello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
                 CBB_cleanup(&cbb);
                 return NULL;
         }
-        if (len > (limit - ret))
-                return NULL;
-        ret += len;
-
-        if ((extdatalen = ret - p - 2) == 0)
-                return p;
 
-        s2n(extdatalen, p);
-        return ret;
+        return (p + len);
 }
 
 unsigned char *
 ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
 {
-        int extdatalen = 0;
-        unsigned char *ret = p;
         size_t len;
         CBB cbb;
 
-        ret += 2;
-        if (ret >= limit)
-                return NULL; /* this really never occurs, but ... */
+        if (p >= limit)
+                return NULL;
 
-        if (!CBB_init_fixed(&cbb, ret, limit - ret))
+        if (!CBB_init_fixed(&cbb, p, limit - p))
                 return NULL;
         if (!tlsext_serverhello_build(s, &cbb)) {
                 CBB_cleanup(&cbb);
@@ -716,20 +703,8 @@ ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned char *limit)
                 CBB_cleanup(&cbb);
                 return NULL;
         }
-        if (len > (limit - ret))
-                return NULL;
-        ret += len;
-
-        /*
-         * Currently the server should not respond with a SupportedCurves
-         * extension.
-         */
-
-        if ((extdatalen = ret - p - 2) == 0)
-                return p;
 
-        s2n(extdatalen, p);
-        return ret;
+        return (p + len);
 }
 
 int