diff --git a/talk/app/webrtc/jsepsessiondescription_unittest.cc b/talk/app/webrtc/jsepsessiondescription_unittest.cc index 83f67cb3ad..e2b59fba20 100644 --- a/talk/app/webrtc/jsepsessiondescription_unittest.cc +++ b/talk/app/webrtc/jsepsessiondescription_unittest.cc @@ -79,16 +79,18 @@ static cricket::SessionDescription* CreateCricketSessionDescription() { cricket::NS_GINGLE_P2P, std::vector(), kCandidateUfragVoice, kCandidatePwdVoice, - cricket::ICEMODE_FULL, NULL, - cricket::Candidates())))); + cricket::ICEMODE_FULL, + cricket::CONNECTIONROLE_NONE, + NULL, cricket::Candidates())))); EXPECT_TRUE(desc->AddTransportInfo( cricket::TransportInfo(cricket::CN_VIDEO, cricket::TransportDescription( cricket::NS_GINGLE_P2P, std::vector(), kCandidateUfragVideo, kCandidatePwdVideo, - cricket::ICEMODE_FULL, NULL, - cricket::Candidates())))); + cricket::ICEMODE_FULL, + cricket::CONNECTIONROLE_NONE, + NULL, cricket::Candidates())))); return desc; } diff --git a/talk/app/webrtc/webrtcsdp.cc b/talk/app/webrtc/webrtcsdp.cc index 9910a5101b..a3bfa71ed3 100644 --- a/talk/app/webrtc/webrtcsdp.cc +++ b/talk/app/webrtc/webrtcsdp.cc @@ -141,6 +141,7 @@ static const char kAttributeCandidateUsername[] = "username"; static const char kAttributeCandidatePassword[] = "password"; static const char kAttributeCandidateGeneration[] = "generation"; static const char kAttributeFingerprint[] = "fingerprint"; +static const char kAttributeSetup[] = "setup"; static const char kAttributeFmtp[] = "fmtp"; static const char kAttributeRtpmap[] = "rtpmap"; static const char kAttributeRtcp[] = "rtcp"; @@ -318,6 +319,9 @@ static bool ParseExtmap(const std::string& line, static bool ParseFingerprintAttribute(const std::string& line, talk_base::SSLFingerprint** fingerprint, SdpParseError* error); +static bool ParseDtlsSetup(const std::string& line, + cricket::ConnectionRole* role, + SdpParseError* error); // Helper functions @@ -902,7 +906,8 @@ bool SdpDeserialize(const std::string& message, SdpParseError* error) { std::string session_id; std::string session_version; - TransportDescription session_td(NS_JINGLE_ICE_UDP, Candidates()); + TransportDescription session_td(NS_JINGLE_ICE_UDP, + std::string(), std::string()); RtpHeaderExtensions session_extmaps; cricket::SessionDescription* desc = new cricket::SessionDescription(); std::vector candidates; @@ -1226,8 +1231,23 @@ void BuildMediaDescription(const ContentInfo* content_info, os << kSdpDelimiterColon << fp->algorithm << kSdpDelimiterSpace << fp->GetRfc4572Fingerprint(); - AddLine(os.str(), message); + + // Inserting setup attribute. + if (transport_info->description.connection_role != + cricket::CONNECTIONROLE_NONE) { + // Making sure we are not using "passive" mode. + cricket::ConnectionRole role = + transport_info->description.connection_role; + ASSERT(role == cricket::CONNECTIONROLE_ACTIVE || + role == cricket::CONNECTIONROLE_ACTPASS); + InitAttrLine(kAttributeSetup, &os); + std::string dtls_role_str = role == cricket::CONNECTIONROLE_ACTPASS ? + cricket::CONNECTIONROLE_ACTPASS_STR : + cricket::CONNECTIONROLE_ACTIVE_STR; + os << kSdpDelimiterColon << dtls_role_str; + AddLine(os.str(), message); + } } } @@ -1796,6 +1816,10 @@ bool ParseSessionDescription(const std::string& message, size_t* pos, return false; } session_td->identity_fingerprint.reset(fingerprint); + } else if (HasAttribute(line, kAttributeSetup)) { + if (!ParseDtlsSetup(line, &(session_td->connection_role), error)) { + return false; + } } else if (HasAttribute(line, kAttributeMsidSemantics)) { std::string semantics; if (!GetValue(line, kAttributeMsidSemantics, &semantics, error)) { @@ -1876,6 +1900,24 @@ static bool ParseFingerprintAttribute(const std::string& line, return true; } +static bool ParseDtlsSetup(const std::string& line, + cricket::ConnectionRole* role, + SdpParseError* error) { + // setup-attr = "a=setup:" role + // role = "active" / "passive" / "actpass" / "holdconn" + std::vector fields; + talk_base::split(line.substr(kLinePrefixLength), kSdpDelimiterColon, &fields); + const size_t expected_fields = 2; + if (fields.size() != expected_fields) { + return ParseFailedExpectFieldNum(line, expected_fields, error); + } + std::string role_str = fields[1]; + if (!cricket::StringToConnectionRole(role_str, role)) { + return ParseFailed(line, "Invalid attribute value.", error); + } + return true; +} + // RFC 3551 // PT encoding media type clock rate channels // name (Hz) @@ -2039,6 +2081,7 @@ bool ParseMediaDescription(const std::string& message, session_td.ice_ufrag, session_td.ice_pwd, session_td.ice_mode, + session_td.connection_role, session_td.identity_fingerprint.get(), Candidates()); @@ -2378,6 +2421,10 @@ bool ParseContent(const std::string& message, return false; } transport->identity_fingerprint.reset(fingerprint); + } else if (HasAttribute(line, kAttributeSetup)) { + if (!ParseDtlsSetup(line, &(transport->connection_role), error)) { + return false; + } } else if (is_rtp) { // // RTP specific attrubtes diff --git a/talk/app/webrtc/webrtcsdp_unittest.cc b/talk/app/webrtc/webrtcsdp_unittest.cc index 9e4c66072e..5fa8b0c890 100644 --- a/talk/app/webrtc/webrtcsdp_unittest.cc +++ b/talk/app/webrtc/webrtcsdp_unittest.cc @@ -485,19 +485,13 @@ class WebRtcSdpTest : public testing::Test { EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kAudioContentName, TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), kCandidateUfragVoice, - kCandidatePwdVoice, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + kCandidatePwdVoice)))); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kVideoContentName, TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), kCandidateUfragVideo, - kCandidatePwdVideo, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + kCandidatePwdVideo)))); // v4 host int port = 1234; @@ -860,9 +854,7 @@ class WebRtcSdpTest : public testing::Test { } TransportInfo transport_info( content_name, TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), - ufrag, pwd, cricket::ICEMODE_FULL, - NULL, Candidates())); + ufrag, pwd)); SessionDescription* desc = const_cast(jdesc->description()); desc->RemoveTransportInfoByName(content_name); @@ -903,16 +895,18 @@ class WebRtcSdpTest : public testing::Test { std::vector(), kCandidateUfragVoice, kCandidatePwdVoice, - cricket::ICEMODE_FULL, &fingerprint, - Candidates())))); + cricket::ICEMODE_FULL, + cricket::CONNECTIONROLE_NONE, + &fingerprint, Candidates())))); EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kVideoContentName, TransportDescription(NS_JINGLE_ICE_UDP, std::vector(), kCandidateUfragVideo, kCandidatePwdVideo, - cricket::ICEMODE_FULL, &fingerprint, - Candidates())))); + cricket::ICEMODE_FULL, + cricket::CONNECTIONROLE_NONE, + &fingerprint, Candidates())))); } void AddExtmap() { @@ -984,11 +978,8 @@ class WebRtcSdpTest : public testing::Test { EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kDataContentName, TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), kCandidateUfragData, - kCandidatePwdData, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + kCandidatePwdData)))); } void AddRtpDataChannel() { @@ -1011,11 +1002,8 @@ class WebRtcSdpTest : public testing::Test { EXPECT_TRUE(desc_.AddTransportInfo( TransportInfo(kDataContentName, TransportDescription(NS_JINGLE_ICE_UDP, - std::vector(), kCandidateUfragData, - kCandidatePwdData, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + kCandidatePwdData)))); } bool TestDeserializeDirection(cricket::MediaContentDirection direction) { @@ -1966,3 +1954,60 @@ TEST_F(WebRtcSdpTest, RoundTripSdpWithSctpDataChannelsWithCandidates) { EXPECT_TRUE(SdpDeserialize(sdp_with_data, &jdesc_output)); EXPECT_EQ(sdp_with_data, webrtc::SdpSerialize(jdesc_output)); } + +TEST_F(WebRtcSdpTest, SerializeDtlsSetupAttribute) { + AddFingerprint(); + TransportInfo audio_transport_info = + *(desc_.GetTransportInfoByName(kAudioContentName)); + EXPECT_EQ(cricket::CONNECTIONROLE_NONE, + audio_transport_info.description.connection_role); + audio_transport_info.description.connection_role = + cricket::CONNECTIONROLE_ACTIVE; + + TransportInfo video_transport_info = + *(desc_.GetTransportInfoByName(kVideoContentName)); + EXPECT_EQ(cricket::CONNECTIONROLE_NONE, + video_transport_info.description.connection_role); + video_transport_info.description.connection_role = + cricket::CONNECTIONROLE_ACTIVE; + + desc_.RemoveTransportInfoByName(kAudioContentName); + desc_.RemoveTransportInfoByName(kVideoContentName); + + desc_.AddTransportInfo(audio_transport_info); + desc_.AddTransportInfo(video_transport_info); + + ASSERT_TRUE(jdesc_.Initialize(desc_.Copy(), + jdesc_.session_id(), + jdesc_.session_version())); + std::string message = webrtc::SdpSerialize(jdesc_); + std::string sdp_with_dtlssetup = kSdpFullString; + + // Fingerprint attribute is necessary to add DTLS setup attribute. + InjectAfter(kAttributeIcePwdVoice, + kFingerprint, &sdp_with_dtlssetup); + InjectAfter(kAttributeIcePwdVideo, + kFingerprint, &sdp_with_dtlssetup); + // Now adding |setup| attribute. + InjectAfter(kFingerprint, + "a=setup:active\r\n", &sdp_with_dtlssetup); + EXPECT_EQ(sdp_with_dtlssetup, message); +} + +TEST_F(WebRtcSdpTest, DeserializeDtlsSetupAttribute) { + JsepSessionDescription jdesc_with_dtlssetup(kDummyString); + std::string sdp_with_dtlssetup = kSdpFullString; + InjectAfter(kSessionTime, + "a=setup:actpass\r\n", + &sdp_with_dtlssetup); + EXPECT_TRUE(SdpDeserialize(sdp_with_dtlssetup, &jdesc_with_dtlssetup)); + cricket::SessionDescription* desc = jdesc_with_dtlssetup.description(); + const cricket::TransportInfo* atinfo = + desc->GetTransportInfoByName("audio_content_name"); + EXPECT_EQ(cricket::CONNECTIONROLE_ACTPASS, + atinfo->description.connection_role); + const cricket::TransportInfo* vtinfo = + desc->GetTransportInfoByName("video_content_name"); + EXPECT_EQ(cricket::CONNECTIONROLE_ACTPASS, + vtinfo->description.connection_role); +} diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index b056757df4..7016e2a165 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -505,6 +505,26 @@ cricket::SecureMediaPolicy WebRtcSession::secure_policy() const { return webrtc_session_desc_factory_->secure(); } +bool WebRtcSession::GetSslRole(talk_base::SSLRole* role) { + if (local_description() == NULL || remote_description() == NULL) { + LOG(LS_INFO) << "Local and Remote descriptions must be applied to get " + << "SSL Role of the session."; + return false; + } + + // TODO(mallinath) - Return role of each transport, as role may differ from + // one another. + // In current implementaion we just return the role of first transport in the + // transport map. + for (cricket::TransportMap::const_iterator iter = transport_proxies().begin(); + iter != transport_proxies().end(); ++iter) { + if (iter->second->impl()) { + return iter->second->impl()->GetSslRole(role); + } + } + return false; +} + void WebRtcSession::CreateOffer(CreateSessionDescriptionObserver* observer, const MediaConstraintsInterface* constraints) { webrtc_session_desc_factory_->CreateOffer(observer, constraints); @@ -517,42 +537,22 @@ void WebRtcSession::CreateAnswer(CreateSessionDescriptionObserver* observer, bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, std::string* err_desc) { - cricket::SecureMediaPolicy secure_policy = - webrtc_session_desc_factory_->secure(); // Takes the ownership of |desc| regardless of the result. talk_base::scoped_ptr desc_temp(desc); - if (error() != cricket::BaseSession::ERROR_NONE) { - return BadLocalSdp(SessionErrorMsg(error()), err_desc); - } - - if (!desc || !desc->description()) { - return BadLocalSdp(kInvalidSdp, err_desc); - } - - if (!VerifyBundleSettings(desc->description())) { - return BadLocalSdp(kBundleWithoutRtcpMux, err_desc); - } - - Action action = GetAction(desc->type()); - if (!ExpectSetLocalDescription(action)) { - std::string type = desc->type(); - return BadLocalSdp(BadStateErrMsg(type, state()), err_desc); - } - if (secure_policy == cricket::SEC_REQUIRED && - !VerifyCrypto(desc->description())) { - return BadLocalSdp(kSdpWithoutCrypto, err_desc); - } - if (action == kAnswer && !VerifyMediaDescriptions( - desc->description(), remote_description()->description())) { - return BadLocalSdp(kMlineMismatch, err_desc); + // Validate SDP. + if (!ValidateSessionDescription(desc, cricket::CS_LOCAL, err_desc)) { + return false; } // Update the initiator flag if this session is the initiator. + Action action = GetAction(desc->type()); if (state() == STATE_INIT && action == kOffer) { set_initiator(true); } + cricket::SecureMediaPolicy secure_policy = + webrtc_session_desc_factory_->secure(); // Update the MediaContentDescription crypto settings as per the policy set. UpdateSessionDescriptionSecurePolicy(secure_policy, desc->description()); @@ -589,40 +589,16 @@ bool WebRtcSession::SetLocalDescription(SessionDescriptionInterface* desc, bool WebRtcSession::SetRemoteDescription(SessionDescriptionInterface* desc, std::string* err_desc) { - cricket::SecureMediaPolicy secure_policy = - webrtc_session_desc_factory_->secure(); // Takes the ownership of |desc| regardless of the result. talk_base::scoped_ptr desc_temp(desc); - if (error() != cricket::BaseSession::ERROR_NONE) { - return BadRemoteSdp(SessionErrorMsg(error()), err_desc); - } - - if (!desc || !desc->description()) { - return BadRemoteSdp(kInvalidSdp, err_desc); - } - - if (!VerifyBundleSettings(desc->description())) { - return BadRemoteSdp(kBundleWithoutRtcpMux, err_desc); - } - - Action action = GetAction(desc->type()); - if (!ExpectSetRemoteDescription(action)) { - std::string type = desc->type(); - return BadRemoteSdp(BadStateErrMsg(type, state()), err_desc); - } - - if (action == kAnswer && !VerifyMediaDescriptions( - desc->description(), local_description()->description())) { - return BadRemoteSdp(kMlineMismatch, err_desc); - } - - if (secure_policy == cricket::SEC_REQUIRED && - !VerifyCrypto(desc->description())) { - return BadRemoteSdp(kSdpWithoutCrypto, err_desc); + // Validate SDP. + if (!ValidateSessionDescription(desc, cricket::CS_REMOTE, err_desc)) { + return false; } // Transport and Media channels will be created only when offer is set. + Action action = GetAction(desc->type()); if (action == kOffer && !CreateChannels(desc->description())) { // TODO(mallinath) - Handle CreateChannel failure, as new local description // is applied. Restore back to old description. @@ -1094,36 +1070,6 @@ void WebRtcSession::OnTransportProxyCandidatesReady( ProcessNewLocalCandidate(proxy->content_name(), candidates); } -bool WebRtcSession::ExpectSetLocalDescription(Action action) { - return ((action == kOffer && state() == STATE_INIT) || - // update local offer - (action == kOffer && state() == STATE_SENTINITIATE) || - // update the current ongoing session. - (action == kOffer && state() == STATE_RECEIVEDACCEPT) || - (action == kOffer && state() == STATE_SENTACCEPT) || - (action == kOffer && state() == STATE_INPROGRESS) || - // accept remote offer - (action == kAnswer && state() == STATE_RECEIVEDINITIATE) || - (action == kAnswer && state() == STATE_SENTPRACCEPT) || - (action == kPrAnswer && state() == STATE_RECEIVEDINITIATE) || - (action == kPrAnswer && state() == STATE_SENTPRACCEPT)); -} - -bool WebRtcSession::ExpectSetRemoteDescription(Action action) { - return ((action == kOffer && state() == STATE_INIT) || - // update remote offer - (action == kOffer && state() == STATE_RECEIVEDINITIATE) || - // update the current ongoing session - (action == kOffer && state() == STATE_RECEIVEDACCEPT) || - (action == kOffer && state() == STATE_SENTACCEPT) || - (action == kOffer && state() == STATE_INPROGRESS) || - // accept local offer - (action == kAnswer && state() == STATE_SENTINITIATE) || - (action == kAnswer && state() == STATE_RECEIVEDPRACCEPT) || - (action == kPrAnswer && state() == STATE_SENTINITIATE) || - (action == kPrAnswer && state() == STATE_RECEIVEDPRACCEPT)); -} - void WebRtcSession::OnCandidatesAllocationDone() { ASSERT(signaling_thread()->IsCurrent()); if (ice_observer_) { @@ -1378,7 +1324,7 @@ void WebRtcSession::OnDataReceived( } // Returns false if bundle is enabled and rtcp_mux is disabled. -bool WebRtcSession::VerifyBundleSettings(const SessionDescription* desc) { +bool WebRtcSession::ValidateBundleSettings(const SessionDescription* desc) { bool bundle_enabled = desc->HasGroup(cricket::GROUP_TYPE_BUNDLE); if (!bundle_enabled) return true; @@ -1409,4 +1355,79 @@ bool WebRtcSession::HasRtcpMuxEnabled( return description->rtcp_mux(); } +bool WebRtcSession::ValidateSessionDescription( + const SessionDescriptionInterface* sdesc, + cricket::ContentSource source, std::string* error_desc) { + + if (error() != cricket::BaseSession::ERROR_NONE) { + return BadSdp(source, SessionErrorMsg(error()), error_desc); + } + + if (!sdesc || !sdesc->description()) { + return BadSdp(source, kInvalidSdp, error_desc); + } + + std::string type = sdesc->type(); + Action action = GetAction(sdesc->type()); + if (source == cricket::CS_LOCAL) { + if (!ExpectSetLocalDescription(action)) + return BadSdp(source, BadStateErrMsg(type, state()), error_desc); + } else { + if (!ExpectSetRemoteDescription(action)) + return BadSdp(source, BadStateErrMsg(type, state()), error_desc); + } + + // Verify crypto settings. + if (webrtc_session_desc_factory_->secure() == cricket::SEC_REQUIRED && + !VerifyCrypto(sdesc->description())) { + return BadSdp(source, kSdpWithoutCrypto, error_desc); + } + + if (!ValidateBundleSettings(sdesc->description())) { + return BadSdp(source, kBundleWithoutRtcpMux, error_desc); + } + + // Verify m-lines in Answer when compared against Offer. + if (action == kAnswer) { + const cricket::SessionDescription* offer_desc = + (source == cricket::CS_LOCAL) ? remote_description()->description() : + local_description()->description(); + if (!VerifyMediaDescriptions(sdesc->description(), offer_desc)) { + return BadSdp(source, kMlineMismatch, error_desc); + } + } + + return true; +} + +bool WebRtcSession::ExpectSetLocalDescription(Action action) { + return ((action == kOffer && state() == STATE_INIT) || + // update local offer + (action == kOffer && state() == STATE_SENTINITIATE) || + // update the current ongoing session. + (action == kOffer && state() == STATE_RECEIVEDACCEPT) || + (action == kOffer && state() == STATE_SENTACCEPT) || + (action == kOffer && state() == STATE_INPROGRESS) || + // accept remote offer + (action == kAnswer && state() == STATE_RECEIVEDINITIATE) || + (action == kAnswer && state() == STATE_SENTPRACCEPT) || + (action == kPrAnswer && state() == STATE_RECEIVEDINITIATE) || + (action == kPrAnswer && state() == STATE_SENTPRACCEPT)); +} + +bool WebRtcSession::ExpectSetRemoteDescription(Action action) { + return ((action == kOffer && state() == STATE_INIT) || + // update remote offer + (action == kOffer && state() == STATE_RECEIVEDINITIATE) || + // update the current ongoing session + (action == kOffer && state() == STATE_RECEIVEDACCEPT) || + (action == kOffer && state() == STATE_SENTACCEPT) || + (action == kOffer && state() == STATE_INPROGRESS) || + // accept local offer + (action == kAnswer && state() == STATE_SENTINITIATE) || + (action == kAnswer && state() == STATE_RECEIVEDPRACCEPT) || + (action == kPrAnswer && state() == STATE_SENTINITIATE) || + (action == kPrAnswer && state() == STATE_RECEIVEDPRACCEPT)); +} + } // namespace webrtc diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 202ca66e12..0cb049f498 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -130,6 +130,9 @@ class WebRtcSession : public cricket::BaseSession, void set_secure_policy(cricket::SecureMediaPolicy secure_policy); cricket::SecureMediaPolicy secure_policy() const; + // Get current ssl role from transport. + bool GetSslRole(talk_base::SSLRole* role); + // Generic error message callback from WebRtcSession. // TODO - It may be necessary to supply error code as well. sigslot::signal0<> SignalError; @@ -152,9 +155,6 @@ class WebRtcSession : public cricket::BaseSession, return remote_desc_.get(); } - void set_secure(cricket::SecureMediaPolicy secure_policy); - cricket::SecureMediaPolicy secure(); - // Get the id used as a media stream track's "id" field from ssrc. virtual bool GetTrackIdBySsrc(uint32 ssrc, std::string* id); @@ -223,10 +223,6 @@ class WebRtcSession : public cricket::BaseSession, const cricket::Candidates& candidates); virtual void OnCandidatesAllocationDone(); - // Check if a call to SetLocalDescription is acceptable with |action|. - bool ExpectSetLocalDescription(Action action); - // Check if a call to SetRemoteDescription is acceptable with |action|. - bool ExpectSetRemoteDescription(Action action); // Creates local session description with audio and video contents. bool CreateDefaultLocalDescription(); // Enables media channels to allow sending of media. @@ -275,8 +271,20 @@ class WebRtcSession : public cricket::BaseSession, std::string BadStateErrMsg(const std::string& type, State state); void SetIceConnectionState(PeerConnectionInterface::IceConnectionState state); - bool VerifyBundleSettings(const cricket::SessionDescription* desc); + bool ValidateBundleSettings(const cricket::SessionDescription* desc); bool HasRtcpMuxEnabled(const cricket::ContentInfo* content); + // Below methods are helper methods which verifies SDP. + bool ValidateSessionDescription(const SessionDescriptionInterface* sdesc, + cricket::ContentSource source, + std::string* error_desc); + + // Check if a call to SetLocalDescription is acceptable with |action|. + bool ExpectSetLocalDescription(Action action); + // Check if a call to SetRemoteDescription is acceptable with |action|. + bool ExpectSetRemoteDescription(Action action); + // Verifies a=setup attribute as per RFC 5763. + bool ValidateDtlsSetupAttribute(const cricket::SessionDescription* desc, + Action action); talk_base::scoped_ptr voice_channel_; talk_base::scoped_ptr video_channel_; diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc index 2021085aa2..13f54a7cc0 100644 --- a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc +++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc @@ -343,6 +343,13 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer( // 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(); + // We should pass current ssl role to the transport description factory, if + // there is already an existing ongoing session. + talk_base::SSLRole ssl_role; + if (session_->GetSslRole(&ssl_role)) { + request.options.transport_options.prefer_passive_role = + (talk_base::SSL_SERVER == ssl_role); + } cricket::SessionDescription* desc(session_desc_factory_.CreateAnswer( static_cast(session_)->remote_description(), diff --git a/talk/base/messagehandler.cc b/talk/base/messagehandler.cc index 5b3585b162..16f9a21a16 100644 --- a/talk/base/messagehandler.cc +++ b/talk/base/messagehandler.cc @@ -2,26 +2,26 @@ * libjingle * Copyright 2004--2005, Google Inc. * - * Redistribution and use in source and binary forms, with or without + * Redistribution and use in source and binary forms, with or without * modification, are permitted provided that the following conditions are met: * - * 1. Redistributions of source code must retain the above copyright notice, + * 1. Redistributions of source code must retain the above copyright notice, * this list of conditions and the following disclaimer. * 2. Redistributions in binary form must reproduce the above copyright notice, * this list of conditions and the following disclaimer in the documentation * and/or other materials provided with the distribution. - * 3. The name of the author may not be used to endorse or promote products + * 3. The name of the author may not be used to endorse or promote products * derived from this software without specific prior written permission. * * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO - * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, - * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR - * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ @@ -31,7 +31,7 @@ namespace talk_base { MessageHandler::~MessageHandler() { - MessageQueueManager::Instance()->Clear(this); + MessageQueueManager::Clear(this); } } // namespace talk_base diff --git a/talk/base/messagequeue.cc b/talk/base/messagequeue.cc index 5c40622162..15b700ffe4 100644 --- a/talk/base/messagequeue.cc +++ b/talk/base/messagequeue.cc @@ -42,7 +42,7 @@ const uint32 kMaxMsgLatency = 150; // 150 ms //------------------------------------------------------------------ // MessageQueueManager -MessageQueueManager* MessageQueueManager::instance_; +MessageQueueManager* MessageQueueManager::instance_ = NULL; MessageQueueManager* MessageQueueManager::Instance() { // Note: This is not thread safe, but it is first called before threads are @@ -52,6 +52,10 @@ MessageQueueManager* MessageQueueManager::Instance() { return instance_; } +bool MessageQueueManager::IsInitialized() { + return instance_ != NULL; +} + MessageQueueManager::MessageQueueManager() { } @@ -59,6 +63,9 @@ MessageQueueManager::~MessageQueueManager() { } void MessageQueueManager::Add(MessageQueue *message_queue) { + return Instance()->AddInternal(message_queue); +} +void MessageQueueManager::AddInternal(MessageQueue *message_queue) { // MessageQueueManager methods should be non-reentrant, so we // ASSERT that is the case. If any of these ASSERT, please // contact bpm or jbeda. @@ -68,6 +75,12 @@ void MessageQueueManager::Add(MessageQueue *message_queue) { } void MessageQueueManager::Remove(MessageQueue *message_queue) { + // If there isn't a message queue manager instance, then there isn't a queue + // to remove. + if (!instance_) return; + return Instance()->RemoveInternal(message_queue); +} +void MessageQueueManager::RemoveInternal(MessageQueue *message_queue) { ASSERT(!crit_.CurrentThreadIsOwner()); // See note above. // If this is the last MessageQueue, destroy the manager as well so that // we don't leak this object at program shutdown. As mentioned above, this is @@ -91,6 +104,12 @@ void MessageQueueManager::Remove(MessageQueue *message_queue) { } void MessageQueueManager::Clear(MessageHandler *handler) { + // If there isn't a message queue manager instance, then there aren't any + // queues to remove this handler from. + if (!instance_) return; + return Instance()->ClearInternal(handler); +} +void MessageQueueManager::ClearInternal(MessageHandler *handler) { ASSERT(!crit_.CurrentThreadIsOwner()); // See note above. CritScope cs(&crit_); std::vector::iterator iter; @@ -122,7 +141,7 @@ MessageQueue::~MessageQueue() { // is going away. SignalQueueDestroyed(); if (active_) { - MessageQueueManager::Instance()->Remove(this); + MessageQueueManager::Remove(this); Clear(NULL); } if (ss_) { @@ -381,7 +400,7 @@ void MessageQueue::EnsureActive() { ASSERT(crit_.CurrentThreadIsOwner()); if (!active_) { active_ = true; - MessageQueueManager::Instance()->Add(this); + MessageQueueManager::Add(this); } } diff --git a/talk/base/messagequeue.h b/talk/base/messagequeue.h index 331f207363..7b38ba0082 100644 --- a/talk/base/messagequeue.h +++ b/talk/base/messagequeue.h @@ -53,16 +53,26 @@ class MessageQueue; class MessageQueueManager { public: - static MessageQueueManager* Instance(); + static void Add(MessageQueue *message_queue); + static void Remove(MessageQueue *message_queue); + static void Clear(MessageHandler *handler); - void Add(MessageQueue *message_queue); - void Remove(MessageQueue *message_queue); - void Clear(MessageHandler *handler); + // For testing purposes, we expose whether or not the MessageQueueManager + // instance has been initialized. It has no other use relative to the rest of + // the functions of this class, which auto-initialize the underlying + // MessageQueueManager instance when necessary. + static bool IsInitialized(); private: + static MessageQueueManager* Instance(); + MessageQueueManager(); ~MessageQueueManager(); + void AddInternal(MessageQueue *message_queue); + void RemoveInternal(MessageQueue *message_queue); + void ClearInternal(MessageHandler *handler); + static MessageQueueManager* instance_; // This list contains 'active' MessageQueues. std::vector message_queues_; diff --git a/talk/base/messagequeue_unittest.cc b/talk/base/messagequeue_unittest.cc index 8e55548222..55c016685c 100644 --- a/talk/base/messagequeue_unittest.cc +++ b/talk/base/messagequeue_unittest.cc @@ -130,3 +130,10 @@ TEST_F(MessageQueueTest, DiposeHandlerWithPostedMessagePending) { EXPECT_TRUE(deleted); } +TEST(MessageQueueManager, DISABLED_Clear) { + bool deleted = false; + DeletedMessageHandler* handler = new DeletedMessageHandler(&deleted); + delete handler; + EXPECT_TRUE(deleted); + EXPECT_FALSE(MessageQueueManager::IsInitialized()); +} diff --git a/talk/media/base/videocapturer.cc b/talk/media/base/videocapturer.cc index 3bc23731db..0cef81b542 100644 --- a/talk/media/base/videocapturer.cc +++ b/talk/media/base/videocapturer.cc @@ -338,7 +338,7 @@ void VideoCapturer::OnFrameCaptured(VideoCapturer*, scaled_height_ = scaled_height; } if (FOURCC_ARGB == captured_frame->fourcc && - (scaled_width != captured_frame->height || + (scaled_width != captured_frame->width || scaled_height != captured_frame->height)) { CapturedFrame* scaled_frame = const_cast(captured_frame); // Compute new width such that width * height is less than maximum but diff --git a/talk/media/base/videocapturer_unittest.cc b/talk/media/base/videocapturer_unittest.cc index a6ce3ba9b3..ae461a9845 100644 --- a/talk/media/base/videocapturer_unittest.cc +++ b/talk/media/base/videocapturer_unittest.cc @@ -681,3 +681,29 @@ TEST_F(VideoCapturerTest, Whitelist) { capturer_.ConstrainSupportedFormats(vga_format); EXPECT_TRUE(HdFormatInList(*capturer_.GetSupportedFormats())); } + +TEST_F(VideoCapturerTest, BlacklistAllFormats) { + cricket::VideoFormat vga_format(640, 480, + cricket::VideoFormat::FpsToInterval(30), + cricket::FOURCC_I420); + std::vector supported_formats; + // Mock a device that only supports HD formats. + supported_formats.push_back(cricket::VideoFormat(1280, 720, + cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420)); + supported_formats.push_back(cricket::VideoFormat(1920, 1080, + cricket::VideoFormat::FpsToInterval(30), cricket::FOURCC_I420)); + capturer_.ResetSupportedFormats(supported_formats); + EXPECT_EQ(2u, capturer_.GetSupportedFormats()->size()); + // Now, enable the list, which would exclude both formats. However, since + // only HD formats are available, we refuse to filter at all, so we don't + // break this camera. + capturer_.set_enable_camera_list(true); + capturer_.ConstrainSupportedFormats(vga_format); + EXPECT_EQ(2u, capturer_.GetSupportedFormats()->size()); + // To make sure it's not just the camera list being broken, add in VGA and + // try again. This time, only the VGA format should be there. + supported_formats.push_back(vga_format); + capturer_.ResetSupportedFormats(supported_formats); + ASSERT_EQ(1u, capturer_.GetSupportedFormats()->size()); + EXPECT_EQ(vga_format.height, capturer_.GetSupportedFormats()->at(0).height); +} diff --git a/talk/p2p/base/constants.cc b/talk/p2p/base/constants.cc index 12336d4101..27d43096b9 100644 --- a/talk/p2p/base/constants.cc +++ b/talk/p2p/base/constants.cc @@ -262,4 +262,10 @@ const char NS_VOICEMAIL[] = "http://www.google.com/session/voicemail"; const buzz::StaticQName QN_VOICEMAIL_REGARDING = { NS_VOICEMAIL, "regarding" }; #endif +// From RFC 4145, SDP setup attribute values. +const char CONNECTIONROLE_ACTIVE_STR[] = "active"; +const char CONNECTIONROLE_PASSIVE_STR[] = "passive"; +const char CONNECTIONROLE_ACTPASS_STR[] = "actpass"; +const char CONNECTIONROLE_HOLDCONN_STR[] = "holdconn"; + } // namespace cricket diff --git a/talk/p2p/base/constants.h b/talk/p2p/base/constants.h index f7e5671b26..99e006a018 100644 --- a/talk/p2p/base/constants.h +++ b/talk/p2p/base/constants.h @@ -261,6 +261,12 @@ extern const char NS_VOICEMAIL[]; extern const buzz::StaticQName QN_VOICEMAIL_REGARDING; #endif +// RFC 4145, SDP setup attribute values. +extern const char CONNECTIONROLE_ACTIVE_STR[]; +extern const char CONNECTIONROLE_PASSIVE_STR[]; +extern const char CONNECTIONROLE_ACTPASS_STR[]; +extern const char CONNECTIONROLE_HOLDCONN_STR[]; + } // namespace cricket #endif // TALK_P2P_BASE_CONSTANTS_H_ diff --git a/talk/p2p/base/dtlstransport.h b/talk/p2p/base/dtlstransport.h index a6e3b82c1f..61e78f7594 100644 --- a/talk/p2p/base/dtlstransport.h +++ b/talk/p2p/base/dtlstransport.h @@ -55,7 +55,6 @@ class DtlsTransport : public Base { ~DtlsTransport() { Base::DestroyAllChannels(); } - virtual void SetIdentity_w(talk_base::SSLIdentity* identity) { identity_ = identity; } @@ -100,6 +99,74 @@ class DtlsTransport : public Base { if (remote_fp && local_fp) { remote_fingerprint_.reset(new talk_base::SSLFingerprint(*remote_fp)); + + // From RFC 4145, section-4.1, The following are the values that the + // 'setup' attribute can take in an offer/answer exchange: + // Offer Answer + // ________________ + // active passive / holdconn + // passive active / holdconn + // actpass active / passive / holdconn + // holdconn holdconn + // + // Set the role that is most conformant with RFC 5763, Section 5, bullet 1 + // The endpoint MUST use the setup attribute defined in [RFC4145]. + // The endpoint that is the offerer MUST use the setup attribute + // value of setup:actpass and be prepared to receive a client_hello + // before it receives the answer. The answerer MUST use either a + // setup attribute value of setup:active or setup:passive. Note that + // if the answerer uses setup:passive, then the DTLS handshake will + // not begin until the answerer is received, which adds additional + // latency. setup:active allows the answer and the DTLS handshake to + // occur in parallel. Thus, setup:active is RECOMMENDED. Whichever + // party is active MUST initiate a DTLS handshake by sending a + // ClientHello over each flow (host/port quartet). + // IOW - actpass and passive modes should be treated as server and + // active as client. + ConnectionRole local_connection_role = + Base::local_description()->connection_role; + ConnectionRole remote_connection_role = + Base::remote_description()->connection_role; + + bool is_remote_server = false; + if (local_role == CA_OFFER) { + if (local_connection_role != CONNECTIONROLE_ACTPASS) { + LOG(LS_ERROR) << "Offerer must use actpass value for setup attribute"; + return false; + } + + if (remote_connection_role == CONNECTIONROLE_ACTIVE || + remote_connection_role == CONNECTIONROLE_PASSIVE || + remote_connection_role == CONNECTIONROLE_NONE) { + is_remote_server = (remote_connection_role == CONNECTIONROLE_PASSIVE); + } else { + LOG(LS_ERROR) << "Answerer must use either active or passive value " + << "for setup attribute"; + return false; + } + // If remote is NONE or ACTIVE it will act as client. + } else { + if (remote_connection_role != CONNECTIONROLE_ACTPASS && + remote_connection_role != CONNECTIONROLE_NONE) { + LOG(LS_ERROR) << "Offerer must use actpass value for setup attribute"; + return false; + } + + if (local_connection_role == CONNECTIONROLE_ACTIVE || + local_connection_role == CONNECTIONROLE_PASSIVE) { + is_remote_server = (local_connection_role == CONNECTIONROLE_ACTIVE); + } else { + LOG(LS_ERROR) << "Answerer must use either active or passive value " + << "for setup attribute"; + return false; + } + + // If local is passive, local will act as server. + } + + secure_role_ = is_remote_server ? talk_base::SSL_CLIENT : + talk_base::SSL_SERVER; + } else if (local_fp && (local_role == CA_ANSWER)) { LOG(LS_ERROR) << "Local fingerprint supplied when caller didn't offer DTLS"; @@ -128,18 +195,34 @@ class DtlsTransport : public Base { Base::DestroyTransportChannel(base_channel); } + virtual bool GetSslRole_w(talk_base::SSLRole* ssl_role) const { + ASSERT(ssl_role != NULL); + *ssl_role = secure_role_; + return true; + } + private: - virtual void ApplyNegotiatedTransportDescription_w( + virtual bool ApplyNegotiatedTransportDescription_w( TransportChannelImpl* channel) { - channel->SetRemoteFingerprint( + // Set ssl role. Role must be set before fingerprint is applied, which + // initiates DTLS setup. + if (!channel->SetSslRole(secure_role_)) { + LOG(LS_INFO) << "Failed to set ssl role for the channel."; + return false; + } + // Apply remote fingerprint. + if (!channel->SetRemoteFingerprint( remote_fingerprint_->algorithm, reinterpret_cast(remote_fingerprint_-> digest.data()), - remote_fingerprint_->digest.length()); - Base::ApplyNegotiatedTransportDescription_w(channel); + remote_fingerprint_->digest.length())) { + return false; + } + return Base::ApplyNegotiatedTransportDescription_w(channel); } talk_base::SSLIdentity* identity_; + talk_base::SSLRole secure_role_; talk_base::scoped_ptr remote_fingerprint_; }; diff --git a/talk/p2p/base/dtlstransportchannel.cc b/talk/p2p/base/dtlstransportchannel.cc index 6cf400c97e..40b4e31ea2 100644 --- a/talk/p2p/base/dtlstransportchannel.cc +++ b/talk/p2p/base/dtlstransportchannel.cc @@ -102,7 +102,7 @@ DtlsTransportChannelWrapper::DtlsTransportChannelWrapper( downward_(NULL), dtls_state_(STATE_NONE), local_identity_(NULL), - dtls_role_(talk_base::SSL_CLIENT) { + ssl_role_(talk_base::SSL_CLIENT) { channel_->SignalReadableState.connect(this, &DtlsTransportChannelWrapper::OnReadableState); channel_->SignalWritableState.connect(this, @@ -171,18 +171,22 @@ bool DtlsTransportChannelWrapper::SetLocalIdentity( return true; } -void DtlsTransportChannelWrapper::SetIceRole(IceRole role) { - // TODO(ekr@rtfm.com): Forbid this if Connect() has been called. - ASSERT(dtls_state_ < STATE_ACCEPTED); +bool DtlsTransportChannelWrapper::SetSslRole(talk_base::SSLRole role) { + if (dtls_state_ == STATE_OPEN) { + if (ssl_role_ != role) { + LOG(LS_ERROR) << "SSL Role can't be reversed after the session is setup."; + return false; + } + return true; + } - // Set the role that is most conformant with RFC 5763, Section 5, bullet 1: - // The endpoint that is the offerer MUST [...] be prepared to receive - // a client_hello before it receives the answer. - // (IOW, the offerer is the server, and the answerer is the client). - dtls_role_ = (role == ICEROLE_CONTROLLING) ? - talk_base::SSL_SERVER : talk_base::SSL_CLIENT; + ssl_role_ = role; + return true; +} - channel_->SetIceRole(role); +bool DtlsTransportChannelWrapper::GetSslRole(talk_base::SSLRole* role) const { + *role = ssl_role_; + return true; } bool DtlsTransportChannelWrapper::SetRemoteFingerprint( @@ -201,12 +205,12 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint( // hasn't been called. if (dtls_state_ > STATE_OFFERED || (dtls_state_ == STATE_NONE && !digest_alg.empty())) { - LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state"; + LOG_J(LS_ERROR, this) << "Can't set DTLS remote settings in this state."; return false; } if (digest_alg.empty()) { - LOG_J(LS_INFO, this) << "Other side didn't support DTLS"; + LOG_J(LS_INFO, this) << "Other side didn't support DTLS."; dtls_state_ = STATE_NONE; return true; } @@ -230,7 +234,7 @@ bool DtlsTransportChannelWrapper::SetupDtls() { dtls_.reset(talk_base::SSLStreamAdapter::Create(downward)); if (!dtls_) { - LOG_J(LS_ERROR, this) << "Failed to create DTLS adapter"; + LOG_J(LS_ERROR, this) << "Failed to create DTLS adapter."; delete downward; return false; } @@ -239,27 +243,27 @@ bool DtlsTransportChannelWrapper::SetupDtls() { dtls_->SetIdentity(local_identity_->GetReference()); dtls_->SetMode(talk_base::SSL_MODE_DTLS); - dtls_->SetServerRole(dtls_role_); + dtls_->SetServerRole(ssl_role_); dtls_->SignalEvent.connect(this, &DtlsTransportChannelWrapper::OnDtlsEvent); if (!dtls_->SetPeerCertificateDigest( remote_fingerprint_algorithm_, reinterpret_cast(remote_fingerprint_value_.data()), remote_fingerprint_value_.length())) { - LOG_J(LS_ERROR, this) << "Couldn't set DTLS certificate digest"; + LOG_J(LS_ERROR, this) << "Couldn't set DTLS certificate digest."; return false; } // Set up DTLS-SRTP, if it's been enabled. if (!srtp_ciphers_.empty()) { if (!dtls_->SetDtlsSrtpCiphers(srtp_ciphers_)) { - LOG_J(LS_ERROR, this) << "Couldn't set DTLS-SRTP ciphers"; + LOG_J(LS_ERROR, this) << "Couldn't set DTLS-SRTP ciphers."; return false; } } else { - LOG_J(LS_INFO, this) << "Not using DTLS"; + LOG_J(LS_INFO, this) << "Not using DTLS."; } - LOG_J(LS_INFO, this) << "DTLS setup complete"; + LOG_J(LS_INFO, this) << "DTLS setup complete."; return true; } @@ -349,7 +353,7 @@ void DtlsTransportChannelWrapper::OnReadableState(TransportChannel* channel) { ASSERT(talk_base::Thread::Current() == worker_thread_); ASSERT(channel == channel_); LOG_J(LS_VERBOSE, this) - << "DTLSTransportChannelWrapper: channel readable state changed"; + << "DTLSTransportChannelWrapper: channel readable state changed."; if (dtls_state_ == STATE_NONE || dtls_state_ == STATE_OPEN) { set_readable(channel_->readable()); @@ -361,7 +365,7 @@ void DtlsTransportChannelWrapper::OnWritableState(TransportChannel* channel) { ASSERT(talk_base::Thread::Current() == worker_thread_); ASSERT(channel == channel_); LOG_J(LS_VERBOSE, this) - << "DTLSTransportChannelWrapper: channel writable state changed"; + << "DTLSTransportChannelWrapper: channel writable state changed."; switch (dtls_state_) { case STATE_NONE: @@ -416,13 +420,13 @@ void DtlsTransportChannelWrapper::OnReadPacket(TransportChannel* channel, // decide to take this as evidence that the other // side is ready to do DTLS and start the handshake // on our end - LOG_J(LS_WARNING, this) << "Received packet before we know if we are doing " - << "DTLS or not; dropping"; + LOG_J(LS_WARNING, this) << "Received packet before we know if we are " + << "doing DTLS or not; dropping."; break; case STATE_ACCEPTED: // Drop packets received before DTLS has actually started - LOG_J(LS_INFO, this) << "Dropping packet received before DTLS started"; + LOG_J(LS_INFO, this) << "Dropping packet received before DTLS started."; break; case STATE_STARTED: @@ -431,19 +435,20 @@ void DtlsTransportChannelWrapper::OnReadPacket(TransportChannel* channel, // Is this potentially a DTLS packet? if (IsDtlsPacket(data, size)) { if (!HandleDtlsPacket(data, size)) { - LOG_J(LS_ERROR, this) << "Failed to handle DTLS packet"; + LOG_J(LS_ERROR, this) << "Failed to handle DTLS packet."; return; } } else { // Not a DTLS packet; our handshake should be complete by now. if (dtls_state_ != STATE_OPEN) { - LOG_J(LS_ERROR, this) << "Received non-DTLS packet before DTLS complete"; + LOG_J(LS_ERROR, this) << "Received non-DTLS packet before DTLS " + << "complete."; return; } // And it had better be a SRTP packet. if (!IsRtpPacket(data, size)) { - LOG_J(LS_ERROR, this) << "Received unexpected non-DTLS packet"; + LOG_J(LS_ERROR, this) << "Received unexpected non-DTLS packet."; return; } @@ -472,7 +477,7 @@ void DtlsTransportChannelWrapper::OnDtlsEvent(talk_base::StreamInterface* dtls, ASSERT(dtls == dtls_.get()); if (sig & talk_base::SE_OPEN) { // This is the first time. - LOG_J(LS_INFO, this) << "DTLS handshake complete"; + LOG_J(LS_INFO, this) << "DTLS handshake complete."; if (dtls_->GetState() == talk_base::SS_OPEN) { // The check for OPEN shouldn't be necessary but let's make // sure we don't accidentally frob the state if it's closed. diff --git a/talk/p2p/base/dtlstransportchannel.h b/talk/p2p/base/dtlstransportchannel.h index 8321024df2..ed0db68779 100644 --- a/talk/p2p/base/dtlstransportchannel.h +++ b/talk/p2p/base/dtlstransportchannel.h @@ -121,8 +121,9 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { TransportChannelImpl* channel); virtual ~DtlsTransportChannelWrapper(); - virtual void SetIceRole(IceRole ice_role); - // Returns current transport role of the channel. + virtual void SetIceRole(IceRole role) { + channel_->SetIceRole(role); + } virtual IceRole GetIceRole() const { return channel_->GetIceRole(); } @@ -158,6 +159,9 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { // Find out which DTLS-SRTP cipher was negotiated virtual bool GetSrtpCipher(std::string* cipher); + virtual bool GetSslRole(talk_base::SSLRole* role) const; + virtual bool SetSslRole(talk_base::SSLRole role); + // Once DTLS has established (i.e., this channel is writable), this method // extracts the keys negotiated during the DTLS handshake, for use in external // encryption. DTLS-SRTP uses this to extract the needed SRTP keys. @@ -234,7 +238,7 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { std::vector srtp_ciphers_; // SRTP ciphers to use with DTLS. State dtls_state_; talk_base::SSLIdentity* local_identity_; - talk_base::SSLRole dtls_role_; + talk_base::SSLRole ssl_role_; talk_base::Buffer remote_fingerprint_value_; std::string remote_fingerprint_algorithm_; diff --git a/talk/p2p/base/dtlstransportchannel_unittest.cc b/talk/p2p/base/dtlstransportchannel_unittest.cc index c46839f5ec..750710100d 100644 --- a/talk/p2p/base/dtlstransportchannel_unittest.cc +++ b/talk/p2p/base/dtlstransportchannel_unittest.cc @@ -56,6 +56,10 @@ static bool IsRtpLeadByte(uint8 b) { return ((b & 0xC0) == 0x80); } +using cricket::ConnectionRole; + +enum Flags { NF_REOFFER = 0x1, NF_EXPECT_FAILURE = 0x2 }; + class DtlsTestClient : public sigslot::has_slots<> { public: DtlsTestClient(const std::string& name, @@ -77,6 +81,7 @@ class DtlsTestClient : public sigslot::has_slots<> { void CreateIdentity() { identity_.reset(talk_base::SSLIdentity::Generate(name_)); } + talk_base::SSLIdentity* identity() { return identity_.get(); } void SetupSrtp() { ASSERT(identity_.get() != NULL); use_dtls_srtp_ = true; @@ -108,6 +113,9 @@ class DtlsTestClient : public sigslot::has_slots<> { this, &DtlsTestClient::OnFakeTransportChannelReadPacket); } } + + cricket::Transport* transport() { return transport_.get(); } + cricket::FakeTransportChannel* GetFakeChannel(int component) { cricket::TransportChannelImpl* ch = transport_->GetChannel(component); cricket::DtlsTransportChannelWrapper* wrapper = @@ -118,13 +126,20 @@ class DtlsTestClient : public sigslot::has_slots<> { // Offer DTLS if we have an identity; pass in a remote fingerprint only if // both sides support DTLS. - void Negotiate(DtlsTestClient* peer) { - Negotiate(identity_.get(), (identity_) ? peer->identity_.get() : NULL); + void Negotiate(DtlsTestClient* peer, cricket::ContentAction action, + ConnectionRole local_role, ConnectionRole remote_role, + int flags) { + Negotiate(identity_.get(), (identity_) ? peer->identity_.get() : NULL, + action, local_role, remote_role, flags); } // Allow any DTLS configuration to be specified (including invalid ones). void Negotiate(talk_base::SSLIdentity* local_identity, - talk_base::SSLIdentity* remote_identity) { + talk_base::SSLIdentity* remote_identity, + cricket::ContentAction action, + ConnectionRole local_role, + ConnectionRole remote_role, + int flags) { talk_base::scoped_ptr local_fingerprint; talk_base::scoped_ptr remote_fingerprint; if (local_identity) { @@ -137,7 +152,9 @@ class DtlsTestClient : public sigslot::has_slots<> { talk_base::DIGEST_SHA_1, remote_identity)); ASSERT_TRUE(remote_fingerprint.get() != NULL); } - if (use_dtls_srtp_) { + + if (use_dtls_srtp_ && !(flags & NF_REOFFER)) { + // SRTP ciphers will be set only in the beginning. for (std::vector::iterator it = channels_.begin(); it != channels_.end(); ++it) { std::vector ciphers; @@ -150,17 +167,32 @@ class DtlsTestClient : public sigslot::has_slots<> { cricket::NS_GINGLE_P2P : cricket::NS_JINGLE_ICE_UDP; cricket::TransportDescription local_desc( transport_type, std::vector(), kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, local_fingerprint.get(), + cricket::ICEMODE_FULL, local_role, + // If remote if the offerer and has no DTLS support, answer will be + // without any fingerprint. + (action == cricket::CA_ANSWER && !remote_identity) ? + NULL : local_fingerprint.get(), cricket::Candidates()); - ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, - cricket::CA_OFFER)); + cricket::TransportDescription remote_desc( transport_type, std::vector(), kIceUfrag1, kIcePwd1, - cricket::ICEMODE_FULL, remote_fingerprint.get(), + cricket::ICEMODE_FULL, remote_role, remote_fingerprint.get(), cricket::Candidates()); - ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, - cricket::CA_ANSWER)); + bool expect_success = (flags & NF_EXPECT_FAILURE) ? false : true; + // If |expect_success| is false, expect SRTD or SLTD to fail when + // content action is CA_ANSWER. + if (action == cricket::CA_OFFER) { + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, cricket::CA_OFFER)); + ASSERT_EQ(expect_success, transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_ANSWER)); + } else { + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_OFFER)); + ASSERT_EQ(expect_success, transport_->SetLocalTransportDescription( + local_desc, cricket::CA_ANSWER)); + } negotiated_dtls_ = (local_identity && remote_identity); } @@ -373,8 +405,8 @@ class DtlsTransportChannelTest : public testing::Test { use_dtls_srtp_ = true; } - bool Connect() { - Negotiate(); + bool Connect(ConnectionRole client1_role, ConnectionRole client2_role) { + Negotiate(client1_role, client2_role); bool rv = client1_.Connect(&client2_); EXPECT_TRUE(rv); @@ -387,8 +419,20 @@ class DtlsTransportChannelTest : public testing::Test { // Check that we used the right roles. if (use_dtls_) { - client1_.CheckRole(talk_base::SSL_SERVER); - client2_.CheckRole(talk_base::SSL_CLIENT); + talk_base::SSLRole client1_ssl_role = + (client1_role == cricket::CONNECTIONROLE_ACTIVE || + (client2_role == cricket::CONNECTIONROLE_PASSIVE && + client1_role == cricket::CONNECTIONROLE_ACTPASS)) ? + talk_base::SSL_CLIENT : talk_base::SSL_SERVER; + + talk_base::SSLRole client2_ssl_role = + (client2_role == cricket::CONNECTIONROLE_ACTIVE || + (client1_role == cricket::CONNECTIONROLE_PASSIVE && + client2_role == cricket::CONNECTIONROLE_ACTPASS)) ? + talk_base::SSL_CLIENT : talk_base::SSL_SERVER; + + client1_.CheckRole(client1_ssl_role); + client2_.CheckRole(client2_ssl_role); } // Check that we negotiated the right ciphers. @@ -402,11 +446,55 @@ class DtlsTransportChannelTest : public testing::Test { return true; } + + bool Connect() { + // By default, Client1 will be Server and Client2 will be Client. + return Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_ACTIVE); + } + void Negotiate() { + Negotiate(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE); + } + + void Negotiate(ConnectionRole client1_role, ConnectionRole client2_role) { client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); - client2_.Negotiate(&client1_); - client1_.Negotiate(&client2_); + // Expect success from SLTD and SRTD. + client1_.Negotiate(&client2_, cricket::CA_OFFER, + client1_role, client2_role, 0); + client2_.Negotiate(&client1_, cricket::CA_ANSWER, + client2_role, client1_role, 0); + } + + // Negotiate with legacy client |client2|. Legacy client doesn't use setup + // attributes, except NONE. + void NegotiateWithLegacy() { + client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); + client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); + // Expect success from SLTD and SRTD. + client1_.Negotiate(&client2_, cricket::CA_OFFER, + cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_NONE, 0); + client2_.Negotiate(&client1_, cricket::CA_ANSWER, + cricket::CONNECTIONROLE_ACTIVE, + cricket::CONNECTIONROLE_NONE, 0); + } + + void Renegotiate(DtlsTestClient* reoffer_initiator, + ConnectionRole client1_role, ConnectionRole client2_role, + int flags) { + if (reoffer_initiator == &client1_) { + client1_.Negotiate(&client2_, cricket::CA_OFFER, + client1_role, client2_role, flags); + client2_.Negotiate(&client1_, cricket::CA_ANSWER, + client2_role, client1_role, flags); + } else { + client2_.Negotiate(&client1_, cricket::CA_OFFER, + client2_role, client1_role, flags); + client1_.Negotiate(&client2_, cricket::CA_ANSWER, + client1_role, client2_role, flags); + } } void TestTransfer(size_t channel, size_t size, size_t count, bool srtp) { @@ -568,3 +656,96 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { TestTransfer(0, 1000, 100, false); TestTransfer(0, 1000, 100, true); } + +// Testing when the remote is passive. +TEST_F(DtlsTransportChannelTest, TestTransferDtlsAnswererIsPassive) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + SetChannelCount(2); + PrepareDtls(true, true); + PrepareDtlsSrtp(true, true); + ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_PASSIVE)); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); +} + +// Testing with the legacy DTLS client which doesn't use setup attribute. +// In this case legacy is the answerer. +TEST_F(DtlsTransportChannelTest, TestDtlsSetupWithLegacyAsAnswerer) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + PrepareDtls(true, true); + NegotiateWithLegacy(); + talk_base::SSLRole channel1_role; + talk_base::SSLRole channel2_role; + EXPECT_TRUE(client1_.transport()->GetSslRole(&channel1_role)); + EXPECT_TRUE(client2_.transport()->GetSslRole(&channel2_role)); + EXPECT_EQ(talk_base::SSL_SERVER, channel1_role); + EXPECT_EQ(talk_base::SSL_CLIENT, channel2_role); +} + +// Testing re offer/answer after the session is estbalished. Roles will be +// kept same as of the previous negotiation. +TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromOfferer) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + SetChannelCount(2); + PrepareDtls(true, true); + PrepareDtlsSrtp(true, true); + // Initial role for client1 is ACTPASS and client2 is ACTIVE. + ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_ACTIVE)); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); + // Using input roles for the re-offer. + Renegotiate(&client1_, cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_ACTIVE, NF_REOFFER); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); +} + +TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromAnswerer) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + SetChannelCount(2); + PrepareDtls(true, true); + PrepareDtlsSrtp(true, true); + // Initial role for client1 is ACTPASS and client2 is ACTIVE. + ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_ACTIVE)); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); + // Using input roles for the re-offer. + Renegotiate(&client2_, cricket::CONNECTIONROLE_PASSIVE, + cricket::CONNECTIONROLE_ACTPASS, NF_REOFFER); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); +} + +// Test that any change in role after the intial setup will result in failure. +TEST_F(DtlsTransportChannelTest, TestDtlsRoleReversal) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + SetChannelCount(2); + PrepareDtls(true, true); + PrepareDtlsSrtp(true, true); + ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_PASSIVE)); + + // Renegotiate from client2 with actpass and client1 as active. + Renegotiate(&client2_, cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_ACTIVE, + NF_REOFFER | NF_EXPECT_FAILURE); +} + +// Test that using different setup attributes which results in similar ssl +// role as the initial negotiation will result in success. +TEST_F(DtlsTransportChannelTest, TestDtlsReOfferWithDifferentSetupAttr) { + MAYBE_SKIP_TEST(HaveDtlsSrtp); + SetChannelCount(2); + PrepareDtls(true, true); + PrepareDtlsSrtp(true, true); + ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, + cricket::CONNECTIONROLE_PASSIVE)); + // Renegotiate from client2 with actpass and client1 as active. + Renegotiate(&client2_, cricket::CONNECTIONROLE_ACTIVE, + cricket::CONNECTIONROLE_ACTPASS, NF_REOFFER); + TestTransfer(0, 1000, 100, true); + TestTransfer(1, 1000, 100, true); +} diff --git a/talk/p2p/base/fakesession.h b/talk/p2p/base/fakesession.h index 6b96c60e63..d3da2b27f7 100644 --- a/talk/p2p/base/fakesession.h +++ b/talk/p2p/base/fakesession.h @@ -117,6 +117,14 @@ class FakeTransportChannel : public TransportChannelImpl, dtls_fingerprint_ = talk_base::SSLFingerprint(alg, digest, digest_len); return true; } + virtual bool SetSslRole(talk_base::SSLRole role) { + ssl_role_ = role; + return true; + } + virtual bool GetSslRole(talk_base::SSLRole* role) const { + *role = ssl_role_; + return true; + } virtual void Connect() { if (state_ == STATE_INIT) { @@ -275,6 +283,7 @@ class FakeTransportChannel : public TransportChannelImpl, std::string remote_ice_pwd_; IceMode remote_ice_mode_; talk_base::SSLFingerprint dtls_fingerprint_; + talk_base::SSLRole ssl_role_; }; // Fake transport class, which can be passed to anything that needs a Transport. diff --git a/talk/p2p/base/p2ptransportchannel.cc b/talk/p2p/base/p2ptransportchannel.cc index 7a72d1008e..1fd939bb7a 100644 --- a/talk/p2p/base/p2ptransportchannel.cc +++ b/talk/p2p/base/p2ptransportchannel.cc @@ -427,6 +427,15 @@ void P2PTransportChannel::OnUnknownAddress( const Candidate* candidate = NULL; bool known_username = false; std::string remote_password; + + // If we have not received any candidates from remote yet, as it can happen + // in case of trickle, but we have received remote ice_ufrag in O/A, we should + // check against it. + if (!remote_ice_ufrag_.empty() && (remote_username == remote_ice_ufrag_)) { + remote_password = remote_ice_pwd_; + known_username = true; + } + for (it = remote_candidates_.begin(); it != remote_candidates_.end(); ++it) { if (it->username() == remote_username) { remote_password = it->password(); diff --git a/talk/p2p/base/p2ptransportchannel.h b/talk/p2p/base/p2ptransportchannel.h index 74a4483801..17a489fc3b 100644 --- a/talk/p2p/base/p2ptransportchannel.h +++ b/talk/p2p/base/p2ptransportchannel.h @@ -104,6 +104,51 @@ class P2PTransportChannel : public TransportChannelImpl, IceMode remote_ice_mode() const { return remote_ice_mode_; } + // DTLS methods. + virtual bool IsDtlsActive() const { return false; } + + // Default implementation. + virtual bool GetSslRole(talk_base::SSLRole* role) const { + return false; + } + + virtual bool SetSslRole(talk_base::SSLRole role) { + return false; + } + + // Set up the ciphers to use for DTLS-SRTP. + virtual bool SetSrtpCiphers(const std::vector& ciphers) { + return false; + } + + // Find out which DTLS-SRTP cipher was negotiated + virtual bool GetSrtpCipher(std::string* cipher) { + return false; + } + + // Allows key material to be extracted for external encryption. + virtual bool ExportKeyingMaterial( + const std::string& label, + const uint8* context, + size_t context_len, + bool use_context, + uint8* result, + size_t result_len) { + return false; + } + + virtual bool SetLocalIdentity(talk_base::SSLIdentity* identity) { + return false; + } + + // Set DTLS Remote fingerprint. Must be after local identity set. + virtual bool SetRemoteFingerprint( + const std::string& digest_alg, + const uint8* digest, + size_t digest_len) { + return false; + } + private: talk_base::Thread* thread() { return worker_thread_; } PortAllocatorSession* allocator_session() { diff --git a/talk/p2p/base/p2ptransportchannel_unittest.cc b/talk/p2p/base/p2ptransportchannel_unittest.cc index 32504debba..a731b8dbc9 100644 --- a/talk/p2p/base/p2ptransportchannel_unittest.cc +++ b/talk/p2p/base/p2ptransportchannel_unittest.cc @@ -601,9 +601,9 @@ class P2PTransportChannelTestBase : public testing::Test, c.set_password(""); } LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" - << rch->component() << "): " << c.type() << ", " << c.protocol() - << ", " << c.address().ToString() << ", " << c.username() - << ", " << c.generation(); + << rch->component() << "): " << c.type() << ", " + << c.protocol() << ", " << c.address().ToString() << ", " + << c.username() << ", " << c.generation(); rch->OnCandidate(c); } void OnReadPacket(cricket::TransportChannel* channel, const char* data, @@ -1266,6 +1266,165 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { DestroyChannels(); } +TEST_F(P2PTransportChannelTest, TestBundleAllocatorToBundleAllocator) { + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + + CreateChannels(2); + + EXPECT_TRUE_WAIT(ep1_ch1()->readable() && + ep1_ch1()->writable() && + ep2_ch1()->readable() && + ep2_ch1()->writable(), + 1000); + EXPECT_TRUE(ep1_ch1()->best_connection() && + ep2_ch1()->best_connection()); + + EXPECT_FALSE(ep1_ch2()->readable()); + EXPECT_FALSE(ep1_ch2()->writable()); + EXPECT_FALSE(ep2_ch2()->readable()); + EXPECT_FALSE(ep2_ch2()->writable()); + + TestSendRecv(1); // Only 1 channel is writable per Endpoint. + DestroyChannels(); +} + +TEST_F(P2PTransportChannelTest, TestBundleAllocatorToNonBundleAllocator) { + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + // Enable BUNDLE flag at one side. + SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + + CreateChannels(2); + + EXPECT_TRUE_WAIT(ep1_ch1()->readable() && + ep1_ch1()->writable() && + ep2_ch1()->readable() && + ep2_ch1()->writable(), + 1000); + EXPECT_TRUE_WAIT(ep1_ch2()->readable() && + ep1_ch2()->writable() && + ep2_ch2()->readable() && + ep2_ch2()->writable(), + 1000); + + EXPECT_TRUE(ep1_ch1()->best_connection() && + ep2_ch1()->best_connection()); + EXPECT_TRUE(ep1_ch2()->best_connection() && + ep2_ch2()->best_connection()); + + TestSendRecv(2); + DestroyChannels(); +} + +TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithoutBundle) { + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + TestSignalRoleConflict(); +} + +TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithBundle) { + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); + TestSignalRoleConflict(); +} + +// Tests that the ice configs (protocol, tiebreaker and role) can be passed +// down to ports. +TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + + SetIceRole(0, cricket::ICEROLE_CONTROLLING); + SetIceProtocol(0, cricket::ICEPROTO_GOOGLE); + SetIceTiebreaker(0, kTiebreaker1); + SetIceRole(1, cricket::ICEROLE_CONTROLLING); + SetIceProtocol(1, cricket::ICEPROTO_RFC5245); + SetIceTiebreaker(1, kTiebreaker2); + + CreateChannels(1); + + EXPECT_EQ_WAIT(2u, ep1_ch1()->ports().size(), 1000); + + const std::vector ports_before = ep1_ch1()->ports(); + for (size_t i = 0; i < ports_before.size(); ++i) { + EXPECT_EQ(cricket::ICEROLE_CONTROLLING, ports_before[i]->GetIceRole()); + EXPECT_EQ(cricket::ICEPROTO_GOOGLE, ports_before[i]->IceProtocol()); + EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); + } + + ep1_ch1()->SetIceRole(cricket::ICEROLE_CONTROLLED); + ep1_ch1()->SetIceProtocolType(cricket::ICEPROTO_RFC5245); + ep1_ch1()->SetIceTiebreaker(kTiebreaker2); + + const std::vector ports_after = ep1_ch1()->ports(); + for (size_t i = 0; i < ports_after.size(); ++i) { + EXPECT_EQ(cricket::ICEROLE_CONTROLLED, ports_before[i]->GetIceRole()); + EXPECT_EQ(cricket::ICEPROTO_RFC5245, ports_before[i]->IceProtocol()); + // SetIceTiebreaker after Connect() has been called will fail. So expect the + // original value. + EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); + } + + EXPECT_TRUE_WAIT(ep1_ch1()->readable() && + ep1_ch1()->writable() && + ep2_ch1()->readable() && + ep2_ch1()->writable(), + 1000); + + EXPECT_TRUE(ep1_ch1()->best_connection() && + ep2_ch1()->best_connection()); + + TestSendRecv(1); + DestroyChannels(); +} + +// Test that channel will handle connectivity checks received before the +// candidates received and channel has remote ice credentials. +TEST_F(P2PTransportChannelTest, TestSlowSignalingAsIce) { + set_clear_remote_candidates_ufrag_pwd(true); + AddAddress(0, kPublicAddrs[0]); + AddAddress(1, kPublicAddrs[1]); + + // Disable all protocols except TCP. + SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); + SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG); + + SetIceRole(0, cricket::ICEROLE_CONTROLLING); + SetIceProtocol(0, cricket::ICEPROTO_RFC5245); + SetIceTiebreaker(0, kTiebreaker1); + SetIceRole(1, cricket::ICEROLE_CONTROLLED); + SetIceProtocol(1, cricket::ICEPROTO_RFC5245); + SetIceTiebreaker(1, kTiebreaker2); + + // Delay handling of the candidates from Endpoint 1. + // Since remote ICE username and passwords are already provided during + // channel creation time using set_clear_remote_candidates_ufrag_pwd, + // channel should make a connection when it receives remote ping before + // candidates arrives. Channel will create connections only if remote username + // and passwords match with the one received in stun ping messages. + SetSignalingDelay(1, 1000); + CreateChannels(1); + + EXPECT_TRUE_WAIT(ep1_ch1()->readable() && + ep1_ch1()->writable() && + ep2_ch1()->readable() && + ep2_ch1()->writable(), + 1000); + + EXPECT_EQ(cricket::PRFLX_PORT_TYPE, RemoteCandidate(ep1_ch1())->type()); + EXPECT_EQ(cricket::LOCAL_PORT_TYPE, LocalCandidate(ep1_ch1())->type()); + EXPECT_EQ(cricket::LOCAL_PORT_TYPE, RemoteCandidate(ep2_ch1())->type()); + EXPECT_EQ(cricket::LOCAL_PORT_TYPE, LocalCandidate(ep2_ch1())->type()); + + TestSendRecv(1); + DestroyChannels(); +} + // Test what happens when we have 2 users behind the same NAT. This can lead // to interesting behavior because the STUN server will only give out the // address of the outermost NAT. @@ -1387,119 +1546,3 @@ TEST_F(P2PTransportChannelMultihomedTest, TestDrain) { DestroyChannels(); } - -TEST_F(P2PTransportChannelTest, TestBundleAllocatorToBundleAllocator) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - - CreateChannels(2); - - EXPECT_TRUE_WAIT(ep1_ch1()->readable() && - ep1_ch1()->writable() && - ep2_ch1()->readable() && - ep2_ch1()->writable(), - 1000); - EXPECT_TRUE(ep1_ch1()->best_connection() && - ep2_ch1()->best_connection()); - - EXPECT_FALSE(ep1_ch2()->readable()); - EXPECT_FALSE(ep1_ch2()->writable()); - EXPECT_FALSE(ep2_ch2()->readable()); - EXPECT_FALSE(ep2_ch2()->writable()); - - TestSendRecv(1); // Only 1 channel is writable per Endpoint. - DestroyChannels(); -} - -TEST_F(P2PTransportChannelTest, TestBundleAllocatorToNonBundleAllocator) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - // Enable BUNDLE flag at one side. - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - - CreateChannels(2); - - EXPECT_TRUE_WAIT(ep1_ch1()->readable() && - ep1_ch1()->writable() && - ep2_ch1()->readable() && - ep2_ch1()->writable(), - 1000); - EXPECT_TRUE_WAIT(ep1_ch2()->readable() && - ep1_ch2()->writable() && - ep2_ch2()->readable() && - ep2_ch2()->writable(), - 1000); - - EXPECT_TRUE(ep1_ch1()->best_connection() && - ep2_ch1()->best_connection()); - EXPECT_TRUE(ep1_ch2()->best_connection() && - ep2_ch2()->best_connection()); - - TestSendRecv(2); - DestroyChannels(); -} - -TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithoutBundle) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - TestSignalRoleConflict(); -} - -TEST_F(P2PTransportChannelTest, TestIceRoleConflictWithBundle) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - SetAllocatorFlags(0, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - SetAllocatorFlags(1, cricket::PORTALLOCATOR_ENABLE_BUNDLE); - TestSignalRoleConflict(); -} - -// Tests that the ice configs (protocol, tiebreaker and role) can be passed -// down to ports. -TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { - AddAddress(0, kPublicAddrs[0]); - AddAddress(1, kPublicAddrs[1]); - - SetIceRole(0, cricket::ICEROLE_CONTROLLING); - SetIceProtocol(0, cricket::ICEPROTO_GOOGLE); - SetIceTiebreaker(0, kTiebreaker1); - SetIceRole(1, cricket::ICEROLE_CONTROLLING); - SetIceProtocol(1, cricket::ICEPROTO_RFC5245); - SetIceTiebreaker(1, kTiebreaker2); - - CreateChannels(1); - - EXPECT_EQ_WAIT(2u, ep1_ch1()->ports().size(), 1000); - - const std::vector ports_before = ep1_ch1()->ports(); - for (size_t i = 0; i < ports_before.size(); ++i) { - EXPECT_EQ(cricket::ICEROLE_CONTROLLING, ports_before[i]->GetIceRole()); - EXPECT_EQ(cricket::ICEPROTO_GOOGLE, ports_before[i]->IceProtocol()); - EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); - } - - ep1_ch1()->SetIceRole(cricket::ICEROLE_CONTROLLED); - ep1_ch1()->SetIceProtocolType(cricket::ICEPROTO_RFC5245); - ep1_ch1()->SetIceTiebreaker(kTiebreaker2); - - const std::vector ports_after = ep1_ch1()->ports(); - for (size_t i = 0; i < ports_after.size(); ++i) { - EXPECT_EQ(cricket::ICEROLE_CONTROLLED, ports_before[i]->GetIceRole()); - EXPECT_EQ(cricket::ICEPROTO_RFC5245, ports_before[i]->IceProtocol()); - // SetIceTiebreaker after Connect() has been called will fail. So expect the - // original value. - EXPECT_EQ(kTiebreaker1, ports_before[i]->IceTiebreaker()); - } - - EXPECT_TRUE_WAIT(ep1_ch1()->readable() && - ep1_ch1()->writable() && - ep2_ch1()->readable() && - ep2_ch1()->writable(), - 1000); - - EXPECT_TRUE(ep1_ch1()->best_connection() && - ep2_ch1()->best_connection()); - - TestSendRecv(1); -} diff --git a/talk/p2p/base/pseudotcp.cc b/talk/p2p/base/pseudotcp.cc index 6a2c1d26b7..b647fbf3de 100644 --- a/talk/p2p/base/pseudotcp.cc +++ b/talk/p2p/base/pseudotcp.cc @@ -36,6 +36,7 @@ #include "talk/base/byteorder.h" #include "talk/base/common.h" #include "talk/base/logging.h" +#include "talk/base/scoped_ptr.h" #include "talk/base/socket.h" #include "talk/base/stringutils.h" #include "talk/base/timeutils.h" @@ -538,25 +539,24 @@ IPseudoTcpNotify::WriteResult PseudoTcp::packet(uint32 seq, uint8 flags, uint32 now = Now(); - uint8 buffer[MAX_PACKET]; - long_to_bytes(m_conv, buffer); - long_to_bytes(seq, buffer + 4); - long_to_bytes(m_rcv_nxt, buffer + 8); + talk_base::scoped_array buffer(new uint8[MAX_PACKET]); + long_to_bytes(m_conv, buffer.get()); + long_to_bytes(seq, buffer.get() + 4); + long_to_bytes(m_rcv_nxt, buffer.get() + 8); buffer[12] = 0; buffer[13] = flags; - short_to_bytes(static_cast(m_rcv_wnd >> m_rwnd_scale), buffer + 14); + short_to_bytes( + static_cast(m_rcv_wnd >> m_rwnd_scale), buffer.get() + 14); // Timestamp computations - long_to_bytes(now, buffer + 16); - long_to_bytes(m_ts_recent, buffer + 20); + long_to_bytes(now, buffer.get() + 16); + long_to_bytes(m_ts_recent, buffer.get() + 20); m_ts_lastack = m_rcv_nxt; if (len) { size_t bytes_read = 0; - talk_base::StreamResult result = m_sbuf.ReadOffset(buffer + HEADER_SIZE, - len, - offset, - &bytes_read); + talk_base::StreamResult result = m_sbuf.ReadOffset( + buffer.get() + HEADER_SIZE, len, offset, &bytes_read); UNUSED(result); ASSERT(result == talk_base::SR_SUCCESS); ASSERT(static_cast(bytes_read) == len); @@ -573,7 +573,8 @@ IPseudoTcpNotify::WriteResult PseudoTcp::packet(uint32 seq, uint8 flags, << ">"; #endif // _DEBUGMSG - IPseudoTcpNotify::WriteResult wres = m_notify->TcpWritePacket(this, reinterpret_cast(buffer), len + HEADER_SIZE); + IPseudoTcpNotify::WriteResult wres = m_notify->TcpWritePacket( + this, reinterpret_cast(buffer.get()), len + HEADER_SIZE); // Note: When len is 0, this is an ACK packet. We don't read the return value for those, // and thus we won't retry. So go ahead and treat the packet as a success (basically simulate // as if it were dropped), which will prevent our timers from being messed up. diff --git a/talk/p2p/base/rawtransportchannel.h b/talk/p2p/base/rawtransportchannel.h index 0f606b7279..4abea83156 100644 --- a/talk/p2p/base/rawtransportchannel.h +++ b/talk/p2p/base/rawtransportchannel.h @@ -101,6 +101,55 @@ class RawTransportChannel : public TransportChannelImpl, virtual void SetIcePwd(const std::string& ice_pwd) {} virtual void SetRemoteIceMode(IceMode mode) {} + virtual bool GetStats(ConnectionInfos* infos) { + return false; + } + + // DTLS methods. + virtual bool IsDtlsActive() const { return false; } + + // Default implementation. + virtual bool GetSslRole(talk_base::SSLRole* role) const { + return false; + } + + virtual bool SetSslRole(talk_base::SSLRole role) { + return false; + } + + // Set up the ciphers to use for DTLS-SRTP. + virtual bool SetSrtpCiphers(const std::vector& ciphers) { + return false; + } + + // Find out which DTLS-SRTP cipher was negotiated + virtual bool GetSrtpCipher(std::string* cipher) { + return false; + } + + // Allows key material to be extracted for external encryption. + virtual bool ExportKeyingMaterial( + const std::string& label, + const uint8* context, + size_t context_len, + bool use_context, + uint8* result, + size_t result_len) { + return false; + } + + virtual bool SetLocalIdentity(talk_base::SSLIdentity* identity) { + return false; + } + + // Set DTLS Remote fingerprint. Must be after local identity set. + virtual bool SetRemoteFingerprint( + const std::string& digest_alg, + const uint8* digest, + size_t digest_len) { + return false; + } + private: RawTransport* raw_transport_; talk_base::Thread *worker_thread_; diff --git a/talk/p2p/base/session.cc b/talk/p2p/base/session.cc index 74eda019c3..3128393c34 100644 --- a/talk/p2p/base/session.cc +++ b/talk/p2p/base/session.cc @@ -999,9 +999,10 @@ TransportInfos Session::GetEmptyTransportInfos( TransportInfos tinfos; for (ContentInfos::const_iterator content = contents.begin(); content != contents.end(); ++content) { - tinfos.push_back( - TransportInfo(content->name, - TransportDescription(transport_type(), Candidates()))); + tinfos.push_back(TransportInfo(content->name, + TransportDescription(transport_type(), + std::string(), + std::string()))); } return tinfos; } @@ -1558,7 +1559,9 @@ bool Session::SendTransportInfoMessage(const TransportProxy* transproxy, const Candidates& candidates, SessionError* error) { return SendTransportInfoMessage(TransportInfo(transproxy->content_name(), - TransportDescription(transproxy->type(), candidates)), error); + TransportDescription(transproxy->type(), std::vector(), + std::string(), std::string(), ICEMODE_FULL, + CONNECTIONROLE_NONE, NULL, candidates)), error); } bool Session::WriteSessionAction(SignalingProtocol protocol, diff --git a/talk/p2p/base/session_unittest.cc b/talk/p2p/base/session_unittest.cc index 1d072ae0a9..a7620669e9 100644 --- a/talk/p2p/base/session_unittest.cc +++ b/talk/p2p/base/session_unittest.cc @@ -713,7 +713,7 @@ cricket::SessionDescription* NewTestSessionDescription( new TestContentDescription(gingle_content_type, content_type_a)); cricket::TransportDescription desc(cricket::NS_GINGLE_P2P, - cricket::Candidates()); + std::string(), std::string()); offer->AddTransportInfo(cricket::TransportInfo(content_name_a, desc)); if (content_name_a != content_name_b) { @@ -735,7 +735,7 @@ cricket::SessionDescription* NewTestSessionDescription( offer->AddTransportInfo(cricket::TransportInfo (content_name, cricket::TransportDescription( cricket::NS_GINGLE_P2P, - cricket::Candidates()))); + std::string(), std::string()))); return offer; } diff --git a/talk/p2p/base/sessionmessages.cc b/talk/p2p/base/sessionmessages.cc index 031c3d6f6a..7a03d76506 100644 --- a/talk/p2p/base/sessionmessages.cc +++ b/talk/p2p/base/sessionmessages.cc @@ -359,7 +359,7 @@ bool ParseGingleTransportInfos(const buzz::XmlElement* action_elem, // If we don't have media, no need to separate the candidates. if (!has_audio && !has_video) { TransportInfo tinfo(CN_OTHER, - TransportDescription(NS_GINGLE_P2P, Candidates())); + TransportDescription(NS_GINGLE_P2P, std::string(), std::string())); if (!ParseGingleCandidates(action_elem, trans_parsers, translators, CN_OTHER, &tinfo.description.candidates, error)) { @@ -371,10 +371,12 @@ bool ParseGingleTransportInfos(const buzz::XmlElement* action_elem, } // If we have media, separate the candidates. - TransportInfo audio_tinfo(CN_AUDIO, - TransportDescription(NS_GINGLE_P2P, Candidates())); - TransportInfo video_tinfo(CN_VIDEO, - TransportDescription(NS_GINGLE_P2P, Candidates())); + TransportInfo audio_tinfo( + CN_AUDIO, + TransportDescription(NS_GINGLE_P2P, std::string(), std::string())); + TransportInfo video_tinfo( + CN_VIDEO, + TransportDescription(NS_GINGLE_P2P, std::string(), std::string())); for (const buzz::XmlElement* candidate_elem = action_elem->FirstElement(); candidate_elem != NULL; candidate_elem = candidate_elem->NextElement()) { diff --git a/talk/p2p/base/stunport.cc b/talk/p2p/base/stunport.cc index e182a51cba..25d9e8df2d 100644 --- a/talk/p2p/base/stunport.cc +++ b/talk/p2p/base/stunport.cc @@ -254,8 +254,7 @@ void UDPPort::OnReadPacket(talk_base::AsyncPacketSocket* socket, // Even if the response doesn't match one of our outstanding requests, we // will eat it because it might be a response to a retransmitted packet, and // we already cleared the request when we got the first response. - ASSERT(!server_addr_.IsUnresolved()); - if (remote_addr == server_addr_) { + if (!server_addr_.IsUnresolved() && remote_addr == server_addr_) { requests_.CheckResponse(data, size); return; } diff --git a/talk/p2p/base/stunport_unittest.cc b/talk/p2p/base/stunport_unittest.cc index ba36c480ec..3c1c6836f5 100644 --- a/talk/p2p/base/stunport_unittest.cc +++ b/talk/p2p/base/stunport_unittest.cc @@ -71,10 +71,36 @@ class StunPortTest : public testing::Test, &StunPortTest::OnPortError); } + void CreateSharedStunPort(const talk_base::SocketAddress& server_addr) { + socket_.reset(socket_factory_.CreateUdpSocket( + talk_base::SocketAddress(kLocalAddr.ipaddr(), 0), 0, 0)); + ASSERT_TRUE(socket_ != NULL); + socket_->SignalReadPacket.connect(this, &StunPortTest::OnReadPacket); + stun_port_.reset(cricket::UDPPort::Create( + talk_base::Thread::Current(), &network_, socket_.get(), + talk_base::CreateRandomString(16), talk_base::CreateRandomString(22))); + ASSERT_TRUE(stun_port_ != NULL); + stun_port_->set_server_addr(server_addr); + stun_port_->SignalPortComplete.connect(this, + &StunPortTest::OnPortComplete); + stun_port_->SignalPortError.connect(this, + &StunPortTest::OnPortError); + } + void PrepareAddress() { stun_port_->PrepareAddress(); } + void OnReadPacket(talk_base::AsyncPacketSocket* socket, const char* data, + size_t size, const talk_base::SocketAddress& remote_addr) { + stun_port_->HandleIncomingPacket(socket, data, size, remote_addr); + } + + void SendData(const char* data, size_t len) { + stun_port_->HandleIncomingPacket( + socket_.get(), data, len, talk_base::SocketAddress("22.22.22.22", 0)); + } + protected: static void SetUpTestCase() { // Ensure the RNG is inited. @@ -96,8 +122,9 @@ class StunPortTest : public testing::Test, private: talk_base::Network network_; talk_base::BasicPacketSocketFactory socket_factory_; - talk_base::scoped_ptr stun_port_; + talk_base::scoped_ptr stun_port_; talk_base::scoped_ptr stun_server_; + talk_base::scoped_ptr socket_; bool done_; bool error_; int stun_keepalive_delay_; @@ -164,3 +191,28 @@ TEST_F(StunPortTest, TestKeepAliveResponse) { ASSERT_EQ(1U, port()->Candidates().size()); } +// Test that a local candidate can be generated using a shared socket. +TEST_F(StunPortTest, TestSharedSocketPrepareAddress) { + CreateSharedStunPort(kStunAddr); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + ASSERT_EQ(1U, port()->Candidates().size()); + EXPECT_TRUE(kLocalAddr.EqualIPs(port()->Candidates()[0].address())); +} + +// Test that we still a get a local candidate with invalid stun server hostname. +// Also verifing that UDPPort can receive packets when stun address can't be +// resolved. +TEST_F(StunPortTest, TestSharedSocketPrepareAddressInvalidHostname) { + CreateSharedStunPort(kBadHostnameAddr); + PrepareAddress(); + EXPECT_TRUE_WAIT(done(), kTimeoutMs); + ASSERT_EQ(1U, port()->Candidates().size()); + EXPECT_TRUE(kLocalAddr.EqualIPs(port()->Candidates()[0].address())); + + // Send data to port after it's ready. This is to make sure, UDP port can + // handle data with unresolved stun server address. + std::string data = "some random data, sending to cricket::Port."; + SendData(data.c_str(), data.length()); + // No crash is success. +} diff --git a/talk/p2p/base/transport.cc b/talk/p2p/base/transport.cc index 6ccd90b02d..d9064b8b49 100644 --- a/talk/p2p/base/transport.cc +++ b/talk/p2p/base/transport.cc @@ -27,6 +27,7 @@ #include "talk/p2p/base/transport.h" +#include "talk/base/bind.h" #include "talk/base/common.h" #include "talk/base/logging.h" #include "talk/p2p/base/candidate.h" @@ -293,7 +294,8 @@ void Transport::ConnectChannels_w() { TransportDescription desc(NS_GINGLE_P2P, std::vector(), talk_base::CreateRandomString(ICE_UFRAG_LENGTH), talk_base::CreateRandomString(ICE_PWD_LENGTH), - ICEMODE_FULL, NULL, Candidates()); + ICEMODE_FULL, CONNECTIONROLE_NONE, NULL, + Candidates()); SetLocalTransportDescription_w(desc, CA_OFFER); } @@ -424,6 +426,11 @@ bool Transport::GetStats_w(TransportStats* stats) { return true; } +bool Transport::GetSslRole(talk_base::SSLRole* ssl_role) const { + return worker_thread_->Invoke( + Bind(&Transport::GetSslRole_w, this, ssl_role)); +} + void Transport::OnRemoteCandidates(const std::vector& candidates) { for (std::vector::const_iterator iter = candidates.begin(); iter != candidates.end(); @@ -668,19 +675,20 @@ bool Transport::ApplyRemoteTransportDescription_w(TransportChannelImpl* ch) { return true; } -void Transport::ApplyNegotiatedTransportDescription_w( +bool Transport::ApplyNegotiatedTransportDescription_w( TransportChannelImpl* channel) { channel->SetIceProtocolType(protocol_); channel->SetRemoteIceMode(remote_ice_mode_); + return true; } -bool Transport::NegotiateTransportDescription_w(ContentAction local_role_) { +bool Transport::NegotiateTransportDescription_w(ContentAction local_role) { // TODO(ekr@rtfm.com): This is ICE-specific stuff. Refactor into // P2PTransport. const TransportDescription* offer; const TransportDescription* answer; - if (local_role_ == CA_OFFER) { + if (local_role == CA_OFFER) { offer = local_description_.get(); answer = remote_description_.get(); } else { @@ -724,7 +732,8 @@ bool Transport::NegotiateTransportDescription_w(ContentAction local_role_) { for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { - ApplyNegotiatedTransportDescription_w(iter->second.get()); + if (!ApplyNegotiatedTransportDescription_w(iter->second.get())) + return false; } return true; } diff --git a/talk/p2p/base/transport.h b/talk/p2p/base/transport.h index 63c37343ab..381215f5d1 100644 --- a/talk/p2p/base/transport.h +++ b/talk/p2p/base/transport.h @@ -52,6 +52,7 @@ #include "talk/base/criticalsection.h" #include "talk/base/messagequeue.h" #include "talk/base/sigslot.h" +#include "talk/base/sslstreamadapter.h" #include "talk/p2p/base/candidate.h" #include "talk/p2p/base/constants.h" #include "talk/p2p/base/sessiondescription.h" @@ -323,6 +324,8 @@ class Transport : public talk_base::MessageHandler, // Forwards the signal from TransportChannel to BaseSession. sigslot::signal0<> SignalRoleConflict; + virtual bool GetSslRole(talk_base::SSLRole* ssl_role) const; + protected: // These are called by Create/DestroyChannel above in order to create or // destroy the appropriate type of channel. @@ -366,9 +369,13 @@ class Transport : public talk_base::MessageHandler, // Pushes down the transport parameters obtained via negotiation. // Derived classes can set their specific parameters here, but must call the // base as well. - virtual void ApplyNegotiatedTransportDescription_w( + virtual bool ApplyNegotiatedTransportDescription_w( TransportChannelImpl* channel); + virtual bool GetSslRole_w(talk_base::SSLRole* ssl_role) const { + return false; + } + private: struct ChannelMapEntry { ChannelMapEntry() : impl_(NULL), candidates_allocated_(false), ref_(0) {} diff --git a/talk/p2p/base/transport_unittest.cc b/talk/p2p/base/transport_unittest.cc index f9bebdfa53..e3b7badfff 100644 --- a/talk/p2p/base/transport_unittest.cc +++ b/talk/p2p/base/transport_unittest.cc @@ -145,8 +145,7 @@ TEST_F(TransportTest, TestChannelIceParameters) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); transport_->SetIceTiebreaker(99U); cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, NULL, cricket::Candidates()); + cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_OFFER)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); @@ -157,8 +156,7 @@ TEST_F(TransportTest, TestChannelIceParameters) { EXPECT_EQ(kIcePwd1, channel_->ice_pwd()); cricket::TransportDescription remote_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, NULL, cricket::Candidates()); + cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_ANSWER)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); @@ -177,12 +175,12 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInOffer) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLED); cricket::TransportDescription remote_desc( cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, NULL, cricket::Candidates()); + kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, + cricket::CONNECTIONROLE_ACTPASS, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_OFFER)); cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, NULL, cricket::Candidates()); + cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_ANSWER)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); @@ -195,8 +193,7 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInOffer) { TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { transport_->SetIceRole(cricket::ICEROLE_CONTROLLING); cricket::TransportDescription local_desc( - cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_FULL, NULL, cricket::Candidates()); + cricket::NS_JINGLE_ICE_UDP, kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, cricket::CA_OFFER)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, transport_->ice_role()); @@ -206,7 +203,8 @@ TEST_F(TransportTest, TestSetRemoteIceLiteInAnswer) { EXPECT_EQ(cricket::ICEMODE_FULL, channel_->remote_ice_mode()); cricket::TransportDescription remote_desc( cricket::NS_JINGLE_ICE_UDP, std::vector(), - kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, NULL, cricket::Candidates()); + kIceUfrag1, kIcePwd1, cricket::ICEMODE_LITE, + cricket::CONNECTIONROLE_NONE, NULL, cricket::Candidates()); ASSERT_TRUE(transport_->SetRemoteTransportDescription(remote_desc, cricket::CA_ANSWER)); EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel_->GetIceRole()); diff --git a/talk/p2p/base/transportchannel.h b/talk/p2p/base/transportchannel.h index 2b09f54b83..a5e41b9de1 100644 --- a/talk/p2p/base/transportchannel.h +++ b/talk/p2p/base/transportchannel.h @@ -89,30 +89,20 @@ class TransportChannel : public sigslot::has_slots<> { // Returns the most recent error that occurred on this channel. virtual int GetError() = 0; - // TODO(mallinath) - Move this to TransportChannelImpl, after channel.cc - // no longer needs it. - // Returns current transportchannel ICE role. - virtual IceRole GetIceRole() const = 0; - // Returns the current stats for this connection. - virtual bool GetStats(ConnectionInfos* infos) { - return false; - } + virtual bool GetStats(ConnectionInfos* infos) = 0; // Is DTLS active? - virtual bool IsDtlsActive() const { - return false; - } + virtual bool IsDtlsActive() const = 0; + + // Default implementation. + virtual bool GetSslRole(talk_base::SSLRole* role) const = 0; // Set up the ciphers to use for DTLS-SRTP. - virtual bool SetSrtpCiphers(const std::vector& ciphers) { - return false; - } + virtual bool SetSrtpCiphers(const std::vector& ciphers) = 0; // Find out which DTLS-SRTP cipher was negotiated - virtual bool GetSrtpCipher(std::string* cipher) { - return false; - } + virtual bool GetSrtpCipher(std::string* cipher) = 0; // Allows key material to be extracted for external encryption. virtual bool ExportKeyingMaterial(const std::string& label, @@ -120,9 +110,7 @@ class TransportChannel : public sigslot::has_slots<> { size_t context_len, bool use_context, uint8* result, - size_t result_len) { - return false; - } + size_t result_len) = 0; // Signalled each time a packet is received on this channel. sigslot::signal4 SignalCandidatesAllocationDone; diff --git a/talk/p2p/base/transportchannelproxy.cc b/talk/p2p/base/transportchannelproxy.cc index b25f75751d..9f84620678 100644 --- a/talk/p2p/base/transportchannelproxy.cc +++ b/talk/p2p/base/transportchannelproxy.cc @@ -135,6 +135,22 @@ bool TransportChannelProxy::IsDtlsActive() const { return impl_->IsDtlsActive(); } +bool TransportChannelProxy::GetSslRole(talk_base::SSLRole* role) const { + ASSERT(talk_base::Thread::Current() == worker_thread_); + if (!impl_) { + return false; + } + return impl_->GetSslRole(role); +} + +bool TransportChannelProxy::SetSslRole(talk_base::SSLRole role) { + ASSERT(talk_base::Thread::Current() == worker_thread_); + if (!impl_) { + return false; + } + return impl_->SetSslRole(role); +} + bool TransportChannelProxy::SetSrtpCiphers(const std::vector& ciphers) { ASSERT(talk_base::Thread::Current() == worker_thread_); diff --git a/talk/p2p/base/transportchannelproxy.h b/talk/p2p/base/transportchannelproxy.h index 828c0ae05b..ea2843d524 100644 --- a/talk/p2p/base/transportchannelproxy.h +++ b/talk/p2p/base/transportchannelproxy.h @@ -69,6 +69,8 @@ class TransportChannelProxy : public TransportChannel, virtual IceRole GetIceRole() const; virtual bool GetStats(ConnectionInfos* infos); virtual bool IsDtlsActive() const; + virtual bool GetSslRole(talk_base::SSLRole* role) const; + virtual bool SetSslRole(talk_base::SSLRole role); virtual bool SetSrtpCiphers(const std::vector& ciphers); virtual bool GetSrtpCipher(std::string* cipher); virtual bool ExportKeyingMaterial(const std::string& label, diff --git a/talk/p2p/base/transportdescription.cc b/talk/p2p/base/transportdescription.cc new file mode 100644 index 0000000000..5687342728 --- /dev/null +++ b/talk/p2p/base/transportdescription.cc @@ -0,0 +1,52 @@ +/* + * libjingle + * Copyright 2013 Google Inc. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED + * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO + * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; + * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, + * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR + * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "talk/p2p/base/transportdescription.h" + +#include "talk/p2p/base/constants.h" + +namespace cricket { + +bool StringToConnectionRole(const std::string& role_str, ConnectionRole* role) { + const char* const roles[] = { + CONNECTIONROLE_ACTIVE_STR, + CONNECTIONROLE_PASSIVE_STR, + CONNECTIONROLE_ACTPASS_STR, + CONNECTIONROLE_HOLDCONN_STR + }; + + for (size_t i = 0; i < ARRAY_SIZE(roles); ++i) { + if (stricmp(roles[i], role_str.c_str()) == 0) { + *role = static_cast(CONNECTIONROLE_ACTIVE + i); + return true; + } + } + return false; +} + +} // namespace cricket + diff --git a/talk/p2p/base/transportdescription.h b/talk/p2p/base/transportdescription.h index 92da8d66c2..64fbb89a80 100644 --- a/talk/p2p/base/transportdescription.h +++ b/talk/p2p/base/transportdescription.h @@ -75,6 +75,26 @@ enum IceMode { ICEMODE_LITE // As defined in http://tools.ietf.org/html/rfc5245#section-4.2 }; +// RFC 4145 - http://tools.ietf.org/html/rfc4145#section-4 +// 'active': The endpoint will initiate an outgoing connection. +// 'passive': The endpoint will accept an incoming connection. +// 'actpass': The endpoint is willing to accept an incoming +// connection or to initiate an outgoing connection. +enum ConnectionRole { + CONNECTIONROLE_NONE = 0, + CONNECTIONROLE_ACTIVE, + CONNECTIONROLE_PASSIVE, + CONNECTIONROLE_ACTPASS, + CONNECTIONROLE_HOLDCONN, +}; + +extern const char CONNECTIONROLE_ACTIVE_STR[]; +extern const char CONNECTIONROLE_PASSIVE_STR[]; +extern const char CONNECTIONROLE_ACTPASS_STR[]; +extern const char CONNECTIONROLE_HOLDCONN_STR[]; + +bool StringToConnectionRole(const std::string& role_str, ConnectionRole* role); + typedef std::vector Candidates; struct TransportDescription { @@ -85,6 +105,7 @@ struct TransportDescription { const std::string& ice_ufrag, const std::string& ice_pwd, IceMode ice_mode, + ConnectionRole role, const talk_base::SSLFingerprint* identity_fingerprint, const Candidates& candidates) : transport_type(transport_type), @@ -92,19 +113,24 @@ struct TransportDescription { ice_ufrag(ice_ufrag), ice_pwd(ice_pwd), ice_mode(ice_mode), + connection_role(role), identity_fingerprint(CopyFingerprint(identity_fingerprint)), candidates(candidates) {} TransportDescription(const std::string& transport_type, - const Candidates& candidates) + const std::string& ice_ufrag, + const std::string& ice_pwd) : transport_type(transport_type), + ice_ufrag(ice_ufrag), + ice_pwd(ice_pwd), ice_mode(ICEMODE_FULL), - candidates(candidates) {} + connection_role(CONNECTIONROLE_NONE) {} TransportDescription(const TransportDescription& from) : transport_type(from.transport_type), transport_options(from.transport_options), ice_ufrag(from.ice_ufrag), ice_pwd(from.ice_pwd), ice_mode(from.ice_mode), + connection_role(from.connection_role), identity_fingerprint(CopyFingerprint(from.identity_fingerprint.get())), candidates(from.candidates) {} @@ -118,6 +144,7 @@ struct TransportDescription { ice_ufrag = from.ice_ufrag; ice_pwd = from.ice_pwd; ice_mode = from.ice_mode; + connection_role = from.connection_role; identity_fingerprint.reset(CopyFingerprint( from.identity_fingerprint.get())); @@ -147,6 +174,7 @@ struct TransportDescription { std::string ice_ufrag; std::string ice_pwd; IceMode ice_mode; + ConnectionRole connection_role; talk_base::scoped_ptr identity_fingerprint; Candidates candidates; diff --git a/talk/p2p/base/transportdescriptionfactory.cc b/talk/p2p/base/transportdescriptionfactory.cc index 8fbfff144f..0c129437cc 100644 --- a/talk/p2p/base/transportdescriptionfactory.cc +++ b/talk/p2p/base/transportdescriptionfactory.cc @@ -73,10 +73,12 @@ TransportDescription* TransportDescriptionFactory::CreateOffer( // If we are trying to establish a secure transport, add a fingerprint. if (secure_ == SEC_ENABLED || secure_ == SEC_REQUIRED) { // Fail if we can't create the fingerprint. - if (!CreateIdentityDigest(desc.get())) { + // If we are the initiator set role to "actpass". + if (!SetSecurityInfo(desc.get(), CONNECTIONROLE_ACTPASS)) { return NULL; } } + return desc.release(); } @@ -101,7 +103,7 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( desc->transport_type = NS_GINGLE_P2P; // Offer is hybrid, we support GICE: use GICE. } else if ((!offer || offer->transport_type == NS_GINGLE_P2P) && - (protocol_ == ICEPROTO_HYBRID || protocol_ == ICEPROTO_GOOGLE)) { + (protocol_ == ICEPROTO_HYBRID || protocol_ == ICEPROTO_GOOGLE)) { // Offer is GICE, we support hybrid or GICE: use GICE. desc->transport_type = NS_GINGLE_P2P; } else { @@ -126,7 +128,11 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( // The offer supports DTLS, so answer with DTLS, as long as we support it. if (secure_ == SEC_ENABLED || secure_ == SEC_REQUIRED) { // Fail if we can't create the fingerprint. - if (!CreateIdentityDigest(desc.get())) { + // Setting DTLS role to active. + ConnectionRole role = (options.prefer_passive_role) ? + CONNECTIONROLE_PASSIVE : CONNECTIONROLE_ACTIVE; + + if (!SetSecurityInfo(desc.get(), role)) { return NULL; } } @@ -140,8 +146,8 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( return desc.release(); } -bool TransportDescriptionFactory::CreateIdentityDigest( - TransportDescription* desc) const { +bool TransportDescriptionFactory::SetSecurityInfo( + TransportDescription* desc, ConnectionRole role) const { if (!identity_) { LOG(LS_ERROR) << "Cannot create identity digest with no identity"; return false; @@ -154,6 +160,8 @@ bool TransportDescriptionFactory::CreateIdentityDigest( return false; } + // Assign security role. + desc->connection_role = role; return true; } diff --git a/talk/p2p/base/transportdescriptionfactory.h b/talk/p2p/base/transportdescriptionfactory.h index 32836f3e81..ddf2799818 100644 --- a/talk/p2p/base/transportdescriptionfactory.h +++ b/talk/p2p/base/transportdescriptionfactory.h @@ -37,8 +37,9 @@ class SSLIdentity; namespace cricket { struct TransportOptions { - TransportOptions() : ice_restart(false) {} + TransportOptions() : ice_restart(false), prefer_passive_role(false) {} bool ice_restart; + bool prefer_passive_role; }; // Creates transport descriptions according to the supplied configuration. @@ -71,7 +72,8 @@ class TransportDescriptionFactory { const TransportDescription* current_description) const; private: - bool CreateIdentityDigest(TransportDescription* description) const; + bool SetSecurityInfo(TransportDescription* description, + ConnectionRole role) const; TransportProtocol protocol_; SecurePolicy secure_; diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 1bce2acdcc..76fafae8d0 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -1003,8 +1003,13 @@ bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) { &dtls_buffer[offset], SRTP_MASTER_KEY_SALT_LEN); std::vector *send_key, *recv_key; + talk_base::SSLRole role; + if (!channel->GetSslRole(&role)) { + LOG(LS_WARNING) << "GetSslRole failed"; + return false; + } - if (channel->GetIceRole() == ICEROLE_CONTROLLING) { + if (role == talk_base::SSL_SERVER) { send_key = &server_write_key; recv_key = &client_write_key; } else { diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc index 6e04915704..f2e576ca92 100644 --- a/talk/session/media/mediasession_unittest.cc +++ b/talk/session/media/mediasession_unittest.cc @@ -219,25 +219,19 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { current_desc.reset(new SessionDescription()); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("audio", - TransportDescription("", std::vector(), + TransportDescription("", current_audio_ufrag, - current_audio_pwd, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + current_audio_pwd)))); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("video", - TransportDescription("", std::vector(), + TransportDescription("", current_video_ufrag, - current_video_pwd, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + current_video_pwd)))); EXPECT_TRUE(current_desc->AddTransportInfo( TransportInfo("data", - TransportDescription("", std::vector(), + TransportDescription("", current_data_ufrag, - current_data_pwd, - cricket::ICEMODE_FULL, - NULL, Candidates())))); + current_data_pwd)))); } if (offer) { desc.reset(f1_.CreateOffer(options, current_desc.get()));