[Asterisk-code-review] res_rtp_asterisk: iterate all local addresses looking to populate ICE. (asterisk[13])
Jaco Kroon
asteriskteam at digium.com
Tue Dec 3 12:41:05 CST 2019
Jaco Kroon has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/13362 )
Change subject: res_rtp_asterisk: iterate all local addresses looking to populate ICE.
......................................................................
res_rtp_asterisk: iterate all local addresses looking to populate ICE.
By using pjproject to give us a list of candidates, and then filtering,
if the host has >32 addresses configured, then it is possible that we
end up filtering out all 32 of those, and ending up with no candidates
at all. Instead, get getifaddrs (which pjsip is using underlying
anyway) to retrieve all local addresses, and iterate those, adding the
first 32 addresses not excluded by the ICE ACL.
In my setup at any point in time I've got between 6 and 328 addresses on
any given system. The lower limit is the lower limit but the upper
limit is growing on a near daily basis currently.
Change-Id: I109eaffc3e2b432f00bf958e3caa0f38cacb4edb
Signed-off-by: Jaco Kroon <jaco at uls.co.za>
---
M res/res_rtp_asterisk.c
1 file changed, 120 insertions(+), 87 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/62/13362/1
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 7ebe5cc..87c137f 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -248,8 +248,8 @@
/*! \brief Structure which contains ICE host candidate mapping information */
struct ast_ice_host_candidate {
- pj_sockaddr local;
- pj_sockaddr advertised;
+ struct ast_sockaddr local;
+ struct ast_sockaddr advertised;
unsigned int include_local;
AST_RWLIST_ENTRY(ast_ice_host_candidate) next;
};
@@ -643,29 +643,6 @@
AST_RWLIST_UNLOCK(&host_candidates);
}
-/*! \brief Applies the ICE host candidate mapping */
-static unsigned int host_candidate_overrides_apply(unsigned int count, unsigned int max_count, pj_sockaddr addrs[])
-{
- int pos;
- struct ast_ice_host_candidate *candidate;
- unsigned int added = 0;
-
- AST_RWLIST_RDLOCK(&host_candidates);
- for (pos = 0; pos < count; pos++) {
- AST_LIST_TRAVERSE(&host_candidates, candidate, next) {
- if (!pj_sockaddr_cmp(&candidate->local, &addrs[pos])) {
- pj_sockaddr_copy_addr(&addrs[pos], &candidate->advertised);
- if (candidate->include_local && (count + (++added)) <= max_count) {
- pj_sockaddr_cp(&addrs[count + (added - 1)], &candidate->local);
- }
- break;
- }
- }
- }
- AST_RWLIST_UNLOCK(&host_candidates);
- return added;
-}
-
/*! \brief Helper function which updates an ast_sockaddr with the candidate used for the component */
static void update_address_with_ice_candidate(pj_ice_sess *ice, enum ast_rtp_ice_component_type component,
struct ast_sockaddr *cand_address)
@@ -3021,18 +2998,12 @@
* \retval 0 if address is not ICE blacklisted
* \retval 1 if address is ICE blacklisted
*/
-static int rtp_address_is_ice_blacklisted(const pj_sockaddr_t *address)
+static int rtp_address_is_ice_blacklisted(const struct ast_sockaddr *address)
{
- struct ast_sockaddr saddr;
int result = 0;
- if (ast_sockaddr_from_pj_sockaddr(&saddr, address) < 0) {
- ast_log(LOG_ERROR, "Failed to convert pj_sockddr_t to ast_sockaddr - ICE blacklisting (default)\n");
- return 1;
- }
-
ast_rwlock_rdlock(&ice_acl_lock);
- result |= ast_apply_acl(ice_acl, &saddr, "RTP ICE ACL: ") == AST_SENSE_DENY;
+ result |= ast_apply_acl(ice_acl, address, "RTP ICE ACL: ") == AST_SENSE_DENY;
ast_rwlock_unlock(&ice_acl_lock);
return result;
@@ -3065,40 +3036,100 @@
static void rtp_add_candidates_to_ice(struct ast_rtp_instance *instance, struct ast_rtp *rtp, struct ast_sockaddr *addr, int port, int component,
int transport)
{
- pj_sockaddr address[PJ_ICE_MAX_CAND];
- unsigned int count = PJ_ARRAY_SIZE(address), pos = 0;
- unsigned int max_count = PJ_ARRAY_SIZE(address);
- int basepos = -1;
+ static struct ast_sockaddr lo6 = { .len = 0 };
- /* Add all the local interface IP addresses */
+ unsigned int count = 0;
+ struct ifaddrs *ifa, *ia;
+ struct ast_sockaddr tmp;
+ pj_sockaddr pjtmp;
+ struct ast_ice_host_candidate *candidate;
+ int af_inet_ok = 0, af_inet6_ok = 0;
+
if (ast_sockaddr_is_ipv4(addr)) {
- pj_enum_ip_interface(pj_AF_INET(), &count, address);
+ af_inet_ok = 1;
} else if (ast_sockaddr_is_any(addr)) {
- pj_enum_ip_interface(pj_AF_UNSPEC(), &count, address);
+ af_inet_ok = af_inet6_ok = 1;
} else {
- pj_enum_ip_interface(pj_AF_INET6(), &count, address);
+ af_inet6_ok = 1;
}
- count += host_candidate_overrides_apply(count, max_count, address);
+ if (getifaddrs(&ifa) < 0) {
+ /* If we can't get addresses, we can't load ICE candidates */
+ ast_log(LOG_ERROR, "Error obtaining list of local addresses: %s\n",
+ strerror(errno));
+ } else {
+ for (ia = ifa; ia && count < PJ_ICE_MAX_CAND; ia = ia->ifa_next) {
+ if (!ia->ifa_addr || (ia->ifa_flags & IFF_UP) == 0)
+ continue;
- for (pos = 0; pos < count; pos++) {
- if (!rtp_address_is_ice_blacklisted(&address[pos])) {
- if (basepos == -1) {
- basepos = pos;
+ ast_sockaddr_setnull(&tmp);
+ ast_sockaddr_from_sockaddr(&tmp, ia->ifa_addr);
+ if (ast_sockaddr_isnull(&tmp))
+ continue;
+
+
+ if (ia->ifa_addr->sa_family == AF_INET) {
+ const struct sockaddr_in *sa_in = (struct sockaddr_in*)ia->ifa_addr;
+ if (!af_inet_ok)
+ continue;
+
+ /* Skip 127.0.0.0/8 (loopback) */
+ /* Don't use IFF_LOOPBACK check since one could assign usable publics to the loopback */
+ if ((sa_in->sin_addr.s_addr & htonl(0xFF000000)) == htonl(0x7F000000))
+ continue;
+
+ /* Skip 0.0.0.0/8 based on RFC1122, and from pjproject */
+ if ((sa_in->sin_addr.s_addr & htonl(0xFF000000)) == 0)
+ continue;
+ } else if (ia->ifa_addr->sa_family != AF_INET6) {
+ if (!af_inet6_ok)
+ continue;
+
+ if (ast_sockaddr_isnull(&lo6)) {
+ ast_sockaddr_parse(&lo6, "::1", PARSE_PORT_IGNORE);
+ }
+
+ /* Filter ::1 */
+ if (ast_sockaddr_cmp_addr(&lo6, &tmp) == 0)
+ continue;
+ } else {
+ /* Only AF_INET (IPv4) and AF_INET6 (IPv6) supported. */
+ continue;
}
- pj_sockaddr_set_port(&address[pos], port);
+
+ AST_RWLIST_RDLOCK(&host_candidates);
+ AST_LIST_TRAVERSE(&host_candidates, candidate, next) {
+ if (ast_sockaddr_cmp(&candidate->local, &tmp) == 0) {
+ if (!rtp_address_is_ice_blacklisted(&candidate->advertised)) {
+ ast_sockaddr_to_pj_sockaddr(&candidate->advertised, &pjtmp);
+ pj_sockaddr_set_port(&pjtmp, port);
+ ast_rtp_ice_add_cand(instance, rtp, component, transport,
+ PJ_ICE_CAND_TYPE_HOST, 65535, &pjtmp, &pjtmp, NULL,
+ pj_sockaddr_get_len(&pjtmp));
+ ++count;
+ }
+
+ if (!candidate->include_local)
+ ast_sockaddr_setnull(&tmp);
+
+ }
+ }
+ AST_RWLIST_UNLOCK(&host_candidates);
+
+ if (ast_sockaddr_isnull(&tmp) || count >= PJ_ICE_MAX_CAND)
+ continue;
+
+ ast_sockaddr_to_pj_sockaddr(&tmp, &pjtmp);
+ pj_sockaddr_set_port(&pjtmp, port);
ast_rtp_ice_add_cand(instance, rtp, component, transport,
- PJ_ICE_CAND_TYPE_HOST, 65535, &address[pos], &address[pos], NULL,
- pj_sockaddr_get_len(&address[pos]));
+ PJ_ICE_CAND_TYPE_HOST, 65535, &pjtmp, &pjtmp, NULL,
+ pj_sockaddr_get_len(&pjtmp));
+ ++count;
}
}
- if (basepos == -1) {
- /* start with first address unless excluded above */
- basepos = 0;
- }
/* If configured to use a STUN server to get our external mapped address do so */
- if (count && stunaddr.sin_addr.s_addr && !stun_address_is_blacklisted(addr) &&
+ if (stunaddr.sin_addr.s_addr && !stun_address_is_blacklisted(addr) &&
(ast_sockaddr_is_ipv4(addr) || ast_sockaddr_is_any(addr))) {
struct sockaddr_in answer;
int rsp;
@@ -3112,41 +3143,44 @@
? rtp->rtcp->s : rtp->s, &stunaddr, NULL, &answer);
ao2_lock(instance);
if (!rsp) {
- pj_sockaddr base;
+ struct ast_rtp_engine_ice_candidate *candidate;
+ pj_sockaddr ext, base;
+ pj_str_t mapped = pj_str(ast_strdupa(ast_inet_ntoa(answer.sin_addr)));
+ int srflx = 1, baseset;
+ struct ao2_iterator i;
- /* Use the first local IPv4 host candidate as the base */
- for (pos = basepos; pos < count; pos++) {
- if (address[pos].addr.sa_family == PJ_AF_INET &&
- !rtp_address_is_ice_blacklisted(&address[pos])) {
- pj_sockaddr_cp(&base, &address[pos]);
- break;
- }
+ pj_sockaddr_init(pj_AF_INET(), &ext, &mapped, ntohs(answer.sin_port));
+
+ if (ast_sockaddr_is_any(addr)) {
+ baseset = 0;
+ } else {
+ baseset = 1;
+ ast_sockaddr_to_pj_sockaddr(addr, &base);
}
-
- if (pos < count) {
- pj_sockaddr ext;
- pj_str_t mapped = pj_str(ast_strdupa(ast_inet_ntoa(answer.sin_addr)));
- int srflx = 1;
-
- pj_sockaddr_init(pj_AF_INET(), &ext, &mapped, ntohs(answer.sin_port));
-
- /*
- * If the returned address is the same as one of our host
- * candidates, don't send the srflx
- */
- for (pos = 0; pos < count; pos++) {
- if (pj_sockaddr_cmp(&address[pos], &ext) == 0 &&
- !rtp_address_is_ice_blacklisted(&address[pos])) {
- srflx = 0;
- break;
- }
+ /*
+ * If the returned address is the same as one of our host
+ * candidates, don't send the srflx
+ */
+ i = ao2_iterator_init(rtp->ice_local_candidates, 0);
+ while (srflx && (candidate = ao2_iterator_next(&i))) {
+ if (!baseset && ast_sockaddr_is_ipv4(&candidate->address) && !ast_sockaddr_is_any(&candidate->address)) {
+ baseset = 1;
+ ast_sockaddr_to_pj_sockaddr(&candidate->address, &base);
}
- if (srflx) {
- ast_rtp_ice_add_cand(instance, rtp, component, transport,
- PJ_ICE_CAND_TYPE_SRFLX, 65535, &ext, &base, &base,
- pj_sockaddr_get_len(&ext));
+ if (pj_sockaddr_cmp(&candidate->address, &ext) == 0) {
+ srflx = 0;
}
+
+ ao2_ref(candidate, -1);
+ }
+ ao2_iterator_destroy(&i);
+
+ if (srflx) {
+ ast_sockaddr_to_pj_sockaddr(addr, &base);
+ ast_rtp_ice_add_cand(instance, rtp, component, transport,
+ PJ_ICE_CAND_TYPE_SRFLX, 65535, &ext, &base, &base,
+ pj_sockaddr_get_len(&ext));
}
}
}
@@ -6799,7 +6833,6 @@
AST_RWLIST_WRLOCK(&host_candidates);
for (var = ast_variable_browse(cfg, "ice_host_candidates"); var; var = var->next) {
struct ast_sockaddr local_addr, advertised_addr;
- pj_str_t address;
unsigned int include_local_address = 0;
char *sep;
@@ -6831,8 +6864,8 @@
candidate->include_local = include_local_address;
- pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&address, ast_sockaddr_stringify(&local_addr)), &candidate->local);
- pj_sockaddr_parse(pj_AF_UNSPEC(), 0, pj_cstr(&address, ast_sockaddr_stringify(&advertised_addr)), &candidate->advertised);
+ ast_sockaddr_copy(&candidate->local, &local_addr);
+ ast_sockaddr_copy(&candidate->advertised, &advertised_addr);
AST_RWLIST_INSERT_TAIL(&host_candidates, candidate, next);
}
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13362
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Change-Id: I109eaffc3e2b432f00bf958e3caa0f38cacb4edb
Gerrit-Change-Number: 13362
Gerrit-PatchSet: 1
Gerrit-Owner: Jaco Kroon <jaco at uls.co.za>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191203/32139a75/attachment-0001.html>
More information about the asterisk-code-review
mailing list