Bug 1345511 - pt 1 - nICEr changes to support stun addr gathering from main process. r=bwc draft
authorMichael Froman <mfroman@mozilla.com>
Thu, 16 Mar 2017 12:06:09 -0500
changeset 502597 c0437700ad77ee3b7f98947d3505551ca9ed43e9
parent 502579 8744e9f8eb99f1290aae81985812d57364f18708
child 502598 d5c948a804285b62c263d766b363c838889c406c
push id50334
push userbmo:mfroman@nostrum.com
push dateWed, 22 Mar 2017 03:17:49 +0000
reviewersbwc
bugs1345511
milestone55.0a1
Bug 1345511 - pt 1 - nICEr changes to support stun addr gathering from main process. r=bwc Expose a tweaked version of nr_ice_get_local_addresses to allow callers to provide pre-fetched stun addrs if they are available. By default, the normal call to nr_ice_gather calls this with no pre-fetched stun addrs (read non-e10s). In e10s, the stun addrs are discovered on the main process and provided to nr_ice_get_local_addreses. When nr_ice_gather is called from the content process the local addresses have already been gathered. In the past, nr_ice_get_local_addresses also applied policy (by removing duplicate addrs, and, based on stun prefs, removing loopback and/or link_local addrs. This functionality has been moved to nr_ice_set_local_addresses where other policy is being applied (like default route only, forcing specific interfaces, and prioritization). Because we're now serializing nr_local_addr (wrapped by NrIceStunAddr), we can't assume that certain pointer references in the source nr_local_addr are correct when calling nr_local_addr_copy. New non-pointer-arithmetic version of setting up the pointer on the copied nr_local_addr is used. Also easier to understand when walking up to it the first time. MozReview-Commit-ID: KVRFl4dfr7J
media/mtransport/test/stunserver.cpp
media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
media/mtransport/third_party/nICEr/src/net/local_addr.c
media/mtransport/third_party/nICEr/src/net/transport_addr.c
media/mtransport/third_party/nICEr/src/stun/addrs.c
media/mtransport/third_party/nICEr/src/stun/addrs.h
media/mtransport/third_party/nICEr/src/stun/stun_util.c
media/mtransport/third_party/nICEr/src/stun/stun_util.h
--- a/media/mtransport/test/stunserver.cpp
+++ b/media/mtransport/test/stunserver.cpp
@@ -263,16 +263,23 @@ int TestStunServer::Initialize(int addre
   int i;
 
   r = nr_stun_find_local_addresses(addrs, max_addrs, &addr_ct);
   if (r) {
     MOZ_MTLOG(ML_ERROR, "Couldn't retrieve addresses");
     return R_INTERNAL;
   }
 
+  // removes duplicates and, based on prefs, loopback and link_local addrs
+  r = nr_stun_filter_local_addresses(addrs, &addr_ct);
+  if (r) {
+    MOZ_MTLOG(ML_ERROR, "Couldn't filter addresses");
+    return R_INTERNAL;
+  }
+
   if (addr_ct < 1) {
     MOZ_MTLOG(ML_ERROR, "No local addresses");
     return R_INTERNAL;
   }
 
   for (i = 0; i < addr_ct; ++i) {
     if (addrs[i].addr.addr->sa_family == address_family) {
       break;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c
@@ -668,119 +668,148 @@ 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) {
+      // if default addr is found in local addrs, copy the more fully
+      // complete local addr to the output arg.  Don't need to worry
+      // about comparing ports here.
       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 default addr is not in local addrs, just copy the transport addr
+    // to output arg.
     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)
+int nr_ice_set_local_addresses(nr_ice_ctx *ctx,
+                               nr_local_addr* stun_addrs, int stun_addr_ct)
   {
     int r,_status;
     nr_local_addr local_addrs[MAXADDRS];
     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 gather local addresses, trying default route",ctx->label);
-      }
+    if (ctx->local_addrs) {
+      r_log(LOG_ICE,LOG_ERR,"ICE(%s): local addresses already set",ctx->label);
+      ABORT(R_ALREADY);
+    }
+    if (!stun_addrs || !stun_addr_ct) {
+      r_log(LOG_ICE,LOG_ERR,"ICE(%s): no stun addrs provided",ctx->label);
+      ABORT(R_BAD_ARGS);
+    }
 
-      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;
+    addr_ct = MIN(stun_addr_ct, MAXADDRS);
+    r_log(LOG_ICE, LOG_DEBUG, "ICE(%s): copy %d pre-fetched stun addrs", ctx->label, addr_ct);
+    for (i=0; i<addr_ct; ++i) {
+      if (r=nr_local_addr_copy(&local_addrs[i], &stun_addrs[i])) {
+        ABORT(r);
       }
+    }
+
+    // removes duplicates and, based on prefs, loopback and link_local addrs
+    if (r=nr_stun_filter_local_addresses(local_addrs, &addr_ct)) {
+      ABORT(r);
+    }
 
-      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);
+    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++;
         }
-        addrs = default_addrs;
-        addr_ct = default_addr_ct;
       }
