Add node connection validation

Added IP address verification to ensure a node is reachable (discarding private IP ranges, and unreachable addresses)
Added node duplication check to avoid overload of IP/Port for different nodes

Fixes #108

config/prod.exs

@@ -189,6 +189,8 @@ config :archethic, ArchEthic.P2P.BootstrappingSeeds,
   # TODO: define the default list of P2P seeds once the network will be more open to new miners
   genesis_seeds: System.get_env("ARCHETHIC_P2P_BOOTSTRAPPING_SEEDS")
 
+config :archethic, ArchEthic.Mining.PendingTransactionValidation, validate_connection: true
+
 config :archethic, ArchEthicWeb.FaucetController,
   enabled: System.get_env("ARCHETHIC_NETWORK_TYPE") == "testnet"
 

config/test.exs

@@ -99,6 +99,8 @@ config :archethic, ArchEthic.P2P.Transport, MockTransport
 
 config :archethic, ArchEthic.P2P.BootstrappingSeeds, enabled: false
 
+config :archethic, ArchEthic.Mining.PendingTransactionValidation, validate_connection: true
+
 config :archethic, ArchEthic.Reward.NetworkPoolScheduler, enabled: false
 config :archethic, ArchEthic.Reward.WithdrawScheduler, enabled: false
 

lib/archethic/mining/pending_transaction_validation.ex

@@ -8,7 +8,7 @@ defmodule ArchEthic.Mining.PendingTransactionValidation do
 
   alias ArchEthic.Governance
   alias ArchEthic.Governance.Code.Proposal, as: CodeProposal
-
+  alias ArchEthic.Networking
   alias ArchEthic.OracleChain
 
   alias ArchEthic.P2P
@@ -33,6 +33,12 @@ defmodule ArchEthic.Mining.PendingTransactionValidation do
 
   require Logger
 
+  @validate_connection Application.compile_env(
+                         :archethic,
+                         [__MODULE__, :validate_connection],
+                         false
+                       )
+
   @doc """
   Determines if the transaction is accepted into the network
   """
@@ -114,8 +120,9 @@ defmodule ArchEthic.Mining.PendingTransactionValidation do
          },
          previous_public_key: previous_public_key
        }) do
-    with {:ok, _, _, _, _, key_certificate} <- Node.decode_transaction_content(content),
+    with {:ok, ip, port, _, _, key_certificate} <- Node.decode_transaction_content(content),
          root_ca_public_key <- Crypto.get_root_ca_public_key(previous_public_key),
