Merge pull request ClickHouse#59870 from rschu1ze/be-less-boring-32

boringssl --> OpenSSL 3.2

Author: rschu1ze

.github/workflows/master.yml

@@ -238,14 +238,15 @@ jobs:
       build_name: binary_riscv64
       data: ${{ needs.RunConfig.outputs.data }}
       checkout_depth: 0
-  BuilderBinS390X:
-    needs: [RunConfig, BuilderDebRelease]
-    if: ${{ !failure() && !cancelled() }}
-    uses: ./.github/workflows/reusable_build.yml
-    with:
-      build_name: binary_s390x
-      data: ${{ needs.RunConfig.outputs.data }}
-      checkout_depth: 0
+  # disabled because s390x refused to build in the migration to OpenSSL
+  # BuilderBinS390X:
+  #   needs: [RunConfig, BuilderDebRelease]
+  #   if: ${{ !failure() && !cancelled() }}
+  #   uses: ./.github/workflows/reusable_build.yml
+  #   with:
+  #     build_name: binary_s390x
+  #     data: ${{ needs.RunConfig.outputs.data }}
+  #     checkout_depth: 0
 ############################################################################################
 ##################################### Docker images  #######################################
 ############################################################################################
@@ -296,7 +297,7 @@ jobs:
       - BuilderBinFreeBSD
       - BuilderBinPPC64
       - BuilderBinRISCV64
-      - BuilderBinS390X
+      # - BuilderBinS390X # disabled because s390x refused to build in the migration to OpenSSL
       - BuilderBinAmd64Compat
       - BuilderBinAarch64V80Compat
       - BuilderBinClangTidy

.gitmodules

@@ -173,9 +173,6 @@
 [submodule "contrib/libpq"]
 	path = contrib/libpq
 	url = https://github.com/ClickHouse/libpq
-[submodule "contrib/boringssl"]
-	path = contrib/boringssl
-	url = https://github.com/ClickHouse/boringssl
 [submodule "contrib/NuRaft"]
 	path = contrib/NuRaft
 	url = https://github.com/ClickHouse/NuRaft
@@ -275,9 +272,6 @@
 [submodule "contrib/crc32-s390x"]
 	path = contrib/crc32-s390x
 	url = https://github.com/linux-on-ibm-z/crc32-s390x
-[submodule "contrib/openssl"]
-	path = contrib/openssl
-	url = https://github.com/openssl/openssl
 [submodule "contrib/google-benchmark"]
 	path = contrib/google-benchmark
 	url = https://github.com/google/benchmark
@@ -366,6 +360,9 @@
 [submodule "contrib/idna"]
 	path = contrib/idna
 	url = https://github.com/ada-url/idna.git
+[submodule "contrib/openssl"]
+	path = contrib/openssl
+	url = https://github.com/ClickHouse/openssl.git
 [submodule "contrib/double-conversion"]
 	path = contrib/double-conversion
 	url = https://github.com/ClickHouse/double-conversion.git

CMakeLists.txt

@@ -455,8 +455,6 @@ endif ()
 
 enable_testing() # Enable for tests without binary
 
-option(ENABLE_OPENSSL "This option performs a build with OpenSSL. NOTE! This option is insecure and should never be used. By default, ClickHouse uses and only supports BoringSSL" OFF)
-
 if (ARCH_S390X)
     set(ENABLE_OPENSSL_DYNAMIC_DEFAULT ON)
 else ()

base/poco/Crypto/src/OpenSSLInitializer.cpp

@@ -23,6 +23,9 @@
 #include <openssl/conf.h>
 #endif
 
+#if __has_feature(address_sanitizer)
+#include <sanitizer/lsan_interface.h>
+#endif
 
 using Poco::RandomInputStream;
 using Poco::Thread;
@@ -67,21 +70,27 @@ void OpenSSLInitializer::initialize()
 		SSL_library_init();
 		SSL_load_error_strings();
 		OpenSSL_add_all_algorithms();
-		
+
 		char seed[SEEDSIZE];
 		RandomInputStream rnd;
 		rnd.read(seed, sizeof(seed));
