From 6919bfb0251b1c40bbf4f09b71cf9e6dd56e193e Mon Sep 17 00:00:00 2001 From: apoorv-2204 Date: Thu, 29 Dec 2022 12:55:10 +0530 Subject: [PATCH 1/4] attempt to resolve upnp_port_forwarding_fail_on_local_ip_change#698 --- .../ip_lookup/nat_discovery/miniupnp.ex | 65 ++++++++++++++++--- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex index 2bf80731d..bb589b0fe 100644 --- a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex +++ b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex @@ -43,8 +43,31 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do end end + @spec do_open_port(:inet.ip_address(), non_neg_integer()) :: :ok | :error defp do_open_port(local_ip, port) do - opts = [ + with {reason, status} when status != 0 <- System.cmd(@upnpc, map_query(local_ip, port)), + {:error, e} <- parse_reason(reason), + :ok <- handle_error(e, local_ip, port), + {_, 0} <- System.cmd(@upnpc, map_query(local_ip, port)) do + :ok + else + {_, 0} -> + :ok + + {reason, _status} -> + Logger.debug(reason) + :error + + :error -> + Logger.debug("Unkonwn error", port: port) + :error + end + end + + @protocol "tcp" + @spec map_query(:inet.ip_address(), non_neg_integer()) :: [String.t()] + defp map_query(local_ip, port) do + [ # Add redirection "-a", # Local ip @@ -54,18 +77,44 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do # Remote port to open "#{port}", # Protocol - "tcp", + @protocol, # Lifetime "0" ] + end - case System.cmd(@upnpc, opts) do - {_, 0} -> - :ok + @spec parse_reason(String.t()) :: + {:error, :conflict_in_mapping_entry | :unknown} + defp parse_reason(reason) do + # upon more condtions , refactor with cond do end + if Regex.scan(~r/ConflictInMappingEntry/, reason, capture: :all) != [] do + Logger.warning("Port is employed to another host.") + {:error, :conflict_in_mapping_entry} + else + {:error, :unknown} + end + end - {reason, _status} -> - Logger.debug(reason) - :error + @spec handle_error(:conflict_in_mapping_entry | :unknown, :inet.ip_address(), non_neg_integer()) :: + :ok | :error + defp handle_error(:conflict_in_mapping_entry, _, port) do + case System.cmd(@upnpc, revoke_query(port)) do + {_, 0} -> :ok + _ -> :error end end + + defp handle_error(:unknown, _, _), do: :error + + @spec revoke_query(non_neg_integer()) :: [String.t()] + defp revoke_query(port) do + [ + # deleting redirection + "-d", + # external port to delete + "#{port}", + # Protocol + @protocol + ] + end end From 5996c81122a059a87790b099ba7150a3c4626c21 Mon Sep 17 00:00:00 2001 From: apoorv-2204 Date: Fri, 6 Jan 2023 00:09:30 +0530 Subject: [PATCH 2/4] review changes --- .../ip_lookup/nat_discovery/miniupnp.ex | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex index bb589b0fe..15c6bf42e 100644 --- a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex +++ b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex @@ -44,23 +44,19 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do end @spec do_open_port(:inet.ip_address(), non_neg_integer()) :: :ok | :error - defp do_open_port(local_ip, port) do - with {reason, status} when status != 0 <- System.cmd(@upnpc, map_query(local_ip, port)), - {:error, e} <- parse_reason(reason), - :ok <- handle_error(e, local_ip, port), - {_, 0} <- System.cmd(@upnpc, map_query(local_ip, port)) do - :ok - else + def do_open_port(local_ip, port, retires \\ 2) + + def do_open_port(_local_ip, _port, 0), do: :error + + def do_open_port(local_ip, port, retires) do + case System.cmd(@upnpc, map_query(local_ip, port)) do {_, 0} -> :ok - {reason, _status} -> - Logger.debug(reason) - :error + {reason, status} when status != 0 -> + handle_error(reason, local_ip, port) - :error -> - Logger.debug("Unkonwn error", port: port) - :error + do_open_port(local_ip, port, retires - 1) end end @@ -68,6 +64,9 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do @spec map_query(:inet.ip_address(), non_neg_integer()) :: [String.t()] defp map_query(local_ip, port) do [ + # description + "-e", + "Archethic_Node", # Add redirection "-a", # Local ip @@ -83,29 +82,18 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do ] end - @spec parse_reason(String.t()) :: - {:error, :conflict_in_mapping_entry | :unknown} - defp parse_reason(reason) do - # upon more condtions , refactor with cond do end + @spec handle_error( + reason :: String.t(), + local_ip :: :inet.ip_address(), + port :: non_neg_integer() + ) :: :error | any() + defp handle_error(reason, _local_ip, port) do if Regex.scan(~r/ConflictInMappingEntry/, reason, capture: :all) != [] do Logger.warning("Port is employed to another host.") - {:error, :conflict_in_mapping_entry} - else - {:error, :unknown} + System.cmd(@upnpc, revoke_query(port)) end end - @spec handle_error(:conflict_in_mapping_entry | :unknown, :inet.ip_address(), non_neg_integer()) :: - :ok | :error - defp handle_error(:conflict_in_mapping_entry, _, port) do - case System.cmd(@upnpc, revoke_query(port)) do - {_, 0} -> :ok - _ -> :error - end - end - - defp handle_error(:unknown, _, _), do: :error - @spec revoke_query(non_neg_integer()) :: [String.t()] defp revoke_query(port) do [ From ff506ed6e3691aa6b063c8fc351d23866c4c298f Mon Sep 17 00:00:00 2001 From: apoorv-2204 Date: Fri, 6 Jan 2023 09:33:20 +0530 Subject: [PATCH 3/4] review changes --- .../networking/ip_lookup/nat_discovery/miniupnp.ex | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex index 15c6bf42e..8bf67bb71 100644 --- a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex +++ b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex @@ -44,11 +44,11 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do end @spec do_open_port(:inet.ip_address(), non_neg_integer()) :: :ok | :error - def do_open_port(local_ip, port, retires \\ 2) + defp do_open_port(local_ip, port, retires \\ 2) - def do_open_port(_local_ip, _port, 0), do: :error + defp do_open_port(_local_ip, _port, 0), do: :error - def do_open_port(local_ip, port, retires) do + defp do_open_port(local_ip, port, retires) do case System.cmd(@upnpc, map_query(local_ip, port)) do {_, 0} -> :ok @@ -64,9 +64,6 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do @spec map_query(:inet.ip_address(), non_neg_integer()) :: [String.t()] defp map_query(local_ip, port) do [ - # description - "-e", - "Archethic_Node", # Add redirection "-a", # Local ip From 6ba27a69a66e04224e91131913ef8e97d54ec103 Mon Sep 17 00:00:00 2001 From: apoorv-2204 Date: Fri, 6 Jan 2023 23:39:47 +0530 Subject: [PATCH 4/4] review changes --- .../networking/ip_lookup/nat_discovery/miniupnp.ex | 8 ++++---- mix.lock | 4 ++-- src/c/nat/miniupnp | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex index 8bf67bb71..77e3e4aed 100644 --- a/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex +++ b/lib/archethic/networking/ip_lookup/nat_discovery/miniupnp.ex @@ -44,19 +44,19 @@ defmodule Archethic.Networking.IPLookup.NATDiscovery.MiniUPNP do end @spec do_open_port(:inet.ip_address(), non_neg_integer()) :: :ok | :error - defp do_open_port(local_ip, port, retires \\ 2) + defp do_open_port(local_ip, port, retries \\ 2) defp do_open_port(_local_ip, _port, 0), do: :error - defp do_open_port(local_ip, port, retires) do + defp do_open_port(local_ip, port, retries) do case System.cmd(@upnpc, map_query(local_ip, port)) do {_, 0} -> :ok - {reason, status} when status != 0 -> + {reason, _} -> handle_error(reason, local_ip, port) - do_open_port(local_ip, port, retires - 1) + do_open_port(local_ip, port, retries - 1) end end diff --git a/mix.lock b/mix.lock index 846697d8b..ac71e4c17 100644 --- a/mix.lock +++ b/mix.lock @@ -32,7 +32,7 @@ "exjsonpath": {:hex, :exjsonpath, "0.9.0", "87e593eb0deb53aa0688ca9f9edc9fb3456aca83c82245f83201ea04d696feba", [:mix], [], "hexpm", "8d7a8e9ba784e1f7a67c6f1074a3ac91a3a79a45969514ee5d95cea5bf749627"}, "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"}, "floki": {:hex, :floki, "0.34.0", "002d0cc194b48794d74711731db004fafeb328fe676976f160685262d43706a8", [:mix], [], "hexpm", "9c3a9f43f40dde00332a589bd9d389b90c1f518aef500364d00636acc5ebc99c"}, - "flow": {:hex, :flow, "1.2.1", "cfe984b2078ced0bc92807737909abe1b158288256244cc77d03ad96cbef1571", [:mix], [{:gen_stage, "~> 1.0", [hex: :gen_stage, repo: "hexpm", optional: false]}], "hexpm", "3f9fe6a4b28b8e82822d7a851c8d8fe3ac3c0e597aa2cf3cccd81f9af561abee"}, + "flow": {:hex, :flow, "1.2.2", "b23a52f55e38556c4bc06bcb8530c81bdb440d42b7fae1f78a7c0563d662aee9", [:mix], [{:gen_stage, "~> 1.0", [hex: :gen_stage, repo: "hexpm", optional: false]}], "hexpm", "ab442b82456d4217562bde5852da18189b8386006d221d514c98ad49d1e80040"}, "gen_stage": {:hex, :gen_stage, "1.1.2", "b1656cd4ba431ed02c5656fe10cb5423820847113a07218da68eae5d6a260c23", [:mix], [], "hexpm", "9e39af23140f704e2b07a3e29d8f05fd21c2aaf4088ff43cb82be4b9e3148d02"}, "gen_state_machine": {:hex, :gen_state_machine, "3.0.0", "1e57f86a494e5c6b14137ebef26a7eb342b3b0070c7135f2d6768ed3f6b6cdff", [:mix], [], "hexpm", "0a59652574bebceb7309f6b749d2a41b45fdeda8dbb4da0791e355dd19f0ed15"}, "git_hooks": {:hex, :git_hooks, "0.7.3", "09489e94d88dfc767662e22aff2b6208bd7cf555a19dd0e1477cca4683ce0701", [:mix], [{:blankable, "~> 1.0.0", [hex: :blankable, repo: "hexpm", optional: false]}, {:recase, "~> 0.7.0", [hex: :recase, repo: "hexpm", optional: false]}], "hexpm", "d6ddedeb4d3a8602bc3f84e087a38f6150a86d9e790628ed8bc70e6d90681659"}, @@ -56,7 +56,7 @@ "phoenix": {:hex, :phoenix, "1.6.15", "0a1d96bbc10747fd83525370d691953cdb6f3ccbac61aa01b4acb012474b047d", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 1.0 or ~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.2", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "d70ab9fbf6b394755ea88b644d34d79d8b146e490973151f248cacd122d20672"}, "phoenix_html": {:hex, :phoenix_html, "3.2.0", "1c1219d4b6cb22ac72f12f73dc5fad6c7563104d083f711c3fcd8551a1f4ae11", [:mix], [{:plug, "~> 1.5", [hex: :plug, repo: "hexpm", optional: true]}], "hexpm", "36ec97ba56d25c0136ef1992c37957e4246b649d620958a1f9fa86165f8bc54f"}, "phoenix_live_dashboard": {:hex, :phoenix_live_dashboard, "0.7.2", "97cc4ff2dba1ebe504db72cb45098cb8e91f11160528b980bd282cc45c73b29c", [:mix], [{:ecto, "~> 3.6.2 or ~> 3.7", [hex: :ecto, repo: "hexpm", optional: true]}, {:ecto_mysql_extras, "~> 0.5", [hex: :ecto_mysql_extras, repo: "hexpm", optional: true]}, {:ecto_psql_extras, "~> 0.7", [hex: :ecto_psql_extras, repo: "hexpm", optional: true]}, {:mime, "~> 1.6 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:phoenix_live_view, "~> 0.18.3", [hex: :phoenix_live_view, repo: "hexpm", optional: false]}, {:telemetry_metrics, "~> 0.6 or ~> 1.0", [hex: :telemetry_metrics, repo: "hexpm", optional: false]}], "hexpm", "0e5fdf063c7a3b620c566a30fcf68b7ee02e5e46fe48ee46a6ec3ba382dc05b7"}, - "phoenix_live_view": {:hex, :phoenix_live_view, "0.18.3", "2e3d009422addf8b15c3dccc65ce53baccbe26f7cfd21d264680b5867789a9c1", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.6.15 or ~> 1.7.0", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 3.1", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c8845177a866e017dcb7083365393c8f00ab061b8b6b2bda575891079dce81b2"}, + "phoenix_live_view": {:hex, :phoenix_live_view, "0.18.4", "7b06aefe43efbbee7f33fa0875b409400243e4c96d7bb2f962d0a0c0b5ddacb1", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix, "~> 1.6.15 or ~> 1.7.0", [hex: :phoenix, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 3.1", [hex: :phoenix_html, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4.2 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "b37778447c9d3e9904899e132d62a7d484fa8abcc10433b1b820a9b907b82852"}, "phoenix_pubsub": {:hex, :phoenix_pubsub, "2.1.1", "ba04e489ef03763bf28a17eb2eaddc2c20c6d217e2150a61e3298b0f4c2012b5", [:mix], [], "hexpm", "81367c6d1eea5878ad726be80808eb5a787a23dee699f96e72b1109c57cdd8d9"}, "phoenix_template": {:hex, :phoenix_template, "1.0.0", "c57bc5044f25f007dc86ab21895688c098a9f846a8dda6bc40e2d0ddc146e38f", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}], "hexpm", "1b066f99a26fd22064c12b2600a9a6e56700f591bf7b20b418054ea38b4d4357"}, "phoenix_view": {:hex, :phoenix_view, "2.0.2", "6bd4d2fd595ef80d33b439ede6a19326b78f0f1d8d62b9a318e3d9c1af351098", [:mix], [{:phoenix_html, "~> 2.14.2 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}], "hexpm", "a929e7230ea5c7ee0e149ffcf44ce7cf7f4b6d2bfe1752dd7c084cdff152d36f"}, diff --git a/src/c/nat/miniupnp b/src/c/nat/miniupnp index 207cf440a..014c9df8e 160000 --- a/src/c/nat/miniupnp +++ b/src/c/nat/miniupnp @@ -1 +1 @@ -Subproject commit 207cf440a22c075cb55fb067a850be4f9c204e6e +Subproject commit 014c9df8ee7a36e5bf85aa619062a2d4b95ec8f6