THRIFT-3165: Disable TLSv1.0 and TLSv1.1 by default

[HTHou]

What changed

• Set the C++ TSSLSocketFactory default SSLTLS context to keep version-flexible negotiation while using TLS 1.2 as the default protocol floor.
• Add a custom SSLContext factory constructor so applications can adjust OpenSSL context options before socket creation when they need a different protocol range.
• Update the C++ SSL security tests and README note for the default behavior.

Validation

• PATH="/opt/homebrew/opt/bison/bin:$PATH" BOOST_ROOT=/opt/homebrew/opt/boost ./configure --with-boost=/opt/homebrew/opt/boost --with-boost-libdir=/opt/homebrew/opt/boost/lib --with-openssl=/opt/homebrew/opt/openssl@3
• PATH="/opt/homebrew/opt/bison/bin:$PATH" make -C lib/cpp CPPFLAGS="-I/opt/homebrew/opt/openssl@3/include"
• PATH="/opt/homebrew/opt/bison/bin:$PATH" make -C lib/cpp/test CPPFLAGS="-I/opt/homebrew/opt/openssl@3/include" BOOST_SYSTEM_LDADD= SecurityTest SecurityFromBufferTest
• cd lib/cpp/test && DYLD_LIBRARY_PATH="../.libs:.libs:$DYLD_LIBRARY_PATH" ./SecurityTest -- ../../../test/keys
• cd lib/cpp/test && DYLD_LIBRARY_PATH="../.libs:.libs:$DYLD_LIBRARY_PATH" ./SecurityFromBufferTest -- ../../../test/keys

The SSL matrix tests print expected shutdown messages for protocol combinations that do not complete negotiation.

AI assistance

Co-Authored-By: Codex noreply@openai.com

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates Thrift C++ SSL defaults to require TLS 1.2+ by default, while enabling applications to inject a custom SSLContext when different protocol ranges are needed.

Changes:

• Raise the default SSLTLS protocol floor by disabling TLS 1.0 and TLS 1.1 in SSLContext.
• Add TSSLSocketFactory constructor overload that accepts an SSLContext factory callback.
• Update SSL security matrix expectations, add targeted tests, and document the new default behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
lib/cpp/src/thrift/transport/TSSLSocket.cpp	Enforces TLS 1.2+ for SSLTLS and introduces factory-based SSLContext construction with safer init/cleanup handling.
lib/cpp/src/thrift/transport/TSSLSocket.h	Adds SSLContextFactory type + new TSSLSocketFactory ctor; updates protocol documentation.
lib/cpp/test/SecurityTest.cpp	Adds tests for default/custom SSL context options; updates expected protocol compatibility matrix.
lib/cpp/test/SecurityFromBufferTest.cpp	Updates expected protocol compatibility matrix for buffer-based SSL tests.
lib/cpp/README.md	Documents new default minimum TLS version and how to override via custom context factory.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Jens-G]

Code review

Found 2 issues:

1. ctx_.reset() is called outside the mutex in the destructor, regressing the fix from THRIFT-2225 (commit 301dfa94d). That commit explicitly placed ctx_.reset() inside Guard guard(mutex_) to prevent a race between concurrent destructor calls and createSocket() threads reading ctx_ without the lock. The refactor here splits the lock into cleanupOpenSSLState() but leaves the reset unguarded.

thrift/lib/cpp/src/thrift/transport/TSSLSocket.cpp

Lines 925 to 929 in 009a524

	TSSLSocketFactory::~TSSLSocketFactory() {
	ctx_.reset();
	cleanupOpenSSLState();
	}

2. The SSL_OP_NO_TLSv1 and SSL_OP_NO_TLSv1_1 assertions in the new test cases are not wrapped in #if guards, unlike the adjacent SSL_OP_NO_SSLv2 check on line 236. On OpenSSL 3.0+ these constants can evaluate to 0, making the unguarded BOOST_CHECK((options & SSL_OP_NO_TLSv1) != 0) assertions fail unconditionally even when the protocol floor is correctly set.

thrift/lib/cpp/test/SecurityTest.cpp

Lines 235 to 242 in 009a524

	#if SSL_OP_NO_SSLv2 != 0
	BOOST_CHECK((options & SSL_OP_NO_SSLv2) != 0);
	#endif
	BOOST_CHECK((options & SSL_OP_NO_SSLv3) != 0);
	BOOST_CHECK((options & SSL_OP_NO_TLSv1) != 0);
	BOOST_CHECK((options & SSL_OP_NO_TLSv1_1) != 0);
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[HTHou]

Thanks @Jens-G, addressed both in the latest push.

• Moved ctx_.reset() back under TSSLSocketFactory::mutex_ via cleanupOpenSSLState(), preserving the THRIFT-2225 locking/order.
• Made the SSL_OP_NO_* option assertions conditional at runtime when each option bit is observable, including the custom-context checks.

Validation run locally:

• git diff --check
• object compile of TSSLSocket.cpp and SecurityTest.cpp with OpenSSL 3 / Homebrew Boost

[Copilot]

Pull request overview

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

[HTHou]

Updated in the latest push. custom_ssl_context_options now explicitly resets the captured SSLContext before the test exits, so the extra shared reference is released before TSSLSocketFactory reaches its destructor/cleanup path.

[HTHou]

Updated in the latest push. The TSSLException message checks now compare against std::string(ex.what()), making the assertions explicit content comparisons.