Add support for aggressive nomination in controlling agent

Author: mickel8

lib/ex_ice/ice_agent.ex

@@ -61,6 +61,12 @@ defmodule ExICE.ICEAgent do
   * `ice_transport_policy` - candidate types to be used.
     * `all` - all ICE candidates will be considered (default).
     * `relay` - only relay candidates will be considered.
+  * `aggressive_nomination` - whether to use aggressive nomination from RFC 5245.
+  ExICE aims to implement RFC 8445, which removes aggressive nomination.
+  In particular, RFC 8445 assumes that data can be sent on any valid pair (no need for nomination).
+  While this behavior is supported by most of the implementations, some of them still require
+  a pair to be nominated by the controlling agent before they start sending data.
+  Defaults to true.
   * `on_gathering_state_change` - where to send gathering state change notifications. Defaults to a process that spawns `ExICE`.
   * `on_connection_state_change` - where to send connection state change notifications. Defaults to a process that spawns `ExICE`.
   * `on_data` - where to send data. Defaults to a process that spawns `ExICE`.
@@ -78,6 +84,7 @@ defmodule ExICE.ICEAgent do
             }
           ],
           ice_transport_policy: :all | :relay,
+          aggressive_nomination: boolean(),
           on_gathering_state_change: pid() | nil,
           on_connection_state_change: pid() | nil,
           on_data: pid() | nil,

lib/ex_ice/priv/conn_check_handler/controlled.ex

@@ -203,11 +203,12 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlled do
 
           %ICEAgent{ice_agent | selected_pair_id: pair.id}
         else
+          Logger.debug("Not selecting a new pair as it has lower priority.")
           ice_agent
         end
 
       true ->
-        Logger.debug("Not selecting a new pair as it has lower priority or has the same id")
+        Logger.debug("Not selecting a new pair as it has the same id")
         ice_agent
     end
   end

lib/ex_ice/priv/conn_check_handler/controlling.ex

@@ -97,7 +97,12 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlling do
   def update_nominated_flag(ice_agent, _pair_id, false), do: ice_agent
 
   @impl true
-  def update_nominated_flag(%ICEAgent{eoc: true} = ice_agent, pair_id, true) do
+  def update_nominated_flag(
+        %ICEAgent{eoc: eoc, aggressive_nomination: aggressive_nomination} = ice_agent,
+        pair_id,
+        true
+      )
+      when (aggressive_nomination == false and eoc == true) or aggressive_nomination == true do
     Logger.debug("Nomination succeeded. Selecting pair: #{inspect(pair_id)}")
 
     pair = Map.fetch!(ice_agent.checklist, pair_id)
@@ -106,12 +111,11 @@ defmodule ExICE.Priv.ConnCheckHandler.Controlling do
 
     # the controlling agent could nominate only when eoc was set
     # and checklist finished
-    unless Checklist.finished?(ice_agent.checklist) do
+    if not ice_agent.aggressive_nomination and not Checklist.finished?(ice_agent.checklist) do
       Logger.warning("Nomination succeeded but checklist hasn't finished.")
     end
 
-    ice_agent = %ICEAgent{ice_agent | nominating?: {false, nil}, selected_pair_id: pair.id}
-    ICEAgent.change_connection_state(ice_agent, :completed)
+    %ICEAgent{ice_agent | nominating?: {false, nil}, selected_pair_id: pair.id}
   end
 
   defp resolve_pair(ice_agent, pair) do

lib/ex_ice/priv/ice_agent.ex

@@ -59,6 +59,7 @@ defmodule ExICE.Priv.ICEAgent do
     :on_gathering_state_change,
     :on_data,
     :on_new_candidate,
+    :aggressive_nomination,
     :if_discovery_module,
     :transport_module,
     :gatherer,
@@ -149,6 +150,7 @@ defmodule ExICE.Priv.ICEAgent do
       on_gathering_state_change: opts[:on_gathering_state_change] || controlling_process,
       on_data: opts[:on_data] || controlling_process,
       on_new_candidate: opts[:on_new_candidate] || controlling_process,
+      aggressive_nomination: Keyword.get(opts, :aggressive_nomination, true),
       if_discovery_module: if_discovery_module,
       transport_module: transport_module,
       gatherer: Gatherer.new(if_discovery_module, transport_module, ip_filter, ports),
