From 743b9b258c4eda14ce0e13ee80873fcd3ca2aafb Mon Sep 17 00:00:00 2001
From: Peter Thatcher <peter@signal.org>
Date: Wed, 27 May 2020 09:51:05 -0700
Subject: [PATCH] Disable remote ICE candidate DNS lookups when the
 IceTransportPolicy is Relay or None

Bug: webrtc:11597
Change-Id: Id3884a2b5f0fc35880c7401c43ca25fee8346519
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175960
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31389}
---
 AUTHORS                                    |  1 +
 p2p/base/p2p_transport_channel.cc          |  7 +-
 p2p/base/p2p_transport_channel_unittest.cc | 80 ++++++++++++++++++++++
 3 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/AUTHORS b/AUTHORS
index 499c340639..90b6cb7d4d 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -108,6 +108,7 @@ Opera Software ASA <*@opera.com>
 Optical Tone Ltd <*@opticaltone.com>
 Pengutronix e.K. <*@pengutronix.de>
 RingCentral, Inc. <*@ringcentral.com>
+Signal Messenger, LLC <*@signal.org>
 Sinch AB <*@sinch.com>
 struktur AG <*@struktur.de>
 Telenor Digital AS <*@telenor.com>
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 90d3e14d1c..b96857a2ac 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -1195,7 +1195,12 @@ void P2PTransportChannel::AddRemoteCandidate(const Candidate& candidate) {
   }
 
   if (new_remote_candidate.address().IsUnresolvedIP()) {
-    ResolveHostnameCandidate(new_remote_candidate);
+    // Don't do DNS lookups if the IceTransportPolicy is "none" or "relay".
+    bool sharing_host = ((allocator_->candidate_filter() & CF_HOST) != 0);
+    bool sharing_stun = ((allocator_->candidate_filter() & CF_REFLEXIVE) != 0);
+    if (sharing_host || sharing_stun) {
+      ResolveHostnameCandidate(new_remote_candidate);
+    }
     return;
   }
 
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index ea8d66f890..059b13fcc2 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -5728,4 +5728,84 @@ TEST(P2PTransportChannel, InjectIceController) {
       /* event_log = */ nullptr, &factory);
 }
 
+TEST_F(P2PTransportChannelTest, DisableDnsLookupsWithTransportPolicyRelay) {
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+  auto* ep1 = GetEndpoint(0);
+  ep1->allocator_->SetCandidateFilter(CF_RELAY);
+
+  rtc::MockAsyncResolver mock_async_resolver;
+  webrtc::MockAsyncResolverFactory mock_async_resolver_factory;
+  ON_CALL(mock_async_resolver_factory, Create())
+      .WillByDefault(Return(&mock_async_resolver));
+  ep1->async_resolver_factory_ = &mock_async_resolver_factory;
+
+  bool lookup_started = false;
+  ON_CALL(mock_async_resolver, Start(_))
+      .WillByDefault(Assign(&lookup_started, true));
+
+  CreateChannels();
+
+  ep1_ch1()->AddRemoteCandidate(
+      CreateUdpCandidate(LOCAL_PORT_TYPE, "hostname.test", 1, 100));
+
+  EXPECT_FALSE(lookup_started);
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest, DisableDnsLookupsWithTransportPolicyNone) {
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+  auto* ep1 = GetEndpoint(0);
+  ep1->allocator_->SetCandidateFilter(CF_NONE);
+
+  rtc::MockAsyncResolver mock_async_resolver;
+  webrtc::MockAsyncResolverFactory mock_async_resolver_factory;
+  ON_CALL(mock_async_resolver_factory, Create())
+      .WillByDefault(Return(&mock_async_resolver));
+  ep1->async_resolver_factory_ = &mock_async_resolver_factory;
+
+  bool lookup_started = false;
+  ON_CALL(mock_async_resolver, Start(_))
+      .WillByDefault(Assign(&lookup_started, true));
+
+  CreateChannels();
+
+  ep1_ch1()->AddRemoteCandidate(
+      CreateUdpCandidate(LOCAL_PORT_TYPE, "hostname.test", 1, 100));
+
+  EXPECT_FALSE(lookup_started);
+
+  DestroyChannels();
+}
+
+TEST_F(P2PTransportChannelTest, EnableDnsLookupsWithTransportPolicyNoHost) {
+  ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+                     kDefaultPortAllocatorFlags);
+  auto* ep1 = GetEndpoint(0);
+  ep1->allocator_->SetCandidateFilter(CF_ALL & ~CF_HOST);
+
+  rtc::MockAsyncResolver mock_async_resolver;
+  webrtc::MockAsyncResolverFactory mock_async_resolver_factory;
+  EXPECT_CALL(mock_async_resolver_factory, Create())
+      .WillOnce(Return(&mock_async_resolver));
+  EXPECT_CALL(mock_async_resolver, Destroy(_));
+
+  ep1->async_resolver_factory_ = &mock_async_resolver_factory;
+
+  bool lookup_started = false;
+  EXPECT_CALL(mock_async_resolver, Start(_))
+      .WillOnce(Assign(&lookup_started, true));
+
+  CreateChannels();
+
+  ep1_ch1()->AddRemoteCandidate(
+      CreateUdpCandidate(LOCAL_PORT_TYPE, "hostname.test", 1, 100));
+
+  EXPECT_TRUE(lookup_started);
+
+  DestroyChannels();
+}
+
 }  // namespace cricket