-		RAND_seed(seed, SEEDSIZE);
-		
+        {
+#   if __has_feature(address_sanitizer)
+            /// Leak sanitizer (part of address sanitizer) thinks that a few bytes of memory in OpenSSL are allocated during but never released.
+            __lsan::ScopedDisabler lsan_disabler;
+#endif
+		    RAND_seed(seed, SEEDSIZE);
+        }
+
 		int nMutexes = CRYPTO_num_locks();
 		_mutexes = new Poco::FastMutex[nMutexes];
 		CRYPTO_set_locking_callback(&OpenSSLInitializer::lock);
 // Not needed on Windows (see SF #110: random unhandled exceptions when linking with ssl).
 // https://sourceforge.net/p/poco/bugs/110/
 //
 // From http://www.openssl.org/docs/crypto/threads.html :
-// "If the application does not register such a callback using CRYPTO_THREADID_set_callback(), 
-//  then a default implementation is used - on Windows and BeOS this uses the system's 
+// "If the application does not register such a callback using CRYPTO_THREADID_set_callback(),
+//  then a default implementation is used - on Windows and BeOS this uses the system's
 //  default thread identifying APIs"
 		CRYPTO_set_id_callback(&OpenSSLInitializer::id);
 		CRYPTO_set_dynlock_create_callback(&OpenSSLInitializer::dynlockCreate);
@@ -100,7 +109,7 @@ void OpenSSLInitializer::uninitialize()
 		CRYPTO_set_locking_callback(0);
 		CRYPTO_set_id_callback(0);
 		delete [] _mutexes;
-		
+
 		CONF_modules_free();
 	}
 }

base/poco/NetSSL_OpenSSL/src/Context.cpp

@@ -592,6 +592,7 @@ void Context::createSSLContext()
 	SSL_CTX_set_default_passwd_cb(_pSSLContext, &SSLManager::privateKeyPassphraseCallback);
 	Utility::clearErrorStack();
 	SSL_CTX_set_options(_pSSLContext, SSL_OP_ALL);
+	SSL_CTX_set_options(_pSSLContext, SSL_OP_IGNORE_UNEXPECTED_EOF);
 }
 
 

base/poco/NetSSL_OpenSSL/src/SSLManager.cpp

@@ -125,7 +125,7 @@ void SSLManager::initializeClient(PrivateKeyPassphraseHandlerPtr ptrPassphraseHa
 Context::Ptr SSLManager::defaultServerContext()
 {
 	Poco::FastMutex::ScopedLock lock(_mutex);
-	
+
 	if (!_ptrDefaultServerContext)
 		initDefaultContext(true);
 
@@ -150,7 +150,7 @@ Context::Ptr SSLManager::defaultClientContext()
 			_ptrDefaultClientContext->disableProtocols(Context::PROTO_SSLV2 | Context::PROTO_SSLV3);
 		}
 	}
-		
+
 	return _ptrDefaultClientContext;
 }
 
@@ -256,7 +256,7 @@ void SSLManager::initDefaultContext(bool server)
 	Context::Params params;
 	// mandatory options
 	params.privateKeyFile = config.getString(prefix + CFG_PRIV_KEY_FILE, "");
-	params.certificateFile = config.getString(prefix + CFG_CERTIFICATE_FILE, params.privateKeyFile);	
+	params.certificateFile = config.getString(prefix + CFG_CERTIFICATE_FILE, params.privateKeyFile);
 	params.caLocation = config.getString(prefix + CFG_CA_LOCATION, "");
 
 	if (server && params.certificateFile.empty() && params.privateKeyFile.empty())
@@ -283,7 +283,7 @@ void SSLManager::initDefaultContext(bool server)
 	params.ecdhCurve    = config.getString(prefix + CFG_ECDH_CURVE, "");
 
 	Context::Usage usage;
-	
+
 	if (server)
 	{
 		if (requireTLSv1_2)
@@ -308,7 +308,7 @@ void SSLManager::initDefaultContext(bool server)
 			usage = Context::CLIENT_USE;
 		_ptrDefaultClientContext = new Context(usage, params);
 	}
-	
+
 	std::string disabledProtocolsList = config.getString(prefix + CFG_DISABLE_PROTOCOLS, "");
 	Poco::StringTokenizer dpTok(disabledProtocolsList, ";,", Poco::StringTokenizer::TOK_TRIM | Poco::StringTokenizer::TOK_IGNORE_EMPTY);
 	int disabledProtocols = 0;
