Fix WebRTC IP leaks. WebRTC binds to individual NICs and listens for incoming Stun packets. Sending stun through this specific NIC binding could make OS route the packet differently hence exposing non-VPN public IP. The fix here is 1. to bind to any address (0:0:0:0) instead. This way, the routing will be the same as how chrome/http is. 2. also, remove the any all 0s addresses which happens when we bind to all 0s. BUG=4276 R=juberti@webrtc.org Review URL: https://webrtc-codereview.appspot.com/39129004 Cr-Commit-Position: refs/heads/master@{#8418} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8418 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc index 07180d5..bbebe8a 100644 --- a/webrtc/base/network.cc +++ b/webrtc/base/network.cc
@@ -153,6 +153,26 @@ } } +void NetworkManagerBase::GetAnyAddressNetworks(NetworkList* networks) { + if (!ipv4_any_address_network_) { + const rtc::IPAddress ipv4_any_address(INADDR_ANY); + ipv4_any_address_network_.reset( + new rtc::Network("any", "any", ipv4_any_address, 0)); + ipv4_any_address_network_->AddIP(ipv4_any_address); + } + networks->push_back(ipv4_any_address_network_.get()); + + if (ipv6_enabled()) { + if (!ipv6_any_address_network_) { + const rtc::IPAddress ipv6_any_address(in6addr_any); + ipv6_any_address_network_.reset( + new rtc::Network("any", "any", ipv6_any_address, 0)); + ipv6_any_address_network_->AddIP(ipv6_any_address); + } + networks->push_back(ipv6_any_address_network_.get()); + } +} + void NetworkManagerBase::GetNetworks(NetworkList* result) const { int ipv6_networks = 0; result->clear();
diff --git a/webrtc/base/network.h b/webrtc/base/network.h index cd01300..36aa5f8 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h
@@ -19,6 +19,7 @@ #include "webrtc/base/basictypes.h" #include "webrtc/base/ipaddress.h" #include "webrtc/base/messagehandler.h" +#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/sigslot.h" #if defined(WEBRTC_POSIX) @@ -78,6 +79,12 @@ // include ignored networks. virtual void GetNetworks(NetworkList* networks) const = 0; + // "AnyAddressNetwork" is a network which only contains single "any address" + // IP address. (i.e. INADDR_ANY for IPv4 or in6addr_any for IPv6). This is + // useful as binding to such interfaces allow default routing behavior like + // http traffic. + virtual void GetAnyAddressNetworks(NetworkList* networks) = 0; + // Dumps a list of networks available to LS_INFO. virtual void DumpNetworks(bool include_ignored) {} @@ -98,6 +105,7 @@ virtual ~NetworkManagerBase(); virtual void GetNetworks(std::vector<Network*>* networks) const; + virtual void GetAnyAddressNetworks(NetworkList* networks); bool ipv6_enabled() const { return ipv6_enabled_; } void set_ipv6_enabled(bool enabled) { ipv6_enabled_ = enabled; } @@ -127,6 +135,9 @@ NetworkMap networks_map_; bool ipv6_enabled_; + + rtc::scoped_ptr<rtc::Network> ipv4_any_address_network_; + rtc::scoped_ptr<rtc::Network> ipv6_any_address_network_; }; // Basic implementation of the NetworkManager interface that gets list
diff --git a/webrtc/base/virtualsocket_unittest.cc b/webrtc/base/virtualsocket_unittest.cc index e441a0f..e9d57f8 100644 --- a/webrtc/base/virtualsocket_unittest.cc +++ b/webrtc/base/virtualsocket_unittest.cc
@@ -605,16 +605,18 @@ } } - void BandwidthTest(const SocketAddress& send_address, - const SocketAddress& recv_address) { + // It is important that initial_addr's port has to be 0 such that the + // incremental port behavior could ensure the 2 Binds result in different + // address. + void BandwidthTest(const SocketAddress& initial_addr) { AsyncSocket* send_socket = - ss_->CreateAsyncSocket(send_address.family(), SOCK_DGRAM); + ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM); AsyncSocket* recv_socket = - ss_->CreateAsyncSocket(recv_address.family(), SOCK_DGRAM); - ASSERT_EQ(0, send_socket->Bind(send_address)); - ASSERT_EQ(0, recv_socket->Bind(recv_address)); - EXPECT_EQ(send_socket->GetLocalAddress().family(), send_address.family()); - EXPECT_EQ(recv_socket->GetLocalAddress().family(), recv_address.family()); + ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM); + ASSERT_EQ(0, send_socket->Bind(initial_addr)); + ASSERT_EQ(0, recv_socket->Bind(initial_addr)); + EXPECT_EQ(send_socket->GetLocalAddress().family(), initial_addr.family()); + EXPECT_EQ(recv_socket->GetLocalAddress().family(), initial_addr.family()); ASSERT_EQ(0, send_socket->Connect(recv_socket->GetLocalAddress())); uint32 bandwidth = 64 * 1024; @@ -634,8 +636,10 @@ ss_->set_bandwidth(0); } - void DelayTest(const SocketAddress& send_addr, - const SocketAddress& recv_addr) { + // It is important that initial_addr's port has to be 0 such that the + // incremental port behavior could ensure the 2 Binds result in different + // address. + void DelayTest(const SocketAddress& initial_addr) { time_t seed = ::time(NULL); LOG(LS_VERBOSE) << "seed = " << seed; srand(static_cast<unsigned int>(seed)); @@ -648,13 +652,13 @@ ss_->UpdateDelayDistribution(); AsyncSocket* send_socket = - ss_->CreateAsyncSocket(send_addr.family(), SOCK_DGRAM); + ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM); AsyncSocket* recv_socket = - ss_->CreateAsyncSocket(recv_addr.family(), SOCK_DGRAM); - ASSERT_EQ(0, send_socket->Bind(send_addr)); - ASSERT_EQ(0, recv_socket->Bind(recv_addr)); - EXPECT_EQ(send_socket->GetLocalAddress().family(), send_addr.family()); - EXPECT_EQ(recv_socket->GetLocalAddress().family(), recv_addr.family()); + ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM); + ASSERT_EQ(0, send_socket->Bind(initial_addr)); + ASSERT_EQ(0, recv_socket->Bind(initial_addr)); + EXPECT_EQ(send_socket->GetLocalAddress().family(), initial_addr.family()); + EXPECT_EQ(recv_socket->GetLocalAddress().family(), initial_addr.family()); ASSERT_EQ(0, send_socket->Connect(recv_socket->GetLocalAddress())); Thread* pthMain = Thread::Current(); @@ -836,28 +840,20 @@ } TEST_F(VirtualSocketServerTest, bandwidth_v4) { - SocketAddress send_address("1.1.1.1", 1000); - SocketAddress recv_address("1.1.1.2", 1000); - BandwidthTest(send_address, recv_address); + BandwidthTest(kIPv4AnyAddress); } TEST_F(VirtualSocketServerTest, bandwidth_v6) { - SocketAddress send_address("::1", 1000); - SocketAddress recv_address("::2", 1000); - BandwidthTest(send_address, recv_address); + BandwidthTest(kIPv6AnyAddress); } TEST_F(VirtualSocketServerTest, delay_v4) { - SocketAddress send_address("1.1.1.1", 1000); - SocketAddress recv_address("1.1.1.2", 1000); - DelayTest(send_address, recv_address); + DelayTest(kIPv4AnyAddress); } // See: https://code.google.com/p/webrtc/issues/detail?id=2409 TEST_F(VirtualSocketServerTest, DISABLED_delay_v6) { - SocketAddress send_address("::1", 1000); - SocketAddress recv_address("::2", 1000); - DelayTest(send_address, recv_address); + DelayTest(kIPv6AnyAddress); } // Works, receiving socket sees 127.0.0.2.
diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 1a3735b..6543585 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h
@@ -38,6 +38,7 @@ PORTALLOCATOR_ENABLE_SHARED_UFRAG = 0x80, PORTALLOCATOR_ENABLE_SHARED_SOCKET = 0x100, PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE = 0x200, + PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION = 0x400, }; const uint32 kDefaultPortAllocatorFlags = 0;
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 64fce88..9ca8aa5 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc
@@ -385,7 +385,16 @@ void BasicPortAllocatorSession::DoAllocate() { bool done_signal_needed = false; std::vector<rtc::Network*> networks; - allocator_->network_manager()->GetNetworks(&networks); + + // If the adapter enumeration is disabled, we'll just bind to any address + // instead of specific NIC. This is to ensure the same routing for http + // traffic by OS is also used here to avoid any local or public IP leakage + // during stun process. + if (flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) { + allocator_->network_manager()->GetAnyAddressNetworks(&networks); + } else { + allocator_->network_manager()->GetNetworks(&networks); + } if (networks.empty()) { LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated"; done_signal_needed = true; @@ -477,7 +486,14 @@ PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE) != 0); // Push down the candidate_filter to individual port. - port->set_candidate_filter(allocator_->candidate_filter()); + uint32 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) { + candidate_filter &= ~CF_HOST; + } + port->set_candidate_filter(candidate_filter); PortData data(port, seq); ports_.push_back(data); @@ -595,27 +611,44 @@ bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) { uint32 filter = allocator_->candidate_filter(); - bool allowed = false; - if (filter & CF_RELAY) { - allowed |= (c.type() == RELAY_PORT_TYPE); + + // 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 + // send. All 0s is not a valid ICE candidate address and should be filtered + // out. + if (c.address().IsAnyIP()) { + return false; } - if (filter & CF_REFLEXIVE) { - // We allow host candidates if the filter allows server-reflexive candidates - // and the candidate is a public IP. Because we don't generate - // server-reflexive candidates if they have the same IP as the host - // candidate (i.e. when the host candidate is a public IP), filtering to - // only server-reflexive candidates won't work right when the host - // candidates have public IPs. - allowed |= (c.type() == STUN_PORT_TYPE) || - (c.type() == LOCAL_PORT_TYPE && !c.address().IsPrivateIP()); - } + if (c.type() == RELAY_PORT_TYPE) { + return (filter & CF_RELAY); + } else if (c.type() == STUN_PORT_TYPE) { + return (filter & CF_REFLEXIVE); + } else if (c.type() == LOCAL_PORT_TYPE) { + if ((filter & CF_REFLEXIVE) && !c.address().IsPrivateIP()) { + // We allow host candidates if the filter allows server-reflexive + // candidates and the candidate is a public IP. Because we don't generate + // server-reflexive candidates if they have the same IP as the host + // candidate (i.e. when the host candidate is a public IP), filtering to + // only server-reflexive candidates won't work right when the host + // candidates have public IPs. + return true; + } - if (filter & CF_HOST) { - allowed |= (c.type() == LOCAL_PORT_TYPE); - } + // This is just to prevent the case when binding to any address (all 0s), if + // somehow the host candidate address is not all 0s. Either because local + // installed proxy changes the address or a packet has been sent for any + // reason before getsockname is called. + if (flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) { + LOG(LS_WARNING) << "Received non-0 host address: " + << c.address().ToString() + << " when adapter enumeration is disabled"; + return false; + } - return allowed; + return (filter & CF_HOST); + } + return false; } void BasicPortAllocatorSession::OnPortAllocationComplete(
diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 6f83beb..b32d312 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc
@@ -37,6 +37,7 @@ static const SocketAddress kClientAddr("11.11.11.11", 0); static const SocketAddress kPrivateAddr("192.168.1.11", 0); +static const SocketAddress kPrivateAddr2("192.168.1.12", 0); static const SocketAddress kClientIPv6Addr( "2401:fa00:4:1000:be30:5bff:fee5:c3", 0); static const SocketAddress kClientAddr2("22.22.22.22", 0); @@ -227,6 +228,29 @@ } } + void CheckDisableAdapterEnumeration() { + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + session_->set_flags(cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION); + session_->StartGettingPorts(); + EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); + + // Only 2 candidates as local UDP/TCP are all 0s and get trimmed out. + EXPECT_EQ(2U, candidates_.size()); + EXPECT_EQ(2U, ports_.size()); // One stunport and one turnport. + + EXPECT_PRED5(CheckCandidate, candidates_[0], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp", + rtc::SocketAddress(kNatAddr.ipaddr(), 0)); + EXPECT_EQ( + rtc::EmptySocketAddressWithFamily(candidates_[0].address().family()), + candidates_[0].related_address()); + + EXPECT_PRED5(CheckCandidate, candidates_[1], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp", + rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0)); + EXPECT_EQ(kNatAddr.ipaddr(), candidates_[1].related_address().ipaddr()); + } + protected: cricket::BasicPortAllocator& allocator() { return *allocator_; @@ -425,6 +449,28 @@ EXPECT_TRUE(candidate_allocation_done_); } +// Test that we should only get STUN and TURN candidates when adapter +// enumeration is disabled. +TEST_F(PortAllocatorTest, TestDisableAdapterEnumeration) { + AddInterface(kClientAddr); + // GTURN is not configured here. + ResetWithNatServer(kStunAddr); + AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); + + CheckDisableAdapterEnumeration(); +} + +// Test that even with multiple interfaces, the result should be only 1 Stun +// candidate since we bind to any address (i.e. all 0s). +TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationMultipleInterfaces) { + AddInterface(kPrivateAddr); + AddInterface(kPrivateAddr2); + ResetWithNatServer(kStunAddr); + AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); + + CheckDisableAdapterEnumeration(); +} + // Test that we can get OnCandidatesAllocationDone callback when all the ports // are disabled. TEST_F(PortAllocatorTest, TestDisableAllPorts) {