From 032f9ca0bc8d8f381e2b8336a42286c7ae3c1478 Mon Sep 17 00:00:00 2001 From: Michael Froman Date: Thu, 16 Mar 2017 12:06:09 -0500 Subject: [PATCH] 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 | 7 + .../third_party/nICEr/src/ice/ice_ctx.c | 129 +++++++++++------- .../third_party/nICEr/src/ice/ice_ctx.h | 1 + .../third_party/nICEr/src/net/local_addr.c | 11 +- .../nICEr/src/net/transport_addr.c | 22 ++- .../third_party/nICEr/src/stun/addrs.c | 8 +- .../third_party/nICEr/src/stun/addrs.h | 2 +- .../third_party/nICEr/src/stun/stun_util.c | 55 +++++--- .../third_party/nICEr/src/stun/stun_util.h | 3 + 9 files changed, 158 insertions(+), 80 deletions(-) diff --git a/media/mtransport/test/stunserver.cpp b/media/mtransport/test/stunserver.cpp index 7e75fb847e0a..991e45228fe2 100644 --- a/media/mtransport/test/stunserver.cpp +++ b/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; diff --git a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c index b4ae8c8ea3cb..386dd47d912a 100644 --- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.c +++ b/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;iforce_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; iforce_net_interface[0] && addr_ct) { + /* Limit us to only addresses on a single interface */ + int force_addr_ct = 0; + for(i=0;iforce_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;iinterface_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;iinterface_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); diff --git a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h b/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h index b2edf3c42730..1ac2386e3227 100644 --- a/media/mtransport/third_party/nICEr/src/ice/ice_ctx.h +++ b/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); diff --git a/media/mtransport/third_party/nICEr/src/net/local_addr.c b/media/mtransport/third_party/nICEr/src/net/local_addr.c index c251f2215d56..448f25a99167 100644 --- a/media/mtransport/third_party/nICEr/src/net/local_addr.c +++ b/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) diff --git a/media/mtransport/third_party/nICEr/src/net/transport_addr.c b/media/mtransport/third_party/nICEr/src/net/transport_addr.c index 045e7b8f5701..99564a08e8b9 100644 --- a/media/mtransport/third_party/nICEr/src/net/transport_addr.c +++ b/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) diff --git a/media/mtransport/third_party/nICEr/src/stun/addrs.c b/media/mtransport/third_party/nICEr/src/stun/addrs.c index c90191f73d4e..4316c3d063c9 100644 --- a/media/mtransport/third_party/nICEr/src/stun/addrs.c +++ b/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; } diff --git a/media/mtransport/third_party/nICEr/src/stun/addrs.h b/media/mtransport/third_party/nICEr/src/stun/addrs.h index 61a3496d13b6..e4a14f5e9f01 100644 --- a/media/mtransport/third_party/nICEr/src/stun/addrs.h +++ b/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 diff --git a/media/mtransport/third_party/nICEr/src/stun/stun_util.c b/media/mtransport/third_party/nICEr/src/stun/stun_util.c index e347e542f482..a5f79bea5c8d 100644 --- a/media/mtransport/third_party/nICEr/src/stun/stun_util.c +++ b/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; diff --git a/media/mtransport/third_party/nICEr/src/stun/stun_util.h b/media/mtransport/third_party/nICEr/src/stun/stun_util.h index c57e84d12fce..627b1d1b139e 100644 --- a/media/mtransport/third_party/nICEr/src/stun/stun_util.h +++ b/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);