diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index b13113aa3a..bdd33d7d37 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -633,6 +633,8 @@ bool PeerConnection::Initialize( stats_.reset(new StatsCollector(this)); + enable_ice_renomination_ = configuration.enable_ice_renomination; + // Initialize the WebRtcSession. It creates transport channels etc. if (!session_->Initialize(factory_->options(), std::move(cert_generator), configuration)) { @@ -1262,6 +1264,8 @@ bool PeerConnection::SetConfiguration(const RTCConfiguration& configuration) { // TODO(deadbeef): Shouldn't have to hop to the worker thread twice... session_->SetIceConfig(session_->ParseIceConfig(configuration)); + + enable_ice_renomination_ = configuration.enable_ice_renomination; return true; } @@ -1615,6 +1619,8 @@ bool PeerConnection::GetOptionsForOffer( cricket::TransportOptions(); } } + session_options->enable_ice_renomination = enable_ice_renomination_; + if (!ExtractMediaSessionOptions(rtc_options, true, session_options)) { return false; } @@ -1651,6 +1657,13 @@ bool PeerConnection::GetOptionsForOffer( return true; } +void PeerConnection::InitializeOptionsForAnswer( + cricket::MediaSessionOptions* session_options) { + session_options->recv_audio = false; + session_options->recv_video = false; + session_options->enable_ice_renomination = enable_ice_renomination_; +} + void PeerConnection::FinishOptionsForAnswer( cricket::MediaSessionOptions* session_options) { // TODO(deadbeef): Once we have transceivers, enumerate them here instead of @@ -1685,8 +1698,7 @@ void PeerConnection::FinishOptionsForAnswer( bool PeerConnection::GetOptionsForAnswer( const MediaConstraintsInterface* constraints, cricket::MediaSessionOptions* session_options) { - session_options->recv_audio = false; - session_options->recv_video = false; + InitializeOptionsForAnswer(session_options); if (!ParseConstraintsForAnswer(constraints, session_options)) { return false; } @@ -1699,8 +1711,7 @@ bool PeerConnection::GetOptionsForAnswer( bool PeerConnection::GetOptionsForAnswer( const RTCOfferAnswerOptions& options, cricket::MediaSessionOptions* session_options) { - session_options->recv_audio = false; - session_options->recv_video = false; + InitializeOptionsForAnswer(session_options); if (!ExtractMediaSessionOptions(options, false, session_options)) { return false; } diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index c4a9a60053..042e7f1bd4 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -239,6 +239,9 @@ class PeerConnection : public PeerConnectionInterface, const RTCOfferAnswerOptions& options, cricket::MediaSessionOptions* session_options); + void InitializeOptionsForAnswer( + cricket::MediaSessionOptions* session_options); + // Helper function for options processing. // Deprecated. virtual void FinishOptionsForAnswer( @@ -414,6 +417,8 @@ class PeerConnection : public PeerConnectionInterface, bool remote_peer_supports_msid_ = false; + bool enable_ice_renomination_ = false; + std::vector>> senders_; std::vector< diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc index 33b7b2ff4e..fb3c1887ba 100644 --- a/webrtc/api/peerconnection_unittest.cc +++ b/webrtc/api/peerconnection_unittest.cc @@ -393,6 +393,15 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, bool ExpectIceRestart() const { return expect_ice_restart_; } + void SetExpectIceRenomination(bool expect_renomination) { + expect_ice_renomination_ = expect_renomination; + } + void SetExpectRemoteIceRenomination(bool expect_renomination) { + expect_remote_ice_renomination_ = expect_renomination; + } + bool ExpectIceRenomination() { return expect_ice_renomination_; } + bool ExpectRemoteIceRenomination() { return expect_remote_ice_renomination_; } + void SetReceiveAudioVideo(bool audio, bool video) { SetReceiveAudio(audio); SetReceiveVideo(video); @@ -670,6 +679,42 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, } } + void VerifyLocalIceRenomination() { + ASSERT_TRUE(peer_connection_->local_description() != nullptr); + const cricket::SessionDescription* desc = + peer_connection_->local_description()->description(); + const cricket::ContentInfos& contents = desc->contents(); + + for (auto content : contents) { + if (content.rejected) + continue; + const cricket::TransportDescription* transport_desc = + desc->GetTransportDescriptionByName(content.name); + const auto& options = transport_desc->transport_options; + auto iter = std::find(options.begin(), options.end(), + cricket::ICE_RENOMINATION_STR); + EXPECT_EQ(ExpectIceRenomination(), iter != options.end()); + } + } + + void VerifyRemoteIceRenomination() { + ASSERT_TRUE(peer_connection_->remote_description() != nullptr); + const cricket::SessionDescription* desc = + peer_connection_->remote_description()->description(); + const cricket::ContentInfos& contents = desc->contents(); + + for (auto content : contents) { + if (content.rejected) + continue; + const cricket::TransportDescription* transport_desc = + desc->GetTransportDescriptionByName(content.name); + const auto& options = transport_desc->transport_options; + auto iter = std::find(options.begin(), options.end(), + cricket::ICE_RENOMINATION_STR); + EXPECT_EQ(ExpectRemoteIceRenomination(), iter != options.end()); + } + } + int GetAudioOutputLevelStats(webrtc::MediaStreamTrackInterface* track) { rtc::scoped_refptr observer(new rtc::RefCountedObject()); @@ -1030,6 +1075,8 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, typedef std::pair IceUfragPwdPair; std::map ice_ufrag_pwd_; bool expect_ice_restart_ = false; + bool expect_ice_renomination_ = false; + bool expect_remote_ice_renomination_ = false; // Needed to keep track of number of frames sent. rtc::scoped_refptr fake_audio_capture_module_; @@ -2128,6 +2175,32 @@ TEST_F(P2PTestConductor, IceRestart) { EXPECT_NE(receiver_candidate, receiver_candidate_restart); } +TEST_F(P2PTestConductor, IceRenominationDisabled) { + config()->enable_ice_renomination = false; + ASSERT_TRUE(CreateTestClients()); + LocalP2PTest(); + + initializing_client()->VerifyLocalIceRenomination(); + receiving_client()->VerifyLocalIceRenomination(); + initializing_client()->VerifyRemoteIceRenomination(); + receiving_client()->VerifyRemoteIceRenomination(); +} + +TEST_F(P2PTestConductor, IceRenominationEnabled) { + config()->enable_ice_renomination = true; + ASSERT_TRUE(CreateTestClients()); + initializing_client()->SetExpectIceRenomination(true); + initializing_client()->SetExpectRemoteIceRenomination(true); + receiving_client()->SetExpectIceRenomination(true); + receiving_client()->SetExpectRemoteIceRenomination(true); + LocalP2PTest(); + + initializing_client()->VerifyLocalIceRenomination(); + receiving_client()->VerifyLocalIceRenomination(); + initializing_client()->VerifyRemoteIceRenomination(); + receiving_client()->VerifyRemoteIceRenomination(); +} + // This test sets up a call between two parties with audio, and video. // It then renegotiates setting the video m-line to "port 0", then later // renegotiates again, enabling video. diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 1dba14c41a..624c67f9cb 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -317,6 +317,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // If set to true, this means the ICE transport should presume TURN-to-TURN // candidate pairs will succeed, even before a binding response is received. bool presume_writable_when_fully_relayed = false; + // If true, "renomination" will be added to the ice options in the transport + // description. + bool enable_ice_renomination = false; // If true, ICE role is redetermined when peerconnection sets a local // transport description that indicates an ICE restart. bool redetermine_role_on_ice_restart = true; @@ -329,18 +332,13 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // The default value for constraint offerToReceiveX:true. static const int kOfferToReceiveMediaTrue = 1; - int offer_to_receive_video; - int offer_to_receive_audio; - bool voice_activity_detection; - bool ice_restart; - bool use_rtp_mux; + int offer_to_receive_video = kUndefined; + int offer_to_receive_audio = kUndefined; + bool voice_activity_detection = true; + bool ice_restart = false; + bool use_rtp_mux = true; - RTCOfferAnswerOptions() - : offer_to_receive_video(kUndefined), - offer_to_receive_audio(kUndefined), - voice_activity_detection(true), - ice_restart(false), - use_rtp_mux(true) {} + RTCOfferAnswerOptions() = default; RTCOfferAnswerOptions(int offer_to_receive_video, int offer_to_receive_audio, diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index 9f6c563b62..19823f8c34 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -163,13 +163,11 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { void SetIceTiebreaker(uint64_t tiebreaker) override { channel_->SetIceTiebreaker(tiebreaker); } - void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - channel_->SetIceCredentials(ice_ufrag, ice_pwd); + void SetIceParameters(const IceParameters& ice_params) override { + channel_->SetIceParameters(ice_params); } - void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - channel_->SetRemoteIceCredentials(ice_ufrag, ice_pwd); + void SetRemoteIceParameters(const IceParameters& ice_params) override { + channel_->SetRemoteIceParameters(ice_params); } void SetRemoteIceMode(IceMode mode) override { channel_->SetRemoteIceMode(mode); diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index d79525490e..153aa56790 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -88,15 +88,13 @@ class FakeTransportChannel : public TransportChannelImpl, void SetIceTiebreaker(uint64_t tiebreaker) override { tiebreaker_ = tiebreaker; } - void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - ice_ufrag_ = ice_ufrag; - ice_pwd_ = ice_pwd; + void SetIceParameters(const IceParameters& ice_params) override { + ice_ufrag_ = ice_params.ufrag; + ice_pwd_ = ice_params.pwd; } - void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - remote_ice_ufrag_ = ice_ufrag; - remote_ice_pwd_ = ice_pwd; + void SetRemoteIceParameters(const IceParameters& params) override { + remote_ice_ufrag_ = params.ufrag; + remote_ice_pwd_ = params.pwd; } void SetRemoteIceMode(IceMode mode) override { remote_ice_mode_ = mode; } diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc index 03b0ed426a..f5f8a665aa 100644 --- a/webrtc/p2p/base/p2ptransportchannel.cc +++ b/webrtc/p2p/base/p2ptransportchannel.cc @@ -308,38 +308,37 @@ TransportChannelState P2PTransportChannel::ComputeState() const { return TransportChannelState::STATE_COMPLETED; } -void P2PTransportChannel::SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) { +void P2PTransportChannel::SetIceParameters(const IceParameters& ice_params) { ASSERT(worker_thread_ == rtc::Thread::Current()); - ice_ufrag_ = ice_ufrag; - ice_pwd_ = ice_pwd; + ice_parameters_ = ice_params; // Note: Candidate gathering will restart when MaybeStartGathering is next // called. } -void P2PTransportChannel::SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) { +void P2PTransportChannel::SetRemoteIceParameters( + const IceParameters& ice_params) { ASSERT(worker_thread_ == rtc::Thread::Current()); + LOG(LS_INFO) << "Remote supports ICE renomination ? " + << ice_params.renomination; IceParameters* current_ice = remote_ice(); - IceParameters new_ice(ice_ufrag, ice_pwd); - if (!current_ice || *current_ice != new_ice) { + if (!current_ice || *current_ice != ice_params) { // Keep the ICE credentials so that newer connections // are prioritized over the older ones. - remote_ice_parameters_.push_back(new_ice); + remote_ice_parameters_.push_back(ice_params); } // Update the pwd of remote candidate if needed. for (RemoteCandidate& candidate : remote_candidates_) { - if (candidate.username() == ice_ufrag && candidate.password().empty()) { - candidate.set_password(ice_pwd); + if (candidate.username() == ice_params.ufrag && + candidate.password().empty()) { + candidate.set_password(ice_params.pwd); } } // We need to update the credentials and generation for any peer reflexive // candidates. for (Connection* conn : connections_) { - conn->MaybeSetRemoteIceCredentialsAndGeneration( - ice_ufrag, ice_pwd, - static_cast(remote_ice_parameters_.size() - 1)); + conn->MaybeSetRemoteIceParametersAndGeneration( + ice_params, static_cast(remote_ice_parameters_.size() - 1)); } // Updating the remote ICE candidate generation could change the sort order. RequestSortAndStateUpdate(); @@ -434,22 +433,23 @@ const IceConfig& P2PTransportChannel::config() const { } void P2PTransportChannel::MaybeStartGathering() { - if (ice_ufrag_.empty() || ice_pwd_.empty()) { + if (ice_parameters_.ufrag.empty() || ice_parameters_.pwd.empty()) { return; } // Start gathering if we never started before, or if an ICE restart occurred. if (allocator_sessions_.empty() || IceCredentialsChanged(allocator_sessions_.back()->ice_ufrag(), - allocator_sessions_.back()->ice_pwd(), ice_ufrag_, - ice_pwd_)) { + allocator_sessions_.back()->ice_pwd(), + ice_parameters_.ufrag, ice_parameters_.pwd)) { if (gathering_state_ != kIceGatheringGathering) { gathering_state_ = kIceGatheringGathering; SignalGatheringState(this); } // Time for a new allocator. std::unique_ptr pooled_session = - allocator_->TakePooledSession(transport_name(), component(), ice_ufrag_, - ice_pwd_); + allocator_->TakePooledSession(transport_name(), component(), + ice_parameters_.ufrag, + ice_parameters_.pwd); if (pooled_session) { AddAllocatorSession(std::move(pooled_session)); PortAllocatorSession* raw_pooled_session = @@ -465,7 +465,8 @@ void P2PTransportChannel::MaybeStartGathering() { } } else { AddAllocatorSession(allocator_->CreateSession( - transport_name(), component(), ice_ufrag_, ice_pwd_)); + transport_name(), component(), ice_parameters_.ufrag, + ice_parameters_.pwd)); LOG(LS_INFO) << "Start getting ports"; allocator_sessions_.back()->StartGettingPorts(); } @@ -1625,7 +1626,10 @@ void P2PTransportChannel::PingConnection(Connection* conn) { bool use_candidate_attr = false; uint32_t nomination = 0; if (ice_role_ == ICEROLE_CONTROLLING) { - if (remote_supports_renomination_) { + bool renomination_supported = ice_parameters_.renomination && + !remote_ice_parameters_.empty() && + remote_ice_parameters_.back().renomination; + if (renomination_supported) { nomination = GetNominationAttr(conn); } else { use_candidate_attr = diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 25510c2f64..a6dfd88f87 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -43,18 +43,6 @@ extern const int STABILIZING_WRITABLE_CONNECTION_PING_INTERVAL; extern const int STABLE_WRITABLE_CONNECTION_PING_INTERVAL; static const int MIN_PINGS_AT_WEAK_PING_INTERVAL = 3; -struct IceParameters { - std::string ufrag; - std::string pwd; - IceParameters(const std::string& ice_ufrag, const std::string& ice_pwd) - : ufrag(ice_ufrag), pwd(ice_pwd) {} - - bool operator==(const IceParameters& other) { - return ufrag == other.ufrag && pwd == other.pwd; - } - bool operator!=(const IceParameters& other) { return !(*this == other); } -}; - // Adds the port on which the candidate originated. class RemoteCandidate : public Candidate { public: @@ -88,10 +76,8 @@ class P2PTransportChannel : public TransportChannelImpl, void SetIceRole(IceRole role) override; IceRole GetIceRole() const override { return ice_role_; } void SetIceTiebreaker(uint64_t tiebreaker) override; - void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override; - void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override; + void SetIceParameters(const IceParameters& ice_params) override; + void SetRemoteIceParameters(const IceParameters& ice_params) override; void SetRemoteIceMode(IceMode mode) override; // TODO(deadbeef): Deprecated. Remove when Chromium's // IceTransportChannel does not depend on this. @@ -209,11 +195,6 @@ class P2PTransportChannel : public TransportChannelImpl, return remote_candidates_; } - // Public for unit tests. - void set_remote_supports_renomination(bool remote_supports_renomination) { - remote_supports_renomination_ = remote_supports_renomination; - } - private: rtc::Thread* thread() const { return worker_thread_; } bool IsGettingPorts() { return allocator_session()->IsGettingPorts(); } @@ -395,8 +376,7 @@ class P2PTransportChannel : public TransportChannelImpl, bool had_connection_ = false; // if connections_ has ever been nonempty typedef std::map OptionMap; OptionMap options_; - std::string ice_ufrag_; - std::string ice_pwd_; + IceParameters ice_parameters_; std::vector remote_ice_parameters_; IceMode remote_ice_mode_; IceRole ice_role_; @@ -410,10 +390,6 @@ class P2PTransportChannel : public TransportChannelImpl, IceConfig config_; int last_sent_packet_id_ = -1; // -1 indicates no packet was sent before. bool started_pinging_ = false; - // TODO(honghaiz): Put this and ICE role inside ICEParameters and rename this - // as renomination. Set its value in subsequent CLs based on signaling - // exchange. - bool remote_supports_renomination_ = false; // The value put in the "nomination" attribute for the next nominated // connection. A zero-value indicates the connection will not be nominated. uint32_t nomination_ = 0; diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc index 26db621a9d..d7e87737a4 100644 --- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc +++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc @@ -92,16 +92,19 @@ static const SocketAddress kTurnUdpExtAddr("99.99.99.5", 0); static const cricket::RelayCredentials kRelayCredentials("test", "test"); // Based on ICE_UFRAG_LENGTH -static const char* kIceUfrag[4] = {"UF00", "UF01", - "UF02", "UF03"}; +const char* kIceUfrag[4] = {"UF00", "UF01", "UF02", "UF03"}; // Based on ICE_PWD_LENGTH -static const char* kIcePwd[4] = {"TESTICEPWD00000000000000", - "TESTICEPWD00000000000001", - "TESTICEPWD00000000000002", - "TESTICEPWD00000000000003"}; +const char* kIcePwd[4] = { + "TESTICEPWD00000000000000", "TESTICEPWD00000000000001", + "TESTICEPWD00000000000002", "TESTICEPWD00000000000003"}; +const cricket::IceParameters kIceParams[4] = { + {kIceUfrag[0], kIcePwd[0], false}, + {kIceUfrag[1], kIcePwd[1], false}, + {kIceUfrag[2], kIcePwd[2], false}, + {kIceUfrag[3], kIcePwd[3], false}}; -static const uint64_t kLowTiebreaker = 11111; -static const uint64_t kHighTiebreaker = 22222; +const uint64_t kLowTiebreaker = 11111; +const uint64_t kHighTiebreaker = 22222; enum { MSG_ADD_CANDIDATES, MSG_REMOVE_CANDIDATES }; @@ -131,7 +134,7 @@ cricket::Candidate CreateUdpCandidate(const std::string& type, return c; } -} // namespace { +} // namespace namespace cricket { @@ -305,34 +308,38 @@ class P2PTransportChannelTestBase : public testing::Test, return ep2_.GetChannelData(channel); } + IceParameters IceParamsWithRenomination(const IceParameters& ice, + bool renomination) { + IceParameters new_ice = ice; + new_ice.renomination = renomination; + return new_ice; + } + void CreateChannels(int num, const IceConfig& ep1_config, - const IceConfig& ep2_config) { - std::string ice_ufrag_ep1_cd1_ch = kIceUfrag[0]; - std::string ice_pwd_ep1_cd1_ch = kIcePwd[0]; - std::string ice_ufrag_ep2_cd1_ch = kIceUfrag[1]; - std::string ice_pwd_ep2_cd1_ch = kIcePwd[1]; - ep1_.cd1_.ch_.reset(CreateChannel( - 0, ICE_CANDIDATE_COMPONENT_DEFAULT, ice_ufrag_ep1_cd1_ch, - ice_pwd_ep1_cd1_ch, ice_ufrag_ep2_cd1_ch, ice_pwd_ep2_cd1_ch)); - ep2_.cd1_.ch_.reset(CreateChannel( - 1, ICE_CANDIDATE_COMPONENT_DEFAULT, ice_ufrag_ep2_cd1_ch, - ice_pwd_ep2_cd1_ch, ice_ufrag_ep1_cd1_ch, ice_pwd_ep1_cd1_ch)); + const IceConfig& ep2_config, + bool renomination = false) { + IceParameters ice_ep1_cd1_ch = + IceParamsWithRenomination(kIceParams[0], renomination); + IceParameters ice_ep2_cd1_ch = + IceParamsWithRenomination(kIceParams[1], renomination); + ep1_.cd1_.ch_.reset(CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, + ice_ep1_cd1_ch, ice_ep2_cd1_ch)); + ep2_.cd1_.ch_.reset(CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, + ice_ep2_cd1_ch, ice_ep1_cd1_ch)); ep1_.cd1_.ch_->SetIceConfig(ep1_config); ep2_.cd1_.ch_->SetIceConfig(ep2_config); ep1_.cd1_.ch_->MaybeStartGathering(); ep2_.cd1_.ch_->MaybeStartGathering(); if (num == 2) { - std::string ice_ufrag_ep1_cd2_ch = kIceUfrag[2]; - std::string ice_pwd_ep1_cd2_ch = kIcePwd[2]; - std::string ice_ufrag_ep2_cd2_ch = kIceUfrag[3]; - std::string ice_pwd_ep2_cd2_ch = kIcePwd[3]; - ep1_.cd2_.ch_.reset(CreateChannel( - 0, ICE_CANDIDATE_COMPONENT_DEFAULT, ice_ufrag_ep1_cd2_ch, - ice_pwd_ep1_cd2_ch, ice_ufrag_ep2_cd2_ch, ice_pwd_ep2_cd2_ch)); - ep2_.cd2_.ch_.reset(CreateChannel( - 1, ICE_CANDIDATE_COMPONENT_DEFAULT, ice_ufrag_ep2_cd2_ch, - ice_pwd_ep2_cd2_ch, ice_ufrag_ep1_cd2_ch, ice_pwd_ep1_cd2_ch)); + IceParameters ice_ep1_cd2_ch = + IceParamsWithRenomination(kIceParams[2], renomination); + IceParameters ice_ep2_cd2_ch = + IceParamsWithRenomination(kIceParams[3], renomination); + ep1_.cd2_.ch_.reset(CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, + ice_ep1_cd2_ch, ice_ep2_cd2_ch)); + ep2_.cd2_.ch_.reset(CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, + ice_ep2_cd2_ch, ice_ep1_cd2_ch)); ep1_.cd2_.ch_->SetIceConfig(ep1_config); ep2_.cd2_.ch_->SetIceConfig(ep2_config); ep1_.cd2_.ch_->MaybeStartGathering(); @@ -342,15 +349,13 @@ class P2PTransportChannelTestBase : public testing::Test, void CreateChannels(int num) { IceConfig default_config; - CreateChannels(num, default_config, default_config); + CreateChannels(num, default_config, default_config, false); } P2PTransportChannel* CreateChannel(int endpoint, int component, - const std::string& local_ice_ufrag, - const std::string& local_ice_pwd, - const std::string& remote_ice_ufrag, - const std::string& remote_ice_pwd) { + const IceParameters& local_ice, + const IceParameters& remote_ice) { P2PTransportChannel* channel = new P2PTransportChannel( "test content name", component, GetAllocator(endpoint)); channel->SignalReadyToSend.connect( @@ -365,9 +370,9 @@ class P2PTransportChannelTestBase : public testing::Test, this, &P2PTransportChannelTestBase::OnRoleConflict); channel->SignalSelectedCandidatePairChanged.connect( this, &P2PTransportChannelTestBase::OnSelectedCandidatePairChanged); - channel->SetIceCredentials(local_ice_ufrag, local_ice_pwd); - if (remote_ice_credential_source_ == FROM_SETICECREDENTIALS) { - channel->SetRemoteIceCredentials(remote_ice_ufrag, remote_ice_pwd); + channel->SetIceParameters(local_ice); + if (remote_ice_parameter_source_ == FROM_SETICEPARAMETERS) { + channel->SetRemoteIceParameters(remote_ice); } channel->SetIceRole(GetEndpoint(endpoint)->ice_role()); channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker()); @@ -598,13 +603,13 @@ class P2PTransportChannelTestBase : public testing::Test, } // This test waits for the transport to become receiving and writable on both - // end points. Once they are, the end points set new local ice credentials and + // end points. Once they are, the end points set new local ice parameters and // restart the ice gathering. Finally it waits for the transport to select a // new connection using the newly generated ice candidates. // Before calling this function the end points must be configured. void TestHandleIceUfragPasswordChanged() { - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[1]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[0]); EXPECT_TRUE_WAIT_MARGIN(ep1_ch1()->receiving() && ep1_ch1()->writable() && ep2_ch1()->receiving() && ep2_ch1()->writable(), 1000, 1000); @@ -614,11 +619,12 @@ class P2PTransportChannelTestBase : public testing::Test, const Candidate* old_remote_candidate1 = RemoteCandidate(ep1_ch1()); const Candidate* old_remote_candidate2 = RemoteCandidate(ep2_ch1()); - ep1_ch1()->SetIceCredentials(kIceUfrag[2], kIcePwd[2]); - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); + ep1_ch1()->SetIceParameters(kIceParams[2]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); ep1_ch1()->MaybeStartGathering(); - ep2_ch1()->SetIceCredentials(kIceUfrag[3], kIcePwd[3]); - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); + ep2_ch1()->SetIceParameters(kIceParams[3]); + + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); ep2_ch1()->MaybeStartGathering(); EXPECT_TRUE_WAIT_MARGIN(LocalCandidate(ep1_ch1())->generation() != @@ -745,7 +751,7 @@ class P2PTransportChannelTestBase : public testing::Test, return; } for (auto& c : data->candidates) { - if (remote_ice_credential_source_ != FROM_CANDIDATE) { + if (remote_ice_parameter_source_ != FROM_CANDIDATE) { c.set_username(""); c.set_password(""); } @@ -830,13 +836,13 @@ class P2PTransportChannelTestBase : public testing::Test, return GetChannelData(ch)->ch_packets_; } - enum RemoteIceCredentialSource { FROM_CANDIDATE, FROM_SETICECREDENTIALS }; + enum RemoteIceParameterSource { FROM_CANDIDATE, FROM_SETICEPARAMETERS }; - // How does the test pass ICE credentials to the P2PTransportChannel? - // On the candidate itself, or through SetIceCredentials? + // How does the test pass ICE parameters to the P2PTransportChannel? + // On the candidate itself, or through SetRemoteIceParameters? // Goes through the candidate itself by default. - void set_remote_ice_credential_source(RemoteIceCredentialSource source) { - remote_ice_credential_source_ = source; + void set_remote_ice_parameter_source(RemoteIceParameterSource source) { + remote_ice_parameter_source_ = source; } void set_force_relay(bool relay) { @@ -865,7 +871,7 @@ class P2PTransportChannelTestBase : public testing::Test, rtc::SocksProxyServer socks_server2_; Endpoint ep1_; Endpoint ep2_; - RemoteIceCredentialSource remote_ice_credential_source_ = FROM_CANDIDATE; + RemoteIceParameterSource remote_ice_parameter_source_ = FROM_CANDIDATE; bool force_relay_; int selected_candidate_pair_switches_ = 0; @@ -991,7 +997,7 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase { SetAllocatorFlags(1, allocator_flags2); SetAllocationStepDelay(1, delay); - set_remote_ice_credential_source(FROM_SETICECREDENTIALS); + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); } void ConfigureEndpoint(int endpoint, Config config) { switch (config) { @@ -1202,11 +1208,11 @@ TEST_F(P2PTransportChannelTest, GetStats) { TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, kDefaultPortAllocatorFlags); - // Emulate no remote credentials coming in. - set_remote_ice_credential_source(FROM_CANDIDATE); + // Emulate no remote parameters coming in. + set_remote_ice_parameter_source(FROM_CANDIDATE); CreateChannels(1); - // Only have remote credentials come in for ep2, not ep1. - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); + // Only have remote parameters come in for ep2, not ep1. + ep2_ch1()->SetRemoteIceParameters(kIceParams[0]); // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive // candidate. @@ -1217,8 +1223,8 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { ASSERT_TRUE_WAIT(ep2_ch1()->selected_connection() != nullptr, 3000); // Add two sets of remote ICE credentials, so that the ones used by the // candidate will be generation 1 instead of 0. - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[1]); // The caller should have the selected connection connected to the peer // reflexive candidate. const Connection* selected_connection = nullptr; @@ -1244,11 +1250,11 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignaling) { TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { ConfigureEndpoints(OPEN, NAT_SYMMETRIC, kDefaultPortAllocatorFlags, kDefaultPortAllocatorFlags); - // Emulate no remote credentials coming in. - set_remote_ice_credential_source(FROM_CANDIDATE); + // Emulate no remote parameters coming in. + set_remote_ice_parameter_source(FROM_CANDIDATE); CreateChannels(1); - // Only have remote credentials come in for ep2, not ep1. - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); + // Only have remote parameters come in for ep2, not ep1. + ep2_ch1()->SetRemoteIceParameters(kIceParams[0]); // Pause sending ep2's candidates to ep1 until ep1 receives the peer reflexive // candidate. PauseCandidates(1); @@ -1258,8 +1264,8 @@ TEST_F(P2PTransportChannelTest, PeerReflexiveCandidateBeforeSignalingWithNAT) { ASSERT_TRUE_WAIT(ep2_ch1()->selected_connection() != nullptr, 3000); // Add two sets of remote ICE credentials, so that the ones used by the // candidate will be generation 1 instead of 0. - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[1]); // The caller's selected connection should be connected to the peer reflexive // candidate. @@ -1298,21 +1304,22 @@ TEST_F(P2PTransportChannelTest, // it's prioritized above the current candidate pair. GetEndpoint(0)->allocator_->set_candidate_filter(CF_RELAY); GetEndpoint(1)->allocator_->set_candidate_filter(CF_RELAY); - // Setting this allows us to control when SetRemoteIceCredentials is called. - set_remote_ice_credential_source(FROM_CANDIDATE); + // Setting this allows us to control when SetRemoteIceParameters is called. + set_remote_ice_parameter_source(FROM_CANDIDATE); CreateChannels(1); // Wait for the initial connection to be made. - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[0], kIcePwd[0]); + ep1_ch1()->SetRemoteIceParameters(kIceParams[1]); + ep2_ch1()->SetRemoteIceParameters(kIceParams[0]); EXPECT_TRUE_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && ep2_ch1()->receiving() && ep2_ch1()->writable(), kDefaultTimeout); // Simulate an ICE restart on ep2, but don't signal the candidate or new - // ICE credentials until after a prflx connection has been made. + // ICE parameters until after a prflx connection has been made. PauseCandidates(1); - ep2_ch1()->SetIceCredentials(kIceUfrag[3], kIcePwd[3]); - ep1_ch1()->SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); + ep2_ch1()->SetIceParameters(kIceParams[3]); + + ep1_ch1()->SetRemoteIceParameters(kIceParams[3]); ep2_ch1()->MaybeStartGathering(); // The caller should have the selected connection connected to the peer @@ -1324,8 +1331,9 @@ TEST_F(P2PTransportChannelTest, ep1_ch1()->selected_connection(); // Now simulate the ICE restart on ep1. - ep1_ch1()->SetIceCredentials(kIceUfrag[2], kIcePwd[2]); - ep2_ch1()->SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); + ep1_ch1()->SetIceParameters(kIceParams[2]); + + ep2_ch1()->SetRemoteIceParameters(kIceParams[2]); ep1_ch1()->MaybeStartGathering(); // Finally send the candidates from ep2's ICE restart and verify that ep1 uses @@ -1341,7 +1349,7 @@ TEST_F(P2PTransportChannelTest, // Test that if remote candidates don't have ufrag and pwd, we still work. TEST_F(P2PTransportChannelTest, RemoteCandidatesWithoutUfragPwd) { - set_remote_ice_credential_source(FROM_SETICECREDENTIALS); + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags, kDefaultPortAllocatorFlags); CreateChannels(1); @@ -1410,10 +1418,10 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionsFromActiveToPassive) { SetAllowTcpListen(0, true); // actpass. SetAllowTcpListen(1, false); // active. - // We want SetRemoteIceCredentials to be called as it normally would. - // Otherwise we won't know what credentials to use for the expected + // We want SetRemoteIceParameters to be called as it normally would. + // Otherwise we won't know what parameters to use for the expected // prflx TCP candidates. - set_remote_ice_credential_source(FROM_SETICECREDENTIALS); + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); // Pause candidate so we could verify the candidate properties. PauseCandidates(0); @@ -1700,9 +1708,8 @@ TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) { kDefaultPortAllocatorFlags); // Only configure one channel so we can control when the remote candidate // is added. - GetEndpoint(0)->cd1_.ch_.reset( - CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[0], - kIcePwd[0], kIceUfrag[1], kIcePwd[1])); + GetEndpoint(0)->cd1_.ch_.reset(CreateChannel( + 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1])); IceConfig config; config.presume_writable_when_fully_relayed = true; ep1_ch1()->SetIceConfig(config); @@ -1748,12 +1755,10 @@ TEST_F(P2PTransportChannelTest, TurnToPrflxPresumedWritable) { test_turn_server()->set_enable_permission_checks(false); IceConfig config; config.presume_writable_when_fully_relayed = true; - GetEndpoint(0)->cd1_.ch_.reset( - CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[0], - kIcePwd[0], kIceUfrag[1], kIcePwd[1])); - GetEndpoint(1)->cd1_.ch_.reset( - CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[1], - kIcePwd[1], kIceUfrag[0], kIcePwd[0])); + GetEndpoint(0)->cd1_.ch_.reset(CreateChannel( + 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1])); + GetEndpoint(1)->cd1_.ch_.reset(CreateChannel( + 1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[1], kIceParams[0])); ep1_ch1()->SetIceConfig(config); ep2_ch1()->SetIceConfig(config); // Don't signal candidates from channel 2, so that channel 1 sees the TURN @@ -1789,12 +1794,10 @@ TEST_F(P2PTransportChannelTest, PresumedWritablePreferredOverUnreliable) { kDefaultPortAllocatorFlags); IceConfig config; config.presume_writable_when_fully_relayed = true; - GetEndpoint(0)->cd1_.ch_.reset( - CreateChannel(0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[0], - kIcePwd[0], kIceUfrag[1], kIcePwd[1])); - GetEndpoint(1)->cd1_.ch_.reset( - CreateChannel(1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceUfrag[1], - kIcePwd[1], kIceUfrag[0], kIcePwd[0])); + GetEndpoint(0)->cd1_.ch_.reset(CreateChannel( + 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1])); + GetEndpoint(1)->cd1_.ch_.reset(CreateChannel( + 1, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[1], kIceParams[0])); ep1_ch1()->SetIceConfig(config); ep2_ch1()->SetIceConfig(config); ep1_ch1()->MaybeStartGathering(); @@ -1837,7 +1840,7 @@ class P2PTransportChannelSameNatTest : public P2PTransportChannelTestBase { static_cast(nat_type - NAT_FULL_CONE)); ConfigureEndpoint(outer_nat, 0, config1); ConfigureEndpoint(outer_nat, 1, config2); - set_remote_ice_credential_source(FROM_SETICECREDENTIALS); + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); } void ConfigureEndpoint(rtc::NATSocketServer::Translator* nat, int endpoint, Config config) { @@ -2027,11 +2030,12 @@ TEST_F(P2PTransportChannelMultihomedTest, TestIceRenomination) { SetAllocatorFlags(0, kOnlyLocalPorts); SetAllocatorFlags(1, kOnlyLocalPorts); + // We want it to set the remote ICE parameters when creating channels. + set_remote_ice_parameter_source(FROM_SETICEPARAMETERS); // Make the receiving timeout shorter for testing. IceConfig config = CreateIceConfig(1000, GATHER_ONCE); - // Create channels and let them go writable, as usual. - CreateChannels(1, config, config); - ep1_ch1()->set_remote_supports_renomination(true); + // Create channels with ICE renomination and let them go writable as usual. + CreateChannels(1, config, config, true); EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() && ep2_ch1()->receiving() && ep2_ch1()->writable(), @@ -2601,8 +2605,8 @@ class P2PTransportChannelPingTest : public testing::Test, protected: void PrepareChannel(P2PTransportChannel* ch) { ch->SetIceRole(ICEROLE_CONTROLLING); - ch->SetIceCredentials(kIceUfrag[0], kIcePwd[0]); - ch->SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ch->SetIceParameters(kIceParams[0]); + ch->SetRemoteIceParameters(kIceParams[1]); ch->SignalSelectedCandidatePairChanged.connect( this, &P2PTransportChannelPingTest::OnSelectedCandidatePairChanged); ch->SignalReadyToSend.connect(this, @@ -2881,20 +2885,20 @@ TEST_F(P2PTransportChannelPingTest, TestStunPingIntervals) { } // Test that we start pinging as soon as we have a connection and remote ICE -// credentials. +// parameters. TEST_F(P2PTransportChannelPingTest, PingingStartedAsSoonAsPossible) { rtc::ScopedFakeClock clock; FakePortAllocator pa(rtc::Thread::Current(), nullptr); P2PTransportChannel ch("TestChannel", 1, &pa); ch.SetIceRole(ICEROLE_CONTROLLING); - ch.SetIceCredentials(kIceUfrag[0], kIcePwd[0]); + ch.SetIceParameters(kIceParams[0]); ch.MaybeStartGathering(); EXPECT_EQ_WAIT(IceGatheringState::kIceGatheringComplete, ch.gathering_state(), kDefaultTimeout); // Simulate a binding request being received, creating a peer reflexive - // candidate pair while we still don't have remote ICE credentials. + // candidate pair while we still don't have remote ICE parameters. IceMessage request; request.SetType(STUN_BINDING_REQUEST); request.AddAttribute( @@ -2910,14 +2914,14 @@ TEST_F(P2PTransportChannelPingTest, PingingStartedAsSoonAsPossible) { ASSERT_NE(nullptr, conn); // Simulate waiting for a second (and change) and verify that no pings were - // sent, since we don't yet have remote ICE credentials. + // sent, since we don't yet have remote ICE parameters. SIMULATED_WAIT(conn->num_pings_sent() > 0, 1025, clock); EXPECT_EQ(0, conn->num_pings_sent()); - // Set remote ICE credentials. Now we should be able to ping. Ensure that + // Set remote ICE parameters. Now we should be able to ping. Ensure that // the first ping is sent as soon as possible, within one simulated clock // tick. - ch.SetRemoteIceCredentials(kIceUfrag[1], kIcePwd[1]); + ch.SetRemoteIceParameters(kIceParams[1]); EXPECT_TRUE_SIMULATED_WAIT(conn->num_pings_sent() > 0, 1, clock); } @@ -2981,7 +2985,7 @@ TEST_F(P2PTransportChannelPingTest, TestSignalStateChanged) { // is added with an old ufrag, it will be discarded. If it is added with a // ufrag that was not seen before, it will be used to create connections // although the ICE pwd in the remote candidate will be set when the ICE -// credentials arrive. If a remote candidate is added with the current ICE +// parameters arrive. If a remote candidate is added with the current ICE // ufrag, its pwd and generation will be set properly. TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) { FakePortAllocator pa(rtc::Thread::Current(), nullptr); @@ -2998,10 +3002,10 @@ TEST_F(P2PTransportChannelPingTest, TestAddRemoteCandidateWithVariousUfrags) { EXPECT_TRUE(candidate.password().empty()); EXPECT_TRUE(FindNextPingableConnectionAndPingIt(&ch) == nullptr); - // Set the remote credentials with the "future" ufrag. + // Set the remote ICE parameters with the "future" ufrag. // This should set the ICE pwd in the remote candidate of |conn1|, making // it pingable. - ch.SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); + ch.SetRemoteIceParameters(kIceParams[2]); EXPECT_EQ(kIceUfrag[2], candidate.username()); EXPECT_EQ(kIcePwd[2], candidate.password()); EXPECT_EQ(conn1, FindNextPingableConnectionAndPingIt(&ch)); @@ -3261,8 +3265,8 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) { // Test that the request from an unknown address contains a ufrag from an old // generation. port->set_sent_binding_response(false); - ch.SetRemoteIceCredentials(kIceUfrag[2], kIcePwd[2]); - ch.SetRemoteIceCredentials(kIceUfrag[3], kIcePwd[3]); + ch.SetRemoteIceParameters(kIceParams[2]); + ch.SetRemoteIceParameters(kIceParams[3]); port->SignalUnknownAddress(port, rtc::SocketAddress("5.5.5.5", 5), PROTO_UDP, &request, kIceUfrag[2], false); Connection* conn5 = WaitForConnectionTo(&ch, "5.5.5.5", 5); @@ -3732,7 +3736,7 @@ TEST_F(P2PTransportChannelPingTest, TestStopPortAllocatorSessions) { // Start a new session. Even though conn1, which belongs to an older // session, becomes unwritable and writable again, it should not stop the // current session. - ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]); + ch.SetIceParameters(kIceParams[1]); ch.MaybeStartGathering(); conn1->Prune(); conn1->ReceivedPingResponse(LOW_RTT, "id"); @@ -3788,7 +3792,7 @@ TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnPortAfterIceRestart) { // Do an ICE restart, change the role, and expect the old port to have its // role updated. - ch.SetIceCredentials(kIceUfrag[1], kIcePwd[1]); + ch.SetIceParameters(kIceParams[1]); ch.MaybeStartGathering(); ch.SetIceRole(ICEROLE_CONTROLLED); EXPECT_EQ(ICEROLE_CONTROLLED, conn->port()->GetIceRole()); diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index c648450b9a..2f487dc179 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1423,19 +1423,18 @@ void Connection::HandleRoleConflictFromPeer() { port_->SignalRoleConflict(port_); } -void Connection::MaybeSetRemoteIceCredentialsAndGeneration( - const std::string& ice_ufrag, - const std::string& ice_pwd, +void Connection::MaybeSetRemoteIceParametersAndGeneration( + const IceParameters& ice_params, int generation) { - if (remote_candidate_.username() == ice_ufrag && + if (remote_candidate_.username() == ice_params.ufrag && remote_candidate_.password().empty()) { - remote_candidate_.set_password(ice_pwd); + remote_candidate_.set_password(ice_params.pwd); } // TODO(deadbeef): A value of '0' for the generation is used for both // generation 0 and "generation unknown". It should be changed to an // rtc::Optional to fix this. - if (remote_candidate_.username() == ice_ufrag && - remote_candidate_.password() == ice_pwd && + if (remote_candidate_.username() == ice_params.ufrag && + remote_candidate_.password() == ice_params.pwd && remote_candidate_.generation() == 0) { remote_candidate_.set_generation(generation); } diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 2dc9deaddb..2acea42f68 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -590,14 +590,11 @@ class Connection : public CandidatePairInterface, uint32_t ComputeNetworkCost() const; - // Update the ICE password and/or generation of the remote candidate if a - // ufrag in |remote_ice_parameters| matches the candidate's ufrag, and the + // Update the ICE password and/or generation of the remote candidate if the + // ufrag in |params| matches the candidate's ufrag, and the // candidate's password and/or ufrag has not been set. - // |remote_ice_parameters| should be a list of known ICE parameters ordered - // by generation. - void MaybeSetRemoteIceCredentialsAndGeneration(const std::string& ice_ufrag, - const std::string& ice_pwd, - int generation); + void MaybeSetRemoteIceParametersAndGeneration(const IceParameters& params, + int generation); // If |remote_candidate_| is peer reflexive and is equivalent to // |new_candidate| except the type, update |remote_candidate_| to diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index 9fbe9524c3..7bd14c4cad 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -321,15 +321,13 @@ bool Transport::RemoveRemoteCandidates(const std::vector& candidates, bool Transport::ApplyLocalTransportDescription(TransportChannelImpl* ch, std::string* error_desc) { - ch->SetIceCredentials(local_description_->ice_ufrag, - local_description_->ice_pwd); + ch->SetIceParameters(local_description_->GetIceParameters()); return true; } bool Transport::ApplyRemoteTransportDescription(TransportChannelImpl* ch, std::string* error_desc) { - ch->SetRemoteIceCredentials(remote_description_->ice_ufrag, - remote_description_->ice_pwd); + ch->SetRemoteIceParameters(remote_description_->GetIceParameters()); return true; } diff --git a/webrtc/p2p/base/transportchannelimpl.h b/webrtc/p2p/base/transportchannelimpl.h index 910ec3133e..0c7fa506b7 100644 --- a/webrtc/p2p/base/transportchannelimpl.h +++ b/webrtc/p2p/base/transportchannelimpl.h @@ -44,15 +44,26 @@ class TransportChannelImpl : public TransportChannel { // TODO(pthatcher): Remove this once it's no longer called in // remoting/protocol/libjingle_transport_factory.cc virtual void SetIceProtocolType(IceProtocolType type) {} - // SetIceCredentials only need to be implemented by the ICE - // transport channels. Non-ICE transport channels can just ignore. - // The ufrag and pwd must be set before candidate gathering can start. + // TODO(honghaiz): Remove this once the call in chromoting is removed. virtual void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) = 0; - // SetRemoteIceCredentials only need to be implemented by the ICE - // transport channels. Non-ICE transport channels can just ignore. + const std::string& ice_pwd) { + SetIceParameters(IceParameters(ice_ufrag, ice_pwd, false)); + } + // TODO(honghaiz): Remove this once the call in chromoting is removed. virtual void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) = 0; + const std::string& ice_pwd) { + SetRemoteIceParameters(IceParameters(ice_ufrag, ice_pwd, false)); + } + + // SetIceParameters only needs to be implemented by the ICE transport + // channels. Non-ICE transport channels should pass them down to the inner + // ICE transport channel. The ufrag and pwd in |ice_params| must be set + // before candidate gathering can start. + virtual void SetIceParameters(const IceParameters& ice_params) = 0; + // SetRemoteIceParameters only needs to be implemented by the ICE transport + // channels. Non-ICE transport channels should pass them down to the inner + // ICE transport channel. + virtual void SetRemoteIceParameters(const IceParameters& ice_params) = 0; // SetRemoteIceMode must be implemented only by the ICE transport channels. virtual void SetRemoteIceMode(IceMode mode) = 0; diff --git a/webrtc/p2p/base/transportdescription.h b/webrtc/p2p/base/transportdescription.h index 42e45a670f..df8ee92003 100644 --- a/webrtc/p2p/base/transportdescription.h +++ b/webrtc/p2p/base/transportdescription.h @@ -60,11 +60,33 @@ enum ConnectionRole { CONNECTIONROLE_HOLDCONN, }; +struct IceParameters { + // TODO(honghaiz): Include ICE mode in this structure to match the ORTC + // struct: + // http://ortc.org/wp-content/uploads/2016/03/ortc.html#idl-def-RTCIceParameters + std::string ufrag; + std::string pwd; + bool renomination = false; + IceParameters() = default; + IceParameters(const std::string& ice_ufrag, + const std::string& ice_pwd, + bool ice_renomination) + : ufrag(ice_ufrag), pwd(ice_pwd), renomination(ice_renomination) {} + + bool operator==(const IceParameters& other) { + return ufrag == other.ufrag && pwd == other.pwd && + renomination == other.renomination; + } + bool operator!=(const IceParameters& other) { return !(*this == other); } +}; + extern const char CONNECTIONROLE_ACTIVE_STR[]; extern const char CONNECTIONROLE_PASSIVE_STR[]; extern const char CONNECTIONROLE_ACTPASS_STR[]; extern const char CONNECTIONROLE_HOLDCONN_STR[]; +constexpr auto ICE_RENOMINATION_STR = "renomination"; + bool StringToConnectionRole(const std::string& role_str, ConnectionRole* role); bool ConnectionRoleToString(const ConnectionRole& role, std::string* role_str); @@ -125,6 +147,10 @@ struct TransportDescription { } bool secure() const { return identity_fingerprint != NULL; } + IceParameters GetIceParameters() { + return IceParameters(ice_ufrag, ice_pwd, HasOption(ICE_RENOMINATION_STR)); + } + static rtc::SSLFingerprint* CopyFingerprint( const rtc::SSLFingerprint* from) { if (!from) diff --git a/webrtc/p2p/base/transportdescriptionfactory.cc b/webrtc/p2p/base/transportdescriptionfactory.cc index e57b7e3efa..61e2a9f346 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.cc +++ b/webrtc/p2p/base/transportdescriptionfactory.cc @@ -37,6 +37,9 @@ TransportDescription* TransportDescriptionFactory::CreateOffer( desc->ice_ufrag = current_description->ice_ufrag; desc->ice_pwd = current_description->ice_pwd; } + if (options.enable_ice_renomination) { + desc->AddOption(ICE_RENOMINATION_STR); + } // If we are trying to establish a secure transport, add a fingerprint. if (secure_ == SEC_ENABLED || secure_ == SEC_REQUIRED) { @@ -71,6 +74,9 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( desc->ice_ufrag = current_description->ice_ufrag; desc->ice_pwd = current_description->ice_pwd; } + if (options.enable_ice_renomination) { + desc->AddOption(ICE_RENOMINATION_STR); + } // Negotiate security params. if (offer && offer->identity_fingerprint.get()) { diff --git a/webrtc/p2p/base/transportdescriptionfactory.h b/webrtc/p2p/base/transportdescriptionfactory.h index 828aa6d22c..2dde5b1258 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.h +++ b/webrtc/p2p/base/transportdescriptionfactory.h @@ -21,9 +21,11 @@ class SSLIdentity; namespace cricket { struct TransportOptions { - TransportOptions() : ice_restart(false), prefer_passive_role(false) {} - bool ice_restart; - bool prefer_passive_role; + bool ice_restart = false; + bool prefer_passive_role = false; + // If true, ICE renomination is supported and will be used if it is also + // supported by the remote side. + bool enable_ice_renomination = false; }; // Creates transport descriptions according to the supplied configuration. diff --git a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc index 38675ba401..14be338a0b 100644 --- a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc +++ b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc @@ -59,16 +59,7 @@ class TransportDescriptionFactoryTest : public testing::Test { // in the offer and answer is changed. // If |dtls| is true, the test verifies that the finger print is not changed. void TestIceRestart(bool dtls) { - if (dtls) { - f1_.set_secure(cricket::SEC_ENABLED); - f2_.set_secure(cricket::SEC_ENABLED); - f1_.set_certificate(cert1_); - f2_.set_certificate(cert2_); - } else { - f1_.set_secure(cricket::SEC_DISABLED); - f2_.set_secure(cricket::SEC_DISABLED); - } - + SetDtls(dtls); cricket::TransportOptions options; // The initial offer / answer exchange. std::unique_ptr offer(f1_.CreateOffer(options, NULL)); @@ -109,7 +100,50 @@ class TransportDescriptionFactoryTest : public testing::Test { } } + void TestIceRenomination(bool dtls) { + SetDtls(dtls); + + cricket::TransportOptions options; + // The initial offer / answer exchange. + std::unique_ptr offer( + f1_.CreateOffer(options, nullptr)); + std::unique_ptr answer( + f2_.CreateAnswer(offer.get(), options, nullptr)); + VerifyRenomination(offer.get(), false); + VerifyRenomination(answer.get(), false); + + options.enable_ice_renomination = true; + std::unique_ptr renomination_offer( + f1_.CreateOffer(options, offer.get())); + VerifyRenomination(renomination_offer.get(), true); + + std::unique_ptr renomination_answer( + f2_.CreateAnswer(renomination_offer.get(), options, answer.get())); + VerifyRenomination(renomination_answer.get(), true); + } + protected: + void VerifyRenomination(TransportDescription* desc, + bool renomination_expected) { + ASSERT_TRUE(desc != nullptr); + std::vector& options = desc->transport_options; + auto iter = std::find(options.begin(), options.end(), + cricket::ICE_RENOMINATION_STR); + EXPECT_EQ(renomination_expected, iter != options.end()); + } + + void SetDtls(bool dtls) { + if (dtls) { + f1_.set_secure(cricket::SEC_ENABLED); + f2_.set_secure(cricket::SEC_ENABLED); + f1_.set_certificate(cert1_); + f2_.set_certificate(cert2_); + } else { + f1_.set_secure(cricket::SEC_DISABLED); + f2_.set_secure(cricket::SEC_DISABLED); + } + } + TransportDescriptionFactory f1_; TransportDescriptionFactory f2_; @@ -256,3 +290,16 @@ TEST_F(TransportDescriptionFactoryTest, TestIceRestart) { TEST_F(TransportDescriptionFactoryTest, TestIceRestartWithDtls) { TestIceRestart(true); } + +// Test that ice renomination is set in an updated offer and answer +// if |TransportDescriptionOptions::enable_ice_renomination| is true. +TEST_F(TransportDescriptionFactoryTest, TestIceRenomination) { + TestIceRenomination(false); +} + +// Test that ice renomination is set in an updated offer and answer +// if |TransportDescriptionOptions::enable_ice_renomination| is true and DTLS +// is enabled. +TEST_F(TransportDescriptionFactoryTest, TestIceRenominationWithDtls) { + TestIceRenomination(true); +} diff --git a/webrtc/p2p/quic/quictransportchannel.h b/webrtc/p2p/quic/quictransportchannel.h index eec82be73f..22b69b7f0f 100644 --- a/webrtc/p2p/quic/quictransportchannel.h +++ b/webrtc/p2p/quic/quictransportchannel.h @@ -142,13 +142,11 @@ class QuicTransportChannel : public TransportChannelImpl, void SetIceTiebreaker(uint64_t tiebreaker) override { channel_->SetIceTiebreaker(tiebreaker); } - void SetIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - channel_->SetIceCredentials(ice_ufrag, ice_pwd); + void SetIceParameters(const IceParameters& ice_params) override { + channel_->SetIceParameters(ice_params); } - void SetRemoteIceCredentials(const std::string& ice_ufrag, - const std::string& ice_pwd) override { - channel_->SetRemoteIceCredentials(ice_ufrag, ice_pwd); + void SetRemoteIceParameters(const IceParameters& ice_params) override { + channel_->SetRemoteIceParameters(ice_params); } void SetRemoteIceMode(IceMode mode) override { channel_->SetRemoteIceMode(mode); diff --git a/webrtc/p2p/quic/quictransportchannel_unittest.cc b/webrtc/p2p/quic/quictransportchannel_unittest.cc index 49ca29cd14..c973f2da8f 100644 --- a/webrtc/p2p/quic/quictransportchannel_unittest.cc +++ b/webrtc/p2p/quic/quictransportchannel_unittest.cc @@ -134,9 +134,8 @@ class QuicTestPeer : public sigslot::has_slots<> { std::vector(), kIceUfrag, kIcePwd, cricket::ICEMODE_FULL, remote_connection_role, remote_fingerprint); - quic_channel_.SetIceCredentials(local_desc.ice_ufrag, local_desc.ice_pwd); - quic_channel_.SetRemoteIceCredentials(remote_desc.ice_ufrag, - remote_desc.ice_pwd); + quic_channel_.SetIceParameters(local_desc.GetIceParameters()); + quic_channel_.SetRemoteIceParameters(remote_desc.GetIceParameters()); } // Creates fingerprint from certificate. diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc index 65fe363f8e..bfb5692f0f 100644 --- a/webrtc/pc/mediasession.cc +++ b/webrtc/pc/mediasession.cc @@ -686,11 +686,13 @@ static bool IsRtxCodec(const C& codec) { static TransportOptions GetTransportOptions(const MediaSessionOptions& options, const std::string& content_name) { + TransportOptions transport_options; auto it = options.transport_options.find(content_name); - if (it == options.transport_options.end()) { - return TransportOptions(); + if (it != options.transport_options.end()) { + transport_options = it->second; } - return it->second; + transport_options.enable_ice_renomination = options.enable_ice_renomination; + return transport_options; } // Create a media content to be offered in a session-initiate, diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h index b39a8e5499..289bc35642 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -160,6 +160,7 @@ struct MediaSessionOptions { // bps. -1 == auto. int video_bandwidth; int data_bandwidth; + bool enable_ice_renomination = false; // content name ("mid") => options. std::map transport_options; std::string rtcp_cname; diff --git a/webrtc/pc/mediasession_unittest.cc b/webrtc/pc/mediasession_unittest.cc index 281d306c05..6125d5f90e 100644 --- a/webrtc/pc/mediasession_unittest.cc +++ b/webrtc/pc/mediasession_unittest.cc @@ -267,6 +267,16 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { return true; } + // Returns true if the transport info contains "renomination" as an + // ICE option. + bool GetIceRenomination(const TransportInfo* transport_info) { + const std::vector& ice_options = + transport_info->description.transport_options; + auto iter = std::find(ice_options.begin(), ice_options.end(), + cricket::ICE_RENOMINATION_STR); + return iter != ice_options.end(); + } + void TestTransportInfo(bool offer, const MediaSessionOptions& options, bool has_current_desc) { const std::string current_audio_ufrag = "current_audio_ufrag"; @@ -312,6 +322,7 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { EXPECT_EQ(static_cast(cricket::ICE_PWD_LENGTH), ti_audio->description.ice_pwd.size()); } + EXPECT_EQ(options.enable_ice_renomination, GetIceRenomination(ti_audio)); } else { EXPECT_TRUE(ti_audio == NULL); @@ -335,6 +346,7 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { ti_video->description.ice_pwd.size()); } } + EXPECT_EQ(options.enable_ice_renomination, GetIceRenomination(ti_video)); } else { EXPECT_TRUE(ti_video == NULL); } @@ -357,6 +369,8 @@ class MediaSessionDescriptionFactoryTest : public testing::Test { ti_data->description.ice_pwd.size()); } } + EXPECT_EQ(options.enable_ice_renomination, GetIceRenomination(ti_data)); + } else { EXPECT_TRUE(ti_video == NULL); } @@ -2140,6 +2154,13 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfoOfferAudio) { TestTransportInfo(true, options, false); } +TEST_F(MediaSessionDescriptionFactoryTest, + TestTransportInfoOfferIceRenomination) { + MediaSessionOptions options; + options.enable_ice_renomination = true; + TestTransportInfo(true, options, false); +} + TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfoOfferAudioCurrent) { MediaSessionOptions options; options.recv_audio = true; @@ -2189,7 +2210,14 @@ TEST_F(MediaSessionDescriptionFactoryTest, TestTransportInfoAnswerAudio) { } TEST_F(MediaSessionDescriptionFactoryTest, - TestTransportInfoAnswerAudioCurrent) { + TestTransportInfoAnswerIceRenomination) { + MediaSessionOptions options; + options.enable_ice_renomination = true; + TestTransportInfo(false, options, false); +} + +TEST_F(MediaSessionDescriptionFactoryTest, + TestTransportInfoAnswerAudioCurrent) { MediaSessionOptions options; options.recv_audio = true; TestTransportInfo(false, options, true);