Merge branch 'fix/2209-remoteip' into 'develop'

#2209 RemoteIP CIDR helper, config and doc improvements

See merge request pleroma/pleroma!3057

config/config.exs

@@ -677,7 +677,18 @@ config :pleroma, :rate_limit,
 
 config :pleroma, Pleroma.Workers.PurgeExpiredActivity, enabled: true, min_lifetime: 600
 
-config :pleroma, Pleroma.Plugs.RemoteIp, enabled: true
+config :pleroma, Pleroma.Plugs.RemoteIp,
+  enabled: true,
+  headers: ["x-forwarded-for"],
+  proxies: [],
+  reserved: [
+    "127.0.0.0/8",
+    "::1/128",
+    "fc00::/7",
+    "10.0.0.0/8",
+    "172.16.0.0/12",
+    "192.168.0.0/16"
+  ]
 
 config :pleroma, :static_fe, enabled: false
 

config/description.exs

@@ -3265,20 +3265,22 @@ config :pleroma, :config_description, [
       %{
         key: :headers,
         type: {:list, :string},
-        description:
+        description: """
-          "A list of strings naming the `req_headers` to use when deriving the `remote_ip`. Order does not matter. Default: `~w[forwarded x-forwarded-for x-client-ip x-real-ip]`."
+          A list of strings naming the HTTP headers to use when deriving the true client IP. Default: `["x-forwarded-for"]`.
+        """
       },
       %{
         key: :proxies,
         type: {:list, :string},
         description:
-          "A list of strings in [CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of known proxies. Default: `[]`."
+          "A list of upstream proxy IP subnets in CIDR notation from which we will parse the content of `headers`. Defaults to `[]`. IPv4 entries without a bitmask will be assumed to be /32 and IPv6 /128."
       },
       %{
         key: :reserved,
         type: {:list, :string},
-        description:
+        description: """
-          "Defaults to [localhost](https://en.wikipedia.org/wiki/Localhost) and [private network](https://en.wikipedia.org/wiki/Private_network)."
+          A list of reserved IP subnets in CIDR notation which should be ignored if found in `headers`. Defaults to `["127.0.0.0/8", "::1/128", "fc00::/7", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"]`
+        """
       }
     ]
   },

docs/configuration/cheatsheet.md

@@ -426,9 +426,9 @@ This will make Pleroma listen on `127.0.0.1` port `8080` and generate urls start
 Available options:
 
 * `enabled` - Enable/disable the plug. Defaults to `false`.