@@ -329,27 +329,28 @@ void SSLManager::initDefaultContext(bool server)
 		_ptrDefaultServerContext->disableProtocols(disabledProtocols);
 	else
 		_ptrDefaultClientContext->disableProtocols(disabledProtocols);
-		
-	bool cacheSessions = config.getBool(prefix + CFG_CACHE_SESSIONS, false);
-	if (server)
-	{
-		std::string sessionIdContext = config.getString(prefix + CFG_SESSION_ID_CONTEXT, config.getString("application.name", ""));
-		_ptrDefaultServerContext->enableSessionCache(cacheSessions, sessionIdContext);
-		if (config.hasProperty(prefix + CFG_SESSION_CACHE_SIZE))
-		{
-			int cacheSize = config.getInt(prefix + CFG_SESSION_CACHE_SIZE);
-			_ptrDefaultServerContext->setSessionCacheSize(cacheSize);
-		}
-		if (config.hasProperty(prefix + CFG_SESSION_TIMEOUT))
-		{
-			int timeout = config.getInt(prefix + CFG_SESSION_TIMEOUT);
-			_ptrDefaultServerContext->setSessionTimeout(timeout);
-		}
-	}
-	else
-	{
-		_ptrDefaultClientContext->enableSessionCache(cacheSessions);
-	}
+
+    /// Temporarily disabled during the transition from boringssl to OpenSSL due to tsan issues.
+	/// bool cacheSessions = config.getBool(prefix + CFG_CACHE_SESSIONS, false);
+	/// if (server)
+	/// {
+	/// 	std::string sessionIdContext = config.getString(prefix + CFG_SESSION_ID_CONTEXT, config.getString("application.name", ""));
+	/// 	_ptrDefaultServerContext->enableSessionCache(cacheSessions, sessionIdContext);
+	/// 	if (config.hasProperty(prefix + CFG_SESSION_CACHE_SIZE))
+	/// 	{
+	/// 		int cacheSize = config.getInt(prefix + CFG_SESSION_CACHE_SIZE);
+	/// 		_ptrDefaultServerContext->setSessionCacheSize(cacheSize);
+	/// 	}
+	/// 	if (config.hasProperty(prefix + CFG_SESSION_TIMEOUT))
+	/// 	{
+	/// 		int timeout = config.getInt(prefix + CFG_SESSION_TIMEOUT);
+	/// 		_ptrDefaultServerContext->setSessionTimeout(timeout);
+	/// 	}
+	/// }
+	/// else
+	/// {
+	/// 	_ptrDefaultClientContext->enableSessionCache(cacheSessions);
+	/// }
 	bool extendedVerification = config.getBool(prefix + CFG_EXTENDED_VERIFICATION, false);
 	if (server)
 		_ptrDefaultServerContext->enableExtendedCertificateVerification(extendedVerification);
@@ -378,7 +379,7 @@ void SSLManager::initPassphraseHandler(bool server)
 {
 	if (server && _ptrServerPassphraseHandler) return;
 	if (!server && _ptrClientPassphraseHandler) return;
-	
+
 	std::string prefix = server ? CFG_SERVER_PREFIX : CFG_CLIENT_PREFIX;
 	Poco::Util::AbstractConfiguration& config = appConfig();
 
@@ -399,7 +400,7 @@ void SSLManager::initPassphraseHandler(bool server)
 	}
 	else throw Poco::Util::UnknownOptionException(std::string("No passphrase handler known with the name ") + className);
 }
-	
+
 
 void SSLManager::initCertificateHandler(bool server)
 {

contrib/CMakeLists.txt

@@ -37,11 +37,7 @@ function(add_contrib cmake_folder)
     message(STATUS "Adding contrib module ${base_folders} (configuring with ${cmake_folder})")
     add_subdirectory (${cmake_folder})
 endfunction()
-if (ENABLE_OPENSSL OR ENABLE_OPENSSL_DYNAMIC)
-    add_contrib (openssl-cmake openssl)
-else ()
-    add_contrib (boringssl-cmake boringssl)
-endif ()
+add_contrib (openssl-cmake openssl)
 add_contrib (miniselect-cmake miniselect)
 add_contrib (pdqsort-cmake pdqsort)
 add_contrib (pocketfft-cmake pocketfft)