Skip to content

Commit

Permalink
Expose a link-local network interfaces enumeration option
Browse files Browse the repository at this point in the history
The bug 8432 is caused by trying to connect through a
"link-local" interface (IP address 169.254.0.x/16),
which is listed among the iPhone network interfaces.
The bug is not happening if the link-local network interfaces
are skipped in the ICE candidate gethering process.

To control this behaviour an option - disable_link_local_networks -
is added inside the RTCConfiguration.
It is used to set the new BasicPortAllocatorSession flag -
PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS.
The port allocator flag is added if the configuration option is set.

IPIsLinkLocal IPAddress function and its friends (IPIsLoopback, IPIsPrivate)
are refactored to work on both IPv4 and IPv6.
Unit test IPIsLinkLocal.

Bonus: fix a bug in IPIsLinkLocalV6:
take into account just 10 network mask bits instead of 16.

Bug: webrtc:8432
Change-Id: Ibe8f677a36098057b7fcad5c798380727b23359b
Reviewed-on: https://webrtc-review.googlesource.com/36380
Reviewed-by: Taylor Brandstetter <[email protected]>
Reviewed-by: Peter Thatcher <[email protected]>
Reviewed-by: Kári Helgason <[email protected]>
Reviewed-by: Zhi Huang <[email protected]>
Commit-Queue: Taylor Brandstetter <[email protected]>
Cr-Commit-Position: refs/heads/master@{#21922}
  • Loading branch information
danielvidexio authored and Commit Bot committed Feb 6, 2018
1 parent 5dfde18 commit 2870b0a
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 34 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ Telenor Digital AS <*@telenor.com>
Temasys Communications <*@temasys.io>
The Chromium Authors <*@chromium.org>
The WebRTC Authors <*@webrtc.org>
Videxio AS <*@videxio.com>
Vonage Holdings Corp. <*@vonage.com>
Wire Swiss GmbH <*@wire.com>
Miguel Paris <[email protected]>
Expand Down
4 changes: 4 additions & 0 deletions api/peerconnectioninterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,10 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
// Can be set to INT_MAX to effectively disable the limit.
int max_ipv6_networks = cricket::kDefaultMaxIPv6Networks;

// Exclude link-local network interfaces
// from considertaion for gathering ICE candidates.
bool disable_link_local_networks = false;

// If set to true, use RTP data channels instead of SCTP.
// TODO(deadbeef): Remove this. We no longer commit to supporting RTP data
// channels, though some applications are still working on moving off of
Expand Down
4 changes: 4 additions & 0 deletions p2p/base/portallocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ enum {
// the application to work in a wider variety of environments, at the expense
// of having to allocate additional candidates.
PORTALLOCATOR_ENABLE_ANY_ADDRESS_PORTS = 0x8000,

// Exclude link-local network interfaces
// from considertaion after adapter enumeration.
PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS = 0x10000,
};

// Defines various reasons that have caused ICE regathering.
Expand Down
8 changes: 8 additions & 0 deletions p2p/client/basicportallocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,14 @@ std::vector<rtc::Network*> BasicPortAllocatorSession::GetNetworks() {
network_manager->GetAnyAddressNetworks(&networks);
}
}
// Filter out link-local networks if needed.
if (flags() & PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS) {
networks.erase(std::remove_if(networks.begin(), networks.end(),
[](rtc::Network* network) {
return IPIsLinkLocal(network->prefix());
}),
networks.end());
}
// Do some more filtering, depending on the network ignore mask and "disable
// costly networks" flag.
networks.erase(std::remove_if(networks.begin(), networks.end(),
Expand Down
7 changes: 7 additions & 0 deletions pc/peerconnection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
bool disable_ipv6;
bool disable_ipv6_on_wifi;
int max_ipv6_networks;
bool disable_link_local_networks;
bool enable_rtp_data_channel;
rtc::Optional<int> screencast_min_bitrate;
rtc::Optional<bool> combined_audio_video_bwe;
Expand Down Expand Up @@ -628,6 +629,7 @@ bool PeerConnectionInterface::RTCConfiguration::operator==(
media_config == o.media_config && disable_ipv6 == o.disable_ipv6 &&
disable_ipv6_on_wifi == o.disable_ipv6_on_wifi &&
max_ipv6_networks == o.max_ipv6_networks &&
disable_link_local_networks == o.disable_link_local_networks &&
enable_rtp_data_channel == o.enable_rtp_data_channel &&
screencast_min_bitrate == o.screencast_min_bitrate &&
combined_audio_video_bwe == o.combined_audio_video_bwe &&
Expand Down Expand Up @@ -4240,6 +4242,11 @@ bool PeerConnection::InitializePortAllocator_n(
RTC_LOG(LS_INFO) << "Do not gather candidates on high-cost networks";
}

if (configuration.disable_link_local_networks) {
portallocator_flags |= cricket::PORTALLOCATOR_DISABLE_LINK_LOCAL_NETWORKS;
RTC_LOG(LS_INFO) << "Disable candidates on link-local network interfaces.";
}

port_allocator_->set_flags(portallocator_flags);
// No step delay is used while allocating ports.
port_allocator_->set_step_delay(cricket::kMinimumStepDelay);
Expand Down
88 changes: 57 additions & 31 deletions rtc_base/ipaddress.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ static const in6_addr k6To4Prefix = {{{0x20, 0x02, 0}}};
static const in6_addr kTeredoPrefix = {{{0x20, 0x01, 0x00, 0x00}}};
static const in6_addr kV4CompatibilityPrefix = {{{0}}};
static const in6_addr k6BonePrefix = {{{0x3f, 0xfe, 0}}};
static const in6_addr kPrivateNetworkPrefix = {{{0xFD}}};

static bool IsPrivateV4(uint32_t ip);
static bool IPIsHelper(const IPAddress& ip,
const in6_addr& tomatch, int length);
static in_addr ExtractMappedAddress(const in6_addr& addr);

uint32_t IPAddress::v4AddressAsHostOrderInteger() const {
Expand Down Expand Up @@ -223,12 +225,27 @@ std::ostream& operator<<(std::ostream& os, const InterfaceAddress& ip) {
return os;
}

bool IsPrivateV4(uint32_t ip_in_host_order) {
return ((ip_in_host_order >> 24) == 127) ||
((ip_in_host_order >> 24) == 10) ||
static bool IPIsPrivateNetworkV4(const IPAddress& ip) {
uint32_t ip_in_host_order = ip.v4AddressAsHostOrderInteger();
return ((ip_in_host_order >> 24) == 10) ||
((ip_in_host_order >> 20) == ((172 << 4) | 1)) ||
((ip_in_host_order >> 16) == ((192 << 8) | 168)) ||
((ip_in_host_order >> 16) == ((169 << 8) | 254));
((ip_in_host_order >> 16) == ((192 << 8) | 168));
}

static bool IPIsPrivateNetworkV6(const IPAddress& ip) {
return IPIsHelper(ip, kPrivateNetworkPrefix, 8);
}

bool IPIsPrivateNetwork(const IPAddress& ip) {
switch (ip.family()) {
case AF_INET: {
return IPIsPrivateNetworkV4(ip);
}
case AF_INET6: {
return IPIsPrivateNetworkV6(ip);
}
}
return false;
}

in_addr ExtractMappedAddress(const in6_addr& in6) {
Expand Down Expand Up @@ -294,43 +311,29 @@ bool IPIsAny(const IPAddress& ip) {
return false;
}

bool IPIsLoopback(const IPAddress& ip) {
switch (ip.family()) {
case AF_INET: {
return (ip.v4AddressAsHostOrderInteger() >> 24) == 127;
}
case AF_INET6: {
return ip == IPAddress(in6addr_loopback);
}
}
return false;
static bool IPIsLoopbackV4(const IPAddress& ip) {
uint32_t ip_in_host_order = ip.v4AddressAsHostOrderInteger();
return ((ip_in_host_order >> 24) == 127);
}

bool IPIsLinkLocal(const IPAddress& ip) {
static bool IPIsLoopbackV6(const IPAddress& ip) {
return ip == IPAddress(in6addr_loopback);
}

bool IPIsLoopback(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);
return IPIsLoopbackV4(ip);
}
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 IPIsLoopbackV6(ip);
}
}
return false;
}

bool IPIsPrivate(const IPAddress& ip) {
switch (ip.family()) {
case AF_INET: {
return IsPrivateV4(ip.v4AddressAsHostOrderInteger());
}
case AF_INET6: {
return IPIsLinkLocal(ip) || IPIsLoopback(ip);
}
}
return false;
return IPIsLinkLocal(ip) || IPIsLoopback(ip) || IPIsPrivateNetwork(ip);
}

bool IPIsUnspec(const IPAddress& ip) {
Expand Down Expand Up @@ -458,6 +461,29 @@ bool IPIs6To4(const IPAddress& ip) {
return IPIsHelper(ip, k6To4Prefix, 16);
}

static bool IPIsLinkLocalV4(const IPAddress& ip) {
uint32_t ip_in_host_order = ip.v4AddressAsHostOrderInteger();
return ((ip_in_host_order >> 16) == ((169 << 8) | 254));
}

static bool IPIsLinkLocalV6(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] & 0xC0) == 0x80);
}

bool IPIsLinkLocal(const IPAddress& ip) {
switch (ip.family()) {
case AF_INET: {
return IPIsLinkLocalV4(ip);
}
case AF_INET6: {
return IPIsLinkLocalV6(ip);
}
}
return false;
}

// 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.
Expand Down
5 changes: 5 additions & 0 deletions rtc_base/ipaddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ bool IPFromString(const std::string& str, int flags,
bool IPIsAny(const IPAddress& ip);
bool IPIsLoopback(const IPAddress& ip);
bool IPIsLinkLocal(const IPAddress& ip);
// Identify a private network address like "192.168.111.222"
// (see https://en.wikipedia.org/wiki/Private_network )
bool IPIsPrivateNetwork(const IPAddress& ip);
// Identify if an IP is "private", that is a loopback
// or an address belonging to a link-local or a private network.
bool IPIsPrivate(const IPAddress& ip);
bool IPIsUnspec(const IPAddress& ip);
size_t HashIP(const IPAddress& ip);
Expand Down
23 changes: 23 additions & 0 deletions rtc_base/ipaddress_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ static const unsigned int kIPv4AddrSize = 4;
static const unsigned int kIPv6AddrSize = 16;
static const unsigned int kIPv4RFC1918Addr = 0xC0A80701;
static const unsigned int kIPv4PublicAddr = 0x01020304;
static const unsigned int kIPv4LinkLocalAddr = 0xA9FE10C1; // 169.254.16.193
static const in6_addr kIPv6LinkLocalAddr = {{{0xfe, 0x80, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00,
0xbe, 0x30, 0x5b, 0xff,
Expand Down Expand Up @@ -581,6 +582,28 @@ TEST(IPAddressTest, TestIsLoopback) {
EXPECT_TRUE(IPIsLoopback(IPAddress(in6addr_loopback)));
}

TEST(IPAddressTest, TestIsLinkLocal) {
// "any" addresses
EXPECT_FALSE(IPIsLinkLocal(IPAddress(INADDR_ANY)));
EXPECT_FALSE(IPIsLinkLocal(IPAddress(in6addr_any)));
// loopback addresses
EXPECT_FALSE(IPIsLinkLocal(IPAddress(INADDR_LOOPBACK)));
EXPECT_FALSE(IPIsLinkLocal(IPAddress(in6addr_loopback)));
// public addresses
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv4PublicAddr)));
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv6PublicAddr)));
// private network addresses
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv4RFC1918Addr)));
// mapped addresses
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv4MappedAnyAddr)));
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv4MappedPublicAddr)));
EXPECT_FALSE(IPIsLinkLocal(IPAddress(kIPv4MappedRFC1918Addr)));

