moar

Author: sgfn

lib/ex_ice/candidate.ex

@@ -107,6 +107,7 @@ defmodule ExICE.Candidate do
   def family(%__MODULE__{address: {_, _, _, _}}), do: :ipv4
   def family(%__MODULE__{address: {_, _, _, _, _, _, _, _}}), do: :ipv6
 
+  @spec tcp_type(t()) :: tcp_type() | nil
   def tcp_type(%__MODULE__{tcp_type: tt}), do: tt
 
   @doc false

lib/ex_ice/ice_agent.ex

@@ -71,7 +71,12 @@ defmodule ExICE.ICEAgent do
     until role is set, adding remote candidates or gathering local candidates won't possible, and calls to these
     functions will be ignored. Defaults to `nil`.
   * `ip_filter` - filter applied when gathering host candidates
-  * `ports` - ports that will be used when gathering host candidates, otherwise the ports are chosen by the OS
+  * `transport` - transport protocol to be used.
+    * `udp` - use UDP only (default).
+    * `tcp`- use TCP only. Note that relay candidates for TCP transport are not yet supported.
+  * `ports` - ports that will be used when gathering host candidates, otherwise the ports are chosen by the OS.
+    Warning: when using `transport: :tcp` and setting this option, make sure that this port range is used only by ICE Agents
+    from this Erlang VM instance. Refer to the source of `ExICE.Priv.Transport.TCP` for more info.
   * `ice_servers` - list of STUN/TURN servers
   * `ice_transport_policy` - candidate types to be used.
     * `all` - all ICE candidates will be considered (default).
@@ -90,13 +95,11 @@ defmodule ExICE.ICEAgent do
   * `logger_metadata` - a keyword list of metadata to be attached to the Logger for all logs emitted by the ICEAgent process.
   This function takes host's ip as an argument and should return srflx's ip as a result or nil if for given host candidate
   there should be no srflx one.
-  * `transport` - transport protocol to be used.
-    * `udp` - use UDP only (default).
-    * `tcp`- use TCP only. Note that relay candidates for TCP transport are not yet supported.
   """
   @type opts() :: [
           role: role() | nil,
           ip_filter: ip_filter(),
+          transport: :udp | :tcp,
           ports: Enumerable.t(non_neg_integer()),
           ice_servers: [
             %{
@@ -112,8 +115,7 @@ defmodule ExICE.ICEAgent do
           on_data: pid() | nil,
           on_new_candidate: pid() | nil,
           host_to_srflx_ip_mapper: host_to_srflx_ip_mapper() | nil,
-          logger_metadata: Enumerable.t({atom(), term()}),
-          transport: :udp | :tcp
+          logger_metadata: Enumerable.t({atom(), term()})
         ]
 
   @doc """

lib/ex_ice/priv/app.ex

@@ -11,10 +11,10 @@ defmodule ExICE.Priv.App do
     children =
       [{Registry, keys: :unique, name: ExICE.Priv.Registry}] ++
         if kernel_ver >= {9, 1} do
-          [{ExICE.Priv.MDNS.Resolver, :gen_udp}]
+          [{ExICE.Priv.MDNS.Resolver, ExICE.Priv.Transport.UDP}]
         else
           Logger.warning("""
-          Not starting MDNS resolver as it requires kernel version >= 9.1.
+          Not starting mDNS resolver as it requires kernel version >= 9.1.
           Detected kernel version: #{inspect(kernel_ver)}
           """)
 

lib/ex_ice/priv/ice_agent.ex

@@ -376,7 +376,7 @@ defmodule ExICE.Priv.ICEAgent do
 
     %{
       ice_agent
-      | sockets: sockets,
+      | sockets: Enum.map(sockets, fn %{socket: socket} -> socket end),
         gathering_transactions: relay_gathering_transactions
     }
     |> update_gathering_state()
@@ -2685,7 +2685,7 @@ defmodule ExICE.Priv.ICEAgent do
   end
 
   defp change_gathering_state(ice_agent, new_gathering_state, opts \\ []) do
-    Logger.debug("Gatering state change: #{ice_agent.gathering_state} -> #{new_gathering_state}")
+    Logger.debug("Gathering state change: #{ice_agent.gathering_state} -> #{new_gathering_state}")
 
     if opts[:notify] != false do
       notify(ice_agent.on_gathering_state_change, {:gathering_state_change, new_gathering_state})

