Don't use link-local networks to determine the lowest cost of networks
On Chrome Remote Desktop for iOS, if all of these are true: * Enable the PORTALLOCATOR_DISABLE_COSTLY_NETWORKS flag. * Connect an iPhone to a Mac. * Turn off WiFi on the iPhone and keep mobile data on. * Connect to a host on a different network from iPhone. Then the connection can never succeed. The reason is that iOS uses a special network interface with link-local IP to communicate with the Mac. BasicPortAllocator sees that interface and thinks it costs less than the cellular networks, then it removes all cellular networks. However, that link-local network cannot connect to a peer on external network. This CL changes the behavior of the PORTALLOCATOR_DISABLE_COSTLY_NETWORKS so that it ignores the cost of link-local networks when determining the lowest network cost. Bug: webrtc:8780 Change-Id: I9bde50426b356fdbf89d4b826fc7b28c8c523c10 Reviewed-on: https://webrtc-review.googlesource.com/41460 Commit-Queue: Yuwei Huang <yuweih@google.com> Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org> Cr-Commit-Position: refs/heads/master@{#21738}
This commit is contained in:
@ -644,6 +644,13 @@ std::vector<rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
|
||||
if (flags() & PORTALLOCATOR_DISABLE_COSTLY_NETWORKS) {
|
||||
uint16_t lowest_cost = rtc::kNetworkCostMax;
|
||||
for (rtc::Network* network : networks) {
|
||||
// Don't determine the lowest cost from a link-local network.
|
||||
// On iOS, a device connected to the computer will get a link-local
|
||||
// network for communicating with the computer, however this network can't
|
||||
// be used to connect to a peer outside the network.
|
||||
if (rtc::IPIsLinkLocal(network->GetBestIP())) {
|
||||
continue;
|
||||
}
|
||||
lowest_cost = std::min<uint16_t>(lowest_cost, network->GetCost());
|
||||
}
|
||||
networks.erase(std::remove_if(networks.begin(), networks.end(),
|
||||
|
@ -849,6 +849,57 @@ TEST_F(BasicPortAllocatorTest,
|
||||
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", cellular);
|
||||
}
|
||||
|
||||
// Test that if both PORTALLOCATOR_DISABLE_COSTLY_NETWORKS is set, and there is
|
||||
// a WiFi network with link-local IP address and a cellular network, then the
|
||||
// cellular candidate will still be gathered.
|
||||
TEST_F(BasicPortAllocatorTest,
|
||||
CellNotRemovedWhenCostlyNetworksDisabledAndWifiIsLinkLocal) {
|
||||
SocketAddress wifi_link_local("169.254.0.1", 0);
|
||||
SocketAddress cellular(IPAddress(0x12345601U), 0);
|
||||
AddInterface(wifi_link_local, "test_wlan0", rtc::ADAPTER_TYPE_WIFI);
|
||||
AddInterface(cellular, "test_cell0", rtc::ADAPTER_TYPE_CELLULAR);
|
||||
|
||||
allocator().set_flags(cricket::PORTALLOCATOR_DISABLE_STUN |
|
||||
cricket::PORTALLOCATOR_DISABLE_RELAY |
|
||||
cricket::PORTALLOCATOR_DISABLE_TCP |
|
||||
cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS);
|
||||
EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
|
||||
session_->StartGettingPorts();
|
||||
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
|
||||
kDefaultAllocationTimeout, fake_clock);
|
||||
// Make sure we got both wifi and cell candidates.
|
||||
EXPECT_EQ(2U, candidates_.size());
|
||||
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", wifi_link_local);
|
||||
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", cellular);
|
||||
}
|
||||
|
||||
// Test that if both PORTALLOCATOR_DISABLE_COSTLY_NETWORKS is set, and there is
|
||||
// a WiFi network with link-local IP address, a WiFi network with a normal IP
|
||||
// address and a cellular network, then the cellular candidate will not be
|
||||
// gathered.
|
||||
TEST_F(BasicPortAllocatorTest,
|
||||
CellRemovedWhenCostlyNetworksDisabledAndBothWifisPresent) {
|
||||
SocketAddress wifi(IPAddress(0x12345600U), 0);
|
||||
SocketAddress wifi_link_local("169.254.0.1", 0);
|
||||
SocketAddress cellular(IPAddress(0x12345601U), 0);
|
||||
AddInterface(wifi, "test_wlan0", rtc::ADAPTER_TYPE_WIFI);
|
||||
AddInterface(wifi_link_local, "test_wlan1", rtc::ADAPTER_TYPE_WIFI);
|
||||
AddInterface(cellular, "test_cell0", rtc::ADAPTER_TYPE_CELLULAR);
|
||||
|
||||
allocator().set_flags(cricket::PORTALLOCATOR_DISABLE_STUN |
|
||||
cricket::PORTALLOCATOR_DISABLE_RELAY |
|
||||
cricket::PORTALLOCATOR_DISABLE_TCP |
|
||||
cricket::PORTALLOCATOR_DISABLE_COSTLY_NETWORKS);
|
||||
EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
|
||||
session_->StartGettingPorts();
|
||||
EXPECT_TRUE_SIMULATED_WAIT(candidate_allocation_done_,
|
||||
kDefaultAllocationTimeout, fake_clock);
|
||||
// Make sure we got only wifi candidates.
|
||||
EXPECT_EQ(2U, candidates_.size());
|
||||
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", wifi);
|
||||
EXPECT_PRED4(HasCandidate, candidates_, "local", "udp", wifi_link_local);
|
||||
}
|
||||
|
||||
// Test that no more than allocator.max_ipv6_networks() IPv6 networks are used
|
||||
// to gather candidates.
|
||||
TEST_F(BasicPortAllocatorTest, MaxIpv6NetworksLimitEnforced) {
|
||||
|
@ -303,6 +303,21 @@ bool IPIsLoopback(const IPAddress& ip) {
|
||||
return false;
|
||||
}
|
||||
|
||||
bool IPIsLinkLocal(const IPAddress& ip) {
|
||||
switch (ip.family()) {
|
||||
case AF_INET: {
|
||||
uint32_t ip_in_host_order = ip.v4AddressAsHostOrderInteger();
|
||||
return (ip_in_host_order >> 16) == ((169 << 8) | 254);
|
||||
}
|
||||
case AF_INET6: {
|
||||
// Can't use the helper because the prefix is 10 bits.
|
||||
in6_addr addr = ip.ipv6_address();
|
||||
return addr.s6_addr[0] == 0xFE && addr.s6_addr[1] == 0x80;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
bool IPIsPrivate(const IPAddress& ip) {
|
||||
switch (ip.family()) {
|
||||
case AF_INET: {
|
||||
@ -440,12 +455,6 @@ bool IPIs6To4(const IPAddress& ip) {
|
||||
return IPIsHelper(ip, k6To4Prefix, 16);
|
||||
}
|
||||
|
||||
bool IPIsLinkLocal(const IPAddress& ip) {
|
||||
// Can't use the helper because the prefix is 10 bits.
|
||||
in6_addr addr = ip.ipv6_address();
|
||||
return addr.s6_addr[0] == 0xFE && addr.s6_addr[1] == 0x80;
|
||||
}
|
||||
|
||||
// According to http://www.ietf.org/rfc/rfc2373.txt, Appendix A, page 19. An
|
||||
// address which contains MAC will have its 11th and 12th bytes as FF:FE as well
|
||||
// as the U/L bit as 1.
|
||||
|
@ -154,6 +154,7 @@ bool IPFromString(const std::string& str, int flags,
|
||||
InterfaceAddress* out);
|
||||
bool IPIsAny(const IPAddress& ip);
|
||||
bool IPIsLoopback(const IPAddress& ip);
|
||||
bool IPIsLinkLocal(const IPAddress& ip);
|
||||
bool IPIsPrivate(const IPAddress& ip);
|
||||
bool IPIsUnspec(const IPAddress& ip);
|
||||
size_t HashIP(const IPAddress& ip);
|
||||
@ -161,7 +162,6 @@ size_t HashIP(const IPAddress& ip);
|
||||
// These are only really applicable for IPv6 addresses.
|
||||
bool IPIs6Bone(const IPAddress& ip);
|
||||
bool IPIs6To4(const IPAddress& ip);
|
||||
bool IPIsLinkLocal(const IPAddress& ip);
|
||||
bool IPIsMacBased(const IPAddress& ip);
|
||||
bool IPIsSiteLocal(const IPAddress& ip);
|
||||
bool IPIsTeredo(const IPAddress& ip);
|
||||
|
Reference in New Issue
Block a user