[libnice] Fwd: Controlled agent will be not ready when it is behind a NAT in our latest master branch

[hui yu]

Hi,



When the controlled agent is in a private network behind a NAT and the
controlling agent is in a public network:

I try to use the threaded-example.c to do a test, but I got a failed result:

   Only the controlling agent got a ready state and the controlled agent could
not get ready state.



1.      Gathering candidates:

Controlled agent gathered: left_local_address and left_server_reflex_address

Controlling agent gathered: right_local_address

2.      Connectivity checks and Negotiation:

1)     Controlling agent

11) make a nominated pair: [right_local_address,
left_server_reflex_address];

22) send a stun request;

33) get a stun successful response;

44) Negotiation complete;

2)     Controlled agent:

11) Receive a stun request from controlling agent;

22) Send a stun response to controlling agent;

33) According to a stun request from controlling agent and get a nominated
pair: [left_local_address, right_local_address];

44) Send a stun request with this nominated pair to controlling agent.

55) Receive a stun response from controlling agent and get a mapped address
and make a new discovery pair [left_server_reflex_address,
right_local_address];

66) The new discover pair is a valid pair, but it cannot become a nominated
pair in current master code.

77) Negotiation failed.



I look up the source code and find maybe there is two mistakes in the code,
I think if the nominated value in the new discovery pair can inherit from
the parent pairt.



The fix code is in red with yellow background.



/////////////////////////////////////

static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent,
guint stream_id, NiceComponent *component, NiceCandidate *local_cand,
CandidateCheckPair *parent_pair)

{

  CandidateCheckPair *pair = g_slice_new0 (CandidateCheckPair);

  NiceStream *stream = agent_find_stream (agent, stream_id);



  pair->agent = agent;

  pair->stream_id = stream_id;

  pair->component_id = component->id;;

  pair->local = local_cand;

  pair->remote = parent_pair->remote;

  pair->sockptr = local_cand->sockptr;

  pair->state = NICE_CHECK_DISCOVERED;

  nice_debug ("Agent %p : new pair %p state DISCOVERED", agent, pair);

  {

      gchar tmpbuf1[INET6_ADDRSTRLEN];

      gchar tmpbuf2[INET6_ADDRSTRLEN];

      nice_address_to_string (&pair->local->addr, tmpbuf1);

      nice_address_to_string (&pair->remote->addr, tmpbuf2);

      nice_debug ("Agent %p : new pair %p : [%s]:%u --> [%s]:%u", agent,
pair,

          tmpbuf1, nice_address_get_port (&pair->local->addr),

          tmpbuf2, nice_address_get_port (&pair->remote->addr));

  }

  g_snprintf (pair->foundation, NICE_CANDIDATE_PAIR_MAX_FOUNDATION, "%s:%s",

      local_cand->foundation, parent_pair->remote->foundation);

  if (agent->controlling_mode == TRUE)

    pair->priority = nice_candidate_pair_priority (pair->local->priority,

        pair->remote->priority);

  else

    pair->priority = nice_candidate_pair_priority (pair->remote->priority,

        pair->local->priority);

  pair->nominated = FALSE;  -------------à pair->nominated = parent_pai->
nominated;

  pair->prflx_priority = ensure_unique_priority (component,

      peer_reflexive_candidate_priority (agent, local_cand));

  nice_debug ("Agent %p : added a new peer-discovered pair with foundation
of '%s'.",  agent, pair->foundation);



  stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list,
pair,

      (GCompareFunc)conn_check_compare);



  return pair;

}





static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent
*agent, NiceStream *stream, NiceComponent *component, CandidateCheckPair
*p, NiceSocket *sockptr, struct sockaddr *mapped_sockaddr, NiceCandidate
*local_candidate, NiceCandidate *remote_candidate)

{

  CandidateCheckPair *new_pair = NULL;

  NiceAddress mapped;

  GSList *i, *j;

  NiceCandidate *local_cand = NULL;



  nice_address_set_from_sockaddr (&mapped, mapped_sockaddr);



  for (j = component->local_candidates; j; j = j->next) {

    NiceCandidate *cand = j->data;

    if (nice_address_equal (&mapped, &cand->addr)) {

      local_cand = cand;



      /* We always need to select the peer-reflexive Candidate Pair in the
case

       * of a TCP-ACTIVE local candidate, so we find it even if an incoming

       * check matched an existing pair because it could be the original

       * ACTIVE-PASSIVE candidate pair which was retriggered */

      for (i = stream->conncheck_list; i; i = i->next) {

        CandidateCheckPair *pair = i->data;

        if (pair->local == cand && remote_candidate == pair->remote) {

          new_pair = pair;

          break;

        }

      }

      break;

    }

  }



  if (new_pair) {

p->state = NICE_CHECK_SUCCEEDED;

new_pair-> nominated  = p-> nominated;

    nice_debug ("Agent %p : conncheck %p SUCCEEDED.", agent, p);

    priv_conn_check_unfreeze_related (agent, stream, p);

    nice_component_add_valid_candidate (component, remote_candidate);

  }

  else {

    if (!local_cand) {

      if (!agent->force_relay)

        local_cand = discovery_add_peer_reflexive_candidate (agent,

                                                             stream->id,

                                                             component->id,

                                                            &mapped,

                                                             sockptr,


local_candidate,

                                                             remote_
candidate);

      p->state = NICE_CHECK_FAILED;

      nice_debug ("Agent %p : pair %p state FAILED", agent, p);

    }



    /* step: add a new discovered pair (see RFC 5245 7.1.3.2.2

                      "Constructing a Valid Pair") */

    if (local_cand)

      new_pair = priv_add_peer_reflexive_pair (agent, stream->id, component,

          local_cand, p);

    nice_debug ("Agent %p : conncheck %p FAILED, %p DISCOVERED.", agent, p,
new_pair);

  }



  /* note: this is same as "adding to VALID LIST" in the spec

     text */

  if (new_pair) {

    new_pair->valid = TRUE;

    nice_component_add_valid_candidate (component, remote_candidate);

  }



  return new_pair;

}



/////////////////////////////////////





Hui
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/nice/attachments/20170425/deb9697a/attachment-0001.html>