-      else {
-        addrs = local_addrs;
-      }
+      addr_ct = force_addr_ct;
+    }
 
-      /* Sort interfaces by preference */
-      if(ctx->interface_prioritizer) {
-        for(i=0;i<addr_ct;i++){
-          if(r=nr_interface_prioritizer_add_interface(ctx->interface_prioritizer,addrs+i)) {
-            r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to add interface ",ctx->label);
-            ABORT(r);
-          }
-        }
-        if(r=nr_interface_prioritizer_sort_preference(ctx->interface_prioritizer)) {
-          r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to sort interface by preference",ctx->label);
+    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 */
+    if(ctx->interface_prioritizer) {
+      for(i=0;i<addr_ct;i++){
+        if(r=nr_interface_prioritizer_add_interface(ctx->interface_prioritizer,addrs+i)) {
+          r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to add interface ",ctx->label);
           ABORT(r);
         }
       }
-
-      if (r=nr_ice_ctx_set_local_addrs(ctx,addrs,addr_ct)) {
+      if(r=nr_interface_prioritizer_sort_preference(ctx->interface_prioritizer)) {
+        r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to sort interface by preference",ctx->label);
         ABORT(r);
       }
     }
 
+    if (r=nr_ice_ctx_set_local_addrs(ctx,addrs,addr_ct)) {
+      ABORT(r);
+    }
+
     _status=0;
   abort:
     return(_status);
   }
 
 int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg)
   {
     int r,_status;
     nr_ice_media_stream *stream;
+    nr_local_addr stun_addrs[MAXADDRS];
+    int stun_addr_ct;
 
-    if ((r=nr_ice_get_local_addresses(ctx)))
-      ABORT(r);
+    if (!ctx->local_addrs) {
+      if((r=nr_stun_find_local_addresses(stun_addrs,MAXADDRS,&stun_addr_ct))) {
+        ABORT(r);
+      }
+      if((r=nr_ice_set_local_addresses(ctx,stun_addrs,stun_addr_ct))) {
+        ABORT(r);
+      }
+    }
 
     if(STAILQ_EMPTY(&ctx->streams)) {
       r_log(LOG_ICE,LOG_ERR,"ICE(%s): Missing streams to initialize",ctx->label);
       ABORT(R_BAD_ARGS);
     }
 
     r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): Initializing candidates",ctx->label);
     ctx->done_cb=done_cb;
--- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
+++ b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h
@@ -172,16 +172,17 @@ int nr_ice_ctx_create_with_credentials(c
 #define NR_ICE_CTX_FLAGS_RELAY_ONLY                        (1<<2)
 #define NR_ICE_CTX_FLAGS_HIDE_HOST_CANDIDATES              (1<<3)
 #define NR_ICE_CTX_FLAGS_ONLY_DEFAULT_ADDRS                (1<<4)
 #define NR_ICE_CTX_FLAGS_ONLY_PROXY                        (1<<5)
 
 void nr_ice_ctx_add_flags(nr_ice_ctx *ctx, UINT4 flags);
 void nr_ice_ctx_remove_flags(nr_ice_ctx *ctx, UINT4 flags);
 int nr_ice_ctx_destroy(nr_ice_ctx **ctxp);
+int nr_ice_set_local_addresses(nr_ice_ctx *ctx, nr_local_addr* stun_addrs, int stun_addr_ct);
 int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg);
 int nr_ice_add_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand);
 void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg);
 int nr_ice_add_media_stream(nr_ice_ctx *ctx,char *label,int components, nr_ice_media_stream **streamp);
 int nr_ice_remove_media_stream(nr_ice_ctx *ctx,nr_ice_media_stream **streamp);
 int nr_ice_get_global_attributes(nr_ice_ctx *ctx,char ***attrsp, int *attrctp);
 int nr_ice_ctx_deliver_packet(nr_ice_ctx *ctx, nr_ice_component *comp, nr_transport_addr *source_addr, UCHAR *data, int len);
 int nr_ice_ctx_is_known_id(nr_ice_ctx *ctx, UCHAR id[12]);