-* `headers` - A list of strings naming the `req_headers` to use when deriving the `remote_ip`. Order does not matter. Defaults to `["x-forwarded-for"]`.
+* `headers` - A list of strings naming the HTTP headers to use when deriving the true client IP address. Defaults to `["x-forwarded-for"]`.
-* `proxies` - A list of strings in [CIDR](https://en.wikipedia.org/wiki/CIDR) notation specifying the IPs of known proxies. Defaults to `[]`.
+* `proxies` - A list of upstream proxy IP subnets in CIDR notation from which we will parse the content of `headers`. Defaults to `[]`. IPv4 entries without a bitmask will be assumed to be /32 and IPv6 /128.
-* `reserved` - Defaults to [localhost](https://en.wikipedia.org/wiki/Localhost) and [private network](https://en.wikipedia.org/wiki/Private_network).
+* `reserved` - A list of reserved IP subnets in CIDR notation which should be ignored if found in `headers`. Defaults to `["127.0.0.0/8", "::1/128", "fc00::/7", "10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"]`.
 
 
 ### :rate_limit

lib/pleroma/plugs/remote_ip.ex

@@ -7,48 +7,42 @@ defmodule Pleroma.Plugs.RemoteIp do
   This is a shim to call [`RemoteIp`](https://git.pleroma.social/pleroma/remote_ip) but with runtime configuration.
   """
 
+  alias Pleroma.Config
   import Plug.Conn
 
   @behaviour Plug
 
-  @headers ~w[
-    x-forwarded-for
-  ]
-
-  # https://en.wikipedia.org/wiki/Localhost
-  # https://en.wikipedia.org/wiki/Private_network
-  @reserved ~w[
-    127.0.0.0/8
-    ::1/128
-    fc00::/7
-    10.0.0.0/8
-    172.16.0.0/12
-    192.168.0.0/16
-  ]
-
   def init(_), do: nil
 
   def call(%{remote_ip: original_remote_ip} = conn, _) do
-    config = Pleroma.Config.get(__MODULE__, [])
+    if Config.get([__MODULE__, :enabled]) do
-
+      %{remote_ip: new_remote_ip} = conn = RemoteIp.call(conn, remote_ip_opts())
-    if Keyword.get(config, :enabled, false) do
-      %{remote_ip: new_remote_ip} = conn = RemoteIp.call(conn, remote_ip_opts(config))
       assign(conn, :remote_ip_found, original_remote_ip != new_remote_ip)
     else
       conn
     end
   end
 
-  defp remote_ip_opts(config) do
+  defp remote_ip_opts do
-    headers = config |> Keyword.get(:headers, @headers) |> MapSet.new()
+    headers = Config.get([__MODULE__, :headers], []) |> MapSet.new()
-    reserved = Keyword.get(config, :reserved, @reserved)
+    reserved = Config.get([__MODULE__, :reserved], [])
 
     proxies =
-      config
+      Config.get([__MODULE__, :proxies], [])
-      |> Keyword.get(:proxies, [])
       |> Enum.concat(reserved)
-      |> Enum.map(&InetCidr.parse/1)
+      |> Enum.map(&maybe_add_cidr/1)
 
     {headers, proxies}
   end
+
+  defp maybe_add_cidr(proxy) when is_binary(proxy) do
+    proxy =
+      cond do
+        "/" in String.codepoints(proxy) -> proxy
+        InetCidr.v4?(InetCidr.parse_address!(proxy)) -> proxy <> "/32"
+        InetCidr.v6?(InetCidr.parse_address!(proxy)) -> proxy <> "/128"
+      end
+
+    InetCidr.parse(proxy, true)
+  end
 end

test/plugs/remote_ip_test.exs

@@ -3,13 +3,27 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Plugs.RemoteIpTest do
-  use ExUnit.Case, async: true
+  use ExUnit.Case
   use Plug.Test
 
   alias Pleroma.Plugs.RemoteIp
 
-  import Pleroma.Tests.Helpers, only: [clear_config: 1, clear_config: 2]
+  import Pleroma.Tests.Helpers, only: [clear_config: 2]
-  setup do: clear_config(RemoteIp)
+
+  setup do:
+          clear_config(RemoteIp,
+            enabled: true,
+            headers: ["x-forwarded-for"],
+            proxies: [],
+            reserved: [
+              "127.0.0.0/8",
+              "::1/128",
+              "fc00::/7",
+              "10.0.0.0/8",
+              "172.16.0.0/12",
+              "192.168.0.0/16"
+            ]
+          )
 
   test "disabled" do
     Pleroma.Config.put(RemoteIp, enabled: false)
@@ -25,8 +39,6 @@ defmodule Pleroma.Plugs.RemoteIpTest do
   end
 
   test "enabled" do
-    Pleroma.Config.put(RemoteIp, enabled: true)
-
     conn =
       conn(:get, "/")
       |> put_req_header("x-forwarded-for", "1.1.1.1")
@@ -54,8 +66,6 @@ defmodule Pleroma.Plugs.RemoteIpTest do
   end
 
   test "custom proxies" do
-    Pleroma.Config.put(RemoteIp, enabled: true)
-
     conn =
       conn(:get, "/")
       |> put_req_header("x-forwarded-for", "173.245.48.1, 1.1.1.1, 173.245.48.2")
@@ -72,4 +82,27 @@ defmodule Pleroma.Plugs.RemoteIpTest do
 
     assert conn.remote_ip == {1, 1, 1, 1}
   end
+
+  test "proxies set without CIDR format" do
+    Pleroma.Config.put([RemoteIp, :proxies], ["173.245.48.1"])
+
+    conn =
+      conn(:get, "/")
+      |> put_req_header("x-forwarded-for", "173.245.48.1, 1.1.1.1")
+      |> RemoteIp.call(nil)
+
+    assert conn.remote_ip == {1, 1, 1, 1}
+  end
+
+  test "proxies set `nonsensical` CIDR" do
+    Pleroma.Config.put([RemoteIp, :reserved], ["127.0.0.0/8"])
+    Pleroma.Config.put([RemoteIp, :proxies], ["10.0.0.3/24"])
+
+    conn =
+      conn(:get, "/")
+      |> put_req_header("x-forwarded-for", "10.0.0.3, 1.1.1.1")
+      |> RemoteIp.call(nil)
+
+    assert conn.remote_ip == {1, 1, 1, 1}
+  end
 end