zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reland "Enable any address ports by default."

Browse Source 
This reverts commit 056a68da896d9a578b9ea83e56d261648ea0adc6.

Reason for revert: Trying to reland.

Original change's description:
> Revert "Enable any address ports by default."
> 
> This reverts commit f04148c810aad2a0809dc8978650c55308381c47.
> 
> Reason for revert: Speculative revert. I suspect this is breaking a
> downstream test (I'll reland if it is not the culprit).
> 
> Original change's description:
> > Enable any address ports by default.
> > 
> > Ports not bound to any specific network interface are allocated by
> > default. These any address ports are pruned after allocation,
> > conditional on the allocation results of normal ports that are bound to
> > the enumerated interfaces.
> > 
> > Bug: webrtc:9313
> > Change-Id: I3ce12eeab0cf3547224e5f8c188d061fc530e145
> > Reviewed-on: https://webrtc-review.googlesource.com/78383
> > Commit-Queue: Qingsi Wang <qingsi@google.com>
> > Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#23673}
> 
> TBR=deadbeef@webrtc.org,pthatcher@webrtc.org,qingsi@google.com
> 
> Change-Id: I3b3dc42c7de46d198d4b9c270020dcf1100dd907
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:9313
> Reviewed-on: https://webrtc-review.googlesource.com/84300
> Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
> Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#23678}

TBR=deadbeef@webrtc.org,mbonadei@webrtc.org,pthatcher@webrtc.org,qingsi@google.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:9313
Change-Id: I98442346babb5d8953d37dc5825efaf79804ed7f
Reviewed-on: https://webrtc-review.googlesource.com/85000
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23720}
This commit is contained in:
Mirko Bonadei
2018-06-22 12:06:07 +00:00
committed by Commit Bot
parent 87e4479924
commit ac5bbd940e
10 changed files with 433 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
p2p/base/p2pconstants.cc
View File

@ -72,4 +72,8 @@ const int CONNECTION_WRITE_TIMEOUT = 15 * 1000; // 15 seconds
// of increased memory, but in some networks (2G), we observe up to 60s RTTs.
const int CONNECTION_RESPONSE_TIMEOUT = 60 * 1000; // 60 seconds
// TODO(qingsi): Review and calibrate the value (bugs.webrtc.org/9427).
const int kMaxWaitMsBeforeSignalingAnyAddressPortsAndCandidates =
2.5 * 1000; // 2.5 seconds
} // namespace cricket

7
p2p/base/p2pconstants.h
View File

@ -102,7 +102,12 @@ extern const int CONNECTION_RESPONSE_TIMEOUT;
// The minimum time we will wait before destroying a connection after creating
// it.
extern const int MIN_CONNECTION_LIFETIME;
// TODO(qingsi): Rename all constants to kConstant style.
//
// The maximum time in milliseconds we will wait before signaling any address
// ports and candidates gathered from these ports, if the candidate allocation
// is not done yet.
extern const int kMaxWaitMsBeforeSignalingAnyAddressPortsAndCandidates;
} // namespace cricket
#endif // P2P_BASE_P2PCONSTANTS_H_

46
p2p/base/p2ptransportchannel_unittest.cc
View File

