Reland "Enable TLS Client Hello extension permutation by default"

This is a reland of commit e13945bf0761d34b902ecbd4e1cc6deb1788a2c9
with additional backward compability defaulting to the new value.

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: Ic194e4f763029e65c1a15a6bbaabcfbcd2866eac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/358120
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42702}
This commit is contained in:
Philipp Hancke 2024-07-30 09:03:49 -07:00 committed by WebRTC LUCI CQ
parent c51a5c00c4
commit b5ff55adee
5 changed files with 40 additions and 12 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);
rtc::SSLAdapter* ssl_adapter = info.ssl_adapter_factory->CreateAdapter(
accepted_socket, /*permute_extensions=*/true);
ssl_adapter->StartSSL("");
accepted_socket = ssl_adapter;
}

View File

@ -47,6 +47,7 @@
#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
@ -196,6 +197,10 @@ 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.
@ -283,7 +288,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);
ssl_ctx_ = CreateContext(ssl_mode_, false, permute_extension_);
}
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.
}
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
// If X509 objects aren't used, we can use these methods to avoid
// 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);
}
#ifdef OPENSSL_IS_BORINGSSL
SSL_CTX_set_permute_extensions(ctx, permute_extension);
#endif
return ctx;
}
@ -1069,9 +1079,11 @@ void OpenSSLAdapterFactory::SetIgnoreBadCert(bool ignore) {
ignore_bad_cert_ = ignore;
}
OpenSSLAdapter* OpenSSLAdapterFactory::CreateAdapter(Socket* socket) {
OpenSSLAdapter* OpenSSLAdapterFactory::CreateAdapter(Socket* socket,
bool permute_extension) {
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) {
return nullptr;
}

View File

@ -78,7 +78,9 @@ 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);
static SSL_CTX* CreateContext(SSLMode mode,
bool enable_cache,
bool permute_extension);
protected:
void OnConnectEvent(Socket* socket) override;
@ -170,6 +172,9 @@ 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.
@ -206,7 +211,11 @@ 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) override;
OpenSSLAdapter* CreateAdapter(Socket* socket,
bool permute_extensions) override;
OpenSSLAdapter* CreateAdapter(Socket* socket) override {
return CreateAdapter(socket, /*permute_extensions=*/true);
}
private:
// Holds the SSLMode (DTLS,TLS) that will be used to set the session cache.
@ -219,7 +228,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 TLS-TURN connections.
// in currently only used for TURN/TLS 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,6 +22,8 @@
namespace rtc {
namespace {
constexpr bool kPermuteExtensions = true;
class MockAsyncSocket : public Socket {
public:
virtual ~MockAsyncSocket() = default;
@ -96,7 +98,7 @@ TEST(OpenSSLAdapterFactoryTest, CreateSingleOpenSSLAdapter) {
OpenSSLAdapterFactory adapter_factory;
Socket* async_socket = new MockAsyncSocket();
auto simple_adapter = std::unique_ptr<OpenSSLAdapter>(
adapter_factory.CreateAdapter(async_socket));
adapter_factory.CreateAdapter(async_socket, kPermuteExtensions));
EXPECT_NE(simple_adapter, nullptr);
}
@ -112,7 +114,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));
adapter_factory.CreateAdapter(async_socket, kPermuteExtensions));
EXPECT_NE(simple_adapter, nullptr);
}

View File

@ -52,7 +52,12 @@ class SSLAdapterFactory {
virtual void SetIgnoreBadCert(bool ignore) = 0;
// 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;
[[deprecated(
"Use CreateAdapter(socket, permute_extensions) "
"instead")]] virtual SSLAdapter*
CreateAdapter(Socket* socket) = 0;
static std::unique_ptr<SSLAdapterFactory> Create();
};