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();