lib/ex_ice/priv/mdns/resolver.ex

@@ -12,7 +12,7 @@ defmodule ExICE.Priv.MDNS.Resolver do
   @rtx_timeout_ms 500
 
   @spec start_link(module()) :: GenServer.on_start()
-  def start_link(transport_module \\ :gen_udp) do
+  def start_link(transport_module \\ ExICE.Priv.Transport.UDP) do
     GenServer.start_link(__MODULE__, transport_module, name: __MODULE__)
   end
 
@@ -28,39 +28,44 @@ defmodule ExICE.Priv.MDNS.Resolver do
 
   @impl true
   def init(transport_module) do
-    Logger.debug("Starting MDNS Resolver")
+    Logger.debug("Starting mDNS Resolver")
     {:ok, %{transport_module: transport_module, cache: %{}}, {:continue, nil}}
   end
 
   @impl true
   def handle_continue(_, state) do
     ret =
-      state.transport_module.open(
-        # Listen on the port specific to mDNS traffic.
-        # `add_membership` option only defines an address.
-        @mdns_port,
-        mode: :binary,
-        reuseaddr: true,
-        active: true,
-        # Allow other apps to bind to @mdns_port.
-        # If there are multiple sockets, bound to the same port,
-        # and subscribed to the same group (in fact, if one socket
-        # subscribes to some group, all other sockets bound to
-        # the same port also join this group), all those sockets
-        # will receive every message. In other words, `reuseport` for
-        # multicast works differently than for casual sockets.
-        reuseport: true,
-        # Support running two ICE agents on a single machine.
-        # In other case, our request won't be delivered to the mDNS address owner
-        # running on the same machine (e.g., a web browser).
-        multicast_loop: true,
-        # Receive responses - they are sent to the multicast address.
-        # The second argument specifies interfaces where we should listen
-        # for multicast traffic.
-        # This option works on interfaces i.e. it affects all sockets
-        # bound to the same port.
-        add_membership: {{224, 0, 0, 251}, {0, 0, 0, 0}}
-      )
+      if state.transport_module.transport() == :udp do
+        state.transport_module.setup_socket(
+          nil,
+          # Listen on the port specific to mDNS traffic.
+          # `add_membership` option only defines an address.
+          @mdns_port,
+          mode: :binary,
+          reuseaddr: true,
+          active: true,
+          # Allow other apps to bind to @mdns_port.
+          # If there are multiple sockets, bound to the same port,
+          # and subscribed to the same group (in fact, if one socket
+          # subscribes to some group, all other sockets bound to
+          # the same port also join this group), all those sockets
+          # will receive every message. In other words, `reuseport` for
+          # multicast works differently than for casual sockets.
+          reuseport: true,
+          # Support running two ICE agents on a single machine.
+          # In other case, our request won't be delivered to the mDNS address owner
+          # running on the same machine (e.g., a web browser).
+          multicast_loop: true,
+          # Receive responses - they are sent to the multicast address.
+          # The second argument specifies interfaces where we should listen
+          # for multicast traffic.
+          # This option works on interfaces i.e. it affects all sockets
+          # bound to the same port.
+          add_membership: {{224, 0, 0, 251}, {0, 0, 0, 0}}
+        )
+      else
+        {:error, :using_tcp_transport}
+      end
 
     case ret do
       {:ok, socket} ->
@@ -69,7 +74,7 @@ defmodule ExICE.Priv.MDNS.Resolver do
 
       {:error, reason} ->
         Logger.warning("""
-        Couldn't start MDNS resolver, reason: #{reason}. MDNS candidates won't be resolved.
+        Couldn't start mDNS resolver, reason: #{reason}. mDNS candidates won't be resolved.
         """)
 
         {:stop, {:shutdown, reason}, state}

lib/ex_ice/priv/transport/tcp.ex

@@ -22,7 +22,7 @@ defmodule ExICE.Priv.Transport.TCP do
   def setup_socket(ip, port, socket_opts, tp_opts) do
     case Registry.lookup(ExICE.Priv.Registry, {ip, port}) do
       # This protects us from reusing ports ONLY within the same VM instance
