Bug 1309585: use default address dicovery as backup for interface iteration. r=bwc draft
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Mon, 24 Oct 2016 15:55:02 -0700
changeset 429828 418bb84e925eefe5ae6b687277a05ec2acd273de
parent 428476 215f9686117673a2c914ed207bc7da9bb8d741ad
child 535066 48e8aa70881b73dc38c0a9ef26b24eb23d9c536e
push id33673
push userdrno@ohlmeier.org
push dateWed, 26 Oct 2016 17:18:37 +0000
reviewersbwc
bugs1309585
milestone52.0a1
Bug 1309585: use default address dicovery as backup for interface iteration. r=bwc MozReview-Commit-ID: ds9rDlddi9
media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
media/mtransport/third_party/nICEr/src/net/local_addr.c
media/mtransport/third_party/nICEr/src/stun/addrs.c
media/mtransport/third_party/nICEr/src/stun/stun_util.c
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
@@ -667,26 +667,30 @@ static int nr_ice_get_default_local_addr
   {
     int r,_status;
     nr_transport_addr default_addr;
     int i;
 
     if ((r=nr_ice_get_default_address(ctx, ip_version, &default_addr)))
         ABORT(r);
 
-    for(i=0; i<addr_ct; ++i) {
+    for (i=0; i < addr_ct; ++i) {
       if (!nr_transport_addr_cmp(&default_addr, &addrs[i].addr,
                                  NR_TRANSPORT_ADDR_CMP_MODE_ADDR)) {
         if ((r=nr_local_addr_copy(addrp, &addrs[i])))
           ABORT(r);
         break;
       }
     }
-    if (i==addr_ct)
-      ABORT(R_NOT_FOUND);
+
+    if (i == addr_ct) {
+      if ((r=nr_transport_addr_copy(&addrp->addr, &default_addr)))
+        ABORT(r);
+      strlcpy(addrp->addr.ifname, "default route", sizeof(addrp->addr.ifname));
+    }
 
     _status=0;
   abort:
     return(_status);
   }
 
 static int nr_ice_get_local_addresses(nr_ice_ctx *ctx)
   {
@@ -695,47 +699,50 @@ static int nr_ice_get_local_addresses(nr
     nr_local_addr *addrs = 0;
     int i,addr_ct;
     nr_local_addr default_addrs[2];
     int default_addr_ct = 0;
 
     if (!ctx->local_addrs) {
       /* First, gather all the local addresses we have */
       if((r=nr_stun_find_local_addresses(local_addrs,MAXADDRS,&addr_ct))) {
-        r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to find local addresses",ctx->label);
-        ABORT(r);
+        r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to gather local addresses, trying default route",ctx->label);
       }
 
-      if (ctx->force_net_interface[0]) {
+      if (ctx->force_net_interface[0] && addr_ct) {
         /* Limit us to only addresses on a single interface */
         int force_addr_ct = 0;
         for(i=0;i<addr_ct;i++){
           if (!strcmp(local_addrs[i].addr.ifname, ctx->force_net_interface)) {
             // copy it down in the array, if needed
             if (i != force_addr_ct) {
               if (r=nr_local_addr_copy(&local_addrs[force_addr_ct], &local_addrs[i])) {
                 ABORT(r);
               }
             }
             force_addr_ct++;
           }
         }
         addr_ct = force_addr_ct;
       }
 
-      if (ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS) {
+      if ((!addr_ct) || (ctx->flags & NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS)) {
         /* Get just the default IPv4 and IPv6 addrs */
         if(!nr_ice_get_default_local_address(ctx, NR_IPV4, local_addrs, addr_ct,
                                              &default_addrs[default_addr_ct])) {
           ++default_addr_ct;
         }
         if(!nr_ice_get_default_local_address(ctx, NR_IPV6, local_addrs, addr_ct,
                                              &default_addrs[default_addr_ct])) {
           ++default_addr_ct;
         }
+        if (!default_addr_ct) {
+          r_log(LOG_ICE,LOG_ERR,"ICE(%s): failed to find default addresses",ctx->label);
+          ABORT(R_FAILED);
+        }
         addrs = default_addrs;
         addr_ct = default_addr_ct;
       }
       else {
         addrs = local_addrs;
       }
 
       /* Sort interfaces by preference */
--- a/media/mtransport/third_party/nICEr/src/net/local_addr.c
+++ b/media/mtransport/third_party/nICEr/src/net/local_addr.c
@@ -51,10 +51,11 @@ int nr_local_addr_fmt_info_string(nr_loc
 
     const char *type = (addr_type & NR_INTERFACE_TYPE_WIRED) ? "wired" :
                        (addr_type & NR_INTERFACE_TYPE_WIFI) ? "wifi" :
                        (addr_type & NR_INTERFACE_TYPE_MOBILE) ? "mobile" :
                        "unknown";
 
     snprintf(buf, len, "%s%s, estimated speed: %d kbps",
              vpn, type, addr->interface.estimated_speed);
+    buf[len - 1] = '\0';
     return (0);
   }
--- a/media/mtransport/third_party/nICEr/src/stun/addrs.c
+++ b/media/mtransport/third_party/nICEr/src/stun/addrs.c
@@ -144,51 +144,52 @@ abort:
       if (my_fn) free(my_fn);
     }
     return(_status);
 }
 
 static int
 stun_get_win32_addrs(nr_local_addr addrs[], int maxaddrs, int *count)
 {
-    int r,_status;
+    int r, _status;
     PIP_ADAPTER_ADDRESSES AdapterAddresses = NULL, tmpAddress = NULL;
-    ULONG buflen;
+    // recomended per https://msdn.microsoft.com/en-us/library/windows/desktop/aa365915(v=vs.85).aspx
+    static const ULONG initialBufLen = 15000;
+    ULONG buflen = initialBufLen;
     char bin_hashed_ifname[NR_MD5_HASH_LENGTH];
     char hex_hashed_ifname[MAXIFNAME];
     int n = 0;
 
     *count = 0;
 
     if (maxaddrs <= 0)
-      ABORT(R_INTERNAL);
+      ABORT(R_BAD_ARGS);
 
-    /* Call GetAdaptersAddresses() twice.  First, just to get the buf length */
+    /* According to MSDN (see above) we have try GetAdapterAddresses() multiple times */
+    for (n = 0; n < 5; n++) {
+      AdapterAddresses = (PIP_ADAPTER_ADDRESSES) RMALLOC(buflen);
+      if (AdapterAddresses == NULL) {
+        r_log(NR_LOG_STUN, LOG_ERR, "Error allocating buf for GetAdaptersAddresses()");
+        ABORT(R_NO_MEMORY);
+      }
 
-    buflen = 0;
+      r = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_SKIP_MULTICAST | GAA_FLAG_SKIP_DNS_SERVER, NULL, AdapterAddresses, &buflen);
+      if (r == NO_ERROR) {
+        break;
+      }
+      r_log(NR_LOG_STUN, LOG_ERR, "GetAdaptersAddresses() returned error (%d)", r);
+      RFREE(AdapterAddresses);
+    }
 
-    r = GetAdaptersAddresses(AF_UNSPEC, 0, NULL, AdapterAddresses, &buflen);
-    if (r != ERROR_BUFFER_OVERFLOW) {
-      r_log(NR_LOG_STUN, LOG_ERR, "Error getting buf len from GetAdaptersAddresses()");
+    if (n >= 5) {
+      r_log(NR_LOG_STUN, LOG_ERR, "5 failures calling GetAdaptersAddresses()");
       ABORT(R_INTERNAL);
     }
 
-    AdapterAddresses = (PIP_ADAPTER_ADDRESSES) RMALLOC(buflen);
-    if (AdapterAddresses == NULL) {
-      r_log(NR_LOG_STUN, LOG_ERR, "Error allocating buf for GetAdaptersAddresses()");
-      ABORT(R_NO_MEMORY);
-    }
-
-    /* for real, this time */
-
-    r = GetAdaptersAddresses(AF_UNSPEC, 0, NULL, AdapterAddresses, &buflen);
-    if (r != NO_ERROR) {
-      r_log(NR_LOG_STUN, LOG_ERR, "Error getting addresses from GetAdaptersAddresses()");
-      ABORT(R_INTERNAL);
-    }
+    n = 0;
 
     /* Loop through the adapters */
 
     for (tmpAddress = AdapterAddresses; tmpAddress != NULL; tmpAddress = tmpAddress->Next) {
 
       if (tmpAddress->OperStatus != IfOperStatusUp)
         continue;
 
@@ -253,17 +254,20 @@ nr_stun_is_duplicate_addr(nr_local_addr 
 
 static int
 stun_getifaddrs(nr_local_addr addrs[], int maxaddrs, int *count)
 {
   int r,_status;
   struct ifaddrs* if_addrs_head=NULL;
   struct ifaddrs* if_addr;
 
-  *count=0;
+  *count = 0;
+
+  if (maxaddrs <= 0)
+    ABORT(R_BAD_ARGS);
 
   if (getifaddrs(&if_addrs_head) == -1) {
     r_log(NR_LOG_STUN, LOG_ERR, "getifaddrs error e = %d", errno);
     ABORT(R_INTERNAL);
   }
 
   if_addr = if_addrs_head;
 
@@ -390,16 +394,17 @@ nr_stun_remove_duplicate_addrs(nr_local_
             if ((r=nr_local_addr_copy(&tmp[n], &addrs[i])))
                 ABORT(r);
             ++n;
         }
     }
 
     *count = n;
 
+    memset(addrs, 0, *count * sizeof(*addrs));
     /* copy temporary array into passed in/out array */
     for (i = 0; i < *count; ++i) {
         if ((r=nr_local_addr_copy(&addrs[i], &tmp[i])))
             ABORT(r);
     }
 
     _status = 0;
   abort:
@@ -407,30 +412,32 @@ nr_stun_remove_duplicate_addrs(nr_local_
     return _status;
 }
 
 #ifndef USE_PLATFORM_NR_STUN_GET_ADDRS
 
 int
 nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int drop_loopback, int drop_link_local, int *count)
 {
-    int _status=0;
+    int r,_status=0;
     int i;
     char typestr[100];
 
 #ifdef WIN32
     _status = stun_get_win32_addrs(addrs, maxaddrs, count);
 #else
     _status = stun_getifaddrs(addrs, maxaddrs, count);
 #endif
 
-    nr_stun_remove_duplicate_addrs(addrs, drop_loopback, drop_link_local, count);
+    if ((r=nr_stun_remove_duplicate_addrs(addrs, drop_loopback, drop_link_local, count)))
+      ABORT(r);
 
     for (i = 0; i < *count; ++i) {
-    nr_local_addr_fmt_info_string(addrs+i,typestr,sizeof(typestr));
-        r_log(NR_LOG_STUN, LOG_DEBUG, "Address %d: %s on %s, type: %s\n",
+      nr_local_addr_fmt_info_string(addrs+i,typestr,sizeof(typestr));
+      r_log(NR_LOG_STUN, LOG_DEBUG, "Address %d: %s on %s, type: %s\n",
             i,addrs[i].addr.as_string,addrs[i].addr.ifname,typestr);
     }
 
+abort:
     return _status;
 }
 
 #endif
--- a/media/mtransport/third_party/nICEr/src/stun/stun_util.c
+++ b/media/mtransport/third_party/nICEr/src/stun/stun_util.c
@@ -114,22 +114,22 @@ nr_stun_xor_mapped_address(UINT4 magicCo
   abort:
     return _status;
 }
 
 int
 nr_stun_find_local_addresses(nr_local_addr addrs[], int maxaddrs, int *count)
 {
     int r,_status;
-    NR_registry *children = 0;
+    //NR_registry *children = 0;
+
+    *count = 0;
 
     if ((r=NR_reg_get_child_count(NR_STUN_REG_PREF_ADDRESS_PRFX, (unsigned int*)count)))
-        if (r == R_NOT_FOUND)
-            *count = 0;
-        else
+        if (r != R_NOT_FOUND)
             ABORT(r);
 
     if (*count == 0) {
         char allow_loopback;
         char allow_link_local;
 
         if ((r=NR_reg_get_char(NR_STUN_REG_PREF_ALLOW_LOOPBACK_ADDRS, &allow_loopback))) {
             if (r == R_NOT_FOUND)
@@ -177,17 +177,17 @@ nr_stun_find_local_addresses(nr_local_ad
         }
     }
 #endif
 
   done:
 
      _status=0;
  abort:
-     RFREE(children);
+     //RFREE(children);
      return _status;
 }
 
 int
 nr_stun_different_transaction(UCHAR *msg, int len, nr_stun_message *req)
 {
     int _status;
     nr_stun_message_header header;