mirror of
https://github.com/outbackdingo/firezone.git
synced 2026-01-27 10:18:54 +00:00
fix(portal): prevent resource addresses like **test.com (#9384)
These aren't allowed on the clients, causing an error. Fixes: #9054 --------- Signed-off-by: Jamil <jamilbk@users.noreply.github.com> Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
This commit is contained in:
@@ -123,7 +123,13 @@ defmodule Domain.Resources.Resource.Changeset do
|
||||
defp validate_dns_address(changeset) do
|
||||
changeset
|
||||
|> validate_length(:address, min: 1, max: 253)
|
||||
# Reject IPs (IPv4 and IPv6)
|
||||
|> validate_not_an_ip_address()
|
||||
|> validate_contains_only_valid_dns_characters()
|
||||
|> validate_dns_parts()
|
||||
end
|
||||
|
||||
defp validate_not_an_ip_address(changeset) do
|
||||
changeset
|
||||
|> validate_change(:address, fn field, address ->
|
||||
cond do
|
||||
String.match?(address, ~r/^(\d+\.){3}\d+(\/\d+)?$/) ->
|
||||
@@ -139,12 +145,20 @@ defmodule Domain.Resources.Resource.Changeset do
|
||||
[]
|
||||
end
|
||||
end)
|
||||
end
|
||||
|
||||
defp validate_contains_only_valid_dns_characters(changeset) do
|
||||
changeset
|
||||
|> validate_format(
|
||||
:address,
|
||||
~r/^[a-zA-Z0-9\p{L}\-*?]+(?:\.[a-zA-Z0-9\p{L}\-*?]+)*$/u,
|
||||
~r/^(?:[a-zA-Z0-9\p{L}*?](?:[a-zA-Z0-9\p{L}\-*?]*[a-zA-Z0-9\p{L}*?])?)(?:\.(?:[a-zA-Z0-9\p{L}*?](?:[a-zA-Z0-9\p{L}\-*?]*[a-zA-Z0-9\p{L}*?])?))*$/u,
|
||||
message:
|
||||
"must be a valid hostname (letters, digits, hyphens, dots; wildcards *, ?, ** allowed)"
|
||||
)
|
||||
end
|
||||
|
||||
defp validate_dns_parts(changeset) do
|
||||
changeset
|
||||
|> validate_change(:address, fn field, dns_address ->
|
||||
parts = String.split(dns_address, ".")
|
||||
|
||||
@@ -155,6 +169,9 @@ defmodule Domain.Resources.Resource.Changeset do
|
||||
end
|
||||
|
||||
cond do
|
||||
Enum.any?(parts, &(String.length(&1) > 63)) ->
|
||||
[{field, "each label must not exceed 63 characters"}]
|
||||
|
||||
String.contains?(tld, ["*", "?"]) ->
|
||||
[{field, "TLD cannot contain wildcards"}]
|
||||
|
||||
@@ -164,6 +181,9 @@ defmodule Domain.Resources.Resource.Changeset do
|
||||
"#{tld} cannot be used as a TLD. Try adding a DNS alias to /etc/hosts on the Gateway(s) instead"}
|
||||
]
|
||||
|
||||
Enum.any?(domain_parts, fn part -> String.contains?(part, "**") and part != "**" end) ->
|
||||
[{field, "wildcard pattern must not contain ** in the middle of a label"}]
|
||||
|
||||
Enum.all?(parts, &(&1 == "*")) ->
|
||||
[{field, "wildcard pattern must include a valid domain"}]
|
||||
|
||||
|
||||
@@ -2,6 +2,91 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
use Domain.DataCase, async: true
|
||||
import Domain.Resources.Resource.Changeset
|
||||
|
||||
@valid_dns_addresses [
|
||||
"**.foo.google.com",
|
||||
"?.foo.google.com",
|
||||
"web-*.foo.google.com",
|
||||
"web-?.foo.google.com",
|
||||
"web-*.google.com",
|
||||
"web-?.google.com",
|
||||
"sub.**.example.com",
|
||||
"sub.**.foo.bar.google.com",
|
||||
"**.google.com",
|
||||
"?.google.com",
|
||||
"*.*.*.*.google.com",
|
||||
"?.?.?.?.google.com",
|
||||
"*.google",
|
||||
"?.google",
|
||||
"google",
|
||||
"example.com",
|
||||
"example.weird",
|
||||
"1234567890.com",
|
||||
"#{String.duplicate("a", 63)}.com",
|
||||
"такі.справи",
|
||||
"subdomain.subdomain2.example.space",
|
||||
"single.label",
|
||||
"a-b-c.d-e-f.g-h-i",
|
||||
"xn--fssq61j.com",
|
||||
"*.xn--fssq61j.com",
|
||||
"**.xn--fssq61j.com",
|
||||
"sub.**.xn--fssq61j.com",
|
||||
"a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.com",
|
||||
"sub.domain",
|
||||
"sub-domain.com",
|
||||
"sub--domain.com"
|
||||
]
|
||||
|
||||
@invalid_dns_addresses [
|
||||
"2600::1/32",
|
||||
"2600::1",
|
||||
"1.1.1.1/32",
|
||||
"1.1.1.1",
|
||||
".example.com",
|
||||
"example..com",
|
||||
"**example.com",
|
||||
"example. com.**",
|
||||
"example.com.**",
|
||||
"foo.**bar.com",
|
||||
"**",
|
||||
"**.",
|
||||
"*.",
|
||||
"*.*",
|
||||
"?",
|
||||
"?.",
|
||||
"example.com.",
|
||||
"exa&mple.com",
|
||||
"",
|
||||
"http://example.com/",
|
||||
"//example.com/",
|
||||
"example.com/",
|
||||
".example.com",
|
||||
"example.",
|
||||
"example.com:80",
|
||||
"-example.com",
|
||||
"example-.com",
|
||||
"example_com",
|
||||
"example..com",
|
||||
"too.long.#{String.duplicate("a", 256)}",
|
||||
"a.#{String.duplicate("a", 64)}.com",
|
||||
"example.com/",
|
||||
"example.com?",
|
||||
"example.com*",
|
||||
"*.com.",
|
||||
"**.com.",
|
||||
"foo.**",
|
||||
"foo**.com",
|
||||
"**.example.com.",
|
||||
"example..**",
|
||||
"**example.com",
|
||||
"***.foo.com",
|
||||
"**.**test.com",
|
||||
"**.foo**test.com",
|
||||
"foo.**bar",
|
||||
"**test",
|
||||
"*.com",
|
||||
"?.com"
|
||||
]
|
||||
|
||||
setup do
|
||||
account = Fixtures.Accounts.create_account()
|
||||
actor = Fixtures.Actors.create_actor(type: :account_admin_user, account: account)
|
||||
@@ -30,7 +115,9 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
})
|
||||
|
||||
assert changeset.changes[:address] == cidr
|
||||
assert changeset.valid?
|
||||
|
||||
assert changeset.valid?,
|
||||
"Expected '#{string}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
@@ -48,7 +135,8 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
"2607:f8b0:4012:0::200e/128:80"
|
||||
]
|
||||
|> Enum.each(fn string ->
|
||||
refute create(%{type: :cidr, address: string}).valid?
|
||||
refute create(%{type: :cidr, address: string}).valid?,
|
||||
"Expected '#{string}' to be invalid"
|
||||
end)
|
||||
end
|
||||
|
||||
@@ -68,7 +156,9 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
})
|
||||
|
||||
assert changeset.changes[:address] == ip
|
||||
assert changeset.valid?
|
||||
|
||||
assert changeset.valid?,
|
||||
"Expected '#{string}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
@@ -83,33 +173,13 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
"[2607:f8b0:4012:0::200e]:80"
|
||||
]
|
||||
|> Enum.each(fn string ->
|
||||
refute create(%{type: :ip, address: string}).valid?
|
||||
refute create(%{type: :ip, address: string}).valid?,
|
||||
"Expected '#{string}' to be invalid"
|
||||
end)
|
||||
end
|
||||
|
||||
test "accepts valid DNS addresses" do
|
||||
for valid_address <- [
|
||||
"**.foo.google.com",
|
||||
"?.foo.google.com",
|
||||
"web-*.foo.google.com",
|
||||
"web-?.foo.google.com",
|
||||
"web-*.google.com",
|
||||
"web-?.google.com",
|
||||
"**.*.?.foo.foo.com",
|
||||
"**.google.com",
|
||||
"?.google.com",
|
||||
"*.*.*.*.google.com",
|
||||
"?.?.?.?.google.com",
|
||||
"*.google",
|
||||
"?.google",
|
||||
"google",
|
||||
"example.com",
|
||||
"example.weird",
|
||||
"1234567890.com",
|
||||
"#{String.duplicate("a", 63)}.com",
|
||||
"такі.справи",
|
||||
"subdomain.subdomain2.example.space"
|
||||
] do
|
||||
for valid_address <- @valid_dns_addresses do
|
||||
changeset =
|
||||
create(%{
|
||||
name: "foo",
|
||||
@@ -118,33 +188,14 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
address_description: valid_address
|
||||
})
|
||||
|
||||
assert changeset.valid?
|
||||
assert changeset.valid?,
|
||||
"Expected '#{valid_address}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
"1.1.1.1/32",
|
||||
"1.1.1.1",
|
||||
".example.com",
|
||||
"example..com",
|
||||
"**",
|
||||
"**.",
|
||||
"*.",
|
||||
"*.*",
|
||||
"?",
|
||||
"?.",
|
||||
"example.com.",
|
||||
"exa&mple.com",
|
||||
"",
|
||||
"http://example.com/",
|
||||
"//example.com/",
|
||||
"example.com/",
|
||||
".example.com",
|
||||
"example.",
|
||||
"example.com:80"
|
||||
]
|
||||
|> Enum.each(fn invalid_address ->
|
||||
refute create(%{type: :dns, address: invalid_address}).valid?
|
||||
end)
|
||||
for invalid_address <- @invalid_dns_addresses do
|
||||
changeset = create(%{type: :dns, address: invalid_address})
|
||||
refute changeset.valid?, "Expected '#{invalid_address}' to be invalid"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -170,7 +221,9 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
)
|
||||
|
||||
assert changeset.changes.address == cidr
|
||||
assert changeset.valid?
|
||||
|
||||
assert changeset.valid?,
|
||||
"Expected '#{string}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
@@ -189,7 +242,7 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
]
|
||||
|> Enum.each(fn invalid_cidr ->
|
||||
{changeset, _} = update(resource, %{type: :cidr, address: invalid_cidr}, subject)
|
||||
refute changeset.valid?
|
||||
refute changeset.valid?, "Expected '#{invalid_cidr}' to be invalid"
|
||||
end)
|
||||
end
|
||||
|
||||
@@ -213,7 +266,9 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
)
|
||||
|
||||
assert changeset.changes.address == ip
|
||||
assert changeset.valid?
|
||||
|
||||
assert changeset.valid?,
|
||||
"Expected '#{string}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
@@ -229,33 +284,12 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
]
|
||||
|> Enum.each(fn invalid_ip ->
|
||||
{changeset, _} = update(resource, %{type: :ip, address: invalid_ip}, subject)
|
||||
refute changeset.valid?
|
||||
refute changeset.valid?, "Expected '#{invalid_ip}' to be invalid"
|
||||
end)
|
||||
end
|
||||
|
||||
test "accepts valid DNS addresses", %{resource: resource, subject: subject} do
|
||||
for valid_address <- [
|
||||
"**.foo.google.com",
|
||||
"?.foo.google.com",
|
||||
"web-*.foo.google.com",
|
||||
"web-?.foo.google.com",
|
||||
"web-*.google.com",
|
||||
"web-?.google.com",
|
||||
"**.*.?.foo.foo.com",
|
||||
"**.google.com",
|
||||
"?.google.com",
|
||||
"*.*.*.*.google.com",
|
||||
"?.?.?.?.google.com",
|
||||
"*.google",
|
||||
"?.google",
|
||||
"google",
|
||||
"example.com",
|
||||
"example.weird",
|
||||
"1234567890.com",
|
||||
"#{String.duplicate("a", 63)}.com",
|
||||
"такі.справи",
|
||||
"subdomain.subdomain2.example.space"
|
||||
] do
|
||||
for valid_address <- @valid_dns_addresses do
|
||||
{changeset, _} =
|
||||
update(
|
||||
resource,
|
||||
@@ -268,36 +302,14 @@ defmodule Domain.Resources.Resource.ChangesetTest do
|
||||
subject
|
||||
)
|
||||
|
||||
assert changeset.valid?
|
||||
assert changeset.valid?,
|
||||
"Expected '#{valid_address}' to be valid, got: #{inspect(changeset.errors)}"
|
||||
end
|
||||
|
||||
[
|
||||
"2600::1/32",
|
||||
"2600::1",
|
||||
"1.1.1.1/32",
|
||||
"1.1.1.1",
|
||||
".example.com",
|
||||
"example..com",
|
||||
"**",
|
||||
"**.",
|
||||
"*.",
|
||||
"*.*",
|
||||
"?",
|
||||
"?.",
|
||||
"example.com.",
|
||||
"exa&mple.com",
|
||||
"",
|
||||
"http://example.com/",
|
||||
"//example.com/",
|
||||
"example.com/",
|
||||
".example.com",
|
||||
"example.",
|
||||
"example.com:80"
|
||||
]
|
||||
|> Enum.each(fn invalid_address ->
|
||||
for invalid_address <- @invalid_dns_addresses do
|
||||
{changeset, _} = update(resource, %{type: :dns, address: invalid_address}, subject)
|
||||
refute changeset.valid?
|
||||
end)
|
||||
refute changeset.valid?, "Expected '#{invalid_address}' to be invalid"
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user