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

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

This change filters out local ports when CF_HOST is not originally specified to prevent these ports from sending out STUN which leaks IP address.

BUG=webrtc:4946
R=pthatcher@webrtc.org

Committed: https://crrev.com/898d21c1d4981d3bd7b090a198d224d69d1ddf10
Cr-Commit-Position: refs/heads/master@{#10121}

[guoweis_webrtc, 2015-09-30 01:06:50]

guoweis@webrtc.org changed reviewers:
+ pthatcher@webrtc.org

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

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:480: // port is not filtered out, it'll
send STUN ping and cause IP address leak.
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."

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:484: (port->SharedSocket() ||
c.protocol() == TCP_PROTOCOL_NAME));
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));

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/portall...
File webrtc/p2p/client/portallocator_unittest.cc (right):

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/portall...
webrtc/p2p/client/portallocator_unittest.cc:588: // Test that when enumeration
is disabled, we should not have any port when
any port => any ports

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

PTAL.

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
File webrtc/p2p/client/basicportallocator.cc (right):

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:480: // port is not filtered out, it'll
send STUN ping and cause IP address leak.
On 2015/09/30 05:32:10, pthatcher1 wrote:
> 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."

Done.

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/basicpo...
webrtc/p2p/client/basicportallocator.cc:484: (port->SharedSocket() ||
c.protocol() == TCP_PROTOCOL_NAME));
On 2015/09/30 05:32:10, pthatcher1 wrote:
> 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));

Done.

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/portall...
File webrtc/p2p/client/portallocator_unittest.cc (right):

https://codereview.webrtc.org/1378753003/diff/20001/webrtc/p2p/client/portall...
webrtc/p2p/client/portallocator_unittest.cc:588: // Test that when enumeration
is disabled, we should not have any port when
On 2015/09/30 05:32:10, pthatcher1 wrote:
> any port => any ports

Done.

[pthatcher1, 2015-09-30 17:39:38]

lgtm

[guoweis_webrtc, 2015-09-30 17:42:24]

The CQ bit was checked by guoweis@webrtc.org

[commit-bot: I haz the power, 2015-09-30 17:42:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1378753003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1378753003/40001

[guoweis_webrtc, 2015-09-30 17:55:09]

Committed patchset #3 (id:40001) manually as
898d21c1d4981d3bd7b090a198d224d69d1ddf10 (presubmit successful).

[commit-bot: I haz the power, 2015-09-30 17:55:13]

Patchset 3 (id:??) landed as
https://crrev.com/898d21c1d4981d3bd7b090a198d224d69d1ddf10
Cr-Commit-Position: refs/heads/master@{#10121}