From 38f8893235f3b80ae9ca89db66d62ca819b51c01 Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Thu, 13 Aug 2015 22:24:02 -0700 Subject: [PATCH] WebRTC Bug 4865 Bug 4865: even without STUN/TURN, as long as the peer is on the open internet, the connectivity should work. This is actually a regression even for hangouts. We need to issue the 0.0.0.0 candidate into Port::candidates_ and filter it out later. The reason is that when we create connection, we need a local candidate to match the remote candidate. The same connection later will be updated with the prflx local candidate once the STUN ping response is received. BUG=webrtc:4865 R=juberti@webrtc.org Review URL: https://codereview.webrtc.org/1274013002 . Cr-Commit-Position: refs/heads/master@{#9708} --- webrtc/base/virtualsocket_unittest.cc | 47 +++++++ webrtc/base/virtualsocketserver.cc | 48 ++++++- webrtc/base/virtualsocketserver.h | 14 +- webrtc/p2p/client/basicportallocator.cc | 44 +++--- webrtc/p2p/client/portallocator_unittest.cc | 143 ++++++++++++++++---- 5 files changed, 247 insertions(+), 49 deletions(-) diff --git a/webrtc/base/virtualsocket_unittest.cc b/webrtc/base/virtualsocket_unittest.cc index e9d57f8f30..a656691b50 100644 --- a/webrtc/base/virtualsocket_unittest.cc +++ b/webrtc/base/virtualsocket_unittest.cc @@ -149,6 +149,41 @@ class VirtualSocketServerTest : public testing::Test { } } + // Test a client can bind to the any address, and all sent packets will have + // the default route as the source address. Also, it can receive packets sent + // to the default route. + void TestDefaultRoute(const IPAddress& default_route) { + ss_->SetDefaultRoute(default_route); + + // Create client1 bound to the any address. + AsyncSocket* socket = + ss_->CreateAsyncSocket(default_route.family(), SOCK_DGRAM); + socket->Bind(EmptySocketAddressWithFamily(default_route.family())); + SocketAddress client1_any_addr = socket->GetLocalAddress(); + EXPECT_TRUE(client1_any_addr.IsAnyIP()); + TestClient* client1 = new TestClient(new AsyncUDPSocket(socket)); + + // Create client2 bound to the default route. + AsyncSocket* socket2 = + ss_->CreateAsyncSocket(default_route.family(), SOCK_DGRAM); + socket2->Bind(SocketAddress(default_route, 0)); + SocketAddress client2_addr = socket2->GetLocalAddress(); + EXPECT_FALSE(client2_addr.IsAnyIP()); + TestClient* client2 = new TestClient(new AsyncUDPSocket(socket2)); + + // Client1 sends to client2, client2 should see the default route as + // client1's address. + SocketAddress client1_addr; + EXPECT_EQ(6, client1->SendTo("bizbaz", 6, client2_addr)); + EXPECT_TRUE(client2->CheckNextPacket("bizbaz", 6, &client1_addr)); + EXPECT_EQ(client1_addr, + SocketAddress(default_route, client1_any_addr.port())); + + // Client2 can send back to client1's default route address. + EXPECT_EQ(3, client2->SendTo("foo", 3, client1_addr)); + EXPECT_TRUE(client1->CheckNextPacket("foo", 3, &client2_addr)); + } + void BasicTest(const SocketAddress& initial_addr) { AsyncSocket* socket = ss_->CreateAsyncSocket(initial_addr.family(), SOCK_DGRAM); @@ -791,6 +826,18 @@ TEST_F(VirtualSocketServerTest, basic_v6) { BasicTest(ipv6_test_addr); } +TEST_F(VirtualSocketServerTest, TestDefaultRoute_v4) { + IPAddress ipv4_default_addr(0x01020304); + TestDefaultRoute(ipv4_default_addr); +} + +TEST_F(VirtualSocketServerTest, TestDefaultRoute_v6) { + IPAddress ipv6_default_addr; + EXPECT_TRUE( + IPFromString("2401:fa00:4:1000:be30:5bff:fee5:c3", &ipv6_default_addr)); + TestDefaultRoute(ipv6_default_addr); +} + TEST_F(VirtualSocketServerTest, connect_v4) { ConnectTest(kIPv4AnyAddress); } diff --git a/webrtc/base/virtualsocketserver.cc b/webrtc/base/virtualsocketserver.cc index 9b0e48d1d8..c2f0e01dc0 100644 --- a/webrtc/base/virtualsocketserver.cc +++ b/webrtc/base/virtualsocketserver.cc @@ -17,6 +17,7 @@ #include #include +#include "webrtc/base/checks.h" #include "webrtc/base/common.h" #include "webrtc/base/logging.h" #include "webrtc/base/physicalsocketserver.h" @@ -661,7 +662,22 @@ VirtualSocket* VirtualSocketServer::LookupBinding(const SocketAddress& addr) { SocketAddress normalized(addr.ipaddr().Normalized(), addr.port()); AddressMap::iterator it = bindings_->find(normalized); - return (bindings_->end() != it) ? it->second : NULL; + if (it != bindings_->end()) { + return it->second; + } + + IPAddress default_ip = GetDefaultRoute(addr.ipaddr().family()); + if (!IPIsUnspec(default_ip) && addr.ipaddr() == default_ip) { + // If we can't find a binding for the packet which is sent to the interface + // corresponding to the default route, it should match a binding with the + // correct port to the any address. + SocketAddress sock_addr = + EmptySocketAddressWithFamily(addr.ipaddr().family()); + sock_addr.SetPort(addr.port()); + return LookupBinding(sock_addr); + } + + return nullptr; } int VirtualSocketServer::Unbind(const SocketAddress& addr, @@ -866,8 +882,18 @@ void VirtualSocketServer::AddPacketToNetwork(VirtualSocket* sender, // Find the delay for crossing the many virtual hops of the network. uint32 transit_delay = GetRandomTransitDelay(); + // When the incoming packet is from a binding of the any address, translate it + // to the default route here such that the recipient will see the default + // route. + SocketAddress sender_addr = sender->local_addr_; + IPAddress default_ip = GetDefaultRoute(sender_addr.ipaddr().family()); + if (sender_addr.IsAnyIP() && !IPIsUnspec(default_ip)) { + sender_addr.SetIP(default_ip); + } + // Post the packet as a message to be delivered (on our own thread) - Packet* p = new Packet(data, data_size, sender->local_addr_); + Packet* p = new Packet(data, data_size, sender_addr); + uint32 ts = TimeAfter(send_delay + transit_delay); if (ordered) { // Ensure that new packets arrive after previous ones @@ -1080,4 +1106,22 @@ bool VirtualSocketServer::CanInteractWith(VirtualSocket* local, return false; } +IPAddress VirtualSocketServer::GetDefaultRoute(int family) { + if (family == AF_INET) { + return default_route_v4_; + } + if (family == AF_INET6) { + return default_route_v6_; + } + return IPAddress(); +} +void VirtualSocketServer::SetDefaultRoute(const IPAddress& from_addr) { + DCHECK(!IPIsAny(from_addr)); + if (from_addr.family() == AF_INET) { + default_route_v4_ = from_addr; + } else if (from_addr.family() == AF_INET6) { + default_route_v6_ = from_addr; + } +} + } // namespace rtc diff --git a/webrtc/base/virtualsocketserver.h b/webrtc/base/virtualsocketserver.h index b96269c52c..c708bb4a89 100644 --- a/webrtc/base/virtualsocketserver.h +++ b/webrtc/base/virtualsocketserver.h @@ -38,6 +38,11 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> { SocketServer* socketserver() { return server_; } + // The default route indicates which local address to use when a socket is + // bound to the 'any' address, e.g. 0.0.0.0. + IPAddress GetDefaultRoute(int family); + void SetDefaultRoute(const IPAddress& from_addr); + // Limits the network bandwidth (maximum bytes per second). Zero means that // all sends occur instantly. Defaults to 0. uint32 bandwidth() const { return bandwidth_; } @@ -224,6 +229,9 @@ class VirtualSocketServer : public SocketServer, public sigslot::has_slots<> { AddressMap* bindings_; ConnectionMap* connections_; + IPAddress default_route_v4_; + IPAddress default_route_v6_; + uint32 bandwidth_; uint32 network_capacity_; uint32 send_buffer_capacity_; @@ -248,9 +256,6 @@ class VirtualSocket : public AsyncSocket, public MessageHandler { SocketAddress GetLocalAddress() const override; SocketAddress GetRemoteAddress() const override; - // Used by server sockets to set the local address without binding. - void SetLocalAddress(const SocketAddress& addr); - // Used by TurnPortTest to mimic a case where proxy returns local host address // instead of the original one TurnPort was bound against. Please see WebRTC // issue 3927 for more detail. @@ -297,6 +302,9 @@ class VirtualSocket : public AsyncSocket, public MessageHandler { int SendUdp(const void* pv, size_t cb, const SocketAddress& addr); int SendTcp(const void* pv, size_t cb); + // Used by server sockets to set the local address without binding. + void SetLocalAddress(const SocketAddress& addr); + VirtualSocketServer* server_; int family_; int type_; diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 3f5aa0a1ff..c5cfbd94a1 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -21,6 +21,7 @@ #include "webrtc/p2p/base/tcpport.h" #include "webrtc/p2p/base/turnport.h" #include "webrtc/p2p/base/udpport.h" +#include "webrtc/base/checks.h" #include "webrtc/base/common.h" #include "webrtc/base/helpers.h" #include "webrtc/base/logging.h" @@ -440,26 +441,32 @@ void BasicPortAllocatorSession::OnCandidateReady( if (data->complete()) return; - // Send candidates whose protocol is enabled. - std::vector candidates; ProtocolType pvalue; - bool candidate_allowed_to_send = CheckCandidateFilter(c); - if (StringToProto(c.protocol().c_str(), &pvalue) && - data->sequence()->ProtocolEnabled(pvalue) && - candidate_allowed_to_send) { - candidates.push_back(c); - } + bool candidate_signalable = CheckCandidateFilter(c); + bool candidate_pairable = + candidate_signalable || + (c.address().IsAnyIP() && + (port->SharedSocket() || c.protocol() == TCP_PROTOCOL_NAME)); + bool candidate_protocol_enabled = + StringToProto(c.protocol().c_str(), &pvalue) && + data->sequence()->ProtocolEnabled(pvalue); - if (!candidates.empty()) { + if (candidate_signalable && candidate_protocol_enabled) { + std::vector candidates; + candidates.push_back(c); SignalCandidatesReady(this, candidates); } - // Moving to READY state as we have atleast one candidate from the port. - // Since this port has atleast one candidate we should forward this port - // to listners, to allow connections from this port. - // Also we should make sure that candidate gathered from this port is allowed - // to send outside. - if (!data->ready() && candidate_allowed_to_send) { + // Port has been made ready. Nothing to do here. + if (data->ready()) { + return; + } + + // Move the port to the READY state, either because we have a usable candidate + // from the port, or simply because the port is bound to the any address and + // therefore has no host candidate. This will trigger the port to start + // creating candidate pairs (connections) and issue connectivity checks. + if (candidate_pairable) { data->set_ready(); SignalPortReady(this, port); } @@ -508,9 +515,10 @@ void BasicPortAllocatorSession::OnProtocolEnabled(AllocationSequence* seq, if (!CheckCandidateFilter(potentials[i])) continue; ProtocolType pvalue; - if (!StringToProto(potentials[i].protocol().c_str(), &pvalue)) - continue; - if (pvalue == proto) { + bool candidate_protocol_enabled = + StringToProto(potentials[i].protocol().c_str(), &pvalue) && + pvalue == proto; + if (candidate_protocol_enabled) { candidates.push_back(potentials[i]); } } diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index 7dc25140cc..73bf45bd76 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -88,7 +88,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { fss_(new rtc::FirewallSocketServer(vss_.get())), ss_scope_(fss_.get()), nat_factory_(vss_.get(), kNatUdpAddr, kNatTcpAddr), - nat_socket_factory_(&nat_factory_), + nat_socket_factory_(new rtc::BasicPacketSocketFactory(&nat_factory_)), stun_server_(cricket::TestStunServer::Create(Thread::Current(), kStunAddr)), relay_server_(Thread::Current(), kRelayUdpIntAddr, kRelayUdpExtAddr, @@ -110,9 +110,20 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { void AddInterface(const SocketAddress& addr) { network_manager_.AddInterface(addr); } + void AddInterfaceAsDefaultRoute(const SocketAddress& addr) { + AddInterface(addr); + // When a binding comes from the any address, the |addr| will be used as the + // srflx address. + vss_->SetDefaultRoute(addr.ipaddr()); + } bool SetPortRange(int min_port, int max_port) { return allocator_->SetPortRange(min_port, max_port); } + // No STUN/TURN configuration. + void ResetWithNoServers() { + allocator_.reset(new cricket::BasicPortAllocator(&network_manager_)); + allocator_->set_step_delay(cricket::kMinimumStepDelay); + } void ResetWithNatServer(const rtc::SocketAddress& stun_server) { nat_server_.reset(new rtc::NATServer( rtc::NAT_OPEN_CONE, vss_.get(), kNatUdpAddr, kNatTcpAddr, vss_.get(), @@ -123,7 +134,19 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { stun_servers.insert(stun_server); } allocator_.reset(new cricket::BasicPortAllocator( - &network_manager_, &nat_socket_factory_, stun_servers)); + &network_manager_, nat_socket_factory_.get(), stun_servers)); + allocator().set_step_delay(cricket::kMinimumStepDelay); + } + + void ResetWithStunServer(const rtc::SocketAddress& stun_server) { + nat_socket_factory_.reset(new rtc::BasicPacketSocketFactory()); + + ServerAddresses stun_servers; + if (!stun_server.IsNil()) { + stun_servers.insert(stun_server); + } + allocator_.reset(new cricket::BasicPortAllocator( + &network_manager_, nat_socket_factory_.get(), stun_servers)); allocator().set_step_delay(cricket::kMinimumStepDelay); } @@ -232,27 +255,51 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { } } - void CheckDisableAdapterEnumeration() { + // This function starts the port/address gathering and check the existence of + // candidates as specified. When |expect_stun_candidate| is true, + // |stun_candidate_addr| carries the expected reflective address, which is + // also the related address for TURN candidate if it is expected. Otherwise, + // it should be ignore. + void CheckDisableAdapterEnumeration( + uint32 total_ports, + const rtc::IPAddress& stun_candidate_addr, + const rtc::IPAddress& relay_candidate_udp_transport_addr, + const rtc::IPAddress& relay_candidate_tcp_transport_addr) { EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); - session_->set_flags(cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION); + session_->set_flags(cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION | + cricket::PORTALLOCATOR_ENABLE_SHARED_UFRAG | + cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); + allocator().set_allow_tcp_listen(false); 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. + uint32 total_candidates = 0; + if (!IPIsUnspec(stun_candidate_addr)) { + ++total_candidates; + EXPECT_PRED5(CheckCandidate, candidates_[0], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp", + rtc::SocketAddress(stun_candidate_addr, 0)); + EXPECT_EQ( + rtc::EmptySocketAddressWithFamily(candidates_[0].address().family()), + candidates_[0].related_address()); + } + if (!IPIsUnspec(relay_candidate_udp_transport_addr)) { + ++total_candidates; + EXPECT_PRED5(CheckCandidate, candidates_[1], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp", + rtc::SocketAddress(relay_candidate_udp_transport_addr, 0)); + EXPECT_EQ(stun_candidate_addr, candidates_[1].related_address().ipaddr()); + } + if (!IPIsUnspec(relay_candidate_tcp_transport_addr)) { + ++total_candidates; + EXPECT_PRED5(CheckCandidate, candidates_[2], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "relay", "udp", + rtc::SocketAddress(relay_candidate_tcp_transport_addr, 0)); + EXPECT_EQ(stun_candidate_addr, candidates_[2].related_address().ipaddr()); + } - EXPECT_PRED5(CheckCandidate, candidates_[0], - cricket::ICE_CANDIDATE_COMPONENT_RTP, "stun", "udp", - rtc::SocketAddress(kNatUdpAddr.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(kNatUdpAddr.ipaddr(), candidates_[1].related_address().ipaddr()); + EXPECT_EQ(total_candidates, candidates_.size()); + EXPECT_EQ(total_ports, ports_.size()); } protected: @@ -293,7 +340,7 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { rtc::SocketServerScope ss_scope_; rtc::scoped_ptr nat_server_; rtc::NATSocketFactory nat_factory_; - rtc::BasicPacketSocketFactory nat_socket_factory_; + rtc::scoped_ptr nat_socket_factory_; rtc::scoped_ptr stun_server_; cricket::TestRelayServer relay_server_; cricket::TestTurnServer turn_server_; @@ -455,24 +502,68 @@ TEST_F(PortAllocatorTest, TestGetAllPortsNoAdapters) { // Test that we should only get STUN and TURN candidates when adapter // enumeration is disabled. -TEST_F(PortAllocatorTest, TestDisableAdapterEnumeration) { +TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationBehindNat) { AddInterface(kClientAddr); // GTURN is not configured here. ResetWithNatServer(kStunAddr); AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); - - CheckDisableAdapterEnumeration(); + // Expect to see 3 ports: STUN, TURN/UDP and TCP ports, and both STUN and + // TURN/UDP candidates. + CheckDisableAdapterEnumeration(3U, kNatUdpAddr.ipaddr(), + kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); } -// 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) { +// Test that even with multiple interfaces, the result should still be one STUN +// and one TURN candidate since we bind to any address (i.e. all 0s). +TEST_F(PortAllocatorTest, + TestDisableAdapterEnumerationBehindNatMultipleInterfaces) { AddInterface(kPrivateAddr); AddInterface(kPrivateAddr2); ResetWithNatServer(kStunAddr); AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); + // Expect to see 3 ports: STUN, TURN/UDP and TCP ports, and both STUN and + // TURN/UDP candidates. + CheckDisableAdapterEnumeration(3U, kNatUdpAddr.ipaddr(), + kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); +} - CheckDisableAdapterEnumeration(); +// Test that we should get STUN, TURN/UDP and TURN/TCP candidates when a +// TURN/TCP server is specified. +TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationBehindNatWithTcp) { + turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP); + AddInterface(kClientAddr); + // GTURN is not configured here. + ResetWithNatServer(kStunAddr); + AddTurnServers(kTurnUdpIntAddr, kTurnTcpIntAddr); + // Expect to see 4 ports - STUN, TURN/UDP, TURN/TCP and TCP port. STUN, + // TURN/UDP, and TURN/TCP candidates. + CheckDisableAdapterEnumeration(4U, kNatUdpAddr.ipaddr(), + kTurnUdpExtAddr.ipaddr(), + kTurnUdpExtAddr.ipaddr()); +} + +// Test that we should only get STUN and TURN candidates when adapter +// enumeration is disabled. Since the endpoint is not behind NAT, the srflx +// address should be the public client interface. +TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationWithoutNat) { + AddInterfaceAsDefaultRoute(kClientAddr); + ResetWithStunServer(kStunAddr); + AddTurnServers(kTurnUdpIntAddr, rtc::SocketAddress()); + // Expect to see 3 ports: STUN, TURN/UDP and TCP ports, but only both STUN and + // TURN candidates. The STUN candidate should have kClientAddr as srflx + // address, and TURN candidate with kClientAddr as the related address. + CheckDisableAdapterEnumeration(3U, kClientAddr.ipaddr(), + kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); +} + +// Test that when adapter enumeration is disabled, for endpoints without +// STUN/TURN specified, no candidate is generated. +TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationWithoutNatOrServers) { + AddInterfaceAsDefaultRoute(kClientAddr); + ResetWithNoServers(); + // Expect to see 2 ports: STUN and TCP ports, but no candidate. + CheckDisableAdapterEnumeration(2U, rtc::IPAddress(), rtc::IPAddress(), + rtc::IPAddress()); } // Disable for asan, see