From 02833d0b44e17b580d3fcda4a8b30fe633f75aa0 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sun, 1 Feb 2026 14:59:54 -0600 Subject: [PATCH] Add rate limiting to auth and API endpoints using Hammer --- CLAUDE.md | 19 +++ config/config.exs | 4 + config/test.exs | 4 +- lib/towerops/application.ex | 2 + lib/towerops_web/plugs/rate_limit.ex | 101 ++++++++++++ lib/towerops_web/router.ex | 24 ++- mix.exs | 3 +- mix.lock | 1 + test/towerops_web/plugs/rate_limit_test.exs | 163 ++++++++++++++++++++ 9 files changed, 314 insertions(+), 7 deletions(-) create mode 100644 lib/towerops_web/plugs/rate_limit.ex create mode 100644 test/towerops_web/plugs/rate_limit_test.exs diff --git a/CLAUDE.md b/CLAUDE.md index 654195fc..a054a31f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,6 +120,25 @@ All LiveViews, LiveComponents, and HTML modules automatically get these imports/ - Gettext for translations - Verified routes with `~p` sigil +### Rate Limiting + +Uses Hammer (ETS-backed) for rate limiting to prevent brute-force attacks. + +**Endpoints Protected:** +- **Auth endpoints** (10 req/min per IP): `/users/log-in`, `/users/register`, `/users/reset-password`, TOTP verification, mobile auth QR flows +- **API v1 endpoints** (1000 req/min per IP): `/api/v1/*` - relaxed for Terraform/automation +- **Admin API endpoints**: Not rate limited (superuser only, different attack profile) + +**Implementation:** +- Plug: `ToweropsWeb.Plugs.RateLimit` with `:auth` or `:api` type +- Uses client's real IP from `X-Forwarded-For` or `X-Real-IP` headers (for proxy/load balancer) +- Returns `429 Too Many Requests` with `Retry-After` header when limit exceeded + +**Configuration:** +- Disabled in test environment via `config :towerops, :rate_limiting_enabled, false` +- ETS backend configured in `config/config.exs` +- Hammer process started in `application.ex` supervision tree + ### Asset Pipeline - **Tailwind CSS v4**: Uses new `@import "tailwindcss"` syntax in `assets/css/app.css`, no `tailwind.config.js` needed diff --git a/config/config.exs b/config/config.exs index a850f459..3827859c 100644 --- a/config/config.exs +++ b/config/config.exs @@ -20,6 +20,10 @@ config :esbuild, env: %{"NODE_PATH" => [Path.expand("../deps", __DIR__), Mix.Project.build_path()]} ] +# Rate limiting with Hammer (ETS backend) +config :hammer, + backend: {Hammer.Backend.ETS, [expiry_ms: to_timeout(minute: 10), cleanup_interval_ms: to_timeout(minute: 1)]} + config :honeybadger, api_key: "hbp_xe5xMnpLZ2XJsXQJujoEkgCmiqCfwa0uYA3Y", environment_name: config_env(), diff --git a/config/test.exs b/config/test.exs index 618ed120..cce5b486 100644 --- a/config/test.exs +++ b/config/test.exs @@ -61,4 +61,6 @@ config :towerops, ping_module: Towerops.Monitoring.PingMock, poller_module: Towerops.Snmp.PollerMock, snmp_adapter: Towerops.Snmp.SnmpMock, - sql_sandbox: true + sql_sandbox: true, + # Disable rate limiting in tests to avoid blocking concurrent test requests + rate_limiting_enabled: false diff --git a/lib/towerops/application.ex b/lib/towerops/application.ex index 3d51adfd..c26333ad 100644 --- a/lib/towerops/application.ex +++ b/lib/towerops/application.ex @@ -81,6 +81,8 @@ defmodule Towerops.Application do {Cluster.Supervisor, [topologies, [name: Towerops.ClusterSupervisor]]}, ToweropsWeb.Telemetry, Towerops.Repo, + # Rate limiting backend (ETS-based, per-node) + {Hammer.Backend.ETS, [expiry_ms: to_timeout(minute: 10), cleanup_interval_ms: to_timeout(minute: 1)]}, {Oban, Application.fetch_env!(:towerops, Oban)}, {DNSCluster, query: Application.get_env(:towerops, :dns_cluster_query) || :ignore}, pubsub_spec(), diff --git a/lib/towerops_web/plugs/rate_limit.ex b/lib/towerops_web/plugs/rate_limit.ex new file mode 100644 index 00000000..6fc411ab --- /dev/null +++ b/lib/towerops_web/plugs/rate_limit.ex @@ -0,0 +1,101 @@ +defmodule ToweropsWeb.Plugs.RateLimit do + @moduledoc """ + Rate limiting plug using Hammer to prevent brute-force attacks. + + Applies different rate limits based on the type of endpoint: + - Auth endpoints (login, register, password reset): 10 requests per minute per IP + - API endpoints: 100 requests per minute per IP + + Uses the client's real IP address from X-Forwarded-For or X-Real-IP headers + when behind a proxy/load balancer. + """ + + import Phoenix.Controller, only: [json: 2] + import Plug.Conn + + require Logger + + @auth_limit 10 + @auth_period to_timeout(minute: 1) + + @api_limit 1000 + @api_period to_timeout(minute: 1) + + @type limit_type :: :auth | :api + + def init(opts) do + type = Keyword.get(opts, :type, :api) + + if type not in [:auth, :api] do + raise ArgumentError, "RateLimit plug :type must be :auth or :api, got: #{inspect(type)}" + end + + type + end + + def call(conn, type) do + if rate_limiting_enabled?() do + check_rate_limit(conn, type) + else + conn + end + end + + defp rate_limiting_enabled? do + Application.get_env(:towerops, :rate_limiting_enabled, true) + end + + defp check_rate_limit(conn, type) do + remote_ip = get_remote_ip(conn) + bucket_key = "#{type}:#{remote_ip}" + + {limit, period} = limits_for(type) + + case Hammer.check_rate(bucket_key, period, limit) do + {:allow, _count} -> + conn + + {:deny, retry_after} -> + Logger.warning("Rate limit exceeded for #{bucket_key}, retry_after: #{retry_after}ms") + + conn + |> put_resp_header("retry-after", Integer.to_string(div(retry_after, 1000))) + |> put_status(:too_many_requests) + |> json(%{error: "Too many requests. Please try again later."}) + |> halt() + end + end + + defp limits_for(:auth), do: {@auth_limit, @auth_period} + defp limits_for(:api), do: {@api_limit, @api_period} + + defp get_remote_ip(conn) do + # Try X-Forwarded-For first (set by Traefik and other proxies) + case get_req_header(conn, "x-forwarded-for") do + [forwarded | _] -> + forwarded + |> String.split(",") + |> List.first() + |> String.trim() + + [] -> + get_fallback_ip(conn) + end + end + + defp get_fallback_ip(conn) do + case get_req_header(conn, "x-real-ip") do + [real_ip | _] -> + real_ip + + [] -> + format_remote_ip(conn.remote_ip) + end + end + + defp format_remote_ip({a, b, c, d}), do: "#{a}.#{b}.#{c}.#{d}" + + defp format_remote_ip({a, b, c, d, e, f, g, h}) do + Enum.map_join([a, b, c, d, e, f, g, h], ":", &Integer.to_string(&1, 16)) + end +end diff --git a/lib/towerops_web/router.ex b/lib/towerops_web/router.ex index 1ebc63b5..c66c32d6 100644 --- a/lib/towerops_web/router.ex +++ b/lib/towerops_web/router.ex @@ -9,6 +9,7 @@ defmodule ToweropsWeb.Router do import ToweropsWeb.UserAuth alias ToweropsWeb.Api.V1 + alias ToweropsWeb.Plugs.RateLimit pipeline :browser do plug :accepts, ["html"] @@ -53,6 +54,15 @@ defmodule ToweropsWeb.Router do plug ToweropsWeb.Plugs.ApiAuth end + # Rate limiting pipelines + pipeline :rate_limit_auth do + plug RateLimit, type: :auth + end + + pipeline :rate_limit_api do + plug RateLimit, type: :api + end + # Health check endpoint for Kubernetes probes (no authentication required) scope "/", ToweropsWeb do get "/health", HealthController, :index @@ -80,16 +90,18 @@ defmodule ToweropsWeb.Router do # See ToweropsWeb.AgentChannel for implementation # Mobile Auth API routes (no authentication required for login flow) + # Rate limited to prevent brute-force attacks on QR token verification scope "/api/v1/mobile/auth", ToweropsWeb.Api do - pipe_through :api + pipe_through [:api, :rate_limit_auth] post "/qr/verify", MobileAuthController, :verify_qr_token post "/qr/complete", MobileAuthController, :complete_qr_login end # Mobile API routes (requires mobile authentication) + # Rate limited to prevent API abuse scope "/api/v1/mobile", ToweropsWeb.Api do - pipe_through :mobile_api + pipe_through [:mobile_api, :rate_limit_api] # Auth session management get "/auth/session", MobileAuthController, :get_session @@ -104,8 +116,9 @@ defmodule ToweropsWeb.Router do end # API v1 routes (requires API token authentication) + # Rate limited to prevent API abuse scope "/api/v1", V1 do - pipe_through :api_v1 + pipe_through [:api_v1, :rate_limit_api] resources "/sites", SitesController, except: [:new, :edit] resources "/devices", DevicesController, except: [:new, :edit] @@ -157,9 +170,10 @@ defmodule ToweropsWeb.Router do end ## Authentication routes + ## Rate limited to prevent brute-force attacks scope "/", ToweropsWeb do - pipe_through [:browser, :redirect_if_user_is_authenticated] + pipe_through [:browser, :redirect_if_user_is_authenticated, :rate_limit_auth] live "/users/register", UserRegistrationLive, :new end @@ -171,7 +185,7 @@ defmodule ToweropsWeb.Router do end scope "/", ToweropsWeb do - pipe_through [:browser] + pipe_through [:browser, :rate_limit_auth] get "/users/log-in", UserSessionController, :new get "/users/log-in/totp", UserSessionController, :totp_verification_form diff --git a/mix.exs b/mix.exs index fd5de870..c424f6b9 100644 --- a/mix.exs +++ b/mix.exs @@ -85,7 +85,8 @@ defmodule Towerops.MixProject do {:dialyxir, "~> 1.4", only: [:dev, :test], runtime: false}, {:credo, "~> 1.7", only: [:dev, :test], runtime: false}, {:sobelow, "~> 0.14.1", only: [:dev, :test], runtime: false}, - {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false} + {:mix_audit, "~> 2.1", only: [:dev, :test], runtime: false}, + {:hammer, "~> 6.2"} ] end diff --git a/mix.lock b/mix.lock index d00f15d8..113291c0 100644 --- a/mix.lock +++ b/mix.lock @@ -25,6 +25,7 @@ "gen_smtp": {:hex, :gen_smtp, "1.3.0", "62c3d91f0dcf6ce9db71bcb6881d7ad0d1d834c7f38c13fa8e952f4104a8442e", [:rebar3], [{:ranch, ">= 1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "0b73fbf069864ecbce02fe653b16d3f35fd889d0fdd4e14527675565c39d84e6"}, "gettext": {:hex, :gettext, "1.0.2", "5457e1fd3f4abe47b0e13ff85086aabae760497a3497909b8473e0acee57673b", [:mix], [{:expo, "~> 0.5.1 or ~> 1.0", [hex: :expo, repo: "hexpm", optional: false]}], "hexpm", "eab805501886802071ad290714515c8c4a17196ea76e5afc9d06ca85fb1bfeb3"}, "hackney": {:hex, :hackney, "1.25.0", "390e9b83f31e5b325b9f43b76e1a785cbdb69b5b6cd4e079aa67835ded046867", [:rebar3], [{:certifi, "~> 2.15.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~> 6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~> 1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~> 1.4", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.4.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~> 1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~> 0.7.1", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "7209bfd75fd1f42467211ff8f59ea74d6f2a9e81cbcee95a56711ee79fd6b1d4"}, + "hammer": {:hex, :hammer, "6.2.1", "5ae9c33e3dceaeb42de0db46bf505bd9c35f259c8defb03390cd7556fea67ee2", [:mix], [{:poolboy, "~> 1.5", [hex: :poolboy, repo: "hexpm", optional: false]}], "hexpm", "b9476d0c13883d2dc0cc72e786bac6ac28911fba7cc2e04b70ce6a6d9c4b2bdc"}, "heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "0435d4ca364a608cc75e2f8683d374e55abbae26", [tag: "v2.2.0", sparse: "optimized", depth: 1]}, "honeybadger": {:hex, :honeybadger, "0.24.1", "13ffe56b4d148649c8fbb0e091fefecc5d8e8eb7ade684b6900085a947d741d5", [:mix], [{:ecto, ">= 2.0.0", [hex: :ecto, repo: "hexpm", optional: true]}, {:hackney, "~> 1.1", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}, {:phoenix, ">= 1.0.0 and < 2.0.0", [hex: :phoenix, repo: "hexpm", optional: true]}, {:plug, ">= 1.0.0 and < 2.0.0", [hex: :plug, repo: "hexpm", optional: true]}, {:process_tree, "~> 0.2.1", [hex: :process_tree, repo: "hexpm", optional: false]}, {:req, "~> 0.5.0", [hex: :req, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "0c97d5a82c42298b9935dbc0a7e3c14372c8f55257f603828258ef9f7e0da892"}, "hpax": {:hex, :hpax, "1.0.3", "ed67ef51ad4df91e75cc6a1494f851850c0bd98ebc0be6e81b026e765ee535aa", [:mix], [], "hexpm", "8eab6e1cfa8d5918c2ce4ba43588e894af35dbd8e91e6e55c817bca5847df34a"}, diff --git a/test/towerops_web/plugs/rate_limit_test.exs b/test/towerops_web/plugs/rate_limit_test.exs new file mode 100644 index 00000000..d0c3c58b --- /dev/null +++ b/test/towerops_web/plugs/rate_limit_test.exs @@ -0,0 +1,163 @@ +defmodule ToweropsWeb.Plugs.RateLimitTest do + use ToweropsWeb.ConnCase, async: false + + alias ToweropsWeb.Plugs.RateLimit + + # Use async: false because we're testing Hammer state + + setup do + # Enable rate limiting for this test module (disabled by default in test env) + Application.put_env(:towerops, :rate_limiting_enabled, true) + + on_exit(fn -> + # Restore default test config (disabled) + Application.put_env(:towerops, :rate_limiting_enabled, false) + end) + + :ok + end + + describe "init/1" do + test "accepts :auth type" do + assert :auth = RateLimit.init(type: :auth) + end + + test "accepts :api type" do + assert :api = RateLimit.init(type: :api) + end + + test "defaults to :api type" do + assert :api = RateLimit.init([]) + end + + test "raises on invalid type" do + assert_raise ArgumentError, ~r/must be :auth or :api/, fn -> + RateLimit.init(type: :invalid) + end + end + end + + describe "call/2 with :auth type" do + test "allows requests under the limit", %{conn: conn} do + # Use a unique IP for this test + unique_ip = "10.0.0.#{:rand.uniform(255)}" + + conn = + conn + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:auth) + + refute conn.halted + end + + test "blocks requests over the limit", %{conn: conn} do + # Use a unique IP for this test + unique_ip = "10.1.0.#{:rand.uniform(255)}" + + # Make 10 allowed requests (auth limit is 10) + Enum.each(1..10, fn _ -> + conn = + build_conn() + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:auth) + + refute conn.halted + end) + + # 11th request should be blocked + conn = + conn + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:auth) + + assert conn.halted + assert conn.status == 429 + assert get_resp_header(conn, "retry-after") != [] + + body = Jason.decode!(conn.resp_body) + assert body["error"] =~ "Too many requests" + end + end + + describe "call/2 with :api type" do + test "allows requests under the limit", %{conn: conn} do + unique_ip = "10.2.0.#{:rand.uniform(255)}" + + conn = + conn + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:api) + + refute conn.halted + end + + test "allows higher limit than auth", %{conn: conn} do + unique_ip = "10.3.0.#{:rand.uniform(255)}" + + # Make 50 requests - should all be allowed (API limit is 100) + Enum.each(1..50, fn _ -> + conn = + build_conn() + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:api) + + refute conn.halted + end) + end + end + + describe "IP extraction" do + test "uses X-Forwarded-For header first", %{conn: conn} do + unique_ip = "192.168.1.#{:rand.uniform(255)}" + + # Exhaust the limit for this IP + Enum.each(1..10, fn _ -> + build_conn() + |> put_req_header("x-forwarded-for", unique_ip) + |> RateLimit.call(:auth) + end) + + # Different IP should still work + different_ip = "192.168.2.#{:rand.uniform(255)}" + + conn = + conn + |> put_req_header("x-forwarded-for", different_ip) + |> RateLimit.call(:auth) + + refute conn.halted + end + + test "handles comma-separated X-Forwarded-For", %{conn: conn} do + unique_ip = "203.0.113.#{:rand.uniform(255)}" + + # X-Forwarded-For can have multiple IPs (client, proxy1, proxy2) + conn = + conn + |> put_req_header("x-forwarded-for", "#{unique_ip}, 10.0.0.1, 10.0.0.2") + |> RateLimit.call(:auth) + + refute conn.halted + end + + test "falls back to X-Real-IP", %{conn: conn} do + unique_ip = "198.51.100.#{:rand.uniform(255)}" + + conn = + conn + |> put_req_header("x-real-ip", unique_ip) + |> RateLimit.call(:auth) + + refute conn.halted + end + + test "falls back to remote_ip when no headers", %{conn: conn} do + # No forwarding headers, uses conn.remote_ip (127.0.0.1) + # Since other tests might have used this IP, we just verify the plug works + # by checking it doesn't crash - actual rate limiting is tested elsewhere + result = RateLimit.call(conn, :auth) + # Either allowed or blocked is fine - we're testing the IP extraction path + assert is_struct(result, Plug.Conn) + end + end +end