--- a/media/mtransport/third_party/nICEr/src/net/local_addr.c
+++ b/media/mtransport/third_party/nICEr/src/net/local_addr.c
@@ -34,19 +34,26 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 */
 
 #include <nr_api.h>
 #include <string.h>
 #include "local_addr.h"
 
 int nr_local_addr_copy(nr_local_addr *to, nr_local_addr *from)
   {
-    nr_transport_addr_copy(&(to->addr), &(from->addr));
+    int r,_status;
+
+    if (r=nr_transport_addr_copy(&(to->addr), &(from->addr))) {
+      ABORT(r);
+    }
     to->interface = from->interface;
-    return(0);
+
+    _status=0;
+  abort:
+    return(_status);
   }
 
 int nr_local_addr_fmt_info_string(nr_local_addr *addr, char *buf, int len)
   {
     int addr_type = addr->interface.type;
     const char *vpn = (addr_type & NR_INTERFACE_TYPE_VPN) ? "VPN on " : "";
 
     const char *type = (addr_type & NR_INTERFACE_TYPE_WIRED) ? "wired" :
--- a/media/mtransport/third_party/nICEr/src/net/transport_addr.c
+++ b/media/mtransport/third_party/nICEr/src/net/transport_addr.c
@@ -160,20 +160,36 @@ int nr_sockaddr_to_transport_addr(struct
     _status=0;
   abort:
     return(_status);
   }
 
 
 int nr_transport_addr_copy(nr_transport_addr *to, nr_transport_addr *from)
   {
+    int _status;
+
     memcpy(to,from,sizeof(nr_transport_addr));
-    to->addr=(struct sockaddr *)((char *)to + ((char *)from->addr - (char *)from));
 
-    return(0);
+    // with IPC serialization, we should not assume that the pointer
+    // in from->addr is correct
+    switch (to->ip_version) {
+      case NR_IPV4:
+        to->addr=(struct sockaddr *)&to->u.addr4;
+        break;
+      case NR_IPV6:
+        to->addr=(struct sockaddr *)&to->u.addr6;
+        break;
+      default:
+        ABORT(R_BAD_ARGS);
+    }
+
+    _status=0;
+  abort:
+    return(_status);
   }
 
 int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, nr_transport_addr *from)
   {
     int r,_status;
     char save_ifname[MAXIFNAME];
 
     strncpy(save_ifname, to->ifname, MAXIFNAME);
--- a/media/mtransport/third_party/nICEr/src/stun/addrs.c
+++ b/media/mtransport/third_party/nICEr/src/stun/addrs.c
@@ -411,34 +411,30 @@ nr_stun_remove_duplicate_addrs(nr_local_
   abort:
     RFREE(tmp);
     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)
+nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int *count)
 {
-    int r,_status=0;
+    int _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
 
-    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",
             i,addrs[i].addr.as_string,addrs[i].addr.ifname,typestr);
     }
 
-abort:
     return _status;
 }
 
 #endif
