diff --git a/api/candidate.cc b/api/candidate.cc index 85083e5d1a..930b874d66 100644 --- a/api/candidate.cc +++ b/api/candidate.cc @@ -263,7 +263,7 @@ Candidate Candidate::ToSanitizedCopy(bool use_hostname_address, } void Candidate::ComputeFoundation(const rtc::SocketAddress& base_address, - uint64_t seed) { + uint64_t tie_breaker) { // https://www.rfc-editor.org/rfc/rfc5245#section-4.1.1.3 // The foundation is an identifier, scoped within a session. Two candidates // MUST have the same foundation ID when all of the following are true: @@ -282,9 +282,14 @@ void Candidate::ComputeFoundation(const rtc::SocketAddress& base_address, rtc::StringBuilder sb; sb << type_name() << base_address.ipaddr().ToString() << protocol_ << relay_protocol_; - // The random seed is passed to prevent a reverse CRC32 lookup of the address - // based on the foundation. - sb << rtc::ToString(seed); + + // https://www.rfc-editor.org/rfc/rfc5245#section-5.2 + // [...] it is possible for both agents to mistakenly believe they are + // controlled or controlling. To resolve this, each agent MUST select a random + // number, called the tie-breaker, uniformly distributed between 0 and (2**64) + // - 1 (that is, a 64-bit positive integer). This number is used in + // connectivity checks to detect and repair this case [...] + sb << rtc::ToString(tie_breaker); foundation_ = rtc::ToString(rtc::ComputeCrc32(sb.Release())); } diff --git a/api/candidate.h b/api/candidate.h index f8f73d2827..95359620c4 100644 --- a/api/candidate.h +++ b/api/candidate.h @@ -254,9 +254,10 @@ class RTC_EXPORT Candidate { // then the foundation will be different. Two candidate pairs with // the same foundation pairs are likely to have similar network // characteristics. Foundations are used in the frozen algorithm. - // A session wide (peerconnection) seed is applied to the foundation, + // A session wide (peerconnection) tie-breaker is applied to the foundation, // adds additional randomness and must be the same for all candidates. - void ComputeFoundation(const rtc::SocketAddress& base_address, uint64_t seed); + void ComputeFoundation(const rtc::SocketAddress& base_address, + uint64_t tie_breaker); // https://www.rfc-editor.org/rfc/rfc5245#section-7.2.1.3 // Call to populate the foundation field for a new peer reflexive remote diff --git a/p2p/base/fake_port_allocator.h b/p2p/base/fake_port_allocator.h index 4ce3058e10..a51a7ca4ee 100644 --- a/p2p/base/fake_port_allocator.h +++ b/p2p/base/fake_port_allocator.h @@ -128,7 +128,6 @@ class FakePortAllocatorSession : public PortAllocatorSession { field_trials_)); RTC_DCHECK(port_); port_->SetIceTiebreaker(allocator_->ice_tiebreaker()); - port_->SetFoundationSeed(allocator_->foundation_seed()); port_->SubscribePortDestroyed( [this](PortInterface* port) { OnPortDestroyed(port); }); AddPort(port_.get()); diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 81e337442c..917167c864 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -199,14 +199,6 @@ uint64_t Port::IceTiebreaker() const { return tiebreaker_; } -void Port::SetFoundationSeed(uint64_t foundation_seed) { - foundation_seed_ = foundation_seed; -} - -uint64_t Port::FoundationSeed() const { - return foundation_seed_; -} - bool Port::SharedSocket() const { return shared_socket_; } @@ -263,8 +255,8 @@ void Port::AddAddress(const rtc::SocketAddress& address, type, generation_, "", network_->id(), network_cost_); // Set the relay protocol before computing the foundation field. c.set_relay_protocol(relay_protocol); - // TODO(bugs.webrtc.org/14605): ensure FoundationSeed() is set. - c.ComputeFoundation(base_address, foundation_seed_); + // TODO(bugs.webrtc.org/14605): ensure IceTiebreaker() is set. + c.ComputeFoundation(base_address, tiebreaker_); c.set_priority( c.GetPriority(type_preference, network_->preference(), relay_preference, diff --git a/p2p/base/port.h b/p2p/base/port.h index 1758aa98f3..4021c250fb 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -211,9 +211,6 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { void SetIceTiebreaker(uint64_t tiebreaker) override; uint64_t IceTiebreaker() const override; - void SetFoundationSeed(uint64_t foundation_seed) override; - uint64_t FoundationSeed() const override; - bool SharedSocket() const override; void ResetSharedSocket() { shared_socket_ = false; } @@ -497,7 +494,6 @@ class RTC_EXPORT Port : public PortInterface, public sigslot::has_slots<> { bool enable_port_packets_; IceRole ice_role_; uint64_t tiebreaker_; - uint64_t foundation_seed_; bool shared_socket_; // Information to use when going through a proxy. std::string user_agent_; diff --git a/p2p/base/port_allocator.cc b/p2p/base/port_allocator.cc index baf7cb535f..52fc8c1d39 100644 --- a/p2p/base/port_allocator.cc +++ b/p2p/base/port_allocator.cc @@ -100,8 +100,7 @@ PortAllocator::PortAllocator() step_delay_(kDefaultStepDelay), allow_tcp_listen_(true), candidate_filter_(CF_ALL), - tiebreaker_(rtc::CreateRandomId64()), - foundation_seed_(rtc::CreateRandomId64()) { + tiebreaker_(rtc::CreateRandomId64()) { // The allocator will be attached to a thread in Initialize. thread_checker_.Detach(); } diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h index 1aee4c1ccf..63ecfc6904 100644 --- a/p2p/base/port_allocator.h +++ b/p2p/base/port_allocator.h @@ -452,7 +452,6 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> { Candidate SanitizeCandidate(const Candidate& c) const; uint64_t ice_tiebreaker() const { return tiebreaker_; } - uint64_t foundation_seed() const { return foundation_seed_; } uint32_t flags() const { CheckRunOnValidThreadIfInitialized(); @@ -665,14 +664,8 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> { std::vector>::const_iterator FindPooledSession(const IceParameters* ice_credentials = nullptr) const; - // ICE Tie-breaker as described in - // https://www.rfc-editor.org/rfc/rfc5245#section-5.2 + // ICE tie breaker. uint64_t tiebreaker_; - // A random value that is included in the foundation of all ICE candidates - // generated by ports derived from this allocator. See - // https://www.rfc-editor.org/rfc/rfc5245#section-4.1.1.3 - // Never sent over the network. - uint64_t foundation_seed_; }; } // namespace cricket diff --git a/p2p/base/port_interface.h b/p2p/base/port_interface.h index e9f61ab328..8a1d18d8ba 100644 --- a/p2p/base/port_interface.h +++ b/p2p/base/port_interface.h @@ -62,11 +62,6 @@ class PortInterface { virtual void SetIceTiebreaker(uint64_t tiebreaker) = 0; virtual uint64_t IceTiebreaker() const = 0; - // Methods to set/get the PortAllocator random value which is used to seed the - // foundation. - virtual void SetFoundationSeed(uint64_t foundation_seed) = 0; - virtual uint64_t FoundationSeed() const = 0; - virtual bool SharedSocket() const = 0; virtual bool SupportsProtocol(absl::string_view protocol) const = 0; diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc index eeb940a630..ab3ff86fd1 100644 --- a/p2p/base/port_unittest.cc +++ b/p2p/base/port_unittest.cc @@ -109,7 +109,6 @@ const uint32_t kDefaultPrflxPriority = ICE_TYPE_PREFERENCE_PRFLX << 24 | constexpr int kTiebreaker1 = 11111; constexpr int kTiebreaker2 = 22222; constexpr int kTiebreakerDefault = 44444; -constexpr int kFoundationSeed = 42; const char* data = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890"; @@ -556,7 +555,6 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { username_, password_, true, absl::nullopt, &field_trials_); port->SetIceTiebreaker(kTiebreakerDefault); - port->SetFoundationSeed(kFoundationSeed); return port; } @@ -569,7 +567,6 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { MakeNetworkMultipleAddrs(global_addr, link_local_addr), 0, 0, username_, password_, true, absl::nullopt, &field_trials_); port->SetIceTiebreaker(kTiebreakerDefault); - port->SetFoundationSeed(kFoundationSeed); return port; } std::unique_ptr CreateTcpPort(const SocketAddress& addr) { @@ -580,7 +577,6 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { auto port = TCPPort::Create(&main_, socket_factory, MakeNetwork(addr), 0, 0, username_, password_, true, &field_trials_); port->SetIceTiebreaker(kTiebreakerDefault); - port->SetFoundationSeed(kFoundationSeed); return port; } std::unique_ptr CreateStunPort(const SocketAddress& addr, @@ -591,7 +587,6 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> { username_, password_, stun_servers, absl::nullopt, &field_trials_); port->SetIceTiebreaker(kTiebreakerDefault); - port->SetFoundationSeed(kFoundationSeed); return port; } std::unique_ptr CreateRelayPort(const SocketAddress& addr, diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc index 2a37a5bb0a..cc38a66727 100644 --- a/p2p/client/basic_port_allocator.cc +++ b/p2p/client/basic_port_allocator.cc @@ -1484,7 +1484,6 @@ void AllocationSequence::CreateUDPPorts() { if (port) { port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); - port->SetFoundationSeed(session_->allocator()->foundation_seed()); // If shared socket is enabled, STUN candidate will be allocated by the // UDPPort. if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) { @@ -1521,7 +1520,6 @@ void AllocationSequence::CreateTCPPorts() { session_->allocator()->field_trials()); if (port) { port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); - port->SetFoundationSeed(session_->allocator()->foundation_seed()); session_->AddAllocatedPort(port.release(), this); // Since TCPPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1552,7 +1550,6 @@ void AllocationSequence::CreateStunPorts() { session_->allocator()->field_trials()); if (port) { port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); - port->SetFoundationSeed(session_->allocator()->foundation_seed()); session_->AddAllocatedPort(port.release(), this); // Since StunPort is not created using shared socket, `port` will not be // added to the dequeue. @@ -1656,7 +1653,6 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config, } RTC_DCHECK(port != NULL); port->SetIceTiebreaker(session_->allocator()->ice_tiebreaker()); - port->SetFoundationSeed(session_->allocator()->foundation_seed()); session_->AddAllocatedPort(port.release(), this); } }