From 59ce41dfb837adf4222b25490cde2e394384ad15 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Miguel=20Par=C3=ADs=20D=C3=ADaz?= Date: Fri, 31 Mar 2017 20:20:38 -0400 Subject: [PATCH] conncheck: consider answer received when remote credentials are set MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Consider that the answer is received when remote credentials are set instead of when a remote candidate is set, which could not happen or could cause more delay for the connection establishment. Ported to git master by Olivier Crête Differential Revision: https://phabricator.freedesktop.org/D1704 --- agent/agent.c | 4 +- agent/conncheck.c | 225 +++++++++++++++++++++++++++--------------------------- agent/conncheck.h | 2 +- 3 files changed, 117 insertions(+), 114 deletions(-) diff --git a/agent/agent.c b/agent/agent.c index 555fd16..4d9381c 100644 --- a/agent/agent.c +++ b/agent/agent.c @@ -3240,6 +3240,8 @@ nice_agent_set_remote_credentials ( g_strlcpy (stream->remote_ufrag, ufrag, NICE_STREAM_MAX_UFRAG); g_strlcpy (stream->remote_password, pwd, NICE_STREAM_MAX_PWD); + conn_check_remote_credentials_set(agent, stream); + ret = TRUE; goto done; } @@ -3342,8 +3344,6 @@ _set_remote_candidates_locked (NiceAgent *agent, NiceStream *stream, } } - conn_check_remote_candidates_set(agent, stream, component); - if (added > 0) { conn_check_schedule_next (agent); } diff --git a/agent/conncheck.c b/agent/conncheck.c index dda2f2f..2abbc5e 100644 --- a/agent/conncheck.c +++ b/agent/conncheck.c @@ -1243,124 +1243,124 @@ static GSList *prune_cancelled_conn_check (GSList *conncheck_list) /* * Handle any processing steps for connectivity checks after - * remote candidates have been set. This function handles + * remote credentials have been set. This function handles * the special case where answerer has sent us connectivity - * checks before the answer (containing candidate information), + * checks before the answer (containing credentials information), * reaches us. The special case is documented in sect 7.2 * if ICE spec (ID-19). */ -void conn_check_remote_candidates_set(NiceAgent *agent, NiceStream *stream, NiceComponent *component) +void conn_check_remote_credentials_set(NiceAgent *agent, NiceStream *stream) { - GSList *j, *k, *l, *m, *n; + GSList *j, *k, *l, *m, *n, *o; for (j = stream->conncheck_list; j ; j = j->next) { CandidateCheckPair *pair = j->data; - if (pair->component_id == component->id) { - gboolean match = FALSE; - - /* performn delayed processing of spec steps section 7.2.1.4, - and section 7.2.1.5 */ - priv_preprocess_conn_check_pending_data (agent, stream, component, pair); - - for (k = component->incoming_checks; k; k = k->next) { - IncomingCheck *icheck = k->data; - /* sect 7.2.1.3., "Learning Peer Reflexive Candidates", has to - * be handled separately */ - for (l = component->remote_candidates; l; l = l->next) { - NiceCandidate *cand = l->data; - if (nice_address_equal (&icheck->from, &cand->addr)) { - match = TRUE; - break; - } - } - if (match != TRUE) { - /* note: we have gotten an incoming connectivity check from - * an address that is not a known remote candidate */ - - NiceCandidate *local_candidate = NULL; - NiceCandidate *remote_candidate = NULL; - - if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE || - agent->compatibility == NICE_COMPATIBILITY_MSN || - agent->compatibility == NICE_COMPATIBILITY_OC2007) { - /* We need to find which local candidate was used */ - uint8_t uname[NICE_STREAM_MAX_UNAME]; - guint uname_len; - - nice_debug ("Agent %p: We have a peer-reflexive candidate in a " - "stored pending check", agent); - - for (m = component->remote_candidates; - m != NULL && remote_candidate == NULL; m = m->next) { - for (n = component->local_candidates; n; n = n->next) { - NiceCandidate *rcand = m->data; - NiceCandidate *lcand = n->data; - - uname_len = priv_create_username (agent, stream, - component->id, rcand, lcand, - uname, sizeof (uname), TRUE); - - stun_debug ("pending check, comparing usernames of len %d and %d, equal=%d", - icheck->username_len, uname_len, - icheck->username && uname_len == icheck->username_len && - memcmp (uname, icheck->username, icheck->username_len) == 0); - stun_debug_bytes (" first username: ", - icheck->username, - icheck->username? icheck->username_len : 0); - stun_debug_bytes (" second username: ", uname, uname_len); - - if (icheck->username && - uname_len == icheck->username_len && - memcmp (uname, icheck->username, icheck->username_len) == 0) { - local_candidate = lcand; - remote_candidate = rcand; - break; - } - } - } - } else { - for (l = component->local_candidates; l; l = l->next) { - NiceCandidate *cand = l->data; - if (nice_address_equal (&cand->addr, &icheck->local_socket->addr)) { - local_candidate = cand; + NiceComponent *component = + nice_stream_find_component_by_id (stream, pair->component_id); + gboolean match = FALSE; + + /* performn delayed processing of spec steps section 7.2.1.4, + and section 7.2.1.5 */ + priv_preprocess_conn_check_pending_data (agent, stream, component, pair); + + for (k = component->incoming_checks; k; k = k->next) { + IncomingCheck *icheck = k->data; + /* sect 7.2.1.3., "Learning Peer Reflexive Candidates", has to + * be handled separately */ + for (l = component->remote_candidates; l; l = l->next) { + NiceCandidate *cand = l->data; + if (nice_address_equal (&icheck->from, &cand->addr)) { + match = TRUE; + break; + } + } + if (match != TRUE) { + /* note: we have gotten an incoming connectivity check from + * an address that is not a known remote candidate */ + + NiceCandidate *local_candidate = NULL; + NiceCandidate *remote_candidate = NULL; + + if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE || + agent->compatibility == NICE_COMPATIBILITY_MSN || + agent->compatibility == NICE_COMPATIBILITY_OC2007) { + /* We need to find which local candidate was used */ + uint8_t uname[NICE_STREAM_MAX_UNAME]; + guint uname_len; + + nice_debug ("Agent %p: We have a peer-reflexive candidate in a " + "stored pending check", agent); + + for (m = component->remote_candidates; + m != NULL && remote_candidate == NULL; m = m->next) { + for (n = component->local_candidates; n; n = n->next) { + NiceCandidate *rcand = m->data; + NiceCandidate *lcand = n->data; + + uname_len = priv_create_username (agent, stream, + component->id, rcand, lcand, + uname, sizeof (uname), TRUE); + + stun_debug ("pending check, comparing usernames of len %d and %d, equal=%d", + icheck->username_len, uname_len, + icheck->username && uname_len == icheck->username_len && + memcmp (uname, icheck->username, icheck->username_len) == 0); + stun_debug_bytes (" first username: ", + icheck->username, + icheck->username? icheck->username_len : 0); + stun_debug_bytes (" second username: ", uname, uname_len); + + if (icheck->username && + uname_len == icheck->username_len && + memcmp (uname, icheck->username, icheck->username_len) == 0) { + local_candidate = lcand; + remote_candidate = rcand; break; } } } - - if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE && - local_candidate == NULL) { - /* if we couldn't match the username, then the matching remote - * candidate hasn't been received yet.. we must wait */ - nice_debug ("Agent %p : Username check failed. pending check has " - "to wait to be processed", agent); - } else { - NiceCandidate *candidate; - - nice_debug ("Agent %p : Discovered peer reflexive from early i-check", - agent); - candidate = - discovery_learn_remote_peer_reflexive_candidate (agent, - stream, - component, - icheck->priority, - &icheck->from, - icheck->local_socket, - local_candidate, remote_candidate); - if (candidate) { - if (local_candidate && - local_candidate->transport == NICE_CANDIDATE_TRANSPORT_TCP_PASSIVE) - priv_conn_check_add_for_candidate_pair_matched (agent, - stream->id, component, local_candidate, candidate, NICE_CHECK_DISCOVERED); - else - conn_check_add_for_candidate (agent, stream->id, component, candidate); - - if (icheck->use_candidate) - priv_mark_pair_nominated (agent, stream, component, local_candidate, candidate); - priv_schedule_triggered_check (agent, stream, component, icheck->local_socket, candidate, icheck->use_candidate); + } else { + for (l = component->local_candidates; l; l = l->next) { + NiceCandidate *cand = l->data; + if (nice_address_equal (&cand->addr, &icheck->local_socket->addr)) { + local_candidate = cand; + break; } } } + + if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE && + local_candidate == NULL) { + /* if we couldn't match the username, then the matching remote + * candidate hasn't been received yet.. we must wait */ + nice_debug ("Agent %p : Username check failed. pending check has " + "to wait to be processed", agent); + } else { + NiceCandidate *candidate; + + nice_debug ("Agent %p : Discovered peer reflexive from early i-check", + agent); + candidate = + discovery_learn_remote_peer_reflexive_candidate (agent, + stream, + component, + icheck->priority, + &icheck->from, + icheck->local_socket, + local_candidate, remote_candidate); + if (candidate) { + if (local_candidate && + local_candidate->transport == NICE_CANDIDATE_TRANSPORT_TCP_PASSIVE) + priv_conn_check_add_for_candidate_pair_matched (agent, + stream->id, component, local_candidate, candidate, NICE_CHECK_DISCOVERED); + else + conn_check_add_for_candidate (agent, stream->id, component, candidate); + + if (icheck->use_candidate) + priv_mark_pair_nominated (agent, stream, component, local_candidate, candidate); + priv_schedule_triggered_check (agent, stream, component, icheck->local_socket, candidate, icheck->use_candidate); + } + } } } } @@ -1368,9 +1368,12 @@ void conn_check_remote_candidates_set(NiceAgent *agent, NiceStream *stream, Nice /* Once we process the pending checks, we should free them to avoid * reprocessing them again if a dribble-mode set_remote_candidates * is called */ - g_slist_free_full (component->incoming_checks, - (GDestroyNotify) incoming_check_free); - component->incoming_checks = NULL; + for (o = stream->components; o; o = o->next) { + NiceComponent *component = o->data; + g_slist_free_full (component->incoming_checks, + (GDestroyNotify) incoming_check_free); + component->incoming_checks = NULL; + } stream->conncheck_list = prune_cancelled_conn_check (stream->conncheck_list); @@ -3628,14 +3631,14 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, if (stream->initial_binding_request_received != TRUE) agent_signal_initial_binding_request_received (agent, stream); - if (component->remote_candidates && remote_candidate == NULL) { + if (remote_candidate == NULL) { nice_debug ("Agent %p : No matching remote candidate for incoming check ->" "peer-reflexive candidate.", agent); remote_candidate = discovery_learn_remote_peer_reflexive_candidate ( agent, stream, component, priority, from, nicesock, local_candidate, remote_candidate2 ? remote_candidate2 : remote_candidate); - if(remote_candidate) { + if(remote_candidate && stream->remote_ufrag != NULL) { if (local_candidate && local_candidate->transport == NICE_CANDIDATE_TRANSPORT_TCP_PASSIVE) { CandidateCheckPair *pair; @@ -3654,10 +3657,10 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, priv_reply_to_conn_check (agent, stream, component, local_candidate, remote_candidate, from, nicesock, rbuf_len, &msg, use_candidate); - if (component->remote_candidates == NULL) { + if (stream->remote_ufrag == NULL) { /* case: We've got a valid binding request to a local candidate - * but we do not yet know remote credentials nor - * candidates. As per sect 7.2 of ICE (ID-19), we send a reply + * but we do not yet know remote credentials. + * As per sect 7.2 of ICE (ID-19), we send a reply * immediately but postpone all other processing until * we get information about the remote candidates */ diff --git a/agent/conncheck.h b/agent/conncheck.h index 431c606..10319cc 100644 --- a/agent/conncheck.h +++ b/agent/conncheck.h @@ -105,7 +105,7 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair); void conn_check_prune_stream (NiceAgent *agent, NiceStream *stream); gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, NiceComponent *component, NiceSocket *udp_socket, const NiceAddress *from, gchar *buf, guint len); gint conn_check_compare (const CandidateCheckPair *a, const CandidateCheckPair *b); -void conn_check_remote_candidates_set(NiceAgent *agent, NiceStream *stream, NiceComponent *component); +void conn_check_remote_credentials_set(NiceAgent *agent, NiceStream *stream); NiceCandidateTransport conn_check_match_transport (NiceCandidateTransport transport); void conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *component, -- 2.7.4