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}
This commit is contained in:
committed by
WebRTC LUCI CQ
parent
fbe5d7c3d4
commit
08b882d762
@ -1583,7 +1583,7 @@ void Connection::MaybeUpdateLocalCandidate(StunRequest* request,
|
||||
// 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);
|
||||
|
||||
@ -894,6 +894,7 @@ int P2PTransportChannel::check_receiving_interval() const {
|
||||
|
||||
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 @@ void P2PTransportChannel::MaybeStartGathering() {
|
||||
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 @@ void P2PTransportChannel::MaybeStartGathering() {
|
||||
AddAllocatorSession(allocator_->CreateSession(
|
||||
transport_name(), component(), ice_parameters_.ufrag,
|
||||
ice_parameters_.pwd));
|
||||
allocator_sessions_.back()->set_ice_tiebreaker(tiebreaker_);
|
||||
allocator_sessions_.back()->StartGettingPorts();
|
||||
}
|
||||
}
|
||||
|
||||
@ -101,8 +101,10 @@ std::string Port::ComputeFoundation(absl::string_view type,
|
||||
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()));
|
||||
}
|
||||
|
||||
|
||||
@ -386,10 +386,10 @@ class Port : public PortInterface, public sigslot::has_slots<> {
|
||||
// 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();
|
||||
|
||||
@ -68,7 +68,8 @@ PortAllocatorSession::PortAllocatorSession(absl::string_view content_name,
|
||||
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 @@ PortAllocator::PortAllocator()
|
||||
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 @@ bool PortAllocator::SetConfiguration(
|
||||
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 @@ bool PortAllocator::SetConfiguration(
|
||||
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 @@ std::unique_ptr<PortAllocatorSession> PortAllocator::CreateSession(
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@ -206,6 +206,10 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> {
|
||||
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 @@ class RTC_EXPORT PortAllocatorSession : public sigslot::has_slots<> {
|
||||
|
||||
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 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> {
|
||||
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 @@ class RTC_EXPORT PortAllocator : public sigslot::has_slots<> {
|
||||
// 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
|
||||
|
||||
@ -1557,6 +1557,7 @@ void AllocationSequence::CreateUDPPorts() {
|
||||
}
|
||||
|
||||
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 @@ void AllocationSequence::CreateTCPPorts() {
|
||||
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 @@ void AllocationSequence::CreateStunPorts() {
|
||||
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 @@ void AllocationSequence::CreateTurnPort(const RelayServerConfig& config) {
|
||||
}
|
||||
}
|
||||
RTC_DCHECK(port != NULL);
|
||||
port->SetIceTiebreaker(session_->ice_tiebreaker());
|
||||
session_->AddAllocatedPort(port.release(), this);
|
||||
}
|
||||
}
|
||||
|
||||
@ -62,6 +62,9 @@ JsepTransportController::JsepTransportController(
|
||||
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() {
|
||||
|
||||
Reference in New Issue
Block a user