From b8b52c1f07ba21f298e96f73b57426ce8b3c12ab Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Tue, 4 Nov 2025 15:44:57 +1100 Subject: [PATCH] fix(portal): do not allow ports for upstream DNS servers (#10772) DNS servers are standarised to be contacted on port 53. This is also hard-coded within `connlib` when we contact an upstream server. As such, we should disallow users inputting any custom port for upstream DNS servers. Luckily - or perhaps because it doesn't presently work - no users in production have actually put in a port. Resolves: #8330 --- .../apps/api/test/api/client/channel_test.exs | 2 +- .../lib/domain/accounts/config/changeset.ex | 3 ++- .../apps/domain/test/domain/accounts_test.exs | 22 ++++++++++++++++++- .../domain/test/support/fixtures/accounts.ex | 2 +- .../web/test/web/live/settings/dns_test.exs | 2 +- 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/elixir/apps/api/test/api/client/channel_test.exs b/elixir/apps/api/test/api/client/channel_test.exs index df6d92b81..2355cdc18 100644 --- a/elixir/apps/api/test/api/client/channel_test.exs +++ b/elixir/apps/api/test/api/client/channel_test.exs @@ -9,7 +9,7 @@ defmodule API.Client.ChannelTest do config: %{ clients_upstream_dns: [ %{protocol: "ip_port", address: "1.1.1.1"}, - %{protocol: "ip_port", address: "8.8.8.8:53"} + %{protocol: "ip_port", address: "8.8.8.8"} ], search_domain: "example.com" }, diff --git a/elixir/apps/domain/lib/domain/accounts/config/changeset.ex b/elixir/apps/domain/lib/domain/accounts/config/changeset.ex index d42ac229d..2468fe5dc 100644 --- a/elixir/apps/domain/lib/domain/accounts/config/changeset.ex +++ b/elixir/apps/domain/lib/domain/accounts/config/changeset.ex @@ -106,7 +106,8 @@ defmodule Domain.Accounts.Config.Changeset do defp validate_ip_port(changeset) do validate_change(changeset, :address, fn :address, address -> case IPPort.cast(address) do - {:ok, _ip} -> [] + {:ok, %IPPort{port: nil}} -> [] + {:ok, %IPPort{}} -> [address: "must not include a port"] _ -> [address: "must be a valid IP address"] end end) diff --git a/elixir/apps/domain/test/domain/accounts_test.exs b/elixir/apps/domain/test/domain/accounts_test.exs index e7f660092..b94fe5ec8 100644 --- a/elixir/apps/domain/test/domain/accounts_test.exs +++ b/elixir/apps/domain/test/domain/accounts_test.exs @@ -524,7 +524,7 @@ defmodule Domain.AccountsTest do attrs = %{ config: %{ clients_upstream_dns: [ - %{protocol: "ip_port", address: "1.1.1.1:53"}, + %{protocol: "ip_port", address: "1.1.1.1"}, %{protocol: "ip_port", address: "1.1.1.1 "} ] } @@ -539,6 +539,26 @@ defmodule Domain.AccountsTest do } end + test "does not allow ports", %{account: account} do + attrs = %{ + config: %{ + clients_upstream_dns: [ + %{protocol: "ip_port", address: "1.1.1.1:53"} + ] + } + } + + assert {:error, changeset} = update_account_by_id(account.id, attrs) + + assert errors_on(changeset) == %{ + config: %{ + clients_upstream_dns: [ + %{address: ["must not include a port"]} + ] + } + } + end + test "returns error on dns config address in IPv4 sentinel range", %{account: account} do attrs = %{ config: %{ diff --git a/elixir/apps/domain/test/support/fixtures/accounts.ex b/elixir/apps/domain/test/support/fixtures/accounts.ex index 826668d4c..6cc41f5a3 100644 --- a/elixir/apps/domain/test/support/fixtures/accounts.ex +++ b/elixir/apps/domain/test/support/fixtures/accounts.ex @@ -14,7 +14,7 @@ defmodule Domain.Fixtures.Accounts do 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"} + %{protocol: "ip_port", address: "9.9.9.9"} ] }, features: %{ diff --git a/elixir/apps/web/test/web/live/settings/dns_test.exs b/elixir/apps/web/test/web/live/settings/dns_test.exs index 8e808a887..9f1c1bc63 100644 --- a/elixir/apps/web/test/web/live/settings/dns_test.exs +++ b/elixir/apps/web/test/web/live/settings/dns_test.exs @@ -259,7 +259,7 @@ defmodule Web.Live.Settings.DNSTest do conn: conn } do addr1 = %{address: "8.8.8.8"} - addr1_dup = %{address: "8.8.8.8:53"} + addr1_dup = %{address: "8.8.8.8"} addr2 = %{address: "1.1.1.1"} attrs = %{