@@ -430,6 +432,13 @@ defmodule ExICE.Priv.ICEAgent do
     update_connection_state(ice_agent)
   end
 
+  def end_of_candidates(%__MODULE__{role: :controlling, aggressive_nomination: true} = ice_agent) do
+    Logger.debug("Setting end-of-candidates flag")
+    ice_agent = %{ice_agent | eoc: true}
+    # we might need to move to the completed state
+    update_connection_state(ice_agent)
+  end
+
   def end_of_candidates(%__MODULE__{role: :controlling} = ice_agent) do
     Logger.debug("Setting end-of-candidates flag.")
     ice_agent = %{ice_agent | eoc: true}
@@ -2189,7 +2198,11 @@ defmodule ExICE.Priv.ICEAgent do
     end
   end
 
-  defp time_to_nominate?(%__MODULE__{state: :connected} = ice_agent) do
+  # In aggressive nomination, there is no additional connectivity check.
+  # Instead, every connectivity check includes UseCandidate flag.
+  defp time_to_nominate?(%__MODULE__{aggressive_nomination: true}), do: false
+
+  defp time_to_nominate?(%__MODULE__{aggressive_nomination: false, state: :connected} = ice_agent) do
     {nominating?, _} = ice_agent.nominating?
     # if we are not during nomination and we know there won't be further candidates,
     # there are no checks waiting or in-progress,
@@ -2515,8 +2528,19 @@ defmodule ExICE.Priv.ICEAgent do
     end
   end
 
+  # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity
   defp update_connection_state(%__MODULE__{state: :checking} = ice_agent) do
     cond do
+      # in aggressive nomination, we might move directly from checking to completed
+      ice_agent.selected_pair_id != nil and ice_agent.eoc == true and
+        ice_agent.gathering_state == :complete and Checklist.finished?(ice_agent.checklist) ->
+        Logger.debug("""
+        Found a valid pair, there won't be any further local or remote candidates. \
+        Changing connection state to complete.\
+        """)
+
+        change_connection_state(ice_agent, :completed)
+
       Checklist.get_valid_pair(ice_agent.checklist) != nil ->
         Logger.debug("Found a valid pair. Changing connection state to connected")
         change_connection_state(ice_agent, :connected)
@@ -2550,7 +2574,7 @@ defmodule ExICE.Priv.ICEAgent do
           Checklist.finished?(ice_agent.checklist) ->
         change_connection_state(ice_agent, :failed)
 
-      # Assuming the controlling side uses regulard nomination,
+      # Assuming the controlling side uses regular nomination,
       # the controlled side could move to the completed
       # state as soon as it receives nomination request (or after
       # successful triggered check caused by nomination request).
@@ -2570,9 +2594,12 @@ defmodule ExICE.Priv.ICEAgent do
 
         change_connection_state(ice_agent, :completed)
 
-      ice_agent.role == :controlling and ice_agent.selected_pair_id != nil ->
+      ice_agent.role == :controlling and ice_agent.selected_pair_id != nil and
+        ice_agent.eoc == true and ice_agent.gathering_state == :complete and
+          Checklist.finished?(ice_agent.checklist) ->
         Logger.debug("""
-        We have selected pair and we are the controlling agent. Changing state to completed.\
+        Finished all conn checks, there won't be any further local or remote candidates
+        and we have selected pair. Changing connection state to completed.\
         """)
 
         change_connection_state(ice_agent, :completed)
@@ -2727,7 +2754,14 @@ defmodule ExICE.Priv.ICEAgent do
 
     # we can nominate only when being the controlling agent
     # the controlled agent uses nominate? flag according to 7.3.1.5
-    nominate = pair.nominate? and ice_agent.role == :controlling
+    nominate =
+      ice_agent.role == :controlling and (pair.nominate? or ice_agent.aggressive_nomination)
+
+    # set nominate? flag in case we are running aggressive nomination
+    # but don't override it if we are controlled agent and it was already set to true
+    pair = %CandidatePair{pair | nominate?: pair.nominate? || nominate}
+    ice_agent = put_in(ice_agent.checklist[pair.id], pair)
+
     req = binding_request(ice_agent, nominate)
 
     raw_req = Message.encode(req)