Commit a04fa4d4 authored by Fabrice Bellet's avatar Fabrice Bellet Committed by Olivier Crête

discovery: use different port numbers for every local host candidates

This constraint is added to handle the situation where the agent runs on
a box doing SNAT on one of its outgoing network interface. The NAT does
usually its best to ensure that source port number is preserved on the
external NAT address and port. This is called "port preservation" in RFC
4787.

When two local host candidates are allowed to have the same source port
number, we increase the risk that a first local host candidate *is* the
NAT mapping address and port of a second local host candidate, because
of the "port preservation" effect. When it happens, a server reflexive
candidate and a host candidate will have the same address and port.

For that situation to happen, a stun request must be emitted from the
internal address first, the NAT mapping doing the port preservation will
be created for the internal address, and when a stun request is sent
from the external address thereafter, a new NAT mapping will be created,
but without port preservation, because the previous mapping already took
that reservation.

The problem will occur on the remote agent, when receiving a stun request
from this address and port, that has no way to know wheather it comes from
the host or the server reflexive candidate, if both have been advertised
remotely, resulting in pair type mislabelling.

This case may happen more easily when a source port range is reduced.
parent 0b80cbba
Pipeline #141932 failed with stages
in 47 minutes and 36 seconds
......@@ -3179,15 +3179,19 @@ nice_agent_gather_candidates (
current_port = start_port;
host_candidate = NULL;
while (res == HOST_CANDIDATE_CANT_CREATE_SOCKET) {
while (res == HOST_CANDIDATE_CANT_CREATE_SOCKET ||
res == HOST_CANDIDATE_DUPLICATE_PORT) {
nice_debug ("Agent %p: Trying to create host candidate on port %d", agent, current_port);
nice_address_set_port (addr, current_port);
res = discovery_add_local_host_candidate (agent, stream->id, cid,
addr, transport, &host_candidate);
if (current_port > 0)
current_port++;
if (current_port > component->max_port) current_port = component->min_port;
if (current_port == 0 || current_port == start_port)
if (current_port > component->max_port)
current_port = component->min_port;
if (current_port == start_port && res != HOST_CANDIDATE_DUPLICATE_PORT)
break;
if (current_port == 0 && res != HOST_CANDIDATE_DUPLICATE_PORT)
break;
}
......@@ -3196,7 +3200,7 @@ nice_agent_gather_candidates (
agent);
continue;
} else if (res == HOST_CANDIDATE_FAILED) {
nice_debug ("Agent %p: Could ot retrieive component %d/%d", agent,
nice_debug ("Agent %p: Could not retrieve component %d/%d", agent,
stream->id, cid);
continue;
} else if (res == HOST_CANDIDATE_CANT_CREATE_SOCKET) {
......@@ -3208,6 +3212,10 @@ nice_agent_gather_candidates (
component->id);
}
continue;
} else if (res == HOST_CANDIDATE_DUPLICATE_PORT) {
nice_debug ("Agent %p: Ignoring local candidate, duplicate port",
agent);
continue;
}
found_local_address = TRUE;
......
......@@ -603,6 +603,28 @@ void priv_generate_candidate_credentials (NiceAgent *agent,
}
static gboolean
priv_local_host_candidate_duplicate_port (NiceComponent *component,
NiceCandidate *candidate)
{
GSList *i;
if (candidate->transport == NICE_CANDIDATE_TRANSPORT_TCP_ACTIVE)
return FALSE;
for (i = component->local_candidates; i; i = i->next) {
NiceCandidate *c = i->data;
if (candidate->transport == c->transport &&
nice_address_ip_version (&candidate->addr) ==
nice_address_ip_version (&c->addr) &&
nice_address_get_port (&candidate->addr) ==
nice_address_get_port (&c->addr))
return TRUE;
}
return FALSE;
}
/*
* Creates a local host candidate for 'component_id' of stream
* 'stream_id'.
......@@ -668,6 +690,11 @@ HostCandidateResult discovery_add_local_host_candidate (
candidate->addr = nicesock->addr;
candidate->base_addr = nicesock->addr;
if (priv_local_host_candidate_duplicate_port (component, candidate)) {
res = HOST_CANDIDATE_DUPLICATE_PORT;
goto errors;
}
if (!priv_add_local_candidate_pruned (agent, stream_id, component,
candidate)) {
res = HOST_CANDIDATE_REDUNDANT;
......
......@@ -103,7 +103,8 @@ typedef enum {
HOST_CANDIDATE_SUCCESS,
HOST_CANDIDATE_FAILED,
HOST_CANDIDATE_CANT_CREATE_SOCKET,
HOST_CANDIDATE_REDUNDANT
HOST_CANDIDATE_REDUNDANT,
HOST_CANDIDATE_DUPLICATE_PORT
} HostCandidateResult;
HostCandidateResult
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment