From 51d6611b96b1b7c24bf8252e4ff75d2611d7f08b Mon Sep 17 00:00:00 2001 From: Meena Renganathan Date: Mon, 12 Sep 2022 09:05:38 -0700 Subject: [PATCH 1/5] Committing the ClickHouse core changes and other libraries to support OpenSSL. BoringSSL is still set as default --- CMakeLists.txt | 14 ++++++++++++++ contrib/CMakeLists.txt | 6 +++++- contrib/krb5-cmake/CMakeLists.txt | 6 ++++++ contrib/libpq-cmake/CMakeLists.txt | 6 ++++++ programs/server/Server.cpp | 8 ++++++-- src/CMakeLists.txt | 8 +++++++- src/Compression/CompressionFactory.cpp | 4 ++++ src/Core/config_core.h.in | 1 + src/configure_config.cmake | 3 +++ 9 files changed, 52 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 64fb870b61b..52626c7badf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,6 +495,20 @@ endif () enable_testing() # Enable for tests without binary +option(ENABLE_CH_BUNDLE_BORINGSSL "Provide the user to allow building of OpenSSL library. By default, uses in-house ClickHouse BoringSSL" ON) + +message (STATUS "ENABLE_CH_BUNDLE_BORINGSSL: ${ENABLE_CH_BUNDLE_BORINGSSL}") +if (ENABLE_CH_BUNDLE_BORINGSSL) + message (STATUS "Uses in-house ClickHouse BoringSSL library") +else () + message (STATUS "Build and uses OpenSSL library instead of BoringSSL") +endif () + +if (NOT ENABLE_CH_BUNDLE_BORINGSSL) + set(ENABLE_SSL 1) + target_compile_options(global-group INTERFACE "-Wno-deprecated-declarations") +endif () + # when installing to /usr - place configs to /etc but for /usr/local place to /usr/local/etc if (CMAKE_INSTALL_PREFIX STREQUAL "/usr") set (CLICKHOUSE_ETC_DIR "/etc") diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index 08b91c1b81c..a1baca69c1c 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -74,7 +74,11 @@ add_contrib (re2-cmake re2) add_contrib (xz-cmake xz) add_contrib (brotli-cmake brotli) add_contrib (double-conversion-cmake double-conversion) -add_contrib (boringssl-cmake boringssl) +if (ENABLE_CH_BUNDLE_BORINGSSL) + add_contrib (boringssl-cmake boringssl) +else () + add_contrib (openssl-cmake openssl) +endif () add_contrib (poco-cmake poco) add_contrib (croaring-cmake croaring) add_contrib (zstd-cmake zstd) diff --git a/contrib/krb5-cmake/CMakeLists.txt b/contrib/krb5-cmake/CMakeLists.txt index 214d23bc2a9..95f8e7e0d21 100644 --- a/contrib/krb5-cmake/CMakeLists.txt +++ b/contrib/krb5-cmake/CMakeLists.txt @@ -578,6 +578,12 @@ if(CMAKE_SYSTEM_NAME MATCHES "Darwin") list(APPEND ALL_SRCS "${CMAKE_CURRENT_BINARY_DIR}/include_private/kcmrpc.c") endif() +if (NOT ENABLE_CH_BUNDLE_BORINGSSL) + list(REMOVE_ITEM ALL_SRCS "${KRB5_SOURCE_DIR}/lib/crypto/openssl/enc_provider/aes.c") + list(APPEND ALL_SRCS "${CMAKE_CURRENT_SOURCE_DIR}/aes.c") +endif () + + target_sources(_krb5 PRIVATE ${ALL_SRCS} ) diff --git a/contrib/libpq-cmake/CMakeLists.txt b/contrib/libpq-cmake/CMakeLists.txt index 280c0381393..26ece28bd18 100644 --- a/contrib/libpq-cmake/CMakeLists.txt +++ b/contrib/libpq-cmake/CMakeLists.txt @@ -59,6 +59,12 @@ set(SRCS add_library(_libpq ${SRCS}) +if (NOT ENABLE_CH_BUNDLE_BORINGSSL) + add_definitions(-DHAVE_BIO_METH_NEW) + add_definitions(-DHAVE_HMAC_CTX_NEW) + add_definitions(-DHAVE_HMAC_CTX_FREE) +endif () + target_include_directories (_libpq SYSTEM PUBLIC ${LIBPQ_SOURCE_DIR}) target_include_directories (_libpq SYSTEM PUBLIC "${LIBPQ_SOURCE_DIR}/include") target_include_directories (_libpq SYSTEM PRIVATE "${LIBPQ_SOURCE_DIR}/configs") diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 93df877ab8e..ab55e527431 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -79,7 +79,9 @@ #include #include #include +#if USE_BORINGSSL #include +#endif #include #include #include @@ -1263,8 +1265,9 @@ int Server::main(const std::vector & /*args*/) global_context->updateStorageConfiguration(*config); global_context->updateInterserverCredentials(*config); - +#if USE_BORINGSSL CompressionCodecEncrypted::Configuration::instance().tryLoad(*config, "encryption_codecs"); +#endif #if USE_SSL CertificateReloader::instance().tryLoad(*config); #endif @@ -1467,9 +1470,10 @@ int Server::main(const std::vector & /*args*/) global_context->getMergeTreeSettings().sanityCheck(background_pool_tasks); global_context->getReplicatedMergeTreeSettings().sanityCheck(background_pool_tasks); } - +#if USE_BORINGSSL /// try set up encryption. There are some errors in config, error will be printed and server wouldn't start. CompressionCodecEncrypted::Configuration::instance().load(config(), "encryption_codecs"); +#endif SCOPE_EXIT({ /// Stop reloading of the main config. This must be done before `global_context->shutdown()` because diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 3dc42746d67..c0246486110 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -247,7 +247,13 @@ add_object_library(clickhouse_access Access) add_object_library(clickhouse_backups Backups) add_object_library(clickhouse_core Core) add_object_library(clickhouse_core_mysql Core/MySQL) -add_object_library(clickhouse_compression Compression) +if (ENABLE_CH_BUNDLE_BORINGSSL) + add_object_library(clickhouse_compression Compression) +else () + add_headers_and_sources(dbms Compression) + list(REMOVE_ITEM dbms_headers Compression/CompressionCodecEncrypted.h) + list(REMOVE_ITEM dbms_sources Compression/CompressionCodecEncrypted.cpp) +endif () add_object_library(clickhouse_querypipeline QueryPipeline) add_object_library(clickhouse_datatypes DataTypes) add_object_library(clickhouse_datatypes_serializations DataTypes/Serializations) diff --git a/src/Compression/CompressionFactory.cpp b/src/Compression/CompressionFactory.cpp index 7291d42f681..2d07dba6cdf 100644 --- a/src/Compression/CompressionFactory.cpp +++ b/src/Compression/CompressionFactory.cpp @@ -176,7 +176,9 @@ void registerCodecDelta(CompressionCodecFactory & factory); void registerCodecT64(CompressionCodecFactory & factory); void registerCodecDoubleDelta(CompressionCodecFactory & factory); void registerCodecGorilla(CompressionCodecFactory & factory); +#if USE_BORINGSSL void registerCodecEncrypted(CompressionCodecFactory & factory); +#endif void registerCodecFPC(CompressionCodecFactory & factory); #endif @@ -193,7 +195,9 @@ CompressionCodecFactory::CompressionCodecFactory() registerCodecT64(*this); registerCodecDoubleDelta(*this); registerCodecGorilla(*this); +#if USE_BORINGSSL registerCodecEncrypted(*this); +#endif registerCodecFPC(*this); #ifdef ENABLE_QPL_COMPRESSION registerCodecDeflateQpl(*this); diff --git a/src/Core/config_core.h.in b/src/Core/config_core.h.in index 46c77593d4e..963226d70e5 100644 --- a/src/Core/config_core.h.in +++ b/src/Core/config_core.h.in @@ -22,3 +22,4 @@ #cmakedefine01 USE_ODBC #cmakedefine01 USE_REPLXX #cmakedefine01 USE_JEMALLOC +#cmakedefine01 USE_BORINGSSL diff --git a/src/configure_config.cmake b/src/configure_config.cmake index 3f3ddf54716..a2ab5bc82e5 100644 --- a/src/configure_config.cmake +++ b/src/configure_config.cmake @@ -103,3 +103,6 @@ endif() if (TARGET ch_contrib::jemalloc) set(USE_JEMALLOC 1) endif() +if (ENABLE_CH_BUNDLE_BORINGSSL) + set(USE_BORINGSSL 1) +endif () From bb1771e159760365ad1bd9ff221b9ce4b9246e31 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 30 Sep 2022 10:35:49 -0700 Subject: [PATCH 2/5] Update the macro ENABLE_CH_BUNDLE_BORINGSSLL to ENABLE_EXTERNAL_OPENSSL --- CMakeLists.txt | 8 ++++---- contrib/CMakeLists.txt | 2 +- contrib/krb5-cmake/CMakeLists.txt | 2 +- contrib/libpq-cmake/CMakeLists.txt | 2 +- src/CMakeLists.txt | 2 +- src/configure_config.cmake | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 52626c7badf..47bdbafb0d8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,16 +495,16 @@ endif () enable_testing() # Enable for tests without binary -option(ENABLE_CH_BUNDLE_BORINGSSL "Provide the user to allow building of OpenSSL library. By default, uses in-house ClickHouse BoringSSL" ON) +option(ENABLE_EXTERNAL_OPENSSL "Provide the user to allow building of OpenSSL library. By default, uses in-house ClickHouse BoringSSL. It is not recommended and may be insecure" OFF) -message (STATUS "ENABLE_CH_BUNDLE_BORINGSSL: ${ENABLE_CH_BUNDLE_BORINGSSL}") -if (ENABLE_CH_BUNDLE_BORINGSSL) +message (STATUS "ENABLE_EXTERNAL_OPENSSL: ${ENABLE_EXTERNAL_OPENSSL}") +if (NOT ENABLE_EXTERNAL_OPENSSL) message (STATUS "Uses in-house ClickHouse BoringSSL library") else () message (STATUS "Build and uses OpenSSL library instead of BoringSSL") endif () -if (NOT ENABLE_CH_BUNDLE_BORINGSSL) +if (ENABLE_EXTERNAL_OPENSSL) set(ENABLE_SSL 1) target_compile_options(global-group INTERFACE "-Wno-deprecated-declarations") endif () diff --git a/contrib/CMakeLists.txt b/contrib/CMakeLists.txt index a1baca69c1c..06e697e9dbf 100644 --- a/contrib/CMakeLists.txt +++ b/contrib/CMakeLists.txt @@ -74,7 +74,7 @@ add_contrib (re2-cmake re2) add_contrib (xz-cmake xz) add_contrib (brotli-cmake brotli) add_contrib (double-conversion-cmake double-conversion) -if (ENABLE_CH_BUNDLE_BORINGSSL) +if (NOT ENABLE_EXTERNAL_OPENSSL) add_contrib (boringssl-cmake boringssl) else () add_contrib (openssl-cmake openssl) diff --git a/contrib/krb5-cmake/CMakeLists.txt b/contrib/krb5-cmake/CMakeLists.txt index 95f8e7e0d21..8478def3cb1 100644 --- a/contrib/krb5-cmake/CMakeLists.txt +++ b/contrib/krb5-cmake/CMakeLists.txt @@ -578,7 +578,7 @@ if(CMAKE_SYSTEM_NAME MATCHES "Darwin") list(APPEND ALL_SRCS "${CMAKE_CURRENT_BINARY_DIR}/include_private/kcmrpc.c") endif() -if (NOT ENABLE_CH_BUNDLE_BORINGSSL) +if (ENABLE_EXTERNAL_OPENSSL) list(REMOVE_ITEM ALL_SRCS "${KRB5_SOURCE_DIR}/lib/crypto/openssl/enc_provider/aes.c") list(APPEND ALL_SRCS "${CMAKE_CURRENT_SOURCE_DIR}/aes.c") endif () diff --git a/contrib/libpq-cmake/CMakeLists.txt b/contrib/libpq-cmake/CMakeLists.txt index 26ece28bd18..1f02f24bb24 100644 --- a/contrib/libpq-cmake/CMakeLists.txt +++ b/contrib/libpq-cmake/CMakeLists.txt @@ -59,7 +59,7 @@ set(SRCS add_library(_libpq ${SRCS}) -if (NOT ENABLE_CH_BUNDLE_BORINGSSL) +if (ENABLE_EXTERNAL_OPENSSL) add_definitions(-DHAVE_BIO_METH_NEW) add_definitions(-DHAVE_HMAC_CTX_NEW) add_definitions(-DHAVE_HMAC_CTX_FREE) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c0246486110..2cf83eb0d01 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -247,7 +247,7 @@ add_object_library(clickhouse_access Access) add_object_library(clickhouse_backups Backups) add_object_library(clickhouse_core Core) add_object_library(clickhouse_core_mysql Core/MySQL) -if (ENABLE_CH_BUNDLE_BORINGSSL) +if (NOT ENABLE_EXTERNAL_OPENSSL) add_object_library(clickhouse_compression Compression) else () add_headers_and_sources(dbms Compression) diff --git a/src/configure_config.cmake b/src/configure_config.cmake index a2ab5bc82e5..9efa1127c64 100644 --- a/src/configure_config.cmake +++ b/src/configure_config.cmake @@ -103,6 +103,6 @@ endif() if (TARGET ch_contrib::jemalloc) set(USE_JEMALLOC 1) endif() -if (ENABLE_CH_BUNDLE_BORINGSSL) +if (NOT ENABLE_EXTERNAL_OPENSSL) set(USE_BORINGSSL 1) endif () From d220c6ada6abbea8f517b335175a47dae1580555 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 5 Oct 2022 09:12:10 +0200 Subject: [PATCH 3/5] Ignore core.autocrlf for tests references Signed-off-by: Azat Khuzhin --- .gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitattributes b/.gitattributes index a23f027122b..56d6fecf4b8 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,4 @@ contrib/* linguist-vendored *.h linguist-language=C++ tests/queries/0_stateless/data_json/* binary +tests/queries/0_stateless/*.reference -crlf From 89b1a5188f3890af9bd8a39d5e51f51ab6307ca5 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 5 Oct 2022 22:59:44 +0300 Subject: [PATCH 4/5] Update CMakeLists.txt --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f1a22fe717b..7764c2b72e7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -495,7 +495,7 @@ endif () enable_testing() # Enable for tests without binary -option(ENABLE_EXTERNAL_OPENSSL "Provide the user to allow building of OpenSSL library. By default, uses in-house ClickHouse BoringSSL. It is not recommended and may be insecure" OFF) +option(ENABLE_EXTERNAL_OPENSSL "This option is insecure and not recommended for any occasions. If it is enabled, it allows building with alternative OpenSSL library. By default, ClickHouse is using BoringSSL, which is better. Do not use this option." OFF) message (STATUS "ENABLE_EXTERNAL_OPENSSL: ${ENABLE_EXTERNAL_OPENSSL}") if (NOT ENABLE_EXTERNAL_OPENSSL) From a46206571e8ff7abc2b2f075cffc31989e02f0ab Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 5 Oct 2022 23:00:49 +0300 Subject: [PATCH 5/5] Update CMakeLists.txt --- CMakeLists.txt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7764c2b72e7..0c802728061 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -497,14 +497,8 @@ enable_testing() # Enable for tests without binary option(ENABLE_EXTERNAL_OPENSSL "This option is insecure and not recommended for any occasions. If it is enabled, it allows building with alternative OpenSSL library. By default, ClickHouse is using BoringSSL, which is better. Do not use this option." OFF) -message (STATUS "ENABLE_EXTERNAL_OPENSSL: ${ENABLE_EXTERNAL_OPENSSL}") -if (NOT ENABLE_EXTERNAL_OPENSSL) - message (STATUS "Uses in-house ClickHouse BoringSSL library") -else () - message (STATUS "Build and uses OpenSSL library instead of BoringSSL") -endif () - if (ENABLE_EXTERNAL_OPENSSL) + message (STATUS "Build and uses OpenSSL library instead of BoringSSL. This is strongly discouraged. Your build of ClickHouse will be unsupported.") set(ENABLE_SSL 1) target_compile_options(global-group INTERFACE "-Wno-deprecated-declarations") endif ()