diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 4468207edf..1d9221fa74 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -784,7 +784,6 @@ rtc_static_library("rtc_base") { ":stringutils", "../api:array_view", "../api:scoped_refptr", - "../system_wrappers:field_trial", "network:sent_packet", "system:file_wrapper", "third_party/base64", @@ -1402,7 +1401,6 @@ if (rtc_include_tests) { ":stringutils", ":testclient", "../api:array_view", - "../test:field_trial", "../test:fileutils", "../test:test_support", "memory:fifo_buffer", diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index 5131b30ef9..e80efd1ffd 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -37,7 +37,6 @@ #include "rtc_base/stream.h" #include "rtc_base/thread.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" #if (OPENSSL_VERSION_NUMBER < 0x10100000L) #error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP" @@ -275,9 +274,7 @@ OpenSSLStreamAdapter::OpenSSLStreamAdapter(StreamInterface* stream) ssl_(nullptr), ssl_ctx_(nullptr), ssl_mode_(SSL_MODE_TLS), - ssl_max_version_(SSL_PROTOCOL_TLS_12), - support_legacy_tls_protocols_flag_( - webrtc::field_trial::IsEnabled("WebRTC-LegacyTlsProtocols")) {} + ssl_max_version_(SSL_PROTOCOL_TLS_12) {} OpenSSLStreamAdapter::~OpenSSLStreamAdapter() { Cleanup(0); @@ -955,34 +952,25 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() { return nullptr; } - if (support_legacy_tls_protocols_flag_) { - // TODO(https://bugs.webrtc.org/10261): Completely remove this branch in - // M75. - SSL_CTX_set_min_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); - switch (ssl_max_version_) { - case SSL_PROTOCOL_TLS_10: - SSL_CTX_set_max_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); - break; - case SSL_PROTOCOL_TLS_11: - SSL_CTX_set_max_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_1_VERSION); - break; - case SSL_PROTOCOL_TLS_12: - default: - SSL_CTX_set_max_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION); - break; - } - } else { - // TODO(https://bugs.webrtc.org/10261): Make this the default in M75. - SSL_CTX_set_min_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION); - SSL_CTX_set_max_proto_version( - ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION); + // TODO(https://bugs.webrtc.org/10261): Evaluate and drop (D)TLS 1.0 and 1.1 + // support by default. + SSL_CTX_set_min_proto_version( + ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); + switch (ssl_max_version_) { + case SSL_PROTOCOL_TLS_10: + SSL_CTX_set_max_proto_version( + ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); + break; + case SSL_PROTOCOL_TLS_11: + SSL_CTX_set_max_proto_version( + ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_1_VERSION); + break; + case SSL_PROTOCOL_TLS_12: + default: + SSL_CTX_set_max_proto_version( + ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION); + break; } - #ifdef OPENSSL_IS_BORINGSSL // SSL_CTX_set_current_time_cb is only supported in BoringSSL. if (g_use_time_callback_for_testing) { diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h index bca2fde1e7..40d17795aa 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -217,9 +217,6 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter { // A 50-ms initial timeout ensures rapid setup on fast connections, but may // be too aggressive for low bandwidth links. int dtls_handshake_timeout_ms_ = 50; - - // TODO(https://bugs.webrtc.org/10261): Completely remove this option in M75. - const bool support_legacy_tls_protocols_flag_; }; ///////////////////////////////////////////////////////////////////////////// diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index 99345ac7a9..04d0fc5dd4 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -90,11 +90,6 @@ bool IsGcmCryptoSuiteName(const std::string& crypto_suite); enum SSLRole { SSL_CLIENT, SSL_SERVER }; enum SSLMode { SSL_MODE_TLS, SSL_MODE_DTLS }; - -// Note: By default TLS_10, TLS_11, and DTLS_10 will all be upgraded to DTLS1_2 -// unless the trial flag WebRTC-LegacyTlsProtocols/Enabled/ is passed in. These -// protocol versions will be completely removed in M75 -// TODO(https://bugs.webrtc.org/10261). enum SSLProtocolVersion { SSL_PROTOCOL_TLS_10, SSL_PROTOCOL_TLS_11, diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc index 585d080869..7319c015fc 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -24,12 +24,11 @@ #include "rtc_base/ssl_identity.h" #include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/stream.h" -#include "test/field_trial.h" +using ::testing::WithParamInterface; +using ::testing::Values; using ::testing::Combine; using ::testing::tuple; -using ::testing::Values; -using ::testing::WithParamInterface; static const int kBlockSize = 4096; static const char kExporterLabel[] = "label"; @@ -285,7 +284,6 @@ class SSLStreamAdapterTestBase : public testing::Test, const std::string& client_cert_pem, const std::string& client_private_key_pem, bool dtls, - bool legacy_tls_protocols = false, rtc::KeyParams client_key_type = rtc::KeyParams(rtc::KT_DEFAULT), rtc::KeyParams server_key_type = rtc::KeyParams(rtc::KT_DEFAULT)) : client_cert_pem_(client_cert_pem), @@ -303,8 +301,7 @@ class SSLStreamAdapterTestBase : public testing::Test, damage_(false), dtls_(dtls), handshake_wait_(5000), - identities_set_(false), - legacy_tls_protocols_(legacy_tls_protocols) { + identities_set_(false) { // Set use of the test RNG to get predictable loss patterns. rtc::SetRandomTestMode(true); } @@ -317,10 +314,6 @@ class SSLStreamAdapterTestBase : public testing::Test, void SetUp() override { CreateStreams(); - // Enable legacy protocols if required - webrtc::test::ScopedFieldTrials trial( - legacy_tls_protocols_ ? "WebRTC-LegacyTlsProtocols/Enabled/" : ""); - client_ssl_.reset(rtc::SSLStreamAdapter::Create(client_stream_)); server_ssl_.reset(rtc::SSLStreamAdapter::Create(server_stream_)); @@ -653,7 +646,6 @@ class SSLStreamAdapterTestBase : public testing::Test, bool dtls_; int handshake_wait_; bool identities_set_; - bool legacy_tls_protocols_; }; class SSLStreamAdapterTestTLS @@ -664,7 +656,6 @@ class SSLStreamAdapterTestTLS : SSLStreamAdapterTestBase("", "", false, - false, ::testing::get<0>(GetParam()), ::testing::get<1>(GetParam())), client_buffer_(kFifoBufferSize), @@ -781,13 +772,9 @@ class SSLStreamAdapterTestDTLS public WithParamInterface> { public: SSLStreamAdapterTestDTLS() - : SSLStreamAdapterTestDTLS(/*legacy_tls_protocols=*/false) {} - - SSLStreamAdapterTestDTLS(bool legacy_tls_protocols) : SSLStreamAdapterTestBase("", "", true, - legacy_tls_protocols, ::testing::get<0>(GetParam()), ::testing::get<1>(GetParam())), client_buffer_(kBufferCapacity, kDefaultBufferSize), @@ -959,13 +946,6 @@ class SSLStreamAdapterTestDTLSCertChain : public SSLStreamAdapterTestDTLS { } }; -// Enable legacy TLS protocols in DTLS. -class SSLStreamAdapterTestDTLSLegacyProtocols - : public SSLStreamAdapterTestDTLS { - public: - SSLStreamAdapterTestDTLSLegacyProtocols() - : SSLStreamAdapterTestDTLS(/*legacy_tls_protocols=*/true) {} -}; // Basic tests: TLS // Test that we can make a handshake work @@ -1402,7 +1382,7 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { // Test getting the used DTLS ciphers. // DTLS 1.2 enabled for neither client nor server -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, TestGetSslCipherSuite) { +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); TestHandshake(); @@ -1419,64 +1399,6 @@ TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, TestGetSslCipherSuite) { server_cipher, ::testing::get<1>(GetParam()).type())); } -// Test getting the used DTLS 1.2 ciphers. -// DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used. -TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, - TestGetSslCipherSuiteDtls12Both) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12); - TestHandshake(); - - int client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - int server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); - - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(true)); - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(false)); - - ASSERT_EQ(client_cipher, server_cipher); - ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher( - server_cipher, ::testing::get<1>(GetParam()).type())); -} - -// DTLS 1.2 enabled for client only -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, - TestGetSslCipherSuiteDtls12Client) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12); - TestHandshake(); - - int client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - int server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); - - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(true)); - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(false)); - - ASSERT_EQ(client_cipher, server_cipher); - ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher( - server_cipher, ::testing::get<1>(GetParam()).type())); -} - -// DTLS 1.2 enabled for server only -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, - TestGetSslCipherSuiteDtls12Server) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10); - TestHandshake(); - - int client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - int server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); - - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(true)); - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(false)); - - ASSERT_EQ(client_cipher, server_cipher); - ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher( - server_cipher, ::testing::get<1>(GetParam()).type())); -} - // Test getting the used DTLS 1.2 ciphers. // DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used. TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { @@ -1496,10 +1418,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { server_cipher, ::testing::get<1>(GetParam()).type())); } -// Test getting the used DTLS ciphers. -// DTLS 1.0 enabled for client and server, both will be upgraded to DTLS 1.2 -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); +// DTLS 1.2 enabled for client only -> DTLS 1.0 will be used. +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Client) { + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12); TestHandshake(); int client_cipher; @@ -1507,8 +1428,26 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { int server_cipher; ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(true)); - ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(false)); + ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(true)); + ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(false)); + + ASSERT_EQ(client_cipher, server_cipher); + ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher( + server_cipher, ::testing::get<1>(GetParam()).type())); +} + +// DTLS 1.2 enabled for server only -> DTLS 1.0 will be used. +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Server) { + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10); + TestHandshake(); + + int client_cipher; + ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); + int server_cipher; + ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); + + ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(true)); + ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(false)); ASSERT_EQ(client_cipher, server_cipher); ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher(