admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Disable DTLS 1.0, TLS 1.0 and TLS 1.1 downgrade in WebRTC."

Browse Source 
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}
This commit is contained in:
Benjamin Wright 2019-04-01 18:25:23 +00:00 committed by Commit Bot
parent ae4b62318d
commit af1f8655b2
5 changed files with 46 additions and 129 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
rtc_base/BUILD.gn
View File

@ -784,7 +784,6 @@ rtc_static_library("rtc_base") {
":stringutils", ":stringutils",
"../api:array_view", "../api:array_view",
"../api:scoped_refptr", "../api:scoped_refptr",
"../system_wrappers:field_trial",
"network:sent_packet", "network:sent_packet",
"system:file_wrapper", "system:file_wrapper",
"third_party/base64", "third_party/base64",
@ -1402,7 +1401,6 @@ if (rtc_include_tests) {
":stringutils", ":stringutils",
":testclient", ":testclient",
"../api:array_view", "../api:array_view",
"../test:field_trial",
"../test:fileutils", "../test:fileutils",
"../test:test_support", "../test:test_support",
"memory:fifo_buffer", "memory:fifo_buffer",

50
rtc_base/openssl_stream_adapter.cc
View File

@ -37,7 +37,6 @@
#include "rtc_base/stream.h" #include "rtc_base/stream.h"
#include "rtc_base/thread.h" #include "rtc_base/thread.h"
#include "rtc_base/time_utils.h" #include "rtc_base/time_utils.h"
#include "system_wrappers/include/field_trial.h"
#if (OPENSSL_VERSION_NUMBER < 0x10100000L) #if (OPENSSL_VERSION_NUMBER < 0x10100000L)
#error "webrtc requires at least OpenSSL version 1.1.0, to support DTLS-SRTP" #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_(nullptr),
ssl_ctx_(nullptr), ssl_ctx_(nullptr),
ssl_mode_(SSL_MODE_TLS), ssl_mode_(SSL_MODE_TLS),
ssl_max_version_(SSL_PROTOCOL_TLS_12), ssl_max_version_(SSL_PROTOCOL_TLS_12) {}
support_legacy_tls_protocols_flag_(
webrtc::field_trial::IsEnabled("WebRTC-LegacyTlsProtocols")) {}
OpenSSLStreamAdapter::~OpenSSLStreamAdapter() { OpenSSLStreamAdapter::~OpenSSLStreamAdapter() {
Cleanup(0); Cleanup(0);
@ -955,34 +952,25 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() {
return nullptr; return nullptr;
} }
if (support_legacy_tls_protocols_flag_) { // TODO(https://bugs.webrtc.org/10261): Evaluate and drop (D)TLS 1.0 and 1.1
// TODO(https://bugs.webrtc.org/10261): Completely remove this branch in // support by default.
// M75. SSL_CTX_set_min_proto_version(
SSL_CTX_set_min_proto_version( ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION);
ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); switch (ssl_max_version_) {
switch (ssl_max_version_) { case SSL_PROTOCOL_TLS_10:
case SSL_PROTOCOL_TLS_10: SSL_CTX_set_max_proto_version(
SSL_CTX_set_max_proto_version( ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION);
ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_VERSION); break;
break; case SSL_PROTOCOL_TLS_11:
case SSL_PROTOCOL_TLS_11: SSL_CTX_set_max_proto_version(
SSL_CTX_set_max_proto_version( ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_1_VERSION);
ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_VERSION : TLS1_1_VERSION); break;
break; case SSL_PROTOCOL_TLS_12:
case SSL_PROTOCOL_TLS_12: default:
default: SSL_CTX_set_max_proto_version(
SSL_CTX_set_max_proto_version( ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION);
ctx, ssl_mode_ == SSL_MODE_DTLS ? DTLS1_2_VERSION : TLS1_2_VERSION); break;
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);
} }
#ifdef OPENSSL_IS_BORINGSSL #ifdef OPENSSL_IS_BORINGSSL
// SSL_CTX_set_current_time_cb is only supported in BoringSSL. // SSL_CTX_set_current_time_cb is only supported in BoringSSL.
if (g_use_time_callback_for_testing) { if (g_use_time_callback_for_testing) {

3
rtc_base/openssl_stream_adapter.h
View File

@ -217,9 +217,6 @@ class OpenSSLStreamAdapter final : public SSLStreamAdapter {
// A 50-ms initial timeout ensures rapid setup on fast connections, but may // A 50-ms initial timeout ensures rapid setup on fast connections, but may
// be too aggressive for low bandwidth links. // be too aggressive for low bandwidth links.
int dtls_handshake_timeout_ms_ = 50; 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_;
}; };
///////////////////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////////////////

5
rtc_base/ssl_stream_adapter.h
View File

@ -90,11 +90,6 @@ bool IsGcmCryptoSuiteName(const std::string& crypto_suite);
enum SSLRole { SSL_CLIENT, SSL_SERVER }; enum SSLRole { SSL_CLIENT, SSL_SERVER };
enum SSLMode { SSL_MODE_TLS, SSL_MODE_DTLS }; 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 { enum SSLProtocolVersion {
SSL_PROTOCOL_TLS_10, SSL_PROTOCOL_TLS_10,
SSL_PROTOCOL_TLS_11, SSL_PROTOCOL_TLS_11,

115
rtc_base/ssl_stream_adapter_unittest.cc
View File

@ -24,12 +24,11 @@
#include "rtc_base/ssl_identity.h" #include "rtc_base/ssl_identity.h"
#include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/stream.h" #include "rtc_base/stream.h"
#include "test/field_trial.h"
using ::testing::WithParamInterface;
using ::testing::Values;
using ::testing::Combine; using ::testing::Combine;
using ::testing::tuple; using ::testing::tuple;
using ::testing::Values;
using ::testing::WithParamInterface;
static const int kBlockSize = 4096; static const int kBlockSize = 4096;
static const char kExporterLabel[] = "label"; static const char kExporterLabel[] = "label";
@ -285,7 +284,6 @@ class SSLStreamAdapterTestBase : public testing::Test,
const std::string& client_cert_pem, const std::string& client_cert_pem,
const std::string& client_private_key_pem, const std::string& client_private_key_pem,
bool dtls, bool dtls,
bool legacy_tls_protocols = false,
rtc::KeyParams client_key_type = rtc::KeyParams(rtc::KT_DEFAULT), rtc::KeyParams client_key_type = rtc::KeyParams(rtc::KT_DEFAULT),
rtc::KeyParams server_key_type = rtc::KeyParams(rtc::KT_DEFAULT)) rtc::KeyParams server_key_type = rtc::KeyParams(rtc::KT_DEFAULT))
: client_cert_pem_(client_cert_pem), : client_cert_pem_(client_cert_pem),
@ -303,8 +301,7 @@ class SSLStreamAdapterTestBase : public testing::Test,
damage_(false), damage_(false),
dtls_(dtls), dtls_(dtls),
handshake_wait_(5000), handshake_wait_(5000),
identities_set_(false), identities_set_(false) {
legacy_tls_protocols_(legacy_tls_protocols) {
// Set use of the test RNG to get predictable loss patterns. // Set use of the test RNG to get predictable loss patterns.
rtc::SetRandomTestMode(true); rtc::SetRandomTestMode(true);
} }
@ -317,10 +314,6 @@ class SSLStreamAdapterTestBase : public testing::Test,
void SetUp() override { void SetUp() override {
CreateStreams(); CreateStreams();
// Enable legacy protocols if required
webrtc::test::ScopedFieldTrials trial(
legacy_tls_protocols_ ? "WebRTC-LegacyTlsProtocols/Enabled/" : "");
client_ssl_.reset(rtc::SSLStreamAdapter::Create(client_stream_)); client_ssl_.reset(rtc::SSLStreamAdapter::Create(client_stream_));
server_ssl_.reset(rtc::SSLStreamAdapter::Create(server_stream_)); server_ssl_.reset(rtc::SSLStreamAdapter::Create(server_stream_));
@ -653,7 +646,6 @@ class SSLStreamAdapterTestBase : public testing::Test,
bool dtls_; bool dtls_;
int handshake_wait_; int handshake_wait_;
bool identities_set_; bool identities_set_;
bool legacy_tls_protocols_;
}; };
class SSLStreamAdapterTestTLS class SSLStreamAdapterTestTLS
@ -664,7 +656,6 @@ class SSLStreamAdapterTestTLS
: SSLStreamAdapterTestBase("", : SSLStreamAdapterTestBase("",
"", "",
false, false,
false,
::testing::get<0>(GetParam()), ::testing::get<0>(GetParam()),
::testing::get<1>(GetParam())), ::testing::get<1>(GetParam())),
client_buffer_(kFifoBufferSize), client_buffer_(kFifoBufferSize),
@ -781,13 +772,9 @@ class SSLStreamAdapterTestDTLS
public WithParamInterface<tuple<rtc::KeyParams, rtc::KeyParams>> { public WithParamInterface<tuple<rtc::KeyParams, rtc::KeyParams>> {
public: public:
SSLStreamAdapterTestDTLS() SSLStreamAdapterTestDTLS()
: SSLStreamAdapterTestDTLS(/*legacy_tls_protocols=*/false) {}
SSLStreamAdapterTestDTLS(bool legacy_tls_protocols)
: SSLStreamAdapterTestBase("", : SSLStreamAdapterTestBase("",
"", "",
true, true,
legacy_tls_protocols,
::testing::get<0>(GetParam()), ::testing::get<0>(GetParam()),
::testing::get<1>(GetParam())), ::testing::get<1>(GetParam())),
client_buffer_(kBufferCapacity, kDefaultBufferSize), 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 // Basic tests: TLS
// Test that we can make a handshake work // Test that we can make a handshake work
@ -1402,7 +1382,7 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) {
// Test getting the used DTLS ciphers. // Test getting the used DTLS ciphers.
// DTLS 1.2 enabled for neither client nor server -> DTLS 1.0 will be used. // 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); SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10);
TestHandshake(); TestHandshake();
@ -1419,64 +1399,6 @@ TEST_P(SSLStreamAdapterTestDTLSLegacyProtocols, TestGetSslCipherSuite) {
server_cipher, ::testing::get<1>(GetParam()).type())); 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. // Test getting the used DTLS 1.2 ciphers.
// DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used. // DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used.
TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) {
@ -1496,10 +1418,9 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) {
server_cipher, ::testing::get<1>(GetParam()).type())); server_cipher, ::testing::get<1>(GetParam()).type()));
} }
// Test getting the used DTLS ciphers. // DTLS 1.2 enabled for client only -> DTLS 1.0 will be used.
// DTLS 1.0 enabled for client and server, both will be upgraded to DTLS 1.2 TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Client) {
TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12);
SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10);
TestHandshake(); TestHandshake();
int client_cipher; int client_cipher;
@ -1507,8 +1428,26 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) {
int server_cipher; int server_cipher;
ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher));
ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(true)); ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_10, GetSslVersion(true));
ASSERT_EQ(rtc::SSL_PROTOCOL_DTLS_12, GetSslVersion(false)); 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_EQ(client_cipher, server_cipher);
ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher( ASSERT_TRUE(rtc::SSLStreamAdapter::IsAcceptableCipher(