Revert "external/boringssl: Sync to 66e61c577d39e757bf491468f651..."
Revert submission 1835013-bssl_update_sep2021
Reason for revert: DroidMonitor: Potential culprit for Bug 201683809 - verifying through Forrest before revert submission. This is part of the standard investigation process, and does not mean your CL will be reverted.
Reverted Changes:
I27d7b79e3:Fix wpa_supplicant build with newer BoringSSL
I4f2228ef8:external/boringssl: Sync to 66e61c577d39e757bf4914...
Change-Id: If2184c4aa55b7dc89e037362e4d5cbbea1107ae2
diff --git a/src/ssl/encrypted_client_hello.cc b/src/ssl/encrypted_client_hello.cc
index 64fee3d..b70f66c 100644
--- a/src/ssl/encrypted_client_hello.cc
+++ b/src/ssl/encrypted_client_hello.cc
@@ -31,6 +31,12 @@
#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.
@@ -78,17 +84,159 @@
return true;
}
-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;
+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
CBS 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,
+ 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,
TLSEXT_TYPE_supported_versions)) {
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER);
@@ -119,131 +267,6 @@
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);
@@ -252,31 +275,56 @@
return true;
}
-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) {
+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) {
*out_is_decrypt_error = false;
- // 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))) {
+ // 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);
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());
+ 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;
+ }
#if defined(BORINGSSL_UNSAFE_FUZZER_MODE)
// In fuzzer mode, disable encryption to improve coverage. We reserve a short
@@ -288,75 +336,124 @@
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
return false;
}
- if (!out->CopyFrom(payload)) {
+ if (!out_encoded_client_hello_inner->CopyFrom(payload)) {
return false;
}
#else
- // Attempt to decrypt into |out|.
- if (!out->Init(payload.size())) {
+ // Attempt to decrypt into |out_encoded_client_hello_inner|.
+ if (!out_encoded_client_hello_inner->Init(payload.size())) {
OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE);
return false;
}
- size_t len;
- if (!EVP_HPKE_CTX_open(hpke_ctx, out->data(), &len, out->size(),
- payload.data(), payload.size(), aad.data(),
- aad.size())) {
+ 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()))) {
*out_is_decrypt_error = true;
OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED);
return false;
}
- out->Shrink(len);
+ out_encoded_client_hello_inner->Shrink(encoded_client_hello_inner_len);
#endif
return true;
}
-static bool is_hex_component(Span<const uint8_t> in) {
- if (in.size() < 2 || in[0] != '0' || (in[1] != 'x' && in[1] != 'X')) {
- return false;
+static bool parse_ipv4_number(Span<const uint8_t> in, uint32_t *out) {
+ // See https://url.spec.whatwg.org/#ipv4-number-parser.
+ uint32_t base = 10;
+ if (in.size() >= 2 && in[0] == '0' && (in[1] == 'x' || in[1] == 'X')) {
+ in = in.subspan(2);
+ base = 16;
+ } else if (in.size() >= 1 && in[0] == '0') {
+ in = in.subspan(1);
+ base = 8;
}
- for (uint8_t b : in.subspan(2)) {
- if (!('0' <= b && b <= '9') && !('a' <= b && b <= 'f') &&
- !('A' <= b && b <= 'F')) {
+ *out = 0;
+ for (uint8_t c : in) {
+ uint32_t d;
+ if ('0' <= c && c <= '9') {
+ d = c - '0';
+ } else if ('a' <= c && c <= 'f') {
+ d = c - 'a' + 10;
+ } else if ('A' <= c && c <= 'F') {
+ d = c - 'A' + 10;
+ } else {
return false;
}
+ if (d >= base ||
+ *out > UINT32_MAX / base) {
+ return false;
+ }
+ *out *= base;
+ if (*out > UINT32_MAX - d) {
+ return false;
+ }
+ *out += d;
}
return true;
}
-static bool is_decimal_component(Span<const uint8_t> in) {
- if (in.empty()) {
+static bool is_ipv4_address(Span<const uint8_t> in) {
+ // See https://url.spec.whatwg.org/#concept-ipv4-parser
+ uint32_t numbers[4];
+ size_t num_numbers = 0;
+ while (!in.empty()) {
+ if (num_numbers == 4) {
+ // Too many components.
+ return false;
+ }
+ // Find the next dot-separated component.
+ auto dot = std::find(in.begin(), in.end(), '.');
+ if (dot == in.begin()) {
+ // Empty components are not allowed.
+ return false;
+ }
+ Span<const uint8_t> component;
+ if (dot == in.end()) {
+ component = in;
+ in = Span<const uint8_t>();
+ } else {
+ component = in.subspan(0, dot - in.begin());
+ in = in.subspan(dot - in.begin() + 1); // Skip the dot.
+ }
+ if (!parse_ipv4_number(component, &numbers[num_numbers])) {
+ return false;
+ }
+ num_numbers++;
+ }
+ if (num_numbers == 0) {
return false;
}
- for (uint8_t b : in) {
- if (!('0' <= b && b <= '9')) {
+ for (size_t i = 0; i < num_numbers - 1; i++) {
+ if (numbers[i] > 255) {
return false;
}
}
- return true;
+ return num_numbers == 1 ||
+ numbers[num_numbers - 1] < 1u << (8 * (5 - num_numbers));
}
bool ssl_is_valid_ech_public_name(Span<const uint8_t> public_name) {
- // See draft-ietf-tls-esni-13, Section 4 and RFC 5890, Section 2.3.1. The
+ // See draft-ietf-tls-esni-11, Section 4 and RFC5890, 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 remaining = public_name;
- if (remaining.empty()) {
+ auto copy = public_name;
+ if (copy.empty()) {
return false;
}
- Span<const uint8_t> last;
- while (!remaining.empty()) {
+ while (!copy.empty()) {
// Find the next dot-separated component.
- auto dot = std::find(remaining.begin(), remaining.end(), '.');
+ auto dot = std::find(copy.begin(), copy.end(), '.');
Span<const uint8_t> component;
- if (dot == remaining.end()) {
- component = remaining;
- last = component;
- remaining = Span<const uint8_t>();
+ if (dot == copy.end()) {
+ component = copy;
+ copy = Span<const uint8_t>();
} else {
- component = remaining.subspan(0, dot - remaining.begin());
- // Skip the dot.
- remaining = remaining.subspan(dot - remaining.begin() + 1);
- if (remaining.empty()) {
+ component = copy.subspan(0, dot - copy.begin());
+ copy = copy.subspan(dot - copy.begin() + 1); // Skip the dot.
+ if (copy.empty()) {
// Trailing dots are not allowed.
return false;
}
@@ -375,15 +472,7 @@
}
}
- // The WHATWG URL parser additionally does not allow any DNS names that end in
- // a numeric component. See:
- // https://url.spec.whatwg.org/#concept-host-parser
- // https://url.spec.whatwg.org/#ends-in-a-number-checker
- //
- // The WHATWG parser is formulated in terms of parsing decimal, octal, and
- // hex, along with a separate ASCII digits check. The ASCII digits check
- // subsumes the decimal and octal check, so we only need to check two cases.
- return !is_hex_component(last) && !is_decimal_component(last);
+ return !is_ipv4_address(public_name);
}
static bool parse_ech_config(CBS *cbs, ECHConfig *out, bool *out_supported,
@@ -419,8 +508,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_u8(&contents, &out->maximum_name_length) ||
- !CBS_get_u8_length_prefixed(&contents, &public_name) ||
+ !CBS_get_u16(&contents, &out->maximum_name_length) ||
+ !CBS_get_u16_length_prefixed(&contents, &public_name) ||
CBS_len(&public_name) == 0 ||
!CBS_get_u16_length_prefixed(&contents, &extensions) ||
CBS_len(&contents) != 0) {
@@ -684,6 +773,15 @@
#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);
@@ -716,32 +814,38 @@
// 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
- // 5 inner encrypted_client_hello
+ // 4 ech_is_inner
// 4+1+2*2 supported_versions (TLS 1.3, GREASE)
// 4+1+10*2 outer_extensions (key_share, sigalgs, sct, alpn,
// supported_groups, status_request, psk_key_exchange_modes,
// compress_certificate, GREASE x2)
//
// The server_name extension has an overhead of 9 bytes. For now, arbitrarily
- // 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);
+ // 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);
bssl::ScopedCBB cbb;
CBB enc_cbb, payload_cbb;
uint8_t *payload;
- if (!CBB_init(cbb.get(), 256) ||
+ if (!CBB_init(cbb.get(), extension_len) ||
!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, payload_len) ||
- !RAND_bytes(payload, payload_len) ||
- !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) {
+ !CBB_add_space(&payload_cbb, &payload, in_len + overhead) ||
+ !RAND_bytes(payload, in_len + overhead) ||
+ !CBBFinishArray(cbb.get(), &hs->ech_client_bytes)) {
return false;
}
+ assert(hs->ech_client_bytes.size() == extension_len);
return true;
}
@@ -752,22 +856,22 @@
}
// Construct ClientHelloInner and EncodedClientHelloInner. See
- // draft-ietf-tls-esni-13, sections 5.1 and 6.1.
- ScopedCBB cbb, encoded_cbb;
+ // draft-ietf-tls-esni-10, sections 5.1 and 6.1.
+ bssl::ScopedCBB cbb, encoded;
CBB body;
bool needs_psk_binder;
- Array<uint8_t> hello_inner;
+ bssl::Array<uint8_t> hello_inner;
if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) ||
- !CBB_init(encoded_cbb.get(), 256) ||
+ !CBB_init(encoded.get(), 256) ||
!ssl_write_client_hello_without_extensions(hs, &body,
ssl_client_hello_inner,
/*empty_session_id=*/false) ||
- !ssl_write_client_hello_without_extensions(hs, encoded_cbb.get(),
+ !ssl_write_client_hello_without_extensions(hs, encoded.get(),
ssl_client_hello_inner,
/*empty_session_id=*/true) ||
- !ssl_add_clienthello_tlsext(hs, &body, encoded_cbb.get(),
- &needs_psk_binder, ssl_client_hello_inner,
- CBB_len(&body)) ||
+ !ssl_add_clienthello_tlsext(hs, &body, encoded.get(), &needs_psk_binder,
+ ssl_client_hello_inner, CBB_len(&body),
+ /*omit_ech_len=*/0) ||
!ssl->method->finish_message(ssl, cbb.get(), &hello_inner)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
@@ -780,12 +884,13 @@
return false;
}
// Also update the EncodedClientHelloInner.
- 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(),
+ if (CBB_len(encoded.get()) < binder_len) {
+ OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
+ return false;
+ }
+ OPENSSL_memcpy(const_cast<uint8_t *>(CBB_data(encoded.get())) +
+ CBB_len(encoded.get()) - binder_len,
+ hello_inner.data() + hello_inner.size() - binder_len,
binder_len);
}
@@ -793,82 +898,74 @@
return false;
}
- // 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;
- }
-
- // 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.
+ // 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());
- size_t payload_len = encoded.size() + aead_overhead(aead);
- CBB enc_cbb, payload_cbb;
- if (!CBB_init(cbb.get(), 256) ||
+ const size_t extension_len =
+ compute_extension_length(aead, enc.size(), CBB_len(encoded.get()));
+ 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);
+ 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) ||
!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) ||
- !CBB_add_zeros(&payload_cbb, payload_len) ||
- !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) {
+ !CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb)) {
return false;
}
-
- // 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());
+ if (!CBB_add_bytes(&payload_cbb, CBB_data(encoded.get()),
+ CBB_len(encoded.get()))) {
+ return false;
+ }
#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()),
+ uint8_t *payload;
+ size_t payload_len =
+ CBB_len(encoded.get()) + 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, CBB_data(encoded.get()),
+ CBB_len(encoded.get()), CBB_data(aad.get()),
CBB_len(aad.get())) ||
- payload_len != payload_span.size()) {
+ !CBB_did_write(&payload_cbb, payload_len)) {
return false;
}
#endif // BORINGSSL_UNSAFE_FUZZER_MODE
+ if (!CBBFinishArray(cbb.get(), &hs->ech_client_bytes)) {
+ return false;
+ }
+ // The |aad| calculation relies on |extension_length| being correct.
+ assert(hs->ech_client_bytes.size() == extension_len);
return true;
}
@@ -948,13 +1045,7 @@
return 0;
}
- // 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.
+ // See draft-ietf-tls-esni-10, section 4.
ScopedCBB cbb;
CBB contents, child;
uint8_t *public_key;
@@ -975,8 +1066,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_u8(&contents, max_name_len) ||
- !CBB_add_u8_length_prefixed(&contents, &child) ||
+ !CBB_add_u16(&contents, max_name_len) ||
+ !CBB_add_u16_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/src/ssl/extensions.cc b/src/ssl/extensions.cc
index 3baef6d..4950d26 100644
--- a/src/ssl/extensions.cc
+++ b/src/ssl/extensions.cc
@@ -210,24 +210,16 @@
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 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 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_len(&session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH) {
return false;
}
@@ -240,16 +232,16 @@
// Skip past DTLS cookie
if (SSL_is_dtls(out->ssl)) {
CBS cookie;
- if (!CBS_get_u8_length_prefixed(cbs, &cookie) ||
+ if (!CBS_get_u8_length_prefixed(&client_hello, &cookie) ||
CBS_len(&cookie) > DTLS1_COOKIE_LENGTH) {
return false;
}
}
CBS cipher_suites, compression_methods;
- if (!CBS_get_u16_length_prefixed(cbs, &cipher_suites) ||
+ if (!CBS_get_u16_length_prefixed(&client_hello, &cipher_suites) ||
CBS_len(&cipher_suites) < 2 || (CBS_len(&cipher_suites) & 1) != 0 ||
- !CBS_get_u8_length_prefixed(cbs, &compression_methods) ||
+ !CBS_get_u8_length_prefixed(&client_hello, &compression_methods) ||
CBS_len(&compression_methods) < 1) {
return false;
}
@@ -261,22 +253,23 @@
// If the ClientHello ends here then it's valid, but doesn't have any
// extensions.
- if (CBS_len(cbs) == 0) {
- out->extensions = nullptr;
+ if (CBS_len(&client_hello) == 0) {
+ out->extensions = NULL;
out->extensions_len = 0;
- } 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);
+ return true;
}
- out->client_hello = CBS_data(©);
- out->client_hello_len = CBS_len(©) - CBS_len(cbs);
+ // 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);
+
return true;
}
@@ -626,30 +619,20 @@
// Encrypted ClientHello (ECH)
//
-// https://tools.ietf.org/html/draft-ietf-tls-esni-13
+// https://tools.ietf.org/html/draft-ietf-tls-esni-10
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) {
- 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()) {
+ if (type == ssl_client_hello_inner || hs->ech_client_bytes.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_u8(&ech_body, ECH_CLIENT_OUTER) ||
- !CBB_add_bytes(&ech_body, hs->ech_client_outer.data(),
- hs->ech_client_outer.size()) ||
+ !CBB_add_bytes(&ech_body, hs->ech_client_bytes.data(),
+ hs->ech_client_bytes.size()) ||
!CBB_flush(out)) {
return false;
}
@@ -664,10 +647,8 @@
}
// The ECH extension may not be sent in TLS 1.2 ServerHello, only TLS 1.3
- // 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) {
+ // EncryptedExtension.
+ if (ssl_protocol_version(ssl) < TLS1_3_VERSION) {
*out_alert = SSL_AD_UNSUPPORTED_EXTENSION;
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
return false;
@@ -678,7 +659,17 @@
return false;
}
- if (ssl->s3->ech_status == ssl_ech_rejected &&
+ // 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 &&
!hs->ech_retry_configs.CopyFrom(*contents)) {
*out_alert = SSL_AD_INTERNAL_ERROR;
return false;
@@ -689,23 +680,10 @@
static bool ext_ech_parse_clienthello(SSL_HANDSHAKE *hs, uint8_t *out_alert,
CBS *contents) {
- if (contents == nullptr) {
+ if (contents != nullptr) {
+ hs->ech_present = true;
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;
}
@@ -737,6 +715,32 @@
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.
//
@@ -1938,10 +1942,13 @@
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 ||
- // 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.
+ // 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
type == ssl_client_hello_outer) {
return false;
}
@@ -1984,6 +1991,7 @@
// Fill in a placeholder zero binder of the appropriate length. It will be
// computed and filled in later after length prefixes are computed.
+ uint8_t zero_binder[EVP_MAX_MD_SIZE] = {0};
size_t binder_len = EVP_MD_size(ssl_session_get_digest(ssl->session.get()));
CBB contents, identity, ticket, binders, binder;
@@ -1996,7 +2004,7 @@
!CBB_add_u32(&identity, obfuscated_ticket_age) ||
!CBB_add_u16_length_prefixed(&contents, &binders) ||
!CBB_add_u8_length_prefixed(&binders, &binder) ||
- !CBB_add_zeros(&binder, binder_len)) {
+ !CBB_add_bytes(&binder, zero_binder, binder_len)) {
return false;
}
@@ -2176,7 +2184,10 @@
// If offering ECH, the extension only applies to ClientHelloInner, but we
// send the extension in both ClientHellos. This ensures that, if the server
// handshakes with ClientHelloOuter, it can skip past early data. See
- // draft-ietf-tls-esni-13, section 6.1.
+ // https://github.com/tlswg/draft-ietf-tls-esni/pull/415
+ //
+ // TODO(https://crbug.com/boringssl/275): Replace this with a reference to the
+ // right section in the next draft.
if (!CBB_add_u16(out_compressible, TLSEXT_TYPE_early_data) ||
!CBB_add_u16(out_compressible, 0) ||
!CBB_flush(out_compressible)) {
@@ -3100,6 +3111,13 @@
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,
@@ -3306,12 +3324,14 @@
static bool add_padding_extension(CBB *cbb, uint16_t ext, size_t len) {
CBB child;
+ uint8_t *ptr;
if (!CBB_add_u16(cbb, ext) || //
!CBB_add_u16_length_prefixed(cbb, &child) ||
- !CBB_add_zeros(&child, len)) {
+ !CBB_add_space(&child, &ptr, len)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
+ OPENSSL_memset(ptr, 0, len);
return CBB_flush(cbb);
}
@@ -3384,6 +3404,34 @@
}
}
+ // 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))) {
@@ -3425,8 +3473,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) {
+ ssl_client_hello_type_t type, size_t header_len,
+ size_t omit_ech_len) {
*out_needs_psk_binder = false;
if (type == ssl_client_hello_inner) {
@@ -3459,14 +3507,20 @@
size_t i = hs->extension_permutation.empty()
? unpermuted
: hs->extension_permutation[unpermuted];
- 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;
- }
+ 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;
+ }
- const size_t bytes_written = CBB_len(&extensions) - len_before;
+ bytes_written = CBB_len(&extensions) - len_before;
+ }
if (bytes_written != 0) {
hs->extensions.sent |= (1u << i);
}
@@ -3490,8 +3544,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) + psk_extension_len;
+ header_len += SSL3_HM_HEADER_LENGTH + 2 + CBB_len(&extensions) +
+ omit_ech_len + psk_extension_len;
size_t padding_len = 0;
// The final extension must be non-empty. WebSphere Application
@@ -3665,10 +3719,18 @@
return true;
}
-static bool ssl_scan_serverhello_tlsext(SSL_HANDSHAKE *hs, const CBS *cbs,
+static bool ssl_scan_serverhello_tlsext(SSL_HANDSHAKE *hs, CBS *cbs,
int *out_alert) {
- CBS extensions = *cbs;
- if (!tls1_check_duplicate_extensions(&extensions)) {
+ SSL *const ssl = hs->ssl;
+ // Before TLS 1.3, ServerHello extensions blocks may be omitted if empty.
+ if (CBS_len(cbs) == 0 && ssl_protocol_version(ssl) < TLS1_3_VERSION) {
+ return true;
+ }
+
+ // Decode the extensions block and check it is valid.
+ CBS extensions;
+ if (!CBS_get_u16_length_prefixed(cbs, &extensions) ||
+ !tls1_check_duplicate_extensions(&extensions)) {
*out_alert = SSL_AD_DECODE_ERROR;
return false;
}
@@ -3790,7 +3852,7 @@
return true;
}
-bool ssl_parse_serverhello_tlsext(SSL_HANDSHAKE *hs, const CBS *cbs) {
+bool ssl_parse_serverhello_tlsext(SSL_HANDSHAKE *hs, CBS *cbs) {
SSL *const ssl = hs->ssl;
int alert = SSL_AD_DECODE_ERROR;
if (!ssl_scan_serverhello_tlsext(hs, cbs, &alert)) {
@@ -3818,8 +3880,8 @@
return ssl_ticket_aead_ignore_ticket;
}
// Split the ticket into the ticket and the MAC.
- auto ticket_mac = ticket.last(mac_len);
- ticket = ticket.first(ticket.size() - mac_len);
+ auto ticket_mac = ticket.subspan(ticket.size() - mac_len);
+ ticket = ticket.subspan(0, ticket.size() - mac_len);
HMAC_Update(hmac_ctx, ticket.data(), ticket.size());
HMAC_Final(hmac_ctx, mac, NULL);
assert(mac_len == ticket_mac.size());
diff --git a/src/ssl/handshake.cc b/src/ssl/handshake.cc
index fc85e21..db4ee71 100644
--- a/src/ssl/handshake.cc
+++ b/src/ssl/handshake.cc
@@ -126,7 +126,8 @@
SSL_HANDSHAKE::SSL_HANDSHAKE(SSL *ssl_arg)
: ssl(ssl_arg),
- ech_is_inner(false),
+ ech_present(false),
+ ech_is_inner_present(false),
ech_authenticated_reject(false),
scts_requested(false),
handshake_finalized(false),
@@ -267,15 +268,12 @@
}
bool ssl_parse_extensions(const CBS *cbs, uint8_t *out_alert,
- std::initializer_list<SSLExtension *> extensions,
+ Span<const SSL_EXTENSION_TYPE> ext_types,
bool ignore_unknown) {
// Reset everything.
- for (SSLExtension *ext : extensions) {
- ext->present = false;
- CBS_init(&ext->data, nullptr, 0);
- if (!ext->allowed) {
- assert(!ignore_unknown);
- }
+ for (const SSL_EXTENSION_TYPE &ext_type : ext_types) {
+ *ext_type.out_present = false;
+ CBS_init(ext_type.out_data, nullptr, 0);
}
CBS copy = *cbs;
@@ -289,10 +287,10 @@
return false;
}
- SSLExtension *found = nullptr;
- for (SSLExtension *ext : extensions) {
- if (type == ext->type && ext->allowed) {
- found = ext;
+ const SSL_EXTENSION_TYPE *found = nullptr;
+ for (const SSL_EXTENSION_TYPE &ext_type : ext_types) {
+ if (type == ext_type.type) {
+ found = &ext_type;
break;
}
}
@@ -307,14 +305,14 @@
}
// Duplicate ext_types are forbidden.
- if (found->present) {
+ if (*found->out_present) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DUPLICATE_EXTENSION);
*out_alert = SSL_AD_ILLEGAL_PARAMETER;
return false;
}
- found->present = true;
- found->data = data;
+ *found->out_present = 1;
+ *found->out_data = data;
}
return true;
diff --git a/src/ssl/handshake_client.cc b/src/ssl/handshake_client.cc
index 17b41e0..ba8f4b7 100644
--- a/src/ssl/handshake_client.cc
+++ b/src/ssl/handshake_client.cc
@@ -333,7 +333,8 @@
!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)) ||
+ &needs_psk_binder, type, CBB_len(&body),
+ /*omit_ech_len=*/0) ||
!ssl->method->finish_message(ssl, cbb.get(), &msg)) {
return false;
}
@@ -353,31 +354,42 @@
return ssl->method->add_message(ssl, std::move(msg));
}
-static bool parse_server_version(const SSL_HANDSHAKE *hs, uint16_t *out_version,
- uint8_t *out_alert,
- const ParsedServerHello &server_hello) {
- // If the outer version is not TLS 1.2, use it.
- // TODO(davidben): This function doesn't quite match the RFC8446 formulation.
- if (server_hello.legacy_version != TLS1_2_VERSION) {
- *out_version = server_hello.legacy_version;
+static bool parse_supported_versions(SSL_HANDSHAKE *hs, uint16_t *version,
+ const CBS *in) {
+ // If the outer version is not TLS 1.2, or there is no extensions block, use
+ // the outer version.
+ if (*version != TLS1_2_VERSION || CBS_len(in) == 0) {
return true;
}
- SSLExtension supported_versions(TLSEXT_TYPE_supported_versions);
- CBS extensions = server_hello.extensions;
- if (!ssl_parse_extensions(&extensions, out_alert, {&supported_versions},
- /*ignore_unknown=*/true)) {
+ SSL *const ssl = hs->ssl;
+ CBS copy = *in, extensions;
+ if (!CBS_get_u16_length_prefixed(©, &extensions) ||
+ CBS_len(©) != 0) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return false;
}
- if (!supported_versions.present) {
- *out_version = server_hello.legacy_version;
- return true;
+ bool have_supported_versions;
+ CBS supported_versions;
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_supported_versions, &have_supported_versions,
+ &supported_versions},
+ };
+
+ uint8_t alert = SSL_AD_DECODE_ERROR;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
+ /*ignore_unknown=*/true)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ return false;
}
- if (!CBS_get_u16(&supported_versions.data, out_version) ||
- CBS_len(&supported_versions.data) != 0) {
- *out_alert = SSL_AD_DECODE_ERROR;
+ // Override the outer version with the extension, if present.
+ if (have_supported_versions &&
+ (!CBS_get_u16(&supported_versions, version) ||
+ CBS_len(&supported_versions) != 0)) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return false;
}
@@ -433,7 +445,7 @@
}
void ssl_done_writing_client_hello(SSL_HANDSHAKE *hs) {
- hs->ech_client_outer.Reset();
+ hs->ech_client_bytes.Reset();
hs->cookie.Reset();
hs->key_share_bytes.Reset();
}
@@ -645,38 +657,6 @@
return ssl_hs_flush;
}
-bool ssl_parse_server_hello(ParsedServerHello *out, uint8_t *out_alert,
- const SSLMessage &msg) {
- if (msg.type != SSL3_MT_SERVER_HELLO) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
- *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) ||
- !CBS_get_u8_length_prefixed(&body, &out->session_id) ||
- CBS_len(&out->session_id) > SSL3_SESSION_ID_SIZE ||
- !CBS_get_u16(&body, &out->cipher_suite) ||
- !CBS_get_u8(&body, &out->compression_method)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- *out_alert = SSL_AD_DECODE_ERROR;
- return false;
- }
- // In TLS 1.2 and below, empty extensions blocks may be omitted. In TLS 1.3,
- // ServerHellos always have extensions, so this can be applied generically.
- CBS_init(&out->extensions, nullptr, 0);
- if ((CBS_len(&body) != 0 &&
- !CBS_get_u16_length_prefixed(&body, &out->extensions)) ||
- CBS_len(&body) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- *out_alert = SSL_AD_DECODE_ERROR;
- return false;
- }
- return true;
-}
-
static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
SSLMessage msg;
@@ -684,12 +664,26 @@
return ssl_hs_read_server_hello;
}
- ParsedServerHello server_hello;
- uint16_t server_version;
- uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!ssl_parse_server_hello(&server_hello, &alert, msg) ||
- !parse_server_version(hs, &server_version, &alert, server_hello)) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_SERVER_HELLO)) {
+ return ssl_hs_error;
+ }
+
+ CBS server_hello = msg.body, server_random, session_id;
+ uint16_t server_version, cipher_suite;
+ uint8_t compression_method;
+ if (!CBS_get_u16(&server_hello, &server_version) ||
+ !CBS_get_bytes(&server_hello, &server_random, SSL3_RANDOM_SIZE) ||
+ !CBS_get_u8_length_prefixed(&server_hello, &session_id) ||
+ CBS_len(&session_id) > SSL3_SESSION_ID_SIZE ||
+ !CBS_get_u16(&server_hello, &cipher_suite) ||
+ !CBS_get_u8(&server_hello, &compression_method)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ // Use the supported_versions extension if applicable.
+ if (!parse_supported_versions(hs, &server_version, &server_hello)) {
return ssl_hs_error;
}
@@ -743,7 +737,7 @@
}
// Copy over the server random.
- OPENSSL_memcpy(ssl->s3->server_random, CBS_data(&server_hello.random),
+ OPENSSL_memcpy(ssl->s3->server_random, CBS_data(&server_random),
SSL3_RANDOM_SIZE);
// Enforce the TLS 1.3 anti-downgrade feature.
@@ -766,26 +760,28 @@
}
}
+ const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite);
+ if (cipher == NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_CIPHER_RETURNED);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ return ssl_hs_error;
+ }
+ hs->new_cipher = cipher;
+
// The cipher must be allowed in the selected version and enabled.
- const SSL_CIPHER *cipher = SSL_get_cipher_by_value(server_hello.cipher_suite);
uint32_t mask_a, mask_k;
ssl_get_client_disabled(hs, &mask_a, &mask_k);
- if (cipher == nullptr ||
- (cipher->algorithm_mkey & mask_k) ||
- (cipher->algorithm_auth & mask_a) ||
+ if ((cipher->algorithm_mkey & mask_k) || (cipher->algorithm_auth & mask_a) ||
SSL_CIPHER_get_min_version(cipher) > ssl_protocol_version(ssl) ||
SSL_CIPHER_get_max_version(cipher) < ssl_protocol_version(ssl) ||
- !sk_SSL_CIPHER_find(SSL_get_ciphers(ssl), nullptr, cipher)) {
+ !sk_SSL_CIPHER_find(SSL_get_ciphers(ssl), NULL, cipher)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
- hs->new_cipher = cipher;
-
if (hs->session_id_len != 0 &&
- CBS_mem_equal(&server_hello.session_id, hs->session_id,
- hs->session_id_len)) {
+ CBS_mem_equal(&session_id, hs->session_id, hs->session_id_len)) {
// Echoing the ClientHello session ID in TLS 1.2, whether from the session
// or a synthetic one, indicates resumption. If there was no session (or if
// the session was only offered in ECH ClientHelloInner), this was the
@@ -803,7 +799,7 @@
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
- if (ssl->session->cipher != hs->new_cipher) {
+ if (ssl->session->cipher != cipher) {
OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
@@ -826,11 +822,10 @@
return ssl_hs_error;
}
// Note: session_id could be empty.
- hs->new_session->session_id_length = CBS_len(&server_hello.session_id);
- OPENSSL_memcpy(hs->new_session->session_id,
- CBS_data(&server_hello.session_id),
- CBS_len(&server_hello.session_id));
- hs->new_session->cipher = hs->new_cipher;
+ hs->new_session->session_id_length = CBS_len(&session_id);
+ OPENSSL_memcpy(hs->new_session->session_id, CBS_data(&session_id),
+ CBS_len(&session_id));
+ hs->new_session->cipher = cipher;
}
// Now that the cipher is known, initialize the handshake hash and hash the
@@ -850,17 +845,26 @@
}
// Only the NULL compression algorithm is supported.
- if (server_hello.compression_method != 0) {
+ if (compression_method != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_COMPRESSION_ALGORITHM);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
- if (!ssl_parse_serverhello_tlsext(hs, &server_hello.extensions)) {
+ // TLS extensions
+ if (!ssl_parse_serverhello_tlsext(hs, &server_hello)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
return ssl_hs_error;
}
+ // There should be nothing left over in the record.
+ if (CBS_len(&server_hello) != 0) {
+ // wrong packet length
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
if (ssl->session != NULL &&
hs->extended_master_secret != ssl->session->extended_master_secret) {
if (ssl->session->extended_master_secret) {
diff --git a/src/ssl/handshake_server.cc b/src/ssl/handshake_server.cc
index fdf9511..c8a23a1 100644
--- a/src/ssl/handshake_server.cc
+++ b/src/ssl/handshake_server.cc
@@ -504,91 +504,6 @@
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;
@@ -668,19 +583,98 @@
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;
- }
-
- // 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;
@@ -757,6 +751,12 @@
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;
}
@@ -973,7 +973,8 @@
}
static void copy_suffix(Span<uint8_t> out, Span<const uint8_t> in) {
- out = out.last(in.size());
+ out = out.subspan(out.size() - in.size());
+ assert(out.size() == in.size());
OPENSSL_memcpy(out.data(), in.data(), in.size());
}
diff --git a/src/ssl/internal.h b/src/ssl/internal.h
index ab23d29..3b7326a 100644
--- a/src/ssl/internal.h
+++ b/src/ssl/internal.h
@@ -146,7 +146,6 @@
#include <stdlib.h>
-#include <initializer_list>
#include <limits>
#include <new>
#include <type_traits>
@@ -694,8 +693,7 @@
// InitHash initializes the handshake hash based on the PRF and contents of
// the handshake transcript. Subsequent calls to |Update| will update the
// rolling hash. It returns one on success and zero on failure. It is an error
- // to call this function after the handshake buffer is released. This may be
- // called multiple times to change the hash function.
+ // to call this function after the handshake buffer is released.
bool InitHash(uint16_t version, const SSL_CIPHER *cipher);
// UpdateForHelloRetryRequest resets the rolling hash with the
@@ -1451,7 +1449,7 @@
Span<const uint8_t> public_name;
Span<const uint8_t> cipher_suites;
uint16_t kem_id = 0;
- uint8_t maximum_name_length = 0;
+ uint16_t maximum_name_length = 0;
uint8_t config_id = 0;
};
@@ -1488,10 +1486,6 @@
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
@@ -1503,13 +1497,18 @@
Span<const uint8_t> encoded_client_hello_inner,
const SSL_CLIENT_HELLO *client_hello_outer);
-// 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,
+// 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,
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
@@ -1519,14 +1518,13 @@
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 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.
+// 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.
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);
+ const SSLTranscript &transcript,
+ Span<const uint8_t> server_hello);
// 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,9 +1830,8 @@
// cookie is the value of the cookie received from the server, if any.
Array<uint8_t> cookie;
- // 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_client_bytes contains the ECH extension to send in the ClientHello.
+ Array<uint8_t> ech_client_bytes;
// ech_retry_configs, on the client, contains the retry configs from the
// server as a serialized ECHConfigList.
@@ -1942,9 +1939,13 @@
// influence the handshake on match.
UniquePtr<SSL_HANDSHAKE_HINTS> hints;
- // ech_is_inner, on the server, indicates whether the ClientHello contained an
- // inner ECH extension.
- bool ech_is_inner : 1;
+ // 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_authenticated_reject, on the client, indicates whether an ECH rejection
// handshake has been authenticated.
@@ -2162,22 +2163,6 @@
// flight. It returns true on success and false on error.
bool ssl_add_client_hello(SSL_HANDSHAKE *hs);
-struct ParsedServerHello {
- CBS raw;
- uint16_t legacy_version = 0;
- CBS random;
- CBS session_id;
- uint16_t cipher_suite = 0;
- uint8_t compression_method = 0;
- CBS extensions;
-};
-
-// ssl_parse_server_hello parses |msg| as a ServerHello. On success, it writes
-// the result to |*out| and returns true. Otherwise, it returns false and sets
-// |*out_alert| to an alert to send to the peer.
-bool ssl_parse_server_hello(ParsedServerHello *out, uint8_t *out_alert,
- const SSLMessage &msg);
-
enum ssl_cert_verify_context_t {
ssl_cert_verify_server,
ssl_cert_verify_client,
@@ -2219,25 +2204,19 @@
bool ssl_negotiate_alps(SSL_HANDSHAKE *hs, uint8_t *out_alert,
const SSL_CLIENT_HELLO *client_hello);
-struct SSLExtension {
- SSLExtension(uint16_t type_arg, bool allowed_arg = true)
- : type(type_arg), allowed(allowed_arg), present(false) {
- CBS_init(&data, nullptr, 0);
- }
-
+struct SSL_EXTENSION_TYPE {
uint16_t type;
- bool allowed;
- bool present;
- CBS data;
+ bool *out_present;
+ CBS *out_data;
};
// ssl_parse_extensions parses a TLS extensions block out of |cbs| and advances
-// it. It writes the parsed extensions to pointers in |extensions|. On success,
-// it fills in the |present| and |data| fields and returns true. Otherwise, it
-// sets |*out_alert| to an alert to send and returns false. Unknown extensions
-// are rejected unless |ignore_unknown| is true.
+// it. It writes the parsed extensions to pointers denoted by |ext_types|. On
+// success, it fills in the |out_present| and |out_data| fields and returns
+// true. Otherwise, it sets |*out_alert| to an alert to send and returns false.
+// Unknown extensions are rejected unless |ignore_unknown| is true.
bool ssl_parse_extensions(const CBS *cbs, uint8_t *out_alert,
- std::initializer_list<SSLExtension *> extensions,
+ Span<const SSL_EXTENSION_TYPE> ext_types,
bool ignore_unknown);
// ssl_verify_peer_cert verifies the peer certificate for |hs|.
@@ -2276,9 +2255,6 @@
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);
@@ -2339,7 +2315,7 @@
#define TLSEXT_CHANNEL_ID_SIZE 128
-// From RFC 4492, used in encoding the curve type in ECParameters
+// From RFC4492, used in encoding the curve type in ECParameters
#define NAMED_CURVE_TYPE 3
struct CERT {
@@ -3316,15 +3292,19 @@
// 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);
+ ssl_client_hello_type_t type, size_t header_len,
+ size_t omit_ech_len);
bool ssl_add_serverhello_tlsext(SSL_HANDSHAKE *hs, CBB *out);
bool ssl_parse_clienthello_tlsext(SSL_HANDSHAKE *hs,
const SSL_CLIENT_HELLO *client_hello);
-bool ssl_parse_serverhello_tlsext(SSL_HANDSHAKE *hs, const CBS *extensions);
+bool ssl_parse_serverhello_tlsext(SSL_HANDSHAKE *hs, CBS *cbs);
#define tlsext_tick_md EVP_sha256
diff --git a/src/ssl/ssl_cipher.cc b/src/ssl/ssl_cipher.cc
index 60b3e2c..4f5049c 100644
--- a/src/ssl/ssl_cipher.cc
+++ b/src/ssl/ssl_cipher.cc
@@ -234,7 +234,7 @@
SSL_HANDSHAKE_MAC_DEFAULT,
},
- // GCM ciphersuites from RFC 5288
+ // GCM ciphersuites from RFC5288
// Cipher 9C
{
@@ -346,7 +346,7 @@
SSL_HANDSHAKE_MAC_DEFAULT,
},
- // GCM based TLS v1.2 ciphersuites from RFC 5289
+ // GCM based TLS v1.2 ciphersuites from RFC5289
// Cipher C02B
{
diff --git a/src/ssl/ssl_lib.cc b/src/ssl/ssl_lib.cc
index 03864e1..3c9fc90 100644
--- a/src/ssl/ssl_lib.cc
+++ b/src/ssl/ssl_lib.cc
@@ -1023,7 +1023,7 @@
int SSL_peek(SSL *ssl, void *buf, int num) {
if (ssl->quic_method != nullptr) {
OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
- return -1;
+ return 0;
}
int ret = ssl_read_impl(ssl);
@@ -1044,7 +1044,7 @@
if (ssl->quic_method != nullptr) {
OPENSSL_PUT_ERROR(SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
- return -1;
+ return 0;
}
if (ssl->do_handshake == NULL) {
diff --git a/src/ssl/ssl_test.cc b/src/ssl/ssl_test.cc
index 7ab5054..76f88c7 100644
--- a/src/ssl/ssl_test.cc
+++ b/src/ssl/ssl_test.cc
@@ -1675,8 +1675,8 @@
return false;
}
}
- if (!CBB_add_u8(&contents, params.max_name_len) ||
- !CBB_add_u8_length_prefixed(&contents, &child) ||
+ if (!CBB_add_u16(&contents, params.max_name_len) ||
+ !CBB_add_u16_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, 0x0d,
+ 0xfe, 0x0a,
// length
- 0x00, 0x41,
+ 0x00, 0x43,
// 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
- 0x10,
+ 0x00, 0x10,
// contents.public_name
- 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61, 0x6d,
- 0x70, 0x6c, 0x65,
+ 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e, 0x65, 0x78, 0x61,
+ 0x6d, 0x70, 0x6c, 0x65,
// contents.extensions
0x00, 0x00};
uint8_t *ech_config;
@@ -2069,26 +2069,20 @@
EXPECT_EQ(client_hello_len, client_hello_len_baseline);
EXPECT_EQ(ech_len, 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);
+ 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);
- for (size_t name_len = 128 + 1; name_len < 128 + 32; name_len++) {
+ // 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}) {
SCOPED_TRACE(name_len);
ASSERT_TRUE(GetECHLength(ctx.get(), &client_hello_len, &ech_len, 128,
std::string(name_len, 'a').c_str()));
- 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;
+ EXPECT_EQ(client_hello_len, client_hello_len_129);
+ EXPECT_EQ(ech_len, ech_len_129);
}
}
@@ -2119,39 +2113,44 @@
EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span(
"abcdefhijklmnopqrstuvwxyz-ABCDEFGHIJKLMNOPQRSTUVWXYZ-01234567899")));
- // Inputs with trailing numeric components are rejected.
+ // Inputs that parse as IPv4 addresses are rejected.
EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.0.0.1")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.1")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.01")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.0x01")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.0X01")));
- // Leading zeros and values that overflow |uint32_t| are still rejected.
- EXPECT_FALSE(ssl_is_valid_ech_public_name(
- str_to_span("example.123456789000000000000000")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(
- str_to_span("example.012345678900000000000000")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(
- str_to_span("example.0x123456789abcdefABCDEF0")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(
- str_to_span("example.0x0123456789abcdefABCDEF")));
- // Adding a non-digit or non-hex character makes it a valid DNS name again.
- // Single-component numbers are rejected.
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0177.0.0.01")));
+ EXPECT_FALSE(
+ ssl_is_valid_ech_public_name(str_to_span("0x7f.0x.0x.0x00000001")));
+ EXPECT_FALSE(
+ ssl_is_valid_ech_public_name(str_to_span("0XAB.0XCD.0XEF.0X01")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("255.255.255.255")));
+ // Out-of-bounds or overflowing components are not IP addresses.
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("256.255.255.255")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("255.0x100.255.255")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("255.255.255.0400")));
EXPECT_TRUE(ssl_is_valid_ech_public_name(
- str_to_span("example.1234567890a")));
- EXPECT_TRUE(ssl_is_valid_ech_public_name(
- str_to_span("example.01234567890a")));
- EXPECT_TRUE(ssl_is_valid_ech_public_name(
- str_to_span("example.0x123456789abcdefg")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("1")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("01")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x01")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0X01")));
- // Numbers with trailing dots are rejected. (They are already rejected by the
- // LDH label rules, but the WHATWG URL parser additionally rejects them.)
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("1.")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("01.")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x01.")));
- EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0X01.")));
+ str_to_span("255.255.255.0x100000000")));
+ // Invalid characters for the base are not IP addresses.
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("12a.0.0.1")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0xg.0.0.1")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("08.0.0.1")));
+ // Trailing components can be merged into a single component.
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.0.1")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.1")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("2130706433")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x7f000001")));
+ // Merged components must respect their limits.
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0xff")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0xffff")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0xffffff")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0xffffffff")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0x100")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0.0x10000")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0x1000000")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0x100000000")));
+ // Correctly handle overflow in decimal and octal.
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("037777777777")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("040000000000")));
+ EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("4294967295")));
+ EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("4294967296")));
}
// When using the built-in verifier, test that |SSL_get0_ech_name_override| is
diff --git a/src/ssl/ssl_transcript.cc b/src/ssl/ssl_transcript.cc
index 58fd21e..1599c80 100644
--- a/src/ssl/ssl_transcript.cc
+++ b/src/ssl/ssl_transcript.cc
@@ -158,14 +158,20 @@
return true;
}
+// InitDigestWithData calls |EVP_DigestInit_ex| on |ctx| with |md| and then
+// writes the data in |buf| to it.
+static bool InitDigestWithData(EVP_MD_CTX *ctx, const EVP_MD *md,
+ const BUF_MEM *buf) {
+ if (!EVP_DigestInit_ex(ctx, md, NULL)) {
+ return false;
+ }
+ EVP_DigestUpdate(ctx, buf->data, buf->length);
+ return true;
+}
+
bool SSLTranscript::InitHash(uint16_t version, const SSL_CIPHER *cipher) {
const EVP_MD *md = ssl_get_handshake_digest(version, cipher);
- if (Digest() == md) {
- // No need to re-hash the buffer.
- return true;
- }
- return EVP_DigestInit_ex(hash_.get(), md, nullptr) &&
- EVP_DigestUpdate(hash_.get(), buffer_->data, buffer_->length);
+ return InitDigestWithData(hash_.get(), md, buffer_.get());
}
void SSLTranscript::FreeBuffer() {
diff --git a/src/ssl/ssl_x509.cc b/src/ssl/ssl_x509.cc
index 680f253..98f1f6a 100644
--- a/src/ssl/ssl_x509.cc
+++ b/src/ssl/ssl_x509.cc
@@ -379,9 +379,8 @@
const char *name;
size_t name_len;
SSL_get0_ech_name_override(ssl, &name, &name_len);
- UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
- if (!ctx ||
- !X509_STORE_CTX_init(ctx.get(), verify_store, leaf, cert_chain) ||
+ ScopedX509_STORE_CTX ctx;
+ if (!X509_STORE_CTX_init(ctx.get(), verify_store, leaf, cert_chain) ||
!X509_STORE_CTX_set_ex_data(ctx.get(),
SSL_get_ex_data_X509_STORE_CTX_idx(), ssl) ||
// We need to inherit the verify parameters. These can be determined by
@@ -412,11 +411,11 @@
verify_ret = X509_verify_cert(ctx.get());
}
- session->verify_result = X509_STORE_CTX_get_error(ctx.get());
+ session->verify_result = ctx->error;
// If |SSL_VERIFY_NONE|, the error is non-fatal, but we keep the result.
if (verify_ret <= 0 && hs->config->verify_mode != SSL_VERIFY_NONE) {
- *out_alert = SSL_alert_from_verify_result(session->verify_result);
+ *out_alert = SSL_alert_from_verify_result(ctx->error);
return false;
}
@@ -465,9 +464,9 @@
return false;
}
- UniquePtr<X509_STORE_CTX> ctx(X509_STORE_CTX_new());
- if (!ctx || !X509_STORE_CTX_init(ctx.get(), hs->ssl->ctx->cert_store,
- leaf.get(), nullptr)) {
+ ScopedX509_STORE_CTX ctx;
+ if (!X509_STORE_CTX_init(ctx.get(), hs->ssl->ctx->cert_store, leaf.get(),
+ NULL)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_X509_LIB);
return false;
}
@@ -477,13 +476,9 @@
ERR_clear_error();
// Remove the leaf from the generated chain.
- UniquePtr<STACK_OF(X509)> chain(X509_STORE_CTX_get1_chain(ctx.get()));
- if (!chain) {
- return false;
- }
- X509_free(sk_X509_shift(chain.get()));
+ X509_free(sk_X509_shift(ctx->chain));
- if (!ssl_cert_set_chain(hs->config->cert.get(), chain.get())) {
+ if (!ssl_cert_set_chain(hs->config->cert.get(), ctx->chain)) {
return false;
}
@@ -711,6 +706,13 @@
return X509_STORE_load_locations(ctx->cert_store, ca_file, ca_dir);
}
+void SSL_set_verify_result(SSL *ssl, long result) {
+ check_ssl_x509_method(ssl);
+ if (result != X509_V_OK) {
+ abort();
+ }
+}
+
long SSL_get_verify_result(const SSL *ssl) {
check_ssl_x509_method(ssl);
SSL_SESSION *session = SSL_get_session(ssl);
diff --git a/src/ssl/test/fuzzer.h b/src/ssl/test/fuzzer.h
index 00b5e84..509cfdb 100644
--- a/src/ssl/test/fuzzer.h
+++ b/src/ssl/test/fuzzer.h
@@ -231,6 +231,16 @@
0x01, 'a', 0x02, 'a', 'a', 0x03, 'a', 'a', 'a',
};
+const uint8_t kECHConfig[] = {
+ 0xfe, 0x0a, 0x00, 0x47, 0x2a, 0x00, 0x20, 0x00, 0x20, 0x6c, 0x55,
+ 0x96, 0x41, 0x3d, 0x12, 0x4e, 0x63, 0x3d, 0x39, 0x7a, 0xe9, 0xbc,
+ 0xec, 0xb2, 0x55, 0xd0, 0xe6, 0xaa, 0xbd, 0xa9, 0x79, 0xb8, 0x86,
+ 0x9a, 0x13, 0x61, 0xc6, 0x69, 0xac, 0xb4, 0x21, 0x00, 0x0c, 0x00,
+ 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x02, 0x00, 0x01, 0x00, 0x03,
+ 0x00, 0x10, 0x00, 0x0e, 0x70, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x2e,
+ 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x00, 0x00,
+};
+
const uint8_t kECHKey[] = {
0x35, 0x6d, 0x45, 0x06, 0xb3, 0x88, 0x89, 0x2e, 0xd6, 0x87, 0x84,
0xd2, 0x2d, 0x6f, 0x83, 0x48, 0xad, 0xf2, 0xfd, 0x08, 0x51, 0x73,
@@ -448,20 +458,11 @@
if (role_ == kServer) {
bssl::UniquePtr<SSL_ECH_KEYS> keys(SSL_ECH_KEYS_new());
bssl::ScopedEVP_HPKE_KEY key;
- uint8_t *ech_config;
- size_t ech_config_len;
if (!keys ||
!EVP_HPKE_KEY_init(key.get(), EVP_hpke_x25519_hkdf_sha256(), kECHKey,
sizeof(kECHKey)) ||
- // Match |echConfig| in |addEncryptedClientHelloTests| from runner.go.
- !SSL_marshal_ech_config(&ech_config, &ech_config_len,
- /*config_id=*/42, key.get(), "public.example",
- /*max_name_len=*/64)) {
- return false;
- }
- bssl::UniquePtr<uint8_t> free_ech_config(ech_config);
- if (!SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/true, ech_config,
- ech_config_len, key.get()) ||
+ !SSL_ECH_KEYS_add(keys.get(), /*is_retry_config=*/true, kECHConfig,
+ sizeof(kECHConfig), key.get()) ||
!SSL_CTX_set1_ech_keys(ctx_.get(), keys.get())) {
return false;
}
diff --git a/src/ssl/test/runner/common.go b/src/ssl/test/runner/common.go
index bf6a3d1..782eb36 100644
--- a/src/ssl/test/runner/common.go
+++ b/src/ssl/test/runner/common.go
@@ -128,7 +128,8 @@
extensionChannelID uint16 = 30032 // not IANA assigned
extensionDelegatedCredentials uint16 = 0x22 // draft-ietf-tls-subcerts-06
extensionDuplicate uint16 = 0xffff // not IANA assigned
- extensionEncryptedClientHello uint16 = 0xfe0d // not IANA assigned
+ extensionEncryptedClientHello uint16 = 0xfe0a // not IANA assigned
+ extensionECHIsInner uint16 = 0xda09 // not IANA assigned
extensionECHOuterExtensions uint16 = 0xfd00 // not IANA assigned
)
@@ -175,7 +176,7 @@
CertTypeRSAFixedDH = 3 // A certificate containing a static DH key
CertTypeDSSFixedDH = 4 // A certificate containing a static DH key
- // See RFC 4492 sections 3 and 5.5.
+ // See RFC4492 sections 3 and 5.5.
CertTypeECDSASign = 64 // A certificate containing an ECDSA-capable public key, signed with ECDSA.
CertTypeRSAFixedECDH = 65 // A certificate containing an ECDH-capable public key, signed with RSA.
CertTypeECDSAFixedECDH = 66 // A certificate containing an ECDH-capable public key, signed with ECDSA.
@@ -242,9 +243,6 @@
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)
@@ -871,53 +869,40 @@
// retry configs.
SendECHRetryConfigs []byte
- // AlwaysSendECHRetryConfigs, if true, causes the ECH server to send retry
- // configs unconditionally, including in the TLS 1.2 ServerHello.
- AlwaysSendECHRetryConfigs bool
+ // SendECHRetryConfigsInTLS12ServerHello, if true, causes the ECH server to
+ // send retry configs in the TLS 1.2 ServerHello.
+ SendECHRetryConfigsInTLS12ServerHello bool
- // AlwaysSendECHHelloRetryRequest, if true, causes the ECH server to send
- // the ECH HelloRetryRequest extension unconditionally.
- AlwaysSendECHHelloRetryRequest bool
+ // SendInvalidECHIsInner, if not empty, causes the client to send the
+ // specified byte string in the ech_is_inner extension.
+ SendInvalidECHIsInner []byte
- // 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
+ // OmitECHIsInner, if true, causes the client to omit the ech_is_inner
// extension on the ClientHelloInner message.
- OmitECHInner bool
+ OmitECHIsInner bool
- // OmitSecondECHInner, if true, causes the client to omit the
- // encrypted_client_hello extension on the second ClientHelloInner message.
- OmitSecondECHInner bool
+ // OmitSecondECHIsInner, if true, causes the client to omit the ech_is_inner
+ // extension on the second ClientHelloInner message.
+ OmitSecondECHIsInner 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
+ // 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
// 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
@@ -956,11 +941,6 @@
// 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.
@@ -1031,10 +1011,6 @@
// normally expected to look ahead for ChangeCipherSpec.)
EmptyTicketSessionID bool
- // NewSessionIDLength, if non-zero is the length of the session ID to use
- // when issung new sessions.
- NewSessionIDLength int
-
// SendClientHelloSessionID, if not nil, is the session ID sent in the
// ClientHello.
SendClientHelloSessionID []byte
diff --git a/src/ssl/test/runner/handshake_client.go b/src/ssl/test/runner/handshake_client.go
index 5d04994..424b206 100644
--- a/src/ssl/test/runner/handshake_client.go
+++ b/src/ssl/test/runner/handshake_client.go
@@ -26,18 +26,19 @@
const echBadPayloadByte = 0xff
type clientHandshakeState struct {
- 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
+ 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
}
func mapClientHelloVersion(vers uint16, isDTLS bool) uint16 {
@@ -339,6 +340,10 @@
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 {
@@ -528,6 +533,9 @@
// 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)
}
@@ -535,24 +543,8 @@
hello.prefixExtensions = append(hello.prefixExtensions, extensionALPN)
hello.prefixExtensions = append(hello.prefixExtensions, extensionNextProtoNeg)
}
-
- // 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 isInner && len(c.config.ECHOuterExtensions) > 0 && !c.config.Bugs.OnlyCompressSecondClientHelloInner {
+ applyECHOuterExtensions(hello, c.config.ECHOuterExtensions)
}
if maxVersion >= VersionTLS13 {
@@ -824,9 +816,9 @@
hello.hasEarlyData = innerHello.hasEarlyData
}
- if (isInner && !c.config.Bugs.OmitECHInner) || c.config.Bugs.AlwaysSendECHInner {
- hello.echInner = true
- hello.invalidECHInner = c.config.Bugs.SendInvalidECHInner
+ if (isInner && !c.config.Bugs.OmitECHIsInner) || c.config.Bugs.AlwaysSendECHIsInner {
+ hello.echIsInner = true
+ hello.invalidECHIsInner = c.config.Bugs.SendInvalidECHIsInner
}
if innerHello != nil {
@@ -835,9 +827,9 @@
}
if c.config.Bugs.CorruptEncryptedClientHello {
if c.config.Bugs.NullAllCiphers {
- hello.echOuter.payload = []byte{echBadPayloadByte}
+ hello.clientECH.payload = []byte{echBadPayloadByte}
} else {
- hello.echOuter.payload[0] ^= 1
+ hello.clientECH.payload[0] ^= 1
}
}
}
@@ -886,31 +878,27 @@
enc = enc[:1]
}
- encodedInner := innerHello.marshalForEncodedInner()
- padding := make([]byte, c.config.Bugs.ClientECHPadding)
- if c.config.Bugs.BadClientECHPadding {
- padding[0] = 1
- }
- encodedInner = append(encodedInner, padding...)
+ aad := newByteBuilder()
+ aad.addU16(hs.echHPKEContext.KDF())
+ aad.addU16(hs.echHPKEContext.AEAD())
+ aad.addU8(configID)
+ aad.addU16LengthPrefixed().addBytes(enc)
+ hello.marshalForOuterAAD(aad.addU24LengthPrefixed())
- // Encode ClientHelloOuter with a placeholder payload string.
- payloadLength := len(encodedInner)
- if !c.config.Bugs.NullAllCiphers {
- payloadLength += hs.echHPKEContext.Overhead()
+ encodedInner := innerHello.marshalForEncodedInner()
+ payload := hs.echHPKEContext.Seal(encodedInner, aad.finish())
+
+ if c.config.Bugs.NullAllCiphers {
+ payload = encodedInner
}
- hello.echOuter = &echClientOuter{
+
+ // Place the ECH extension in the outer CH.
+ hello.clientECH = &clientECH{
kdfID: hs.echHPKEContext.KDF(),
aeadID: hs.echHPKEContext.AEAD(),
configID: configID,
enc: enc,
- 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
+ payload: payload,
}
if c.config.Bugs.RecordClientHelloInner != nil {
@@ -925,73 +913,24 @@
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")
@@ -1005,17 +944,20 @@
// Reset the encryption state, in case we sent 0-RTT data.
c.out.resetCipher()
- if c.echAccepted {
- if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.innerHello, hs.hello); err != nil {
+ if hs.innerHello != nil {
+ if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.innerHello, nil); err != nil {
return err
}
- hs.writeClientHash(hs.innerHello.marshal())
+ if err := hs.applyHelloRetryRequest(helloRetryRequest, hs.hello, hs.innerHello); err != nil {
+ return err
+ }
+ hs.innerFinishedHash.WriteHandshake(hs.innerHello.marshal(), c.sendHandshakeSeq)
} 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 {
@@ -1048,12 +990,6 @@
}
}
- // 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 {
@@ -1077,19 +1013,9 @@
return fmt.Errorf("tls: server sent non-matching cipher suite %04x vs %04x", hs.suite.id, hs.serverHello.cipherSuite)
}
- 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 haveHelloRetryRequest && helloRetryRequest.hasSelectedGroup && helloRetryRequest.selectedGroup != hs.serverHello.keyShare.group {
+ c.sendAlert(alertHandshakeFailure)
+ return errors.New("tls: ServerHello parameters did not match HelloRetryRequest")
}
if !bytes.Equal(hs.hello.sessionID, hs.serverHello.sessionID) {
@@ -1113,6 +1039,9 @@
c.didResume = true
}
hs.finishedHash.addEntropy(pskSecret)
+ if hs.innerHello != nil {
+ hs.innerFinishedHash.addEntropy(pskSecret)
+ }
if !hs.serverHello.hasKeyShare {
c.sendAlert(alertUnsupportedExtension)
@@ -1137,6 +1066,39 @@
}
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
@@ -1503,53 +1465,58 @@
}
// applyHelloRetryRequest updates |hello| in-place based on |helloRetryRequest|.
-// 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 {
+// 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 {
c := hs.c
firstHelloBytes := hello.marshal()
+ isInner := innerHello == nil && hs.echHPKEContext != nil
if len(helloRetryRequest.cookie) > 0 {
hello.tls13Cookie = helloRetryRequest.cookie
}
- 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 innerHello != nil {
+ hello.keyShares = innerHello.keyShares
+ } else {
+ 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 !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 c.config.Bugs.SecondClientHelloMissingKeyShare {
- hello.hasKeyShares = false
- }
-
- if c.config.Bugs.OmitSecondECHInner {
- hello.echInner = false
+ if isInner && c.config.Bugs.OmitSecondECHIsInner {
+ hello.echIsInner = false
}
hello.hasEarlyData = c.config.Bugs.SendEarlyDataOnSecondClientHello
@@ -1557,51 +1524,56 @@
if c.config.Bugs.PSKBinderFirst && c.config.Bugs.OnlyCorruptSecondPSKBinder {
hello.prefixExtensions = append(hello.prefixExtensions, extensionPreSharedKey)
}
- // The first ClientHello may have set this due to OnlyCompressSecondClientHelloInner.
- hello.reorderOuterExtensionsWithoutCompressing = false
+ // 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)
+ }
if c.config.Bugs.OmitPSKsOnSecondClientHello {
hello.pskIdentities = nil
hello.pskBinders = nil
}
hello.raw = 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 innerHello != nil {
if c.config.Bugs.OmitSecondEncryptedClientHello {
- outerHello.echOuter = nil
+ hello.clientECH = nil
} else {
configID := c.config.ClientECHConfig.ConfigID
if c.config.Bugs.CorruptSecondEncryptedClientHelloConfigID {
configID ^= 1
}
- if err := hs.encryptClientHello(outerHello, hello, configID, nil); err != nil {
+ if err := hs.encryptClientHello(hello, innerHello, configID, nil); err != nil {
return err
}
if c.config.Bugs.CorruptSecondEncryptedClientHello {
if c.config.Bugs.NullAllCiphers {
- outerHello.echOuter.payload = []byte{echBadPayloadByte}
+ hello.clientECH.payload = []byte{echBadPayloadByte}
} else {
- outerHello.echOuter.payload[0] ^= 1
+ hello.clientECH.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/src/ssl/test/runner/handshake_messages.go b/src/ssl/test/runner/handshake_messages.go
index f2ef2fc..d666a87 100644
--- a/src/ssl/test/runner/handshake_messages.go
+++ b/src/ssl/test/runner/handshake_messages.go
@@ -260,7 +260,7 @@
ConfigID uint8
KEM uint16
PublicKey []byte
- MaxNameLen uint8
+ MaxNameLen uint16
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.addU8(template.MaxNameLen)
- contents.addU8LengthPrefixed().addBytes([]byte(template.PublicName))
+ contents.addU16(template.MaxNameLen)
+ contents.addU16LengthPrefixed().addBytes([]byte(template.PublicName))
extensions := contents.addU16LengthPrefixed()
// Mandatory extensions have the high bit set.
if template.UnsupportedExtension {
@@ -318,12 +318,9 @@
Key []byte
}
-const (
- echClientTypeOuter byte = 0
- echClientTypeInner byte = 1
-)
-
-type echClientOuter struct {
+// The contents of a CH "encrypted_client_hello" extension.
+// https://tools.ietf.org/html/draft-ietf-tls-esni-10
+type clientECH struct {
kdfID uint16
aeadID uint16
configID uint8
@@ -332,64 +329,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
- 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
+ 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
// The following fields are only filled in by |unmarshal| and ignored when
// marshaling a new ClientHello.
- echPayloadStart int
- echPayloadEnd int
- rawExtensions []byte
+ extensionStart int
+ echExtensionStart int
+ echExtensionEnd int
+ rawExtensions map[uint16][]byte
}
func (m *clientHelloMsg) marshalKeyShares(bb *byteBuilder) {
@@ -408,6 +405,7 @@
const (
clientHelloNormal clientHelloType = iota
+ clientHelloOuterAAD
clientHelloEncodedInner
)
@@ -473,26 +471,35 @@
body: serverNameList.finish(),
})
}
- if m.echOuter != nil {
+ if m.clientECH != nil && typ != clientHelloOuterAAD {
body := newByteBuilder()
- 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)
+ 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)
+
extensions = append(extensions, extension{
id: extensionEncryptedClientHello,
body: body.finish(),
})
}
- if m.echInner {
- body := newByteBuilder()
- body.addU8(echClientTypeInner)
- // If unset, invalidECHInner is empty, which is the correct serialization.
- body.addBytes(m.invalidECHInner)
+ if m.echIsInner {
extensions = append(extensions, extension{
- id: extensionEncryptedClientHello,
+ id: extensionECHIsInner,
+ // If unset, invalidECHIsInner is empty, which is the correct
+ // serialization.
+ body: m.invalidECHIsInner,
+ })
+ }
+ if m.outerExtensions != nil && typ == clientHelloEncodedInner {
+ body := newByteBuilder()
+ extensionsList := body.addU8LengthPrefixed()
+ for _, extID := range m.outerExtensions {
+ extensionsList.addU16(extID)
+ }
+ extensions = append(extensions, extension{
+ id: extensionECHOuterExtensions,
body: body.finish(),
})
}
@@ -661,7 +668,7 @@
if m.sctListSupported {
extensions = append(extensions, extension{id: extensionSignedCertificateTimestamp})
}
- if len(m.customExtension) > 0 {
+ if l := len(m.customExtension); l > 0 {
extensions = append(extensions, extension{
id: extensionCustom,
body: []byte(m.customExtension),
@@ -719,51 +726,35 @@
})
}
+ // 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{}{}
}
}
- // 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{}{}
- }
- }
- }
+ // 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 each of the remaining extensions in their original order.
for _, ext := range extensions {
- if _, written := extsWritten[ext.id]; !written {
+ if _, ok := extMap[ext.id]; ok {
extensionsBB.addU16(ext.id)
extensionsBB.addU16LengthPrefixed().addBytes(ext.body)
}
@@ -788,6 +779,10 @@
}
}
+func (m *clientHelloMsg) marshalForOuterAAD(bb *byteBuilder) {
+ m.marshalBody(bb, clientHelloOuterAAD)
+}
+
func (m *clientHelloMsg) marshalForEncodedInner() []byte {
hello := newByteBuilder()
m.marshalBody(hello, clientHelloEncodedInner)
@@ -896,6 +891,8 @@
}
}
+ m.extensionStart = len(data) - len(reader)
+
m.nextProtoNeg = false
m.serverName = ""
m.ocspStapling = false
@@ -912,6 +909,7 @@
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
@@ -922,7 +920,6 @@
if !reader.readU16LengthPrefixed(&extensions) || len(reader) != 0 || !checkDuplicateExtensions(extensions) {
return false
}
- m.rawExtensions = extensions
for len(extensions) > 0 {
var extension uint16
var body byteReader
@@ -930,6 +927,7 @@
!extensions.readU16LengthPrefixed(&body) {
return false
}
+ m.rawExtensions[extension] = body
switch extension {
case extensionServerName:
var names byteReader
@@ -948,33 +946,24 @@
}
}
case extensionEncryptedClientHello:
- var typ byte
- if !body.readU8(&typ) {
+ 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 {
return false
}
- 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:
+ m.clientECH = &ech
+ case extensionECHIsInner:
+ if len(body) != 0 {
return false
}
+ m.echIsInner = true
case extensionNextProtoNeg:
if len(body) != 0 {
return false
@@ -1206,6 +1195,13 @@
}
}
+ // 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
}
@@ -1218,17 +1214,11 @@
len(sessionID) != 0 || // Copied from |helloOuter|
!reader.readU16LengthPrefixedBytes(&cipherSuites) ||
!reader.readU8LengthPrefixedBytes(&compressionMethods) ||
- !reader.readU16LengthPrefixed(&extensions) {
+ !reader.readU16LengthPrefixed(&extensions) ||
+ len(reader) != 0 {
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()
@@ -1239,7 +1229,6 @@
newExtensions := body.addU16LengthPrefixed()
var seenOuterExtensions bool
- outerExtensions := byteReader(helloOuter.rawExtensions)
copied := make(map[uint16]struct{})
for len(extensions) > 0 {
var extType uint16
@@ -1257,35 +1246,28 @@
return nil, errors.New("tls: duplicate ech_outer_extensions extension")
}
seenOuterExtensions = true
- var extList byteReader
- if !extBody.readU8LengthPrefixed(&extList) || len(extList) == 0 || len(extBody) != 0 {
+ var outerExtensions byteReader
+ if !extBody.readU8LengthPrefixed(&outerExtensions) || len(outerExtensions) == 0 || len(extBody) != 0 {
return nil, errors.New("tls: error parsing ech_outer_extensions")
}
- for len(extList) != 0 {
+ for len(outerExtensions) != 0 {
var newExtType uint16
- if !extList.readU16(&newExtType) {
+ if !outerExtensions.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")
}
- 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
- }
+ if _, ok := copied[newExtType]; ok {
+ return nil, errors.New("tls: duplicate extension in ech_outer_extensions")
}
+ 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{}{}
}
}
@@ -1505,6 +1487,25 @@
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
@@ -1906,18 +1907,16 @@
}
type helloRetryRequestMsg struct {
- raw []byte
- vers uint16
- sessionID []byte
- cipherSuite uint16
- compressionMethod uint8
- hasSelectedGroup bool
- selectedGroup CurveID
- cookie []byte
- customExtension string
- echConfirmation []byte
- echConfirmationOffset int
- duplicateExtensions bool
+ raw []byte
+ vers uint16
+ sessionID []byte
+ cipherSuite uint16
+ compressionMethod uint8
+ hasSelectedGroup bool
+ selectedGroup CurveID
+ cookie []byte
+ customExtension string
+ duplicateExtensions bool
}
func (m *helloRetryRequestMsg) marshal() []byte {
@@ -1961,10 +1960,6 @@
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()
@@ -2010,17 +2005,9 @@
m.hasSelectedGroup = true
m.selectedGroup = CurveID(v)
case extensionCookie:
- if !body.readU16LengthPrefixedBytes(&m.cookie) ||
- len(m.cookie) == 0 ||
- len(body) != 0 {
+ if !body.readU16LengthPrefixedBytes(&m.cookie) || 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/src/ssl/test/runner/handshake_server.go b/src/ssl/test/runner/handshake_server.go
index 4f41184..b9d7667 100644
--- a/src/ssl/test/runner/handshake_server.go
+++ b/src/ssl/test/runner/handshake_server.go
@@ -186,21 +186,14 @@
return errors.New("tls: no GREASE extension found")
}
- 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 {
+ if clientECH := hs.clientHello.clientECH; clientECH != nil {
for _, candidate := range config.ServerECHConfigs {
- if candidate.ECHConfig.ConfigID != echOuter.configID {
+ if candidate.ECHConfig.ConfigID != clientECH.configID {
continue
}
var found bool
for _, suite := range candidate.ECHConfig.CipherSuites {
- if echOuter.kdfID == suite.KDF && echOuter.aeadID == suite.AEAD {
+ if clientECH.kdfID == suite.KDF && clientECH.aeadID == suite.AEAD {
found = true
break
}
@@ -210,7 +203,7 @@
}
info := []byte("tls ech\x00")
info = append(info, candidate.ECHConfig.Raw...)
- hs.echHPKEContext, err = hpke.SetupBaseReceiverX25519(echOuter.kdfID, echOuter.aeadID, echOuter.enc, candidate.Key, info)
+ hs.echHPKEContext, err = hpke.SetupBaseReceiverX25519(clientECH.kdfID, clientECH.aeadID, clientECH.enc, candidate.Key, info)
if err != nil {
continue
}
@@ -227,7 +220,7 @@
} else {
c.echAccepted = true
hs.clientHello = clientHelloInner
- hs.echConfigID = echOuter.configID
+ hs.echConfigID = clientECH.configID
}
}
}
@@ -456,17 +449,29 @@
}
func (hs *serverHandshakeState) decryptClientHello(helloOuter *clientHelloMsg) (helloInner *clientHelloMsg, err error) {
- // 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:])
+ // 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:])
// In fuzzer mode, the payload is cleartext.
- encoded := helloOuter.echOuter.payload
+ encoded := helloOuter.clientECH.payload
if !hs.c.config.Bugs.NullAllCiphers {
var err error
- encoded, err = hs.echHPKEContext.Open(helloOuter.echOuter.payload, aad)
+ encoded, err = hs.echHPKEContext.Open(helloOuter.clientECH.payload, aad.finish())
if err != nil {
// Wrap |err| so the caller can implement trial decryption.
return nil, &echDecryptError{err}
@@ -500,8 +505,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.echInner {
- return nil, errors.New("tls: ClientHelloInner missing inner encrypted_client_hello extension")
+ if !helloInner.echIsInner {
+ return nil, errors.New("tls: ClientHelloInner missing ech_is_inner extension")
}
return helloInner, nil
@@ -544,6 +549,13 @@
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 {
@@ -742,21 +754,6 @@
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()
@@ -790,19 +787,19 @@
}
if c.echAccepted {
- if newClientHello.echOuter == nil {
+ if newClientHello.clientECH == nil {
c.sendAlert(alertMissingExtension)
return errors.New("tls: second ClientHelloOuter had no encrypted_client_hello extension")
}
- if newClientHello.echOuter.configID != hs.echConfigID ||
- newClientHello.echOuter.kdfID != hs.echHPKEContext.KDF() ||
- newClientHello.echOuter.aeadID != hs.echHPKEContext.AEAD() {
+ if newClientHello.clientECH.configID != hs.echConfigID ||
+ newClientHello.clientECH.kdfID != hs.echHPKEContext.KDF() ||
+ newClientHello.clientECH.aeadID != hs.echHPKEContext.AEAD() {
c.sendAlert(alertIllegalParameter)
return errors.New("tls: ECH parameters changed in second ClientHelloOuter")
}
- if len(newClientHello.echOuter.enc) != 0 {
+ if len(newClientHello.clientECH.enc) != 0 {
c.sendAlert(alertIllegalParameter)
- return errors.New("tls: second ClientHelloOuter had non-empty ECH enc")
+ return errors.New("tls: second ClientECH had non-empty enc")
}
newClientHello, err = hs.decryptClientHello(newClientHello)
if err != nil {
@@ -822,6 +819,11 @@
// 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
@@ -1004,13 +1006,13 @@
hs.finishedHash.addEntropy(hs.finishedHash.zeroSecret())
}
- // 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
+ // 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
}
- copy(randomSuffix, hs.finishedHash.echAcceptConfirmation(hs.clientHello.random, echAcceptConfirmationLabel, hs.hello.marshal()))
+ echAcceptConfirmation := hs.finishedHash.deriveSecretPeek([]byte("ech accept confirmation"), hs.hello.marshal())
+ copy(hs.hello.random[24:], echAcceptConfirmation)
hs.hello.raw = nil
}
@@ -1694,7 +1696,7 @@
serverExtensions.serverNameAck = c.config.Bugs.SendServerNameAck
- if (c.vers >= VersionTLS13 && hs.clientHello.echOuter != nil) || c.config.Bugs.AlwaysSendECHRetryConfigs {
+ if (c.vers >= VersionTLS13 || c.config.Bugs.SendECHRetryConfigsInTLS12ServerHello) && hs.clientHello.clientECH != nil {
if len(config.Bugs.SendECHRetryConfigs) > 0 {
serverExtensions.echRetryConfigs = config.Bugs.SendECHRetryConfigs
} else if len(config.ServerECHConfigs) > 0 {
@@ -1847,11 +1849,7 @@
// Generate a session ID if we're to save the session.
if !hs.hello.extensions.ticketSupported && config.ServerSessionCache != nil {
- l := config.Bugs.NewSessionIDLength
- if l == 0 {
- l = 32
- }
- hs.hello.sessionID = make([]byte, l)
+ hs.hello.sessionID = make([]byte, 32)
if _, err := io.ReadFull(config.rand(), hs.hello.sessionID); err != nil {
c.sendAlert(alertInternalError)
return errors.New("tls: short read from Rand: " + err.Error())
diff --git a/src/ssl/test/runner/hpke/hpke.go b/src/ssl/test/runner/hpke/hpke.go
index 36dc637..a08538b 100644
--- a/src/ssl/test/runner/hpke/hpke.go
+++ b/src/ssl/test/runner/hpke/hpke.go
@@ -136,8 +136,6 @@
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/src/ssl/test/runner/prf.go b/src/ssl/test/runner/prf.go
index 5731be0..66c427f 100644
--- a/src/ssl/test/runner/prf.go
+++ b/src/ssl/test/runner/prf.go
@@ -381,6 +381,7 @@
return b
}
+// The following are labels for traffic secret derivation in TLS 1.3.
var (
externalPSKBinderLabel = []byte("ext binder")
resumptionPSKBinderLabel = []byte("res binder")
@@ -395,25 +396,21 @@
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 RFC8446.
+// section 7.1 of draft ietf-tls-tls13-16.
func (h *finishedHash) deriveSecret(label []byte) []byte {
return hkdfExpandLabel(h.suite.hash(), h.secret, label, h.appendContextHashes(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, clientRandom, h.zeroSecret())
- hashCopy := copyHash(h.hash, h.suite.hash())
- hashCopy.Write(extraMessages)
- return hkdfExpandLabel(h.suite.hash(), secret, label, hashCopy.Sum(nil), echAcceptConfirmationLength)
+// 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())
}
// The following are context strings for CertificateVerify in TLS 1.3.
diff --git a/src/ssl/test/runner/runner.go b/src/ssl/test/runner/runner.go
index cfff714..3306c88 100644
--- a/src/ssl/test/runner/runner.go
+++ b/src/ssl/test/runner/runner.go
@@ -1636,7 +1636,6 @@
if err != nil {
return err
}
- statusChan <- statusMsg{test: test, statusType: statusShimStarted, pid: shim.cmd.Process.Pid}
defer shim.close()
localErr := doExchanges(test, shim, resumeCount, &transcripts)
@@ -3833,7 +3832,7 @@
},
},
shouldFail: true,
- expectedError: ":WRONG_CIPHER_RETURNED:",
+ expectedError: ":UNKNOWN_CIPHER_RETURNED:",
})
testCases = append(testCases, testCase{
name: "ServerHelloBogusCipher-TLS13",
@@ -13278,7 +13277,7 @@
testCases = append(testCases, testCase{
testType: serverTest,
- name: "Server-ShortSessionID-TLS13",
+ name: "ShortSessionID-TLS13",
config: Config{
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
@@ -13289,7 +13288,7 @@
testCases = append(testCases, testCase{
testType: serverTest,
- name: "Server-FullSessionID-TLS13",
+ name: "FullSessionID-TLS13",
config: Config{
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
@@ -13298,62 +13297,6 @@
},
})
- // The server should reject ClientHellos whose session IDs are too long.
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Server-TooLongSessionID-TLS13",
- config: Config{
- MaxVersion: VersionTLS13,
- Bugs: ProtocolBugs{
- SendClientHelloSessionID: make([]byte, 33),
- },
- },
- shouldFail: true,
- expectedError: ":DECODE_ERROR:",
- expectedLocalError: "remote error: error decoding message",
- })
- testCases = append(testCases, testCase{
- testType: serverTest,
- name: "Server-TooLongSessionID-TLS12",
- config: Config{
- MaxVersion: VersionTLS12,
- Bugs: ProtocolBugs{
- SendClientHelloSessionID: make([]byte, 33),
- },
- },
- shouldFail: true,
- expectedError: ":DECODE_ERROR:",
- expectedLocalError: "remote error: error decoding message",
- })
-
- // Test that the client correctly accepts or rejects short session IDs from
- // the server. Our tests use 32 bytes by default, so the boundary condition
- // is already covered.
- testCases = append(testCases, testCase{
- name: "Client-ShortSessionID",
- config: Config{
- MaxVersion: VersionTLS12,
- SessionTicketsDisabled: true,
- Bugs: ProtocolBugs{
- NewSessionIDLength: 1,
- },
- },
- resumeSession: true,
- })
- testCases = append(testCases, testCase{
- name: "Client-TooLongSessionID",
- config: Config{
- MaxVersion: VersionTLS12,
- SessionTicketsDisabled: true,
- Bugs: ProtocolBugs{
- NewSessionIDLength: 33,
- },
- },
- shouldFail: true,
- expectedError: ":DECODE_ERROR:",
- expectedLocalError: "remote error: error decoding message",
- })
-
// Test that the client sends a fake session ID in TLS 1.3. We cover both
// normal and resumption handshakes to capture interactions with the
// session resumption path.
@@ -13884,7 +13827,7 @@
},
},
shouldFail: true,
- expectedError: ":DECODE_ERROR:",
+ expectedError: ":WRONG_VERSION_NUMBER:",
})
testCases = append(testCases, testCase{
@@ -16645,19 +16588,19 @@
expectedError: ":INVALID_CLIENT_HELLO_INNER:",
})
- // When inner ECH extension is absent from the ClientHelloInner, the
+ // When ech_is_inner extension is absent from the ClientHelloInner, the
// server should fail the connection.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-MissingECHInner" + suffix,
+ name: prefix + "ECH-Server-MissingECHIsInner" + suffix,
config: Config{
ServerName: "secret.example",
DefaultCurves: defaultCurves,
ClientECHConfig: echConfig.ECHConfig,
Bugs: ProtocolBugs{
- OmitECHInner: !hrr,
- OmitSecondECHInner: hrr,
+ OmitECHIsInner: !hrr,
+ OmitSecondECHIsInner: hrr,
},
},
flags: []string{
@@ -16687,7 +16630,11 @@
extensionCustom,
},
Bugs: ProtocolBugs{
- CustomExtension: "test",
+ CustomExtension: "test",
+ // Ensure ClientHelloOuter's extension order is different
+ // from ClientHelloInner. This tests that the server
+ // correctly reconstructs the extension order.
+ FirstExtensionInClientHelloOuter: extensionSupportedCurves,
OnlyCompressSecondClientHelloInner: hrr,
},
},
@@ -16703,84 +16650,6 @@
},
})
- // 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
@@ -16799,10 +16668,6 @@
},
Bugs: ProtocolBugs{
OnlyCompressSecondClientHelloInner: hrr,
- // Don't duplicate the extension in ClientHelloOuter.
- ECHOuterExtensionOrder: []uint16{
- extensionSupportedCurves,
- },
},
},
flags: []string{
@@ -16812,9 +16677,7 @@
},
shouldFail: true,
expectedLocalError: "remote error: illegal parameter",
- // The decoding algorithm relies on the ordering requirement, so
- // duplicates appear as missing extensions.
- expectedError: ":OUTER_EXTENSION_NOT_FOUND:",
+ expectedError: ":DUPLICATE_EXTENSION:",
})
// Test that the server rejects references to missing extensions in
@@ -17030,12 +16893,12 @@
})
}
- // Test that the ECH server handles a short enc value by falling back to
- // ClientHelloOuter.
+ // Test that the ECH server handles a short ClientECH.enc value by
+ // falling back to ClientHelloOuter.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ShortEnc",
+ name: prefix + "ECH-Server-ShortClientECHEnc",
config: Config{
ServerName: "secret.example",
ClientECHConfig: echConfig.ECHConfig,
@@ -17249,54 +17112,6 @@
},
})
- // 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,
@@ -17363,19 +17178,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-RejectRetryConfigs",
+ name: prefix + "ECH-GREASE-Client-TLS12-Retry-Configs",
config: Config{
- MinVersion: VersionTLS12,
- MaxVersion: VersionTLS12,
- ServerECHConfigs: []ServerECHConfig{echConfig},
+ MinVersion: VersionTLS12,
+ MaxVersion: VersionTLS12,
Bugs: ProtocolBugs{
- ExpectClientECH: true,
- AlwaysSendECHRetryConfigs: true,
+ ExpectClientECH: true,
+ SendECHRetryConfigs: CreateECHConfigList(echConfig.ECHConfig.Raw, unsupportedVersion),
+ SendECHRetryConfigsInTLS12ServerHello: true,
},
},
flags: []string{"-enable-ech-grease"},
@@ -17383,101 +17198,8 @@
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{
@@ -17498,63 +17220,60 @@
expectedError: ":ERROR_PARSING_EXTENSION:",
})
- // Test that the server responds to an inner ECH extension with the
+ // Test that the server responds to an empty ech_is_inner extension with the
// acceptance confirmation.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHInner",
+ name: prefix + "ECH-Server-ECHIsInner",
config: Config{
MinVersion: VersionTLS13,
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
- 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,
+ AlwaysSendECHIsInner: true,
},
},
resumeSession: true,
})
// Test that server fails the handshake when it sees a non-empty
- // inner ECH extension.
+ // ech_is_inner extension.
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHInner-NotEmpty",
+ name: prefix + "ECH-Server-ECHIsInner-NotEmpty",
config: Config{
MinVersion: VersionTLS13,
MaxVersion: VersionTLS13,
Bugs: ProtocolBugs{
- AlwaysSendECHInner: true,
- SendInvalidECHInner: []byte{42, 42, 42},
+ AlwaysSendECHIsInner: true,
+ SendInvalidECHIsInner: []byte{42, 42, 42},
},
},
shouldFail: true,
- expectedLocalError: "remote error: error decoding message",
+ expectedLocalError: "remote error: illegal parameter",
expectedError: ":ERROR_PARSING_EXTENSION:",
})
- // Test that a TLS 1.3 server that receives an inner ECH extension can
+ // 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
// negotiate TLS 1.2 without clobbering the downgrade signal.
if protocol != quic {
testCases = append(testCases, testCase{
testType: serverTest,
protocol: protocol,
- name: prefix + "ECH-Server-ECHInner-Absent-TLS12",
+ name: prefix + "ECH-Server-ECHIsInner-Absent-TLS12",
config: Config{
MinVersion: VersionTLS12,
MaxVersion: VersionTLS13,
@@ -17562,7 +17281,7 @@
// Omit supported_versions extension so the server negotiates
// TLS 1.2.
OmitSupportedVersions: true,
- AlwaysSendECHInner: true,
+ AlwaysSendECHIsInner: true,
},
},
// Check that the client sees the TLS 1.3 downgrade signal in
@@ -17572,6 +17291,26 @@
})
}
+ // 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,
@@ -18148,7 +17887,9 @@
})
// Test that the client rejects ClientHelloOuter handshakes that attempt
- // to resume the ClientHelloInner's ticket, at TLS 1.2 and TLS 1.3.
+ // 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.
testCases = append(testCases, testCase{
testType: clientTest,
protocol: protocol,
@@ -18178,6 +17919,9 @@
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,
@@ -18597,31 +18341,6 @@
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/src/ssl/test/test_config.cc b/src/ssl/test/test_config.cc
index 7d1cefa..12a9f7a 100644
--- a/src/ssl/test/test_config.cc
+++ b/src/ssl/test/test_config.cc
@@ -810,7 +810,7 @@
GetTestState(ssl)->cert_verified = true;
if (config->verify_fail) {
- X509_STORE_CTX_set_error(store_ctx, X509_V_ERR_APPLICATION_VERIFICATION);
+ store_ctx->error = X509_V_ERR_APPLICATION_VERIFICATION;
return 0;
}
diff --git a/src/ssl/tls13_both.cc b/src/ssl/tls13_both.cc
index 226c67b..0354f39 100644
--- a/src/ssl/tls13_both.cc
+++ b/src/ssl/tls13_both.cc
@@ -235,14 +235,15 @@
}
// Parse out the extensions.
- SSLExtension status_request(
- TLSEXT_TYPE_status_request,
- !ssl->server && hs->config->ocsp_stapling_enabled);
- SSLExtension sct(
- TLSEXT_TYPE_certificate_timestamp,
- !ssl->server && hs->config->signed_cert_timestamps_enabled);
+ bool have_status_request = false, have_sct = false;
+ CBS status_request, sct;
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_status_request, &have_status_request, &status_request},
+ {TLSEXT_TYPE_certificate_timestamp, &have_sct, &sct},
+ };
+
uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!ssl_parse_extensions(&extensions, &alert, {&status_request, &sct},
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
/*ignore_unknown=*/false)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return false;
@@ -250,14 +251,20 @@
// All Certificate extensions are parsed, but only the leaf extensions are
// stored.
- if (status_request.present) {
+ if (have_status_request) {
+ if (ssl->server || !hs->config->ocsp_stapling_enabled) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
+ return false;
+ }
+
uint8_t status_type;
CBS ocsp_response;
- if (!CBS_get_u8(&status_request.data, &status_type) ||
+ if (!CBS_get_u8(&status_request, &status_type) ||
status_type != TLSEXT_STATUSTYPE_ocsp ||
- !CBS_get_u24_length_prefixed(&status_request.data, &ocsp_response) ||
+ !CBS_get_u24_length_prefixed(&status_request, &ocsp_response) ||
CBS_len(&ocsp_response) == 0 ||
- CBS_len(&status_request.data) != 0) {
+ CBS_len(&status_request) != 0) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return false;
}
@@ -272,8 +279,14 @@
}
}
- if (sct.present) {
- if (!ssl_is_sct_list_valid(&sct.data)) {
+ if (have_sct) {
+ if (ssl->server || !hs->config->signed_cert_timestamps_enabled) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
+ return false;
+ }
+
+ if (!ssl_is_sct_list_valid(&sct)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_ERROR_PARSING_EXTENSION);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return false;
@@ -281,7 +294,7 @@
if (sk_CRYPTO_BUFFER_num(certs.get()) == 1) {
hs->new_session->signed_cert_timestamp_list.reset(
- CRYPTO_BUFFER_new_from_CBS(&sct.data, ssl->ctx->pool));
+ CRYPTO_BUFFER_new_from_CBS(&sct, ssl->ctx->pool));
if (hs->new_session->signed_cert_timestamp_list == nullptr) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return false;
diff --git a/src/ssl/tls13_client.cc b/src/ssl/tls13_client.cc
index af2120c..bff7fb9 100644
--- a/src/ssl/tls13_client.cc
+++ b/src/ssl/tls13_client.cc
@@ -101,73 +101,6 @@
return true;
}
-static bool parse_server_hello_tls13(const SSL_HANDSHAKE *hs,
- ParsedServerHello *out, uint8_t *out_alert,
- const SSLMessage &msg) {
- if (!ssl_parse_server_hello(out, out_alert, msg)) {
- return false;
- }
- // The RFC8446 version of the structure fixes some legacy values.
- // Additionally, the session ID must echo the original one.
- if (out->legacy_version != TLS1_2_VERSION ||
- out->compression_method != 0 ||
- !CBS_mem_equal(&out->session_id, hs->session_id, hs->session_id_len) ||
- CBS_len(&out->extensions) == 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- *out_alert = SSL_AD_DECODE_ERROR;
- return false;
- }
- 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);
@@ -184,17 +117,36 @@
return ssl_hs_error;
}
- ParsedServerHello server_hello;
- uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!parse_server_hello_tls13(hs, &server_hello, &alert, msg)) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_SERVER_HELLO)) {
return ssl_hs_error;
}
- // The cipher suite must be one we offered. We currently offer all supported
- // TLS 1.3 ciphers, so check the version.
- const SSL_CIPHER *cipher = SSL_get_cipher_by_value(server_hello.cipher_suite);
- if (cipher == nullptr ||
+ CBS body = msg.body, extensions, server_random, session_id;
+ uint16_t server_version, cipher_suite;
+ uint8_t compression_method;
+ if (!CBS_get_u16(&body, &server_version) ||
+ !CBS_get_bytes(&body, &server_random, SSL3_RANDOM_SIZE) ||
+ !CBS_get_u8_length_prefixed(&body, &session_id) ||
+ !CBS_mem_equal(&session_id, hs->session_id, hs->session_id_len) ||
+ !CBS_get_u16(&body, &cipher_suite) ||
+ !CBS_get_u8(&body, &compression_method) ||
+ compression_method != 0 ||
+ !CBS_get_u16_length_prefixed(&body, &extensions) ||
+ CBS_len(&extensions) == 0 ||
+ CBS_len(&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;
+ }
+
+ if (!CBS_mem_equal(&server_random, kHelloRetryRequest, SSL3_RANDOM_SIZE)) {
+ hs->tls13_state = state_read_server_hello;
+ return ssl_hs_ok;
+ }
+
+ const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite);
+ // Check if the cipher is a TLS 1.3 cipher.
+ if (cipher == NULL ||
SSL_CIPHER_get_min_version(cipher) > ssl_protocol_version(ssl) ||
SSL_CIPHER_get_max_version(cipher) < ssl_protocol_version(ssl)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
@@ -204,60 +156,32 @@
hs->new_cipher = cipher;
- 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;
- }
- }
+ bool have_cookie, have_key_share, have_supported_versions;
+ CBS cookie, key_share, supported_versions;
+ SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ {TLSEXT_TYPE_cookie, &have_cookie, &cookie},
+ {TLSEXT_TYPE_supported_versions, &have_supported_versions,
+ &supported_versions},
+ };
- // 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),
- 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)) {
+ uint8_t alert = SSL_AD_DECODE_ERROR;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
+ /*ignore_unknown=*/false)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
- if (!cookie.present && !key_share.present) {
+ if (!have_cookie && !have_key_share) {
OPENSSL_PUT_ERROR(SSL, SSL_R_EMPTY_HELLO_RETRY_REQUEST);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
}
- if (cookie.present) {
+ if (have_cookie) {
CBS cookie_value;
- if (!CBS_get_u16_length_prefixed(&cookie.data, &cookie_value) ||
+ if (!CBS_get_u16_length_prefixed(&cookie, &cookie_value) ||
CBS_len(&cookie_value) == 0 ||
- CBS_len(&cookie.data) != 0) {
+ CBS_len(&cookie) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
@@ -268,10 +192,9 @@
}
}
- if (key_share.present) {
+ if (have_key_share) {
uint16_t group_id;
- if (!CBS_get_u16(&key_share.data, &group_id) ||
- CBS_len(&key_share.data) != 0) {
+ if (!CBS_get_u16(&key_share, &group_id) || CBS_len(&key_share) != 0) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
return ssl_hs_error;
@@ -298,16 +221,23 @@
}
}
- // 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)) {
+ // 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)) {
return ssl_hs_error;
}
- if (ssl->s3->ech_status == ssl_ech_accepted &&
- !hs->inner_transcript.Update(msg.raw)) {
- 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;
+ }
}
// HelloRetryRequest should be the end of the flight.
@@ -337,8 +267,7 @@
// Build the second ClientHelloInner, if applicable. The second ClientHello
// uses an empty string for |enc|.
- if (hs->ssl->s3->ech_status == ssl_ech_accepted &&
- !ssl_encrypt_client_hello(hs, {})) {
+ if (hs->selected_ech_config && !ssl_encrypt_client_hello(hs, {})) {
return ssl_hs_error;
}
@@ -357,70 +286,83 @@
if (!ssl->method->get_message(ssl, &msg)) {
return ssl_hs_read_message;
}
- ParsedServerHello server_hello;
- uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!parse_server_hello_tls13(hs, &server_hello, &alert, msg)) {
- ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
+ if (!ssl_check_message_type(ssl, msg, SSL3_MT_SERVER_HELLO)) {
+ return ssl_hs_error;
+ }
+
+ CBS body = msg.body, server_random, session_id, extensions;
+ uint16_t server_version;
+ uint16_t cipher_suite;
+ uint8_t compression_method;
+ if (!CBS_get_u16(&body, &server_version) ||
+ !CBS_get_bytes(&body, &server_random, SSL3_RANDOM_SIZE) ||
+ !CBS_get_u8_length_prefixed(&body, &session_id) ||
+ !CBS_mem_equal(&session_id, hs->session_id, hs->session_id_len) ||
+ !CBS_get_u16(&body, &cipher_suite) ||
+ !CBS_get_u8(&body, &compression_method) ||
+ compression_method != 0 ||
+ !CBS_get_u16_length_prefixed(&body, &extensions) ||
+ CBS_len(&body) != 0) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
+ return ssl_hs_error;
+ }
+
+ if (server_version != TLS1_2_VERSION) {
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_VERSION_NUMBER);
return ssl_hs_error;
}
// Forbid a second HelloRetryRequest.
- if (is_hello_retry_request(server_hello)) {
+ if (CBS_mem_equal(&server_random, kHelloRetryRequest, SSL3_RANDOM_SIZE)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNEXPECTED_MESSAGE);
OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_MESSAGE);
return ssl_hs_error;
}
- // Check the cipher suite, in case this is after HelloRetryRequest.
- if (SSL_CIPHER_get_value(hs->new_cipher) != server_hello.cipher_suite) {
+ OPENSSL_memcpy(ssl->s3->server_random, CBS_data(&server_random),
+ SSL3_RANDOM_SIZE);
+
+ // Check if the cipher is a TLS 1.3 cipher.
+ const SSL_CIPHER *cipher = SSL_get_cipher_by_value(cipher_suite);
+ if (cipher == nullptr ||
+ SSL_CIPHER_get_min_version(cipher) > ssl_protocol_version(ssl) ||
+ SSL_CIPHER_get_max_version(cipher) < ssl_protocol_version(ssl)) {
OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
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);
+ // Check that the cipher matches the one in the HelloRetryRequest.
+ if (ssl->s3->used_hello_retry_request && hs->new_cipher != cipher) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_WRONG_CIPHER_RETURNED);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
+ return ssl_hs_error;
}
- OPENSSL_memcpy(ssl->s3->server_random, CBS_data(&server_hello.random),
- SSL3_RANDOM_SIZE);
+ // Parse out the extensions.
+ bool have_key_share = false, have_pre_shared_key = false,
+ have_supported_versions = false;
+ CBS key_share, pre_shared_key, supported_versions;
+ SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_key_share, &have_key_share, &key_share},
+ {TLSEXT_TYPE_pre_shared_key, &have_pre_shared_key, &pre_shared_key},
+ {TLSEXT_TYPE_supported_versions, &have_supported_versions,
+ &supported_versions},
+ };
- // 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, 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},
+ uint8_t alert = SSL_AD_DECODE_ERROR;
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
/*ignore_unknown=*/false)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
- // Recheck supported_versions, in case this is after HelloRetryRequest.
+ // Recheck supported_versions, in case this is the second ServerHello.
uint16_t version;
- if (!supported_versions.present ||
- !CBS_get_u16(&supported_versions.data, &version) ||
- CBS_len(&supported_versions.data) != 0 ||
+ if (!have_supported_versions ||
+ !CBS_get_u16(&supported_versions, &version) ||
version != ssl->version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SECOND_SERVERHELLO_VERSION_MISMATCH);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
@@ -428,9 +370,15 @@
}
alert = SSL_AD_DECODE_ERROR;
- if (pre_shared_key.present) {
+ if (have_pre_shared_key) {
+ if (ssl->session == NULL) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_UNEXPECTED_EXTENSION);
+ ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_UNSUPPORTED_EXTENSION);
+ return ssl_hs_error;
+ }
+
if (!ssl_ext_pre_shared_key_parse_serverhello(hs, &alert,
- &pre_shared_key.data)) {
+ &pre_shared_key)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
@@ -441,7 +389,7 @@
return ssl_hs_error;
}
- if (ssl->session->cipher->algorithm_prf != hs->new_cipher->algorithm_prf) {
+ if (ssl->session->cipher->algorithm_prf != cipher->algorithm_prf) {
OPENSSL_PUT_ERROR(SSL, SSL_R_OLD_SESSION_PRF_HASH_MISMATCH);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
@@ -474,11 +422,13 @@
return ssl_hs_error;
}
- hs->new_session->cipher = hs->new_cipher;
+ hs->new_session->cipher = cipher;
+ hs->new_cipher = cipher;
+
+ size_t hash_len =
+ EVP_MD_size(ssl_get_handshake_digest(ssl_protocol_version(ssl), cipher));
// Set up the key schedule and incorporate the PSK into the running secret.
- size_t hash_len = EVP_MD_size(
- ssl_get_handshake_digest(ssl_protocol_version(ssl), hs->new_cipher));
if (!tls13_init_key_schedule(
hs, ssl->s3->session_reused
? MakeConstSpan(hs->new_session->secret,
@@ -487,7 +437,7 @@
return ssl_hs_error;
}
- if (!key_share.present) {
+ if (!have_key_share) {
// We do not support psk_ke and thus always require a key share.
OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_KEY_SHARE);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
@@ -498,13 +448,53 @@
Array<uint8_t> dhe_secret;
alert = SSL_AD_DECODE_ERROR;
if (!ssl_ext_key_share_parse_serverhello(hs, &dhe_secret, &alert,
- &key_share.data)) {
+ &key_share)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
- if (!tls13_advance_key_schedule(hs, dhe_secret) ||
- !ssl_hash_message(hs, msg) ||
+ 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 (have_pre_shared_key) {
+ 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) ||
!tls13_derive_handshake_secrets(hs)) {
return ssl_hs_error;
}
@@ -542,16 +532,14 @@
return ssl_hs_error;
}
- CBS body = msg.body, extensions;
- if (!CBS_get_u16_length_prefixed(&body, &extensions) ||
- CBS_len(&body) != 0) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
- ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
+ CBS body = msg.body;
+ if (!ssl_parse_serverhello_tlsext(hs, &body)) {
+ OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
return ssl_hs_error;
}
-
- if (!ssl_parse_serverhello_tlsext(hs, &extensions)) {
- OPENSSL_PUT_ERROR(SSL, SSL_R_PARSE_TLSEXT);
+ if (CBS_len(&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;
}
@@ -638,19 +626,25 @@
}
- SSLExtension sigalgs(TLSEXT_TYPE_signature_algorithms),
- ca(TLSEXT_TYPE_certificate_authorities);
+ bool have_sigalgs = false, have_ca = false;
+ CBS sigalgs, ca;
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_signature_algorithms, &have_sigalgs, &sigalgs},
+ {TLSEXT_TYPE_certificate_authorities, &have_ca, &ca},
+ };
+
CBS body = msg.body, context, extensions, supported_signature_algorithms;
uint8_t alert = SSL_AD_DECODE_ERROR;
if (!CBS_get_u8_length_prefixed(&body, &context) ||
// The request context is always empty during the handshake.
CBS_len(&context) != 0 ||
- !CBS_get_u16_length_prefixed(&body, &extensions) || //
+ !CBS_get_u16_length_prefixed(&body, &extensions) ||
CBS_len(&body) != 0 ||
- !ssl_parse_extensions(&extensions, &alert, {&sigalgs, &ca},
+ !ssl_parse_extensions(&extensions, &alert, ext_types,
/*ignore_unknown=*/true) ||
- !sigalgs.present ||
- !CBS_get_u16_length_prefixed(&sigalgs.data,
+ (have_ca && CBS_len(&ca) == 0) ||
+ !have_sigalgs ||
+ !CBS_get_u16_length_prefixed(&sigalgs,
&supported_signature_algorithms) ||
!tls1_parse_peer_sigalgs(hs, &supported_signature_algorithms)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
@@ -658,8 +652,8 @@
return ssl_hs_error;
}
- if (ca.present) {
- hs->ca_names = ssl_parse_client_CA_list(ssl, &alert, &ca.data);
+ if (have_ca) {
+ hs->ca_names = ssl_parse_client_CA_list(ssl, &alert, &ca);
if (!hs->ca_names) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
@@ -1082,17 +1076,23 @@
return nullptr;
}
- SSLExtension early_data(TLSEXT_TYPE_early_data);
+ // Parse out the extensions.
+ bool have_early_data = false;
+ CBS early_data;
+ const SSL_EXTENSION_TYPE ext_types[] = {
+ {TLSEXT_TYPE_early_data, &have_early_data, &early_data},
+ };
+
uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!ssl_parse_extensions(&extensions, &alert, {&early_data},
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
/*ignore_unknown=*/true)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return nullptr;
}
- if (early_data.present) {
- if (!CBS_get_u32(&early_data.data, &session->ticket_max_early_data) ||
- CBS_len(&early_data.data) != 0) {
+ if (have_early_data) {
+ if (!CBS_get_u32(&early_data, &session->ticket_max_early_data) ||
+ CBS_len(&early_data) != 0) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR);
OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR);
return nullptr;
diff --git a/src/ssl/tls13_enc.cc b/src/ssl/tls13_enc.cc
index c7b75a6..174f5f1 100644
--- a/src/ssl/tls13_enc.cc
+++ b/src/ssl/tls13_enc.cc
@@ -489,8 +489,8 @@
return false;
}
- auto msg_binder = msg.last(verify_data_len);
- OPENSSL_memcpy(msg_binder.data(), verify_data, verify_data_len);
+ OPENSSL_memcpy(msg.data() + msg.size() - verify_data_len, verify_data,
+ verify_data_len);
if (out_binder_len != nullptr) {
*out_binder_len = verify_data_len;
}
@@ -537,46 +537,57 @@
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) {
+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) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
return false;
}
- 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;
+ 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;
ScopedEVP_MD_CTX ctx;
if (!transcript.CopyToHashContext(ctx.get(), transcript.Digest()) ||
- !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)) {
+ !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)) {
return false;
}
- uint8_t secret[EVP_MAX_MD_SIZE];
- size_t secret_len;
- if (!HKDF_extract(secret, &secret_len, transcript.Digest(),
- client_random.data(), client_random.size(), kZeros,
- transcript.DigestLen())) {
+ // 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))) {
return false;
}
- 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));
+ 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;
}
BSSL_NAMESPACE_END
diff --git a/src/ssl/tls13_server.cc b/src/ssl/tls13_server.cc
index 2f000e5..501d01f 100644
--- a/src/ssl/tls13_server.cc
+++ b/src/ssl/tls13_server.cc
@@ -246,7 +246,8 @@
return ssl_hs_error;
}
- // The PRF hash is now known.
+ // The PRF hash is now known. Set up the key schedule and hash the
+ // ClientHello.
if (!hs->transcript.InitHash(ssl_protocol_version(ssl), hs->new_cipher)) {
return ssl_hs_error;
}
@@ -269,7 +270,7 @@
return ssl_ticket_aead_ignore_ticket;
}
- // Per RFC 8446, section 4.2.9, servers MUST abort the handshake if the client
+ // Per RFC8446, section 4.2.9, servers MUST abort the handshake if the client
// sends pre_shared_key without psk_key_exchange_modes.
CBS unused;
if (!ssl_client_hello_get_extension(client_hello, &unused,
@@ -570,34 +571,12 @@
!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)) {
+ !CBB_add_u16(&extensions, group_id) ||
+ !ssl_add_message_cbb(ssl, cbb.get())) {
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_message(ssl, std::move(hrr)) ||
- !ssl->method->add_change_cipher_spec(ssl)) {
+ if (!ssl->method->add_change_cipher_spec(ssl)) {
return ssl_hs_error;
}
@@ -623,8 +602,8 @@
}
if (ssl->s3->ech_status == ssl_ech_accepted) {
- // If we previously accepted the ClientHelloInner, the second ClientHello
- // must contain an outer encrypted_client_hello extension.
+ // If we previously accepted the ClientHelloInner, check that the second
+ // ClientHello contains an encrypted_client_hello extension.
CBS ech_body;
if (!ssl_client_hello_get_extension(&client_hello, &ech_body,
TLSEXT_TYPE_encrypted_client_hello)) {
@@ -632,12 +611,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 type, config_id;
+ uint8_t config_id;
CBS enc, payload;
- if (!CBS_get_u8(&ech_body, &type) || //
- type != ECH_CLIENT_OUTER || //
- !CBS_get_u16(&ech_body, &kdf_id) || //
+ 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) ||
@@ -648,6 +627,8 @@
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())) ||
@@ -660,9 +641,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, payload)) {
+ 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)) {
// 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);
@@ -780,18 +761,18 @@
return ssl_hs_error;
}
- assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner);
- if (hs->ech_is_inner) {
+ assert(ssl->s3->ech_status != ssl_ech_accepted || hs->ech_is_inner_present);
+ if (hs->ech_is_inner_present) {
// 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, ssl->s3->client_random,
- hs->transcript,
- /*is_hrr=*/false, server_hello, offset)) {
+ Span<uint8_t> random_suffix =
+ random.subspan(SSL3_RANDOM_SIZE - ECH_CONFIRMATION_SIGNAL_LEN);
+ if (!ssl_ech_accept_confirmation(hs, random_suffix, hs->transcript,
+ server_hello)) {
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(),
@@ -1060,15 +1041,20 @@
return ssl_hs_error;
}
- SSLExtension application_settings(TLSEXT_TYPE_application_settings);
+ // Parse out the extensions.
+ bool have_application_settings = false;
+ CBS application_settings;
+ SSL_EXTENSION_TYPE ext_types[] = {{TLSEXT_TYPE_application_settings,
+ &have_application_settings,
+ &application_settings}};
uint8_t alert = SSL_AD_DECODE_ERROR;
- if (!ssl_parse_extensions(&extensions, &alert, {&application_settings},
+ if (!ssl_parse_extensions(&extensions, &alert, ext_types,
/*ignore_unknown=*/false)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
- if (!application_settings.present) {
+ if (!have_application_settings) {
OPENSSL_PUT_ERROR(SSL, SSL_R_MISSING_EXTENSION);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_MISSING_EXTENSION);
return ssl_hs_error;
@@ -1077,7 +1063,7 @@
// Note that, if 0-RTT was accepted, these values will already have been
// initialized earlier.
if (!hs->new_session->peer_application_settings.CopyFrom(
- application_settings.data) ||
+ application_settings) ||
!ssl_hash_message(hs, msg)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
return ssl_hs_error;