// link-local network addresses
EXPECT_TRUE(IPIsLinkLocal(IPAddress(kIPv4LinkLocalAddr)));
EXPECT_TRUE(IPIsLinkLocal(IPAddress(kIPv6LinkLocalAddr)));
}

// Verify that IPIsAny catches all cases of "any" address.
TEST(IPAddressTest, TestIsAny) {
IPAddress addr;
Expand Down
11 changes: 8 additions & 3 deletions sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ @implementation RTCConfiguration
@synthesize candidateNetworkPolicy = _candidateNetworkPolicy;
@synthesize continualGatheringPolicy = _continualGatheringPolicy;
@synthesize maxIPv6Networks = _maxIPv6Networks;
@synthesize disableLinkLocalNetworks = _disableLinkLocalNetworks;
@synthesize audioJitterBufferMaxPackets = _audioJitterBufferMaxPackets;
@synthesize audioJitterBufferFastAccelerate = _audioJitterBufferFastAccelerate;
@synthesize iceConnectionReceivingTimeout = _iceConnectionReceivingTimeout;
Expand Down Expand Up @@ -75,6 +76,7 @@ - (instancetype)initWithNativeConfiguration:
_continualGatheringPolicy =
[[self class] continualGatheringPolicyForNativePolicy:nativePolicy];
_maxIPv6Networks = config.max_ipv6_networks;
_disableLinkLocalNetworks = config.disable_link_local_networks;
_audioJitterBufferMaxPackets = config.audio_jitter_buffer_max_packets;
_audioJitterBufferFastAccelerate = config.audio_jitter_buffer_fast_accelerate;
_iceConnectionReceivingTimeout = config.ice_connection_receiving_timeout;
Expand All @@ -100,10 +102,11 @@ - (instancetype)initWithNativeConfiguration:
}

