Skip to content

Commit

Permalink
IPs can be configured to bypass signed queries #9696
Browse files Browse the repository at this point in the history
The configuration `allowedIps` supports a list of IP or subnet in v4 or
v6. If the request is coming from one of those ranges, and it is not
signed at all, then it will be accepted.

However, if the request is signed, the `allowedIps` has no effect at all.
This is because we want to be able to test signed queries even from an
allowed IP.
  • Loading branch information
PowerKiKi committed Jul 24, 2023
1 parent 5311a4c commit 27e9dba
Show file tree
Hide file tree
Showing 6 changed files with 207 additions and 10 deletions.
1 change: 1 addition & 0 deletions src/ConfigProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public function __invoke(): array
'signedQueries' => [
'required' => true,
'keys' => [],
'allowedIps' => [],
],
'dependencies' => [
'invokables' => [
Expand Down
29 changes: 27 additions & 2 deletions src/Middleware/SignedQueryMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Ecodev\Felix\Middleware;

use Cake\Chronos\Chronos;
use Ecodev\Felix\Validator\IPRange;
use Exception;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
Expand All @@ -30,8 +31,11 @@
*/
final class SignedQueryMiddleware implements MiddlewareInterface
{
public function __construct(private readonly array $keys, private readonly bool $required = true)
{
public function __construct(
private readonly array $keys,
private readonly array $allowedIps,
private readonly bool $required = true
) {
if ($this->required && !$this->keys) {
throw new Exception('Signed queries are required, but no keys are configured');
}
Expand All @@ -50,6 +54,10 @@ private function verify(ServerRequestInterface $request): void
{
$autorization = $request->getHeader('authorization')[0] ?? '';
if (!$autorization) {
if ($this->isAllowedIp($request)) {
return;
}

throw new Exception('Missing `Authorization` HTTP header in signed query', 403);
}

Expand Down Expand Up @@ -107,4 +115,21 @@ private function getOperations(ServerRequestInterface $request): mixed

throw new Exception('Could not find GraphQL operations in request', 403);
}

private function isAllowedIp(ServerRequestInterface $request): bool
{
$remoteAddress = $request->getServerParams()['REMOTE_ADDR'] ?? '';

if (!$remoteAddress || !is_string($remoteAddress)) {
return false;
}

foreach ($this->allowedIps as $allowedIp) {
if (IPRange::matches($remoteAddress, $allowedIp)) {
return true;
}
}

return false;
}
}
2 changes: 1 addition & 1 deletion src/Middleware/SignedQueryMiddlewareFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,6 @@ public function __invoke(ContainerInterface $container, $requestedName, ?array $
$config = $container->get('config');
$signedQueries = $config['signedQueries'];

return new SignedQueryMiddleware($signedQueries['keys'], $signedQueries['required']);
return new SignedQueryMiddleware($signedQueries['keys'], $signedQueries['allowedIps'], $signedQueries['required']);
}
}
98 changes: 98 additions & 0 deletions src/Validator/IPRange.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
<?php

declare(strict_types=1);

namespace Ecodev\Felix\Validator;

abstract class IPRange
{
final public static function matches(string $ip, string $cidr): bool
{
if (str_contains($ip, ':')) {
return self::matchesIPv6($ip, $cidr);
}

return self::matchesIPv4($ip, $cidr);
}

private static function matchesIPv4(string $ip, string $cidr): bool
{
$mask = 32;
$subnet = $cidr;

if (str_contains($cidr, '/')) {
[$subnet, $mask] = explode('/', $cidr, 2);
$mask = (int) $mask;
}

if ($mask < 0 || $mask > 32) {
return false;
}

$ip = ip2long($ip);
$subnet = ip2long($subnet);
if (false === $ip || false === $subnet) {
// Invalid data
return false;
}

return 0 === substr_compare(
sprintf('%032b', $ip),
sprintf('%032b', $subnet),
0,
$mask
);
}

private static function matchesIPv6(string $ip, string $cidr): bool
{
$mask = 128;
$subnet = $cidr;

if (str_contains($cidr, '/')) {
[$subnet, $mask] = explode('/', $cidr, 2);
$mask = (int) $mask;
}

if ($mask < 0 || $mask > 128) {
return false;
}

$ip = inet_pton($ip);
$subnet = inet_pton($subnet);

if (false === $ip || false === $subnet) {
// Invalid data
return false;
}

// mask 0: if it's a valid IP, it's valid
if ($mask === 0) {
return (bool) unpack('n*', $ip);
}

/** @see http://stackoverflow.com/questions/7951061/matching-ipv6-address-to-a-cidr-subnet, MW answer */
$binMask = str_repeat('f', (int) ($mask / 4));
switch ($mask % 4) {
case 0:
break;
case 1:
$binMask .= '8';

break;
case 2:
$binMask .= 'c';

break;
case 3:
$binMask .= 'e';

break;
}

$binMask = str_pad($binMask, 32, '0');
$binMask = pack('H*', $binMask);

return ($ip & $binMask) === $subnet;
}
}
32 changes: 25 additions & 7 deletions tests/Middleware/SignedQueryMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,29 @@ protected function tearDown(): void
/**
* @dataProvider dataProviderQuery
*/
public function testRequiredSignedQuery(array $keys, string $body, null|array $parsedBody, string $signature, string $expectExceptionMessage = ''): void
public function testRequiredSignedQuery(array $keys, string $body, null|array $parsedBody, string $signature, string $expectExceptionMessage = '', string $ip = ''): void
{
$this->process($keys, true, $body, $parsedBody, $signature, $expectExceptionMessage);
$this->process($keys, true, $ip, $body, $parsedBody, $signature, $expectExceptionMessage);
}

/**
* @dataProvider dataProviderQuery
*/
public function testNonRequiredSignedQuery(array $keys, string $body, null|array $parsedBody, string $signature): void
{
$this->process($keys, false, $body, $parsedBody, $signature, '');
$this->process($keys, false, '', $body, $parsedBody, $signature, '');
}

public function testThrowIfNoKeys(): void
{
$this->expectExceptionMessage('Signed queries are required, but no keys are configured');
$this->expectExceptionCode(0);
new SignedQueryMiddleware([]);
new SignedQueryMiddleware([], []);
}

private function process(array $keys, bool $required, string $body, null|array $parsedBody, string $signature, string $expectExceptionMessage): void
private function process(array $keys, bool $required, string $ip, string $body, null|array $parsedBody, string $signature, string $expectExceptionMessage): void
{
$request = new ServerRequest();
$request = new ServerRequest(['REMOTE_ADDR' => $ip]);
$request = $request->withBody(new CallbackStream(fn () => $body))->withParsedBody($parsedBody);

if ($signature) {
Expand All @@ -61,7 +61,7 @@ private function process(array $keys, bool $required, string $body, null|array $
->method('handle')
->willReturn(new Response());

$middleware = new SignedQueryMiddleware($keys, $required);
$middleware = new SignedQueryMiddleware($keys, ['1.2.3.4', '2a01:198:603:0::/65'], $required);

if ($expectExceptionMessage) {
$this->expectExceptionMessage($expectExceptionMessage);
Expand Down Expand Up @@ -180,5 +180,23 @@ public function dataProviderQuery(): iterable
'v1.1577964600.' . str_repeat('a', 64),
'Could not find GraphQL operations in request',
];

yield 'no header, but allowed IPv4' => [
[$key1],
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
null,
'',
'',
'1.2.3.4',
];

yield 'simple but wrong key will still error even if IP is allowed, because we want to be able to test signature even when allowed' => [
[$key2],
'{"operationName":"CurrentUser","variables":{},"query":"query CurrentUser { viewer { id }}',
null,
'v1.1577964600.a4d664cd3d9903e4fecf6f9f671ad953586a7faeb16e67c306fd9f29999dfdd7',
'Invalid signed query',
'1.2.3.4',
];
}
}
55 changes: 55 additions & 0 deletions tests/Validator/IPRangeTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php

declare(strict_types=1);

namespace EcodevTests\Felix\Validator;

use Ecodev\Felix\Validator\IPRange;
use PHPUnit\Framework\TestCase;

class IPRangeTest extends TestCase
{
/**
* @dataProvider IPv4Data
* @dataProvider IPv6Data
*/
public function testMatches(bool $result, string $remoteAddr, string $cidr): void
{
self::assertSame($result, IPRange::matches($remoteAddr, $cidr));
}

public function IPv4Data(): iterable
{
return [
'valid - exact (no mask; /32 equiv)' => [true, '192.168.1.1', '192.168.1.1'],
'valid - entirety of class-c (/1)' => [true, '192.168.1.1', '192.168.1.1/1'],
'valid - class-c private subnet (/24)' => [true, '192.168.1.1', '192.168.1.0/24'],
'valid - any subnet (/0)' => [true, '1.2.3.4', '0.0.0.0/0'],
'valid - subnet expands to all' => [true, '1.2.3.4', '192.168.1.0/0'],
'invalid - class-a invalid subnet' => [false, '192.168.1.1', '1.2.3.4/1'],
'invalid - CIDR mask out-of-range' => [false, '192.168.1.1', '192.168.1.1/33'],
'invalid - invalid cidr notation' => [false, '1.2.3.4', '256.256.256/0'],
'invalid - invalid IP address' => [false, 'an_invalid_ip', '192.168.1.0/24'],
'invalid - empty IP address' => [false, '', '1.2.3.4/1'],
'invalid - proxy wildcard' => [false, '192.168.20.13', '*'],
'invalid - proxy missing netmask' => [false, '192.168.20.13', '0.0.0.0'],
'invalid - request IP with invalid proxy wildcard' => [false, '0.0.0.0', '*'],
];
}

public function IPv6Data(): iterable
{
return [
'valid - ipv4 subnet' => [true, '2a01:198:603:0:396e:4789:8e99:890f', '2a01:198:603:0::/65'],
'valid - exact' => [true, '0:0:0:0:0:0:0:1', '::1'],
'valid - all subnets' => [true, '0:0:603:0:396e:4789:8e99:0001', '::/0'],
'valid - subnet expands to all' => [true, '0:0:603:0:396e:4789:8e99:0001', '2a01:198:603:0::/0'],
'invalid - not in subnet' => [false, '2a00:198:603:0:396e:4789:8e99:890f', '2a01:198:603:0::/65'],
'invalid - does not match exact' => [false, '2a01:198:603:0:396e:4789:8e99:890f', '::1'],
'invalid - compressed notation, does not match exact' => [false, '0:0:603:0:396e:4789:8e99:0001', '::1'],
'invalid - garbage IP' => [false, '}__test|O:21:&quot;JDatabaseDriverMysqli&quot;:3:{s:2', '::1'],
'invalid - invalid cidr' => [false, '2a01:198:603:0:396e:4789:8e99:890f', 'unknown'],
'invalid - empty IP address' => [false, '', '::1'],
];
}
}

0 comments on commit 27e9dba

Please sign in to comment.