Bug 1562386 - Sort network attributes before hashing them to get a networkID r=mayhemer,michal,valentin
authorJeremy Lempereur <jeremy.lempereur@gmail.com>
Tue, 10 Sep 2019 11:02:25 +0000
changeset 557136 8a98e2a91bdc0cd3513843cd5ac185a72edb14e4
parent 557135 81641e8ed75ca797b483ce3baefcec7e0060ab7d
child 557137 6d54a8115393018d4e2b51c6c106cdbb9b369e50
push id2195
push userffxbld-merge
push dateMon, 25 Nov 2019 12:02:33 +0000
treeherdermozilla-release@19adee6f7bb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer, michal, valentin
bugs1562386
milestone71.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1562386 - Sort network attributes before hashing them to get a networkID r=mayhemer,michal,valentin Differential Revision: https://phabricator.services.mozilla.com/D38418
netwerk/system/mac/nsNetworkLinkService.mm
netwerk/system/win32/nsNotifyAddrListener.cpp
--- a/netwerk/system/mac/nsNetworkLinkService.mm
+++ b/netwerk/system/mac/nsNetworkLinkService.mm
@@ -1,12 +1,15 @@
 /* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+#include <numeric>
+#include <vector>
+#include <algorithm>
 
 #include <sys/socket.h>
 #include <sys/sysctl.h>
 
 #include <net/if.h>
 #include <net/if_dl.h>
 #include <net/if_types.h>
 #include <net/route.h>
@@ -195,16 +198,22 @@ static bool scanArp(char* ip, char* mac,
     if (matchIp(sdl, sin2, ip, mac, maclen)) {
       return true;
     }
   }
 
   return false;
 }
 
+/*
+ * Fetch the routing table and only return the first gateway,
+ * Which is the default gateway.
+ *
+ * Returns 0 if the default gateway's IP has been found.
+ */
 static int routingTable(char* gw, size_t aGwLen) {
   size_t needed;
   int mib[6];
   struct rt_msghdr* rtm;
   struct sockaddr* sa;
   struct sockaddr_in* sockin;
 
   mib[0] = CTL_NET;
@@ -218,16 +227,18 @@ static int routingTable(char* gw, size_t
   }
 
   UniquePtr<char[]> buf(new char[needed]);
 
   if (sysctl(mib, 6, &buf[0], &needed, nullptr, 0) < 0) {
     return 3;
   }
 
+  // There's no need to iterate over the routing table
+  // We're only looking for the first (default) gateway
   rtm = reinterpret_cast<struct rt_msghdr*>(&buf[0]);
   sa = reinterpret_cast<struct sockaddr*>(rtm + 1);
   sa = reinterpret_cast<struct sockaddr*>(SA_SIZE(sa) + (char*)sa);
   sockin = reinterpret_cast<struct sockaddr_in*>(sa);
   inet_ntop(AF_INET, &sockin->sin_addr.s_addr, gw, aGwLen - 1);
 
   return 0;
 }
@@ -248,24 +259,20 @@ static bool ipv4NetworkId(SHA1Sum* sha1)
       sha1->update(mac, strlen(mac));
       return true;
     }
   }
   return false;
 }
 
 static bool ipv6NetworkId(SHA1Sum* sha1) {
-  const int kMaxPrefixes = 8;
   struct ifaddrs* ifap;
-  struct in6_addr prefixStore[kMaxPrefixes];
-  struct in6_addr netmaskStore[kMaxPrefixes];
-  int prefixCount = 0;
+  using prefix_and_netmask = std::pair<in6_addr, in6_addr>;
+  std::vector<prefix_and_netmask> prefixAndNetmaskStore;
 
-  memset(prefixStore, 0, sizeof(prefixStore));
-  memset(netmaskStore, 0, sizeof(netmaskStore));
   if (!getifaddrs(&ifap)) {
     struct ifaddrs* ifa;
     for (ifa = ifap; ifa; ifa = ifa->ifa_next) {
       if (ifa->ifa_addr == NULL) {
         continue;
       }
       if ((AF_INET6 == ifa->ifa_addr->sa_family) &&
           !(ifa->ifa_flags & (IFF_POINTOPOINT | IFF_LOOPBACK))) {
@@ -278,51 +285,54 @@ static bool ipv6NetworkId(SHA1Sum* sha1)
             struct in6_addr prefix;
             memset(&prefix, 0, sizeof(prefix));
             // Get the prefix by combining the address and netmask.
             for (size_t i = 0; i < sizeof(prefix); ++i) {
               prefix.s6_addr[i] =
                   sin_addr->sin6_addr.s6_addr[i] & sin_netmask->sin6_addr.s6_addr[i];
             }
 
-            int match = 0;
-            // check if prefix was already found
-            for (int i = 0; i < prefixCount; i++) {
-              if (!memcmp(&prefixStore[i], &prefix, sizeof(prefix)) &&
-                  !memcmp(&netmaskStore[i], &sin_netmask->sin6_addr,
-                          sizeof(sin_netmask->sin6_addr))) {
-                // a match
-                match = 1;
-                break;
-              }
-            }
-            if (match) {
-              // prefix already found
+            // check if prefix and netmask was already found
+            auto prefixAndNetmask = std::make_pair(prefix, sin_netmask->sin6_addr);
+            auto foundPosition = std::find_if(
+                prefixAndNetmaskStore.begin(), prefixAndNetmaskStore.end(),
+                [&prefixAndNetmask](prefix_and_netmask current) {
+                  return memcmp(&prefixAndNetmask.first, &current.first, sizeof(in6_addr)) == 0 &&
+                         memcmp(&prefixAndNetmask.second, &current.second, sizeof(in6_addr)) == 0;
+                });
+            if (foundPosition != prefixAndNetmaskStore.end()) {
               continue;
             }
-            memcpy(&prefixStore[prefixCount], &prefix, sizeof(prefix));
-            memcpy(&netmaskStore[prefixCount], &sin_netmask->sin6_addr,
-                   sizeof(sin_netmask->sin6_addr));
-            prefixCount++;
-            if (prefixCount == kMaxPrefixes) {
-              // reach maximum number of prefixes
-              break;
-            }
+            prefixAndNetmaskStore.push_back(prefixAndNetmask);
           }
         }
       }
     }
     freeifaddrs(ifap);
   }
