Bref\SymfonyRequestBridge using proxy settings instead of sourceIp

[rmbl]

I'm using the Bref runtime to run a Symfony application in Lambda with CloudFront in front of it. Getting the source ip address with that layout is hard, esp. because the SymfonyRequestBridge is overriding the proxy configuration when starting up.

While reading the code I was wondering, why the Bridge is using the proxy configuration at all, instead of using the Lambda provided clientIp which already contains the correct value.

By replacing (SymfonyRequestBridge:40)

            'REMOTE_ADDR' => '127.0.0.1',

with

            'REMOTE_ADDR' => $event->getRequestContext()['http']['sourceIp'],

I was able to fix this issue for me while testing.

[alexander-schranz]

Thank you for creating an issue.

Cloudfront as I know would set a X-Forwarded-For header which should be in the following lines correcty be set at:

runtime/src/bref/src/SymfonyRequestBridge.php

Lines 45 to 47 in aa6bd3e

	foreach ($event->getHeaders() as $name => $values) {
	$server['HTTP_'.strtoupper($name)] = $values[0];
	}

That symfony uses the X-Forwarded-For Header as client IP you need to make sure to define the CloudFront Servers as trusted proxies as it would use instead the false server:

https://github.com/symfony/symfony/blob/dd8c1dd9bac12563dcd962c9fd7e7ad1af9120b5/src/Symfony/Component/HttpFoundation/Request.php#L807-L811

For this have a look at the trusted proxies configuration:

https://symfony.com/doc/current/deployment/proxies.html#solution-settrustedproxies

Let us know if that would fix the issue.

[rmbl]

It would not as the configuration is overwritten in the first line of that function:

runtime/src/bref/src/SymfonyRequestBridge.php

Line 25 in aa6bd3e

Request::setTrustedProxies(['127.0.0.1'], Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO);

That's what I tried first and that then led me to the other fix as I'm used to using that value from node.js lambdas.

[alexander-schranz]

@rmbl if you comment that line out it works like expected after configuring the trusted proxies?

Maybe we should merge the set trusted proxies together e.g.:

Request::setTrustedProxies(array_unique(array_merge(['127.0.0.1'], Request::getTrustedProxies())), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_HOST | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO); 

Or we find a better position to call Request::setTrustedProxies before we are starting the Kernel so the Kernel can overwrite setTrustedProxies.

[alexander-schranz]

There seems already been a discussion here about the proxies be set there:

#31 (comment)

Before the refractoring it was called inside the handlers: https://github.com/php-runtime/runtime/pull/21/files

And so was overwriteable and now with the request bridge it is not longer. So I marked this issue as a bug.

So maybe we should move the Request::setTrustedProxies calls back to the Handlers as this is a function that is also only required to be called once and don't need to be called for every request again.

[rmbl]

Sounds good.
The overwriting was surprising as even the bref documentation mentions setting the proxy settings using the framework.yaml configuration.

[alexander-schranz]

@rmbl can you give #104 a try and let us know if that would work?

[rmbl]

I'll try it tomorrow. Thanks!

[rmbl]

Proxy configuration is overwriteable with the patch. Looks good for that. Guess I'll have to add the CloudFront IPs through an EventListener before the Firewall handles it.

[alexander-schranz]

The cloudfront IPs you should able to add in the framework trusted proxies configuration like document in the symfony docs: https://symfony.com/doc/current/deployment/proxies.html#solution-settrustedproxies

There should no other thing be required. Else it would still be an issue.

See also the part about changing ips: https://symfony.com/doc/current/deployment/proxies.html#but-what-if-the-ip-of-my-reverse-proxy-changes-constantly

As you are sure you always behind a trusted proxies and your server can not be called without the trusted proxies you can the special REMOTE_ADDR string as always trusted.

[rmbl]

CloudFront has a huge amount of IPs (that can change). So it's not really possible to configure it there.

REMOTE_ADDR doesn't help as there are several layers. REMOTE_ADDR gets resolved to 127.0.0.1 and is removed by the proxy configuration. We recieve sth. like 127.0.0.1,<cloudfront ip>,<real remote ip> so we actually need the third ip.

[alexander-schranz]

But if the REMOTE_ADDR is the one calling and is forwarding the x-forwarded-for headers correctly the REMOTE_ADDR should be enough. Best is if you try to debug e.g.: $request->isFromTrustedProxy() method e.g. by adding a response listener with e.g.:

$response->headers->set('X-IS_TRUSTED_PROXY', $request->isFromTrustedProxy() ? 'YES' : 'NO');
$response->headers->set('X-REMOTE_ADDR', $request->server->get('REMOTE_ADDR', 'none'));
$response->headers->set('X-TRUSTED_PROXIES', json_encode($request->getTrustedProxies()));

Aslong as isFromTrustedProxy is false it will error. And configure REMOTE_ADDR as trusted proxies will normally trust all the X-FORWARDED-* headers. Can you tell what the returns of the above headers are in your case?