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}

p2p/base/p2p_constants.cc

@@ -41,6 +41,8 @@ const char CONNECTIONROLE_PASSIVE_STR[] = "passive";
 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;

p2p/base/p2p_constants.h

@@ -48,6 +48,9 @@ extern const char CONNECTIONROLE_PASSIVE_STR[];
 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

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::SanitizeLocalCandidate(
 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;

p2p/base/p2p_transport_channel_unittest.cc

@@ -5247,6 +5247,59 @@ TEST_F(P2PTransportChannelTest,
   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,

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 @@ Connection* TurnPort::CreateConnection(const Candidate& remote_candidate,
                                        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 @@ Connection* TurnPort::CreateConnection(const Candidate& remote_candidate,
       return conn;
     }
   }
-  return NULL;
+  return nullptr;
 }
 
 bool TurnPort::FailAndPruneConnection(const rtc::SocketAddress& address) {