diff --git a/elixir/apps/api/lib/api/client/views/interface.ex b/elixir/apps/api/lib/api/client/views/interface.ex index 819b3459f..fcf27187b 100644 --- a/elixir/apps/api/lib/api/client/views/interface.ex +++ b/elixir/apps/api/lib/api/client/views/interface.ex @@ -1,10 +1,15 @@ defmodule API.Client.Views.Interface do alias Domain.Clients + alias Domain.Config.Configuration.ClientsUpstreamDNS def render(%Clients.Client{} = client) do upstream_dns = Clients.fetch_client_config!(client) |> Keyword.fetch!(:upstream_dns) + |> Enum.map(fn dns_config -> + addr = ClientsUpstreamDNS.normalize_dns_address(dns_config) + %{dns_config | address: addr} + end) %{ upstream_dns: upstream_dns, diff --git a/elixir/apps/api/test/api/client/channel_test.exs b/elixir/apps/api/test/api/client/channel_test.exs index 24c3c0fee..b81d0db8d 100644 --- a/elixir/apps/api/test/api/client/channel_test.exs +++ b/elixir/apps/api/test/api/client/channel_test.exs @@ -7,7 +7,10 @@ defmodule API.Client.ChannelTest do Fixtures.Config.upsert_configuration( account: account, - clients_upstream_dns: [%{address: "1.1.1.1"}] + clients_upstream_dns: [ + %{protocol: "ip_port", address: "1.1.1.1"}, + %{protocol: "ip_port", address: "8.8.8.8:53"} + ] ) actor_group = Fixtures.Actors.create_group(account: account) @@ -138,7 +141,14 @@ defmodule API.Client.ChannelTest do ipv4: client.ipv4, ipv6: client.ipv6, upstream_dns: [ - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "1.1.1.1"} + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "1.1.1.1:53" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8:53" + } ] } end diff --git a/elixir/apps/domain/lib/domain/config/configuration.ex b/elixir/apps/domain/lib/domain/config/configuration.ex index 79cd49760..b797848f2 100644 --- a/elixir/apps/domain/lib/domain/config/configuration.ex +++ b/elixir/apps/domain/lib/domain/config/configuration.ex @@ -1,12 +1,10 @@ defmodule Domain.Config.Configuration do use Domain, :schema alias Domain.Config.Logo + alias Domain.Config.Configuration.ClientsUpstreamDNS schema "configurations" do - embeds_many :clients_upstream_dns, ClientsUpstreamDNS, on_replace: :delete, primary_key: false do - field :address, :string - end - + embeds_many :clients_upstream_dns, ClientsUpstreamDNS, on_replace: :delete embeds_one :logo, Logo, on_replace: :delete belongs_to :account, Domain.Accounts.Account diff --git a/elixir/apps/domain/lib/domain/config/configuration/changeset.ex b/elixir/apps/domain/lib/domain/config/configuration/changeset.ex index 35b029298..5e1848920 100644 --- a/elixir/apps/domain/lib/domain/config/configuration/changeset.ex +++ b/elixir/apps/domain/lib/domain/config/configuration/changeset.ex @@ -1,6 +1,7 @@ defmodule Domain.Config.Configuration.Changeset do use Domain, :changeset import Domain.Config, only: [config_changeset: 2] + alias Domain.Config.Configuration.ClientsUpstreamDNS @fields ~w[clients_upstream_dns logo]a @@ -9,10 +10,8 @@ defmodule Domain.Config.Configuration.Changeset do configuration |> cast(attrs, []) |> cast_embed(:logo) - |> cast_embed( - :clients_upstream_dns, - with: &clients_upstream_dns_changeset/2 - ) + |> cast_embed(:clients_upstream_dns) + |> validate_unique_dns() Enum.reduce(@fields, changeset, fn field, changeset -> config_changeset(changeset, field) @@ -20,21 +19,20 @@ defmodule Domain.Config.Configuration.Changeset do |> ensure_no_overridden_changes(configuration.account_id) end - def clients_upstream_dns_changeset( - dns_config \\ %Domain.Config.Configuration.ClientsUpstreamDNS{}, - attrs - ) do - Ecto.Changeset.cast( - dns_config, - attrs, - [:address] - ) - |> validate_required(:address) - |> trim_change(:address) - |> Domain.Validator.validate_one_of(:address, [ - &Domain.Validator.validate_fqdn/2, - &Domain.Validator.validate_uri(&1, &2, schemes: ["https"]) - ]) + defp validate_unique_dns(changeset) do + dns_addrs = + apply_changes(changeset) + |> Map.get(:clients_upstream_dns) + |> Enum.map(&ClientsUpstreamDNS.normalize_dns_address/1) + |> Enum.reject(&is_nil/1) + + duplicates = dns_addrs -- Enum.uniq(dns_addrs) + + if duplicates != [] do + add_error(changeset, :clients_upstream_dns, "no duplicates allowed") + else + changeset + end end defp ensure_no_overridden_changes(changeset, account_id) do diff --git a/elixir/apps/domain/lib/domain/config/configuration/clients_upstream_dns.ex b/elixir/apps/domain/lib/domain/config/configuration/clients_upstream_dns.ex new file mode 100644 index 000000000..bd4e2bb65 --- /dev/null +++ b/elixir/apps/domain/lib/domain/config/configuration/clients_upstream_dns.ex @@ -0,0 +1,63 @@ +defmodule Domain.Config.Configuration.ClientsUpstreamDNS do + @moduledoc """ + Embedded Schema for Clients Upstream DNS + """ + use Domain, :schema + import Domain.Validator + import Ecto.Changeset + alias Domain.Types.IPPort + + @primary_key false + embedded_schema do + field :protocol, Ecto.Enum, values: [:ip_port, :dns_over_tls, :dns_over_http] + field :address, :string + end + + def changeset(dns_config \\ %__MODULE__{}, attrs) do + dns_config + |> cast(attrs, [:protocol, :address]) + |> validate_required([:protocol, :address]) + |> trim_change(:address) + |> validate_inclusion(:protocol, supported_protocols(), + message: "this type of DNS provider is not supported yet" + ) + |> validate_address() + end + + def supported_protocols do + ~w[ip_port]a + end + + def validate_address(changeset) do + if has_errors?(changeset, :protocol) do + changeset + else + case fetch_field(changeset, :protocol) do + {_changes_or_data, :ip_port} -> validate_ip_port(changeset) + :error -> changeset + end + end + end + + def validate_ip_port(changeset) do + validate_change(changeset, :address, fn :address, address -> + case IPPort.cast(address) do + {:ok, _ip} -> [] + _ -> [address: "must be a valid IP address"] + end + end) + end + + def normalize_dns_address(%__MODULE__{protocol: :ip_port, address: address}) do + case IPPort.cast(address) do + {:ok, ip} -> IPPort.put_default_port(ip, default_dns_port()) |> to_string() + _ -> address + end + end + + def normalize_dns_address(%__MODULE__{protocol: _, address: address}) do + address + end + + def default_dns_port, do: 53 +end diff --git a/elixir/apps/domain/lib/domain/config/definitions.ex b/elixir/apps/domain/lib/domain/config/definitions.ex index f7169eb9e..0e0a2f1c9 100644 --- a/elixir/apps/domain/lib/domain/config/definitions.ex +++ b/elixir/apps/domain/lib/domain/config/definitions.ex @@ -454,9 +454,9 @@ defmodule Domain.Config.Definitions do defconfig( :clients_upstream_dns, {:json_array, {:embed, Domain.Config.Configuration.ClientsUpstreamDNS}, - validate_unique: true}, + validate_unique: false}, default: [], - changeset: {Domain.Config.Configuration.Changeset, :clients_upstream_dns_changeset, []} + changeset: {Domain.Config.Configuration.ClientsUpstreamDNS, :changeset, []} ) ############################################## diff --git a/elixir/apps/domain/lib/domain/types/ip_port.ex b/elixir/apps/domain/lib/domain/types/ip_port.ex index 7b5e78581..a1785b3b7 100644 --- a/elixir/apps/domain/lib/domain/types/ip_port.ex +++ b/elixir/apps/domain/lib/domain/types/ip_port.ex @@ -44,7 +44,7 @@ defmodule Domain.Types.IPPort do def cast_address(address) do address |> String.to_charlist() - |> :inet.parse_address() + |> :inet.parse_strict_address() end defp cast_port(nil), do: {:ok, nil} @@ -88,4 +88,9 @@ defmodule Domain.Types.IPPort do ip = ip |> :inet.ntoa() |> List.to_string() "[#{ip}]:#{port}" end + + def put_default_port(ip_port, default_port) do + port = ip_port.port || default_port + %{ip_port | port: port} + end end diff --git a/elixir/apps/domain/test/domain/config_test.exs b/elixir/apps/domain/test/domain/config_test.exs index da6741ba9..c801166a3 100644 --- a/elixir/apps/domain/test/domain/config_test.exs +++ b/elixir/apps/domain/test/domain/config_test.exs @@ -93,7 +93,18 @@ defmodule Domain.ConfigTest do assert fetch_resolved_configs!(account.id, [:clients_upstream_dns, :clients_upstream_dns]) == %{ clients_upstream_dns: [ - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "1.1.1.1"} + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "1.1.1.1" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "2606:4700:4700::1111" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8:853" + } ] } end @@ -143,7 +154,20 @@ defmodule Domain.ConfigTest do %{ clients_upstream_dns: {{:db, :clients_upstream_dns}, - [%Domain.Config.Configuration.ClientsUpstreamDNS{address: "1.1.1.1"}]} + [ + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "1.1.1.1" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "2606:4700:4700::1111" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8:853" + } + ]} } end @@ -444,25 +468,25 @@ defmodule Domain.ConfigTest do config = get_account_config_by_account_id(account.id) attrs = %{ - clients_upstream_dns: [%{address: "!!!"}] + clients_upstream_dns: [%{protocol: "ip_port", address: "!!!"}] } assert {:error, changeset} = update_config(config, attrs) assert errors_on(changeset) == %{ clients_upstream_dns: [ - %{address: ["does not contain a scheme or a host", "!!! is not a valid FQDN"]} + %{address: ["must be a valid IP address"]} ] } end test "returns error when trying to change overridden value", %{account: account} do - put_system_env_override(:clients_upstream_dns, [%{address: "1.2.3.4"}]) + put_system_env_override(:clients_upstream_dns, [%{protocol: "ip_port", address: "1.2.3.4"}]) config = get_account_config_by_account_id(account.id) attrs = %{ - clients_upstream_dns: [%{address: "4.1.2.3"}] + clients_upstream_dns: [%{protocol: "ip_port", address: "4.1.2.3"}] } assert {:error, changeset} = update_config(config, attrs) @@ -479,25 +503,47 @@ defmodule Domain.ConfigTest do config = get_account_config_by_account_id(account.id) attrs = %{ - clients_upstream_dns: [%{address: " foobar.com"}, %{address: "google.com "}] + clients_upstream_dns: [ + %{protocol: "ip_port", address: " 1.1.1.1"}, + %{protocol: "ip_port", address: "8.8.8.8 "} + ] } assert {:ok, config} = update_config(config, attrs) assert config.clients_upstream_dns == [ - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "foobar.com"}, - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "google.com"} + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "1.1.1.1" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8" + } ] end test "changes database config value when it did not exist", %{account: account} do config = get_account_config_by_account_id(account.id) - attrs = %{clients_upstream_dns: [%{address: "foobar.com"}, %{address: "google.com"}]} + + attrs = %{ + clients_upstream_dns: [ + %{protocol: "ip_port", address: "1.1.1.1"}, + %{protocol: "ip_port", address: "8.8.8.8"} + ] + } + assert {:ok, config} = update_config(config, attrs) assert config.clients_upstream_dns == [ - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "foobar.com"}, - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "google.com"} + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "1.1.1.1" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8" + } ] end @@ -505,12 +551,25 @@ defmodule Domain.ConfigTest do Fixtures.Config.upsert_configuration(account: account) config = get_account_config_by_account_id(account.id) - attrs = %{clients_upstream_dns: [%{address: "foobar.com"}, %{address: "google.com"}]} + + attrs = %{ + clients_upstream_dns: [ + %{protocol: "ip_port", address: "8.8.8.8"}, + %{protocol: "ip_port", address: "8.8.4.4"} + ] + } + assert {:ok, config} = update_config(config, attrs) assert config.clients_upstream_dns == [ - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "foobar.com"}, - %Domain.Config.Configuration.ClientsUpstreamDNS{address: "google.com"} + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.8.8" + }, + %Domain.Config.Configuration.ClientsUpstreamDNS{ + protocol: :ip_port, + address: "8.8.4.4" + } ] end end diff --git a/elixir/apps/domain/test/domain/types/ip_port_test.exs b/elixir/apps/domain/test/domain/types/ip_port_test.exs new file mode 100644 index 000000000..e3be5a6cb --- /dev/null +++ b/elixir/apps/domain/test/domain/types/ip_port_test.exs @@ -0,0 +1,47 @@ +defmodule Domain.Types.IPPortTest do + use ExUnit.Case, async: true + import Domain.Types.IPPort + + describe "cast_address/1" do + test "uses strict parsing for IP addresses" do + assert cast_address("1.1.1") == {:error, :einval} + assert cast_address("1.1.1.1") == {:ok, {1, 1, 1, 1}} + end + end + + describe "cast/1" do + test "only allows port numbers between 1 - 65_535" do + assert cast("1.1.1.1:0") == {:error, [message: "is invalid"]} + + assert cast("1.1.1.1:1") == + {:ok, %Domain.Types.IPPort{type: :ipv4, address: {1, 1, 1, 1}, port: 1}} + + assert cast("1.1.1.1:65535") == + {:ok, %Domain.Types.IPPort{type: :ipv4, address: {1, 1, 1, 1}, port: 65_535}} + + assert cast("1.1.1.1:65536") == {:error, [message: "is invalid"]} + end + end + + describe "put_default_port/1" do + test "sets default port when one does not exist" do + {:ok, ip_port} = cast("1.1.1.1") + + assert put_default_port(ip_port, 53) == %Domain.Types.IPPort{ + type: :ipv4, + address: {1, 1, 1, 1}, + port: 53 + } + end + + test "does not set default port when port exists" do + {:ok, ip_port} = cast("1.1.1.1:853") + + assert put_default_port(ip_port, 53) == %Domain.Types.IPPort{ + type: :ipv4, + address: {1, 1, 1, 1}, + port: 853 + } + end + end +end diff --git a/elixir/apps/domain/test/support/fixtures/config.ex b/elixir/apps/domain/test/support/fixtures/config.ex index a6cf1a67d..bcc216324 100644 --- a/elixir/apps/domain/test/support/fixtures/config.ex +++ b/elixir/apps/domain/test/support/fixtures/config.ex @@ -4,7 +4,11 @@ defmodule Domain.Fixtures.Config do def configuration_attrs(attrs \\ %{}) do Enum.into(attrs, %{ - clients_upstream_dns: [%{address: "1.1.1.1"}] + clients_upstream_dns: [ + %{protocol: "ip_port", address: "1.1.1.1"}, + %{protocol: "ip_port", address: "2606:4700:4700::1111"}, + %{protocol: "ip_port", address: "8.8.8.8:853"} + ] }) end diff --git a/elixir/apps/web/lib/web/live/settings/dns.ex b/elixir/apps/web/lib/web/live/settings/dns.ex index 6c99de579..44815708e 100644 --- a/elixir/apps/web/lib/web/live/settings/dns.ex +++ b/elixir/apps/web/lib/web/live/settings/dns.ex @@ -1,6 +1,7 @@ defmodule Web.Settings.DNS do use Web, :live_view alias Domain.Config + alias Domain.Config.Configuration.ClientsUpstreamDNS def mount(_params, _session, socket) do {:ok, config} = Config.fetch_account_config(socket.assigns.subject) @@ -56,14 +57,26 @@ defmodule Web.Settings.DNS do
<.inputs_for :let={dns} field={@form[:clients_upstream_dns]}> -
- <.input label="Address" field={dns[:address]} placeholder="DNS Server Address" /> +
+
+ <.input + type="select" + label="Protocol" + field={dns[:protocol]} + placeholder="Protocol" + options={dns_options()} + value={dns[:protocol].value} + /> +
+
+ <.input label="Address" field={dns[:address]} placeholder="DNS Server Address" /> +
+ <.error :for={msg <- @errors} data-validation-error-for="clients_upstream_dns"> + <%= msg %> +
- <.error :for={msg <- @errors} data-validation-error-for="clients_upstream_dns"> - <%= msg %> - <.submit_button> Save @@ -157,4 +170,21 @@ defmodule Web.Settings.DNS do existing_servers ++ [%{address: ""}] ) end + + defp dns_options do + options = [ + [key: "IP", value: "ip_port"], + [key: "DNS over TLS", value: "dns_over_tls"], + [key: "DNS over HTTPS", value: "dns_over_https"] + ] + + supported = Enum.map(ClientsUpstreamDNS.supported_protocols(), &to_string/1) + + Enum.map(options, fn option -> + case option[:value] in supported do + true -> option + false -> option ++ [disabled: true] + end + end) + end end diff --git a/elixir/apps/web/test/web/live/settings/dns/index_test.exs b/elixir/apps/web/test/web/live/settings/dns/index_test.exs index 20b145357..f490633e4 100644 --- a/elixir/apps/web/test/web/live/settings/dns/index_test.exs +++ b/elixir/apps/web/test/web/live/settings/dns/index_test.exs @@ -52,7 +52,8 @@ defmodule Web.Live.Settings.DNS.IndexTest do assert find_inputs(form) == [ "configuration[clients_upstream_dns][0][_persistent_id]", - "configuration[clients_upstream_dns][0][address]" + "configuration[clients_upstream_dns][0][address]", + "configuration[clients_upstream_dns][0][protocol]" ] end @@ -77,8 +78,10 @@ defmodule Web.Live.Settings.DNS.IndexTest do |> find_inputs() == [ "configuration[clients_upstream_dns][0][_persistent_id]", "configuration[clients_upstream_dns][0][address]", + "configuration[clients_upstream_dns][0][protocol]", "configuration[clients_upstream_dns][1][_persistent_id]", - "configuration[clients_upstream_dns][1][address]" + "configuration[clients_upstream_dns][1][address]", + "configuration[clients_upstream_dns][1][protocol]" ] end @@ -107,8 +110,10 @@ defmodule Web.Live.Settings.DNS.IndexTest do |> find_inputs() == [ "configuration[clients_upstream_dns][0][_persistent_id]", "configuration[clients_upstream_dns][0][address]", + "configuration[clients_upstream_dns][0][protocol]", "configuration[clients_upstream_dns][1][_persistent_id]", - "configuration[clients_upstream_dns][1][address]" + "configuration[clients_upstream_dns][1][address]", + "configuration[clients_upstream_dns][1][protocol]" ] empty_attrs = %{ @@ -123,20 +128,23 @@ defmodule Web.Live.Settings.DNS.IndexTest do |> form("form") |> find_inputs() == [ "configuration[clients_upstream_dns][0][_persistent_id]", - "configuration[clients_upstream_dns][0][address]" + "configuration[clients_upstream_dns][0][address]", + "configuration[clients_upstream_dns][0][protocol]" ] end - test "warns when duplicates found", %{ + test "warns when duplicate IPv4 addresses found", %{ account: account, identity: identity, conn: conn } do - addr = %{address: "8.8.8.8"} + addr1 = %{address: "8.8.8.8"} + addr1_dup = %{address: "8.8.8.8:53"} + addr2 = %{address: "1.1.1.1"} attrs = %{ configuration: %{ - clients_upstream_dns: %{"0" => addr} + clients_upstream_dns: %{"0" => addr1} } } @@ -150,8 +158,16 @@ defmodule Web.Live.Settings.DNS.IndexTest do |> render_submit() assert lv - |> form("form", %{configuration: %{clients_upstream_dns: %{"1" => addr}}}) - |> render_change() =~ "should not contain duplicates" + |> form("form", %{configuration: %{clients_upstream_dns: %{"1" => addr1}}}) + |> render_change() =~ "no duplicates allowed" + + refute lv + |> form("form", %{configuration: %{clients_upstream_dns: %{"1" => addr2}}}) + |> render_change() =~ "no duplicates allowed" + + assert lv + |> form("form", %{configuration: %{clients_upstream_dns: %{"1" => addr1_dup}}}) + |> render_change() =~ "no duplicates allowed" end test "does not display 'cannot be empty' error message", %{