From ef54125aa68606bbff8ecab48e321973c87f7bcf Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 21 Apr 2026 13:17:43 -0500 Subject: [PATCH] fix(security): only trust forwarded-ip headers from known proxies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The remote_ip plug previously trusted Cf-Connecting-Ip and X-Forwarded-For on every request, so any client reaching a pod directly (cluster-internal, kubectl port-forward, misconfigured Service) could spoof conn.remote_ip, which flows into session storage and logs. Now only honour those headers when the immediate TCP peer sits inside a configured CIDR list. Default list covers loopback, RFC1918, CGNAT, link-local, and IPv6 ULA — matches the k8s pod/service network. Override via config :microwaveprop, :trusted_proxies, or per-environment via the TRUSTED_PROXY_CIDRS env var in runtime.exs. Also refresh the stale "nginx/dokku proxy" moduledoc — deployment has been k8s + Cloudflare for a while. --- config/runtime.exs | 15 ++ lib/microwaveprop_web/plugs/remote_ip.ex | 150 +++++++++++++--- .../plugs/remote_ip_test.exs | 170 +++++++++++++++--- 3 files changed, 293 insertions(+), 42 deletions(-) diff --git a/config/runtime.exs b/config/runtime.exs index 509cf408..58044bc1 100644 --- a/config/runtime.exs +++ b/config/runtime.exs @@ -25,6 +25,21 @@ end config :microwaveprop, MicrowavepropWeb.Endpoint, http: [port: String.to_integer(System.get_env("PORT", "4000"))] +# MicrowavepropWeb.Plugs.RemoteIp will only honour `Cf-Connecting-Ip` / +# `X-Forwarded-For` when the immediate TCP peer (conn.remote_ip) sits inside +# one of these CIDRs. Set TRUSTED_PROXY_CIDRS to a comma-separated list to +# override; leave unset to keep the module default (loopback + RFC1918 + +# ULA + link-local), which matches the k8s cluster pod/service network. +if trusted_proxy_cidrs = System.get_env("TRUSTED_PROXY_CIDRS") do + cidrs = + trusted_proxy_cidrs + |> String.split(",", trim: true) + |> Enum.map(&String.trim/1) + |> Enum.reject(&(&1 == "")) + + config :microwaveprop, :trusted_proxies, cidrs +end + if prometheus_token = System.get_env("PROMETHEUS_AUTH_TOKEN") do config :microwaveprop, :prometheus_auth_token, prometheus_token end diff --git a/lib/microwaveprop_web/plugs/remote_ip.ex b/lib/microwaveprop_web/plugs/remote_ip.ex index df74b7c1..3d4a24f4 100644 --- a/lib/microwaveprop_web/plugs/remote_ip.ex +++ b/lib/microwaveprop_web/plugs/remote_ip.ex @@ -1,38 +1,71 @@ defmodule MicrowavepropWeb.Plugs.RemoteIp do @moduledoc """ - Extracts the client IP from X-Forwarded-For (set by nginx/dokku proxy) - and assigns it to conn.remote_ip and Logger metadata. + Extracts the client IP from `Cf-Connecting-Ip` / `X-Forwarded-For` headers, + but only when the immediate peer (`conn.remote_ip`) is a trusted proxy. + + Deployment: the app runs as pods in the k8s `prop` namespace behind the + cluster ingress, which itself sits behind Cloudflare. The only hosts that + should ever terminate TCP to a pod are ingress replicas on the cluster + pod network, so we only honour forwarded-IP headers when the raw socket + peer falls inside a configured CIDR list. + + Trusted proxies are configured via `config :microwaveprop, :trusted_proxies` + (a list of CIDR strings). The default list covers loopback and the RFC1918 + / ULA ranges used by k8s cluster pod networks. `config/runtime.exs` reads + `TRUSTED_PROXY_CIDRS` (comma-separated) in production for tuning without + a rebuild. + + Rationale: without this guard any client that reaches a pod directly + (cluster-internal, `kubectl port-forward`, a misconfigured Service) could + spoof `Cf-Connecting-Ip` to set `conn.remote_ip`, which flows into session + storage and logs. """ @behaviour Plug - @impl true - def init(opts), do: opts + import Bitwise + + require Logger - @impl true - # Cloudflare sets Cf-Connecting-Ip with the real client IP. - # Fall back to X-Forwarded-For for non-tunneled requests (dev, direct). @ip_headers ["cf-connecting-ip", "x-forwarded-for"] - def call(conn, _opts) do - ip = - Enum.find_value(@ip_headers, fn header -> - case Plug.Conn.get_req_header(conn, header) do - [value | _] -> - value - |> String.split(",") - |> hd() - |> String.trim() - |> parse_ip() + # RFC1918 + CGNAT + loopback + link-local + IPv6 loopback + ULA. + # Matches typical k8s pod/service CIDRs and the ingress mesh. + @default_trusted_proxies [ + "127.0.0.0/8", + "10.0.0.0/8", + "172.16.0.0/12", + "192.168.0.0/16", + "100.64.0.0/10", + "169.254.0.0/16", + "::1/128", + "fc00::/7", + "fe80::/10" + ] - _ -> - nil - end + @impl true + def init(opts) do + cidrs = + Keyword.get_lazy(opts, :trusted_proxies, fn -> + Application.get_env(:microwaveprop, :trusted_proxies, @default_trusted_proxies) end) + parsed = + cidrs + |> Enum.map(&parse_cidr/1) + |> Enum.reject(&is_nil/1) + + %{trusted_proxies: parsed} + end + + @impl true + def call(conn, %{trusted_proxies: trusted_proxies}) do conn = - if ip do - %{conn | remote_ip: ip} + if peer_trusted?(conn.remote_ip, trusted_proxies) do + case extract_forwarded_ip(conn) do + nil -> conn + ip -> %{conn | remote_ip: ip} + end else conn end @@ -46,6 +79,22 @@ defmodule MicrowavepropWeb.Plugs.RemoteIp do conn end + defp extract_forwarded_ip(conn) do + Enum.find_value(@ip_headers, fn header -> + case Plug.Conn.get_req_header(conn, header) do + [value | _] -> + value + |> String.split(",") + |> hd() + |> String.trim() + |> parse_ip() + + _ -> + nil + end + end) + end + defp parse_ip(str) do case :inet.parse_address(String.to_charlist(str)) do {:ok, ip} -> ip @@ -53,6 +102,63 @@ defmodule MicrowavepropWeb.Plugs.RemoteIp do end end + # Parses "10.0.0.0/8" or "fc00::/7" into {ip_integer, mask_integer, bit_width}. + # Invalid entries are logged and dropped at init-time. + defp parse_cidr(cidr) when is_binary(cidr) do + case String.split(cidr, "/", parts: 2) do + [ip_str, prefix_str] -> + with {prefix, ""} <- Integer.parse(prefix_str), + {:ok, ip} <- :inet.parse_address(String.to_charlist(ip_str)), + bit_width = bit_width_for(ip), + true <- prefix >= 0 and prefix <= bit_width do + mask = cidr_mask(prefix, bit_width) + {ip_to_integer(ip), mask, bit_width} + else + _ -> + Logger.warning("RemoteIp: could not parse CIDR #{inspect(cidr)}") + nil + end + + _ -> + Logger.warning("RemoteIp: CIDR missing /prefix: #{inspect(cidr)}") + nil + end + end + + defp parse_cidr(_), do: nil + + defp bit_width_for(ip) when tuple_size(ip) == 4, do: 32 + defp bit_width_for(ip) when tuple_size(ip) == 8, do: 128 + + # Build a prefix-bit mask: e.g. prefix=24, bit_width=32 -> 0xFFFFFF00. + defp cidr_mask(0, _bit_width), do: 0 + + defp cidr_mask(prefix, bit_width) do + host_bits = bit_width - prefix + all_ones = (1 <<< bit_width) - 1 + all_ones - ((1 <<< host_bits) - 1) + end + + defp peer_trusted?(peer_ip, trusted) when tuple_size(peer_ip) in [4, 8] do + peer_width = bit_width_for(peer_ip) + peer_int = ip_to_integer(peer_ip) + + Enum.any?(trusted, fn {cidr_int, mask, bit_width} -> + bit_width == peer_width and band(peer_int, mask) == band(cidr_int, mask) + end) + end + + defp peer_trusted?(_, _), do: false + + defp ip_to_integer({a, b, c, d}) do + (a <<< 24) + (b <<< 16) + (c <<< 8) + d + end + + defp ip_to_integer({a, b, c, d, e, f, g, h}) do + (a <<< 112) + (b <<< 96) + (c <<< 80) + (d <<< 64) + + (e <<< 48) + (f <<< 32) + (g <<< 16) + h + end + defp format_ip({a, b, c, d}), do: "#{a}.#{b}.#{c}.#{d}" defp format_ip({a, b, c, d, e, f, g, h}), do: "#{a}:#{b}:#{c}:#{d}:#{e}:#{f}:#{g}:#{h}" defp format_ip(_), do: "unknown" diff --git a/test/microwaveprop_web/plugs/remote_ip_test.exs b/test/microwaveprop_web/plugs/remote_ip_test.exs index 2f81a9c7..6d1f016c 100644 --- a/test/microwaveprop_web/plugs/remote_ip_test.exs +++ b/test/microwaveprop_web/plugs/remote_ip_test.exs @@ -6,35 +6,47 @@ defmodule MicrowavepropWeb.Plugs.RemoteIpTest do alias MicrowavepropWeb.Plugs.RemoteIp - defp call_plug(conn) do - RemoteIp.call(conn, RemoteIp.init([])) + # A trusted peer that looks like a k8s cluster pod or ingress. + @trusted_peer {10, 42, 0, 1} + # An external peer: must NOT be able to spoof headers. + @untrusted_peer {8, 8, 8, 8} + + defp call_plug(conn, opts \\ []) do + RemoteIp.call(conn, RemoteIp.init(opts)) end - describe "call/2" do - test "extracts IP from cf-connecting-ip header" do + defp with_peer(conn, ip) do + Map.put(conn, :remote_ip, ip) + end + + describe "call/2 with trusted peer" do + test "uses cf-connecting-ip when immediate peer is trusted" do conn = :get |> conn("/") + |> with_peer(@trusted_peer) |> put_req_header("cf-connecting-ip", "1.2.3.4") |> call_plug() assert conn.remote_ip == {1, 2, 3, 4} end - test "falls back to x-forwarded-for when no cf-connecting-ip" do + test "falls back to x-forwarded-for when no cf-connecting-ip (trusted peer)" do conn = :get |> conn("/") + |> with_peer(@trusted_peer) |> put_req_header("x-forwarded-for", "5.6.7.8") |> call_plug() assert conn.remote_ip == {5, 6, 7, 8} end - test "prefers cf-connecting-ip over x-forwarded-for" do + test "prefers cf-connecting-ip over x-forwarded-for (trusted peer)" do conn = :get |> conn("/") + |> with_peer(@trusted_peer) |> put_req_header("cf-connecting-ip", "1.2.3.4") |> put_req_header("x-forwarded-for", "5.6.7.8") |> call_plug() @@ -42,49 +54,167 @@ defmodule MicrowavepropWeb.Plugs.RemoteIpTest do assert conn.remote_ip == {1, 2, 3, 4} end - test "takes first IP from comma-separated x-forwarded-for" do + test "takes first IP from comma-separated x-forwarded-for (trusted peer)" do conn = :get |> conn("/") + |> with_peer(@trusted_peer) |> put_req_header("x-forwarded-for", "10.0.0.1, 10.0.0.2, 10.0.0.3") |> call_plug() assert conn.remote_ip == {10, 0, 0, 1} end - test "preserves original remote_ip when no headers present" do - original_ip = {127, 0, 0, 1} - + test "preserves original remote_ip when no headers present (trusted peer)" do conn = :get |> conn("/") - |> Map.put(:remote_ip, original_ip) + |> with_peer(@trusted_peer) |> call_plug() - assert conn.remote_ip == original_ip + assert conn.remote_ip == @trusted_peer end - test "handles IPv6 addresses" do + test "handles IPv6 header value (trusted peer)" do conn = :get |> conn("/") - |> put_req_header("cf-connecting-ip", "::1") + |> with_peer(@trusted_peer) + |> put_req_header("cf-connecting-ip", "2001:db8::1") |> call_plug() - assert conn.remote_ip == {0, 0, 0, 0, 0, 0, 0, 1} + assert conn.remote_ip == {8193, 3512, 0, 0, 0, 0, 0, 1} end - test "ignores malformed IP addresses" do - original_ip = {127, 0, 0, 1} - + test "handles loopback peer (IPv4)" do conn = :get |> conn("/") - |> Map.put(:remote_ip, original_ip) + |> with_peer({127, 0, 0, 1}) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug() + + assert conn.remote_ip == {1, 2, 3, 4} + end + + test "handles loopback peer (IPv6 ::1)" do + conn = + :get + |> conn("/") + |> with_peer({0, 0, 0, 0, 0, 0, 0, 1}) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug() + + assert conn.remote_ip == {1, 2, 3, 4} + end + + test "ignores malformed IP in header (trusted peer)" do + conn = + :get + |> conn("/") + |> with_peer(@trusted_peer) |> put_req_header("cf-connecting-ip", "not-an-ip") |> call_plug() - assert conn.remote_ip == original_ip + assert conn.remote_ip == @trusted_peer + end + end + + describe "call/2 with untrusted peer" do + test "ignores cf-connecting-ip from untrusted peer (spoof attempt)" do + conn = + :get + |> conn("/") + |> with_peer(@untrusted_peer) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug() + + assert conn.remote_ip == @untrusted_peer + end + + test "ignores x-forwarded-for from untrusted peer (spoof attempt)" do + conn = + :get + |> conn("/") + |> with_peer(@untrusted_peer) + |> put_req_header("x-forwarded-for", "1.2.3.4, 5.6.7.8") + |> call_plug() + + assert conn.remote_ip == @untrusted_peer + end + + test "ignores both spoofed headers from untrusted peer" do + conn = + :get + |> conn("/") + |> with_peer(@untrusted_peer) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> put_req_header("x-forwarded-for", "9.9.9.9") + |> call_plug() + + assert conn.remote_ip == @untrusted_peer + end + end + + describe "call/2 with IPv6 peers" do + test "trusts IPv6 ULA peer when configured" do + # fc00::/7 ULA range — included in default trusted list. + peer = {0xFC00, 0, 0, 0, 0, 0, 0, 1} + + conn = + :get + |> conn("/") + |> with_peer(peer) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug(trusted_proxies: ["fc00::/7"]) + + assert conn.remote_ip == {1, 2, 3, 4} + end + + test "does not trust a public IPv6 peer when only ULA is trusted" do + peer = {0x2001, 0xDB8, 0, 0, 0, 0, 0, 1} + + conn = + :get + |> conn("/") + |> with_peer(peer) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug(trusted_proxies: ["fc00::/7"]) + + assert conn.remote_ip == peer + end + end + + describe "call/2 with custom trusted_proxies option" do + test "explicit empty list trusts nothing — even loopback is ignored" do + conn = + :get + |> conn("/") + |> with_peer({127, 0, 0, 1}) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug(trusted_proxies: []) + + assert conn.remote_ip == {127, 0, 0, 1} + end + + test "single-host CIDR (/32) trusts only that exact IP" do + conn = + :get + |> conn("/") + |> with_peer({203, 0, 113, 5}) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug(trusted_proxies: ["203.0.113.5/32"]) + + assert conn.remote_ip == {1, 2, 3, 4} + + conn2 = + :get + |> conn("/") + |> with_peer({203, 0, 113, 6}) + |> put_req_header("cf-connecting-ip", "1.2.3.4") + |> call_plug(trusted_proxies: ["203.0.113.5/32"]) + + assert conn2.remote_ip == {203, 0, 113, 6} end end end