Fixing the behavior of the candidate filter with pooled candidates.

webrtc/p2p/client/basicportallocator.cc

Index: webrtc/p2p/client/basicportallocator.cc
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index bf61bc88fc9612074b6eaa5e387b026645027eb2..14f7b13212db920adbfd7bc957eb1894ef798e8d 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -167,6 +167,29 @@ BasicPortAllocatorSession::~BasicPortAllocatorSession() {
     delete sequences_[i];
 }
 
+void BasicPortAllocatorSession::SetCandidateFilter(uint32_t filter) {
+  if (filter == candidate_filter_) {
+    return;
+  }
+  // We assume the filter will only change from "ALL" to something else.
+  RTC_DCHECK(candidate_filter_ == CF_ALL);
+  candidate_filter_ = filter;
+  for (PortData& port : ports_) {
+    if (!port.has_pairable_candidate()) {
+      continue;
+    }
+    const auto& candidates = port.port()->Candidates();
+    // Setting a filter may cause a ready port to become non-ready
+    // if it no longer has any pairable candidates.
+    if (!std::any_of(candidates.begin(), candidates.end(),
+                     [this, &port](const Candidate& candidate) {
+                       return CandidatePairable(candidate, port.port());
+                     })) {
+      port.set_has_pairable_candidate(false);
+    }
+  }
+}
+
 void BasicPortAllocatorSession::StartGettingPorts() {
   network_thread_ = rtc::Thread::Current();
   if (!socket_factory_) {
@@ -195,7 +218,7 @@ void BasicPortAllocatorSession::ClearGettingPorts() {
 std::vector<PortInterface*> BasicPortAllocatorSession::ReadyPorts() const {
   std::vector<PortInterface*> ret;
   for (const PortData& port : ports_) {
-    if (port.ready() || port.complete()) {
+    if (port.has_pairable_candidate() && !port.error()) {
       ret.push_back(port.port());
     }
   }
@@ -214,12 +237,32 @@ std::vector<Candidate> BasicPortAllocatorSession::ReadyCandidates() const {
           !data.sequence()->ProtocolEnabled(pvalue)) {
         continue;
       }
-      candidates.push_back(candidate);
+      candidates.push_back(SanitizeRelatedAddress(candidate));
     }
   }
   return candidates;
 }
 
+Candidate BasicPortAllocatorSession::SanitizeRelatedAddress(
+    const Candidate& c) const {
+  Candidate copy = c;
+  // If adapter enumeration is disabled or host candidates are disabled,
+  // clear the raddr of STUN candidates to avoid local address leakage.
+  bool filter_stun_related_address =
+      ((flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) &&
+       (flags() & PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE)) ||
+      !(candidate_filter_ & CF_HOST);
+  // If the candidate filter doesn't allow reflexive addresses, empty TURN raddr
+  // to avoid reflexive address leakage.
+  bool filter_turn_related_address = !(candidate_filter_ & CF_REFLEXIVE);
+  if ((c.type() == STUN_PORT_TYPE && filter_stun_related_address) ||
+      (c.type() == RELAY_PORT_TYPE && filter_turn_related_address)) {
+    copy.set_related_address(
+        rtc::EmptySocketAddressWithFamily(copy.address().family()));
+  }
+  return copy;
+}
+
 bool BasicPortAllocatorSession::CandidatesAllocationDone() const {
   // Done only if all required AllocationSequence objects
   // are created.
@@ -483,17 +526,6 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port,
   port->set_send_retransmit_count_attribute(
       (flags() & PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE) != 0);
 
-  // Push down the candidate_filter to individual port.
-  uint32_t candidate_filter = allocator_->candidate_filter();
-
-  // When adapter enumeration is disabled, disable CF_HOST at port level so
-  // local address is not leaked by stunport in the candidate's related address.
-  if ((flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) &&
-      (flags() & PORTALLOCATOR_DISABLE_DEFAULT_LOCAL_CANDIDATE)) {
-    candidate_filter &= ~CF_HOST;
-  }
-  port->set_candidate_filter(candidate_filter);
-
   PortData data(port, seq);
   ports_.push_back(data);
 
@@ -529,44 +561,29 @@ void BasicPortAllocatorSession::OnCandidateReady(
   }
 
   ProtocolType pvalue;
-  bool candidate_signalable = CheckCandidateFilter(c);
-
-  // When device enumeration is disabled (to prevent non-default IP addresses
-  // from leaking), we ping from some local candidates even though we don't
-  // signal them. However, if host candidates are also disabled (for example, to
-  // prevent even default IP addresses from leaking), we still don't want to
-  // ping from them, even if device enumeration is disabled.  Thus, we check for
-  // both device enumeration and host candidates being disabled.
-  bool network_enumeration_disabled = c.address().IsAnyIP();
-  bool can_ping_from_candidate =
-      (port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME);
-  bool host_canidates_disabled = !(allocator_->candidate_filter() & CF_HOST);
-
-  bool candidate_pairable =
-      candidate_signalable ||
-      (network_enumeration_disabled && can_ping_from_candidate &&
-       !host_canidates_disabled);
   bool candidate_protocol_enabled =
       StringToProto(c.protocol().c_str(), &pvalue) &&
       data->sequence()->ProtocolEnabled(pvalue);
 
-  if (candidate_signalable && candidate_protocol_enabled) {
+  if (CheckCandidateFilter(c) && candidate_protocol_enabled) {
     std::vector<Candidate> candidates;
-    candidates.push_back(c);
+    candidates.push_back(SanitizeRelatedAddress(c));
     SignalCandidatesReady(this, candidates);
   }
 
-  // Port has been made ready. Nothing to do here.
-  if (data->ready()) {
+  // Port has already been marked as having a pairable candidate.
+  // Nothing to do here.
+  if (data->has_pairable_candidate()) {
     return;
   }
 
-  // Move the port to the READY state, either because we have a usable candidate
-  // from the port, or simply because the port is bound to the any address and
-  // therefore has no host candidate. This will trigger the port to start
-  // creating candidate pairs (connections) and issue connectivity checks.
-  if (candidate_pairable) {
-    data->set_ready();
+  // Mark that the port has a pairable candidate, either because we have a
+  // usable candidate from the port, or simply because the port is bound to the
+  // any address and therefore has no host candidate. This will trigger the port
+  // to start creating candidate pairs (connections) and issue connectivity
+  // checks.
+  if (CandidatePairable(c, port)) {
+    data->set_has_pairable_candidate(true);
     SignalPortReady(this, port);
   }
 }
@@ -613,8 +630,9 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq,
 
     const std::vector<Candidate>& potentials = it->port()->Candidates();
     for (size_t i = 0; i < potentials.size(); ++i) {
-      if (!CheckCandidateFilter(potentials[i]))
+      if (!CheckCandidateFilter(potentials[i])) {
         continue;
+      }
       ProtocolType pvalue;
       bool candidate_protocol_enabled =
           StringToProto(potentials[i].protocol().c_str(), &pvalue) &&
@@ -631,7 +649,7 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq,
 }
 
 bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
-  uint32_t filter = allocator_->candidate_filter();
+  uint32_t filter = candidate_filter_;
 
   // When binding to any address, before sending packets out, the getsockname
   // returns all 0s, but after sending packets, it'll be the NIC used to
@@ -661,6 +679,26 @@ bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) const {
   return false;
 }
 
+bool BasicPortAllocatorSession::CandidatePairable(const Candidate& c,
+                                                  const Port* port) const {
+  bool candidate_signalable = CheckCandidateFilter(c);
+
+  // When device enumeration is disabled (to prevent non-default IP addresses
+  // from leaking), we ping from some local candidates even though we don't
+  // signal them. However, if host candidates are also disabled (for example, to
+  // prevent even default IP addresses from leaking), we still don't want to
+  // ping from them, even if device enumeration is disabled.  Thus, we check for
+  // both device enumeration and host candidates being disabled.
+  bool network_enumeration_disabled = c.address().IsAnyIP();
+  bool can_ping_from_candidate =
+      (port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME);
+  bool host_candidates_disabled = !(candidate_filter_ & CF_HOST);
+
+  return candidate_signalable ||
+         (network_enumeration_disabled && can_ping_from_candidate &&
+          !host_candidates_disabled);
+}
+
 void BasicPortAllocatorSession::OnPortAllocationComplete(
     AllocationSequence* seq) {
   // Send candidate allocation complete signal if all ports are done.