From c5808c6959baebb1dd199e0ed96184cd358b1fe4 Mon Sep 17 00:00:00 2001 From: Frank Hunleth Date: Fri, 12 May 2023 16:10:55 -0400 Subject: [PATCH] Protect InternetChecker from being tricked by a captive portal This is a partial fix for the InternetChecker to keep it from being tricked by a captive portal that resolves DNS queries to itself. What was happening was that the remote host that was being checked got redirected to the captive portal. Since the ping test is simplistic, it only makes a TCP connection to the specified host and port, it was successful due to the port being 443. This made the InternetChecker think that it had connected to a remote host and it hadn't. The updated behavior is to verify that the IP address to check is not on the LAN (off subnet). This, of course, isn't perfect since the captive portal can make up off-LAN IP addresses just as easy as local ones, but the hope is that it is a low risk way of reducing false positives. --- lib/vintage_net/connectivity/inspector.ex | 20 +++++++++++++++++++ .../connectivity/internet_checker.ex | 7 ++++++- test/support/utils.ex | 13 ++++++++++++ .../connectivity/inspector_test.exs | 18 +++++++++++++++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/lib/vintage_net/connectivity/inspector.ex b/lib/vintage_net/connectivity/inspector.ex index ef4a0dcc..83660254 100644 --- a/lib/vintage_net/connectivity/inspector.ex +++ b/lib/vintage_net/connectivity/inspector.ex @@ -81,6 +81,26 @@ defmodule VintageNet.Connectivity.Inspector do end end + @doc """ + Returns true if the specified address is not on the network interface + + This function is useful for checking whether an address is on the Internet if + you don't trust the DNS server. Captive portals, for example, can give back + IP addresses that are local. It's not guaranteed, but it would be pointless to + check those IP's if you're looking for the Internet. + """ + @spec routed_address?(VintageNet.ifname(), :inet.ip_address()) :: boolean() + def routed_address?(ifname, ip_address) do + case get_addresses(ifname) do + [] -> + # If we don't even have an IP address, then there's no Internet for sure. + false + + our_addresses -> + not on_interface?(ip_address, our_addresses) + end + end + @doc false @spec check_ports(result(), [port()], [ip_address_and_mask()], cache()) :: result() def check_ports(result, [], _our_addresses, _cache), do: result diff --git a/lib/vintage_net/connectivity/internet_checker.ex b/lib/vintage_net/connectivity/internet_checker.ex index 00a57a0a..a59f9408 100644 --- a/lib/vintage_net/connectivity/internet_checker.ex +++ b/lib/vintage_net/connectivity/internet_checker.ex @@ -131,7 +131,12 @@ defmodule VintageNet.Connectivity.InternetChecker do end defp reload_ping_list(%{status: :unknown, ping_list: []} = state) do - ping_list = HostList.create_ping_list(state.configured_hosts) + # Create the ping list and filter out anything that's on the same LAN since + # pinging those addresses would be inconclusive. + ping_list = + HostList.create_ping_list(state.configured_hosts) + |> Enum.filter(&Inspector.routed_address?(state.ifname, &1)) + %{state | ping_list: ping_list} end diff --git a/test/support/utils.ex b/test/support/utils.ex index 3ee608d6..7acf5fd5 100644 --- a/test/support/utils.ex +++ b/test/support/utils.ex @@ -71,4 +71,17 @@ defmodule VintageNetTest.Utils do defp ipv4_address_field?({:addr, {_, _, _, _}}), do: true defp ipv4_address_field?(_), do: false + + @spec get_loopback_ifname() :: VintageNet.ifname() + def get_loopback_ifname() do + {:ok, addrs} = :inet.getifaddrs() + [{ifname, _info} | _rest] = Enum.filter(addrs, &loopback_interface?/1) + to_string(ifname) + end + + defp loopback_interface?({[?l, ?o | _anything], fields}) do + Enum.member?(fields[:flags], :up) and Enum.any?(fields, &ipv4_address_field?/1) + end + + defp loopback_interface?({_ifname, _fields}), do: false end diff --git a/test/vintage_net/connectivity/inspector_test.exs b/test/vintage_net/connectivity/inspector_test.exs index ee70df3a..eb29cbff 100644 --- a/test/vintage_net/connectivity/inspector_test.exs +++ b/test/vintage_net/connectivity/inspector_test.exs @@ -2,6 +2,8 @@ defmodule VintageNet.Connectivity.InspectorTest do use ExUnit.Case alias VintageNet.Connectivity.Inspector + alias VintageNetTest.Utils + doctest Inspector test "on_interface?/2" do @@ -17,6 +19,22 @@ defmodule VintageNet.Connectivity.InspectorTest do refute Inspector.on_interface?({1, 2, 3, 10, 0, 0, 0, 1}, if_addrs) end + test "routed_address?/2" do + ifname = Utils.get_loopback_ifname() + + # Anything on 127.x.y.z should be on the LAN (aka not routed) + refute Inspector.routed_address?(ifname, {127, 0, 0, 1}) + refute Inspector.routed_address?(ifname, {127, 0, 1, 1}) + refute Inspector.routed_address?(ifname, {127, 1, 1, 1}) + + # Anything not on 127.x.y.z is off LAN (aka routed, let's pretend) + assert Inspector.routed_address?(ifname, {128, 0, 0, 1}) + assert Inspector.routed_address?(ifname, {10, 10, 10, 10}) + + # Anything on interfaces that we don't know about return false + refute Inspector.routed_address?("bogus0", {10, 10, 10, 10}) + end + test "finds connections using port sockets" do # Run a super slow HTTP request to test {:ok, socket} = :gen_tcp.connect('neverssl.com', 80, [:binary, {:active, false}])