--- a/media/mtransport/third_party/nICEr/src/stun/addrs.h
+++ b/media/mtransport/third_party/nICEr/src/stun/addrs.h
@@ -32,12 +32,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 
 
 #ifndef _addrs_h_
 #define _addrs_h_
 
 #include "transport_addr.h"
 #include "local_addr.h"
 
-int nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int remove_loopback, int remove_link_local, int *count);
+int nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int *count);
 int nr_stun_remove_duplicate_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link_local, int *count);
 
 #endif
--- a/media/mtransport/third_party/nICEr/src/stun/stun_util.c
+++ b/media/mtransport/third_party/nICEr/src/stun/stun_util.c
@@ -111,46 +111,65 @@ nr_stun_xor_mapped_address(UINT4 magicCo
     }
 
     _status = 0;
   abort:
     return _status;
 }
 
 int
+nr_stun_filter_local_addresses(nr_local_addr addrs[], int *count)
+{
+    int r,_status;
+    char allow_loopback = 0;
+    char allow_link_local = 0;
+
+    if ((r=NR_reg_get_char(NR_STUN_REG_PREF_ALLOW_LOOPBACK_ADDRS,
+                           &allow_loopback))) {
+        if (r != R_NOT_FOUND) {
+            ABORT(r);
+        }
+    }
+
+    if ((r=NR_reg_get_char(NR_STUN_REG_PREF_ALLOW_LINK_LOCAL_ADDRS,
+                           &allow_link_local))) {
+        if (r != R_NOT_FOUND) {
+            ABORT(r);
+        }
+    }
+
+    if ((r=nr_stun_remove_duplicate_addrs(addrs,
+                                          !allow_loopback,
+                                          !allow_link_local,
+                                          count))) {
+        ABORT(r);
+    }
+
+    _status=0;
+ abort:
+    return _status;
+}
+
+int
 nr_stun_find_local_addresses(nr_local_addr addrs[], int maxaddrs, int *count)
 {
     int r,_status;
     //NR_registry *children = 0;
 
     *count = 0;
 
+#if 0
+    // this really goes with the code commented out below. (mjf)
     if ((r=NR_reg_get_child_count(NR_STUN_REG_PREF_ADDRESS_PRFX, (unsigned int*)count)))
         if (r != R_NOT_FOUND)
             ABORT(r);
+#endif
 
     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)
-                allow_loopback = 0;
-            else
-                ABORT(r);
-        }
-
-        if ((r=NR_reg_get_char(NR_STUN_REG_PREF_ALLOW_LINK_LOCAL_ADDRS, &allow_link_local))) {
-            if (r == R_NOT_FOUND)
-                allow_link_local = 0;
-            else
-                ABORT(r);
-        }
-
-        if ((r=nr_stun_get_addrs(addrs, maxaddrs, !allow_loopback, !allow_link_local, count)))
+        if ((r=nr_stun_get_addrs(addrs, maxaddrs, count)))
             ABORT(r);
 
         goto done;
     }
 
     if (*count >= maxaddrs) {
         r_log(NR_LOG_STUN, LOG_INFO, "Address list truncated from %d to %d", *count, maxaddrs);
        *count = maxaddrs;
--- a/media/mtransport/third_party/nICEr/src/stun/stun_util.h
+++ b/media/mtransport/third_party/nICEr/src/stun/stun_util.h
@@ -39,16 +39,19 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 #include "local_addr.h"
 
 extern int NR_LOG_STUN;
 
 int nr_stun_startup(void);
 
 int nr_stun_xor_mapped_address(UINT4 magicCookie, UINT12 transactionId, nr_transport_addr *from, nr_transport_addr *to);
 
+// removes duplicates, and based on prefs, loopback and link_local addresses
+int nr_stun_filter_local_addresses(nr_local_addr addrs[], int *count);
+
 int nr_stun_find_local_addresses(nr_local_addr addrs[], int maxaddrs, int *count);
 
 int nr_stun_different_transaction(UCHAR *msg, int len, nr_stun_message *req);
 
 char* nr_stun_msg_type(int type);
 
 int nr_random_alphanum(char *alphanum, int size);