-
-
Notifications
You must be signed in to change notification settings - Fork 41
Added IP-based rate limiting for Copi (fixes #1877) #1998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
immortal71
wants to merge
4
commits into
OWASP:master
Choose a base branch
from
immortal71:feat/rate-limiting-clean
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Security Policy for Copi | ||
|
|
||
| ## Rate Limiting | ||
|
|
||
| Copi implements IP-based rate limiting to protect against CAPEC-212 (Functionality Misuse) attacks and ensure service availability. | ||
|
|
||
| ## Protected Actions | ||
|
|
||
| 1. **Game Creation**: Limited to prevent abuse | ||
| - Default: 10 games per IP per hour | ||
| - Configurable via `MAX_GAMES_PER_IP` and `GAME_CREATION_WINDOW_SECONDS` | ||
|
|
||
| 2. **Player Creation**: Separate limit from game creation | ||
| - Default: 20 players per IP per hour | ||
| - Configurable via `MAX_PLAYERS_PER_IP` and `PLAYER_CREATION_WINDOW_SECONDS` | ||
|
|
||
| 3. **WebSocket Connections**: Prevents connection flooding | ||
| - Default: 50 connections per IP per 5 minutes | ||
| - Configurable via `MAX_CONNECTIONS_PER_IP` and `CONNECTION_WINDOW_SECONDS` | ||
|
|
||
| ## Configuration | ||
|
|
||
| All limits can be adjusted via environment variables in production: | ||
|
|
||
| ```bash | ||
| MAX_GAMES_PER_IP=10 | ||
| GAME_CREATION_WINDOW_SECONDS=3600 | ||
| MAX_PLAYERS_PER_IP=20 | ||
| PLAYER_CREATION_WINDOW_SECONDS=3600 | ||
| MAX_CONNECTIONS_PER_IP=50 | ||
| CONNECTION_WINDOW_SECONDS=300 | ||
| ``` | ||
|
|
||
| ### Reporting Security Issues | ||
|
|
||
| If you discover a security vulnerability, please email [email protected] with details. | ||
| Do not create public GitHub issues for security problems. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,229 @@ | ||
| defmodule Copi.RateLimiter do | ||
| @moduledoc """ | ||
| Rate limiter to prevent abuse by limiting requests per IP address. | ||
|
|
||
| This module implements rate limiting for game creation, player creation, and user connections | ||
| to protect against CAPEC 212 (Functionality Misuse) attacks. | ||
| """ | ||
|
|
||
| use GenServer | ||
| require Logger | ||
|
|
||
| @cleanup_interval :timer.minutes(5) | ||
|
|
||
| # Client API | ||
|
|
||
| @doc """ | ||
| Starts the rate limiter GenServer. | ||
| """ | ||
| def start_link(opts \\ []) do | ||
| GenServer.start_link(__MODULE__, opts, name: __MODULE__) | ||
| end | ||
|
|
||
| @doc """ | ||
| Checks if a request from the given IP for the specified action should be allowed. | ||
|
|
||
| Returns `{:ok, remaining}` if allowed, `{:error, :rate_limited, retry_after}` if blocked. | ||
| """ | ||
| def check_rate(ip_address, action) when action in [:game_creation, :player_creation, :connection] do | ||
| GenServer.call(__MODULE__, {:check_rate, ip_address, action}) | ||
| end | ||
|
|
||
| @doc """ | ||
| Atomically checks and records a rate limit action in a single operation. | ||
| This prevents race conditions where multiple concurrent requests could bypass the limit. | ||
|
|
||
| Returns `{:ok, remaining}` if allowed (and records the action), | ||
| `{:error, :rate_limited, retry_after}` if blocked (does not record). | ||
| """ | ||
| def check_and_record(ip_address, action) when action in [:game_creation, :player_creation, :connection] do | ||
| GenServer.call(__MODULE__, {:check_and_record, ip_address, action}) | ||
| end | ||
|
|
||
| @doc """ | ||
| Records a successful action for rate limiting tracking. | ||
| """ | ||
| def record_action(ip_address, action) when action in [:game_creation, :player_creation, :connection] do | ||
| GenServer.cast(__MODULE__, {:record_action, ip_address, action}) | ||
| end | ||
|
|
||
| @doc """ | ||
| Clears all rate limit data for an IP address (useful for testing). | ||
| """ | ||
| def clear_ip(ip_address) do | ||
| GenServer.cast(__MODULE__, {:clear_ip, ip_address}) | ||
| end | ||
|
|
||
| @doc """ | ||
| Gets current rate limit configuration. | ||
| """ | ||
| def get_config do | ||
| GenServer.call(__MODULE__, :get_config) | ||
| end | ||
|
|
||
| # Server callbacks | ||
|
|
||
| @impl true | ||
| def init(_opts) do | ||
| # Schedule periodic cleanup | ||
| schedule_cleanup() | ||
|
|
||
| config = %{ | ||
| game_creation: %{ | ||
| max_requests: get_env(:max_games_per_ip, 10), | ||
| window_seconds: get_env(:game_creation_window_seconds, 3600) | ||
| }, | ||
| player_creation: %{ | ||
| max_requests: get_env(:max_players_per_ip, 20), | ||
| window_seconds: get_env(:player_creation_window_seconds, 3600) | ||
| }, | ||
| connection: %{ | ||
| max_requests: get_env(:max_connections_per_ip, 50), | ||
| window_seconds: get_env(:connection_window_seconds, 300) | ||
| } | ||
| } | ||
|
|
||
| state = %{ | ||
| requests: %{}, | ||
| config: config | ||
| } | ||
|
|
||
| Logger.info("RateLimiter started with config: #{inspect(config)}") | ||
|
|
||
| {:ok, state} | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_call({:check_rate, ip_address, action}, _from, state) do | ||
| now = System.system_time(:second) | ||
| config = state.config[action] | ||
|
|
||
| ip_requests = get_ip_requests(state, ip_address, action) | ||
|
|
||
| # Filter out expired requests | ||
| valid_requests = Enum.filter(ip_requests, fn timestamp -> | ||
| now - timestamp < config.window_seconds | ||
| end) | ||
|
|
||
| count = length(valid_requests) | ||
| remaining = max(0, config.max_requests - count) | ||
|
|
||
| if count < config.max_requests do | ||
| {:reply, {:ok, remaining}, state} | ||
| else | ||
| oldest_request = List.first(valid_requests) | ||
| retry_after = oldest_request + config.window_seconds - now | ||
|
|
||
| Logger.warning( | ||
| "Rate limit exceeded for IP #{inspect(ip_address)}, action: #{action}, " <> | ||
| "count: #{count}/#{config.max_requests}, retry_after: #{retry_after}s" | ||
| ) | ||
|
|
||
| {:reply, {:error, :rate_limited, retry_after}, state} | ||
| end | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_call({:check_and_record, ip_address, action}, _from, state) do | ||
| now = System.system_time(:second) | ||
| config = state.config[action] | ||
|
|
||
| ip_requests = get_ip_requests(state, ip_address, action) | ||
|
|
||
| # Filter out expired requests | ||
| valid_requests = Enum.filter(ip_requests, fn timestamp -> | ||
| now - timestamp < config.window_seconds | ||
| end) | ||
|
|
||
| count = length(valid_requests) | ||
| remaining = max(0, config.max_requests - count) | ||
|
|
||
| if count < config.max_requests do | ||
| # Atomically record the action before returning success | ||
| updated_requests = [now | valid_requests] | ||
| new_requests = put_in(state.requests, [ip_address, action], updated_requests) | ||
| new_state = %{state | requests: new_requests} | ||
|
|
||
| {:reply, {:ok, remaining}, new_state} | ||
| else | ||
| oldest_request = List.first(valid_requests) | ||
| retry_after = oldest_request + config.window_seconds - now | ||
|
|
||
| Logger.warning( | ||
| "Rate limit exceeded for IP #{inspect(ip_address)}, action: #{action}, " <> | ||
| "count: #{count}/#{config.max_requests}, retry_after: #{retry_after}s" | ||
| ) | ||
|
|
||
| {:reply, {:error, :rate_limited, retry_after}, state} | ||
| end | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_call(:get_config, _from, state) do | ||
| {:reply, state.config, state} | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_cast({:record_action, ip_address, action}, state) do | ||
| now = System.system_time(:second) | ||
|
|
||
| ip_requests = get_ip_requests(state, ip_address, action) | ||
| updated_requests = [now | ip_requests] | ||
|
|
||
| new_requests = put_in( | ||
| state.requests, | ||
| [ip_address, action], | ||
| updated_requests | ||
| ) | ||
|
Comment on lines
+173
to
+177
|
||
|
|
||
| {:noreply, %{state | requests: new_requests}} | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_cast({:clear_ip, ip_address}, state) do | ||
| new_requests = Map.delete(state.requests, ip_address) | ||
| {:noreply, %{state | requests: new_requests}} | ||
| end | ||
|
|
||
| @impl true | ||
| def handle_info(:cleanup, state) do | ||
| now = System.system_time(:second) | ||
|
|
||
| cleaned_requests = state.requests | ||
| |> Enum.map(fn {ip, actions} -> | ||
| cleaned_actions = actions | ||
| |> Enum.map(fn {action, timestamps} -> | ||
| config = state.config[action] | ||
| valid_timestamps = Enum.filter(timestamps, fn timestamp -> | ||
| now - timestamp < config.window_seconds | ||
| end) | ||
| {action, valid_timestamps} | ||
| end) | ||
| |> Enum.filter(fn {_action, timestamps} -> length(timestamps) > 0 end) | ||
| |> Map.new() | ||
|
|
||
| {ip, cleaned_actions} | ||
| end) | ||
| |> Enum.filter(fn {_ip, actions} -> map_size(actions) > 0 end) | ||
| |> Map.new() | ||
|
|
||
| schedule_cleanup() | ||
|
|
||
| {:noreply, %{state | requests: cleaned_requests}} | ||
| end | ||
|
|
||
| # Private helpers | ||
|
|
||
| defp get_ip_requests(state, ip_address, action) do | ||
| get_in(state.requests, [ip_address, action]) || [] | ||
| end | ||
|
|
||
| defp schedule_cleanup do | ||
| Process.send_after(self(), :cleanup, @cleanup_interval) | ||
| end | ||
|
|
||
| defp get_env(key, default) do | ||
| Application.get_env(:copi, __MODULE__, []) | ||
| |> Keyword.get(key, default) | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| defmodule CopiWeb.Helpers.IPHelper do | ||
| @moduledoc """ | ||
| Helper functions for extracting and formatting IP addresses from socket connections. | ||
| """ | ||
|
|
||
| @doc """ | ||
| Extracts the IP address from a LiveView socket connection. | ||
|
|
||
| Returns a string representation of the IP address (IPv4 or IPv6). | ||
| Raises an error if the IP address cannot be determined, as this should never | ||
| happen in a properly configured backend environment. | ||
|
|
||
| ## Examples | ||
|
|
||
| iex> get_connect_ip(socket) | ||
| "192.168.1.1" | ||
|
|
||
| iex> get_connect_ip(socket) | ||
| "2001:db8::1" | ||
| """ | ||
| def get_connect_ip(socket) do | ||
| case Phoenix.LiveView.get_connect_info(socket, :peer_data) do | ||
| %{address: address} when is_tuple(address) -> | ||
| # Use Erlang's :inet.ntoa for proper IPv4/IPv6 formatting | ||
| address | ||
| |> :inet.ntoa() | ||
| |> to_string() | ||
|
|
||
| nil -> | ||
| raise "Unable to determine IP address from socket connection. peer_data is nil. " <> | ||
| "Ensure endpoint.ex has :peer_data in connect_info list." | ||
|
|
||
| other -> | ||
| raise "Unexpected peer_data format: #{inspect(other)}" | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
put_in/3with a list of keys on a potentially non-existent nested path will raise an error if the IP address key doesn't exist in the map. UseKernel.put_in/3or ensure the path exists first, or consider usingMap.put/3withMap.get/3for safer nested map updates.