Enable TLS Client Hello extension permutation by default

similar to the previous change for DTLS. This affects native TURN/TLS
connections which are already using this in Chromium.

BUG=webrtc:422225803

Change-Id: I605f106371f2dbe23b1ad5f8385e0e01abe7c48f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/357903
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42688}
This commit is contained in:
Philipp Hancke 2024-07-29 09:03:53 -07:00 committed by WebRTC LUCI CQ
parent e8a3380ca0
commit e13945bf07
5 changed files with 33 additions and 12 deletions

View File

@ -140,8 +140,8 @@ void TurnServer::AcceptConnection(rtc::Socket* server_socket) {
if (accepted_socket != NULL) { if (accepted_socket != NULL) {
const ServerSocketInfo& info = server_listen_sockets_[server_socket]; const ServerSocketInfo& info = server_listen_sockets_[server_socket];
if (info.ssl_adapter_factory) { if (info.ssl_adapter_factory) {
rtc::SSLAdapter* ssl_adapter = rtc::SSLAdapter* ssl_adapter = info.ssl_adapter_factory->CreateAdapter(
info.ssl_adapter_factory->CreateAdapter(accepted_socket); accepted_socket, /*permute_extensions=*/true);
ssl_adapter->StartSSL(""); ssl_adapter->StartSSL("");
accepted_socket = ssl_adapter; accepted_socket = ssl_adapter;
} }

View File

@ -47,6 +47,7 @@
#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/str_join.h"
#include "rtc_base/strings/string_builder.h" #include "rtc_base/strings/string_builder.h"
#include "rtc_base/thread.h" #include "rtc_base/thread.h"
#include "system_wrappers/include/field_trial.h"
////////////////////////////////////////////////////////////////////// //////////////////////////////////////////////////////////////////////
// SocketBIO // SocketBIO
@ -196,6 +197,10 @@ OpenSSLAdapter::OpenSSLAdapter(Socket* socket,
ssl_ctx_(nullptr), ssl_ctx_(nullptr),
ssl_mode_(SSL_MODE_TLS), ssl_mode_(SSL_MODE_TLS),
ignore_bad_cert_(false), ignore_bad_cert_(false),
#ifdef OPENSSL_IS_BORINGSSL
permute_extension_(
!webrtc::field_trial::IsDisabled("WebRTC-PermuteTlsClientHello")),
#endif
custom_cert_verifier_status_(false) { custom_cert_verifier_status_(false) {
// If a factory is used, take a reference on the factory's SSL_CTX. // If a factory is used, take a reference on the factory's SSL_CTX.
// Otherwise, we'll create our own later. // Otherwise, we'll create our own later.
@ -283,7 +288,7 @@ int OpenSSLAdapter::BeginSSL() {
// need to create one, and specify `false` to disable session caching. // need to create one, and specify `false` to disable session caching.
if (ssl_session_cache_ == nullptr) { if (ssl_session_cache_ == nullptr) {
RTC_DCHECK(!ssl_ctx_); RTC_DCHECK(!ssl_ctx_);
ssl_ctx_ = CreateContext(ssl_mode_, false); ssl_ctx_ = CreateContext(ssl_mode_, false, permute_extension_);
} }
if (!ssl_ctx_) { if (!ssl_ctx_) {
@ -949,7 +954,9 @@ int OpenSSLAdapter::NewSSLSessionCallback(SSL* ssl, SSL_SESSION* session) {
return 1; // We've taken ownership of the session; OpenSSL shouldn't free it. return 1; // We've taken ownership of the session; OpenSSL shouldn't free it.
} }
SSL_CTX* OpenSSLAdapter::CreateContext(SSLMode mode, bool enable_cache) { SSL_CTX* OpenSSLAdapter::CreateContext(SSLMode mode,
bool enable_cache,
bool permute_extension) {
#ifdef WEBRTC_USE_CRYPTO_BUFFER_CALLBACK #ifdef WEBRTC_USE_CRYPTO_BUFFER_CALLBACK
// If X509 objects aren't used, we can use these methods to avoid // If X509 objects aren't used, we can use these methods to avoid
// linking the sizable crypto/x509 code. // linking the sizable crypto/x509 code.
@ -1011,6 +1018,9 @@ SSL_CTX* OpenSSLAdapter::CreateContext(SSLMode mode, bool enable_cache) {
SSL_CTX_sess_set_new_cb(ctx, &OpenSSLAdapter::NewSSLSessionCallback); SSL_CTX_sess_set_new_cb(ctx, &OpenSSLAdapter::NewSSLSessionCallback);
} }
#ifdef OPENSSL_IS_BORINGSSL
SSL_CTX_set_permute_extensions(ctx, permute_extension);
#endif
return ctx; return ctx;
} }
@ -1069,9 +1079,11 @@ void OpenSSLAdapterFactory::SetIgnoreBadCert(bool ignore) {
ignore_bad_cert_ = ignore; ignore_bad_cert_ = ignore;
} }
OpenSSLAdapter* OpenSSLAdapterFactory::CreateAdapter(Socket* socket) { OpenSSLAdapter* OpenSSLAdapterFactory::CreateAdapter(Socket* socket,
bool permute_extension) {
if (ssl_session_cache_ == nullptr) { if (ssl_session_cache_ == nullptr) {
SSL_CTX* ssl_ctx = OpenSSLAdapter::CreateContext(ssl_mode_, true); SSL_CTX* ssl_ctx =
OpenSSLAdapter::CreateContext(ssl_mode_, true, permute_extension);
if (ssl_ctx == nullptr) { if (ssl_ctx == nullptr) {
return nullptr; return nullptr;
} }

View File

@ -78,7 +78,9 @@ class OpenSSLAdapter final : public SSLAdapter {
// OpenSSLAdapterFactory will call this method to create its own internal // OpenSSLAdapterFactory will call this method to create its own internal
// SSL_CTX, and OpenSSLAdapter will also call this when used without a // SSL_CTX, and OpenSSLAdapter will also call this when used without a
// factory. // factory.
static SSL_CTX* CreateContext(SSLMode mode, bool enable_cache); static SSL_CTX* CreateContext(SSLMode mode,
bool enable_cache,
bool permute_extension);
protected: protected:
void OnConnectEvent(Socket* socket) override; void OnConnectEvent(Socket* socket) override;
@ -170,6 +172,9 @@ class OpenSSLAdapter final : public SSLAdapter {
std::vector<std::string> alpn_protocols_; std::vector<std::string> alpn_protocols_;
// List of elliptic curves to be used in the TLS elliptic curves extension. // List of elliptic curves to be used in the TLS elliptic curves extension.
std::vector<std::string> elliptic_curves_; std::vector<std::string> elliptic_curves_;
#ifdef OPENSSL_IS_BORINGSSL
const bool permute_extension_;
#endif
// Holds the result of the call to run of the ssl_cert_verify_->Verify() // Holds the result of the call to run of the ssl_cert_verify_->Verify()
bool custom_cert_verifier_status_; bool custom_cert_verifier_status_;
// Flag to cancel pending timeout task. // Flag to cancel pending timeout task.
@ -206,7 +211,8 @@ class OpenSSLAdapterFactory : public SSLAdapterFactory {
// Constructs a new socket using the shared OpenSSLSessionCache. This means // Constructs a new socket using the shared OpenSSLSessionCache. This means
// existing SSLSessions already in the cache will be reused instead of // existing SSLSessions already in the cache will be reused instead of
// re-created for improved performance. // re-created for improved performance.
OpenSSLAdapter* CreateAdapter(Socket* socket) override; OpenSSLAdapter* CreateAdapter(Socket* socket,
bool permute_extensions) override;
private: private:
// Holds the SSLMode (DTLS,TLS) that will be used to set the session cache. // Holds the SSLMode (DTLS,TLS) that will be used to set the session cache.
@ -219,7 +225,7 @@ class OpenSSLAdapterFactory : public SSLAdapterFactory {
// Holds a cache of existing SSL Sessions. // Holds a cache of existing SSL Sessions.
std::unique_ptr<OpenSSLSessionCache> ssl_session_cache_; std::unique_ptr<OpenSSLSessionCache> ssl_session_cache_;
// Provides an optional custom callback for verifying SSL certificates, this // Provides an optional custom callback for verifying SSL certificates, this
// in currently only used for TLS-TURN connections. // in currently only used for TURN/TLS connections.
SSLCertificateVerifier* ssl_cert_verifier_ = nullptr; SSLCertificateVerifier* ssl_cert_verifier_ = nullptr;
// TODO(benwright): Remove this when context is moved to OpenSSLCommon. // TODO(benwright): Remove this when context is moved to OpenSSLCommon.
// Hold a friend class to the OpenSSLAdapter to retrieve the context. // Hold a friend class to the OpenSSLAdapter to retrieve the context.

View File

@ -22,6 +22,8 @@
namespace rtc { namespace rtc {
namespace { namespace {
constexpr bool kPermuteExtensions = true;
class MockAsyncSocket : public Socket { class MockAsyncSocket : public Socket {
public: public:
virtual ~MockAsyncSocket() = default; virtual ~MockAsyncSocket() = default;
@ -96,7 +98,7 @@ TEST(OpenSSLAdapterFactoryTest, CreateSingleOpenSSLAdapter) {
OpenSSLAdapterFactory adapter_factory; OpenSSLAdapterFactory adapter_factory;
Socket* async_socket = new MockAsyncSocket(); Socket* async_socket = new MockAsyncSocket();
auto simple_adapter = std::unique_ptr<OpenSSLAdapter>( auto simple_adapter = std::unique_ptr<OpenSSLAdapter>(
adapter_factory.CreateAdapter(async_socket)); adapter_factory.CreateAdapter(async_socket, kPermuteExtensions));
EXPECT_NE(simple_adapter, nullptr); EXPECT_NE(simple_adapter, nullptr);
} }
@ -112,7 +114,7 @@ TEST(OpenSSLAdapterFactoryTest, CreateWorksWithCustomVerifier) {
adapter_factory.SetCertVerifier(cert_verifier.get()); adapter_factory.SetCertVerifier(cert_verifier.get());
Socket* async_socket = new MockAsyncSocket(); Socket* async_socket = new MockAsyncSocket();
auto simple_adapter = std::unique_ptr<OpenSSLAdapter>( auto simple_adapter = std::unique_ptr<OpenSSLAdapter>(
adapter_factory.CreateAdapter(async_socket)); adapter_factory.CreateAdapter(async_socket, kPermuteExtensions));
EXPECT_NE(simple_adapter, nullptr); EXPECT_NE(simple_adapter, nullptr);
} }

View File

@ -52,7 +52,8 @@ class SSLAdapterFactory {
virtual void SetIgnoreBadCert(bool ignore) = 0; virtual void SetIgnoreBadCert(bool ignore) = 0;
// Creates a new SSL adapter, but from a shared context. // Creates a new SSL adapter, but from a shared context.
virtual SSLAdapter* CreateAdapter(Socket* socket) = 0; virtual SSLAdapter* CreateAdapter(Socket* socket,
bool permute_extensions) = 0;
static std::unique_ptr<SSLAdapterFactory> Create(); static std::unique_ptr<SSLAdapterFactory> Create();
}; };