Skip to content

C++ client: build with OpenSSL by default (1.x) and bundle the runtime libs#17973

Open
hongzhi-gao wants to merge 3 commits into
apache:masterfrom
hongzhi-gao:ci/cpp-build-ssl
Open

C++ client: build with OpenSSL by default (1.x) and bundle the runtime libs#17973
hongzhi-gao wants to merge 3 commits into
apache:masterfrom
hongzhi-gao:ci/cpp-build-ssl

Conversation

@hongzhi-gao

Copy link
Copy Markdown
Contributor

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).

- 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 HTHou left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我看了一下这个 PR,目前建议先不要合,主要有几个 blocker/risk:

  1. 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 兼容。

  2. 动态库 bundling 也有同样的分发问题。
    InstallOpenSSLRuntime.cmake 会把 libssl/libcrypto 复制到 SDK 的 lib/ 目录。如果这些是 OpenSSL 1.1.x,同样是在 ASF 包里分发 pre-3.0 OpenSSL 二进制,不能只在 DEPENDENCIES.md 里写成 Apache License 2.0 解决。

  3. 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 处理。

  4. CMake fallback 可能仍然混用 OpenSSL 3 target。
    find_package(OpenSSL QUIET) 先运行,如果系统先找到 OpenSSL 3.x,CMake 可能已经创建了 OpenSSL::SSL / OpenSSL::Crypto imported targets。后面只是 unset cache 变量,不会移除或重建这些 targets。结果可能是 fallback 构建了 1.1.1w,但 target_link_libraries(... OpenSSL::SSL OpenSSL::Crypto) 仍然链接到之前的 3.x target。需要避免先创建 3.x targets,或者在 fallback 后显式重设 target properties 和普通变量。

  5. 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 前,最好有明确的项目级决策。

@HTHou

HTHou commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

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.

  1. The OpenSSL 1.1.1w fallback is statically linked into the SDK binary.
    FetchOpenSSL.cmake builds OpenSSL 1.1.1w with ./config ... no-shared, then links it into iotdb_session. OpenSSL 1.1.1 is not Apache-2.0 licensed. OpenSSL states that 3.0+ is Apache-2.0, while pre-3.0 releases use the old OpenSSL/SSLeay license: https://openssl-library.org/source/license/. This means the current implementation would distribute pre-3.0 OpenSSL code inside an ASF convenience binary. ASF third-party licensing policy says incompatible components must not be distributed in ASF source releases or convenience binaries: https://www.apache.org/legal/resolved.html. Please avoid statically linking or redistributing OpenSSL 1.1.1 unless the PMC/legal path explicitly approves it. Safer options are to rely on user/system-provided OpenSSL as an optional dependency, or to make the Thrift/OpenSSL path work with OpenSSL 3.x.

  2. Bundling shared OpenSSL libraries has the same distribution concern.
    InstallOpenSSLRuntime.cmake copies libssl/libcrypto into the SDK lib/ directory. If those are OpenSSL 1.1.x libraries, the ASF package is still redistributing pre-3.0 OpenSSL binaries. This cannot be fixed merely by listing OpenSSL as Apache License 2.0 in DEPENDENCIES.md.

  3. The OpenSSL license entry in DEPENDENCIES.md is inaccurate.
    The current entry says OpenSSL | 1.x ... | Apache License 2.0, but Apache-2.0 applies to OpenSSL 3.0+, not 1.1.1w. If any OpenSSL 1.1 usage remains, the metadata and LICENSE/NOTICE handling need to reflect the OpenSSL/SSLeay license correctly.

  4. The CMake fallback can still mix OpenSSL 3 targets with the 1.1.1 fallback.
    find_package(OpenSSL QUIET) runs first. If it finds OpenSSL 3.x, CMake may already create OpenSSL::SSL and OpenSSL::Crypto imported targets. Unsetting cache variables later does not remove or recreate those targets. The result can be that the fallback builds 1.1.1w, but target_link_libraries(... OpenSSL::SSL OpenSSL::Crypto) still links the previously created 3.x targets. Please avoid creating the 3.x targets in the first place, or explicitly reset target properties and normal variables after selecting the fallback.

  5. The Linux package is not fully self-contained without a loader path.
    Placing libssl/libcrypto next to libiotdb_session.so does not guarantee the dynamic loader will find them. I do not see an $ORIGIN/relative install RPATH on iotdb_session. If shared OpenSSL bundling remains, please set the installed loader path and add a smoke test that runs on a host without system OpenSSL 1.1 installed.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants