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.

diff --git a/agent/agent.c b/agent/agent.c
index 4a2a0d8..83a4d5e 100644 (file)
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -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;

diff --git a/agent/discovery.c b/agent/discovery.c
index 44e82d7..55b5e8c 100644 (file)
--- a/agent/discovery.c
+++ b/agent/discovery.c
@@ -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;

diff --git a/agent/discovery.h b/agent/discovery.h
index 3d71930..3ef99b2 100644 (file)
--- a/agent/discovery.h
+++ b/agent/discovery.h
@@ -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