From 7914b8cb4127e1bca4d288f2f53f08dc13468b6a Mon Sep 17 00:00:00 2001 From: deadbeef Date: Fri, 21 Apr 2017 03:23:33 -0700 Subject: [PATCH] Negotiate the same SRTP crypto suites for every DTLS association formed. Before this CL, we would negotiate: - No crypto suites for data m= sections. - A full set for audio m= sections. - The full set, minus SRTP_AES128_CM_SHA1_32 for video m= sections. However, this doesn't make sense with BUNDLE, since any DTLS association could end up being used for any type of media. If video is "bundled on" the audio transport (which is typical), it will actually end up using SRTP_AES128_CM_SHA1_32. So, this CL moves the responsibility of deciding SRTP crypto suites out of BaseChannel and into DtlsTransport. The only two possibilities are now "normal set" or "normal set + GCM", if enabled by the PC factory options. This fixes an issue (see linked bug) that was occurring when audio/video were "bundled onto" the data transport. Since the data transport wasn't negotiating any SRTP crypto suites, none were available to use for audio/video, so the application would get black video/no audio. This CL doesn't affect the SDES SRTP crypto suite negotiation; it only affects the negotiation in the DLTS handshake, through the use_srtp extension. BUG=chromium:711243 Review-Url: https://codereview.webrtc.org/2815513012 Cr-Commit-Position: refs/heads/master@{#17810} --- webrtc/api/peerconnectioninterface.h | 1 + webrtc/base/sslstreamadapter.cc | 15 +++ webrtc/base/sslstreamadapter.h | 9 +- webrtc/p2p/base/dtlstransportchannel.cc | 49 +--------- webrtc/p2p/base/dtlstransportchannel.h | 22 ++--- .../p2p/base/dtlstransportchannel_unittest.cc | 93 +++++-------------- webrtc/p2p/base/dtlstransportinternal.h | 11 +-- webrtc/p2p/base/fakedtlstransport.h | 44 ++------- webrtc/p2p/base/faketransportcontroller.h | 23 ++++- webrtc/p2p/base/transportcontroller.cc | 23 ++--- webrtc/p2p/base/transportcontroller.h | 11 ++- webrtc/pc/channel.cc | 41 -------- webrtc/pc/channel.h | 14 --- webrtc/pc/channel_unittest.cc | 72 +------------- webrtc/pc/channelmanager.cc | 19 ---- webrtc/pc/channelmanager.h | 5 - webrtc/pc/mediasession.cc | 75 +++++++-------- webrtc/pc/mediasession.h | 26 +++--- webrtc/pc/peerconnection_integrationtest.cc | 50 ++++++++++ webrtc/pc/peerconnectionfactory.cc | 10 +- webrtc/pc/peerconnectioninterface_unittest.cc | 2 +- webrtc/pc/test/mock_webrtcsession.h | 9 +- webrtc/pc/webrtcsession_unittest.cc | 47 +++++----- 23 files changed, 235 insertions(+), 436 deletions(-) diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index e384fba3db..401ef0480b 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -882,6 +882,7 @@ class PeerConnectionFactoryInterface : public rtc::RefCountInterface { rtc::CryptoOptions crypto_options; }; + // Set the options to be used for subsequently created PeerConnections. virtual void SetOptions(const Options& options) = 0; // |allocator| and |cert_generator| may be null, in which case default diff --git a/webrtc/base/sslstreamadapter.cc b/webrtc/base/sslstreamadapter.cc index 2f601c6257..0927704cd4 100644 --- a/webrtc/base/sslstreamadapter.cc +++ b/webrtc/base/sslstreamadapter.cc @@ -95,6 +95,21 @@ CryptoOptions CryptoOptions::NoGcm() { return options; } +std::vector GetSupportedDtlsSrtpCryptoSuites( + const rtc::CryptoOptions& crypto_options) { + std::vector crypto_suites; + if (crypto_options.enable_gcm_crypto_suites) { + crypto_suites.push_back(rtc::SRTP_AEAD_AES_256_GCM); + crypto_suites.push_back(rtc::SRTP_AEAD_AES_128_GCM); + } + // Note: SRTP_AES128_CM_SHA1_80 is what is required to be supported (by + // draft-ietf-rtcweb-security-arch), but SRTP_AES128_CM_SHA1_32 is allowed as + // well, and saves a few bytes per packet if it ends up selected. + crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_32); + crypto_suites.push_back(rtc::SRTP_AES128_CM_SHA1_80); + return crypto_suites; +} + SSLStreamAdapter* SSLStreamAdapter::Create(StreamInterface* stream) { return new OpenSSLStreamAdapter(stream); } diff --git a/webrtc/base/sslstreamadapter.h b/webrtc/base/sslstreamadapter.h index e1d1d85215..62a724996e 100644 --- a/webrtc/base/sslstreamadapter.h +++ b/webrtc/base/sslstreamadapter.h @@ -38,9 +38,7 @@ const int SRTP_AEAD_AES_128_GCM = 0x0007; const int SRTP_AEAD_AES_256_GCM = 0x0008; #endif -// Cipher suite to use for SRTP. Typically a 80-bit HMAC will be used, except -// in applications (voice) where the additional bandwidth may be significant. -// A 80-bit HMAC is always used for SRTCP. +// Names of SRTP profiles listed above. // 128-bit AES with 80-bit SHA-1 HMAC. extern const char CS_AES_CM_128_HMAC_SHA1_80[]; // 128-bit AES with 32-bit SHA-1 HMAC. @@ -82,6 +80,11 @@ struct CryptoOptions { bool enable_gcm_crypto_suites = false; }; +// Returns supported crypto suites, given |crypto_options|. +// CS_AES_CM_128_HMAC_SHA1_32 will be preferred by default. +std::vector GetSupportedDtlsSrtpCryptoSuites( + const rtc::CryptoOptions& crypto_options); + // SSLStreamAdapter : A StreamInterfaceAdapter that does SSL/TLS. // After SSL has been started, the stream will only open on successful // SSL verification of certificates, and the communication is diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index c4e1f5bcba..fc16edff14 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -108,12 +108,14 @@ void StreamInterfaceChannel::Close() { state_ = rtc::SS_CLOSED; } -DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport) +DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport, + const rtc::CryptoOptions& crypto_options) : transport_name_(ice_transport->transport_name()), component_(ice_transport->component()), network_thread_(rtc::Thread::Current()), ice_transport_(ice_transport), downward_(NULL), + srtp_ciphers_(GetSupportedDtlsSrtpCryptoSuites(crypto_options)), ssl_role_(rtc::SSL_CLIENT), ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_12) { ice_transport_->SignalWritableState.connect(this, @@ -318,51 +320,6 @@ bool DtlsTransport::SetupDtls() { return true; } -bool DtlsTransport::SetSrtpCryptoSuites(const std::vector& ciphers) { - if (srtp_ciphers_ == ciphers) - return true; - - if (dtls_state() == DTLS_TRANSPORT_CONNECTING) { - LOG(LS_WARNING) << "Ignoring new SRTP ciphers while DTLS is negotiating"; - return true; - } - - if (dtls_state() == DTLS_TRANSPORT_CONNECTED) { - // We don't support DTLS renegotiation currently. If new set of srtp ciphers - // are different than what's being used currently, we will not use it. - // So for now, let's be happy (or sad) with a warning message. - int current_srtp_cipher; - if (!dtls_->GetDtlsSrtpCryptoSuite(¤t_srtp_cipher)) { - LOG(LS_ERROR) - << "Failed to get the current SRTP cipher for DTLS transport"; - return false; - } - const std::vector::const_iterator iter = - std::find(ciphers.begin(), ciphers.end(), current_srtp_cipher); - if (iter == ciphers.end()) { - std::string requested_str; - for (size_t i = 0; i < ciphers.size(); ++i) { - requested_str.append(" "); - requested_str.append(rtc::SrtpCryptoSuiteToName(ciphers[i])); - requested_str.append(" "); - } - LOG(LS_WARNING) << "Ignoring new set of SRTP ciphers, as DTLS " - << "renegotiation is not supported currently " - << "current cipher = " << current_srtp_cipher << " and " - << "requested = " << "[" << requested_str << "]"; - } - return true; - } - - if (dtls_state() != DTLS_TRANSPORT_NEW) { - LOG(LS_ERROR) << "Can't set SRTP ciphers for a closed session"; - return false; - } - - srtp_ciphers_ = ciphers; - return true; -} - bool DtlsTransport::GetSrtpCryptoSuite(int* cipher) { if (dtls_state() != DTLS_TRANSPORT_CONNECTED) { return false; diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index d16d8a64dd..65ff4424fb 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -86,9 +86,12 @@ class StreamInterfaceChannel : public rtc::StreamInterface { // into packet writes on ice_transport_. class DtlsTransport : public DtlsTransportInternal { public: - // The parameters here is: - // ice_transport -- the ice transport we are wrapping - explicit DtlsTransport(IceTransportInternal* ice_transport); + // |ice_transport| is the ICE transport this DTLS transport is wrapping. + // + // |crypto_options| are the options used for the DTLS handshake. This affects + // whether GCM crypto suites are negotiated. + explicit DtlsTransport(IceTransportInternal* ice_transport, + const rtc::CryptoOptions& crypto_options); ~DtlsTransport() override; DtlsTransportState dtls_state() const override { return dtls_state_; } @@ -122,11 +125,6 @@ class DtlsTransport : public DtlsTransportInternal { virtual bool SetSslMaxProtocolVersion(rtc::SSLProtocolVersion version); - // Set up the ciphers to use for DTLS-SRTP. If this method is not called - // before DTLS starts, or |ciphers| is empty, SRTP keys won't be negotiated. - // This method should be called before SetupDtls. - bool SetSrtpCryptoSuites(const std::vector& ciphers) override; - // Find out which DTLS-SRTP cipher was negotiated bool GetSrtpCryptoSuite(int* cipher) override; @@ -174,14 +172,6 @@ class DtlsTransport : public DtlsTransportInternal { return ice_transport_->SetOption(opt, value); } - bool SetSrtpCiphers(const std::vector& ciphers) override { - std::vector crypto_suites; - for (const auto cipher : ciphers) { - crypto_suites.push_back(rtc::SrtpCryptoSuiteFromName(cipher)); - } - return SetSrtpCryptoSuites(crypto_suites); - } - std::string ToString() const { const char RECEIVING_ABBREV[2] = {'_', 'R'}; const char WRITABLE_ABBREV[2] = {'_', 'W'}; diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index 17e7a0bcad..4286d76be3 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -77,10 +77,6 @@ class DtlsTestClient : public sigslot::has_slots<> { const rtc::scoped_refptr& certificate() { return certificate_; } - void SetupSrtp() { - EXPECT_TRUE(certificate_ != nullptr); - use_dtls_srtp_ = true; - } void SetupMaxProtocolVersion(rtc::SSLProtocolVersion version) { ssl_max_version_ = version; } @@ -97,7 +93,7 @@ class DtlsTestClient : public sigslot::has_slots<> { this, &DtlsTestClient::OnFakeTransportChannelReadPacket); cricket::DtlsTransport* dtls = - new cricket::DtlsTransport(fake_ice_channel); + new cricket::DtlsTransport(fake_ice_channel, rtc::CryptoOptions()); dtls->SetLocalCertificate(certificate_); dtls->ice_transport()->SetIceRole(role); dtls->ice_transport()->SetIceTiebreaker( @@ -109,8 +105,7 @@ class DtlsTestClient : public sigslot::has_slots<> { this, &DtlsTestClient::OnTransportChannelReadPacket); dtls->SignalSentPacket.connect( this, &DtlsTestClient::OnTransportChannelSentPacket); - fake_dtls_transports_.push_back( - std::unique_ptr(dtls)); + dtls_transports_.push_back(std::unique_ptr(dtls)); fake_ice_transports_.push_back( std::unique_ptr(fake_ice_channel)); transport_->AddChannel(dtls, i); @@ -129,7 +124,7 @@ class DtlsTestClient : public sigslot::has_slots<> { } cricket::DtlsTransport* GetDtlsTransport(int component) { - for (const auto& dtls : fake_dtls_transports_) { + for (const auto& dtls : dtls_transports_) { if (dtls->component() == component) { return dtls.get(); } @@ -146,18 +141,6 @@ class DtlsTestClient : public sigslot::has_slots<> { local_role, remote_role, flags); } - void MaybeSetSrtpCryptoSuites() { - if (!use_dtls_srtp_) { - return; - } - std::vector ciphers; - ciphers.push_back(rtc::SRTP_AES128_CM_SHA1_80); - // SRTP ciphers will be set only in the beginning. - for (const auto& dtls : fake_dtls_transports_) { - EXPECT_TRUE(dtls->SetSrtpCryptoSuites(ciphers)); - } - } - void SetLocalTransportDescription( const rtc::scoped_refptr& cert, cricket::ContentAction action, @@ -193,10 +176,6 @@ class DtlsTestClient : public sigslot::has_slots<> { ConnectionRole local_role, ConnectionRole remote_role, int flags) { - if (!(flags & NF_REOFFER)) { - // SRTP ciphers will be set only in the beginning. - MaybeSetSrtpCryptoSuites(); - } if (action == cricket::CA_OFFER) { SetLocalTransportDescription(local_cert, cricket::CA_OFFER, local_role, flags); @@ -221,10 +200,10 @@ class DtlsTestClient : public sigslot::has_slots<> { } bool all_dtls_transports_writable() const { - if (fake_dtls_transports_.empty()) { + if (dtls_transports_.empty()) { return false; } - for (const auto& dtls : fake_dtls_transports_) { + for (const auto& dtls : dtls_transports_) { if (!dtls->writable()) { return false; } @@ -233,10 +212,10 @@ class DtlsTestClient : public sigslot::has_slots<> { } bool all_ice_transports_writable() const { - if (fake_dtls_transports_.empty()) { + if (dtls_transports_.empty()) { return false; } - for (const auto& dtls : fake_dtls_transports_) { + for (const auto& dtls : dtls_transports_) { if (!dtls->ice_transport()->writable()) { return false; } @@ -270,7 +249,7 @@ class DtlsTestClient : public sigslot::has_slots<> { } void CheckSrtp(int expected_crypto_suite) { - for (const auto& dtls : fake_dtls_transports_) { + for (const auto& dtls : dtls_transports_) { int crypto_suite; bool rv = dtls->GetSrtpCryptoSuite(&crypto_suite); @@ -285,7 +264,7 @@ class DtlsTestClient : public sigslot::has_slots<> { } void CheckSsl() { - for (const auto& dtls : fake_dtls_transports_) { + for (const auto& dtls : dtls_transports_) { int cipher; bool rv = dtls->GetSslCipherSuite(&cipher); @@ -301,7 +280,7 @@ class DtlsTestClient : public sigslot::has_slots<> { } void SendPackets(size_t transport, size_t size, size_t count, bool srtp) { - RTC_CHECK(transport < fake_dtls_transports_.size()); + RTC_CHECK(transport < dtls_transports_.size()); std::unique_ptr packet(new char[size]); size_t sent = 0; do { @@ -316,8 +295,8 @@ class DtlsTestClient : public sigslot::has_slots<> { int flags = (certificate_ && srtp) ? cricket::PF_SRTP_BYPASS : 0; rtc::PacketOptions packet_options; packet_options.packet_id = kFakePacketId; - int rv = fake_dtls_transports_[transport]->SendPacket( - packet.get(), size, packet_options, flags); + int rv = dtls_transports_[transport]->SendPacket(packet.get(), size, + packet_options, flags); ASSERT_GT(rv, 0); ASSERT_EQ(size, static_cast(rv)); ++sent; @@ -325,13 +304,13 @@ class DtlsTestClient : public sigslot::has_slots<> { } int SendInvalidSrtpPacket(size_t transport, size_t size) { - RTC_CHECK(transport < fake_dtls_transports_.size()); + RTC_CHECK(transport < dtls_transports_.size()); std::unique_ptr packet(new char[size]); // Fill the packet with 0 to form an invalid SRTP packet. memset(packet.get(), 0, size); rtc::PacketOptions packet_options; - return fake_dtls_transports_[transport]->SendPacket( + return dtls_transports_[transport]->SendPacket( packet.get(), size, packet_options, cricket::PF_SRTP_BYPASS); } @@ -435,11 +414,10 @@ class DtlsTestClient : public sigslot::has_slots<> { std::string name_; rtc::scoped_refptr certificate_; std::vector> fake_ice_transports_; - std::vector> fake_dtls_transports_; + std::vector> dtls_transports_; std::unique_ptr transport_; size_t packet_size_ = 0u; std::set received_; - bool use_dtls_srtp_ = false; rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12; int received_dtls_client_hellos_ = 0; int received_dtls_server_hellos_ = 0; @@ -458,7 +436,6 @@ class DtlsTransportChannelTestBase { client2_("P2"), channel_ct_(1), use_dtls_(false), - use_dtls_srtp_(false), ssl_expected_version_(rtc::SSL_PROTOCOL_DTLS_12) {} void SetChannelCount(size_t channel_ct) { @@ -480,18 +457,6 @@ class DtlsTransportChannelTestBase { if (c1 && c2) use_dtls_ = true; } - void PrepareDtlsSrtp(bool c1, bool c2) { - if (!use_dtls_) - return; - - if (c1) - client1_.SetupSrtp(); - if (c2) - client2_.SetupSrtp(); - - if (c1 && c2) - use_dtls_srtp_ = true; - } // Negotiate local/remote fingerprint before or after the underlying // tranpsort is connected? @@ -506,8 +471,6 @@ class DtlsTransportChannelTestBase { } else { client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); - client1_.MaybeSetSrtpCryptoSuites(); - client2_.MaybeSetSrtpCryptoSuites(); // This is equivalent to an offer being processed on both sides, but an // answer not yet being received on the initiating side. So the // connection will be made before negotiation has finished on both sides. @@ -551,11 +514,14 @@ class DtlsTransportChannelTestBase { client2_.CheckRole(client2_ssl_role); } - // Check that we negotiated the right ciphers. - if (use_dtls_srtp_) { - client1_.CheckSrtp(rtc::SRTP_AES128_CM_SHA1_80); - client2_.CheckSrtp(rtc::SRTP_AES128_CM_SHA1_80); + if (use_dtls_) { + // Check that we negotiated the right ciphers. Since GCM ciphers are not + // negotiated by default, we should end up with SRTP_AES128_CM_SHA1_32. + client1_.CheckSrtp(rtc::SRTP_AES128_CM_SHA1_32); + client2_.CheckSrtp(rtc::SRTP_AES128_CM_SHA1_32); } else { + // If DTLS isn't actually being used, GetSrtpCryptoSuite should return + // false. client1_.CheckSrtp(rtc::SRTP_INVALID_CRYPTO_SUITE); client2_.CheckSrtp(rtc::SRTP_INVALID_CRYPTO_SUITE); } @@ -630,7 +596,6 @@ class DtlsTransportChannelTestBase { DtlsTestClient client2_; int channel_ct_; bool use_dtls_; - bool use_dtls_srtp_; rtc::SSLProtocolVersion ssl_expected_version_; }; @@ -751,10 +716,9 @@ TEST_F(DtlsTransportChannelTest, TestDtls12Client2) { ASSERT_TRUE(Connect()); } -// Connect with DTLS, negotiate DTLS-SRTP, and transfer SRTP using bypass. +// Connect with DTLS, negotiating DTLS-SRTP, and transfer SRTP using bypass. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtp) { PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect()); TestTransfer(0, 1000, 100, true); } @@ -763,7 +727,6 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtp) { // returned. TEST_F(DtlsTransportChannelTest, TestTransferDtlsInvalidSrtpPacket) { PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect()); int result = client1_.SendInvalidSrtpPacket(0, 100); ASSERT_EQ(-1, result); @@ -772,14 +735,12 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsInvalidSrtpPacket) { // Connect with DTLS. A does DTLS-SRTP but B does not. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpRejected) { PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, false); ASSERT_TRUE(Connect()); } // Connect with DTLS. B does DTLS-SRTP but A does not. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpNotOffered) { PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(false, true); ASSERT_TRUE(Connect()); } @@ -787,7 +748,6 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpNotOffered) { TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpTwoChannels) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect()); TestTransfer(0, 1000, 100, true); TestTransfer(1, 1000, 100, true); @@ -796,7 +756,6 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpTwoChannels) { // Create a single channel with DTLS, and send normal data and SRTP data on it. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect()); TestTransfer(0, 1000, 100, false); TestTransfer(0, 1000, 100, true); @@ -806,7 +765,6 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { TEST_F(DtlsTransportChannelTest, TestTransferDtlsAnswererIsPassive) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE)); TestTransfer(0, 1000, 100, true); @@ -827,7 +785,6 @@ TEST_F(DtlsTransportChannelTest, TestDtlsSetupWithLegacyAsAnswerer) { TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromOfferer) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); // Initial role for client1 is ACTPASS and client2 is ACTIVE. ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE)); @@ -843,7 +800,6 @@ TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromOfferer) { TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromAnswerer) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); // Initial role for client1 is ACTPASS and client2 is ACTIVE. ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE)); @@ -860,7 +816,6 @@ TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromAnswerer) { TEST_F(DtlsTransportChannelTest, TestDtlsRoleReversal) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE)); @@ -875,7 +830,6 @@ TEST_F(DtlsTransportChannelTest, TestDtlsRoleReversal) { TEST_F(DtlsTransportChannelTest, TestDtlsReOfferWithDifferentSetupAttr) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE)); // Renegotiate from client2 with actpass and client1 as active. @@ -890,7 +844,6 @@ TEST_F(DtlsTransportChannelTest, TestDtlsReOfferWithDifferentSetupAttr) { TEST_F(DtlsTransportChannelTest, TestRenegotiateBeforeConnect) { SetChannelCount(2); PrepareDtls(true, true, rtc::KT_DEFAULT); - PrepareDtlsSrtp(true, true); Negotiate(); Renegotiate(&client1_, cricket::CONNECTIONROLE_ACTPASS, diff --git a/webrtc/p2p/base/dtlstransportinternal.h b/webrtc/p2p/base/dtlstransportinternal.h index 7a1e8ad3e9..9ec46639d4 100644 --- a/webrtc/p2p/base/dtlstransportinternal.h +++ b/webrtc/p2p/base/dtlstransportinternal.h @@ -29,7 +29,9 @@ enum PacketFlags { // crypto provided by the transport (e.g. DTLS) }; -// DtlsTransportInternal is an internal interface that does DTLS. +// DtlsTransportInternal is an internal interface that does DTLS, also +// negotiating SRTP crypto suites so that it may be used for DTLS-SRTP. +// // Once the public interface is supported, // (https://www.w3.org/TR/webrtc/#rtcdtlstransport-interface) // the DtlsTransportInterface will be split from this class. @@ -49,13 +51,6 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { virtual bool SetSslRole(rtc::SSLRole role) = 0; - // Sets up the ciphers to use for DTLS-SRTP. - virtual bool SetSrtpCryptoSuites(const std::vector& ciphers) = 0; - - // Keep the original one for backward compatibility until all dependencies - // move away. TODO(zhihuang): Remove this function. - virtual bool SetSrtpCiphers(const std::vector& ciphers) = 0; - // Finds out which DTLS-SRTP cipher was negotiated. // TODO(zhihuang): Remove this once all dependencies implement this. virtual bool GetSrtpCryptoSuite(int* cipher) = 0; diff --git a/webrtc/p2p/base/fakedtlstransport.h b/webrtc/p2p/base/fakedtlstransport.h index da3537f1fd..9d5859c4dd 100644 --- a/webrtc/p2p/base/fakedtlstransport.h +++ b/webrtc/p2p/base/fakedtlstransport.h @@ -86,7 +86,6 @@ class FakeDtlsTransport : public DtlsTransportInternal { dest_ = dest; if (local_cert_ && dest_->local_cert_) { do_dtls_ = true; - NegotiateSrtpCiphers(); } SetWritable(true); if (!asymmetric) { @@ -132,16 +131,12 @@ class FakeDtlsTransport : public DtlsTransportInternal { remote_cert_ = cert; } bool IsDtlsActive() const override { return do_dtls_; } - bool SetSrtpCryptoSuites(const std::vector& ciphers) override { - srtp_ciphers_ = ciphers; - return true; - } bool GetSrtpCryptoSuite(int* crypto_suite) override { - if (chosen_crypto_suite_ != rtc::SRTP_INVALID_CRYPTO_SUITE) { - *crypto_suite = chosen_crypto_suite_; - return true; + if (!do_dtls_) { + return false; } - return false; + *crypto_suite = rtc::SRTP_AES128_CM_SHA1_80; + return true; } bool GetSslCipherSuite(int* cipher_suite) override { return false; } rtc::scoped_refptr GetLocalCertificate() const override { @@ -159,12 +154,11 @@ class FakeDtlsTransport : public DtlsTransportInternal { bool use_context, uint8_t* result, size_t result_len) override { - if (chosen_crypto_suite_ != rtc::SRTP_INVALID_CRYPTO_SUITE) { - memset(result, 0xff, result_len); - return true; + if (!do_dtls_) { + return false; } - - return false; + memset(result, 0xff, result_len); + return true; } void set_ssl_max_protocol_version(rtc::SSLProtocolVersion version) { ssl_max_version_ = version; @@ -172,13 +166,6 @@ class FakeDtlsTransport : public DtlsTransportInternal { rtc::SSLProtocolVersion ssl_max_protocol_version() const { return ssl_max_version_; } - bool SetSrtpCiphers(const std::vector& ciphers) override { - std::vector crypto_suites; - for (const auto cipher : ciphers) { - crypto_suites.push_back(rtc::SrtpCryptoSuiteFromName(cipher)); - } - return SetSrtpCryptoSuites(crypto_suites); - } IceTransportInternal* ice_transport() override { return ice_transport_; } @@ -213,19 +200,6 @@ class FakeDtlsTransport : public DtlsTransportInternal { SignalReadPacket(this, data, len, time, flags); } - void NegotiateSrtpCiphers() { - for (std::vector::const_iterator it1 = srtp_ciphers_.begin(); - it1 != srtp_ciphers_.end(); ++it1) { - for (std::vector::const_iterator it2 = dest_->srtp_ciphers_.begin(); - it2 != dest_->srtp_ciphers_.end(); ++it2) { - if (*it1 == *it2) { - chosen_crypto_suite_ = *it1; - return; - } - } - } - } - void set_receiving(bool receiving) { if (receiving_ == receiving) { return; @@ -253,8 +227,6 @@ class FakeDtlsTransport : public DtlsTransportInternal { rtc::scoped_refptr local_cert_; rtc::FakeSSLCertificate* remote_cert_ = nullptr; bool do_dtls_ = false; - std::vector srtp_ciphers_; - int chosen_crypto_suite_ = rtc::SRTP_INVALID_CRYPTO_SUITE; rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12; rtc::SSLFingerprint dtls_fingerprint_; rtc::SSLRole ssl_role_ = rtc::SSL_CLIENT; diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index 35af390d81..9a696bca5a 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -35,26 +35,39 @@ class FakeTransportController : public TransportController { FakeTransportController() : TransportController(rtc::Thread::Current(), rtc::Thread::Current(), - nullptr) {} + nullptr, + /*redetermine_role_on_ice_restart=*/true, + rtc::CryptoOptions()) {} explicit FakeTransportController(bool redetermine_role_on_ice_restart) : TransportController(rtc::Thread::Current(), rtc::Thread::Current(), nullptr, - redetermine_role_on_ice_restart) {} + redetermine_role_on_ice_restart, + rtc::CryptoOptions()) {} explicit FakeTransportController(IceRole role) : TransportController(rtc::Thread::Current(), rtc::Thread::Current(), - nullptr) { + nullptr, + /*redetermine_role_on_ice_restart=*/true, + rtc::CryptoOptions()) { SetIceRole(role); } explicit FakeTransportController(rtc::Thread* network_thread) - : TransportController(rtc::Thread::Current(), network_thread, nullptr) {} + : TransportController(rtc::Thread::Current(), + network_thread, + nullptr, + /*redetermine_role_on_ice_restart=*/true, + rtc::CryptoOptions()) {} FakeTransportController(rtc::Thread* network_thread, IceRole role) - : TransportController(rtc::Thread::Current(), network_thread, nullptr) { + : TransportController(rtc::Thread::Current(), + network_thread, + nullptr, + /*redetermine_role_on_ice_restart=*/true, + rtc::CryptoOptions()) { SetIceRole(role); } diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index df85a7c358..db2d8d9e34 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -65,22 +65,17 @@ class TransportController::ChannelPair { RTC_DISALLOW_COPY_AND_ASSIGN(ChannelPair); }; -TransportController::TransportController(rtc::Thread* signaling_thread, - rtc::Thread* network_thread, - PortAllocator* port_allocator, - bool redetermine_role_on_ice_restart) +TransportController::TransportController( + rtc::Thread* signaling_thread, + rtc::Thread* network_thread, + PortAllocator* port_allocator, + bool redetermine_role_on_ice_restart, + const rtc::CryptoOptions& crypto_options) : signaling_thread_(signaling_thread), network_thread_(network_thread), port_allocator_(port_allocator), - redetermine_role_on_ice_restart_(redetermine_role_on_ice_restart) {} - -TransportController::TransportController(rtc::Thread* signaling_thread, - rtc::Thread* network_thread, - PortAllocator* port_allocator) - : TransportController(signaling_thread, - network_thread, - port_allocator, - true) {} + redetermine_role_on_ice_restart_(redetermine_role_on_ice_restart), + crypto_options_(crypto_options) {} TransportController::~TransportController() { // Channel destructors may try to send packets, so this needs to happen on @@ -362,7 +357,7 @@ DtlsTransportInternal* TransportController::CreateDtlsTransportChannel_n( const std::string&, int, IceTransportInternal* ice) { - DtlsTransport* dtls = new DtlsTransport(ice); + DtlsTransport* dtls = new DtlsTransport(ice, crypto_options_); dtls->SetSslMaxProtocolVersion(ssl_max_version_); return dtls; } diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index 091e70e993..7cc59e8dfa 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -42,14 +42,14 @@ class TransportController : public sigslot::has_slots<>, // If |redetermine_role_on_ice_restart| is true, ICE role is redetermined // upon setting a local transport description that indicates an ICE restart. // For the constructor that doesn't take this parameter, it defaults to true. + // + // |crypto_options| is used to determine if created DTLS transports negotiate + // GCM crypto suites or not. TransportController(rtc::Thread* signaling_thread, rtc::Thread* network_thread, PortAllocator* port_allocator, - bool redetermine_role_on_ice_restart); - - TransportController(rtc::Thread* signaling_thread, - rtc::Thread* network_thread, - PortAllocator* port_allocator); + bool redetermine_role_on_ice_restart, + const rtc::CryptoOptions& crypto_options); virtual ~TransportController(); @@ -262,6 +262,7 @@ class TransportController : public sigslot::has_slots<>, IceRole ice_role_ = ICEROLE_CONTROLLING; bool redetermine_role_on_ice_restart_; uint64_t ice_tiebreaker_ = rtc::CreateRandomId64(); + rtc::CryptoOptions crypto_options_; rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12; rtc::scoped_refptr certificate_; rtc::AsyncInvoker invoker_; diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index d45e220d5c..59fca4a9d2 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -242,14 +242,6 @@ bool BaseChannel::InitNetwork_n( SetTransports_n(rtp_dtls_transport, rtcp_dtls_transport, rtp_packet_transport, rtcp_packet_transport); - if (rtp_dtls_transport_ && - !SetDtlsSrtpCryptoSuites_n(rtp_dtls_transport_, false)) { - return false; - } - if (rtcp_dtls_transport_ && - !SetDtlsSrtpCryptoSuites_n(rtcp_dtls_transport_, true)) { - return false; - } if (rtp_transport_.rtcp_mux_required()) { rtcp_mux_filter_.SetActive(); } @@ -590,11 +582,6 @@ int BaseChannel::SetOption_n(SocketType type, return transport ? transport->SetOption(opt, value) : -1; } -bool BaseChannel::SetCryptoOptions(const rtc::CryptoOptions& crypto_options) { - crypto_options_ = crypto_options; - return true; -} - void BaseChannel::OnWritableState(rtc::PacketTransportInternal* transport) { RTC_DCHECK(transport == rtp_transport_.rtp_packet_transport() || transport == rtp_transport_.rtcp_packet_transport()); @@ -1021,19 +1008,6 @@ void BaseChannel::SignalDtlsSrtpSetupFailure_s(bool rtcp) { SignalDtlsSrtpSetupFailure(this, rtcp); } -bool BaseChannel::SetDtlsSrtpCryptoSuites_n(DtlsTransportInternal* transport, - bool rtcp) { - std::vector crypto_suites; - // We always use the default SRTP crypto suites for RTCP, but we may use - // different crypto suites for RTP depending on the media type. - if (!rtcp) { - GetSrtpCryptoSuites_n(&crypto_suites); - } else { - GetDefaultSrtpCryptoSuites(crypto_options(), &crypto_suites); - } - return transport->SetSrtpCryptoSuites(crypto_suites); -} - bool BaseChannel::ShouldSetupDtlsSrtp_n() const { // Since DTLS is applied to all transports, checking RTP should be enough. return rtp_dtls_transport_ && rtp_dtls_transport_->IsDtlsActive(); @@ -1956,11 +1930,6 @@ void VoiceChannel::OnAudioMonitorUpdate(AudioMonitor* monitor, SignalAudioMonitor(this, info); } -void VoiceChannel::GetSrtpCryptoSuites_n( - std::vector* crypto_suites) const { - GetSupportedAudioCryptoSuites(crypto_options(), crypto_suites); -} - VideoChannel::VideoChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, @@ -2212,11 +2181,6 @@ void VideoChannel::OnMediaMonitorUpdate( SignalMediaMonitor(this, info); } -void VideoChannel::GetSrtpCryptoSuites_n( - std::vector* crypto_suites) const { - GetSupportedVideoCryptoSuites(crypto_options(), crypto_suites); -} - RtpDataChannel::RtpDataChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, rtc::Thread* signaling_thread, @@ -2487,9 +2451,4 @@ void RtpDataChannel::OnDataChannelReadyToSend(bool writable) { new DataChannelReadyToSendMessageData(writable)); } -void RtpDataChannel::GetSrtpCryptoSuites_n( - std::vector* crypto_suites) const { - GetSupportedDataCryptoSuites(crypto_options(), crypto_suites); -} - } // namespace cricket diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index 2bec643b47..56d51f64da 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -203,8 +203,6 @@ class BaseChannel virtual cricket::MediaType media_type() = 0; - bool SetCryptoOptions(const rtc::CryptoOptions& crypto_options); - // This function returns true if we require SRTP for call setup. bool srtp_required_for_testing() const { return srtp_required_; } @@ -306,8 +304,6 @@ class BaseChannel // |rtcp_channel| indicates whether to set up the RTP or RTCP filter. bool SetupDtlsSrtp_n(bool rtcp); void MaybeSetupDtlsSrtp_n(); - // Set the DTLS-SRTP cipher policy on this channel as appropriate. - bool SetDtlsSrtpCryptoSuites_n(DtlsTransportInternal* transport, bool rtcp); // Should be called whenever the conditions for // IsReadyToReceiveMedia/IsReadyToSendMedia are satisfied (or unsatisfied). @@ -359,13 +355,7 @@ class BaseChannel // From MessageHandler void OnMessage(rtc::Message* pmsg) override; - const rtc::CryptoOptions& crypto_options() const { - return crypto_options_; - } - // Handled in derived classes - // Get the SRTP crypto suites to use for RTP media - virtual void GetSrtpCryptoSuites_n(std::vector* crypto_suites) const = 0; virtual void OnConnectionMonitorUpdate(ConnectionMonitor* monitor, const std::vector& infos) = 0; @@ -419,7 +409,6 @@ class BaseChannel bool has_received_packet_ = false; bool dtls_keyed_ = false; const bool srtp_required_ = true; - rtc::CryptoOptions crypto_options_; int rtp_abs_sendtime_extn_id_ = -1; // MediaChannel related members that should be accessed from the worker @@ -534,7 +523,6 @@ class VoiceChannel : public BaseChannel { bool SetOutputVolume_w(uint32_t ssrc, double volume); void OnMessage(rtc::Message* pmsg) override; - void GetSrtpCryptoSuites_n(std::vector* crypto_suites) const override; void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos) override; @@ -617,7 +605,6 @@ class VideoChannel : public BaseChannel { webrtc::RtpParameters parameters); void OnMessage(rtc::Message* pmsg) override; - void GetSrtpCryptoSuites_n(std::vector* crypto_suites) const override; void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos) override; @@ -726,7 +713,6 @@ class RtpDataChannel : public BaseChannel { void UpdateMediaSendRecvState_w() override; void OnMessage(rtc::Message* pmsg) override; - void GetSrtpCryptoSuites_n(std::vector* crypto_suites) const override; void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos) override; diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index 99cca88637..baacac8eb7 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -97,10 +97,9 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { SECURE = 0x4, SSRC_MUX = 0x8, DTLS = 0x10, - GCM_CIPHER = 0x20, // Use BaseChannel with PacketTransportInternal rather than // DtlsTransportInternal. - RAW_PACKET_TRANSPORT = 0x40, + RAW_PACKET_TRANSPORT = 0x20, }; ChannelTest(bool verify_playout, @@ -263,9 +262,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { worker_thread, network_thread, signaling_thread, engine, ch, cricket::CN_AUDIO, (flags & RTCP_MUX_REQUIRED) != 0, (flags & SECURE) != 0); - rtc::CryptoOptions crypto_options; - crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; - channel->SetCryptoOptions(crypto_options); if (!channel->NeedsRtcpTransport()) { fake_rtcp_dtls_transport = nullptr; } @@ -469,25 +465,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { bool CheckNoRtcp2() { return media_channel2_->CheckNoRtcp(); } - // Checks that the channel is using GCM iff GCM_CIPHER is set in flags. - // Returns true if so. - bool CheckGcmCipher(typename T::Channel* channel, int flags) { - int suite; - cricket::FakeDtlsTransport* transport = - (channel == channel1_.get()) ? fake_rtp_dtls_transport1_.get() - : fake_rtp_dtls_transport2_.get(); - RTC_DCHECK(transport); - if (!transport->GetSrtpCryptoSuite(&suite)) { - return false; - } - - if (flags & GCM_CIPHER) { - return rtc::IsGcmCryptoSuite(suite); - } else { - return (suite != rtc::SRTP_INVALID_CRYPTO_SUITE && - !rtc::IsGcmCryptoSuite(suite)); - } - } void CreateContent(int flags, const cricket::AudioCodec& audio_codec, @@ -1339,13 +1316,10 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { } // Test that we properly send SRTP with RTCP in both directions. - // You can pass in DTLS, RTCP_MUX, GCM_CIPHER and RAW_PACKET_TRANSPORT as - // flags. + // You can pass in DTLS, RTCP_MUX, and RAW_PACKET_TRANSPORT as flags. void SendSrtpToSrtp(int flags1_in = 0, int flags2_in = 0) { - RTC_CHECK((flags1_in & - ~(RTCP_MUX | DTLS | GCM_CIPHER | RAW_PACKET_TRANSPORT)) == 0); - RTC_CHECK((flags2_in & - ~(RTCP_MUX | DTLS | GCM_CIPHER | RAW_PACKET_TRANSPORT)) == 0); + RTC_CHECK((flags1_in & ~(RTCP_MUX | DTLS | RAW_PACKET_TRANSPORT)) == 0); + RTC_CHECK((flags2_in & ~(RTCP_MUX | DTLS | RAW_PACKET_TRANSPORT)) == 0); int flags1 = SECURE | flags1_in; int flags2 = SECURE | flags2_in; @@ -1363,14 +1337,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_TRUE(channel2_->secure()); EXPECT_EQ(dtls1 && dtls2, channel1_->secure_dtls()); EXPECT_EQ(dtls1 && dtls2, channel2_->secure_dtls()); - // We can only query the negotiated cipher suite for DTLS-SRTP transport - // channels. - if (dtls1 && dtls2) { - // A GCM cipher is only used if both channels support GCM ciphers. - int common_gcm_flags = flags1 & flags2 & GCM_CIPHER; - EXPECT_TRUE(CheckGcmCipher(channel1_.get(), common_gcm_flags)); - EXPECT_TRUE(CheckGcmCipher(channel2_.get(), common_gcm_flags)); - } SendRtp1(); SendRtp2(); SendRtcp1(); @@ -2124,9 +2090,6 @@ cricket::VideoChannel* ChannelTest::CreateChannel( cricket::VideoChannel* channel = new cricket::VideoChannel( worker_thread, network_thread, signaling_thread, ch, cricket::CN_VIDEO, (flags & RTCP_MUX_REQUIRED) != 0, (flags & SECURE) != 0); - rtc::CryptoOptions crypto_options; - crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; - channel->SetCryptoOptions(crypto_options); if (!channel->NeedsRtcpTransport()) { fake_rtcp_dtls_transport = nullptr; } @@ -2349,18 +2312,6 @@ TEST_F(VoiceChannelSingleThreadTest, SendDtlsSrtpToDtlsSrtp) { Base::SendSrtpToSrtp(DTLS, DTLS); } -TEST_F(VoiceChannelSingleThreadTest, SendDtlsSrtpToDtlsSrtpGcmBoth) { - Base::SendSrtpToSrtp(DTLS | GCM_CIPHER, DTLS | GCM_CIPHER); -} - -TEST_F(VoiceChannelSingleThreadTest, SendDtlsSrtpToDtlsSrtpGcmOne) { - Base::SendSrtpToSrtp(DTLS | GCM_CIPHER, DTLS); -} - -TEST_F(VoiceChannelSingleThreadTest, SendDtlsSrtpToDtlsSrtpGcmTwo) { - Base::SendSrtpToSrtp(DTLS, DTLS | GCM_CIPHER); -} - TEST_F(VoiceChannelSingleThreadTest, SendDtlsSrtpToDtlsSrtpRtcpMux) { Base::SendSrtpToSrtp(DTLS | RTCP_MUX, DTLS | RTCP_MUX); } @@ -2682,18 +2633,6 @@ TEST_F(VoiceChannelDoubleThreadTest, SendDtlsSrtpToDtlsSrtp) { Base::SendSrtpToSrtp(DTLS, DTLS); } -TEST_F(VoiceChannelDoubleThreadTest, SendDtlsSrtpToDtlsSrtpGcmBoth) { - Base::SendSrtpToSrtp(DTLS | GCM_CIPHER, DTLS | GCM_CIPHER); -} - -TEST_F(VoiceChannelDoubleThreadTest, SendDtlsSrtpToDtlsSrtpGcmOne) { - Base::SendSrtpToSrtp(DTLS | GCM_CIPHER, DTLS); -} - -TEST_F(VoiceChannelDoubleThreadTest, SendDtlsSrtpToDtlsSrtpGcmTwo) { - Base::SendSrtpToSrtp(DTLS, DTLS | GCM_CIPHER); -} - TEST_F(VoiceChannelDoubleThreadTest, SendDtlsSrtpToDtlsSrtpRtcpMux) { Base::SendSrtpToSrtp(DTLS | RTCP_MUX, DTLS | RTCP_MUX); } @@ -3368,9 +3307,6 @@ cricket::RtpDataChannel* ChannelTest::CreateChannel( cricket::RtpDataChannel* channel = new cricket::RtpDataChannel( worker_thread, network_thread, signaling_thread, ch, cricket::CN_DATA, (flags & RTCP_MUX_REQUIRED) != 0, (flags & SECURE) != 0); - rtc::CryptoOptions crypto_options; - crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; - channel->SetCryptoOptions(crypto_options); if (!channel->NeedsRtcpTransport()) { fake_rtcp_dtls_transport = nullptr; } diff --git a/webrtc/pc/channelmanager.cc b/webrtc/pc/channelmanager.cc index f2e0d719ff..995a07bf05 100644 --- a/webrtc/pc/channelmanager.cc +++ b/webrtc/pc/channelmanager.cc @@ -88,22 +88,6 @@ bool ChannelManager::SetVideoRtxEnabled(bool enable) { } } -bool ChannelManager::SetCryptoOptions( - const rtc::CryptoOptions& crypto_options) { - return worker_thread_->Invoke(RTC_FROM_HERE, Bind( - &ChannelManager::SetCryptoOptions_w, this, crypto_options)); -} - -bool ChannelManager::SetCryptoOptions_w( - const rtc::CryptoOptions& crypto_options) { - if (!video_channels_.empty() || !voice_channels_.empty() || - !data_channels_.empty()) { - LOG(LS_WARNING) << "Not changing crypto options in existing channels."; - } - crypto_options_ = crypto_options; - return true; -} - void ChannelManager::GetSupportedAudioSendCodecs( std::vector* codecs) const { *codecs = media_engine_->audio_send_codecs(); @@ -247,7 +231,6 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( new VoiceChannel(worker_thread_, network_thread_, signaling_thread, media_engine_.get(), media_channel, content_name, rtcp_packet_transport == nullptr, srtp_required); - voice_channel->SetCryptoOptions(crypto_options_); if (!voice_channel->Init_w(rtp_dtls_transport, rtcp_dtls_transport, rtp_packet_transport, rtcp_packet_transport)) { @@ -333,7 +316,6 @@ VideoChannel* ChannelManager::CreateVideoChannel_w( VideoChannel* video_channel = new VideoChannel( worker_thread_, network_thread_, signaling_thread, media_channel, content_name, rtcp_packet_transport == nullptr, srtp_required); - video_channel->SetCryptoOptions(crypto_options_); if (!video_channel->Init_w(rtp_dtls_transport, rtcp_dtls_transport, rtp_packet_transport, rtcp_packet_transport)) { delete video_channel; @@ -402,7 +384,6 @@ RtpDataChannel* ChannelManager::CreateRtpDataChannel_w( RtpDataChannel* data_channel = new RtpDataChannel( worker_thread_, network_thread_, signaling_thread, media_channel, content_name, rtcp_transport == nullptr, srtp_required); - data_channel->SetCryptoOptions(crypto_options_); if (!data_channel->Init_w(rtp_transport, rtcp_transport, rtp_transport, rtcp_transport)) { LOG(LS_WARNING) << "Failed to init data channel."; diff --git a/webrtc/pc/channelmanager.h b/webrtc/pc/channelmanager.h index b763fa14fc..e62ad6fcd6 100644 --- a/webrtc/pc/channelmanager.h +++ b/webrtc/pc/channelmanager.h @@ -146,10 +146,6 @@ class ChannelManager { // engines will start offering an RTX codec. Must be called before Init(). bool SetVideoRtxEnabled(bool enable); - // Define crypto options to set on newly created channels. Doesn't change - // options on already created channels. - bool SetCryptoOptions(const rtc::CryptoOptions& crypto_options); - // Starts/stops the local microphone and enables polling of the input level. bool capturing() const { return capturing_; } @@ -175,7 +171,6 @@ class ChannelManager { bool InitMediaEngine_w(); void DestructorDeletes_w(); void Terminate_w(); - bool SetCryptoOptions_w(const rtc::CryptoOptions& crypto_options); VoiceChannel* CreateVoiceChannel_w( webrtc::MediaControllerInterface* media_controller, DtlsTransportInternal* rtp_dtls_transport, diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 0df65297db..34ccdce775 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -35,10 +35,10 @@ namespace { const char kInline[] = "inline:"; -void GetSupportedCryptoSuiteNames(void (*func)(const rtc::CryptoOptions&, - std::vector*), - const rtc::CryptoOptions& crypto_options, - std::vector* names) { +void GetSupportedSdesCryptoSuiteNames(void (*func)(const rtc::CryptoOptions&, + std::vector*), + const rtc::CryptoOptions& crypto_options, + std::vector* names) { std::vector crypto_suites; func(crypto_options, &crypto_suites); for (const auto crypto : crypto_suites) { @@ -179,8 +179,8 @@ bool FindMatchingCrypto(const CryptoParamsVec& cryptos, } // For audio, HMAC 32 is prefered over HMAC 80 because of the low overhead. -void GetSupportedAudioCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { +void GetSupportedAudioSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites) { if (crypto_options.enable_gcm_crypto_suites) { crypto_suites->push_back(rtc::SRTP_AEAD_AES_256_GCM); crypto_suites->push_back(rtc::SRTP_AEAD_AES_128_GCM); @@ -189,36 +189,15 @@ void GetSupportedAudioCryptoSuites(const rtc::CryptoOptions& crypto_options, crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); } -void GetSupportedAudioCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, +void GetSupportedAudioSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names) { - GetSupportedCryptoSuiteNames(GetSupportedAudioCryptoSuites, - crypto_options, crypto_suite_names); + GetSupportedSdesCryptoSuiteNames(GetSupportedAudioSdesCryptoSuites, + crypto_options, crypto_suite_names); } -void GetSupportedVideoCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { - GetDefaultSrtpCryptoSuites(crypto_options, crypto_suites); -} - -void GetSupportedVideoCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suite_names) { - GetSupportedCryptoSuiteNames(GetSupportedVideoCryptoSuites, - crypto_options, crypto_suite_names); -} - -void GetSupportedDataCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { - GetDefaultSrtpCryptoSuites(crypto_options, crypto_suites); -} - -void GetSupportedDataCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suite_names) { - GetSupportedCryptoSuiteNames(GetSupportedDataCryptoSuites, - crypto_options, crypto_suite_names); -} - -void GetDefaultSrtpCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites) { +void GetSupportedVideoSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites) { if (crypto_options.enable_gcm_crypto_suites) { crypto_suites->push_back(rtc::SRTP_AEAD_AES_256_GCM); crypto_suites->push_back(rtc::SRTP_AEAD_AES_128_GCM); @@ -226,10 +205,27 @@ void GetDefaultSrtpCryptoSuites(const rtc::CryptoOptions& crypto_options, crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); } -void GetDefaultSrtpCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, +void GetSupportedVideoSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names) { - GetSupportedCryptoSuiteNames(GetDefaultSrtpCryptoSuites, - crypto_options, crypto_suite_names); + GetSupportedSdesCryptoSuiteNames(GetSupportedVideoSdesCryptoSuites, + crypto_options, crypto_suite_names); +} + +void GetSupportedDataSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites) { + if (crypto_options.enable_gcm_crypto_suites) { + crypto_suites->push_back(rtc::SRTP_AEAD_AES_256_GCM); + crypto_suites->push_back(rtc::SRTP_AEAD_AES_128_GCM); + } + crypto_suites->push_back(rtc::SRTP_AES128_CM_SHA1_80); +} + +void GetSupportedDataSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suite_names) { + GetSupportedSdesCryptoSuiteNames(GetSupportedDataSdesCryptoSuites, + crypto_options, crypto_suite_names); } // Support any GCM cipher (if enabled through options). For video support only @@ -1678,7 +1674,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer( std::unique_ptr audio(new AudioContentDescription()); std::vector crypto_suites; - GetSupportedAudioCryptoSuiteNames(options.crypto_options, &crypto_suites); + GetSupportedAudioSdesCryptoSuiteNames(options.crypto_options, &crypto_suites); if (!CreateMediaContentOffer( options, audio_codecs, @@ -1728,7 +1724,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer( std::unique_ptr video(new VideoContentDescription()); std::vector crypto_suites; - GetSupportedVideoCryptoSuiteNames(options.crypto_options, &crypto_suites); + GetSupportedVideoSdesCryptoSuiteNames(options.crypto_options, &crypto_suites); if (!CreateMediaContentOffer( options, video_codecs, @@ -1804,7 +1800,8 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer( data->set_protocol( secure_transport ? kMediaProtocolDtlsSctp : kMediaProtocolSctp); } else { - GetSupportedDataCryptoSuiteNames(options.crypto_options, &crypto_suites); + GetSupportedDataSdesCryptoSuiteNames(options.crypto_options, + &crypto_suites); } if (!CreateMediaContentOffer( diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index 9474256ee3..edf2dd0f40 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -613,21 +613,21 @@ VideoContentDescription* GetFirstVideoContentDescription( DataContentDescription* GetFirstDataContentDescription( SessionDescription* sdesc); -void GetSupportedAudioCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites); -void GetSupportedVideoCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites); -void GetSupportedDataCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites); -void GetDefaultSrtpCryptoSuites(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suites); -void GetSupportedAudioCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, +// Helper functions to return crypto suites used for SDES. +void GetSupportedAudioSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites); +void GetSupportedVideoSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites); +void GetSupportedDataSdesCryptoSuites(const rtc::CryptoOptions& crypto_options, + std::vector* crypto_suites); +void GetSupportedAudioSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names); -void GetSupportedVideoCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, +void GetSupportedVideoSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names); -void GetSupportedDataCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, - std::vector* crypto_suite_names); -void GetDefaultSrtpCryptoSuiteNames(const rtc::CryptoOptions& crypto_options, +void GetSupportedDataSdesCryptoSuiteNames( + const rtc::CryptoOptions& crypto_options, std::vector* crypto_suite_names); } // namespace cricket diff --git a/webrtc/pc/peerconnection_integrationtest.cc b/webrtc/pc/peerconnection_integrationtest.cc index f8126501b6..0ee4cbd719 100644 --- a/webrtc/pc/peerconnection_integrationtest.cc +++ b/webrtc/pc/peerconnection_integrationtest.cc @@ -2069,6 +2069,28 @@ TEST_F(PeerConnectionIntegrationTest, expected_cipher_suite); } +// Verify that media can be transmitted end-to-end when GCM crypto suites are +// enabled. Note that the above tests, such as GcmCipherUsedWhenGcmSupported, +// only verify that a GCM cipher is negotiated, and not necessarily that SRTP +// works with it. +TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithGcmCipher) { + PeerConnectionFactory::Options gcm_options; + gcm_options.crypto_options.enable_gcm_crypto_suites = true; + ASSERT_TRUE( + CreatePeerConnectionWrappersWithOptions(gcm_options, gcm_options)); + ConnectFakeSignaling(); + // Do normal offer/answer and wait for some frames to be received in each + // direction. + caller()->AddAudioVideoMediaStream(); + callee()->AddAudioVideoMediaStream(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ExpectNewFramesReceivedWithWait( + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + // This test sets up a call between two parties with audio, video and an RTP // data channel. TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithRtpDataChannel) { @@ -2385,6 +2407,34 @@ TEST_F(PeerConnectionIntegrationTest, AddSctpDataChannelInSubsequentOffer) { kDefaultTimeout); } +// Set up a connection initially just using SCTP data channels, later upgrading +// to audio/video, ensuring frames are received end-to-end. Effectively the +// inverse of the test above. +// This was broken in M57; see https://crbug.com/711243 +TEST_F(PeerConnectionIntegrationTest, SctpDataChannelToAudioVideoUpgrade) { + ASSERT_TRUE(CreatePeerConnectionWrappers()); + ConnectFakeSignaling(); + // Do initial offer/answer with just data channel. + caller()->CreateDataChannel(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + // Wait until data can be sent over the data channel. + ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout); + ASSERT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout); + ASSERT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout); + + // Do subsequent offer/answer with two-way audio and video. Audio and video + // should end up bundled on the DTLS/ICE transport already used for data. + caller()->AddAudioVideoMediaStream(); + callee()->AddAudioVideoMediaStream(); + caller()->CreateAndSetAndSignalOffer(); + ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout); + ExpectNewFramesReceivedWithWait( + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount, + kMaxWaitForFramesMs); +} + #endif // HAVE_SCTP // Test that the ICE connection and gathering states eventually reach diff --git a/webrtc/pc/peerconnectionfactory.cc b/webrtc/pc/peerconnectionfactory.cc index 8269a1ad85..f9e6dced50 100644 --- a/webrtc/pc/peerconnectionfactory.cc +++ b/webrtc/pc/peerconnectionfactory.cc @@ -219,7 +219,6 @@ bool PeerConnectionFactory::Initialize() { std::move(media_engine), worker_thread_, network_thread_)); channel_manager_->SetVideoRtxEnabled(true); - channel_manager_->SetCryptoOptions(options_.crypto_options); if (!channel_manager_->Init()) { return false; } @@ -229,9 +228,6 @@ bool PeerConnectionFactory::Initialize() { void PeerConnectionFactory::SetOptions(const Options& options) { options_ = options; - if (channel_manager_) { - channel_manager_->SetCryptoOptions(options.crypto_options); - } } rtc::scoped_refptr @@ -370,9 +366,9 @@ cricket::TransportController* PeerConnectionFactory::CreateTransportController( cricket::PortAllocator* port_allocator, bool redetermine_role_on_ice_restart) { RTC_DCHECK(signaling_thread_->IsCurrent()); - return new cricket::TransportController(signaling_thread_, network_thread_, - port_allocator, - redetermine_role_on_ice_restart); + return new cricket::TransportController( + signaling_thread_, network_thread_, port_allocator, + redetermine_role_on_ice_restart, options_.crypto_options); } rtc::Thread* PeerConnectionFactory::signaling_thread() { diff --git a/webrtc/pc/peerconnectioninterface_unittest.cc b/webrtc/pc/peerconnectioninterface_unittest.cc index 91c79ebb76..fcde2a7869 100644 --- a/webrtc/pc/peerconnectioninterface_unittest.cc +++ b/webrtc/pc/peerconnectioninterface_unittest.cc @@ -662,7 +662,7 @@ class PeerConnectionFactoryForTest : public webrtc::PeerConnectionFactory { bool redetermine_role_on_ice_restart) override { transport_controller = new cricket::TransportController( rtc::Thread::Current(), rtc::Thread::Current(), port_allocator, - redetermine_role_on_ice_restart); + redetermine_role_on_ice_restart, rtc::CryptoOptions()); return transport_controller; } diff --git a/webrtc/pc/test/mock_webrtcsession.h b/webrtc/pc/test/mock_webrtcsession.h index d476377370..cf4253b1e0 100644 --- a/webrtc/pc/test/mock_webrtcsession.h +++ b/webrtc/pc/test/mock_webrtcsession.h @@ -34,9 +34,12 @@ class MockWebRtcSession : public webrtc::WebRtcSession { rtc::Thread::Current(), nullptr, std::unique_ptr( - new cricket::TransportController(rtc::Thread::Current(), - rtc::Thread::Current(), - nullptr)), + new cricket::TransportController( + rtc::Thread::Current(), + rtc::Thread::Current(), + nullptr, + /*redetermine_role_on_ice_restart=*/true, + rtc::CryptoOptions())), std::unique_ptr()) {} MOCK_METHOD0(voice_channel, cricket::VoiceChannel*()); MOCK_METHOD0(video_channel, cricket::VideoChannel*()); diff --git a/webrtc/pc/webrtcsession_unittest.cc b/webrtc/pc/webrtcsession_unittest.cc index 09b9090940..757e14e049 100644 --- a/webrtc/pc/webrtcsession_unittest.cc +++ b/webrtc/pc/webrtcsession_unittest.cc @@ -420,16 +420,18 @@ class WebRtcSessionTest // otherwise one will be generated using the |cert_generator|. void Init( std::unique_ptr cert_generator, - PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { + PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy, + const rtc::CryptoOptions& crypto_options) { ASSERT_TRUE(session_.get() == NULL); fake_sctp_transport_factory_ = new FakeSctpTransportFactory(); session_.reset(new WebRtcSessionForTest( media_controller_.get(), rtc::Thread::Current(), rtc::Thread::Current(), rtc::Thread::Current(), allocator_.get(), &observer_, std::unique_ptr( - new cricket::TransportController(rtc::Thread::Current(), - rtc::Thread::Current(), - allocator_.get())), + new cricket::TransportController( + rtc::Thread::Current(), rtc::Thread::Current(), + allocator_.get(), + /*redetermine_role_on_ice_restart=*/true, crypto_options)), std::unique_ptr( fake_sctp_transport_factory_))); session_->SignalDataChannelOpenMessage.connect( @@ -444,6 +446,7 @@ class WebRtcSessionTest EXPECT_TRUE(session_->Initialize(options_, std::move(cert_generator), configuration_)); session_->set_metrics_observer(metrics_observer_); + crypto_options_ = crypto_options; } void OnDataChannelOpenMessage(const std::string& label, @@ -453,11 +456,8 @@ class WebRtcSessionTest } void Init() { - Init(nullptr, PeerConnectionInterface::kRtcpMuxPolicyNegotiate); - } - - void Init(PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { - Init(nullptr, rtcp_mux_policy); + Init(nullptr, PeerConnectionInterface::kRtcpMuxPolicyNegotiate, + rtc::CryptoOptions()); } void InitWithBundlePolicy( @@ -469,7 +469,12 @@ class WebRtcSessionTest void InitWithRtcpMuxPolicy( PeerConnectionInterface::RtcpMuxPolicy rtcp_mux_policy) { PeerConnectionInterface::RTCConfiguration configuration; - Init(rtcp_mux_policy); + Init(nullptr, rtcp_mux_policy, rtc::CryptoOptions()); + } + + void InitWithCryptoOptions(const rtc::CryptoOptions& crypto_options) { + Init(nullptr, PeerConnectionInterface::kRtcpMuxPolicyNegotiate, + crypto_options); } // Successfully init with DTLS; with a certificate generated and supplied or @@ -486,7 +491,8 @@ class WebRtcSessionTest RTC_CHECK(false); } Init(std::move(cert_generator), - PeerConnectionInterface::kRtcpMuxPolicyNegotiate); + PeerConnectionInterface::kRtcpMuxPolicyNegotiate, + rtc::CryptoOptions()); } // Init with DTLS with a store that will fail to generate a certificate. @@ -495,15 +501,14 @@ class WebRtcSessionTest new FakeRTCCertificateGenerator()); cert_generator->set_should_fail(true); Init(std::move(cert_generator), - PeerConnectionInterface::kRtcpMuxPolicyNegotiate); + PeerConnectionInterface::kRtcpMuxPolicyNegotiate, + rtc::CryptoOptions()); } void InitWithGcm() { rtc::CryptoOptions crypto_options; crypto_options.enable_gcm_crypto_suites = true; - channel_manager_->SetCryptoOptions(crypto_options); - with_gcm_ = true; - Init(); + InitWithCryptoOptions(crypto_options); } void SendAudioVideoStream1() { @@ -599,9 +604,7 @@ class WebRtcSessionTest session_options->data_channel_type = cricket::DCT_QUIC; } - if (with_gcm_) { - session_options->crypto_options.enable_gcm_crypto_suites = true; - } + session_options->crypto_options = crypto_options_; } void GetOptionsForAnswer(cricket::MediaSessionOptions* session_options) { @@ -618,9 +621,7 @@ class WebRtcSessionTest session_options->data_channel_type = session_->data_channel_type(); } - if (with_gcm_) { - session_options->crypto_options.enable_gcm_crypto_suites = true; - } + session_options->crypto_options = crypto_options_; } // Creates a local offer and applies it. Starts ICE. @@ -1542,7 +1543,7 @@ class WebRtcSessionTest // Last values received from data channel creation signal. std::string last_data_channel_label_; InternalDataChannelInit last_data_channel_config_; - bool with_gcm_ = false; + rtc::CryptoOptions crypto_options_; }; TEST_P(WebRtcSessionTest, TestInitializeWithDtls) { @@ -3370,7 +3371,7 @@ TEST_F(WebRtcSessionTest, TestAddChannelToConnectedBundle) { configuration_.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle; options_.disable_encryption = true; - Init(PeerConnectionInterface::kRtcpMuxPolicyRequire); + InitWithRtcpMuxPolicy(PeerConnectionInterface::kRtcpMuxPolicyRequire); // Negotiate an audio channel with MAX_BUNDLE enabled. SendAudioOnlyStream2();