Update to draft-ietf-tls-esni-13.
Later CLs will clean up the ClientHello construction a bit (draft-12
avoids computing ClientHelloOuter twice). I suspect the transcript
handling on the client can also be simpler, but I'll see what's
convenient after I've changed how ClientHelloOuter is constructed.
Changes of note between draft-10 and draft-13:
- There is now an ECH confirmation signal in both HRR and SH. We don't
actually make much use of this in our client right now, but it
resolves a bunch of weird issues around HRR, including edge cases if
HRR applies to one ClientHello but not the other.
- The confirmation signal no longer depends on key_share and PSK, so we
don't have to work around a weird ordering issue.
- ech_is_inner is now folded into the main encrypted_client_hello code
point. This works better with some stuff around HRR.
- Padding is moved from the padding extension, computed with
ClientHelloInner, to something we fill in afterwards. This makes it
easier to pad up the whole thing to a multiple of 32. I've accordingly
updated to the latest recommended padding construction, and updated
the GREASE logic to match.
- ech_outer_extensions is much easier to process because the order is
required to be consistent. We were doing that anyway, and now a simple
linear scan works.
- ClientHelloOuterAAD now uses an all zero placeholder payload of the
same length. This lets us simplify the server code, but, for now, I've
kept the client code the same. I'll follow this up with a CL to avoid
computing ClientHelloOuter twice.
- ClientHelloOuterAAD is allowed to contain a placeholder PSK. I haven't
filled that in and will do it in a follow-up CL.
Bug: 275
Change-Id: I7464345125c53968b2fe692f9268e392120fc2eb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48912
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index 2f85410..6879134 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -80,6 +80,7 @@
SSL,156,HTTP_REQUEST
SSL,157,INAPPROPRIATE_FALLBACK
SSL,303,INCONSISTENT_CLIENT_HELLO
+SSL,321,INCONSISTENT_ECH_NEGOTIATION
SSL,259,INVALID_ALPN_PROTOCOL
SSL,315,INVALID_ALPN_PROTOCOL_LIST
SSL,314,INVALID_CLIENT_HELLO_INNER
@@ -132,6 +133,7 @@
SSL,187,OLD_SESSION_CIPHER_NOT_RETURNED
SSL,268,OLD_SESSION_PRF_HASH_MISMATCH
SSL,188,OLD_SESSION_VERSION_NOT_RETURNED
+SSL,320,OUTER_EXTENSION_NOT_FOUND
SSL,189,OUTPUT_ALIASES_INPUT
SSL,190,PARSE_TLSEXT
SSL,191,PATH_TOO_LONG
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 996a0f9..eae3c4b 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3569,7 +3569,7 @@
//
// ECH support in BoringSSL is still experimental and under development.
//
-// See https://tools.ietf.org/html/draft-ietf-tls-esni-10.
+// See https://tools.ietf.org/html/draft-ietf-tls-esni-13.
// SSL_set_enable_ech_grease configures whether the client will send a GREASE
// ECH extension when no supported ECHConfig is available.
@@ -5548,6 +5548,8 @@
#define SSL_R_INVALID_ECH_PUBLIC_NAME 317
#define SSL_R_INVALID_ECH_CONFIG_LIST 318
#define SSL_R_ECH_REJECTED 319
+#define SSL_R_OUTER_EXTENSION_NOT_FOUND 320
+#define SSL_R_INCONSISTENT_ECH_NEGOTIATION 321
#define SSL_R_SSLV3_ALERT_CLOSE_NOTIFY 1000
#define SSL_R_SSLV3_ALERT_UNEXPECTED_MESSAGE 1010
#define SSL_R_SSLV3_ALERT_BAD_RECORD_MAC 1020
diff --git a/include/openssl/tls1.h b/include/openssl/tls1.h
index 9f38c81..a3136c0 100644
--- a/include/openssl/tls1.h
+++ b/include/openssl/tls1.h
@@ -179,7 +179,7 @@
#define TLS1_AD_UNKNOWN_PSK_IDENTITY 115
#define TLS1_AD_CERTIFICATE_REQUIRED 116
#define TLS1_AD_NO_APPLICATION_PROTOCOL 120
-#define TLS1_AD_ECH_REQUIRED 121 // draft-ietf-tls-esni-10
+#define TLS1_AD_ECH_REQUIRED 121 // draft-ietf-tls-esni-13
// ExtensionType values from RFC 6066
#define TLSEXT_TYPE_server_name 0
@@ -246,10 +246,9 @@
// extension number.
#define TLSEXT_TYPE_application_settings 17513
-// ExtensionType values from draft-ietf-tls-esni-10. This is not an IANA defined
+// ExtensionType values from draft-ietf-tls-esni-13. This is not an IANA defined
// extension number.
-#define TLSEXT_TYPE_encrypted_client_hello 0xfe0a
-#define TLSEXT_TYPE_ech_is_inner 0xda09
+#define TLSEXT_TYPE_encrypted_client_hello 0xfe0d
#define TLSEXT_TYPE_ech_outer_extensions 0xfd00
// ExtensionType value from RFC 6962
diff --git a/ssl/encrypted_client_hello.cc b/ssl/encrypted_client_hello.cc
index 4b837ff..5192cc6 100644
--- a/ssl/encrypted_client_hello.cc
+++ b/ssl/encrypted_client_hello.cc
@@ -31,12 +31,6 @@
#include "internal.h"
-#if defined(OPENSSL_MSAN)
-#define NO_SANITIZE_MEMORY __attribute__((no_sanitize("memory")))
-#else
-#define NO_SANITIZE_MEMORY
-#endif
-
BSSL_NAMESPACE_BEGIN
// ECH reuses the extension code point for the version number.
@@ -84,159 +78,17 @@
return true;
}
-bool ssl_decode_client_hello_inner(
- SSL *ssl, uint8_t *out_alert, Array<uint8_t> *out_client_hello_inner,
- Span<const uint8_t> encoded_client_hello_inner,
- const SSL_CLIENT_HELLO *client_hello_outer) {
- SSL_CLIENT_HELLO client_hello_inner;
- if (!ssl_client_hello_init(ssl, &client_hello_inner,
- encoded_client_hello_inner)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- // TLS 1.3 ClientHellos must have extensions, and EncodedClientHelloInners use
- // ClientHelloOuter's session_id.
- if (client_hello_inner.extensions_len == 0 ||
- client_hello_inner.session_id_len != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- client_hello_inner.session_id = client_hello_outer->session_id;
- client_hello_inner.session_id_len = client_hello_outer->session_id_len;
-
- // Begin serializing a message containing the ClientHelloInner in |cbb|.
- ScopedCBB cbb;
- CBB body, extensions;
- if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
- !ssl_client_hello_write_without_extensions(&client_hello_inner, &body) ||
- !CBB_add_u16_length_prefixed(&body, &extensions)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
-
- // Sort the extensions in ClientHelloOuter, so ech_outer_extensions may be
- // processed in O(n*log(n)) time, rather than O(n^2).
- struct Extension {
- uint16_t extension = 0;
- Span<const uint8_t> body;
- bool copied = false;
- };
-
- // MSan's libc interceptors do not handle |bsearch|. See b/182583130.
- auto compare_extension = [](const void *a, const void *b)
- NO_SANITIZE_MEMORY -> int {
- const Extension *extension_a = reinterpret_cast<const Extension *>(a);
- const Extension *extension_b = reinterpret_cast<const Extension *>(b);
- if (extension_a->extension < extension_b->extension) {
- return -1;
- } else if (extension_a->extension > extension_b->extension) {
- return 1;
- }
- return 0;
- };
- GrowableArray<Extension> sorted_extensions;
- CBS unsorted_extensions(MakeConstSpan(client_hello_outer->extensions,
- client_hello_outer->extensions_len));
- while (CBS_len(&unsorted_extensions) > 0) {
- Extension extension;
- CBS extension_body;
- if (!CBS_get_u16(&unsorted_extensions, &extension.extension) ||
- !CBS_get_u16_length_prefixed(&unsorted_extensions, &extension_body)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
- extension.body = extension_body;
- if (!sorted_extensions.Push(extension)) {
- return false;
- }
- }
- qsort(sorted_extensions.data(), sorted_extensions.size(), sizeof(Extension),
- compare_extension);
-
- // Copy extensions from |client_hello_inner|, expanding ech_outer_extensions.
- CBS inner_extensions(MakeConstSpan(client_hello_inner.extensions,
- client_hello_inner.extensions_len));
- while (CBS_len(&inner_extensions) > 0) {
- uint16_t extension_id;
- CBS extension_body;
- if (!CBS_get_u16(&inner_extensions, &extension_id) ||
- !CBS_get_u16_length_prefixed(&inner_extensions, &extension_body)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- if (extension_id != TLSEXT_TYPE_ech_outer_extensions) {
- if (!CBB_add_u16(&extensions, extension_id) ||
- !CBB_add_u16(&extensions, CBS_len(&extension_body)) ||
- !CBB_add_bytes(&extensions, CBS_data(&extension_body),
- CBS_len(&extension_body))) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
- continue;
- }
-
- // Replace ech_outer_extensions with the corresponding outer extensions.
- CBS outer_extensions;
- if (!CBS_get_u8_length_prefixed(&extension_body, &outer_extensions) ||
- CBS_len(&extension_body) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- while (CBS_len(&outer_extensions) > 0) {
- uint16_t extension_needed;
- if (!CBS_get_u16(&outer_extensions, &extension_needed)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- if (extension_needed == TLSEXT_TYPE_encrypted_client_hello) {
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- // Find the referenced extension.
- Extension key;
- key.extension = extension_needed;
- Extension *result = reinterpret_cast<Extension *>(
- bsearch(&key, sorted_extensions.data(), sorted_extensions.size(),
- sizeof(Extension), compare_extension));
- if (result == nullptr) {
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
-
- // Extensions may be referenced at most once, to bound the result size.
- if (result->copied) {
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
- return false;
- }
- result->copied = true;
-
- if (!CBB_add_u16(&extensions, extension_needed) ||
- !CBB_add_u16(&extensions, result->body.size()) ||
- !CBB_add_bytes(&extensions, result->body.data(),
- result->body.size())) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- return false;
- }
- }
- }
- if (!CBB_flush(&body)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
-
- // See https://github.com/tlswg/draft-ietf-tls-esni/pull/411
+static bool is_valid_client_hello_inner(SSL *ssl, uint8_t *out_alert,
+ Span<const uint8_t> body) {
+ // See draft-ietf-tls-esni-13, section 7.1.
+ SSL_CLIENT_HELLO client_hello;
CBS extension;
- if (!ssl_client_hello_init(ssl, &client_hello_inner,
- MakeConstSpan(CBB_data(&body), CBB_len(&body))) ||
- !ssl_client_hello_get_extension(&client_hello_inner, &extension,
- TLSEXT_TYPE_ech_is_inner) ||
- CBS_len(&extension) != 0 ||
- ssl_client_hello_get_extension(&client_hello_inner, &extension,
- TLSEXT_TYPE_encrypted_client_hello) ||
- !ssl_client_hello_get_extension(&client_hello_inner, &extension,
+ if (!ssl_client_hello_init(ssl, &client_hello, body) ||
+ !ssl_client_hello_get_extension(&client_hello, &extension,
+ TLSEXT_TYPE_encrypted_client_hello) ||
+ CBS_len(&extension) != 1 || //
+ CBS_data(&extension)[0] != ECH_CLIENT_INNER ||
+ !ssl_client_hello_get_extension(&client_hello, &extension,
TLSEXT_TYPE_supported_versions)) {
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER);
@@ -267,6 +119,131 @@
return false;
}
}
+ return true;
+}
+
+bool ssl_decode_client_hello_inner(
+ SSL *ssl, uint8_t *out_alert, Array<uint8_t> *out_client_hello_inner,
+ Span<const uint8_t> encoded_client_hello_inner,
+ const SSL_CLIENT_HELLO *client_hello_outer) {
+ SSL_CLIENT_HELLO client_hello_inner;
+ CBS cbs = encoded_client_hello_inner;
+ if (!ssl_parse_client_hello_with_trailing_data(ssl, &cbs,
+ &client_hello_inner)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ // The remaining data is padding.
+ uint8_t padding;
+ while (CBS_get_u8(&cbs, &padding)) {
+ if (padding != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return false;
+ }
+ }
+
+ // TLS 1.3 ClientHellos must have extensions, and EncodedClientHelloInners use
+ // ClientHelloOuter's session_id.
+ if (client_hello_inner.extensions_len == 0 ||
+ client_hello_inner.session_id_len != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ client_hello_inner.session_id = client_hello_outer->session_id;
+ client_hello_inner.session_id_len = client_hello_outer->session_id_len;
+
+ // Begin serializing a message containing the ClientHelloInner in |cbb|.
+ ScopedCBB cbb;
+ CBB body, extensions_cbb;
+ if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
+ !ssl_client_hello_write_without_extensions(&client_hello_inner, &body) ||
+ !CBB_add_u16_length_prefixed(&body, &extensions_cbb)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+
+ auto inner_extensions = MakeConstSpan(client_hello_inner.extensions,
+ client_hello_inner.extensions_len);
+ CBS ext_list_wrapper;
+ if (!ssl_client_hello_get_extension(&client_hello_inner, &ext_list_wrapper,
+ TLSEXT_TYPE_ech_outer_extensions)) {
+ // No ech_outer_extensions. Copy everything.
+ if (!CBB_add_bytes(&extensions_cbb, inner_extensions.data(),
+ inner_extensions.size())) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+ } else {
+ const size_t offset = CBS_data(&ext_list_wrapper) - inner_extensions.data();
+ auto inner_extensions_before =
+ inner_extensions.subspan(0, offset - 4 /* extension header */);
+ auto inner_extensions_after =
+ inner_extensions.subspan(offset + CBS_len(&ext_list_wrapper));
+ if (!CBB_add_bytes(&extensions_cbb, inner_extensions_before.data(),
+ inner_extensions_before.size())) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+
+ // Expand ech_outer_extensions. See draft-ietf-tls-esni-13, Appendix B.
+ CBS ext_list;
+ if (!CBS_get_u8_length_prefixed(&ext_list_wrapper, &ext_list) ||
+ CBS_len(&ext_list) == 0 || CBS_len(&ext_list_wrapper) != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ CBS outer_extensions;
+ CBS_init(&outer_extensions, client_hello_outer->extensions,
+ client_hello_outer->extensions_len);
+ while (CBS_len(&ext_list) != 0) {
+ // Find the next extension to copy.
+ uint16_t want;
+ if (!CBS_get_u16(&ext_list, &want)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ // Seek to |want| in |outer_extensions|. |ext_list| is required to match
+ // ClientHelloOuter in order.
+ uint16_t found;
+ CBS ext_body;
+ do {
+ if (CBS_len(&outer_extensions) == 0) {
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_OUTER_EXTENSION_NOT_FOUND);
+ return false;
+ }
+ if (!CBS_get_u16(&outer_extensions, &found) ||
+ !CBS_get_u16_length_prefixed(&outer_extensions, &ext_body)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ } while (found != want);
+ // Copy the extension.
+ if (!CBB_add_u16(&extensions_cbb, found) ||
+ !CBB_add_u16(&extensions_cbb, CBS_len(&ext_body)) ||
+ !CBB_add_bytes(&extensions_cbb, CBS_data(&ext_body),
+ CBS_len(&ext_body))) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ }
+
+ if (!CBB_add_bytes(&extensions_cbb, inner_extensions_after.data(),
+ inner_extensions_after.size())) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+ }
+ if (!CBB_flush(&body)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+
+ if (!is_valid_client_hello_inner(
+ ssl, out_alert, MakeConstSpan(CBB_data(&body), CBB_len(&body)))) {
+ return false;
+ }
if (!ssl->method->finish_message(ssl, cbb.get(), out_client_hello_inner)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
@@ -275,56 +252,31 @@
return true;
}
-bool ssl_client_hello_decrypt(
- EVP_HPKE_CTX *hpke_ctx, Array<uint8_t> *out_encoded_client_hello_inner,
- bool *out_is_decrypt_error, const SSL_CLIENT_HELLO *client_hello_outer,
- uint16_t kdf_id, uint16_t aead_id, const uint8_t config_id,
- Span<const uint8_t> enc, Span<const uint8_t> payload) {
+bool ssl_client_hello_decrypt(EVP_HPKE_CTX *hpke_ctx, Array<uint8_t> *out,
+ bool *out_is_decrypt_error,
+ const SSL_CLIENT_HELLO *client_hello_outer,
+ Span<const uint8_t> payload) {
*out_is_decrypt_error = false;
- // Compute the ClientHello portion of the ClientHelloOuterAAD value. See
- // draft-ietf-tls-esni-10, section 5.2.
- ScopedCBB aad;
- CBB enc_cbb, outer_hello_cbb, extensions_cbb;
- if (!CBB_init(aad.get(), 256) ||
- !CBB_add_u16(aad.get(), kdf_id) ||
- !CBB_add_u16(aad.get(), aead_id) ||
- !CBB_add_u8(aad.get(), config_id) ||
- !CBB_add_u16_length_prefixed(aad.get(), &enc_cbb) ||
- !CBB_add_bytes(&enc_cbb, enc.data(), enc.size()) ||
- !CBB_add_u24_length_prefixed(aad.get(), &outer_hello_cbb) ||
- !ssl_client_hello_write_without_extensions(client_hello_outer,
- &outer_hello_cbb) ||
- !CBB_add_u16_length_prefixed(&outer_hello_cbb, &extensions_cbb)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
+ // The ClientHelloOuterAAD is |client_hello_outer| with |payload| (which must
+ // point within |client_hello_outer->extensions|) replaced with zeros. See
+ // draft-ietf-tls-esni-13, section 5.2.
+ Array<uint8_t> aad;
+ if (!aad.CopyFrom(MakeConstSpan(client_hello_outer->client_hello,
+ client_hello_outer->client_hello_len))) {
return false;
}
- CBS extensions(MakeConstSpan(client_hello_outer->extensions,
- client_hello_outer->extensions_len));
- while (CBS_len(&extensions) > 0) {
- uint16_t extension_id;
- CBS extension_body;
- if (!CBS_get_u16(&extensions, &extension_id) ||
- !CBS_get_u16_length_prefixed(&extensions, &extension_body)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return false;
- }
- if (extension_id == TLSEXT_TYPE_encrypted_client_hello) {
- continue;
- }
- if (!CBB_add_u16(&extensions_cbb, extension_id) ||
- !CBB_add_u16(&extensions_cbb, CBS_len(&extension_body)) ||
- !CBB_add_bytes(&extensions_cbb, CBS_data(&extension_body),
- CBS_len(&extension_body))) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- return false;
- }
- }
- if (!CBB_flush(aad.get())) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
- return false;
- }
+ // We assert with |uintptr_t| because the comparison would be UB if they
+ // didn't alias.
+ assert(reinterpret_cast<uintptr_t>(client_hello_outer->extensions) <=
+ reinterpret_cast<uintptr_t>(payload.data()));
+ assert(reinterpret_cast<uintptr_t>(client_hello_outer->extensions +
+ client_hello_outer->extensions_len) >=
+ reinterpret_cast<uintptr_t>(payload.data() + payload.size()));
+ Span<uint8_t> payload_aad = MakeSpan(aad).subspan(
+ payload.data() - client_hello_outer->client_hello, payload.size());
+ OPENSSL_memset(payload_aad.data(), 0, payload_aad.size());
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
// In fuzzer mode, disable encryption to improve coverage. We reserve a short
@@ -336,26 +288,24 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
return false;
}
- if (!out_encoded_client_hello_inner->CopyFrom(payload)) {
+ if (!out->CopyFrom(payload)) {
return false;
}
#else
- // Attempt to decrypt into |out_encoded_client_hello_inner|.
- if (!out_encoded_client_hello_inner->Init(payload.size())) {
+ // Attempt to decrypt into |out|.
+ if (!out->Init(payload.size())) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return false;
}
- size_t encoded_client_hello_inner_len;
- if (!EVP_HPKE_CTX_open(hpke_ctx, out_encoded_client_hello_inner->data(),
- &encoded_client_hello_inner_len,
- out_encoded_client_hello_inner->size(), payload.data(),
- payload.size(), CBB_data(aad.get()),
- CBB_len(aad.get()))) {
+ size_t len;
+ if (!EVP_HPKE_CTX_open(hpke_ctx, out->data(), &len, out->size(),
+ payload.data(), payload.size(), aad.data(),
+ aad.size())) {
*out_is_decrypt_error = true;
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
return false;
}
- out_encoded_client_hello_inner->Shrink(encoded_client_hello_inner_len);
+ out->Shrink(len);
#endif
return true;
}
@@ -397,6 +347,9 @@
static bool is_ipv4_address(Span<const uint8_t> in) {
// See https://url.spec.whatwg.org/#concept-ipv4-parser
+ //
+ // TODO(https://crbug.com/boringssl/275): Revise this, and maybe the spec, per
+ // https://groups.google.com/a/chromium.org/g/blink-dev/c/7QN5nxjwIfM/m/q9dw9MxoAwAJ
uint32_t numbers[4];
size_t num_numbers = 0;
while (!in.empty()) {
@@ -436,7 +389,7 @@
}
bool ssl_is_valid_ech_public_name(Span<const uint8_t> public_name) {
- // See draft-ietf-tls-esni-11, Section 4 and RFC 5890, Section 2.3.1. The
+ // See draft-ietf-tls-esni-13, Section 4 and RFC 5890, Section 2.3.1. The
// public name must be a dot-separated sequence of LDH labels and not begin or
// end with a dot.
auto copy = public_name;
@@ -508,8 +461,8 @@
CBS_len(&public_key) == 0 ||
!CBS_get_u16_length_prefixed(&contents, &cipher_suites) ||
CBS_len(&cipher_suites) == 0 || CBS_len(&cipher_suites) % 4 != 0 ||
- !CBS_get_u16(&contents, &out->maximum_name_length) ||
- !CBS_get_u16_length_prefixed(&contents, &public_name) ||
+ !CBS_get_u8(&contents, &out->maximum_name_length) ||
+ !CBS_get_u8_length_prefixed(&contents, &public_name) ||
CBS_len(&public_name) == 0 ||
!CBS_get_u16_length_prefixed(&contents, &extensions) ||
CBS_len(&contents) != 0) {
@@ -773,15 +726,6 @@
#endif
}
-static size_t compute_extension_length(const EVP_HPKE_AEAD *aead,
- size_t enc_len, size_t in_len) {
- size_t ret = 4; // HpkeSymmetricCipherSuite cipher_suite
- ret++; // uint8 config_id
- ret += 2 + enc_len; // opaque enc<1..2^16-1>
- ret += 2 + in_len + aead_overhead(aead); // opaque payload<1..2^16-1>
- return ret;
-}
-
// random_size returns a random value between |min| and |max|, inclusive.
static size_t random_size(size_t min, size_t max) {
assert(min < max);
@@ -814,38 +758,32 @@
// 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
+ // 5 inner encrypted_client_hello
// 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. For now, arbitrarily
- // estimate maximum_name_length to be between 32 and 100 bytes.
- //
- // TODO(https://crbug.com/boringssl/275): 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
- const size_t overhead = aead_overhead(aead);
- const size_t in_len = random_size(128, 196);
- const size_t extension_len =
- compute_extension_length(aead, sizeof(enc), in_len);
+ // estimate maximum_name_length to be between 32 and 100 bytes. Then round up
+ // to a multiple of 32, to match draft-ietf-tls-esni-13, section 6.1.3.
+ const size_t payload_len =
+ 32 * random_size(128 / 32, 224 / 32) + aead_overhead(aead);
bssl::ScopedCBB cbb;
CBB enc_cbb, payload_cbb;
uint8_t *payload;
- if (!CBB_init(cbb.get(), extension_len) ||
+ if (!CBB_init(cbb.get(), 256) ||
!CBB_add_u16(cbb.get(), kdf_id) ||
!CBB_add_u16(cbb.get(), EVP_HPKE_AEAD_id(aead)) ||
!CBB_add_u8(cbb.get(), config_id) ||
!CBB_add_u16_length_prefixed(cbb.get(), &enc_cbb) ||
!CBB_add_bytes(&enc_cbb, enc, sizeof(enc)) ||
!CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb) ||
- !CBB_add_space(&payload_cbb, &payload, in_len + overhead) ||
- !RAND_bytes(payload, in_len + overhead) ||
- !CBBFinishArray(cbb.get(), &hs->ech_client_bytes)) {
+ !CBB_add_space(&payload_cbb, &payload, payload_len) ||
+ !RAND_bytes(payload, payload_len) ||
+ !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) {
return false;
}
- assert(hs->ech_client_bytes.size() == extension_len);
return true;
}
@@ -856,11 +794,11 @@
}
// Construct ClientHelloInner and EncodedClientHelloInner. See
- // draft-ietf-tls-esni-10, sections 5.1 and 6.1.
+ // draft-ietf-tls-esni-13, sections 5.1 and 6.1.
ScopedCBB cbb, encoded_cbb;
CBB body;
bool needs_psk_binder;
- Array<uint8_t> hello_inner, encoded;
+ Array<uint8_t> hello_inner;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
!CBB_init(encoded_cbb.get(), 256) ||
!ssl_write_client_hello_without_extensions(hs, &body,
@@ -871,10 +809,8 @@
/*empty_session_id=*/true) ||
!ssl_add_clienthello_tlsext(hs, &body, encoded_cbb.get(),
&needs_psk_binder, ssl_client_hello_inner,
- CBB_len(&body),
- /*omit_ech_len=*/0) ||
- !ssl->method->finish_message(ssl, cbb.get(), &hello_inner) ||
- !CBBFinishArray(encoded_cbb.get(), &encoded)) {
+ CBB_len(&body)) ||
+ !ssl->method->finish_message(ssl, cbb.get(), &hello_inner)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
@@ -886,7 +822,10 @@
return false;
}
// Also update the EncodedClientHelloInner.
- auto encoded_binder = MakeSpan(encoded).last(binder_len);
+ auto encoded_binder =
+ MakeSpan(const_cast<uint8_t *>(CBB_data(encoded_cbb.get())),
+ CBB_len(encoded_cbb.get()))
+ .last(binder_len);
auto hello_inner_binder = MakeConstSpan(hello_inner).last(binder_len);
OPENSSL_memcpy(encoded_binder.data(), hello_inner_binder.data(),
binder_len);
@@ -896,72 +835,82 @@
return false;
}
- // Construct ClientHelloOuterAAD. See draft-ietf-tls-esni-10, section 5.2.
- // TODO(https://crbug.com/boringssl/275): This ends up constructing the
- // ClientHelloOuter twice. Revisit this in the next draft, which uses a more
- // forgiving construction.
- const EVP_HPKE_KDF *kdf = EVP_HPKE_CTX_kdf(hs->ech_hpke_ctx.get());
- const EVP_HPKE_AEAD *aead = EVP_HPKE_CTX_aead(hs->ech_hpke_ctx.get());
- const size_t extension_len =
- compute_extension_length(aead, enc.size(), encoded.size());
- bssl::ScopedCBB aad;
- CBB outer_hello;
- CBB enc_cbb;
- if (!CBB_init(aad.get(), 256) ||
- !CBB_add_u16(aad.get(), EVP_HPKE_KDF_id(kdf)) ||
- !CBB_add_u16(aad.get(), EVP_HPKE_AEAD_id(aead)) ||
- !CBB_add_u8(aad.get(), hs->selected_ech_config->config_id) ||
- !CBB_add_u16_length_prefixed(aad.get(), &enc_cbb) ||
- !CBB_add_bytes(&enc_cbb, enc.data(), enc.size()) ||
- !CBB_add_u24_length_prefixed(aad.get(), &outer_hello) ||
- !ssl_write_client_hello_without_extensions(hs, &outer_hello,
- ssl_client_hello_outer,
- /*empty_session_id=*/false) ||
- !ssl_add_clienthello_tlsext(hs, &outer_hello, /*out_encoded=*/nullptr,
- &needs_psk_binder, ssl_client_hello_outer,
- CBB_len(&outer_hello),
- /*omit_ech_len=*/4 + extension_len) ||
- !CBB_flush(aad.get())) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ // Pad the EncodedClientHelloInner. See draft-ietf-tls-esni-13, section 6.1.3.
+ size_t padding_len = 0;
+ size_t maximum_name_length = hs->selected_ech_config->maximum_name_length;
+ if (ssl->hostname) {
+ size_t hostname_len = strlen(ssl->hostname.get());
+ if (hostname_len <= maximum_name_length) {
+ padding_len = maximum_name_length - hostname_len;
+ }
+ } else {
+ // No SNI. Pad up to |maximum_name_length|, including server_name extension
+ // overhead.
+ padding_len = 9 + maximum_name_length;
+ }
+ // Pad the whole thing to a multiple of 32 bytes.
+ padding_len += 31 - ((CBB_len(encoded_cbb.get()) + padding_len - 1) % 32);
+ Array<uint8_t> encoded;
+ if (!CBB_add_zeros(encoded_cbb.get(), padding_len) ||
+ !CBBFinishArray(encoded_cbb.get(), &encoded)) {
return false;
}
- // ClientHelloOuter may not require a PSK binder. Otherwise, we have a
- // circular dependency.
- assert(!needs_psk_binder);
- CBB payload_cbb;
- if (!CBB_init(cbb.get(), extension_len) ||
+ // Encrypt |encoded|. See draft-ietf-tls-esni-13, section 6.1.1. First,
+ // assemble the extension with a placeholder value for ClientHelloOuterAAD.
+ // See draft-ietf-tls-esni-13, section 5.2.
+ const EVP_HPKE_KDF *kdf = EVP_HPKE_CTX_kdf(hs->ech_hpke_ctx.get());
+ const EVP_HPKE_AEAD *aead = EVP_HPKE_CTX_aead(hs->ech_hpke_ctx.get());
+ size_t payload_len = encoded.size() + aead_overhead(aead);
+ CBB enc_cbb, payload_cbb;
+ if (!CBB_init(cbb.get(), 256) ||
!CBB_add_u16(cbb.get(), EVP_HPKE_KDF_id(kdf)) ||
!CBB_add_u16(cbb.get(), EVP_HPKE_AEAD_id(aead)) ||
!CBB_add_u8(cbb.get(), hs->selected_ech_config->config_id) ||
!CBB_add_u16_length_prefixed(cbb.get(), &enc_cbb) ||
!CBB_add_bytes(&enc_cbb, enc.data(), enc.size()) ||
- !CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb)) {
- return false;
- }
-#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
- // In fuzzer mode, the server expects a cleartext payload.
- if (!CBB_add_bytes(&payload_cbb, encoded.data(), encoded.size())) {
- return false;
- }
-#else
- uint8_t *payload;
- size_t payload_len =
- encoded.size() + EVP_AEAD_max_overhead(EVP_HPKE_AEAD_aead(aead));
- if (!CBB_reserve(&payload_cbb, &payload, payload_len) ||
- !EVP_HPKE_CTX_seal(hs->ech_hpke_ctx.get(), payload, &payload_len,
- payload_len, encoded.data(), encoded.size(),
- CBB_data(aad.get()), CBB_len(aad.get())) ||
- !CBB_did_write(&payload_cbb, payload_len)) {
- return false;
- }
-#endif // BORINGSSL_UNSAFE_FUZZER_MODE
- if (!CBBFinishArray(cbb.get(), &hs->ech_client_bytes)) {
+ !CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb) ||
+ !CBB_add_zeros(&payload_cbb, payload_len) ||
+ !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) {
return false;
}
- // The |aad| calculation relies on |extension_length| being correct.
- assert(hs->ech_client_bytes.size() == extension_len);
+ // Construct ClientHelloOuterAAD.
+ // TODO(https://crbug.com/boringssl/275): This ends up constructing the
+ // ClientHelloOuter twice. Instead, reuse |aad| for the ClientHello, now that
+ // draft-12 made the length prefixes match.
+ bssl::ScopedCBB aad;
+ if (!CBB_init(aad.get(), 256) ||
+ !ssl_write_client_hello_without_extensions(hs, aad.get(),
+ ssl_client_hello_outer,
+ /*empty_session_id=*/false) ||
+ !ssl_add_clienthello_tlsext(hs, aad.get(), /*out_encoded=*/nullptr,
+ &needs_psk_binder, ssl_client_hello_outer,
+ CBB_len(aad.get()))) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+
+ // ClientHelloOuter may not require a PSK binder. Otherwise, we have a
+ // circular dependency.
+ assert(!needs_psk_binder);
+
+ // Replace the payload in |hs->ech_client_outer| with the encrypted value.
+ auto payload_span = MakeSpan(hs->ech_client_outer).last(payload_len);
+#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
+ // In fuzzer mode, the server expects a cleartext payload.
+ assert(payload_span.size() == encoded.size());
+ OPENSSL_memcpy(payload_span.data(), encoded.data(), encoded.size());
+#else
+ if (!EVP_HPKE_CTX_seal(hs->ech_hpke_ctx.get(), payload_span.data(),
+ &payload_len, payload_span.size(), encoded.data(),
+ encoded.size(), CBB_data(aad.get()),
+ CBB_len(aad.get())) ||
+ payload_len != payload_span.size()) {
+ return false;
+ }
+#endif // BORINGSSL_UNSAFE_FUZZER_MODE
+
return true;
}
@@ -1041,7 +990,13 @@
return 0;
}
- // See draft-ietf-tls-esni-10, section 4.
+ // The maximum name length is encoded in one byte.
+ if (max_name_len > 0xff) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_LENGTH);
+ return 0;
+ }
+
+ // See draft-ietf-tls-esni-13, section 4.
ScopedCBB cbb;
CBB contents, child;
uint8_t *public_key;
@@ -1062,8 +1017,8 @@
!CBB_add_u16(&child, EVP_HPKE_AES_128_GCM) ||
!CBB_add_u16(&child, EVP_HPKE_HKDF_SHA256) ||
!CBB_add_u16(&child, EVP_HPKE_CHACHA20_POLY1305) ||
- !CBB_add_u16(&contents, max_name_len) ||
- !CBB_add_u16_length_prefixed(&contents, &child) ||
+ !CBB_add_u8(&contents, max_name_len) ||
+ !CBB_add_u8_length_prefixed(&contents, &child) ||
!CBB_add_bytes(&child, public_name_u8.data(), public_name_u8.size()) ||
// TODO(https://crbug.com/boringssl/275): Reserve some GREASE extensions
// and include some.
diff --git a/ssl/extensions.cc b/ssl/extensions.cc
index 6fec445..ba55c93 100644
--- a/ssl/extensions.cc
+++ b/ssl/extensions.cc
@@ -210,16 +210,24 @@
bool ssl_client_hello_init(const SSL *ssl, SSL_CLIENT_HELLO *out,
Span<const uint8_t> body) {
+ CBS cbs = body;
+ if (!ssl_parse_client_hello_with_trailing_data(ssl, &cbs, out) ||
+ CBS_len(&cbs) != 0) {
+ return false;
+ }
+ return true;
+}
+
+bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
+ SSL_CLIENT_HELLO *out) {
OPENSSL_memset(out, 0, sizeof(*out));
out->ssl = const_cast<SSL *>(ssl);
- out->client_hello = body.data();
- out->client_hello_len = body.size();
- CBS client_hello, random, session_id;
- CBS_init(&client_hello, out->client_hello, out->client_hello_len);
- if (!CBS_get_u16(&client_hello, &out->version) ||
- !CBS_get_bytes(&client_hello, &random, SSL3_RANDOM_SIZE) ||
- !CBS_get_u8_length_prefixed(&client_hello, &session_id) ||
+ CBS copy = *cbs;
+ CBS random, session_id;
+ if (!CBS_get_u16(cbs, &out->version) ||
+ !CBS_get_bytes(cbs, &random, SSL3_RANDOM_SIZE) ||
+ !CBS_get_u8_length_prefixed(cbs, &session_id) ||
CBS_len(&session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH) {
return false;
}
@@ -232,16 +240,16 @@
// Skip past DTLS cookie
if (SSL_is_dtls(out->ssl)) {
CBS cookie;
- if (!CBS_get_u8_length_prefixed(&client_hello, &cookie) ||
+ if (!CBS_get_u8_length_prefixed(cbs, &cookie) ||
CBS_len(&cookie) > DTLS1_COOKIE_LENGTH) {
return false;
}
}
CBS cipher_suites, compression_methods;
- if (!CBS_get_u16_length_prefixed(&client_hello, &cipher_suites) ||
+ if (!CBS_get_u16_length_prefixed(cbs, &cipher_suites) ||
CBS_len(&cipher_suites) < 2 || (CBS_len(&cipher_suites) & 1) != 0 ||
- !CBS_get_u8_length_prefixed(&client_hello, &compression_methods) ||
+ !CBS_get_u8_length_prefixed(cbs, &compression_methods) ||
CBS_len(&compression_methods) < 1) {
return false;
}
@@ -253,23 +261,22 @@
// If the ClientHello ends here then it's valid, but doesn't have any
// extensions.
- if (CBS_len(&client_hello) == 0) {
- out->extensions = NULL;
+ if (CBS_len(cbs) == 0) {
+ out->extensions = nullptr;
out->extensions_len = 0;
- return true;
+ } else {
+ // Extract extensions and check it is valid.
+ CBS extensions;
+ if (!CBS_get_u16_length_prefixed(cbs, &extensions) ||
+ !tls1_check_duplicate_extensions(&extensions)) {
+ return false;
+ }
+ out->extensions = CBS_data(&extensions);
+ out->extensions_len = CBS_len(&extensions);
}
- // Extract extensions and check it is valid.
- CBS extensions;
- if (!CBS_get_u16_length_prefixed(&client_hello, &extensions) ||
- !tls1_check_duplicate_extensions(&extensions) ||
- CBS_len(&client_hello) != 0) {
- return false;
- }
-
- out->extensions = CBS_data(&extensions);
- out->extensions_len = CBS_len(&extensions);
-
+ out->client_hello = CBS_data(©);
+ out->client_hello_len = CBS_len(©) - CBS_len(cbs);
return true;
}
@@ -619,20 +626,30 @@
// Encrypted ClientHello (ECH)
//
-// https://tools.ietf.org/html/draft-ietf-tls-esni-10
+// https://tools.ietf.org/html/draft-ietf-tls-esni-13
static bool ext_ech_add_clienthello(const SSL_HANDSHAKE *hs, CBB *out,
CBB *out_compressible,
ssl_client_hello_type_t type) {
- if (type == ssl_client_hello_inner || hs->ech_client_bytes.empty()) {
+ if (type == ssl_client_hello_inner) {
+ if (!CBB_add_u16(out, TLSEXT_TYPE_encrypted_client_hello) ||
+ !CBB_add_u16(out, /* length */ 1) ||
+ !CBB_add_u8(out, ECH_CLIENT_INNER)) {
+ return false;
+ }
+ return true;
+ }
+
+ if (hs->ech_client_outer.empty()) {
return true;
}
CBB ech_body;
if (!CBB_add_u16(out, TLSEXT_TYPE_encrypted_client_hello) ||
!CBB_add_u16_length_prefixed(out, &ech_body) ||
- !CBB_add_bytes(&ech_body, hs->ech_client_bytes.data(),
- hs->ech_client_bytes.size()) ||
+ !CBB_add_u8(&ech_body, ECH_CLIENT_OUTER) ||
+ !CBB_add_bytes(&ech_body, hs->ech_client_outer.data(),
+ hs->ech_client_outer.size()) ||
!CBB_flush(out)) {
return false;
}
@@ -647,8 +664,10 @@
}
// The ECH extension may not be sent in TLS 1.2 ServerHello, only TLS 1.3
- // EncryptedExtension.
- if (ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+ // EncryptedExtensions. It also may not be sent in response to an inner ECH
+ // extension.
+ if (ssl_protocol_version(ssl) < TLS1_3_VERSION ||
+ ssl->s3->ech_status == ssl_ech_accepted) {
*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
return false;
@@ -659,17 +678,7 @@
return false;
}
- // The server may only send retry configs in response to ClientHelloOuter (or
- // ECH GREASE), not ClientHelloInner. The unsolicited extension rule checks
- // this implicitly because the ClientHelloInner has no encrypted_client_hello
- // extension.
- //
- // TODO(https://crbug.com/boringssl/275): If
- // https://github.com/tlswg/draft-ietf-tls-esni/pull/422 is merged, a later
- // draft will fold encrypted_client_hello and ech_is_inner together. Then this
- // assert should become a runtime check.
- assert(ssl->s3->ech_status != ssl_ech_accepted);
- if (hs->selected_ech_config &&
+ if (ssl->s3->ech_status == ssl_ech_rejected &&
!hs->ech_retry_configs.CopyFrom(*contents)) {
*out_alert = SSL_AD_INTERNAL_ERROR;
return false;
@@ -680,10 +689,23 @@
static bool ext_ech_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
- if (contents != nullptr) {
- hs->ech_present = true;
+ if (contents == nullptr) {
return true;
}
+
+ uint8_t type;
+ if (!CBS_get_u8(contents, &type)) {
+ return false;
+ }
+ if (type == ECH_CLIENT_OUTER) {
+ // Outer ECH extensions are handled outside the callback.
+ return true;
+ }
+ if (type != ECH_CLIENT_INNER || CBS_len(contents) != 0) {
+ return false;
+ }
+
+ hs->ech_is_inner = true;
return true;
}
@@ -715,32 +737,6 @@
return CBB_flush(out);
}
-static bool ext_ech_is_inner_add_clienthello(const SSL_HANDSHAKE *hs, CBB *out,
- CBB *out_compressible,
- ssl_client_hello_type_t type) {
- if (type == ssl_client_hello_inner) {
- if (!CBB_add_u16(out, TLSEXT_TYPE_ech_is_inner) ||
- !CBB_add_u16(out, 0 /* empty extension */)) {
- return false;
- }
- }
- return true;
-}
-
-static bool ext_ech_is_inner_parse_clienthello(SSL_HANDSHAKE *hs,
- uint8_t *out_alert,
- CBS *contents) {
- if (contents == nullptr) {
- return true;
- }
- if (CBS_len(contents) > 0) {
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- return false;
- }
- hs->ech_is_inner_present = true;
- return true;
-}
-
// Renegotiation indication.
//
@@ -1942,13 +1938,10 @@
const SSL *const ssl = hs->ssl;
if (hs->max_version < TLS1_3_VERSION || ssl->session == nullptr ||
ssl_session_protocol_version(ssl->session.get()) < TLS1_3_VERSION ||
- // The ClientHelloOuter cannot include the PSK extension.
- //
- // TODO(https://crbug.com/boringssl/275): draft-ietf-tls-esni-10 mandates
- // this, but it risks breaking the ClientHelloOuter flow on 0-RTT reject.
- // Later drafts will recommend including a placeholder one, at which point
- // we will need to synthesize a ticket. See
- // https://github.com/tlswg/draft-ietf-tls-esni/issues/408
+ // TODO(https://crbug.com/boringssl/275): Should we synthesize a
+ // placeholder PSK, at least when we offer early data? Otherwise
+ // ClientHelloOuter will contain an early_data extension without a
+ // pre_shared_key extension and potentially break the recovery flow.
type == ssl_client_hello_outer) {
return false;
}
@@ -3110,13 +3103,6 @@
ext_ech_add_serverhello,
},
{
- TLSEXT_TYPE_ech_is_inner,
- ext_ech_is_inner_add_clienthello,
- forbid_parse_serverhello,
- ext_ech_is_inner_parse_clienthello,
- dont_add_serverhello,
- },
- {
TLSEXT_TYPE_extended_master_secret,
ext_ems_add_clienthello,
ext_ems_parse_serverhello,
@@ -3401,34 +3387,6 @@
}
}
- // Pad the server name. See draft-ietf-tls-esni-10, section 6.1.2.
- // TODO(https://crbug.com/boringssl/275): Ideally we'd pad the whole thing to
- // reduce the output range. See
- // https://github.com/tlswg/draft-ietf-tls-esni/issues/433
- size_t padding_len = 0;
- size_t maximum_name_length = hs->selected_ech_config->maximum_name_length;
- if (ssl->hostname) {
- size_t hostname_len = strlen(ssl->hostname.get());
- if (hostname_len <= maximum_name_length) {
- padding_len = maximum_name_length - hostname_len;
- } else {
- // If the server underestimated the maximum size, pad to a multiple of 32.
- padding_len = 31 - (hostname_len - 1) % 32;
- // If the input is close to |maximum_name_length|, pad to the next
- // multiple for at least 32 bytes of length ambiguity.
- if (hostname_len + padding_len < maximum_name_length + 32) {
- padding_len += 32;
- }
- }
- } else {
- // No SNI. Pad up to |maximum_name_length|, including server_name extension
- // overhead.
- padding_len = 9 + maximum_name_length;
- }
- if (!add_padding_extension(&extensions, TLSEXT_TYPE_padding, padding_len)) {
- return false;
- }
-
// Uncompressed extensions are encoded as-is.
if (!CBB_add_bytes(&extensions_encoded, CBB_data(&extensions),
CBB_len(&extensions))) {
@@ -3470,8 +3428,8 @@
bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, CBB *out_encoded,
bool *out_needs_psk_binder,
- ssl_client_hello_type_t type, size_t header_len,
- size_t omit_ech_len) {
+ ssl_client_hello_type_t type,
+ size_t header_len) {
*out_needs_psk_binder = false;
if (type == ssl_client_hello_inner) {
@@ -3504,20 +3462,14 @@
size_t i = hs->extension_permutation.empty()
? unpermuted
: hs->extension_permutation[unpermuted];
- size_t bytes_written;
- if (omit_ech_len != 0 &&
- kExtensions[i].value == TLSEXT_TYPE_encrypted_client_hello) {
- bytes_written = omit_ech_len;
- } else {
- const size_t len_before = CBB_len(&extensions);
- if (!kExtensions[i].add_clienthello(hs, &extensions, &extensions, type)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_ADDING_EXTENSION);
- ERR_add_error_dataf("extension %u", (unsigned)kExtensions[i].value);
- return false;
- }
-
- bytes_written = CBB_len(&extensions) - len_before;
+ const size_t len_before = CBB_len(&extensions);
+ if (!kExtensions[i].add_clienthello(hs, &extensions, &extensions, type)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_ADDING_EXTENSION);
+ ERR_add_error_dataf("extension %u", (unsigned)kExtensions[i].value);
+ return false;
}
+
+ const size_t bytes_written = CBB_len(&extensions) - len_before;
if (bytes_written != 0) {
hs->extensions.sent |= (1u << i);
}
@@ -3541,8 +3493,8 @@
size_t psk_extension_len = ext_pre_shared_key_clienthello_length(hs, type);
if (!SSL_is_dtls(ssl) && !ssl->quic_method &&
!ssl->s3->used_hello_retry_request) {
- header_len += SSL3_HM_HEADER_LENGTH + 2 + CBB_len(&extensions) +
- omit_ech_len + psk_extension_len;
+ header_len +=
+ SSL3_HM_HEADER_LENGTH + 2 + CBB_len(&extensions) + psk_extension_len;
size_t padding_len = 0;
// The final extension must be non-empty. WebSphere Application
diff --git a/ssl/handshake.cc b/ssl/handshake.cc
index 3608888..fc85e21 100644
--- a/ssl/handshake.cc
+++ b/ssl/handshake.cc
@@ -126,8 +126,7 @@
SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg)
: ssl(ssl_arg),
- ech_present(false),
- ech_is_inner_present(false),
+ ech_is_inner(false),
ech_authenticated_reject(false),
scts_requested(false),
handshake_finalized(false),
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc
index ee9045e..17b41e0 100644
--- a/ssl/handshake_client.cc
+++ b/ssl/handshake_client.cc
@@ -333,8 +333,7 @@
!ssl_write_client_hello_without_extensions(hs, &body, type,
/*empty_session_id*/ false) ||
!ssl_add_clienthello_tlsext(hs, &body, /*out_encoded=*/nullptr,
- &needs_psk_binder, type, CBB_len(&body),
- /*omit_ech_len=*/0) ||
+ &needs_psk_binder, type, CBB_len(&body)) ||
!ssl->method->finish_message(ssl, cbb.get(), &msg)) {
return false;
}
@@ -434,7 +433,7 @@
}
void ssl_done_writing_client_hello(SSL_HANDSHAKE *hs) {
- hs->ech_client_bytes.Reset();
+ hs->ech_client_outer.Reset();
hs->cookie.Reset();
hs->key_share_bytes.Reset();
}
@@ -653,6 +652,7 @@
*out_alert = SSL_AD_UNEXPECTED_MESSAGE;
return false;
}
+ out->raw = msg.raw;
CBS body = msg.body;
if (!CBS_get_u16(&body, &out->legacy_version) ||
!CBS_get_bytes(&body, &out->random, SSL3_RANDOM_SIZE) ||
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc
index 29fc3a4..fdf9511 100644
--- a/ssl/handshake_server.cc
+++ b/ssl/handshake_server.cc
@@ -504,6 +504,91 @@
return true;
}
+static bool decrypt_ech(SSL_HANDSHAKE *hs, uint8_t *out_alert,
+ const SSL_CLIENT_HELLO *client_hello) {
+ SSL *const ssl = hs->ssl;
+ CBS body;
+ if (!ssl_client_hello_get_extension(client_hello, &body,
+ TLSEXT_TYPE_encrypted_client_hello)) {
+ return true;
+ }
+ uint8_t type;
+ if (!CBS_get_u8(&body, &type)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return false;
+ }
+ if (type != ECH_CLIENT_OUTER) {
+ return true;
+ }
+ // This is a ClientHelloOuter ECH extension. Attempt to decrypt it.
+ uint8_t config_id;
+ uint16_t kdf_id, aead_id;
+ CBS enc, payload;
+ if (!CBS_get_u16(&body, &kdf_id) || //
+ !CBS_get_u16(&body, &aead_id) || //
+ !CBS_get_u8(&body, &config_id) ||
+ !CBS_get_u16_length_prefixed(&body, &enc) ||
+ !CBS_get_u16_length_prefixed(&body, &payload) || //
+ CBS_len(&body) != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return false;
+ }
+
+ {
+ MutexReadLock lock(&ssl->ctx->lock);
+ hs->ech_keys = UpRef(ssl->ctx->ech_keys);
+ }
+
+ if (!hs->ech_keys) {
+ ssl->s3->ech_status = ssl_ech_rejected;
+ return true;
+ }
+
+ for (const auto &config : hs->ech_keys->configs) {
+ hs->ech_hpke_ctx.Reset();
+ if (config_id != config->ech_config().config_id ||
+ !config->SetupContext(hs->ech_hpke_ctx.get(), kdf_id, aead_id, enc)) {
+ // Ignore the error and try another ECHConfig.
+ ERR_clear_error();
+ continue;
+ }
+ Array<uint8_t> encoded_client_hello_inner;
+ bool is_decrypt_error;
+ if (!ssl_client_hello_decrypt(hs->ech_hpke_ctx.get(),
+ &encoded_client_hello_inner,
+ &is_decrypt_error, client_hello, payload)) {
+ if (is_decrypt_error) {
+ // Ignore the error and try another ECHConfig.
+ ERR_clear_error();
+ continue;
+ }
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
+ return false;
+ }
+
+ // Recover the ClientHelloInner from the EncodedClientHelloInner.
+ bssl::Array<uint8_t> client_hello_inner;
+ if (!ssl_decode_client_hello_inner(ssl, out_alert, &client_hello_inner,
+ encoded_client_hello_inner,
+ client_hello)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return false;
+ }
+ hs->ech_client_hello_buf = std::move(client_hello_inner);
+ hs->ech_config_id = config_id;
+ ssl->s3->ech_status = ssl_ech_accepted;
+ return true;
+ }
+
+ // If we did not accept ECH, proceed with the ClientHelloOuter. Note this
+ // could be key mismatch or ECH GREASE, so we must complete the handshake
+ // as usual, except EncryptedExtensions will contain retry configs.
+ ssl->s3->ech_status = ssl_ech_rejected;
+ return true;
+}
+
static bool extract_sni(SSL_HANDSHAKE *hs, uint8_t *out_alert,
const SSL_CLIENT_HELLO *client_hello) {
SSL *const ssl = hs->ssl;
@@ -583,98 +668,19 @@
return ssl_hs_handoff;
}
- // If the ClientHello contains an encrypted_client_hello extension (and no
- // ech_is_inner extension), act as a client-facing server and attempt to
- // decrypt the ClientHelloInner.
- CBS ech_body;
- if (ssl_client_hello_get_extension(&client_hello, &ech_body,
- TLSEXT_TYPE_encrypted_client_hello)) {
- CBS unused;
- if (ssl_client_hello_get_extension(&client_hello, &unused,
- TLSEXT_TYPE_ech_is_inner)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
-
- // Parse a ClientECH out of the extension body.
- uint8_t config_id;
- uint16_t kdf_id, aead_id;
- CBS enc, payload;
- if (!CBS_get_u16(&ech_body, &kdf_id) || //
- !CBS_get_u16(&ech_body, &aead_id) ||
- !CBS_get_u8(&ech_body, &config_id) ||
- !CBS_get_u16_length_prefixed(&ech_body, &enc) ||
- !CBS_get_u16_length_prefixed(&ech_body, &payload) ||
- CBS_len(&ech_body) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
- return ssl_hs_error;
- }
-
- {
- MutexReadLock lock(&ssl->ctx->lock);
- hs->ech_keys = UpRef(ssl->ctx->ech_keys);
- }
-
- if (hs->ech_keys) {
- for (const auto &config : hs->ech_keys->configs) {
- hs->ech_hpke_ctx.Reset();
- if (config_id != config->ech_config().config_id ||
- !config->SetupContext(hs->ech_hpke_ctx.get(), kdf_id, aead_id,
- enc)) {
- // Ignore the error and try another ECHConfig.
- ERR_clear_error();
- continue;
- }
- Array<uint8_t> encoded_client_hello_inner;
- bool is_decrypt_error;
- if (!ssl_client_hello_decrypt(hs->ech_hpke_ctx.get(),
- &encoded_client_hello_inner,
- &is_decrypt_error, &client_hello, kdf_id,
- aead_id, config_id, enc, payload)) {
- if (is_decrypt_error) {
- // Ignore the error and try another ECHConfig.
- ERR_clear_error();
- continue;
- }
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
- return ssl_hs_error;
- }
-
- // Recover the ClientHelloInner from the EncodedClientHelloInner.
- uint8_t alert = SSL_AD_DECODE_ERROR;
- bssl::Array<uint8_t> client_hello_inner;
- if (!ssl_decode_client_hello_inner(ssl, &alert, &client_hello_inner,
- encoded_client_hello_inner,
- &client_hello)) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- return ssl_hs_error;
- }
- hs->ech_client_hello_buf = std::move(client_hello_inner);
-
- // Load the ClientHelloInner into |client_hello|.
- if (!hs->GetClientHello(&msg, &client_hello)) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return ssl_hs_error;
- }
-
- hs->ech_config_id = config_id;
- ssl->s3->ech_status = ssl_ech_accepted;
- break;
- }
- }
-
- // If we did not accept ECH, proceed with the ClientHelloOuter. Note this
- // could be key mismatch or ECH GREASE, so we most complete the handshake
- // as usual, except EncryptedExtensions will contain retry configs.
- if (ssl->s3->ech_status != ssl_ech_accepted) {
- ssl->s3->ech_status = ssl_ech_rejected;
- }
+ uint8_t alert = SSL_AD_DECODE_ERROR;
+ if (!decrypt_ech(hs, &alert, &client_hello)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
}
- uint8_t alert = SSL_AD_DECODE_ERROR;
+ // ECH may have changed which ClientHello we process. Update |msg| and
+ // |client_hello| in case.
+ if (!hs->GetClientHello(&msg, &client_hello)) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return ssl_hs_error;
+ }
+
if (!extract_sni(hs, &alert, &client_hello)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
@@ -751,12 +757,6 @@
return ssl_hs_error;
}
- if (hs->ech_present && hs->ech_is_inner_present) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
- return ssl_hs_error;
- }
-
hs->state = state12_select_certificate;
return ssl_hs_ok;
}
diff --git a/ssl/internal.h b/ssl/internal.h
index 6b7528c..ab23d29 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1451,7 +1451,7 @@
Span<const uint8_t> public_name;
Span<const uint8_t> cipher_suites;
uint16_t kem_id = 0;
- uint16_t maximum_name_length = 0;
+ uint8_t maximum_name_length = 0;
uint8_t config_id = 0;
};
@@ -1488,6 +1488,10 @@
ssl_client_hello_outer,
};
+// ECH_CLIENT_* are types for the ClientHello encrypted_client_hello extension.
+#define ECH_CLIENT_OUTER 0
+#define ECH_CLIENT_INNER 1
+
// ssl_decode_client_hello_inner recovers the full ClientHelloInner from the
// EncodedClientHelloInner |encoded_client_hello_inner| by replacing its
// outer_extensions extension with the referenced extensions from the
@@ -1499,18 +1503,13 @@
Span<const uint8_t> encoded_client_hello_inner,
const SSL_CLIENT_HELLO *client_hello_outer);
-// ssl_client_hello_decrypt attempts to decrypt the given |payload| into
-// |out_encoded_client_hello_inner|. The decrypted value should be an
-// EncodedClientHelloInner. It returns false if any fatal errors occur and true
-// otherwise, regardless of whether the decrypt was successful. It sets
-// |out_encoded_client_hello_inner| to true if the decryption fails, and false
-// otherwise.
-bool ssl_client_hello_decrypt(EVP_HPKE_CTX *hpke_ctx,
- Array<uint8_t> *out_encoded_client_hello_inner,
+// ssl_client_hello_decrypt attempts to decrypt the |payload| and writes the
+// result to |*out|. |payload| must point into |client_hello_outer|. It returns
+// true on success and false on error. On error, it sets |*out_is_decrypt_error|
+// to whether the failure was due to a bad ciphertext.
+bool ssl_client_hello_decrypt(EVP_HPKE_CTX *hpke_ctx, Array<uint8_t> *out,
bool *out_is_decrypt_error,
const SSL_CLIENT_HELLO *client_hello_outer,
- uint16_t kdf_id, uint16_t aead_id,
- uint8_t config_id, Span<const uint8_t> enc,
Span<const uint8_t> payload);
#define ECH_CONFIRMATION_SIGNAL_LEN 8
@@ -1520,13 +1519,14 @@
size_t ssl_ech_confirmation_signal_hello_offset(const SSL *ssl);
// ssl_ech_accept_confirmation computes the server's ECH acceptance signal,
-// writing it to |out|. The signal is computed by concatenating |transcript|
-// with |server_hello|. This function handles the fact that eight bytes of
-// |server_hello| need to be replaced with zeros before hashing. It returns true
-// on success, and false on failure.
+// writing it to |out|. The transcript portion is the concatenation of
+// |transcript| with |msg|. The |ECH_CONFIRMATION_SIGNAL_LEN| bytes from
+// |offset| in |msg| are replaced with zeros before hashing. This function
+// returns true on success, and false on failure.
bool ssl_ech_accept_confirmation(const SSL_HANDSHAKE *hs, Span<uint8_t> out,
- const SSLTranscript &transcript,
- Span<const uint8_t> server_hello);
+ Span<const uint8_t> client_random,
+ const SSLTranscript &transcript, bool is_hrr,
+ Span<const uint8_t> msg, size_t offset);
// ssl_is_valid_ech_public_name returns true if |public_name| is a valid ECH
// public name and false otherwise. It is exported for testing.
@@ -1832,8 +1832,9 @@
// cookie is the value of the cookie received from the server, if any.
Array<uint8_t> cookie;
- // ech_client_bytes contains the ECH extension to send in the ClientHello.
- Array<uint8_t> ech_client_bytes;
+ // ech_client_outer contains the outer ECH extension to send in the
+ // ClientHello, excluding the header and type byte.
+ Array<uint8_t> ech_client_outer;
// ech_retry_configs, on the client, contains the retry configs from the
// server as a serialized ECHConfigList.
@@ -1941,13 +1942,9 @@
// influence the handshake on match.
UniquePtr<SSL_HANDSHAKE_HINTS> hints;
- // ech_present, on the server, indicates whether the ClientHello contained an
- // encrypted_client_hello extension.
- bool ech_present : 1;
-
- // ech_is_inner_present, on the server, indicates whether the ClientHello
- // contained an ech_is_inner extension.
- bool ech_is_inner_present : 1;
+ // ech_is_inner, on the server, indicates whether the ClientHello contained an
+ // inner ECH extension.
+ bool ech_is_inner : 1;
// ech_authenticated_reject, on the client, indicates whether an ECH rejection
// handshake has been authenticated.
@@ -2166,6 +2163,7 @@
bool ssl_add_client_hello(SSL_HANDSHAKE *hs);
struct ParsedServerHello {
+ CBS raw;
uint16_t legacy_version = 0;
CBS random;
CBS session_id;
@@ -2278,6 +2276,9 @@
OPENSSL_EXPORT bool ssl_client_hello_init(const SSL *ssl, SSL_CLIENT_HELLO *out,
Span<const uint8_t> body);
+bool ssl_parse_client_hello_with_trailing_data(const SSL *ssl, CBS *cbs,
+ SSL_CLIENT_HELLO *out);
+
bool ssl_client_hello_get_extension(const SSL_CLIENT_HELLO *client_hello,
CBS *out, uint16_t extension_type);
@@ -3315,14 +3316,10 @@
// ClientHello extension was the pre_shared_key extension and needs a PSK binder
// filled in. The caller should then update |out| and, if applicable,
// |out_encoded| with the binder after completing the whole message.
-//
-// If |omit_ech_len| is non-zero, the ECH extension is omitted, but padding is
-// computed as if there were an extension of length |omit_ech_len|. This is used
-// to compute ClientHelloOuterAAD.
bool ssl_add_clienthello_tlsext(SSL_HANDSHAKE *hs, CBB *out, CBB *out_encoded,
bool *out_needs_psk_binder,
- ssl_client_hello_type_t type, size_t header_len,
- size_t omit_ech_len);
+ ssl_client_hello_type_t type,
+ size_t header_len);
bool ssl_add_serverhello_tlsext(SSL_HANDSHAKE *hs, CBB *out);
bool ssl_parse_clienthello_tlsext(SSL_HANDSHAKE *hs,
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 76f88c7..60d820b 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -1675,8 +1675,8 @@
return false;
}
}
- if (!CBB_add_u16(&contents, params.max_name_len) ||
- !CBB_add_u16_length_prefixed(&contents, &child) ||
+ if (!CBB_add_u8(&contents, params.max_name_len) ||
+ !CBB_add_u8_length_prefixed(&contents, &child) ||
!CBB_add_bytes(
&child, reinterpret_cast<const uint8_t *>(params.public_name.data()),
params.public_name.size()) ||
@@ -1735,9 +1735,9 @@
static const uint8_t kECHConfig[] = {
// version
- 0xfe, 0x0a,
+ 0xfe, 0x0d,
// length
- 0x00, 0x43,
+ 0x00, 0x41,
// contents.config_id
0x01,
// contents.kem_id
@@ -1749,10 +1749,10 @@
// contents.cipher_suites
0x00, 0x08, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x03,
// contents.maximum_name_length
- 0x00, 0x10,
+ 0x10,
// contents.public_name
- 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61,
- 0x6d, 0x70, 0x6c, 0x65,
+ 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, 0x6d,
+ 0x70, 0x6c, 0x65,
// contents.extensions
0x00, 0x00};
uint8_t *ech_config;
@@ -2069,20 +2069,26 @@
EXPECT_EQ(client_hello_len, client_hello_len_baseline);
EXPECT_EQ(ech_len, ech_len_baseline);
- size_t client_hello_len_129, ech_len_129;
- ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len_129, &ech_len_129, 128,
- std::string(129, 'a').c_str()));
- // The padding calculation should not pad beyond the maximum.
- EXPECT_GT(ech_len_129, ech_len_baseline);
+ // Name lengths above the maximum do not get named-based padding, but the
+ // overall input is padded to a multiple of 32.
+ size_t client_hello_len_baseline2, ech_len_baseline2;
+ ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len_baseline2,
+ &ech_len_baseline2, 128,
+ std::string(128 + 32, 'a').c_str()));
+ EXPECT_EQ(ech_len_baseline2, ech_len_baseline + 32);
+ // The ClientHello lengths may match if we are still under the threshold for
+ // padding extension.
+ EXPECT_GE(client_hello_len_baseline2, client_hello_len_baseline);
- // If the SNI exceeds the maximum name length, we apply some generic padding,
- // so close name lengths still match.
- for (size_t name_len : {129, 130, 131, 132}) {
+ for (size_t name_len = 128 + 1; name_len < 128 + 32; name_len++) {
SCOPED_TRACE(name_len);
ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len, &ech_len, 128,
std::string(name_len, 'a').c_str()));
- EXPECT_EQ(client_hello_len, client_hello_len_129);
- EXPECT_EQ(ech_len, ech_len_129);
+ EXPECT_TRUE(ech_len == ech_len_baseline || ech_len == ech_len_baseline2)
+ << ech_len;
+ EXPECT_TRUE(client_hello_len == client_hello_len_baseline ||
+ client_hello_len == client_hello_len_baseline2)
+ << client_hello_len;
}
}
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index d43e7d1..bf6a3d1 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -128,8 +128,7 @@
extensionChannelID uint16 = 30032 // not IANA assigned
extensionDelegatedCredentials uint16 = 0x22 // draft-ietf-tls-subcerts-06
extensionDuplicate uint16 = 0xffff // not IANA assigned
- extensionEncryptedClientHello uint16 = 0xfe0a // not IANA assigned
- extensionECHIsInner uint16 = 0xda09 // not IANA assigned
+ extensionEncryptedClientHello uint16 = 0xfe0d // not IANA assigned
extensionECHOuterExtensions uint16 = 0xfd00 // not IANA assigned
)
@@ -243,6 +242,9 @@
keyUpdateRequested = 1
)
+// draft-ietf-tls-esni-13, sections 7.2 and 7.2.1.
+const echAcceptConfirmationLength = 8
+
// ConnectionState records basic TLS details about the connection.
type ConnectionState struct {
Version uint16 // TLS version used by the connection (e.g. VersionTLS12)
@@ -869,40 +871,53 @@
// retry configs.
SendECHRetryConfigs []byte
- // SendECHRetryConfigsInTLS12ServerHello, if true, causes the ECH server to
- // send retry configs in the TLS 1.2 ServerHello.
- SendECHRetryConfigsInTLS12ServerHello bool
+ // AlwaysSendECHRetryConfigs, if true, causes the ECH server to send retry
+ // configs unconditionally, including in the TLS 1.2 ServerHello.
+ AlwaysSendECHRetryConfigs bool
- // SendInvalidECHIsInner, if not empty, causes the client to send the
- // specified byte string in the ech_is_inner extension.
- SendInvalidECHIsInner []byte
+ // AlwaysSendECHHelloRetryRequest, if true, causes the ECH server to send
+ // the ECH HelloRetryRequest extension unconditionally.
+ AlwaysSendECHHelloRetryRequest bool
- // OmitECHIsInner, if true, causes the client to omit the ech_is_inner
+ // SendInvalidECHInner, if not empty, causes the client to send the
+ // specified byte string after the type field in ClientHelloInner
+ // encrypted_client_hello extension.
+ SendInvalidECHInner []byte
+
+ // OmitECHInner, if true, causes the client to omit the encrypted_client_hello
// extension on the ClientHelloInner message.
- OmitECHIsInner bool
+ OmitECHInner bool
- // OmitSecondECHIsInner, if true, causes the client to omit the ech_is_inner
- // extension on the second ClientHelloInner message.
- OmitSecondECHIsInner bool
+ // OmitSecondECHInner, if true, causes the client to omit the
+ // encrypted_client_hello extension on the second ClientHelloInner message.
+ OmitSecondECHInner bool
- // AlwaysSendECHIsInner, if true, causes the client to send the
- // ech_is_inner extension on all ClientHello messages. The server is then
- // expected to unconditionally confirm the extension when negotiating
- // TLS 1.3 or later.
- AlwaysSendECHIsInner bool
+ // OmitServerHelloECHConfirmation, if true, causes the server to omit the
+ // ECH confirmation in the ServerHello.
+ OmitServerHelloECHConfirmation bool
+
+ // AlwaysSendECHInner, if true, causes the client to send an inner
+ // encrypted_client_hello extension on all ClientHello messages. The server
+ // is then expected to unconditionally confirm the extension when
+ // negotiating TLS 1.3 or later.
+ AlwaysSendECHInner bool
// TruncateClientECHEnc, if true, causes the client to send a shortened
// ClientECH.enc value in its encrypted_client_hello extension.
TruncateClientECHEnc bool
+ // ClientECHPadding is the number of bytes of padding to add to the client
+ // ECH payload.
+ ClientECHPadding int
+
+ // BadClientECHPadding, if true, causes the client ECH padding to contain a
+ // non-zero byte.
+ BadClientECHPadding bool
+
// OfferSessionInClientHelloOuter, if true, causes the client to offer
// sessions in ClientHelloOuter.
OfferSessionInClientHelloOuter bool
- // FirstExtensionInClientHelloOuter, if non-zero, causes the client to place
- // the specified extension first in ClientHelloOuter.
- FirstExtensionInClientHelloOuter uint16
-
// OnlyCompressSecondClientHelloInner, if true, causes the client to
// only apply outer_extensions to the second ClientHello.
OnlyCompressSecondClientHelloInner bool
@@ -941,6 +956,11 @@
// will require to be omitted in ech_outer_extensions.
ExpectECHUncompressedExtensions []uint16
+ // ECHOuterExtensionOrder, if not nil, is an extension order to apply to
+ // ClientHelloOuter, instead of ordering the |ECHOuterExtensions| to match
+ // in both ClientHellos.
+ ECHOuterExtensionOrder []uint16
+
// UseInnerSessionWithClientHelloOuter, if true, causes the server to
// handshake with ClientHelloOuter, but resume the session from
// ClientHelloInner.
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 424b206..5d04994 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -26,19 +26,18 @@
const echBadPayloadByte = 0xff
type clientHandshakeState struct {
- c *Conn
- serverHello *serverHelloMsg
- hello *clientHelloMsg
- innerHello *clientHelloMsg
- echHPKEContext *hpke.Context
- suite *cipherSuite
- finishedHash finishedHash
- innerFinishedHash finishedHash
- keyShares map[CurveID]ecdhCurve
- masterSecret []byte
- session *ClientSessionState
- finishedBytes []byte
- peerPublicKey crypto.PublicKey
+ c *Conn
+ serverHello *serverHelloMsg
+ hello *clientHelloMsg
+ innerHello *clientHelloMsg
+ echHPKEContext *hpke.Context
+ suite *cipherSuite
+ finishedHash finishedHash
+ keyShares map[CurveID]ecdhCurve
+ masterSecret []byte
+ session *ClientSessionState
+ finishedBytes []byte
+ peerPublicKey crypto.PublicKey
}
func mapClientHelloVersion(vers uint16, isDTLS bool) uint16 {
@@ -340,10 +339,6 @@
hs.finishedHash = newFinishedHash(c.wireVersion, c.isDTLS, hs.suite)
hs.finishedHash.WriteHandshake(hs.hello.marshal(), hs.c.sendHandshakeSeq-1)
- if hs.innerHello != nil {
- hs.innerFinishedHash = newFinishedHash(c.wireVersion, c.isDTLS, hs.suite)
- hs.innerFinishedHash.WriteHandshake(hs.innerHello.marshal(), hs.c.sendHandshakeSeq-1)
- }
if c.vers >= VersionTLS13 {
if err := hs.doTLS13Handshake(msg); err != nil {
@@ -533,9 +528,6 @@
// list of prefix extensions. The marshal function will try these
// extensions before any others, followed by any remaining extensions in
// the default order.
- if innerHello != nil && c.config.Bugs.FirstExtensionInClientHelloOuter != 0 {
- hello.prefixExtensions = append(hello.prefixExtensions, c.config.Bugs.FirstExtensionInClientHelloOuter)
- }
if c.config.Bugs.PSKBinderFirst && !c.config.Bugs.OnlyCorruptSecondPSKBinder {
hello.prefixExtensions = append(hello.prefixExtensions, extensionPreSharedKey)
}
@@ -543,8 +535,24 @@
hello.prefixExtensions = append(hello.prefixExtensions, extensionALPN)
hello.prefixExtensions = append(hello.prefixExtensions, extensionNextProtoNeg)
}
- if isInner && len(c.config.ECHOuterExtensions) > 0 && !c.config.Bugs.OnlyCompressSecondClientHelloInner {
- applyECHOuterExtensions(hello, c.config.ECHOuterExtensions)
+
+ // Configure ech_outer_extensions.
+ if isInner {
+ hello.outerExtensions = c.config.ECHOuterExtensions
+ // If |OnlyCompressSecondClientHelloInner| is set, we still configure
+ // |hello.outerExtensions| for ordering, so that we do not introduce an
+ // unsolicited change across HelloRetryRequest.
+ hello.reorderOuterExtensionsWithoutCompressing = c.config.Bugs.OnlyCompressSecondClientHelloInner
+ } else {
+ // Compressed extensions must appear in the same relative order between
+ // ClientHelloInner and ClientHelloOuter. For simplicity, we default to
+ // forcing their order to match, but the caller can override this with
+ // either valid or invalid explicit orders.
+ if c.config.Bugs.ECHOuterExtensionOrder != nil {
+ hello.prefixExtensions = append(hello.prefixExtensions, c.config.Bugs.ECHOuterExtensionOrder...)
+ } else {
+ hello.prefixExtensions = append(hello.prefixExtensions, c.config.ECHOuterExtensions...)
+ }
}
if maxVersion >= VersionTLS13 {
@@ -816,9 +824,9 @@
hello.hasEarlyData = innerHello.hasEarlyData
}
- if (isInner && !c.config.Bugs.OmitECHIsInner) || c.config.Bugs.AlwaysSendECHIsInner {
- hello.echIsInner = true
- hello.invalidECHIsInner = c.config.Bugs.SendInvalidECHIsInner
+ if (isInner && !c.config.Bugs.OmitECHInner) || c.config.Bugs.AlwaysSendECHInner {
+ hello.echInner = true
+ hello.invalidECHInner = c.config.Bugs.SendInvalidECHInner
}
if innerHello != nil {
@@ -827,9 +835,9 @@
}
if c.config.Bugs.CorruptEncryptedClientHello {
if c.config.Bugs.NullAllCiphers {
- hello.clientECH.payload = []byte{echBadPayloadByte}
+ hello.echOuter.payload = []byte{echBadPayloadByte}
} else {
- hello.clientECH.payload[0] ^= 1
+ hello.echOuter.payload[0] ^= 1
}
}
}
@@ -878,27 +886,31 @@
enc = enc[:1]
}
- aad := newByteBuilder()
- aad.addU16(hs.echHPKEContext.KDF())
- aad.addU16(hs.echHPKEContext.AEAD())
- aad.addU8(configID)
- aad.addU16LengthPrefixed().addBytes(enc)
- hello.marshalForOuterAAD(aad.addU24LengthPrefixed())
-
encodedInner := innerHello.marshalForEncodedInner()
- payload := hs.echHPKEContext.Seal(encodedInner, aad.finish())
-
- if c.config.Bugs.NullAllCiphers {
- payload = encodedInner
+ padding := make([]byte, c.config.Bugs.ClientECHPadding)
+ if c.config.Bugs.BadClientECHPadding {
+ padding[0] = 1
}
+ encodedInner = append(encodedInner, padding...)
- // Place the ECH extension in the outer CH.
- hello.clientECH = &clientECH{
+ // Encode ClientHelloOuter with a placeholder payload string.
+ payloadLength := len(encodedInner)
+ if !c.config.Bugs.NullAllCiphers {
+ payloadLength += hs.echHPKEContext.Overhead()
+ }
+ hello.echOuter = &echClientOuter{
kdfID: hs.echHPKEContext.KDF(),
aeadID: hs.echHPKEContext.AEAD(),
configID: configID,
enc: enc,
- payload: payload,
+ payload: make([]byte, payloadLength),
+ }
+ aad := hello.marshal()[4:] // Remove message header
+
+ hello.raw = nil
+ hello.echOuter.payload = hs.echHPKEContext.Seal(encodedInner, aad)
+ if c.config.Bugs.NullAllCiphers {
+ hello.echOuter.payload = encodedInner
}
if c.config.Bugs.RecordClientHelloInner != nil {
@@ -913,24 +925,73 @@
return nil
}
+func (hs *clientHandshakeState) checkECHConfirmation(msg interface{}, hello *clientHelloMsg, finishedHash *finishedHash) bool {
+ var offset int
+ var raw, label []byte
+ if hrr, ok := msg.(*helloRetryRequestMsg); ok {
+ if hrr.echConfirmationOffset == 0 {
+ return false
+ }
+ raw = hrr.raw
+ label = echAcceptConfirmationHRRLabel
+ offset = hrr.echConfirmationOffset
+ } else {
+ raw = msg.(*serverHelloMsg).raw
+ label = echAcceptConfirmationLabel
+ offset = 4 + 2 + 32 - echAcceptConfirmationLength
+ }
+
+ withZeros := append(make([]byte, 0, len(raw)), raw...)
+ for i := 0; i < echAcceptConfirmationLength; i++ {
+ withZeros[i+offset] = 0
+ }
+
+ confirmation := finishedHash.echAcceptConfirmation(hello.random, label, withZeros)
+ return bytes.Equal(confirmation, raw[offset:offset+echAcceptConfirmationLength])
+}
+
func (hs *clientHandshakeState) doTLS13Handshake(msg interface{}) error {
c := hs.c
+ // The first message may be a ServerHello or HelloRetryRequest.
+ helloRetryRequest, haveHelloRetryRequest := msg.(*helloRetryRequestMsg)
+ if haveHelloRetryRequest {
+ hs.finishedHash.UpdateForHelloRetryRequest()
+ }
+
+ // Determine whether the server accepted ECH and drop the unnecessary
+ // transcript.
+ if hs.innerHello != nil {
+ innerFinishedHash := newFinishedHash(c.wireVersion, c.isDTLS, hs.suite)
+ innerFinishedHash.WriteHandshake(hs.innerHello.marshal(), hs.c.sendHandshakeSeq-1)
+ if haveHelloRetryRequest {
+ innerFinishedHash.UpdateForHelloRetryRequest()
+ }
+ if hs.checkECHConfirmation(msg, hs.innerHello, &innerFinishedHash) {
+ c.echAccepted = true
+ // Replace the transcript. For now, leave hs.hello and hs.innerHello
+ // as-is. HelloRetryRequest requires both be available.
+ hs.finishedHash = innerFinishedHash
+ }
+ } else {
+ // When not offering ECH, test that the backend server does not (or does)
+ // send a confirmation as expected.
+ confirmed := hs.checkECHConfirmation(msg, hs.hello, &hs.finishedHash)
+ if hs.hello.echInner && !confirmed {
+ return fmt.Errorf("tls: server did not send ECH confirmation in %T when requested", msg)
+ } else if !hs.hello.echInner && confirmed {
+ return fmt.Errorf("tls: server sent ECH confirmation in %T when not requested", msg)
+ }
+ }
+
// Once the PRF hash is known, TLS 1.3 does not require a handshake buffer.
hs.finishedHash.discardHandshakeBuffer()
// The first server message must be followed by a ChangeCipherSpec.
c.expectTLS13ChangeCipherSpec = true
- // The first message may be a ServerHello or HelloRetryRequest.
- helloRetryRequest, haveHelloRetryRequest := msg.(*helloRetryRequestMsg)
if haveHelloRetryRequest {
- hs.finishedHash.UpdateForHelloRetryRequest()
hs.writeServerHash(helloRetryRequest.marshal())
- if hs.innerHello != nil {
- hs.innerFinishedHash.UpdateForHelloRetryRequest()
- hs.innerFinishedHash.WriteHandshake(helloRetryRequest.marshal(), c.recvHandshakeSeq-1)
- }
if c.config.Bugs.FailIfHelloRetryRequested {
return errors.New("tls: unexpected HelloRetryRequest")
@@ -944,20 +1005,17 @@
// Reset the encryption state, in case we sent 0-RTT data.
c.out.resetCipher()
- if hs.innerHello != nil {
- if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.innerHello, nil); err != nil {
+ if c.echAccepted {
+ if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.innerHello, hs.hello); err != nil {
return err
}
- if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.hello, hs.innerHello); err != nil {
- return err
- }
- hs.innerFinishedHash.WriteHandshake(hs.innerHello.marshal(), c.sendHandshakeSeq)
+ hs.writeClientHash(hs.innerHello.marshal())
} else {
if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.hello, nil); err != nil {
return err
}
+ hs.writeClientHash(hs.hello.marshal())
}
- hs.writeClientHash(hs.hello.marshal())
toWrite := hs.hello.marshal()
if c.config.Bugs.PartialSecondClientHelloAfterFirst {
@@ -990,6 +1048,12 @@
}
}
+ // We no longer need to retain two ClientHellos.
+ if c.echAccepted {
+ hs.hello = hs.innerHello
+ }
+ hs.innerHello = nil
+
var ok bool
hs.serverHello, ok = msg.(*serverHelloMsg)
if !ok {
@@ -1013,9 +1077,19 @@
return fmt.Errorf("tls: server sent non-matching cipher suite %04x vs %04x", hs.suite.id, hs.serverHello.cipherSuite)
}
- if haveHelloRetryRequest && helloRetryRequest.hasSelectedGroup && helloRetryRequest.selectedGroup != hs.serverHello.keyShare.group {
- c.sendAlert(alertHandshakeFailure)
- return errors.New("tls: ServerHello parameters did not match HelloRetryRequest")
+ if haveHelloRetryRequest {
+ if helloRetryRequest.hasSelectedGroup && helloRetryRequest.selectedGroup != hs.serverHello.keyShare.group {
+ c.sendAlert(alertHandshakeFailure)
+ return errors.New("tls: ServerHello parameters did not match HelloRetryRequest")
+ }
+
+ // Both the ServerHello and HelloRetryRequest must have an ECH confirmation.
+ echConfirmed := hs.checkECHConfirmation(hs.serverHello, hs.hello, &hs.finishedHash)
+ if hs.hello.echInner && !echConfirmed {
+ return errors.New("tls: server did not send ECH confirmation in ServerHello when requested")
+ } else if !hs.hello.echInner && echConfirmed {
+ return errors.New("tls: server sent ECH confirmation in ServerHello when not requested")
+ }
}
if !bytes.Equal(hs.hello.sessionID, hs.serverHello.sessionID) {
@@ -1039,9 +1113,6 @@
c.didResume = true
}
hs.finishedHash.addEntropy(pskSecret)
- if hs.innerHello != nil {
- hs.innerFinishedHash.addEntropy(pskSecret)
- }
if !hs.serverHello.hasKeyShare {
c.sendAlert(alertUnsupportedExtension)
@@ -1066,39 +1137,6 @@
}
hs.finishedHash.nextSecret()
hs.finishedHash.addEntropy(ecdheSecret)
- if hs.innerHello != nil {
- hs.innerFinishedHash.nextSecret()
- hs.innerFinishedHash.addEntropy(ecdheSecret)
- }
-
- // Determine whether the server accepted ECH.
- confirmHash := &hs.finishedHash
- if hs.innerHello != nil {
- confirmHash = &hs.innerFinishedHash
- }
- echConfirmed := bytes.Equal(hs.serverHello.random[24:], confirmHash.deriveSecretPeek([]byte("ech accept confirmation"), hs.serverHello.marshalForECHConf())[:8])
- if hs.innerHello != nil {
- c.echAccepted = echConfirmed
- if c.echAccepted {
- hs.hello = hs.innerHello
- hs.finishedHash = hs.innerFinishedHash
- }
- hs.innerHello = nil
- hs.innerFinishedHash = finishedHash{}
- } else {
- // When not offering ECH, we may still expect a confirmation signal to
- // test the backend server behavior.
- if hs.hello.echIsInner {
- if !echConfirmed {
- return errors.New("tls: server did not send ECH confirmation when requested")
- }
- } else {
- if echConfirmed {
- return errors.New("tls: server did sent ECH confirmation when not requested")
- }
- }
- }
-
hs.writeServerHash(hs.serverHello.marshal())
// Derive handshake traffic keys and switch read key to handshake
@@ -1465,58 +1503,53 @@
}
// applyHelloRetryRequest updates |hello| in-place based on |helloRetryRequest|.
-// If |innerHello| is not nil, this is the second ClientHelloOuter and should
-// contain an encrypted copy of |innerHello|
-func (hs *clientHandshakeState) applyHelloRetryRequest(helloRetryRequest *helloRetryRequestMsg, hello, innerHello *clientHelloMsg) error {
+// If |outerHello| is not nil, |outerHello| will be updated to contain an
+// encrypted copy of |hello|.
+func (hs *clientHandshakeState) applyHelloRetryRequest(helloRetryRequest *helloRetryRequestMsg, hello, outerHello *clientHelloMsg) error {
c := hs.c
firstHelloBytes := hello.marshal()
- isInner := innerHello == nil && hs.echHPKEContext != nil
if len(helloRetryRequest.cookie) > 0 {
hello.tls13Cookie = helloRetryRequest.cookie
}
- if innerHello != nil {
- hello.keyShares = innerHello.keyShares
- } else {
- if c.config.Bugs.MisinterpretHelloRetryRequestCurve != 0 {
- helloRetryRequest.hasSelectedGroup = true
- helloRetryRequest.selectedGroup = c.config.Bugs.MisinterpretHelloRetryRequestCurve
+ if c.config.Bugs.MisinterpretHelloRetryRequestCurve != 0 {
+ helloRetryRequest.hasSelectedGroup = true
+ helloRetryRequest.selectedGroup = c.config.Bugs.MisinterpretHelloRetryRequestCurve
+ }
+ if helloRetryRequest.hasSelectedGroup {
+ var hrrCurveFound bool
+ group := helloRetryRequest.selectedGroup
+ for _, curveID := range hello.supportedCurves {
+ if group == curveID {
+ hrrCurveFound = true
+ break
+ }
}
- if helloRetryRequest.hasSelectedGroup {
- var hrrCurveFound bool
- group := helloRetryRequest.selectedGroup
- for _, curveID := range hello.supportedCurves {
- if group == curveID {
- hrrCurveFound = true
- break
- }
- }
- if !hrrCurveFound || hs.keyShares[group] != nil {
- c.sendAlert(alertHandshakeFailure)
- return errors.New("tls: received invalid HelloRetryRequest")
- }
- curve, ok := curveForCurveID(group, c.config)
- if !ok {
- return errors.New("tls: Unable to get curve requested in HelloRetryRequest")
- }
- publicKey, err := curve.offer(c.config.rand())
- if err != nil {
- return err
- }
- hs.keyShares[group] = curve
- hello.keyShares = []keyShareEntry{{
- group: group,
- keyExchange: publicKey,
- }}
+ if !hrrCurveFound || hs.keyShares[group] != nil {
+ c.sendAlert(alertHandshakeFailure)
+ return errors.New("tls: received invalid HelloRetryRequest")
}
-
- if c.config.Bugs.SecondClientHelloMissingKeyShare {
- hello.hasKeyShares = false
+ curve, ok := curveForCurveID(group, c.config)
+ if !ok {
+ return errors.New("tls: Unable to get curve requested in HelloRetryRequest")
}
+ publicKey, err := curve.offer(c.config.rand())
+ if err != nil {
+ return err
+ }
+ hs.keyShares[group] = curve
+ hello.keyShares = []keyShareEntry{{
+ group: group,
+ keyExchange: publicKey,
+ }}
}
- if isInner && c.config.Bugs.OmitSecondECHIsInner {
- hello.echIsInner = false
+ if c.config.Bugs.SecondClientHelloMissingKeyShare {
+ hello.hasKeyShares = false
+ }
+
+ if c.config.Bugs.OmitSecondECHInner {
+ hello.echInner = false
}
hello.hasEarlyData = c.config.Bugs.SendEarlyDataOnSecondClientHello
@@ -1524,56 +1557,51 @@
if c.config.Bugs.PSKBinderFirst && c.config.Bugs.OnlyCorruptSecondPSKBinder {
hello.prefixExtensions = append(hello.prefixExtensions, extensionPreSharedKey)
}
- // The first ClientHello may have skipped this due to OnlyCompressSecondClientHelloInner.
- if isInner && len(c.config.ECHOuterExtensions) > 0 && c.config.Bugs.OnlyCompressSecondClientHelloInner {
- applyECHOuterExtensions(hello, c.config.ECHOuterExtensions)
- }
+ // The first ClientHello may have set this due to OnlyCompressSecondClientHelloInner.
+ hello.reorderOuterExtensionsWithoutCompressing = false
if c.config.Bugs.OmitPSKsOnSecondClientHello {
hello.pskIdentities = nil
hello.pskBinders = nil
}
hello.raw = nil
- if innerHello != nil {
+ if len(hello.pskIdentities) > 0 {
+ generatePSKBinders(c.wireVersion, hello, hs.session, firstHelloBytes, helloRetryRequest.marshal(), c.config)
+ }
+
+ if outerHello != nil {
+ outerHello.raw = nil
+ // We know the server has accepted ECH, so the ClientHelloOuter's fields
+ // are irrelevant. In the general case, the HelloRetryRequest may not
+ // even be valid for ClientHelloOuter. However, we copy the key shares
+ // from ClientHelloInner so they remain eligible for compression.
+ if !c.config.Bugs.MinimalClientHelloOuter {
+ outerHello.keyShares = hello.keyShares
+ }
+
if c.config.Bugs.OmitSecondEncryptedClientHello {
- hello.clientECH = nil
+ outerHello.echOuter = nil
} else {
configID := c.config.ClientECHConfig.ConfigID
if c.config.Bugs.CorruptSecondEncryptedClientHelloConfigID {
configID ^= 1
}
- if err := hs.encryptClientHello(hello, innerHello, configID, nil); err != nil {
+ if err := hs.encryptClientHello(outerHello, hello, configID, nil); err != nil {
return err
}
if c.config.Bugs.CorruptSecondEncryptedClientHello {
if c.config.Bugs.NullAllCiphers {
- hello.clientECH.payload = []byte{echBadPayloadByte}
+ outerHello.echOuter.payload = []byte{echBadPayloadByte}
} else {
- hello.clientECH.payload[0] ^= 1
+ outerHello.echOuter.payload[0] ^= 1
}
}
}
}
- // PSK binders and ECH both must be inserted last because they incorporate
- // the rest of the ClientHello and conflict. See corresponding comment in
- // |createClientHello|.
- if len(hello.pskIdentities) > 0 {
- generatePSKBinders(c.wireVersion, hello, hs.session, firstHelloBytes, helloRetryRequest.marshal(), c.config)
- }
return nil
}
-// applyECHOuterExtensions updates |hello| to compress |outerExtensions| with
-// the ech_outer_extensions mechanism.
-func applyECHOuterExtensions(hello *clientHelloMsg, outerExtensions []uint16) {
- // Ensure that the ech_outer_extensions extension and each of the
- // extensions it names are serialized consecutively.
- hello.prefixExtensions = append(hello.prefixExtensions, extensionECHOuterExtensions)
- hello.prefixExtensions = append(hello.prefixExtensions, outerExtensions...)
- hello.outerExtensions = outerExtensions
-}
-
func (hs *clientHandshakeState) doFullHandshake() error {
c := hs.c
diff --git a/ssl/test/runner/handshake_messages.go b/ssl/test/runner/handshake_messages.go
index d666a87..f2ef2fc 100644
--- a/ssl/test/runner/handshake_messages.go
+++ b/ssl/test/runner/handshake_messages.go
@@ -260,7 +260,7 @@
ConfigID uint8
KEM uint16
PublicKey []byte
- MaxNameLen uint16
+ MaxNameLen uint8
PublicName string
CipherSuites []HPKECipherSuite
// The following fields are only used by CreateECHConfig().
@@ -282,8 +282,8 @@
cipherSuites.addU16(suite.KDF)
cipherSuites.addU16(suite.AEAD)
}
- contents.addU16(template.MaxNameLen)
- contents.addU16LengthPrefixed().addBytes([]byte(template.PublicName))
+ contents.addU8(template.MaxNameLen)
+ contents.addU8LengthPrefixed().addBytes([]byte(template.PublicName))
extensions := contents.addU16LengthPrefixed()
// Mandatory extensions have the high bit set.
if template.UnsupportedExtension {
@@ -318,9 +318,12 @@
Key []byte
}
-// The contents of a CH "encrypted_client_hello" extension.
-// https://tools.ietf.org/html/draft-ietf-tls-esni-10
-type clientECH struct {
+const (
+ echClientTypeOuter byte = 0
+ echClientTypeInner byte = 1
+)
+
+type echClientOuter struct {
kdfID uint16
aeadID uint16
configID uint8
@@ -329,64 +332,64 @@
}
type clientHelloMsg struct {
- raw []byte
- isDTLS bool
- isV2ClientHello bool
- vers uint16
- random []byte
- v2Challenge []byte
- sessionID []byte
- cookie []byte
- cipherSuites []uint16
- compressionMethods []uint8
- nextProtoNeg bool
- serverName string
- clientECH *clientECH
- echIsInner bool
- invalidECHIsInner []byte
- ocspStapling bool
- supportedCurves []CurveID
- supportedPoints []uint8
- hasKeyShares bool
- keyShares []keyShareEntry
- keySharesRaw []byte
- trailingKeyShareData bool
- pskIdentities []pskIdentity
- pskKEModes []byte
- pskBinders [][]uint8
- hasEarlyData bool
- tls13Cookie []byte
- ticketSupported bool
- sessionTicket []uint8
- signatureAlgorithms []signatureAlgorithm
- signatureAlgorithmsCert []signatureAlgorithm
- supportedVersions []uint16
- secureRenegotiation []byte
- alpnProtocols []string
- quicTransportParams []byte
- quicTransportParamsLegacy []byte
- duplicateExtension bool
- channelIDSupported bool
- extendedMasterSecret bool
- srtpProtectionProfiles []uint16
- srtpMasterKeyIdentifier string
- sctListSupported bool
- customExtension string
- hasGREASEExtension bool
- omitExtensions bool
- emptyExtensions bool
- pad int
- compressedCertAlgs []uint16
- delegatedCredentials bool
- alpsProtocols []string
- outerExtensions []uint16
- prefixExtensions []uint16
+ raw []byte
+ isDTLS bool
+ isV2ClientHello bool
+ vers uint16
+ random []byte
+ v2Challenge []byte
+ sessionID []byte
+ cookie []byte
+ cipherSuites []uint16
+ compressionMethods []uint8
+ nextProtoNeg bool
+ serverName string
+ echOuter *echClientOuter
+ echInner bool
+ invalidECHInner []byte
+ ocspStapling bool
+ supportedCurves []CurveID
+ supportedPoints []uint8
+ hasKeyShares bool
+ keyShares []keyShareEntry
+ keySharesRaw []byte
+ trailingKeyShareData bool
+ pskIdentities []pskIdentity
+ pskKEModes []byte
+ pskBinders [][]uint8
+ hasEarlyData bool
+ tls13Cookie []byte
+ ticketSupported bool
+ sessionTicket []uint8
+ signatureAlgorithms []signatureAlgorithm
+ signatureAlgorithmsCert []signatureAlgorithm
+ supportedVersions []uint16
+ secureRenegotiation []byte
+ alpnProtocols []string
+ quicTransportParams []byte
+ quicTransportParamsLegacy []byte
+ duplicateExtension bool
+ channelIDSupported bool
+ extendedMasterSecret bool
+ srtpProtectionProfiles []uint16
+ srtpMasterKeyIdentifier string
+ sctListSupported bool
+ customExtension string
+ hasGREASEExtension bool
+ omitExtensions bool
+ emptyExtensions bool
+ pad int
+ compressedCertAlgs []uint16
+ delegatedCredentials bool
+ alpsProtocols []string
+ outerExtensions []uint16
+ reorderOuterExtensionsWithoutCompressing bool
+ prefixExtensions []uint16
// The following fields are only filled in by |unmarshal| and ignored when
// marshaling a new ClientHello.
- extensionStart int
- echExtensionStart int
- echExtensionEnd int
- rawExtensions map[uint16][]byte
+ echPayloadStart int
+ echPayloadEnd int
+ rawExtensions []byte
}
func (m *clientHelloMsg) marshalKeyShares(bb *byteBuilder) {
@@ -405,7 +408,6 @@
const (
clientHelloNormal clientHelloType = iota
- clientHelloOuterAAD
clientHelloEncodedInner
)
@@ -471,35 +473,26 @@
body: serverNameList.finish(),
})
}
- if m.clientECH != nil && typ != clientHelloOuterAAD {
+ if m.echOuter != nil {
body := newByteBuilder()
- body.addU16(m.clientECH.kdfID)
- body.addU16(m.clientECH.aeadID)
- body.addU8(m.clientECH.configID)
- body.addU16LengthPrefixed().addBytes(m.clientECH.enc)
- body.addU16LengthPrefixed().addBytes(m.clientECH.payload)
-
+ body.addU8(echClientTypeOuter)
+ body.addU16(m.echOuter.kdfID)
+ body.addU16(m.echOuter.aeadID)
+ body.addU8(m.echOuter.configID)
+ body.addU16LengthPrefixed().addBytes(m.echOuter.enc)
+ body.addU16LengthPrefixed().addBytes(m.echOuter.payload)
extensions = append(extensions, extension{
id: extensionEncryptedClientHello,
body: body.finish(),
})
}
- if m.echIsInner {
- extensions = append(extensions, extension{
- id: extensionECHIsInner,
- // If unset, invalidECHIsInner is empty, which is the correct
- // serialization.
- body: m.invalidECHIsInner,
- })
- }
- if m.outerExtensions != nil && typ == clientHelloEncodedInner {
+ if m.echInner {
body := newByteBuilder()
- extensionsList := body.addU8LengthPrefixed()
- for _, extID := range m.outerExtensions {
- extensionsList.addU16(extID)
- }
+ body.addU8(echClientTypeInner)
+ // If unset, invalidECHInner is empty, which is the correct serialization.
+ body.addBytes(m.invalidECHInner)
extensions = append(extensions, extension{
- id: extensionECHOuterExtensions,
+ id: extensionEncryptedClientHello,
body: body.finish(),
})
}
@@ -668,7 +661,7 @@
if m.sctListSupported {
extensions = append(extensions, extension{id: extensionSignedCertificateTimestamp})
}
- if l := len(m.customExtension); l > 0 {
+ if len(m.customExtension) > 0 {
extensions = append(extensions, extension{
id: extensionCustom,
body: []byte(m.customExtension),
@@ -726,35 +719,51 @@
})
}
- // Write each extension in |extensions| to the |hello| message, hoisting
- // the extensions named in |m.prefixExtensions| to the front.
extensionsBB := hello.addU16LengthPrefixed()
extMap := make(map[uint16][]byte)
+ extsWritten := make(map[uint16]struct{})
for _, ext := range extensions {
extMap[ext.id] = ext.body
}
- // Elide each of the extensions named by |m.outerExtensions|.
- if m.outerExtensions != nil && typ == clientHelloEncodedInner {
- for _, extID := range m.outerExtensions {
- delete(extMap, extID)
- }
- }
// Write each of the prefix extensions, if we have it.
for _, extID := range m.prefixExtensions {
if body, ok := extMap[extID]; ok {
extensionsBB.addU16(extID)
extensionsBB.addU16LengthPrefixed().addBytes(body)
+ extsWritten[extID] = struct{}{}
}
}
- // Forget each of the prefix extensions. This loop is separate from the
- // extension-writing loop because |m.prefixExtensions| may contain
- // duplicates.
- for _, extID := range m.prefixExtensions {
- delete(extMap, extID)
+ // Write outer extensions, possibly in compressed form.
+ if m.outerExtensions != nil {
+ if typ == clientHelloEncodedInner && !m.reorderOuterExtensionsWithoutCompressing {
+ extensionsBB.addU16(extensionECHOuterExtensions)
+ list := extensionsBB.addU16LengthPrefixed().addU8LengthPrefixed()
+ for _, extID := range m.outerExtensions {
+ list.addU16(extID)
+ extsWritten[extID] = struct{}{}
+ }
+ } else {
+ for _, extID := range m.outerExtensions {
+ // m.outerExtensions may intentionally contain duplicates to test the
+ // server's reaction. If m.reorderOuterExtensionsWithoutCompressing
+ // is set, we are targetting the second ClientHello and wish to send a
+ // valid first ClientHello. In that case, deduplicate so the error
+ // only appears later.
+ if _, written := extsWritten[extID]; m.reorderOuterExtensionsWithoutCompressing && written {
+ continue
+ }
+ if body, ok := extMap[extID]; ok {
+ extensionsBB.addU16(extID)
+ extensionsBB.addU16LengthPrefixed().addBytes(body)
+ extsWritten[extID] = struct{}{}
+ }
+ }
+ }
}
+
// Write each of the remaining extensions in their original order.
for _, ext := range extensions {
- if _, ok := extMap[ext.id]; ok {
+ if _, written := extsWritten[ext.id]; !written {
extensionsBB.addU16(ext.id)
extensionsBB.addU16LengthPrefixed().addBytes(ext.body)
}
@@ -779,10 +788,6 @@
}
}
-func (m *clientHelloMsg) marshalForOuterAAD(bb *byteBuilder) {
- m.marshalBody(bb, clientHelloOuterAAD)
-}
-
func (m *clientHelloMsg) marshalForEncodedInner() []byte {
hello := newByteBuilder()
m.marshalBody(hello, clientHelloEncodedInner)
@@ -891,8 +896,6 @@
}
}
- m.extensionStart = len(data) - len(reader)
-
m.nextProtoNeg = false
m.serverName = ""
m.ocspStapling = false
@@ -909,7 +912,6 @@
m.customExtension = ""
m.delegatedCredentials = false
m.alpsProtocols = nil
- m.rawExtensions = make(map[uint16][]byte)
if len(reader) == 0 {
// ClientHello is optionally followed by extension data
@@ -920,6 +922,7 @@
if !reader.readU16LengthPrefixed(&extensions) || len(reader) != 0 || !checkDuplicateExtensions(extensions) {
return false
}
+ m.rawExtensions = extensions
for len(extensions) > 0 {
var extension uint16
var body byteReader
@@ -927,7 +930,6 @@
!extensions.readU16LengthPrefixed(&body) {
return false
}
- m.rawExtensions[extension] = body
switch extension {
case extensionServerName:
var names byteReader
@@ -946,24 +948,33 @@
}
}
case extensionEncryptedClientHello:
- m.echExtensionEnd = len(data) - len(extensions)
- m.echExtensionStart = m.echExtensionEnd - len(body) - 4
- var ech clientECH
- if !body.readU16(&ech.kdfID) ||
- !body.readU16(&ech.aeadID) ||
- !body.readU8(&ech.configID) ||
- !body.readU16LengthPrefixedBytes(&ech.enc) ||
- !body.readU16LengthPrefixedBytes(&ech.payload) ||
- len(ech.payload) == 0 ||
- len(body) > 0 {
+ var typ byte
+ if !body.readU8(&typ) {
return false
}
- m.clientECH = &ech
- case extensionECHIsInner:
- if len(body) != 0 {
+ switch typ {
+ case echClientTypeOuter:
+ var echOuter echClientOuter
+ if !body.readU16(&echOuter.kdfID) ||
+ !body.readU16(&echOuter.aeadID) ||
+ !body.readU8(&echOuter.configID) ||
+ !body.readU16LengthPrefixedBytes(&echOuter.enc) ||
+ !body.readU16LengthPrefixedBytes(&echOuter.payload) ||
+ len(echOuter.payload) == 0 ||
+ len(body) > 0 {
+ return false
+ }
+ m.echOuter = &echOuter
+ m.echPayloadEnd = len(data) - len(extensions)
+ m.echPayloadStart = m.echPayloadEnd - len(echOuter.payload)
+ case echClientTypeInner:
+ if len(body) > 0 {
+ return false
+ }
+ m.echInner = true
+ default:
return false
}
- m.echIsInner = true
case extensionNextProtoNeg:
if len(body) != 0 {
return false
@@ -1195,13 +1206,6 @@
}
}
- // Clients may not send both extensions.
- // TODO(davidben): A later draft will likely merge the code points, at which
- // point this check will be redundant.
- if m.echIsInner && m.clientECH != nil {
- return false
- }
-
return true
}
@@ -1214,11 +1218,17 @@
len(sessionID) != 0 || // Copied from |helloOuter|
!reader.readU16LengthPrefixedBytes(&cipherSuites) ||
!reader.readU8LengthPrefixedBytes(&compressionMethods) ||
- !reader.readU16LengthPrefixed(&extensions) ||
- len(reader) != 0 {
+ !reader.readU16LengthPrefixed(&extensions) {
return nil, errors.New("tls: error parsing EncodedClientHelloInner")
}
+ // The remainder of the structure is padding.
+ for _, padding := range reader {
+ if padding != 0 {
+ return nil, errors.New("tls: non-zero padding in EncodedClientHelloInner")
+ }
+ }
+
builder := newByteBuilder()
builder.addU8(typeClientHello)
body := builder.addU24LengthPrefixed()
@@ -1229,6 +1239,7 @@
newExtensions := body.addU16LengthPrefixed()
var seenOuterExtensions bool
+ outerExtensions := byteReader(helloOuter.rawExtensions)
copied := make(map[uint16]struct{})
for len(extensions) > 0 {
var extType uint16
@@ -1246,28 +1257,35 @@
return nil, errors.New("tls: duplicate ech_outer_extensions extension")
}
seenOuterExtensions = true
- var outerExtensions byteReader
- if !extBody.readU8LengthPrefixed(&outerExtensions) || len(outerExtensions) == 0 || len(extBody) != 0 {
+ var extList byteReader
+ if !extBody.readU8LengthPrefixed(&extList) || len(extList) == 0 || len(extBody) != 0 {
return nil, errors.New("tls: error parsing ech_outer_extensions")
}
- for len(outerExtensions) != 0 {
+ for len(extList) != 0 {
var newExtType uint16
- if !outerExtensions.readU16(&newExtType) {
+ if !extList.readU16(&newExtType) {
return nil, errors.New("tls: error parsing ech_outer_extensions")
}
if newExtType == extensionEncryptedClientHello {
return nil, errors.New("tls: error parsing ech_outer_extensions")
}
- if _, ok := copied[newExtType]; ok {
- return nil, errors.New("tls: duplicate extension in ech_outer_extensions")
+ for {
+ if len(outerExtensions) == 0 {
+ return nil, fmt.Errorf("tls: extension %d not found in ClientHelloOuter", newExtType)
+ }
+ var foundExt uint16
+ var newExtBody []byte
+ if !outerExtensions.readU16(&foundExt) ||
+ !outerExtensions.readU16LengthPrefixedBytes(&newExtBody) {
+ return nil, errors.New("tls: error parsing ClientHelloOuter")
+ }
+ if foundExt == newExtType {
+ newExtensions.addU16(newExtType)
+ newExtensions.addU16LengthPrefixed().addBytes(newExtBody)
+ copied[newExtType] = struct{}{}
+ break
+ }
}
- newExtBody, ok := helloOuter.rawExtensions[newExtType]
- if !ok {
- return nil, fmt.Errorf("tls: extension %d not found in ClientHelloOuter", newExtType)
- }
- newExtensions.addU16(newExtType)
- newExtensions.addU16LengthPrefixed().addBytes(newExtBody)
- copied[newExtType] = struct{}{}
}
}
@@ -1487,25 +1505,6 @@
return true
}
-// marshalForECHConf marshals |m|, but zeroes out the last 8 bytes of the
-// ServerHello.random.
-func (m *serverHelloMsg) marshalForECHConf() []byte {
- ret := m.marshal()
- // Make a copy so we can mutate it.
- ret = append(make([]byte, 0, len(ret)), ret...)
-
- reparsed := new(serverHelloMsg)
- if !reparsed.unmarshal(ret) {
- panic("could not re-parse ServerHello")
- }
- // We rely on |unmarshal| aliasing the |random| into |ret|.
- for i := 24; i < 32; i++ {
- reparsed.random[i] = 0
- }
-
- return ret
-}
-
type encryptedExtensionsMsg struct {
raw []byte
extensions serverExtensions
@@ -1907,16 +1906,18 @@
}
type helloRetryRequestMsg struct {
- raw []byte
- vers uint16
- sessionID []byte
- cipherSuite uint16
- compressionMethod uint8
- hasSelectedGroup bool
- selectedGroup CurveID
- cookie []byte
- customExtension string
- duplicateExtensions bool
+ raw []byte
+ vers uint16
+ sessionID []byte
+ cipherSuite uint16
+ compressionMethod uint8
+ hasSelectedGroup bool
+ selectedGroup CurveID
+ cookie []byte
+ customExtension string
+ echConfirmation []byte
+ echConfirmationOffset int
+ duplicateExtensions bool
}
func (m *helloRetryRequestMsg) marshal() []byte {
@@ -1960,6 +1961,10 @@
extensions.addU16(extensionCustom)
extensions.addU16LengthPrefixed().addBytes([]byte(m.customExtension))
}
+ if len(m.echConfirmation) > 0 {
+ extensions.addU16(extensionEncryptedClientHello)
+ extensions.addU16LengthPrefixed().addBytes(m.echConfirmation)
+ }
}
m.raw = retryRequestMsg.finish()
@@ -2005,9 +2010,17 @@
m.hasSelectedGroup = true
m.selectedGroup = CurveID(v)
case extensionCookie:
- if !body.readU16LengthPrefixedBytes(&m.cookie) || len(body) != 0 {
+ if !body.readU16LengthPrefixedBytes(&m.cookie) ||
+ len(m.cookie) == 0 ||
+ len(body) != 0 {
return false
}
+ case extensionEncryptedClientHello:
+ if len(body) != echAcceptConfirmationLength {
+ return false
+ }
+ m.echConfirmation = body
+ m.echConfirmationOffset = len(m.raw) - len(extensions) - len(body)
default:
// Unknown extensions are illegal from the server.
return false
diff --git a/ssl/test/runner/handshake_server.go b/ssl/test/runner/handshake_server.go
index 1464944..4f41184 100644
--- a/ssl/test/runner/handshake_server.go
+++ b/ssl/test/runner/handshake_server.go
@@ -186,14 +186,21 @@
return errors.New("tls: no GREASE extension found")
}
- if clientECH := hs.clientHello.clientECH; clientECH != nil {
+ if config.Bugs.ExpectClientECH && hs.clientHello.echOuter == nil {
+ return errors.New("tls: expected client to offer ECH")
+ }
+ if config.Bugs.ExpectNoClientECH && hs.clientHello.echOuter != nil {
+ return errors.New("tls: expected client not to offer ECH")
+ }
+
+ if echOuter := hs.clientHello.echOuter; echOuter != nil {
for _, candidate := range config.ServerECHConfigs {
- if candidate.ECHConfig.ConfigID != clientECH.configID {
+ if candidate.ECHConfig.ConfigID != echOuter.configID {
continue
}
var found bool
for _, suite := range candidate.ECHConfig.CipherSuites {
- if clientECH.kdfID == suite.KDF && clientECH.aeadID == suite.AEAD {
+ if echOuter.kdfID == suite.KDF && echOuter.aeadID == suite.AEAD {
found = true
break
}
@@ -203,7 +210,7 @@
}
info := []byte("tls ech\x00")
info = append(info, candidate.ECHConfig.Raw...)
- hs.echHPKEContext, err = hpke.SetupBaseReceiverX25519(clientECH.kdfID, clientECH.aeadID, clientECH.enc, candidate.Key, info)
+ hs.echHPKEContext, err = hpke.SetupBaseReceiverX25519(echOuter.kdfID, echOuter.aeadID, echOuter.enc, candidate.Key, info)
if err != nil {
continue
}
@@ -220,7 +227,7 @@
} else {
c.echAccepted = true
hs.clientHello = clientHelloInner
- hs.echConfigID = clientECH.configID
+ hs.echConfigID = echOuter.configID
}
}
}
@@ -449,29 +456,17 @@
}
func (hs *serverHandshakeState) decryptClientHello(helloOuter *clientHelloMsg) (helloInner *clientHelloMsg, err error) {
- // See draft-ietf-tls-esni-10, section 5.2.
- aad := newByteBuilder()
- aad.addU16(helloOuter.clientECH.kdfID)
- aad.addU16(helloOuter.clientECH.aeadID)
- aad.addU8(helloOuter.clientECH.configID)
- aad.addU16LengthPrefixed().addBytes(helloOuter.clientECH.enc)
- // ClientHelloOuterAAD.outer_hello is ClientHelloOuter without the
- // encrypted_client_hello extension. Construct this by piecing together
- // the preserved portions from offsets and updating the length prefix.
- //
- // TODO(davidben): If https://github.com/tlswg/draft-ietf-tls-esni/pull/442
- // is merged, a later iteration will hopefully be simpler.
- outerHello := aad.addU24LengthPrefixed()
- outerHello.addBytes(helloOuter.raw[4:helloOuter.extensionStart])
- extensions := outerHello.addU16LengthPrefixed()
- extensions.addBytes(helloOuter.raw[helloOuter.extensionStart+2 : helloOuter.echExtensionStart])
- extensions.addBytes(helloOuter.raw[helloOuter.echExtensionEnd:])
+ // ClientHelloOuterAAD is ClientHelloOuter with the payload replaced by
+ // zeros. See draft-ietf-tls-esni-13, section 5.2.
+ aad := make([]byte, len(helloOuter.raw)-4)
+ copy(aad, helloOuter.raw[4:helloOuter.echPayloadStart])
+ copy(aad[helloOuter.echPayloadEnd-4:], helloOuter.raw[helloOuter.echPayloadEnd:])
// In fuzzer mode, the payload is cleartext.
- encoded := helloOuter.clientECH.payload
+ encoded := helloOuter.echOuter.payload
if !hs.c.config.Bugs.NullAllCiphers {
var err error
- encoded, err = hs.echHPKEContext.Open(helloOuter.clientECH.payload, aad.finish())
+ encoded, err = hs.echHPKEContext.Open(helloOuter.echOuter.payload, aad)
if err != nil {
// Wrap |err| so the caller can implement trial decryption.
return nil, &echDecryptError{err}
@@ -505,8 +500,8 @@
if helloInner.nextProtoNeg || len(helloInner.supportedPoints) != 0 || helloInner.ticketSupported || helloInner.secureRenegotiation != nil || helloInner.extendedMasterSecret {
return nil, errors.New("tls: ClientHelloInner included a TLS-1.2-only extension")
}
- if !helloInner.echIsInner {
- return nil, errors.New("tls: ClientHelloInner missing ech_is_inner extension")
+ if !helloInner.echInner {
+ return nil, errors.New("tls: ClientHelloInner missing inner encrypted_client_hello extension")
}
return helloInner, nil
@@ -549,13 +544,6 @@
return err
}
- if config.Bugs.ExpectClientECH && hs.clientHello.clientECH == nil {
- return errors.New("tls: expected client to offer ECH")
- }
- if config.Bugs.ExpectNoClientECH && hs.clientHello.clientECH != nil {
- return errors.New("tls: expected client not to offer ECH")
- }
-
// Select the cipher suite.
var preferenceList, supportedList []uint16
if config.PreferServerCipherSuites {
@@ -754,6 +742,21 @@
if sendHelloRetryRequest {
hs.finishedHash.UpdateForHelloRetryRequest()
+
+ // Emit the ECH confirmation signal when requested.
+ if hs.clientHello.echInner {
+ helloRetryRequest.echConfirmation = make([]byte, 8)
+ helloRetryRequest.echConfirmation = hs.finishedHash.echAcceptConfirmation(hs.clientHello.random, echAcceptConfirmationHRRLabel, helloRetryRequest.marshal())
+ helloRetryRequest.raw = nil
+ } else if config.Bugs.AlwaysSendECHHelloRetryRequest {
+ // When solicited, a random ECH confirmation string should be ignored.
+ helloRetryRequest.echConfirmation = make([]byte, 8)
+ if _, err := io.ReadFull(config.rand(), helloRetryRequest.echConfirmation); err != nil {
+ c.sendAlert(alertInternalError)
+ return fmt.Errorf("tls: short read from Rand: %s", err)
+ }
+ }
+
hs.writeServerHash(helloRetryRequest.marshal())
if c.config.Bugs.PartialServerHelloWithHelloRetryRequest {
data := helloRetryRequest.marshal()
@@ -787,19 +790,19 @@
}
if c.echAccepted {
- if newClientHello.clientECH == nil {
+ if newClientHello.echOuter == nil {
c.sendAlert(alertMissingExtension)
return errors.New("tls: second ClientHelloOuter had no encrypted_client_hello extension")
}
- if newClientHello.clientECH.configID != hs.echConfigID ||
- newClientHello.clientECH.kdfID != hs.echHPKEContext.KDF() ||
- newClientHello.clientECH.aeadID != hs.echHPKEContext.AEAD() {
+ if newClientHello.echOuter.configID != hs.echConfigID ||
+ newClientHello.echOuter.kdfID != hs.echHPKEContext.KDF() ||
+ newClientHello.echOuter.aeadID != hs.echHPKEContext.AEAD() {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: ECH parameters changed in second ClientHelloOuter")
}
- if len(newClientHello.clientECH.enc) != 0 {
+ if len(newClientHello.echOuter.enc) != 0 {
c.sendAlert(alertIllegalParameter)
- return errors.New("tls: second ClientECH had non-empty enc")
+ return errors.New("tls: second ClientHelloOuter had non-empty ECH enc")
}
newClientHello, err = hs.decryptClientHello(newClientHello)
if err != nil {
@@ -819,11 +822,6 @@
// Check that the new ClientHello matches the old ClientHello,
// except for relevant modifications. See RFC 8446, section 4.1.2.
ignoreExtensions := []uint16{extensionPadding}
- // TODO(https://crbug.com/boringssl/275): draft-ietf-tls-esni-10 requires
- // violating the RFC 8446 rules. See
- // https://github.com/tlswg/draft-ietf-tls-esni/issues/358
- // A later draft will likely fix this. Remove this line if it does.
- ignoreExtensions = append(ignoreExtensions, extensionEncryptedClientHello)
if helloRetryRequest.hasSelectedGroup {
newKeyShares := newClientHello.keyShares
@@ -1006,13 +1004,13 @@
hs.finishedHash.addEntropy(hs.finishedHash.zeroSecret())
}
- // Overwrite part of ServerHello.random to signal ECH acceptance to the client.
- if hs.clientHello.echIsInner {
- for i := 24; i < 32; i++ {
- hs.hello.random[i] = 0
+ // Emit the ECH confirmation signal when requested.
+ if hs.clientHello.echInner && !config.Bugs.OmitServerHelloECHConfirmation {
+ randomSuffix := hs.hello.random[len(hs.hello.random)-echAcceptConfirmationLength:]
+ for i := range randomSuffix {
+ randomSuffix[i] = 0
}
- echAcceptConfirmation := hs.finishedHash.deriveSecretPeek([]byte("ech accept confirmation"), hs.hello.marshal())
- copy(hs.hello.random[24:], echAcceptConfirmation)
+ copy(randomSuffix, hs.finishedHash.echAcceptConfirmation(hs.clientHello.random, echAcceptConfirmationLabel, hs.hello.marshal()))
hs.hello.raw = nil
}
@@ -1696,7 +1694,7 @@
serverExtensions.serverNameAck = c.config.Bugs.SendServerNameAck
- if (c.vers >= VersionTLS13 || c.config.Bugs.SendECHRetryConfigsInTLS12ServerHello) && hs.clientHello.clientECH != nil {
+ if (c.vers >= VersionTLS13 && hs.clientHello.echOuter != nil) || c.config.Bugs.AlwaysSendECHRetryConfigs {
if len(config.Bugs.SendECHRetryConfigs) > 0 {
serverExtensions.echRetryConfigs = config.Bugs.SendECHRetryConfigs
} else if len(config.ServerECHConfigs) > 0 {
diff --git a/ssl/test/runner/hpke/hpke.go b/ssl/test/runner/hpke/hpke.go
index a08538b..36dc637 100644
--- a/ssl/test/runner/hpke/hpke.go
+++ b/ssl/test/runner/hpke/hpke.go
@@ -136,6 +136,8 @@
func (c *Context) AEAD() uint16 { return c.aeadID }
+func (c *Context) Overhead() int { return c.aead.Overhead() }
+
func (c *Context) Seal(plaintext, additionalData []byte) []byte {
ciphertext := c.aead.Seal(nil, c.computeNonce(), plaintext, additionalData)
c.incrementSeq()
diff --git a/ssl/test/runner/prf.go b/ssl/test/runner/prf.go
index 66c427f..f5290c3 100644
--- a/ssl/test/runner/prf.go
+++ b/ssl/test/runner/prf.go
@@ -381,7 +381,6 @@
return b
}
-// The following are labels for traffic secret derivation in TLS 1.3.
var (
externalPSKBinderLabel = []byte("ext binder")
resumptionPSKBinderLabel = []byte("res binder")
@@ -396,21 +395,25 @@
resumptionLabel = []byte("res master")
resumptionPSKLabel = []byte("resumption")
+
+ echAcceptConfirmationLabel = []byte("ech accept confirmation")
+ echAcceptConfirmationHRRLabel = []byte("hrr ech accept confirmation")
)
// deriveSecret implements TLS 1.3's Derive-Secret function, as defined in
-// section 7.1 of draft ietf-tls-tls13-16.
+// section 7.1 of RFC8446.
func (h *finishedHash) deriveSecret(label []byte) []byte {
return hkdfExpandLabel(h.suite.hash(), h.secret, label, h.appendContextHashes(nil), h.hash.Size())
}
-// deriveSecretPeek is the same as deriveSecret, but it enables the caller to
-// tentatively append messages to the transcript. The |extraMessages| parameter
-// contains the bytes of these tentative messages.
-func (h *finishedHash) deriveSecretPeek(label []byte, extraMessages []byte) []byte {
- hashPeek := copyHash(h.hash, h.suite.hash())
- hashPeek.Write(extraMessages)
- return hkdfExpandLabel(h.suite.hash(), h.secret, label, hashPeek.Sum(nil), h.hash.Size())
+// echConfirmation computes the ECH accept confirmation signal, as defined in
+// sections 7.2 and 7.2.1 of draft-ietf-tls-esni-13. The transcript hash is
+// computed by concatenating |h| with |extraMessages|.
+func (h *finishedHash) echAcceptConfirmation(clientRandom, label, extraMessages []byte) []byte {
+ secret := hkdf.Extract(h.suite.hash().New, h.zeroSecret(), clientRandom)
+ hashCopy := copyHash(h.hash, h.suite.hash())
+ hashCopy.Write(extraMessages)
+ return hkdfExpandLabel(h.suite.hash(), secret, label, hashCopy.Sum(nil), echAcceptConfirmationLength)
}
// The following are context strings for CertificateVerify in TLS 1.3.
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 1953b88..cfff714 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -16645,19 +16645,19 @@
expectedError: ":INVALID_CLIENT_HELLO_INNER:",
})
- // When ech_is_inner extension is absent from the ClientHelloInner, the
+ // When inner ECH extension is absent from the ClientHelloInner, the
// server should fail the connection.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-MissingECHIsInner" + suffix,
+ name: prefix + "ECH-Server-MissingECHInner" + suffix,
config: Config{
ServerName: "secret.example",
DefaultCurves: defaultCurves,
ClientECHConfig: echConfig.ECHConfig,
Bugs: ProtocolBugs{
- OmitECHIsInner: !hrr,
- OmitSecondECHIsInner: hrr,
+ OmitECHInner: !hrr,
+ OmitSecondECHInner: hrr,
},
},
flags: []string{
@@ -16687,11 +16687,7 @@
extensionCustom,
},
Bugs: ProtocolBugs{
- CustomExtension: "test",
- // Ensure ClientHelloOuter's extension order is different
- // from ClientHelloInner. This tests that the server
- // correctly reconstructs the extension order.
- FirstExtensionInClientHelloOuter: extensionSupportedCurves,
+ CustomExtension: "test",
OnlyCompressSecondClientHelloInner: hrr,
},
},
@@ -16707,6 +16703,84 @@
},
})
+ // Test that the server allows referenced ClientHelloOuter
+ // extensions to be interleaved with other extensions. Only the
+ // relative order must match.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: protocol,
+ name: prefix + "ECH-Server-OuterExtensions-Interleaved" + suffix,
+ config: Config{
+ ServerName: "secret.example",
+ DefaultCurves: defaultCurves,
+ ClientECHConfig: echConfig.ECHConfig,
+ ECHOuterExtensions: []uint16{
+ extensionKeyShare,
+ extensionSupportedCurves,
+ extensionCustom,
+ },
+ Bugs: ProtocolBugs{
+ CustomExtension: "test",
+ OnlyCompressSecondClientHelloInner: hrr,
+ ECHOuterExtensionOrder: []uint16{
+ extensionServerName,
+ extensionKeyShare,
+ extensionSupportedVersions,
+ extensionPSKKeyExchangeModes,
+ extensionSupportedCurves,
+ extensionSignatureAlgorithms,
+ extensionCustom,
+ },
+ },
+ },
+ flags: []string{
+ "-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+ "-ech-server-key", base64FlagValue(echConfig.Key),
+ "-ech-is-retry-config", "1",
+ "-expect-server-name", "secret.example",
+ "-expect-ech-accept",
+ },
+ expectations: connectionExpectations{
+ echAccepted: true,
+ },
+ })
+
+ // Test that the server rejects references to extensions in the
+ // wrong order.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: protocol,
+ name: prefix + "ECH-Server-OuterExtensions-WrongOrder" + suffix,
+ config: Config{
+ ServerName: "secret.example",
+ DefaultCurves: defaultCurves,
+ ClientECHConfig: echConfig.ECHConfig,
+ ECHOuterExtensions: []uint16{
+ extensionKeyShare,
+ extensionSupportedCurves,
+ },
+ Bugs: ProtocolBugs{
+ CustomExtension: "test",
+ OnlyCompressSecondClientHelloInner: hrr,
+ ECHOuterExtensionOrder: []uint16{
+ extensionSupportedCurves,
+ extensionKeyShare,
+ },
+ },
+ },
+ flags: []string{
+ "-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+ "-ech-server-key", base64FlagValue(echConfig.Key),
+ "-ech-is-retry-config", "1",
+ "-expect-server-name", "secret.example",
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: illegal parameter",
+ // The decoding algorithm relies on the ordering requirement, so
+ // the wrong order appears as a missing extension.
+ expectedError: ":OUTER_EXTENSION_NOT_FOUND:",
+ })
+
// Test that the server rejects duplicated values in ech_outer_extensions.
// Besides causing the server to reconstruct an invalid ClientHelloInner
// with duplicated extensions, this behavior would be vulnerable to DoS
@@ -16725,6 +16799,10 @@
},
Bugs: ProtocolBugs{
OnlyCompressSecondClientHelloInner: hrr,
+ // Don't duplicate the extension in ClientHelloOuter.
+ ECHOuterExtensionOrder: []uint16{
+ extensionSupportedCurves,
+ },
},
},
flags: []string{
@@ -16734,7 +16812,9 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- expectedError: ":DUPLICATE_EXTENSION:",
+ // The decoding algorithm relies on the ordering requirement, so
+ // duplicates appear as missing extensions.
+ expectedError: ":OUTER_EXTENSION_NOT_FOUND:",
})
// Test that the server rejects references to missing extensions in
@@ -16950,12 +17030,12 @@
})
}
- // Test that the ECH server handles a short ClientECH.enc value by
- // falling back to ClientHelloOuter.
+ // Test that the ECH server handles a short enc value by falling back to
+ // ClientHelloOuter.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ShortClientECHEnc",
+ name: prefix + "ECH-Server-ShortEnc",
config: Config{
ServerName: "secret.example",
ClientECHConfig: echConfig.ECHConfig,
@@ -17169,6 +17249,54 @@
},
})
+ // Test that the server accepts padding.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: protocol,
+ name: prefix + "ECH-Server-Padding",
+ config: Config{
+ ClientECHConfig: echConfig.ECHConfig,
+ Bugs: ProtocolBugs{
+ ClientECHPadding: 10,
+ },
+ },
+ flags: []string{
+ "-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+ "-ech-server-key", base64FlagValue(echConfig.Key),
+ "-ech-is-retry-config", "1",
+ "-expect-ech-accept",
+ },
+ expectations: connectionExpectations{
+ echAccepted: true,
+ },
+ })
+
+ // Test that the server rejects bad padding.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: protocol,
+ name: prefix + "ECH-Server-BadPadding",
+ config: Config{
+ ClientECHConfig: echConfig.ECHConfig,
+ Bugs: ProtocolBugs{
+ ClientECHPadding: 10,
+ BadClientECHPadding: true,
+ },
+ },
+ flags: []string{
+ "-ech-server-config", base64FlagValue(echConfig.ECHConfig.Raw),
+ "-ech-server-key", base64FlagValue(echConfig.Key),
+ "-ech-is-retry-config", "1",
+ "-expect-ech-accept",
+ },
+ expectations: connectionExpectations{
+ echAccepted: true,
+ },
+ shouldFail: true,
+ expectedError: ":DECODE_ERROR",
+ expectedLocalError: "remote error: illegal parameter",
+ })
+
// Test the client's behavior when the server ignores ECH GREASE.
testCases = append(testCases, testCase{
testType: clientTest,
@@ -17235,19 +17363,19 @@
flags: []string{"-enable-ech-grease"},
})
+ // TLS 1.2 ServerHellos cannot contain retry configs.
if protocol != quic {
- // Test that the client rejects retry configs in TLS 1.2.
testCases = append(testCases, testCase{
testType: clientTest,
protocol: protocol,
- name: prefix + "ECH-GREASE-Client-TLS12-Retry-Configs",
+ name: prefix + "ECH-GREASE-Client-TLS12-RejectRetryConfigs",
config: Config{
- MinVersion: VersionTLS12,
- MaxVersion: VersionTLS12,
+ MinVersion: VersionTLS12,
+ MaxVersion: VersionTLS12,
+ ServerECHConfigs: []ServerECHConfig{echConfig},
Bugs: ProtocolBugs{
- ExpectClientECH: true,
- SendECHRetryConfigs: CreateECHConfigList(echConfig.ECHConfig.Raw, unsupportedVersion),
- SendECHRetryConfigsInTLS12ServerHello: true,
+ ExpectClientECH: true,
+ AlwaysSendECHRetryConfigs: true,
},
},
flags: []string{"-enable-ech-grease"},
@@ -17255,8 +17383,101 @@
expectedLocalError: "remote error: unsupported extension",
expectedError: ":UNEXPECTED_EXTENSION:",
})
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-TLS12-RejectRetryConfigs",
+ config: Config{
+ MinVersion: VersionTLS12,
+ MaxVersion: VersionTLS12,
+ ServerECHConfigs: []ServerECHConfig{echConfig},
+ Bugs: ProtocolBugs{
+ ExpectClientECH: true,
+ AlwaysSendECHRetryConfigs: true,
+ },
+ },
+ flags: []string{
+ "-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig1.ECHConfig.Raw)),
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: unsupported extension",
+ expectedError: ":UNEXPECTED_EXTENSION:",
+ })
}
+ // Retry configs must be rejected when ECH is accepted.
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-Accept-RejectRetryConfigs",
+ config: Config{
+ ServerECHConfigs: []ServerECHConfig{echConfig},
+ Bugs: ProtocolBugs{
+ ExpectClientECH: true,
+ AlwaysSendECHRetryConfigs: true,
+ },
+ },
+ flags: []string{
+ "-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: unsupported extension",
+ expectedError: ":UNEXPECTED_EXTENSION:",
+ })
+
+ // Unsolicited ECH HelloRetryRequest extensions should be rejected.
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-UnsolictedHRRExtension",
+ config: Config{
+ ServerECHConfigs: []ServerECHConfig{echConfig},
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ AlwaysSendECHHelloRetryRequest: true,
+ ExpectMissingKeyShare: true, // Check we triggered HRR.
+ },
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: unsupported extension",
+ expectedError: ":UNEXPECTED_EXTENSION:",
+ })
+
+ // GREASE should ignore ECH HelloRetryRequest extensions.
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-GREASE-IgnoreHRRExtension",
+ config: Config{
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ AlwaysSendECHHelloRetryRequest: true,
+ ExpectMissingKeyShare: true, // Check we triggered HRR.
+ },
+ },
+ flags: []string{"-enable-ech-grease"},
+ })
+
+ // Random ECH HelloRetryRequest extensions also signal ECH reject.
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-Reject-RandomHRRExtension",
+ config: Config{
+ CurvePreferences: []CurveID{CurveP384},
+ Bugs: ProtocolBugs{
+ AlwaysSendECHHelloRetryRequest: true,
+ ExpectMissingKeyShare: true, // Check we triggered HRR.
+ },
+ },
+ flags: []string{
+ "-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+ },
+ shouldFail: true,
+ expectedLocalError: "remote error: ECH required",
+ expectedError: ":ECH_REJECTED:",
+ })
+
// Test that the client aborts with a decode_error alert when it receives a
// syntactically-invalid encrypted_client_hello extension from the server.
testCases = append(testCases, testCase{
@@ -17277,60 +17498,63 @@
expectedError: ":ERROR_PARSING_EXTENSION:",
})
- // Test that the server responds to an empty ech_is_inner extension with the
+ // Test that the server responds to an inner ECH extension with the
// acceptance confirmation.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHIsInner",
+ name: prefix + "ECH-Server-ECHInner",
config: Config{
MinVersion: VersionTLS13,
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
- AlwaysSendECHIsInner: true,
+ AlwaysSendECHInner: true,
+ },
+ },
+ resumeSession: true,
+ })
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ protocol: protocol,
+ name: prefix + "ECH-Server-ECHInner-HelloRetryRequest",
+ config: Config{
+ MinVersion: VersionTLS13,
+ MaxVersion: VersionTLS13,
+ // Force a HelloRetryRequest.
+ DefaultCurves: []CurveID{},
+ Bugs: ProtocolBugs{
+ AlwaysSendECHInner: true,
},
},
resumeSession: true,
})
// Test that server fails the handshake when it sees a non-empty
- // ech_is_inner extension.
+ // inner ECH extension.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHIsInner-NotEmpty",
+ name: prefix + "ECH-Server-ECHInner-NotEmpty",
config: Config{
MinVersion: VersionTLS13,
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
- AlwaysSendECHIsInner: true,
- SendInvalidECHIsInner: []byte{42, 42, 42},
+ AlwaysSendECHInner: true,
+ SendInvalidECHInner: []byte{42, 42, 42},
},
},
shouldFail: true,
- expectedLocalError: "remote error: illegal parameter",
+ expectedLocalError: "remote error: error decoding message",
expectedError: ":ERROR_PARSING_EXTENSION:",
})
- // When ech_is_inner extension is absent, the server should not accept ECH.
- testCases = append(testCases, testCase{
- testType: serverTest,
- protocol: protocol,
- name: prefix + "ECH-Server-ECHIsInner-Absent",
- config: Config{
- MinVersion: VersionTLS13,
- MaxVersion: VersionTLS13,
- },
- resumeSession: true,
- })
-
- // Test that a TLS 1.3 server that receives an ech_is_inner extension can
+ // Test that a TLS 1.3 server that receives an inner ECH extension can
// negotiate TLS 1.2 without clobbering the downgrade signal.
if protocol != quic {
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHIsInner-Absent-TLS12",
+ name: prefix + "ECH-Server-ECHInner-Absent-TLS12",
config: Config{
MinVersion: VersionTLS12,
MaxVersion: VersionTLS13,
@@ -17338,7 +17562,7 @@
// Omit supported_versions extension so the server negotiates
// TLS 1.2.
OmitSupportedVersions: true,
- AlwaysSendECHIsInner: true,
+ AlwaysSendECHInner: true,
},
},
// Check that the client sees the TLS 1.3 downgrade signal in
@@ -17348,26 +17572,6 @@
})
}
- // Test that the handshake fails when the server has no ECHConfigs and the
- // ClientHello contains both encrypted_client_hello and ech_is_inner
- // extensions.
- testCases = append(testCases, testCase{
- testType: serverTest,
- protocol: protocol,
- name: prefix + "ECH-Server-Disabled-EncryptedClientHello-ECHIsInner",
- config: Config{
- MinVersion: VersionTLS13,
- MaxVersion: VersionTLS13,
- ClientECHConfig: echConfig.ECHConfig,
- Bugs: ProtocolBugs{
- AlwaysSendECHIsInner: true,
- },
- },
- shouldFail: true,
- expectedLocalError: "remote error: illegal parameter",
- expectedError: ":UNEXPECTED_EXTENSION:",
- })
-
// Test the client can negotiate ECH, with and without HelloRetryRequest.
testCases = append(testCases, testCase{
testType: clientTest,
@@ -17944,9 +18148,7 @@
})
// Test that the client rejects ClientHelloOuter handshakes that attempt
- // to resume the ClientHelloInner's ticket. In draft-ietf-tls-esni-10,
- // the confirmation signal is computed in an odd order, so this requires
- // an explicit check on the client.
+ // to resume the ClientHelloInner's ticket, at TLS 1.2 and TLS 1.3.
testCases = append(testCases, testCase{
testType: clientTest,
protocol: protocol,
@@ -17976,9 +18178,6 @@
expectations: connectionExpectations{echAccepted: true},
resumeExpectations: &connectionExpectations{echAccepted: false},
})
-
- // Test the above, but the server now attempts to resume the
- // ClientHelloInner's ticket at TLS 1.2.
if protocol != quic {
testCases = append(testCases, testCase{
testType: clientTest,
@@ -18398,6 +18597,31 @@
expectedError: ":ECH_REJECTED:",
expectedLocalError: "remote error: ECH required",
})
+
+ // Test that the client checks both HelloRetryRequest and ServerHello
+ // for a confirmation signal.
+ testCases = append(testCases, testCase{
+ testType: clientTest,
+ protocol: protocol,
+ name: prefix + "ECH-Client-HelloRetryRequest-MissingServerHelloConfirmation",
+ config: Config{
+ MinVersion: VersionTLS13,
+ MaxVersion: VersionTLS13,
+ CurvePreferences: []CurveID{CurveP384},
+ ServerECHConfigs: []ServerECHConfig{echConfig},
+ Bugs: ProtocolBugs{
+ ExpectMissingKeyShare: true, // Check we triggered HRR.
+ OmitServerHelloECHConfirmation: true,
+ },
+ },
+ resumeSession: true,
+ flags: []string{
+ "-ech-config-list", base64FlagValue(CreateECHConfigList(echConfig.ECHConfig.Raw)),
+ "-expect-hrr", // Check we triggered HRR.
+ },
+ shouldFail: true,
+ expectedError: ":INCONSISTENT_ECH_NEGOTIATION:",
+ })
}
}
diff --git a/ssl/tls13_client.cc b/ssl/tls13_client.cc
index bd7e63f..af2120c 100644
--- a/ssl/tls13_client.cc
+++ b/ssl/tls13_client.cc
@@ -120,6 +120,54 @@
return true;
}
+static bool is_hello_retry_request(const ParsedServerHello &server_hello) {
+ return Span<const uint8_t>(server_hello.random) == kHelloRetryRequest;
+}
+
+static bool check_ech_confirmation(const SSL_HANDSHAKE *hs, bool *out_accepted,
+ uint8_t *out_alert,
+ const ParsedServerHello &server_hello) {
+ const bool is_hrr = is_hello_retry_request(server_hello);
+ size_t offset;
+ if (is_hrr) {
+ // We check for an unsolicited extension when parsing all of them.
+ SSLExtension ech(TLSEXT_TYPE_encrypted_client_hello);
+ if (!ssl_parse_extensions(&server_hello.extensions, out_alert, {&ech},
+ /*ignore_unknown=*/true)) {
+ return false;
+ }
+ if (!ech.present) {
+ *out_accepted = false;
+ return true;
+ }
+ if (CBS_len(&ech.data) != ECH_CONFIRMATION_SIGNAL_LEN) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ *out_alert = SSL_AD_DECODE_ERROR;
+ return false;
+ }
+ offset = CBS_data(&ech.data) - CBS_data(&server_hello.raw);
+ } else {
+ offset = ssl_ech_confirmation_signal_hello_offset(hs->ssl);
+ }
+
+ if (!hs->selected_ech_config) {
+ *out_accepted = false;
+ return true;
+ }
+
+ uint8_t expected[ECH_CONFIRMATION_SIGNAL_LEN];
+ if (!ssl_ech_accept_confirmation(hs, expected, hs->inner_client_random,
+ hs->inner_transcript, is_hrr,
+ server_hello.raw, offset)) {
+ *out_alert = SSL_AD_INTERNAL_ERROR;
+ return false;
+ }
+
+ *out_accepted = CRYPTO_memcmp(CBS_data(&server_hello.raw) + offset, expected,
+ sizeof(expected)) == 0;
+ return true;
+}
+
static enum ssl_hs_wait_t do_read_hello_retry_request(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
assert(ssl->s3->have_version);
@@ -156,17 +204,46 @@
hs->new_cipher = cipher;
- if (!CBS_mem_equal(&server_hello.random, kHelloRetryRequest,
- SSL3_RANDOM_SIZE)) {
+ const bool is_hrr = is_hello_retry_request(server_hello);
+ if (!hs->transcript.InitHash(ssl_protocol_version(ssl), hs->new_cipher) ||
+ (is_hrr && !hs->transcript.UpdateForHelloRetryRequest())) {
+ return ssl_hs_error;
+ }
+ if (hs->selected_ech_config) {
+ if (!hs->inner_transcript.InitHash(ssl_protocol_version(ssl),
+ hs->new_cipher) ||
+ (is_hrr && !hs->inner_transcript.UpdateForHelloRetryRequest())) {
+ return ssl_hs_error;
+ }
+ }
+
+ // Determine which ClientHello the server is responding to. Run
+ // |check_ech_confirmation| unconditionally, so we validate the extension
+ // contents.
+ bool ech_accepted;
+ if (!check_ech_confirmation(hs, &ech_accepted, &alert, server_hello)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
+ }
+ if (hs->selected_ech_config) {
+ ssl->s3->ech_status = ech_accepted ? ssl_ech_accepted : ssl_ech_rejected;
+ }
+
+ if (!is_hrr) {
hs->tls13_state = state_read_server_hello;
return ssl_hs_ok;
}
+ // The ECH extension, if present, was already parsed by
+ // |check_ech_confirmation|.
SSLExtension cookie(TLSEXT_TYPE_cookie), key_share(TLSEXT_TYPE_key_share),
- supported_versions(TLSEXT_TYPE_supported_versions);
- if (!ssl_parse_extensions(&server_hello.extensions, &alert,
- {&cookie, &key_share, &supported_versions},
- /*ignore_unknown=*/false)) {
+ supported_versions(TLSEXT_TYPE_supported_versions),
+ ech_unused(TLSEXT_TYPE_encrypted_client_hello,
+ hs->selected_ech_config || hs->config->ech_grease_enabled);
+ if (!ssl_parse_extensions(
+ &server_hello.extensions, &alert,
+ {&cookie, &key_share, &supported_versions, &ech_unused},
+ /*ignore_unknown=*/false)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
@@ -221,23 +298,16 @@
}
}
- // We do not know whether ECH was chosen until ServerHello and must
- // concurrently update both transcripts.
- //
- // TODO(https://crbug.com/boringssl/275): A later draft will likely add an ECH
- // signal to HRR and change this.
- if (!hs->transcript.InitHash(ssl_protocol_version(ssl), hs->new_cipher) ||
- !hs->transcript.UpdateForHelloRetryRequest() ||
- !ssl_hash_message(hs, msg)) {
+ // Although we now know whether ClientHelloInner was used, we currently
+ // maintain both transcripts up to ServerHello. We could swap transcripts
+ // early, but then ClientHello construction and |check_ech_confirmation|
+ // become more complex.
+ if (!ssl_hash_message(hs, msg)) {
return ssl_hs_error;
}
- if (hs->selected_ech_config) {
- if (!hs->inner_transcript.InitHash(ssl_protocol_version(ssl),
- hs->new_cipher) ||
- !hs->inner_transcript.UpdateForHelloRetryRequest() ||
- !hs->inner_transcript.Update(msg.raw)) {
- return ssl_hs_error;
- }
+ if (ssl->s3->ech_status == ssl_ech_accepted &&
+ !hs->inner_transcript.Update(msg.raw)) {
+ return ssl_hs_error;
}
// HelloRetryRequest should be the end of the flight.
@@ -267,7 +337,8 @@
// Build the second ClientHelloInner, if applicable. The second ClientHello
// uses an empty string for |enc|.
- if (hs->selected_ech_config && !ssl_encrypt_client_hello(hs, {})) {
+ if (hs->ssl->s3->ech_status == ssl_ech_accepted &&
+ !ssl_encrypt_client_hello(hs, {})) {
return ssl_hs_error;
}
@@ -294,8 +365,7 @@
}
// Forbid a second HelloRetryRequest.
- if (CBS_mem_equal(&server_hello.random, kHelloRetryRequest,
- SSL3_RANDOM_SIZE)) {
+ if (is_hello_retry_request(server_hello)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
return ssl_hs_error;
@@ -308,11 +378,36 @@
return ssl_hs_error;
}
+ if (ssl->s3->ech_status == ssl_ech_accepted) {
+ if (ssl->s3->used_hello_retry_request) {
+ // HelloRetryRequest and ServerHello must accept ECH consistently.
+ bool ech_accepted;
+ if (!check_ech_confirmation(hs, &ech_accepted, &alert, server_hello)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return ssl_hs_error;
+ }
+ if (!ech_accepted) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_INCONSISTENT_ECH_NEGOTIATION);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ return ssl_hs_error;
+ }
+ }
+
+ hs->transcript = std::move(hs->inner_transcript);
+ hs->extensions.sent = hs->inner_extensions_sent;
+ // Report the inner random value through |SSL_get_client_random|.
+ OPENSSL_memcpy(ssl->s3->client_random, hs->inner_client_random,
+ SSL3_RANDOM_SIZE);
+ }
+
OPENSSL_memcpy(ssl->s3->server_random, CBS_data(&server_hello.random),
SSL3_RANDOM_SIZE);
+ // When offering ECH, |ssl->session| is only offered in ClientHelloInner.
+ const bool pre_shared_key_allowed =
+ ssl->session != nullptr && ssl->s3->ech_status != ssl_ech_rejected;
SSLExtension key_share(TLSEXT_TYPE_key_share),
- pre_shared_key(TLSEXT_TYPE_pre_shared_key, ssl->session != nullptr),
+ pre_shared_key(TLSEXT_TYPE_pre_shared_key, pre_shared_key_allowed),
supported_versions(TLSEXT_TYPE_supported_versions);
if (!ssl_parse_extensions(&server_hello.extensions, &alert,
{&key_share, &pre_shared_key, &supported_versions},
@@ -408,48 +503,8 @@
return ssl_hs_error;
}
- if (!tls13_advance_key_schedule(hs, dhe_secret)) {
- return ssl_hs_error;
- }
-
- // Determine whether the server accepted ECH.
- //
- // TODO(https://crbug.com/boringssl/275): This is a bit late in the process of
- // parsing ServerHello. |ssl->session| is only valid for ClientHelloInner, so
- // the decisions made based on PSK need to be double-checked. draft-11 will
- // fix this, at which point this logic can be moved before any processing.
- if (hs->selected_ech_config) {
- uint8_t ech_confirmation[ECH_CONFIRMATION_SIGNAL_LEN];
- if (!hs->inner_transcript.InitHash(ssl_protocol_version(ssl),
- hs->new_cipher) ||
- !ssl_ech_accept_confirmation(hs, ech_confirmation, hs->inner_transcript,
- msg.raw)) {
- return ssl_hs_error;
- }
-
- if (CRYPTO_memcmp(ech_confirmation,
- ssl->s3->server_random + sizeof(ssl->s3->server_random) -
- sizeof(ech_confirmation),
- sizeof(ech_confirmation)) == 0) {
- ssl->s3->ech_status = ssl_ech_accepted;
- hs->transcript = std::move(hs->inner_transcript);
- hs->extensions.sent = hs->inner_extensions_sent;
- // Report the inner random value through |SSL_get_client_random|.
- OPENSSL_memcpy(ssl->s3->client_random, hs->inner_client_random,
- SSL3_RANDOM_SIZE);
- } else {
- // Resuming against the ClientHelloOuter was an unsolicited extension.
- if (pre_shared_key.present) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
- return ssl_hs_error;
- }
- ssl->s3->ech_status = ssl_ech_rejected;
- }
- }
-
-
- if (!ssl_hash_message(hs, msg) ||
+ if (!tls13_advance_key_schedule(hs, dhe_secret) ||
+ !ssl_hash_message(hs, msg) ||
!tls13_derive_handshake_secrets(hs)) {
return ssl_hs_error;
}
diff --git a/ssl/tls13_enc.cc b/ssl/tls13_enc.cc
index 9c54a4d..6942887 100644
--- a/ssl/tls13_enc.cc
+++ b/ssl/tls13_enc.cc
@@ -537,57 +537,46 @@
ECH_CONFIRMATION_SIGNAL_LEN;
}
-bool ssl_ech_accept_confirmation(
- const SSL_HANDSHAKE *hs, bssl::Span<uint8_t> out,
- const SSLTranscript &transcript,
- bssl::Span<const uint8_t> server_hello) {
- // We hash |server_hello|, with the last |ECH_CONFIRMATION_SIGNAL_LEN| bytes
- // of the random value zeroed.
- static const uint8_t kZeroes[ECH_CONFIRMATION_SIGNAL_LEN] = {0};
- const size_t offset = ssl_ech_confirmation_signal_hello_offset(hs->ssl);
- if (server_hello.size() < offset + ECH_CONFIRMATION_SIGNAL_LEN) {
+bool ssl_ech_accept_confirmation(const SSL_HANDSHAKE *hs, Span<uint8_t> out,
+ Span<const uint8_t> client_random,
+ const SSLTranscript &transcript, bool is_hrr,
+ Span<const uint8_t> msg, size_t offset) {
+ // See draft-ietf-tls-esni-13, sections 7.2 and 7.2.1.
+ static const uint8_t kZeros[EVP_MAX_MD_SIZE] = {0};
+
+ // We hash |msg|, with bytes from |offset| zeroed.
+ if (msg.size() < offset + ECH_CONFIRMATION_SIGNAL_LEN) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
- auto before_zeroes = server_hello.subspan(0, offset);
- auto after_zeroes =
- server_hello.subspan(offset + ECH_CONFIRMATION_SIGNAL_LEN);
- uint8_t context_hash[EVP_MAX_MD_SIZE];
- unsigned context_hash_len;
+ auto before_zeros = msg.subspan(0, offset);
+ auto after_zeros = msg.subspan(offset + ECH_CONFIRMATION_SIGNAL_LEN);
+ uint8_t context[EVP_MAX_MD_SIZE];
+ unsigned context_len;
ScopedEVP_MD_CTX ctx;
if (!transcript.CopyToHashContext(ctx.get(), transcript.Digest()) ||
- !EVP_DigestUpdate(ctx.get(), before_zeroes.data(),
- before_zeroes.size()) ||
- !EVP_DigestUpdate(ctx.get(), kZeroes, sizeof(kZeroes)) ||
- !EVP_DigestUpdate(ctx.get(), after_zeroes.data(), after_zeroes.size()) ||
- !EVP_DigestFinal_ex(ctx.get(), context_hash, &context_hash_len)) {
+ !EVP_DigestUpdate(ctx.get(), before_zeros.data(), before_zeros.size()) ||
+ !EVP_DigestUpdate(ctx.get(), kZeros, ECH_CONFIRMATION_SIGNAL_LEN) ||
+ !EVP_DigestUpdate(ctx.get(), after_zeros.data(), after_zeros.size()) ||
+ !EVP_DigestFinal_ex(ctx.get(), context, &context_len)) {
return false;
}
- // 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.
- //
- // TODO(https://crbug.com/boringssl/275): draft-11 will avoid this.
- uint8_t accept_confirmation_buf[EVP_MAX_MD_SIZE];
- bssl::Span<uint8_t> accept_confirmation =
- MakeSpan(accept_confirmation_buf, transcript.DigestLen());
- if (!hkdf_expand_label(accept_confirmation, transcript.Digest(),
- hs->secret(), label_to_span("ech accept confirmation"),
- MakeConstSpan(context_hash, context_hash_len))) {
+ uint8_t secret[EVP_MAX_MD_SIZE];
+ size_t secret_len;
+ if (!HKDF_extract(secret, &secret_len, transcript.Digest(), kZeros,
+ transcript.DigestLen(), client_random.data(),
+ client_random.size())) {
return false;
}
- static_assert(ECH_CONFIRMATION_SIGNAL_LEN < EVP_MAX_MD_SIZE,
- "ECH confirmation signal too big");
- if (out.size() != ECH_CONFIRMATION_SIGNAL_LEN) {
- OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
- return false;
- }
- OPENSSL_memcpy(out.data(), accept_confirmation.data(), out.size());
- return true;
+ assert(out.size() == ECH_CONFIRMATION_SIGNAL_LEN);
+ return hkdf_expand_label(out, transcript.Digest(),
+ MakeConstSpan(secret, secret_len),
+ is_hrr ? label_to_span("hrr ech accept confirmation")
+ : label_to_span("ech accept confirmation"),
+ MakeConstSpan(context, context_len));
}
BSSL_NAMESPACE_END
diff --git a/ssl/tls13_server.cc b/ssl/tls13_server.cc
index 79968bc..2f000e5 100644
--- a/ssl/tls13_server.cc
+++ b/ssl/tls13_server.cc
@@ -570,12 +570,34 @@
!CBB_add_u16(&extensions, ssl->version) ||
!CBB_add_u16(&extensions, TLSEXT_TYPE_key_share) ||
!CBB_add_u16(&extensions, 2 /* length */) ||
- !CBB_add_u16(&extensions, group_id) ||
- !ssl_add_message_cbb(ssl, cbb.get())) {
+ !CBB_add_u16(&extensions, group_id)) {
return ssl_hs_error;
}
+ if (hs->ech_is_inner) {
+ // Fill a placeholder for the ECH confirmation value.
+ if (!CBB_add_u16(&extensions, TLSEXT_TYPE_encrypted_client_hello) ||
+ !CBB_add_u16(&extensions, ECH_CONFIRMATION_SIGNAL_LEN) ||
+ !CBB_add_zeros(&extensions, ECH_CONFIRMATION_SIGNAL_LEN)) {
+ return ssl_hs_error;
+ }
+ }
+ Array<uint8_t> hrr;
+ if (!ssl->method->finish_message(ssl, cbb.get(), &hrr)) {
+ return ssl_hs_error;
+ }
+ if (hs->ech_is_inner) {
+ // Now that the message is encoded, fill in the whole value.
+ size_t offset = hrr.size() - ECH_CONFIRMATION_SIGNAL_LEN;
+ if (!ssl_ech_accept_confirmation(
+ hs, MakeSpan(hrr).last(ECH_CONFIRMATION_SIGNAL_LEN),
+ ssl->s3->client_random, hs->transcript, /*is_hrr=*/true, hrr,
+ offset)) {
+ return ssl_hs_error;
+ }
+ }
- if (!ssl->method->add_change_cipher_spec(ssl)) {
+ if (!ssl->method->add_message(ssl, std::move(hrr)) ||
+ !ssl->method->add_change_cipher_spec(ssl)) {
return ssl_hs_error;
}
@@ -601,8 +623,8 @@
}
if (ssl->s3->ech_status == ssl_ech_accepted) {
- // If we previously accepted the ClientHelloInner, check that the second
- // ClientHello contains an encrypted_client_hello extension.
+ // If we previously accepted the ClientHelloInner, the second ClientHello
+ // must contain an outer encrypted_client_hello extension.
CBS ech_body;
if (!ssl_client_hello_get_extension(&client_hello, &ech_body,
TLSEXT_TYPE_encrypted_client_hello)) {
@@ -610,12 +632,12 @@
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
return ssl_hs_error;
}
-
- // Parse a ClientECH out of the extension body.
uint16_t kdf_id, aead_id;
- uint8_t config_id;
+ uint8_t type, config_id;
CBS enc, payload;
- if (!CBS_get_u16(&ech_body, &kdf_id) || //
+ if (!CBS_get_u8(&ech_body, &type) || //
+ type != ECH_CLIENT_OUTER || //
+ !CBS_get_u16(&ech_body, &kdf_id) || //
!CBS_get_u16(&ech_body, &aead_id) ||
!CBS_get_u8(&ech_body, &config_id) ||
!CBS_get_u16_length_prefixed(&ech_body, &enc) ||
@@ -626,8 +648,6 @@
return ssl_hs_error;
}
- // Check that ClientECH.cipher_suite is unchanged and that
- // ClientECH.enc is empty.
if (kdf_id != EVP_HPKE_KDF_id(EVP_HPKE_CTX_kdf(hs->ech_hpke_ctx.get())) ||
aead_id !=
EVP_HPKE_AEAD_id(EVP_HPKE_CTX_aead(hs->ech_hpke_ctx.get())) ||
@@ -640,9 +660,9 @@
// Decrypt the payload with the HPKE context from the first ClientHello.
Array<uint8_t> encoded_client_hello_inner;
bool unused;
- if (!ssl_client_hello_decrypt(
- hs->ech_hpke_ctx.get(), &encoded_client_hello_inner, &unused,
- &client_hello, kdf_id, aead_id, config_id, enc, payload)) {
+ if (!ssl_client_hello_decrypt(hs->ech_hpke_ctx.get(),
+ &encoded_client_hello_inner, &unused,
+ &client_hello, payload)) {
// Decryption failure is fatal in the second ClientHello.
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR);
@@ -760,17 +780,18 @@
return ssl_hs_error;
}
- assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner_present);
- if (hs->ech_is_inner_present) {
+ assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner);
+ if (hs->ech_is_inner) {
// Fill in the ECH confirmation signal.
+ const size_t offset = ssl_ech_confirmation_signal_hello_offset(ssl);
Span<uint8_t> random_suffix = random.last(ECH_CONFIRMATION_SIGNAL_LEN);
- if (!ssl_ech_accept_confirmation(hs, random_suffix, hs->transcript,
- server_hello)) {
+ if (!ssl_ech_accept_confirmation(hs, random_suffix, ssl->s3->client_random,
+ hs->transcript,
+ /*is_hrr=*/false, server_hello, offset)) {
return ssl_hs_error;
}
// Update |server_hello|.
- const size_t offset = ssl_ech_confirmation_signal_hello_offset(ssl);
Span<uint8_t> server_hello_out =
MakeSpan(server_hello).subspan(offset, ECH_CONFIRMATION_SIGNAL_LEN);
OPENSSL_memcpy(server_hello_out.data(), random_suffix.data(),
