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

@@ -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

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
 

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

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}])