DTLS: cleanup extension permutation

which shipped in M129.

BUG=webrtc:42225803

Change-Id: I5021c7878069a1cd0eafd078b73fa57c5b9b2155
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364360
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#43313}
This commit is contained in:
Philipp Hancke 2024-10-24 11:30:02 +02:00 committed by WebRTC LUCI CQ
parent 440ef02505
commit 0e5d73510d
9 changed files with 13 additions and 111 deletions

View File

@ -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)),

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

@ -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;
}

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,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.

View File

@ -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<OpenSSLAdapter>(
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<OpenSSLAdapter>(
adapter_factory.CreateAdapter(async_socket, kPermuteExtensions));
adapter_factory.CreateAdapter(async_socket));
EXPECT_NE(simple_adapter, nullptr);
}

View File

@ -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_) {

View File

@ -224,10 +224,6 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter {
// Our key and certificate.
#ifdef OPENSSL_IS_BORINGSSL
std::unique_ptr<BoringSSLIdentity> 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<OpenSSLIdentity> identity_;
#endif

View File

@ -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<SSLAdapterFactory> Create();
};

View File

@ -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