ms-ice: ensure distinct candidate priority for multihomed hosts

Summary:
Offering multiple host candidates with equal priorities could lead to unpredictable candidate pair selection by our counterparty.

Fixes call disconnection by MS Lync client after 30 seconds while VPN (2nd IP) was active on libnice host.

Maniphest Tasks: T3324

Reviewers: pwithnall

Projects: #libnice

Reviewed By: pwithnall

Subscribers: pwithnall

Differential Revision: https://phabricator.freedesktop.org/D234

3 files changed, 57 insertions, 17 deletions

diff --git a/agent/candidate.c b/agent/candidate.c
index a472c4a..0277a79 100644
--- a/agent/candidate.c
+++ b/agent/candidate.c
@@ -52,6 +52,7 @@
 
 #include "agent.h"
 #include "component.h"
+#include "interfaces.h"
 
 G_DEFINE_BOXED_TYPE (NiceCandidate, nice_candidate, nice_candidate_copy,
     nice_candidate_free);
@@ -144,6 +145,43 @@ nice_candidate_ice_local_preference_full (guint direction_preference,
       other_preference);
 }
 
+static guint8
+nice_candidate_ip_local_preference (const NiceCandidate *candidate)
+{
+  guint8 preference = 0;
+  gchar ip_string[INET6_ADDRSTRLEN];
+  GList/*<owned gchar*>*/ *ips = NULL;
+  GList/*<unowned gchar*>*/ *iter;
+
+  /* Ensure otherwise identical host candidates with only different IP addresses
+   * (multihomed host) get assigned different priorities. Position of the IP in
+   * the list obtained from nice_interfaces_get_local_ips() serves here as the
+   * distinguishing value of other_preference. Reflexive and relayed candidates
+   * are likewise differentiated by their base address.
+   *
+   * This is required by RFC 5245 Section 4.1.2.1:
+   *   https://tools.ietf.org/html/rfc5245#section-4.1.2.1
+   */
+  if (candidate->type == NICE_CANDIDATE_TYPE_HOST) {
+    nice_address_to_string (&candidate->addr, ip_string);
+  } else {
+    nice_address_to_string (&candidate->base_addr, ip_string);
+  }
+
+  ips = nice_interfaces_get_local_ips (TRUE);
+
+  for (iter = ips; iter; iter = g_list_next (iter)) {
+    if (g_strcmp0 (ip_string, iter->data) == 0) {
+      break;
+    }
+    ++preference;
+  }
+
+  g_list_free_full (ips, g_free);
+
+  return preference;
+}
+
 static guint16
 nice_candidate_ice_local_preference (const NiceCandidate *candidate)
 {
@@ -178,7 +216,8 @@ nice_candidate_ice_local_preference (const NiceCandidate *candidate)
         break;
     }
 
-  return nice_candidate_ice_local_preference_full (direction_preference, 1);
+  return nice_candidate_ice_local_preference_full (direction_preference,
+      nice_candidate_ip_local_preference (candidate));
 }
 
 static guint32
@@ -214,7 +253,7 @@ nice_candidate_ms_ice_local_preference (const NiceCandidate *candidate)
     }
 
   return nice_candidate_ms_ice_local_preference_full(transport_preference,
-      direction_preference, 0);
+      direction_preference, nice_candidate_ip_local_preference (candidate));
 }
 
 static guint8
diff --git a/agent/conncheck.c b/agent/conncheck.c
index 33491bf..1b687b6 100644
--- a/agent/conncheck.c
+++ b/agent/conncheck.c
@@ -579,6 +579,7 @@ static guint32 peer_reflexive_candidate_priority (NiceAgent *agent,
 
   candidate_priority->transport = local_candidate->transport;
   candidate_priority->component_id = local_candidate->component_id;
+  candidate_priority->base_addr = local_candidate->addr;
   if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE) {
     priority = nice_candidate_jingle_priority (candidate_priority);
   } else if (agent->compatibility == NICE_COMPATIBILITY_MSN ||
diff --git a/agent/discovery.c b/agent/discovery.c
index f3a702d..882a174 100644
--- a/agent/discovery.c
+++ b/agent/discovery.c
@@ -623,6 +623,10 @@ discovery_add_server_reflexive_candidate (
   candidate->component_id = component_id;
   candidate->addr = *address;
 
+  /* step: link to the base candidate+socket */
+  candidate->sockptr = base_socket;
+  candidate->base_addr = base_socket->addr;
+
   if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE) {
     candidate->priority = nice_candidate_jingle_priority (candidate);
   } else if (agent->compatibility == NICE_COMPATIBILITY_MSN ||
@@ -636,10 +640,6 @@ discovery_add_server_reflexive_candidate (
         agent->reliable, nat_assisted);
   }
 
-  /* step: link to the base candidate+socket */
-  candidate->sockptr = base_socket;
-  candidate->base_addr = base_socket->addr;
-
   priv_generate_candidate_credentials (agent, candidate);
   priv_assign_foundation (agent, candidate);
 
@@ -732,6 +732,17 @@ discovery_add_relay_candidate (
   candidate->addr = *address;
   candidate->turn = turn_server_ref (turn);
 
+  /* step: link to the base candidate+socket */
+  relay_socket = nice_udp_turn_socket_new (agent->main_context, address,
+      base_socket, &turn->server,
+      turn->username, turn->password,
+      agent_to_turn_socket_compatibility (agent));
+  if (!relay_socket)
+    goto errors;
+
+  candidate->sockptr = relay_socket;
+  candidate->base_addr = base_socket->addr;
+
   if (agent->compatibility == NICE_COMPATIBILITY_GOOGLE) {
     candidate->priority = nice_candidate_jingle_priority (candidate);
   } else if (agent->compatibility == NICE_COMPATIBILITY_MSN ||
@@ -745,17 +756,6 @@ discovery_add_relay_candidate (
         agent->reliable, FALSE);
   }
 
-  /* step: link to the base candidate+socket */
-  relay_socket = nice_udp_turn_socket_new (agent->main_context, address,
-      base_socket, &turn->server,
-      turn->username, turn->password,
-      agent_to_turn_socket_compatibility (agent));
-  if (!relay_socket)
-    goto errors;
-
-  candidate->sockptr = relay_socket;
-  candidate->base_addr = base_socket->addr;
-
   priv_generate_candidate_credentials (agent, candidate);
 
   /* Google uses the turn username as the candidate username */