Don't use the client_random entropy for GREASE.

No sense in tempting middleboxes unnecessarily.

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

ssl/handshake.cc

@@ -116,6 +116,8 @@
 
 #include <utility>
 
+#include <openssl/rand.h>
+
 #include "../crypto/internal.h"
 #include "internal.h"
 
@@ -143,7 +145,8 @@ SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg)
       next_proto_neg_seen(false),
       ticket_expected(false),
       extended_master_secret(false),
-      pending_private_key_op(false) {
+      pending_private_key_op(false),
+      grease_seeded(false) {
 }
 
 SSL_HANDSHAKE::~SSL_HANDSHAKE() {
@@ -351,18 +354,19 @@ enum ssl_verify_result_t ssl_verify_peer_cert(SSL_HANDSHAKE *hs) {
   return ret;
 }
 
-uint16_t ssl_get_grease_value(const SSL *ssl, enum ssl_grease_index_t index) {
-  // Use the client_random or server_random for entropy. This both avoids
-  // calling |RAND_bytes| on a single byte repeatedly and ensures the values are
-  // deterministic. This allows the same ClientHello be sent twice for a
-  // HelloRetryRequest or the same group be advertised in both supported_groups
-  // and key_shares.
-  uint16_t ret = ssl->server ? ssl->s3->server_random[index]
-                             : ssl->s3->client_random[index];
-  // The first four bytes of server_random are a timestamp prior to TLS 1.3, but
-  // servers have no fields to GREASE until TLS 1.3.
-  assert(!ssl->server || ssl_protocol_version(ssl) >= TLS1_3_VERSION);
+uint16_t ssl_get_grease_value(SSL_HANDSHAKE *hs,
+                              enum ssl_grease_index_t index) {
+  // Draw entropy for all GREASE values at once. This avoids calling
+  // |RAND_bytes| repeatedly and makes the values consistent within a
+  // connection. The latter is so the second ClientHello matches after
+  // HelloRetryRequest and so supported_groups and key_shares are consistent.
+  if (!hs->grease_seeded) {
+    RAND_bytes(hs->grease_seed, sizeof(hs->grease_seed));
+    hs->grease_seeded = true;
+  }
+
   // This generates a random value of the form 0xωaωa, for all 0 ≤ ω < 16.
+  uint16_t ret = hs->grease_seed[index];
   ret = (ret & 0xf0) | 0x0a;
   ret |= ret << 8;
   return ret;

ssl/handshake_client.cc

@@ -223,7 +223,7 @@ static int ssl_write_client_cipher_list(SSL_HANDSHAKE *hs, CBB *out) {
 
   // Add a fake cipher suite. See draft-davidben-tls-grease-01.
   if (ssl->ctx->grease_enabled &&
-      !CBB_add_u16(&child, ssl_get_grease_value(ssl, ssl_grease_cipher))) {
+      !CBB_add_u16(&child, ssl_get_grease_value(hs, ssl_grease_cipher))) {
     return 0;
   }
 

ssl/internal.h

@@ -1281,6 +1281,16 @@ enum ssl_hs_wait_t {
   ssl_hs_certificate_verify,
 };
 
+enum ssl_grease_index_t {
+  ssl_grease_cipher = 0,
+  ssl_grease_group,
+  ssl_grease_extension1,
+  ssl_grease_extension2,
+  ssl_grease_version,
+  ssl_grease_ticket_extension,
+  ssl_grease_last_index = ssl_grease_ticket_extension,
+};
+
 struct SSL_HANDSHAKE {
   explicit SSL_HANDSHAKE(SSL *ssl);
   ~SSL_HANDSHAKE();
@@ -1493,6 +1503,9 @@ struct SSL_HANDSHAKE {
   // in progress.
   bool pending_private_key_op:1;
 
+  // grease_seeded is true if |grease_seed| has been initialized.
+  bool grease_seeded:1;
+
   // client_version is the value sent or received in the ClientHello version.
   uint16_t client_version = 0;
 
@@ -1508,6 +1521,10 @@ struct SSL_HANDSHAKE {
   // TLS 1.3 variant.
   uint8_t session_id[SSL_MAX_SSL_SESSION_ID_LENGTH] = {0};
   uint8_t session_id_len = 0;
+
+  // grease_seed is the entropy for GREASE values. It is valid if
+  // |grease_seeded| is true.
+  uint8_t grease_seed[ssl_grease_last_index + 1] = {0};
 };
 
 UniquePtr<SSL_HANDSHAKE> ssl_handshake_new(SSL *ssl);
@@ -1650,20 +1667,11 @@ int ssl_client_cipher_list_contains_cipher(const SSL_CLIENT_HELLO *client_hello,
 
 // GREASE.
 
-enum ssl_grease_index_t {
-  ssl_grease_cipher = 0,
-  ssl_grease_group,
-  ssl_grease_extension1,
-  ssl_grease_extension2,
-  ssl_grease_version,
-  ssl_grease_ticket_extension,
-};
-
-// ssl_get_grease_value returns a GREASE value for |ssl|. For a given
+// ssl_get_grease_value returns a GREASE value for |hs|. For a given
 // connection, the values for each index will be deterministic. This allows the
 // same ClientHello be sent twice for a HelloRetryRequest or the same group be
 // advertised in both supported_groups and key_shares.
-uint16_t ssl_get_grease_value(const SSL *ssl, enum ssl_grease_index_t index);
+uint16_t ssl_get_grease_value(SSL_HANDSHAKE *hs, enum ssl_grease_index_t index);
 
 
 // Signature algorithms.

ssl/t1_lib.cc

@@ -2115,7 +2115,7 @@ static bool ext_key_share_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
     // Add a fake group. See draft-davidben-tls-grease-01.
     if (ssl->ctx->grease_enabled &&
         (!CBB_add_u16(&kse_bytes,
-                      ssl_get_grease_value(ssl, ssl_grease_group)) ||
+                      ssl_get_grease_value(hs, ssl_grease_group)) ||
          !CBB_add_u16(&kse_bytes, 1 /* length */) ||
          !CBB_add_u8(&kse_bytes, 0 /* one byte key share */))) {
       return false;
@@ -2287,7 +2287,7 @@ static bool ext_supported_versions_add_clienthello(SSL_HANDSHAKE *hs, CBB *out)
 
   // Add a fake version. See draft-davidben-tls-grease-01.
   if (ssl->ctx->grease_enabled &&
-      !CBB_add_u16(&versions, ssl_get_grease_value(ssl, ssl_grease_version))) {
+      !CBB_add_u16(&versions, ssl_get_grease_value(hs, ssl_grease_version))) {
     return false;
   }
 
@@ -2377,7 +2377,7 @@ static bool ext_supported_groups_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
   // Add a fake group. See draft-davidben-tls-grease-01.
   if (ssl->ctx->grease_enabled &&
       !CBB_add_u16(&groups_bytes,
-                   ssl_get_grease_value(ssl, ssl_grease_group))) {
+                   ssl_get_grease_value(hs, ssl_grease_group))) {
     return false;
   }
 
@@ -2813,7 +2813,7 @@ int ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, size_t header_len) {
   uint16_t grease_ext1 = 0;
   if (ssl->ctx->grease_enabled) {
     // Add a fake empty extension. See draft-davidben-tls-grease-01.
-    grease_ext1 = ssl_get_grease_value(ssl, ssl_grease_extension1);
+    grease_ext1 = ssl_get_grease_value(hs, ssl_grease_extension1);
     if (!CBB_add_u16(&extensions, grease_ext1) ||
         !CBB_add_u16(&extensions, 0 /* zero length */)) {
       OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
@@ -2841,7 +2841,7 @@ int ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, size_t header_len) {
 
   if (ssl->ctx->grease_enabled) {
     // Add a fake non-empty extension. See draft-davidben-tls-grease-01.
-    uint16_t grease_ext2 = ssl_get_grease_value(ssl, ssl_grease_extension2);
+    uint16_t grease_ext2 = ssl_get_grease_value(hs, ssl_grease_extension2);
 
     // The two fake extensions must not have the same value. GREASE values are
     // of the form 0x1a1a, 0x2a2a, 0x3a3a, etc., so XOR to generate a different

ssl/tls13_server.cc

@@ -211,7 +211,7 @@ static int add_new_session_tickets(SSL_HANDSHAKE *hs) {
 
     // Add a fake extension. See draft-davidben-tls-grease-01.
     if (!CBB_add_u16(&extensions,
-                     ssl_get_grease_value(ssl, ssl_grease_ticket_extension)) ||
+                     ssl_get_grease_value(hs, ssl_grease_ticket_extension)) ||
         !CBB_add_u16(&extensions, 0 /* empty */)) {
       return 0;
     }