diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc index b973044c94..278d945378 100644 --- a/webrtc/api/rtcstatscollector_unittest.cc +++ b/webrtc/api/rtcstatscollector_unittest.cc @@ -101,6 +101,8 @@ void PrintTo(const RTCTransportStats& stats, ::std::ostream* os) { namespace { const int64_t kGetStatsReportTimeoutMs = 1000; +const bool kDefaultRtcpEnabled = false; +const bool kDefaultSrtpRequired = true; struct CertificateInfo { rtc::scoped_refptr certificate; @@ -657,12 +659,13 @@ TEST_F(RTCStatsCollectorTest, CollectRTCCodecStats) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( test_->worker_thread(), test_->network_thread(), test_->media_engine(), - voice_media_channel, nullptr, "VoiceContentName", false); + voice_media_channel, nullptr, "VoiceContentName", kDefaultRtcpEnabled, + kDefaultSrtpRequired); MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( test_->worker_thread(), test_->network_thread(), video_media_channel, - nullptr, "VideoContentName", false); + nullptr, "VideoContentName", kDefaultRtcpEnabled, kDefaultSrtpRequired); // Audio cricket::VoiceMediaInfo voice_media_info; @@ -1307,7 +1310,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( test_->worker_thread(), test_->network_thread(), test_->media_engine(), - voice_media_channel, nullptr, "VoiceContentName", false); + voice_media_channel, nullptr, "VoiceContentName", kDefaultRtcpEnabled, + kDefaultSrtpRequired); cricket::VoiceMediaInfo voice_media_info; @@ -1377,7 +1381,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) { MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( test_->worker_thread(), test_->network_thread(), video_media_channel, - nullptr, "VideoContentName", false); + nullptr, "VideoContentName", kDefaultRtcpEnabled, kDefaultSrtpRequired); cricket::VideoMediaInfo video_media_info; @@ -1451,7 +1455,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) { MockVoiceMediaChannel* voice_media_channel = new MockVoiceMediaChannel(); cricket::VoiceChannel voice_channel( test_->worker_thread(), test_->network_thread(), test_->media_engine(), - voice_media_channel, nullptr, "VoiceContentName", false); + voice_media_channel, nullptr, "VoiceContentName", kDefaultRtcpEnabled, + kDefaultSrtpRequired); cricket::VoiceMediaInfo voice_media_info; @@ -1516,7 +1521,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) { MockVideoMediaChannel* video_media_channel = new MockVideoMediaChannel(); cricket::VideoChannel video_channel( test_->worker_thread(), test_->network_thread(), video_media_channel, - nullptr, "VideoContentName", false); + nullptr, "VideoContentName", kDefaultRtcpEnabled, kDefaultSrtpRequired); cricket::VideoMediaInfo video_media_info; diff --git a/webrtc/api/rtpsenderreceiver_unittest.cc b/webrtc/api/rtpsenderreceiver_unittest.cc index 4e98f0c1e1..f4f550e906 100644 --- a/webrtc/api/rtpsenderreceiver_unittest.cc +++ b/webrtc/api/rtpsenderreceiver_unittest.cc @@ -62,12 +62,14 @@ class RtpSenderReceiverTest : public testing::Test { stream_(MediaStream::Create(kStreamLabel1)) { // Create channels to be used by the RtpSenders and RtpReceivers. channel_manager_.Init(); + bool rtcp_enabled = false; + bool srtp_required = true; voice_channel_ = channel_manager_.CreateVoiceChannel( &fake_media_controller_, &fake_transport_controller_, cricket::CN_AUDIO, - nullptr, false, cricket::AudioOptions()); + nullptr, rtcp_enabled, srtp_required, cricket::AudioOptions()); video_channel_ = channel_manager_.CreateVideoChannel( &fake_media_controller_, &fake_transport_controller_, cricket::CN_VIDEO, - nullptr, false, cricket::VideoOptions()); + nullptr, rtcp_enabled, srtp_required, cricket::VideoOptions()); voice_media_channel_ = media_engine_->GetVoiceChannel(0); video_media_channel_ = media_engine_->GetVideoChannel(0); RTC_CHECK(voice_channel_); diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc index baa35d3659..bf60d0ed95 100644 --- a/webrtc/api/statscollector_unittest.cc +++ b/webrtc/api/statscollector_unittest.cc @@ -49,6 +49,11 @@ using webrtc::PeerConnectionInterface; using webrtc::StatsReport; using webrtc::StatsReports; +namespace { +const bool kDefaultRtcpEnabled = false; +const bool kDefaultSrtpRequired = true; +} + namespace cricket { class ChannelManager; @@ -853,9 +858,9 @@ TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) { Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); StatsReports reports; // returned values. cricket::VideoSenderInfo video_sender_info; cricket::VideoMediaInfo stats_read; @@ -900,9 +905,9 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); StatsReports reports; // returned values. cricket::VideoSenderInfo video_sender_info; @@ -976,8 +981,9 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { StatsCollectorForTest stats(&pc_); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, "video", false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, "video", + kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingVideoTrackStats(); stats.AddStream(stream_); @@ -1012,9 +1018,9 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingVideoTrackStats(); stats.AddStream(stream_); @@ -1081,8 +1087,9 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The transport_name known by the video channel. const std::string kVcName("vcname"); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVcName, false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, kVcName, + kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingVideoTrackStats(); stats.AddStream(stream_); @@ -1139,8 +1146,9 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsAbsent) { MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The transport_name known by the video channel. const std::string kVcName("vcname"); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVcName, false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, kVcName, + kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingVideoTrackStats(); stats.AddStream(stream_); @@ -1165,8 +1173,9 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The transport_name known by the video channel. const std::string kVcName("vcname"); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVcName, false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, kVcName, + kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingVideoTrackStats(); stats.AddStream(stream_); @@ -1220,9 +1229,9 @@ TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) { Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); AddIncomingVideoTrackStats(); stats.AddStream(stream_); @@ -1529,9 +1538,9 @@ TEST_F(StatsCollectorTest, FilterOutNegativeInitialValues) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); // Create a local stream with a local audio track and adds it to the stats. if (stream_ == NULL) @@ -1636,9 +1645,9 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingAudioTrackStats(); stats.AddStream(stream_); stats.AddLocalAudioTrack(audio_track_, kSsrcOfTrack); @@ -1672,9 +1681,9 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); AddIncomingAudioTrackStats(); stats.AddStream(stream_); @@ -1702,9 +1711,9 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); AddOutgoingAudioTrackStats(); stats.AddStream(stream_); stats.AddLocalAudioTrack(audio_track_.get(), kSsrcOfTrack); @@ -1764,9 +1773,9 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); // Create a local stream with a local audio track and adds it to the stats. AddOutgoingAudioTrackStats(); @@ -1852,9 +1861,9 @@ TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) { MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. const std::string kVcName("vcname"); - cricket::VoiceChannel voice_channel(worker_thread_, network_thread_, - media_engine_, media_channel, nullptr, - kVcName, false); + cricket::VoiceChannel voice_channel( + worker_thread_, network_thread_, media_engine_, media_channel, nullptr, + kVcName, kDefaultRtcpEnabled, kDefaultSrtpRequired); // Create a local stream with a local audio track and adds it to the stats. AddOutgoingAudioTrackStats(); @@ -1909,9 +1918,9 @@ TEST_F(StatsCollectorTest, VerifyVideoSendSsrcStats) { .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); StatsReports reports; // returned values. cricket::VideoSenderInfo video_sender_info; cricket::VideoMediaInfo stats_read; @@ -1954,9 +1963,9 @@ TEST_F(StatsCollectorTest, VerifyVideoReceiveSsrcStats) { .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true))); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); - cricket::VideoChannel video_channel(worker_thread_, network_thread_, - media_channel, nullptr, kVideoChannelName, - false); + cricket::VideoChannel video_channel( + worker_thread_, network_thread_, media_channel, nullptr, + kVideoChannelName, kDefaultRtcpEnabled, kDefaultSrtpRequired); StatsReports reports; // returned values. cricket::VideoReceiverInfo video_receiver_info; cricket::VideoMediaInfo stats_read; diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index c5582d3630..b05862d37c 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -165,7 +165,7 @@ static bool VerifyMediaDescriptions( // Checks that each non-rejected content has SDES crypto keys or a DTLS // fingerprint. Mismatches, such as replying with a DTLS fingerprint to SDES // keys, will be caught in Transport negotiation, and backstopped by Channel's -// |secure_required| check. +// |srtp_required| check. static bool VerifyCrypto(const SessionDescription* desc, bool dtls_enabled, std::string* error) { @@ -231,30 +231,6 @@ static bool VerifyIceUfragPwdPresent(const SessionDescription* desc) { return true; } -// Forces |sdesc->crypto_required| to the appropriate state based on the -// current security policy, to ensure a failure occurs if there is an error -// in crypto negotiation. -// Called when processing the local session description. -static void UpdateSessionDescriptionSecurePolicy(cricket::CryptoType type, - SessionDescription* sdesc) { - if (!sdesc) { - return; - } - - // Updating the |crypto_required_| in MediaContentDescription to the - // appropriate state based on the current security policy. - for (cricket::ContentInfos::iterator iter = sdesc->contents().begin(); - iter != sdesc->contents().end(); ++iter) { - if (cricket::IsMediaContent(&*iter)) { - MediaContentDescription* mdesc = - static_cast (iter->description); - if (mdesc) { - mdesc->set_crypto_required(type); - } - } - } -} - static bool GetAudioSsrcByTrackId(const SessionDescription* session_description, const std::string& track_id, uint32_t* ssrc) { @@ -641,10 +617,6 @@ cricket::BaseChannel* WebRtcSession::GetChannel( return nullptr; } -void WebRtcSession::SetSdesPolicy(cricket::SecurePolicy secure_policy) { - webrtc_session_desc_factory_->SetSdesPolicy(secure_policy); -} - cricket::SecurePolicy WebRtcSession::SdesPolicy() const { return webrtc_session_desc_factory_->SdesPolicy(); } @@ -697,14 +669,6 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, transport_controller_->SetIceRole(cricket::ICEROLE_CONTROLLING); } - cricket::SecurePolicy sdes_policy = - webrtc_session_desc_factory_->SdesPolicy(); - cricket::CryptoType crypto_required = dtls_enabled_ ? - cricket::CT_DTLS : (sdes_policy == cricket::SEC_REQUIRED ? - cricket::CT_SDES : cricket::CT_NONE); - // Update the MediaContentDescription crypto settings as per the policy set. - UpdateSessionDescriptionSecurePolicy(crypto_required, desc->description()); - local_desc_.reset(desc_temp.release()); // Transport and Media channels will be created only when offer is set. @@ -1671,7 +1635,8 @@ bool WebRtcSession::CreateVoiceChannel(const cricket::ContentInfo* content, bool create_rtcp_transport_channel = !require_rtcp_mux; voice_channel_.reset(channel_manager_->CreateVoiceChannel( media_controller_, transport_controller_.get(), content->name, - bundle_transport, create_rtcp_transport_channel, audio_options_)); + bundle_transport, create_rtcp_transport_channel, SrtpRequired(), + audio_options_)); if (!voice_channel_) { return false; } @@ -1695,7 +1660,8 @@ bool WebRtcSession::CreateVideoChannel(const cricket::ContentInfo* content, bool create_rtcp_transport_channel = !require_rtcp_mux; video_channel_.reset(channel_manager_->CreateVideoChannel( media_controller_, transport_controller_.get(), content->name, - bundle_transport, create_rtcp_transport_channel, video_options_)); + bundle_transport, create_rtcp_transport_channel, SrtpRequired(), + video_options_)); if (!video_channel_) { return false; } @@ -1727,8 +1693,9 @@ bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content, rtcp_mux_policy_ == PeerConnectionInterface::kRtcpMuxPolicyRequire; bool create_rtcp_transport_channel = !sctp && !require_rtcp_mux; data_channel_.reset(channel_manager_->CreateDataChannel( - transport_controller_.get(), media_controller_, content->name, - bundle_transport, create_rtcp_transport_channel, data_channel_type_)); + media_controller_, transport_controller_.get(), content->name, + bundle_transport, create_rtcp_transport_channel, SrtpRequired(), + data_channel_type_)); if (!data_channel_) { return false; } @@ -1936,6 +1903,11 @@ bool WebRtcSession::ReadyToUseRemoteCandidate( return transport_controller_->ReadyForRemoteCandidates(transport_name); } +bool WebRtcSession::SrtpRequired() const { + return dtls_enabled_ || + webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED; +} + void WebRtcSession::OnTransportControllerGatheringState( cricket::IceGatheringState state) { ASSERT(signaling_thread()->IsCurrent()); diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index 1aa3e187ae..06d3375e15 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -197,7 +197,6 @@ class WebRtcSession : cricket::BaseChannel* GetChannel(const std::string& content_name); - void SetSdesPolicy(cricket::SecurePolicy secure_policy); cricket::SecurePolicy SdesPolicy() const; // Get current ssl role from transport. @@ -449,6 +448,10 @@ class WebRtcSession : const SessionDescriptionInterface* remote_desc, bool* valid); + // Returns true if SRTP (either using DTLS-SRTP or SDES) is required by + // this session. + bool SrtpRequired() const; + void OnTransportControllerConnectionState(cricket::IceConnectionState state); void OnTransportControllerReceiving(bool receiving); void OnTransportControllerGatheringState(cricket::IceGatheringState state); diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 51d985c3d7..fe3468820e 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -3644,8 +3644,8 @@ TEST_F(WebRtcSessionTest, TestCryptoAfterSetLocalDescription) { SessionDescriptionInterface* jsep_offer_str = CreateSessionDescription(JsepSessionDescription::kOffer, offer_str, NULL); SetLocalDescriptionWithoutError(jsep_offer_str); - EXPECT_TRUE(session_->voice_channel()->secure_required()); - EXPECT_TRUE(session_->video_channel()->secure_required()); + EXPECT_TRUE(session_->voice_channel()->srtp_required_for_testing()); + EXPECT_TRUE(session_->video_channel()->srtp_required_for_testing()); } // This test verifies the crypto parameter when security is disabled. @@ -3663,8 +3663,8 @@ TEST_F(WebRtcSessionTest, TestCryptoAfterSetLocalDescriptionWithDisabled) { SessionDescriptionInterface* jsep_offer_str = CreateSessionDescription(JsepSessionDescription::kOffer, offer_str, NULL); SetLocalDescriptionWithoutError(jsep_offer_str); - EXPECT_FALSE(session_->voice_channel()->secure_required()); - EXPECT_FALSE(session_->video_channel()->secure_required()); + EXPECT_FALSE(session_->voice_channel()->srtp_required_for_testing()); + EXPECT_FALSE(session_->video_channel()->srtp_required_for_testing()); } // This test verifies that an answer contains new ufrag and password if an offer diff --git a/webrtc/p2p/base/transportdescription.h b/webrtc/p2p/base/transportdescription.h index df8ee92003..e7f9263b3e 100644 --- a/webrtc/p2p/base/transportdescription.h +++ b/webrtc/p2p/base/transportdescription.h @@ -28,6 +28,8 @@ namespace cricket { // SEC_ENABLED: Crypto in outgoing offer and answer (if supplied in offer). // SEC_REQUIRED: Crypto in outgoing offer and answer. Fail any offer with absent // or unsupported crypto. +// TODO(deadbeef): Remove this or rename it to something more appropriate, like +// SdesPolicy. enum SecurePolicy { SEC_DISABLED, SEC_ENABLED, diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index eb2f6340ea..1f29261ed2 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -164,7 +164,8 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, MediaChannel* media_channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp) + bool rtcp, + bool srtp_required) : worker_thread_(worker_thread), network_thread_(network_thread), @@ -172,6 +173,7 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, transport_controller_(transport_controller), rtcp_enabled_(rtcp), + srtp_required_(srtp_required), media_channel_(media_channel), selected_candidate_pair_(nullptr) { RTC_DCHECK(worker_thread_ == rtc::Thread::Current()); @@ -726,7 +728,7 @@ bool BaseChannel::SendPacket(bool rtcp, // Update the length of the packet now that we've added the auth tag. packet->SetSize(len); - } else if (secure_required_) { + } else if (srtp_required_) { // The audio/video engines may attempt to send RTCP packets as soon as the // streams are created, so don't treat this as an error for RTCP. // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=6809 @@ -815,7 +817,7 @@ void BaseChannel::HandlePacket(bool rtcp, rtc::CopyOnWriteBuffer* packet, } packet->SetSize(len); - } else if (secure_required_) { + } else if (srtp_required_) { // Our session description indicates that SRTP is required, but we got a // packet before our SRTP filter is active. This means either that // a) we got SRTP packets before we received the SDES keys, in which case @@ -1091,7 +1093,7 @@ bool BaseChannel::SetRtpTransportParameters( return true; } - // Cache secure_required_ for belt and suspenders check on SendPacket + // Cache srtp_required_ for belt and suspenders check on SendPacket return network_thread_->Invoke( RTC_FROM_HERE, Bind(&BaseChannel::SetRtpTransportParameters_n, this, content, action, src, error_desc)); @@ -1104,10 +1106,6 @@ bool BaseChannel::SetRtpTransportParameters_n( std::string* error_desc) { RTC_DCHECK(network_thread_->IsCurrent()); - if (src == CS_LOCAL) { - set_secure_required(content->crypto_required() != CT_NONE); - } - if (!SetSrtp_n(content->cryptos(), action, src, error_desc)) { return false; } @@ -1476,13 +1474,15 @@ VoiceChannel::VoiceChannel(rtc::Thread* worker_thread, VoiceMediaChannel* media_channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp) + bool rtcp, + bool srtp_required) : BaseChannel(worker_thread, network_thread, media_channel, transport_controller, content_name, - rtcp), + rtcp, + srtp_required), media_engine_(media_engine), received_media_(false) {} @@ -1889,13 +1889,15 @@ VideoChannel::VideoChannel(rtc::Thread* worker_thread, VideoMediaChannel* media_channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp) + bool rtcp, + bool srtp_required) : BaseChannel(worker_thread, network_thread, media_channel, transport_controller, content_name, - rtcp) {} + rtcp, + srtp_required) {} bool VideoChannel::Init_w(const std::string* bundle_transport_name) { if (!BaseChannel::Init_w(bundle_transport_name)) { @@ -2150,13 +2152,15 @@ DataChannel::DataChannel(rtc::Thread* worker_thread, DataMediaChannel* media_channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp) + bool rtcp, + bool srtp_required) : BaseChannel(worker_thread, network_thread, media_channel, transport_controller, content_name, - rtcp), + rtcp, + srtp_required), data_channel_type_(cricket::DCT_NONE), ready_to_send_data_(false) {} diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index 308903c2db..64c5f8253f 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -76,12 +76,15 @@ class BaseChannel public ConnectionStatsGetter { public: // |rtcp| represents whether or not this channel uses RTCP. + // If |srtp_required| is true, the channel will not send or receive any + // RTP/RTCP packets without using SRTP (either using SDES or DTLS-SRTP). BaseChannel(rtc::Thread* worker_thread, rtc::Thread* network_thread, MediaChannel* channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp); + bool rtcp, + bool srtp_required); virtual ~BaseChannel(); bool Init_w(const std::string* bundle_transport_name); // Deinit may be called multiple times and is simply ignored if it's already @@ -100,8 +103,6 @@ class BaseChannel // DTLS-based keying. If you turned off SRTP later, however // you could have secure() == false and dtls_secure() == true. bool secure_dtls() const { return dtls_keyed_; } - // This function returns true if we require secure channel for call setup. - bool secure_required() const { return secure_required_; } bool writable() const { return writable_; } @@ -186,6 +187,9 @@ class BaseChannel 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_; } + protected: virtual MediaChannel* media_channel() const { return media_channel_; } @@ -206,9 +210,6 @@ class BaseChannel void set_remote_content_direction(MediaContentDirection direction) { remote_content_direction_ = direction; } - void set_secure_required(bool secure_required) { - secure_required_ = secure_required; - } // These methods verify that: // * The required content description directions have been set. // * The channel is enabled. @@ -397,7 +398,7 @@ class BaseChannel bool was_ever_writable_ = false; bool has_received_packet_ = false; bool dtls_keyed_ = false; - bool secure_required_ = false; + const bool srtp_required_ = true; rtc::CryptoOptions crypto_options_; int rtp_abs_sendtime_extn_id_ = -1; @@ -425,7 +426,8 @@ class VoiceChannel : public BaseChannel { VoiceMediaChannel* channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp); + bool rtcp, + bool srtp_required); ~VoiceChannel(); bool Init_w(const std::string* bundle_transport_name); @@ -542,7 +544,8 @@ class VideoChannel : public BaseChannel { VideoMediaChannel* channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp); + bool rtcp, + bool srtp_required); ~VideoChannel(); bool Init_w(const std::string* bundle_transport_name); @@ -620,7 +623,8 @@ class DataChannel : public BaseChannel { DataMediaChannel* media_channel, TransportController* transport_controller, const std::string& content_name, - bool rtcp); + bool rtcp, + bool srtp_required); ~DataChannel(); bool Init_w(const std::string* bundle_transport_name); diff --git a/webrtc/pc/channel_unittest.cc b/webrtc/pc/channel_unittest.cc index 85a7e697a8..5de3c94555 100644 --- a/webrtc/pc/channel_unittest.cc +++ b/webrtc/pc/channel_unittest.cc @@ -190,10 +190,9 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { typename T::MediaChannel* ch, cricket::TransportController* transport_controller, int flags) { - typename T::Channel* channel = - new typename T::Channel(worker_thread, network_thread, engine, ch, - transport_controller, cricket::CN_AUDIO, - (flags & RTCP) != 0); + typename T::Channel* channel = new typename T::Channel( + worker_thread, network_thread, engine, ch, transport_controller, + cricket::CN_AUDIO, (flags & RTCP) != 0, (flags & SECURE) != 0); rtc::CryptoOptions crypto_options; crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; channel->SetCryptoOptions(crypto_options); @@ -2042,10 +2041,9 @@ cricket::VideoChannel* ChannelTest::CreateChannel( cricket::FakeVideoMediaChannel* ch, cricket::TransportController* transport_controller, int flags) { - cricket::VideoChannel* channel = - new cricket::VideoChannel(worker_thread, network_thread, ch, - transport_controller, cricket::CN_VIDEO, - (flags & RTCP) != 0); + cricket::VideoChannel* channel = new cricket::VideoChannel( + worker_thread, network_thread, ch, transport_controller, + cricket::CN_VIDEO, (flags & RTCP) != 0, (flags & SECURE) != 0); rtc::CryptoOptions crypto_options; crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; channel->SetCryptoOptions(crypto_options); @@ -3315,10 +3313,9 @@ cricket::DataChannel* ChannelTest::CreateChannel( cricket::FakeDataMediaChannel* ch, cricket::TransportController* transport_controller, int flags) { - cricket::DataChannel* channel = - new cricket::DataChannel(worker_thread, network_thread, ch, - transport_controller, cricket::CN_DATA, - (flags & RTCP) != 0); + cricket::DataChannel* channel = new cricket::DataChannel( + worker_thread, network_thread, ch, transport_controller, cricket::CN_DATA, + (flags & RTCP) != 0, (flags & SECURE) != 0); rtc::CryptoOptions crypto_options; crypto_options.enable_gcm_crypto_suites = (flags & GCM_CIPHER) != 0; channel->SetCryptoOptions(crypto_options); diff --git a/webrtc/pc/channelmanager.cc b/webrtc/pc/channelmanager.cc index fec57004a6..a0b3609ecf 100644 --- a/webrtc/pc/channelmanager.cc +++ b/webrtc/pc/channelmanager.cc @@ -218,11 +218,12 @@ VoiceChannel* ChannelManager::CreateVoiceChannel( const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const AudioOptions& options) { return worker_thread_->Invoke( RTC_FROM_HERE, Bind(&ChannelManager::CreateVoiceChannel_w, this, media_controller, transport_controller, content_name, - bundle_transport_name, rtcp, options)); + bundle_transport_name, rtcp, srtp_required, options)); } VoiceChannel* ChannelManager::CreateVoiceChannel_w( @@ -231,6 +232,7 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const AudioOptions& options) { ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); @@ -240,9 +242,9 @@ VoiceChannel* ChannelManager::CreateVoiceChannel_w( if (!media_channel) return nullptr; - VoiceChannel* voice_channel = - new VoiceChannel(worker_thread_, network_thread_, media_engine_.get(), - media_channel, transport_controller, content_name, rtcp); + VoiceChannel* voice_channel = new VoiceChannel( + worker_thread_, network_thread_, media_engine_.get(), media_channel, + transport_controller, content_name, rtcp, srtp_required); voice_channel->SetCryptoOptions(crypto_options_); if (!voice_channel->Init_w(bundle_transport_name)) { delete voice_channel; @@ -281,11 +283,12 @@ VideoChannel* ChannelManager::CreateVideoChannel( const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const VideoOptions& options) { return worker_thread_->Invoke( RTC_FROM_HERE, Bind(&ChannelManager::CreateVideoChannel_w, this, media_controller, transport_controller, content_name, - bundle_transport_name, rtcp, options)); + bundle_transport_name, rtcp, srtp_required, options)); } VideoChannel* ChannelManager::CreateVideoChannel_w( @@ -294,6 +297,7 @@ VideoChannel* ChannelManager::CreateVideoChannel_w( const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const VideoOptions& options) { ASSERT(initialized_); ASSERT(worker_thread_ == rtc::Thread::Current()); @@ -306,7 +310,7 @@ VideoChannel* ChannelManager::CreateVideoChannel_w( VideoChannel* video_channel = new VideoChannel(worker_thread_, network_thread_, media_channel, - transport_controller, content_name, rtcp); + transport_controller, content_name, rtcp, srtp_required); video_channel->SetCryptoOptions(crypto_options_); if (!video_channel->Init_w(bundle_transport_name)) { delete video_channel; @@ -341,36 +345,28 @@ void ChannelManager::DestroyVideoChannel_w(VideoChannel* video_channel) { } DataChannel* ChannelManager::CreateDataChannel( - TransportController* transport_controller, - const std::string& content_name, - const std::string* bundle_transport_name, - bool rtcp, - DataChannelType channel_type) { - return CreateDataChannel(transport_controller, nullptr, content_name, - bundle_transport_name, rtcp, channel_type); -} - -DataChannel* ChannelManager::CreateDataChannel( - TransportController* transport_controller, webrtc::MediaControllerInterface* media_controller, + TransportController* transport_controller, const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, DataChannelType channel_type) { return worker_thread_->Invoke( RTC_FROM_HERE, - Bind(&ChannelManager::CreateDataChannel_w, this, transport_controller, - content_name, bundle_transport_name, rtcp, channel_type, - media_controller)); + Bind(&ChannelManager::CreateDataChannel_w, this, media_controller, + transport_controller, content_name, bundle_transport_name, rtcp, + srtp_required, channel_type)); } DataChannel* ChannelManager::CreateDataChannel_w( + webrtc::MediaControllerInterface* media_controller, TransportController* transport_controller, const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, - DataChannelType data_channel_type, - webrtc::MediaControllerInterface* media_controller) { + bool srtp_required, + DataChannelType data_channel_type) { // This is ok to alloc from a thread other than the worker thread. ASSERT(initialized_); MediaConfig config; @@ -385,9 +381,11 @@ DataChannel* ChannelManager::CreateDataChannel_w( return NULL; } + // Only RTP data channels need SRTP. + srtp_required = srtp_required && data_channel_type == DCT_RTP; DataChannel* data_channel = new DataChannel(worker_thread_, network_thread_, media_channel, - transport_controller, content_name, rtcp); + transport_controller, content_name, rtcp, srtp_required); data_channel->SetCryptoOptions(crypto_options_); if (!data_channel->Init_w(bundle_transport_name)) { LOG(LS_WARNING) << "Failed to init data channel."; diff --git a/webrtc/pc/channelmanager.h b/webrtc/pc/channelmanager.h index cc4ad23c7b..7df49bb07d 100644 --- a/webrtc/pc/channelmanager.h +++ b/webrtc/pc/channelmanager.h @@ -94,6 +94,7 @@ class ChannelManager { const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const AudioOptions& options); // Destroys a voice channel created with the Create API. void DestroyVoiceChannel(VoiceChannel* voice_channel); @@ -105,20 +106,17 @@ class ChannelManager { const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const VideoOptions& options); // Destroys a video channel created with the Create API. void DestroyVideoChannel(VideoChannel* video_channel); - DataChannel* CreateDataChannel(TransportController* transport_controller, - const std::string& content_name, - const std::string* bundle_transport_name, - bool rtcp, - DataChannelType data_channel_type); DataChannel* CreateDataChannel( - TransportController* transport_controller, webrtc::MediaControllerInterface* media_controller, + TransportController* transport_controller, const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, DataChannelType data_channel_type); // Destroys a data channel created with the Create API. void DestroyDataChannel(DataChannel* data_channel); @@ -168,6 +166,7 @@ class ChannelManager { const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const AudioOptions& options); void DestroyVoiceChannel_w(VoiceChannel* voice_channel); VideoChannel* CreateVideoChannel_w( @@ -176,15 +175,17 @@ class ChannelManager { const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, + bool srtp_required, const VideoOptions& options); void DestroyVideoChannel_w(VideoChannel* video_channel); DataChannel* CreateDataChannel_w( + webrtc::MediaControllerInterface* media_controller, TransportController* transport_controller, const std::string& content_name, const std::string* bundle_transport_name, bool rtcp, - DataChannelType data_channel_type, - webrtc::MediaControllerInterface* media_controller); + bool srtp_required, + DataChannelType data_channel_type); void DestroyDataChannel_w(DataChannel* data_channel); std::unique_ptr media_engine_; diff --git a/webrtc/pc/channelmanager_unittest.cc b/webrtc/pc/channelmanager_unittest.cc index 4e14453c63..df3e9e5ba3 100644 --- a/webrtc/pc/channelmanager_unittest.cc +++ b/webrtc/pc/channelmanager_unittest.cc @@ -20,6 +20,11 @@ #include "webrtc/p2p/base/faketransportcontroller.h" #include "webrtc/pc/channelmanager.h" +namespace cricket { +const bool kDefaultRtcpEnabled = false; +const bool kDefaultSrtpRequired = true; +} + namespace cricket { static const AudioCodec kAudioCodecs[] = { @@ -98,16 +103,16 @@ TEST_F(ChannelManagerTest, StartupShutdownOnThread) { TEST_F(ChannelManagerTest, CreateDestroyChannels) { EXPECT_TRUE(cm_->Init()); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_mc_, transport_controller_, cricket::CN_AUDIO, nullptr, false, - AudioOptions()); + &fake_mc_, transport_controller_, cricket::CN_AUDIO, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, AudioOptions()); EXPECT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( - &fake_mc_, transport_controller_, cricket::CN_VIDEO, nullptr, false, - VideoOptions()); + &fake_mc_, transport_controller_, cricket::CN_VIDEO, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, VideoOptions()); EXPECT_TRUE(video_channel != nullptr); - cricket::DataChannel* data_channel = - cm_->CreateDataChannel(transport_controller_, cricket::CN_DATA, nullptr, - false, cricket::DCT_RTP); + cricket::DataChannel* data_channel = cm_->CreateDataChannel( + &fake_mc_, transport_controller_, cricket::CN_DATA, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, cricket::DCT_RTP); EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel); @@ -126,16 +131,16 @@ TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) { transport_controller_ = new cricket::FakeTransportController(&network_, ICEROLE_CONTROLLING); cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel( - &fake_mc_, transport_controller_, cricket::CN_AUDIO, nullptr, false, - AudioOptions()); + &fake_mc_, transport_controller_, cricket::CN_AUDIO, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, AudioOptions()); EXPECT_TRUE(voice_channel != nullptr); cricket::VideoChannel* video_channel = cm_->CreateVideoChannel( - &fake_mc_, transport_controller_, cricket::CN_VIDEO, nullptr, false, - VideoOptions()); + &fake_mc_, transport_controller_, cricket::CN_VIDEO, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, VideoOptions()); EXPECT_TRUE(video_channel != nullptr); - cricket::DataChannel* data_channel = - cm_->CreateDataChannel(transport_controller_, cricket::CN_DATA, nullptr, - false, cricket::DCT_RTP); + cricket::DataChannel* data_channel = cm_->CreateDataChannel( + &fake_mc_, transport_controller_, cricket::CN_DATA, nullptr, + kDefaultRtcpEnabled, kDefaultSrtpRequired, cricket::DCT_RTP); EXPECT_TRUE(data_channel != nullptr); cm_->DestroyVideoChannel(video_channel); cm_->DestroyVoiceChannel(voice_channel); diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index fda38dfdff..2f973deb7f 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -749,9 +749,6 @@ static bool CreateMediaContentOffer( MediaContentDescriptionImpl* offer) { offer->AddCodecs(codecs); - if (secure_policy == SEC_REQUIRED) { - offer->set_crypto_required(CT_SDES); - } offer->set_rtcp_mux(options.rtcp_mux_enabled); if (offer->type() == cricket::MEDIA_TYPE_VIDEO) { offer->set_rtcp_reduced_size(true); @@ -777,7 +774,7 @@ static bool CreateMediaContentOffer( } #endif - if (offer->crypto_required() == CT_SDES && offer->cryptos().empty()) { + if (secure_policy == SEC_REQUIRED && offer->cryptos().empty()) { return false; } return true; @@ -1068,8 +1065,7 @@ static bool CreateMediaContentAnswer( } } - if (answer->cryptos().empty() && - (offer->crypto_required() == CT_SDES || sdes_policy == SEC_REQUIRED)) { + if (answer->cryptos().empty() && sdes_policy == SEC_REQUIRED) { return false; } diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index 4d6418159a..45de0d2b1a 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -26,13 +26,10 @@ #ifdef HAVE_SRTP #define ASSERT_CRYPTO(cd, s, cs) \ - ASSERT_EQ(cricket::CT_NONE, cd->crypto_required()); \ ASSERT_EQ(s, cd->cryptos().size()); \ ASSERT_EQ(std::string(cs), cd->cryptos()[0].cipher_suite) #else -#define ASSERT_CRYPTO(cd, s, cs) \ - ASSERT_EQ(cricket::CT_NONE, cd->crypto_required()); \ - ASSERT_EQ(0U, cd->cryptos().size()); +#define ASSERT_CRYPTO(cd, s, cs) ASSERT_EQ(0, cd->cryptos().size()); #endif typedef std::vector Candidates;