Disallow netmasks in devices' ip to prevent overlaps in firewall (#1140)

* fix: disallow netmasks in devices' ip to prevent overlaps in firewall

/32 is still allowed for ipv4 and /128 is still allowed for ipv6.
No migration is needed since netmasks for devices ips were broken
before-hand.

* chore: rename `validate_single_ip` function and reword its error message

* Update pre-commit to allow slash

* fix restore cache

* Refactor a bit

* Fix clobbered restore-keys

* Break cache

Co-authored-by: Jamil Bou Kheir <jamilbk@users.noreply.github.com>

.github/workflows/static_analysis.yml

@@ -33,7 +33,6 @@ jobs:
           key: ${{ runner.os }}-mix-otp-25-${{ hashFiles('**/mix.lock') }}
           restore-keys: |
             ${{ runner.os }}-mix-otp-25-${{ hashFiles('**/mix.lock') }}
-            ${{ runner.os }}-mix-otp-25-
       - uses: actions/cache@v3.0.11
         name: Setup ruby cache
         with:

.pre-commit-config.yaml

@@ -61,4 +61,4 @@ repos:
           - -b
           - master
           - --pattern
-          - '^(?!((chore|feat|feature|bug|fix|build|ci|docs|style|refactor|perf|test|revert)\/[a-zA-Z0-9\-\.]+)$).*'
+          - '^(?!((chore|feat|feature|bug|fix|build|ci|docs|style|refactor|perf|test|revert)\/[a-zA-Z0-9\-\.\/]+)$).*'

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

@@ -13,6 +13,7 @@ defmodule FzHttp.Devices.Device do
       validate_fqdn_or_ip: 2,
       validate_omitted: 2,
       validate_no_duplicates: 2,
+      validate_no_mask: 2,
       validate_list_of_ips_or_cidrs: 2
     ]
 
@@ -155,6 +156,8 @@ defmodule FzHttp.Devices.Device do
     |> validate_exclusion(:ipv6, [ipv6_address()])
     |> validate_in_network(:ipv4)
     |> validate_in_network(:ipv6)
+    |> validate_no_mask(:ipv4)
+    |> validate_no_mask(:ipv6)
     |> unique_constraint(:public_key)
     |> unique_constraint([:user_id, :name])
   end

apps/fz_http/lib/fz_http/validators/common.ex

@@ -110,6 +110,22 @@ defmodule FzHttp.Validators.Common do
     end)
   end
 
+  def validate_no_mask(%Ecto.Changeset{changes: %{ipv4: %{netmask: nm}}} = changeset, :ipv4)
+      when nm in [nil, 32],
+      do: changeset
+
+  def validate_no_mask(%Ecto.Changeset{changes: %{ipv6: %{netmask: nm}}} = changeset, :ipv6)
+      when nm in [nil, 128],
+      do: changeset
+
+  def validate_no_mask(%Ecto.Changeset{changes: %{ipv4: %{netmask: _}}} = changeset, :ipv4),
+    do: netmask_error(changeset, :ipv4)
+
+  def validate_no_mask(%Ecto.Changeset{changes: %{ipv6: %{netmask: _}}} = changeset, :ipv6),
+    do: netmask_error(changeset, :ipv6)
+
+  def validate_no_mask(changeset, _), do: changeset
+
   defp split_comma_list(text) do
     text
     |> String.split(",")
@@ -123,4 +139,12 @@ defmodule FzHttp.Validators.Common do
       []
     end
   end
+
+  defp netmask_error(changeset, ip_type) do
+    add_error(
+      changeset,
+      ip_type,
+      "Only IPs without netmask are supported."
+    )
+  end
 end

apps/fz_http/test/fz_http/devices_test.exs

@@ -251,6 +251,38 @@ defmodule FzHttp.DevicesTest do
       %{use_site_mtu: true, mtu: 1000}
     ]
 
+    test "updates device with /32 netmask", %{device: device} do
+      ipv4 = "10.3.2.9/32"
+      {:ok, test_device} = Devices.update_device(device, %{ipv4: ipv4})
+      assert "#{test_device.ipv4}" == "10.3.2.9"
+    end
+
+    test "updates device with /128 netmask", %{device: device} do
+      ipv6 = "fd00::3:2:9/128"
+      {:ok, test_device} = Devices.update_device(device, %{ipv6: ipv6})
+      assert "#{test_device.ipv6}" == "fd00::3:2:9"
+    end
+
+    test "prevents updating device with ipv4 netmask", %{device: device} do
+      attrs = %{ipv4: "10.3.2.9/24"}
+      {:error, changeset} = Devices.update_device(device, attrs)
+
+      assert changeset.errors[:ipv4] == {
+               "Only IPs without netmask are supported.",
+               []
+             }
+    end
+
+    test "prevents updating device with ipv6 netmask", %{device: device} do
+      attrs = %{ipv6: "fd00::3:2:9/120"}
+      {:error, changeset} = Devices.update_device(device, attrs)
+
+      assert changeset.errors[:ipv6] == {
+               "Only IPs without netmask are supported.",
+               []
+             }
+    end
+
     test "updates device", %{device: device} do
       {:ok, test_device} = Devices.update_device(device, @attrs)
       assert @attrs = test_device

