WebRTC might leak srflx ip address when multiple_routes disabled and IceTransportType is relay

webrtc/p2p/client/basicportallocator.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 455 matching lines @@
   ASSERT(rtc::Thread::Current() == network_thread_);
   PortData* data = FindPort(port);
   ASSERT(data != NULL);
   // Discarding any candidate signal if port allocation status is
   // already in completed state.
   if (data->complete())
     return;
 
   ProtocolType pvalue;
   bool candidate_signalable = CheckCandidateFilter(c);
+
+  // Here we only allow the port if the CF_HOST was originally specified in the
+  // allocator's candidate filter, as when enumeration is disabled, the port's
+  // filter has been modified to prevent local address leakage. If this local
+  // port is not filtered out, it'll send STUN ping and cause IP address leak.

[pthatcher1, 2015/09/30 05:32:10]

I think this could be a little more, perhaps with something like 

"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."

[guoweis_webrtc, 2015/09/30 16:29:28]

Done.

   bool candidate_pairable =
       candidate_signalable ||
-      (c.address().IsAnyIP() &&
+      ((allocator_->candidate_filter() & CF_HOST) && c.address().IsAnyIP() &&
        (port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME));

[pthatcher1, 2015/09/30 05:32:10]

This is getting a little hard to read.  Perhaps some named variables would help:

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));

[guoweis_webrtc, 2015/09/30 16:29:28]

Done.

   bool candidate_protocol_enabled =
       StringToProto(c.protocol().c_str(), &pvalue) &&
       data->sequence()->ProtocolEnabled(pvalue);
 
   if (candidate_signalable && candidate_protocol_enabled) {
     std::vector<Candidate> candidates;
     candidates.push_back(c);
     SignalCandidatesReady(this, candidates);
   }
 
@@ skipping 685 matching lines @@
   ServerAddresses servers;
   for (size_t i = 0; i < relays.size(); ++i) {
     if (relays[i].type == turn_type && SupportsProtocol(relays[i], type)) {
       servers.insert(relays[i].ports.front().address);
     }
   }
   return servers;
 }
 
 }  // namespace cricket