diff --git a/p2p/base/dtls_transport_unittest.cc b/p2p/base/dtls_transport_unittest.cc index f870c1e207..93653c116f 100644 --- a/p2p/base/dtls_transport_unittest.cc +++ b/p2p/base/dtls_transport_unittest.cc @@ -92,6 +92,8 @@ class DtlsTestClient : public sigslot::has_slots<> { fake_ice_transport_->SetAsync(true); fake_ice_transport_->SetAsyncDelay(async_delay_ms); fake_ice_transport_->SetIceRole(role); + fake_ice_transport_->SetIceTiebreaker((role == ICEROLE_CONTROLLING) ? 1 + : 2); // Hook the raw packets so that we can verify they are encrypted. fake_ice_transport_->RegisterReceivedPacketCallback( this, [&](rtc::PacketTransportInternal* transport, diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h index 788299bb38..285bfff59c 100644 --- a/p2p/base/fake_ice_transport.h +++ b/p2p/base/fake_ice_transport.h @@ -147,6 +147,10 @@ class FakeIceTransport : public IceTransportInternal { // Fake IceTransportInternal implementation. const std::string& transport_name() const override { return name_; } int component() const override { return component_; } + uint64_t IceTiebreaker() const { + RTC_DCHECK_RUN_ON(network_thread_); + return tiebreaker_; + } IceMode remote_ice_mode() const { RTC_DCHECK_RUN_ON(network_thread_); return remote_ice_mode_; @@ -208,6 +212,10 @@ class FakeIceTransport : public IceTransportInternal { RTC_DCHECK_RUN_ON(network_thread_); return role_; } + void SetIceTiebreaker(uint64_t tiebreaker) override { + RTC_DCHECK_RUN_ON(network_thread_); + tiebreaker_ = tiebreaker; + } void SetIceParameters(const IceParameters& ice_params) override { RTC_DCHECK_RUN_ON(network_thread_); ice_parameters_ = ice_params; @@ -398,6 +406,7 @@ class FakeIceTransport : public IceTransportInternal { Candidates remote_candidates_ RTC_GUARDED_BY(network_thread_); IceConfig ice_config_ RTC_GUARDED_BY(network_thread_); IceRole role_ RTC_GUARDED_BY(network_thread_) = ICEROLE_UNKNOWN; + uint64_t tiebreaker_ RTC_GUARDED_BY(network_thread_) = 0; IceParameters ice_parameters_ RTC_GUARDED_BY(network_thread_); IceParameters remote_ice_parameters_ RTC_GUARDED_BY(network_thread_); IceMode remote_ice_mode_ RTC_GUARDED_BY(network_thread_) = ICEMODE_FULL; diff --git a/p2p/base/ice_transport_internal.h b/p2p/base/ice_transport_internal.h index 48f12f9f50..46717cc230 100644 --- a/p2p/base/ice_transport_internal.h +++ b/p2p/base/ice_transport_internal.h @@ -254,6 +254,8 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal { virtual void SetIceRole(IceRole role) = 0; + virtual void SetIceTiebreaker(uint64_t tiebreaker) = 0; + virtual void SetIceCredentials(absl::string_view ice_ufrag, absl::string_view ice_pwd); diff --git a/p2p/base/mock_ice_transport.h b/p2p/base/mock_ice_transport.h index b7b986e022..ef6bdce3c0 100644 --- a/p2p/base/mock_ice_transport.h +++ b/p2p/base/mock_ice_transport.h @@ -57,6 +57,7 @@ class MockIceTransport : public IceTransportInternal { const std::string& transport_name() const override { return transport_name_; } int component() const override { return 0; } void SetIceRole(IceRole role) override {} + void SetIceTiebreaker(uint64_t tiebreaker) override {} // The ufrag and pwd in `ice_params` must be set // before candidate gathering can start. void SetIceParameters(const IceParameters& ice_params) override {} diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc index dcea5b0e47..241b423be8 100644 --- a/p2p/base/p2p_transport_channel.cc +++ b/p2p/base/p2p_transport_channel.cc @@ -162,6 +162,7 @@ P2PTransportChannel::P2PTransportChannel( error_(0), remote_ice_mode_(ICEMODE_FULL), ice_role_(ICEROLE_UNKNOWN), + ice_tiebreaker_(0), gathering_state_(kIceGatheringNew), weak_ping_interval_(GetWeakPingIntervalInFieldTrial(field_trials)), config_(RECEIVING_TIMEOUT, @@ -315,6 +316,17 @@ IceRole P2PTransportChannel::GetIceRole() const { return ice_role_; } +void P2PTransportChannel::SetIceTiebreaker(uint64_t tiebreaker) { + RTC_DCHECK_RUN_ON(network_thread_); + if (!ports_.empty() || !pruned_ports_.empty()) { + RTC_LOG(LS_ERROR) + << "Attempt to change tiebreaker after Port has been allocated."; + return; + } + + ice_tiebreaker_ = tiebreaker; +} + IceTransportState P2PTransportChannel::GetState() const { RTC_DCHECK_RUN_ON(network_thread_); return state_; @@ -914,7 +926,7 @@ void P2PTransportChannel::OnPortReady(PortAllocatorSession* session, // if one is pending. port->SetIceRole(ice_role_); - port->SetIceTiebreaker(allocator_->ice_tiebreaker()); + port->SetIceTiebreaker(ice_tiebreaker_); ports_.push_back(port); port->SignalUnknownAddress.connect(this, &P2PTransportChannel::OnUnknownAddress); diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h index 2750af1332..f7472df38a 100644 --- a/p2p/base/p2p_transport_channel.h +++ b/p2p/base/p2p_transport_channel.h @@ -128,6 +128,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, bool receiving() const override; void SetIceRole(IceRole role) override; IceRole GetIceRole() const override; + void SetIceTiebreaker(uint64_t tiebreaker) override; void SetIceParameters(const IceParameters& ice_params) override; void SetRemoteIceParameters(const IceParameters& ice_params) override; void SetRemoteIceMode(IceMode mode) override; @@ -438,6 +439,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal, RTC_GUARDED_BY(network_thread_); IceMode remote_ice_mode_ RTC_GUARDED_BY(network_thread_); IceRole ice_role_ RTC_GUARDED_BY(network_thread_); + uint64_t ice_tiebreaker_ RTC_GUARDED_BY(network_thread_); IceGatheringState gathering_state_ RTC_GUARDED_BY(network_thread_); std::unique_ptr regathering_controller_ RTC_GUARDED_BY(network_thread_); diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 29bdd990f3..79ca2a5f26 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -134,6 +134,9 @@ const cricket::IceParameters kIceParams[4] = { {kIceUfrag[2], kIcePwd[2], false}, {kIceUfrag[3], kIcePwd[3], false}}; +const uint64_t kLowTiebreaker = 11111; +const uint64_t kHighTiebreaker = 22222; + cricket::IceConfig CreateIceConfig( int receiving_timeout, cricket::ContinualGatheringPolicy continual_gathering_policy, @@ -374,6 +377,8 @@ class P2PTransportChannelTestBase : public ::testing::Test, void SetIceRole(IceRole role) { role_ = role; } IceRole ice_role() { return role_; } + void SetIceTiebreaker(uint64_t tiebreaker) { tiebreaker_ = tiebreaker; } + uint64_t GetIceTiebreaker() { return tiebreaker_; } void OnRoleConflict(bool role_conflict) { role_conflict_ = role_conflict; } bool role_conflict() { return role_conflict_; } void SetAllocationStepDelay(uint32_t delay) { @@ -482,6 +487,7 @@ class P2PTransportChannelTestBase : public ::testing::Test, channel->SetRemoteIceParameters(remote_ice); } channel->SetIceRole(GetEndpoint(endpoint)->ice_role()); + channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker()); return channel; } @@ -556,6 +562,9 @@ class P2PTransportChannelTestBase : public ::testing::Test, void SetIceRole(int endpoint, IceRole role) { GetEndpoint(endpoint)->SetIceRole(role); } + void SetIceTiebreaker(int endpoint, uint64_t tiebreaker) { + GetEndpoint(endpoint)->SetIceTiebreaker(tiebreaker); + } bool GetRoleConflict(int endpoint) { return GetEndpoint(endpoint)->role_conflict(); } @@ -769,6 +778,31 @@ class P2PTransportChannelTestBase : public ::testing::Test, EXPECT_EQ(1u, RemoteCandidate(ep1_ch1())->generation()); } + void TestSignalRoleConflict() { + rtc::ScopedFakeClock clock; + // Default EP1 is in controlling state. + SetIceTiebreaker(0, kLowTiebreaker); + + SetIceRole(1, ICEROLE_CONTROLLING); + SetIceTiebreaker(1, kHighTiebreaker); + + // Creating channels with both channels role set to CONTROLLING. + CreateChannels(); + // Since both the channels initiated with controlling state and channel2 + // has higher tiebreaker value, channel1 should receive SignalRoleConflict. + EXPECT_TRUE_SIMULATED_WAIT(GetRoleConflict(0), kShortTimeout, clock); + EXPECT_FALSE(GetRoleConflict(1)); + + EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()), + kShortTimeout, clock); + + EXPECT_TRUE(ep1_ch1()->selected_connection() && + ep2_ch1()->selected_connection()); + + TestSendRecv(&clock); + DestroyChannels(); + } + void TestPacketInfoIsSet(rtc::PacketInfo info) { EXPECT_NE(info.packet_type, rtc::PacketType::kUnknown); EXPECT_NE(info.protocol, rtc::PacketInfoProtocolType::kUnknown); @@ -1805,36 +1839,13 @@ TEST_F(P2PTransportChannelTest, TestTcpConnectionTcptypeSet) { } TEST_F(P2PTransportChannelTest, TestIceRoleConflict) { - rtc::ScopedFakeClock clock; AddAddress(0, kPublicAddrs[0]); AddAddress(1, kPublicAddrs[1]); - - // Creating channels with both channels role set to CONTROLLING. - SetIceRole(0, ICEROLE_CONTROLLING); - SetIceRole(1, ICEROLE_CONTROLLING); - - CreateChannels(); - bool first_endpoint_has_lower_tiebreaker = - GetEndpoint(0)->allocator_->ice_tiebreaker() < - GetEndpoint(1)->allocator_->ice_tiebreaker(); - // Since both the channels initiated with controlling state, the channel with - // the lower tiebreaker should receive SignalRoleConflict. - EXPECT_TRUE_SIMULATED_WAIT( - GetRoleConflict(first_endpoint_has_lower_tiebreaker ? 0 : 1), - kShortTimeout, clock); - EXPECT_FALSE(GetRoleConflict(first_endpoint_has_lower_tiebreaker ? 1 : 0)); - - EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()), - kShortTimeout, clock); - - EXPECT_TRUE(ep1_ch1()->selected_connection() && - ep2_ch1()->selected_connection()); - - TestSendRecv(&clock); - DestroyChannels(); + TestSignalRoleConflict(); } -// Tests that the ice configs (protocol and role) can be passed down to ports. +// Tests that the ice configs (protocol, tiebreaker and role) can be passed +// down to ports. TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { rtc::ScopedFakeClock clock; AddAddress(0, kPublicAddrs[0]); @@ -1843,7 +1854,9 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { // Give the first connection the higher tiebreaker so its role won't // change unless we tell it to. SetIceRole(0, ICEROLE_CONTROLLING); + SetIceTiebreaker(0, kHighTiebreaker); SetIceRole(1, ICEROLE_CONTROLLING); + SetIceTiebreaker(1, kLowTiebreaker); CreateChannels(); @@ -1852,13 +1865,18 @@ TEST_F(P2PTransportChannelTest, TestIceConfigWillPassDownToPort) { const std::vector ports_before = ep1_ch1()->ports(); for (size_t i = 0; i < ports_before.size(); ++i) { EXPECT_EQ(ICEROLE_CONTROLLING, ports_before[i]->GetIceRole()); + EXPECT_EQ(kHighTiebreaker, ports_before[i]->IceTiebreaker()); } ep1_ch1()->SetIceRole(ICEROLE_CONTROLLED); + ep1_ch1()->SetIceTiebreaker(kLowTiebreaker); const std::vector ports_after = ep1_ch1()->ports(); for (size_t i = 0; i < ports_after.size(); ++i) { EXPECT_EQ(ICEROLE_CONTROLLED, ports_before[i]->GetIceRole()); + // SetIceTiebreaker after ports have been created will fail. So expect the + // original value. + EXPECT_EQ(kHighTiebreaker, ports_before[i]->IceTiebreaker()); } EXPECT_TRUE_SIMULATED_WAIT(CheckConnected(ep1_ch1(), ep2_ch1()), @@ -2244,7 +2262,9 @@ TEST_F(P2PTransportChannelTest, // With conflicting ICE roles, endpoint 1 has the higher tie breaker and will // send a binding error response. SetIceRole(0, ICEROLE_CONTROLLING); + SetIceTiebreaker(0, kHighTiebreaker); SetIceRole(1, ICEROLE_CONTROLLING); + SetIceTiebreaker(1, kLowTiebreaker); // We want the remote TURN candidate to show up as prflx. To do this we need // to configure the server to accept packets from an address we haven't // explicitly installed permission for. diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 505ed5059e..7152826725 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -58,6 +58,7 @@ JsepTransportController::JsepTransportController( }), config_(std::move(config)), active_reset_srtp_params_(config.active_reset_srtp_params), + ice_tiebreaker_(port_allocator ? port_allocator->ice_tiebreaker() : 0), bundles_(config.bundle_policy) { // The `transport_observer` is assumed to be non-null. RTC_DCHECK(config_.transport_observer); @@ -403,6 +404,7 @@ JsepTransportController::CreateIceTransport(const std::string& transport_name, transport_name, component, std::move(init)); RTC_DCHECK(transport); transport->internal()->SetIceRole(ice_role_); + transport->internal()->SetIceTiebreaker(ice_tiebreaker_); transport->internal()->SetIceConfig(ice_config_); return transport; } diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index da0b921473..cf2b411f42 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -507,6 +507,7 @@ class JsepTransportController : public sigslot::has_slots<> { cricket::IceConfig ice_config_; cricket::IceRole ice_role_ = cricket::ICEROLE_CONTROLLING; + uint64_t ice_tiebreaker_; rtc::scoped_refptr certificate_; BundleManager bundles_;