diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index d8eb6b5013..5cb3fea8da 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -112,7 +112,8 @@ rtc_library("rtc_base_approved") { } if (is_nacl) { - public_deps += [ "//native_client_sdk/src/libraries/nacl_io" ] # no-presubmit-check TODO(webrtc:8603) + public_deps += # no-presubmit-check TODO(webrtc:8603) + [ "//native_client_sdk/src/libraries/nacl_io" ] } if (is_android) { @@ -602,7 +603,8 @@ rtc_library("rtc_json") { deps = [ ":stringutils" ] all_dependent_configs = [ "//third_party/jsoncpp:jsoncpp_config" ] if (rtc_build_json) { - public_deps = [ "//third_party/jsoncpp" ] # no-presubmit-check TODO(webrtc:8603) + public_deps = # no-presubmit-check TODO(webrtc:8603) + [ "//third_party/jsoncpp" ] } else { include_dirs = [ "$rtc_jsoncpp_root" ] @@ -759,6 +761,7 @@ rtc_library("rtc_base") { "../api:function_view", "../api:scoped_refptr", "../api/task_queue", + "../system_wrappers:field_trial", "network:sent_packet", "system:file_wrapper", "system:rtc_export", @@ -966,7 +969,9 @@ rtc_library("rtc_base") { } if (is_nacl) { - public_deps += [ "//native_client_sdk/src/libraries/nacl_io" ] # no-presubmit-check TODO(webrtc:8603) + public_deps += # no-presubmit-check TODO(webrtc:8603) + [ "//native_client_sdk/src/libraries/nacl_io" ] + defines += [ "timezone=_timezone" ] sources -= [ "ifaddrs_converter.cc" ] } @@ -1333,6 +1338,7 @@ if (rtc_include_tests) { "../api:array_view", "../api/task_queue", "../api/task_queue:task_queue_test", + "../test:field_trial", "../test:fileutils", "../test:rtc_expect_death", "../test:test_main", diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc index 28e8106e77..32af96b65f 100644 --- a/rtc_base/openssl_stream_adapter.cc +++ b/rtc_base/openssl_stream_adapter.cc @@ -36,6 +36,7 @@ #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" @@ -273,7 +274,11 @@ OpenSSLStreamAdapter::OpenSSLStreamAdapter(StreamInterface* stream) ssl_(nullptr), ssl_ctx_(nullptr), ssl_mode_(SSL_MODE_TLS), - ssl_max_version_(SSL_PROTOCOL_TLS_12) {} + ssl_max_version_(SSL_PROTOCOL_TLS_12), + // Default is to support legacy TLS protocols. + // This will be changed to default non-support in M82 or M83. + support_legacy_tls_protocols_flag_( + !webrtc::field_trial::IsDisabled("WebRTC-LegacyTlsProtocols")) {} OpenSSLStreamAdapter::~OpenSSLStreamAdapter() { Cleanup(0); @@ -959,25 +964,34 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() { return nullptr; } - // 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; + if (support_legacy_tls_protocols_flag_) { + // TODO(https://bugs.webrtc.org/10261): Completely remove this branch in + // M84. + 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 M84. + 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 // 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 67f0ab73bc..f8dd5b1358 100644 --- a/rtc_base/openssl_stream_adapter.h +++ b/rtc_base/openssl_stream_adapter.h @@ -216,6 +216,9 @@ 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 M84. + const bool support_legacy_tls_protocols_flag_; }; ///////////////////////////////////////////////////////////////////////////// diff --git a/rtc_base/ssl_stream_adapter.h b/rtc_base/ssl_stream_adapter.h index 484657ebaf..2c317110a3 100644 --- a/rtc_base/ssl_stream_adapter.h +++ b/rtc_base/ssl_stream_adapter.h @@ -90,6 +90,12 @@ bool IsGcmCryptoSuiteName(const std::string& crypto_suite); 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). 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 d9cfe1b9bf..e0ddafcec2 100644 --- a/rtc_base/ssl_stream_adapter_unittest.cc +++ b/rtc_base/ssl_stream_adapter_unittest.cc @@ -24,19 +24,23 @@ #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"; static const unsigned char kExporterContext[] = "context"; static int kExporterContextLen = sizeof(kExporterContext); -static const char kRSA_PRIVATE_KEY_PEM[] = - "-----BEGIN RSA PRIVATE KEY-----\n" +// A private key used for testing, broken into pieces in order to avoid +// issues with Git's checks for private keys in repos. +#define RSA_PRIVATE_KEY_HEADER "-----BEGIN RSA PRIVATE KEY-----\n" + +static const char kRSA_PRIVATE_KEY_PEM[] = RSA_PRIVATE_KEY_HEADER "MIICdwIBADANBgkqhkiG9w0BAQEFAASCAmEwggJdAgEAAoGBAMYRkbhmI7kVA/rM\n" "czsZ+6JDhDvnkF+vn6yCAGuRPV03zuRqZtDy4N4to7PZu9PjqrRl7nDMXrG3YG9y\n" "rlIAZ72KjcKKFAJxQyAKLCIdawKRyp8RdK3LEySWEZb0AV58IadqPZDTNHHRX8dz\n" @@ -53,6 +57,8 @@ static const char kRSA_PRIVATE_KEY_PEM[] = "UCXiYxSsu20QNVw=\n" "-----END RSA PRIVATE KEY-----\n"; +#undef RSA_PRIVATE_KEY_HEADER + static const char kCERT_PEM[] = "-----BEGIN CERTIFICATE-----\n" "MIIBmTCCAQKgAwIBAgIEbzBSAjANBgkqhkiG9w0BAQsFADARMQ8wDQYDVQQDEwZX\n" @@ -767,24 +773,18 @@ class SSLStreamAdapterTestTLS rtc::MemoryStream recv_stream_; }; -class SSLStreamAdapterTestDTLS - : public SSLStreamAdapterTestBase, - public WithParamInterface> { +class SSLStreamAdapterTestDTLSBase : public SSLStreamAdapterTestBase { public: - SSLStreamAdapterTestDTLS() - : SSLStreamAdapterTestBase("", - "", - true, - ::testing::get<0>(GetParam()), - ::testing::get<1>(GetParam())), + SSLStreamAdapterTestDTLSBase(rtc::KeyParams param1, rtc::KeyParams param2) + : SSLStreamAdapterTestBase("", "", true, param1, param2), client_buffer_(kBufferCapacity, kDefaultBufferSize), server_buffer_(kBufferCapacity, kDefaultBufferSize), packet_size_(1000), count_(0), sent_(0) {} - SSLStreamAdapterTestDTLS(const std::string& cert_pem, - const std::string& private_key_pem) + SSLStreamAdapterTestDTLSBase(const std::string& cert_pem, + const std::string& private_key_pem) : SSLStreamAdapterTestBase(cert_pem, private_key_pem, true), client_buffer_(kBufferCapacity, kDefaultBufferSize), server_buffer_(kBufferCapacity, kDefaultBufferSize), @@ -883,15 +883,30 @@ class SSLStreamAdapterTestDTLS } } - private: + protected: BufferQueueStream client_buffer_; BufferQueueStream server_buffer_; + + private: size_t packet_size_; int count_; int sent_; std::set received_; }; +class SSLStreamAdapterTestDTLS + : public SSLStreamAdapterTestDTLSBase, + public WithParamInterface> { + public: + SSLStreamAdapterTestDTLS() + : SSLStreamAdapterTestDTLSBase(::testing::get<0>(GetParam()), + ::testing::get<1>(GetParam())) {} + + SSLStreamAdapterTestDTLS(const std::string& cert_pem, + const std::string& private_key_pem) + : SSLStreamAdapterTestDTLSBase(cert_pem, private_key_pem) {} +}; + rtc::StreamResult SSLDummyStreamBase::Write(const void* data, size_t data_len, size_t* written, @@ -1380,25 +1395,6 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { ASSERT_EQ(kCERT_PEM, server_peer_cert->ToPEMString()); } -// Test getting the used DTLS ciphers. -// DTLS 1.2 enabled for neither client nor server -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, 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) { @@ -1418,27 +1414,11 @@ TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { server_cipher, ::testing::get<1>(GetParam()).type())); } -// 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; - 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(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Server) { - SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10); +// Test getting the used DTLS ciphers. +// DTLS 1.0 is max version for client and server, this will only work if +// legacy is enabled. +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); TestHandshake(); int client_cipher; @@ -1475,3 +1455,162 @@ INSTANTIATE_TEST_SUITE_P( Values(rtc::KeyParams::RSA(1024, 65537), rtc::KeyParams::RSA(1152, 65537), rtc::KeyParams::ECDSA(rtc::EC_NIST_P256)))); + +// Tests for enabling / disabling legacy TLS protocols in DTLS. +class SSLStreamAdapterTestDTLSLegacyProtocols + : public SSLStreamAdapterTestDTLSBase { + public: + SSLStreamAdapterTestDTLSLegacyProtocols() + : SSLStreamAdapterTestDTLSBase(rtc::KeyParams::ECDSA(rtc::EC_NIST_P256), + rtc::KeyParams::ECDSA(rtc::EC_NIST_P256)) { + } + + // Do not use the SetUp version from the parent class. + void SetUp() override {} + + // The legacy TLS protocols flag is read when the OpenSSLStreamAdapter is + // initialized, so we set the experiment while creationg client_ssl_ + // and server_ssl_. + + void ConfigureClient(std::string experiment) { + webrtc::test::ScopedFieldTrials trial(experiment); + client_stream_ = + new SSLDummyStreamDTLS(this, "c2s", &client_buffer_, &server_buffer_); + client_ssl_.reset(rtc::SSLStreamAdapter::Create(client_stream_)); + client_ssl_->SignalEvent.connect( + static_cast(this), + &SSLStreamAdapterTestBase::OnEvent); + client_identity_ = rtc::SSLIdentity::Generate("client", client_key_type_); + client_ssl_->SetIdentity(client_identity_); + } + + void ConfigureServer(std::string experiment) { + // webrtc::test::ScopedFieldTrials trial(experiment); + server_stream_ = + new SSLDummyStreamDTLS(this, "s2c", &server_buffer_, &client_buffer_); + server_ssl_.reset(rtc::SSLStreamAdapter::Create(server_stream_)); + server_ssl_->SignalEvent.connect( + static_cast(this), + &SSLStreamAdapterTestBase::OnEvent); + server_identity_ = rtc::SSLIdentity::Generate("server", server_key_type_); + server_ssl_->SetIdentity(server_identity_); + } +}; + +// 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(""); + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, 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); +} + +// Test getting the used DTLS 1.2 ciphers. +// DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslCipherSuiteDtls12Both) { + ConfigureClient(""); + ConfigureServer(""); + 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); +} + +// DTLS 1.2 enabled for client only -> DTLS 1.0 will be used. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslCipherSuiteDtls12Client) { + ConfigureClient(""); + ConfigureServer(""); + 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); +} + +// DTLS 1.2 enabled for server only -> DTLS 1.0 will be used. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslCipherSuiteDtls12Server) { + ConfigureClient(""); + ConfigureServer(""); + 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); +} + +// Client has legacy TLS versions disabled, server has DTLS 1.0 only. +// This is meant to cause a failure. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslVersionLegacyDisabledServer10) { + ConfigureClient("WebRTC-LegacyTlsProtocols/Disabled/"); + ConfigureServer(""); + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12); + // Handshake should fail. + TestHandshake(false); +} + +// Both client and server have legacy TLS versions disabled and support +// DTLS 1.2. This should work. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslVersionLegacyDisabledServer12) { + ConfigureClient("WebRTC-LegacyTlsProtocols/Disabled/"); + ConfigureServer("WebRTC-LegacyTlsProtocols/Disabled/"); + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12); + TestHandshake(); +} + +// Both client and server have legacy TLS versions enabled and support DTLS 1.0. +// This should work. +TEST_F(SSLStreamAdapterTestDTLSLegacyProtocols, + TestGetSslVersionLegacyEnabledClient10Server10) { + ConfigureClient("WebRTC-LegacyTlsProtocols/Enabled/"); + ConfigureServer("WebRTC-LegacyTlsProtocols/Enabled/"); + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); + TestHandshake(); +} + +// Legacy protocols are disabled, 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/"); + SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); + TestHandshake(false); +}