From d01ea6f95d54c0b0727fcd260c65054372b8ef8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zieli=C5=84ski?= Date: Thu, 17 Oct 2024 16:53:47 +0100 Subject: [PATCH 1/5] [CORS Proxy] Rate-limit entire /64 subnets instead of specific addresses. This prevents a person with an entire /64 subnet from exhausting the storage or getting more than their fair share of the tokens. ## Implementation Converts a string-based IP address into a binary string, then zeros the first 64 bits in that string and re-encodes it as a human-readable IP string. ## Testing instructions Run unit tests ```php php ./packages/playground/website-deployment/tests.php ``` cc @brandonpayton for reviews --- .../website-deployment/cors-proxy-config.php | 91 +++++++++++++++++-- .../playground/website-deployment/tests.php | 35 +++++++ 2 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 packages/playground/website-deployment/tests.php diff --git a/packages/playground/website-deployment/cors-proxy-config.php b/packages/playground/website-deployment/cors-proxy-config.php index cea6bc5058..3a67b3d1bf 100644 --- a/packages/playground/website-deployment/cors-proxy-config.php +++ b/packages/playground/website-deployment/cors-proxy-config.php @@ -86,12 +86,13 @@ public function obtain_token($remote_ip, $bucket_config) { return false; } - // @TODO: Handle IPv6 addresses in a way that cannot lead to storage exhaustion. - $ipv6_remote_ip = $remote_ip; - if (filter_var($remote_ip, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { - // Convert IPv4 to IPv6 mapped address for storage - $ipv6_remote_ip = '::ffff:' . $remote_ip; - } + /** + * Aggregate addresses into /64 subnets. + * This prevents a person with an entire /64 subnet from + * exhausting the storage or getting more than their fair + * share of the tokens. + */ + $remote_subnet = playground_ip_to_a_64_subnet($remote_ip); $token_query = <<<'SQL' INSERT INTO cors_proxy_rate_limiting ( @@ -151,7 +152,7 @@ public function obtain_token($remote_ip, $bucket_config) { mysqli_stmt_bind_param( $token_statement, 'sii', - $ipv6_remote_ip, + $remote_subnet, $bucket_config->capacity, $bucket_config->fill_rate_per_minute ); @@ -165,6 +166,7 @@ public function obtain_token($remote_ip, $bucket_config) { return false; } + } function playground_cors_proxy_maybe_rate_limit() { @@ -194,4 +196,77 @@ function playground_cors_proxy_maybe_rate_limit() { } finally { $token_bucket->dispose(); } -} \ No newline at end of file +} + +/** + * Converts the IP address to a key used for rate limiting. + * It groups addresses into buckets of size /64. + * + * @return string The encoded IPv6 address or null if the input is not a valid IP address. + */ +function playground_ip_to_a_64_subnet(string $ip_v4_or_v6): string { + $ipv6_remote_ip = $ip_v4_or_v6; + if (filter_var($ip_v4_or_v6, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { + /** + * Convert IPv4 to IPv6 mapped address for storage. + * Do not group these addresses into buckets since + * the number of IPv4 addresses is less than the number + * of IPv6 addresses. + */ + $ipv6_remote_ip = '::ffff:' . $ip_v4_or_v6; + } else { + /** + * Zero out the last 64 bits of the IPv6 address for storage. + * This means we're only considering the first 64 bits of the + * address for rate limiting. This way, a person with an entire + * /64 subnet cannot get more than their fair share of the + * tokens. + */ + $ipv6_block = playground_get_ipv6_block($ipv6_remote_ip, 64); + if ($ipv6_block === null) { + error_log('Failed to get IPv6 block for ' . $ip_v4_or_v6); + // Use the original IP address as a fallback when the block + // cannot be determined. + return $ip_v4_or_v6; + } + $ipv6_remote_ip = playground_encode_ipv6($ipv6_block); + } + return $ipv6_remote_ip; +} + +/** + * Returns the /64 subnet of the given IPv6 address. + */ +function playground_get_ipv6_block(string $ipv6_remote_ip, int $block_size=64): ?string { + $ip = inet_pton($ipv6_remote_ip); + // $ip is a binary string of length 16, each bit represents a bit + // of the ipv6 address. + if ($ip === false) { + return null; + } + + if($block_size % 8 !== 0) { + // We're using a naive substr-based approach that reasons about + // groups of 8 bits (characters) and not separately about each bit. + // This approach can only support block sizes that are multiplies + // of 8. + throw new Exception('Block size must be a multiple of 8.'); + } + + $subnet_length = $block_size / 8; + $requested_block = substr($ip, 0, $subnet_length); + $backfill_zeros = str_repeat(chr(0), 16 - $subnet_length); + return $requested_block . $backfill_zeros; +} + +function playground_encode_ipv6(string $ipv6): string { + $hex_string = bin2hex($ipv6); + $hex_string_length = strlen($hex_string); + + // Split the hex string into 4 groups of 4 characters. + $groups = []; + for ($i = 0; $i < $hex_string_length; $i += 4) { + $groups[] = substr($hex_string, $i, 4); + } + return strtoupper(implode(':', $groups)); +} diff --git a/packages/playground/website-deployment/tests.php b/packages/playground/website-deployment/tests.php new file mode 100644 index 0000000000..e8cb3c9096 --- /dev/null +++ b/packages/playground/website-deployment/tests.php @@ -0,0 +1,35 @@ + Date: Tue, 22 Oct 2024 13:20:49 -0400 Subject: [PATCH 2/5] Add an implementation note --- .../website-deployment/cors-proxy-config.php | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/packages/playground/website-deployment/cors-proxy-config.php b/packages/playground/website-deployment/cors-proxy-config.php index 3a67b3d1bf..06f9a3ea44 100644 --- a/packages/playground/website-deployment/cors-proxy-config.php +++ b/packages/playground/website-deployment/cors-proxy-config.php @@ -109,7 +109,7 @@ public function obtain_token($remote_ip, $bucket_config) { ? AS fill_rate_per_minute ), bucket AS ( - SELECT + SELECT remote_addr, tokens AS previous_tokens, updated_at AS previous_updated_at, @@ -128,7 +128,7 @@ public function obtain_token($remote_ip, $bucket_config) { config.remote_addr, config.capacity, config.fill_rate_per_minute, - -- Make sure we stay within bounds. + -- Make sure we stay within bounds. GREATEST( 0, COALESCE(bucket.available_tokens, config.capacity) - 1 @@ -147,7 +147,7 @@ public function obtain_token($remote_ip, $bucket_config) { NOW() ) SQL; - + $token_statement = mysqli_prepare($this->dbh, $token_query); mysqli_stmt_bind_param( $token_statement, @@ -163,7 +163,7 @@ public function obtain_token($remote_ip, $bucket_config) { ) { return true; } - + return false; } @@ -185,7 +185,7 @@ function playground_cors_proxy_maybe_rate_limit() { error_log('Invalid IP address: ' . var_export($remote_ip, true)); return false; } - + $token_bucket = new PlaygroundCorsProxyTokenBucket(); try { if (!$token_bucket->obtain_token($remote_ip, $bucket_config)) { @@ -201,7 +201,7 @@ function playground_cors_proxy_maybe_rate_limit() { /** * Converts the IP address to a key used for rate limiting. * It groups addresses into buckets of size /64. - * + * * @return string The encoded IPv6 address or null if the input is not a valid IP address. */ function playground_ip_to_a_64_subnet(string $ip_v4_or_v6): string { @@ -209,7 +209,7 @@ function playground_ip_to_a_64_subnet(string $ip_v4_or_v6): string { if (filter_var($ip_v4_or_v6, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) { /** * Convert IPv4 to IPv6 mapped address for storage. - * Do not group these addresses into buckets since + * Do not group these addresses into buckets since * the number of IPv4 addresses is less than the number * of IPv6 addresses. */ @@ -221,6 +221,14 @@ function playground_ip_to_a_64_subnet(string $ip_v4_or_v6): string { * address for rate limiting. This way, a person with an entire * /64 subnet cannot get more than their fair share of the * tokens. + * + * NOTE: This measure may be a bit heavy-handed to start with, + * but we will not know until we are actually seeing requests + * from IPv6 requests in the wild. If individual users are + * experiencing too much rate-limiting, we can consider + * limiting individual addresses first and only addressing + * entire blocks once an IPv6 block has a number of IPs + * that are hitting rate limits. */ $ipv6_block = playground_get_ipv6_block($ipv6_remote_ip, 64); if ($ipv6_block === null) { From 8a746e8c4510b54c5b2aa1d461cca69458809cb5 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Tue, 22 Oct 2024 23:14:36 -0400 Subject: [PATCH 3/5] Fix typo --- packages/playground/website-deployment/cors-proxy-config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playground/website-deployment/cors-proxy-config.php b/packages/playground/website-deployment/cors-proxy-config.php index 06f9a3ea44..bc27a73803 100644 --- a/packages/playground/website-deployment/cors-proxy-config.php +++ b/packages/playground/website-deployment/cors-proxy-config.php @@ -256,7 +256,7 @@ function playground_get_ipv6_block(string $ipv6_remote_ip, int $block_size=64): if($block_size % 8 !== 0) { // We're using a naive substr-based approach that reasons about // groups of 8 bits (characters) and not separately about each bit. - // This approach can only support block sizes that are multiplies + // This approach can only support block sizes that are multiples // of 8. throw new Exception('Block size must be a multiple of 8.'); } From e91602134be5ae304ed03d9a65b3264e88fd73e8 Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Tue, 22 Oct 2024 23:34:33 -0400 Subject: [PATCH 4/5] Enforce max IPv6 block size --- .../playground/website-deployment/cors-proxy-config.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/playground/website-deployment/cors-proxy-config.php b/packages/playground/website-deployment/cors-proxy-config.php index bc27a73803..5153a17722 100644 --- a/packages/playground/website-deployment/cors-proxy-config.php +++ b/packages/playground/website-deployment/cors-proxy-config.php @@ -253,7 +253,7 @@ function playground_get_ipv6_block(string $ipv6_remote_ip, int $block_size=64): return null; } - if($block_size % 8 !== 0) { + if ($block_size % 8 !== 0) { // We're using a naive substr-based approach that reasons about // groups of 8 bits (characters) and not separately about each bit. // This approach can only support block sizes that are multiples @@ -261,6 +261,11 @@ function playground_get_ipv6_block(string $ipv6_remote_ip, int $block_size=64): throw new Exception('Block size must be a multiple of 8.'); } + $max_block_size = 128; + if ($block_size > $max_block_size) { + throw new Exception('Block size must be less than or equal to 128.'); + } + $subnet_length = $block_size / 8; $requested_block = substr($ip, 0, $subnet_length); $backfill_zeros = str_repeat(chr(0), 16 - $subnet_length); From 86f5019e26b28e43b71fc9d07b15e795bc9a07cd Mon Sep 17 00:00:00 2001 From: Brandon Payton Date: Tue, 22 Oct 2024 23:35:08 -0400 Subject: [PATCH 5/5] Test IPv6 block constraints --- .../playground/website-deployment/tests.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/playground/website-deployment/tests.php b/packages/playground/website-deployment/tests.php index e8cb3c9096..0a850f0886 100644 --- a/packages/playground/website-deployment/tests.php +++ b/packages/playground/website-deployment/tests.php @@ -10,6 +10,20 @@ function assert_equal($expected, $actual, $message='') { } } +function assert_throws($expected_message, $callback) { + try { + $callback(); + } catch (Exception $e) { + if ($e->getMessage() !== $expected_message) { + echo "Test failed.\nExpected: $expected_message\nActual: {$e->getMessage()}\n"; + die(); + } + return; + } + echo "Test failed.\nExpected: $expected_message\nActual: No exception was thrown\n"; + die(); +} + assert_equal( '2607:B4C0:0000:0000:0000:0000:0000:0000', playground_ip_to_a_64_subnet( @@ -32,4 +46,24 @@ function assert_equal($expected, $actual, $message='') { 'A part of the IPv4 range was lost' ); +assert_throws( + 'Block size must be a multiple of 8.', + function () { + playground_get_ipv6_block( + '2607:B4C0:AAAA:BBBB:CCCC:DDDD:EEEE:FFFF', + 8 - 1 + ); + } +); + +assert_throws( + 'Block size must be less than or equal to 128.', + function () { + playground_get_ipv6_block( + '2607:B4C0:AAAA:BBBB:CCCC:DDDD:EEEE:FFFF', + 128 + 8 + ); + } +); + echo 'All tests passed';