From 9cda08004416b0d369badbae81066eeb25e0039c Mon Sep 17 00:00:00 2001 From: Kamil Dudka Date: Mon, 7 May 2018 15:57:12 +0200 Subject: [PATCH] Resolves: #1541646 - update to 0.1.14-85-g34d6044 --- ....patch => libnice-0.1.14-85-g34d6044.patch | 1432 +++++++++++++++++ libnice-0.1.14-tests-koji.patch | 2 +- libnice.spec | 11 +- 3 files changed, 1442 insertions(+), 3 deletions(-) rename libnice-0.1.14-70-gfb2f1f7.patch => libnice-0.1.14-85-g34d6044.patch (87%) diff --git a/libnice-0.1.14-70-gfb2f1f7.patch b/libnice-0.1.14-85-g34d6044.patch similarity index 87% rename from libnice-0.1.14-70-gfb2f1f7.patch rename to libnice-0.1.14-85-g34d6044.patch index db148cb..541d010 100644 --- a/libnice-0.1.14-70-gfb2f1f7.patch +++ b/libnice-0.1.14-85-g34d6044.patch @@ -10529,3 +10529,1435 @@ index 5b08311..c8a4edf 100644 -- 2.13.6 +From db6166ee247a8f9fa4ebe2a08d223de73cbd2e96 Mon Sep 17 00:00:00 2001 +From: Jozsef Vass +Date: Mon, 8 Jan 2018 10:53:23 -0800 +Subject: [PATCH 01/15] agent: Fixes incompatible pointer type warning on OSX. + +The variable tie is actually never read. +--- + agent/conncheck.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index c8a4edf..38a90cd 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -3421,7 +3421,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + states" and 8.1.2 "Updating States", ID-19) */ + priv_update_check_list_state_for_ready (agent, stream, component); + } else if (res == STUN_USAGE_ICE_RETURN_ROLE_CONFLICT) { +- guint64 tie; ++ uint64_t tie; + gboolean controlled_mode; + + /* case: role conflict error, need to restart with new role */ +-- +2.14.3 + + +From ae3e5acc775ee6c1701ff9a2404b14e4d5dd6c20 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Sun, 26 Nov 2017 17:13:19 +0100 +Subject: [PATCH 02/15] conncheck: rework early stun requests handling +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +With this patch we simplify the code used to handle the incoming stun +request when remote candidates or remote credentials have not been +received yet. + +When the remote credentials is unknown, the stun request is stored +in a list of incoming_checks for later processing, and no further +processing is done, except responding to the request. + +When the remote credentials are received, the triggered checks for these +incoming checks can now be queued, and the related pairs are created. + +If the remote candidates have not been received when the stun request +on a valid local port arrives, a peer-reflexive remote candidate will be +created. This candidate may need to be updated later when remote +candidates are finally received, including candidate priority and +foundation, and also related pairs. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1889 +--- + agent/agent.c | 42 ++++++++++++-- + agent/conncheck.c | 168 ++++++++++-------------------------------------------- + agent/conncheck.h | 1 + + 3 files changed, 66 insertions(+), 145 deletions(-) + +diff --git a/agent/agent.c b/agent/agent.c +index 0773c53..49fc371 100644 +--- a/agent/agent.c ++++ b/agent/agent.c +@@ -3300,6 +3300,28 @@ nice_agent_add_local_address (NiceAgent *agent, NiceAddress *addr) + return TRUE; + } + ++/* Recompute foundations of all candidate pairs from a given stream ++ * having a specific remote candidate ++ */ ++static void priv_update_pair_foundations (NiceAgent *agent, ++ guint stream_id, NiceCandidate *remote) ++{ ++ NiceStream *stream = agent_find_stream (agent, stream_id); ++ if (stream) { ++ GSList *i; ++ for (i = stream->conncheck_list; i; i = i->next) { ++ CandidateCheckPair *pair = i->data; ++ if (pair->remote == remote) { ++ g_snprintf (pair->foundation, ++ NICE_CANDIDATE_PAIR_MAX_FOUNDATION, "%s:%s", ++ pair->local->foundation, pair->remote->foundation); ++ nice_debug ("Agent %p : Updating pair %p foundation to '%s'", ++ agent, pair, pair->foundation); ++ } ++ } ++ } ++} ++ + static gboolean priv_add_remote_candidate ( + NiceAgent *agent, + guint stream_id, +@@ -3331,8 +3353,7 @@ static gboolean priv_add_remote_candidate ( + + /* If it was a discovered remote peer reflexive candidate, then it should + * be updated according to RFC 5245 section 7.2.1.3 */ +- if (candidate && candidate->type == NICE_CANDIDATE_TYPE_PEER_REFLEXIVE && +- candidate->priority == priority) { ++ if (candidate && candidate->type == NICE_CANDIDATE_TYPE_PEER_REFLEXIVE) { + nice_debug ("Agent %p : Updating existing peer-rfx remote candidate to %s", + agent, _cand_type_to_sdp (type)); + candidate->type = type; +@@ -3375,6 +3396,13 @@ static gboolean priv_add_remote_candidate ( + g_free (candidate->password); + candidate->password = g_strdup (password); + } ++ ++ /* since the type of the existing candidate may have changed, ++ * the pairs priority and foundation related to this candidate need ++ * to be recomputed. ++ */ ++ recalculate_pair_priorities (agent); ++ priv_update_pair_foundations (agent, stream_id, candidate); + } + else { + /* case 2: add a new candidate */ +@@ -3429,12 +3457,14 @@ static gboolean priv_add_remote_candidate ( + if (foundation) + g_strlcpy (candidate->foundation, foundation, + NICE_CANDIDATE_MAX_FOUNDATION); +- } + +- if (conn_check_add_for_candidate (agent, stream_id, component, candidate) < 0) { +- goto errors; ++ /* We only create a pair when a candidate is new, and not when ++ * updating an existing one. ++ */ ++ if (conn_check_add_for_candidate (agent, stream_id, ++ component, candidate) < 0) ++ goto errors; + } +- + return TRUE; + + errors: +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 38a90cd..11ef9c9 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -1690,34 +1690,6 @@ gint conn_check_compare (const CandidateCheckPair *a, const CandidateCheckPair * + return 0; + } + +-/* +- * Preprocesses a new connectivity check by going through list +- * of a any stored early incoming connectivity checks from +- * the remote peer. If a matching incoming check has been already +- * received, update the state of the new outgoing check 'pair'. +- * +- * @param agent context pointer +- * @param stream which stream (of the agent) +- * @param component pointer to component object to which 'pair'has been added +- * @param pair newly added connectivity check +- */ +-static void priv_preprocess_conn_check_pending_data (NiceAgent *agent, NiceStream *stream, NiceComponent *component, CandidateCheckPair *pair) +-{ +- GSList *i; +- for (i = component->incoming_checks; i; i = i->next) { +- IncomingCheck *icheck = i->data; +- if (nice_address_equal (&icheck->from, &pair->remote->addr) && +- icheck->local_socket == pair->sockptr) { +- nice_debug ("Agent %p : Updating check %p with stored early-icheck %p, %p/%u/%u (agent/stream/component).", agent, pair, icheck, agent, stream->id, component->id); +- priv_schedule_triggered_check (agent, stream, component, +- icheck->local_socket, pair->remote); +- if (icheck->use_candidate) +- priv_mark_pair_nominated (agent, stream, component, pair->local, pair->remote); +- } +- } +-} +- +- + /* + * Handle any processing steps for connectivity checks after + * remote credentials have been set. This function handles +@@ -1728,126 +1700,39 @@ static void priv_preprocess_conn_check_pending_data (NiceAgent *agent, NiceStrea + */ + void conn_check_remote_credentials_set(NiceAgent *agent, NiceStream *stream) + { +- GSList *j, *k, *l, *m, *n, *o; ++ GSList *j, *k, *l, *m; + +- for (j = stream->conncheck_list; j ; j = j->next) { +- CandidateCheckPair *pair = j->data; +- 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 (j = stream->components; j ; j = j->next) { ++ NiceComponent *component = j->data; + + 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; ++ NiceCandidate *rcand = l->data; ++ NiceCandidate *lcand = NULL; ++ if (nice_address_equal (&rcand->addr, &icheck->from)) { ++ for (m = component->local_candidates; m; m = m->next) { ++ NiceCandidate *cand = m->data; + if (nice_address_equal (&cand->addr, &icheck->local_socket->addr)) { +- local_candidate = cand; ++ lcand = 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); +- +- priv_schedule_triggered_check (agent, stream, component, +- icheck->local_socket, candidate); +- if (icheck->use_candidate) +- priv_mark_pair_nominated (agent, stream, component, local_candidate, candidate); +- } ++ g_assert (lcand != NULL); ++ priv_schedule_triggered_check (agent, stream, component, ++ icheck->local_socket, rcand); ++ if (icheck->use_candidate) ++ priv_mark_pair_nominated (agent, stream, component, ++ lcand, rcand); ++ break; + } + } + } +- } +- +- /* Once we process the pending checks, we should free them to avoid +- * reprocessing them again if a dribble-mode set_remote_candidates +- * is called */ +- for (o = stream->components; o; o = o->next) { +- NiceComponent *component = o->data; ++ /* 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; +@@ -2964,8 +2849,8 @@ static gboolean priv_schedule_triggered_check (NiceAgent *agent, NiceStream *str + + if (i) { + nice_debug ("Agent %p : Adding a triggered check to conn.check list (local=%p).", agent, local); +- p = priv_add_new_check_pair (agent, stream->id, component, +- local, remote_cand, NICE_CHECK_WAITING); ++ p = priv_conn_check_add_for_candidate_pair_matched (agent, stream->id, ++ component, local, remote_cand, NICE_CHECK_WAITING); + priv_add_pair_to_triggered_check_queue (agent, p); + return TRUE; + } +@@ -3018,7 +2903,12 @@ static void priv_reply_to_conn_check (NiceAgent *agent, NiceStream *stream, + ms_ice2_legacy_conncheck_send(msg, sockptr, toaddr); + } + +- if (rcand) { ++ /* We react to this stun request when we have the remote credentials. ++ * When credentials are not yet known, this request is stored ++ * in incoming_checks for later processing when returning from this ++ * function. ++ */ ++ if (rcand && stream->remote_ufrag[0]) { + priv_schedule_triggered_check (agent, stream, component, sockptr, rcand); + if (use_candidate) + priv_mark_pair_nominated (agent, stream, component, lcand, rcand); +@@ -3114,7 +3004,7 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint + * Recalculates priorities of all candidate pairs. This + * is required after a conflict in ICE roles. + */ +-static void priv_recalculate_pair_priorities (NiceAgent *agent) ++void recalculate_pair_priorities (NiceAgent *agent) + { + GSList *i, *j; + +@@ -3143,7 +3033,7 @@ static void priv_check_for_role_conflict (NiceAgent *agent, gboolean control) + agent->controlling_mode = control; + /* the pair priorities depend on the roles, so recalculation + * is needed */ +- priv_recalculate_pair_priorities (agent); ++ recalculate_pair_priorities (agent); + } + else + nice_debug ("Agent %p : Role conflict, staying with role \"%s\".", +diff --git a/agent/conncheck.h b/agent/conncheck.h +index e16dc67..8cfe2d6 100644 +--- a/agent/conncheck.h ++++ b/agent/conncheck.h +@@ -119,5 +119,6 @@ conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *co + NiceSocket *sock); + + guint32 ensure_unique_priority (NiceComponent *component, guint32 priority); ++void recalculate_pair_priorities (NiceAgent *agent); + + #endif /*_NICE_CONNCHECK_H */ +-- +2.14.3 + + +From 025d84b53bd4ffc0626dd25aa8351319f4d77944 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Sun, 26 Nov 2017 17:36:27 +0100 +Subject: [PATCH 03/15] test-new-dribble: make credentials swap asymmetric +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +the first case of test-new-dribble (standard-test) is updated, by making +the credentials swap between the left and right agent asymmetric. +Previously, ragent started to receive stun requests without initially +knowing lagent candidates. Now, ragent also ignores lagent credentials. +This modification allows to test changes introduced by the previous +commit. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1890 +--- + tests/test-new-dribble.c | 55 ++++++++++++++++++++---------------------------- + 1 file changed, 23 insertions(+), 32 deletions(-) + +diff --git a/tests/test-new-dribble.c b/tests/test-new-dribble.c +index 3e60ae3..947f55d 100644 +--- a/tests/test-new-dribble.c ++++ b/tests/test-new-dribble.c +@@ -269,7 +269,7 @@ static gpointer stun_thread_func (const gpointer user_data) + return NULL; + } + +-static void set_credentials (NiceAgent *lagent, guint lstream, ++static void swap_credentials (NiceAgent *lagent, guint lstream, + NiceAgent *ragent, guint rstream) + { + gchar *ufrag = NULL, *password = NULL; +@@ -279,12 +279,6 @@ static void set_credentials (NiceAgent *lagent, guint lstream, + + g_free (ufrag); + g_free (password); +- +- nice_agent_get_local_credentials (ragent, rstream, &ufrag, &password); +- nice_agent_set_remote_credentials (lagent, lstream, ufrag, password); +- +- g_free (ufrag); +- g_free (password); + } + + static void cb_candidate_gathering_done(NiceAgent *agent, guint stream_id, gpointer data) +@@ -500,12 +494,10 @@ static void standard_test(NiceAgent *lagent, NiceAgent *ragent) + g_cancellable_reset (global_cancellable); + g_assert (ragent_candidate_gathering_done); + +- set_credentials (lagent, global_ls_id, ragent, global_rs_id); + + g_debug ("Setting local candidates of ragent as remote candidates of lagent"); +- swap_candidates (ragent, global_rs_id, +- lagent, global_ls_id, +- TRUE); ++ swap_candidates (ragent, global_rs_id, lagent, global_ls_id, TRUE); ++ swap_credentials (ragent, global_rs_id, lagent, global_ls_id); + + while (!data_received) + g_main_context_iteration (NULL, TRUE); +@@ -514,9 +506,9 @@ static void standard_test(NiceAgent *lagent, NiceAgent *ragent) + data_received); + + g_debug ("Setting local candidates of lagent as remote candidates of ragent"); +- swap_candidates (lagent, global_ls_id, +- ragent, global_rs_id, +- FALSE); ++ swap_candidates (lagent, global_ls_id, ragent, global_rs_id, FALSE); ++ swap_credentials (lagent, global_ls_id, ragent, global_rs_id); ++ + while (!lagent_candidate_gathering_done) + g_main_context_iteration (NULL, TRUE); + g_cancellable_reset (global_cancellable); +@@ -557,22 +549,21 @@ static void bad_credentials_test(NiceAgent *lagent, NiceAgent *ragent) + g_cancellable_reset (global_cancellable); + g_assert (ragent_candidate_gathering_done); + +- swap_candidates (ragent, global_rs_id, +- lagent, global_ls_id, +- FALSE); ++ g_debug ("Setting local candidates of ragent as remote candidates of lagent"); ++ swap_candidates (ragent, global_rs_id, lagent, global_ls_id, FALSE); ++ + while (global_lagent_state != NICE_COMPONENT_STATE_FAILED) + g_main_context_iteration (NULL, TRUE); + g_cancellable_reset (global_cancellable); + + // Set the correct credentials and swap candidates +- set_credentials (lagent, global_ls_id, ragent, global_rs_id); +- swap_candidates (ragent, global_rs_id, +- lagent, global_ls_id, +- FALSE); ++ g_debug ("Setting local candidates of ragent as remote candidates of lagent"); ++ swap_candidates (ragent, global_rs_id, lagent, global_ls_id, FALSE); ++ swap_credentials (lagent, global_ls_id, ragent, global_rs_id); + +- swap_candidates (lagent, global_ls_id, +- ragent, global_rs_id, +- FALSE); ++ g_debug ("Setting local candidates of lagent as remote candidates of ragent"); ++ swap_candidates (lagent, global_ls_id, ragent, global_rs_id, FALSE); ++ swap_credentials (ragent, global_rs_id, lagent, global_ls_id); + + while (!data_received) + g_main_context_iteration (NULL, TRUE); +@@ -628,15 +619,14 @@ static void bad_candidate_test(NiceAgent *lagent,NiceAgent *ragent) + + g_assert (global_lagent_state == NICE_COMPONENT_STATE_FAILED && + !data_received); +- set_credentials (lagent, global_ls_id, ragent, global_rs_id); + +- swap_candidates (ragent, global_rs_id, +- lagent, global_ls_id, +- FALSE); ++ g_debug ("Setting local candidates of ragent as remote candidates of lagent"); ++ swap_candidates (ragent, global_rs_id, lagent, global_ls_id, FALSE); ++ swap_credentials (ragent, global_rs_id, lagent, global_ls_id); + +- swap_candidates (lagent, global_ls_id, +- ragent, global_rs_id, +- FALSE); ++ g_debug ("Setting local candidates of lagent as remote candidates of ragent"); ++ swap_candidates (lagent, global_ls_id, ragent, global_rs_id, FALSE); ++ swap_credentials (lagent, global_ls_id, ragent, global_rs_id); + + while (!data_received) + g_main_context_iteration (NULL, TRUE); +@@ -655,7 +645,8 @@ static void new_candidate_test(NiceAgent *lagent, NiceAgent *ragent) + g_debug ("test-dribblemode:%s", G_STRFUNC); + + init_test (lagent, ragent, TRUE); +- set_credentials (lagent, global_ls_id, ragent, global_rs_id); ++ swap_credentials (lagent, global_ls_id, ragent, global_rs_id); ++ swap_credentials (ragent, global_rs_id, lagent, global_ls_id); + + nice_agent_gather_candidates (lagent, global_ls_id); + while (!got_stun_packet) +-- +2.14.3 + + +From b5dd5e2aa72a68ac9f027bbcc22700db84a35677 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Sun, 26 Nov 2017 17:49:25 +0100 +Subject: [PATCH 04/15] conncheck: the conncheck send function may fail +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +With this patch, we put the pair in state failed if we cannot send +the connection check, for example due to missing local credentials. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1891 +--- + agent/conncheck.c | 12 ++++++++++-- + 1 file changed, 10 insertions(+), 2 deletions(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 11ef9c9..9618c3a 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -442,7 +442,11 @@ static gboolean priv_conn_check_initiate (NiceAgent *agent, CandidateCheckPair * + { + pair->state = NICE_CHECK_IN_PROGRESS; + nice_debug ("Agent %p : pair %p state IN_PROGRESS", agent, pair); +- conn_check_send (agent, pair); ++ if (conn_check_send (agent, pair)) { ++ pair->state = NICE_CHECK_FAILED; ++ nice_debug ("Agent %p : pair %p state FAILED", agent, pair); ++ return FALSE; ++ } + return TRUE; + } + +@@ -1070,7 +1074,11 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent) + if (pair) { + priv_print_conn_check_lists (agent, G_STRFUNC, + ", got a pair from triggered check list"); +- conn_check_send (agent, pair); ++ if (conn_check_send (agent, pair)) { ++ pair->state = NICE_CHECK_FAILED; ++ nice_debug ("Agent %p : pair %p state FAILED", agent, pair); ++ return FALSE; ++ } + return TRUE; + } + +-- +2.14.3 + + +From 47aa02885cda9ddf6e938f966a926be000611c5a Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Sun, 26 Nov 2017 18:10:12 +0100 +Subject: [PATCH 05/15] conncheck: factorize pair state debug +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1892 +--- + agent/conncheck.c | 69 +++++++++++++++++++++++++------------------------------ + 1 file changed, 31 insertions(+), 38 deletions(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 9618c3a..00d02c5 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -123,22 +123,29 @@ priv_state_to_string (NiceCheckState state) + { + switch (state) { + case NICE_CHECK_WAITING: +- return "waiting"; ++ return "WAITING"; + case NICE_CHECK_IN_PROGRESS: +- return "in progress"; ++ return "IN_PROGRESS"; + case NICE_CHECK_SUCCEEDED: +- return "succeeded"; ++ return "SUCCEEDED"; + case NICE_CHECK_FAILED: +- return "failed"; ++ return "FAILED"; + case NICE_CHECK_FROZEN: +- return "frozen"; ++ return "FROZEN"; + case NICE_CHECK_DISCOVERED: +- return "discovered"; ++ return "DISCOVERED"; + default: + g_assert_not_reached (); + } + } + ++#define SET_PAIR_STATE( a, p, s ) G_STMT_START{\ ++ g_assert (p); \ ++ p->state = s; \ ++ nice_debug ("Agent %p : pair %p state %s (%s)", \ ++ a, p, priv_state_to_string (s), G_STRFUNC); \ ++}G_STMT_END ++ + static const gchar * + priv_ice_return_to_string (StunUsageIceReturn ice_return) + { +@@ -251,8 +258,7 @@ priv_add_pair_to_triggered_check_queue (NiceAgent *agent, CandidateCheckPair *pa + { + g_assert (pair); + +- pair->state = NICE_CHECK_IN_PROGRESS; +- nice_debug ("Agent %p : pair %p state IN_PROGRESS", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_IN_PROGRESS); + if (agent->triggered_check_queue == NULL || + g_slist_find (agent->triggered_check_queue, pair) == NULL) + agent->triggered_check_queue = g_slist_append (agent->triggered_check_queue, pair); +@@ -440,11 +446,9 @@ priv_find_first_frozen_check_list (NiceAgent *agent) + */ + static gboolean priv_conn_check_initiate (NiceAgent *agent, CandidateCheckPair *pair) + { +- pair->state = NICE_CHECK_IN_PROGRESS; +- nice_debug ("Agent %p : pair %p state IN_PROGRESS", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_IN_PROGRESS); + if (conn_check_send (agent, pair)) { +- pair->state = NICE_CHECK_FAILED; +- nice_debug ("Agent %p : pair %p state FAILED", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_FAILED); + return FALSE; + } + return TRUE; +@@ -495,8 +499,7 @@ static gboolean priv_conn_check_unfreeze_next (NiceAgent *agent, NiceStream *str + if (pair) { + nice_debug ("Agent %p : Pair %p with s/c-id %u/%u (%s) unfrozen.", + agent, pair, pair->stream_id, pair->component_id, pair->foundation); +- pair->state = NICE_CHECK_WAITING; +- nice_debug ("Agent %p : pair %p state WAITING", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_WAITING); + result = TRUE; + } + } +@@ -535,8 +538,7 @@ static void priv_conn_check_unfreeze_related (NiceAgent *agent, NiceStream *stre + strncmp (p->foundation, ok_check->foundation, + NICE_CANDIDATE_PAIR_MAX_FOUNDATION) == 0) { + nice_debug ("Agent %p : Unfreezing check %p (after successful check %p).", agent, p, ok_check); +- p->state = NICE_CHECK_WAITING; +- nice_debug ("Agent %p : pair %p state WAITING", agent, p); ++ SET_PAIR_STATE (agent, p, NICE_CHECK_WAITING); + } + } + } +@@ -559,8 +561,7 @@ static void priv_conn_check_unfreeze_related (NiceAgent *agent, NiceStream *stre + if (p->state == NICE_CHECK_FROZEN && + priv_foundation_matches_a_valid_pair (p->foundation, stream)) { + nice_debug ("Agent %p : Unfreezing check %p from stream %u (after successful check %p).", agent, p, s->id, ok_check); +- p->state = NICE_CHECK_WAITING; +- nice_debug ("Agent %p : pair %p state WAITING", agent, p); ++ SET_PAIR_STATE (agent, p, NICE_CHECK_WAITING); + } + } + } else if (priv_is_checklist_frozen (s)) { +@@ -576,8 +577,7 @@ static void priv_conn_check_unfreeze_related (NiceAgent *agent, NiceStream *stre + if (priv_foundation_matches_a_valid_pair (p->foundation, stream)) { + match_found = TRUE; + nice_debug ("Agent %p : Unfreezing check %p from stream %u (after successful check %p).", agent, p, s->id, ok_check); +- p->state = NICE_CHECK_WAITING; +- nice_debug ("Agent %p : pair %p state WAITING", agent, p); ++ SET_PAIR_STATE (agent, p, NICE_CHECK_WAITING); + } + } + +@@ -675,8 +675,7 @@ candidate_check_pair_fail (NiceStream *stream, NiceAgent *agent, CandidateCheckP + NiceComponent *component; + + component = nice_stream_find_component_by_id (stream, p->component_id); +- p->state = NICE_CHECK_FAILED; +- nice_debug ("Agent %p : pair %p state FAILED", agent, p); ++ SET_PAIR_STATE (agent, p, NICE_CHECK_FAILED); + priv_free_all_stun_transactions (p, component); + } + +@@ -841,8 +840,7 @@ timer_return_timeout: + if (pair) { + priv_print_conn_check_lists (agent, G_STRFUNC, + ", got a pair in Frozen state"); +- pair->state = NICE_CHECK_WAITING; +- nice_debug ("Agent %p : pair %p state WAITING", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_WAITING); + priv_conn_check_initiate (agent, pair); + return TRUE; + } +@@ -1075,8 +1073,7 @@ static gboolean priv_conn_check_tick_unlocked (NiceAgent *agent) + priv_print_conn_check_lists (agent, G_STRFUNC, + ", got a pair from triggered check list"); + if (conn_check_send (agent, pair)) { +- pair->state = NICE_CHECK_FAILED; +- nice_debug ("Agent %p : pair %p state FAILED", agent, pair); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_FAILED); + return FALSE; + } + return TRUE; +@@ -2067,8 +2064,8 @@ static CandidateCheckPair *priv_add_new_check_pair (NiceAgent *agent, + g_snprintf (pair->foundation, NICE_CANDIDATE_PAIR_MAX_FOUNDATION, "%s:%s", local->foundation, remote->foundation); + + pair->priority = agent_candidate_pair_priority (agent, local, remote); +- pair->state = initial_state; +- nice_debug ("Agent %p : creating new pair %p state %d", agent, pair, initial_state); ++ nice_debug ("Agent %p : creating a new pair", agent); ++ SET_PAIR_STATE (agent, pair, initial_state); + { + gchar tmpbuf1[INET6_ADDRSTRLEN]; + gchar tmpbuf2[INET6_ADDRSTRLEN]; +@@ -2976,10 +2973,10 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint + pair->local = local_cand; + pair->remote = parent_pair->remote; + pair->sockptr = local_cand->sockptr; +- pair->state = NICE_CHECK_DISCOVERED; + parent_pair->discovered_pair = pair; + pair->succeeded_pair = parent_pair; +- nice_debug ("Agent %p : new pair %p state DISCOVERED", agent, pair); ++ nice_debug ("Agent %p : creating a new pair", agent); ++ SET_PAIR_STATE (agent, pair, NICE_CHECK_DISCOVERED); + { + gchar tmpbuf1[INET6_ADDRSTRLEN]; + gchar tmpbuf2[INET6_ADDRSTRLEN]; +@@ -3099,10 +3096,9 @@ static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent * + */ + if (new_pair == p) + p->valid = TRUE; +- p->state = NICE_CHECK_SUCCEEDED; ++ SET_PAIR_STATE (agent, p, NICE_CHECK_SUCCEEDED); + priv_remove_pair_from_triggered_check_queue (agent, p); + priv_free_all_stun_transactions (p, component); +- nice_debug ("Agent %p : conncheck %p SUCCEEDED.", agent, p); + nice_component_add_valid_candidate (component, remote_candidate); + } + else { +@@ -3135,11 +3131,9 @@ static CandidateCheckPair *priv_process_response_check_for_reflexive(NiceAgent * + /* step: The agent sets the state of the pair that *generated* the check to + * Succeeded, RFC 5245, 7.1.3.2.3, "Updating Pair States" + */ +- p->state = NICE_CHECK_SUCCEEDED; ++ SET_PAIR_STATE (agent, p, NICE_CHECK_SUCCEEDED); + priv_remove_pair_from_triggered_check_queue (agent, p); + priv_free_all_stun_transactions (p, component); +- nice_debug ("Agent %p : conncheck %p SUCCEEDED, %p DISCOVERED.", +- agent, p, new_pair); + } + + if (new_pair && new_pair->valid) +@@ -3226,10 +3220,9 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + * "Discovering Peer Reflexive Candidates" ICE ID-19) */ + + if (res == STUN_USAGE_ICE_RETURN_NO_MAPPED_ADDRESS) { +- p->state = NICE_CHECK_SUCCEEDED; ++ nice_debug ("Agent %p : Mapped address not found", agent); ++ SET_PAIR_STATE (agent, p, NICE_CHECK_SUCCEEDED); + p->valid = TRUE; +- nice_debug ("Agent %p : Mapped address not found." +- " conncheck %p SUCCEEDED.", agent, p); + nice_component_add_valid_candidate (component, p->remote); + } else + ok_pair = priv_process_response_check_for_reflexive (agent, +-- +2.14.3 + + +From 05f1e30239a448385709df0edd43ec3ac5173218 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Sun, 26 Nov 2017 19:31:39 +0100 +Subject: [PATCH 06/15] conncheck: make debug more homonegeous +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1893 +--- + agent/conncheck.c | 35 ++++++++++++++++++----------------- + 1 file changed, 18 insertions(+), 17 deletions(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 00d02c5..25bfd80 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -753,8 +753,8 @@ timer_return_timeout: + /* case: error, abort processing */ + nice_address_to_string (&p->local->addr, tmpbuf1); + nice_address_to_string (&p->remote->addr, tmpbuf2); +- nice_debug ("Agent %p : Retransmissions failed, giving up on " +- "connectivity check %p", agent, p); ++ nice_debug ("Agent %p : Retransmissions failed, giving up on pair %p", ++ agent, p); + nice_debug ("Agent %p : Failed pair is [%s]:%u --> [%s]:%u", agent, + tmpbuf1, nice_address_get_port (&p->local->addr), + tmpbuf2, nice_address_get_port (&p->remote->addr)); +@@ -973,7 +973,7 @@ priv_conn_check_tick_stream_nominate (NiceStream *stream, NiceAgent *agent) + p = p->succeeded_pair; + } + g_assert (p->state == NICE_CHECK_SUCCEEDED); +- nice_debug ("Agent %p : restarting check %p with " ++ nice_debug ("Agent %p : restarting check of pair %p with " + "USE-CANDIDATE attrib (regular nomination)", agent, p); + p->use_candidate_on_next_check = TRUE; + priv_add_pair_to_triggered_check_queue (agent, p); +@@ -996,7 +996,8 @@ priv_conn_check_tick_stream_nominate (NiceStream *stream, NiceAgent *agent) + if (p->component_id == component->id && + (p->state == NICE_CHECK_SUCCEEDED || + p->state == NICE_CHECK_DISCOVERED)) { +- nice_debug ("Agent %p : restarting check %p as the nominated pair.", agent, p); ++ nice_debug ("Agent %p : restarting check of pair %p as the " ++ "nominated pair.", agent, p); + p->nominated = TRUE; + priv_update_selected_pair (agent, component, p); + priv_add_pair_to_triggered_check_queue (agent, p); +@@ -2081,7 +2082,9 @@ static CandidateCheckPair *priv_add_new_check_pair (NiceAgent *agent, + stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair, + (GCompareFunc)conn_check_compare); + +- nice_debug ("Agent %p : added a new conncheck %p with foundation of '%s' to list %u.", agent, pair, pair->foundation, stream_id); ++ nice_debug ("Agent %p : added a new pair %p with foundation '%s' to " ++ "stream %u component %u.", agent, pair, pair->foundation, stream_id, ++ component->id); + + /* implement the hard upper limit for number of + checks (see sect 5.7.3 ICE ID-19): */ +@@ -2117,9 +2120,6 @@ static CandidateCheckPair *priv_conn_check_add_for_candidate_pair_matched ( + { + CandidateCheckPair *pair; + +- nice_debug ("Agent %p : Adding check pair between %s and %s for s%d/c%d", +- agent, local->foundation, remote->foundation, +- stream_id, component->id); + pair = priv_add_new_check_pair (agent, stream_id, component, local, remote, + initial_state); + if (component->state == NICE_COMPONENT_STATE_CONNECTED || +@@ -2997,7 +2997,8 @@ static CandidateCheckPair *priv_add_peer_reflexive_pair (NiceAgent *agent, guint + pair->nominated = FALSE; + 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); ++ nice_debug ("Agent %p : added a new peer-discovered pair with " ++ "foundation '%s'.", agent, pair->foundation); + + stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair, + (GCompareFunc)conn_check_compare); +@@ -3190,7 +3191,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + + CandidateCheckPair *ok_pair = NULL; + +- nice_debug ("Agent %p : conncheck %p MATCHED.", agent, p); ++ nice_debug ("Agent %p : pair %p MATCHED.", agent, p); + priv_remove_stun_transaction (p, stun, component); + + /* step: verify that response came from the same IP address we +@@ -3201,7 +3202,7 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + if (nice_debug_is_enabled ()) { + gchar tmpbuf[INET6_ADDRSTRLEN]; + gchar tmpbuf2[INET6_ADDRSTRLEN]; +- nice_debug ("Agent %p : conncheck %p FAILED" ++ nice_debug ("Agent %p : pair %p FAILED" + " (mismatch of source address).", agent, p); + nice_address_to_string (&p->remote->addr, tmpbuf); + nice_address_to_string (from, tmpbuf2); +@@ -3316,7 +3317,8 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + gboolean controlled_mode; + + /* case: role conflict error, need to restart with new role */ +- nice_debug ("Agent %p : conncheck %p ROLE CONFLICT, restarting", agent, p); ++ nice_debug ("Agent %p : Role conflict with pair %p, restarting", ++ agent, p); + + /* note: this res value indicates that the role of the peer + * agent has not changed after the tie-breaker comparison, so +@@ -3341,7 +3343,6 @@ static gboolean priv_map_reply_to_conn_check_request (NiceAgent *agent, NiceStre + priv_add_pair_to_triggered_check_queue (agent, p); + } else { + /* case: STUN error, the check STUN context was freed */ +- nice_debug ("Agent %p : conncheck %p FAILED.", agent, p); + candidate_check_pair_fail (stream, agent, p); + } + return TRUE; +@@ -4228,8 +4229,8 @@ gboolean conn_check_handle_inbound_stun (NiceAgent *agent, NiceStream *stream, + agent_signal_initial_binding_request_received (agent, stream); + + if (remote_candidate == NULL) { +- nice_debug ("Agent %p : No matching remote candidate for incoming check ->" +- "peer-reflexive candidate.", agent); ++ 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, +@@ -4332,8 +4333,8 @@ conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *co + if ((p->local != NULL && p->local->sockptr == sock) || + (p->remote != NULL && p->remote->sockptr == sock) || + (p->sockptr == sock)) { +- nice_debug ("Agent %p : Retransmissions failed, giving up on " +- "connectivity check %p", agent, p); ++ nice_debug ("Agent %p : Retransmissions failed, giving up on pair %p", ++ agent, p); + candidate_check_pair_fail (stream, agent, p); + conn_check_free_item (p); + stream->conncheck_list = g_slist_delete_link (stream->conncheck_list, l); +-- +2.14.3 + + +From 00dfcc6a625e6c6ed758a4b3b4d0858113508a2f Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Mon, 27 Nov 2017 23:56:17 +0100 +Subject: [PATCH 07/15] socket: ping the stun server address on the right + socket +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Verify the compatibility of the socket domain with the stun server +IP address, before sending a request. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1894 +--- + agent/agent.c | 12 +++++++----- + agent/conncheck.c | 4 +++- + 2 files changed, 10 insertions(+), 6 deletions(-) + +diff --git a/agent/agent.c b/agent/agent.c +index 49fc371..3306378 100644 +--- a/agent/agent.c ++++ b/agent/agent.c +@@ -3062,11 +3062,13 @@ nice_agent_gather_candidates ( + if (nice_address_set_from_string (&stun_server, agent->stun_server_ip)) { + nice_address_set_port (&stun_server, agent->stun_server_port); + +- priv_add_new_candidate_discovery_stun (agent, +- host_candidate->sockptr, +- stun_server, +- stream, +- cid); ++ if (nice_address_ip_version (&host_candidate->addr) == ++ nice_address_ip_version (&stun_server)) ++ priv_add_new_candidate_discovery_stun (agent, ++ host_candidate->sockptr, ++ stun_server, ++ stream, ++ cid); + } + } + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 25bfd80..4d91f41 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -1459,7 +1459,9 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent) + for (k = component->local_candidates; k; k = k->next) { + NiceCandidate *candidate = (NiceCandidate *) k->data; + if (candidate->type == NICE_CANDIDATE_TYPE_HOST && +- candidate->transport == NICE_CANDIDATE_TRANSPORT_UDP) { ++ candidate->transport == NICE_CANDIDATE_TRANSPORT_UDP && ++ nice_address_ip_version (&candidate->addr) == ++ nice_address_ip_version (&stun_server)) { + /* send the conncheck */ + nice_debug ("Agent %p : resending STUN on %s to keep the " + "candidate alive.", agent, candidate->foundation); +-- +2.14.3 + + +From ea05a3d51990d17bbe25984eb5730849f46bfae0 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Wed, 29 Nov 2017 11:04:04 +0100 +Subject: [PATCH 08/15] conncheck: dont fail a stream with a empty conncheck + list +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Since commit 17f30e4, we may have a stream with an empty conncheck list, +and such a stream obviously should not be tested for failed components. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1895 +--- + agent/conncheck.c | 9 ++++++--- + 1 file changed, 6 insertions(+), 3 deletions(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 4d91f41..0ebe7e9 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -1832,6 +1832,9 @@ static void priv_update_check_list_failed_components (NiceAgent *agent, NiceStre + * must be fetched before entering the loop*/ + guint c, components = stream->n_components; + ++ if (stream->conncheck_list == NULL) ++ return; ++ + for (i = agent->discovery_list; i; i = i->next) { + CandidateDiscovery *d = i->data; + +@@ -1846,8 +1849,8 @@ static void priv_update_check_list_failed_components (NiceAgent *agent, NiceStre + + /* note: iterate the conncheck list for each component separately */ + for (c = 0; c < components; c++) { +- NiceComponent *comp = NULL; +- if (!agent_find_component (agent, stream->id, c+1, NULL, &comp)) ++ NiceComponent *component = NULL; ++ if (!agent_find_component (agent, stream->id, c+1, NULL, &component)) + continue; + + nominated = 0; +@@ -1873,7 +1876,7 @@ static void priv_update_check_list_failed_components (NiceAgent *agent, NiceStre + * Set the component to FAILED only if it actually had remote candidates + * that failed.. */ + if (completed && nominated == 0 && +- comp != NULL && comp->remote_candidates != NULL) ++ component != NULL && component->remote_candidates != NULL) + agent_signal_component_state_change (agent, + stream->id, + (c + 1), /* component-id */ +-- +2.14.3 + + +From a9ac0487b0d1708d780d7c0b7a2206c71a8c7163 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Thu, 30 Nov 2017 20:11:22 +0100 +Subject: [PATCH 09/15] discovery: ignore all non-relay local candidates when + relay is forced +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The tcp server reflexive discovered local candidates must be ignored +when force_relay is set. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1899 +--- + agent/discovery.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/agent/discovery.c b/agent/discovery.c +index 4cc99c2..e2142a2 100644 +--- a/agent/discovery.c ++++ b/agent/discovery.c +@@ -688,7 +688,8 @@ discovery_discover_tcp_server_reflexive_candidates ( + + caddr = c->addr; + nice_address_set_port (&caddr, 0); +- if (c->transport != NICE_CANDIDATE_TRANSPORT_UDP && ++ if (agent->force_relay == FALSE && ++ c->transport != NICE_CANDIDATE_TRANSPORT_UDP && + c->type == NICE_CANDIDATE_TYPE_HOST && + nice_address_equal (&base_addr, &caddr)) { + nice_address_set_port (address, nice_address_get_port (&c->addr)); +-- +2.14.3 + + +From 5a644f459dc75c80dfb19c7772f74e37a0258771 Mon Sep 17 00:00:00 2001 +From: Fabrice Bellet +Date: Mon, 11 Dec 2017 08:50:33 +0100 +Subject: [PATCH 10/15] agent: make candidate username and password immutable +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +With this patch we prevent the username and the password of a candidate +to be modified during a session, as required by the RFC, sect 9.1.2. +This is also needed from a memory management point of view, because the +password string pointer may be recorded in the components stun agent +sent_ids[] struct key member, and freeing these values there may cause +an use-after-free condition, when an inbound stun is received from this +candidate. This behavior has been observed with pidgin, xmpp, and +farstream when a same remote candidates are "updated" several times, +even if the credentials don't change in this case. + +Reviewed-by: Olivier Crête +Differential Revision: https://phabricator.freedesktop.org/D1917 +--- + agent/agent.c | 19 +++++++++++++------ + 1 file changed, 13 insertions(+), 6 deletions(-) + +diff --git a/agent/agent.c b/agent/agent.c +index 3306378..dbece3b 100644 +--- a/agent/agent.c ++++ b/agent/agent.c +@@ -3388,15 +3388,22 @@ static gboolean priv_add_remote_candidate ( + * this is essential to overcome a race condition where we might receive + * a valid binding request from a valid candidate that wasn't yet added to + * our list of candidates.. this 'update' will make the peer-rflx a +- * server-rflx/host candidate again and restore that user/pass it needed +- * to have in the first place */ ++ * server-rflx/host candidate again */ + if (username) { +- g_free (candidate->username); +- candidate->username = g_strdup (username); ++ if (candidate->username == NULL) ++ candidate->username = g_strdup (username); ++ else if (g_strcmp0 (username, candidate->username)) ++ nice_debug ("Agent %p : Candidate username '%s' is not allowed " ++ "to change to '%s' now (ICE restart only).", agent, ++ candidate->username, username); + } + if (password) { +- g_free (candidate->password); +- candidate->password = g_strdup (password); ++ if (candidate->password == NULL) ++ candidate->password = g_strdup (password); ++ else if (g_strcmp0 (password, candidate->password)) ++ nice_debug ("Agent %p : candidate password '%s' is not allowed " ++ "to change to '%s' now (ICE restart only).", agent, ++ candidate->password, password); + } + + /* since the type of the existing candidate may have changed, +-- +2.14.3 + + +From 54fb03427ebc13413cd1ddd5d9e91c1751eac0cb Mon Sep 17 00:00:00 2001 +From: Jakub Adam +Date: Sat, 3 Feb 2018 23:59:20 +0100 +Subject: [PATCH 11/15] discovery: ignore bogus Skype for Business srflx + addresses + +If main SfB TURN server sends our allocation request to an alternate +server, the response will have XOR_MAPPED_ADDRESS containing the IP +address of the turn server that proxied the message instead of our own +actual external IP. + +Before we create server reflexive candidates upon receiving an allocate +response, check that the TURN port got assigned on the same server we +sent out allocate request to. Otherwise, the request was proxied and +XOR_MAPPED_ADDRESS contains a bogus value we should ignore. + +Issue introduced by 59fcf95d505c3995f858b826d10cd48321ed383e. +Differential Revision: https://phabricator.freedesktop.org/D1949 +--- + agent/conncheck.c | 31 +++++++++++++++++++++---------- + 1 file changed, 21 insertions(+), 10 deletions(-) + +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 0ebe7e9..19729c2 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -3587,9 +3587,13 @@ static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage * + NiceAddress niceaddr; + NiceCandidate *relay_cand; + ++ nice_address_set_from_sockaddr (&niceaddr, &relayaddr.addr); ++ + if (res == STUN_USAGE_TURN_RETURN_MAPPED_SUCCESS) { ++ NiceAddress mappedniceaddr; ++ + /* We also received our mapped address */ +- nice_address_set_from_sockaddr (&niceaddr, &sockaddr.addr); ++ nice_address_set_from_sockaddr (&mappedniceaddr, &sockaddr.addr); + + /* TCP or TLS TURNS means the server-reflexive address was + * on a TCP connection, which cannot be used for server-reflexive +@@ -3601,21 +3605,28 @@ static gboolean priv_map_reply_to_relay_request (NiceAgent *agent, StunMessage * + d->agent, + d->stream->id, + d->component->id, +- &niceaddr, ++ &mappedniceaddr, + NICE_CANDIDATE_TRANSPORT_UDP, + d->nicesock, + FALSE); + } +- if (d->agent->use_ice_tcp) +- discovery_discover_tcp_server_reflexive_candidates ( +- d->agent, +- d->stream->id, +- d->component->id, +- &niceaddr, +- d->nicesock); ++ if (d->agent->use_ice_tcp) { ++ if ((agent->compatibility == NICE_COMPATIBILITY_OC2007 || ++ agent->compatibility == NICE_COMPATIBILITY_OC2007R2) && ++ !nice_address_equal_no_port (&niceaddr, &d->turn->server)) { ++ nice_debug("TURN port got allocated on an alternate server, " ++ "ignoring bogus srflx address"); ++ } else { ++ discovery_discover_tcp_server_reflexive_candidates ( ++ d->agent, ++ d->stream->id, ++ d->component->id, ++ &mappedniceaddr, ++ d->nicesock); ++ } ++ } + } + +- nice_address_set_from_sockaddr (&niceaddr, &relayaddr.addr); + if (nice_socket_is_reliable (d->nicesock)) { + relay_cand = discovery_add_relay_candidate ( + d->agent, +-- +2.14.3 + + +From 922ee4e61b4d9c6b403933f4a3261e67589d5099 Mon Sep 17 00:00:00 2001 +From: Jakub Adam +Date: Wed, 19 Apr 2017 14:17:04 +0200 +Subject: [PATCH 12/15] agent: don't require "reliable" be TRUE in order to use + "ice-tcp" + +Setting writable socket callbacks doesn't have to be limited to reliable +agents. TCP sockets need the callback in any case for correct operation +and calling nice_socket_set_writable_callback() on a NiceSocket that has +UDP as its base has no effect. + +Differential Revision: https://phabricator.freedesktop.org/D1726 +--- + agent/agent.c | 11 ++++------- + agent/conncheck.c | 5 ++--- + 2 files changed, 6 insertions(+), 10 deletions(-) + +diff --git a/agent/agent.c b/agent/agent.c +index dbece3b..89e3514 100644 +--- a/agent/agent.c ++++ b/agent/agent.c +@@ -2547,9 +2547,8 @@ priv_add_new_candidate_discovery_turn (NiceAgent *agent, + if (nicesock == NULL) + return; + +- if (agent->reliable) +- nice_socket_set_writable_callback (nicesock, _tcp_sock_is_writable, +- component); ++ nice_socket_set_writable_callback (nicesock, _tcp_sock_is_writable, component); ++ + if (turn->type == NICE_RELAY_TYPE_TURN_TLS && + agent->compatibility == NICE_COMPATIBILITY_GOOGLE) { + nicesock = nice_pseudossl_socket_new (nicesock, +@@ -3033,10 +3032,8 @@ nice_agent_gather_candidates ( + found_local_address = TRUE; + nice_address_set_port (addr, 0); + +- +- if (agent->reliable) +- nice_socket_set_writable_callback (host_candidate->sockptr, +- _tcp_sock_is_writable, component); ++ nice_socket_set_writable_callback (host_candidate->sockptr, ++ _tcp_sock_is_writable, component); + + #ifdef HAVE_GUPNP + if (agent->upnp_enabled && agent->upnp && +diff --git a/agent/conncheck.c b/agent/conncheck.c +index 19729c2..64a3cb8 100644 +--- a/agent/conncheck.c ++++ b/agent/conncheck.c +@@ -2669,9 +2669,8 @@ int conn_check_send (NiceAgent *agent, CandidateCheckPair *pair) + pair->sockptr = new_socket; + _priv_set_socket_tos (agent, pair->sockptr, stream2->tos); + +- if (agent->reliable) +- nice_socket_set_writable_callback (pair->sockptr, +- _tcp_sock_is_writable, component2); ++ nice_socket_set_writable_callback (pair->sockptr, _tcp_sock_is_writable, ++ component2); + + nice_component_attach_socket (component2, new_socket); + } +-- +2.14.3 + + +From e6217f8eba6ea17d90eac67ef5fa5412fbf10dad Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= +Date: Fri, 4 May 2018 15:44:05 +0200 +Subject: [PATCH 13/15] Ignore function case warnings + +This makes GLib usage annoying as it makes GSourceFunc casts invalid. +--- + configure.ac | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/configure.ac b/configure.ac +index 16988ad..36bd622 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -136,6 +136,7 @@ AS_IF([test "x$enable_compile_warnings" != "xno" -a \ + NICE_ADD_FLAG([-Wcast-align]) + NICE_ADD_FLAG([-Wformat-nonliteral]) + NICE_ADD_FLAG([-Wformat-security]) ++ NICE_ADD_FLAG([-Wno-cast-function-type]) + ]) + AS_IF([test "$enable_compile_warnings" = "yes" -o \ + "$enable_compile_warnings" = "maximum" -o \ +-- +2.14.3 + + +From 3a9d92818b4c2f083e26fe164a1be82212bd4061 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= +Date: Fri, 4 May 2018 16:44:45 +0200 +Subject: [PATCH 14/15] stund: Pass the right length for ipv6 + +--- + stun/tools/stund.c | 6 +++++- + 1 file changed, 5 insertions(+), 1 deletion(-) + +diff --git a/stun/tools/stund.c b/stun/tools/stund.c +index addc4fa..c148e51 100644 +--- a/stun/tools/stund.c ++++ b/stun/tools/stund.c +@@ -100,6 +100,8 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + struct sockaddr_in6 in6; + struct sockaddr_storage storage; + } addr; ++ int len; ++ + if (fd == -1) + { + perror ("Error opening IP port"); +@@ -118,6 +120,7 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + { + case AF_INET: + addr.in.sin_port = htons (port); ++ len = sizeof (struct sockaddr_in); + break; + + case AF_INET6: +@@ -125,13 +128,14 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + setsockopt (fd, SOL_IPV6, IPV6_V6ONLY, &yes, sizeof (yes)); + #endif + addr.in6.sin6_port = htons (port); ++ len = sizeof (struct sockaddr_in6); + break; + + default: + assert (0); /* should never be reached */ + } + +- if (bind (fd, &addr.addr, sizeof (struct sockaddr))) ++ if (bind (fd, &addr.addr, len)) + { + perror ("Error opening IP port"); + goto error; +-- +2.14.3 + + +From 34d60446ddfcdb98f2543611151ef8fbc5be4805 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= +Date: Fri, 4 May 2018 16:50:45 +0200 +Subject: [PATCH 15/15] stund: Pass sockaddr_storage size for both families + +--- + stun/tools/stund.c | 9 ++------- + 1 file changed, 2 insertions(+), 7 deletions(-) + +diff --git a/stun/tools/stund.c b/stun/tools/stund.c +index c148e51..00a0881 100644 +--- a/stun/tools/stund.c ++++ b/stun/tools/stund.c +@@ -100,15 +100,12 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + struct sockaddr_in6 in6; + struct sockaddr_storage storage; + } addr; +- int len; + + if (fd == -1) + { + perror ("Error opening IP port"); + return -1; + } +- if (fd < 3) +- goto error; + + memset (&addr, 0, sizeof (addr)); + addr.storage.ss_family = fam; +@@ -120,7 +117,6 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + { + case AF_INET: + addr.in.sin_port = htons (port); +- len = sizeof (struct sockaddr_in); + break; + + case AF_INET6: +@@ -128,14 +124,13 @@ int listen_socket (int fam, int type, int proto, unsigned int port) + setsockopt (fd, SOL_IPV6, IPV6_V6ONLY, &yes, sizeof (yes)); + #endif + addr.in6.sin6_port = htons (port); +- len = sizeof (struct sockaddr_in6); + break; + + default: + assert (0); /* should never be reached */ + } + +- if (bind (fd, &addr.addr, len)) ++ if (bind (fd, &addr.addr, sizeof (struct sockaddr_storage))) + { + perror ("Error opening IP port"); + goto error; +@@ -192,7 +187,7 @@ static int dgram_process (int sock, StunAgent *oldagent, StunAgent *newagent) + StunValidationStatus validation; + StunAgent *agent = NULL; + +- addr_len = sizeof (struct sockaddr_in); ++ addr_len = sizeof (struct sockaddr_storage); + len = recvfrom (sock, buf, sizeof(buf), 0, &addr.addr, &addr_len); + if (len == (size_t)-1) + return -1; +-- +2.14.3 + diff --git a/libnice-0.1.14-tests-koji.patch b/libnice-0.1.14-tests-koji.patch index f9feb34..997e712 100644 --- a/libnice-0.1.14-tests-koji.patch +++ b/libnice-0.1.14-tests-koji.patch @@ -22,7 +22,7 @@ index 16988ad..b7b74fa 100644 AC_CONFIG_FILES([ Makefile -@@ -262,7 +262,7 @@ AC_SUBST(gstplugindir) +@@ -263,7 +263,7 @@ AC_SUBST(gstplugindir) AC_SUBST(gstplugin010dir) AM_CONDITIONAL(WITH_GSTREAMER, test "$with_gstreamer" = yes) diff --git a/libnice.spec b/libnice.spec index c0960ef..26f66dc 100644 --- a/libnice.spec +++ b/libnice.spec @@ -1,16 +1,20 @@ # disable building of plugin for gstreamer 0.10 %bcond_with gst010 +%global upstream_date 20180504 +%global upstream_rnum 85 +%global upstream_hash 34d6044 + Name: libnice Version: 0.1.14 -Release: 6.20171128gitfb2f1f7%{?dist} +Release: 7.%{upstream_date}git%{upstream_hash}%{?dist} Summary: GLib ICE implementation Group: System Environment/Libraries License: LGPLv2 and MPLv1.1 URL: https://nice.freedesktop.org/wiki/ Source0: https://nice.freedesktop.org/releases/%{name}-%{version}.tar.gz -Patch1: libnice-0.1.14-70-gfb2f1f7.patch +Patch1: libnice-0.1.14-%{upstream_rnum}-g%{upstream_hash}.patch # make tests compile on i686 Patch2: libnice-0.1.14-tests-i686.patch @@ -150,6 +154,9 @@ make check %changelog +* Mon May 07 2018 Kamil Dudka - 0.1.14-7.20180504git34d6044 +- update to 0.1.14-85-g34d6044 (#1541646) + * Mon Apr 16 2018 Kamil Dudka - 0.1.14-6.20171128gitfb2f1f7 - temporarily make the upstream test-suite run on Intel arches only - disable test-send-recv, which fails in Koji