-  if (!prefixCount) {
+  if (prefixAndNetmaskStore.empty()) {
     return false;
   }
-  for (int i = 0; i < prefixCount; i++) {
-    sha1->update(&prefixStore[i], sizeof(prefixStore[i]));
-    sha1->update(&netmaskStore[i], sizeof(netmaskStore[i]));
+
+  // getifaddrs does not guarantee the interfaces will always be in the same order.
+  // We want to make sure the hash remains consistent Regardless of the interface order.
+  std::sort(prefixAndNetmaskStore.begin(), prefixAndNetmaskStore.end(),
+            [](prefix_and_netmask a, prefix_and_netmask b) {
+              // compare prefixStore
+              int comparedPrefix = memcmp(&a.first, &b.first, sizeof(in6_addr));
+              if (comparedPrefix == 0) {
+                // compare netmaskStore
+                return memcmp(&a.second, &b.second, sizeof(in6_addr)) < 0;
+              }
+              return comparedPrefix < 0;
+            });
+
+  for (const auto& prefixAndNetmask : prefixAndNetmaskStore) {
+    sha1->update(&prefixAndNetmask.first, sizeof(in6_addr));
+    sha1->update(&prefixAndNetmask.second, sizeof(in6_addr));
   }
   return true;
 }
 
 void nsNetworkLinkService::calculateNetworkId(void) {
   MOZ_ASSERT(NS_IsMainThread());
 
   nsCOMPtr<nsIEventTarget> target = do_GetService(NS_STREAMTRANSPORTSERVICE_CONTRACTID);
--- a/netwerk/system/win32/nsNotifyAddrListener.cpp
+++ b/netwerk/system/win32/nsNotifyAddrListener.cpp
@@ -3,16 +3,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 // We define this to make our use of inet_ntoa() pass. The "proper" function
 // inet_ntop() doesn't exist on Windows XP.
 #define _WINSOCK_DEPRECATED_NO_WARNINGS
 
+#include <algorithm>
+#include <vector>
+
 #include <stdarg.h>
 #include <windef.h>
 #include <winbase.h>
 #include <wingdi.h>
 #include <winuser.h>
 #include <ole2.h>
 #include <netcon.h>
 #include <objbase.h>
@@ -179,30 +182,49 @@ void nsNotifyAddrListener::calculateNetw
   RefPtr<IEnumNetworks> enumNetworks;
   hr = nlm->GetNetworks(NLM_ENUM_NETWORK_CONNECTED,
                         getter_AddRefs(enumNetworks));
   if (NS_WARN_IF(FAILED(hr))) {
     LOG(("GetNetworks error: %X", hr));
     return;
   }
 
+  // We will hash the found network ids
+  // for privacy reasons
   SHA1Sum sha1;
   uint32_t networkCount = 0;
+
+  // The networks stored in enumNetworks
+  // are not ordered. We will sort them
+  // To keep a consistent hash
+  // regardless of the found networks order.
+  std::vector<GUID> nwGUIDS;
+
+  // Consume the found networks iterator
   while (true) {
     RefPtr<INetwork> network;
     hr = enumNetworks->Next(1, getter_AddRefs(network), nullptr);
     if (hr != S_OK) {
       break;
     }
 
     GUID nwGUID;
     hr = network->GetNetworkId(&nwGUID);
     if (hr != S_OK) {
       continue;
     }
+    nwGUIDS.push_back(nwGUID);
+  }
+
+  std::sort(nwGUIDS.begin(), nwGUIDS.end(), [](const GUID& a, const GUID& b) {
+    return memcmp(&a, &b, sizeof(GUID)) < 0;
+  });
+
+  // Hash the sorted network ids
+  for (const GUID& nwGUID : nwGUIDS) {
     networkCount++;
     sha1.update(&nwGUID, sizeof(nwGUID));
 
     nsPrintfCString guid("%08lX%04X%04X%02X%02X%02X%02X%02X%02X%02X%02X%lX",
                          nwGUID.Data1, nwGUID.Data2, nwGUID.Data3,
                          nwGUID.Data4[0], nwGUID.Data4[1], nwGUID.Data4[2],
                          nwGUID.Data4[3], nwGUID.Data4[4], nwGUID.Data4[5],
                          nwGUID.Data4[6], nwGUID.Data4[7]);