From f997215d0f6ec72e0402cbedfbfd00c21499ad4b Mon Sep 17 00:00:00 2001 From: Fabrice Bellet Date: Wed, 12 Feb 2020 15:48:12 +0100 Subject: [PATCH] candidate: ensuring stun priority uniqueness no more needed The uniqueness of candidate priorities is achieved by the iteration on the ip local addresses for local host candidates, and also on their base address for reflexive and relay candidates. Helper function checking its uniqueness at allocation time is not required anyore. The priority of the stun request (prflx_priority) is built from the priority of the local candidate of the pair, according the RFC 5245, section 7.1.2.1. This priority must be identical to a virtual "local candidate of type peer-reflexive that would be learned as a consequence of a check from this local candidate." Outgoing stun requests will come from local candidates of type host or type relayed. The priority uniqueness of local candidates of type host implies the uniqueness of the computed peer-reflexive priority. And relay local candidates cannot produce a peer-reflexive local candidate by design, so we can safely use their unique local priority too in the stun request. --- agent/conncheck.c | 86 ++++++++++++++----------------------------------------- agent/conncheck.h | 2 -- agent/discovery.c | 6 ---- 3 files changed, 21 insertions(+), 73 deletions(-) diff --git a/agent/conncheck.c b/agent/conncheck.c index 535da60..fca972b 100644 --- a/agent/conncheck.c +++ b/agent/conncheck.c @@ -1482,6 +1482,26 @@ static guint32 peer_reflexive_candidate_priority (NiceAgent *agent, return priority; } +/* Returns the priority of a local candidate of type peer-reflexive that + * would be learned as a consequence of a check from this local + * candidate. See RFC 5245, section 7.1.2.1. "PRIORITY and USE-CANDIDATE". + * RFC 5245 is more explanatory than RFC 8445 on this detail. + * + * Apply to local candidates of type host only, because candidates of type + * relay are supposed to have a public IP address, that wont generate + * a peer-reflexive address. Server-reflexive candidates are not + * concerned too, because no STUN request is sent with a local candidate + * of this type. + */ +static guint32 stun_request_prflx_priority (NiceAgent *agent, + NiceCandidate *local_candidate) +{ + if (local_candidate->type == NICE_CANDIDATE_TYPE_HOST) + return peer_reflexive_candidate_priority (agent, local_candidate); + else + return local_candidate->priority; +} + static void ms_ice2_legacy_conncheck_send(StunMessage *msg, NiceSocket *sock, const NiceAddress *remote_addr) { @@ -2407,69 +2427,6 @@ static void priv_mark_pair_nominated (NiceAgent *agent, NiceStream *stream, Nice } } -guint32 -ensure_unique_priority (NiceStream *stream, NiceComponent *component, - guint32 priority) -{ - GSList *item; - - again: - if (priority == 0) - priority--; - - for (item = component->local_candidates; item; item = item->next) { - NiceCandidate *cand = item->data; - - if (cand->priority == priority) { - priority--; - goto again; - } - } - - return priority; -} - -static guint32 -ensure_unique_prflx_priority (NiceStream *stream, NiceComponent *component, - guint32 local_priority, guint32 prflx_priority) -{ - GSList *item; - - /* First, ensure we provide the same value for pairs having - * the same local candidate, ie the same local candidate priority - * for the sake of coherency with the stun server behaviour that - * stores a unique priority value per remote candidate, from the - * first stun request it receives (it depends on the kind of NAT - * typically, but for NAT that preserves the binding this is required). - */ - for (item = stream->conncheck_list; item; item = item->next) { - CandidateCheckPair *p = item->data; - - if (p->component_id == component->id && - p->local->priority == local_priority) { - return p->prflx_priority; - } - } - - /* Second, ensure uniqueness across all other prflx_priority values */ - again: - if (prflx_priority == 0) - prflx_priority--; - - for (item = stream->conncheck_list; item; item = item->next) { - CandidateCheckPair *p = item->data; - - if (p->component_id == component->id && - p->prflx_priority == prflx_priority) { - prflx_priority--; - goto again; - } - } - - return prflx_priority; -} - - /* * Creates a new connectivity check pair and adds it to * the agent's list of checks. @@ -2513,8 +2470,7 @@ static CandidateCheckPair *priv_add_new_check_pair (NiceAgent *agent, tmpbuf1, nice_address_get_port (&pair->local->addr), tmpbuf2, nice_address_get_port (&pair->remote->addr)); } - pair->prflx_priority = ensure_unique_prflx_priority (stream, component, - local->priority, peer_reflexive_candidate_priority (agent, local)); + pair->prflx_priority = stun_request_prflx_priority (agent, local); stream->conncheck_list = g_slist_insert_sorted (stream->conncheck_list, pair, (GCompareFunc)conn_check_compare); diff --git a/agent/conncheck.h b/agent/conncheck.h index b9e3909..d20b122 100644 --- a/agent/conncheck.h +++ b/agent/conncheck.h @@ -118,8 +118,6 @@ void conn_check_prune_socket (NiceAgent *agent, NiceStream *stream, NiceComponent *component, NiceSocket *sock); -guint32 ensure_unique_priority (NiceStream *stream, NiceComponent *component, - guint32 priority); void recalculate_pair_priorities (NiceAgent *agent); void conn_check_update_selected_pair (NiceAgent *agent, NiceComponent *component, CandidateCheckPair *pair); diff --git a/agent/discovery.c b/agent/discovery.c index 61c0129..44e82d7 100644 --- a/agent/discovery.c +++ b/agent/discovery.c @@ -645,8 +645,6 @@ HostCandidateResult discovery_add_local_host_candidate ( agent->reliable, FALSE); } - candidate->priority = ensure_unique_priority (stream, component, - candidate->priority); priv_generate_candidate_credentials (agent, candidate); priv_assign_foundation (agent, candidate); @@ -737,8 +735,6 @@ discovery_add_server_reflexive_candidate ( agent->reliable, nat_assisted); } - candidate->priority = ensure_unique_priority (stream, component, - candidate->priority); priv_generate_candidate_credentials (agent, candidate); priv_assign_foundation (agent, candidate); @@ -856,8 +852,6 @@ discovery_add_relay_candidate ( agent->reliable, FALSE); } - candidate->priority = ensure_unique_priority (stream, component, - candidate->priority); priv_generate_candidate_credentials (agent, candidate); /* Google uses the turn username as the candidate username */ -- 2.7.4