[
https://issues.apache.org/jira/browse/THRIFT-6073?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6073.
--------------------------------
Assignee: Hongzhi Gao
Resolution: Fixed
> Allow injecting external SSL_CTX into C++ SSLContext
> ----------------------------------------------------
>
> Key: THRIFT-6073
> URL: https://issues.apache.org/jira/browse/THRIFT-6073
> Project: Thrift
> Issue Type: New Feature
> Components: C++ - Library
> Affects Versions: 0.24.0
> Environment: C++ libthrift; default OpenSSL 3.x build. SSLContext
> injection verified via SecurityTest on standard OpenSSL CI configuration.
> Reporter: Hongzhi Gao
> Assignee: Hongzhi Gao
> Priority: Major
> Fix For: 0.24.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> h2. Summary
> Add a backend-neutral extension point to the C++ TSSLSocket stack:
> applications can inject a pre-configured OpenSSL SSL_CTX through the existing
> SSLContextFactory / TSSLSocketFactory hook.
> This makes it easier for applications to supply a fully configured TLS
> context—protocol options, cipher suites, certificate loading, and other
> OpenSSL settings—in application code, without patching libthrift or replacing
> the transport layer.
> h2. Problem
> Thrift C++ provides SSL/TLS via TSSLSocket / TSSLSocketFactory
> (OpenSSL-backed). TSSLSocketFactory already accepts a custom
> SSLContextFactory, but SSLContext could only be constructed from SSLProtocol.
> Applications that need TLS setup beyond the default helpers cannot supply an
> externally configured SSL_CTX without patching Thrift locally.
> Specific gaps today:
> * loadCertificate() / loadPrivateKey() cover the common single cert/key case;
> multi-step or non-standard certificate loading has no injection point at the
> SSLContext level.
> * Protocol methods, cipher configuration, and other OpenSSL options not
> exposed by TSSLSocketFactory cannot be applied to a Thrift-managed context
> without forking.
> h2. Proposed solution
> Add {{explicit SSLContext(SSL_CTX* ctx)}} to wrap an application-configured
> SSL_CTX. Applications configure the context with OpenSSL (or an
> OpenSSL-compatible library) in application code, then pass it through the
> existing SSLContextFactory hook. Thrift continues to handle handshake, I/O,
> and certificate verification policy via TSSLSocketFactory.
> No provider-specific build options, protocol enums, or certificate-loading
> APIs in libthrift.
> h2. Design decisions
> *Backend-neutral hook (post-review reshape):*
> An earlier draft added Tongsuo/NTLS-specific build plumbing,
> {{SSLProtocol::NTLS}}, and {{loadSign*}} / {{loadEnc*}} APIs. Per reviewer
> feedback, those were removed. The merged approach is a single
> injected-context constructor on {{SSLContext}}, reusing the existing
> {{SSLContextFactory}} hook.
> *Ownership semantics:*
> The injected constructor unconditionally takes ownership of the {{SSL_CTX*}}.
> The caller must not call {{SSL_CTX_free}} after passing the pointer.
> {{~SSLContext}} always calls {{SSL_CTX_free}}.
> An initial implementation used {{takeOwnership=true/false}}. That was dropped
> in PR commit dcc2a06c4 because:
> * {{takeOwnership=false}} required {{SSL_CTX_up_ref}} semantics to avoid
> use-after-free, and the API was easy to misuse.
> * Adding {{bool takeOwnership_}} changed {{sizeof(SSLContext)}} in a public
> installed header without an ABI versioning mechanism in libthrift.
> *TLS version floor:*
> {{SSLContext(SSLProtocol)}} with default {{SSLTLS}} applies {{SSL_OP_NO_SSLv2
> | SSL_OP_NO_SSLv3 | SSL_OP_NO_TLSv1 | SSL_OP_NO_TLSv1_1}}. The injected
> constructor wraps the context as-is and does *not* apply those options.
> Callers who need a TLS 1.2+ floor must configure protocol options on the
> {{SSL_CTX}} before wrapping. This is documented in Doxygen and matches the
> threat-model expectation that the operator/application owns TLS posture.
> *Tests:*
> Coverage is via {{SecurityTest}} ({{wrapped_ssl_context}},
> {{wrapped_ssl_context_null}}) on standard OpenSSL CI builds. Optional
> provider-specific integration tests were intentionally not included in the
> upstream patch.
> h2. Scope
> *In scope:*
> * {{explicit SSLContext(SSL_CTX* ctx)}} wrapper constructor
> * Documentation in lib/cpp/README.md
> * SecurityTest coverage for wrapped and null contexts
> *Out of scope / removed from earlier draft:*
> * Provider-specific build flags (e.g. WITH_TONGSUO)
> * Protocol enums or dual-certificate helper APIs in libthrift
> * Optional local integration tests tied to a specific TLS provider
> h2. Acceptance criteria
> * SSLContext can be constructed from an application-provided SSL_CTX*
> * Ownership of the injected SSL_CTX is transferred to SSLContext; destructor
> frees the context
> * Null SSL_CTX is rejected with a clear error
> * Existing default OpenSSL / SSLProtocol code path is unchanged
> * SecurityTest passes on standard OpenSSL CI builds (wrapped_ssl_context,
> wrapped_ssl_context_null)
> * Threat-model cross-check recorded (see below)
> h2. Threat model cross-check (AGENTS.md §6)
> Cross-checked against doc/thrift-threat-model.md:
> * §3 / Q4 — Thrift is not secure-by-default; TLS posture remains
> operator/application responsibility. Injection does not change this.
> * §3 / Q6 — Cryptographic guarantees come from the underlying TLS library;
> Thrift wraps SSL_CTX only.
> * §8 P7 — TLS properties depend on correct SSL_CTX configuration by the
> operator/application.
> * Trust boundaries — No new trust boundary; TSSLSocket / AccessManager /
> factory lifetime contracts unchanged.
> * Application responsibility — Injected path does not apply SSL_OP_NO_TLSv1*;
> callers configure a version floor if needed.
> * doc/thrift-threat-model.md — No update required.
> h2. References
> * GitHub PR: https://github.com/apache/thrift/pull/3606
> * Motivation: enable advanced TLS configuration in application code without
> maintaining a private transport fork
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
