From 580fcaf18d41bf6c7513e90a2520d0fd8f0c244c Mon Sep 17 00:00:00 2001 From: Tobias Thierer Date: Thu, 12 Sep 2019 20:23:01 +0100 Subject: external/boringssl: Sync to e60b080dda138e1dd02d99fb34641ac22e46c85d. This includes the following changes: https://boringssl.googlesource.com/boringssl/+log/a8ffaf1bf2ec64cbbb17863ede06ba506b3db8b8..e60b080dda138e1dd02d99fb34641ac22e46c85d Bug: 137267623 Bug: 140918050 Test: atest CtsLibcoreTestCases CtsLibcoreOkHttpTestCases Change-Id: I00eeca876b9070a7163ec284433fc2ec5ea5ef01 --- src/CMakeLists.txt | 22 +- src/crypto/asn1/tasn_enc.c | 2 +- src/crypto/chacha/asm/chacha-armv4.pl | 6 +- src/crypto/fipsmodule/aes/asm/aes-armv4.pl | 6 +- src/crypto/fipsmodule/aes/asm/bsaes-armv7.pl | 6 +- src/crypto/fipsmodule/bn/asm/armv4-mont.pl | 6 +- src/crypto/fipsmodule/cipher/cipher.c | 5 +- src/crypto/fipsmodule/ec/ec_key.c | 4 +- src/crypto/fipsmodule/modes/asm/ghash-armv4.pl | 6 +- .../fipsmodule/modes/asm/ghash-neon-armv8.pl | 6 +- src/crypto/fipsmodule/self_check/self_check.c | 17 +- src/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl | 6 +- src/crypto/fipsmodule/sha/asm/sha256-armv4.pl | 6 +- src/crypto/fipsmodule/sha/asm/sha512-armv4.pl | 6 +- src/crypto/fipsmodule/sha/asm/sha512-armv8.pl | 3 +- src/crypto/internal.h | 9 + src/include/openssl/cipher.h | 4 +- src/include/openssl/ec_key.h | 4 +- src/include/openssl/ssl.h | 10 +- src/ssl/handshake_client.cc | 6 +- src/ssl/internal.h | 14 +- src/ssl/ssl_test.cc | 407 +++++++++++++-------- src/ssl/ssl_versions.cc | 44 +-- src/ssl/test/bssl_shim.cc | 15 +- src/ssl/test/handshake_util.cc | 12 +- src/ssl/test/handshaker.cc | 4 +- src/ssl/tls13_client.cc | 25 +- src/ssl/tls13_enc.cc | 42 ++- src/ssl/tls13_server.cc | 36 +- 29 files changed, 465 insertions(+), 274 deletions(-) (limited to 'src') diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c3992a93..d79d35fb 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -104,6 +104,10 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(CLANG 1) endif() +if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten") + set(EMSCRIPTEN 1) +endif() + if(CMAKE_COMPILER_IS_GNUCXX OR CLANG) # Note clang-cl is odd and sets both CLANG and MSVC. We base our configuration # primarily on our normal Clang one. @@ -117,7 +121,14 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CLANG) # honor it. Suppress it here to compensate. See https://crbug.com/772117. set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wno-deprecated-declarations") else() - set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wall -ggdb -fvisibility=hidden -fno-common") + if(EMSCRIPTEN) + # emscripten's emcc/clang does not accept the "-ggdb" flag. + set(C_CXX_FLAGS "${C_CXX_FLAGS} -g") + else() + set(C_CXX_FLAGS "${C_CXX_FLAGS} -ggdb") + endif() + + set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wall -fvisibility=hidden -fno-common") endif() if(CLANG) @@ -231,11 +242,10 @@ if(WIN32) add_definitions(-DNOMINMAX) # Allow use of fopen. add_definitions(-D_CRT_SECURE_NO_WARNINGS) - # VS 2017 and higher supports STL-only warning suppressions. Manually add to - # C++ only to work around a CMake quoting bug when using NASM with the Visual - # Studio generator. This will be fixed in CMake 3.13.0. See - # https://gitlab.kitware.com/cmake/cmake/merge_requests/2179 - add_compile_options($<$:-D_STL_EXTRA_DISABLED_WARNINGS=4774\ 4987>) + # VS 2017 and higher supports STL-only warning suppressions. + # A bug in CMake < 3.13.0 may cause the space in this value to + # cause issues when building with NASM. In that case, update CMake. + add_definitions("-D_STL_EXTRA_DISABLED_WARNINGS=4774 4987") endif() if((CMAKE_COMPILER_IS_GNUCXX AND CMAKE_C_COMPILER_VERSION VERSION_GREATER "4.7.99") OR diff --git a/src/crypto/asn1/tasn_enc.c b/src/crypto/asn1/tasn_enc.c index d89ec8a7..3722a519 100644 --- a/src/crypto/asn1/tasn_enc.c +++ b/src/crypto/asn1/tasn_enc.c @@ -192,7 +192,7 @@ int ASN1_item_ex_i2d(ASN1_VALUE **pval, unsigned char **out, /* Use indefinite length constructed if requested */ if (aclass & ASN1_TFLG_NDEF) ndef = 2; - /* fall through */ + OPENSSL_FALLTHROUGH; case ASN1_ITYPE_SEQUENCE: i = asn1_enc_restore(&seqcontlen, out, pval, it); diff --git a/src/crypto/chacha/asm/chacha-armv4.pl b/src/crypto/chacha/asm/chacha-armv4.pl index 06be3a66..ad26ed93 100755 --- a/src/crypto/chacha/asm/chacha-armv4.pl +++ b/src/crypto/chacha/asm/chacha-armv4.pl @@ -44,9 +44,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } sub AUTOLOAD() # thunk [simplified] x86-style perlasm diff --git a/src/crypto/fipsmodule/aes/asm/aes-armv4.pl b/src/crypto/fipsmodule/aes/asm/aes-armv4.pl index 9eebb224..fbb19950 100644 --- a/src/crypto/fipsmodule/aes/asm/aes-armv4.pl +++ b/src/crypto/fipsmodule/aes/asm/aes-armv4.pl @@ -49,9 +49,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $s0="r0"; diff --git a/src/crypto/fipsmodule/aes/asm/bsaes-armv7.pl b/src/crypto/fipsmodule/aes/asm/bsaes-armv7.pl index 932b3b68..34aecbec 100644 --- a/src/crypto/fipsmodule/aes/asm/bsaes-armv7.pl +++ b/src/crypto/fipsmodule/aes/asm/bsaes-armv7.pl @@ -60,9 +60,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } my ($inp,$out,$len,$key)=("r0","r1","r2","r3"); diff --git a/src/crypto/fipsmodule/bn/asm/armv4-mont.pl b/src/crypto/fipsmodule/bn/asm/armv4-mont.pl index f3aa4be5..a8e89f6b 100644 --- a/src/crypto/fipsmodule/bn/asm/armv4-mont.pl +++ b/src/crypto/fipsmodule/bn/asm/armv4-mont.pl @@ -64,9 +64,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $num="r0"; # starts as num argument, but holds &tp[num-1] diff --git a/src/crypto/fipsmodule/cipher/cipher.c b/src/crypto/fipsmodule/cipher/cipher.c index 39e038be..c50c6c5c 100644 --- a/src/crypto/fipsmodule/cipher/cipher.c +++ b/src/crypto/fipsmodule/cipher/cipher.c @@ -125,9 +125,10 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in) { return 1; } -void EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx) { +int EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx) { EVP_CIPHER_CTX_cleanup(ctx); EVP_CIPHER_CTX_init(ctx); + return 1; } int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, @@ -191,7 +192,7 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, case EVP_CIPH_CFB_MODE: ctx->num = 0; - // fall-through + OPENSSL_FALLTHROUGH; case EVP_CIPH_CBC_MODE: assert(EVP_CIPHER_CTX_iv_length(ctx) <= sizeof(ctx->iv)); diff --git a/src/crypto/fipsmodule/ec/ec_key.c b/src/crypto/fipsmodule/ec/ec_key.c index 3851c198..fcdc687b 100644 --- a/src/crypto/fipsmodule/ec/ec_key.c +++ b/src/crypto/fipsmodule/ec/ec_key.c @@ -369,8 +369,8 @@ int EC_KEY_check_fips(const EC_KEY *key) { return 1; } -int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key, BIGNUM *x, - BIGNUM *y) { +int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key, const BIGNUM *x, + const BIGNUM *y) { EC_POINT *point = NULL; int ok = 0; diff --git a/src/crypto/fipsmodule/modes/asm/ghash-armv4.pl b/src/crypto/fipsmodule/modes/asm/ghash-armv4.pl index 54c80f75..cdb6fb48 100644 --- a/src/crypto/fipsmodule/modes/asm/ghash-armv4.pl +++ b/src/crypto/fipsmodule/modes/asm/ghash-armv4.pl @@ -88,9 +88,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $Xi="r0"; # argument block diff --git a/src/crypto/fipsmodule/modes/asm/ghash-neon-armv8.pl b/src/crypto/fipsmodule/modes/asm/ghash-neon-armv8.pl index f57017d0..e72d0dce 100644 --- a/src/crypto/fipsmodule/modes/asm/ghash-neon-armv8.pl +++ b/src/crypto/fipsmodule/modes/asm/ghash-neon-armv8.pl @@ -62,9 +62,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } my ($Xi, $Htbl, $inp, $len) = map("x$_", (0..3)); # argument block diff --git a/src/crypto/fipsmodule/self_check/self_check.c b/src/crypto/fipsmodule/self_check/self_check.c index 3d47e69e..71d1c18d 100644 --- a/src/crypto/fipsmodule/self_check/self_check.c +++ b/src/crypto/fipsmodule/self_check/self_check.c @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -36,12 +37,18 @@ #if !defined(_MSC_VER) #if defined(BORINGSSL_FIPS) && defined(OPENSSL_ANDROID) -// FIPS builds on Android will attempt to write flag files to -// /dev/boringssl/selftest/ named after the module hash. If the flag file -// exists, it's assumed that self-tests have already passed and thus do not need -// to be repeated. +// FIPS builds on Android will test for flag files, named after the module hash, +// in /dev/boringssl/selftest/. If such a flag file exists, it's assumed that +// self-tests have already passed and thus do not need to be repeated. (The +// integrity tests always run, however.) +// +// If self-tests complete successfully and the environment variable named in +// |kFlagWriteEnableEnvVar| is present, then the flag file will be created. The +// flag file isn't written without the environment variable being set in order +// to avoid SELinux violations on Android. #define BORINGSSL_FIPS_SELF_TEST_FLAG_FILE static const char kFlagPrefix[] = "/dev/boringssl/selftest/"; +static const char kFlagWriteEnableEnvVar[] = "BORINGSSL_SELF_TEST_CREATE_FLAG"; #endif static void hexdump(const uint8_t *in, size_t len) { @@ -611,7 +618,7 @@ int BORINGSSL_self_test( #if defined(BORINGSSL_FIPS_SELF_TEST_FLAG_FILE) // Tests were successful. Write flag file if requested. - if (flag_path_valid) { + if (flag_path_valid && getenv(kFlagWriteEnableEnvVar) != NULL) { const int fd = open(flag_path, O_WRONLY | O_CREAT | O_TRUNC, 0644); if (fd >= 0) { close(fd); diff --git a/src/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl b/src/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl index ca825146..2cda3860 100644 --- a/src/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl +++ b/src/crypto/fipsmodule/sha/asm/sha1-armv4-large.pl @@ -85,9 +85,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $ctx="r0"; diff --git a/src/crypto/fipsmodule/sha/asm/sha256-armv4.pl b/src/crypto/fipsmodule/sha/asm/sha256-armv4.pl index 15d78de6..0c8adbb7 100644 --- a/src/crypto/fipsmodule/sha/asm/sha256-armv4.pl +++ b/src/crypto/fipsmodule/sha/asm/sha256-armv4.pl @@ -54,9 +54,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $ctx="r0"; $t0="r0"; diff --git a/src/crypto/fipsmodule/sha/asm/sha512-armv4.pl b/src/crypto/fipsmodule/sha/asm/sha512-armv4.pl index c8c715e5..d559e812 100644 --- a/src/crypto/fipsmodule/sha/asm/sha512-armv4.pl +++ b/src/crypto/fipsmodule/sha/asm/sha512-armv4.pl @@ -67,9 +67,11 @@ if ($flavour && $flavour ne "void") { ( $xlate="${dir}../../../perlasm/arm-xlate.pl" and -f $xlate) or die "can't locate arm-xlate.pl"; - open STDOUT,"| \"$^X\" $xlate $flavour $output"; + open OUT,"| \"$^X\" $xlate $flavour $output"; + *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } $ctx="r0"; # parameter block diff --git a/src/crypto/fipsmodule/sha/asm/sha512-armv8.pl b/src/crypto/fipsmodule/sha/asm/sha512-armv8.pl index 205b06dc..1afaf58e 100644 --- a/src/crypto/fipsmodule/sha/asm/sha512-armv8.pl +++ b/src/crypto/fipsmodule/sha/asm/sha512-armv8.pl @@ -50,7 +50,8 @@ if ($flavour && $flavour ne "void") { open OUT,"| \"$^X\" $xlate $flavour $output"; *STDOUT=*OUT; } else { - open STDOUT,">$output"; + open OUT,">$output"; + *STDOUT=*OUT; } if ($output =~ /512/) { diff --git a/src/crypto/internal.h b/src/crypto/internal.h index 76a317aa..4772ebea 100644 --- a/src/crypto/internal.h +++ b/src/crypto/internal.h @@ -187,6 +187,15 @@ typedef __uint128_t uint128_t; #define OPENSSL_FALLTHROUGH [[gnu::fallthrough]] #elif defined(__GNUC__) && __GNUC__ >= 7 // gcc 7 #define OPENSSL_FALLTHROUGH __attribute__ ((fallthrough)) +#elif defined(__clang__) +#if __has_attribute(fallthrough) && __clang_major__ >= 5 +// Clang 3.5, at least, complains about "error: declaration does not declare +// anything", possibily because we put a semicolon after this macro in +// practice. Thus limit it to >= Clang 5, which does work. +#define OPENSSL_FALLTHROUGH __attribute__ ((fallthrough)) +#else // clang versions that do not support fallthrough. +#define OPENSSL_FALLTHROUGH +#endif #else // C++11 on gcc 6, and all other cases #define OPENSSL_FALLTHROUGH #endif diff --git a/src/include/openssl/cipher.h b/src/include/openssl/cipher.h index ea7a940a..17b7b91c 100644 --- a/src/include/openssl/cipher.h +++ b/src/include/openssl/cipher.h @@ -136,8 +136,8 @@ OPENSSL_EXPORT int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in); // EVP_CIPHER_CTX_reset calls |EVP_CIPHER_CTX_cleanup| followed by -// |EVP_CIPHER_CTX_init|. -OPENSSL_EXPORT void EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx); +// |EVP_CIPHER_CTX_init| and returns one. +OPENSSL_EXPORT int EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx); // Cipher context configuration. diff --git a/src/include/openssl/ec_key.h b/src/include/openssl/ec_key.h index 3b1a5666..be0faaf8 100644 --- a/src/include/openssl/ec_key.h +++ b/src/include/openssl/ec_key.h @@ -174,8 +174,8 @@ OPENSSL_EXPORT int EC_KEY_check_fips(const EC_KEY *key); // EC_KEY_set_public_key_affine_coordinates sets the public key in |key| to // (|x|, |y|). It returns one on success and zero otherwise. OPENSSL_EXPORT int EC_KEY_set_public_key_affine_coordinates(EC_KEY *key, - BIGNUM *x, - BIGNUM *y); + const BIGNUM *x, + const BIGNUM *y); // EC_KEY_key2buf encodes the public key in |key| to an allocated octet string // and sets |*out_buf| to point to it. It returns the length of the encoded diff --git a/src/include/openssl/ssl.h b/src/include/openssl/ssl.h index 1ef9f84b..d3ca63c9 100644 --- a/src/include/openssl/ssl.h +++ b/src/include/openssl/ssl.h @@ -3140,6 +3140,13 @@ OPENSSL_EXPORT int SSL_delegated_credential_used(const SSL *ssl); // |SSL_process_quic_post_handshake| to process it. It is an error to call // |SSL_read| and |SSL_write| in QUIC. // +// 0-RTT behaves similarly to |TLS_method|'s usual behavior. |SSL_do_handshake| +// returns early as soon as the client (respectively, server) is allowed to send +// 0-RTT (respectively, half-RTT) data. The caller should then call +// |SSL_do_handshake| again to consume the remaining handshake messages and +// confirm the handshake. As a client, |SSL_ERROR_EARLY_DATA_REJECTED| and +// |SSL_reset_early_data_reject| behave as usual. +// // Note that secrets for an encryption level may be available to QUIC before the // level is active in TLS. Callers should use |SSL_quic_read_level| to determine // the active read level for |SSL_provide_quic_data|. |SSL_do_handshake| will @@ -3155,7 +3162,8 @@ OPENSSL_EXPORT int SSL_delegated_credential_used(const SSL *ssl); // |SSL_quic_max_handshake_flight_len| to get the maximum buffer length at each // encryption level. // -// Note: 0-RTT is not currently supported via this API. +// Note: 0-RTT support is incomplete and does not currently handle QUIC +// transport parameters and server SETTINGS frame. // ssl_encryption_level_t represents a specific QUIC encryption level used to // transmit handshake messages. diff --git a/src/ssl/handshake_client.cc b/src/ssl/handshake_client.cc index 4e8ca6ab..8be9f6bb 100644 --- a/src/ssl/handshake_client.cc +++ b/src/ssl/handshake_client.cc @@ -459,7 +459,11 @@ static enum ssl_hs_wait_t do_enter_early_data(SSL_HANDSHAKE *hs) { if (!tls13_init_early_key_schedule( hs, MakeConstSpan(ssl->session->master_key, ssl->session->master_key_length)) || - !tls13_derive_early_secrets(hs) || + !tls13_derive_early_secret(hs) || + !tls13_set_early_secret_for_quic(hs)) { + return ssl_hs_error; + } + if (ssl->quic_method == nullptr && !tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_seal, hs->early_traffic_secret())) { return ssl_hs_error; diff --git a/src/ssl/internal.h b/src/ssl/internal.h index f55e0470..ec3594c2 100644 --- a/src/ssl/internal.h +++ b/src/ssl/internal.h @@ -1265,9 +1265,17 @@ bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level, enum evp_aead_direction_t direction, Span traffic_secret); -// tls13_derive_early_secrets derives the early traffic secret. It returns true -// on success and false on error. -bool tls13_derive_early_secrets(SSL_HANDSHAKE *hs); +// tls13_derive_early_secret derives the early traffic secret. It returns true +// on success and false on error. Unlike with other traffic secrets, this +// function does not pass the keys to QUIC. Call +// |tls13_set_early_secret_for_quic| to do so. This is done to due to an +// ordering complication around resolving HelloRetryRequest on the server. +bool tls13_derive_early_secret(SSL_HANDSHAKE *hs); + +// tls13_set_early_secret_for_quic passes the early traffic secrets, as +// derived by |tls13_derive_early_secret|, to QUIC. It returns true on success +// and false on error. +bool tls13_set_early_secret_for_quic(SSL_HANDSHAKE *hs); // tls13_derive_handshake_secrets derives the handshake traffic secret. It // returns true on success and false on error. diff --git a/src/ssl/ssl_test.cc b/src/ssl/ssl_test.cc index 03760b6e..2df005a2 100644 --- a/src/ssl/ssl_test.cc +++ b/src/ssl/ssl_test.cc @@ -4698,7 +4698,9 @@ static_assert(ssl_encryption_application < kNumQUICLevels, class MockQUICTransport { public: - MockQUICTransport() { + enum class Role { kClient, kServer }; + + explicit MockQUICTransport(Role role) : role_(role) { // The caller is expected to configure initial secrets. levels_[ssl_encryption_initial].write_secret = {1}; levels_[ssl_encryption_initial].read_secret = {1}; @@ -4735,18 +4737,38 @@ class MockQUICTransport { return false; } - if (level != ssl_encryption_early_data && - (read_secret == nullptr || write_secret == nullptr)) { - ADD_FAILURE() << "key was unexpectedly null"; - return false; + bool expect_read_secret = true, expect_write_secret = true; + if (level == ssl_encryption_early_data) { + if (role_ == Role::kClient) { + expect_read_secret = false; + } else { + expect_write_secret = false; + } } - if (read_secret != nullptr) { + + if (expect_read_secret) { + if (read_secret == nullptr) { + ADD_FAILURE() << "read secret was unexpectedly null"; + return false; + } levels_[level].read_secret.assign(read_secret, read_secret + secret_len); + } else if (read_secret != nullptr) { + ADD_FAILURE() << "unexpected read secret"; + return false; } - if (write_secret != nullptr) { + + if (expect_write_secret) { + if (write_secret == nullptr) { + ADD_FAILURE() << "write secret was unexpectedly null"; + return false; + } levels_[level].write_secret.assign(write_secret, write_secret + secret_len); + } else if (write_secret != nullptr) { + ADD_FAILURE() << "unexpected write secret"; + return false; } + levels_[level].cipher = SSL_CIPHER_get_id(cipher); return true; } @@ -4783,20 +4805,22 @@ class MockQUICTransport { ssl_encryption_level_t level, size_t num = std::numeric_limits::max()) { if (levels_[level].read_secret.empty()) { - ADD_FAILURE() << "data read before keys configured"; + ADD_FAILURE() << "data read before keys configured in level " << level; return false; } // The peer may not have configured any keys yet. if (peer_->levels_[level].write_secret.empty()) { + out->clear(); return true; } // Check the peer computed the same key. if (peer_->levels_[level].write_secret != levels_[level].read_secret) { - ADD_FAILURE() << "peer write key does not match read key"; + ADD_FAILURE() << "peer write key does not match read key in level " + << level; return false; } if (peer_->levels_[level].cipher != levels_[level].cipher) { - ADD_FAILURE() << "peer cipher does not match"; + ADD_FAILURE() << "peer cipher does not match in level " << level; return false; } std::vector *peer_data = &peer_->levels_[level].write_data; @@ -4807,6 +4831,7 @@ class MockQUICTransport { } private: + Role role_; MockQUICTransport *peer_ = nullptr; bool has_alert_ = false; @@ -4824,21 +4849,24 @@ class MockQUICTransport { class MockQUICTransportPair { public: - MockQUICTransportPair() { - server_.set_peer(&client_); + MockQUICTransportPair() + : client_(MockQUICTransport::Role::kClient), + server_(MockQUICTransport::Role::kServer) { client_.set_peer(&server_); + server_.set_peer(&client_); } ~MockQUICTransportPair() { - server_.set_peer(nullptr); client_.set_peer(nullptr); + server_.set_peer(nullptr); } MockQUICTransport *client() { return &client_; } MockQUICTransport *server() { return &server_; } bool SecretsMatch(ssl_encryption_level_t level) const { - return client_.PeerSecretsMatch(level); + return client_.HasSecrets(level) && server_.HasSecrets(level) && + client_.PeerSecretsMatch(level); } private: @@ -4890,24 +4918,80 @@ class QUICMethodTest : public testing::Test { SSL_set_connect_state(client_.get()); SSL_set_accept_state(server_.get()); - ex_data_.Set(client_.get(), transport_.client()); - ex_data_.Set(server_.get(), transport_.server()); + transport_.reset(new MockQUICTransportPair); + ex_data_.Set(client_.get(), transport_->client()); + ex_data_.Set(server_.get(), transport_->server()); return true; } - bool CreateSecondClientAndServer() { - client_.reset(SSL_new(client_ctx_.get())); - server_.reset(SSL_new(server_ctx_.get())); - if (!client_ || !server_) { - return false; + // CompleteHandshakesForQUIC runs |SSL_do_handshake| on |client_| and + // |server_| until each completes once. It returns true on success and false + // on failure. + bool CompleteHandshakesForQUIC() { + bool client_done = false, server_done = false; + while (!client_done || !server_done) { + if (!client_done) { + if (!ProvideHandshakeData(client_.get())) { + ADD_FAILURE() << "ProvideHandshakeData(client_) failed"; + return false; + } + int client_ret = SSL_do_handshake(client_.get()); + if (client_ret == 1) { + client_done = true; + } else { + EXPECT_EQ(client_ret, -1); + EXPECT_EQ(SSL_get_error(client_.get(), client_ret), + SSL_ERROR_WANT_READ); + } + } + + if (!server_done) { + if (!ProvideHandshakeData(server_.get())) { + ADD_FAILURE() << "ProvideHandshakeData(server_) failed"; + return false; + } + int server_ret = SSL_do_handshake(server_.get()); + if (server_ret == 1) { + server_done = true; + } else { + EXPECT_EQ(server_ret, -1); + EXPECT_EQ(SSL_get_error(server_.get(), server_ret), + SSL_ERROR_WANT_READ); + } + } } + return true; + } - SSL_set_connect_state(client_.get()); - SSL_set_accept_state(server_.get()); + bssl::UniquePtr CreateClientSessionForQUIC() { + g_last_session = nullptr; + SSL_CTX_sess_set_new_cb(client_ctx_.get(), SaveLastSession); + if (!CreateClientAndServer() || + !CompleteHandshakesForQUIC()) { + return nullptr; + } - ex_data_.Set(client_.get(), second_transport_.client()); - ex_data_.Set(server_.get(), second_transport_.server()); - return true; + // The server sent NewSessionTicket messages in the handshake. + if (!ProvideHandshakeData(client_.get()) || + !SSL_process_quic_post_handshake(client_.get())) { + return nullptr; + } + + return std::move(g_last_session); + } + + void ExpectHandshakeSuccess() { + EXPECT_TRUE(transport_->SecretsMatch(ssl_encryption_application)); + EXPECT_EQ(ssl_encryption_application, SSL_quic_read_level(client_.get())); + EXPECT_EQ(ssl_encryption_application, SSL_quic_write_level(client_.get())); + EXPECT_EQ(ssl_encryption_application, SSL_quic_read_level(server_.get())); + EXPECT_EQ(ssl_encryption_application, SSL_quic_write_level(server_.get())); + EXPECT_FALSE(transport_->client()->has_alert()); + EXPECT_FALSE(transport_->server()->has_alert()); + + // SSL_do_handshake is now idempotent. + EXPECT_EQ(SSL_do_handshake(client_.get()), 1); + EXPECT_EQ(SSL_do_handshake(server_.get()), 1); } // The following functions may be configured on an |SSL_QUIC_METHOD| as @@ -4942,8 +5026,7 @@ class QUICMethodTest : public testing::Test { bssl::UniquePtr server_ctx_; static UnownedSSLExData ex_data_; - MockQUICTransportPair transport_; - MockQUICTransportPair second_transport_; + std::unique_ptr transport_; bssl::UniquePtr client_; bssl::UniquePtr server_; @@ -4966,33 +5049,13 @@ TEST_F(QUICMethodTest, Basic) { SSL_CTX_sess_set_new_cb(client_ctx_.get(), SaveLastSession); ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); - ASSERT_TRUE(CreateClientAndServer()); - for (;;) { - ASSERT_TRUE(ProvideHandshakeData(client_.get())); - int client_ret = SSL_do_handshake(client_.get()); - if (client_ret != 1) { - ASSERT_EQ(client_ret, -1); - ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); - } - - ASSERT_TRUE(ProvideHandshakeData(server_.get())); - int server_ret = SSL_do_handshake(server_.get()); - if (server_ret != 1) { - ASSERT_EQ(server_ret, -1); - ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); - } - - if (client_ret == 1 && server_ret == 1) { - break; - } - } + ASSERT_TRUE(CreateClientAndServer()); + ASSERT_TRUE(CompleteHandshakesForQUIC()); - EXPECT_EQ(SSL_do_handshake(client_.get()), 1); - EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + ExpectHandshakeSuccess(); + EXPECT_FALSE(SSL_session_reused(client_.get())); + EXPECT_FALSE(SSL_session_reused(server_.get())); // The server sent NewSessionTicket messages in the handshake. EXPECT_FALSE(g_last_session); @@ -5001,35 +5064,13 @@ TEST_F(QUICMethodTest, Basic) { EXPECT_TRUE(g_last_session); // Create a second connection to verify resumption works. - ASSERT_TRUE(CreateSecondClientAndServer()); + ASSERT_TRUE(CreateClientAndServer()); bssl::UniquePtr session = std::move(g_last_session); SSL_set_session(client_.get(), session.get()); - for (;;) { - ASSERT_TRUE(ProvideHandshakeData(client_.get())); - int client_ret = SSL_do_handshake(client_.get()); - if (client_ret != 1) { - ASSERT_EQ(client_ret, -1); - ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); - } - - ASSERT_TRUE(ProvideHandshakeData(server_.get())); - int server_ret = SSL_do_handshake(server_.get()); - if (server_ret != 1) { - ASSERT_EQ(server_ret, -1); - ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); - } - - if (client_ret == 1 && server_ret == 1) { - break; - } - } + ASSERT_TRUE(CompleteHandshakesForQUIC()); - EXPECT_EQ(SSL_do_handshake(client_.get()), 1); - EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + ExpectHandshakeSuccess(); EXPECT_TRUE(SSL_session_reused(client_.get())); EXPECT_TRUE(SSL_session_reused(server_.get())); } @@ -5056,32 +5097,133 @@ TEST_F(QUICMethodTest, HelloRetryRequest) { OPENSSL_ARRAY_SIZE(kServerPrefs))); ASSERT_TRUE(CreateClientAndServer()); + ASSERT_TRUE(CompleteHandshakesForQUIC()); + ExpectHandshakeSuccess(); +} - for (;;) { - ASSERT_TRUE(ProvideHandshakeData(client_.get())); - int client_ret = SSL_do_handshake(client_.get()); - if (client_ret != 1) { - ASSERT_EQ(client_ret, -1); - ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); +TEST_F(QUICMethodTest, ZeroRTTAccept) { + const SSL_QUIC_METHOD quic_method = { + SetEncryptionSecretsCallback, + AddHandshakeDataCallback, + FlushFlightCallback, + SendAlertCallback, + }; + + SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); + SSL_CTX_set_early_data_enabled(client_ctx_.get(), 1); + SSL_CTX_set_early_data_enabled(server_ctx_.get(), 1); + ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); + ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); + + bssl::UniquePtr session = CreateClientSessionForQUIC(); + ASSERT_TRUE(session); + + ASSERT_TRUE(CreateClientAndServer()); + SSL_set_session(client_.get(), session.get()); + + // The client handshake should return immediately into the early data state. + ASSERT_EQ(SSL_do_handshake(client_.get()), 1); + EXPECT_TRUE(SSL_in_early_data(client_.get())); + // The transport should have keys for sending 0-RTT data. + EXPECT_TRUE( + transport_->client()->HasSecrets(ssl_encryption_early_data)); + + // The server will consume the ClientHello and also enter the early data + // state. + ASSERT_TRUE(ProvideHandshakeData(server_.get())); + ASSERT_EQ(SSL_do_handshake(server_.get()), 1); + EXPECT_TRUE(SSL_in_early_data(server_.get())); + EXPECT_TRUE(transport_->SecretsMatch(ssl_encryption_early_data)); + // The transport should have keys for sending half-RTT data. + EXPECT_TRUE( + transport_->server()->HasSecrets(ssl_encryption_application)); + + // Finish up the client and server handshakes. + ASSERT_TRUE(CompleteHandshakesForQUIC()); + + // Both sides can now exchange 1-RTT data. + ExpectHandshakeSuccess(); + EXPECT_TRUE(SSL_session_reused(client_.get())); + EXPECT_TRUE(SSL_session_reused(server_.get())); + EXPECT_FALSE(SSL_in_early_data(client_.get())); + EXPECT_FALSE(SSL_in_early_data(server_.get())); + EXPECT_TRUE(SSL_early_data_accepted(client_.get())); + EXPECT_TRUE(SSL_early_data_accepted(server_.get())); +} + +TEST_F(QUICMethodTest, ZeroRTTReject) { + const SSL_QUIC_METHOD quic_method = { + SetEncryptionSecretsCallback, + AddHandshakeDataCallback, + FlushFlightCallback, + SendAlertCallback, + }; + + SSL_CTX_set_session_cache_mode(client_ctx_.get(), SSL_SESS_CACHE_BOTH); + SSL_CTX_set_early_data_enabled(client_ctx_.get(), 1); + SSL_CTX_set_early_data_enabled(server_ctx_.get(), 1); + ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); + ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); + + bssl::UniquePtr session = CreateClientSessionForQUIC(); + ASSERT_TRUE(session); + + for (bool reject_hrr : {false, true}) { + SCOPED_TRACE(reject_hrr); + + ASSERT_TRUE(CreateClientAndServer()); + if (reject_hrr) { + // Configure the server to prefer P-256, which will reject 0-RTT via + // HelloRetryRequest. + int p256 = NID_X9_62_prime256v1; + ASSERT_TRUE(SSL_set1_curves(server_.get(), &p256, 1)); + } else { + // Disable 0-RTT on the server, so it will reject it. + SSL_set_early_data_enabled(server_.get(), 0); } + SSL_set_session(client_.get(), session.get()); + + // The client handshake should return immediately into the early data state. + ASSERT_EQ(SSL_do_handshake(client_.get()), 1); + EXPECT_TRUE(SSL_in_early_data(client_.get())); + // The transport should have keys for sending 0-RTT data. + EXPECT_TRUE(transport_->client()->HasSecrets(ssl_encryption_early_data)); + // The server will consume the ClientHello, but it will not accept 0-RTT. ASSERT_TRUE(ProvideHandshakeData(server_.get())); - int server_ret = SSL_do_handshake(server_.get()); - if (server_ret != 1) { - ASSERT_EQ(server_ret, -1); - ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); - } + ASSERT_EQ(SSL_do_handshake(server_.get()), -1); + EXPECT_EQ(SSL_ERROR_WANT_READ, SSL_get_error(server_.get(), -1)); + EXPECT_FALSE(SSL_in_early_data(server_.get())); + EXPECT_FALSE(transport_->server()->HasSecrets(ssl_encryption_early_data)); - if (client_ret == 1 && server_ret == 1) { - break; + // The client consumes the server response and signals 0-RTT rejection. + for (;;) { + ASSERT_TRUE(ProvideHandshakeData(client_.get())); + ASSERT_EQ(-1, SSL_do_handshake(client_.get())); + int err = SSL_get_error(client_.get(), -1); + if (err == SSL_ERROR_EARLY_DATA_REJECTED) { + break; + } + ASSERT_EQ(SSL_ERROR_WANT_READ, err); } - } - EXPECT_EQ(SSL_do_handshake(client_.get()), 1); - EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + // As in TLS over TCP, 0-RTT rejection is sticky. + ASSERT_EQ(-1, SSL_do_handshake(client_.get())); + ASSERT_EQ(SSL_ERROR_EARLY_DATA_REJECTED, SSL_get_error(client_.get(), -1)); + + // Finish up the client and server handshakes. + SSL_reset_early_data_reject(client_.get()); + ASSERT_TRUE(CompleteHandshakesForQUIC()); + + // Both sides can now exchange 1-RTT data. + ExpectHandshakeSuccess(); + EXPECT_TRUE(SSL_session_reused(client_.get())); + EXPECT_TRUE(SSL_session_reused(server_.get())); + EXPECT_FALSE(SSL_in_early_data(client_.get())); + EXPECT_FALSE(SSL_in_early_data(server_.get())); + EXPECT_FALSE(SSL_early_data_accepted(client_.get())); + EXPECT_FALSE(SSL_early_data_accepted(server_.get())); + } } // Test only releasing data to QUIC one byte at a time on request, to maximize @@ -5137,11 +5279,7 @@ TEST_F(QUICMethodTest, Async) { } } - EXPECT_EQ(SSL_do_handshake(client_.get()), 1); - EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + ExpectHandshakeSuccess(); } // Test buffering write data until explicit flushes. @@ -5188,31 +5326,9 @@ TEST_F(QUICMethodTest, Buffered) { buffered_flights.Set(client_.get(), &client_flight); buffered_flights.Set(server_.get(), &server_flight); - for (;;) { - ASSERT_TRUE(ProvideHandshakeData(client_.get())); - int client_ret = SSL_do_handshake(client_.get()); - if (client_ret != 1) { - ASSERT_EQ(client_ret, -1); - ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); - } - - ASSERT_TRUE(ProvideHandshakeData(server_.get())); - int server_ret = SSL_do_handshake(server_.get()); - if (server_ret != 1) { - ASSERT_EQ(server_ret, -1); - ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); - } - - if (client_ret == 1 && server_ret == 1) { - break; - } - } + ASSERT_TRUE(CompleteHandshakesForQUIC()); - EXPECT_EQ(SSL_do_handshake(client_.get()), 1); - EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + ExpectHandshakeSuccess(); } // Test that excess data at one level is rejected. That is, if a single @@ -5262,13 +5378,13 @@ TEST_F(QUICMethodTest, ExcessProvidedData) { EXPECT_EQ(ERR_GET_REASON(err), SSL_R_BUFFERED_MESSAGES_ON_CIPHER_CHANGE); // The client sends an alert in response to this. - ASSERT_TRUE(transport_.client()->has_alert()); - EXPECT_EQ(transport_.client()->alert_level(), ssl_encryption_initial); - EXPECT_EQ(transport_.client()->alert(), SSL_AD_UNEXPECTED_MESSAGE); + ASSERT_TRUE(transport_->client()->has_alert()); + EXPECT_EQ(transport_->client()->alert_level(), ssl_encryption_initial); + EXPECT_EQ(transport_->client()->alert(), SSL_AD_UNEXPECTED_MESSAGE); // Sanity-check client did get far enough to process the ServerHello and // install keys. - EXPECT_TRUE(transport_.client()->HasSecrets(ssl_encryption_handshake)); + EXPECT_TRUE(transport_->client()->HasSecrets(ssl_encryption_handshake)); } // Test that |SSL_provide_quic_data| will reject data at the wrong level. @@ -5298,7 +5414,7 @@ TEST_F(QUICMethodTest, ProvideWrongLevel) { // Data cannot be provided at the next level. std::vector data; ASSERT_TRUE( - transport_.client()->ReadHandshakeData(&data, ssl_encryption_initial)); + transport_->client()->ReadHandshakeData(&data, ssl_encryption_initial)); ASSERT_FALSE(SSL_provide_quic_data(client_.get(), ssl_encryption_handshake, data.data(), data.size())); ERR_clear_error(); @@ -5312,7 +5428,7 @@ TEST_F(QUICMethodTest, ProvideWrongLevel) { // Data cannot be provided at the previous level. ASSERT_TRUE( - transport_.client()->ReadHandshakeData(&data, ssl_encryption_handshake)); + transport_->client()->ReadHandshakeData(&data, ssl_encryption_handshake)); ASSERT_FALSE(SSL_provide_quic_data(client_.get(), ssl_encryption_initial, data.data(), data.size())); } @@ -5357,32 +5473,13 @@ TEST_F(QUICMethodTest, BadPostHandshake) { ASSERT_TRUE(SSL_CTX_set_quic_method(client_ctx_.get(), &quic_method)); ASSERT_TRUE(SSL_CTX_set_quic_method(server_ctx_.get(), &quic_method)); ASSERT_TRUE(CreateClientAndServer()); - - for (;;) { - ASSERT_TRUE(ProvideHandshakeData(client_.get())); - int client_ret = SSL_do_handshake(client_.get()); - if (client_ret != 1) { - ASSERT_EQ(client_ret, -1); - ASSERT_EQ(SSL_get_error(client_.get(), client_ret), SSL_ERROR_WANT_READ); - } - - ASSERT_TRUE(ProvideHandshakeData(server_.get())); - int server_ret = SSL_do_handshake(server_.get()); - if (server_ret != 1) { - ASSERT_EQ(server_ret, -1); - ASSERT_EQ(SSL_get_error(server_.get(), server_ret), SSL_ERROR_WANT_READ); - } - - if (client_ret == 1 && server_ret == 1) { - break; - } - } + ASSERT_TRUE(CompleteHandshakesForQUIC()); EXPECT_EQ(SSL_do_handshake(client_.get()), 1); EXPECT_EQ(SSL_do_handshake(server_.get()), 1); - EXPECT_TRUE(transport_.SecretsMatch(ssl_encryption_application)); - EXPECT_FALSE(transport_.client()->has_alert()); - EXPECT_FALSE(transport_.server()->has_alert()); + EXPECT_TRUE(transport_->SecretsMatch(ssl_encryption_application)); + EXPECT_FALSE(transport_->client()->has_alert()); + EXPECT_FALSE(transport_->server()->has_alert()); // Junk sent as part of post-handshake data should cause an error. uint8_t kJunk[] = {0x17, 0x0, 0x0, 0x4, 0xB, 0xE, 0xE, 0xF}; diff --git a/src/ssl/ssl_versions.cc b/src/ssl/ssl_versions.cc index e25fce6d..df5ffd26 100644 --- a/src/ssl/ssl_versions.cc +++ b/src/ssl/ssl_versions.cc @@ -63,24 +63,16 @@ static const uint16_t kDTLSVersions[] = { DTLS1_VERSION, }; -static void get_method_versions(const SSL_PROTOCOL_METHOD *method, - const uint16_t **out, size_t *out_num) { - if (method->is_dtls) { - *out = kDTLSVersions; - *out_num = OPENSSL_ARRAY_SIZE(kDTLSVersions); - } else { - *out = kTLSVersions; - *out_num = OPENSSL_ARRAY_SIZE(kTLSVersions); - } +static Span get_method_versions( + const SSL_PROTOCOL_METHOD *method) { + return method->is_dtls ? Span(kDTLSVersions) + : Span(kTLSVersions); } bool ssl_method_supports_version(const SSL_PROTOCOL_METHOD *method, uint16_t version) { - const uint16_t *versions; - size_t num_versions; - get_method_versions(method, &versions, &num_versions); - for (size_t i = 0; i < num_versions; i++) { - if (versions[i] == version) { + for (uint16_t supported : get_method_versions(method)) { + if (supported == version) { return true; } } @@ -282,12 +274,9 @@ bool ssl_supports_version(SSL_HANDSHAKE *hs, uint16_t version) { } bool ssl_add_supported_versions(SSL_HANDSHAKE *hs, CBB *cbb) { - const uint16_t *versions; - size_t num_versions; - get_method_versions(hs->ssl->method, &versions, &num_versions); - for (size_t i = 0; i < num_versions; i++) { - if (ssl_supports_version(hs, versions[i]) && - !CBB_add_u16(cbb, versions[i])) { + for (uint16_t version : get_method_versions(hs->ssl->method)) { + if (ssl_supports_version(hs, version) && + !CBB_add_u16(cbb, version)) { return false; } } @@ -296,11 +285,8 @@ bool ssl_add_supported_versions(SSL_HANDSHAKE *hs, CBB *cbb) { bool ssl_negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert, uint16_t *out_version, const CBS *peer_versions) { - const uint16_t *versions; - size_t num_versions; - get_method_versions(hs->ssl->method, &versions, &num_versions); - for (size_t i = 0; i < num_versions; i++) { - if (!ssl_supports_version(hs, versions[i])) { + for (uint16_t version : get_method_versions(hs->ssl->method)) { + if (!ssl_supports_version(hs, version)) { continue; } @@ -312,20 +298,20 @@ bool ssl_negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert, // own.) // // See https://bugs.openjdk.java.net/browse/JDK-8211806. - if (versions[i] == TLS1_3_VERSION && hs->apply_jdk11_workaround) { + if (version == TLS1_3_VERSION && hs->apply_jdk11_workaround) { continue; } CBS copy = *peer_versions; while (CBS_len(©) != 0) { - uint16_t version; - if (!CBS_get_u16(©, &version)) { + uint16_t peer_version; + if (!CBS_get_u16(©, &peer_version)) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); *out_alert = SSL_AD_DECODE_ERROR; return false; } - if (version == versions[i]) { + if (peer_version == version) { *out_version = version; return true; } diff --git a/src/ssl/test/bssl_shim.cc b/src/ssl/test/bssl_shim.cc index f58c1510..261f6c60 100644 --- a/src/ssl/test/bssl_shim.cc +++ b/src/ssl/test/bssl_shim.cc @@ -204,7 +204,7 @@ static int DoRead(SSL *ssl, uint8_t *out, size_t max_out) { return -1; } } - } while (config->async && RetryAsync(ssl, ret)); + } while (RetryAsync(ssl, ret)); if (config->peek_then_read && ret > 0) { std::unique_ptr buf(new uint8_t[static_cast(ret)]); @@ -233,7 +233,6 @@ static int DoRead(SSL *ssl, uint8_t *out, size_t max_out) { // operations. It returns the result of the final |SSL_write| call. static int WriteAll(SSL *ssl, const void *in_, size_t in_len) { const uint8_t *in = reinterpret_cast(in_); - const TestConfig *config = GetTestConfig(ssl); int ret; do { ret = SSL_write(ssl, in, in_len); @@ -241,29 +240,27 @@ static int WriteAll(SSL *ssl, const void *in_, size_t in_len) { in += ret; in_len -= ret; } - } while ((config->async && RetryAsync(ssl, ret)) || (ret > 0 && in_len > 0)); + } while (RetryAsync(ssl, ret) || (ret > 0 && in_len > 0)); return ret; } // DoShutdown calls |SSL_shutdown|, resolving any asynchronous operations. It // returns the result of the final |SSL_shutdown| call. static int DoShutdown(SSL *ssl) { - const TestConfig *config = GetTestConfig(ssl); int ret; do { ret = SSL_shutdown(ssl); - } while (config->async && RetryAsync(ssl, ret)); + } while (RetryAsync(ssl, ret)); return ret; } // DoSendFatalAlert calls |SSL_send_fatal_alert|, resolving any asynchronous // operations. It returns the result of the final |SSL_send_fatal_alert| call. static int DoSendFatalAlert(SSL *ssl, uint8_t alert) { - const TestConfig *config = GetTestConfig(ssl); int ret; do { ret = SSL_send_fatal_alert(ssl, alert); - } while (config->async && RetryAsync(ssl, ret)); + } while (RetryAsync(ssl, ret)); return ret; } @@ -833,7 +830,7 @@ static bool DoExchange(bssl::UniquePtr *out_session, ret = CheckIdempotentError("SSL_do_handshake", ssl, [&]() -> int { return SSL_do_handshake(ssl); }); - } while (config->async && RetryAsync(ssl, ret)); + } while (RetryAsync(ssl, ret)); if (config->forbid_renegotiation_after_handshake) { SSL_set_renegotiate_mode(ssl, ssl_renegotiate_never); @@ -854,7 +851,7 @@ static bool DoExchange(bssl::UniquePtr *out_session, if (config->handshake_twice) { do { ret = SSL_do_handshake(ssl); - } while (config->async && RetryAsync(ssl, ret)); + } while (RetryAsync(ssl, ret)); if (ret != 1) { return false; } diff --git a/src/ssl/test/handshake_util.cc b/src/ssl/test/handshake_util.cc index afead7f8..4b1dcc84 100644 --- a/src/ssl/test/handshake_util.cc +++ b/src/ssl/test/handshake_util.cc @@ -38,14 +38,13 @@ using namespace bssl; bool RetryAsync(SSL *ssl, int ret) { - // No error; don't retry. - if (ret >= 0) { + const TestConfig *config = GetTestConfig(ssl); + TestState *test_state = GetTestState(ssl); + // No error or not async; don't retry. + if (ret >= 0 || !config->async) { return false; } - TestState *test_state = GetTestState(ssl); - assert(GetTestConfig(ssl)->async); - if (test_state->packeted_bio != nullptr && PacketedBioAdvanceClock(test_state->packeted_bio)) { // The DTLS retransmit logic silently ignores write failures. So the test @@ -71,8 +70,7 @@ bool RetryAsync(SSL *ssl, int ret) { AsyncBioAllowWrite(test_state->async_bio, 1); return true; case SSL_ERROR_WANT_CHANNEL_ID_LOOKUP: { - UniquePtr pkey = - LoadPrivateKey(GetTestConfig(ssl)->send_channel_id); + UniquePtr pkey = LoadPrivateKey(config->send_channel_id); if (!pkey) { return false; } diff --git a/src/ssl/test/handshaker.cc b/src/ssl/test/handshaker.cc index 9888876c..a6bf6432 100644 --- a/src/ssl/test/handshaker.cc +++ b/src/ssl/test/handshaker.cc @@ -35,7 +35,7 @@ bool HandbackReady(SSL *ssl, int ret) { } bool Handshaker(const TestConfig *config, int rfd, int wfd, - Span input, int control) { + Span input, int control) { UniquePtr ctx = config->SetupCtx(/*old_ctx=*/nullptr); if (!ctx) { return false; @@ -80,7 +80,7 @@ bool Handshaker(const TestConfig *config, int rfd, int wfd, } continue; } - if (!config->async || !RetryAsync(ssl.get(), ret)) { + if (!RetryAsync(ssl.get(), ret)) { break; } } diff --git a/src/ssl/tls13_client.cc b/src/ssl/tls13_client.cc index a7d0d892..12f4738b 100644 --- a/src/ssl/tls13_client.cc +++ b/src/ssl/tls13_client.cc @@ -633,12 +633,16 @@ static enum ssl_hs_wait_t do_send_end_of_early_data(SSL_HANDSHAKE *hs) { if (ssl->s3->early_data_accepted) { hs->can_early_write = false; - ScopedCBB cbb; - CBB body; - if (!ssl->method->init_message(ssl, cbb.get(), &body, - SSL3_MT_END_OF_EARLY_DATA) || - !ssl_add_message_cbb(ssl, cbb.get())) { - return ssl_hs_error; + // QUIC omits the EndOfEarlyData message. See draft-ietf-quic-tls-22, + // section 8.3. + if (ssl->quic_method == nullptr) { + ScopedCBB cbb; + CBB body; + if (!ssl->method->init_message(ssl, cbb.get(), &body, + SSL3_MT_END_OF_EARLY_DATA) || + !ssl_add_message_cbb(ssl, cbb.get())) { + return ssl_hs_error; + } } } @@ -911,6 +915,15 @@ bool tls13_process_new_session_ticket(SSL *ssl, const SSLMessage &msg) { OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); return false; } + + // QUIC does not use the max_early_data_size parameter and always sets it to + // a fixed value. See draft-ietf-quic-tls-22, section 4.5. + if (ssl->quic_method != nullptr && + session->ticket_max_early_data != 0xffffffff) { + ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); + OPENSSL_PUT_ERROR(SSL, SSL_R_DECODE_ERROR); + return false; + } } // Generate a session ID for this session. Some callers expect all sessions to diff --git a/src/ssl/tls13_enc.cc b/src/ssl/tls13_enc.cc index b8bebe1b..83b3d624 100644 --- a/src/ssl/tls13_enc.cc +++ b/src/ssl/tls13_enc.cc @@ -178,6 +178,8 @@ bool tls13_set_traffic_key(SSL *ssl, enum ssl_encryption_level_t level, // encryption itself will be handled by the SSL_QUIC_METHOD. traffic_aead = SSLAEADContext::CreatePlaceholderForQUIC(version, session->cipher); + // QUIC never installs early data keys at the TLS layer. + assert(level != ssl_encryption_early_data); } if (!traffic_aead) { @@ -226,7 +228,7 @@ static const char kTLS13LabelServerHandshakeTraffic[] = "s hs traffic"; static const char kTLS13LabelClientApplicationTraffic[] = "c ap traffic"; static const char kTLS13LabelServerApplicationTraffic[] = "s ap traffic"; -bool tls13_derive_early_secrets(SSL_HANDSHAKE *hs) { +bool tls13_derive_early_secret(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; if (!derive_secret(hs, hs->early_traffic_secret(), label_to_span(kTLS13LabelClientEarlyTraffic)) || @@ -234,26 +236,30 @@ bool tls13_derive_early_secrets(SSL_HANDSHAKE *hs) { hs->early_traffic_secret())) { return false; } + return true; +} - if (ssl->quic_method != nullptr) { - if (ssl->server) { - if (!ssl->quic_method->set_encryption_secrets( - ssl, ssl_encryption_early_data, nullptr, - hs->early_traffic_secret().data(), - hs->early_traffic_secret().size())) { - OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_INTERNAL_ERROR); - return false; - } - } else { - if (!ssl->quic_method->set_encryption_secrets( - ssl, ssl_encryption_early_data, hs->early_traffic_secret().data(), - nullptr, hs->early_traffic_secret().size())) { - OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_INTERNAL_ERROR); - return false; - } +bool tls13_set_early_secret_for_quic(SSL_HANDSHAKE *hs) { + SSL *const ssl = hs->ssl; + if (ssl->quic_method == nullptr) { + return true; + } + if (ssl->server) { + if (!ssl->quic_method->set_encryption_secrets( + ssl, ssl_encryption_early_data, hs->early_traffic_secret().data(), + /*write_secret=*/nullptr, hs->early_traffic_secret().size())) { + OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_INTERNAL_ERROR); + return false; + } + } else { + if (!ssl->quic_method->set_encryption_secrets( + ssl, ssl_encryption_early_data, /*read_secret=*/nullptr, + hs->early_traffic_secret().data(), + hs->early_traffic_secret().size())) { + OPENSSL_PUT_ERROR(SSL, SSL_R_QUIC_INTERNAL_ERROR); + return false; } } - return true; } diff --git a/src/ssl/tls13_server.cc b/src/ssl/tls13_server.cc index f1891cf9..a52a49c5 100644 --- a/src/ssl/tls13_server.cc +++ b/src/ssl/tls13_server.cc @@ -146,7 +146,10 @@ static bool add_new_session_tickets(SSL_HANDSHAKE *hs, bool *out_sent_tickets) { } session->ticket_age_add_valid = true; if (ssl->enable_early_data) { - session->ticket_max_early_data = kMaxEarlyDataAccepted; + // QUIC does not use the max_early_data_size parameter and always sets it + // to a fixed value. See draft-ietf-quic-tls-22, section 4.5. + session->ticket_max_early_data = + ssl->quic_method != nullptr ? 0xffffffff : kMaxEarlyDataAccepted; } static_assert(kNumTickets < 256, "Too many tickets"); @@ -442,7 +445,7 @@ static enum ssl_hs_wait_t do_select_session(SSL_HANDSHAKE *hs) { } if (ssl->s3->early_data_accepted) { - if (!tls13_derive_early_secrets(hs)) { + if (!tls13_derive_early_secret(hs)) { return ssl_hs_error; } } else if (hs->early_data_offered) { @@ -468,6 +471,15 @@ static enum ssl_hs_wait_t do_select_session(SSL_HANDSHAKE *hs) { return ssl_hs_error; } + // Note we defer releasing the early traffic secret to QUIC until after ECDHE + // is resolved. The early traffic secret should be derived before the key + // schedule incorporates ECDHE, but doing so may reject 0-RTT. To avoid + // confusing the caller, we split derivation and releasing the secret to QUIC. + if (ssl->s3->early_data_accepted && + !tls13_set_early_secret_for_quic(hs)) { + return ssl_hs_error; + } + ssl->method->next_message(ssl); hs->tls13_state = state_send_server_hello; return ssl_hs_ok; @@ -731,7 +743,8 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) { // Finished early. See RFC 8446, section 4.6.1. static const uint8_t kEndOfEarlyData[4] = {SSL3_MT_END_OF_EARLY_DATA, 0, 0, 0}; - if (!hs->transcript.Update(kEndOfEarlyData)) { + if (ssl->quic_method == nullptr && + !hs->transcript.Update(kEndOfEarlyData)) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); return ssl_hs_error; } @@ -772,7 +785,9 @@ static enum ssl_hs_wait_t do_send_server_finished(SSL_HANDSHAKE *hs) { static enum ssl_hs_wait_t do_read_second_client_flight(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; if (ssl->s3->early_data_accepted) { - if (!tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_open, + // QUIC never receives handshake messages under 0-RTT keys. + if (ssl->quic_method == nullptr && + !tls13_set_traffic_key(ssl, ssl_encryption_early_data, evp_aead_open, hs->early_traffic_secret())) { return ssl_hs_error; } @@ -780,6 +795,19 @@ static enum ssl_hs_wait_t do_read_second_client_flight(SSL_HANDSHAKE *hs) { hs->can_early_read = true; hs->in_early_data = true; } + + // QUIC doesn't use an EndOfEarlyData message (draft-ietf-quic-tls-22, + // section 8.3), so we switch to client_handshake_secret before the early + // return. + if (ssl->quic_method != nullptr) { + if (!tls13_set_traffic_key(ssl, ssl_encryption_handshake, evp_aead_open, + hs->client_handshake_secret())) { + return ssl_hs_error; + } + hs->tls13_state = state_read_client_certificate; + return ssl->s3->early_data_accepted ? ssl_hs_early_return : ssl_hs_ok; + } + hs->tls13_state = state_process_end_of_early_data; return ssl->s3->early_data_accepted ? ssl_hs_read_end_of_early_data : ssl_hs_ok; -- cgit v1.2.3