commit 1a444daca6da74ad748c681dd3aa5a5d283b1e43
author David Benjamin <davidben@google.com> Sat Jan 21 14:27:45 2017 -0500
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Fri Jan 27 23:22:14 2017 +0000
tree 08b71ecc08f399e91888737c833ea5f3577d536d
parent 360a4c26169c4c704eebfc0ec973db4c31999ad8

Detach V2ClientHello hashing hack from ssl_hash_message_t.

This is kind of annoying (even new state is needed to keep the layering
right). As part of aligning the read paths of the TLS 1.2 and TLS 1.3
state machine, we'll want to move to states calling
ssl_hash_current_message when the process the message, rather than when
the message is read. Right now the TLS 1.2 optional message story
(reuse_message) depends on all messages preceded by an optional message
using ssl_hash_message. For instance, if TLS 1.2 decided to place
CertificateStatus before ServerKeyExchange, we would not be able to
handle it.

However, V2ClientHello, by being handled in the message layer, relies on
ssl_get_message-driven hashing to replace the usual ClientHello hash
with a hash of something custom. This switches things so rather than
ClientHellos being always pre-hashed by the message layer, simulated
ClientHellos no-op ssl_hash_current_message.

This just replaces one hack with another (V2ClientHello is inherently
nasty), but this hack should be more compatible with future plans.

BUG=128

Change-Id: If807ea749d91e306a37bb2362ecc69b84bf224c9
Reviewed-on: https://boringssl-review.googlesource.com/13265
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

ssl/s3_both.c

diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index 70ed435..e53e869 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -506,7 +506,7 @@
   return 1;
 }
 
-static int read_v2_client_hello(SSL *ssl, int *out_is_v2_client_hello) {
+static int read_v2_client_hello(SSL *ssl) {
   /* Read the first 5 bytes, the size of the TLS record header. This is
    * sufficient to detect a V2ClientHello and ensures that we never read beyond
    * the first record. */
@@ -534,7 +534,6 @@
   if ((p[0] & 0x80) == 0 || p[2] != SSL2_MT_CLIENT_HELLO ||
       p[3] != SSL3_VERSION_MAJOR) {
     /* Not a V2ClientHello. */
-    *out_is_v2_client_hello = 0;
     return 1;
   }
 
@@ -656,7 +655,9 @@
   ssl_read_buffer_consume(ssl, 2 + msg_length);
   ssl_read_buffer_discard(ssl);
 
-  *out_is_v2_client_hello = 1;
+  ssl->s3->is_v2_hello = 1;
+  /* This is the first message, so hs must be non-NULL. */
+  ssl->s3->hs->v2_clienthello = 1;
   return 1;
 }
 
@@ -672,18 +673,10 @@
 
   if (ssl->server && !ssl->s3->v2_hello_done) {
     /* Bypass the record layer for the first message to handle V2ClientHello. */
-    assert(hash_message == ssl_hash_message);
-    int is_v2_client_hello = 0;
-    int ret = read_v2_client_hello(ssl, &is_v2_client_hello);
+    int ret = read_v2_client_hello(ssl);
     if (ret <= 0) {
       return ret;
     }
-    if (is_v2_client_hello) {
-      /* This is the first message, so hs must be non-NULL. */
-      ssl->s3->hs->v2_clienthello = 1;
-      /* V2ClientHello is hashed separately. */
-      hash_message = ssl_dont_hash_message;
-    }
     ssl->s3->v2_hello_done = 1;
   }
 
@@ -752,6 +745,11 @@
 }
 
 int ssl_hash_current_message(SSL *ssl) {
+  /* V2ClientHellos are hashed implicitly. */
+  if (ssl->s3->is_v2_hello) {
+    return 1;
+  }
+
   CBS cbs;
   ssl->method->get_current_message(ssl, &cbs);
   return ssl3_update_handshake_hash(ssl, CBS_data(&cbs), CBS_len(&cbs));
@@ -766,6 +764,7 @@
     ssl->init_msg = NULL;
     ssl->init_num = 0;
     ssl->init_buf->length = 0;
+    ssl->s3->is_v2_hello = 0;
   }
 
   if (free_buffer) {