@ -52,6 +52,7 @@ static const int kOnlyLocalPorts = cricket::PORTALLOCATOR_DISABLE_STUN |
cricket::PORTALLOCATOR_DISABLE_RELAY |
cricket::PORTALLOCATOR_DISABLE_TCP;
static const int LOW_RTT = 20;
static const SocketAddress kAnyAddr("0.0.0.0", 0);
// Addresses on the public internet.
static const SocketAddress kPublicAddrs[2] = {SocketAddress("11.11.11.11", 0),
SocketAddress("22.22.22.22", 0)};
@ -426,6 +427,7 @@ class P2PTransportChannelTestBase : public testing::Test,
rtc::NATSocketServer* nat() { return nss_.get(); }
rtc::FirewallSocketServer* fw() { return ss_.get(); }
rtc::VirtualSocketServer* vss() { return vss_.get(); }
Endpoint* GetEndpoint(int endpoint) {
if (endpoint == 0) {
@ -1042,10 +1044,12 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase {
AddAddress(endpoint, kPublicAddrs[endpoint]);
// Block all UDP
fw()->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kPublicAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_UDP, rtc::FD_ANY, kAnyAddr);
if (config == BLOCK_UDP_AND_INCOMING_TCP) {
// Block TCP inbound to the endpoint
fw()->AddRule(false, rtc::FP_TCP, SocketAddress(),
kPublicAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, SocketAddress(), kAnyAddr);
} else if (config == BLOCK_ALL_BUT_OUTGOING_HTTP) {
// Block all TCP to/from the endpoint except 80/443 out
fw()->AddRule(true, rtc::FP_TCP, kPublicAddrs[endpoint],
@ -1054,12 +1058,14 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase {
SocketAddress(rtc::IPAddress(INADDR_ANY), 443));
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY,
kPublicAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY, kAnyAddr);
} else if (config == PROXY_HTTPS) {
// Block all TCP to/from the endpoint except to the proxy server
fw()->AddRule(true, rtc::FP_TCP, kPublicAddrs[endpoint],
kHttpsProxyAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY,
kPublicAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY, kAnyAddr);
SetProxy(endpoint, rtc::PROXY_HTTPS);
} else if (config == PROXY_SOCKS) {
// Block all TCP to/from the endpoint except to the proxy server
@ -1067,6 +1073,7 @@ class P2PTransportChannelTest : public P2PTransportChannelTestBase {
kSocksProxyAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY,
kPublicAddrs[endpoint]);
fw()->AddRule(false, rtc::FP_TCP, rtc::FD_ANY, kAnyAddr);
SetProxy(endpoint, rtc::PROXY_SOCKS5);
}
break;
@ -1709,16 +1716,8 @@ TEST_F(P2PTransportChannelTest, IncomingOnlyOpen) {
// connections. This has been observed in some scenarios involving
// VPNs/firewalls.
TEST_F(P2PTransportChannelTest, CanOnlyMakeOutgoingTcpConnections) {
// The PORTALLOCATOR_ENABLE_ANY_ADDRESS_PORTS flag is required if the
// application needs this use case to work, since the application must accept
// the tradeoff that more candidates need to be allocated.
//
// TODO(deadbeef): Later, make this flag the default, and do more elegant
// things to ensure extra candidates don't waste resources?
ConfigureEndpoints(
OPEN, OPEN,
kDefaultPortAllocatorFlags | PORTALLOCATOR_ENABLE_ANY_ADDRESS_PORTS,
kDefaultPortAllocatorFlags);
ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
kDefaultPortAllocatorFlags);
// In order to simulate nothing working but outgoing TCP connections, prevent
// the endpoint from binding to its interface's address as well as the
// "any" addresses. It can then only make a connection by using "Connect()".
@ -3069,6 +3068,33 @@ TEST_F(P2PTransportChannelMultihomedTest, TestRestoreBackupConnection) {
DestroyChannels();
}
// Test that when explicit binding to network interfaces is disallowed, we may
// still establish a connection by using the any address fallback.
TEST_F(P2PTransportChannelMultihomedTest,
BindingToAnyAddressRevealsViableRouteWhenExplicitBindingFails) {
rtc::ScopedFakeClock clock;
AddAddress(0, kPublicAddrs[0]);
AddAddress(1, kPublicAddrs[1]);
fw()->SetUnbindableIps({kPublicAddrs[0].ipaddr()});
// When bound to the any address, the port allocator should discover the
// alternative local address.
vss()->SetAlternativeLocalAddress(kAnyAddr.ipaddr(),
kAlternateAddrs[0].ipaddr());
SetAllocatorFlags(0, kOnlyLocalPorts);
SetAllocatorFlags(1, kOnlyLocalPorts);
IceConfig default_config;
CreateChannels(default_config, default_config);
EXPECT_TRUE_SIMULATED_WAIT(ep1_ch1()->receiving() && ep1_ch1()->writable() &&
ep2_ch1()->receiving() &&
ep2_ch1()->writable(),
kMediumTimeout, clock);
EXPECT_TRUE(
ep1_ch1()->selected_connection() && ep2_ch1()->selected_connection() &&
LocalCandidate(ep1_ch1())->address().EqualIPs(kAlternateAddrs[0]));
DestroyChannels();
}
// A collection of tests which tests a single P2PTransportChannel by sending
// pings.
class P2PTransportChannelPingTest : public testing::Test,

12
p2p/base/portallocator.h
View File

@ -75,13 +75,11 @@ enum {
// When specified, do not collect IPv6 ICE candidates on Wi-Fi.
PORTALLOCATOR_ENABLE_IPV6_ON_WIFI = 0x4000,
// When this flag is set, ports not bound to any specific network interface
// will be used, in addition to normal ports bound to the enumerated
// interfaces. Without this flag, these "any address" ports would only be
// used when network enumeration fails or is disabled. But under certain
// conditions, these ports may succeed where others fail, so they may allow
// the application to work in a wider variety of environments, at the expense
// of having to allocate additional candidates.
// This flag is deprecated; we now always enable any address ports, only
// using them if they end up using interfaces that weren't otherwise
// accessible.
//
// TODO(qingsi): Remove this flag when downstream projects no longer use it.
PORTALLOCATOR_ENABLE_ANY_ADDRESS_PORTS = 0x8000,
// Exclude link-local network interfaces

4
p2p/base/stunserver.cc
View File

@ -54,7 +54,7 @@ void StunServer::OnPacket(rtc::AsyncPacketSocket* socket,
void StunServer::OnBindingRequest(StunMessage* msg,
const rtc::SocketAddress& remote_addr) {
StunMessage response;
GetStunBindReqponse(msg, remote_addr, &response);
GetStunBindResponse(msg, remote_addr, &response);
SendResponse(response, remote_addr);
}
@ -83,7 +83,7 @@ void StunServer::SendResponse(const StunMessage& msg,
RTC_LOG_ERR(LS_ERROR) << "sendto";
}
void StunServer::GetStunBindReqponse(StunMessage* request,
void StunServer::GetStunBindResponse(StunMessage* request,
const rtc::SocketAddress& remote_addr,
StunMessage* response) const {
response->SetType(STUN_BINDING_RESPONSE);

2
p2p/base/stunserver.h
View File

@ -52,7 +52,7 @@ class StunServer : public sigslot::has_slots<> {
void SendResponse(const StunMessage& msg, const rtc::SocketAddress& addr);
// A helper method to compose a STUN binding response.
void GetStunBindReqponse(StunMessage* request,
void GetStunBindResponse(StunMessage* request,
const rtc::SocketAddress& remote_addr,
StunMessage* response) const;

2
p2p/base/teststunserver.cc
View File

@ -27,7 +27,7 @@ void TestStunServer::OnBindingRequest(StunMessage* msg,
StunServer::OnBindingRequest(msg, remote_addr);
} else {
StunMessage response;
GetStunBindReqponse(msg, fake_stun_addr_, &response);
GetStunBindResponse(msg, fake_stun_addr_, &response);
SendResponse(response, remote_addr);
}
}