-      # See `TCP.Client` for more info
+      # See `ExICE.Priv.Transport.TCP.Client.setup_socket/5` for more info
       [{_pid, _}] ->
         # TODO: Consider using another (custom) reason to distinguish from POSIX EADDRINUSE
         {:error, :eaddrinuse}

lib/ex_ice/priv/transport/tcp_client.ex

@@ -2,26 +2,44 @@ defmodule ExICE.Priv.Transport.TCP.Client do
   @moduledoc false
 
   use GenServer
+
   require Logger
 
+  alias ExICE.Priv.Transport
+
   @connect_timeout_ms 500
 
   @spec start_link(Keyword.t()) :: GenServer.on_start()
   def start_link(opts \\ []) do
     GenServer.start_link(__MODULE__, opts ++ [ice_agent: self()])
   end
 
+  @spec setup_socket(
+          GenServer.server(),
+          :inet.ip_address(),
+          :inet.port_number(),
+          [Transport.open_option()],
+          Transport.transport_options()
+        ) :: {:ok, Transport.socket()} | {:error, term()}
   def setup_socket(pid, ip, port, socket_opts, tp_opts) do
     GenServer.call(pid, {:setup_socket, ip, port, socket_opts, tp_opts})
   end
 
   # HACK: using listen sockets here is ugly, but was easier to fit into the existing ICE Agent implementation.
   #       This should be changed, especially because we're going to want to close the listen sockets
   #       after the connection is successfully established.
+  @spec send(
+          GenServer.server(),
+          Transport.socket(),
+          {:inet.ip_address(), :inet.port_number()},
+          binary(),
+          Transport.transport_options()
+        ) :: :ok | {:error, term()}
   def send(pid, listen_socket, dest, packet, tp_opts \\ []) do
     GenServer.call(pid, {:send, listen_socket, dest, packet, tp_opts})
   end
 
+  @spec close(GenServer.server(), Transport.socket()) :: :ok
   def close(pid, listen_socket) do
     GenServer.call(pid, {:close, listen_socket})
   end
@@ -55,8 +73,6 @@ defmodule ExICE.Priv.Transport.TCP.Client do
     # * This, however, does not protect us from binding to sockets opened with SO_REUSEPORT,
     #   that are in use by another OS process with the same effective UID as this one,
     #   which can happen i.e. when we run ICE Agents in two separate VM instances.
-    #
-    # TODO: warn about this in the documentation of ICE Agent's `:ports` option
     socket_opts = socket_opts ++ [ip: ip, reuseport: true]
     tcp_type = Keyword.fetch!(tp_opts, :tcp_type)
 
@@ -71,11 +87,12 @@ defmodule ExICE.Priv.Transport.TCP.Client do
         end
 
         state =
-          %{state |
-            listen_socket: listen_socket,
-            socket_opts: socket_opts,
-            tcp_type: tcp_type,
-            connections: %{}
+          %{
+            state
+            | listen_socket: listen_socket,
+              socket_opts: socket_opts,
+              tcp_type: tcp_type,
+              connections: %{}
           }
 
         {:ok, _} = Registry.register(ExICE.Priv.Registry, local, self())
@@ -195,7 +212,12 @@ defmodule ExICE.Priv.Transport.TCP.Client do
     {remote_ip, remote_port} = remote
 
     # TODO: determine how big of a timeout we should use here
-    case :gen_tcp.connect(remote_ip, remote_port, state.socket_opts ++ [port: local_port], @connect_timeout_ms) do
+    case :gen_tcp.connect(
+           remote_ip,
+           remote_port,
+           state.socket_opts ++ [port: local_port],
+           @connect_timeout_ms
+         ) do
       {:ok, socket} ->
         Logger.debug("""
         Successfully initiated new connection.

lib/ex_ice/priv/transport/udp.ex

@@ -11,7 +11,8 @@ defmodule ExICE.Priv.Transport.UDP do
 
   @impl true
   def setup_socket(ip, port, socket_opts, _tp_opts \\ []) do
-    :gen_udp.open(port, socket_opts ++ [ip: ip])
+    ip_opt = if ip, do: [ip: ip], else: []
+    :gen_udp.open(port, socket_opts ++ ip_opt)
   end
 
   @impl true