From 0a22d05860519da02bdcb3e32158adb733c6fe91 Mon Sep 17 00:00:00 2001 From: Rohit Roy Chowdhury Date: Thu, 4 Nov 2021 20:30:18 +0530 Subject: [PATCH] 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 | 2 + config/test.exs | 2 + .../mining/pending_transaction_validation.ex | 23 +++++- lib/archethic/networking.ex | 40 +++++++++- lib/archethic/p2p.ex | 22 ++++++ test/archethic/networking_test.exs | 42 +++++++++++ test/archethic/p2p_test.exs | 75 +++++++++++++++++++ 7 files changed, 203 insertions(+), 3 deletions(-) create mode 100644 test/archethic/networking_test.exs diff --git a/config/prod.exs b/config/prod.exs index fced9d921..5e4263be1 100755 --- a/config/prod.exs +++ b/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" diff --git a/config/test.exs b/config/test.exs index 14722c432..494543f09 100755 --- a/config/test.exs +++ b/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 diff --git a/lib/archethic/mining/pending_transaction_validation.ex b/lib/archethic/mining/pending_transaction_validation.ex index 7e10cc2c5..c379e1651 100644 --- a/lib/archethic/mining/pending_transaction_validation.ex +++ b/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 diff --git a/lib/archethic/networking.ex b/lib/archethic/networking.ex index 370e04402..d547ff6e6 100644 --- a/lib/archethic/networking.ex +++ b/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,7 +18,7 @@ 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 @@ -25,4 +26,41 @@ defmodule ArchEthic.Networking do """ @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 diff --git a/lib/archethic/p2p.ex b/lib/archethic/p2p.ex index 702c5e780..f93b9b0af 100644 --- a/lib/archethic/p2p.ex +++ b/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 diff --git a/test/archethic/networking_test.exs b/test/archethic/networking_test.exs new file mode 100644 index 000000000..a49a65abb --- /dev/null +++ b/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 diff --git a/test/archethic/p2p_test.exs b/test/archethic/p2p_test.exs index 607439ae1..e70a4597a 100644 --- a/test/archethic/p2p_test.exs +++ b/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