Deduplicate a bunch of replicated code in the extension handling

ok tb@

1 files changed, 50 insertions, 86 deletions

diff --git a/src/lib/libssl/ssl_tlsext.c b/src/lib/libssl/ssl_tlsext.c
index cd939decbf..c99ad671dc 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.37 2019/01/28 15:44:33 beck Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.38 2019/01/28 15:52:17 beck Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -25,6 +25,24 @@
 #include "ssl_sigalgs.h"
 #include "ssl_tlsext.h"
 
+
+static int
+tlsext_u16_prefixed_builder(CBB *parent, uint8_t *bytes, size_t len)
+{
+        CBB child;
+
+        if (!CBB_add_u16_length_prefixed(parent, &child))
+                return 0;
+
+        if (!CBB_add_bytes(&child, bytes, len))
+                return 0;
+
+        if (!CBB_flush(parent))
+                return 0;
+
+        return 1;
+}
+
 /*
  * Supported Application-Layer Protocol Negotiation - RFC 7301
  */
@@ -40,19 +58,10 @@ tlsext_alpn_client_needs(SSL *s)
 int
 tlsext_alpn_client_build(SSL *s, CBB *cbb)
 {
-        CBB protolist;
-
-        if (!CBB_add_u16_length_prefixed(cbb, &protolist))
-                return 0;
 
-        if (!CBB_add_bytes(&protolist, s->internal->alpn_client_proto_list,
-            s->internal->alpn_client_proto_list_len))
-                return 0;
-
-        if (!CBB_flush(cbb))
-                return 0;
-
-        return 1;
+        return (tlsext_u16_prefixed_builder(cbb,
+                s->internal->alpn_client_proto_list,
+                s->internal->alpn_client_proto_list_len));
 }
 
 int
@@ -1239,16 +1248,10 @@ tlsext_keyshare_client_needs(SSL *s)
 }
 
 int
-tlsext_keyshare_client_build(SSL *s, CBB *cbb)
+tlsext_keyshare_x25519_generate(SSL *s, CBB *keyshare)
 {
         uint8_t *public_key = NULL, *private_key = NULL;
-        CBB client_shares, key_exchange;
-
-        /* Generate and provide key shares. */
-        if (!CBB_add_u16_length_prefixed(cbb, &client_shares))
-                return 0;
-
-        /* XXX - other groups. */
+        CBB key_exchange;
 
         /* Generate X25519 key pair. */
         if ((public_key = malloc(X25519_KEY_LENGTH)) == NULL)
@@ -1258,16 +1261,13 @@ tlsext_keyshare_client_build(SSL *s, CBB *cbb)
         X25519_keypair(public_key, private_key);
 
         /* Add the group and serialize the public key. */
-        if (!CBB_add_u16(&client_shares, tls1_ec_nid2curve_id(NID_X25519)))
+        if (!CBB_add_u16(keyshare, tls1_ec_nid2curve_id(NID_X25519)))
                 goto err;
-        if (!CBB_add_u16_length_prefixed(&client_shares, &key_exchange))
+        if (!CBB_add_u16_length_prefixed(keyshare, &key_exchange))
                 goto err;
         if (!CBB_add_bytes(&key_exchange, public_key, X25519_KEY_LENGTH))
                 goto err;
 
-        if (!CBB_flush(cbb))
-                goto err;
-
         S3I(s)->hs_tls13.x25519_public = public_key;
         S3I(s)->hs_tls13.x25519_private = private_key;
 
@@ -1281,6 +1281,22 @@ tlsext_keyshare_client_build(SSL *s, CBB *cbb)
 }
 
 int
+tlsext_keyshare_client_build(SSL *s, CBB *cbb)
+{
+        CBB client_shares;
+
+        /* Generate and provide key shares. */
+        if (!CBB_add_u16_length_prefixed(cbb, &client_shares))
+                return 0;
+
+        if (!tlsext_keyshare_x25519_generate(s, &client_shares) ||
+            !CBB_flush(cbb))
+                return 0;
+
+        return (1);
+}
+
+int
 tlsext_keyshare_server_parse(SSL *s, CBS *cbs, int *alert)
 {
         CBS client_shares;
@@ -1342,43 +1358,15 @@ tlsext_keyshare_server_needs(SSL *s)
 int
 tlsext_keyshare_server_build(SSL *s, CBB *cbb)
 {
-        uint8_t *public_key = NULL, *private_key = NULL;
-        CBB key_exchange;
-
-        /* XXX deduplicate with client code */
-
         /* X25519 */
         if (S3I(s)->hs_tls13.x25519_peer_public == NULL)
                 return 0;
 
-        /* Generate X25519 key pair. */
-        if ((public_key = malloc(X25519_KEY_LENGTH)) == NULL)
-                goto err;
-        if ((private_key = malloc(X25519_KEY_LENGTH)) == NULL)
-                goto err;
-        X25519_keypair(public_key, private_key);
-
-        /* Add the group and serialize the public key. */
-        if (!CBB_add_u16(cbb, tls1_ec_nid2curve_id(NID_X25519)))
-                goto err;
-        if (!CBB_add_u16_length_prefixed(cbb, &key_exchange))
-                goto err;
-        if (!CBB_add_bytes(&key_exchange, public_key, X25519_KEY_LENGTH))
-                goto err;
-
-        if (!CBB_flush(cbb))
-                goto err;
-
-        S3I(s)->hs_tls13.x25519_public = public_key;
-        S3I(s)->hs_tls13.x25519_private = private_key;
+        if (!tlsext_keyshare_x25519_generate(s, cbb) ||
+            !CBB_flush(cbb))
+                return 0;
 
         return 1;
-
- err:
-        freezero(public_key, X25519_KEY_LENGTH);
-        freezero(private_key, X25519_KEY_LENGTH);
-
-        return 0;
 }
 
 int
@@ -1555,19 +1543,8 @@ tlsext_cookie_client_needs(SSL *s)
 int
 tlsext_cookie_client_build(SSL *s, CBB *cbb)
 {
-        CBB cookie;
-
-        if (!CBB_add_u16_length_prefixed(cbb, &cookie))
-                return 0;
-
-        if (!CBB_add_bytes(&cookie, S3I(s)->hs_tls13.cookie,
-            S3I(s)->hs_tls13.cookie_len))
-                return 0;
-
-        if (!CBB_flush(cbb))
-                return 0;
-
-        return 1;
+        return (tlsext_u16_prefixed_builder(cbb,
+            S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len));
 }
 
 int
@@ -1619,21 +1596,8 @@ tlsext_cookie_server_needs(SSL *s)
 int
 tlsext_cookie_server_build(SSL *s, CBB *cbb)
 {
-        CBB cookie;
-
-        /* XXX deduplicate with client code */
-
-        if (!CBB_add_u16_length_prefixed(cbb, &cookie))
-                return 0;
-
-        if (!CBB_add_bytes(&cookie, S3I(s)->hs_tls13.cookie,
-            S3I(s)->hs_tls13.cookie_len))
-                return 0;
-
-        if (!CBB_flush(cbb))
-                return 0;
-
-        return 1;
+        return (tlsext_u16_prefixed_builder(cbb,
+            S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len));
 }
 
 int
@@ -1891,7 +1855,7 @@ tlsext_funcs(struct tls_extension *tlsext, int is_server)
         if (is_server)
                 return &tlsext->server;
 
-        return &tlsext->client; 
+        return &tlsext->client;
 }
 
 static int