This is an automated email from the ASF dual-hosted git repository.

wkaras pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 71f7129334 Reduce the number of checks for BoringSSL (#11564)
71f7129334 is described below

commit 71f7129334a203695821331560483a9f8deb0bb0
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
---
 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 aa914fe258..733af8b6f3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -459,17 +459,21 @@ check_symbol_exists(BIO_meth_get_create "openssl/bio.h" 
HAVE_BIO_METH_GET_CREATE
 check_symbol_exists(BIO_meth_get_destroy "openssl/bio.h" 
HAVE_BIO_METH_GET_DESTROY)
 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)
@@ -485,6 +489,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 fe85c656c5..fff47a7f63 100644
--- a/include/tscore/ink_config.h.cmake.in
+++ b/include/tscore/ink_config.h.cmake.in
@@ -165,7 +165,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 f059a2ef07..edd7561abb 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 b8fb50a556..4795a8f092 100644
--- a/src/iocore/net/SSLUtils.cc
+++ b/src/iocore/net/SSLUtils.cc
@@ -299,7 +299,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)
@@ -334,7 +334,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)
 {
@@ -460,7 +460,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) {
@@ -1117,14 +1117,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 3a7779fc4d..c03ab9e197 100644
--- a/src/iocore/net/TLSBasicSupport.cc
+++ b/src/iocore/net/TLSBasicSupport.cc
@@ -98,7 +98,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;

Reply via email to