Remove support for legacy TLS protocols by default
Now this requires explicitly passing the
WebRTC-LegacyTlsProtocols/Enabled/ field trial flag or an override.
Bug: webrtc:10261
Change-Id: Ib880bcc50cec0a21dcaa4784c228cacb020e5568
NOKEYCHECK: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190282
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32488}
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc
index 5790b1b..f59b4ed 100644
--- a/rtc_base/openssl_stream_adapter.cc
+++ b/rtc_base/openssl_stream_adapter.cc
@@ -278,7 +278,7 @@
bool ShouldAllowLegacyTLSProtocols() {
return g_use_legacy_tls_protocols_override.load()
? g_allow_legacy_tls_protocols.load()
- : !webrtc::field_trial::IsDisabled("WebRTC-LegacyTlsProtocols");
+ : webrtc::field_trial::IsEnabled("WebRTC-LegacyTlsProtocols");
}
OpenSSLStreamAdapter::OpenSSLStreamAdapter(
diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h
index 3da0b09..7bff726 100644
--- a/rtc_base/ssl_stream_adapter.h
+++ b/rtc_base/ssl_stream_adapter.h
@@ -93,11 +93,11 @@
enum SSLRole { SSL_CLIENT, SSL_SERVER };
enum SSLMode { SSL_MODE_TLS, SSL_MODE_DTLS };
-// Note: TLS_10, TLS_11, and DTLS_10 will all be ignored, and only
-// DTLS1_2 will be accepted, if the trial flag
-// WebRTC-LegacyTlsProtocols/Disabled/ is passed in. Support for these
-// protocol versions will be completely removed in M84 or later.
-// TODO(https://bugs.webrtc.org/10261).
+// Note: TLS_10, TLS_11, and DTLS_10 will all be ignored, and only DTLS1_2 will
+// be accepted unless the trial flag WebRTC-LegacyTlsProtocols/Enabled/ is
+// passed in or an explicit override is used. Support for the legacy protocol
+// versions will be completely removed in the future.
+// See https://bugs.webrtc.org/10261.
enum SSLProtocolVersion {
SSL_PROTOCOL_NOT_GIVEN = -1,
SSL_PROTOCOL_TLS_10 = 0,
diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc
index 1ba2f3e..379acac 100644
--- a/rtc_base/ssl_stream_adapter_unittest.cc
+++ b/rtc_base/ssl_stream_adapter_unittest.cc
@@ -1458,10 +1458,9 @@
}
// Test getting the used DTLS ciphers.
-// DTLS 1.0 is max version for client and server, this will only work if
-// legacy is enabled.
+// DTLS 1.2 is max version for client and server.
TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) {
- SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10);
+ SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12);
TestHandshake();
int client_cipher;
@@ -1469,8 +1468,8 @@
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(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(
@@ -1529,7 +1528,7 @@
}
void ConfigureServer(std::string experiment) {
- // webrtc::test::ScopedFieldTrials trial(experiment);
+ webrtc::test::ScopedFieldTrials trial(experiment);
server_stream_ =
new SSLDummyStreamDTLS(this, "s2c", &server_buffer_, &client_buffer_);
server_ssl_ =
@@ -1545,8 +1544,8 @@
// Test getting the used DTLS ciphers.
// DTLS 1.2 enabled for neither client nor server -> DTLS 1.0 will be used.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, TestGetSslCipherSuite) {
- ConfigureClient("");
- ConfigureServer("");
+ ConfigureClient("WebRTC-LegacyTlsProtocols/Enabled/");
+ ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10);
TestHandshake();
@@ -1584,8 +1583,8 @@
// DTLS 1.2 enabled for client only -> DTLS 1.0 will be used.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols,
TestGetSslCipherSuiteDtls12Client) {
- ConfigureClient("");
- ConfigureServer("");
+ ConfigureClient("WebRTC-LegacyTlsProtocols/Enabled/");
+ ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12);
TestHandshake();
@@ -1603,8 +1602,8 @@
// DTLS 1.2 enabled for server only -> DTLS 1.0 will be used.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols,
TestGetSslCipherSuiteDtls12Server) {
- ConfigureClient("");
- ConfigureServer("");
+ ConfigureClient("WebRTC-LegacyTlsProtocols/Enabled/");
+ ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10);
TestHandshake();
@@ -1623,8 +1622,8 @@
// This is meant to cause a failure.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols,
TestGetSslVersionLegacyDisabledServer10) {
- ConfigureClient("WebRTC-LegacyTlsProtocols/Disabled/");
- ConfigureServer("");
+ ConfigureClient("");
+ ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12);
// Handshake should fail.
TestHandshake(false);
@@ -1634,8 +1633,8 @@
// DTLS 1.2. This should work.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols,
TestGetSslVersionLegacyDisabledServer12) {
- ConfigureClient("WebRTC-LegacyTlsProtocols/Disabled/");
- ConfigureServer("WebRTC-LegacyTlsProtocols/Disabled/");
+ ConfigureClient("");
+ ConfigureServer("");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12);
TestHandshake();
}
@@ -1650,12 +1649,12 @@
TestHandshake();
}
-// Legacy protocols are disabled, max TLS version is 1.0
+// Legacy protocols are disabled in the client, max TLS version is 1.0
// This should be a configuration error, and handshake should fail.
TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols,
TestGetSslVersionLegacyDisabledClient10Server10) {
- ConfigureClient("WebRTC-LegacyTlsProtocols/Disabled/");
- ConfigureServer("WebRTC-LegacyTlsProtocols/Disabled/");
+ ConfigureClient("");
+ ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/");
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10);
TestHandshake(false);
}