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

--HG--
extra : rebase_source : c0437700ad77ee3b7f98947d3505551ca9ed43e9

media/mtransport/test/stunserver.cpp

@@ -268,6 +268,13 @@ int TestStunServer::Initialize(int address_family) {
     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;

media/mtransport/third_party/nICEr/src/ice/ice_ctx.c

@@ -673,6 +673,9 @@ static int nr_ice_get_default_local_address(nr_ice_ctx *ctx, int ip_version, nr_
         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])))
@@ -681,6 +684,8 @@ static int nr_ice_get_default_local_address(nr_ice_ctx *ctx, int ip_version, nr_
       }
     }
 
+    // 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);
@@ -692,7 +697,8 @@ static int nr_ice_get_default_local_address(nr_ice_ctx *ctx, int ip_version, nr_
     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];
@@ -701,69 +707,84 @@ static int nr_ice_get_local_addresses(nr_ice_ctx *ctx)
     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);
-              }
+    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 (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++;
           }
+          force_addr_ct++;
         }
-        addr_ct = force_addr_ct;
       }
+      addr_ct = force_addr_ct;
+    }
 
-      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;
+    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;
       }
-      else {
-        addrs = local_addrs;
+      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_interface_prioritizer_sort_preference(ctx->interface_prioritizer)) {
-          r_log(LOG_ICE,LOG_ERR,"ICE(%s): unable to sort interface by preference",ctx->label);
+    /* 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);
@@ -773,9 +794,17 @@ 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);

media/mtransport/third_party/nICEr/src/ice/ice_ctx.h

@@ -177,6 +177,7 @@ int nr_ice_ctx_create_with_credentials(char *label, UINT4 flags, char* ufrag, ch
 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);

media/mtransport/third_party/nICEr/src/net/local_addr.c

@@ -39,9 +39,16 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 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)

media/mtransport/third_party/nICEr/src/net/transport_addr.c

@@ -165,10 +165,26 @@ int nr_sockaddr_to_transport_addr(struct sockaddr *saddr, int protocol, int keep
 
 int nr_transport_addr_copy(nr_transport_addr *to, nr_transport_addr *from)
   {
-    memcpy(to,from,sizeof(nr_transport_addr));
-    to->addr=(struct sockaddr *)((char *)to + ((char *)from->addr - (char *)from));
+    int _status;
 
-    return(0);
+    memcpy(to,from,sizeof(nr_transport_addr));
+
+    // 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)

media/mtransport/third_party/nICEr/src/stun/addrs.c

@@ -416,9 +416,9 @@ nr_stun_remove_duplicate_addrs(nr_local_addr addrs[], int remove_loopback, int r
 #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];
 
@@ -428,16 +428,12 @@ nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int drop_loopback, int dr
     _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;
 }
 

media/mtransport/third_party/nICEr/src/stun/addrs.h

@@ -37,7 +37,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #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

media/mtransport/third_party/nICEr/src/stun/stun_util.c

@@ -115,6 +115,39 @@ nr_stun_xor_mapped_address(UINT4 magicCookie, UINT12 transactionId, nr_transport
     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)
 {
@@ -123,29 +156,15 @@ nr_stun_find_local_addresses(nr_local_addr addrs[], int maxaddrs, int *count)
 
     *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;

media/mtransport/third_party/nICEr/src/stun/stun_util.h

@@ -44,6 +44,9 @@ 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);