From fe3bc9d5aeffed8bbfb34c330d8b991abd1a1aba Mon Sep 17 00:00:00 2001 From: Guo-wei Shieh Date: Thu, 20 Aug 2015 08:48:20 -0700 Subject: [PATCH] Relanding "Generate localhost candidate when no STUN/TURN and portallocator has the right flag spefied." Migrated from https://codereview.webrtc.org/1275703006/ which causes test failures for android. On android, loopback interface was used as local interface to generate candidates. Add a test case to make sure this won't be broken in the future. Also observed some failures under content_browsertests in chromium.fyi bot but can't repro locally. Might just be temporary test issue. BUG=webrtc:4517 TBR=pthatcher@webrtc.org Review URL: https://codereview.webrtc.org/1299333003 . Cr-Commit-Position: refs/heads/master@{#9746} --- talk/app/webrtc/peerconnectioninterface.h | 4 ++ talk/app/webrtc/webrtcsession.cc | 7 ++ webrtc/base/ipaddress.cc | 11 ++- webrtc/base/ipaddress.h | 2 + webrtc/p2p/base/port_unittest.cc | 2 +- webrtc/p2p/base/portallocator.h | 3 + webrtc/p2p/base/stunport.cc | 24 +++++-- webrtc/p2p/base/stunport.h | 23 +++++-- webrtc/p2p/base/stunport_unittest.cc | 2 +- webrtc/p2p/base/turnport_unittest.cc | 2 +- webrtc/p2p/client/basicportallocator.cc | 31 +++++---- webrtc/p2p/client/fakeportallocator.h | 3 +- webrtc/p2p/client/portallocator_unittest.cc | 75 ++++++++++++++++----- 13 files changed, 143 insertions(+), 46 deletions(-) diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index ac6280fc24..b3a56006d4 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -237,6 +237,9 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // TODO(pthatcher): Rename this ice_servers, but update Chromium // at the same time. IceServers servers; + // A localhost candidate is signaled whenever a candidate with the any + // address is allocated. + bool enable_localhost_ice_candidate; BundlePolicy bundle_policy; RtcpMuxPolicy rtcp_mux_policy; TcpCandidatePolicy tcp_candidate_policy; @@ -245,6 +248,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface { RTCConfiguration() : type(kAll), + enable_localhost_ice_candidate(false), bundle_policy(kBundlePolicyBalanced), rtcp_mux_policy(kRtcpMuxPolicyNegotiate), tcp_candidate_policy(kTcpCandidatePolicyEnabled), diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index fde45104d6..be567b88b4 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -725,6 +725,13 @@ bool WebRtcSession::Initialize( } port_allocator()->set_candidate_filter( ConvertIceTransportTypeToCandidateFilter(rtc_configuration.type)); + + if (rtc_configuration.enable_localhost_ice_candidate) { + port_allocator()->set_flags( + port_allocator()->flags() | + cricket::PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE); + } + return true; } diff --git a/webrtc/base/ipaddress.cc b/webrtc/base/ipaddress.cc index 768a7092a3..3dd856a64a 100644 --- a/webrtc/base/ipaddress.cc +++ b/webrtc/base/ipaddress.cc @@ -498,4 +498,13 @@ int IPAddressPrecedence(const IPAddress& ip) { return 0; } -} // Namespace talk base +IPAddress GetLoopbackIP(int family) { + if (family == AF_INET) { + return rtc::IPAddress(INADDR_LOOPBACK); + } + if (family == AF_INET6) { + return rtc::IPAddress(in6addr_loopback); + } + return rtc::IPAddress(); +} +} // Namespace rtc diff --git a/webrtc/base/ipaddress.h b/webrtc/base/ipaddress.h index 606878b77d..0f32d3a166 100644 --- a/webrtc/base/ipaddress.h +++ b/webrtc/base/ipaddress.h @@ -179,6 +179,8 @@ int IPAddressPrecedence(const IPAddress& ip); // Returns 'ip' truncated to be 'length' bits long. IPAddress TruncateIP(const IPAddress& ip, int length); +IPAddress GetLoopbackIP(int family); + // Returns the number of contiguously set bits, counting from the MSB in network // byte order, in this IPAddress. Bits after the first 0 encountered are not // counted. diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 85a7e239d2..2c122da6fc 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -436,7 +436,7 @@ class PortTest : public testing::Test, public sigslot::has_slots<> { PacketSocketFactory* socket_factory) { return UDPPort::Create(main_, socket_factory, &network_, addr.ipaddr(), 0, 0, username_, password_, - std::string()); + std::string(), false); } TCPPort* CreateTcpPort(const SocketAddress& addr) { return CreateTcpPort(addr, &socket_factory_); diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h index 6d9680e26a..c89684727b 100644 --- a/webrtc/p2p/base/portallocator.h +++ b/webrtc/p2p/base/portallocator.h @@ -43,6 +43,9 @@ enum { PORTALLOCATOR_ENABLE_SHARED_SOCKET = 0x100, PORTALLOCATOR_ENABLE_STUN_RETRANSMIT_ATTRIBUTE = 0x200, PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION = 0x400, + // When specified, a loopback candidate will be generated if + // PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION is specified. + PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE = 0x800, }; const uint32 kDefaultPortAllocatorFlags = 0; diff --git a/webrtc/p2p/base/stunport.cc b/webrtc/p2p/base/stunport.cc index 925bd8e9de..696d323e64 100644 --- a/webrtc/p2p/base/stunport.cc +++ b/webrtc/p2p/base/stunport.cc @@ -15,6 +15,7 @@ #include "webrtc/p2p/base/stun.h" #include "webrtc/base/common.h" #include "webrtc/base/helpers.h" +#include "webrtc/base/ipaddress.h" #include "webrtc/base/logging.h" #include "webrtc/base/nethelpers.h" @@ -164,14 +165,16 @@ UDPPort::UDPPort(rtc::Thread* thread, rtc::AsyncPacketSocket* socket, const std::string& username, const std::string& password, - const std::string& origin) + const std::string& origin, + bool emit_localhost_for_anyaddress) : Port(thread, factory, network, socket->GetLocalAddress().ipaddr(), username, password), requests_(thread), socket_(socket), error_(0), ready_(false), - stun_keepalive_delay_(KEEPALIVE_DELAY) { + stun_keepalive_delay_(KEEPALIVE_DELAY), + emit_localhost_for_anyaddress_(emit_localhost_for_anyaddress) { requests_.set_origin(origin); } @@ -183,14 +186,16 @@ UDPPort::UDPPort(rtc::Thread* thread, uint16 max_port, const std::string& username, const std::string& password, - const std::string& origin) + const std::string& origin, + bool emit_localhost_for_anyaddress) : Port(thread, LOCAL_PORT_TYPE, factory, network, ip, min_port, max_port, username, password), requests_(thread), socket_(NULL), error_(0), ready_(false), - stun_keepalive_delay_(KEEPALIVE_DELAY) { + stun_keepalive_delay_(KEEPALIVE_DELAY), + emit_localhost_for_anyaddress_(emit_localhost_for_anyaddress) { requests_.set_origin(origin); } @@ -280,7 +285,16 @@ int UDPPort::GetError() { void UDPPort::OnLocalAddressReady(rtc::AsyncPacketSocket* socket, const rtc::SocketAddress& address) { - AddAddress(address, address, rtc::SocketAddress(), UDP_PROTOCOL_NAME, "", "", + // When adapter enumeration is disabled and binding to the any address, the + // loopback address will be issued as a candidate instead if + // |emit_localhost_for_anyaddress| is true. This is to allow connectivity on + // demo pages without STUN/TURN to work. + rtc::SocketAddress addr = address; + if (addr.IsAnyIP() && emit_localhost_for_anyaddress_) { + addr.SetIP(rtc::GetLoopbackIP(addr.family())); + } + + AddAddress(addr, addr, rtc::SocketAddress(), UDP_PROTOCOL_NAME, "", "", LOCAL_PORT_TYPE, ICE_TYPE_PREFERENCE_HOST, 0, false); MaybePrepareStunCandidate(); } diff --git a/webrtc/p2p/base/stunport.h b/webrtc/p2p/base/stunport.h index d840a97126..2967c15d95 100644 --- a/webrtc/p2p/base/stunport.h +++ b/webrtc/p2p/base/stunport.h @@ -34,9 +34,11 @@ class UDPPort : public Port { rtc::AsyncPacketSocket* socket, const std::string& username, const std::string& password, - const std::string& origin) { + const std::string& origin, + bool emit_localhost_for_anyaddress) { UDPPort* port = new UDPPort(thread, factory, network, socket, - username, password, origin); + username, password, origin, + emit_localhost_for_anyaddress); if (!port->Init()) { delete port; port = NULL; @@ -52,10 +54,12 @@ class UDPPort : public Port { uint16 max_port, const std::string& username, const std::string& password, - const std::string& origin) { + const std::string& origin, + bool emit_localhost_for_anyaddress) { UDPPort* port = new UDPPort(thread, factory, network, ip, min_port, max_port, - username, password, origin); + username, password, origin, + emit_localhost_for_anyaddress); if (!port->Init()) { delete port; port = NULL; @@ -110,7 +114,8 @@ class UDPPort : public Port { uint16 max_port, const std::string& username, const std::string& password, - const std::string& origin); + const std::string& origin, + bool emit_localhost_for_anyaddress); UDPPort(rtc::Thread* thread, rtc::PacketSocketFactory* factory, @@ -118,7 +123,8 @@ class UDPPort : public Port { rtc::AsyncPacketSocket* socket, const std::string& username, const std::string& password, - const std::string& origin); + const std::string& origin, + bool emit_localhost_for_anyaddress); bool Init(); @@ -202,6 +208,9 @@ class UDPPort : public Port { bool ready_; int stun_keepalive_delay_; + // This is true when PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE is specified. + bool emit_localhost_for_anyaddress_; + friend class StunBindingRequest; }; @@ -245,7 +254,7 @@ class StunPort : public UDPPort { const ServerAddresses& servers, const std::string& origin) : UDPPort(thread, factory, network, ip, min_port, max_port, username, - password, origin) { + password, origin, false) { // UDPPort will set these to local udp, updating these to STUN. set_type(STUN_PORT_TYPE); set_server_addresses(servers); diff --git a/webrtc/p2p/base/stunport_unittest.cc b/webrtc/p2p/base/stunport_unittest.cc index 8897dd4b98..173bcaebed 100644 --- a/webrtc/p2p/base/stunport_unittest.cc +++ b/webrtc/p2p/base/stunport_unittest.cc @@ -82,7 +82,7 @@ class StunPortTest : public testing::Test, rtc::Thread::Current(), &socket_factory_, &network_, socket_.get(), rtc::CreateRandomString(16), rtc::CreateRandomString(22), - std::string())); + std::string(), false)); ASSERT_TRUE(stun_port_ != NULL); ServerAddresses stun_servers; stun_servers.insert(server_addr); diff --git a/webrtc/p2p/base/turnport_unittest.cc b/webrtc/p2p/base/turnport_unittest.cc index 58195fbc62..8cec78f306 100644 --- a/webrtc/p2p/base/turnport_unittest.cc +++ b/webrtc/p2p/base/turnport_unittest.cc @@ -260,7 +260,7 @@ class TurnPortTest : public testing::Test, udp_port_.reset(UDPPort::Create(main_, &socket_factory_, &network_, kLocalAddr2.ipaddr(), 0, 0, kIceUfrag2, kIcePwd2, - std::string())); + std::string(), false)); // UDP port will be controlled. udp_port_->SetIceRole(cricket::ICEROLE_CONTROLLED); udp_port_->SignalPortComplete.connect( diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc index 037db8f529..f90c52164c 100644 --- a/webrtc/p2p/client/basicportallocator.cc +++ b/webrtc/p2p/client/basicportallocator.cc @@ -562,6 +562,14 @@ bool BasicPortAllocatorSession::CheckCandidateFilter(const Candidate& c) { return true; } + // If PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE is specified and it's + // loopback address, we should allow it as it's for demo page connectivity + // when no TURN/STUN specified. + if (c.address().IsLoopbackIP() && + (flags() & PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE) != 0) { + return true; + } + // 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 @@ -824,20 +832,19 @@ void AllocationSequence::CreateUDPPorts() { // TODO(mallinath) - Remove UDPPort creating socket after shared socket // is enabled completely. UDPPort* port = NULL; + bool emit_localhost_for_anyaddress = + IsFlagSet(PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE); if (IsFlagSet(PORTALLOCATOR_ENABLE_SHARED_SOCKET) && udp_socket_) { - port = UDPPort::Create(session_->network_thread(), - session_->socket_factory(), network_, - udp_socket_.get(), - session_->username(), session_->password(), - session_->allocator()->origin()); + port = UDPPort::Create( + session_->network_thread(), session_->socket_factory(), network_, + udp_socket_.get(), session_->username(), session_->password(), + session_->allocator()->origin(), emit_localhost_for_anyaddress); } else { - port = UDPPort::Create(session_->network_thread(), - session_->socket_factory(), - network_, ip_, - session_->allocator()->min_port(), - session_->allocator()->max_port(), - session_->username(), session_->password(), - session_->allocator()->origin()); + port = UDPPort::Create( + session_->network_thread(), session_->socket_factory(), network_, ip_, + session_->allocator()->min_port(), session_->allocator()->max_port(), + session_->username(), session_->password(), + session_->allocator()->origin(), emit_localhost_for_anyaddress); } if (port) { diff --git a/webrtc/p2p/client/fakeportallocator.h b/webrtc/p2p/client/fakeportallocator.h index 60edfee776..9e53bc04aa 100644 --- a/webrtc/p2p/client/fakeportallocator.h +++ b/webrtc/p2p/client/fakeportallocator.h @@ -53,7 +53,8 @@ class FakePortAllocatorSession : public PortAllocatorSession { 0, username(), password(), - std::string())); + std::string(), + false)); AddPort(port_.get()); } ++port_config_count_; diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc index b5f635bd0f..e3b6422548 100644 --- a/webrtc/p2p/client/portallocator_unittest.cc +++ b/webrtc/p2p/client/portallocator_unittest.cc @@ -36,6 +36,7 @@ using rtc::SocketAddress; using rtc::Thread; static const SocketAddress kClientAddr("11.11.11.11", 0); +static const SocketAddress kLoopbackAddr("127.0.0.1", 0); static const SocketAddress kPrivateAddr("192.168.1.11", 0); static const SocketAddress kPrivateAddr2("192.168.1.12", 0); static const SocketAddress kClientIPv6Addr( @@ -243,39 +244,51 @@ class PortAllocatorTest : public testing::Test, public sigslot::has_slots<> { // it should be ignore. void CheckDisableAdapterEnumeration( uint32 total_ports, + const rtc::IPAddress& host_candidate_addr, 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 | + if (!session_) { + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + } + session_->set_flags(session_->flags() | + cricket::PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION | cricket::PORTALLOCATOR_ENABLE_SHARED_SOCKET); allocator().set_allow_tcp_listen(false); session_->StartGettingPorts(); EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); uint32 total_candidates = 0; - if (!stun_candidate_addr.IsNil()) { + if (!host_candidate_addr.IsNil()) { + EXPECT_PRED5(CheckCandidate, candidates_[total_candidates], + cricket::ICE_CANDIDATE_COMPONENT_RTP, "local", "udp", + rtc::SocketAddress(host_candidate_addr, 0)); ++total_candidates; - EXPECT_PRED5(CheckCandidate, candidates_[0], + } + if (!stun_candidate_addr.IsNil()) { + EXPECT_PRED5(CheckCandidate, candidates_[total_candidates], 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()); + EXPECT_EQ(rtc::EmptySocketAddressWithFamily( + candidates_[total_candidates].address().family()), + candidates_[total_candidates].related_address()); + ++total_candidates; } if (!relay_candidate_udp_transport_addr.IsNil()) { - ++total_candidates; - EXPECT_PRED5(CheckCandidate, candidates_[1], + EXPECT_PRED5(CheckCandidate, candidates_[total_candidates], 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()); + EXPECT_EQ(stun_candidate_addr, + candidates_[total_candidates].related_address().ipaddr()); + ++total_candidates; } if (!relay_candidate_tcp_transport_addr.IsNil()) { - ++total_candidates; - EXPECT_PRED5(CheckCandidate, candidates_[2], + EXPECT_PRED5(CheckCandidate, candidates_[total_candidates], 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_EQ(stun_candidate_addr, + candidates_[total_candidates].related_address().ipaddr()); + ++total_candidates; } EXPECT_EQ(total_candidates, candidates_.size()); @@ -382,6 +395,18 @@ TEST_F(PortAllocatorTest, TestNoNetworkInterface) { EXPECT_EQ(0U, candidates_.size()); } +// Test that we could use loopback interface as host candidate. +TEST_F(PortAllocatorTest, TestLoopbackNetworkInterface) { + AddInterface(kLoopbackAddr); + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + session_->set_flags(cricket::PORTALLOCATOR_DISABLE_STUN | + cricket::PORTALLOCATOR_DISABLE_RELAY | + cricket::PORTALLOCATOR_DISABLE_TCP); + session_->StartGettingPorts(); + EXPECT_TRUE_WAIT(candidate_allocation_done_, kDefaultAllocationTimeout); + EXPECT_EQ(1U, candidates_.size()); +} + // Tests that we can get all the desired addresses successfully. TEST_F(PortAllocatorTest, TestGetAllPortsWithMinimumStepDelay) { AddInterface(kClientAddr); @@ -508,7 +533,7 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationBehindNat) { 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(), + CheckDisableAdapterEnumeration(3U, rtc::IPAddress(), kNatUdpAddr.ipaddr(), kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); } @@ -522,7 +547,7 @@ TEST_F(PortAllocatorTest, 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(), + CheckDisableAdapterEnumeration(3U, rtc::IPAddress(), kNatUdpAddr.ipaddr(), kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); } @@ -536,7 +561,7 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationBehindNatWithTcp) { 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(), + CheckDisableAdapterEnumeration(4U, rtc::IPAddress(), kNatUdpAddr.ipaddr(), kTurnUdpExtAddr.ipaddr(), kTurnUdpExtAddr.ipaddr()); } @@ -551,7 +576,7 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationWithoutNat) { // 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(), + CheckDisableAdapterEnumeration(3U, rtc::IPAddress(), kClientAddr.ipaddr(), kTurnUdpExtAddr.ipaddr(), rtc::IPAddress()); } @@ -562,6 +587,22 @@ TEST_F(PortAllocatorTest, TestDisableAdapterEnumerationWithoutNatOrServers) { ResetWithNoServersOrNat(); // Expect to see 2 ports: STUN and TCP ports, but no candidate. CheckDisableAdapterEnumeration(2U, rtc::IPAddress(), rtc::IPAddress(), + rtc::IPAddress(), rtc::IPAddress()); +} + +// Test that when adapter enumeration is disabled, with +// PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE specified, for endpoints not behind +// a NAT, there are a localhost candidate in addition to a STUN candidate. +TEST_F(PortAllocatorTest, + TestDisableAdapterEnumerationWithoutNatLocalhostCandidateRequested) { + AddInterfaceAsDefaultRoute(kClientAddr); + ResetWithStunServerNoNat(kStunAddr); + EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP)); + session_->set_flags(cricket::PORTALLOCATOR_ENABLE_LOCALHOST_CANDIDATE); + // Expect to see 2 ports: STUN and TCP ports, localhost candidate and STUN + // candidate. + CheckDisableAdapterEnumeration(2U, rtc::GetLoopbackIP(AF_INET), + kClientAddr.ipaddr(), rtc::IPAddress(), rtc::IPAddress()); }