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

Removing "crypto_required" from MediaContentDescription.

Browse Source 
"Crypto required" is a property of the PeerConnection of construction
time; it has nothing to do with SDP. So I'm moving it out of
MediaContentDescription and putting it in the BaseChannel constructor
instead. This is more intuitive, and provides the added assurance that
"secure_required_" can't be flipped from "true" to "false".

BUG=None

Review-Url: https://codereview.webrtc.org/2537343003
Cr-Commit-Position: refs/heads/master@{#15579}
This commit is contained in:
deadbeef 2016-12-13 11:29:11 -08:00 committed by Commit bot
parent 00fd520c8c
commit 7af91ddd6b
15 changed files with 184 additions and 189 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
webrtc/api/rtcstatscollector_unittest.cc
View File

@ -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<rtc::RTCCertificate> 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;

6
webrtc/api/rtpsenderreceiver_unittest.cc
View File

@ -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_);

97
webrtc/api/statscollector_unittest.cc
View File

@ -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;

54
webrtc/api/webrtcsession.cc
View File

@ -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<MediaContentDescription*> (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());

5
webrtc/api/webrtcsession.h
View File

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

8
webrtc/api/webrtcsession_unittest.cc
View File

@ -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

2
webrtc/p2p/base/transportdescription.h
View File

@ -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,

32
webrtc/pc/channel.cc
View File

@ -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<bool>(
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) {}

24
webrtc/pc/channel.h
View File

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

21
webrtc/pc/channel_unittest.cc
View File

@ -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<VideoTraits>::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<DataTraits>::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);

44
webrtc/pc/channelmanager.cc
View File

@ -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<VoiceChannel*>(
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<VideoChannel*>(
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<DataChannel*>(
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.";

17
webrtc/pc/channelmanager.h
View File

@ -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<MediaEngineInterface> media_engine_;

33
webrtc/pc/channelmanager_unittest.cc
View File

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

8
webrtc/pc/mediasession.cc
View File

@ -749,9 +749,6 @@ static bool CreateMediaContentOffer(
MediaContentDescriptionImpl<C>* 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;
}

5
webrtc/pc/mediasession_unittest.cc
View File

@ -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<cricket::Candidate> Candidates;