Sanitize the selected candidate pair in the public API.

The public API to obtain the selected candidate pair is changed to
GetSelectedCandidatePair in the ICE transport, and the returned pair
has address-sanitized candidates.

Bug: chromium:993878
Change-Id: I44f9d2385a84f9e22447108be2e57ef9e62671eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/149080
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28869}

p2p/base/connection.cc

@@ -720,6 +720,38 @@ void Connection::HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg) {
   }
 }
 
+CandidatePair Connection::ToCandidatePairAndSanitizeIfNecessary() const {
+  auto get_sanitized_copy = [](const Candidate& c) {
+    bool use_hostname_address = c.type() == LOCAL_PORT_TYPE;
+    bool filter_related_address = c.type() == STUN_PORT_TYPE;
+    return c.ToSanitizedCopy(use_hostname_address, filter_related_address);
+  };
+
+  CandidatePair pair;
+  if (port_->Network()->GetMdnsResponder() != nullptr) {
+    // When the mDNS obfuscation of local IPs is enabled, we sanitize local
+    // candidates.
+    pair.local = get_sanitized_copy(local_candidate());
+  } else {
+    pair.local = local_candidate();
+  }
+
+  if (!remote_candidate().address().hostname().empty()) {
+    // If the remote endpoint signaled us a hostname candidate, we assume it is
+    // supposed to be sanitized in the stats.
+    //
+    // A prflx remote candidate should not have a hostname set.
+    RTC_DCHECK(remote_candidate().type() != PRFLX_PORT_TYPE);
+    // A remote hostname candidate should have a resolved IP before we can form
+    // a candidate pair.
+    RTC_DCHECK(!remote_candidate().address().IsUnresolvedIP());
+    pair.remote = get_sanitized_copy(remote_candidate());
+  } else {
+    pair.remote = remote_candidate();
+  }
+  return pair;
+}
+
 void Connection::ReceivedPingResponse(
     int rtt,
     const std::string& request_id,
@@ -1107,33 +1139,9 @@ void Connection::MaybeUpdateLocalCandidate(ConnectionRequest* request,
 }
 
 void Connection::CopyCandidatesToStatsAndSanitizeIfNecessary() {
-  auto get_sanitized_copy = [](const Candidate& c) {
-    bool use_hostname_address = c.type() == LOCAL_PORT_TYPE;
-    bool filter_related_address = c.type() == STUN_PORT_TYPE;
-    return c.ToSanitizedCopy(use_hostname_address, filter_related_address);
-  };
-
-  if (port_->Network()->GetMdnsResponder() != nullptr) {
-    // When the mDNS obfuscation of local IPs is enabled, we sanitize local
-    // candidates.
-    stats_.local_candidate = get_sanitized_copy(local_candidate());
-  } else {
-    stats_.local_candidate = local_candidate();
-  }
-
-  if (!remote_candidate().address().hostname().empty()) {
-    // If the remote endpoint signaled us a hostname candidate, we assume it is
-    // supposed to be sanitized in the stats.
-    //
-    // A prflx remote candidate should not have a hostname set.
-    RTC_DCHECK(remote_candidate().type() != PRFLX_PORT_TYPE);
-    // A remote hostname candidate should have a resolved IP before we can form
-    // a candidate pair.
-    RTC_DCHECK(!remote_candidate().address().IsUnresolvedIP());
-    stats_.remote_candidate = get_sanitized_copy(remote_candidate());
-  } else {
-    stats_.remote_candidate = remote_candidate();
-  }
+  auto pair = ToCandidatePairAndSanitizeIfNecessary();
+  stats_.local_candidate = pair.local_candidate();
+  stats_.remote_candidate = pair.remote_candidate();
 }
 
 bool Connection::rtt_converged() const {

p2p/base/connection.h

@@ -36,6 +36,16 @@ class Port;
 // Forward declaration so that a ConnectionRequest can contain a Connection.
 class Connection;
 
+struct CandidatePair final : public CandidatePairInterface {
+  ~CandidatePair() override = default;
+
+  const Candidate& local_candidate() const override { return local; }
+  const Candidate& remote_candidate() const override { return remote; }
+
+  Candidate local;
+  Candidate remote;
+};
+
 // A ConnectionRequest is a simple STUN ping used to determine writability.
 class ConnectionRequest : public StunRequest {
  public:
@@ -228,6 +238,10 @@ class Connection : public CandidatePairInterface,
   void HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg);
   int64_t last_data_received() const { return last_data_received_; }
 
+  // Returns the equivalent candidate pair and sanitizes the local and the
+  // remote candidates if necessary.
+  CandidatePair ToCandidatePairAndSanitizeIfNecessary() const;
+
   // Debugging description of this connection
   std::string ToDebugId() const;
   std::string ToString() const;

p2p/base/fake_ice_transport.h

@@ -207,6 +207,10 @@ class FakeIceTransport : public IceTransportInternal {
   absl::optional<int> GetRttEstimate() override { return absl::nullopt; }
 
   const Connection* selected_connection() const override { return nullptr; }
+  absl::optional<const CandidatePair> GetSelectedCandidatePair()
+      const override {
+    return absl::nullopt;
+  }
 
   // Fake PacketTransportInternal implementation.
   bool writable() const override { return writable_; }

p2p/base/ice_transport_internal.h

@@ -263,8 +263,14 @@ class RTC_EXPORT IceTransportInternal : public rtc::PacketTransportInternal {
   // absl::optional if there is none.
   virtual absl::optional<int> GetRttEstimate() = 0;
 
+  // TODO(qingsi): Remove this method once Chrome does not depend on it anymore.
   virtual const Connection* selected_connection() const = 0;
 
+  // Returns the selected candidate pair, or an empty absl::optional if there is
+  // none.
+  virtual absl::optional<const CandidatePair> GetSelectedCandidatePair()
+      const = 0;
+
   sigslot::signal1<IceTransportInternal*> SignalGatheringState;
 
   // Handles sending and receiving of candidates.

p2p/base/mock_ice_transport.h

@@ -63,6 +63,10 @@ class MockIceTransport : public IceTransportInternal {
   void SetIceConfig(const IceConfig& config) override {}
   absl::optional<int> GetRttEstimate() override { return absl::nullopt; }
   const Connection* selected_connection() const override { return nullptr; }
+  absl::optional<const CandidatePair> GetSelectedCandidatePair()
+      const override {
+    return absl::nullopt;
+  }
   void MaybeStartGathering() override {}
   void AddRemoteCandidate(const Candidate& candidate) override {}
   void RemoveRemoteCandidate(const Candidate& candidate) override {}

p2p/base/p2p_transport_channel.cc

@@ -380,6 +380,16 @@ absl::optional<int> P2PTransportChannel::GetRttEstimate() {
   }
 }
 
+absl::optional<const CandidatePair>
+P2PTransportChannel::GetSelectedCandidatePair() const {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  if (selected_connection_ == nullptr) {
+    return absl::nullopt;
+  }
+
+  return selected_connection_->ToCandidatePairAndSanitizeIfNecessary();
+}
+
 // A channel is considered ICE completed once there is at most one active
 // connection per network and at least one active connection.
 IceTransportState P2PTransportChannel::ComputeState() const {
@@ -1965,7 +1975,6 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
   }
 
   // Create event for candidate pair change.
-
   if (selected_connection_) {
     CandidatePairChangeEvent pair_change;
     pair_change.reason = reason;
@@ -1975,7 +1984,6 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
         selected_connection_->last_data_received();
     SignalCandidatePairChanged(pair_change);
   }
-
   SignalNetworkRouteChanged(network_route_);
 }
 

p2p/base/p2p_transport_channel.h

@@ -135,6 +135,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
                 std::vector<CandidateStats>* candidate_stats_list) override;
   absl::optional<int> GetRttEstimate() override;
   const Connection* selected_connection() const override;
+  absl::optional<const CandidatePair> GetSelectedCandidatePair() const override;
 
   // TODO(honghaiz): Remove this method once the reference of it in
   // Chromoting is removed.

p2p/base/p2p_transport_channel_unittest.cc

@@ -4991,6 +4991,59 @@ TEST_F(P2PTransportChannelTest,
   DestroyChannels();
 }
 
+// A similar test as above to check the selected candidate pair is sanitized
+// when it is queried via GetSelectedCandidatePair.
+TEST_F(P2PTransportChannelTest,
+       SelectedCandidatePairSanitizedWhenMdnsObfuscationEnabled) {
+  NiceMock<rtc::MockAsyncResolver> mock_async_resolver;
+  webrtc::MockAsyncResolverFactory mock_async_resolver_factory;
+  EXPECT_CALL(mock_async_resolver_factory, Create())
+      .WillOnce(Return(&mock_async_resolver));
+
+  // ep1 and ep2 will gather host candidates with addresses
+  // kPublicAddrs[0] and kPublicAddrs[1], respectively.
+  ConfigureEndpoints(OPEN, OPEN, kOnlyLocalPorts, kOnlyLocalPorts);
+  // ICE parameter will be set up when creating the channels.
+  set_remote_ice_parameter_source(FROM_SETICEPARAMETERS);
+  GetEndpoint(0)->network_manager_.set_mdns_responder(
+      absl::make_unique<webrtc::FakeMdnsResponder>(rtc::Thread::Current()));
+  GetEndpoint(1)->async_resolver_factory_ = &mock_async_resolver_factory;
+  CreateChannels();
+  // Pause sending candidates from both endpoints until we find out what port
+  // number is assigned to ep1's host candidate.
+  PauseCandidates(0);
+  PauseCandidates(1);
+  ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout);
+  const auto& candidates_data = GetEndpoint(0)->saved_candidates_[0];
+  ASSERT_EQ(1u, candidates_data->candidates.size());
+  const auto& local_candidate_ep1 = candidates_data->candidates[0];
+  ASSERT_TRUE(local_candidate_ep1.type() == LOCAL_PORT_TYPE);
+  // This is the underlying private IP address of the same candidate at ep1,
+  // and let the mock resolver of ep2 receive the correct resolution.
+  rtc::SocketAddress resolved_address_ep1(local_candidate_ep1.address());
+  resolved_address_ep1.SetResolvedIP(kPublicAddrs[0].ipaddr());
+  EXPECT_CALL(mock_async_resolver, GetResolvedAddress(_, _))
+      .WillOnce(DoAll(SetArgPointee<1>(resolved_address_ep1), Return(true)));
+  ResumeCandidates(0);
+  ResumeCandidates(1);
+
+  ASSERT_TRUE_WAIT(ep1_ch1()->selected_connection() != nullptr &&
+                       ep2_ch1()->selected_connection() != nullptr,
+                   kMediumTimeout);
+
+  const auto pair_ep1 = ep1_ch1()->GetSelectedCandidatePair();
+  ASSERT_TRUE(pair_ep1.has_value());
+  EXPECT_EQ(LOCAL_PORT_TYPE, pair_ep1->local_candidate().type());
+  EXPECT_TRUE(pair_ep1->local_candidate().address().IsUnresolvedIP());
+
+  const auto pair_ep2 = ep2_ch1()->GetSelectedCandidatePair();
+  ASSERT_TRUE(pair_ep2.has_value());
+  EXPECT_EQ(LOCAL_PORT_TYPE, pair_ep2->remote_candidate().type());
+  EXPECT_TRUE(pair_ep2->remote_candidate().address().IsUnresolvedIP());
+
+  DestroyChannels();
+}
+
 class MockMdnsResponder : public webrtc::MdnsResponderInterface {
  public:
   MOCK_METHOD2(CreateNameForAddress,