Merge pull request #367 from firezone/365/allow_hostnames_for_endpoint

Allow FQDNs for device endpoint

apps/fz_common/lib/fz_net.ex

@@ -10,6 +10,8 @@ defmodule FzCommon.FzNet do
   # credo:disable-for-next-line Credo.Check.Readability.MaxLineLength
   @cidr6_regex ~r/^s*((([0-9A-Fa-f]{1,4}:){7}([0-9A-Fa-f]{1,4}|:))|(([0-9A-Fa-f]{1,4}:){6}(:[0-9A-Fa-f]{1,4}|((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){5}(((:[0-9A-Fa-f]{1,4}){1,2})|:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3})|:))|(([0-9A-Fa-f]{1,4}:){4}(((:[0-9A-Fa-f]{1,4}){1,3})|((:[0-9A-Fa-f]{1,4})?:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){3}(((:[0-9A-Fa-f]{1,4}){1,4})|((:[0-9A-Fa-f]{1,4}){0,2}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){2}(((:[0-9A-Fa-f]{1,4}){1,5})|((:[0-9A-Fa-f]{1,4}){0,3}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(([0-9A-Fa-f]{1,4}:){1}(((:[0-9A-Fa-f]{1,4}){1,6})|((:[0-9A-Fa-f]{1,4}){0,4}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:))|(:(((:[0-9A-Fa-f]{1,4}){1,7})|((:[0-9A-Fa-f]{1,4}){0,5}:((25[0-5]|2[0-4]d|1dd|[1-9]?d)(.(25[0-5]|2[0-4]d|1dd|[1-9]?d)){3}))|:)))(%.+)?s*(\/([0-9]|[1-9][0-9]|1[0-1][0-9]|12[0-8]))$/
 
+  @fqdn_regex ~r/\A(?=^.{4,253}$)(^((?!-)[a-zA-Z0-9-]{0,62}[a-zA-Z0-9]\.)+[a-zA-Z]{2,63}$)\z/
+
   # XXX: Consider using InetCidr for this
   def ip_type(str) when is_binary(str) do
     charlist =
@@ -52,4 +54,8 @@ defmodule FzCommon.FzNet do
       :inet.ntoa(addr) |> List.to_string()
     end
   end
+
+  def valid_fqdn?(fqdn) when is_binary(fqdn) do
+    String.match?(fqdn, @fqdn_regex)
+  end
 end

apps/fz_common/test/fz_net_test.exs

@@ -39,6 +39,24 @@ defmodule FzCommon.FzNetTest do
     end
   end
 
+  describe "valid_fqdn/1" do
+    test "foobar is invalid" do
+      refute FzNet.valid_fqdn?("foobar")
+    end
+
+    test "-foobar is invalid" do
+      refute FzNet.valid_fqdn?("-foobar")
+    end
+
+    test "foobar.com is valid" do
+      assert FzNet.valid_fqdn?("foobar.com")
+    end
+
+    test "ff99.example.com is valid" do
+      assert FzNet.valid_fqdn?("ff00.example.com")
+    end
+  end
+
   describe "valid_cidr?/1" do
     test "::/0f is an invalid CIDR" do
       refute FzNet.valid_cidr?("::/0f")

apps/fz_http/lib/fz_http/devices/device.ex

@@ -8,7 +8,7 @@ defmodule FzHttp.Devices.Device do
 
   import FzHttp.SharedValidators,
     only: [
-      validate_ip: 2,
+      validate_fqdn_or_ip: 2,
       validate_omitted: 2,
       validate_list_of_ips: 2,
       validate_no_duplicates: 2,
@@ -100,7 +100,7 @@ defmodule FzHttp.Devices.Device do
     |> validate_list_of_ips_or_cidrs(:allowed_ips)
     |> validate_list_of_ips(:dns_servers)
     |> validate_no_duplicates(:dns_servers)
-    |> validate_ip(:endpoint)
+    |> validate_fqdn_or_ip(:endpoint)
     |> validate_number(:persistent_keepalives,
       greater_than_or_equal_to: 0,
       less_than_or_equal_to: 120

apps/fz_http/lib/fz_http/settings/setting.ex

@@ -16,7 +16,7 @@ defmodule FzHttp.Settings.Setting do
 
   import FzHttp.SharedValidators,
     only: [
-      validate_ip: 2,
+      validate_fqdn_or_ip: 2,
       validate_list_of_ips: 2,
       validate_list_of_ips_or_cidrs: 2,
       validate_no_duplicates: 2
@@ -59,7 +59,7 @@ defmodule FzHttp.Settings.Setting do
 
   defp validate_kv_pair(changeset, "default.device.endpoint") do
     changeset
-    |> validate_ip(:value)
+    |> validate_fqdn_or_ip(:value)
   end
 
   defp validate_kv_pair(changeset, "default.device.persistent_keepalives") do

apps/fz_http/lib/fz_http/shared_validators.ex

@@ -8,6 +8,7 @@ defmodule FzHttp.SharedValidators do
   import FzCommon.FzNet,
     only: [
       valid_ip?: 1,
+      valid_fqdn?: 1,
       valid_cidr?: 1
     ]
 
@@ -32,6 +33,23 @@ defmodule FzHttp.SharedValidators do
     end)
   end
 
+  def validate_fqdn_or_ip(changeset, field) when is_atom(field) do
+    validate_change(changeset, field, fn _current_field, value ->
+      try do
+        for ip <- String.split(value, ",") do
+          unless valid_ip?(String.trim(ip)) or valid_fqdn?(String.trim(ip)) do
+            throw(ip)
+          end
+        end
+
+        []
+      catch
+        ip ->
+          [{field, "is invalid: #{String.trim(ip)} is not a valid fqdn or IPv4 / IPv6 address"}]
+      end
+    end)
+  end
+
   def validate_ip(changeset, field) when is_atom(field) do
     validate_change(changeset, field, fn _current_field, value ->
       try do

apps/fz_http/test/fz_http/devices_test.exs

@@ -74,6 +74,36 @@ defmodule FzHttp.DevicesTest do
       allowed_ips: "0.0.0.0/0, ::/0, ::0/0, 192.168.1.0/24"
     }
 
+    @valid_endpoint_ipv4_attrs %{
+      use_default_endpoint: false,
+      endpoint: "5.5.5.5"
+    }
+
+    @valid_endpoint_ipv6_attrs %{
+      use_default_endpoint: false,
+      endpoint: "fd00::1"
+    }
+
+    @valid_endpoint_host_attrs %{
+      use_default_endpoint: false,
+      endpoint: "valid-endpoint.example.com"
+    }
+
+    @invalid_endpoint_ipv4_attrs %{
+      use_default_endpoint: false,
+      endpoint: "265.1.1.1"
+    }
+
+    @invalid_endpoint_ipv6_attrs %{
+      use_default_endpoint: false,
+      endpoint: "deadbeef::1"
+    }
+
+    @invalid_endpoint_host_attrs %{
+      use_default_endpoint: false,
+      endpoint: "can't have this"
+    }
+
     @invalid_allowed_ips_attrs %{
       allowed_ips: "1.1.1.1, 11, foobar"
     }
@@ -100,6 +130,48 @@ defmodule FzHttp.DevicesTest do
       assert @valid_dns_servers_attrs = test_device
     end
 
+    test "updates device with valid ipv4 endpoint", %{device: device} do
+      {:ok, test_device} = Devices.update_device(device, @valid_endpoint_ipv4_attrs)
+      assert @valid_endpoint_ipv4_attrs = test_device
+    end
+
+    test "updates device with valid ipv6 endpoint", %{device: device} do
+      {:ok, test_device} = Devices.update_device(device, @valid_endpoint_ipv6_attrs)
+      assert @valid_endpoint_ipv6_attrs = test_device
+    end
+
+    test "updates device with valid host endpoint", %{device: device} do
+      {:ok, test_device} = Devices.update_device(device, @valid_endpoint_host_attrs)
+      assert @valid_endpoint_host_attrs = test_device
+    end
+
+    test "prevents updating device with invalid ipv4 endpoint", %{device: device} do
+      {:error, changeset} = Devices.update_device(device, @invalid_endpoint_ipv4_attrs)
+
+      assert changeset.errors[:endpoint] == {
+               "is invalid: 265.1.1.1 is not a valid fqdn or IPv4 / IPv6 address",
+               []
+             }
+    end
+
+    test "prevents updating device with invalid ipv6 endpoint", %{device: device} do
+      {:error, changeset} = Devices.update_device(device, @invalid_endpoint_ipv6_attrs)
+
+      assert changeset.errors[:endpoint] == {
+               "is invalid: deadbeef::1 is not a valid fqdn or IPv4 / IPv6 address",
+               []
+             }
+    end
+
+    test "prevents updating device with invalid host endpoint", %{device: device} do
+      {:error, changeset} = Devices.update_device(device, @invalid_endpoint_host_attrs)
+
+      assert changeset.errors[:endpoint] == {
+               "is invalid: can't have this is not a valid fqdn or IPv4 / IPv6 address",
+               []
+             }
+    end
+
     test "prevents updating device with invalid dns_servers", %{device: device} do
       {:error, changeset} = Devices.update_device(device, @invalid_dns_servers_attrs)
 

apps/fz_http/test/fz_http/settings_test.exs

@@ -14,11 +14,18 @@ defmodule FzHttp.SettingsTest do
 
     import FzHttp.SettingsFixtures
 
-    @valid_settings %{
-      "default.device.dns_servers" => "8.8.8.8",
-      "default.device.allowed_ips" => "::/0",
-      "default.device.endpoint" => "172.10.10.10"
-    }
+    @valid_settings [
+      %{
+        "default.device.dns_servers" => "8.8.8.8",
+        "default.device.allowed_ips" => "::/0",
+        "default.device.endpoint" => "172.10.10.10"
+      },
+      %{
+        "default.device.dns_servers" => "8.8.8.8",
+        "default.device.allowed_ips" => "::/0",
+        "default.device.endpoint" => "foobar.example.com"
+      }
+    ]
     @invalid_settings %{
       "default.device.dns_servers" => "foobar",
       "default.device.allowed_ips" => nil,
@@ -39,20 +46,24 @@ defmodule FzHttp.SettingsTest do
 
     test "update_setting/2 with valid data updates the setting via provided setting" do
       for key <- @setting_keys do
-        value = @valid_settings[key]
-        setting = Settings.get_setting!(key: key)
-        assert {:ok, %Setting{} = setting} = Settings.update_setting(setting, %{value: value})
-        assert setting.key == key
-        assert setting.value == value
+        for valid_setting <- @valid_settings do
+          value = valid_setting[key]
+          setting = Settings.get_setting!(key: key)
+          assert {:ok, %Setting{} = setting} = Settings.update_setting(setting, %{value: value})
+          assert setting.key == key
+          assert setting.value == value
+        end
       end
     end
 
     test "update_setting/2 with valid data updates the setting via key, value" do
       for key <- @setting_keys do
-        value = @valid_settings[key]
-        assert {:ok, %Setting{} = setting} = Settings.update_setting(key, value)
-        assert setting.key == key
-        assert setting.value == value
+        for valid_setting <- @valid_settings do
+          value = valid_setting[key]
+          assert {:ok, %Setting{} = setting} = Settings.update_setting(key, value)
+          assert setting.key == key
+          assert setting.value == value
+        end
       end
     end