+         true <- valid_connection?(@validate_connection, ip, port, previous_public_key),
          true <-
            Crypto.verify_key_certificate?(
              previous_public_key,
@@ -260,4 +267,16 @@ defmodule ArchEthic.Mining.PendingTransactionValidation do
         previous_public_key
     end
   end
+
+  defp valid_connection?(true, ip, port, previous_public_key) do
+    with true <- Networking.valid_ip?(ip),
+         false <- P2P.duplicating_node?(ip, port, previous_public_key) do
+      true
+    else
+      _ ->
+        false
+    end
+  end
+
+  defp valid_connection?(false, _, _, _), do: true
 end

lib/archethic/networking.ex

@@ -6,6 +6,7 @@ defmodule ArchEthic.Networking do
   alias __MODULE__.IPLookup
   alias __MODULE__.PortForwarding
 
+  @ip_validate_regex ~r/(^0\.)|(^127\.)|(^10\.)|(^172\.1[6-9]\.)|(^172\.2[0-9]\.)|(^172\.3[0-1]\.)|(^192\.168\.)/
   @doc """
   Provides current host IP address by leveraging the IP lookup provider.
 
@@ -17,12 +18,49 @@ defmodule ArchEthic.Networking do
   defdelegate get_node_ip, to: IPLookup
 
   @doc """
-  Try to open the port from the configuration. 
+  Try to open the port from the configuration.
 
   If not possible try other random port. Otherwise assume the port is open
 
   A force parameter can be given to use a random port if the port publication doesn't work
   """
   @spec try_open_port(:inet.port_number(), boolean()) :: :inet.port_number()
   defdelegate try_open_port(port, force?), to: PortForwarding
+
+  @doc ~S"""
+  Filters private IP address ranges
+
+  ## Example
+
+      iex> ArchEthic.Networking.valid_ip?({0,0,0,0})
+      false
+
+      iex> ArchEthic.Networking.valid_ip?({127,0,0,1})
+      false
+
+      iex> ArchEthic.Networking.valid_ip?({192,168,1,1})
+      false
+
+      iex> ArchEthic.Networking.valid_ip?({10,10,0,1})
+      false
+
+      iex> ArchEthic.Networking.valid_ip?({172,16,0,1})
+      false
+
+      iex> ArchEthic.Networking.valid_ip?({54,39,186,147})
+      true
+  """
+  @spec valid_ip?(:inet.ip_address()) :: boolean()
+  def valid_ip?(ip) do
+    case :inet.ntoa(ip) do
+      {:error, :einval} ->
+        false
+
+      ip_str ->
+        !Regex.match?(
+          @ip_validate_regex,
+          to_string(ip_str)
+        )
+    end
+  end
 end

lib/archethic/p2p.ex

@@ -538,4 +538,26 @@ defmodule ArchEthic.P2P do
         do_reply_atomic(rest, message, compare_fun)
     end
   end
+
+  @doc """
+  Check for possible duplicate nodes (IP spoofing).
+
+  Returns true if matching node {ip,port} has a different first public key.
+  """
+  @spec duplicating_node?(
+          :inet.ip_address(),
+          :inet.port_number(),
+          ArchEthic.Crypto.key(),
+          list(Node.t())
+        ) ::
+          boolean()
+  def duplicating_node?(tx_ip, tx_port, prev_public_key, nodes \\ list_nodes()) do
+    case Enum.find(nodes, &(&1.ip == tx_ip and &1.port == tx_port)) do
+      nil ->
+        false
+
+      %Node{first_public_key: first_public_key} ->
+        TransactionChain.get_first_public_key(prev_public_key) != first_public_key
+    end
+  end
 end

test/archethic/networking_test.exs

@@ -0,0 +1,42 @@
+defmodule ArchEthic.NetworkingTest do
+  use ExUnit.Case, async: true
+  use ExUnitProperties
+
+  alias ArchEthic.Networking
+
+  doctest ArchEthic.Networking
+
+  describe "valid_ip?/1" do
+    property "should return false for loopback address range" do
+      ip_generator = tuple({constant(127), integer(0..255), integer(0..255), integer(0..255)})
+
+      check all(ip <- ip_generator) do
+        refute Networking.valid_ip?(ip)
+      end
+    end
+
+    property "should return false for Class A private address range" do
+      ip_generator = tuple({constant(10), integer(0..255), integer(0..255), integer(0..255)})
+
+      check all(ip <- ip_generator) do
+        refute Networking.valid_ip?(ip)
+      end
+    end
+
+    property "should return false for Class B private address range" do
+      ip_generator = tuple({constant(172), integer(16..31), integer(0..255), integer(0..255)})
+
+      check all(ip <- ip_generator) do
+        refute Networking.valid_ip?(ip)
+      end
+    end
+
+    property "should return false for Class C private address range" do
+      ip_generator = tuple({constant(192), constant(168), integer(0..255), integer(0..255)})
+
+      check all(ip <- ip_generator) do
+        refute Networking.valid_ip?(ip)
+      end
+    end
+  end
+end

test/archethic/p2p_test.exs

@@ -259,4 +259,79 @@ defmodule ArchEthic.P2PTest do
                })
     end
   end
+
+  describe "duplicating_node?/3" do
+    test "should return true for duplicate node" do
+      MockDB
+      |> stub(:get_first_public_key, fn _ ->
+        <<1::16, 1::8>>
+      end)
+
+      assert P2P.duplicating_node?(
+               {127, 0, 0, 1},
+               3000,
+               <<1::16, 1::8>>,
+               [
+                 %Node{
+                   ip: {127, 0, 0, 1},
+                   port: 3000,
+                   first_public_key: <<0::16, 0::8>>
+                 }
+               ]
+             )
+    end
+
+    test "should return false for original node" do
+      MockDB
+      |> stub(:get_first_public_key, fn _ ->
+        <<1::16, 1::8>>
+      end)
+
+      refute P2P.duplicating_node?(
+               {127, 0, 0, 1},
+               3000,
+               <<1::16, 1::8>>,
+               [
+                 %Node{
+                   ip: {127, 0, 0, 1},
+                   port: 3000,
+                   first_public_key: <<1::16, 1::8>>
+                 }
+               ]
+             )
+    end
+
+    test "should return false for node with different ip/port" do
+      MockDB
+      |> stub(:get_first_public_key, fn _ ->
+        <<1::16, 1::8>>
+      end)
+
+      refute P2P.duplicating_node?(
+               {127, 0, 0, 2},
+               3000,
+               <<1::16, 1::8>>,
+               [
+                 %Node{
+                   ip: {127, 0, 0, 1},
+                   port: 3000,
+                   first_public_key: <<1::16, 1::8>>
+                 }
+               ]
+             )
+
+      refute P2P.duplicating_node?(
+               {127, 0, 0, 1},
+               3001,
+               <<1::16, 1::8>>,
+               [
+                 %Node{
+                   ip: {127, 0, 0, 1},
+                   port: 3000,
+                   first_public_key: <<1::16, 1::8>>
+                 }
+               ]
+             )
+    end
+  end
 end