mix.lock

@@ -14,7 +14,7 @@
   "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"},
   "cowlib": {:hex, :cowlib, "2.11.0", "0b9ff9c346629256c42ebe1eeb769a83c6cb771a6ee5960bd110ab0b9b872063", [:make, :rebar3], [], "hexpm", "2b3e9da0b21c4565751a6d4901c20d1b4cc25cbb7fd50d91d2ab6dd287bc86a9"},
   "credo": {:hex, :credo, "1.6.7", "323f5734350fd23a456f2688b9430e7d517afb313fbd38671b8a4449798a7854", [:mix], [{:bunt, "~> 0.2.1", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "41e110bfb007f7eda7f897c10bf019ceab9a0b269ce79f015d54b0dcf4fc7dd3"},
-  "db_connection": {:hex, :db_connection, "2.4.2", "f92e79aff2375299a16bcb069a14ee8615c3414863a6fef93156aee8e86c2ff3", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "4fe53ca91b99f55ea249693a0229356a08f4d1a7931d8ffa79289b145fe83668"},
+  "db_connection": {:hex, :db_connection, "2.4.3", "3b9aac9f27347ec65b271847e6baeb4443d8474289bd18c1d6f4de655b70c94d", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "c127c15b0fa6cfb32eed07465e05da6c815b032508d4ed7c116122871df73c12"},
   "decimal": {:hex, :decimal, "2.0.0", "a78296e617b0f5dd4c6caf57c714431347912ffb1d0842e998e9792b5642d697", [:mix], [], "hexpm", "34666e9c55dea81013e77d9d87370fe6cb6291d1ef32f46a1600230b1d44f577"},
   "dialyxir": {:hex, :dialyxir, "1.2.0", "58344b3e87c2e7095304c81a9ae65cb68b613e28340690dfe1a5597fd08dec37", [:mix], [{:erlex, ">= 0.2.6", [hex: :erlex, repo: "hexpm", optional: false]}], "hexpm", "61072136427a851674cab81762be4dbeae7679f85b1272b6d25c3a839aff8463"},
   "earmark_parser": {:hex, :earmark_parser, "1.4.29", "149d50dcb3a93d9f3d6f3ecf18c918fb5a2d3c001b5d3305c926cddfbd33355b", [:mix], [], "hexpm", "4902af1b3eb139016aed210888748db8070b8125c2342ce3dcae4f38dcc63503"},
@@ -25,7 +25,7 @@
   "erlex": {:hex, :erlex, "0.2.6", "c7987d15e899c7a2f34f5420d2a2ea0d659682c06ac607572df55a43753aa12e", [:mix], [], "hexpm", "2ed2e25711feb44d52b17d2780eabf998452f6efda104877a3881c2f8c0c0c75"},
   "esaml": {:git, "https://github.com/firezone/esaml.git", "4294a3ac5262582144e117c10a1537287b6c1fe8", []},
   "eternal": {:hex, :eternal, "1.2.2", "d1641c86368de99375b98d183042dd6c2b234262b8d08dfd72b9eeaafc2a1abd", [:mix], [], "hexpm", "2c9fe32b9c3726703ba5e1d43a1d255a4f3f2d8f8f9bc19f094c7cb1a7a9e782"},
-  "ex_doc": {:hex, :ex_doc, "0.29.0", "4a1cb903ce746aceef9c1f9ae8a6c12b742a5461e6959b9d3b24d813ffbea146", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "f096adb8bbca677d35d278223361c7792d496b3fc0d0224c9d4bc2f651af5db1"},
+  "ex_doc": {:hex, :ex_doc, "0.29.1", "b1c652fa5f92ee9cf15c75271168027f92039b3877094290a75abcaac82a9f77", [:mix], [{:earmark_parser, "~> 1.4.19", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_elixir, "~> 0.14", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1", [hex: :makeup_erlang, repo: "hexpm", optional: false]}], "hexpm", "b7745fa6374a36daf484e2a2012274950e084815b936b1319aeebcf7809574f6"},
   "excoveralls": {:hex, :excoveralls, "0.15.0", "ac941bf85f9f201a9626cc42b2232b251ad8738da993cf406a4290cacf562ea4", [:mix], [{:hackney, "~> 1.16", [hex: :hackney, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "9631912006b27eca30a2f3c93562bc7ae15980afb014ceb8147dc5cdd8f376f1"},
   "file_size": {:hex, :file_size, "3.0.1", "ad447a69442a82fc701765a73992d7b1110136fa0d4a9d3190ea685d60034dcd", [:mix], [{:decimal, ">= 1.0.0 and < 3.0.0", [hex: :decimal, repo: "hexpm", optional: true]}, {:number, "~> 1.0", [hex: :number, repo: "hexpm", optional: false]}], "hexpm", "64dd665bc37920480c249785788265f5d42e98830d757c6a477b3246703b8e20"},
   "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},