Skip to content

Remove OpenSSL ENGINE code - #13471

Open
JosiahWI wants to merge 3 commits into
apache:masterfrom
JosiahWI:feat/remove-engine-support
Open

Remove OpenSSL ENGINE code#13471
JosiahWI wants to merge 3 commits into
apache:masterfrom
JosiahWI:feat/remove-engine-support

Conversation

@JosiahWI

@JosiahWI JosiahWI commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Commit a966bc4 (#11219) accidentally disabled OpenSSL ENGINE support entirely. Although it was unintentional, it seems clear no one is using that API by this point (no one has reported it was broken), and the API is gone in recent OpenSSL versions. This patch removes the dead logic.

Commit a966bc4 (apache#11219) accidentally disabled OpenSSL ENGINE support
entirely. Although it was unintentional, it seems clear no one is using
that API by this point (no one has reported it was broken), and the
API is gone in recent OpenSSL versions. This patch removes the dead logic.
@JosiahWI JosiahWI added this to the 11.0.0 milestone Aug 1, 2026
@JosiahWI JosiahWI self-assigned this Aug 1, 2026
Copilot AI review requested due to automatic review settings August 1, 2026 17:56

Copilot AI 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.

Pull request overview

This PR removes remaining OpenSSL ENGINE-specific code paths from ATS’s TLS utilities and build configuration, aligning with the stated goal of dropping a deprecated/removed OpenSSL API.

Changes:

  • Removed ENGINE header usage and runtime ENGINE initialization in SSLPostConfigInitialize().
  • Removed ENGINE-backed private key loading logic in SSLPrivateKeyHandler().
  • Dropped CMake feature probes for ENGINE APIs.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/iocore/net/SSLUtils.cc Removes ENGINE-related initialization and private-key loading code in TLS setup/handling.
CMakeLists.txt Removes OpenSSL ENGINE symbol checks from the build configuration.

Comment thread src/iocore/net/SSLUtils.cc Outdated
Copilot AI review requested due to automatic review settings August 1, 2026 18:19

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/iocore/net/SSLUtils.cc:775

  • The log message still says "SSL engine configuration file", but this code path now just loads OpenSSL config modules via CONF_modules_load_file() (ENGINE support was removed). Keeping "engine" here is misleading for operators troubleshooting config load failures.
  if (SSLConfigParams::engine_conf_file) {
    OPENSSL_load_builtin_modules();
    if (CONF_modules_load_file(SSLConfigParams::engine_conf_file, nullptr, 0) <= 0) {
      char err_buf[256] = {0};
      ERR_error_string_n(ERR_get_error(), err_buf, sizeof(err_buf));
      Error("Could not load SSL engine configuration file %s: %s", SSLConfigParams::engine_conf_file, err_buf);
    }

src/iocore/net/SSLUtils.cc:856

  • After removing ENGINE-based key loading, pkey is always default-initialized to null here, so if (pkey == nullptr) is now unconditional and adds an unnecessary indentation level. This is easier to read if the conditional wrapper is removed.
  // SSL_CTX_use_PrivateKey() takes its own reference on the key, so this
  // reference must be released on every exit.
  scoped_PKEY pkey;
  if (pkey == nullptr) {
    scoped_BIO bio(BIO_new_mem_buf(secret_data, secret_data_len));

Copilot AI review requested due to automatic review settings August 1, 2026 18:33

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

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

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants