Revert "Enable TLS Client Hello extension permutation by default"

This reverts commit e13945bf0761d34b902ecbd4e1cc6deb1788a2c9.

Reason for revert: Breaks downstream project

Original change's description:
> 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}

Bug: webrtc:422225803
Change-Id: I8020e420e270c0f47cb8e26a210c801e94f8de7d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/357883
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42689}
This commit is contained in:
Mirko Bonadei 2024-07-30 07:55:02 +00:00 committed by WebRTC LUCI CQ
parent e13945bf07
commit dfa7b2b425
5 changed files with 12 additions and 33 deletions

View File

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

View File

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

View File

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

View File

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

View File

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