Revert "Disable DTLS 1.0, TLS 1.0 and TLS 1.1 downgrade in WebRTC."
This reverts commit 7276b974b78ea4f409d8738b1b6f1515f7a8968e.
Reason for revert: Changing to a later Chrome release.
Original change's description:
> Disable DTLS 1.0, TLS 1.0 and TLS 1.1 downgrade in WebRTC.
>
> This change disables DTLS 1.0, TLS 1.0 and TLS 1.1 in WebRTC by default. This
> is part of a larger effort at Google to remove old TLS protocols:
> https://security.googleblog.com/2018/10/modernizing-transport-security.html
>
> For the M74 timeline I have added a disabled by default field trial
> WebRTC-LegacyTlsProtocols which can be enabled to support these cipher suites
> as consumers move away from these legacy cipher protocols but it will be off
> in Chrome.
>
> This is compliant with the webrtc-security-arch specification which states:
>
> All Implementations MUST implement DTLS 1.2 with the
> TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 cipher suite and the P-256
> curve [FIPS186]. Earlier drafts of this specification required DTLS
> 1.0 with the cipher suite TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, and
> at the time of this writing some implementations do not support DTLS
> 1.2; endpoints which support only DTLS 1.2 might encounter
> interoperability issues. The DTLS-SRTP protection profile
> SRTP_AES128_CM_HMAC_SHA1_80 MUST be supported for SRTP.
> Implementations MUST favor cipher suites which support (Perfect
> Forward Secrecy) PFS over non-PFS cipher suites and SHOULD favor AEAD
> over non-AEAD cipher suites.
>
> Bug: webrtc:10261
> Change-Id: I847c567592911cc437f095376ad67585b4355fc0
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/125141
> Commit-Queue: Benjamin Wright <benwright@webrtc.org>
> Reviewed-by: David Benjamin <davidben@webrtc.org>
> Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#27006}
TBR=steveanton@webrtc.org,davidben@webrtc.org,qingsi@webrtc.org,benwright@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:10261
Change-Id: I34727e65c069e1fb2ad71838828ad0a22b5fe811
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130367
Commit-Queue: Benjamin Wright <benwright@webrtc.org>
Reviewed-by: Benjamin Wright <benwright@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27403}diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 4468207..1d9221f 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -784,7 +784,6 @@
":stringutils",
"../api:array_view",
"../api:scoped_refptr",
- "../system_wrappers:field_trial",
"network:sent_packet",
"system:file_wrapper",
"third_party/base64",
@@ -1402,7 +1401,6 @@
":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 5131b30..e80efd1 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 @@
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 @@
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 bca2fde..40d1779 100644
--- a/rtc_base/openssl_stream_adapter.h
+++ b/rtc_base/openssl_stream_adapter.h
@@ -217,9 +217,6 @@
// 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 99345ac..04d0fc5 100644
--- a/rtc_base/ssl_stream_adapter.h
+++ b/rtc_base/ssl_stream_adapter.h
@@ -90,11 +90,6 @@
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 585d080..7319c01 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 @@
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 @@
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 @@
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 @@
bool dtls_;
int handshake_wait_;
bool identities_set_;
- bool legacy_tls_protocols_;
};
class SSLStreamAdapterTestTLS
@@ -664,7 +656,6 @@
: SSLStreamAdapterTestBase("",
"",
false,
- false,
::testing::get<0>(GetParam()),
::testing::get<1>(GetParam())),
client_buffer_(kFifoBufferSize),
@@ -781,13 +772,9 @@
public WithParamInterface<tuple<rtc::KeyParams, rtc::KeyParams>> {
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 @@
}
};
-// 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 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();
@@ -1421,64 +1401,6 @@
// 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) {
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12);
TestHandshake();
@@ -1496,10 +1418,9 @@
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 @@
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(
