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>
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;
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.