Update the ECH GREASE size selection.
We misread (or maybe it changed?) the draft padding scheme. The current
text does not round the whole payload to a multiple of 32, just the
server name as a fallback. Switch the GREASE size selection to match.
Although, we may want to change the draft here. See also
https://github.com/tlswg/draft-ietf-tls-esni/issues/433
While I'm here, update some references from draft-09 to draft-10. Also
make the comment less verbose.
Bug: 275
Change-Id: I3c9f34159890bc3b7d71f6877f34b895bc7f9b17
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47644
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 0d3d944..3ef6d8d 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3573,7 +3573,7 @@
//
// ECH support in BoringSSL is still experimental and under development.
//
-// See https://tools.ietf.org/html/draft-ietf-tls-esni-09.
+// See https://tools.ietf.org/html/draft-ietf-tls-esni-10.
// SSL_set_enable_ech_grease configures whether the client may send ECH GREASE
// as part of this connection.
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index 9417976..5aeb1b4 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -273,7 +273,7 @@
*out_is_decrypt_error = false;
// Compute the ClientHello portion of the ClientHelloOuterAAD value. See
- // draft-ietf-tls-esni-09, section 5.2.
+ // draft-ietf-tls-esni-10, section 5.2.
ScopedCBB ch_outer_aad_cbb;
CBB enc_cbb, outer_hello_cbb, extensions_cbb;
if (!CBB_init(ch_outer_aad_cbb.get(), 0) ||
diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 0a7ffb0..970e7ba 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -593,7 +593,7 @@
// Encrypted ClientHello (ECH)
//
-// https://tools.ietf.org/html/draft-ietf-tls-esni-09
+// https://tools.ietf.org/html/draft-ietf-tls-esni-10
// random_size returns a random value between |min| and |max|, inclusive.
static size_t random_size(size_t min, size_t max) {
@@ -629,48 +629,26 @@
uint8_t private_key_unused[X25519_PRIVATE_KEY_LEN];
X25519_keypair(ech_enc, private_key_unused);
- // To determine a plausible length for the payload, we first estimate the size
- // of a typical EncodedClientHelloInner, with an expected use of
- // outer_extensions. To limit the size, we only consider initial ClientHellos
- // that do not offer resumption.
+ // To determine a plausible length for the payload, we estimate the size of a
+ // typical EncodedClientHelloInner without resumption:
//
- // Field/Extension Size
- // ---------------------------------------------------------------------
- // version 2
- // random 32
- // legacy_session_id 1
- // - Has a U8 length prefix, but body is
- // always empty string in inner CH.
- // cipher_suites 2 (length prefix)
- // - Only includes TLS 1.3 ciphers (3). 6
- // - Maybe also include a GREASE suite. 2
- // legacy_compression_methods 2 (length prefix)
- // - Always has "null" compression method. 1
- // extensions: 2 (length prefix)
- // - encrypted_client_hello (empty). 4 (id + length prefix)
- // - supported_versions. 4 (id + length prefix)
- // - U8 length prefix 1
- // - U16 protocol version (TLS 1.3) 2
- // - outer_extensions. 4 (id + length prefix)
- // - U8 length prefix 1
- // - N extension IDs (2 bytes each):
- // - key_share 2
- // - sigalgs 2
- // - sct 2
- // - alpn 2
- // - supported_groups. 2
- // - status_request. 2
- // - psk_key_exchange_modes. 2
- // - compress_certificate. 2
+ // 2+32+1+2 version, random, legacy_session_id, legacy_compression_methods
+ // 2+4*2 cipher_suites (three TLS 1.3 ciphers, GREASE)
+ // 2 extensions prefix
+ // 4 ech_is_inner
+ // 4+1+2*2 supported_versions (TLS 1.3, GREASE)
+ // 4+1+10*2 outer_extensions (key_share, sigalgs, sct, alpn,
+ // supported_groups, status_request, psk_key_exchange_modes,
+ // compress_certificate, GREASE x2)
//
- // The server_name extension has an overhead of 9 bytes, plus up to an
- // estimated 100 bytes of hostname. Rounding up to a multiple of 32 yields a
- // range of 96 to 192. Note that this estimate does not fully capture
- // optional extensions like GREASE, but the rounding gives some leeway.
-
- uint8_t payload[kAEADOverhead + 192];
- const size_t payload_len =
- kAEADOverhead + 32 * random_size(96 / 32, 192 / 32);
+ // The server_name extension has an overhead of 9 bytes. For now, arbitrarily
+ // estimate maximum_name_length to be between 32 and 100 bytes.
+ //
+ // TODO(davidben): If the padding scheme changes to also round the entire
+ // payload, adjust this to match. See
+ // https://github.com/tlswg/draft-ietf-tls-esni/issues/433
+ uint8_t payload[196 + kAEADOverhead];
+ const size_t payload_len = random_size(128, 196) + kAEADOverhead;
assert(payload_len <= sizeof(payload));
RAND_bytes(payload, payload_len);
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index 15c92c8..2d89dbd 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -297,7 +297,7 @@
}
// The contents of a CH "encrypted_client_hello" extension.
-// https://tools.ietf.org/html/draft-ietf-tls-esni-09
+// https://tools.ietf.org/html/draft-ietf-tls-esni-10
type clientECH struct {
hpkeKDF uint16
hpkeAEAD uint16
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index cda53ec..9c3063f 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -522,7 +522,7 @@
return false;
}
- // Per draft-ietf-tls-esni-09, accept_confirmation is computed with
+ // Per draft-ietf-tls-esni-10, accept_confirmation is computed with
// Derive-Secret, which derives a secret of size Hash.length. That value is
// then truncated to the first 8 bytes. Note this differs from deriving an
// 8-byte secret because the target length is included in the derivation.
