Stop pairing local relay candidates with remote mDNS candidates.
To avoid IP leak from the CreatePermission request, local relay
candidates must not be paired with remote mDNS candidates, per Section
3.3.2 in draft-ietf-rtcweb-mdns-ice-candidates-04.
Bug: webrtc:11038
Change-Id: I13aada79c812712b850293c7e17094dc8f77105a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/157340
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Honghai Zhang <honghaiz@webrtc.org>
Reviewed-by: Alex Drake <alexdrake@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29546}
diff --git a/p2p/base/p2p_constants.cc b/p2p/base/p2p_constants.cc
index 619b739..3414939 100644
--- a/p2p/base/p2p_constants.cc
+++ b/p2p/base/p2p_constants.cc
@@ -41,6 +41,8 @@
const char CONNECTIONROLE_ACTPASS_STR[] = "actpass";
const char CONNECTIONROLE_HOLDCONN_STR[] = "holdconn";
+const char LOCAL_TLD[] = ".local";
+
const int MIN_CHECK_RECEIVING_INTERVAL = 50;
const int RECEIVING_TIMEOUT = MIN_CHECK_RECEIVING_INTERVAL * 50;
const int RECEIVING_SWITCHING_DELAY = 1000;
diff --git a/p2p/base/p2p_constants.h b/p2p/base/p2p_constants.h
index a2be32d..07257d5 100644
--- a/p2p/base/p2p_constants.h
+++ b/p2p/base/p2p_constants.h
@@ -48,6 +48,9 @@
extern const char CONNECTIONROLE_ACTPASS_STR[];
extern const char CONNECTIONROLE_HOLDCONN_STR[];
+// RFC 6762, the .local pseudo-top-level domain used for mDNS names.
+extern const char LOCAL_TLD[];
+
// Constants for time intervals are in milliseconds unless otherwise stated.
//
// Most of the following constants are the default values of IceConfig
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index a1c1450..f95e7ab 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -16,6 +16,7 @@
#include <utility>
#include "absl/algorithm/container.h"
+#include "absl/strings/match.h"
#include "api/candidate.h"
#include "logging/rtc_event_log/ice_logger.h"
#include "p2p/base/candidate_pair_interface.h"
@@ -2700,10 +2701,9 @@
Candidate P2PTransportChannel::SanitizeRemoteCandidate(
const Candidate& c) const {
RTC_DCHECK_RUN_ON(network_thread_);
- // If the remote endpoint signaled us a hostname host candidate, we assume it
+ // If the remote endpoint signaled us an mDNS candidate, we assume it
// is supposed to be sanitized.
- bool use_hostname_address =
- c.type() == LOCAL_PORT_TYPE && !c.address().hostname().empty();
+ bool use_hostname_address = absl::EndsWith(c.address().hostname(), LOCAL_TLD);
// Remove the address for prflx remote candidates. See
// https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatestats.
use_hostname_address |= c.type() == PRFLX_PORT_TYPE;
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 8f7fd4a..0c3474b 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -5247,6 +5247,59 @@
DestroyChannels();
}
+TEST_F(P2PTransportChannelTest,
+ NoPairOfLocalRelayCandidateWithRemoteMdnsCandidate) {
+ const int kOnlyRelayPorts = cricket::PORTALLOCATOR_DISABLE_UDP |
+ cricket::PORTALLOCATOR_DISABLE_STUN |
+ cricket::PORTALLOCATOR_DISABLE_TCP;
+ // We use one endpoint to test the behavior of adding remote candidates, and
+ // this endpoint only gathers relay candidates.
+ ConfigureEndpoints(OPEN, OPEN, kOnlyRelayPorts, kDefaultPortAllocatorFlags);
+ GetEndpoint(0)->cd1_.ch_.reset(CreateChannel(
+ 0, ICE_CANDIDATE_COMPONENT_DEFAULT, kIceParams[0], kIceParams[1]));
+ IceConfig config;
+ // Start gathering and we should have only a single relay port.
+ ep1_ch1()->SetIceConfig(config);
+ ep1_ch1()->MaybeStartGathering();
+ EXPECT_EQ_WAIT(IceGatheringState::kIceGatheringComplete,
+ ep1_ch1()->gathering_state(), kDefaultTimeout);
+ EXPECT_EQ(1u, ep1_ch1()->ports().size());
+ // Add a plain remote host candidate and three remote mDNS candidates with the
+ // host, srflx and relay types. Note that the candidates differ in their
+ // ports.
+ cricket::Candidate host_candidate = CreateUdpCandidate(
+ LOCAL_PORT_TYPE, "1.1.1.1", 1 /* port */, 0 /* priority */);
+ ep1_ch1()->AddRemoteCandidate(host_candidate);
+
+ std::vector<cricket::Candidate> mdns_candidates;
+ mdns_candidates.push_back(CreateUdpCandidate(LOCAL_PORT_TYPE, "example.local",
+ 2 /* port */, 0 /* priority */));
+ mdns_candidates.push_back(CreateUdpCandidate(STUN_PORT_TYPE, "example.local",
+ 3 /* port */, 0 /* priority */));
+ mdns_candidates.push_back(CreateUdpCandidate(RELAY_PORT_TYPE, "example.local",
+ 4 /* port */, 0 /* priority */));
+ // We just resolve the hostname to 1.1.1.1, and add the candidates with this
+ // address directly to simulate the process of adding remote candidates with
+ // the name resolution.
+ for (auto& mdns_candidate : mdns_candidates) {
+ rtc::SocketAddress resolved_address(mdns_candidate.address());
+ resolved_address.SetResolvedIP(0x1111); // 1.1.1.1
+ mdns_candidate.set_address(resolved_address);
+ EXPECT_FALSE(mdns_candidate.address().IsUnresolvedIP());
+ ep1_ch1()->AddRemoteCandidate(mdns_candidate);
+ }
+
+ // All remote candidates should have been successfully added.
+ EXPECT_EQ(4u, ep1_ch1()->remote_candidates().size());
+
+ // Expect that there is no connection paired with any mDNS candidate.
+ ASSERT_EQ(1u, ep1_ch1()->connections().size());
+ ASSERT_NE(nullptr, ep1_ch1()->connections()[0]);
+ EXPECT_EQ(
+ "1.1.1.1:1",
+ ep1_ch1()->connections()[0]->remote_candidate().address().ToString());
+}
+
class MockMdnsResponder : public webrtc::MdnsResponderInterface {
public:
MOCK_METHOD2(CreateNameForAddress,
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index 68535b7..f0795ee 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -16,8 +16,10 @@
#include <vector>
#include "absl/algorithm/container.h"
+#include "absl/strings/match.h"
#include "absl/types/optional.h"
#include "p2p/base/connection.h"
+#include "p2p/base/p2p_constants.h"
#include "p2p/base/stun.h"
#include "rtc_base/async_packet_socket.h"
#include "rtc_base/byte_order.h"
@@ -540,11 +542,18 @@
CandidateOrigin origin) {
// TURN-UDP can only connect to UDP candidates.
if (!SupportsProtocol(remote_candidate.protocol())) {
- return NULL;
+ return nullptr;
}
if (state_ == STATE_DISCONNECTED || state_ == STATE_RECEIVEONLY) {
- return NULL;
+ return nullptr;
+ }
+
+ // If the remote endpoint signaled us an mDNS candidate, we do not form a pair
+ // with the relay candidate to avoid IP leakage in the CreatePermission
+ // request.
+ if (absl::EndsWith(remote_candidate.address().hostname(), LOCAL_TLD)) {
+ return nullptr;
}
// A TURN port will have two candiates, STUN and TURN. STUN may not
@@ -568,7 +577,7 @@
return conn;
}
}
- return NULL;
+ return nullptr;
}
bool TurnPort::FailAndPruneConnection(const rtc::SocketAddress& address) {
