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>
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 3b446a8..cdd1b32 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -116,6 +116,8 @@
#include <utility>
+#include <openssl/rand.h>
+
#include "../crypto/internal.h"
#include "internal.h"
@@ -143,7 +145,8 @@
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 @@
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;
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index 4e2158e..3961ca1 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -223,7 +223,7 @@
// 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;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index fb99101..d4f0e3f 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1281,6 +1281,16 @@
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 @@
// 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 @@
// 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 @@
// 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.
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index dafb885..de02476 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -2115,7 +2115,7 @@
// 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 @@
// 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 @@
// 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 @@
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 @@
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
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 331e0d5..a6a3a0a 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -211,7 +211,7 @@
// 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;
}
