Revert r1.38 as it introduces use of a stack value post function return.

The deduplication is also not quite right - this will be revisited in due
course.

1 files changed, 86 insertions, 50 deletions

diff --git a/lib/libssl/ssl_tlsext.c b/lib/libssl/ssl_tlsext.c
index c74772f683e..3502e5a7214 100644
--- a/lib/libssl/ssl_tlsext.c
+++ b/lib/libssl/ssl_tlsext.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_tlsext.c,v 1.40 2019/01/31 08:11:55 tb Exp $ */
+/* $OpenBSD: ssl_tlsext.c,v 1.41 2019/02/03 14:09:58 jsing Exp $ */
 /*
  * Copyright (c) 2016, 2017, 2019 Joel Sing <jsing@openbsd.org>
  * Copyright (c) 2017 Doug Hogan <doug@openbsd.org>
@@ -25,24 +25,6 @@
 #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
  */
@@ -58,10 +40,19 @@ 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;
 
-	return (tlsext_u16_prefixed_builder(cbb,
-		s->internal->alpn_client_proto_list,
-		s->internal->alpn_client_proto_list_len));
+	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;
 }
 
 int
@@ -1233,10 +1224,16 @@ tlsext_keyshare_client_needs(SSL *s)
 }
 
 int
-tlsext_keyshare_x25519_generate(SSL *s, CBB *keyshare)
+tlsext_keyshare_client_build(SSL *s, CBB *cbb)
 {
 	uint8_t *public_key = NULL, *private_key = NULL;
-	CBB key_exchange;
+	CBB client_shares, key_exchange;
+
+	/* Generate and provide key shares. */
+	if (!CBB_add_u16_length_prefixed(cbb, &client_shares))
+		return 0;
+
+	/* XXX - other groups. */
 
 	/* Generate X25519 key pair. */
 	if ((public_key = malloc(X25519_KEY_LENGTH)) == NULL)
@@ -1246,13 +1243,16 @@ tlsext_keyshare_x25519_generate(SSL *s, CBB *keyshare)
 	X25519_keypair(public_key, private_key);
 
 	/* Add the group and serialize the public key. */
-	if (!CBB_add_u16(keyshare, tls1_ec_nid2curve_id(NID_X25519)))
+	if (!CBB_add_u16(&client_shares, tls1_ec_nid2curve_id(NID_X25519)))
 		goto err;
-	if (!CBB_add_u16_length_prefixed(keyshare, &key_exchange))
+	if (!CBB_add_u16_length_prefixed(&client_shares, &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;
 
@@ -1266,22 +1266,6 @@ tlsext_keyshare_x25519_generate(SSL *s, CBB *keyshare)
 }
 
 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;
@@ -1343,15 +1327,43 @@ 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;
 
-	if (!tlsext_keyshare_x25519_generate(s, cbb) ||
-	    !CBB_flush(cbb))
-		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;
 
 	return 1;
+
+ err:
+	freezero(public_key, X25519_KEY_LENGTH);
+	freezero(private_key, X25519_KEY_LENGTH);
+
+	return 0;
 }
 
 int
@@ -1528,8 +1540,19 @@ tlsext_cookie_client_needs(SSL *s)
 int
 tlsext_cookie_client_build(SSL *s, CBB *cbb)
 {
-	return (tlsext_u16_prefixed_builder(cbb,
-	    S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len));
+	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;
 }
 
 int
@@ -1581,8 +1604,21 @@ tlsext_cookie_server_needs(SSL *s)
 int
 tlsext_cookie_server_build(SSL *s, CBB *cbb)
 {
-	return (tlsext_u16_prefixed_builder(cbb,
-	    S3I(s)->hs_tls13.cookie, S3I(s)->hs_tls13.cookie_len));
+	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;
 }
 
 int
@@ -1840,7 +1876,7 @@ tlsext_funcs(struct tls_extension *tlsext, int is_server)
 	if (is_server)
 		return &tlsext->server;
 
-	return &tlsext->client;
+	return &tlsext->client;	
 }
 
 static int