From 19b147ad7b0d2fbb870f5e973538de0b39bfdd78 Mon Sep 17 00:00:00 2001 From: Jamil Bou Kheir Date: Tue, 28 Dec 2021 18:37:58 -0600 Subject: [PATCH 1/2] Allow FQDNs for device endpoint --- apps/fz_common/lib/fz_net.ex | 6 ++ apps/fz_common/test/fz_net_test.exs | 18 +++++ apps/fz_http/lib/fz_http/devices/device.ex | 4 +- apps/fz_http/lib/fz_http/shared_validators.ex | 18 +++++ apps/fz_http/test/fz_http/devices_test.exs | 72 +++++++++++++++++++ 5 files changed, 116 insertions(+), 2 deletions(-) diff --git a/apps/fz_common/lib/fz_net.ex b/apps/fz_common/lib/fz_net.ex index 17b766d14..bf64a6f50 100644 --- a/apps/fz_common/lib/fz_net.ex +++ b/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 diff --git a/apps/fz_common/test/fz_net_test.exs b/apps/fz_common/test/fz_net_test.exs index d0f125114..0c1ffd82f 100644 --- a/apps/fz_common/test/fz_net_test.exs +++ b/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") diff --git a/apps/fz_http/lib/fz_http/devices/device.ex b/apps/fz_http/lib/fz_http/devices/device.ex index 158a0561f..045604614 100644 --- a/apps/fz_http/lib/fz_http/devices/device.ex +++ b/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 diff --git a/apps/fz_http/lib/fz_http/shared_validators.ex b/apps/fz_http/lib/fz_http/shared_validators.ex index c641acb9d..ed19f858e 100644 --- a/apps/fz_http/lib/fz_http/shared_validators.ex +++ b/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 diff --git a/apps/fz_http/test/fz_http/devices_test.exs b/apps/fz_http/test/fz_http/devices_test.exs index ebf79c391..3b81d5634 100644 --- a/apps/fz_http/test/fz_http/devices_test.exs +++ b/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) From f5bf09b2c9b518fe9a06576bc51fb336fc2dc1a4 Mon Sep 17 00:00:00 2001 From: Jamil Bou Kheir Date: Tue, 28 Dec 2021 18:58:59 -0600 Subject: [PATCH 2/2] Allow fqdn endpoint in settings --- apps/fz_http/lib/fz_http/settings/setting.ex | 4 +- apps/fz_http/test/fz_http/settings_test.exs | 39 +++++++++++++------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/apps/fz_http/lib/fz_http/settings/setting.ex b/apps/fz_http/lib/fz_http/settings/setting.ex index 4863ebef8..35e040f75 100644 --- a/apps/fz_http/lib/fz_http/settings/setting.ex +++ b/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 diff --git a/apps/fz_http/test/fz_http/settings_test.exs b/apps/fz_http/test/fz_http/settings_test.exs index 3601c34d6..bbfc1ddb3 100644 --- a/apps/fz_http/test/fz_http/settings_test.exs +++ b/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