commit 33e8c7893bd36c9aa88e1a4c9c5a0b4cb8177899
author David Benjamin <davidben@google.com> Wed May 19 17:07:18 2021 -0400
committer David Benjamin <davidben@google.com> Wed Jun 09 22:31:34 2021 +0000
tree 4a91985eebd871eb29a974e01c18c66663857954
parent 52b3638f0ae24605ae9c131af9477c3d58f7f2a1

Initialize grease_seed on construction.

This lets ssl_get_grease_value be const. The lazy thing isn't a
deal-breaker (we only need idempotence, and a non-thread-safe const also
works fine), but just initializing it earlier seems simpler.

Bug: 275
Change-Id: Iad228ea4a9146ede9a3849f3339f7ec9e698e6eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47988
Reviewed-by: Adam Langley <agl@google.com>

ssl/handshake.cc

diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index f33547e..9133e16 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -146,7 +146,6 @@
       ticket_expected(false),
       extended_master_secret(false),
       pending_private_key_op(false),
-      grease_seeded(false),
       handback(false),
       hints_requested(false),
       cert_compression_negotiated(false),
@@ -154,6 +153,12 @@
       can_release_private_key(false),
       channel_id_negotiated(false) {
   assert(ssl);
+
+  // 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.
+  RAND_bytes(grease_seed, sizeof(grease_seed));
 }
 
 SSL_HANDSHAKE::~SSL_HANDSHAKE() {
@@ -435,17 +440,8 @@
   return ret;
 }
 
-uint16_t ssl_get_grease_value(SSL_HANDSHAKE *hs,
+uint16_t ssl_get_grease_value(const 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;

ssl/internal.h

diff --git a/ssl/internal.h b/ssl/internal.h
index 1893aa5..7d84312 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1927,9 +1927,6 @@
   // in progress.
   bool pending_private_key_op : 1;
 
-  // grease_seeded is true if |grease_seed| has been initialized.
-  bool grease_seeded : 1;
-
   // handback indicates that a server should pause the handshake after
   // finishing operations that require private key material, in such a way that
   // |SSL_get_error| returns |SSL_ERROR_HANDBACK|.  It is set by
@@ -1974,8 +1971,7 @@
   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.
+  // grease_seed is the entropy for GREASE values.
   uint8_t grease_seed[ssl_grease_last_index + 1] = {0};
 };
 
@@ -2170,7 +2166,8 @@
 // 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(SSL_HANDSHAKE *hs, enum ssl_grease_index_t index);
+uint16_t ssl_get_grease_value(const SSL_HANDSHAKE *hs,
+                              enum ssl_grease_index_t index);
 
 
 // Signature algorithms.