From 07dcf60ee0ccbaaeefba0ca77d88c00e3442f7c3 Mon Sep 17 00:00:00 2001 From: "aluebs@webrtc.org" Date: Fri, 27 Feb 2015 18:42:22 +0000 Subject: [PATCH] Revert 8532 "Ensure only temporary IPv6 address is selected as t..." > Ensure only temporary IPv6 address is selected as the best IP. > > The current logic of IPv6 selection could still have a small chance for non-temporary address to be selected for candidate. The scenario is that when there is no non-deprecated temporary IP, the global ones could be selected. > > Global ones don't necessarily carry MAC. However, instead of comparing whether it has the MAC in it (sometimes 5 out of 6 elements from a MAC are the same, only one diffs), we should just err on the safe side. > > BUG=4348 > R=juberti@webrtc.org, pthatcher@webrtc.org > > Review URL: https://webrtc-codereview.appspot.com/38289004 TBR=guoweis@webrtc.org Review URL: https://webrtc-codereview.appspot.com/38319004 Cr-Commit-Position: refs/heads/master@{#8534} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8534 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/base/network.cc | 28 +++++++++++++++++++++------- webrtc/base/network.h | 21 ++++++++++++++++++--- webrtc/base/network_unittest.cc | 12 +++++------- 3 files changed, 44 insertions(+), 17 deletions(-) diff --git a/webrtc/base/network.cc b/webrtc/base/network.cc index ad1b47d25..bbebe8a36 100644 --- a/webrtc/base/network.cc +++ b/webrtc/base/network.cc @@ -719,8 +719,7 @@ bool Network::SetIPs(const std::vector& ips, bool changed) { return changed; } -// Select the best IP address to use from this Network. For IPv6 addresses, we -// only allow temporary IPv6 addresses to be selected to prevent MAC tracking. +// Select the best IP address to use from this Network. IPAddress Network::GetBestIP() const { if (ips_.size() == 0) { return IPAddress(); @@ -730,17 +729,32 @@ IPAddress Network::GetBestIP() const { return static_cast(ips_.at(0)); } + InterfaceAddress selected_ip, ula_ip; + for (const InterfaceAddress& ip : ips_) { - // Ignore all deprecated and non-temporary addresses. - if ((ip.ipv6_flags() & IPV6_ADDRESS_FLAG_DEPRECATED) || - !(ip.ipv6_flags() & IPV6_ADDRESS_FLAG_TEMPORARY)) { + // Ignore any address which has been deprecated already. + if (ip.ipv6_flags() & IPV6_ADDRESS_FLAG_DEPRECATED) + continue; + + // ULA address should only be returned when we have no other + // global IP. + if (IPIsULA(static_cast(ip))) { + ula_ip = ip; continue; } + selected_ip = ip; - return static_cast(ip); + // Search could stop once a temporary non-deprecated one is found. + if (ip.ipv6_flags() & IPV6_ADDRESS_FLAG_TEMPORARY) + break; } - return IPAddress(); + // No proper global IPv6 address found, use ULA instead. + if (IPIsUnspec(selected_ip) && !IPIsUnspec(ula_ip)) { + selected_ip = ula_ip; + } + + return static_cast(selected_ip); } std::string Network::ToString() const { diff --git a/webrtc/base/network.h b/webrtc/base/network.h index 6f9f88177..089d86bbd 100644 --- a/webrtc/base/network.h +++ b/webrtc/base/network.h @@ -236,9 +236,24 @@ class Network { // interfaces. Key is derived from interface name and it's prefix. std::string key() const { return key_; } - // Returns the Network's current idea of the 'best' IP it has. For privacy - // reasons, in the case of IPv6, only temporary addresses will be selected. - // Additionally, the unique local address (ULA) will never be selected. + // Returns the Network's current idea of the 'best' IP it has. + // Or return an unset IP if this network has no active addresses. + // Here is the rule on how we mark the IPv6 address as ignorable for WebRTC. + // 1) return all global temporary dynamic and non-deprecrated ones. + // 2) if #1 not available, return global ones. + // 3) if #2 not available, use ULA ipv6 as last resort. (ULA stands + // for unique local address, which is not route-able in open + // internet but might be useful for a close WebRTC deployment. + + // TODO(guoweis): rule #3 actually won't happen at current + // implementation. The reason being that ULA address starting with + // 0xfc 0r 0xfd will be grouped into its own Network. The result of + // that is WebRTC will have one extra Network to generate candidates + // but the lack of rule #3 shouldn't prevent turning on IPv6 since + // ULA should only be tried in a close deployment anyway. + + // Note that when not specifying any flag, it's treated as case global + // IPv6 address IPAddress GetBestIP() const; // Keep the original function here for now. diff --git a/webrtc/base/network_unittest.cc b/webrtc/base/network_unittest.cc index 39345936d..662dc70df 100644 --- a/webrtc/base/network_unittest.cc +++ b/webrtc/base/network_unittest.cc @@ -759,20 +759,18 @@ TEST_F(NetworkTest, TestIPv6Selection) { ipv6_network.AddIP(ip); EXPECT_EQ(ipv6_network.GetBestIP(), IPAddress()); - // Add ULA one. ULA is the unique local address which starts with either 0xfc - // or 0xfd. Since it doesn't have the temporary address attribute, it'll be - // ignored. + // Add ULA one. ULA is unique local address which is starting either + // with 0xfc or 0xfd. ipstr = "fd00:fa00:4:1000:be30:5bff:fee5:c4"; ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &ip)); ipv6_network.AddIP(ip); - EXPECT_EQ(ipv6_network.GetBestIP(), IPAddress()); + EXPECT_EQ(ipv6_network.GetBestIP(), static_cast(ip)); - // Add global one. Since it doesn't have the temporary address attribute, - // it'll be ignored. + // Add global one. ipstr = "2401:fa00:4:1000:be30:5bff:fee5:c5"; ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &ip)); ipv6_network.AddIP(ip); - EXPECT_EQ(ipv6_network.GetBestIP(), IPAddress()); + EXPECT_EQ(ipv6_network.GetBestIP(), static_cast(ip)); // Add global dynamic temporary one. ipstr = "2401:fa00:4:1000:be30:5bff:fee5:c6";