- (NSString *)description {
static NSString *formatString = @"RTCConfiguration: "
@"{\n%@\n%@\n%@\n%@\n%@\n%@\n%@\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%@\n%@\n%d\n%d\n}\n";

return
[NSString stringWithFormat:
@"RTCConfiguration: "
@"{\n%@\n%@\n%@\n%@\n%@\n%@\n%@\n%d\n%d\n%d\n%d\n%d\n%d\n%d\n%@\n%@\n%d\n}\n",
[NSString stringWithFormat:formatString,
_iceServers,
[[self class] stringForTransportPolicy:_iceTransportPolicy],
[[self class] stringForBundlePolicy:_bundlePolicy],
Expand All @@ -120,6 +123,7 @@ - (NSString *)description {
_shouldPresumeWritableWhenFullyRelayed,
_iceCheckMinInterval,
_iceRegatherIntervalRange,
_disableLinkLocalNetworks,
_maxIPv6Networks];
}

Expand Down Expand Up @@ -147,6 +151,7 @@ - (NSString *)description {
nativeConfig->continual_gathering_policy = [[self class]
nativeContinualGatheringPolicyForPolicy:_continualGatheringPolicy];
nativeConfig->max_ipv6_networks = _maxIPv6Networks;
nativeConfig->disable_link_local_networks = _disableLinkLocalNetworks;
nativeConfig->audio_jitter_buffer_max_packets = _audioJitterBufferMaxPackets;
nativeConfig->audio_jitter_buffer_fast_accelerate =
_audioJitterBufferFastAccelerate ? true : false;
Expand Down
6 changes: 6 additions & 0 deletions sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,12 @@ RTC_EXPORT
*/
@property(nonatomic, assign) int maxIPv6Networks;

/** Exclude link-local network interfaces
* from considertaion for gathering ICE candidates.
* Defaults to NO.
*/
@property(nonatomic, assign) BOOL disableLinkLocalNetworks;

@property(nonatomic, assign) int audioJitterBufferMaxPackets;
@property(nonatomic, assign) BOOL audioJitterBufferFastAccelerate;
@property(nonatomic, assign) int iceConnectionReceivingTimeout;
Expand Down

0 comments on commit 2870b0a

Please sign in to comment.