From 9d857bc9ae3cb113e894bdbf8bef71ae71d2888b Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Mon, 4 May 2020 12:00:26 +0000 Subject: [PATCH 01/41] Bug 1634145: Avoid leaking nr_ice_sockets due to various kinds of failure. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D73287 --- src/ice/ice_component.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index b1304f6..839e785 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -284,6 +284,11 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon if(r=nr_ice_socket_create(ctx,component,sock,NR_ICE_SOCKET_TYPE_DGRAM,&isock)) ABORT(r); + /* Make sure we don't leak this. Failures might result in it being + * unused, but we hand off references to this in enough places below + * that unwinding it all becomes impractical. */ + STAILQ_INSERT_TAIL(&component->sockets,isock,entry); + if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) { /* Create one host candidate */ if(r=nr_ice_candidate_create(ctx,component,isock,sock,HOST,0,0, @@ -387,8 +392,6 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* Create a STUN server context for this socket */ if ((r=nr_ice_component_create_stun_server_ctx(component,isock,sock,&addrs[i].addr,lufrag,pwd))) ABORT(r); - - STAILQ_INSERT_TAIL(&component->sockets,isock,entry); } _status = 0; @@ -649,6 +652,11 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon if((r=nr_ice_socket_create(ctx, component, buffered_sock, NR_ICE_SOCKET_TYPE_STREAM_TURN, &turn_isock))) ABORT(r); + /* Make sure we don't leak this. Failures might result in it being + * unused, but we hand off references to this in enough places below + * that unwinding it all becomes impractical. */ + STAILQ_INSERT_TAIL(&component->sockets,turn_isock,entry); + /* Attach ourselves to it */ if(r=nr_ice_candidate_create(ctx,component, turn_isock,turn_sock,RELAYED,TCP_TYPE_NONE, @@ -664,7 +672,6 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon if ((r=nr_ice_component_create_stun_server_ctx(component,turn_isock,local_sock,&addr,lufrag,pwd))) ABORT(r); - STAILQ_INSERT_TAIL(&component->sockets,turn_isock,entry); } #endif /* USE_TURN */ } From 7adfbbee4a95e871ffc0142191162bd2fad2df38 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 21 May 2020 19:05:07 +0000 Subject: [PATCH 02/41] Bug 1639916: Clean up pstream if we don't end up using it. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D76359 --- src/ice/ice_peer_ctx.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ice/ice_peer_ctx.c b/src/ice/ice_peer_ctx.c index a3570fa..63b9a28 100644 --- a/src/ice/ice_peer_ctx.c +++ b/src/ice/ice_peer_ctx.c @@ -43,7 +43,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "ice_reg.h" static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg); -static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct); +static void nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct); static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled, const char *mdns_addr); static void nr_ice_peer_ctx_start_trickle_timer(nr_ice_peer_ctx *pctx); @@ -115,8 +115,7 @@ int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_ pstream->local_stream=stream; pstream->pctx=pctx; - if (r=nr_ice_peer_ctx_parse_stream_attributes_int(pctx,stream,pstream,attrs,attr_ct)) - ABORT(r); + nr_ice_peer_ctx_parse_stream_attributes_int(pctx,stream,pstream,attrs,attr_ct); /* Now that we have the ufrag and password, compute all the username/password pairs */ @@ -139,13 +138,17 @@ int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_ ABORT(r); STAILQ_INSERT_TAIL(&pctx->peer_streams,pstream,entry); + pstream=0; _status=0; abort: + if (_status) { + nr_ice_media_stream_destroy(&pstream); + } return(_status); } -static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct) +static void nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct) { int r; int i; @@ -169,7 +172,6 @@ static int nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr } /* Doesn't fail because we just skip errors */ - return(0); } static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled, const char *mdns_addr) From 2ba6f4a4979270d8b06e03f142352c9636b79fab Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 26 May 2020 16:02:11 +0000 Subject: [PATCH 03/41] Bug 1183145: Rename a function to better reflect what it does, and fix a bug where handling of teredo and mac-based IPv6 was mixed up. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D76042 --- src/stun/addrs.c | 22 +++++++++++++++------- src/stun/addrs.h | 2 +- src/stun/stun_util.c | 8 ++++---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/stun/addrs.c b/src/stun/addrs.c index 74491f7..a93813c 100644 --- a/src/stun/addrs.c +++ b/src/stun/addrs.c @@ -367,26 +367,34 @@ nr_stun_is_duplicate_addr(nr_local_addr addrs[], int count, nr_local_addr *addr) } int -nr_stun_remove_duplicate_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link_local, int *count) +nr_stun_filter_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link_local, int *count) { int r, _status; nr_local_addr *tmp = 0; int i; int n; - int contains_regular_ipv6 = 0; + int filter_mac_ipv6 = 0; + int filter_teredo_ipv6 = 0; tmp = RMALLOC(*count * sizeof(*tmp)); if (!tmp) ABORT(R_NO_MEMORY); for (i = 0; i < *count; ++i) { + if (addrs[i].addr.ip_version == NR_IPV6) { if (nr_transport_addr_is_teredo(&addrs[i].addr)) { addrs[i].interface.type |= NR_INTERFACE_TYPE_TEREDO; + /* Prefer teredo over mac-based address. Probably will never see + * both. */ + filter_mac_ipv6 = 1; + } else { + filter_teredo_ipv6 = 1; } - else if (addrs[i].addr.ip_version == NR_IPV6 && - !nr_transport_addr_is_mac_based(&addrs[i].addr)) { - contains_regular_ipv6 = 1; + + if (!nr_transport_addr_is_mac_based(&addrs[i].addr)) { + filter_mac_ipv6 = 1; } + } } n = 0; @@ -401,11 +409,11 @@ nr_stun_remove_duplicate_addrs(nr_local_addr addrs[], int remove_loopback, int r nr_transport_addr_is_link_local(&addrs[i].addr)) { /* skip addrs[i], it's a link-local address */ } - else if (contains_regular_ipv6 && + else if (filter_mac_ipv6 && nr_transport_addr_is_mac_based(&addrs[i].addr)) { /* skip addrs[i], it's MAC based */ } - else if (contains_regular_ipv6 && + else if (filter_teredo_ipv6 && nr_transport_addr_is_teredo(&addrs[i].addr)) { /* skip addrs[i], it's a Teredo address */ } diff --git a/src/stun/addrs.h b/src/stun/addrs.h index e4a14f5..522bd92 100644 --- a/src/stun/addrs.h +++ b/src/stun/addrs.h @@ -38,6 +38,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "local_addr.h" 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); +int nr_stun_filter_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link_local, int *count); #endif diff --git a/src/stun/stun_util.c b/src/stun/stun_util.c index 66bea07..cbb0128 100644 --- a/src/stun/stun_util.c +++ b/src/stun/stun_util.c @@ -133,10 +133,10 @@ nr_stun_filter_local_addresses(nr_local_addr addrs[], int *count) } } - if ((r=nr_stun_remove_duplicate_addrs(addrs, - !allow_loopback, - !allow_link_local, - count))) { + if ((r=nr_stun_filter_addrs(addrs, + !allow_loopback, + !allow_link_local, + count))) { ABORT(r); } From a292f4f5ac3fb77199079a45f41ef92df032d8d1 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 26 May 2020 16:03:38 +0000 Subject: [PATCH 04/41] Bug 1183145: Add a flags field to nr_local_addr so IPv6 addresses can be marked as temporary, and filter non-temp IPv6 if temp IPv6 are available. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D76043 --- src/net/local_addr.c | 6 ++++-- src/net/local_addr.h | 2 ++ src/stun/addrs.c | 12 ++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/net/local_addr.c b/src/net/local_addr.c index 448f25a..a0896d5 100644 --- a/src/net/local_addr.c +++ b/src/net/local_addr.c @@ -45,6 +45,7 @@ int nr_local_addr_copy(nr_local_addr *to, nr_local_addr *from) ABORT(r); } to->interface = from->interface; + to->flags = from->flags; _status=0; abort: @@ -61,8 +62,9 @@ int nr_local_addr_fmt_info_string(nr_local_addr *addr, char *buf, int len) (addr_type & NR_INTERFACE_TYPE_MOBILE) ? "mobile" : "unknown"; - snprintf(buf, len, "%s%s, estimated speed: %d kbps", - vpn, type, addr->interface.estimated_speed); + snprintf(buf, len, "%s%s, estimated speed: %d kbps %s", + vpn, type, addr->interface.estimated_speed, + (addr->flags & NR_ADDR_FLAG_TEMPORARY ? "temporary" : "")); buf[len - 1] = '\0'; return (0); } diff --git a/src/net/local_addr.h b/src/net/local_addr.h index daa5ff3..058f7a5 100644 --- a/src/net/local_addr.h +++ b/src/net/local_addr.h @@ -52,6 +52,8 @@ typedef struct nr_interface_ { typedef struct nr_local_addr_ { nr_transport_addr addr; nr_interface interface; +#define NR_ADDR_FLAG_TEMPORARY 0x1 + int flags; } nr_local_addr; int nr_local_addr_copy(nr_local_addr *to, nr_local_addr *from); diff --git a/src/stun/addrs.c b/src/stun/addrs.c index a93813c..94aa107 100644 --- a/src/stun/addrs.c +++ b/src/stun/addrs.c @@ -373,8 +373,11 @@ nr_stun_filter_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link nr_local_addr *tmp = 0; int i; int n; + /* We prefer temp ipv6 for their privacy properties. If we cannot get + * that, we prefer ipv6 that are not based on mac address. */ int filter_mac_ipv6 = 0; int filter_teredo_ipv6 = 0; + int filter_non_temp_ipv6 = 0; tmp = RMALLOC(*count * sizeof(*tmp)); if (!tmp) @@ -394,6 +397,10 @@ nr_stun_filter_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link if (!nr_transport_addr_is_mac_based(&addrs[i].addr)) { filter_mac_ipv6 = 1; } + + if (addrs[i].flags & NR_ADDR_FLAG_TEMPORARY) { + filter_non_temp_ipv6 = 1; + } } } @@ -417,6 +424,11 @@ nr_stun_filter_addrs(nr_local_addr addrs[], int remove_loopback, int remove_link nr_transport_addr_is_teredo(&addrs[i].addr)) { /* skip addrs[i], it's a Teredo address */ } + else if (filter_non_temp_ipv6 && + (addrs[i].addr.ip_version == NR_IPV6) && + !(addrs[i].flags & NR_ADDR_FLAG_TEMPORARY)) { + /* skip addrs[i], it's a non-temporary ipv6, and we have a temporary */ + } else { /* otherwise, copy it to the temporary array */ if ((r=nr_local_addr_copy(&tmp[n], &addrs[i]))) From 08b6a3cb2849580a959ffe9060a8ecd238f44a0f Mon Sep 17 00:00:00 2001 From: vpoddubchak Date: Tue, 30 Mar 2021 16:28:59 +0300 Subject: [PATCH 05/41] Bug 1183145: Move platform-specific code from addrs.c into separate files. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D75947 --- src/stun/addrs-bsd.c | 68 ++++ src/stun/addrs-bsd.h | 13 + src/stun/addrs-netlink.c | 259 ++++++++++++++ .../{ifaddrs-android.h => addrs-netlink.h} | 28 +- src/stun/addrs-win32.c | 190 +++++++++++ src/stun/addrs-win32.h | 13 + src/stun/addrs.c | 318 +----------------- src/stun/ifaddrs-android.c | 242 ------------- 8 files changed, 556 insertions(+), 575 deletions(-) create mode 100644 src/stun/addrs-bsd.c create mode 100644 src/stun/addrs-bsd.h create mode 100644 src/stun/addrs-netlink.c rename src/stun/{ifaddrs-android.h => addrs-netlink.h} (68%) create mode 100644 src/stun/addrs-win32.c create mode 100644 src/stun/addrs-win32.h delete mode 100644 src/stun/ifaddrs-android.c diff --git a/src/stun/addrs-bsd.c b/src/stun/addrs-bsd.c new file mode 100644 index 0000000..763fe22 --- /dev/null +++ b/src/stun/addrs-bsd.c @@ -0,0 +1,68 @@ +/* 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/. */ + +#if defined(BSD) || defined(DARWIN) +#include "addrs-bsd.h" +#include +#include +#include +#include "util.h" +#include "stun_util.h" +#include "util.h" +#include + +#include /* getifaddrs */ +#include /* getifaddrs */ +#include +#include +#include +#include + +int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) +{ + int r,_status; + struct ifaddrs* if_addrs_head=NULL; + struct ifaddrs* if_addr; + + *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; + + while (if_addr && *count < maxaddrs) { + /* This can be null */ + if (if_addr->ifa_addr) { + switch (if_addr->ifa_addr->sa_family) { + case AF_INET: + case AF_INET6: + if (r=nr_sockaddr_to_transport_addr(if_addr->ifa_addr, IPPROTO_UDP, 0, &(addrs[*count].addr))) { + r_log(NR_LOG_STUN, LOG_ERR, "nr_sockaddr_to_transport_addr error r = %d", r); + } else { + (void)strlcpy(addrs[*count].addr.ifname, if_addr->ifa_name, sizeof(addrs[*count].addr.ifname)); + ++(*count); + } + break; + default: + ; + } + } + + if_addr = if_addr->ifa_next; + } + + _status=0; +abort: + if (if_addrs_head) { + freeifaddrs(if_addrs_head); + } + return(_status); +} +#endif diff --git a/src/stun/addrs-bsd.h b/src/stun/addrs-bsd.h new file mode 100644 index 0000000..b575586 --- /dev/null +++ b/src/stun/addrs-bsd.h @@ -0,0 +1,13 @@ +/* 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/. */ + +#ifndef STUN_IFADDRS_BSD_H_ +#define STUN_IFADDRS_BSD_H_ + +#include "local_addr.h" + +int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count); + +#endif /* STUN_IFADDRS_BSD_H_ */ + diff --git a/src/stun/addrs-netlink.c b/src/stun/addrs-netlink.c new file mode 100644 index 0000000..ec32f17 --- /dev/null +++ b/src/stun/addrs-netlink.c @@ -0,0 +1,259 @@ +/* +Copyright (c) 2011, The WebRTC project authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright + notice, this list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above copyright + notice, this list of conditions and the following disclaimer in + the documentation and/or other materials provided with the + distribution. + + * Neither the name of Google nor the names of its contributors may + be used to endorse or promote products derived from this software + without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +*/ + +#if defined(LINUX) +#include "addrs-netlink.h" +#include +#include +#include +#include "util.h" +#include "stun_util.h" +#include "util.h" +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef ANDROID +/* Work around an Android NDK < r8c bug */ +#undef __unused +#else +#include /* struct ifreq, IFF_POINTTOPOINT */ +#include /* struct iwreq */ +#include /* struct ethtool_cmd */ +#include /* SIOCETHTOOL */ +#endif /* ANDROID */ + + +struct netlinkrequest { + struct nlmsghdr header; + struct ifaddrmsg msg; +}; + +static const int kMaxReadSize = 4096; + +static void set_ifname(nr_local_addr *addr, struct ifaddrmsg* msg) { + assert(sizeof(addr->addr.ifname) > IF_NAMESIZE); + if_indextoname(msg->ifa_index, addr->addr.ifname); +} + +static int get_siocgifflags(nr_local_addr *addr) { + int fd = socket(AF_INET, SOCK_DGRAM, 0); + if (fd == -1) { + assert(0); + return 0; + } + struct ifreq ifr; + memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, addr->addr.ifname, IFNAMSIZ - 1); + int rc = ioctl(fd, SIOCGIFFLAGS, &ifr); + close(fd); + if (rc == -1) { + assert(0); + return 0; + } + return ifr.ifr_flags; +} + +static int set_sockaddr(nr_local_addr *addr, struct ifaddrmsg* msg, struct rtattr* rta) { + assert(rta->rta_type == IFA_ADDRESS); + void *data = RTA_DATA(rta); + size_t len = RTA_PAYLOAD(rta); + if (msg->ifa_family == AF_INET) { + struct sockaddr_in sa; + memset(&sa, 0, sizeof(struct sockaddr_in)); + sa.sin_family = AF_INET; + memcpy(&sa.sin_addr, data, len); + return nr_sockaddr_to_transport_addr((struct sockaddr*)&sa, IPPROTO_UDP, 0, &(addr->addr)); + } else if (msg->ifa_family == AF_INET6) { + struct sockaddr_in6 sa; + memset(&sa, 0, sizeof(struct sockaddr_in6)); + sa.sin6_family = AF_INET6; + /* We do not set sin6_scope_id to ifa_index, because that is only valid for + * link local addresses, and we don't use those anyway */ + memcpy(&sa.sin6_addr, data, len); + return nr_sockaddr_to_transport_addr((struct sockaddr*)&sa, IPPROTO_UDP, 0, &(addr->addr)); + } + + return R_BAD_ARGS; +} + +static int +stun_convert_netlink(nr_local_addr *addr, struct ifaddrmsg *address_msg, struct rtattr* rta) +{ + int r = set_sockaddr(addr, address_msg, rta); + if (r) { + r_log(NR_LOG_STUN, LOG_ERR, "set_sockaddr error r = %d", r); + return r; + } + + set_ifname(addr, address_msg); + + int flags = get_siocgifflags(addr); + if (flags & IFF_POINTOPOINT) + { + addr->interface.type = NR_INTERFACE_TYPE_UNKNOWN | NR_INTERFACE_TYPE_VPN; + /* TODO (Bug 896913): find backend network type of this VPN */ + } + +#if defined(LINUX) && !defined(ANDROID) + struct ethtool_cmd ecmd; + struct ifreq ifr; + struct iwreq wrq; + int e; + int s = socket(AF_INET, SOCK_DGRAM, 0); + + strncpy(ifr.ifr_name, addr->addr.ifname, sizeof(ifr.ifr_name)); + /* TODO (Bug 896851): interface property for Android */ + /* Getting ethtool for ethernet information. */ + ecmd.cmd = ETHTOOL_GSET; + /* In/out param */ + ifr.ifr_data = (void*)&ecmd; + + e = ioctl(s, SIOCETHTOOL, &ifr); + if (e == 0) + { + /* For wireless network, we won't get ethtool, it's a wired + * connection */ + addr->interface.type = NR_INTERFACE_TYPE_WIRED; +#ifdef DONT_HAVE_ETHTOOL_SPEED_HI + addr->interface.estimated_speed = ecmd.speed; +#else + addr->interface.estimated_speed = ((ecmd.speed_hi << 16) | ecmd.speed) * 1000; +#endif + } + + strncpy(wrq.ifr_name, addr->addr.ifname, sizeof(wrq.ifr_name)); + e = ioctl(s, SIOCGIWRATE, &wrq); + if (e == 0) + { + addr->interface.type = NR_INTERFACE_TYPE_WIFI; + addr->interface.estimated_speed = wrq.u.bitrate.value / 1000; + } + + close(s); + +#else + addr->interface.type = NR_INTERFACE_TYPE_UNKNOWN; + addr->interface.estimated_speed = 0; +#endif + return 0; +} + +int +stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) +{ + int _status; + int fd = 0; + + /* Scope everything else since we're using ABORT. */ + { + *count = 0; + + if (maxaddrs <= 0) + ABORT(R_BAD_ARGS); + + fd = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE); + if (fd < 0) { + ABORT(R_INTERNAL); + } + + struct netlinkrequest ifaddr_request; + memset(&ifaddr_request, 0, sizeof(ifaddr_request)); + ifaddr_request.header.nlmsg_flags = NLM_F_ROOT | NLM_F_REQUEST; + ifaddr_request.header.nlmsg_type = RTM_GETADDR; + ifaddr_request.header.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)); + + ssize_t bytes = send(fd, &ifaddr_request, ifaddr_request.header.nlmsg_len, 0); + if ((size_t)bytes != ifaddr_request.header.nlmsg_len) { + ABORT(R_INTERNAL); + } + + char buf[kMaxReadSize]; + ssize_t amount_read = recv(fd, &buf, kMaxReadSize, 0); + while ((amount_read > 0) && (*count != maxaddrs)) { + struct nlmsghdr* header = (struct nlmsghdr*)&buf[0]; + size_t header_size = (size_t)amount_read; + for ( ; NLMSG_OK(header, header_size) && (*count != maxaddrs); + header = NLMSG_NEXT(header, header_size)) { + switch (header->nlmsg_type) { + case NLMSG_DONE: + /* Success. Return. */ + close(fd); + return 0; + case NLMSG_ERROR: + ABORT(R_INTERNAL); + case RTM_NEWADDR: { + struct ifaddrmsg* address_msg = + (struct ifaddrmsg*)NLMSG_DATA(header); + struct rtattr* rta = IFA_RTA(address_msg); + ssize_t payload_len = IFA_PAYLOAD(header); + while (RTA_OK(rta, payload_len)) { + if (rta->rta_type == IFA_ADDRESS) { + int family = address_msg->ifa_family; + if (family == AF_INET || family == AF_INET6) { + if (stun_convert_netlink(&addrs[*count], address_msg, rta)) { + assert(0); + } else { + ++(*count); + } + } + } + /* TODO: Use IFA_LABEL instead of if_indextoname? We would need + * to remember how many nr_local_addr we've converted for this + * ifaddrmsg, and set the label on all of them. */ + rta = RTA_NEXT(rta, payload_len); + } + break; + } + } + } + amount_read = recv(fd, &buf, kMaxReadSize, 0); + } + } + + _status=0; +abort: + close(fd); + return(_status); +} + +#endif /* defined(LINUX) */ diff --git a/src/stun/ifaddrs-android.h b/src/stun/addrs-netlink.h similarity index 68% rename from src/stun/ifaddrs-android.h rename to src/stun/addrs-netlink.h index 5ba174c..b2b07bd 100644 --- a/src/stun/ifaddrs-android.h +++ b/src/stun/addrs-netlink.h @@ -30,28 +30,16 @@ THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ -#ifndef WEBRTC_BASE_IFADDRS_ANDROID_H_ -#define WEBRTC_BASE_IFADDRS_ANDROID_H_ +#ifndef WEBRTC_BASE_IFADDRS_NETLINK_H_ +#define WEBRTC_BASE_IFADDRS_NETLINK_H_ #include #include +#include "local_addr.h" -/* Implementation of getifaddrs for Android. - * Fills out a list of ifaddr structs (see below) which contain information - * about every network interface available on the host. - * See 'man getifaddrs' on Linux or OS X (nb: it is not a POSIX function). */ -struct ifaddrs { - struct ifaddrs* ifa_next; - char* ifa_name; - unsigned int ifa_flags; - struct sockaddr* ifa_addr; - struct sockaddr* ifa_netmask; - /* Real ifaddrs has broadcast, point to point and data members. - * We don't need them (yet?). */ -}; - -int android_getifaddrs(struct ifaddrs** result); -void android_freeifaddrs(struct ifaddrs* addrs); - -#endif /* WEBRTC_BASE_IFADDRS_ANDROID_H_ */ +/* for platforms with netlink (android and linux) */ +/* Filters out things like deprecated addresses, and stuff that doesn't pass + * IPv6 duplicate address detection */ +int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count); +#endif /* WEBRTC_BASE_IFADDRS_NETLINK_H_ */ diff --git a/src/stun/addrs-win32.c b/src/stun/addrs-win32.c new file mode 100644 index 0000000..2e2a508 --- /dev/null +++ b/src/stun/addrs-win32.c @@ -0,0 +1,190 @@ +/* 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/. */ + +#ifdef WIN32 + +#include "addrs-win32.h" +#include +#include +#include +#include "util.h" +#include "stun_util.h" +#include "util.h" +#include +#include "nr_crypto.h" + +#include +#include +#include + +#define WIN32_MAX_NUM_INTERFACES 20 + +#define NR_MD5_HASH_LENGTH 16 + +#define _NR_MAX_KEY_LENGTH 256 +#define _NR_MAX_NAME_LENGTH 512 + +#define _ADAPTERS_BASE_REG "SYSTEM\\CurrentControlSet\\Control\\Network\\{4D36E972-E325-11CE-BFC1-08002BE10318}" + +static int nr_win32_get_adapter_friendly_name(char *adapter_GUID, char **friendly_name) +{ + int r,_status; + HKEY adapter_reg; + TCHAR adapter_key[_NR_MAX_KEY_LENGTH]; + TCHAR keyval_buf[_NR_MAX_KEY_LENGTH]; + TCHAR adapter_GUID_tchar[_NR_MAX_NAME_LENGTH]; + DWORD keyval_len, key_type; + size_t converted_chars, newlen; + char *my_fn = 0; + +#ifdef _UNICODE + mbstowcs_s(&converted_chars, adapter_GUID_tchar, strlen(adapter_GUID)+1, + adapter_GUID, _TRUNCATE); +#else + strlcpy(adapter_GUID_tchar, adapter_GUID, _NR_MAX_NAME_LENGTH); +#endif + + _tcscpy_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT(_ADAPTERS_BASE_REG)); + _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT("\\")); + _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, adapter_GUID_tchar); + _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT("\\Connection")); + + r = RegOpenKeyEx(HKEY_LOCAL_MACHINE, adapter_key, 0, KEY_READ, &adapter_reg); + + if (r != ERROR_SUCCESS) { + r_log(NR_LOG_STUN, LOG_ERR, "Got error %d opening adapter reg key\n", r); + ABORT(R_INTERNAL); + } + + keyval_len = sizeof(keyval_buf); + r = RegQueryValueEx(adapter_reg, TEXT("Name"), NULL, &key_type, + (BYTE *)keyval_buf, &keyval_len); + + RegCloseKey(adapter_reg); + +#ifdef UNICODE + newlen = wcslen(keyval_buf)+1; + my_fn = (char *) RCALLOC(newlen); + if (!my_fn) { + ABORT(R_NO_MEMORY); + } + wcstombs_s(&converted_chars, my_fn, newlen, keyval_buf, _TRUNCATE); +#else + my_fn = r_strdup(keyval_buf); +#endif + + *friendly_name = my_fn; + _status=0; + +abort: + if (_status) { + if (my_fn) free(my_fn); + } + return(_status); +} + +int +stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) +{ + int r, _status; + PIP_ADAPTER_ADDRESSES AdapterAddresses = NULL, tmpAddress = NULL; + // 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_BAD_ARGS); + + /* 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); + } + + 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); + AdapterAddresses = NULL; + } + + if (n >= 5) { + r_log(NR_LOG_STUN, LOG_ERR, "5 failures calling GetAdaptersAddresses()"); + ABORT(R_INTERNAL); + } + + n = 0; + + /* Loop through the adapters */ + + for (tmpAddress = AdapterAddresses; tmpAddress != NULL; tmpAddress = tmpAddress->Next) { + + if (tmpAddress->OperStatus != IfOperStatusUp) + continue; + + if ((tmpAddress->IfIndex != 0) || (tmpAddress->Ipv6IfIndex != 0)) { + IP_ADAPTER_UNICAST_ADDRESS *u = 0; + + if(r=nr_crypto_md5((UCHAR *)tmpAddress->FriendlyName, + wcslen(tmpAddress->FriendlyName) * sizeof(wchar_t), + bin_hashed_ifname)) + ABORT(r); + if(r=nr_bin2hex(bin_hashed_ifname, sizeof(bin_hashed_ifname), + hex_hashed_ifname)) + ABORT(r); + + for (u = tmpAddress->FirstUnicastAddress; u != 0; u = u->Next) { + SOCKET_ADDRESS *sa_addr = &u->Address; + + if ((sa_addr->lpSockaddr->sa_family == AF_INET) || + (sa_addr->lpSockaddr->sa_family == AF_INET6)) { + if ((r=nr_sockaddr_to_transport_addr((struct sockaddr*)sa_addr->lpSockaddr, IPPROTO_UDP, 0, &(addrs[n].addr)))) + ABORT(r); + } + else { + r_log(NR_LOG_STUN, LOG_DEBUG, "Unrecognized sa_family for address on adapter %lu", tmpAddress->IfIndex); + continue; + } + + strlcpy(addrs[n].addr.ifname, hex_hashed_ifname, sizeof(addrs[n].addr.ifname)); + if (tmpAddress->IfType == IF_TYPE_ETHERNET_CSMACD) { + addrs[n].interface.type = NR_INTERFACE_TYPE_WIRED; + } else if (tmpAddress->IfType == IF_TYPE_IEEE80211) { + /* Note: this only works for >= Win Vista */ + addrs[n].interface.type = NR_INTERFACE_TYPE_WIFI; + } else { + addrs[n].interface.type = NR_INTERFACE_TYPE_UNKNOWN; + } +#if (_WIN32_WINNT >= 0x0600) + /* Note: only >= Vista provide link speed information */ + addrs[n].interface.estimated_speed = tmpAddress->TransmitLinkSpeed / 1000; +#else + addrs[n].interface.estimated_speed = 0; +#endif + if (++n >= maxaddrs) + goto done; + } + } + } + + done: + *count = n; + _status = 0; + + abort: + RFREE(AdapterAddresses); + return _status; +} + +#endif //WIN32 + diff --git a/src/stun/addrs-win32.h b/src/stun/addrs-win32.h new file mode 100644 index 0000000..a008021 --- /dev/null +++ b/src/stun/addrs-win32.h @@ -0,0 +1,13 @@ +/* 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/. */ + +#ifndef STUN_IFADDRS_WIN32_H_ +#define STUN_IFADDRS_WIN32_H_ + +#include "local_addr.h" + +int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count); + +#endif /* STUN_IFADDRS_WIN32_H_ */ + diff --git a/src/stun/addrs.c b/src/stun/addrs.c index 94aa107..362b7d8 100644 --- a/src/stun/addrs.c +++ b/src/stun/addrs.c @@ -35,321 +35,17 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #ifdef WIN32 -#include -#include -#include -#else /* !WIN32 */ - -#include -#include -#include - -#ifndef ANDROID -/* This works on linux and BSD, but not android */ -#include /* getifaddrs */ -#include /* getifaddrs */ -#else -#include "ifaddrs-android.h" -#define getifaddrs android_getifaddrs -#define freeifaddrs android_freeifaddrs -#endif - -#ifdef LINUX - -#ifdef ANDROID -/* Work around an Android NDK < r8c bug */ -#undef __unused -#else -#ifndef NOLINUXIF -#include /* struct ifreq, IFF_POINTTOPOINT */ -#include /* struct iwreq */ +#include "addrs-win32.h" +#elif defined(BSD) || defined(DARWIN) +#include "addrs-bsd.h" #else -#include /* struct ifreq, IFF_POINTTOPOINT */ +#include "addrs-netlink.h" #endif -#include /* struct ethtool_cmd */ -#include /* SIOCETHTOOL */ -#endif /* ANDROID */ - -#endif /* LINUX */ - -#endif /* !WIN32 */ #include "stun.h" #include "addrs.h" -#include "nr_crypto.h" #include "util.h" -#if defined(WIN32) - -#define WIN32_MAX_NUM_INTERFACES 20 - -#define NR_MD5_HASH_LENGTH 16 - -#define _NR_MAX_KEY_LENGTH 256 -#define _NR_MAX_NAME_LENGTH 512 - -#define _ADAPTERS_BASE_REG "SYSTEM\\CurrentControlSet\\Control\\Network\\{4D36E972-E325-11CE-BFC1-08002BE10318}" - -static int nr_win32_get_adapter_friendly_name(char *adapter_GUID, char **friendly_name) -{ - int r,_status; - HKEY adapter_reg; - TCHAR adapter_key[_NR_MAX_KEY_LENGTH]; - TCHAR keyval_buf[_NR_MAX_KEY_LENGTH]; - TCHAR adapter_GUID_tchar[_NR_MAX_NAME_LENGTH]; - DWORD keyval_len, key_type; - size_t converted_chars, newlen; - char *my_fn = 0; - -#ifdef _UNICODE - mbstowcs_s(&converted_chars, adapter_GUID_tchar, strlen(adapter_GUID)+1, - adapter_GUID, _TRUNCATE); -#else - strlcpy(adapter_GUID_tchar, adapter_GUID, _NR_MAX_NAME_LENGTH); -#endif - - _tcscpy_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT(_ADAPTERS_BASE_REG)); - _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT("\\")); - _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, adapter_GUID_tchar); - _tcscat_s(adapter_key, _NR_MAX_KEY_LENGTH, TEXT("\\Connection")); - - r = RegOpenKeyEx(HKEY_LOCAL_MACHINE, adapter_key, 0, KEY_READ, &adapter_reg); - - if (r != ERROR_SUCCESS) { - r_log(NR_LOG_STUN, LOG_ERR, "Got error %d opening adapter reg key\n", r); - ABORT(R_INTERNAL); - } - - keyval_len = sizeof(keyval_buf); - r = RegQueryValueEx(adapter_reg, TEXT("Name"), NULL, &key_type, - (BYTE *)keyval_buf, &keyval_len); - - RegCloseKey(adapter_reg); - -#ifdef UNICODE - newlen = wcslen(keyval_buf)+1; - my_fn = (char *) RCALLOC(newlen); - if (!my_fn) { - ABORT(R_NO_MEMORY); - } - wcstombs_s(&converted_chars, my_fn, newlen, keyval_buf, _TRUNCATE); -#else - my_fn = r_strdup(keyval_buf); -#endif - - *friendly_name = my_fn; - _status=0; - -abort: - if (_status) { - 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; - PIP_ADAPTER_ADDRESSES AdapterAddresses = NULL, tmpAddress = NULL; - // 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_BAD_ARGS); - - /* 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); - } - - 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); - AdapterAddresses = NULL; - } - - if (n >= 5) { - r_log(NR_LOG_STUN, LOG_ERR, "5 failures calling GetAdaptersAddresses()"); - ABORT(R_INTERNAL); - } - - n = 0; - - /* Loop through the adapters */ - - for (tmpAddress = AdapterAddresses; tmpAddress != NULL; tmpAddress = tmpAddress->Next) { - - if (tmpAddress->OperStatus != IfOperStatusUp) - continue; - - if ((tmpAddress->IfIndex != 0) || (tmpAddress->Ipv6IfIndex != 0)) { - IP_ADAPTER_UNICAST_ADDRESS *u = 0; - - if(r=nr_crypto_md5((UCHAR *)tmpAddress->FriendlyName, - wcslen(tmpAddress->FriendlyName) * sizeof(wchar_t), - bin_hashed_ifname)) - ABORT(r); - if(r=nr_bin2hex(bin_hashed_ifname, sizeof(bin_hashed_ifname), - hex_hashed_ifname)) - ABORT(r); - - for (u = tmpAddress->FirstUnicastAddress; u != 0; u = u->Next) { - SOCKET_ADDRESS *sa_addr = &u->Address; - - if ((sa_addr->lpSockaddr->sa_family == AF_INET) || - (sa_addr->lpSockaddr->sa_family == AF_INET6)) { - if ((r=nr_sockaddr_to_transport_addr((struct sockaddr*)sa_addr->lpSockaddr, IPPROTO_UDP, 0, &(addrs[n].addr)))) - ABORT(r); - } - else { - r_log(NR_LOG_STUN, LOG_DEBUG, "Unrecognized sa_family for address on adapter %lu", tmpAddress->IfIndex); - continue; - } - - strlcpy(addrs[n].addr.ifname, hex_hashed_ifname, sizeof(addrs[n].addr.ifname)); - if (tmpAddress->IfType == IF_TYPE_ETHERNET_CSMACD) { - addrs[n].interface.type = NR_INTERFACE_TYPE_WIRED; - } else if (tmpAddress->IfType == IF_TYPE_IEEE80211) { - /* Note: this only works for >= Win Vista */ - addrs[n].interface.type = NR_INTERFACE_TYPE_WIFI; - } else { - addrs[n].interface.type = NR_INTERFACE_TYPE_UNKNOWN; - } -#if (_WIN32_WINNT >= 0x0600) - /* Note: only >= Vista provide link speed information */ - addrs[n].interface.estimated_speed = tmpAddress->TransmitLinkSpeed / 1000; -#else - addrs[n].interface.estimated_speed = 0; -#endif - if (++n >= maxaddrs) - goto done; - } - } - } - - done: - *count = n; - _status = 0; - - abort: - RFREE(AdapterAddresses); - return _status; -} - -#else /* WIN32 */ - -static int -nr_stun_is_duplicate_addr(nr_local_addr addrs[], int count, nr_local_addr *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; - - 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; - - while (if_addr && *count < maxaddrs) { - /* This can be null */ - if (if_addr->ifa_addr) { - switch (if_addr->ifa_addr->sa_family) { - case AF_INET: - case AF_INET6: - if (r=nr_sockaddr_to_transport_addr(if_addr->ifa_addr, IPPROTO_UDP, 0, &(addrs[*count].addr))) { - r_log(NR_LOG_STUN, LOG_ERR, "nr_sockaddr_to_transport_addr error r = %d", r); - } else { -#if defined(LINUX) && !defined(ANDROID) && !defined(NOLINUXIF) - struct ethtool_cmd ecmd; - struct ifreq ifr; - struct iwreq wrq; - int e; - int s = socket(AF_INET, SOCK_DGRAM, 0); - - strncpy(ifr.ifr_name, if_addr->ifa_name, sizeof(ifr.ifr_name)); - /* TODO (Bug 896851): interface property for Android */ - /* Getting ethtool for ethernet information. */ - ecmd.cmd = ETHTOOL_GSET; - /* In/out param */ - ifr.ifr_data = (void*)&ecmd; - - e = ioctl(s, SIOCETHTOOL, &ifr); - if (e == 0) - { - /* For wireless network, we won't get ethtool, it's a wired - * connection */ - addrs[*count].interface.type = NR_INTERFACE_TYPE_WIRED; -#ifdef DONT_HAVE_ETHTOOL_SPEED_HI - addrs[*count].interface.estimated_speed = ecmd.speed; -#else - addrs[*count].interface.estimated_speed = ((ecmd.speed_hi << 16) | ecmd.speed) * 1000; -#endif - } - - strncpy(wrq.ifr_name, if_addr->ifa_name, sizeof(wrq.ifr_name)); - e = ioctl(s, SIOCGIWRATE, &wrq); - if (e == 0) - { - addrs[*count].interface.type = NR_INTERFACE_TYPE_WIFI; - addrs[*count].interface.estimated_speed = wrq.u.bitrate.value / 1000; - } - - close(s); - - if (if_addr->ifa_flags & IFF_POINTOPOINT) - { - addrs[*count].interface.type = NR_INTERFACE_TYPE_UNKNOWN | NR_INTERFACE_TYPE_VPN; - /* TODO (Bug 896913): find backend network type of this VPN */ - } -#else - addrs[*count].interface.type = NR_INTERFACE_TYPE_UNKNOWN; - addrs[*count].interface.estimated_speed = 0; -#endif - (void)strlcpy(addrs[*count].addr.ifname, if_addr->ifa_name, sizeof(addrs[*count].addr.ifname)); - ++(*count); - } - break; - default: - ; - } - } - - if_addr = if_addr->ifa_next; - } - - _status=0; -abort: - if (if_addrs_head) { - freeifaddrs(if_addrs_head); - } - return(_status); -} - -#endif - static int nr_stun_is_duplicate_addr(nr_local_addr addrs[], int count, nr_local_addr *addr) { @@ -466,11 +162,7 @@ nr_stun_get_addrs(nr_local_addr addrs[], int maxaddrs, int *count) memset(addrs, 0, maxaddrs * sizeof(nr_local_addr)); } -#ifdef WIN32 - _status = stun_get_win32_addrs(addrs, maxaddrs, count); -#else - _status = stun_getifaddrs(addrs, maxaddrs, count); -#endif + _status = stun_getaddrs_filtered(addrs, maxaddrs, count); for (i = 0; i < *count; ++i) { nr_local_addr_fmt_info_string(addrs+i,typestr,sizeof(typestr)); diff --git a/src/stun/ifaddrs-android.c b/src/stun/ifaddrs-android.c deleted file mode 100644 index d7d390f..0000000 --- a/src/stun/ifaddrs-android.c +++ /dev/null @@ -1,242 +0,0 @@ -/* -Copyright (c) 2011, The WebRTC project authors. All rights reserved. - -Redistribution and use in source and binary forms, with or without -modification, are permitted provided that the following conditions are -met: - - * Redistributions of source code must retain the above copyright - notice, this list of conditions and the following disclaimer. - - * Redistributions in binary form must reproduce the above copyright - notice, this list of conditions and the following disclaimer in - the documentation and/or other materials provided with the - distribution. - - * Neither the name of Google nor the names of its contributors may - be used to endorse or promote products derived from this software - without specific prior written permission. - -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -*/ - -#if defined(ANDROID) -#include "ifaddrs-android.h" -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -struct netlinkrequest { - struct nlmsghdr header; - struct ifaddrmsg msg; -}; - -static const int kMaxReadSize = 4096; - -static int set_ifname(struct ifaddrs* ifaddr, int interface) { - char buf[IFNAMSIZ] = {0}; - char* name = if_indextoname(interface, buf); - if (name == NULL) { - return -1; - } - ifaddr->ifa_name = malloc(strlen(name) + 1); - strncpy(ifaddr->ifa_name, name, strlen(name) + 1); - return 0; -} - -static int set_flags(struct ifaddrs* ifaddr) { - int fd = socket(AF_INET, SOCK_DGRAM, 0); - if (fd == -1) { - return -1; - } - struct ifreq ifr; - memset(&ifr, 0, sizeof(ifr)); - strncpy(ifr.ifr_name, ifaddr->ifa_name, IFNAMSIZ - 1); - int rc = ioctl(fd, SIOCGIFFLAGS, &ifr); - close(fd); - if (rc == -1) { - return -1; - } - ifaddr->ifa_flags = ifr.ifr_flags; - return 0; -} - -static int set_addresses(struct ifaddrs* ifaddr, struct ifaddrmsg* msg, void* data, - size_t len) { - if (msg->ifa_family == AF_INET) { - struct sockaddr_in* sa = malloc(sizeof(struct sockaddr_in)); - memset(sa, 0, sizeof(struct sockaddr_in)); - sa->sin_family = AF_INET; - memcpy(&sa->sin_addr, data, len); - ifaddr->ifa_addr = (struct sockaddr*)sa; - } else if (msg->ifa_family == AF_INET6) { - struct sockaddr_in6* sa = malloc(sizeof(struct sockaddr_in6)); - memset(sa, 0, sizeof(struct sockaddr_in6)); - sa->sin6_family = AF_INET6; - sa->sin6_scope_id = msg->ifa_index; - memcpy(&sa->sin6_addr, data, len); - ifaddr->ifa_addr = (struct sockaddr*)sa; - } else { - return -1; - } - return 0; -} - -static int make_prefixes(struct ifaddrs* ifaddr, int family, int prefixlen) { - char* prefix = NULL; - if (family == AF_INET) { - struct sockaddr_in* mask = malloc(sizeof(struct sockaddr_in)); - memset(mask, 0, sizeof(struct sockaddr_in)); - mask->sin_family = AF_INET; - memset(&mask->sin_addr, 0, sizeof(struct in_addr)); - ifaddr->ifa_netmask = (struct sockaddr*)mask; - if (prefixlen > 32) { - prefixlen = 32; - } - prefix = (char*)&mask->sin_addr; - } else if (family == AF_INET6) { - struct sockaddr_in6* mask = malloc(sizeof(struct sockaddr_in6)); - memset(mask, 0, sizeof(struct sockaddr_in6)); - mask->sin6_family = AF_INET6; - memset(&mask->sin6_addr, 0, sizeof(struct in6_addr)); - ifaddr->ifa_netmask = (struct sockaddr*)mask; - if (prefixlen > 128) { - prefixlen = 128; - } - prefix = (char*)&mask->sin6_addr; - } else { - return -1; - } - for (int i = 0; i < (prefixlen / 8); i++) { - *prefix++ = 0xFF; - } - char remainder = 0xff; - remainder <<= (8 - prefixlen % 8); - *prefix = remainder; - return 0; -} - -static int populate_ifaddrs(struct ifaddrs* ifaddr, struct ifaddrmsg* msg, void* bytes, - size_t len) { - if (set_ifname(ifaddr, msg->ifa_index) != 0) { - return -1; - } - if (set_flags(ifaddr) != 0) { - return -1; - } - if (set_addresses(ifaddr, msg, bytes, len) != 0) { - return -1; - } - if (make_prefixes(ifaddr, msg->ifa_family, msg->ifa_prefixlen) != 0) { - return -1; - } - return 0; -} - -int android_getifaddrs(struct ifaddrs** result) { - int fd = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE); - if (fd < 0) { - return -1; - } - - struct netlinkrequest ifaddr_request; - memset(&ifaddr_request, 0, sizeof(ifaddr_request)); - ifaddr_request.header.nlmsg_flags = NLM_F_ROOT | NLM_F_REQUEST; - ifaddr_request.header.nlmsg_type = RTM_GETADDR; - ifaddr_request.header.nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)); - - ssize_t count = send(fd, &ifaddr_request, ifaddr_request.header.nlmsg_len, 0); - if ((size_t)count != ifaddr_request.header.nlmsg_len) { - close(fd); - return -1; - } - struct ifaddrs* start = NULL; - struct ifaddrs* current = NULL; - char buf[kMaxReadSize]; - ssize_t amount_read = recv(fd, &buf, kMaxReadSize, 0); - while (amount_read > 0) { - struct nlmsghdr* header = (struct nlmsghdr*)&buf[0]; - size_t header_size = (size_t)amount_read; - for ( ; NLMSG_OK(header, header_size); - header = NLMSG_NEXT(header, header_size)) { - switch (header->nlmsg_type) { - case NLMSG_DONE: - /* Success. Return. */ - *result = start; - close(fd); - return 0; - case NLMSG_ERROR: - close(fd); - android_freeifaddrs(start); - return -1; - case RTM_NEWADDR: { - struct ifaddrmsg* address_msg = - (struct ifaddrmsg*)NLMSG_DATA(header); - struct rtattr* rta = IFA_RTA(address_msg); - ssize_t payload_len = IFA_PAYLOAD(header); - while (RTA_OK(rta, payload_len)) { - if (rta->rta_type == IFA_ADDRESS) { - int family = address_msg->ifa_family; - if (family == AF_INET || family == AF_INET6) { - struct ifaddrs* newest = malloc(sizeof(struct ifaddrs)); - memset(newest, 0, sizeof(struct ifaddrs)); - if (current) { - current->ifa_next = newest; - } else { - start = newest; - } - if (populate_ifaddrs(newest, address_msg, RTA_DATA(rta), - RTA_PAYLOAD(rta)) != 0) { - android_freeifaddrs(start); - *result = NULL; - return -1; - } - current = newest; - } - } - rta = RTA_NEXT(rta, payload_len); - } - break; - } - } - } - amount_read = recv(fd, &buf, kMaxReadSize, 0); - } - close(fd); - android_freeifaddrs(start); - return -1; -} - -void android_freeifaddrs(struct ifaddrs* addrs) { - struct ifaddrs* last = NULL; - struct ifaddrs* cursor = addrs; - while (cursor) { - free(cursor->ifa_name); - free(cursor->ifa_addr); - free(cursor->ifa_netmask); - last = cursor; - cursor = cursor->ifa_next; - free(last); - } -} - -#endif /* defined(ANDROID) */ From fc2452fde90a2a6787a5ba4bb5615a6a46e07ca8 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 26 May 2020 15:58:44 +0000 Subject: [PATCH 06/41] Bug 1183145: Teach platform-specific code to filter out inappropriate IPv6 addresses, and mark temp addresses. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D75962 --- src/stun/addrs-bsd.c | 53 +++++++++++++++++++++++++++++++++++----- src/stun/addrs-netlink.c | 12 ++++++++- src/stun/addrs-win32.c | 32 +++++++++++++++++++----- 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/stun/addrs-bsd.c b/src/stun/addrs-bsd.c index 763fe22..5bf3087 100644 --- a/src/stun/addrs-bsd.c +++ b/src/stun/addrs-bsd.c @@ -19,9 +19,44 @@ #include #include +static int +stun_ifaddr_get_v6_flags(struct ifaddrs *ifaddr) +{ + if (ifaddr->ifa_addr->sa_family != AF_INET6) { + return 0; + } + + int flags = 0; + int s = socket(AF_INET6, SOCK_DGRAM, 0); + if (!s) { + r_log(NR_LOG_STUN, LOG_ERR, "socket(AF_INET6, SOCK_DGRAM, 0) failed, errno=%d", errno); + assert(0); + return 0; + } + struct in6_ifreq ifr6; + memset(&ifr6, 0, sizeof(ifr6)); + strncpy(ifr6.ifr_name, ifaddr->ifa_name, sizeof(ifr6.ifr_name)); + /* ifr_addr is a sockaddr_in6, ifa_addr is a sockaddr* */ + struct sockaddr_in6 *sin6 = (struct sockaddr_in6*)ifaddr->ifa_addr; + ifr6.ifr_addr = *sin6; + if (ioctl(s, SIOCGIFAFLAG_IN6, &ifr6) != -1) { + flags = ifr6.ifr_flags; + } else { + r_log(NR_LOG_STUN, LOG_ERR, "ioctl(SIOCGIFAFLAG_IN6) failed, errno=%d", errno); + assert(0); + } + close(s); + return flags; +} + +static int +stun_ifaddr_is_disallowed_v6(int flags) { + return flags & (IN6_IFF_ANYCAST | IN6_IFF_TENTATIVE | IN6_IFF_DUPLICATED | IN6_IFF_DETACHED | IN6_IFF_DEPRECATED); +} + int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) { - int r,_status; + int r,_status,flags; struct ifaddrs* if_addrs_head=NULL; struct ifaddrs* if_addr; @@ -43,11 +78,17 @@ int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) switch (if_addr->ifa_addr->sa_family) { case AF_INET: case AF_INET6: - if (r=nr_sockaddr_to_transport_addr(if_addr->ifa_addr, IPPROTO_UDP, 0, &(addrs[*count].addr))) { - r_log(NR_LOG_STUN, LOG_ERR, "nr_sockaddr_to_transport_addr error r = %d", r); - } else { - (void)strlcpy(addrs[*count].addr.ifname, if_addr->ifa_name, sizeof(addrs[*count].addr.ifname)); - ++(*count); + flags = stun_ifaddr_get_v6_flags(if_addr); + if (!stun_ifaddr_is_disallowed_v6(flags)) { + if (r=nr_sockaddr_to_transport_addr(if_addr->ifa_addr, IPPROTO_UDP, 0, &(addrs[*count].addr))) { + r_log(NR_LOG_STUN, LOG_ERR, "nr_sockaddr_to_transport_addr error r = %d", r); + } else { + if (flags & IN6_IFF_TEMPORARY) { + addrs[*count].flags |= NR_ADDR_FLAG_TEMPORARY; + } + (void)strlcpy(addrs[*count].addr.ifname, if_addr->ifa_name, sizeof(addrs[*count].addr.ifname)); + ++(*count); + } } break; default: diff --git a/src/stun/addrs-netlink.c b/src/stun/addrs-netlink.c index ec32f17..ddecc34 100644 --- a/src/stun/addrs-netlink.c +++ b/src/stun/addrs-netlink.c @@ -116,6 +116,11 @@ static int set_sockaddr(nr_local_addr *addr, struct ifaddrmsg* msg, struct rtatt return R_BAD_ARGS; } +static int +stun_ifaddr_is_disallowed_v6(int flags) { + return flags & (IFA_F_TENTATIVE | IFA_F_OPTIMISTIC | IFA_F_DADFAILED | IFA_F_DEPRECATED); +} + static int stun_convert_netlink(nr_local_addr *addr, struct ifaddrmsg *address_msg, struct rtattr* rta) { @@ -127,6 +132,10 @@ stun_convert_netlink(nr_local_addr *addr, struct ifaddrmsg *address_msg, struct set_ifname(addr, address_msg); + if (address_msg->ifa_flags & IFA_F_TEMPORARY) { + addr->flags |= NR_ADDR_FLAG_TEMPORARY; + } + int flags = get_siocgifflags(addr); if (flags & IFF_POINTOPOINT) { @@ -229,7 +238,8 @@ stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) while (RTA_OK(rta, payload_len)) { if (rta->rta_type == IFA_ADDRESS) { int family = address_msg->ifa_family; - if (family == AF_INET || family == AF_INET6) { + if ((family == AF_INET || family == AF_INET6) && + !stun_ifaddr_is_disallowed_v6(address_msg->ifa_flags)) { if (stun_convert_netlink(&addrs[*count], address_msg, rta)) { assert(0); } else { diff --git a/src/stun/addrs-win32.c b/src/stun/addrs-win32.c index 2e2a508..4fdf5e5 100644 --- a/src/stun/addrs-win32.c +++ b/src/stun/addrs-win32.c @@ -84,6 +84,18 @@ static int nr_win32_get_adapter_friendly_name(char *adapter_GUID, char **friendl return(_status); } +static int stun_win32_address_disallowed(IP_ADAPTER_UNICAST_ADDRESS *addr) +{ + return (addr->DadState != NldsPreferred) && + (addr->DadState != IpDadStatePreferred); +} + +static int stun_win32_address_temp_v6(IP_ADAPTER_UNICAST_ADDRESS *addr) +{ + return (addr->Address.lpSockaddr->sa_family == AF_INET6) && + (addr->SuffixOrigin == IpSuffixOriginRandom); +} + int stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) { @@ -146,16 +158,20 @@ stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) for (u = tmpAddress->FirstUnicastAddress; u != 0; u = u->Next) { SOCKET_ADDRESS *sa_addr = &u->Address; - if ((sa_addr->lpSockaddr->sa_family == AF_INET) || - (sa_addr->lpSockaddr->sa_family == AF_INET6)) { - if ((r=nr_sockaddr_to_transport_addr((struct sockaddr*)sa_addr->lpSockaddr, IPPROTO_UDP, 0, &(addrs[n].addr)))) - ABORT(r); - } - else { + if ((sa_addr->lpSockaddr->sa_family != AF_INET) && + (sa_addr->lpSockaddr->sa_family != AF_INET6)) { r_log(NR_LOG_STUN, LOG_DEBUG, "Unrecognized sa_family for address on adapter %lu", tmpAddress->IfIndex); continue; } + if (stun_win32_address_disallowed(u)) { + continue; + } + + if ((r=nr_sockaddr_to_transport_addr((struct sockaddr*)sa_addr->lpSockaddr, IPPROTO_UDP, 0, &(addrs[n].addr)))) { + ABORT(r); + } + strlcpy(addrs[n].addr.ifname, hex_hashed_ifname, sizeof(addrs[n].addr.ifname)); if (tmpAddress->IfType == IF_TYPE_ETHERNET_CSMACD) { addrs[n].interface.type = NR_INTERFACE_TYPE_WIRED; @@ -171,6 +187,10 @@ stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) #else addrs[n].interface.estimated_speed = 0; #endif + if (stun_win32_address_temp_v6(u)) { + addrs[n].flags |= NR_ADDR_FLAG_TEMPORARY; + } + if (++n >= maxaddrs) goto done; } From 21f56eab6e720da3c4c570bdb0b2b98ca24c317d Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 4 Jun 2020 15:56:46 +0000 Subject: [PATCH 07/41] Bug 1643169: Use ifr_ifru.ifru_flags6 instead of the ifr_flags macro, because the latter does not work properly on FreeBSD. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D78319 --- src/stun/addrs-bsd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stun/addrs-bsd.c b/src/stun/addrs-bsd.c index 5bf3087..5803c87 100644 --- a/src/stun/addrs-bsd.c +++ b/src/stun/addrs-bsd.c @@ -40,7 +40,7 @@ stun_ifaddr_get_v6_flags(struct ifaddrs *ifaddr) struct sockaddr_in6 *sin6 = (struct sockaddr_in6*)ifaddr->ifa_addr; ifr6.ifr_addr = *sin6; if (ioctl(s, SIOCGIFAFLAG_IN6, &ifr6) != -1) { - flags = ifr6.ifr_flags; + flags = ifr6.ifr_ifru.ifru_flags6; } else { r_log(NR_LOG_STUN, LOG_ERR, "ioctl(SIOCGIFAFLAG_IN6) failed, errno=%d", errno); assert(0); From 7ab5c30874f5b75eff17abf7dec6095d6845f817 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 9 Jun 2020 19:44:50 +0000 Subject: [PATCH 08/41] Bug 1644477: Make candidate pair insertion code easier to read/understand. r=mjf Includes removing an error code for a function that never fails, and removing an error return when the function successfully did what it said it would. Differential Revision: https://phabricator.services.mozilla.com/D78929 --- src/ice/ice_candidate_pair.c | 7 ++----- src/ice/ice_candidate_pair.h | 2 +- src/ice/ice_component.c | 12 ++++-------- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/ice/ice_candidate_pair.c b/src/ice/ice_candidate_pair.c index a315a1f..83d88d3 100644 --- a/src/ice/ice_candidate_pair.c +++ b/src/ice/ice_candidate_pair.c @@ -425,8 +425,7 @@ static int nr_ice_candidate_copy_for_triggered_check(nr_ice_cand_pair *pair) copy->nominated = pair->nominated; r_log(LOG_ICE,LOG_INFO,"CAND-PAIR(%s): Adding pair to check list and trigger check queue: %s",pair->codeword,pair->as_string); - if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,copy)) - ABORT(r); + nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,copy); nr_ice_candidate_pair_trigger_check_append(&pair->remote->stream->trigger_check_queue,copy); copy->triggered = 1; @@ -603,7 +602,7 @@ int nr_ice_candidate_pair_trigger_check_append(nr_ice_cand_pair_head *head,nr_ic return(0); } -int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair) +void nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair) { nr_ice_cand_pair *c1; @@ -617,8 +616,6 @@ int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *p c1=TAILQ_NEXT(c1,check_queue_entry); } if(!c1) TAILQ_INSERT_TAIL(head,pair,check_queue_entry); - - return(0); } void nr_ice_candidate_pair_restart_stun_nominated_cb(NR_SOCKET s, int how, void *cb_arg) diff --git a/src/ice/ice_candidate_pair.h b/src/ice/ice_candidate_pair.h index 76bed00..49da1f7 100644 --- a/src/ice/ice_candidate_pair.h +++ b/src/ice/ice_candidate_pair.h @@ -88,7 +88,7 @@ void nr_ice_candidate_pair_dump_state(nr_ice_cand_pair *pair, int log_level); void nr_ice_candidate_pair_cancel(nr_ice_peer_ctx *pctx,nr_ice_cand_pair *pair, int move_to_wait_state); int nr_ice_candidate_pair_select(nr_ice_cand_pair *pair); int nr_ice_candidate_pair_do_triggered_check(nr_ice_peer_ctx *pctx, nr_ice_cand_pair *pair); -int nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair); +void nr_ice_candidate_pair_insert(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair); int nr_ice_candidate_pair_trigger_check_append(nr_ice_cand_pair_head *head,nr_ice_cand_pair *pair); void nr_ice_candidate_pair_restart_stun_nominated_cb(NR_SOCKET s, int how, void *cb_arg); int nr_ice_candidate_pair_destroy(nr_ice_cand_pair **pairp); diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 839e785..3ef04b1 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -1734,8 +1734,7 @@ int nr_ice_component_finalize(nr_ice_component *lcomp, nr_ice_component *rcomp) int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair) { - int r,_status; - int pair_inserted=0; + int _status; /* Pairs for peer reflexive are marked SUCCEEDED immediately */ if (pair->state != NR_ICE_PAIR_STATE_FROZEN && @@ -1744,10 +1743,8 @@ int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair ABORT(R_BAD_ARGS); } - if(r=nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair)) - ABORT(r); - - pair_inserted=1; + /* We do not throw an error after this, because we've inserted the pair. */ + nr_ice_candidate_pair_insert(&pair->remote->stream->check_list,pair); /* Make sure the check timer is running, if the stream was previously * started. We will not start streams just because a pair was created, @@ -1759,13 +1756,12 @@ int nr_ice_component_insert_pair(nr_ice_component *pcomp, nr_ice_cand_pair *pair !pair->remote->stream->pctx->checks_started)){ if(nr_ice_media_stream_start_checks(pair->remote->stream->pctx, pair->remote->stream)) { r_log(LOG_ICE,LOG_WARNING,"ICE-PEER(%s)/CAND-PAIR(%s): Could not restart checks for new pair %s.",pair->remote->stream->pctx->label, pair->codeword, pair->as_string); - ABORT(R_INTERNAL); } } _status=0; abort: - if (_status && !pair_inserted) { + if (_status) { nr_ice_candidate_pair_destroy(&pair); } return(_status); From f0e195e062b4dbe8f8540e00f962ad9dd1af6f60 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 7 Jul 2020 16:33:50 +0000 Subject: [PATCH 09/41] Bug 1649678: Add include needed by OpenBSD. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D82531 --- src/stun/addrs-bsd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stun/addrs-bsd.c b/src/stun/addrs-bsd.c index 5803c87..ec2d084 100644 --- a/src/stun/addrs-bsd.c +++ b/src/stun/addrs-bsd.c @@ -18,6 +18,7 @@ #include #include #include +#include static int stun_ifaddr_get_v6_flags(struct ifaddrs *ifaddr) From 5ec60330cfb49a8abb4eca9ea9ccac44e5190db0 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Fri, 10 Jul 2020 17:11:32 +0000 Subject: [PATCH 10/41] Bug 1651601: Filter gathered candidates when their component is obsolete. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D83138 --- src/ice/ice_ctx.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index 84f428c..ebfd352 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -1,4 +1,3 @@ - /* Copyright (c) 2007, Adobe Systems, Incorporated All rights reserved. @@ -505,7 +504,6 @@ void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg) nr_ice_media_stream *stream; int component_id; - assert(cb_arg); if (!cb_arg) return; @@ -1092,6 +1090,10 @@ int nr_ice_ctx_hide_candidate(nr_ice_ctx *ctx, nr_ice_candidate *cand) return 1; } + if (cand->stream->obsolete) { + return 1; + } + return 0; } From a220d5151f507a5190e24d1a5f5e71d063059fe2 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Fri, 24 Jul 2020 17:01:49 +0000 Subject: [PATCH 11/41] Bug 1651601: Only do the minimum processing for STUN messages on an obsolete stream. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D83545 --- src/ice/ice_component.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 3ef04b1..0083778 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -1034,22 +1034,27 @@ static int nr_ice_component_process_incoming_check(nr_ice_component *comp, nr_tr static int nr_ice_component_stun_server_cb(void *cb_arg,nr_stun_server_ctx *stun_ctx,nr_socket *sock, nr_stun_server_request *req, int *dont_free, int *error) { - nr_ice_component *comp=cb_arg; + nr_ice_component *pcomp=cb_arg; nr_transport_addr local_addr; int r,_status; - if(comp->state==NR_ICE_COMPONENT_FAILED) { + if(pcomp->state==NR_ICE_COMPONENT_FAILED) { *error=400; ABORT(R_REJECTED); } + if (pcomp->local_component->stream->obsolete) { + /* Don't do any triggered check stuff in thiis case. */ + return 0; + } + /* Find the candidate pair that this maps to */ if(r=nr_socket_getaddr(sock,&local_addr)) { *error=500; ABORT(r); } - if (r=nr_ice_component_process_incoming_check(comp, &local_addr, req, error)) + if (r=nr_ice_component_process_incoming_check(pcomp, &local_addr, req, error)) ABORT(r); _status=0; From 68ee2ba102ae10dbcbb67d512f010f9304854698 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Fri, 31 Jul 2020 15:48:18 +0000 Subject: [PATCH 12/41] Bug 1651601: Ignore remote trickle candidates for obsolete streams. r=mjf Depends on D83545 Differential Revision: https://phabricator.services.mozilla.com/D85284 --- src/ice/ice_peer_ctx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ice/ice_peer_ctx.c b/src/ice/ice_peer_ctx.c index 63b9a28..f09f6eb 100644 --- a/src/ice/ice_peer_ctx.c +++ b/src/ice/ice_peer_ctx.c @@ -280,6 +280,10 @@ int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_ int r,_status; int needs_pairing = 0; + if (stream->obsolete) { + return 0; + } + r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): peer (%s) parsing trickle ICE candidate %s",pctx->ctx->label,pctx->label,candidate); r = nr_ice_peer_ctx_find_pstream(pctx, stream, &pstream); if (r) From d8ec55863ce401e8d809c9287ecf9f96e8eb7d39 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 19 Nov 2020 20:29:14 +0000 Subject: [PATCH 13/41] Bug 1677759: Emit per-stream null candidates when the last candidate fails to init. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D97636 --- src/ice/ice_ctx.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index ebfd352..5b6e03f 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -548,11 +548,11 @@ void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg) /* But continue */ } } + } - if (nr_ice_media_stream_is_done_gathering(stream) && - ctx->trickle_cb) { - ctx->trickle_cb(ctx->trickle_cb_arg, ctx, stream, component_id, NULL); - } + if (nr_ice_media_stream_is_done_gathering(stream) && + ctx->trickle_cb) { + ctx->trickle_cb(ctx->trickle_cb_arg, ctx, stream, component_id, NULL); } if(ctx->uninitialized_candidates==0){ From 22ef96ff2f800a48430c88e9dbf7cb961464bbe6 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 19 Nov 2020 20:29:21 +0000 Subject: [PATCH 14/41] Bug 1677759: Mark STUN/TURN client contexts failed when we encounter a socket read/write failure for TCP. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D97637 --- src/ice/ice_socket.c | 40 +++++++++++++++++++++++++----- src/ice/ice_socket.h | 1 + src/stun/nr_socket_buffered_stun.c | 6 +++++ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/ice/ice_socket.c b/src/ice/ice_socket.c index c01f465..a6c8513 100644 --- a/src/ice/ice_socket.c +++ b/src/ice/ice_socket.c @@ -57,17 +57,21 @@ static void nr_ice_socket_readable_cb(NR_SOCKET s, int how, void *cb_arg) r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): Socket ready to read",sock->ctx->label); - /* Re-arm first! */ - if (sock->type != NR_ICE_SOCKET_TYPE_STREAM_TCP) { - r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): rearming",sock->ctx->label); - NR_ASYNC_WAIT(s,how,nr_ice_socket_readable_cb,cb_arg); - } - if(r=nr_socket_recvfrom(sock->sock,buf,sizeof(buf),&len_s,0,&addr)){ if (r != R_WOULDBLOCK && (sock->type != NR_ICE_SOCKET_TYPE_DGRAM)) { /* Report this error upward. Bug 946423 */ r_log(LOG_ICE,LOG_ERR,"ICE(%s): Error %d on reliable socket(%p). Abandoning.",sock->ctx->label, r, s); + nr_ice_socket_failed(sock); + return; } + } + + if (sock->type != NR_ICE_SOCKET_TYPE_STREAM_TCP) { + r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): rearming",sock->ctx->label); + NR_ASYNC_WAIT(s,how,nr_ice_socket_readable_cb,cb_arg); + } + + if (r) { return; } @@ -374,3 +378,27 @@ int nr_ice_socket_deregister(nr_ice_socket *sock, void *handle) return(0); } +void nr_ice_socket_failed(nr_ice_socket *sock) + { + nr_ice_stun_ctx *s1,*s2; + TAILQ_FOREACH_SAFE(s1, &sock->stun_ctxs, entry, s2){ + switch (s1->type) { + case NR_ICE_STUN_NONE: + break; + case NR_ICE_STUN_CLIENT: + nr_stun_client_failed(s1->u.client); + break; + case NR_ICE_STUN_SERVER: + /* Nothing to do here? */ + break; +#ifdef USE_TURN + case NR_ICE_TURN_CLIENT: + nr_turn_client_failed(s1->u.turn_client.turn_client); + break; +#endif + default: + assert(0); + } + } + } + diff --git a/src/ice/ice_socket.h b/src/ice/ice_socket.h index b0fba1e..9b73a26 100644 --- a/src/ice/ice_socket.h +++ b/src/ice/ice_socket.h @@ -89,6 +89,7 @@ int nr_ice_socket_register_stun_client(nr_ice_socket *sock, nr_stun_client_ctx * int nr_ice_socket_register_stun_server(nr_ice_socket *sock, nr_stun_server_ctx *srv,void **handle); int nr_ice_socket_register_turn_client(nr_ice_socket *sock, nr_turn_client_ctx *srv,nr_socket *turn_socket, void **handle); int nr_ice_socket_deregister(nr_ice_socket *sock, void *handle); +void nr_ice_socket_failed(nr_ice_socket *sock); #ifdef __cplusplus } diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index c82c545..5994862 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -604,6 +604,12 @@ static void nr_socket_buffered_stun_writable_cb(NR_SOCKET s, int how, void *arg) if (_status && _status != R_WOULDBLOCK) { r_log(LOG_GENERIC, LOG_ERR, "Failure in writable_cb: %d", _status); nr_socket_buffered_stun_failed(sock); + /* Report this failure up; the only way to do this is a readable callback. + * Once the user tries to read (using nr_socket_buffered_stun_recvfrom), it + * will notice that there has been a failure. */ + if (sock->readable_cb) { + sock->readable_cb(s, NR_ASYNC_WAIT_READ, sock->readable_cb_arg); + } } else if (sock->pending) { nr_socket_buffered_stun_arm_writable_cb(sock); } From 30af8132c14152281c4b089c21741859e01b918d Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Fri, 13 Nov 2020 17:03:20 +0000 Subject: [PATCH 15/41] Bug 1665161: Prefer IFA_LOCAL if present, and fall back to IFA_ADDRESS if not. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D97015 --- src/stun/addrs-netlink.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/stun/addrs-netlink.c b/src/stun/addrs-netlink.c index ddecc34..73e85c6 100644 --- a/src/stun/addrs-netlink.c +++ b/src/stun/addrs-netlink.c @@ -94,7 +94,7 @@ static int get_siocgifflags(nr_local_addr *addr) { } static int set_sockaddr(nr_local_addr *addr, struct ifaddrmsg* msg, struct rtattr* rta) { - assert(rta->rta_type == IFA_ADDRESS); + assert(rta->rta_type == IFA_ADDRESS || rta->rta_type == IFA_LOCAL); void *data = RTA_DATA(rta); size_t len = RTA_PAYLOAD(rta); if (msg->ifa_family == AF_INET) { @@ -235,15 +235,27 @@ stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) (struct ifaddrmsg*)NLMSG_DATA(header); struct rtattr* rta = IFA_RTA(address_msg); ssize_t payload_len = IFA_PAYLOAD(header); + bool found = false; while (RTA_OK(rta, payload_len)) { - if (rta->rta_type == IFA_ADDRESS) { + // This is a bit convoluted. IFA_ADDRESS and IFA_LOCAL are the + // same thing except when using a POINTTOPOINT interface, in + // which case IFA_LOCAL is the local address, and IFA_ADDRESS is + // the remote address. In a reasonable world, that would mean we + // could just use IFA_LOCAL all the time. Sadly, IFA_LOCAL is not + // always set (IPv6 in particular). So, we have to be on the + // lookout for both, and prefer IFA_LOCAL when present. + if (rta->rta_type == IFA_ADDRESS || rta->rta_type == IFA_LOCAL) { int family = address_msg->ifa_family; if ((family == AF_INET || family == AF_INET6) && - !stun_ifaddr_is_disallowed_v6(address_msg->ifa_flags)) { - if (stun_convert_netlink(&addrs[*count], address_msg, rta)) { - assert(0); - } else { - ++(*count); + !stun_ifaddr_is_disallowed_v6(address_msg->ifa_flags) && + !stun_convert_netlink(&addrs[*count], address_msg, rta)) { + found = true; + if (rta->rta_type == IFA_LOCAL) { + // IFA_LOCAL is what we really want; if we find it we're + // done. If this is IFA_ADDRESS instead, we do not proceed + // yet, and allow a subsequent IFA_LOCAL to overwrite what + // we just put in |addrs|. + break; } } } @@ -252,6 +264,10 @@ stun_getaddrs_filtered(nr_local_addr addrs[], int maxaddrs, int *count) * ifaddrmsg, and set the label on all of them. */ rta = RTA_NEXT(rta, payload_len); } + + if (found) { + ++(*count); + } break; } } From 6e84c563ebf8a28cfeedcd1b378046881559397c Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 1 Dec 2020 22:51:41 +0000 Subject: [PATCH 16/41] Bug 1678484: Start the grace period timer before pairing a candidate, just in case that candidate fails immediately. r=drno Differential Revision: https://phabricator.services.mozilla.com/D97701 --- src/ice/ice_peer_ctx.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/ice/ice_peer_ctx.c b/src/ice/ice_peer_ctx.c index f09f6eb..aefb82d 100644 --- a/src/ice/ice_peer_ctx.c +++ b/src/ice/ice_peer_ctx.c @@ -311,17 +311,17 @@ int nr_ice_peer_ctx_parse_trickle_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_ just re-pair the stream which is inefficient but still fine because we suppress duplicate pairing */ if (needs_pairing) { - if(r=nr_ice_media_stream_pair_candidates(pctx, stream, pstream)) { - r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s), stream(%s) failed to pair trickle ICE candidates",pctx->ctx->label,pctx->label,stream->label); - ABORT(r); - } - /* Start the remote trickle grace timeout if it hasn't been started by another trickled candidate or from the SDP. */ if (!pctx->trickle_grace_period_timer) { nr_ice_peer_ctx_start_trickle_timer(pctx); } + if(r=nr_ice_media_stream_pair_candidates(pctx, stream, pstream)) { + r_log(LOG_ICE,LOG_ERR,"ICE(%s): peer (%s), stream(%s) failed to pair trickle ICE candidates",pctx->ctx->label,pctx->label,stream->label); + ABORT(r); + } + /* Start checks if this stream is not checking yet or if it has checked all the available candidates but not had a completed check for all components. @@ -438,15 +438,15 @@ int nr_ice_peer_ctx_pair_new_trickle_candidate(nr_ice_ctx *ctx, nr_ice_peer_ctx if ((r = nr_ice_peer_ctx_find_pstream(pctx, cand->stream, &pstream))) ABORT(r); - if ((r = nr_ice_media_stream_pair_new_trickle_candidate(pctx, pstream, cand))) - ABORT(r); - /* Start the remote trickle grace timeout if it hasn't been started already. */ if (!pctx->trickle_grace_period_timer) { nr_ice_peer_ctx_start_trickle_timer(pctx); } + if ((r = nr_ice_media_stream_pair_new_trickle_candidate(pctx, pstream, cand))) + ABORT(r); + _status=0; abort: return _status; @@ -539,11 +539,9 @@ int nr_ice_peer_ctx_start_checks2(nr_ice_peer_ctx *pctx, int allow_non_first) nr_ice_media_stream *stream; int started = 0; - /* Ensure that any existing grace period timers are cancelled */ - if(pctx->trickle_grace_period_timer) { - NR_async_timer_cancel(pctx->trickle_grace_period_timer); - pctx->trickle_grace_period_timer=0; - } + /* Ensure that grace period timer is running. We might cancel this if we + * didn't actually start any pairs. */ + nr_ice_peer_ctx_start_trickle_timer(pctx); /* Might have added some streams */ pctx->reported_connected = 0; @@ -622,12 +620,11 @@ int nr_ice_peer_ctx_start_checks2(nr_ice_peer_ctx *pctx, int allow_non_first) if (!started) { r_log(LOG_ICE,LOG_NOTICE,"ICE(%s): peer (%s) no checks to start",pctx->ctx->label,pctx->label); + /* Never mind on the grace period timer */ + NR_async_timer_cancel(pctx->trickle_grace_period_timer); + pctx->trickle_grace_period_timer=0; ABORT(R_NOT_FOUND); } - else { - /* Start grace period timer for more remote trickle candidates. */ - nr_ice_peer_ctx_start_trickle_timer(pctx); - } _status=0; abort: From 7631d3e674b5e26a5a1ea3322e0f83914684838a Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 19 Nov 2020 19:08:30 +0000 Subject: [PATCH 17/41] Bug 1626278: Stop requiring additional dispatches to STS for ICE ctx teardown. r=mjf Also some improvements in discipline wrt bare pointers. Differential Revision: https://phabricator.services.mozilla.com/D97016 --- src/ice/ice_ctx.c | 45 +++++++++++++++--------------------------- src/ice/ice_ctx.h | 2 +- src/ice/ice_peer_ctx.c | 28 ++++++++------------------ src/ice/ice_peer_ctx.h | 2 +- 4 files changed, 26 insertions(+), 51 deletions(-) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index 5b6e03f..cac836e 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -62,7 +62,6 @@ static int nr_ice_fetch_stun_servers(int ct, nr_ice_stun_server **out); #ifdef USE_TURN static int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out); #endif /* USE_TURN */ -static void nr_ice_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg); static int nr_ice_ctx_pair_new_trickle_candidates(nr_ice_ctx *ctx, nr_ice_candidate *cand); static int no_op(void **obj) { return 0; @@ -419,20 +418,31 @@ int nr_ice_ctx_create(char *label, UINT4 flags, nr_ice_ctx **ctxp) _status=0; abort: - if(_status && ctx) - nr_ice_ctx_destroy_cb(0,0,ctx); + if (_status && ctx) nr_ice_ctx_destroy(&ctx); return(_status); } -static void nr_ice_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg) - { - nr_ice_ctx *ctx=cb_arg; + void nr_ice_ctx_add_flags(nr_ice_ctx* ctx, UINT4 flags) { + ctx->flags |= flags; + } + + void nr_ice_ctx_remove_flags(nr_ice_ctx* ctx, UINT4 flags) { + ctx->flags &= ~flags; + } + + void nr_ice_ctx_destroy(nr_ice_ctx** ctxp) { + if (!ctxp || !*ctxp) return; + + nr_ice_ctx* ctx = *ctxp; nr_ice_foundation *f1,*f2; nr_ice_media_stream *s1,*s2; int i; nr_ice_stun_id *id1,*id2; + ctx->done_cb = 0; + ctx->trickle_cb = 0; + STAILQ_FOREACH_SAFE(s1, &ctx->streams, entry, s2){ STAILQ_REMOVE(&ctx->streams,s1,nr_ice_media_stream_,entry); nr_ice_media_stream_destroy(&s1); @@ -469,31 +479,8 @@ static void nr_ice_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg) nr_socket_factory_destroy(&ctx->socket_factory); RFREE(ctx); - } - -void nr_ice_ctx_add_flags(nr_ice_ctx *ctx, UINT4 flags) - { - ctx->flags |= flags; - } - -void nr_ice_ctx_remove_flags(nr_ice_ctx *ctx, UINT4 flags) - { - ctx->flags &= ~flags; - } - -int nr_ice_ctx_destroy(nr_ice_ctx **ctxp) - { - if(!ctxp || !*ctxp) - return(0); - - (*ctxp)->done_cb=0; - (*ctxp)->trickle_cb=0; - - NR_ASYNC_SCHEDULE(nr_ice_ctx_destroy_cb,*ctxp); *ctxp=0; - - return(0); } void nr_ice_gather_finished_cb(NR_SOCKET s, int h, void *cb_arg) diff --git a/src/ice/ice_ctx.h b/src/ice/ice_ctx.h index 66cbfaa..015ee90 100644 --- a/src/ice/ice_ctx.h +++ b/src/ice/ice_ctx.h @@ -176,7 +176,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); +void 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_set_target_for_default_local_address_lookup(nr_ice_ctx *ctx, const char *target_ip, UINT2 target_port); int nr_ice_gather(nr_ice_ctx *ctx, NR_async_cb done_cb, void *cb_arg); diff --git a/src/ice/ice_peer_ctx.c b/src/ice/ice_peer_ctx.c index aefb82d..eb49f64 100644 --- a/src/ice/ice_peer_ctx.c +++ b/src/ice/ice_peer_ctx.c @@ -42,7 +42,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "async_timer.h" #include "ice_reg.h" -static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg); static void nr_ice_peer_ctx_parse_stream_attributes_int(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream *pstream, char **attrs, int attr_ct); static int nr_ice_ctx_parse_candidate(nr_ice_peer_ctx *pctx, nr_ice_media_stream *pstream, char *candidate, int trickled, const char *mdns_addr); static void nr_ice_peer_ctx_start_trickle_timer(nr_ice_peer_ctx *pctx); @@ -81,7 +80,7 @@ int nr_ice_peer_ctx_create(nr_ice_ctx *ctx, nr_ice_handler *handler,char *label, _status = 0; abort: if(_status){ - nr_ice_peer_ctx_destroy_cb(0,0,pctx); + nr_ice_peer_ctx_destroy(&pctx); } return(_status); } @@ -476,11 +475,15 @@ int nr_ice_peer_ctx_disable_component(nr_ice_peer_ctx *pctx, nr_ice_media_stream return(_status); } -static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg) - { - nr_ice_peer_ctx *pctx=cb_arg; + void nr_ice_peer_ctx_destroy(nr_ice_peer_ctx** pctxp) { + if (!pctxp || !*pctxp) return; + + nr_ice_peer_ctx* pctx = *pctxp; nr_ice_media_stream *str1,*str2; + /* Stop calling the handler */ + pctx->handler = 0; + NR_async_timer_cancel(pctx->connected_cb_timer); RFREE(pctx->label); @@ -498,25 +501,10 @@ static void nr_ice_peer_ctx_destroy_cb(NR_SOCKET s, int how, void *cb_arg) } RFREE(pctx); - } - -int nr_ice_peer_ctx_destroy(nr_ice_peer_ctx **pctxp) - { - - if(!pctxp || !*pctxp) - return(0); - - /* Stop calling the handler */ - (*pctxp)->handler = 0; - - NR_ASYNC_SCHEDULE(nr_ice_peer_ctx_destroy_cb,*pctxp); *pctxp=0; - - return(0); } - /* Start the checks for the first media stream (S 5.7) The rest remain FROZEN */ int nr_ice_peer_ctx_start_checks(nr_ice_peer_ctx *pctx) diff --git a/src/ice/ice_peer_ctx.h b/src/ice/ice_peer_ctx.h index 93853ea..ec73d96 100644 --- a/src/ice/ice_peer_ctx.h +++ b/src/ice/ice_peer_ctx.h @@ -70,7 +70,7 @@ struct nr_ice_peer_ctx_ { typedef STAILQ_HEAD(nr_ice_peer_ctx_head_, nr_ice_peer_ctx_) nr_ice_peer_ctx_head; int nr_ice_peer_ctx_create(nr_ice_ctx *ctx, nr_ice_handler *handler,char *label, nr_ice_peer_ctx **pctxp); -int nr_ice_peer_ctx_destroy(nr_ice_peer_ctx **pctxp); +void nr_ice_peer_ctx_destroy(nr_ice_peer_ctx** pctxp); int nr_ice_peer_ctx_parse_stream_attributes(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, char **attrs, int attr_ct); int nr_ice_peer_ctx_find_pstream(nr_ice_peer_ctx *pctx, nr_ice_media_stream *stream, nr_ice_media_stream **pstreamp); int nr_ice_peer_ctx_remove_pstream(nr_ice_peer_ctx *pctx, nr_ice_media_stream **pstreamp); From 9944ea4c0b3b7d634bdcc083e1348bf157252e12 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Tue, 5 Jan 2021 12:59:41 +0000 Subject: [PATCH 18/41] Bug 1680771: Use a real LoadInfo for WebrtcTCPSocket, ensure we use that LoadInfo's CookieJarSettings for our DNS/proxy lookup, and remove a flag that was breaking http proxy support for webrtc. r=timhuang,mjf Differential Revision: https://phabricator.services.mozilla.com/D99362 --- src/ice/ice_candidate.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index d3c8d00..3c02181 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -271,7 +271,7 @@ int nr_ice_peer_peer_rflx_candidate_create(nr_ice_ctx *ctx,char *label, nr_ice_c static void nr_ice_candidate_mark_done(nr_ice_candidate *cand, int state) { - if (!cand || !cand->done_cb) { + if (!cand) { assert(0); return; } @@ -294,7 +294,9 @@ static void nr_ice_candidate_mark_done(nr_ice_candidate *cand, int state) cand->done_cb=0; cand->state=state; /* This might destroy cand! */ - done_cb(0,0,cand->cb_arg); + if (done_cb) { + done_cb(0,0,cand->cb_arg); + } } int nr_ice_candidate_destroy(nr_ice_candidate **candp) From 8b33e12cc13f646800f2de86765242099a1aeac0 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 14 Jan 2021 19:00:25 +0000 Subject: [PATCH 19/41] Bug 1416220: Modify mtransport to delegate DNS lookups for TCP sockets to the socket class, instead of using nr_resolver. r=mjf * Modified nr_transport_addr to allow it to represent an fqdn/ip-version/protocol tuple, and taught nICEr to handle the fqdn case appropriately. * Since nr_transport_addr can represent an fqdn, nr_ice_stun_server did not need this ability anymore, and was significantly simplified. * Taught NrIceCtx to handle creation of a V4/V6 pair of nr_ice_stun_server when an fqdn is used, instead of having nICEr create pairs later. Depends on D101657 Differential Revision: https://phabricator.services.mozilla.com/D101658 --- src/ice/ice_candidate.c | 81 ++++++++++++++++-------------- src/ice/ice_component.c | 51 +++++++------------ src/ice/ice_ctx.c | 9 ++-- src/ice/ice_ctx.h | 11 +--- src/net/nr_socket_multi_tcp.c | 40 +++++++++------ src/net/transport_addr.c | 47 +++++++++-------- src/net/transport_addr.h | 3 +- src/stun/nr_socket_buffered_stun.c | 6 +-- 8 files changed, 122 insertions(+), 126 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index 3c02181..c887fe2 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -106,26 +106,11 @@ static const char *nr_tcp_type_name(nr_socket_tcp_type tcp_type) { static int nr_ice_candidate_format_stun_label(char *label, size_t size, nr_ice_candidate *cand) { - int _status; - - *label = 0; - switch(cand->stun_server->type) { - case NR_ICE_STUN_SERVER_TYPE_ADDR: - snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), cand->base.as_string, - cand->stun_server->u.addr.as_string); - break; - case NR_ICE_STUN_SERVER_TYPE_DNSNAME: - snprintf(label, size, "%s(%s|%s:%u)", nr_ctype_name(cand->type), cand->base.as_string, - cand->stun_server->u.dnsname.host, cand->stun_server->u.dnsname.port); - break; - default: - assert(0); - ABORT(R_BAD_ARGS); - } + *label = 0; + snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), + cand->base.as_string, cand->stun_server->addr.as_string); - _status=0; - abort: - return(_status); + return (0); } int nr_ice_candidate_create(nr_ice_ctx *ctx,nr_ice_component *comp,nr_ice_socket *isock, nr_socket *osock, nr_ice_candidate_type ctype, nr_socket_tcp_type tcp_type, nr_ice_stun_server *stun_server, UCHAR component_id, nr_ice_candidate **candp) @@ -605,28 +590,40 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo /* Fall through */ #endif case SERVER_REFLEXIVE: - if(cand->stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if(nr_transport_addr_cmp(&cand->base,&cand->stun_server->u.addr,NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { - r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because of IP version/transport mis-match with STUN/TURN server (%u/%s - %u/%s).", cand->label,cand->base.ip_version,cand->base.protocol==IPPROTO_UDP?"UDP":"TCP",cand->stun_server->u.addr.ip_version,cand->stun_server->u.addr.protocol==IPPROTO_UDP?"UDP":"TCP"); - ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ - } + if (nr_transport_addr_cmp(&cand->base, &cand->stun_server->addr, + NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { + r_log(LOG_ICE, LOG_INFO, + "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because " + "of IP version/transport mis-match with STUN/TURN server " + "(%u/%s - %u/%s).", + cand->label, cand->base.ip_version, + cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", + cand->stun_server->addr.ip_version, + cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" + : "TCP"); + ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ + } - /* Just copy the address */ - if (r=nr_transport_addr_copy(&cand->stun_server_addr, - &cand->stun_server->u.addr)) { - r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); + if(cand->stun_server->addr.fqdn[0] != 0 && + cand->stun_server->addr.protocol == IPPROTO_UDP) { + /* UDP with FQDN, allow resolver to handle this for now, we + * eventually want to allow the UDP socket to handle this for us. */ + r_log( + LOG_ICE, LOG_DEBUG, + "ICE-CANDIDATE(%s): Starting DNS resolution (%u/%s - %u/%s).", + cand->label, cand->base.ip_version, + cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", + cand->stun_server->addr.ip_version, + cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" : "TCP"); + nr_resolver_resource resource; + int port; + resource.domain_name = cand->stun_server->addr.fqdn; + if (r = nr_transport_addr_get_port(&cand->stun_server->addr, &port)) { ABORT(r); } - - if(r=nr_ice_candidate_initialize2(cand)) - ABORT(r); - } - else { - nr_resolver_resource resource; - resource.domain_name=cand->stun_server->u.dnsname.host; - resource.port=cand->stun_server->u.dnsname.port; + resource.port = (uint16_t)port; resource.stun_turn=protocol; - resource.transport_protocol=cand->stun_server->transport; + resource.transport_protocol = cand->stun_server->addr.protocol; switch (cand->base.ip_version) { case NR_IPV4: @@ -653,6 +650,16 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not invoke DNS resolver",cand->label); ABORT(r); } + } else { + /* No DNS handling needed, just copy the address and finish init */ + if (r = nr_transport_addr_copy(&cand->stun_server_addr, + &cand->stun_server->addr)) { + r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); + ABORT(r); + } + + if(r=nr_ice_candidate_initialize2(cand)) + ABORT(r); } break; default: diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 0083778..054d0b1 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -302,16 +302,12 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And a srvrflx candidate for each STUN server */ for(j=0;jstun_server_ct;j++){ /* Skip non-UDP */ - if(ctx->stun_servers[j].transport!=IPPROTO_UDP) - continue; + if (ctx->stun_servers[j].addr.protocol != IPPROTO_UDP) continue; - if (ctx->stun_servers[j].type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, - &ctx->stun_servers[j].u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, &ctx->stun_servers[j].addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of address type mis-match",ctx->label); + continue; } /* Ensure id is set (nr_ice_ctx_set_stun_servers does not) */ @@ -341,16 +337,13 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_candidate *srvflx_cand=0; /* Skip non-UDP */ - if (ctx->turn_servers[j].turn_server.transport != IPPROTO_UDP) + if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_UDP) continue; - if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, - &ctx->turn_servers[j].turn_server.u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, &ctx->turn_servers[j].turn_server.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); + continue; } if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) { @@ -548,8 +541,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And srvrflx candidates for each STUN server */ for(j=0;jstun_server_ct;j++){ - if (ctx->stun_servers[j].transport!=IPPROTO_TCP) - continue; + if (ctx->stun_servers[j].addr.protocol != IPPROTO_TCP) continue; if (isock_psv) { if(r=nr_ice_candidate_create(ctx,component, @@ -583,7 +575,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_socket *turn_isock; /* Skip non-TCP */ - if (ctx->turn_servers[j].turn_server.transport != IPPROTO_TCP) + if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_TCP) continue; /* Create relay candidate */ @@ -591,13 +583,10 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon ABORT(r); addr.protocol = IPPROTO_TCP; - if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addr, - &ctx->turn_servers[j].turn_server.u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addr, &ctx->turn_servers[j].turn_server.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); + continue; } if (!ice_tcp_disabled) { @@ -623,11 +612,9 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon } } - /* If we're going to use TLS, make sure that's recorded */ - if (ctx->turn_servers[j].turn_server.tls) { - strncpy(addr.tls_host, - ctx->turn_servers[j].turn_server.u.dnsname.host, - sizeof(addr.tls_host) - 1); + if (ctx->turn_servers[j].turn_server.addr.fqdn[0] != 0) { + /* If we're going to use TLS, make sure that's recorded */ + addr.tls = ctx->turn_servers[j].turn_server.addr.tls; } if ((r=nr_transport_addr_fmt_addr_string(&addr))) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index cac836e..1348f0c 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -101,10 +101,9 @@ int nr_ice_fetch_stun_servers(int ct, nr_ice_stun_server **out) ABORT(r); port = 3478; } - if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].u.addr)) + if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].addr)) ABORT(r); - servers[i].type = NR_ICE_STUN_SERVER_TYPE_ADDR; RFREE(addr); addr=0; } @@ -281,8 +280,8 @@ int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out) ABORT(r); port = 3478; } - if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].turn_server.u.addr)) + if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].turn_server.addr)) ABORT(r); diff --git a/src/ice/ice_ctx.h b/src/ice/ice_ctx.h index 015ee90..e61c506 100644 --- a/src/ice/ice_ctx.h +++ b/src/ice/ice_ctx.h @@ -53,17 +53,8 @@ extern "C" { #define NR_ICE_STUN_SERVER_TYPE_DNSNAME 2 typedef struct nr_ice_stun_server_ { - int type; - union { - nr_transport_addr addr; - struct { - char host[256]; /* Limit from RFC 1034, plus a 0 byte */ - UINT2 port; - } dnsname; - } u; + nr_transport_addr addr; int id; - int transport; - int tls; /* Whether to use TLS or not */ } nr_ice_stun_server; typedef struct nr_ice_turn_server_ { diff --git a/src/net/nr_socket_multi_tcp.c b/src/net/nr_socket_multi_tcp.c index 12bbea7..c555e32 100644 --- a/src/net/nr_socket_multi_tcp.c +++ b/src/net/nr_socket_multi_tcp.c @@ -169,14 +169,20 @@ static int nr_socket_multi_tcp_create_stun_server_socket( nr_tcp_socket_ctx *tcp_socket_ctx=0; nr_socket * nrsock; - if (stun_server->transport!=IPPROTO_TCP) { - r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping UDP STUN server(addr:%s)",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.as_string); + if (stun_server->addr.protocol != IPPROTO_TCP) { + r_log(LOG_ICE, LOG_INFO, + "%s:%d function %s skipping UDP STUN server(addr:%s)", __FILE__, + __LINE__, __FUNCTION__, stun_server->addr.as_string); ABORT(R_BAD_ARGS); } - if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR && - nr_transport_addr_cmp(&stun_server->u.addr,addr,NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { - r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping STUN with different IP version (%u) than local socket (%u),",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.ip_version,addr->ip_version); + if (nr_transport_addr_cmp(&stun_server->addr, addr, + NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { + r_log(LOG_ICE, LOG_INFO, + "%s:%d function %s skipping STUN with different IP version (%u) " + "than local socket (%u),", + __FILE__, __LINE__, __FUNCTION__, stun_server->addr.ip_version, + addr->ip_version); ABORT(R_BAD_ARGS); } @@ -187,20 +193,22 @@ static int nr_socket_multi_tcp_create_stun_server_socket( if ((r=nr_tcp_socket_ctx_create(nrsock, 0, max_pending, &tcp_socket_ctx))) ABORT(r); - if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - nr_transport_addr stun_server_addr; + nr_transport_addr stun_server_addr; - nr_transport_addr_copy(&stun_server_addr, &stun_server->u.addr); - r=nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); - if (r && r!=R_WOULDBLOCK) { - r_log(LOG_ICE,LOG_WARNING,"%s:%d function %s connect to STUN server(addr:%s) failed with error %d",__FILE__,__LINE__,__FUNCTION__,stun_server_addr.as_string,r); - ABORT(r); - } - - if ((r=nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, sock))) - ABORT(r); + nr_transport_addr_copy(&stun_server_addr, &stun_server->addr); + r = nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); + if (r && r != R_WOULDBLOCK) { + r_log(LOG_ICE, LOG_WARNING, + "%s:%d function %s connect to STUN server(addr:%s) failed with " + "error %d", + __FILE__, __LINE__, __FUNCTION__, stun_server_addr.as_string, r); + ABORT(r); } + if ((r = nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, + sock))) + ABORT(r); + TAILQ_INSERT_TAIL(&sock->sockets, tcp_socket_ctx, entry); _status=0; diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index 4cec4e6..b900708 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -57,7 +57,7 @@ int nr_transport_addr_fmt_addr_string(nr_transport_addr *addr) switch(addr->protocol){ case IPPROTO_TCP: - if (addr->tls_host[0]) { + if (addr->tls) { protocol = "TLS"; } else { protocol = "TCP"; @@ -279,24 +279,28 @@ int nr_ip6_port_to_transport_addr(struct in6_addr* addr6, UINT2 port, int protoc int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, int maxlen) { int _status; - const char *res; - switch(addr->ip_version){ - case NR_IPV4: - res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr,str,maxlen); - break; - case NR_IPV6: - res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr,str,maxlen); - break; - default: - ABORT(R_INTERNAL); - } + if (addr->fqdn[0]) { + strncpy(str, addr->fqdn, maxlen); + } else { + const char* res; + switch (addr->ip_version) { + case NR_IPV4: + res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr, str, maxlen); + break; + case NR_IPV6: + res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr, str, maxlen); + break; + default: + ABORT(R_INTERNAL); + } - if(!res){ - if (errno == ENOSPC){ - ABORT(R_BAD_ARGS); + if (!res) { + if (errno == ENOSPC) { + ABORT(R_BAD_ARGS); + } + ABORT(R_INTERNAL); } - ABORT(R_INTERNAL); } _status=0; @@ -485,10 +489,13 @@ int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport (addr1->protocol != addr2->protocol)) { return(1); } - // now make sure the link local status matches - if (nr_transport_addr_is_link_local(addr1) != - nr_transport_addr_is_link_local(addr2)) { - return(1); + + if (!addr1->fqdn[0] && !addr2->fqdn[0]) { + // now make sure the link local status matches + if (nr_transport_addr_is_link_local(addr1) != + nr_transport_addr_is_link_local(addr2)) { + return(1); + } } return(0); } diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index ac8970c..aa8b695 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -69,8 +69,9 @@ typedef struct nr_transport_addr_ { /* A string version. 56 = 5 ("IP6:[") + 39 (ipv6 address) + 2 ("]:") + 5 (port) + 4 (/UDP) + 1 (null) */ char as_string[56]; - char tls_host[256]; + char fqdn[256]; bool is_proxied; + bool tls; } nr_transport_addr; typedef struct nr_transport_addr_mask_ { diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index 5994862..68e2740 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -346,11 +346,7 @@ static int nr_socket_buffered_stun_recvfrom(void *obj,void * restrict buf, sock->read_state = NR_ICE_SOCKET_READ_NONE; sock->bytes_needed = (sock->framing_type == ICE_TCP_FRAMING) ? sizeof(nr_frame_header) : sizeof(nr_stun_message_header); - assert(!nr_transport_addr_is_wildcard(&sock->remote_addr)); - if (!nr_transport_addr_is_wildcard(&sock->remote_addr)) { - if ((r=nr_transport_addr_copy(from, &sock->remote_addr))) - ABORT(r); - } + if ((r = nr_transport_addr_copy(from, &sock->remote_addr))) ABORT(r); _status=0; abort: From cf69973b6836371de5d92b288cc3a55a544d513d Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 14 Jan 2021 19:02:27 +0000 Subject: [PATCH 20/41] Bug 1416220: Add/improve some logging, and add an assertion. r=mjf Depends on D101659 Differential Revision: https://phabricator.services.mozilla.com/D101660 --- src/ice/ice_candidate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index c887fe2..fb0aef5 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -633,6 +633,7 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo resource.address_family=AF_INET6; break; default: + assert(0); ABORT(R_BAD_ARGS); } From 576858ee35dcc4193115bc37e2fa5d5fea52c8d0 Mon Sep 17 00:00:00 2001 From: Narcis Beleuzu Date: Thu, 14 Jan 2021 23:13:14 +0200 Subject: [PATCH 21/41] Backed out 6 changesets (bug 1416220) for GTest failure on TestGatherDNSStunBogusHostnameTcp Backed out changeset cf60a787b5ad (bug 1416220) Backed out changeset 8e9a13c508c6 (bug 1416220) Backed out changeset 8720dfa5a6e0 (bug 1416220) Backed out changeset d4cadef3d55d (bug 1416220) Backed out changeset d6ec5692e05e (bug 1416220) Backed out changeset a2069e46d965 (bug 1416220) --- src/ice/ice_candidate.c | 82 ++++++++++++++---------------- src/ice/ice_component.c | 51 ++++++++++++------- src/ice/ice_ctx.c | 9 ++-- src/ice/ice_ctx.h | 11 +++- src/net/nr_socket_multi_tcp.c | 40 ++++++--------- src/net/transport_addr.c | 47 ++++++++--------- src/net/transport_addr.h | 3 +- src/stun/nr_socket_buffered_stun.c | 6 ++- 8 files changed, 126 insertions(+), 123 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index fb0aef5..3c02181 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -106,11 +106,26 @@ static const char *nr_tcp_type_name(nr_socket_tcp_type tcp_type) { static int nr_ice_candidate_format_stun_label(char *label, size_t size, nr_ice_candidate *cand) { - *label = 0; - snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), - cand->base.as_string, cand->stun_server->addr.as_string); + int _status; + + *label = 0; + switch(cand->stun_server->type) { + case NR_ICE_STUN_SERVER_TYPE_ADDR: + snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), cand->base.as_string, + cand->stun_server->u.addr.as_string); + break; + case NR_ICE_STUN_SERVER_TYPE_DNSNAME: + snprintf(label, size, "%s(%s|%s:%u)", nr_ctype_name(cand->type), cand->base.as_string, + cand->stun_server->u.dnsname.host, cand->stun_server->u.dnsname.port); + break; + default: + assert(0); + ABORT(R_BAD_ARGS); + } - return (0); + _status=0; + abort: + return(_status); } int nr_ice_candidate_create(nr_ice_ctx *ctx,nr_ice_component *comp,nr_ice_socket *isock, nr_socket *osock, nr_ice_candidate_type ctype, nr_socket_tcp_type tcp_type, nr_ice_stun_server *stun_server, UCHAR component_id, nr_ice_candidate **candp) @@ -590,40 +605,28 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo /* Fall through */ #endif case SERVER_REFLEXIVE: - if (nr_transport_addr_cmp(&cand->base, &cand->stun_server->addr, - NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { - r_log(LOG_ICE, LOG_INFO, - "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because " - "of IP version/transport mis-match with STUN/TURN server " - "(%u/%s - %u/%s).", - cand->label, cand->base.ip_version, - cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", - cand->stun_server->addr.ip_version, - cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" - : "TCP"); - ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ - } + if(cand->stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { + if(nr_transport_addr_cmp(&cand->base,&cand->stun_server->u.addr,NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { + r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because of IP version/transport mis-match with STUN/TURN server (%u/%s - %u/%s).", cand->label,cand->base.ip_version,cand->base.protocol==IPPROTO_UDP?"UDP":"TCP",cand->stun_server->u.addr.ip_version,cand->stun_server->u.addr.protocol==IPPROTO_UDP?"UDP":"TCP"); + ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ + } - if(cand->stun_server->addr.fqdn[0] != 0 && - cand->stun_server->addr.protocol == IPPROTO_UDP) { - /* UDP with FQDN, allow resolver to handle this for now, we - * eventually want to allow the UDP socket to handle this for us. */ - r_log( - LOG_ICE, LOG_DEBUG, - "ICE-CANDIDATE(%s): Starting DNS resolution (%u/%s - %u/%s).", - cand->label, cand->base.ip_version, - cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", - cand->stun_server->addr.ip_version, - cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" : "TCP"); - nr_resolver_resource resource; - int port; - resource.domain_name = cand->stun_server->addr.fqdn; - if (r = nr_transport_addr_get_port(&cand->stun_server->addr, &port)) { + /* Just copy the address */ + if (r=nr_transport_addr_copy(&cand->stun_server_addr, + &cand->stun_server->u.addr)) { + r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); ABORT(r); } - resource.port = (uint16_t)port; + + if(r=nr_ice_candidate_initialize2(cand)) + ABORT(r); + } + else { + nr_resolver_resource resource; + resource.domain_name=cand->stun_server->u.dnsname.host; + resource.port=cand->stun_server->u.dnsname.port; resource.stun_turn=protocol; - resource.transport_protocol = cand->stun_server->addr.protocol; + resource.transport_protocol=cand->stun_server->transport; switch (cand->base.ip_version) { case NR_IPV4: @@ -633,7 +636,6 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo resource.address_family=AF_INET6; break; default: - assert(0); ABORT(R_BAD_ARGS); } @@ -651,16 +653,6 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not invoke DNS resolver",cand->label); ABORT(r); } - } else { - /* No DNS handling needed, just copy the address and finish init */ - if (r = nr_transport_addr_copy(&cand->stun_server_addr, - &cand->stun_server->addr)) { - r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); - ABORT(r); - } - - if(r=nr_ice_candidate_initialize2(cand)) - ABORT(r); } break; default: diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 054d0b1..0083778 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -302,12 +302,16 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And a srvrflx candidate for each STUN server */ for(j=0;jstun_server_ct;j++){ /* Skip non-UDP */ - if (ctx->stun_servers[j].addr.protocol != IPPROTO_UDP) continue; - - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, &ctx->stun_servers[j].addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of address type mis-match",ctx->label); + if(ctx->stun_servers[j].transport!=IPPROTO_UDP) continue; + + if (ctx->stun_servers[j].type == NR_ICE_STUN_SERVER_TYPE_ADDR) { + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, + &ctx->stun_servers[j].u.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of link local mis-match",ctx->label); + continue; + } } /* Ensure id is set (nr_ice_ctx_set_stun_servers does not) */ @@ -337,13 +341,16 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_candidate *srvflx_cand=0; /* Skip non-UDP */ - if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_UDP) + if (ctx->turn_servers[j].turn_server.transport != IPPROTO_UDP) continue; - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, &ctx->turn_servers[j].turn_server.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); - continue; + if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, + &ctx->turn_servers[j].turn_server.u.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); + continue; + } } if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) { @@ -541,7 +548,8 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And srvrflx candidates for each STUN server */ for(j=0;jstun_server_ct;j++){ - if (ctx->stun_servers[j].addr.protocol != IPPROTO_TCP) continue; + if (ctx->stun_servers[j].transport!=IPPROTO_TCP) + continue; if (isock_psv) { if(r=nr_ice_candidate_create(ctx,component, @@ -575,7 +583,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_socket *turn_isock; /* Skip non-TCP */ - if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_TCP) + if (ctx->turn_servers[j].turn_server.transport != IPPROTO_TCP) continue; /* Create relay candidate */ @@ -583,10 +591,13 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon ABORT(r); addr.protocol = IPPROTO_TCP; - if (nr_transport_addr_check_compatibility( - &addr, &ctx->turn_servers[j].turn_server.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); - continue; + if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { + if (nr_transport_addr_check_compatibility( + &addr, + &ctx->turn_servers[j].turn_server.u.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); + continue; + } } if (!ice_tcp_disabled) { @@ -612,9 +623,11 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon } } - if (ctx->turn_servers[j].turn_server.addr.fqdn[0] != 0) { - /* If we're going to use TLS, make sure that's recorded */ - addr.tls = ctx->turn_servers[j].turn_server.addr.tls; + /* If we're going to use TLS, make sure that's recorded */ + if (ctx->turn_servers[j].turn_server.tls) { + strncpy(addr.tls_host, + ctx->turn_servers[j].turn_server.u.dnsname.host, + sizeof(addr.tls_host) - 1); } if ((r=nr_transport_addr_fmt_addr_string(&addr))) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index 1348f0c..cac836e 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -101,9 +101,10 @@ int nr_ice_fetch_stun_servers(int ct, nr_ice_stun_server **out) ABORT(r); port = 3478; } - if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].addr)) + if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].u.addr)) ABORT(r); + servers[i].type = NR_ICE_STUN_SERVER_TYPE_ADDR; RFREE(addr); addr=0; } @@ -280,8 +281,8 @@ int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out) ABORT(r); port = 3478; } - if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].turn_server.addr)) + if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].turn_server.u.addr)) ABORT(r); diff --git a/src/ice/ice_ctx.h b/src/ice/ice_ctx.h index e61c506..015ee90 100644 --- a/src/ice/ice_ctx.h +++ b/src/ice/ice_ctx.h @@ -53,8 +53,17 @@ extern "C" { #define NR_ICE_STUN_SERVER_TYPE_DNSNAME 2 typedef struct nr_ice_stun_server_ { - nr_transport_addr addr; + int type; + union { + nr_transport_addr addr; + struct { + char host[256]; /* Limit from RFC 1034, plus a 0 byte */ + UINT2 port; + } dnsname; + } u; int id; + int transport; + int tls; /* Whether to use TLS or not */ } nr_ice_stun_server; typedef struct nr_ice_turn_server_ { diff --git a/src/net/nr_socket_multi_tcp.c b/src/net/nr_socket_multi_tcp.c index c555e32..12bbea7 100644 --- a/src/net/nr_socket_multi_tcp.c +++ b/src/net/nr_socket_multi_tcp.c @@ -169,20 +169,14 @@ static int nr_socket_multi_tcp_create_stun_server_socket( nr_tcp_socket_ctx *tcp_socket_ctx=0; nr_socket * nrsock; - if (stun_server->addr.protocol != IPPROTO_TCP) { - r_log(LOG_ICE, LOG_INFO, - "%s:%d function %s skipping UDP STUN server(addr:%s)", __FILE__, - __LINE__, __FUNCTION__, stun_server->addr.as_string); + if (stun_server->transport!=IPPROTO_TCP) { + r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping UDP STUN server(addr:%s)",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.as_string); ABORT(R_BAD_ARGS); } - if (nr_transport_addr_cmp(&stun_server->addr, addr, - NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { - r_log(LOG_ICE, LOG_INFO, - "%s:%d function %s skipping STUN with different IP version (%u) " - "than local socket (%u),", - __FILE__, __LINE__, __FUNCTION__, stun_server->addr.ip_version, - addr->ip_version); + if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR && + nr_transport_addr_cmp(&stun_server->u.addr,addr,NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { + r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping STUN with different IP version (%u) than local socket (%u),",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.ip_version,addr->ip_version); ABORT(R_BAD_ARGS); } @@ -193,21 +187,19 @@ static int nr_socket_multi_tcp_create_stun_server_socket( if ((r=nr_tcp_socket_ctx_create(nrsock, 0, max_pending, &tcp_socket_ctx))) ABORT(r); - nr_transport_addr stun_server_addr; + if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { + nr_transport_addr stun_server_addr; - nr_transport_addr_copy(&stun_server_addr, &stun_server->addr); - r = nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); - if (r && r != R_WOULDBLOCK) { - r_log(LOG_ICE, LOG_WARNING, - "%s:%d function %s connect to STUN server(addr:%s) failed with " - "error %d", - __FILE__, __LINE__, __FUNCTION__, stun_server_addr.as_string, r); - ABORT(r); - } + nr_transport_addr_copy(&stun_server_addr, &stun_server->u.addr); + r=nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); + if (r && r!=R_WOULDBLOCK) { + r_log(LOG_ICE,LOG_WARNING,"%s:%d function %s connect to STUN server(addr:%s) failed with error %d",__FILE__,__LINE__,__FUNCTION__,stun_server_addr.as_string,r); + ABORT(r); + } - if ((r = nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, - sock))) - ABORT(r); + if ((r=nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, sock))) + ABORT(r); + } TAILQ_INSERT_TAIL(&sock->sockets, tcp_socket_ctx, entry); diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index b900708..4cec4e6 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -57,7 +57,7 @@ int nr_transport_addr_fmt_addr_string(nr_transport_addr *addr) switch(addr->protocol){ case IPPROTO_TCP: - if (addr->tls) { + if (addr->tls_host[0]) { protocol = "TLS"; } else { protocol = "TCP"; @@ -279,28 +279,24 @@ int nr_ip6_port_to_transport_addr(struct in6_addr* addr6, UINT2 port, int protoc int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, int maxlen) { int _status; + const char *res; - if (addr->fqdn[0]) { - strncpy(str, addr->fqdn, maxlen); - } else { - const char* res; - switch (addr->ip_version) { - case NR_IPV4: - res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr, str, maxlen); - break; - case NR_IPV6: - res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr, str, maxlen); - break; - default: - ABORT(R_INTERNAL); - } - - if (!res) { - if (errno == ENOSPC) { - ABORT(R_BAD_ARGS); - } + switch(addr->ip_version){ + case NR_IPV4: + res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr,str,maxlen); + break; + case NR_IPV6: + res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr,str,maxlen); + break; + default: ABORT(R_INTERNAL); + } + + if(!res){ + if (errno == ENOSPC){ + ABORT(R_BAD_ARGS); } + ABORT(R_INTERNAL); } _status=0; @@ -489,13 +485,10 @@ int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport (addr1->protocol != addr2->protocol)) { return(1); } - - if (!addr1->fqdn[0] && !addr2->fqdn[0]) { - // now make sure the link local status matches - if (nr_transport_addr_is_link_local(addr1) != - nr_transport_addr_is_link_local(addr2)) { - return(1); - } + // now make sure the link local status matches + if (nr_transport_addr_is_link_local(addr1) != + nr_transport_addr_is_link_local(addr2)) { + return(1); } return(0); } diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index aa8b695..ac8970c 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -69,9 +69,8 @@ typedef struct nr_transport_addr_ { /* A string version. 56 = 5 ("IP6:[") + 39 (ipv6 address) + 2 ("]:") + 5 (port) + 4 (/UDP) + 1 (null) */ char as_string[56]; - char fqdn[256]; + char tls_host[256]; bool is_proxied; - bool tls; } nr_transport_addr; typedef struct nr_transport_addr_mask_ { diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index 68e2740..5994862 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -346,7 +346,11 @@ static int nr_socket_buffered_stun_recvfrom(void *obj,void * restrict buf, sock->read_state = NR_ICE_SOCKET_READ_NONE; sock->bytes_needed = (sock->framing_type == ICE_TCP_FRAMING) ? sizeof(nr_frame_header) : sizeof(nr_stun_message_header); - if ((r = nr_transport_addr_copy(from, &sock->remote_addr))) ABORT(r); + assert(!nr_transport_addr_is_wildcard(&sock->remote_addr)); + if (!nr_transport_addr_is_wildcard(&sock->remote_addr)) { + if ((r=nr_transport_addr_copy(from, &sock->remote_addr))) + ABORT(r); + } _status=0; abort: From 868655ffdcf5b347ff564c99aa7d6b9c9f2627fb Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 28 Jan 2021 15:52:43 +0000 Subject: [PATCH 22/41] Bug 1416220: Modify mtransport to delegate DNS lookups for TCP sockets to the socket class, instead of using nr_resolver. r=mjf * Modified nr_transport_addr to allow it to represent an fqdn/ip-version/protocol tuple, and taught nICEr to handle the fqdn case appropriately. * Since nr_transport_addr can represent an fqdn, nr_ice_stun_server did not need this ability anymore, and was significantly simplified. * Taught NrIceCtx to handle creation of a V4/V6 pair of nr_ice_stun_server when an fqdn is used, instead of having nICEr create pairs later. Differential Revision: https://phabricator.services.mozilla.com/D101658 --- src/ice/ice_candidate.c | 81 ++++++++++++++++-------------- src/ice/ice_component.c | 51 +++++++------------ src/ice/ice_ctx.c | 9 ++-- src/ice/ice_ctx.h | 11 +--- src/net/nr_socket_multi_tcp.c | 40 +++++++++------ src/net/transport_addr.c | 47 +++++++++-------- src/net/transport_addr.h | 3 +- src/stun/nr_socket_buffered_stun.c | 6 +-- 8 files changed, 122 insertions(+), 126 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index 3c02181..c887fe2 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -106,26 +106,11 @@ static const char *nr_tcp_type_name(nr_socket_tcp_type tcp_type) { static int nr_ice_candidate_format_stun_label(char *label, size_t size, nr_ice_candidate *cand) { - int _status; - - *label = 0; - switch(cand->stun_server->type) { - case NR_ICE_STUN_SERVER_TYPE_ADDR: - snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), cand->base.as_string, - cand->stun_server->u.addr.as_string); - break; - case NR_ICE_STUN_SERVER_TYPE_DNSNAME: - snprintf(label, size, "%s(%s|%s:%u)", nr_ctype_name(cand->type), cand->base.as_string, - cand->stun_server->u.dnsname.host, cand->stun_server->u.dnsname.port); - break; - default: - assert(0); - ABORT(R_BAD_ARGS); - } + *label = 0; + snprintf(label, size, "%s(%s|%s)", nr_ctype_name(cand->type), + cand->base.as_string, cand->stun_server->addr.as_string); - _status=0; - abort: - return(_status); + return (0); } int nr_ice_candidate_create(nr_ice_ctx *ctx,nr_ice_component *comp,nr_ice_socket *isock, nr_socket *osock, nr_ice_candidate_type ctype, nr_socket_tcp_type tcp_type, nr_ice_stun_server *stun_server, UCHAR component_id, nr_ice_candidate **candp) @@ -605,28 +590,40 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo /* Fall through */ #endif case SERVER_REFLEXIVE: - if(cand->stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if(nr_transport_addr_cmp(&cand->base,&cand->stun_server->u.addr,NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { - r_log(LOG_ICE, LOG_INFO, "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because of IP version/transport mis-match with STUN/TURN server (%u/%s - %u/%s).", cand->label,cand->base.ip_version,cand->base.protocol==IPPROTO_UDP?"UDP":"TCP",cand->stun_server->u.addr.ip_version,cand->stun_server->u.addr.protocol==IPPROTO_UDP?"UDP":"TCP"); - ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ - } + if (nr_transport_addr_cmp(&cand->base, &cand->stun_server->addr, + NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL)) { + r_log(LOG_ICE, LOG_INFO, + "ICE-CANDIDATE(%s): Skipping srflx/relayed candidate because " + "of IP version/transport mis-match with STUN/TURN server " + "(%u/%s - %u/%s).", + cand->label, cand->base.ip_version, + cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", + cand->stun_server->addr.ip_version, + cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" + : "TCP"); + ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ + } - /* Just copy the address */ - if (r=nr_transport_addr_copy(&cand->stun_server_addr, - &cand->stun_server->u.addr)) { - r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); + if(cand->stun_server->addr.fqdn[0] != 0 && + cand->stun_server->addr.protocol == IPPROTO_UDP) { + /* UDP with FQDN, allow resolver to handle this for now, we + * eventually want to allow the UDP socket to handle this for us. */ + r_log( + LOG_ICE, LOG_DEBUG, + "ICE-CANDIDATE(%s): Starting DNS resolution (%u/%s - %u/%s).", + cand->label, cand->base.ip_version, + cand->base.protocol == IPPROTO_UDP ? "UDP" : "TCP", + cand->stun_server->addr.ip_version, + cand->stun_server->addr.protocol == IPPROTO_UDP ? "UDP" : "TCP"); + nr_resolver_resource resource; + int port; + resource.domain_name = cand->stun_server->addr.fqdn; + if (r = nr_transport_addr_get_port(&cand->stun_server->addr, &port)) { ABORT(r); } - - if(r=nr_ice_candidate_initialize2(cand)) - ABORT(r); - } - else { - nr_resolver_resource resource; - resource.domain_name=cand->stun_server->u.dnsname.host; - resource.port=cand->stun_server->u.dnsname.port; + resource.port = (uint16_t)port; resource.stun_turn=protocol; - resource.transport_protocol=cand->stun_server->transport; + resource.transport_protocol = cand->stun_server->addr.protocol; switch (cand->base.ip_version) { case NR_IPV4: @@ -653,6 +650,16 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not invoke DNS resolver",cand->label); ABORT(r); } + } else { + /* No DNS handling needed, just copy the address and finish init */ + if (r = nr_transport_addr_copy(&cand->stun_server_addr, + &cand->stun_server->addr)) { + r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); + ABORT(r); + } + + if(r=nr_ice_candidate_initialize2(cand)) + ABORT(r); } break; default: diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 0083778..054d0b1 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -302,16 +302,12 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And a srvrflx candidate for each STUN server */ for(j=0;jstun_server_ct;j++){ /* Skip non-UDP */ - if(ctx->stun_servers[j].transport!=IPPROTO_UDP) - continue; + if (ctx->stun_servers[j].addr.protocol != IPPROTO_UDP) continue; - if (ctx->stun_servers[j].type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, - &ctx->stun_servers[j].u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, &ctx->stun_servers[j].addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping STUN server because of address type mis-match",ctx->label); + continue; } /* Ensure id is set (nr_ice_ctx_set_stun_servers does not) */ @@ -341,16 +337,13 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_candidate *srvflx_cand=0; /* Skip non-UDP */ - if (ctx->turn_servers[j].turn_server.transport != IPPROTO_UDP) + if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_UDP) continue; - if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addrs[i].addr, - &ctx->turn_servers[j].turn_server.u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addrs[i].addr, &ctx->turn_servers[j].turn_server.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); + continue; } if (!(ctx->flags & NR_ICE_CTX_FLAGS_RELAY_ONLY)) { @@ -548,8 +541,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And srvrflx candidates for each STUN server */ for(j=0;jstun_server_ct;j++){ - if (ctx->stun_servers[j].transport!=IPPROTO_TCP) - continue; + if (ctx->stun_servers[j].addr.protocol != IPPROTO_TCP) continue; if (isock_psv) { if(r=nr_ice_candidate_create(ctx,component, @@ -583,7 +575,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_ice_socket *turn_isock; /* Skip non-TCP */ - if (ctx->turn_servers[j].turn_server.transport != IPPROTO_TCP) + if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_TCP) continue; /* Create relay candidate */ @@ -591,13 +583,10 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon ABORT(r); addr.protocol = IPPROTO_TCP; - if (ctx->turn_servers[j].turn_server.type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - if (nr_transport_addr_check_compatibility( - &addr, - &ctx->turn_servers[j].turn_server.u.addr)) { - r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of link local mis-match",ctx->label); - continue; - } + if (nr_transport_addr_check_compatibility( + &addr, &ctx->turn_servers[j].turn_server.addr)) { + r_log(LOG_ICE,LOG_INFO,"ICE(%s): Skipping TURN server because of address type mis-match",ctx->label); + continue; } if (!ice_tcp_disabled) { @@ -623,11 +612,9 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon } } - /* If we're going to use TLS, make sure that's recorded */ - if (ctx->turn_servers[j].turn_server.tls) { - strncpy(addr.tls_host, - ctx->turn_servers[j].turn_server.u.dnsname.host, - sizeof(addr.tls_host) - 1); + if (ctx->turn_servers[j].turn_server.addr.fqdn[0] != 0) { + /* If we're going to use TLS, make sure that's recorded */ + addr.tls = ctx->turn_servers[j].turn_server.addr.tls; } if ((r=nr_transport_addr_fmt_addr_string(&addr))) diff --git a/src/ice/ice_ctx.c b/src/ice/ice_ctx.c index cac836e..1348f0c 100644 --- a/src/ice/ice_ctx.c +++ b/src/ice/ice_ctx.c @@ -101,10 +101,9 @@ int nr_ice_fetch_stun_servers(int ct, nr_ice_stun_server **out) ABORT(r); port = 3478; } - if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].u.addr)) + if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].addr)) ABORT(r); - servers[i].type = NR_ICE_STUN_SERVER_TYPE_ADDR; RFREE(addr); addr=0; } @@ -281,8 +280,8 @@ int nr_ice_fetch_turn_servers(int ct, nr_ice_turn_server **out) ABORT(r); port = 3478; } - if(r=nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, - &servers[i].turn_server.u.addr)) + if (r = nr_ip4_port_to_transport_addr(ntohl(addr_int), port, IPPROTO_UDP, + &servers[i].turn_server.addr)) ABORT(r); diff --git a/src/ice/ice_ctx.h b/src/ice/ice_ctx.h index 015ee90..e61c506 100644 --- a/src/ice/ice_ctx.h +++ b/src/ice/ice_ctx.h @@ -53,17 +53,8 @@ extern "C" { #define NR_ICE_STUN_SERVER_TYPE_DNSNAME 2 typedef struct nr_ice_stun_server_ { - int type; - union { - nr_transport_addr addr; - struct { - char host[256]; /* Limit from RFC 1034, plus a 0 byte */ - UINT2 port; - } dnsname; - } u; + nr_transport_addr addr; int id; - int transport; - int tls; /* Whether to use TLS or not */ } nr_ice_stun_server; typedef struct nr_ice_turn_server_ { diff --git a/src/net/nr_socket_multi_tcp.c b/src/net/nr_socket_multi_tcp.c index 12bbea7..c555e32 100644 --- a/src/net/nr_socket_multi_tcp.c +++ b/src/net/nr_socket_multi_tcp.c @@ -169,14 +169,20 @@ static int nr_socket_multi_tcp_create_stun_server_socket( nr_tcp_socket_ctx *tcp_socket_ctx=0; nr_socket * nrsock; - if (stun_server->transport!=IPPROTO_TCP) { - r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping UDP STUN server(addr:%s)",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.as_string); + if (stun_server->addr.protocol != IPPROTO_TCP) { + r_log(LOG_ICE, LOG_INFO, + "%s:%d function %s skipping UDP STUN server(addr:%s)", __FILE__, + __LINE__, __FUNCTION__, stun_server->addr.as_string); ABORT(R_BAD_ARGS); } - if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR && - nr_transport_addr_cmp(&stun_server->u.addr,addr,NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { - r_log(LOG_ICE,LOG_INFO,"%s:%d function %s skipping STUN with different IP version (%u) than local socket (%u),",__FILE__,__LINE__,__FUNCTION__,stun_server->u.addr.ip_version,addr->ip_version); + if (nr_transport_addr_cmp(&stun_server->addr, addr, + NR_TRANSPORT_ADDR_CMP_MODE_VERSION)) { + r_log(LOG_ICE, LOG_INFO, + "%s:%d function %s skipping STUN with different IP version (%u) " + "than local socket (%u),", + __FILE__, __LINE__, __FUNCTION__, stun_server->addr.ip_version, + addr->ip_version); ABORT(R_BAD_ARGS); } @@ -187,20 +193,22 @@ static int nr_socket_multi_tcp_create_stun_server_socket( if ((r=nr_tcp_socket_ctx_create(nrsock, 0, max_pending, &tcp_socket_ctx))) ABORT(r); - if (stun_server->type == NR_ICE_STUN_SERVER_TYPE_ADDR) { - nr_transport_addr stun_server_addr; + nr_transport_addr stun_server_addr; - nr_transport_addr_copy(&stun_server_addr, &stun_server->u.addr); - r=nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); - if (r && r!=R_WOULDBLOCK) { - r_log(LOG_ICE,LOG_WARNING,"%s:%d function %s connect to STUN server(addr:%s) failed with error %d",__FILE__,__LINE__,__FUNCTION__,stun_server_addr.as_string,r); - ABORT(r); - } - - if ((r=nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, sock))) - ABORT(r); + nr_transport_addr_copy(&stun_server_addr, &stun_server->addr); + r = nr_socket_connect(tcp_socket_ctx->inner, &stun_server_addr); + if (r && r != R_WOULDBLOCK) { + r_log(LOG_ICE, LOG_WARNING, + "%s:%d function %s connect to STUN server(addr:%s) failed with " + "error %d", + __FILE__, __LINE__, __FUNCTION__, stun_server_addr.as_string, r); + ABORT(r); } + if ((r = nr_tcp_socket_ctx_initialize(tcp_socket_ctx, &stun_server_addr, + sock))) + ABORT(r); + TAILQ_INSERT_TAIL(&sock->sockets, tcp_socket_ctx, entry); _status=0; diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index 4cec4e6..b900708 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -57,7 +57,7 @@ int nr_transport_addr_fmt_addr_string(nr_transport_addr *addr) switch(addr->protocol){ case IPPROTO_TCP: - if (addr->tls_host[0]) { + if (addr->tls) { protocol = "TLS"; } else { protocol = "TCP"; @@ -279,24 +279,28 @@ int nr_ip6_port_to_transport_addr(struct in6_addr* addr6, UINT2 port, int protoc int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, int maxlen) { int _status; - const char *res; - switch(addr->ip_version){ - case NR_IPV4: - res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr,str,maxlen); - break; - case NR_IPV6: - res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr,str,maxlen); - break; - default: - ABORT(R_INTERNAL); - } + if (addr->fqdn[0]) { + strncpy(str, addr->fqdn, maxlen); + } else { + const char* res; + switch (addr->ip_version) { + case NR_IPV4: + res = inet_ntop(AF_INET, &addr->u.addr4.sin_addr, str, maxlen); + break; + case NR_IPV6: + res = inet_ntop(AF_INET6, &addr->u.addr6.sin6_addr, str, maxlen); + break; + default: + ABORT(R_INTERNAL); + } - if(!res){ - if (errno == ENOSPC){ - ABORT(R_BAD_ARGS); + if (!res) { + if (errno == ENOSPC) { + ABORT(R_BAD_ARGS); + } + ABORT(R_INTERNAL); } - ABORT(R_INTERNAL); } _status=0; @@ -485,10 +489,13 @@ int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport (addr1->protocol != addr2->protocol)) { return(1); } - // now make sure the link local status matches - if (nr_transport_addr_is_link_local(addr1) != - nr_transport_addr_is_link_local(addr2)) { - return(1); + + if (!addr1->fqdn[0] && !addr2->fqdn[0]) { + // now make sure the link local status matches + if (nr_transport_addr_is_link_local(addr1) != + nr_transport_addr_is_link_local(addr2)) { + return(1); + } } return(0); } diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index ac8970c..aa8b695 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -69,8 +69,9 @@ typedef struct nr_transport_addr_ { /* A string version. 56 = 5 ("IP6:[") + 39 (ipv6 address) + 2 ("]:") + 5 (port) + 4 (/UDP) + 1 (null) */ char as_string[56]; - char tls_host[256]; + char fqdn[256]; bool is_proxied; + bool tls; } nr_transport_addr; typedef struct nr_transport_addr_mask_ { diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index 5994862..68e2740 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -346,11 +346,7 @@ static int nr_socket_buffered_stun_recvfrom(void *obj,void * restrict buf, sock->read_state = NR_ICE_SOCKET_READ_NONE; sock->bytes_needed = (sock->framing_type == ICE_TCP_FRAMING) ? sizeof(nr_frame_header) : sizeof(nr_stun_message_header); - assert(!nr_transport_addr_is_wildcard(&sock->remote_addr)); - if (!nr_transport_addr_is_wildcard(&sock->remote_addr)) { - if ((r=nr_transport_addr_copy(from, &sock->remote_addr))) - ABORT(r); - } + if ((r = nr_transport_addr_copy(from, &sock->remote_addr))) ABORT(r); _status=0; abort: From 1ea442859ddbf2590474a6f8744dc76f5dc76029 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 28 Jan 2021 15:20:07 +0000 Subject: [PATCH 23/41] Bug 1416220: Add/improve some logging, and add an assertion. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D101660 --- src/ice/ice_candidate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index c887fe2..fb0aef5 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -633,6 +633,7 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo resource.address_family=AF_INET6; break; default: + assert(0); ABORT(R_BAD_ARGS); } From 32d019dd6226400a4e70b99376a6bc3a406e5ace Mon Sep 17 00:00:00 2001 From: vpoddubchak Date: Tue, 30 Mar 2021 13:24:12 +0300 Subject: [PATCH 24/41] Bug 1416220: Get ice_unittest working. r=mjf * Allow nr_resolver to be used for TCP when not running in e10s/socket process mode. * Init IPv6 STUN/TURN servers appropriately. * Fix bug that was preventing STUN server hostname from being configured. * Disable some tests that relied on STUN TCP that hasn't been available for a long time. (This went unnoticed due to the previous problem) * A small logging improvement. Depends on D101660 Differential Revision: https://phabricator.services.mozilla.com/D103332 --- src/ice/ice_candidate.c | 25 ++++++++++++++++++++----- src/ice/ice_component.c | 1 + src/ice/ice_reg.h | 2 ++ 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index fb0aef5..c423fd1 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -566,6 +566,24 @@ static void nr_ice_candidate_fire_ready_cb(NR_SOCKET s, int how, void *cb_arg) cand->ready_cb(0, 0, cand->ready_cb_arg); } +static int nr_ice_candidate_use_nr_resolver(nr_transport_addr *addr) + { + if(addr->fqdn[0] == 0) { + return 0; + } + + char use = 0; + if(addr->protocol == IPPROTO_UDP) { + NR_reg_get_char(NR_ICE_REG_USE_NR_RESOLVER_FOR_UDP, &use); + } else if(addr->protocol == IPPROTO_TCP) { + NR_reg_get_char(NR_ICE_REG_USE_NR_RESOLVER_FOR_TCP, &use); + } else { + assert(0); + } + + return use; + } + int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, void *cb_arg) { int r,_status; @@ -604,10 +622,7 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo ABORT(R_NOT_FOUND); /* Same error code when DNS lookup fails */ } - if(cand->stun_server->addr.fqdn[0] != 0 && - cand->stun_server->addr.protocol == IPPROTO_UDP) { - /* UDP with FQDN, allow resolver to handle this for now, we - * eventually want to allow the UDP socket to handle this for us. */ + if(nr_ice_candidate_use_nr_resolver(&cand->stun_server->addr)) { r_log( LOG_ICE, LOG_DEBUG, "ICE-CANDIDATE(%s): Starting DNS resolution (%u/%s - %u/%s).", @@ -652,7 +667,7 @@ int nr_ice_candidate_initialize(nr_ice_candidate *cand, NR_async_cb ready_cb, vo ABORT(r); } } else { - /* No DNS handling needed, just copy the address and finish init */ + /* No nr_resolver for this, just copy the address and finish init */ if (r = nr_transport_addr_copy(&cand->stun_server_addr, &cand->stun_server->addr)) { r_log(LOG_ICE,LOG_ERR,"ICE-CANDIDATE(%s): Could not copy STUN server addr", cand->label); diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 054d0b1..8d4ca67 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -301,6 +301,7 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon /* And a srvrflx candidate for each STUN server */ for(j=0;jstun_server_ct;j++){ + r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): Checking STUN server %s %s", ctx->label, ctx->stun_servers[j].addr.fqdn, ctx->stun_servers[j].addr.as_string); /* Skip non-UDP */ if (ctx->stun_servers[j].addr.protocol != IPPROTO_UDP) continue; diff --git a/src/ice/ice_reg.h b/src/ice/ice_reg.h index dc0f741..3acc023 100644 --- a/src/ice/ice_reg.h +++ b/src/ice/ice_reg.h @@ -71,6 +71,8 @@ extern "C" { #define NR_ICE_REG_TRICKLE_GRACE_PERIOD "ice.trickle_grace_period" #define NR_ICE_REG_PREF_FORCE_INTERFACE_NAME "ice.forced_interface_name" +#define NR_ICE_REG_USE_NR_RESOLVER_FOR_TCP "ice.tcp.use_nr_resolver" +#define NR_ICE_REG_USE_NR_RESOLVER_FOR_UDP "ice.udp.use_nr_resolver" #ifdef __cplusplus } From 75065e094531fc45233068b3dc8b3e58b0b78825 Mon Sep 17 00:00:00 2001 From: vpoddubchak Date: Tue, 30 Mar 2021 17:07:34 +0300 Subject: [PATCH 25/41] Adapt CMakeFile to the the latest nicer.gyp --- CMakeLists.txt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ca2de37..19713e5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,7 +41,9 @@ list(APPEND nicer__c_srcs "${NICER_SOURCE}/src/net/local_addr.c" "${NICER_SOURCE}/src/net/nr_interface_prioritizer.c" "${NICER_SOURCE}/src/stun/addrs.c" - "${NICER_SOURCE}/src/stun/ifaddrs-android.c" + "${NICER_SOURCE}/src/stun/addrs-bsd.c", + "${NICER_SOURCE}/src/stun/addrs-netlink.c", + "${NICER_SOURCE}/src/stun/addrs-win32.c", "${NICER_SOURCE}/src/stun/nr_socket_turn.c" "${NICER_SOURCE}/src/stun/nr_socket_buffered_stun.c" "${NICER_SOURCE}/src/stun/stun_build.c" @@ -78,7 +80,9 @@ list(APPEND nicer__other_srcs "${NICER_SOURCE}/src/net/local_addr.h" "${NICER_SOURCE}/src/net/nr_interface_prioritizer.h" "${NICER_SOURCE}/src/stun/addrs.h" - "${NICER_SOURCE}/src/stun/ifaddrs-android.h" + "${NICER_SOURCE}/src/stun/addrs-bsd.h", + "${NICER_SOURCE}/src/stun/addrs-netlink.h", + "${NICER_SOURCE}/src/stun/addrs-win32.h", "${NICER_SOURCE}/src/stun/nr_socket_turn.h" "${NICER_SOURCE}/src/stun/nr_socket_buffered_stun.h" "${NICER_SOURCE}/src/stun/stun.h" From c34b317b9b358490313d0775ad554a4ac8e81b3e Mon Sep 17 00:00:00 2001 From: vpoddubchak Date: Tue, 30 Mar 2021 17:21:48 +0300 Subject: [PATCH 26/41] Fixed build --- CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 19713e5..f7d33a6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -41,9 +41,9 @@ list(APPEND nicer__c_srcs "${NICER_SOURCE}/src/net/local_addr.c" "${NICER_SOURCE}/src/net/nr_interface_prioritizer.c" "${NICER_SOURCE}/src/stun/addrs.c" - "${NICER_SOURCE}/src/stun/addrs-bsd.c", - "${NICER_SOURCE}/src/stun/addrs-netlink.c", - "${NICER_SOURCE}/src/stun/addrs-win32.c", + "${NICER_SOURCE}/src/stun/addrs-bsd.c" + "${NICER_SOURCE}/src/stun/addrs-netlink.c" + "${NICER_SOURCE}/src/stun/addrs-win32.c" "${NICER_SOURCE}/src/stun/nr_socket_turn.c" "${NICER_SOURCE}/src/stun/nr_socket_buffered_stun.c" "${NICER_SOURCE}/src/stun/stun_build.c" @@ -80,9 +80,9 @@ list(APPEND nicer__other_srcs "${NICER_SOURCE}/src/net/local_addr.h" "${NICER_SOURCE}/src/net/nr_interface_prioritizer.h" "${NICER_SOURCE}/src/stun/addrs.h" - "${NICER_SOURCE}/src/stun/addrs-bsd.h", - "${NICER_SOURCE}/src/stun/addrs-netlink.h", - "${NICER_SOURCE}/src/stun/addrs-win32.h", + "${NICER_SOURCE}/src/stun/addrs-bsd.h" + "${NICER_SOURCE}/src/stun/addrs-netlink.h" + "${NICER_SOURCE}/src/stun/addrs-win32.h" "${NICER_SOURCE}/src/stun/nr_socket_turn.h" "${NICER_SOURCE}/src/stun/nr_socket_buffered_stun.h" "${NICER_SOURCE}/src/stun/stun.h" From ed7dad3ae4503b64c5331bfb998ce8c51fe60176 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Mon, 5 Apr 2021 14:53:00 +0000 Subject: [PATCH 27/41] Bug 1702323: Fix bug where the tls bit was being stomped by conversion functions. r=mjf Also, some logging that was useful. Depends on D110634 Differential Revision: https://phabricator.services.mozilla.com/D110635 --- src/ice/ice_component.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 8d4ca67..bf43374 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -620,6 +620,12 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon if ((r=nr_transport_addr_fmt_addr_string(&addr))) ABORT(r); + + r_log(LOG_ICE, LOG_DEBUG, + "ICE(%s): Creating socket for address %s (turn server %s)", + ctx->label, addr.as_string, + ctx->turn_servers[j].turn_server.addr); + /* Create a local socket */ if((r=nr_socket_factory_create_socket(ctx->socket_factory,&addr,&local_sock))){ r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): couldn't create socket for address %s",ctx->label,addr.as_string); From a7b57495f03b07ee370f67006d8586482da0b19f Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:39 +0000 Subject: [PATCH 28/41] Bug 857668: Remove some unneeded non-owning references to nr_socket. r=mjf This substantially simplifies the task of replacing the nr_socket a TURN context is using. Differential Revision: https://phabricator.services.mozilla.com/D115281 --- src/ice/ice_component.c | 14 +++++++------- src/stun/nr_socket_turn.c | 2 +- src/stun/nr_socket_turn.h | 2 +- src/stun/stun_server_ctx.c | 22 +++++++++++++++------- src/stun/stun_server_ctx.h | 4 +--- 5 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index bf43374..6e48796 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -178,14 +178,14 @@ int nr_ice_component_destroy(nr_ice_component **componentp) return(0); } -static int nr_ice_component_create_stun_server_ctx(nr_ice_component *component, nr_ice_socket *isock, nr_socket *sock, nr_transport_addr *addr, char *lufrag, Data *pwd) +static int nr_ice_component_create_stun_server_ctx(nr_ice_component *component, nr_ice_socket *isock, nr_transport_addr *addr, char *lufrag, Data *pwd) { char label[256]; int r,_status; /* Create a STUN server context for this socket */ snprintf(label, sizeof(label), "server(%s)", addr->as_string); - if(r=nr_stun_server_ctx_create(label,sock,&isock->stun_server)) + if(r=nr_stun_server_ctx_create(label,&isock->stun_server)) ABORT(r); if(r=nr_ice_socket_register_stun_server(isock,isock->stun_server,&isock->stun_server_handle)) ABORT(r); @@ -365,7 +365,7 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon cand=0; } /* relayed*/ - if(r=nr_socket_turn_create(sock, &turn_sock)) + if(r=nr_socket_turn_create(&turn_sock)) ABORT(r); if(r=nr_ice_candidate_create(ctx,component, isock,turn_sock,RELAYED,0, @@ -384,7 +384,7 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon #endif /* USE_TURN */ /* Create a STUN server context for this socket */ - if ((r=nr_ice_component_create_stun_server_ctx(component,isock,sock,&addrs[i].addr,lufrag,pwd))) + if ((r=nr_ice_component_create_stun_server_ctx(component,isock,&addrs[i].addr,lufrag,pwd))) ABORT(r); } @@ -450,7 +450,7 @@ static int nr_ice_component_create_tcp_host_candidate(struct nr_ice_ctx_ *ctx, nrsock=NULL; /* Create a STUN server context for this socket */ - if ((r=nr_ice_component_create_stun_server_ctx(component,isock_tmp,isock_tmp->sock,&addr,lufrag,pwd))) + if ((r=nr_ice_component_create_stun_server_ctx(component,isock_tmp,&addr,lufrag,pwd))) ABORT(r); if((r=nr_ice_candidate_create(ctx,component,isock_tmp,isock_tmp->sock,HOST,tcp_type,0, @@ -639,7 +639,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon ABORT(r); /* The TURN socket */ - if(r=nr_socket_turn_create(buffered_sock, &turn_sock)) + if(r=nr_socket_turn_create(&turn_sock)) ABORT(r); /* Create an ICE socket */ @@ -663,7 +663,7 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon cand=0; /* Create a STUN server context for this socket */ - if ((r=nr_ice_component_create_stun_server_ctx(component,turn_isock,local_sock,&addr,lufrag,pwd))) + if ((r=nr_ice_component_create_stun_server_ctx(component,turn_isock,&addr,lufrag,pwd))) ABORT(r); } diff --git a/src/stun/nr_socket_turn.c b/src/stun/nr_socket_turn.c index a264eb9..a5334b8 100644 --- a/src/stun/nr_socket_turn.c +++ b/src/stun/nr_socket_turn.c @@ -75,7 +75,7 @@ static nr_socket_vtbl nr_socket_turn_vtbl={ 0 }; -int nr_socket_turn_create(nr_socket *sock, nr_socket **sockp) +int nr_socket_turn_create(nr_socket **sockp) { int r,_status; nr_socket_turn *sturn=0; diff --git a/src/stun/nr_socket_turn.h b/src/stun/nr_socket_turn.h index c9d36d3..1350604 100644 --- a/src/stun/nr_socket_turn.h +++ b/src/stun/nr_socket_turn.h @@ -41,7 +41,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. around TURN. It implements only the nr_socket features actually used by the ICE stack. You can't, for instance, read off the socket */ -int nr_socket_turn_create(nr_socket *sock, nr_socket **sockp); +int nr_socket_turn_create(nr_socket **sockp); int nr_socket_turn_set_ctx(nr_socket *sock, nr_turn_client_ctx *ctx); #endif diff --git a/src/stun/stun_server_ctx.c b/src/stun/stun_server_ctx.c index bc884b7..b92b6b5 100644 --- a/src/stun/stun_server_ctx.c +++ b/src/stun/stun_server_ctx.c @@ -41,7 +41,7 @@ static int nr_stun_server_send_response(nr_stun_server_ctx *ctx, nr_socket *sock static int nr_stun_server_process_request_auth_checks(nr_stun_server_ctx *ctx, nr_stun_message *req, int auth_rule, nr_stun_message *res); -int nr_stun_server_ctx_create(char *label, nr_socket *sock, nr_stun_server_ctx **ctxp) +int nr_stun_server_ctx_create(char *label, nr_stun_server_ctx **ctxp) { int r,_status; nr_stun_server_ctx *ctx=0; @@ -54,8 +54,6 @@ int nr_stun_server_ctx_create(char *label, nr_socket *sock, nr_stun_server_ctx * if(!(ctx->label=r_strdup(label))) ABORT(R_NO_MEMORY); - ctx->sock=sock; - nr_socket_getaddr(sock,&ctx->my_addr); STAILQ_INIT(&ctx->clients); @@ -232,8 +230,13 @@ int nr_stun_server_process_request(nr_stun_server_ctx *ctx, nr_socket *sock, cha nr_stun_server_request info; int error; int dont_free = 0; + nr_transport_addr my_addr; - r_log(NR_LOG_STUN,LOG_DEBUG,"STUN-SERVER(%s): Received(my_addr=%s,peer_addr=%s)",ctx->label,ctx->my_addr.as_string,peer_addr->as_string); + if ((r=nr_socket_getaddr(sock, &my_addr))) { + ABORT(r); + } + + r_log(NR_LOG_STUN,LOG_DEBUG,"STUN-SERVER(%s): Received(my_addr=%s,peer_addr=%s)",ctx->label,my_addr.as_string,peer_addr->as_string); snprintf(string, sizeof(string)-1, "STUN-SERVER(%s): Received ", ctx->label); r_dump(NR_LOG_STUN, LOG_DEBUG, string, (char*)msg, len); @@ -347,7 +350,7 @@ int nr_stun_server_process_request(nr_stun_server_ctx *ctx, nr_socket *sock, cha nr_stun_form_error_response(req, res, 500, "Failed to specify error"); if ((r=nr_stun_server_send_response(ctx, sock, peer_addr, res, clnt))) { - r_log(NR_LOG_STUN,LOG_ERR,"STUN-SERVER(label=%s): Failed sending response (my_addr=%s,peer_addr=%s)",ctx->label,ctx->my_addr.as_string,peer_addr->as_string); + r_log(NR_LOG_STUN,LOG_ERR,"STUN-SERVER(label=%s): Failed sending response (my_addr=%s,peer_addr=%s)",ctx->label,my_addr.as_string,peer_addr->as_string); _status = R_FAILED; } @@ -383,8 +386,13 @@ static int nr_stun_server_send_response(nr_stun_server_ctx *ctx, nr_socket *sock { int r,_status; char string[256]; + nr_transport_addr my_addr; + + if ((r=nr_socket_getaddr(sock, &my_addr))) { + ABORT(r); + } - r_log(NR_LOG_STUN,LOG_DEBUG,"STUN-SERVER(label=%s): Sending(my_addr=%s,peer_addr=%s)",ctx->label,ctx->my_addr.as_string,peer_addr->as_string); + r_log(NR_LOG_STUN,LOG_DEBUG,"STUN-SERVER(label=%s): Sending(my_addr=%s,peer_addr=%s)",ctx->label,my_addr.as_string,peer_addr->as_string); if ((r=nr_stun_encode_message(res))) { /* should never happen */ @@ -394,7 +402,7 @@ static int nr_stun_server_send_response(nr_stun_server_ctx *ctx, nr_socket *sock snprintf(string, sizeof(string)-1, "STUN(%s): Sending to %s ", ctx->label, peer_addr->as_string); r_dump(NR_LOG_STUN, LOG_DEBUG, string, (char*)res->buffer, res->length); - if(r=nr_socket_sendto(sock?sock:ctx->sock,res->buffer,res->length,0,peer_addr)) + if(r=nr_socket_sendto(sock,res->buffer,res->length,0,peer_addr)) ABORT(r); } diff --git a/src/stun/stun_server_ctx.h b/src/stun/stun_server_ctx.h index 343e676..328675e 100644 --- a/src/stun/stun_server_ctx.h +++ b/src/stun/stun_server_ctx.h @@ -63,14 +63,12 @@ typedef STAILQ_HEAD(nr_stun_server_client_head_, nr_stun_server_client_) nr_stun struct nr_stun_server_ctx_ { char *label; - nr_socket *sock; - nr_transport_addr my_addr; nr_stun_server_client_head clients; nr_stun_server_client *default_client; }; -int nr_stun_server_ctx_create(char *label, nr_socket *sock, nr_stun_server_ctx **ctxp); +int nr_stun_server_ctx_create(char *label, nr_stun_server_ctx **ctxp); int nr_stun_server_ctx_destroy(nr_stun_server_ctx **ctxp); int nr_stun_server_add_client(nr_stun_server_ctx *ctx, char *client_label, char *user, Data *pass, nr_stun_server_cb cb, void *cb_arg); int nr_stun_server_remove_client(nr_stun_server_ctx *ctx, void *cb_arg); From a6fa3bd2c841b2a8a94283cf4b7264e286d235d9 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:40 +0000 Subject: [PATCH 29/41] Bug 1170299: Fix const-correctness on some functions. r=mjf Making this part of the stack for bug 857668, since it avoids adding even more const casting there. Differential Revision: https://phabricator.services.mozilla.com/D115282 --- src/net/nr_socket.c | 4 ++-- src/net/nr_socket.h | 8 ++++---- src/net/nr_socket_multi_tcp.c | 16 +++++++--------- src/net/nr_socket_multi_tcp.h | 2 +- src/net/transport_addr.c | 24 ++++++++++++------------ src/net/transport_addr.h | 24 ++++++++++++------------ src/stun/nr_socket_buffered_stun.c | 8 ++++---- src/stun/nr_socket_turn.c | 4 ++-- src/stun/turn_client_ctx.c | 12 ++++++------ src/stun/turn_client_ctx.h | 4 ++-- 10 files changed, 52 insertions(+), 54 deletions(-) diff --git a/src/net/nr_socket.c b/src/net/nr_socket.c index 0c8693a..972a16f 100644 --- a/src/net/nr_socket.c +++ b/src/net/nr_socket.c @@ -85,7 +85,7 @@ int nr_socket_destroy(nr_socket **sockp) } int nr_socket_sendto(nr_socket *sock,const void *msg, size_t len, int flags, - nr_transport_addr *addr) + const nr_transport_addr *addr) { CHECK_DEFINED(ssendto); return sock->vtbl->ssendto(sock->obj,msg,len,flags,addr); @@ -116,7 +116,7 @@ int nr_socket_close(nr_socket *sock) return sock->vtbl->close(sock->obj); } -int nr_socket_connect(nr_socket *sock, nr_transport_addr *addr) +int nr_socket_connect(nr_socket *sock, const nr_transport_addr *addr) { CHECK_DEFINED(connect); return sock->vtbl->connect(sock->obj, addr); diff --git a/src/net/nr_socket.h b/src/net/nr_socket.h index 8e7fbc6..777837f 100644 --- a/src/net/nr_socket.h +++ b/src/net/nr_socket.h @@ -68,12 +68,12 @@ typedef struct nr_socket_vtbl_ { UINT4 version; /* Currently 2 */ int (*destroy)(void **obj); int (*ssendto)(void *obj,const void *msg, size_t len, int flags, - nr_transport_addr *addr); + const nr_transport_addr *addr); int (*srecvfrom)(void *obj,void * restrict buf, size_t maxlen, size_t *len, int flags, nr_transport_addr *addr); int (*getfd)(void *obj, NR_SOCKET *fd); int (*getaddr)(void *obj, nr_transport_addr *addrp); - int (*connect)(void *obj, nr_transport_addr *addr); + int (*connect)(void *obj, const nr_transport_addr *addr); int (*swrite)(void *obj,const void *msg, size_t len, size_t *written); int (*sread)(void *obj,void * restrict buf, size_t maxlen, size_t *len); int (*close)(void *obj); @@ -103,13 +103,13 @@ typedef struct nr_socket_factory_ { int nr_socket_create_int(void *obj, nr_socket_vtbl *vtbl, nr_socket **sockp); int nr_socket_destroy(nr_socket **sockp); int nr_socket_sendto(nr_socket *sock,const void *msg, size_t len, - int flags,nr_transport_addr *addr); + int flags, const nr_transport_addr *addr); int nr_socket_recvfrom(nr_socket *sock,void * restrict buf, size_t maxlen, size_t *len, int flags, nr_transport_addr *addr); int nr_socket_getfd(nr_socket *sock, NR_SOCKET *fd); int nr_socket_getaddr(nr_socket *sock, nr_transport_addr *addrp); int nr_socket_close(nr_socket *sock); -int nr_socket_connect(nr_socket *sock, nr_transport_addr *addr); +int nr_socket_connect(nr_socket *sock, const nr_transport_addr *addr); int nr_socket_write(nr_socket *sock,const void *msg, size_t len, size_t *written, int flags); int nr_socket_read(nr_socket *sock, void * restrict buf, size_t maxlen, size_t *len, int flags); int nr_socket_listen(nr_socket *sock, int backlog); diff --git a/src/net/nr_socket_multi_tcp.c b/src/net/nr_socket_multi_tcp.c index c555e32..7c53876 100644 --- a/src/net/nr_socket_multi_tcp.c +++ b/src/net/nr_socket_multi_tcp.c @@ -107,7 +107,7 @@ static int nr_tcp_socket_ctx_create(nr_socket *nrsock, int is_framed, } static int nr_tcp_socket_ctx_initialize(nr_tcp_socket_ctx *tcpsock, - nr_transport_addr *addr, void* cb_arg) + const nr_transport_addr *addr, void* cb_arg) { int r, _status; NR_SOCKET fd; @@ -138,12 +138,12 @@ typedef struct nr_socket_multi_tcp_ { static int nr_socket_multi_tcp_destroy(void **objp); static int nr_socket_multi_tcp_sendto(void *obj,const void *msg, size_t len, - int flags, nr_transport_addr *to); + int flags, const nr_transport_addr *to); static int nr_socket_multi_tcp_recvfrom(void *obj,void * restrict buf, size_t maxlen, size_t *len, int flags, nr_transport_addr *from); static int nr_socket_multi_tcp_getaddr(void *obj, nr_transport_addr *addrp); static int nr_socket_multi_tcp_close(void *obj); -static int nr_socket_multi_tcp_connect(void *sock, nr_transport_addr *addr); +static int nr_socket_multi_tcp_connect(void *sock, const nr_transport_addr *addr); static int nr_socket_multi_tcp_listen(void *obj, int backlog); static nr_socket_vtbl nr_socket_multi_tcp_vtbl={ @@ -309,14 +309,12 @@ int nr_socket_multi_tcp_set_readable_cb(nr_socket *sock, #define PREALLOC_DONT_CONNECT_UNLESS_SO 2 static int nr_socket_multi_tcp_get_sock_connected_to(nr_socket_multi_tcp *sock, - nr_transport_addr *to, int preallocated_connect_mode, nr_socket **ret_sock) + const nr_transport_addr *to, int preallocated_connect_mode, nr_socket **ret_sock) { int r, _status; nr_tcp_socket_ctx *tcp_sock_ctx; nr_socket * nrsock; - to->protocol=IPPROTO_TCP; - TAILQ_FOREACH(tcp_sock_ctx, &sock->sockets, entry) { if (!nr_transport_addr_is_wildcard(&tcp_sock_ctx->remote_addr)) { if (!nr_transport_addr_cmp(to, &tcp_sock_ctx->remote_addr, NR_TRANSPORT_ADDR_CMP_MODE_ALL)) { @@ -397,7 +395,7 @@ static int nr_socket_multi_tcp_get_sock_connected_to(nr_socket_multi_tcp *sock, } int nr_socket_multi_tcp_stun_server_connect(nr_socket *sock, - nr_transport_addr *addr) + const nr_transport_addr *addr) { int r, _status; nr_socket_multi_tcp *mtcp_sock = (nr_socket_multi_tcp *)sock->obj; @@ -454,7 +452,7 @@ static int nr_socket_multi_tcp_destroy(void **objp) } static int nr_socket_multi_tcp_sendto(void *obj, const void *msg, size_t len, - int flags, nr_transport_addr *to) + int flags, const nr_transport_addr *to) { int r, _status; nr_socket_multi_tcp *sock=(nr_socket_multi_tcp *)obj; @@ -548,7 +546,7 @@ static void nr_tcp_socket_readable_cb(NR_SOCKET s, int how, void *arg) sock->readable_cb(s, how, sock->readable_cb_arg); } -static int nr_socket_multi_tcp_connect(void *obj, nr_transport_addr *addr) +static int nr_socket_multi_tcp_connect(void *obj, const nr_transport_addr *addr) { int r, _status; nr_socket_multi_tcp *sock=(nr_socket_multi_tcp *)obj; diff --git a/src/net/nr_socket_multi_tcp.h b/src/net/nr_socket_multi_tcp.h index a99e165..2dee0dc 100644 --- a/src/net/nr_socket_multi_tcp.h +++ b/src/net/nr_socket_multi_tcp.h @@ -47,6 +47,6 @@ int nr_socket_multi_tcp_set_readable_cb(nr_socket *sock, NR_async_cb readable_cb,void *readable_cb_arg); int nr_socket_multi_tcp_stun_server_connect(nr_socket *sock, - nr_transport_addr *addr); + const nr_transport_addr *addr); #endif diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index b900708..8c58c66 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -162,7 +162,7 @@ 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) +int nr_transport_addr_copy(nr_transport_addr *to, const nr_transport_addr *from) { int _status; @@ -186,7 +186,7 @@ int nr_transport_addr_copy(nr_transport_addr *to, nr_transport_addr *from) return(_status); } -int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, nr_transport_addr *from) +int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, const nr_transport_addr *from) { int r,_status; char save_ifname[MAXIFNAME]; @@ -308,7 +308,7 @@ int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, i return(_status); } -int nr_transport_addr_get_port(nr_transport_addr *addr, int *port) +int nr_transport_addr_get_port(const nr_transport_addr *addr, int *port) { int _status; @@ -350,7 +350,7 @@ int nr_transport_addr_set_port(nr_transport_addr *addr, int port) /* memcmp() may not work if, for instance, the string or interface haven't been made. Hmmm.. */ -int nr_transport_addr_cmp(nr_transport_addr *addr1,nr_transport_addr *addr2,int mode) +int nr_transport_addr_cmp(const nr_transport_addr *addr1,const nr_transport_addr *addr2,int mode) { assert(mode); @@ -391,7 +391,7 @@ int nr_transport_addr_cmp(nr_transport_addr *addr1,nr_transport_addr *addr2,int return(0); } -int nr_transport_addr_is_loopback(nr_transport_addr *addr) +int nr_transport_addr_is_loopback(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -417,7 +417,7 @@ int nr_transport_addr_is_loopback(nr_transport_addr *addr) return(0); } -int nr_transport_addr_is_link_local(nr_transport_addr *addr) +int nr_transport_addr_is_link_local(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -439,7 +439,7 @@ int nr_transport_addr_is_link_local(nr_transport_addr *addr) return(0); } -int nr_transport_addr_is_mac_based(nr_transport_addr *addr) +int nr_transport_addr_is_mac_based(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -463,7 +463,7 @@ int nr_transport_addr_is_mac_based(nr_transport_addr *addr) return(0); } -int nr_transport_addr_is_teredo(nr_transport_addr *addr) +int nr_transport_addr_is_teredo(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -482,7 +482,7 @@ int nr_transport_addr_is_teredo(nr_transport_addr *addr) return(0); } -int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport_addr *addr2) +int nr_transport_addr_check_compatibility(const nr_transport_addr *addr1, const nr_transport_addr *addr2) { // first make sure we're comparing the same ip versions and protocols if ((addr1->ip_version != addr2->ip_version) || @@ -500,7 +500,7 @@ int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport return(0); } -int nr_transport_addr_is_wildcard(nr_transport_addr *addr) +int nr_transport_addr_is_wildcard(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -533,7 +533,7 @@ nr_transport_addr_mask nr_private_ipv4_addrs[] = { {0x64400000, 0xFFC00000} }; -int nr_transport_addr_get_private_addr_range(nr_transport_addr *addr) +int nr_transport_addr_get_private_addr_range(const nr_transport_addr *addr) { switch(addr->ip_version){ case NR_IPV4: @@ -554,7 +554,7 @@ int nr_transport_addr_get_private_addr_range(nr_transport_addr *addr) return(0); } -int nr_transport_addr_is_reliable_transport(nr_transport_addr *addr) +int nr_transport_addr_is_reliable_transport(const nr_transport_addr *addr) { return addr->protocol == IPPROTO_TCP; } diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index aa8b695..a8325dc 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -87,26 +87,26 @@ int nr_str_port_to_transport_addr(const char *str, UINT2 port, int protocol, nr_ int nr_ip6_port_to_transport_addr(struct in6_addr* addr6, UINT2 port, int protocol, nr_transport_addr *addr); int nr_transport_addr_get_addrstring(const nr_transport_addr *addr, char *str, int maxlen); -int nr_transport_addr_get_port(nr_transport_addr *addr, int *port); -int nr_transport_addr_cmp(nr_transport_addr *addr1,nr_transport_addr *addr2,int mode); +int nr_transport_addr_get_port(const nr_transport_addr *addr, int *port); +int nr_transport_addr_cmp(const nr_transport_addr *addr1,const nr_transport_addr *addr2,int mode); #define NR_TRANSPORT_ADDR_CMP_MODE_VERSION 1 #define NR_TRANSPORT_ADDR_CMP_MODE_PROTOCOL 2 #define NR_TRANSPORT_ADDR_CMP_MODE_ADDR 3 #define NR_TRANSPORT_ADDR_CMP_MODE_ALL 4 -int nr_transport_addr_is_wildcard(nr_transport_addr *addr); -int nr_transport_addr_is_loopback(nr_transport_addr *addr); -int nr_transport_addr_get_private_addr_range(nr_transport_addr *addr); -int nr_transport_addr_is_link_local(nr_transport_addr *addr); -int nr_transport_addr_is_mac_based(nr_transport_addr *addr); -int nr_transport_addr_is_teredo(nr_transport_addr *addr); -int nr_transport_addr_check_compatibility(nr_transport_addr *addr1, nr_transport_addr *addr2); -int nr_transport_addr_copy(nr_transport_addr *to, nr_transport_addr *from); -int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, nr_transport_addr *from); +int nr_transport_addr_is_wildcard(const nr_transport_addr *addr); +int nr_transport_addr_is_loopback(const nr_transport_addr *addr); +int nr_transport_addr_get_private_addr_range(const nr_transport_addr *addr); +int nr_transport_addr_is_link_local(const nr_transport_addr *addr); +int nr_transport_addr_is_mac_based(const nr_transport_addr *addr); +int nr_transport_addr_is_teredo(const nr_transport_addr *addr); +int nr_transport_addr_check_compatibility(const nr_transport_addr *addr1, const nr_transport_addr *addr2); +int nr_transport_addr_copy(nr_transport_addr *to, const nr_transport_addr *from); +int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, const nr_transport_addr *from); int nr_transport_addr_fmt_addr_string(nr_transport_addr *addr); int nr_transport_addr_fmt_ifname_addr_string(const nr_transport_addr *addr, char *buf, int len); int nr_transport_addr_set_port(nr_transport_addr *addr, int port); -int nr_transport_addr_is_reliable_transport(nr_transport_addr *addr); +int nr_transport_addr_is_reliable_transport(const nr_transport_addr *addr); #endif diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index 68e2740..5c06519 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -79,13 +79,13 @@ typedef struct nr_socket_buffered_stun_ { static int nr_socket_buffered_stun_destroy(void **objp); static int nr_socket_buffered_stun_sendto(void *obj,const void *msg, size_t len, - int flags, nr_transport_addr *to); + int flags, const nr_transport_addr *to); static int nr_socket_buffered_stun_recvfrom(void *obj,void * restrict buf, size_t maxlen, size_t *len, int flags, nr_transport_addr *from); static int nr_socket_buffered_stun_getfd(void *obj, NR_SOCKET *fd); static int nr_socket_buffered_stun_getaddr(void *obj, nr_transport_addr *addrp); static int nr_socket_buffered_stun_close(void *obj); -static int nr_socket_buffered_stun_connect(void *sock, nr_transport_addr *addr); +static int nr_socket_buffered_stun_connect(void *sock, const nr_transport_addr *addr); static int nr_socket_buffered_stun_write(void *obj,const void *msg, size_t len, size_t *written); static void nr_socket_buffered_stun_writable_cb(NR_SOCKET s, int how, void *arg); static int nr_socket_buffered_stun_listen(void *obj, int backlog); @@ -216,7 +216,7 @@ int nr_socket_buffered_stun_destroy(void **objp) } static int nr_socket_buffered_stun_sendto(void *obj,const void *msg, size_t len, - int flags, nr_transport_addr *to) + int flags, const nr_transport_addr *to) { nr_socket_buffered_stun *sock = (nr_socket_buffered_stun *)obj; int r, _status; @@ -440,7 +440,7 @@ static void nr_socket_buffered_stun_connected_cb(NR_SOCKET s, int how, void *arg } } -static int nr_socket_buffered_stun_connect(void *obj, nr_transport_addr *addr) +static int nr_socket_buffered_stun_connect(void *obj, const nr_transport_addr *addr) { nr_socket_buffered_stun *sock = (nr_socket_buffered_stun *)obj; int r, _status; diff --git a/src/stun/nr_socket_turn.c b/src/stun/nr_socket_turn.c index a5334b8..1a0162b 100644 --- a/src/stun/nr_socket_turn.c +++ b/src/stun/nr_socket_turn.c @@ -53,7 +53,7 @@ typedef struct nr_socket_turn_ { static int nr_socket_turn_destroy(void **objp); static int nr_socket_turn_sendto(void *obj,const void *msg, size_t len, - int flags, nr_transport_addr *to); + int flags, const nr_transport_addr *to); static int nr_socket_turn_recvfrom(void *obj,void * restrict buf, size_t maxlen, size_t *len, int flags, nr_transport_addr *from); static int nr_socket_turn_getfd(void *obj, NR_SOCKET *fd); @@ -118,7 +118,7 @@ static int nr_socket_turn_destroy(void **objp) } static int nr_socket_turn_sendto(void *obj,const void *msg, size_t len, - int flags, nr_transport_addr *addr) + int flags, const nr_transport_addr *addr) { int r,_status; nr_socket_turn *sturn=obj; diff --git a/src/stun/turn_client_ctx.c b/src/stun/turn_client_ctx.c index afed978..008d3c2 100644 --- a/src/stun/turn_client_ctx.c +++ b/src/stun/turn_client_ctx.c @@ -79,10 +79,10 @@ static int nr_turn_client_start_refresh_timer(nr_turn_client_ctx *ctx, nr_turn_stun_ctx *sctx, UINT4 lifetime); static int nr_turn_permission_create(nr_turn_client_ctx *ctx, - nr_transport_addr *addr, + const nr_transport_addr *addr, nr_turn_permission **permp); static int nr_turn_permission_find(nr_turn_client_ctx *ctx, - nr_transport_addr *addr, + const nr_transport_addr *addr, nr_turn_permission **permp); static int nr_turn_permission_destroy(nr_turn_permission **permp); static void nr_turn_client_refresh_cb(NR_SOCKET s, int how, void *arg); @@ -809,7 +809,7 @@ static void nr_turn_client_refresh_cb(NR_SOCKET s, int how, void *arg) We might in the future. Mozilla bug 857736 */ int nr_turn_client_send_indication(nr_turn_client_ctx *ctx, const UCHAR *msg, size_t len, - int flags, nr_transport_addr *remote_addr) + int flags, const nr_transport_addr *remote_addr) { int r,_status; nr_stun_client_send_indication_params params = { { 0 } }; @@ -923,7 +923,7 @@ int nr_turn_client_parse_data_indication(nr_turn_client_ctx *ctx, unused. */ -int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, nr_transport_addr *addr) +int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, const nr_transport_addr *addr) { int r, _status; nr_turn_permission *perm = 0; @@ -962,7 +962,7 @@ int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, nr_transport_addr *addr) return(_status); } -static int nr_turn_permission_create(nr_turn_client_ctx *ctx, nr_transport_addr *addr, +static int nr_turn_permission_create(nr_turn_client_ctx *ctx, const nr_transport_addr *addr, nr_turn_permission **permp) { int r, _status; @@ -1007,7 +1007,7 @@ static int nr_turn_permission_create(nr_turn_client_ctx *ctx, nr_transport_addr } -static int nr_turn_permission_find(nr_turn_client_ctx *ctx, nr_transport_addr *addr, +static int nr_turn_permission_find(nr_turn_client_ctx *ctx, const nr_transport_addr *addr, nr_turn_permission **permp) { nr_turn_permission *perm; diff --git a/src/stun/turn_client_ctx.h b/src/stun/turn_client_ctx.h index 8dfc895..a9eb235 100644 --- a/src/stun/turn_client_ctx.h +++ b/src/stun/turn_client_ctx.h @@ -129,7 +129,7 @@ int nr_turn_client_failed(nr_turn_client_ctx *ctx); int nr_turn_client_deallocate(nr_turn_client_ctx *ctx); int nr_turn_client_send_indication(nr_turn_client_ctx *ctx, const UCHAR *msg, size_t len, - int flags, nr_transport_addr *remote_addr); + int flags, const nr_transport_addr *remote_addr); int nr_turn_client_parse_data_indication(nr_turn_client_ctx *ctx, nr_transport_addr *source_addr, UCHAR *msg, size_t len, @@ -137,6 +137,6 @@ int nr_turn_client_parse_data_indication(nr_turn_client_ctx *ctx, size_t newsize, nr_transport_addr *remote_addr); int nr_turn_client_ensure_perm(nr_turn_client_ctx *ctx, - nr_transport_addr *addr); + const nr_transport_addr *addr); #endif From 5a753dfa4a3dd4ae1703fe566ad7a26fd8b9edf2 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:40 +0000 Subject: [PATCH 30/41] Bug 857668: Remove some fields that serve no purpose besides making this code harder to maintain. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115283 --- src/net/transport_addr.c | 29 +---------------------------- src/net/transport_addr.h | 2 -- 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index 8c58c66..4455018 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -140,15 +140,11 @@ int nr_sockaddr_to_transport_addr(struct sockaddr *saddr, int protocol, int keep addr->ip_version=NR_IPV4; memcpy(&addr->u.addr4,saddr,sizeof(struct sockaddr_in)); - addr->addr=(struct sockaddr *)&addr->u.addr4; - addr->addr_len=sizeof(struct sockaddr_in); } else if(saddr->sa_family==AF_INET6){ addr->ip_version=NR_IPV6; memcpy(&addr->u.addr6, saddr, sizeof(struct sockaddr_in6)); - addr->addr=(struct sockaddr *)&addr->u.addr6; - addr->addr_len=sizeof(struct sockaddr_in6); } else ABORT(R_BAD_ARGS); @@ -164,26 +160,8 @@ int nr_sockaddr_to_transport_addr(struct sockaddr *saddr, int protocol, int keep int nr_transport_addr_copy(nr_transport_addr *to, const nr_transport_addr *from) { - int _status; - 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); + return 0; } int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, const nr_transport_addr *from) @@ -222,8 +200,6 @@ int nr_ip4_port_to_transport_addr(UINT4 ip4, UINT2 port, int protocol, nr_transp addr->u.addr4.sin_family=PF_INET; addr->u.addr4.sin_port=htons(port); addr->u.addr4.sin_addr.s_addr=htonl(ip4); - addr->addr=(struct sockaddr *)&addr->u.addr4; - addr->addr_len=sizeof(struct sockaddr_in); if(r=nr_transport_addr_fmt_addr_string(addr)) ABORT(r); @@ -265,8 +241,6 @@ int nr_ip6_port_to_transport_addr(struct in6_addr* addr6, UINT2 port, int protoc addr->u.addr6.sin6_family=PF_INET6; addr->u.addr6.sin6_port=htons(port); memcpy(addr->u.addr6.sin6_addr.s6_addr, addr6->s6_addr, sizeof(addr6->s6_addr)); - addr->addr=(struct sockaddr *)&addr->u.addr6; - addr->addr_len=sizeof(struct sockaddr_in6); if(r=nr_transport_addr_fmt_addr_string(addr)) ABORT(r); @@ -366,7 +340,6 @@ int nr_transport_addr_cmp(const nr_transport_addr *addr1,const nr_transport_addr if(mode < NR_TRANSPORT_ADDR_CMP_MODE_ADDR) return(0); - assert(addr1->addr_len == addr2->addr_len); switch(addr1->ip_version){ case NR_IPV4: if(addr1->u.addr4.sin_addr.s_addr != addr2->u.addr4.sin_addr.s_addr) diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index a8325dc..7175dac 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -59,8 +59,6 @@ typedef struct nr_transport_addr_ { #define NR_IPV4 4 #define NR_IPV6 6 UCHAR protocol; /* IPPROTO_TCP, IPPROTO_UDP */ - struct sockaddr *addr; - int addr_len; union { struct sockaddr_in addr4; struct sockaddr_in6 addr6; From 2082b3cea6eb95487d02931a2582894a254c660f Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:40 +0000 Subject: [PATCH 31/41] Bug 857668: Add function for getting STUN multi-attributes. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115284 --- src/stun/stun_msg.c | 33 ++++++++++++++++++++------------- src/stun/stun_msg.h | 2 ++ 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/stun/stun_msg.c b/src/stun/stun_msg.c index c808664..7e01686 100644 --- a/src/stun/stun_msg.c +++ b/src/stun/stun_msg.c @@ -154,23 +154,30 @@ nr_stun_message_attribute_destroy(nr_stun_message *msg, nr_stun_message_attribut int nr_stun_message_has_attribute(nr_stun_message *msg, UINT2 type, nr_stun_message_attribute **attribute) { - nr_stun_message_attribute *attr = 0; + nr_stun_message_attribute *attr = 0; + nr_stun_message_get_attribute(msg, type, 0, &attr); - if (attribute) - *attribute = 0; + if (attribute) + *attribute = attr; - TAILQ_FOREACH(attr, &msg->attributes, entry) { - if (attr->type == type) - break; - } - - if (!attr || attr->invalid) - return 0; /* does not have */ + return attr ? 1 : 0; +} - if (attribute) +int +nr_stun_message_get_attribute(nr_stun_message *msg, UINT2 type, UINT2 index, nr_stun_message_attribute **attribute) +{ + nr_stun_message_attribute *attr; + TAILQ_FOREACH(attr, &msg->attributes, entry) { + if (attr->type == type && !attr->invalid) { + if (!index) { *attribute = attr; - - return 1; /* has */ + return 0; + } + --index; + } + } + *attribute = 0; + return R_NOT_FOUND; } #define NR_STUN_MESSAGE_ADD_ATTRIBUTE(__type, __code) \ diff --git a/src/stun/stun_msg.h b/src/stun/stun_msg.h index a943698..ffd68d3 100644 --- a/src/stun/stun_msg.h +++ b/src/stun/stun_msg.h @@ -172,6 +172,8 @@ int nr_stun_message_attribute_destroy(nr_stun_message *msg, nr_stun_message_attr int nr_stun_message_has_attribute(nr_stun_message *msg, UINT2 type, nr_stun_message_attribute **attribute); +int nr_stun_message_get_attribute(nr_stun_message *msg, UINT2 type, UINT2 index, nr_stun_message_attribute **attribute); + int nr_stun_message_add_alternate_server_attribute(nr_stun_message *msg, nr_transport_addr *alternate_server); int nr_stun_message_add_error_code_attribute(nr_stun_message *msg, UINT2 number, char *reason); int nr_stun_message_add_fingerprint_attribute(nr_stun_message *msg); From c4c8c141c3269e0ff382642e0900bf55dba09be8 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:41 +0000 Subject: [PATCH 32/41] Bug 857668: Remove existing STUN/300 handling. r=mjf STUN/300 is only meaningful for specific STUN method types, and the handling depends on the spec that those methods are defined in. Doing any handling in the base stun client code is not appropriate. Differential Revision: https://phabricator.services.mozilla.com/D115285 --- src/stun/stun_client_ctx.c | 10 ---------- src/stun/stun_proc.c | 25 ++++--------------------- 2 files changed, 4 insertions(+), 31 deletions(-) diff --git a/src/stun/stun_client_ctx.c b/src/stun/stun_client_ctx.c index 6461d28..52ae39f 100644 --- a/src/stun/stun_client_ctx.c +++ b/src/stun/stun_client_ctx.c @@ -157,9 +157,6 @@ int nr_stun_client_restart(nr_stun_client_ctx *ctx) int mode; NR_async_cb finished_cb; void *cb_arg; - nr_stun_message_attribute *ec; - nr_stun_message_attribute *as; - if (ctx->state != NR_STUN_CLIENT_STATE_RUNNING) ABORT(R_NOT_PERMITTED); @@ -173,13 +170,6 @@ int nr_stun_client_restart(nr_stun_client_ctx *ctx) finished_cb = ctx->finished_cb; cb_arg = ctx->cb_arg; - if (nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_ERROR_CODE, &ec) - && ec->u.error_code.number == 300) { - if (nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_ALTERNATE_SERVER, &as)) { - nr_transport_addr_copy(&ctx->peer_addr, &as->u.alternate_server); - } - } - nr_stun_client_reset(ctx); if (r=nr_stun_client_start(ctx, mode, finished_cb, cb_arg)) diff --git a/src/stun/stun_proc.c b/src/stun/stun_proc.c index b0679e0..13366e2 100644 --- a/src/stun/stun_proc.c +++ b/src/stun/stun_proc.c @@ -251,27 +251,10 @@ nr_stun_process_error_response(nr_stun_message *res, UINT2 *error_code) switch (attr->u.error_code.number / 100) { case 3: - /* If the error code is 300 through 399, the client SHOULD consider - * the transaction as failed unless the ALTERNATE-SERVER extension is - * being used. See Section 11. */ - - if (attr->u.error_code.number == 300) { - if (!nr_stun_message_has_attribute(res, NR_STUN_ATTR_ALTERNATE_SERVER, 0)) { - r_log(NR_LOG_STUN, LOG_WARNING, "Missing ALTERNATE-SERVER"); - ABORT(R_REJECTED); - } - - /* draft-ietf-behave-rfc3489bis-10.txt S 11 */ - if (!nr_stun_message_has_attribute(res, NR_STUN_ATTR_MESSAGE_INTEGRITY, 0)) { - r_log(NR_LOG_STUN, LOG_WARNING, "Missing MESSAGE-INTEGRITY"); - ABORT(R_REJECTED); - } - - ABORT(R_RETRY); - } - - ABORT(R_REJECTED); - break; + /* We do not treat STUN/300 as retryable. The TURN Allocate handling + * code will reset the ctx if appropriate. */ + ABORT(R_REJECTED); + break; case 4: /* If the error code is 400 through 499, the client declares the From 0669ac6ff10ee5485f69558dc3da1eeaf0f2a661 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:42 +0000 Subject: [PATCH 33/41] Bug 857668: Handle Allocate/300 with ALTERNATE-SERVER. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115286 --- src/net/transport_addr.c | 26 +++++++++ src/net/transport_addr.h | 2 + src/stun/stun_client_ctx.c | 5 +- src/stun/stun_client_ctx.h | 3 +- src/stun/turn_client_ctx.c | 107 ++++++++++++++++++++++++++++++++++++- 5 files changed, 138 insertions(+), 5 deletions(-) diff --git a/src/net/transport_addr.c b/src/net/transport_addr.c index 4455018..efedb37 100644 --- a/src/net/transport_addr.c +++ b/src/net/transport_addr.c @@ -185,6 +185,32 @@ int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, const nr_transport return _status; } +int nr_transport_addr_copy_addrport(nr_transport_addr *to, const nr_transport_addr *from) + { + int r,_status; + + switch (from->ip_version) { + case NR_IPV4: + memcpy(&to->u.addr4, &from->u.addr4, sizeof(to->u.addr4)); + break; + case NR_IPV6: + memcpy(&to->u.addr6, &from->u.addr6, sizeof(to->u.addr6)); + break; + default: + ABORT(R_BAD_ARGS); + } + + to->ip_version = from->ip_version; + + if (r=nr_transport_addr_fmt_addr_string(to)) { + ABORT(r); + } + + _status=0; + abort: + return _status; + } + /* Convenience fxn. Is this the right API?*/ int nr_ip4_port_to_transport_addr(UINT4 ip4, UINT2 port, int protocol, nr_transport_addr *addr) { diff --git a/src/net/transport_addr.h b/src/net/transport_addr.h index 7175dac..c75c70a 100644 --- a/src/net/transport_addr.h +++ b/src/net/transport_addr.h @@ -101,6 +101,8 @@ int nr_transport_addr_is_teredo(const nr_transport_addr *addr); int nr_transport_addr_check_compatibility(const nr_transport_addr *addr1, const nr_transport_addr *addr2); int nr_transport_addr_copy(nr_transport_addr *to, const nr_transport_addr *from); int nr_transport_addr_copy_keep_ifname(nr_transport_addr *to, const nr_transport_addr *from); +/* Copies _just_ the address and port (also handles IP version) */ +int nr_transport_addr_copy_addrport(nr_transport_addr *to, const nr_transport_addr *from); int nr_transport_addr_fmt_addr_string(nr_transport_addr *addr); int nr_transport_addr_fmt_ifname_addr_string(const nr_transport_addr *addr, char *buf, int len); int nr_transport_addr_set_port(nr_transport_addr *addr, int port); diff --git a/src/stun/stun_client_ctx.c b/src/stun/stun_client_ctx.c index 52ae39f..12bf310 100644 --- a/src/stun/stun_client_ctx.c +++ b/src/stun/stun_client_ctx.c @@ -151,8 +151,8 @@ int nr_stun_client_start(nr_stun_client_ctx *ctx, int mode, NR_async_cb finished return(_status); } -int nr_stun_client_restart(nr_stun_client_ctx *ctx) - { + int nr_stun_client_restart(nr_stun_client_ctx* ctx, + const nr_transport_addr* peer_addr) { int r,_status; int mode; NR_async_cb finished_cb; @@ -171,6 +171,7 @@ int nr_stun_client_restart(nr_stun_client_ctx *ctx) cb_arg = ctx->cb_arg; nr_stun_client_reset(ctx); + nr_transport_addr_copy(&ctx->peer_addr, peer_addr); if (r=nr_stun_client_start(ctx, mode, finished_cb, cb_arg)) ABORT(r); diff --git a/src/stun/stun_client_ctx.h b/src/stun/stun_client_ctx.h index 53f5db2..3508b34 100644 --- a/src/stun/stun_client_ctx.h +++ b/src/stun/stun_client_ctx.h @@ -186,7 +186,8 @@ struct nr_stun_client_ctx_ { int nr_stun_client_ctx_create(char *label, nr_socket *sock, nr_transport_addr *peer, UINT4 RTO, nr_stun_client_ctx **ctxp); int nr_stun_client_start(nr_stun_client_ctx *ctx, int mode, NR_async_cb finished_cb, void *cb_arg); -int nr_stun_client_restart(nr_stun_client_ctx *ctx); +int nr_stun_client_restart(nr_stun_client_ctx* ctx, + const nr_transport_addr* peer_addr); int nr_stun_client_force_retransmit(nr_stun_client_ctx *ctx); int nr_stun_client_reset(nr_stun_client_ctx *ctx); int nr_stun_client_ctx_destroy(nr_stun_client_ctx **ctxp); diff --git a/src/stun/turn_client_ctx.c b/src/stun/turn_client_ctx.c index 008d3c2..4d7721f 100644 --- a/src/stun/turn_client_ctx.c +++ b/src/stun/turn_client_ctx.c @@ -216,6 +216,101 @@ static int nr_turn_stun_ctx_start(nr_turn_stun_ctx *ctx) return _status; } +static int nr_turn_stun_ctx_handle_redirect(nr_turn_stun_ctx *ctx) +{ + int r, _status; + nr_turn_client_ctx *tctx = ctx->tctx; + nr_stun_message_attribute *ec; + nr_stun_message_attribute *attr; + nr_transport_addr *alternate_addr = 0; + int index = 0; + + if (ctx->stun->response->header.type != NR_STUN_MSG_ALLOCATE_ERROR_RESPONSE) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect called for something " + "other than an Allocate error response (type=%d)", + tctx->label, ctx->stun->response->header.type); + ABORT(R_BAD_ARGS); + } + + if (!nr_stun_message_has_attribute(ctx->stun->response, + NR_STUN_ATTR_ERROR_CODE, &ec) || + (ec->u.error_code.number != 300)) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect called without a " + "300 response", + tctx->label); + ABORT(R_BAD_ARGS); + } + + while (!alternate_addr && !nr_stun_message_get_attribute( + ctx->stun->response, NR_STUN_ATTR_ALTERNATE_SERVER, index++, &attr)) { + alternate_addr = &attr->u.alternate_server; + + // TODO: Someday we may need to handle IP version switching, but it is + // unclear how that is supposed to work with ICE when the IP version of + // the candidate's base address is fixed... + if (alternate_addr->ip_version != tctx->turn_server_addr.ip_version) { + r_log(NR_LOG_TURN, LOG_INFO, + "TURN(%s): nr_turn_stun_ctx_handle_redirect not trying %s, since it is a different IP version", + tctx->label, alternate_addr->as_string); + alternate_addr = 0; + continue; + } + } + + if (!alternate_addr) { + /* Should we use a different error code depending on why we didn't find + * one? (eg; no ALTERNATE-SERVERS at all, none that we have not tried + * already, none that are of a compatible IP version?) */ + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect did not find a viable " + "ALTERNATE-SERVER", + tctx->label); + ABORT(R_FAILED); + } + + r_log(NR_LOG_TURN, LOG_INFO, + "TURN(%s): nr_turn_stun_ctx_handle_redirect trying %s", + tctx->label, alternate_addr->as_string); + + /* This also handles the call to nr_transport_addr_fmt_addr_string */ + if ((r = nr_transport_addr_copy_addrport(&tctx->turn_server_addr, + alternate_addr))) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect copying ALTERNATE-SERVER " + "failed(%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + + /* TURN server address is now updated. Restart the STUN Allocate ctx. Note + * that we do not attempt to update the local address; if the TURN server + * redirects to something that is not best reached from the already-selected + * local address, oh well. */ + + if (tctx->turn_server_addr.protocol == IPPROTO_TCP) { + /* TCP support in subsequent patch */ + ABORT(R_FAILED); + } + + nr_transport_addr_copy(&ctx->stun->peer_addr, &tctx->turn_server_addr); + + if ((r = nr_turn_stun_ctx_start(ctx))) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect nr_turn_stun_ctx_start " + "failed(%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + + _status = 0; +abort: + return _status; +} + static void nr_turn_stun_ctx_cb(NR_SOCKET s, int how, void *arg) { int r, _status; @@ -287,8 +382,16 @@ static void nr_turn_stun_ctx_cb(NR_SOCKET s, int how, void *arg) } ctx->retry_ct++; - } - else { + } else if (ctx->stun->error_code == 300) { + r_log(NR_LOG_TURN, LOG_INFO, + "TURN(%s): Redirect received, restarting TURN", ctx->tctx->label); + /* We don't limit this with a retry counter, we limit redirects by + * checking whether we've tried the ALTERNATE-SERVER yet. */ + ctx->retry_ct = 0; + if ((r = nr_turn_stun_ctx_handle_redirect(ctx))) { + ABORT(r); + } + } else { ABORT(R_FAILED); } break; From f15427c471786f3c65e4cdc0ff5ca9e1516d9d8b Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:42 +0000 Subject: [PATCH 34/41] Bug 857668: Fix handling of IPv6 literals. r=mjf Also some logging that was useful in debugging this. Differential Revision: https://phabricator.services.mozilla.com/D115287 --- src/ice/ice_component.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ice/ice_component.c b/src/ice/ice_component.c index 6e48796..b35f1e2 100644 --- a/src/ice/ice_component.c +++ b/src/ice/ice_component.c @@ -337,6 +337,8 @@ static int nr_ice_component_initialize_udp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_socket *turn_sock; nr_ice_candidate *srvflx_cand=0; + r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): Checking TURN server %s %s", ctx->label, ctx->turn_servers[j].turn_server.addr.fqdn, ctx->turn_servers[j].turn_server.addr.as_string); + /* Skip non-UDP */ if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_UDP) continue; @@ -575,6 +577,8 @@ static int nr_ice_component_initialize_tcp(struct nr_ice_ctx_ *ctx,nr_ice_compon nr_socket *turn_sock; nr_ice_socket *turn_isock; + r_log(LOG_ICE,LOG_DEBUG,"ICE(%s): Checking TURN server %s %s", ctx->label, ctx->turn_servers[j].turn_server.addr.fqdn, ctx->turn_servers[j].turn_server.addr.as_string); + /* Skip non-TCP */ if (ctx->turn_servers[j].turn_server.addr.protocol != IPPROTO_TCP) continue; From dfae3e15e1d8ddc9978be81b3da7e005f4f56c07 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:43 +0000 Subject: [PATCH 35/41] Bug 857668: Let Allocate/300 without MESSAGE-INTEGRITY slide. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115289 --- src/stun/stun_client_ctx.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/stun/stun_client_ctx.c b/src/stun/stun_client_ctx.c index 12bf310..55b19c7 100644 --- a/src/stun/stun_client_ctx.c +++ b/src/stun/stun_client_ctx.c @@ -451,6 +451,7 @@ int nr_stun_client_process_response(nr_stun_client_ctx *ctx, UCHAR *msg, int len char string[256]; char *username = 0; Data *password = 0; + int allow_unauthed_redirect = 0; nr_stun_message_attribute *attr; nr_transport_addr *mapped_addr = 0; int fail_on_error = 0; @@ -501,10 +502,12 @@ int nr_stun_client_process_response(nr_stun_client_ctx *ctx, UCHAR *msg, int len case NR_TURN_CLIENT_MODE_ALLOCATE_REQUEST: fail_on_error = 1; compute_lt_key = 1; - username = ctx->auth_params.username; - password = &ctx->auth_params.password; - /* do nothing */ - break; + /* Do not require mutual auth on redirect responses to Allocate requests. */ + allow_unauthed_redirect = 1; + username = ctx->auth_params.username; + password = &ctx->auth_params.password; + /* do nothing */ + break; case NR_TURN_CLIENT_MODE_REFRESH_REQUEST: fail_on_error = 1; compute_lt_key = 1; @@ -569,6 +572,13 @@ int nr_stun_client_process_response(nr_stun_client_ctx *ctx, UCHAR *msg, int len "STUN-CLIENT(%s): Received response; processing",ctx->label); response_matched=1; + if (allow_unauthed_redirect && + nr_stun_message_has_attribute(ctx->response, NR_STUN_ATTR_ERROR_CODE, + &attr) && + (attr->u.error_code.number / 100 == 3)) { + password = 0; + } + /* TODO: !nn! currently using password!=0 to mean that auth is required, * TODO: !nn! but we should probably pass that in explicitly via the * TODO: !nn! usage (ctx->mode?) */ From a10d94de0bfab322aefdbdbd473856b84d1c8477 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:43 +0000 Subject: [PATCH 36/41] Bug 857668: Stop ignoring error responses from STUN/TURN servers. r=mjf Also some indentation fixup. Differential Revision: https://phabricator.services.mozilla.com/D115290 --- src/stun/stun_client_ctx.c | 57 +++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 23 deletions(-) diff --git a/src/stun/stun_client_ctx.c b/src/stun/stun_client_ctx.c index 55b19c7..6c36658 100644 --- a/src/stun/stun_client_ctx.c +++ b/src/stun/stun_client_ctx.c @@ -475,27 +475,38 @@ int nr_stun_client_process_response(nr_stun_client_ctx *ctx, UCHAR *msg, int len /* determine password */ switch (ctx->mode) { case NR_STUN_CLIENT_MODE_BINDING_REQUEST_LONG_TERM_AUTH: + /* If the STUN server responds with an error, give up, since we don't + * want to delay the completion of gathering. */ + fail_on_error = 1; compute_lt_key = 1; /* Fall through */ case NR_STUN_CLIENT_MODE_BINDING_REQUEST_SHORT_TERM_AUTH: - password = ctx->params.stun_binding_request.password; - break; + password = ctx->params.stun_binding_request.password; + break; case NR_STUN_CLIENT_MODE_BINDING_REQUEST_NO_AUTH: - /* do nothing */ - break; + /* If the STUN server responds with an error, give up, since we don't + * want to delay the completion of gathering. */ + fail_on_error = 1; + break; case NR_STUN_CLIENT_MODE_BINDING_REQUEST_STUND_0_96: - /* do nothing */ - break; + /* If the STUN server responds with an error, give up, since we don't + * want to delay the completion of gathering. */ + fail_on_error = 1; + break; #ifdef USE_ICE case NR_ICE_CLIENT_MODE_BINDING_REQUEST: - password = &ctx->params.ice_binding_request.password; - break; + /* We do not set fail_on_error here. The error might be transient, and + * retrying isn't going to cause a slowdown. */ + password = &ctx->params.ice_binding_request.password; + break; case NR_ICE_CLIENT_MODE_USE_CANDIDATE: - password = &ctx->params.ice_binding_request.password; - break; + /* We do not set fail_on_error here. The error might be transient, and + * retrying isn't going to cause a slowdown. */ + password = &ctx->params.ice_binding_request.password; + break; #endif /* USE_ICE */ #ifdef USE_TURN @@ -511,26 +522,26 @@ int nr_stun_client_process_response(nr_stun_client_ctx *ctx, UCHAR *msg, int len case NR_TURN_CLIENT_MODE_REFRESH_REQUEST: fail_on_error = 1; compute_lt_key = 1; - username = ctx->auth_params.username; - password = &ctx->auth_params.password; - /* do nothing */ - break; + username = ctx->auth_params.username; + password = &ctx->auth_params.password; + /* do nothing */ + break; case NR_TURN_CLIENT_MODE_PERMISSION_REQUEST: fail_on_error = 1; compute_lt_key = 1; - username = ctx->auth_params.username; - password = &ctx->auth_params.password; - /* do nothing */ - break; + username = ctx->auth_params.username; + password = &ctx->auth_params.password; + /* do nothing */ + break; case NR_TURN_CLIENT_MODE_SEND_INDICATION: - /* do nothing -- we just got our DATA-INDICATION */ - break; + /* do nothing -- we just got our DATA-INDICATION */ + break; #endif /* USE_TURN */ default: - assert(0); - ABORT(R_FAILED); - break; + assert(0); + ABORT(R_FAILED); + break; } if (compute_lt_key) { From de786002a5a018f8b39ccd8702a70ac36eb47084 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:44 +0000 Subject: [PATCH 37/41] Bug 857668: Open a new TCP socket when we receive a redirect in the TCP case. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115291 --- src/ice/ice_candidate.c | 1 + src/stun/nr_socket_buffered_stun.c | 44 ++++++++++++++++++ src/stun/nr_socket_buffered_stun.h | 2 + src/stun/turn_client_ctx.c | 71 +++++++++++++++++++++++++++--- src/stun/turn_client_ctx.h | 17 +++++-- 5 files changed, 124 insertions(+), 11 deletions(-) diff --git a/src/ice/ice_candidate.c b/src/ice/ice_candidate.c index c423fd1..1e286c9 100644 --- a/src/ice/ice_candidate.c +++ b/src/ice/ice_candidate.c @@ -843,6 +843,7 @@ static int nr_ice_start_relay_turn(nr_ice_candidate *cand) cand->u.relayed.server->username, cand->u.relayed.server->password, &cand->stun_server_addr, + cand->component->ctx, &cand->u.relayed.turn)) ABORT(r); diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index 5c06519..caee74e 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -205,6 +205,7 @@ int nr_socket_buffered_stun_destroy(void **objp) /* Cancel waiting on the socket */ if (sock->inner && !nr_socket_getfd(sock->inner, &fd)) { NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_WRITE); + NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_READ); } nr_p_buf_free_chain(sock->p_bufs, &sock->pending_writes); @@ -610,3 +611,46 @@ static void nr_socket_buffered_stun_writable_cb(NR_SOCKET s, int how, void *arg) nr_socket_buffered_stun_arm_writable_cb(sock); } } + +int nr_socket_buffered_stun_reset(nr_socket* sock_arg, nr_socket* new_inner) { + int r, _status; + NR_SOCKET fd; + + nr_socket_buffered_stun* sock = (nr_socket_buffered_stun*)sock_arg->obj; + + if (sock->inner && !nr_socket_getfd(sock->inner, &fd)) { + r_log(LOG_GENERIC, LOG_DEBUG, "In %s, canceling wait on old socket", __FUNCTION__); + NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_WRITE); + NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_READ); + } + + nr_socket_destroy(&sock->inner); + sock->inner = new_inner; + + sock->read_state = NR_ICE_SOCKET_READ_NONE; + sock->connected = 0; + + sock->bytes_read = 0; + sock->bytes_needed = (sock->framing_type == ICE_TCP_FRAMING) + ? sizeof(nr_frame_header) + : sizeof(nr_stun_message_header); + sock->pending = 0; + + nr_p_buf_free_chain(sock->p_bufs, &sock->pending_writes); + nr_p_buf_ctx_destroy(&sock->p_bufs); + + STAILQ_INIT(&sock->pending_writes); + + if ((r = nr_p_buf_ctx_create(sock->buffer_size, &sock->p_bufs))) { + ABORT(r); + } + + if ((r = nr_ip4_port_to_transport_addr(INADDR_ANY, 0, IPPROTO_UDP, + &sock->remote_addr))) { + ABORT(r); + } + + _status = 0; +abort: + return (_status); +} diff --git a/src/stun/nr_socket_buffered_stun.h b/src/stun/nr_socket_buffered_stun.h index fa65e4b..c635ee3 100644 --- a/src/stun/nr_socket_buffered_stun.h +++ b/src/stun/nr_socket_buffered_stun.h @@ -60,5 +60,7 @@ int nr_socket_buffered_stun_create(nr_socket *inner, int max_pending, int nr_socket_buffered_set_connected_to(nr_socket *sock, nr_transport_addr *remote_addr); +int nr_socket_buffered_stun_reset(nr_socket *sock, nr_socket *new_inner); + #endif diff --git a/src/stun/turn_client_ctx.c b/src/stun/turn_client_ctx.c index 4d7721f..272b7e3 100644 --- a/src/stun/turn_client_ctx.c +++ b/src/stun/turn_client_ctx.c @@ -44,6 +44,7 @@ #include "nr_socket_buffered_stun.h" #include "stun.h" #include "turn_client_ctx.h" +#include "ice_ctx.h" int NR_LOG_TURN = 0; @@ -225,6 +226,17 @@ static int nr_turn_stun_ctx_handle_redirect(nr_turn_stun_ctx *ctx) nr_transport_addr *alternate_addr = 0; int index = 0; + if (!tctx->ctx) { + /* If we were to require TCP nr_sockets to allow multiple connect calls by + * disconnecting and re-connecting, we could avoid this requirement. */ + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect is not supported when " + "there is no ICE ctx, and by extension no socket factory, since we " + "need that to create new sockets in the TCP case", + tctx->label); + ABORT(R_BAD_ARGS); + } + if (ctx->stun->response->header.type != NR_STUN_MSG_ALLOCATE_ERROR_RESPONSE) { r_log(NR_LOG_TURN, LOG_ERR, "TURN(%s): nr_turn_stun_ctx_handle_redirect called for something " @@ -291,8 +303,52 @@ static int nr_turn_stun_ctx_handle_redirect(nr_turn_stun_ctx *ctx) * local address, oh well. */ if (tctx->turn_server_addr.protocol == IPPROTO_TCP) { - /* TCP support in subsequent patch */ - ABORT(R_FAILED); + /* For TCP, we need to replace the underlying nr_socket, since we cannot + * un-connect it from the old server. */ + /* If we were to require TCP nr_sockets to allow multiple connect calls by + * disconnecting and re-connecting, we could avoid this stuff, and just + * call nr_socket_connect. */ + nr_transport_addr old_local_addr; + nr_socket* new_socket; + if ((r = nr_socket_getaddr(tctx->sock, &old_local_addr))) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect " + "failed to get old local address (%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + + if ((r = nr_socket_factory_create_socket(tctx->ctx->socket_factory, + &old_local_addr, &new_socket))) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect " + "failed to create new raw TCP socket for redirect (%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + + if ((r = nr_socket_buffered_stun_reset(tctx->sock, new_socket))) { + /* nr_socket_buffered_stun_reset always takes ownership of |new_socket| */ + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect " + "failed to update raw TCP socket (%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + + if ((r = nr_socket_connect(tctx->sock, &tctx->turn_server_addr))) { + if (r != R_WOULDBLOCK) { + r_log(NR_LOG_TURN, LOG_ERR, + "TURN(%s): nr_turn_stun_ctx_handle_redirect nr_socket_connect " + "failed(%d)!", + tctx->label, r); + assert(0); + ABORT(r); + } + } } nr_transport_addr_copy(&ctx->stun->peer_addr, &tctx->turn_server_addr); @@ -418,11 +474,10 @@ static void nr_turn_stun_ctx_cb(NR_SOCKET s, int how, void *arg) } /* nr_turn_client_ctx functions */ -int nr_turn_client_ctx_create(const char *label, nr_socket *sock, - const char *username, Data *password, - nr_transport_addr *addr, - nr_turn_client_ctx **ctxp) -{ +int nr_turn_client_ctx_create(const char* label, nr_socket* sock, + const char* username, Data* password, + nr_transport_addr* addr, nr_ice_ctx* ice_ctx, + nr_turn_client_ctx** ctxp) { nr_turn_client_ctx *ctx=0; int r,_status; @@ -456,6 +511,8 @@ int nr_turn_client_ctx_create(const char *label, nr_socket *sock, } } + ctx->ctx = ice_ctx; + *ctxp=ctx; _status=0; diff --git a/src/stun/turn_client_ctx.h b/src/stun/turn_client_ctx.h index a9eb235..e97504a 100644 --- a/src/stun/turn_client_ctx.h +++ b/src/stun/turn_client_ctx.h @@ -38,6 +38,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #ifndef _turn_client_ctx_h #define _turn_client_ctx_h +struct nr_ice_ctx_; + /* Represents a single set of STUN transactions, i.e., Allocate, Refresh, Permission. It automatically handles @@ -96,6 +98,12 @@ typedef struct nr_turn_client_ctx_ { nr_turn_stun_ctx_head stun_ctxs; nr_turn_permission_head permissions; + /* We need access to the socket factory to create new TCP sockets for handling + * STUN/300 responses. */ + /* If we were to require TCP nr_sockets to allow multiple connect calls by + * disconnecting and re-connecting, we could avoid this requirement. */ + struct nr_ice_ctx_* ctx; + NR_async_cb finished_cb; void *cb_arg; @@ -110,10 +118,11 @@ typedef struct nr_turn_client_ctx_ { extern int NR_LOG_TURN; -int nr_turn_client_ctx_create(const char *label, nr_socket *sock, - const char *username, Data *password, - nr_transport_addr *addr, - nr_turn_client_ctx **ctxp); +int nr_turn_client_ctx_create(const char* label, nr_socket* sock, + const char* username, Data* password, + nr_transport_addr* addr, + struct nr_ice_ctx_* ice_ctx, + nr_turn_client_ctx** ctxp); int nr_turn_client_ctx_destroy(nr_turn_client_ctx **ctxp); int nr_turn_client_allocate(nr_turn_client_ctx *ctx, NR_async_cb finished_cb, void *cb_arg); From 524ef676168be3d33cab5ce575ea9d2b1c44acea Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:44 +0000 Subject: [PATCH 38/41] Bug 857668: Implement redirect loop detection. r=mjf Differential Revision: https://phabricator.services.mozilla.com/D115292 --- src/stun/turn_client_ctx.c | 55 ++++++++++++++++++++++++++++++++++++++ src/stun/turn_client_ctx.h | 10 +++++++ 2 files changed, 65 insertions(+) diff --git a/src/stun/turn_client_ctx.c b/src/stun/turn_client_ctx.c index 272b7e3..707d43a 100644 --- a/src/stun/turn_client_ctx.c +++ b/src/stun/turn_client_ctx.c @@ -92,6 +92,33 @@ static int nr_turn_client_send_stun_request(nr_turn_client_ctx *ctx, nr_stun_message *req, int flags); +int nr_transport_addr_listnode_create(const nr_transport_addr *addr, nr_transport_addr_listnode **listnodep) +{ + nr_transport_addr_listnode *listnode = 0; + int r,_status; + + if (!(listnode=RCALLOC(sizeof(nr_transport_addr_listnode)))) { + ABORT(R_NO_MEMORY); + } + + if ((r = nr_transport_addr_copy(&listnode->value, addr))) { + ABORT(r); + } + + *listnodep = listnode; + listnode = 0; + _status = 0; + +abort: + nr_transport_addr_listnode_destroy(&listnode); + return(_status); +} + +void nr_transport_addr_listnode_destroy(nr_transport_addr_listnode **listnode) +{ + RFREE(*listnode); + *listnode = 0; +} /* nr_turn_stun_ctx functions */ static int nr_turn_stun_ctx_create(nr_turn_client_ctx *tctx, int mode, @@ -126,6 +153,7 @@ static int nr_turn_stun_ctx_create(nr_turn_client_ctx *tctx, int mode, sctx->error_cb=error_cb; sctx->mode=mode; sctx->last_error_code=0; + STAILQ_INIT(&sctx->addresses_tried); /* Add ourselves to the tctx's list */ STAILQ_INSERT_TAIL(&tctx->stun_ctxs, sctx, entry); @@ -154,6 +182,12 @@ static int nr_turn_stun_ctx_destroy(nr_turn_stun_ctx **ctxp) RFREE(ctx->realm); RFREE(ctx->nonce); + while (!STAILQ_EMPTY(&ctx->addresses_tried)) { + nr_transport_addr_listnode *listnode = STAILQ_FIRST(&ctx->addresses_tried); + STAILQ_REMOVE_HEAD(&ctx->addresses_tried, entry); + nr_transport_addr_listnode_destroy(&listnode); + } + RFREE(ctx); return 0; @@ -199,6 +233,7 @@ static int nr_turn_stun_ctx_start(nr_turn_stun_ctx *ctx) { int r, _status; nr_turn_client_ctx *tctx = ctx->tctx; + nr_transport_addr_listnode *address_tried = 0; if ((r=nr_stun_client_reset(ctx->stun))) { r_log(NR_LOG_TURN, LOG_ERR, "TURN(%s): Couldn't reset STUN", @@ -212,6 +247,12 @@ static int nr_turn_stun_ctx_start(nr_turn_stun_ctx *ctx) ABORT(r); } + if ((r=nr_transport_addr_listnode_create(&ctx->stun->peer_addr, &address_tried))) { + ABORT(r); + } + + STAILQ_INSERT_TAIL(&ctx->addresses_tried, address_tried, entry); + _status=0; abort: return _status; @@ -269,6 +310,20 @@ static int nr_turn_stun_ctx_handle_redirect(nr_turn_stun_ctx *ctx) alternate_addr = 0; continue; } + + /* Check if we've already tried this, and ignore if we have */ + nr_transport_addr_listnode *address_tried = 0; + STAILQ_FOREACH(address_tried, &ctx->addresses_tried, entry) { + /* Ignore protocol */ + alternate_addr->protocol = address_tried->value.protocol; + if (!nr_transport_addr_cmp(alternate_addr, &address_tried->value, NR_TRANSPORT_ADDR_CMP_MODE_ALL)) { + r_log(NR_LOG_TURN, LOG_INFO, + "TURN(%s): nr_turn_stun_ctx_handle_redirect already tried %s, ignoring", + tctx->label, alternate_addr->as_string); + alternate_addr = 0; + break; + } + } } if (!alternate_addr) { diff --git a/src/stun/turn_client_ctx.h b/src/stun/turn_client_ctx.h index e97504a..2049f1b 100644 --- a/src/stun/turn_client_ctx.h +++ b/src/stun/turn_client_ctx.h @@ -40,6 +40,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. struct nr_ice_ctx_; +typedef struct nr_transport_addr_listnode_ { + nr_transport_addr value; + STAILQ_ENTRY(nr_transport_addr_listnode_) entry; +} nr_transport_addr_listnode; +typedef STAILQ_HEAD(nr_transport_addr_listnode_head_, nr_transport_addr_listnode_) nr_transport_addr_listnode_head; + /* Represents a single set of STUN transactions, i.e., Allocate, Refresh, Permission. It automatically handles @@ -56,6 +62,8 @@ typedef struct nr_turn_stun_ctx_ { NR_async_cb error_cb; int last_error_code; + nr_transport_addr_listnode_head addresses_tried; + STAILQ_ENTRY(nr_turn_stun_ctx_) entry; } nr_turn_stun_ctx; typedef STAILQ_HEAD(nr_turn_stun_ctx_head_, nr_turn_stun_ctx_) @@ -118,6 +126,8 @@ typedef struct nr_turn_client_ctx_ { extern int NR_LOG_TURN; +int nr_transport_addr_listnode_create(const nr_transport_addr *addr, nr_transport_addr_listnode **listnodep); +void nr_transport_addr_listnode_destroy(nr_transport_addr_listnode **listnode); int nr_turn_client_ctx_create(const char* label, nr_socket* sock, const char* username, Data* password, nr_transport_addr* addr, From 16fe857b7afaf52c73dbcd8c306db8e15ce8b536 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:44 +0000 Subject: [PATCH 39/41] Bug 857668: Remove this extra retry_ct field. r=mjf Nothing used this function before this bug, so this field has never been used anyway. We handle challenge loops with nr_turn_stun_ctx.retry_ct. Differential Revision: https://phabricator.services.mozilla.com/D115293 --- src/stun/stun_client_ctx.c | 6 ------ src/stun/stun_client_ctx.h | 1 - 2 files changed, 7 deletions(-) diff --git a/src/stun/stun_client_ctx.c b/src/stun/stun_client_ctx.c index 6c36658..0fe159b 100644 --- a/src/stun/stun_client_ctx.c +++ b/src/stun/stun_client_ctx.c @@ -160,12 +160,6 @@ int nr_stun_client_start(nr_stun_client_ctx *ctx, int mode, NR_async_cb finished if (ctx->state != NR_STUN_CLIENT_STATE_RUNNING) ABORT(R_NOT_PERMITTED); - assert(ctx->retry_ct <= 2); - if (ctx->retry_ct > 2) - ABORT(R_NOT_PERMITTED); - - ++ctx->retry_ct; - mode = ctx->mode; finished_cb = ctx->finished_cb; cb_arg = ctx->cb_arg; diff --git a/src/stun/stun_client_ctx.h b/src/stun/stun_client_ctx.h index 3508b34..0cc9045 100644 --- a/src/stun/stun_client_ctx.h +++ b/src/stun/stun_client_ctx.h @@ -176,7 +176,6 @@ struct nr_stun_client_ctx_ { UINT4 mapped_addr_check_mask; /* What checks to run on mapped addresses */ int timeout_ms; struct timeval timer_set; - int retry_ct; NR_async_cb finished_cb; void *cb_arg; nr_stun_message *request; From 3dd888a4a167e649679256c0e1c3700b58573376 Mon Sep 17 00:00:00 2001 From: "Byron Campen [:bwc]" Date: Thu, 27 May 2021 21:08:45 +0000 Subject: [PATCH 40/41] Bug 857668: Use IPPROTO_TCP here. r=mjf We only use this with TCP sockets. Depends on D115293 Differential Revision: https://phabricator.services.mozilla.com/D115829 --- src/stun/nr_socket_buffered_stun.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/stun/nr_socket_buffered_stun.c b/src/stun/nr_socket_buffered_stun.c index caee74e..4b5b92f 100644 --- a/src/stun/nr_socket_buffered_stun.c +++ b/src/stun/nr_socket_buffered_stun.c @@ -143,7 +143,7 @@ int nr_socket_buffered_stun_create(nr_socket *inner, int max_pending, sock->inner = inner; sock->framing_type = framing_type; - if ((r=nr_ip4_port_to_transport_addr(INADDR_ANY, 0, IPPROTO_UDP, &sock->remote_addr))) + if ((r=nr_ip4_port_to_transport_addr(INADDR_ANY, 0, IPPROTO_TCP, &sock->remote_addr))) ABORT(r); switch (framing_type) { @@ -645,7 +645,7 @@ int nr_socket_buffered_stun_reset(nr_socket* sock_arg, nr_socket* new_inner) { ABORT(r); } - if ((r = nr_ip4_port_to_transport_addr(INADDR_ANY, 0, IPPROTO_UDP, + if ((r = nr_ip4_port_to_transport_addr(INADDR_ANY, 0, IPPROTO_TCP, &sock->remote_addr))) { ABORT(r); } From 0341cc29ccdfbe31f7493cf9bedc02d349a54e56 Mon Sep 17 00:00:00 2001 From: vpoddubchak Date: Fri, 6 Aug 2021 15:25:39 +0300 Subject: [PATCH 41/41] fixed build --- src/net/nr_socket_local.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/net/nr_socket_local.c b/src/net/nr_socket_local.c index 57b88a1..6ea7f74 100644 --- a/src/net/nr_socket_local.c +++ b/src/net/nr_socket_local.c @@ -107,14 +107,18 @@ int nr_socket_local_create(void *obj, nr_transport_addr *addr, nr_socket **sockp ABORT(R_NO_MEMORY); lcl->sock=-1; - if((lcl->sock=socket(addr->addr->sa_family, stype, addr->protocol))<0){ + + if((lcl->sock=socket((addr->protocol == NR_IPV4) ? + addr->u.addr4.sin_family : + addr->u.addr6.sin6_family, stype, addr->protocol))<0){ r_log(LOG_GENERIC,LOG_CRIT,"Couldn't create socket"); //r_log_e(LOG_GENERIC,LOG_CRIT,"Couldn't create socket"); ABORT(R_INTERNAL); } - if(bind(lcl->sock, addr->addr, addr->addr_len)<0){ + if(bind(lcl->sock, (addr->protocol == NR_IPV4) ? &(addr->u.addr4) : &(addr->u.addr6), + (addr->protocol == NR_IPV4) ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6))<0){ r_log(LOG_GENERIC,LOG_CRIT,"Couldn't bind socket to address %s",addr->as_string); //r_log_e(LOG_GENERIC,LOG_CRIT,"Couldn't bind socket to address %s",addr->as_string); ABORT(R_INTERNAL); @@ -197,7 +201,9 @@ static int nr_socket_local_sendto(void *obj,const void *msg, size_t len, //r_log(LOG_GENERIC,LOG_DEBUG,"Writing to sock (%x:%d), len=%d",lcl,lcl->sock,len); - if((r=sendto(lcl->sock, msg, len, flags, addr->addr, addr->addr_len))<0){ + if((r=sendto(lcl->sock, msg, len, flags, + (addr->protocol == NR_IPV4) ? &(addr->u.addr4) : &(addr->u.addr6), + (addr->protocol == NR_IPV4) ? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6)))<0){ switch (errno) { case EHOSTUNREACH: level = LOG_INFO;