C++ client: build with OpenSSL by default (1.x) and bundle the runtime libs#17973
C++ client: build with OpenSSL by default (1.x) and bundle the runtime libs#17973hongzhi-gao wants to merge 3 commits into
Conversation
- Default WITH_SSL / with.ssl to ON (CMake + Maven), so client-cpp and the CI packaging/verify jobs build with SSL by default. - Bundle the resolved OpenSSL shared libraries into the package lib/ (new InstallOpenSSLRuntime.cmake) so the SDK is self-contained. - Prefer system OpenSSL (any version); pin the from-source fallback to OpenSSL 1.1.1w (1.x), no longer 3.x, and use ./config for 1.1.x. - Encode WITH_SSL in the Thrift build stamp and forward OPENSSL_ROOT_DIR so TSSLSocket links the same OpenSSL we bundle. - CI: install openssl-devel in the manylinux build; point macOS at the keg-only Homebrew OpenSSL. - Update README/dependency docs accordingly.
Thrift 0.21's TSSLSocket.cpp does not compile against OpenSSL 3.x (SSLv3_method/TLSv1_method/ASN1_STRING_data/non-const X509_NAME* were removed), which broke the Windows/macOS packaging CI once SSL was on by default. - FetchOpenSSL: accept a system OpenSSL only when it is 1.x; ignore 3.x and build OpenSSL 1.1.1w from source instead. Rename the fallback version variable to OPENSSL_FALLBACK_VERSION to avoid colliding with find_package's OPENSSL_VERSION output. Windows error now asks for 1.1.1. - CI: pin Windows to 'choco install openssl --version=1.1.1.2100'; drop Homebrew openssl on macOS (only ships 3.x; openssl@1.1 is gone) so the build compiles 1.1.1w from source. Linux manylinux keeps system 1.1.1. - Docs updated for the 1.x-only policy. Verified locally on Windows against a clean OpenSSL 1.1.1 root: find_package accepts 1.1.1, iotdb_session.dll links and imports libssl-1_1-x64.dll/libcrypto-1_1-x64.dll, both bundled into lib/.
On CI runners the image ships OpenSSL 3.x and 'choco install openssl 1.1.1.2100' deploys into the same C:\Program Files\OpenSSL, leaving a mixed install: 1.1.1 headers + top-level lib/ but 3.x lib/VC/ import libs. FindOpenSSL prefers the lib/VC/<arch>/<MD|MT>/ import lib, which binds to the 3.x DLL, so the link fails to resolve the 1.1-only symbol SSL_get_peer_certificate that Thrift's TSSLSocket.obj references. After accepting a 1.x OpenSSL on Windows, pin OpenSSL::SSL/Crypto's IMPORTED_LOCATION/IMPORTED_IMPLIB (all configs) to the top-level lib/libssl.lib and lib/libcrypto.lib, which match the bundled 1.1 runtime DLLs. Verified locally on Windows against an equivalent mixed install: iotdb_session.dll links and imports libssl-1_1-x64.dll/ libcrypto-1_1-x64.dll, both bundled into lib/.
HTHou
left a comment
There was a problem hiding this comment.
我看了一下这个 PR,目前建议先不要合,主要有几个 blocker/risk:
-
OpenSSL 1.1.1w fallback 现在是静态链接到 SDK 二进制里。
FetchOpenSSL.cmake里 fallback 使用./config ... no-shared构建 OpenSSL 1.1.1w,然后把它链接进iotdb_session。OpenSSL 1.1.1 不是 Apache-2.0;OpenSSL 官方说明 3.0+ 才是 Apache-2.0,3.0 之前使用旧的 OpenSSL/SSLeay license:https://openssl-library.org/source/license/ 。这意味着当前实现会把 pre-3.0 OpenSSL 代码分发在 ASF convenience binary 里。按 ASF 第三方许可证政策,这类组件不能直接进入 ASF source release 或 convenience binary:https://www.apache.org/legal/resolved.html 。建议不要静态链接/随包分发 OpenSSL 1.1.1,除非 PMC/legal 明确确认;更安全的方向是依赖用户/系统提供的 OpenSSL,或者修 Thrift/OpenSSL 3.x 兼容。 -
动态库 bundling 也有同样的分发问题。
InstallOpenSSLRuntime.cmake会把libssl/libcrypto复制到 SDK 的lib/目录。如果这些是 OpenSSL 1.1.x,同样是在 ASF 包里分发 pre-3.0 OpenSSL 二进制,不能只在DEPENDENCIES.md里写成 Apache License 2.0 解决。 -
DEPENDENCIES.md的 OpenSSL license 标注不正确。
现在写的是OpenSSL | 1.x ... | Apache License 2.0,这只适用于 OpenSSL 3.0+,不适用于 1.1.1w。即使后续保留某种 OpenSSL 1.1 使用方式,也需要正确写 OpenSSL/SSLeay license,并补齐 LICENSE/NOTICE 处理。 -
CMake fallback 可能仍然混用 OpenSSL 3 target。
find_package(OpenSSL QUIET)先运行,如果系统先找到 OpenSSL 3.x,CMake 可能已经创建了OpenSSL::SSL/OpenSSL::Cryptoimported targets。后面只是 unset cache 变量,不会移除或重建这些 targets。结果可能是 fallback 构建了 1.1.1w,但target_link_libraries(... OpenSSL::SSL OpenSSL::Crypto)仍然链接到之前的 3.x target。需要避免先创建 3.x targets,或者在 fallback 后显式重设 target properties 和普通变量。 -
Linux “self-contained” 还缺 loader path。
仅把libssl/libcrypto放到libiotdb_session.so旁边,不保证运行时会被 loader 找到;iotdb_session没看到设置$ORIGIN/相对 RPATH。即使保留 shared OpenSSL bundling,也需要设置安装后的 loader path,并加一个在宿主机没有系统 OpenSSL 1.1 时也能运行的 smoke test。
另外,OpenSSL 1.1.1 已在 2023-09-11 EOL,不再有公开安全修复:https://openssl-corporation.org/post/2023-09-11-eol-111/ 。把它作为默认 SSL runtime/fallback 前,最好有明确的项目级决策。
|
English version of my review above: I think this PR should not be merged yet. The main blocker is that the current fallback/packaging path distributes OpenSSL 1.1.1 code in the C++ SDK binary/package.
Also, OpenSSL 1.1.1 reached EOL on 2023-09-11 and no longer receives public security fixes: https://openssl-corporation.org/post/2023-09-11-eol-111/. Making it the default SSL runtime/fallback should be an explicit project-level decision. |
Description:
Default WITH_SSL/with.ssl to ON so the C++ client and CI build with SSL.
Bundle the linked OpenSSL shared libs into the package lib/ so the SDK is self-contained.
Pin OpenSSL to 1.x: Thrift 0.21 doesn't compile against 3.x. find_package accepts only 1.x (ignores 3.x); otherwise builds 1.1.1w from source. CI installs OpenSSL 1.1.1 accordingly.
On Windows, pin the OpenSSL import libs to the top-level lib/libssl.lib/libcrypto.lib to avoid a 3.x/1.1 mixed-install link failure (SSL_get_peer_certificate).