This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch 10.0.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit d9bd3cca8fe920764be25c61110c0cf4ed915f6c Author: Masakazu Kitajo <[email protected]> AuthorDate: Mon Jul 22 18:41:46 2024 -0600 Reduce the number of checks for BoringSSL (#11564) * Reduce the number of checks for BoringSSL * Remove a redundant condition check * Fix the way of setting TS_USE_HELLO_CB * Fix syntax errors * Remove a redundant TS_USE_HELLO_CB check (cherry picked from commit 71f7129334a203695821331560483a9f8deb0bb0) --- CMakeLists.txt | 12 +++++++++++- include/iocore/net/TLSSNISupport.h | 8 +++----- include/tscore/HKDF.h | 8 +++++--- include/tscore/ink_config.h.cmake.in | 5 +++++ src/iocore/net/P_SSLClientUtils.h | 2 +- src/iocore/net/P_SSLUtils.h | 3 +-- src/iocore/net/P_TLSKeyLogger.h | 2 +- src/iocore/net/SSLDiags.cc | 2 +- src/iocore/net/SSLUtils.cc | 14 +++++++------- src/iocore/net/TLSBasicSupport.cc | 2 +- src/iocore/net/TLSSNISupport.cc | 14 +++++++------- src/iocore/net/YamlSNIConfig.cc | 2 +- src/iocore/net/quic/QUICConfig.cc | 6 +----- 13 files changed, 45 insertions(+), 35 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e59411340b..34db385ce6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -460,17 +460,21 @@ check_symbol_exists(BIO_meth_get_destroy "openssl/bio.h" HAVE_BIO_METH_GET_DESTR check_symbol_exists(HMAC_CTX_new "openssl/hmac.h" HAVE_HMAC_CTX_NEW) check_symbol_exists(DH_get_2048_256 "openssl/dh.h" TS_USE_GET_DH_2048_256) check_symbol_exists(OPENSSL_NO_TLS_3 "openssl/ssl.h" TS_NO_USE_TLS12) -check_symbol_exists(SSL_CTX_set_client_hello_cb "openssl/ssl.h" TS_USE_HELLO_CB) +check_symbol_exists(SSL_CTX_set_client_hello_cb "openssl/ssl.h" HAVE_SSL_CTX_SET_CLIENT_HELLO_CB) +check_symbol_exists(SSL_CTX_set_select_certificate_cb "openssl/ssl.h" HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB) check_symbol_exists(SSL_set1_verify_cert_store "openssl/ssl.h" TS_HAS_VERIFY_CERT_STORE) check_symbol_exists(SSL_get_shared_curve "openssl/ssl.h" HAVE_SSL_GET_SHARED_CURVE) +check_symbol_exists(SSL_get_curve_name "openssl/ssl.h" HAVE_SSL_GET_CURVE_NAME) check_symbol_exists(SSL_set_max_early_data "openssl/ssl.h" HAVE_SSL_SET_MAX_EARLY_DATA) check_symbol_exists(SSL_read_early_data "openssl/ssl.h" HAVE_SSL_READ_EARLY_DATA) check_symbol_exists(SSL_write_early_data "openssl/ssl.h" HAVE_SSL_WRITE_EARLY_DATA) check_symbol_exists(SSL_in_early_data "openssl/ssl.h" HAVE_SSL_IN_EARLY_DATA) +check_symbol_exists(SSL_error_description "openssl/ssl.h" HAVE_SSL_ERROR_DESCRIPTION) check_symbol_exists(SSL_CTX_set_ciphersuites "openssl/ssl.h" TS_USE_TLS_SET_CIPHERSUITES) check_symbol_exists(SSL_CTX_set_keylog_callback "openssl/ssl.h" TS_HAS_TLS_KEYLOGGING) check_symbol_exists(SSL_CTX_set_tlsext_ticket_key_cb "openssl/ssl.h" HAVE_SSL_CTX_SET_TLSEXT_TICKET_KEY_CB) check_symbol_exists(SSL_get_all_async_fds openssl/ssl.h TS_USE_TLS_ASYNC) +check_symbol_exists(OSSL_PARAM_construct_end "openssl/params.h" HAVE_OSSL_PARAM_CONSTRUCT_END) check_symbol_exists(TLS1_3_VERSION "openssl/ssl.h" TS_USE_TLS13) check_symbol_exists(MD5_Init "openssl/md5.h" HAVE_MD5_INIT) check_symbol_exists(ENGINE_load_dynamic "include/openssl/engine.h" HAVE_ENGINE_LOAD_DYNAMIC) @@ -486,6 +490,12 @@ endif() unset(CMAKE_REQUIRED_FLAGS) +if(HAVE_SSL_CTX_SET_CLIENT_HELLO_CB OR HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB) + set(TS_USE_HELLO_CB TRUE) +else() + set(TS_USE_HELLO_CB FALSE) +endif() + if(HAVE_SSL_SET_MAX_EARLY_DATA OR HAVE_SSL_READ_EARL_DATA OR HAVE_SSL_WRITE_EARLY_DATA diff --git a/include/iocore/net/TLSSNISupport.h b/include/iocore/net/TLSSNISupport.h index 1e6685276f..5bc9b652a5 100644 --- a/include/iocore/net/TLSSNISupport.h +++ b/include/iocore/net/TLSSNISupport.h @@ -45,12 +45,10 @@ public: int perform_sni_action(SSL &ssl); // Callback functions for OpenSSL libraries -#if TS_USE_HELLO_CB || defined(OPENSSL_IS_BORINGSSL) -#ifdef OPENSSL_IS_BORINGSSL - void on_client_hello(const SSL_CLIENT_HELLO *client_hello); -#else +#if HAVE_SSL_CTX_SET_CLIENT_HELLO_CB void on_client_hello(SSL *ssl, int *al, void *arg); -#endif +#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB + void on_client_hello(const SSL_CLIENT_HELLO *client_hello); #endif void on_servername(SSL *ssl, int *al, void *arg); diff --git a/include/tscore/HKDF.h b/include/tscore/HKDF.h index b7f06a776c..1a1b98cd89 100644 --- a/include/tscore/HKDF.h +++ b/include/tscore/HKDF.h @@ -23,14 +23,16 @@ #pragma once -#ifdef OPENSSL_IS_BORINGSSL +#include "tscore/ink_config.h" + +#if __has_include(<openssl/digest.h>) #include <openssl/digest.h> #include <openssl/cipher.h> #else #include <openssl/evp.h> #endif -#ifdef OPENSSL_IS_OPENSSL3 +#if HAVE_OSSL_PARAM_CONSTRUCT_END #include <openssl/core.h> #endif @@ -44,7 +46,7 @@ public: uint16_t length); protected: -#ifdef OPENSSL_IS_OPENSSL3 +#if HAVE_OSSL_PARAM_CONSTRUCT_END EVP_KDF_CTX *_kctx = nullptr; OSSL_PARAM params[5]; #else diff --git a/include/tscore/ink_config.h.cmake.in b/include/tscore/ink_config.h.cmake.in index 50e14e4019..9f86c61eb6 100644 --- a/include/tscore/ink_config.h.cmake.in +++ b/include/tscore/ink_config.h.cmake.in @@ -166,7 +166,12 @@ const int DEFAULT_STACKSIZE = @DEFAULT_STACK_SIZE@; // TODO(cmcfarlen): Verify use of below in iocore/net/SSLNetVConnection (redunant) #cmakedefine01 HAVE_SSL_READ_EARLY_DATA #cmakedefine HAVE_SSL_SET_MAX_EARLY_DATA +#cmakedefine01 HAVE_SSL_CTX_SET_CLIENT_HELLO_CB +#cmakedefine01 HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB #cmakedefine01 HAVE_SSL_GET_SHARED_CURVE +#cmakedefine01 HAVE_SSL_GET_CURVE_NAME +#cmakedefine01 HAVE_SSL_ERROR_DESCRIPTION +#cmakedefine01 HAVE_OSSL_PARAM_CONSTRUCT_END #cmakedefine01 TS_USE_TLS_SET_CIPHERSUITES #define TS_BUILD_CANONICAL_HOST "@CMAKE_HOST@" diff --git a/src/iocore/net/P_SSLClientUtils.h b/src/iocore/net/P_SSLClientUtils.h index 8d4b11a067..d42651508b 100644 --- a/src/iocore/net/P_SSLClientUtils.h +++ b/src/iocore/net/P_SSLClientUtils.h @@ -29,7 +29,7 @@ #include <openssl/ssl.h> // BoringSSL does not have this include file -#ifndef OPENSSL_IS_BORINGSSL +#if __has_include(<openssl/opensslconf.h>) #include <openssl/opensslconf.h> #endif diff --git a/src/iocore/net/P_SSLUtils.h b/src/iocore/net/P_SSLUtils.h index 06c8f09469..f1424381c3 100644 --- a/src/iocore/net/P_SSLUtils.h +++ b/src/iocore/net/P_SSLUtils.h @@ -23,8 +23,7 @@ #define OPENSSL_THREAD_DEFINES -// BoringSSL does not have this include file -#ifndef OPENSSL_IS_BORINGSSL +#if __has_include(<openssl/opensslconf.h>) #include <openssl/opensslconf.h> #endif #include <openssl/ssl.h> diff --git a/src/iocore/net/P_TLSKeyLogger.h b/src/iocore/net/P_TLSKeyLogger.h index bcb82f5134..678cd33291 100644 --- a/src/iocore/net/P_TLSKeyLogger.h +++ b/src/iocore/net/P_TLSKeyLogger.h @@ -21,7 +21,7 @@ #pragma once -#ifndef OPENSSL_IS_BORINGSSL +#if __has_include(<openssl/opensslconf.h>) #include <openssl/opensslconf.h> #endif #include <openssl/ssl.h> diff --git a/src/iocore/net/SSLDiags.cc b/src/iocore/net/SSLDiags.cc index 5718b330b2..a7d5bffa45 100644 --- a/src/iocore/net/SSLDiags.cc +++ b/src/iocore/net/SSLDiags.cc @@ -184,7 +184,7 @@ SSLDiagnostic(const SourceLocation &loc, bool debug, SSLNetVConnection *vc, cons const char * SSLErrorName(int ssl_error) { -#ifdef OPENSSL_IS_BORINGSSL +#if HAVE_SSL_ERROR_DESCRIPTION const char *err_descr = SSL_error_description(ssl_error); return err_descr != nullptr ? err_descr : "unknown SSL error"; #else diff --git a/src/iocore/net/SSLUtils.cc b/src/iocore/net/SSLUtils.cc index b2787fd616..8978989000 100644 --- a/src/iocore/net/SSLUtils.cc +++ b/src/iocore/net/SSLUtils.cc @@ -298,7 +298,7 @@ ssl_verify_client_callback(int preverify_ok, X509_STORE_CTX *ctx) return preverify_ok; } -#if TS_USE_HELLO_CB +#if HAVE_SSL_CTX_SET_CLIENT_HELLO_CB // Pausable callback static int ssl_client_hello_callback(SSL *s, int *al, void *arg) @@ -333,7 +333,7 @@ ssl_client_hello_callback(SSL *s, int *al, void *arg) return SSL_CLIENT_HELLO_SUCCESS; } -#elif defined(OPENSSL_IS_BORINGSSL) +#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB static ssl_select_cert_result_t ssl_client_hello_callback(const SSL_CLIENT_HELLO *client_hello) { @@ -459,7 +459,7 @@ ssl_servername_callback(SSL *ssl, int *al, void *arg) TLSSNISupport *snis = TLSSNISupport::getInstance(ssl); if (snis) { snis->on_servername(ssl, al, arg); -#if !TS_USE_HELLO_CB && !defined(OPENSSL_IS_BORINGSSL) +#if !TS_USE_HELLO_CB // Only call the SNI actions here if not already performed in the HELLO_CB int ret = snis->perform_sni_action(*ssl); if (ret != SSL_TLSEXT_ERR_OK) { @@ -1116,14 +1116,14 @@ void SSLMultiCertConfigLoader::_set_handshake_callbacks(SSL_CTX *ctx) { // Make sure the callbacks are set -#ifndef OPENSSL_IS_BORINGSSL +#if !HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB // For OpenSSL < 1.1.1, we should be able to remove this SSL_CTX_set_cert_cb(ctx, ssl_cert_callback, nullptr); SSL_CTX_set_tlsext_servername_callback(ctx, ssl_servername_callback); +#endif -#if TS_USE_HELLO_CB +#if HAVE_SSL_CTX_SET_CLIENT_HELLO_CB SSL_CTX_set_client_hello_cb(ctx, ssl_client_hello_callback, nullptr); -#endif -#else +#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB SSL_CTX_set_select_certificate_cb(ctx, [](const SSL_CLIENT_HELLO *client_hello) -> ssl_select_cert_result_t { ssl_select_cert_result_t res; res = ssl_client_hello_callback(client_hello); diff --git a/src/iocore/net/TLSBasicSupport.cc b/src/iocore/net/TLSBasicSupport.cc index 423213c463..d9b4d1e40b 100644 --- a/src/iocore/net/TLSBasicSupport.cc +++ b/src/iocore/net/TLSBasicSupport.cc @@ -104,7 +104,7 @@ TLSBasicSupport::get_tls_curve() const return nullptr; } ssl_curve_id curve = this->_get_tls_curve(); -#ifndef OPENSSL_IS_BORINGSSL +#if !HAVE_SSL_GET_CURVE_NAME if (curve == NID_undef) { return nullptr; } diff --git a/src/iocore/net/TLSSNISupport.cc b/src/iocore/net/TLSSNISupport.cc index f1710acaa4..3608a655af 100644 --- a/src/iocore/net/TLSSNISupport.cc +++ b/src/iocore/net/TLSSNISupport.cc @@ -87,22 +87,22 @@ TLSSNISupport::perform_sni_action(SSL &ssl) return SSL_TLSEXT_ERR_OK; } -#if TS_USE_HELLO_CB || defined(OPENSSL_IS_BORINGSSL) +#if TS_USE_HELLO_CB void -#ifdef OPENSSL_IS_BORINGSSL -TLSSNISupport::on_client_hello(const SSL_CLIENT_HELLO *client_hello) -#else +#if HAVE_SSL_CTX_SET_CLIENT_HELLO_CB TLSSNISupport::on_client_hello(SSL *ssl, int * /* al ATS_UNUSED */, void * /* arg ATS_UNUSED */) +#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB +TLSSNISupport::on_client_hello(const SSL_CLIENT_HELLO *client_hello) #endif { const char *servername = nullptr; const unsigned char *p; size_t remaining, len; // Parse the server name if the get extension call succeeds and there are more than 2 bytes to parse -#ifdef OPENSSL_IS_BORINGSSL - if (SSL_early_callback_ctx_extension_get(client_hello, TLSEXT_TYPE_server_name, &p, &remaining) && remaining > 2) -#else +#if HAVE_SSL_CTX_SET_CLIENT_HELLO_CB if (SSL_client_hello_get0_ext(ssl, TLSEXT_TYPE_server_name, &p, &remaining) && remaining > 2) +#elif HAVE_SSL_CTX_SET_SELECT_CERTIFICATE_CB + if (SSL_early_callback_ctx_extension_get(client_hello, TLSEXT_TYPE_server_name, &p, &remaining) && remaining > 2) #endif { // Parse to get to the name, originally from test/handshake_helper.c in openssl tree diff --git a/src/iocore/net/YamlSNIConfig.cc b/src/iocore/net/YamlSNIConfig.cc index d9ed950b06..21a6e91999 100644 --- a/src/iocore/net/YamlSNIConfig.cc +++ b/src/iocore/net/YamlSNIConfig.cc @@ -228,7 +228,7 @@ std::set<std::string> valid_sni_config_keys = {TS_fqdn, TS_http2_max_continuation_frames_per_minute, TS_quic, TS_ip_allow, -#if TS_USE_HELLO_CB || defined(OPENSSL_IS_BORINGSSL) +#if TS_USE_HELLO_CB TS_valid_tls_versions_in, TS_valid_tls_version_min_in, TS_valid_tls_version_max_in, diff --git a/src/iocore/net/quic/QUICConfig.cc b/src/iocore/net/quic/QUICConfig.cc index 23d3c50b58..868b9d6218 100644 --- a/src/iocore/net/quic/QUICConfig.cc +++ b/src/iocore/net/quic/QUICConfig.cc @@ -44,14 +44,10 @@ quic_new_ssl_ctx() SSL_CTX_set_min_proto_version(ssl_ctx, TLS1_3_VERSION); SSL_CTX_set_max_proto_version(ssl_ctx, TLS1_3_VERSION); -#ifndef OPENSSL_IS_BORINGSSL +#ifdef SSL_OP_ENABLE_MIDDLEBOX_COMPAT // FIXME: OpenSSL (1.1.1-alpha) enable this option by default. But this should be removed when OpenSSL disable this by default. SSL_CTX_clear_options(ssl_ctx, SSL_OP_ENABLE_MIDDLEBOX_COMPAT); - SSL_CTX_set_max_early_data(ssl_ctx, UINT32_C(0xFFFFFFFF)); - -#else - // QUIC Transport Parameters are accessible with SSL_set_quic_transport_params and SSL_get_peer_quic_transport_params #endif return ssl_ctx;
