diff --git a/experiments/field_trials.py b/experiments/field_trials.py index cecca11c5b..a11d0c4f45 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -110,9 +110,6 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-Pacer-KeyframeFlushing', 42221435, date(2024, 4, 1)), - FieldTrial('WebRTC-PermuteTlsClientHello', - 42225803, - date(2025, 1, 1)), FieldTrial('WebRTC-DisableTlsSessionTicketKillswitch', 367181089, date(2025, 7, 1)), diff --git a/p2p/base/turn_server.cc b/p2p/base/turn_server.cc index 2ab65283c0..4fd899ee2d 100644 --- a/p2p/base/turn_server.cc +++ b/p2p/base/turn_server.cc @@ -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; } diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc index 3517ed8181..ffafab2c1f 100644 --- a/rtc_base/openssl_adapter.cc +++ b/rtc_base/openssl_adapter.cc @@ -213,10 +213,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. @@ -304,13 +300,7 @@ int OpenSSLAdapter::BeginSSL() { // need to create one, and specify `false` to disable session caching. if (ssl_session_cache_ == nullptr) { RTC_DCHECK(!ssl_ctx_); -#ifdef OPENSSL_IS_BORINGSSL - ssl_ctx_ = - CreateContext(ssl_mode_, /* enable_cache= */ false, permute_extension_); -#else - ssl_ctx_ = CreateContext(ssl_mode_, /* enable_cache= */ false, - /* permute_extension= */ false); -#endif + ssl_ctx_ = CreateContext(ssl_mode_, /* enable_cache= */ false); } if (!ssl_ctx_) { @@ -945,9 +935,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. @@ -1006,7 +994,7 @@ SSL_CTX* OpenSSLAdapter::CreateContext(SSLMode mode, } #ifdef OPENSSL_IS_BORINGSSL - SSL_CTX_set_permute_extensions(ctx, permute_extension); + SSL_CTX_set_permute_extensions(ctx, true); #endif return ctx; } @@ -1066,11 +1054,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; } diff --git a/rtc_base/openssl_adapter.h b/rtc_base/openssl_adapter.h index 61b727c359..246c954d99 100644 --- a/rtc_base/openssl_adapter.h +++ b/rtc_base/openssl_adapter.h @@ -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 alpn_protocols_; // List of elliptic curves to be used in the TLS elliptic curves extension. std::vector 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,11 +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 { - return CreateAdapter(socket, /*permute_extensions=*/true); - } + OpenSSLAdapter* CreateAdapter(Socket* socket) override; private: // Holds the SSLMode (DTLS,TLS) that will be used to set the session cache. diff --git a/rtc_base/openssl_adapter_unittest.cc b/rtc_base/openssl_adapter_unittest.cc index 2dfe34b560..5783a87e7f 100644 --- a/rtc_base/openssl_adapter_unittest.cc +++ b/rtc_base/openssl_adapter_unittest.cc @@ -27,8 +27,6 @@ namespace rtc { namespace { -constexpr bool kPermuteExtensions = true; - class MockAsyncSocket : public Socket { public: virtual ~MockAsyncSocket() = default; @@ -103,7 +101,7 @@ TEST(OpenSSLAdapterFactoryTest, CreateSingleOpenSSLAdapter) { OpenSSLAdapterFactory adapter_factory; Socket* async_socket = new MockAsyncSocket(); auto simple_adapter = std::unique_ptr( - adapter_factory.CreateAdapter(async_socket, kPermuteExtensions)); + adapter_factory.CreateAdapter(async_socket)); EXPECT_NE(simple_adapter, nullptr); } @@ -119,7 +117,7 @@ TEST(OpenSSLAdapterFactoryTest, CreateWorksWithCustomVerifier) { adapter_factory.SetCertVerifier(cert_verifier.get()); Socket* async_socket = new MockAsyncSocket(); auto simple_adapter = std::unique_ptr( - adapter_factory.CreateAdapter(async_socket, kPermuteExtensions)); + adapter_factory.CreateAdapter(async_socket)); EXPECT_NE(simple_adapter, nullptr); } diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index 4ee85684f7..0593f68f2e 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -239,10 +239,6 @@ OpenSSLStreamAdapter::OpenSSLStreamAdapter( ssl_write_needs_read_(false), ssl_(nullptr), ssl_ctx_(nullptr), -#ifdef OPENSSL_IS_BORINGSSL - permute_extension_( - !webrtc::field_trial::IsDisabled("WebRTC-PermuteTlsClientHello")), -#endif ssl_mode_(SSL_MODE_DTLS), ssl_max_version_(SSL_PROTOCOL_DTLS_12), disable_handshake_ticket_(!webrtc::field_trial::IsDisabled( @@ -1025,7 +1021,7 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() { } #ifdef OPENSSL_IS_BORINGSSL - SSL_CTX_set_permute_extensions(ctx, permute_extension_); + SSL_CTX_set_permute_extensions(ctx, true); #endif if (disable_handshake_ticket_) { diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index a664d082f3..0cf79719cf 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -224,10 +224,6 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { // Our key and certificate. #ifdef OPENSSL_IS_BORINGSSL std::unique_ptr identity_; - // We check and store the `WebRTC-PermuteTlsClientHello` field trial config in - // the constructor for convenience to allow tests to apply different - // configurations across instances. - const bool permute_extension_; #else std::unique_ptr identity_; #endif diff --git a/rtc_base/ssl_adapter.h b/rtc_base/ssl_adapter.h index 5e02a2602d..58ad237f67 100644 --- a/rtc_base/ssl_adapter.h +++ b/rtc_base/ssl_adapter.h @@ -52,12 +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; - [[deprecated( - "Use CreateAdapter(socket, permute_extensions) " - "instead")]] virtual SSLAdapter* - CreateAdapter(Socket* socket) = 0; + virtual SSLAdapter* CreateAdapter(Socket* socket) = 0; static std::unique_ptr Create(); }; diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc index 2fd9c92043..c937eed1d1 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -1479,60 +1479,3 @@ TEST_F(SSLStreamAdapterTestDTLS, TestGetSslVersionBytes) { EXPECT_EQ(server_version, kDtls1_2); } -// Tests for enabling the (D)TLS extension permutation which randomizes the -// order of extensions in the client hello. -// These tests are a no-op under OpenSSL. -#ifdef OPENSSL_IS_BORINGSSL -class SSLStreamAdapterTestDTLSExtensionPermutation - : public SSLStreamAdapterTestDTLSBase { - public: - SSLStreamAdapterTestDTLSExtensionPermutation() - : SSLStreamAdapterTestDTLSBase( - rtc::KeyParams::ECDSA(rtc::EC_NIST_P256), - rtc::KeyParams::ECDSA(rtc::EC_NIST_P256), - std::make_pair(rtc::DIGEST_SHA_256, SHA256_DIGEST_LENGTH)) {} - - void Initialize(absl::string_view client_experiment, - absl::string_view server_experiment) { - InitializeClientAndServerStreams(client_experiment, server_experiment); - client_ssl_->SetIdentity( - rtc::SSLIdentity::Create("client", client_key_type_)); - server_ssl_->SetIdentity( - rtc::SSLIdentity::Create("server", server_key_type_)); - } -}; - -TEST_F(SSLStreamAdapterTestDTLSExtensionPermutation, - ClientDefaultServerDefault) { - Initialize("", ""); - TestHandshake(); -} - -TEST_F(SSLStreamAdapterTestDTLSExtensionPermutation, - ClientDisabledServerDisabled) { - Initialize("WebRTC-PermuteTlsClientHello/Disabled/", - "WebRTC-PermuteTlsClientHello/Disabled/"); - TestHandshake(); -} - -TEST_F(SSLStreamAdapterTestDTLSExtensionPermutation, - ClientDisabledServerPermute) { - Initialize("WebRTC-PermuteTlsClientHello/Disabled/", - "WebRTC-PermuteTlsClientHello/Enabled/"); - TestHandshake(); -} - -TEST_F(SSLStreamAdapterTestDTLSExtensionPermutation, - ClientPermuteServerDisabled) { - Initialize("WebRTC-PermuteTlsClientHello/Enabled/", - "WebRTC-PermuteTlsClientHello/Disabled/"); - TestHandshake(); -} - -TEST_F(SSLStreamAdapterTestDTLSExtensionPermutation, - ClientPermuteServerPermute) { - Initialize("WebRTC-PermuteTlsClientHello/Enabled/", - "WebRTC-PermuteTlsClientHello/Enabled/"); - TestHandshake(); -} -#endif // OPENSSL_IS_BORINGSSL