Error 488 Multiple same ICE candidates

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi all,

I would like to report a bug I am encountering and to contribute a patch
fixing it.

* Bug description:  When answering a call on iOS, it gives error 488
(Not Acceptable Here). Reason: too many ICE candidates.

* Bug reproducibility: Low/medium

* Environment:
 - The latest stable PJSIP version, 2.4.5.
 - iOS 9.1
 - PJSIP configured to use STUN, ICE, TURN.
 - #define PJ_IPHONE_OS_HAS_MULTITASKING_SUPPORT 1
 - ICE max candidates set to 16, SDP ICE max candidates set to 8

* Detailed description:
When making a voice call, ICE submodule builds list of a candidates. If
host candidates are allowed, local host addresses are determined. In
some cases we are experiencing a situation where there are multiple ICE
host candidates with exactly the same IP address for the same ICE
component. It is visible ICE is adding the same candidate to the
candidate list from the logs. We had 3 duplicate occurrences of the same
IP and 2 duplicates of another IP address.

This happens when iPhone is connected to multiple networks (4G internet,
wifi, VPN).
I don't know why resolving local IP addresses returns duplicates (maybe
due to aliasing?). But I append a patch (bottom of the mail) that checks
for ICE candidate duplicity in the list, not adding a duplicate entry to
candidate list (increasing MAX ICE candidate count is not a solution).
Algorithm time complexity is not optimal (checks all previous entries on
adding a new one), but very easy to implement and to understand.

The patch is against rev5206. There could be minor modifications needed
to apply the patch as we apply several patches on top of official
repository. But I guess idea is pretty clear.

Cheers.

Dusan Klinec (ph4r05)

diff --git a/pjnath/src/pjnath/ice_strans.c b/pjnath/src/pjnath/ice_strans.c
index c251c7b..5fbbaf1 100644
--- a/pjnath/src/pjnath/ice_strans.c
+++ b/pjnath/src/pjnath/ice_strans.c
@@ -347,6 +347,34 @@ static pj_status_t add_update_turn(pj_ice_strans
*ice_st,
     return PJ_SUCCESS;
 }

+static pj_bool_t pj_ice_cand_equals(pj_ice_sess_cand *lcand,
pj_ice_sess_cand *rcand){
+    if (lcand == NULL && rcand == NULL){
+        return PJ_TRUE;
+    }
+    if (lcand == NULL || rcand == NULL){
+        return PJ_FALSE;
+    }
+
+    if (lcand->type != rcand->type
+        || lcand->status != rcand->status
+        || lcand->comp_id != rcand->comp_id
+        || lcand->transport_id != rcand->transport_id
+        || lcand->local_pref != rcand->local_pref
+        || lcand->prio != rcand->prio
+        || pj_sockaddr_cmp(&lcand->addr, &rcand->addr) != 0
+        || pj_sockaddr_cmp(&lcand->base_addr, &rcand->base_addr) != 0)
+    {
+        return PJ_FALSE;
+    }
+
+
+    if (lcand->foundation.slen != rcand->foundation.slen
+        || strncmp(lcand->foundation.ptr, rcand->foundation.ptr,
(size_t)lcand->foundation.slen) != 0){
+        return PJ_FALSE;
+    }
+
+    return PJ_TRUE;
+}

 /*
  * Create the component.
@@ -471,7 +499,8 @@ static pj_status_t create_comp(pj_ice_strans
*ice_st, unsigned comp_id)
      */
     if (ice_st->cfg.stun.max_host_cands &&
!pj_ice_is_cand_blocked(&ice_st->cfg.opt, PJ_ICE_CAND_TYPE_HOST)) {
         pj_stun_sock_info stun_sock_info;
-        unsigned i;
+        unsigned i,j;
+        pj_bool_t cand_duplicate = PJ_FALSE;

         /* Enumerate addresses */
         status = pj_stun_sock_get_info(comp->stun_sock, &stun_sock_info);
@@ -498,7 +527,7 @@ static pj_status_t create_comp(pj_ice_strans
*ice_st, unsigned comp_id)
             continue;
         }

-        cand = &comp->cand_list[comp->cand_cnt++];
+        cand = &comp->cand_list[comp->cand_cnt];

         cand->type = PJ_ICE_CAND_TYPE_HOST;
         cand->status = PJ_SUCCESS;
@@ -511,6 +540,27 @@ static pj_status_t create_comp(pj_ice_strans
*ice_st, unsigned comp_id)
         pj_ice_calc_foundation(ice_st->pool, &cand->foundation,
                        cand->type, &cand->base_addr);

+        /* Check if not already in list */
+        for (j=0; j<comp->cand_cnt; j++){
+            if (pj_ice_cand_equals(cand, &comp->cand_list[j])){
+                cand_duplicate = PJ_TRUE;
+                break;
+            }
+        }
+
+        if (cand_duplicate){
+            PJ_LOG(4,(ice_st->obj_name,
+                    "Comp %d: host candidate %s is a duplicate",
+                    comp_id, pj_sockaddr_print(&cand->addr, addrinfo,
+                                               sizeof(addrinfo), 3)));
+
+            pj_bzero(&cand->addr, sizeof(cand->addr));
+            pj_bzero(&cand->base_addr, sizeof(cand->base_addr));
+            continue;
+        } else {
+            comp->cand_cnt+=1;
+        }
+
         PJ_LOG(4,(ice_st->obj_name,
               "Comp %d: host candidate %s added",
               comp_id, pj_sockaddr_print(&cand->addr, addrinfo,


============================================================
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.pjsip.org/pipermail/pjsip_lists.pjsip.org/attachments/20151210/f26466b2/attachment.html>


[Index of Archives]     [Asterisk Users]     [Asterisk App Development]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [Linux API]
  Powered by Linux