ice: include tiebreaker in computation of foundation attribute
the foundation attribute is currently calculated as
CRC32(baseaddress, protocol, relayprotocol)
which is a way to satisfy the requirements from
https://www.rfc-editor.org/rfc/rfc5245#section-4.1.1.3
However, this leaks the base address which defeats the
MDNS obfuscation described in
https://datatracker.ietf.org/doc/draft-ietf-mmusic-mdns-ice-candidates/
since the CRC32 can be reversed using a table lookup as shown in
https://github.com/niespodd/webrtc-local-ip-leak/
To defeat that lookup, "seed" the CRC32 with the ICE tie-breaker which is a randomly picked unsigned 64 bit integer described in
https://www.rfc-editor.org/rfc/rfc5245#section-5.2
The tie-breaker is not known to Javascript and adding it scopes the foundation within the peer connection as described in section 4.1.1.3
To manually test (preferably with a DCHECK for IceTiebreaker() in ComputeFoundation)
- gather candidates twice on https://webrtc.github.io/samples/src/content/peerconnection/trickle-ice/ and observe that the foundations are not the same after this change
- create two RTCPeerConnections with {iceCandidatePoolSize: 1}, create a datachannel, call setLocalDescription, inspect the candidates and observe that the foundations are not the same after this change.
Unit test changes have been split into a separate CL for easier integration.
BUG=webrtc:14605
Change-Id: I6bbad1635b48997b00ae74d251ae357bf8afd12f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/280621
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38485}
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index 28761b5..76652db 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -1583,7 +1583,7 @@
// Set the related address and foundation attributes before changing the
// address.
local_candidate_.set_related_address(local_candidate_.address());
- local_candidate_.set_foundation(Port::ComputeFoundation(
+ local_candidate_.set_foundation(port()->ComputeFoundation(
PRFLX_PORT_TYPE, local_candidate_.protocol(),
local_candidate_.relay_protocol(), local_candidate_.address()));
local_candidate_.set_priority(priority);
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 02d1f32..15b1cf6 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -894,6 +894,7 @@
void P2PTransportChannel::MaybeStartGathering() {
RTC_DCHECK_RUN_ON(network_thread_);
+ // TODO(bugs.webrtc.org/14605): ensure tie_breaker_ is set.
if (ice_parameters_.ufrag.empty() || ice_parameters_.pwd.empty()) {
RTC_LOG(LS_ERROR)
<< "Cannot gather candidates because ICE parameters are empty"
@@ -938,6 +939,7 @@
ice_parameters_.ufrag,
ice_parameters_.pwd);
if (pooled_session) {
+ pooled_session->set_ice_tiebreaker(tiebreaker_);
AddAllocatorSession(std::move(pooled_session));
PortAllocatorSession* raw_pooled_session =
allocator_sessions_.back().get();
@@ -954,6 +956,7 @@
AddAllocatorSession(allocator_->CreateSession(
transport_name(), component(), ice_parameters_.ufrag,
ice_parameters_.pwd));
+ allocator_sessions_.back()->set_ice_tiebreaker(tiebreaker_);
allocator_sessions_.back()->StartGettingPorts();
}
}
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 6792939..dea18a4 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -101,8 +101,10 @@
absl::string_view protocol,
absl::string_view relay_protocol,
const rtc::SocketAddress& base_address) {
+ // TODO(bugs.webrtc.org/14605): ensure IceTiebreaker() is set.
rtc::StringBuilder sb;
- sb << type << base_address.ipaddr().ToString() << protocol << relay_protocol;
+ sb << type << base_address.ipaddr().ToString() << protocol << relay_protocol
+ << rtc::ToString(IceTiebreaker());
return rtc::ToString(rtc::ComputeCrc32(sb.Release()));
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index a1377a8..d746f3d 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -386,10 +386,10 @@
// 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.
- static std::string ComputeFoundation(absl::string_view type,
- absl::string_view protocol,
- absl::string_view relay_protocol,
- const rtc::SocketAddress& base_address);
+ std::string ComputeFoundation(absl::string_view type,
+ absl::string_view protocol,
+ absl::string_view relay_protocol,
+ const rtc::SocketAddress& base_address);
protected:
virtual void UpdateNetworkCost();
diff --git a/p2p/base/port_allocator.cc b/p2p/base/port_allocator.cc
index d22b428..522f0be 100644
--- a/p2p/base/port_allocator.cc
+++ b/p2p/base/port_allocator.cc
@@ -68,7 +68,8 @@
content_name_(content_name),
component_(component),
ice_ufrag_(ice_ufrag),
- ice_pwd_(ice_pwd) {
+ ice_pwd_(ice_pwd),
+ tiebreaker_(0) {
// Pooled sessions are allowed to be created with empty content name,
// component, ufrag and password.
RTC_DCHECK(ice_ufrag.empty() == ice_pwd.empty());
@@ -99,7 +100,8 @@
max_ipv6_networks_(kDefaultMaxIPv6Networks),
step_delay_(kDefaultStepDelay),
allow_tcp_listen_(true),
- candidate_filter_(CF_ALL) {
+ candidate_filter_(CF_ALL),
+ tiebreaker_(0) {
// The allocator will be attached to a thread in Initialize.
thread_checker_.Detach();
}
@@ -199,6 +201,7 @@
PortAllocatorSession* pooled_session =
CreateSessionInternal("", 0, iceCredentials.ufrag, iceCredentials.pwd);
pooled_session->set_pooled(true);
+ pooled_session->set_ice_tiebreaker(tiebreaker_);
pooled_session->StartGettingPorts();
pooled_sessions_.push_back(
std::unique_ptr<PortAllocatorSession>(pooled_session));
@@ -206,6 +209,13 @@
return true;
}
+void PortAllocator::SetIceTiebreaker(uint64_t tiebreaker) {
+ tiebreaker_ = tiebreaker;
+ for (auto& pooled_session : pooled_sessions_) {
+ pooled_session->set_ice_tiebreaker(tiebreaker_);
+ }
+}
+
std::unique_ptr<PortAllocatorSession> PortAllocator::CreateSession(
absl::string_view content_name,
int component,
@@ -215,6 +225,7 @@
auto session = std::unique_ptr<PortAllocatorSession>(
CreateSessionInternal(content_name, component, ice_ufrag, ice_pwd));
session->SetCandidateFilter(candidate_filter());
+ session->set_ice_tiebreaker(tiebreaker_);
return session;
}
diff --git a/p2p/base/port_allocator.h b/p2p/base/port_allocator.h
index 420560c..4635843 100644
--- a/p2p/base/port_allocator.h
+++ b/p2p/base/port_allocator.h
@@ -206,6 +206,10 @@
const std::string& ice_pwd() const { return ice_pwd_; }
bool pooled() const { return pooled_; }
+ // TODO(bugs.webrtc.org/14605): move this to the constructor
+ void set_ice_tiebreaker(uint64_t tiebreaker) { tiebreaker_ = tiebreaker; }
+ uint64_t ice_tiebreaker() const { return tiebreaker_; }
+
// Setting this filter should affect not only candidates gathered in the
// future, but candidates already gathered and ports already "ready",
// which would be returned by ReadyCandidates() and ReadyPorts().
@@ -322,6 +326,9 @@
bool pooled_ = false;
+ // TODO(bugs.webrtc.org/14605): move this to the constructor
+ uint64_t tiebreaker_;
+
// SetIceParameters is an implementation detail which only PortAllocator
// should be able to call.
friend class PortAllocator;
@@ -374,6 +381,9 @@
const absl::optional<int>&
stun_candidate_keepalive_interval = absl::nullopt);
+ void SetIceTiebreaker(uint64_t tiebreaker);
+ uint64_t IceTiebreaker() const { return tiebreaker_; }
+
const ServerAddresses& stun_servers() const {
CheckRunOnValidThreadIfInitialized();
return stun_servers_;
@@ -665,6 +675,9 @@
// if ice_credentials is nullptr.
std::vector<std::unique_ptr<PortAllocatorSession>>::const_iterator
FindPooledSession(const IceParameters* ice_credentials = nullptr) const;
+
+ // ICE tie breaker.
+ uint64_t tiebreaker_;
};
} // namespace cricket
diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
index 819bc51..5611403 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -1557,6 +1557,7 @@
}
if (port) {
+ port->SetIceTiebreaker(session_->ice_tiebreaker());
// If shared socket is enabled, STUN candidate will be allocated by the
// UDPPort.
if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET)) {
@@ -1592,6 +1593,7 @@
session_->allocator()->allow_tcp_listen(),
session_->allocator()->field_trials());
if (port) {
+ port->SetIceTiebreaker(session_->ice_tiebreaker());
session_->AddAllocatedPort(port.release(), this);
// Since TCPPort is not created using shared socket, `port` will not be
// added to the dequeue.
@@ -1621,6 +1623,7 @@
session_->allocator()->stun_candidate_keepalive_interval(),
session_->allocator()->field_trials());
if (port) {
+ port->SetIceTiebreaker(session_->ice_tiebreaker());
session_->AddAllocatedPort(port.release(), this);
// Since StunPort is not created using shared socket, `port` will not be
// added to the dequeue.
@@ -1717,6 +1720,7 @@
}
}
RTC_DCHECK(port != NULL);
+ port->SetIceTiebreaker(session_->ice_tiebreaker());
session_->AddAllocatedPort(port.release(), this);
}
}
diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 95ab799..c3fc89a 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc
@@ -62,6 +62,9 @@
RTC_DCHECK(config_.ice_transport_factory);
RTC_DCHECK(config_.on_dtls_handshake_error_);
RTC_DCHECK(config_.field_trials);
+ if (port_allocator_) {
+ port_allocator_->SetIceTiebreaker(ice_tiebreaker_);
+ }
}
JsepTransportController::~JsepTransportController() {
