diff options
author | Pete Bentley <prb@google.com> | 2021-09-23 10:49:18 +0100 |
---|---|---|
committer | Pete Bentley <prb@google.com> | 2021-09-27 19:05:23 +0100 |
commit | 8bb65ff676b006f67cccd75a8f4cd6f8a296409e (patch) | |
tree | 741bef31d3d7f6d72020097ad72fc7b7a1e0ec9c /src/ssl/encrypted_client_hello.cc | |
parent | f54ba3566f9139b088c0328ceccc69b7d5dab09d (diff) | |
download | boringssl-8bb65ff676b006f67cccd75a8f4cd6f8a296409e.tar.gz |
external/boringssl: Sync to 66e61c577d39e757bf491468f651461fa79fd5e1.
This includes the following changes:
https://boringssl.googlesource.com/boringssl/+log/c1571feb5faf5cce844354c63d0f3e842464bea3..66e61c577d39e757bf491468f651461fa79fd5e1
* Allow PKCS7_sign to work for signing kernel modules.
* Speed up constant-time base64 decoding.
* Unwind remnants of ASN1_TFLG_NDEF.
* acvptool: add CS3 support.
* Ignore SIGPIPE in the bssl tool.
* Add FIPS counters for AES-GCM in EVP_AEAD.
* Refresh fuzzer corpus for ECH draft-13.
* Fix the TLS fuzzers for ECH draft-13.
* Clarify that TLS sessions are not application sessions.
* Fix BN_prime_checks_for_validation to align with false-positive rate.
* Add maskHash to RSA_PSS_PARAMS for compat
* Remove ASN1_OP_I2D_* callbacks.
* Don't read it->funcs without checking it->itype.
* Reject missing required fields in i2d functions.
Update-Note: Structures with missing mandatory fields can no longer be
encoded. Note that, apart from the cases already handled by preceding
CLs, tasn_new.c will fill in non-NULL empty objects everywhere. The main
downstream impact I've seen of this particular change is in combination
with other bugs. Consider a caller that does:
* Reject -1 types in ASN1_TYPE and MSTRINGs when encoding.
Update-Note: A default-constructed object with a required ANY or
string-like CHOICE field cannot be encoded until the field is specified.
Note this affects i2d_X509: notBefore and notAfter are string-like
CHOICEs in OpenSSL.
* Correctly handle invalid ASN1_OBJECTs when encoding.
Update-Note: A default-constructed object with a required ASN1_OBJECT
field can no longer be encoded without initializing the ASN1_OBJECT.
Note this affects X509: the signature algorithm is an ASN1_OBJECT. Tests
that try to serialize an X509_new() must fill in all required fields.
(Production code is unlikely to be affected because the output was
unparsable anyway, while tests sometimes wouldn't notice.)
* Check for invalid CHOICE selectors in i2d functions.
Update-Note: An invalid CHOICE object (e.g. GENERAL_NAME) will now fail
when encoded, rather than be silently omitted. In particular, CHOICE
objects are default-initialized by tasn_new.c in an empty -1 state.
Structures containing a required CHOICE field can no longer be encoded
without filling in the CHOICE.
* Fix x509_name_ex_i2d error-handling.
* Correctly propagate errors in i2d functions.
Update-Note: Some error cases which were silently misinterpreted as
missing OPTIONAL elements will now cause encoding to fail.
* acvptool: add hmacDRBG support
* Check for __TRUSTY__ instead of TRUSTY.
* Update comment for ECH draft-13.
* Silence a GCC false positive warning.
* Switch to the new, simpler WHATWG URL formulation.
* Revert "Guard use of sdallocx with BORINGSSL_SDALLOCX"
* Fix calculation of draft-13 ECH confirmation signal.
* Update to draft-ietf-tls-esni-13.
* Reword SSL_get0_ech_name_override documentation.
* Remove SSL_set_verify_result.
* Make most of crypto/x509 opaque.
Update-Note: Patch cl/390055173 into the roll that includes this. This
unexports most of the X.509 structs, aligning with OpenSSL. Use the
accessor APIs instead.
* Remove V_ASN1_APP_CHOOSE.
Update-Note: V_ASN1_APP_CHOOSE is removed. I only found one use, which
has been fixed.
* Rewrite ASN1_PRINTABLE_type and add tests.
* Include SHA512-256 in EVP_get_digestbyname and EVP_MD_do_all.
* NUL is not printable.
Update-Note: ASN1_mbstring_ncopy will no longer allow PrintableString
for strings containing NUL.
* Make RSA_check_key more than 2x as fast.
* Benchmark RSA private key parsing.
* Work around yet another MSVC 2015 SFINAE bug.
* Avoid re-hashing the transcript multiple times.
* Make ssl_parse_extensions a little easier to use.
* Deduplicate our three ServerHello parsers.
* Merge in OpenSSL's X.509 corpus.
* Run X509_print in the certificate fuzzer.
* Fix some error-handling in i2v functions.
* Fix typo.
* OPENSSL_strndup should not return NULL given {NULL, 0}.
* Rewrite name constraints matching with CBS.
* Add some tests for name constraints.
* Fix i2v_GENERAL_NAME to not assume NUL terminated strings
* Do not rely on ASN1_STRING being NUL-terminated.
* Add a CBB_add_zeros helper.
* Linkify RFCs in documentation.
* Refer to RFCs consistently.
* runner: Test session IDs over 32 bytes.
* Process the TLS 1.3 cipher suite in one place.
* Guard use of sdallocx with BORINGSSL_SDALLOCX
* Bump minimum GCC version and note impending VS2015 deprecation.
* Add Span::first() and Span::last().
* Simplify built-in BIOs slightly.
* Fix some error returns from SSL_read and SSL_write.
* Fix negative ENUMERATED values in multi-strings.
* Add a test for ASN1_mbstring_copy and clean up.
* Remove ASN1_TFLG_SET_ORDER.
* Fix ASN1_STRING_print_ex with negative integers.
* Check i2d_ASN1_TYPE's return value in ASN1_STRING_print_ex.
* Document ASN.1 printing functions.
* Move some ASN1 printing functions to crypto/asn1.
* Move a_strex.c back to asn1, split X509_NAME bits out.
* Unwind io_ch abstraction in print functions.
* Implement ASN1_STRING_print_ex_fp, etc., with file BIOs.
* Remove OPENSSL_NO_FP_API ifdefs.
* Move X509_ALGOR to x509.h.
* Unexport BIT_STRING_BITNAME.
* Unexport ub_* constants.
Update-Note: Removed some unnamespaced constants.
* Always use an ASN1_STRING_TABLE global mask of UTF8String.
Update-Note: The global mask for ASN1_STRING_set_by_NID is now always
UTF-8. Callers that want another type should reconsider and, if UTF-8 is
still unsuitable, just pass the actual desired type into
ASN1_mbstring_copy, X509_NAME_ENTRY_set_data, etc
* Document ASN1_mbstring_copy.
* Update ghashv8-armx.pl from upstream.
* Align with upstream on 'close STDOUT' lines.
* Avoid double-expanding variables in CMake.
* Reject years outside 0000-9999 in ASN1_GENERALIZEDTIME_adj.
* Add some tests for time_t to ASN1_TIME conversions.
* Remove ASN1_STRING_FLAG_MSTRING.
Update-Note: ASN1_STRING_FLAG_MSTRING is no longer defined and
X509_time_adj_ex now behaves more predictably. Callers that actually
wanted to lock to a specific type should call ASN1_UTCTIME_adj or
ASN1_GENERALIZEDTIME_adj instead.
* Document another batch of functions.
* Clarify BIO_new_mum_buf's lifetime rules.
* generate_ech.cc: include needed headers
* Don't overread in poly_Rq_mul
* acvp: recognise another style of JSON.
* Revert "Revert "Revert "Disable check that X.509 extensions implies v3."""
Test: atest CtsLibcoreTestCases CtsLibcoreOkHttpTestCases
Change-Id: I4f2228ef815ded0599322186ab7bad49ab1bb5af
Diffstat (limited to 'src/ssl/encrypted_client_hello.cc')
-rw-r--r-- | src/ssl/encrypted_client_hello.cc | 623 |
1 files changed, 266 insertions, 357 deletions
diff --git a/src/ssl/encrypted_client_hello.cc b/src/ssl/encrypted_client_hello.cc index b70f66c1..64fee3d0 100644 --- a/src/ssl/encrypted_client_hello.cc +++ b/src/ssl/encrypted_client_hello.cc @@ -31,12 +31,6 @@ #include "internal.h" -#if defined(OPENSSL_MSAN) -#define NO_SANITIZE_MEMORY __attribute__((no_sanitize("memory"))) -#else -#define NO_SANITIZE_MEMORY -#endif - BSSL_NAMESPACE_BEGIN // ECH reuses the extension code point for the version number. @@ -84,16 +78,71 @@ static bool ssl_client_hello_write_without_extensions( 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; + 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, + TLSEXT_TYPE_supported_versions)) { + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER); + return false; + } + // Parse supported_versions and reject TLS versions prior to TLS 1.3. Older + // versions are incompatible with ECH. + CBS versions; + if (!CBS_get_u8_length_prefixed(&extension, &versions) || + CBS_len(&extension) != 0 || // + CBS_len(&versions) == 0) { + *out_alert = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + return false; + } + while (CBS_len(&versions) != 0) { + uint16_t version; + if (!CBS_get_u16(&versions, &version)) { + *out_alert = SSL_AD_DECODE_ERROR; + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + return false; + } + if (version == SSL3_VERSION || version == TLS1_VERSION || + version == TLS1_1_VERSION || version == TLS1_2_VERSION || + version == DTLS1_VERSION || version == DTLS1_2_VERSION) { + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER); + 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; - if (!ssl_client_hello_init(ssl, &client_hello_inner, - encoded_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 || @@ -106,120 +155,84 @@ bool ssl_decode_client_hello_inner( // Begin serializing a message containing the ClientHelloInner in |cbb|. ScopedCBB cbb; - CBB body, extensions; + 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_add_u16_length_prefixed(&body, &extensions_cbb)) { 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)) { + 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; } - 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); + } 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; } - 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) { + // 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; } - 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; + 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; } - // 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; + // 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; } + } - // 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_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)) { @@ -227,46 +240,10 @@ bool ssl_decode_client_hello_inner( return false; } - // See https://github.com/tlswg/draft-ietf-tls-esni/pull/411 - CBS extension; - if (!ssl_client_hello_init(ssl, &client_hello_inner, - MakeConstSpan(CBB_data(&body), CBB_len(&body))) || - !ssl_client_hello_get_extension(&client_hello_inner, &extension, - TLSEXT_TYPE_ech_is_inner) || - CBS_len(&extension) != 0 || - ssl_client_hello_get_extension(&client_hello_inner, &extension, - TLSEXT_TYPE_encrypted_client_hello) || - !ssl_client_hello_get_extension(&client_hello_inner, &extension, - TLSEXT_TYPE_supported_versions)) { - *out_alert = SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER); - return false; - } - // Parse supported_versions and reject TLS versions prior to TLS 1.3. Older - // versions are incompatible with ECH. - CBS versions; - if (!CBS_get_u8_length_prefixed(&extension, &versions) || - CBS_len(&extension) != 0 || // - CBS_len(&versions) == 0) { - *out_alert = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + if (!is_valid_client_hello_inner( + ssl, out_alert, MakeConstSpan(CBB_data(&body), CBB_len(&body)))) { return false; } - while (CBS_len(&versions) != 0) { - uint16_t version; - if (!CBS_get_u16(&versions, &version)) { - *out_alert = SSL_AD_DECODE_ERROR; - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - return false; - } - if (version == SSL3_VERSION || version == TLS1_VERSION || - version == TLS1_1_VERSION || version == TLS1_2_VERSION || - version == DTLS1_VERSION || version == DTLS1_2_VERSION) { - *out_alert = SSL_AD_ILLEGAL_PARAMETER; - OPENSSL_PUT_ERROR(SSL, SSL_R_INVALID_CLIENT_HELLO_INNER); - return false; - } - } if (!ssl->method->finish_message(ssl, cbb.get(), out_client_hello_inner)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); @@ -275,56 +252,31 @@ bool ssl_decode_client_hello_inner( return true; } -bool ssl_client_hello_decrypt( - EVP_HPKE_CTX *hpke_ctx, Array<uint8_t> *out_encoded_client_hello_inner, - bool *out_is_decrypt_error, const SSL_CLIENT_HELLO *client_hello_outer, - uint16_t kdf_id, uint16_t aead_id, const uint8_t config_id, - Span<const uint8_t> enc, Span<const uint8_t> payload) { +bool ssl_client_hello_decrypt(EVP_HPKE_CTX *hpke_ctx, Array<uint8_t> *out, + bool *out_is_decrypt_error, + const SSL_CLIENT_HELLO *client_hello_outer, + Span<const uint8_t> payload) { *out_is_decrypt_error = false; - // Compute the ClientHello portion of the ClientHelloOuterAAD value. See - // draft-ietf-tls-esni-10, section 5.2. - ScopedCBB aad; - CBB enc_cbb, outer_hello_cbb, extensions_cbb; - if (!CBB_init(aad.get(), 256) || - !CBB_add_u16(aad.get(), kdf_id) || - !CBB_add_u16(aad.get(), aead_id) || - !CBB_add_u8(aad.get(), config_id) || - !CBB_add_u16_length_prefixed(aad.get(), &enc_cbb) || - !CBB_add_bytes(&enc_cbb, enc.data(), enc.size()) || - !CBB_add_u24_length_prefixed(aad.get(), &outer_hello_cbb) || - !ssl_client_hello_write_without_extensions(client_hello_outer, - &outer_hello_cbb) || - !CBB_add_u16_length_prefixed(&outer_hello_cbb, &extensions_cbb)) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); + // The ClientHelloOuterAAD is |client_hello_outer| with |payload| (which must + // point within |client_hello_outer->extensions|) replaced with zeros. See + // draft-ietf-tls-esni-13, section 5.2. + Array<uint8_t> aad; + if (!aad.CopyFrom(MakeConstSpan(client_hello_outer->client_hello, + client_hello_outer->client_hello_len))) { return false; } - CBS extensions(MakeConstSpan(client_hello_outer->extensions, - client_hello_outer->extensions_len)); - while (CBS_len(&extensions) > 0) { - uint16_t extension_id; - CBS extension_body; - if (!CBS_get_u16(&extensions, &extension_id) || - !CBS_get_u16_length_prefixed(&extensions, &extension_body)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); - return false; - } - if (extension_id == TLSEXT_TYPE_encrypted_client_hello) { - continue; - } - if (!CBB_add_u16(&extensions_cbb, extension_id) || - !CBB_add_u16(&extensions_cbb, CBS_len(&extension_body)) || - !CBB_add_bytes(&extensions_cbb, CBS_data(&extension_body), - CBS_len(&extension_body))) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - return false; - } - } - if (!CBB_flush(aad.get())) { - OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); - return false; - } + // We assert with |uintptr_t| because the comparison would be UB if they + // didn't alias. + assert(reinterpret_cast<uintptr_t>(client_hello_outer->extensions) <= + reinterpret_cast<uintptr_t>(payload.data())); + assert(reinterpret_cast<uintptr_t>(client_hello_outer->extensions + + client_hello_outer->extensions_len) >= + reinterpret_cast<uintptr_t>(payload.data() + payload.size())); + Span<uint8_t> payload_aad = MakeSpan(aad).subspan( + payload.data() - client_hello_outer->client_hello, payload.size()); + OPENSSL_memset(payload_aad.data(), 0, payload_aad.size()); #if defined(BORINGSSL_UNSAFE_FUZZER_MODE) // In fuzzer mode, disable encryption to improve coverage. We reserve a short @@ -336,124 +288,75 @@ bool ssl_client_hello_decrypt( OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED); return false; } - if (!out_encoded_client_hello_inner->CopyFrom(payload)) { + if (!out->CopyFrom(payload)) { return false; } #else - // Attempt to decrypt into |out_encoded_client_hello_inner|. - if (!out_encoded_client_hello_inner->Init(payload.size())) { + // Attempt to decrypt into |out|. + if (!out->Init(payload.size())) { OPENSSL_PUT_ERROR(SSL, ERR_R_MALLOC_FAILURE); return false; } - size_t encoded_client_hello_inner_len; - if (!EVP_HPKE_CTX_open(hpke_ctx, out_encoded_client_hello_inner->data(), - &encoded_client_hello_inner_len, - out_encoded_client_hello_inner->size(), payload.data(), - payload.size(), CBB_data(aad.get()), - CBB_len(aad.get()))) { + size_t len; + if (!EVP_HPKE_CTX_open(hpke_ctx, out->data(), &len, out->size(), + payload.data(), payload.size(), aad.data(), + aad.size())) { *out_is_decrypt_error = true; OPENSSL_PUT_ERROR(SSL, SSL_R_DECRYPTION_FAILED); return false; } - out_encoded_client_hello_inner->Shrink(encoded_client_hello_inner_len); + out->Shrink(len); #endif return true; } -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; - } - *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) { +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; + } + for (uint8_t b : in.subspan(2)) { + if (!('0' <= b && b <= '9') && !('a' <= b && b <= 'f') && + !('A' <= b && b <= 'F')) { return false; } - *out += d; } return true; } -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) { +static bool is_decimal_component(Span<const uint8_t> in) { + if (in.empty()) { return false; } - for (size_t i = 0; i < num_numbers - 1; i++) { - if (numbers[i] > 255) { + for (uint8_t b : in) { + if (!('0' <= b && b <= '9')) { return false; } } - return num_numbers == 1 || - numbers[num_numbers - 1] < 1u << (8 * (5 - num_numbers)); + return true; } bool ssl_is_valid_ech_public_name(Span<const uint8_t> public_name) { - // See draft-ietf-tls-esni-11, Section 4 and RFC5890, Section 2.3.1. The + // See draft-ietf-tls-esni-13, Section 4 and RFC 5890, Section 2.3.1. The // public name must be a dot-separated sequence of LDH labels and not begin or // end with a dot. - auto copy = public_name; - if (copy.empty()) { + auto remaining = public_name; + if (remaining.empty()) { return false; } - while (!copy.empty()) { + Span<const uint8_t> last; + while (!remaining.empty()) { // Find the next dot-separated component. - auto dot = std::find(copy.begin(), copy.end(), '.'); + auto dot = std::find(remaining.begin(), remaining.end(), '.'); Span<const uint8_t> component; - if (dot == copy.end()) { - component = copy; - copy = Span<const uint8_t>(); + if (dot == remaining.end()) { + component = remaining; + last = component; + remaining = Span<const uint8_t>(); } else { - component = copy.subspan(0, dot - copy.begin()); - copy = copy.subspan(dot - copy.begin() + 1); // Skip the dot. - if (copy.empty()) { + component = remaining.subspan(0, dot - remaining.begin()); + // Skip the dot. + remaining = remaining.subspan(dot - remaining.begin() + 1); + if (remaining.empty()) { // Trailing dots are not allowed. return false; } @@ -472,7 +375,15 @@ bool ssl_is_valid_ech_public_name(Span<const uint8_t> public_name) { } } - return !is_ipv4_address(public_name); + // 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); } static bool parse_ech_config(CBS *cbs, ECHConfig *out, bool *out_supported, @@ -508,8 +419,8 @@ static bool parse_ech_config(CBS *cbs, ECHConfig *out, bool *out_supported, CBS_len(&public_key) == 0 || !CBS_get_u16_length_prefixed(&contents, &cipher_suites) || CBS_len(&cipher_suites) == 0 || CBS_len(&cipher_suites) % 4 != 0 || - !CBS_get_u16(&contents, &out->maximum_name_length) || - !CBS_get_u16_length_prefixed(&contents, &public_name) || + !CBS_get_u8(&contents, &out->maximum_name_length) || + !CBS_get_u8_length_prefixed(&contents, &public_name) || CBS_len(&public_name) == 0 || !CBS_get_u16_length_prefixed(&contents, &extensions) || CBS_len(&contents) != 0) { @@ -773,15 +684,6 @@ static size_t aead_overhead(const EVP_HPKE_AEAD *aead) { #endif } -static size_t compute_extension_length(const EVP_HPKE_AEAD *aead, - size_t enc_len, size_t in_len) { - size_t ret = 4; // HpkeSymmetricCipherSuite cipher_suite - ret++; // uint8 config_id - ret += 2 + enc_len; // opaque enc<1..2^16-1> - ret += 2 + in_len + aead_overhead(aead); // opaque payload<1..2^16-1> - return ret; -} - // random_size returns a random value between |min| and |max|, inclusive. static size_t random_size(size_t min, size_t max) { assert(min < max); @@ -814,38 +716,32 @@ static bool setup_ech_grease(SSL_HANDSHAKE *hs) { // 2+32+1+2 version, random, legacy_session_id, legacy_compression_methods // 2+4*2 cipher_suites (three TLS 1.3 ciphers, GREASE) // 2 extensions prefix - // 4 ech_is_inner + // 5 inner encrypted_client_hello // 4+1+2*2 supported_versions (TLS 1.3, GREASE) // 4+1+10*2 outer_extensions (key_share, sigalgs, sct, alpn, // supported_groups, status_request, psk_key_exchange_modes, // compress_certificate, GREASE x2) // // The server_name extension has an overhead of 9 bytes. For now, arbitrarily - // estimate maximum_name_length to be between 32 and 100 bytes. - // - // TODO(https://crbug.com/boringssl/275): If the padding scheme changes to - // also round the entire payload, adjust this to match. See - // https://github.com/tlswg/draft-ietf-tls-esni/issues/433 - const size_t overhead = aead_overhead(aead); - const size_t in_len = random_size(128, 196); - const size_t extension_len = - compute_extension_length(aead, sizeof(enc), in_len); + // estimate maximum_name_length to be between 32 and 100 bytes. Then round up + // to a multiple of 32, to match draft-ietf-tls-esni-13, section 6.1.3. + const size_t payload_len = + 32 * random_size(128 / 32, 224 / 32) + aead_overhead(aead); bssl::ScopedCBB cbb; CBB enc_cbb, payload_cbb; uint8_t *payload; - if (!CBB_init(cbb.get(), extension_len) || + if (!CBB_init(cbb.get(), 256) || !CBB_add_u16(cbb.get(), kdf_id) || !CBB_add_u16(cbb.get(), EVP_HPKE_AEAD_id(aead)) || !CBB_add_u8(cbb.get(), config_id) || !CBB_add_u16_length_prefixed(cbb.get(), &enc_cbb) || !CBB_add_bytes(&enc_cbb, enc, sizeof(enc)) || !CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb) || - !CBB_add_space(&payload_cbb, &payload, in_len + overhead) || - !RAND_bytes(payload, in_len + overhead) || - !CBBFinishArray(cbb.get(), &hs->ech_client_bytes)) { + !CBB_add_space(&payload_cbb, &payload, payload_len) || + !RAND_bytes(payload, payload_len) || + !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) { return false; } - assert(hs->ech_client_bytes.size() == extension_len); return true; } @@ -856,22 +752,22 @@ bool ssl_encrypt_client_hello(SSL_HANDSHAKE *hs, Span<const uint8_t> enc) { } // Construct ClientHelloInner and EncodedClientHelloInner. See - // draft-ietf-tls-esni-10, sections 5.1 and 6.1. - bssl::ScopedCBB cbb, encoded; + // draft-ietf-tls-esni-13, sections 5.1 and 6.1. + ScopedCBB cbb, encoded_cbb; CBB body; bool needs_psk_binder; - bssl::Array<uint8_t> hello_inner; + Array<uint8_t> hello_inner; if (!ssl->method->init_message(ssl, cbb.get(), &body, SSL3_MT_CLIENT_HELLO) || - !CBB_init(encoded.get(), 256) || + !CBB_init(encoded_cbb.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.get(), + !ssl_write_client_hello_without_extensions(hs, encoded_cbb.get(), ssl_client_hello_inner, /*empty_session_id=*/true) || - !ssl_add_clienthello_tlsext(hs, &body, encoded.get(), &needs_psk_binder, - ssl_client_hello_inner, CBB_len(&body), - /*omit_ech_len=*/0) || + !ssl_add_clienthello_tlsext(hs, &body, encoded_cbb.get(), + &needs_psk_binder, ssl_client_hello_inner, + CBB_len(&body)) || !ssl->method->finish_message(ssl, cbb.get(), &hello_inner)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return false; @@ -884,13 +780,12 @@ bool ssl_encrypt_client_hello(SSL_HANDSHAKE *hs, Span<const uint8_t> enc) { return false; } // Also update the EncodedClientHelloInner. - 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, + auto encoded_binder = + MakeSpan(const_cast<uint8_t *>(CBB_data(encoded_cbb.get())), + CBB_len(encoded_cbb.get())) + .last(binder_len); + auto hello_inner_binder = MakeConstSpan(hello_inner).last(binder_len); + OPENSSL_memcpy(encoded_binder.data(), hello_inner_binder.data(), binder_len); } @@ -898,74 +793,82 @@ bool ssl_encrypt_client_hello(SSL_HANDSHAKE *hs, Span<const uint8_t> enc) { return false; } - // Construct ClientHelloOuterAAD. See draft-ietf-tls-esni-10, section 5.2. - // TODO(https://crbug.com/boringssl/275): This ends up constructing the - // ClientHelloOuter twice. Revisit this in the next draft, which uses a more - // forgiving construction. + // 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. const EVP_HPKE_KDF *kdf = EVP_HPKE_CTX_kdf(hs->ech_hpke_ctx.get()); const EVP_HPKE_AEAD *aead = EVP_HPKE_CTX_aead(hs->ech_hpke_ctx.get()); - const size_t extension_len = - compute_extension_length(aead, enc.size(), CBB_len(encoded.get())); + size_t payload_len = encoded.size() + aead_overhead(aead); + CBB enc_cbb, payload_cbb; + if (!CBB_init(cbb.get(), 256) || + !CBB_add_u16(cbb.get(), EVP_HPKE_KDF_id(kdf)) || + !CBB_add_u16(cbb.get(), EVP_HPKE_AEAD_id(aead)) || + !CBB_add_u8(cbb.get(), hs->selected_ech_config->config_id) || + !CBB_add_u16_length_prefixed(cbb.get(), &enc_cbb) || + !CBB_add_bytes(&enc_cbb, enc.data(), enc.size()) || + !CBB_add_u16_length_prefixed(cbb.get(), &payload_cbb) || + !CBB_add_zeros(&payload_cbb, payload_len) || + !CBBFinishArray(cbb.get(), &hs->ech_client_outer)) { + 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; - 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_write_client_hello_without_extensions(hs, aad.get(), ssl_client_hello_outer, /*empty_session_id=*/false) || - !ssl_add_clienthello_tlsext(hs, &outer_hello, /*out_encoded=*/nullptr, + !ssl_add_clienthello_tlsext(hs, aad.get(), /*out_encoded=*/nullptr, &needs_psk_binder, ssl_client_hello_outer, - CBB_len(&outer_hello), - /*omit_ech_len=*/4 + extension_len) || - !CBB_flush(aad.get())) { + 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); - 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)) { - return false; - } + // 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. - if (!CBB_add_bytes(&payload_cbb, CBB_data(encoded.get()), - CBB_len(encoded.get()))) { - return false; - } + assert(payload_span.size() == encoded.size()); + OPENSSL_memcpy(payload_span.data(), encoded.data(), encoded.size()); #else - 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()), + if (!EVP_HPKE_CTX_seal(hs->ech_hpke_ctx.get(), payload_span.data(), + &payload_len, payload_span.size(), encoded.data(), + encoded.size(), CBB_data(aad.get()), CBB_len(aad.get())) || - !CBB_did_write(&payload_cbb, payload_len)) { + payload_len != payload_span.size()) { 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; } @@ -1045,7 +948,13 @@ int SSL_marshal_ech_config(uint8_t **out, size_t *out_len, uint8_t config_id, return 0; } - // See draft-ietf-tls-esni-10, section 4. + // The maximum name length is encoded in one byte. + if (max_name_len > 0xff) { + OPENSSL_PUT_ERROR(SSL, SSL_R_BAD_LENGTH); + return 0; + } + + // See draft-ietf-tls-esni-13, section 4. ScopedCBB cbb; CBB contents, child; uint8_t *public_key; @@ -1066,8 +975,8 @@ int SSL_marshal_ech_config(uint8_t **out, size_t *out_len, uint8_t config_id, !CBB_add_u16(&child, EVP_HPKE_AES_128_GCM) || !CBB_add_u16(&child, EVP_HPKE_HKDF_SHA256) || !CBB_add_u16(&child, EVP_HPKE_CHACHA20_POLY1305) || - !CBB_add_u16(&contents, max_name_len) || - !CBB_add_u16_length_prefixed(&contents, &child) || + !CBB_add_u8(&contents, max_name_len) || + !CBB_add_u8_length_prefixed(&contents, &child) || !CBB_add_bytes(&child, public_name_u8.data(), public_name_u8.size()) || // TODO(https://crbug.com/boringssl/275): Reserve some GREASE extensions // and include some. |