From f358aea7bff8091c608e1afd8cf395ec2702ff76 Mon Sep 17 00:00:00 2001 From: "guoweis@webrtc.org" Date: Wed, 18 Feb 2015 18:44:01 +0000 Subject: [PATCH] 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 --- webrtc/base/network.cc | 20 ++++++ webrtc/base/network.h | 11 ++++ webrtc/base/virtualsocket_unittest.cc | 52 +++++++-------- webrtc/p2p/base/portallocator.h | 1 + webrtc/p2p/client/basicportallocator.cc | 71 +++++++++++++++------ webrtc/p2p/client/portallocator_unittest.cc | 46 +++++++++++++ 6 files changed, 154 insertions(+), 47 deletions(-) diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc index 07180d51c7..bbebe8a36e 100644 --- a/webrtc/base/network.cc +++ b/webrtc/base/network.cc @@ -153,6 +153,26 @@ NetworkManagerBase::~NetworkManagerBase() { } } +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 cd013001b9..36aa5f82e4 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 @@ class NetworkManager { // 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 @@ class NetworkManagerBase : public NetworkManager { virtual ~NetworkManagerBase(); virtual void GetNetworks(std::vector* 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 @@ class NetworkManagerBase : public NetworkManager { NetworkMap networks_map_; bool ipv6_enabled_; + + rtc::scoped_ptr ipv4_any_address_network_; + rtc::scoped_ptr 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 e441a0f438..e9d57f8f30 100644 --- a/webrtc/base/virtualsocket_unittest.cc +++ b/webrtc/base/virtualsocket_unittest.cc @@ -605,16 +605,18 @@ class VirtualSocketServerTest : public testing::Test { } } - 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 @@ class VirtualSocketServerTest : public testing::Test { 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(seed)); @@ -648,13 +652,13 @@ class VirtualSocketServerTest : public testing::Test { 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, TcpSendsPacketsInOrder_v6) { } 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 1a3735b16d..654358504b 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -38,6 +38,7 @@ enum { 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 64fce889ad..9ca8aa5162 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -385,7 +385,16 @@ void BasicPortAllocatorSession::OnAllocate() { void BasicPortAllocatorSession::DoAllocate() { bool done_signal_needed = false; std::vector 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 @@ void BasicPortAllocatorSession::AddAllocatedPort(Port* port, 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 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq, 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 6f83beb6ca..b32d3124d0 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -37,6 +37,7 @@ using rtc::Thread; 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 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { } } + 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 @@ TEST_F(PortAllocatorTest, TestGetAllPortsNoAdapters) { 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) {