From 8248126c19847afc8ec4271fa36fecbc0c69cca9 Mon Sep 17 00:00:00 2001 From: Karol Konkol Date: Wed, 23 Jul 2025 14:40:30 +0200 Subject: [PATCH 1/3] Allow for defining public ips for 1:1 NAT --- lib/ex_ice/ice_agent.ex | 26 +++++++- lib/ex_ice/priv/ice_agent.ex | 22 ++++++- lib/ex_ice/priv/nat_mapper.ex | 73 ++++++++++++++++++++ test/priv/ice_agent_test.exs | 121 ++++++++++++++++++++++++++++++++++ 4 files changed, 238 insertions(+), 4 deletions(-) create mode 100644 lib/ex_ice/priv/nat_mapper.ex diff --git a/lib/ex_ice/ice_agent.ex b/lib/ex_ice/ice_agent.ex index 1f8fb09..2254319 100644 --- a/lib/ex_ice/ice_agent.ex +++ b/lib/ex_ice/ice_agent.ex @@ -47,6 +47,19 @@ defmodule ExICE.ICEAgent do """ @type ip_filter() :: (:inet.ip_address() -> boolean) + @typedoc """ + Mapping function used instead of STUN server. Maps local ip addresses into public ones. + These public addresses are then used to create server reflexive candidates. + + Note that each returned IP address must be unique. + If the mapping function repeatedly returns the same address, + it will be ignored, and only one server reflexive candidate will be created. + + This function is meant to be used for server implementations where the public addresses are well known + and NAT use 1 to 1 port mapping. + """ + @type map_to_nat_ip() :: (:inet.ip_address() -> :inet.ip_address() | nil) + @typedoc """ ICE Agent configuration options. All notifications are by default sent to a process that spawns `ExICE`. @@ -71,6 +84,16 @@ defmodule ExICE.ICEAgent do * `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`. * `on_new_candidate` - where to send new candidates. Defaults to a process that spawns `ExICE`. + * `map_to_nat_ip` - Mapping function used instead of STUN server. Maps + local ip addresses into public ones. + These public addresses are then used to create server reflexive candidates. + + Note that each returned IP address must be unique. + If the mapping function repeatedly returns the same address, + it will be ignored, and only one server reflexive candidate will be created. + + This function is meant to be used for server implementations where the public addresses are well known + and NAT use 1 to 1 port mapping. """ @type opts() :: [ role: role() | nil, @@ -88,7 +111,8 @@ defmodule ExICE.ICEAgent do on_gathering_state_change: pid() | nil, on_connection_state_change: pid() | nil, on_data: pid() | nil, - on_new_candidate: pid() | nil + on_new_candidate: pid() | nil, + map_to_nat_ip: map_to_nat_ip() | nil ] @doc """ diff --git a/lib/ex_ice/priv/ice_agent.ex b/lib/ex_ice/priv/ice_agent.ex index 616e744..0441cae 100644 --- a/lib/ex_ice/priv/ice_agent.ex +++ b/lib/ex_ice/priv/ice_agent.ex @@ -10,6 +10,7 @@ defmodule ExICE.Priv.ICEAgent do ConnCheckHandler, Gatherer, IfDiscovery, + NATMapper, Transport, Utils } @@ -98,7 +99,8 @@ defmodule ExICE.Priv.ICEAgent do selected_candidate_pair_changes: 0, # binding requests that failed to pass checks required to assign them to specific candidate pair # e.g. missing required attributes, role conflict, authentication, etc. - unmatched_requests: 0 + unmatched_requests: 0, + map_to_nat_ip: nil ] @spec unmarshal_remote_candidate(String.t()) :: {:ok, Candidate.t()} | {:error, term()} @@ -165,7 +167,8 @@ defmodule ExICE.Priv.ICEAgent do local_ufrag: local_ufrag, local_pwd: local_pwd, stun_servers: stun_servers, - turn_servers: turn_servers + turn_servers: turn_servers, + map_to_nat_ip: opts[:map_to_nat_ip] } end @@ -324,12 +327,25 @@ defmodule ExICE.Priv.ICEAgent do ice_agent = %__MODULE__{ice_agent | local_preferences: local_preferences} + srflx_cands = + NATMapper.create_srflx_candidates( + host_cands, + ice_agent.map_to_nat_ip, + ice_agent.local_preferences + ) + ice_agent = Enum.reduce(host_cands, ice_agent, fn host_cand, ice_agent -> add_local_cand(ice_agent, host_cand) end) - for %cand_mod{} = cand <- host_cands do + ice_agent = + Enum.reduce(srflx_cands, ice_agent, fn cand, ice_agent -> + # don't pair reflexive candidate, it should be pruned anyway - see sec. 6.1.2.4 + put_in(ice_agent.local_cands[cand.base.id], cand) + end) + + for %cand_mod{} = cand <- host_cands ++ srflx_cands do notify(ice_agent.on_new_candidate, {:new_candidate, cand_mod.marshal(cand)}) end diff --git a/lib/ex_ice/priv/nat_mapper.ex b/lib/ex_ice/priv/nat_mapper.ex new file mode 100644 index 0000000..68c22d0 --- /dev/null +++ b/lib/ex_ice/priv/nat_mapper.ex @@ -0,0 +1,73 @@ +defmodule ExICE.Priv.NATMapper do + @moduledoc false + + require Logger + + alias ExICE.ICEAgent + alias ExICE.Priv.Candidate + + @spec create_srflx_candidates([Candidate.Host.t()], ICEAgent.map_to_nat_ip(), %{ + :inet.ip_address() => non_neg_integer() + }) :: [Candidate.Srflx.t()] + def create_srflx_candidates(_host_cands, nil, _local_preferences) do + [] + end + + def create_srflx_candidates(host_cands, map_to_nat_ip, local_preferences) do + {cands, _external_ips} = + Enum.reduce(host_cands, {[], []}, fn host_cand, {cands, external_ips} -> + external_ip = map_to_nat_ip.(host_cand.base.address) + + if valid_external_ip?(external_ip, host_cand.base.address, external_ips) do + priority = + Candidate.priority!(local_preferences, host_cand.base.address, :srflx) + + cand = + Candidate.Srflx.new( + address: external_ip, + port: host_cand.base.port, + base_address: host_cand.base.address, + base_port: host_cand.base.port, + priority: priority, + transport_module: host_cand.base.transport_module, + socket: host_cand.base.socket + ) + + Logger.debug("New srflx candidate from NAT mapping: #{inspect(cand)}") + + {[cand | cands], [external_ip | external_ips]} + else + {cands, external_ips} + end + end) + + cands + end + + defp valid_external_ip?(external_ip, host_ip, external_ips) do + same_type? = :inet.is_ipv4_address(external_ip) == :inet.is_ipv4_address(host_ip) + + cond do + host_ip == external_ip -> + log_warning(host_ip, external_ip, "external IP is the same as local IP") + false + + not :inet.is_ip_address(external_ip) or not same_type? -> + log_warning(host_ip, external_ip, "not valid IP address") + false + + external_ip in external_ips -> + log_warning(host_ip, external_ip, "address already in use") + false + + true -> + true + end + end + + defp log_warning(host_ip, external_ip, reason), + do: + Logger.warning( + "Ignoring NAT mapping: #{inspect(host_ip)} to #{inspect(external_ip)}, #{inspect(reason)}" + ) +end diff --git a/test/priv/ice_agent_test.exs b/test/priv/ice_agent_test.exs index 0b6cf05..d98d50c 100644 --- a/test/priv/ice_agent_test.exs +++ b/test/priv/ice_agent_test.exs @@ -2631,6 +2631,127 @@ defmodule ExICE.Priv.ICEAgentTest do end end + describe "NAT mapping" do + alias ExICE.Priv.Candidate + + @ipv4 {10, 10, 10, 10} + @ipv6 {0, 0, 0, 0, 0, 0, 0, 1} + @invalid_ip :invalid_ip + + test "adds srflx candidate" do + ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @ipv4 end) + + assert [%Candidate.Srflx{base: %{address: @ipv4}}] = srflx_candidates(ice_agent) + + assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} + assert_receive {:ex_ice, _pid, {:new_candidate, srflx_cand}} + + assert host_cand =~ "typ host" + assert srflx_cand =~ "typ srflx" + end + + test "creates only one candidate if external ip repeats itself" do + ice_agent = spawn_ice_agent(IfDiscovery.MockMulti, fn _ip -> @ipv4 end) + + assert [%Candidate.Srflx{base: %{address: @ipv4}}] = srflx_candidates(ice_agent) + + assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} + assert_receive {:ex_ice, _pid, {:new_candidate, host_cand_2}} + assert_receive {:ex_ice, _pid, {:new_candidate, srflx_cand}} + + assert host_cand =~ "typ host" + assert host_cand_2 =~ "typ host" + assert srflx_cand =~ "typ srflx" + end + + test "ignores one to one mapping" do + ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn ip -> ip end) + + assert [] == srflx_candidates(ice_agent) + + assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} + refute_receive {:ex_ice, _pid, {:new_candidate, _srflx_cand}} + + assert host_cand =~ "typ host" + end + + test "ignores if ip types is not the same" do + ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @ipv6 end) + + assert [] == srflx_candidates(ice_agent) + end + + test "ignores when function returns nil value" do + ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> nil end) + + assert [] == srflx_candidates(ice_agent) + end + + test "ignores when function returns invalid value" do + ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @invalid_ip end) + + assert [] == srflx_candidates(ice_agent) + end + + test "works with STUN enabled" do + ice_agent = + ICEAgent.new( + controlling_process: self(), + role: :controlled, + transport_module: Transport.Mock, + if_discovery_module: IfDiscovery.MockSingle, + ice_servers: [%{urls: "stun:192.168.0.3:19302"}], + map_to_nat_ip: fn _ip -> @ipv4 end + ) + |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") + |> ICEAgent.gather_candidates() + + [%Candidate.Srflx{base: %{address: @ipv4, port: srflx_port}}] = srflx_candidates(ice_agent) + + [socket] = ice_agent.sockets + + # assert no transactions are started until handle_ta_timeout is called + assert nil == Transport.Mock.recv(socket) + + # assert ice agent started gathering transaction by sending a binding request + ice_agent = ICEAgent.handle_ta_timeout(ice_agent) + assert packet = Transport.Mock.recv(socket) + assert {:ok, req} = ExSTUN.Message.decode(packet) + assert req.type.class == :request + assert req.type.method == :binding + + resp = + Message.new(req.transaction_id, %Type{class: :success_response, method: :binding}, [ + %XORMappedAddress{address: @ipv4, port: srflx_port} + ]) + |> Message.encode() + + ice_agent = ICEAgent.handle_udp(ice_agent, socket, @stun_ip, @stun_port, resp) + + # assert there isn't new srflx candidate + assert [%Candidate.Srflx{}] = srflx_candidates(ice_agent) + end + + defp spawn_ice_agent(discovery_module, map_to_nat_ip) do + %ICEAgent{gathering_state: :complete} = + ICEAgent.new( + controlling_process: self(), + role: :controlled, + transport_module: Transport.Mock, + if_discovery_module: discovery_module, + map_to_nat_ip: map_to_nat_ip + ) + |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") + |> ICEAgent.gather_candidates() + end + + defp srflx_candidates(ice_agent) do + ice_agent.local_cands + |> Map.values() + |> Enum.filter(&(&1.base.type == :srflx)) + end + end + test "relay ice_transport_policy" do ice_agent = ICEAgent.new( From bad6656b908541f9c63918f9f1bf598489caab6a Mon Sep 17 00:00:00 2001 From: Karol Konkol Date: Wed, 23 Jul 2025 18:15:06 +0200 Subject: [PATCH 2/3] Requested changes --- lib/ex_ice/ice_agent.ex | 27 ++++------ lib/ex_ice/priv/gatherer.ex | 99 +++++++++++++++++++++++++++++++++++ lib/ex_ice/priv/ice_agent.ex | 11 ++-- lib/ex_ice/priv/nat_mapper.ex | 73 -------------------------- test/priv/ice_agent_test.exs | 12 ++--- 5 files changed, 121 insertions(+), 101 deletions(-) delete mode 100644 lib/ex_ice/priv/nat_mapper.ex diff --git a/lib/ex_ice/ice_agent.ex b/lib/ex_ice/ice_agent.ex index 2254319..38338a5 100644 --- a/lib/ex_ice/ice_agent.ex +++ b/lib/ex_ice/ice_agent.ex @@ -48,17 +48,19 @@ defmodule ExICE.ICEAgent do @type ip_filter() :: (:inet.ip_address() -> boolean) @typedoc """ - Mapping function used instead of STUN server. Maps local ip addresses into public ones. - These public addresses are then used to create server reflexive candidates. + Function called for each host candidate to derive a new "fabricated" srflx candidate from it. + 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. Note that each returned IP address must be unique. If the mapping function repeatedly returns the same address, it will be ignored, and only one server reflexive candidate will be created. - This function is meant to be used for server implementations where the public addresses are well known - and NAT use 1 to 1 port mapping. + This function is meant to be used for server implementations where the public addresses are well known. + NAT uses 1 to 1 port mapping and using STUN server for discovering public IP is undesirable + (e.g. because of unknown response time). """ - @type map_to_nat_ip() :: (:inet.ip_address() -> :inet.ip_address() | nil) + @type host_to_srflx_ip_mapper() :: (:inet.ip_address() -> :inet.ip_address() | nil) @typedoc """ ICE Agent configuration options. @@ -84,16 +86,9 @@ defmodule ExICE.ICEAgent do * `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`. * `on_new_candidate` - where to send new candidates. Defaults to a process that spawns `ExICE`. - * `map_to_nat_ip` - Mapping function used instead of STUN server. Maps - local ip addresses into public ones. - These public addresses are then used to create server reflexive candidates. - - Note that each returned IP address must be unique. - If the mapping function repeatedly returns the same address, - it will be ignored, and only one server reflexive candidate will be created. - - This function is meant to be used for server implementations where the public addresses are well known - and NAT use 1 to 1 port mapping. + * `host_to_srflx_ip_mapper` - function called for each host candidate to derive a new "fabricated" srflx candidate from it. + 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. """ @type opts() :: [ role: role() | nil, @@ -112,7 +107,7 @@ defmodule ExICE.ICEAgent do on_connection_state_change: pid() | nil, on_data: pid() | nil, on_new_candidate: pid() | nil, - map_to_nat_ip: map_to_nat_ip() | nil + host_to_srflx_ip_mapper: host_to_srflx_ip_mapper() | nil ] @doc """ diff --git a/lib/ex_ice/priv/gatherer.ex b/lib/ex_ice/priv/gatherer.ex index fb84373..430f5ba 100644 --- a/lib/ex_ice/priv/gatherer.ex +++ b/lib/ex_ice/priv/gatherer.ex @@ -1,6 +1,7 @@ defmodule ExICE.Priv.Gatherer do @moduledoc false + alias ExICE.ICEAgent alias ExICE.Priv.{Candidate, Transport, Utils} alias ExSTUN.Message alias ExSTUN.Message.Type @@ -139,6 +140,104 @@ defmodule ExICE.Priv.Gatherer do end end + @spec fabricate_srflx_candidates([Candidate.Host.t()], ICEAgent.host_to_srflx_ip_mapper(), %{ + :inet.ip_address() => non_neg_integer() + }) :: [Candidate.Srflx.t()] + def fabricate_srflx_candidates(_host_cands, nil, _local_preferences) do + [] + end + + def fabricate_srflx_candidates(host_cands, host_to_srflx_ip_mapper, local_preferences) do + do_fabricate_srflx_candidates( + host_cands, + host_to_srflx_ip_mapper, + local_preferences, + [], + [] + ) + end + + defp do_fabricate_srflx_candidates( + [], + _host_to_srflx_ip_mapper, + _local_preferences, + srflx_cands, + _external_ips + ) do + srflx_cands + end + + defp do_fabricate_srflx_candidates( + [host_cand | rest], + host_to_srflx_ip_mapper, + local_preferences, + srflx_cands, + external_ips + ) do + external_ip = host_to_srflx_ip_mapper.(host_cand.base.address) + + if valid_external_ip?(external_ip, host_cand.base.address, external_ips) do + priority = + Candidate.priority!(local_preferences, host_cand.base.address, :srflx) + + cand = + Candidate.Srflx.new( + address: external_ip, + port: host_cand.base.port, + base_address: host_cand.base.address, + base_port: host_cand.base.port, + priority: priority, + transport_module: host_cand.base.transport_module, + socket: host_cand.base.socket + ) + + Logger.debug("New srflx candidate from NAT mapping: #{inspect(cand)}") + + do_fabricate_srflx_candidates( + rest, + host_to_srflx_ip_mapper, + local_preferences, + [cand | srflx_cands], + [external_ip | external_ips] + ) + else + do_fabricate_srflx_candidates( + rest, + host_to_srflx_ip_mapper, + local_preferences, + srflx_cands, + external_ips + ) + end + end + + defp valid_external_ip?(external_ip, host_ip, external_ips) do + same_type? = :inet.is_ipv4_address(external_ip) == :inet.is_ipv4_address(host_ip) + + cond do + host_ip == external_ip -> + log_warning(host_ip, external_ip, "external IP is the same as local IP") + false + + not :inet.is_ip_address(external_ip) or not same_type? -> + log_warning(host_ip, external_ip, "not valid IP address") + false + + external_ip in external_ips -> + log_warning(host_ip, external_ip, "address already in use") + false + + true -> + true + end + end + + defp log_warning(host_ip, external_ip, reason), + do: + Logger.warning( + "Ignoring NAT mapping: #{inspect(host_ip)} to #{inspect(external_ip)}, #{inspect(reason)}" + ) + defp loopback_if?({_int_name, int}) do :loopback in int[:flags] end diff --git a/lib/ex_ice/priv/ice_agent.ex b/lib/ex_ice/priv/ice_agent.ex index 0441cae..f854bd3 100644 --- a/lib/ex_ice/priv/ice_agent.ex +++ b/lib/ex_ice/priv/ice_agent.ex @@ -10,7 +10,6 @@ defmodule ExICE.Priv.ICEAgent do ConnCheckHandler, Gatherer, IfDiscovery, - NATMapper, Transport, Utils } @@ -91,6 +90,7 @@ defmodule ExICE.Priv.ICEAgent do stun_servers: [], turn_servers: [], resolved_turn_servers: [], + host_to_srflx_ip_mapper: nil, # stats bytes_sent: 0, bytes_received: 0, @@ -99,8 +99,7 @@ defmodule ExICE.Priv.ICEAgent do selected_candidate_pair_changes: 0, # binding requests that failed to pass checks required to assign them to specific candidate pair # e.g. missing required attributes, role conflict, authentication, etc. - unmatched_requests: 0, - map_to_nat_ip: nil + unmatched_requests: 0 ] @spec unmarshal_remote_candidate(String.t()) :: {:ok, Candidate.t()} | {:error, term()} @@ -168,7 +167,7 @@ defmodule ExICE.Priv.ICEAgent do local_pwd: local_pwd, stun_servers: stun_servers, turn_servers: turn_servers, - map_to_nat_ip: opts[:map_to_nat_ip] + host_to_srflx_ip_mapper: opts[:host_to_srflx_ip_mapper] } end @@ -328,9 +327,9 @@ defmodule ExICE.Priv.ICEAgent do ice_agent = %__MODULE__{ice_agent | local_preferences: local_preferences} srflx_cands = - NATMapper.create_srflx_candidates( + Gatherer.fabricate_srflx_candidates( host_cands, - ice_agent.map_to_nat_ip, + ice_agent.host_to_srflx_ip_mapper, ice_agent.local_preferences ) diff --git a/lib/ex_ice/priv/nat_mapper.ex b/lib/ex_ice/priv/nat_mapper.ex deleted file mode 100644 index 68c22d0..0000000 --- a/lib/ex_ice/priv/nat_mapper.ex +++ /dev/null @@ -1,73 +0,0 @@ -defmodule ExICE.Priv.NATMapper do - @moduledoc false - - require Logger - - alias ExICE.ICEAgent - alias ExICE.Priv.Candidate - - @spec create_srflx_candidates([Candidate.Host.t()], ICEAgent.map_to_nat_ip(), %{ - :inet.ip_address() => non_neg_integer() - }) :: [Candidate.Srflx.t()] - def create_srflx_candidates(_host_cands, nil, _local_preferences) do - [] - end - - def create_srflx_candidates(host_cands, map_to_nat_ip, local_preferences) do - {cands, _external_ips} = - Enum.reduce(host_cands, {[], []}, fn host_cand, {cands, external_ips} -> - external_ip = map_to_nat_ip.(host_cand.base.address) - - if valid_external_ip?(external_ip, host_cand.base.address, external_ips) do - priority = - Candidate.priority!(local_preferences, host_cand.base.address, :srflx) - - cand = - Candidate.Srflx.new( - address: external_ip, - port: host_cand.base.port, - base_address: host_cand.base.address, - base_port: host_cand.base.port, - priority: priority, - transport_module: host_cand.base.transport_module, - socket: host_cand.base.socket - ) - - Logger.debug("New srflx candidate from NAT mapping: #{inspect(cand)}") - - {[cand | cands], [external_ip | external_ips]} - else - {cands, external_ips} - end - end) - - cands - end - - defp valid_external_ip?(external_ip, host_ip, external_ips) do - same_type? = :inet.is_ipv4_address(external_ip) == :inet.is_ipv4_address(host_ip) - - cond do - host_ip == external_ip -> - log_warning(host_ip, external_ip, "external IP is the same as local IP") - false - - not :inet.is_ip_address(external_ip) or not same_type? -> - log_warning(host_ip, external_ip, "not valid IP address") - false - - external_ip in external_ips -> - log_warning(host_ip, external_ip, "address already in use") - false - - true -> - true - end - end - - defp log_warning(host_ip, external_ip, reason), - do: - Logger.warning( - "Ignoring NAT mapping: #{inspect(host_ip)} to #{inspect(external_ip)}, #{inspect(reason)}" - ) -end diff --git a/test/priv/ice_agent_test.exs b/test/priv/ice_agent_test.exs index d98d50c..b484599 100644 --- a/test/priv/ice_agent_test.exs +++ b/test/priv/ice_agent_test.exs @@ -2631,11 +2631,11 @@ defmodule ExICE.Priv.ICEAgentTest do end end - describe "NAT mapping" do + describe "host to prefabricated srflx mapper" do alias ExICE.Priv.Candidate @ipv4 {10, 10, 10, 10} - @ipv6 {0, 0, 0, 0, 0, 0, 0, 1} + @ipv6 {64_512, 0, 0, 0, 0, 0, 0, 1} @invalid_ip :invalid_ip test "adds srflx candidate" do @@ -2650,7 +2650,7 @@ defmodule ExICE.Priv.ICEAgentTest do assert srflx_cand =~ "typ srflx" end - test "creates only one candidate if external ip repeats itself" do + test "creates only one candidate if external ip is repeated" do ice_agent = spawn_ice_agent(IfDiscovery.MockMulti, fn _ip -> @ipv4 end) assert [%Candidate.Srflx{base: %{address: @ipv4}}] = srflx_candidates(ice_agent) @@ -2701,7 +2701,7 @@ defmodule ExICE.Priv.ICEAgentTest do transport_module: Transport.Mock, if_discovery_module: IfDiscovery.MockSingle, ice_servers: [%{urls: "stun:192.168.0.3:19302"}], - map_to_nat_ip: fn _ip -> @ipv4 end + host_to_srflx_ip_mapper: fn _ip -> @ipv4 end ) |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") |> ICEAgent.gather_candidates() @@ -2732,14 +2732,14 @@ defmodule ExICE.Priv.ICEAgentTest do assert [%Candidate.Srflx{}] = srflx_candidates(ice_agent) end - defp spawn_ice_agent(discovery_module, map_to_nat_ip) do + defp spawn_ice_agent(discovery_module, host_to_srflx_ip_mapper) do %ICEAgent{gathering_state: :complete} = ICEAgent.new( controlling_process: self(), role: :controlled, transport_module: Transport.Mock, if_discovery_module: discovery_module, - map_to_nat_ip: map_to_nat_ip + host_to_srflx_ip_mapper: host_to_srflx_ip_mapper ) |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") |> ICEAgent.gather_candidates() From 99fc27a6002d7fa0eb2754479b6640fdc47f264e Mon Sep 17 00:00:00 2001 From: Karol Konkol Date: Thu, 24 Jul 2025 12:44:37 +0200 Subject: [PATCH 3/3] Requested changes --- test/priv/gatherer_test.exs | 86 ++++++++++++++++++++++++++++++++++++ test/priv/ice_agent_test.exs | 70 +++++------------------------ 2 files changed, 97 insertions(+), 59 deletions(-) diff --git a/test/priv/gatherer_test.exs b/test/priv/gatherer_test.exs index c3e4c77..95162f1 100644 --- a/test/priv/gatherer_test.exs +++ b/test/priv/gatherer_test.exs @@ -100,4 +100,90 @@ defmodule ExICE.Priv.GathererTest do assert port in port_range end end + + describe "host to prefabricated srflx mapper" do + @ipv4 {10, 10, 10, 10} + @ipv6 {64_512, 0, 0, 0, 0, 0, 0, 1} + @invalid_ip :invalid_ip + + @ipv4_filter &:inet.is_ipv4_address(&1) + + test "adds srflx candidate" do + ip_filter = fn + {192, 168, 0, 2} -> false + _ -> true + end + + {local_preferences, host_cands} = setup_gatherer(ip_filter) + + assert [%Candidate.Srflx{base: %{address: @ipv4}}] = + Gatherer.fabricate_srflx_candidates( + host_cands, + fn _ip -> @ipv4 end, + local_preferences + ) + end + + test "creates only one candidate if external ip is repeated" do + {local_preferences, host_cands} = setup_gatherer(@ipv4_filter) + + assert [%Candidate.Srflx{base: %{address: @ipv4}}] = + Gatherer.fabricate_srflx_candidates( + host_cands, + fn _ip -> @ipv4 end, + local_preferences + ) + end + + test "ignores one to one mapping" do + {local_preferences, host_cands} = setup_gatherer(@ipv4_filter) + + assert [] == + Gatherer.fabricate_srflx_candidates( + host_cands, + fn ip -> ip end, + local_preferences + ) + end + + test "ignores if ip types is not the same" do + {local_preferences, host_cands} = setup_gatherer(@ipv4_filter) + + assert [] == + Gatherer.fabricate_srflx_candidates( + host_cands, + fn _ip -> @ipv6 end, + local_preferences + ) + end + + test "ignores when function returns nil value" do + {local_preferences, host_cands} = setup_gatherer(@ipv4_filter) + + assert [] == + Gatherer.fabricate_srflx_candidates( + host_cands, + fn _ip -> nil end, + local_preferences + ) + end + + test "ignores when function returns invalid value" do + {local_preferences, host_cands} = setup_gatherer(@ipv4_filter) + + assert [] == + Gatherer.fabricate_srflx_candidates( + host_cands, + fn _ip -> @invalid_ip end, + local_preferences + ) + end + + defp setup_gatherer(ip_filter) do + gatherer = Gatherer.new(IfDiscovery.Mock, Transport.Mock, ip_filter, [0]) + assert {:ok, sockets} = Gatherer.open_sockets(gatherer) + + Gatherer.gather_host_candidates(gatherer, %{}, sockets) + end + end end diff --git a/test/priv/ice_agent_test.exs b/test/priv/ice_agent_test.exs index b484599..356cceb 100644 --- a/test/priv/ice_agent_test.exs +++ b/test/priv/ice_agent_test.exs @@ -2635,64 +2635,29 @@ defmodule ExICE.Priv.ICEAgentTest do alias ExICE.Priv.Candidate @ipv4 {10, 10, 10, 10} - @ipv6 {64_512, 0, 0, 0, 0, 0, 0, 1} - @invalid_ip :invalid_ip test "adds srflx candidate" do - ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @ipv4 end) - - assert [%Candidate.Srflx{base: %{address: @ipv4}}] = srflx_candidates(ice_agent) - - assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} - assert_receive {:ex_ice, _pid, {:new_candidate, srflx_cand}} - - assert host_cand =~ "typ host" - assert srflx_cand =~ "typ srflx" - end - - test "creates only one candidate if external ip is repeated" do - ice_agent = spawn_ice_agent(IfDiscovery.MockMulti, fn _ip -> @ipv4 end) + ice_agent = + %ICEAgent{gathering_state: :complete} = + ICEAgent.new( + controlling_process: self(), + role: :controlled, + transport_module: Transport.Mock, + if_discovery_module: IfDiscovery.MockSingle, + host_to_srflx_ip_mapper: fn _ip -> @ipv4 end + ) + |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") + |> ICEAgent.gather_candidates() assert [%Candidate.Srflx{base: %{address: @ipv4}}] = srflx_candidates(ice_agent) assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} - assert_receive {:ex_ice, _pid, {:new_candidate, host_cand_2}} assert_receive {:ex_ice, _pid, {:new_candidate, srflx_cand}} assert host_cand =~ "typ host" - assert host_cand_2 =~ "typ host" assert srflx_cand =~ "typ srflx" end - test "ignores one to one mapping" do - ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn ip -> ip end) - - assert [] == srflx_candidates(ice_agent) - - assert_receive {:ex_ice, _pid, {:new_candidate, host_cand}} - refute_receive {:ex_ice, _pid, {:new_candidate, _srflx_cand}} - - assert host_cand =~ "typ host" - end - - test "ignores if ip types is not the same" do - ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @ipv6 end) - - assert [] == srflx_candidates(ice_agent) - end - - test "ignores when function returns nil value" do - ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> nil end) - - assert [] == srflx_candidates(ice_agent) - end - - test "ignores when function returns invalid value" do - ice_agent = spawn_ice_agent(IfDiscovery.MockSingle, fn _ip -> @invalid_ip end) - - assert [] == srflx_candidates(ice_agent) - end - test "works with STUN enabled" do ice_agent = ICEAgent.new( @@ -2732,19 +2697,6 @@ defmodule ExICE.Priv.ICEAgentTest do assert [%Candidate.Srflx{}] = srflx_candidates(ice_agent) end - defp spawn_ice_agent(discovery_module, host_to_srflx_ip_mapper) do - %ICEAgent{gathering_state: :complete} = - ICEAgent.new( - controlling_process: self(), - role: :controlled, - transport_module: Transport.Mock, - if_discovery_module: discovery_module, - host_to_srflx_ip_mapper: host_to_srflx_ip_mapper - ) - |> ICEAgent.set_remote_credentials("remoteufrag", "remotepwd") - |> ICEAgent.gather_candidates() - end - defp srflx_candidates(ice_agent) do ice_agent.local_cands |> Map.values()