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

Properly handle different transports having different SSL roles.

Browse Source 
This meant splitting "transport_options" into audio/video/data options,
for when creating the answer, and giving "GetSslRole" a "transport_name"
parameter so we can retrieve the current role on a per-transport basis.

BUG=webrtc:4525
R=pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1516993002 .

Cr-Commit-Position: refs/heads/master@{#11192}
This commit is contained in:
Taylor Brandstetter 2016-01-08 15:35:57 -08:00
parent 25702cb162
commit f475d365a2
11 changed files with 170 additions and 55 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
talk/app/webrtc/peerconnection.cc
View File

@ -461,7 +461,11 @@ bool ConvertRtcOptionsForOffer(
}
session_options->vad_enabled = rtc_options.voice_activity_detection;
session_options->transport_options.ice_restart = rtc_options.ice_restart;
session_options->audio_transport_options.ice_restart =
rtc_options.ice_restart;
session_options->video_transport_options.ice_restart =
rtc_options.ice_restart;
session_options->data_transport_options.ice_restart = rtc_options.ice_restart;
session_options->bundle_enabled = rtc_options.use_rtp_mux;
return true;
@ -507,10 +511,14 @@ bool ParseConstraintsForAnswer(const MediaConstraintsInterface* constraints,
if (FindConstraint(constraints, MediaConstraintsInterface::kIceRestart,
&value, &mandatory_constraints_satisfied)) {
session_options->transport_options.ice_restart = value;
session_options->audio_transport_options.ice_restart = value;
session_options->video_transport_options.ice_restart = value;
session_options->data_transport_options.ice_restart = value;
} else {
// kIceRestart defaults to false according to spec.
session_options->transport_options.ice_restart = false;
session_options->audio_transport_options.ice_restart = false;
session_options->video_transport_options.ice_restart = false;
session_options->data_transport_options.ice_restart = false;
}
if (!constraints) {
@ -962,7 +970,7 @@ void PeerConnection::SetLocalDescription(
// SCTP sids.
rtc::SSLRole role;
if (session_->data_channel_type() == cricket::DCT_SCTP &&
session_->GetSslRole(&role)) {
session_->GetSslRole(session_->data_channel(), &role)) {
AllocateSctpSids(role);
}
@ -1040,7 +1048,7 @@ void PeerConnection::SetRemoteDescription(
// SCTP sids.
rtc::SSLRole role;
if (session_->data_channel_type() == cricket::DCT_SCTP &&
session_->GetSslRole(&role)) {
session_->GetSslRole(session_->data_channel(), &role)) {
AllocateSctpSids(role);
}
@ -1833,7 +1841,7 @@ rtc::scoped_refptr<DataChannel> PeerConnection::InternalCreateDataChannel(
if (session_->data_channel_type() == cricket::DCT_SCTP) {
if (new_config.id < 0) {
rtc::SSLRole role;
if (session_->GetSslRole(&role) &&
if ((session_->GetSslRole(session_->data_channel(), &role)) &&
!sid_allocator_.AllocateSid(role, &new_config.id)) {
LOG(LS_ERROR) << "No id can be allocated for the SCTP data channel.";
return nullptr;

14
talk/app/webrtc/peerconnectioninterface_unittest.cc
View File

@ -2323,7 +2323,9 @@ TEST(CreateSessionOptionsTest, GetDefaultMediaSessionOptionsForOffer) {
EXPECT_FALSE(options.has_video());
EXPECT_TRUE(options.bundle_enabled);
EXPECT_TRUE(options.vad_enabled);
EXPECT_FALSE(options.transport_options.ice_restart);
EXPECT_FALSE(options.audio_transport_options.ice_restart);
EXPECT_FALSE(options.video_transport_options.ice_restart);
EXPECT_FALSE(options.data_transport_options.ice_restart);
}
// Test that a correct MediaSessionOptions is created for an offer if
@ -2358,18 +2360,22 @@ TEST(CreateSessionOptionsTest,
// Test that a correct MediaSessionOptions is created to restart ice if
// IceRestart is set. It also tests that subsequent MediaSessionOptions don't
// have |transport_options.ice_restart| set.
// have |audio_transport_options.ice_restart| etc. set.
TEST(CreateSessionOptionsTest, GetMediaSessionOptionsForOfferWithIceRestart) {
RTCOfferAnswerOptions rtc_options;
rtc_options.ice_restart = true;
cricket::MediaSessionOptions options;
EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options));
EXPECT_TRUE(options.transport_options.ice_restart);
EXPECT_TRUE(options.audio_transport_options.ice_restart);
EXPECT_TRUE(options.video_transport_options.ice_restart);
EXPECT_TRUE(options.data_transport_options.ice_restart);
rtc_options = RTCOfferAnswerOptions();
EXPECT_TRUE(ConvertRtcOptionsForOffer(rtc_options, &options));
EXPECT_FALSE(options.transport_options.ice_restart);
EXPECT_FALSE(options.audio_transport_options.ice_restart);
EXPECT_FALSE(options.video_transport_options.ice_restart);
EXPECT_FALSE(options.data_transport_options.ice_restart);
}
// Test that the MediaConstraints in an answer don't affect if audio and video

20
talk/app/webrtc/webrtcsession.cc
View File

@ -761,14 +761,20 @@ cricket::SecurePolicy WebRtcSession::SdesPolicy() const {
return webrtc_session_desc_factory_->SdesPolicy();
}
bool WebRtcSession::GetSslRole(rtc::SSLRole* role) {
bool WebRtcSession::GetSslRole(const std::string& transport_name,
rtc::SSLRole* role) {
if (!local_desc_ || !remote_desc_) {
LOG(LS_INFO) << "Local and Remote descriptions must be applied to get "
<< "SSL Role of the session.";
return false;
}
return transport_controller_->GetSslRole(role);
return transport_controller_->GetSslRole(transport_name, role);
}
bool WebRtcSession::GetSslRole(const cricket::BaseChannel* channel,
rtc::SSLRole* role) {
return channel && GetSslRole(channel->transport_name(), role);
}
void WebRtcSession::CreateOffer(
@ -970,15 +976,12 @@ bool WebRtcSession::UpdateSessionState(
return BadPranswerSdp(source, GetSessionErrorMsg(), err_desc);
}
} else if (action == kAnswer) {
if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
}
const cricket::ContentGroup* local_bundle =
local_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
const cricket::ContentGroup* remote_bundle =
remote_desc_->description()->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
if (local_bundle && remote_bundle) {
// The answerer decides the transport to bundle on
// The answerer decides the transport to bundle on.
const cricket::ContentGroup* answer_bundle =
(source == cricket::CS_LOCAL ? local_bundle : remote_bundle);
if (!EnableBundle(*answer_bundle)) {
@ -986,6 +989,11 @@ bool WebRtcSession::UpdateSessionState(
return BadAnswerSdp(source, kEnableBundleFailed, err_desc);
}
}
// Only push down the transport description after enabling BUNDLE; we don't
// want to push down a description on a transport about to be destroyed.
if (!PushdownTransportDescription(source, cricket::CA_ANSWER, &td_err)) {
return BadAnswerSdp(source, MakeTdErrorString(td_err), err_desc);
}
EnableChannels();
SetState(STATE_INPROGRESS);
if (!PushdownMediaDescription(cricket::CA_ANSWER, source, err_desc)) {

6
talk/app/webrtc/webrtcsession.h
View File

@ -204,7 +204,11 @@ class WebRtcSession : public AudioProviderInterface,
cricket::SecurePolicy SdesPolicy() const;
// Get current ssl role from transport.
bool GetSslRole(rtc::SSLRole* role);
bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role);
// Get current SSL role for this channel's transport.
// If |transport| is null, returns false.
bool GetSslRole(const cricket::BaseChannel* channel, rtc::SSLRole* role);
void CreateOffer(
CreateSessionDescriptionObserver* observer,

69
talk/app/webrtc/webrtcsession_unittest.cc
View File

@ -1957,6 +1957,67 @@ TEST_P(WebRtcSessionTest, TestCreateAnswerReceiveOfferWithoutEncryption) {
SetLocalDescriptionWithoutError(answer);
}
// Test that we can create and set an answer correctly when different
// SSL roles have been negotiated for different transports.
// See: https://bugs.chromium.org/p/webrtc/issues/detail?id=4525
TEST_P(WebRtcSessionTest, TestCreateAnswerWithDifferentSslRoles) {
SendAudioVideoStream1();
InitWithDtls(GetParam());
SetFactoryDtlsSrtp();
SessionDescriptionInterface* offer = CreateOffer();
SetLocalDescriptionWithoutError(offer);
cricket::MediaSessionOptions options;
options.recv_video = true;
// First, negotiate different SSL roles.
SessionDescriptionInterface* answer =
CreateRemoteAnswer(offer, options, cricket::SEC_DISABLED);
TransportInfo* audio_transport_info =
answer->description()->GetTransportInfoByName("audio");
audio_transport_info->description.connection_role =
cricket::CONNECTIONROLE_ACTIVE;
TransportInfo* video_transport_info =
answer->description()->GetTransportInfoByName("video");
video_transport_info->description.connection_role =
cricket::CONNECTIONROLE_PASSIVE;
SetRemoteDescriptionWithoutError(answer);
// Now create an offer in the reverse direction, and ensure the initial
// offerer responds with an answer with correct SSL roles.
offer = CreateRemoteOfferWithVersion(options, cricket::SEC_DISABLED,
kSessionVersion,
session_->remote_description());
SetRemoteDescriptionWithoutError(offer);
answer = CreateAnswer(nullptr);
audio_transport_info = answer->description()->GetTransportInfoByName("audio");
EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
audio_transport_info->description.connection_role);
video_transport_info = answer->description()->GetTransportInfoByName("video");
EXPECT_EQ(cricket::CONNECTIONROLE_ACTIVE,
video_transport_info->description.connection_role);
SetLocalDescriptionWithoutError(answer);
// Lastly, start BUNDLE-ing on "audio", expecting that the "passive" role of
// audio is transferred over to video in the answer that completes the BUNDLE
// negotiation.
options.bundle_enabled = true;
offer = CreateRemoteOfferWithVersion(options, cricket::SEC_DISABLED,
kSessionVersion,
session_->remote_description());
SetRemoteDescriptionWithoutError(offer);
answer = CreateAnswer(nullptr);
audio_transport_info = answer->description()->GetTransportInfoByName("audio");
EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
audio_transport_info->description.connection_role);
video_transport_info = answer->description()->GetTransportInfoByName("video");
EXPECT_EQ(cricket::CONNECTIONROLE_PASSIVE,
video_transport_info->description.connection_role);
SetLocalDescriptionWithoutError(answer);
}
TEST_F(WebRtcSessionTest, TestSetLocalOfferTwice) {
Init();
SendNothing();
@ -3625,7 +3686,9 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithNewUfragAndPassword) {
SetLocalDescriptionWithoutError(answer.release());
// Receive an offer with new ufrag and password.
options.transport_options.ice_restart = true;
options.audio_transport_options.ice_restart = true;
options.video_transport_options.ice_restart = true;
options.data_transport_options.ice_restart = true;
rtc::scoped_ptr<JsepSessionDescription> updated_offer1(
CreateRemoteOffer(options, session_->remote_description()));
SetRemoteDescriptionWithoutError(updated_offer1.release());
@ -3656,7 +3719,9 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithOldUfragAndPassword) {
SetLocalDescriptionWithoutError(answer.release());
// Receive an offer without changed ufrag or password.
options.transport_options.ice_restart = false;
options.audio_transport_options.ice_restart = false;
options.video_transport_options.ice_restart = false;
options.data_transport_options.ice_restart = false;
rtc::scoped_ptr<JsepSessionDescription> updated_offer2(
CreateRemoteOffer(options, session_->remote_description()));
SetRemoteDescriptionWithoutError(updated_offer2.release());

27
talk/app/webrtc/webrtcsessiondescriptionfactory.cc
View File

@ -392,7 +392,9 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
return;
}
if (session_->local_description() &&
!request.options.transport_options.ice_restart) {
!request.options.audio_transport_options.ice_restart &&
!request.options.video_transport_options.ice_restart &&
!request.options.data_transport_options.ice_restart) {
// Include all local ice candidates in the SessionDescription unless
// the an ice restart has been requested.
CopyCandidatesFromSessionDescription(session_->local_description(), offer);
@ -405,12 +407,25 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
// According to http://tools.ietf.org/html/rfc5245#section-9.2.1.1
// an answer should also contain new ice ufrag and password if an offer has
// been received with new ufrag and password.
request.options.transport_options.ice_restart = session_->IceRestartPending();
request.options.audio_transport_options.ice_restart =
session_->IceRestartPending();
request.options.video_transport_options.ice_restart =
session_->IceRestartPending();
request.options.data_transport_options.ice_restart =
session_->IceRestartPending();
// We should pass current ssl role to the transport description factory, if
// there is already an existing ongoing session.
rtc::SSLRole ssl_role;
if (session_->GetSslRole(&ssl_role)) {
request.options.transport_options.prefer_passive_role =
if (session_->GetSslRole(session_->voice_channel(), &ssl_role)) {
request.options.audio_transport_options.prefer_passive_role =
(rtc::SSL_SERVER == ssl_role);
}
if (session_->GetSslRole(session_->video_channel(), &ssl_role)) {
request.options.video_transport_options.prefer_passive_role =
(rtc::SSL_SERVER == ssl_role);
}
if (session_->GetSslRole(session_->data_channel(), &ssl_role)) {
request.options.data_transport_options.prefer_passive_role =
(rtc::SSL_SERVER == ssl_role);
}
@ -439,7 +454,9 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
return;
}
if (session_->local_description() &&
!request.options.transport_options.ice_restart) {
!request.options.audio_transport_options.ice_restart &&
!request.options.video_transport_options.ice_restart &&
!request.options.data_transport_options.ice_restart) {
// Include all local ice candidates in the SessionDescription unless
// the remote peer has requested an ice restart.
CopyCandidatesFromSessionDescription(session_->local_description(), answer);

34
talk/session/media/mediasession.cc
View File

@ -549,8 +549,8 @@ static bool AddStreamParams(
// Updates the transport infos of the |sdesc| according to the given
// |bundle_group|. The transport infos of the content names within the
// |bundle_group| should be updated to use the ufrag and pwd of the first
// content within the |bundle_group|.
// |bundle_group| should be updated to use the ufrag, pwd and DTLS role of the
// first content within the |bundle_group|.
static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
SessionDescription* sdesc) {
// The bundle should not be empty.
@ -571,6 +571,8 @@ static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
selected_transport_info->description.ice_ufrag;
const std::string& selected_pwd =
selected_transport_info->description.ice_pwd;
ConnectionRole selected_connection_role =
selected_transport_info->description.connection_role;
for (TransportInfos::iterator it =
sdesc->transport_infos().begin();
it != sdesc->transport_infos().end(); ++it) {
@ -578,6 +580,7 @@ static bool UpdateTransportInfoForBundle(const ContentGroup& bundle_group,
it->content_name != selected_content_name) {
it->description.ice_ufrag = selected_ufrag;
it->description.ice_pwd = selected_pwd;
it->description.connection_role = selected_connection_role;
}
}
return true;
@ -1600,7 +1603,7 @@ bool MediaSessionDescriptionFactory::AddAudioContentForOffer(
}
desc->AddContent(content_name, NS_JINGLE_RTP, audio.release());
if (!AddTransportOffer(content_name, options.transport_options,
if (!AddTransportOffer(content_name, options.audio_transport_options,
current_description, desc)) {
return false;
}
@ -1660,7 +1663,7 @@ bool MediaSessionDescriptionFactory::AddVideoContentForOffer(
}
desc->AddContent(content_name, NS_JINGLE_RTP, video.release());
if (!AddTransportOffer(content_name, options.transport_options,
if (!AddTransportOffer(content_name, options.video_transport_options,
current_description, desc)) {
return false;
}
@ -1724,7 +1727,7 @@ bool MediaSessionDescriptionFactory::AddDataContentForOffer(
SetMediaProtocol(secure_transport, data.get());
desc->AddContent(content_name, NS_JINGLE_RTP, data.release());
}
if (!AddTransportOffer(content_name, options.transport_options,
if (!AddTransportOffer(content_name, options.data_transport_options,
current_description, desc)) {
return false;
}
@ -1739,10 +1742,9 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
SessionDescription* answer) const {
const ContentInfo* audio_content = GetFirstAudioContent(offer);
scoped_ptr<TransportDescription> audio_transport(
CreateTransportAnswer(audio_content->name, offer,
options.transport_options,
current_description));
scoped_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
audio_content->name, offer, options.audio_transport_options,
current_description));
if (!audio_transport) {
return false;
}
@ -1798,10 +1800,9 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
StreamParamsVec* current_streams,
SessionDescription* answer) const {
const ContentInfo* video_content = GetFirstVideoContent(offer);
scoped_ptr<TransportDescription> video_transport(
CreateTransportAnswer(video_content->name, offer,
options.transport_options,
current_description));
scoped_ptr<TransportDescription> video_transport(CreateTransportAnswer(
video_content->name, offer, options.video_transport_options,
current_description));
if (!video_transport) {
return false;
}
@ -1854,10 +1855,9 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
StreamParamsVec* current_streams,
SessionDescription* answer) const {
const ContentInfo* data_content = GetFirstDataContent(offer);
scoped_ptr<TransportDescription> data_transport(
CreateTransportAnswer(data_content->name, offer,
options.transport_options,
current_description));
scoped_ptr<TransportDescription> data_transport(CreateTransportAnswer(
data_content->name, offer, options.data_transport_options,
current_description));
if (!data_transport) {
return false;
}

8
talk/session/media/mediasession.h
View File

@ -134,6 +134,10 @@ struct MediaSessionOptions {
bool HasSendMediaStream(MediaType type) const;
// TODO(deadbeef): Put all the audio/video/data-specific options into a map
// structure (content name -> options).
// MediaSessionDescriptionFactory assumes there will never be more than one
// audio/video/data content, but this will change with unified plan.
bool recv_audio;
bool recv_video;
DataChannelType data_channel_type;
@ -144,7 +148,9 @@ struct MediaSessionOptions {
// bps. -1 == auto.
int video_bandwidth;
int data_bandwidth;
TransportOptions transport_options;
TransportOptions audio_transport_options;
TransportOptions video_transport_options;
TransportOptions data_transport_options;
struct Stream {
Stream(MediaType type,

16
webrtc/p2p/base/transportcontroller.cc
View File

@ -66,9 +66,10 @@ void TransportController::SetIceRole(IceRole ice_role) {
rtc::Bind(&TransportController::SetIceRole_w, this, ice_role));
}
bool TransportController::GetSslRole(rtc::SSLRole* role) {
return worker_thread_->Invoke<bool>(
rtc::Bind(&TransportController::GetSslRole_w, this, role));
bool TransportController::GetSslRole(const std::string& transport_name,
rtc::SSLRole* role) {
return worker_thread_->Invoke<bool>(rtc::Bind(
&TransportController::GetSslRole_w, this, transport_name, role));
}
bool TransportController::SetLocalCertificate(
@ -343,13 +344,16 @@ void TransportController::SetIceRole_w(IceRole ice_role) {
}
}
bool TransportController::GetSslRole_w(rtc::SSLRole* role) {
bool TransportController::GetSslRole_w(const std::string& transport_name,
rtc::SSLRole* role) {
RTC_DCHECK(worker_thread()->IsCurrent());
if (transports_.empty()) {
Transport* t = GetTransport_w(transport_name);
if (!t) {
return false;
}
return transports_.begin()->second->GetSslRole(role);
return t->GetSslRole(role);
}
bool TransportController::SetLocalCertificate_w(

8
webrtc/p2p/base/transportcontroller.h
View File

@ -48,11 +48,7 @@ class TransportController : public sigslot::has_slots<>,
void SetIceConfig(const IceConfig& config);
void SetIceRole(IceRole ice_role);
// TODO(deadbeef) - Return role of each transport, as role may differ from
// one another.
// In current implementaion we just return the role of the first transport
// alphabetically.
bool GetSslRole(rtc::SSLRole* role);
bool GetSslRole(const std::string& transport_name, rtc::SSLRole* role);
// Specifies the identity to use in this session.
// Can only be called once.
@ -160,7 +156,7 @@ class TransportController : public sigslot::has_slots<>,
bool SetSslMaxProtocolVersion_w(rtc::SSLProtocolVersion version);
void SetIceConfig_w(const IceConfig& config);
void SetIceRole_w(IceRole ice_role);
bool GetSslRole_w(rtc::SSLRole* role);
bool GetSslRole_w(const std::string& transport_name, rtc::SSLRole* role);
bool SetLocalCertificate_w(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
bool GetLocalCertificate_w(

3
webrtc/p2p/base/transportcontroller_unittest.cc
View File

@ -262,7 +262,8 @@ TEST_F(TransportControllerTest, TestGetSslRole) {
ASSERT_NE(nullptr, channel);
ASSERT_TRUE(channel->SetSslRole(rtc::SSL_CLIENT));
rtc::SSLRole role;
EXPECT_TRUE(transport_controller_->GetSslRole(&role));
EXPECT_FALSE(transport_controller_->GetSslRole("video", &role));
EXPECT_TRUE(transport_controller_->GetSslRole("audio", &role));
EXPECT_EQ(rtc::SSL_CLIENT, role);
}