commit 7d10ab594cc85a0411041d086c6c101f3e98b4ee
author David Benjamin <davidben@google.com> Fri Nov 02 18:31:17 2018 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Nov 09 19:01:04 2018 +0000
tree 710bf0c30ea18cc58e2dbab789e56f3dad846c88
parent 4706ea728ef6a883247c69f7c2b3a5597251f040

Abstract hs_buf a little.

Having to lazily create it is a little wordy, and we append to it in
three places now. V2ClientHello makes this slightly finicky, but I think
this is still clearer.

Change-Id: If931db0b56efd7f0728c0b7d119886864dd7933a
Reviewed-on: https://boringssl-review.googlesource.com/c/32824
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 9d78b1f..fa86bda 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1053,6 +1053,10 @@
 // handshake data that has not been consumed by |get_message|.
 bool tls_has_unprocessed_handshake_data(const SSL *ssl);
 
+// tls_append_handshake_data appends |data| to the handshake buffer. It returns
+// true on success and false on allocation failure.
+bool tls_append_handshake_data(SSL *ssl, Span<const uint8_t> data);
+
 // dtls_has_unprocessed_handshake_data behaves like
 // |tls_has_unprocessed_handshake_data| for DTLS.
 bool dtls_has_unprocessed_handshake_data(const SSL *ssl);

ssl/s3_both.cc

diff --git a/ssl/s3_both.cc b/ssl/s3_both.cc
index 55e9aaa..f835dc2 100644
--- a/ssl/s3_both.cc
+++ b/ssl/s3_both.cc
@@ -411,7 +411,7 @@
   OPENSSL_memcpy(random + (SSL3_RANDOM_SIZE - rand_len), CBS_data(&challenge),
                  rand_len);
 
-  // Write out an equivalent TLS ClientHello.
+  // Write out an equivalent TLS ClientHello directly to the handshake buffer.
   size_t max_v3_client_hello = SSL3_HM_HEADER_LENGTH + 2 /* version */ +
                                SSL3_RANDOM_SIZE + 1 /* session ID length */ +
                                2 /* cipher list length */ +
@@ -419,7 +419,11 @@
                                1 /* compression length */ + 1 /* compression */;
   ScopedCBB client_hello;
   CBB hello_body, cipher_suites;
-  if (!BUF_MEM_reserve(ssl->s3->hs_buf.get(), max_v3_client_hello) ||
+  if (!ssl->s3->hs_buf) {
+    ssl->s3->hs_buf.reset(BUF_MEM_new());
+  }
+  if (!ssl->s3->hs_buf ||
+      !BUF_MEM_reserve(ssl->s3->hs_buf.get(), max_v3_client_hello) ||
       !CBB_init_fixed(client_hello.get(), (uint8_t *)ssl->s3->hs_buf->data,
                       ssl->s3->hs_buf->max) ||
       !CBB_add_u8(client_hello.get(), SSL3_MT_CLIENT_HELLO) ||
@@ -539,18 +543,18 @@
   return ssl->s3->hs_buf && ssl->s3->hs_buf->length > msg_len;
 }
 
-ssl_open_record_t ssl3_open_handshake(SSL *ssl, size_t *out_consumed,
-                                      uint8_t *out_alert, Span<uint8_t> in) {
-  *out_consumed = 0;
+bool tls_append_handshake_data(SSL *ssl, Span<const uint8_t> data) {
   // Re-create the handshake buffer if needed.
   if (!ssl->s3->hs_buf) {
     ssl->s3->hs_buf.reset(BUF_MEM_new());
-    if (!ssl->s3->hs_buf) {
-      *out_alert = SSL_AD_INTERNAL_ERROR;
-      return ssl_open_record_error;
-    }
   }
+  return ssl->s3->hs_buf &&
+         BUF_MEM_append(ssl->s3->hs_buf.get(), data.data(), data.size());
+}
 
+ssl_open_record_t ssl3_open_handshake(SSL *ssl, size_t *out_consumed,
+                                      uint8_t *out_alert, Span<uint8_t> in) {
+  *out_consumed = 0;
   // Bypass the record layer for the first message to handle V2ClientHello.
   if (ssl->server && !ssl->s3->v2_hello_done) {
     // Ask for the first 5 bytes, the size of the TLS record header. This is
@@ -619,7 +623,7 @@
   }
 
   // Append the entire handshake record to the buffer.
-  if (!BUF_MEM_append(ssl->s3->hs_buf.get(), body.data(), body.size())) {
+  if (!tls_append_handshake_data(ssl, body)) {
     *out_alert = SSL_AD_INTERNAL_ERROR;
     return ssl_open_record_error;
   }

ssl/s3_pkt.cc

diff --git a/ssl/s3_pkt.cc b/ssl/s3_pkt.cc
index e9b652e..f0ae8a2 100644
--- a/ssl/s3_pkt.cc
+++ b/ssl/s3_pkt.cc
@@ -318,11 +318,7 @@
       return ssl_open_record_error;
     }
 
-    if (!ssl->s3->hs_buf) {
-      ssl->s3->hs_buf.reset(BUF_MEM_new());
-    }
-    if (!ssl->s3->hs_buf ||
-        !BUF_MEM_append(ssl->s3->hs_buf.get(), body.data(), body.size())) {
+    if (!tls_append_handshake_data(ssl, body)) {
       *out_alert = SSL_AD_INTERNAL_ERROR;
       return ssl_open_record_error;
     }

ssl/ssl_lib.cc

diff --git a/ssl/ssl_lib.cc b/ssl/ssl_lib.cc
index fd560ff..8a88802 100644
--- a/ssl/ssl_lib.cc
+++ b/ssl/ssl_lib.cc
@@ -846,15 +846,7 @@
     return 0;
   }
 
-  // Re-create the handshake buffer if needed.
-  if (!ssl->s3->hs_buf) {
-    ssl->s3->hs_buf.reset(BUF_MEM_new());
-    if (!ssl->s3->hs_buf) {
-      return 0;
-    }
-  }
-
-  return BUF_MEM_append(ssl->s3->hs_buf.get(), data, len);
+  return tls_append_handshake_data(ssl, MakeConstSpan(data, len));
 }
 
 int SSL_do_handshake(SSL *ssl) {