From 898d21c1d4981d3bd7b090a198d224d69d1ddf10 Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Wed, 30 Sep 2015 10:54:55 -0700 Subject: [PATCH] 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 Review URL: https://codereview.webrtc.org/1378753003 . Cr-Commit-Position: refs/heads/master@{#10121} --- webrtc/p2p/client/basicportallocator.cc | 16 ++++++++++++++-- webrtc/p2p/client/portallocator_unittest.cc | 12 ++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 3c77b4feab..f343b352ee 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -473,10 +473,22 @@ 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 || - (c.address().IsAnyIP() && - (port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME)); + (network_enumeration_disabled && can_ping_from_candidate && + !host_canidates_disabled); bool candidate_protocol_enabled = StringToProto(c.protocol().c_str(), &pvalue) && data->sequence()->ProtocolEnabled(pvalue); diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 92f995a3d7..b0c77d36c6 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -585,6 +585,18 @@ TEST_F(PortAllocatorTest, TestGetAllPortsNoAdapters) { EXPECT_TRUE(candidate_allocation_done_); } +// Test that when enumeration is disabled, we should not have any ports when +// candidate_filter() is set to CF_RELAY and no relay is specified. +TEST_F(PortAllocatorTest, + TestDisableAdapterEnumerationWithoutNatRelayTransportOnly) { + AddInterfaceAsDefaultRoute(kClientAddr); + ResetWithStunServerNoNat(kStunAddr); + allocator().set_candidate_filter(cricket::CF_RELAY); + // Expect to see no ports and no candidates. + CheckDisableAdapterEnumeration(0U, rtc::IPAddress(), rtc::IPAddress(), + rtc::IPAddress(), rtc::IPAddress()); +} + // Test that we should only get STUN and TURN candidates when adapter // enumeration is disabled. TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationBehindNat) {