Don't duplicate ServerHello construction code.

This also fixes a minor bug (that doesn't matter because we don't
implement DTLS 1.3). init_message must be paired with finish_message to
correctly handle the DTLS header.

Change-Id: I4b65c82d4b691d5b77d9e20513983145098d6f8f
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/46785
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

ssl/tls13_server.cc

@@ -708,26 +708,14 @@ static enum ssl_hs_wait_t do_read_second_client_hello(SSL_HANDSHAKE *hs) {
   return ssl_hs_ok;
 }
 
-static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
+static bool make_server_hello(SSL_HANDSHAKE *hs, Array<uint8_t> *out) {
   SSL *const ssl = hs->ssl;
-
-  Span<uint8_t> random(ssl->s3->server_random);
-  RAND_bytes(random.data(), random.size());
-
-  assert(!hs->ech_accept || hs->ech_is_inner_present);
-
-  if (hs->ech_is_inner_present) {
-    // Construct the ServerHelloECHConf message, which is the same as
-    // ServerHello, except the last 8 bytes of its random field are zeroed out.
-    Span<uint8_t> random_suffix = random.subspan(24);
-    OPENSSL_memset(random_suffix.data(), 0, random_suffix.size());
-
   ScopedCBB cbb;
   CBB body, extensions, session_id;
-    if (!ssl->method->init_message(ssl, cbb.get(), &body,
-                                   SSL3_MT_SERVER_HELLO) ||
+  if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) ||
       !CBB_add_u16(&body, TLS1_2_VERSION) ||
-        !CBB_add_bytes(&body, random.data(), random.size()) ||
+      !CBB_add_bytes(&body, ssl->s3->server_random,
+                     sizeof(ssl->s3->server_random)) ||
       !CBB_add_u8_length_prefixed(&body, &session_id) ||
       !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
       !CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
@@ -736,34 +724,37 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
       !ssl_ext_pre_shared_key_add_serverhello(hs, &extensions) ||
       !ssl_ext_key_share_add_serverhello(hs, &extensions) ||
       !ssl_ext_supported_versions_add_serverhello(hs, &extensions) ||
-        !CBB_flush(cbb.get())) {
-      return ssl_hs_error;
+      !ssl->method->finish_message(ssl, cbb.get(), out)) {
+    return false;
+  }
+  return true;
 }
 
-    // Note that |cbb| includes the message type and length fields, but not the
-    // record layer header.
-    if (!tls13_ech_accept_confirmation(
-            hs, random_suffix,
-            bssl::MakeConstSpan(CBB_data(cbb.get()), CBB_len(cbb.get())))) {
+static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
+  SSL *const ssl = hs->ssl;
+
+  Span<uint8_t> random(ssl->s3->server_random);
+  RAND_bytes(random.data(), random.size());
+
+  assert(!hs->ech_accept || hs->ech_is_inner_present);
+
+  if (hs->ech_is_inner_present) {
+    // Construct the ServerHelloECHConf message, which is the same as
+    // ServerHello, except the last 8 bytes of its random field are zeroed out.
+    Span<uint8_t> random_suffix = random.subspan(24);
+    OPENSSL_memset(random_suffix.data(), 0, random_suffix.size());
+
+    Array<uint8_t> server_hello_ech_conf;
+    if (!make_server_hello(hs, &server_hello_ech_conf) ||
+        !tls13_ech_accept_confirmation(hs, random_suffix,
+                                       server_hello_ech_conf)) {
       return ssl_hs_error;
     }
   }
 
-  // Send a ServerHello.
-  ScopedCBB cbb;
-  CBB body, extensions, session_id;
-  if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_SERVER_HELLO) ||
-      !CBB_add_u16(&body, TLS1_2_VERSION) ||
-      !CBB_add_bytes(&body, random.data(), random.size()) ||
-      !CBB_add_u8_length_prefixed(&body, &session_id) ||
-      !CBB_add_bytes(&session_id, hs->session_id, hs->session_id_len) ||
-      !CBB_add_u16(&body, SSL_CIPHER_get_protocol_id(hs->new_cipher)) ||
-      !CBB_add_u8(&body, 0) ||
-      !CBB_add_u16_length_prefixed(&body, &extensions) ||
-      !ssl_ext_pre_shared_key_add_serverhello(hs, &extensions) ||
-      !ssl_ext_key_share_add_serverhello(hs, &extensions) ||
-      !ssl_ext_supported_versions_add_serverhello(hs, &extensions) ||
-      !ssl_add_message_cbb(ssl, cbb.get())) {
+  Array<uint8_t> server_hello;
+  if (!make_server_hello(hs, &server_hello) ||
+      !ssl->method->add_message(ssl, std::move(server_hello))) {
     return ssl_hs_error;
   }
 
@@ -782,6 +773,8 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) {
   }
 
   // Send EncryptedExtensions.
+  ScopedCBB cbb;
+  CBB body;
   if (!ssl->method->init_message(ssl, cbb.get(), &body,
                                  SSL3_MT_ENCRYPTED_EXTENSIONS) ||
       !ssl_add_serverhello_tlsext(hs, &body) ||