Baldinof / roadrunner-bundle

A RoadRunner worker integrated in your Symfony app
MIT License
266 stars 47 forks source link

Running RoadRunner in ECS behind an ALB #69

Closed digitalkaoz closed 2 years ago

digitalkaoz commented 2 years ago

Hey,

startup works fine, but i see the app somehow assumes the current domain is the internal ECS Container IP. How can we fix this?

So basically its a FORWARDED issue i guess.

My config:

version: "2.7"

server:
  env:
    - APP_RUNTIME: Baldinof\RoadRunnerBundle\Runtime\Runtime

  command: "php bin/console baldinof:roadrunner:worker"

http:
  address: "0.0.0.0:8000"
  middleware: [ "static", "gzip" ]
  #pool:
  #  num_workers: 8
  static:
    dir: "public"
    forbid: [ ".php", ".htaccess" ]

logs:
  level: warn

it works fine locally but not behind an LB

in our Symfony Front Controller we had to do this

$trustedProxies = $_ENV['TRUSTED_PROXIES'];
if ($trustedProxies) {
    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_FORWARDED | Request::HEADER_X_FORWARDED_AWS_ELB);
}

using public/index.php somehow didnt work (the symfony/runtime is installed)

Baldinof commented 2 years ago

Hi!

What version of Symfony / RR / this bundle are you using?

I think the code you modified here https://github.com/Baldinof/roadrunner-bundle/pull/70 might not be needed anymore as Symfony handle that with config now (https://symfony.com/doc/current/deployment/proxies.html#solution-settrustedproxies).

I will try to have a closer look tomorow

digitalkaoz commented 2 years ago

I think its not about about trusted proxies, its about parsing the FORWARD header. Symfony5.4 & Roadrunner 2.8

Baldinof @.***> schrieb am Do., 17. Feb. 2022, 22:35:

Hi!

What version of Symfony / RR / this bundle are you using?

I think the code you modified here #70 https://github.com/Baldinof/roadrunner-bundle/pull/70 might not be needed anymore as Symfony handle that with config now ( https://symfony.com/doc/current/deployment/proxies.html#solution-settrustedproxies ).

I will try to have a closer look tomorow

— Reply to this email directly, view it on GitHub https://github.com/Baldinof/roadrunner-bundle/issues/69#issuecomment-1043472128, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACHVV5F6IVHCCBKUB6IFQLU3VS3JANCNFSM5OVG5CLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

Baldinof commented 2 years ago

There is a trusted_headers config option to trust the Forwarded header.

Also I think if you are behind a AWL LB you need to use REMOTE_ADDR in trusted_proxies see: https://symfony.com/doc/current/deployment/proxies.html#but-what-if-the-ip-of-my-reverse-proxy-changes-constantly

The problem is Symfony will resolve the REMOTE_ADDR on worker boot, but the value might change during a worker life as the request might came from another LB.

I faced this issued on Google Cloud Run and used a middleware like this:

class TrustedProxiesMiddleware implements MiddlewareInterface
{
    public function __construct(private string $trustedProxies, private int $trustedHeaderSet)
   {}

    public function process(Request $request, HttpKernelInterface $next): \Iterator
    {
        if (str_contains($this->trustedProxies, 'REMOTE_ADDR')) {
            $proxies = [];
            foreach(explode(',', $this->trustedProxies) as $proxy) {
                $proxy = trim($proxy);
                if ($proxy === 'REMOTE_ADDR') {
                    $proxy = $request->server->get('REMOTE_ADDR');
                }
                if ($proxy) {
                    $proxies[] = $proxy;
                }
            }

            Request::setTrustedProxies($proxies, $this->trustedHeaderSet);
        }

        yield $next->handle($request);
    }
}

Where the injected $trustedProxies and $trustedHeaderSet args should be kernel parameters kernel.trusted_proxies and kernel.trusted_headers.

If it works for you, I think this middleware could be added to this bundle.

digitalkaoz commented 2 years ago

Hey,

sounds good, doesnt work, maybe i made a mistake, here are my changes:

// framework.yml
framework:
  trusted_proxies: '%env(string:TRUSTED_PROXIES)%,REMOTE_ADDR'
  trusted_headers: ['forwarded']
// baldinof.yml
services:
  Shopware\Production\TrustedProxiesMiddleware:
    arguments:
      $trustedHeaderSet: '%kernel.trusted_headers%'
      $trustedProxies: '%kernel.trusted_proxies%'

baldinof_road_runner:
  middlewares:
    - Shopware\Production\TrustedProxiesMiddleware
<?php
namespace Shopware\Production;

use Baldinof\RoadRunnerBundle\Http\MiddlewareInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\HttpKernelInterface;

class TrustedProxiesMiddleware implements MiddlewareInterface
{
    private string $trustedProxies;
    private int $trustedHeaderSet;

    public function __construct(string $trustedProxies, int $trustedHeaderSet)
    {
        $this->trustedHeaderSet = $trustedHeaderSet;
        $this->trustedProxies = $trustedProxies;
    }

    public function process(Request $request, HttpKernelInterface $next): \Iterator
    {
        if (str_contains($this->trustedProxies, 'REMOTE_ADDR')) {
            $proxies = [];
            foreach(explode(',', $this->trustedProxies) as $proxy) {
                $proxy = trim($proxy);
                if ($proxy === 'REMOTE_ADDR') {
                    $proxy = $request->server->get('REMOTE_ADDR');
                }
                if ($proxy) {
                    $proxies[] = $proxy;
                }
            }

            Request::setTrustedProxies($proxies, $this->trustedHeaderSet);
        }

        yield $next->handle($request);
    }
}
//.rr.yaml
version: "2.7"

server:
  env:
    - APP_RUNTIME: Baldinof\RoadRunnerBundle\Runtime\Runtime
  command: "php bin/console baldinof:roadrunner:worker"

http:
  address: "0.0.0.0:8000"
  middleware: [ "static", "gzip" ]
  #pool:
  #  num_workers: 8

  static:
    dir: "public"
    forbid: [ ".php", ".htaccess" ]

logs:
  level: warn

(basically following https://symfony.com/doc/current/deployment/proxies.html)

i openend a Ticket at roadrunner itself too (for the FORWARDED header stuff): https://github.com/roadrunner-server/http/pull/9 maybe its needed to work properly (have to wait for a 2.8.1 release)

any hint what might be missing?

The whole application works fine when running with Nginx & FPM.

My Setup is ALB->RR->Symfony

Baldinof commented 2 years ago

OK, thanks for trying this out.

I tried a fix here https://github.com/Baldinof/roadrunner-bundle/pull/71 Can you test with the branch from this PR?

digitalkaoz commented 2 years ago

still nope...

the app itself requests the following variables (which should be set correctly when the trusted proxies/headers stuff ist set correctly):

            host: '{{ app.request.host }}',
            port: {{ app.request.port }},
            scheme: '{{ app.request.scheme }}',
            schemeAndHttpHost: '{{ app.request.schemeAndHttpHost }}',
            uri: '{{ app.request.uri }}',
digitalkaoz commented 2 years ago

i have a feeling we need to wait for a new RR version? from what i can read in your code, it seems correct to me

Baldinof commented 2 years ago

I think this should work on the Symfony side:

I tried with this controller

    #[Route('/test', name: 'test')]
    public function test(Request $request)
    {
        return $this->json([
            'https' => $request->isSecure(),
            'generated_url' => $this->generateUrl('test', referenceType: UrlGeneratorInterface::ABSOLUTE_URL),
            'current_url' => $request->getUri(),
            'real_ip' => $request->getClientIp(),
        ]);
    }

With config:

framework:
    trusted_proxies: 'REMOTE_ADDR'
    trusted_headers: ['forwarded']

Then running curl http://localhost:8080/test -H 'Forwarded: for=192.0.2.60;proto=https;by=203.0.113.43;host=example.org' returns:

{
   "current_url" : "https://example.org/test",
   "generated_url" : "https://example.org/test",
   "https" : true,
   "real_ip" : "192.0.2.60"
}

So it seems OK.

To what I understand from the fix on the RR side, it will make RR hide from the PHP process that it was behind a proxy. The Symfony way of trusting should work as RR is passing the Forwarded header.

What is the result of your twig template?

digitalkaoz commented 2 years ago

You run the symfony App with RR and your bundle?

Baldinof @.***> schrieb am Sa., 19. Feb. 2022, 19:27:

I think this should work on the Symfony side:

I tried with this controller

#[Route('/test', name: 'test')]
public function test(Request $request)
{
    return $this->json([
        'https' => $request->isSecure(),
        'generated_url' => $this->generateUrl('test', referenceType: UrlGeneratorInterface::ABSOLUTE_URL),
        'current_url' => $request->getUri(),
        'real_ip' => $request->getClientIp(),
    ]);
}

With config:

framework: trusted_proxies: 'REMOTE_ADDR' trusted_headers: ['forwarded']

Then running curl http://localhost:8080/test -H 'Forwarded: for=192.0.2.60;proto=https;by=203.0.113.43;host=example.org' returns:

{ "current_url" : "https://example.org/test", "generated_url" : "https://example.org/test", "https" : true, "real_ip" : "192.0.2.60" }

So it seems OK.

To what I understand from the fix on the RR side, it will make RR hide frome the PHP process it was behind a proxy. The Symfony way of trusting should work as RR is passing the Forwarded header.

What is the result of your twig template?

— Reply to this email directly, view it on GitHub https://github.com/Baldinof/roadrunner-bundle/issues/69#issuecomment-1046077994, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACHVV6MRJMRCQ56ZFUVETTU37OJZANCNFSM5OVG5CLQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

digitalkaoz commented 2 years ago

Hey,

running locally through RR and your bundle, i get these:

curl http://localhost:8080/test -H 'Forwarded: for=192.0.2.60;proto=https;by=203.0.113.43;host=example.org'

{
 host: 'example.org',
port: 443,
scheme: 'https',
schemeAndHttpHost: 'https://example.org',
uri: 'https://example.org/admin',
}

so now im a bit puzzled why its not working in AWS :/ In the end if RR fixes it so PHP thinks its running not behind an ALB its fine for me, but i dont understand the issue here

Baldinof commented 2 years ago

Yeah that's weird :/

Maybe you could try to dump the content of $request->headers->all() to see what Symfony is receiving on AWS?

digitalkaoz commented 2 years ago

yeah time for some debugging i guess

Baldinof commented 2 years ago

From what I see on AWS doc it seems there are using X-Forwarded-For/Proto/Port.

Maybe you should try these in trusted_headers config

digitalkaoz commented 2 years ago

mhhh, sometimes its the FORWARDED Header only:

The Symfony App itself got the

x-forwarded-for x-forwarded-proto x-forwarded-port

Headers, but sometimes its only the Forwarded Header, im puzzled (maybe only the ELB Healthchecker), i will allow those headers too and we'll see

digitalkaoz commented 2 years ago

hey @Baldinof i built a debug controller, and its working perfectly fine:

{
    "https": true,
    "host": "xxx.com",
    "port": 443,
    "current_url": "https:\/\/xxx.com\/api\/debug",
    "real_ip": "77.66.55.44"
}

so i assume everything is correct, sadly when it comes to Shopware, its not working as it should, it still gives me the internal IP stuff, maybe the original front controller of symfony is doing something different than your handler?

<?php declare(strict_types=1);

use Shopware\Production\HttpKernel;
use Symfony\Component\Dotenv\Dotenv;
use Symfony\Component\ErrorHandler\Debug;
use Symfony\Component\HttpFoundation\Request;

if (\PHP_VERSION_ID < 70403) {
    header('Content-type: text/html; charset=utf-8', true, 503);

    echo '<h2>Error</h2>';
    echo 'Your server is running PHP version ' . \PHP_VERSION . ' but Shopware 6 requires at least PHP 7.4.3';
    exit(1);
}

$classLoader = require __DIR__ . '/../vendor/autoload.php';

if (!file_exists(dirname(__DIR__) . '/install.lock')) {
    $basePath = 'recovery/install';
    $baseURL = str_replace(basename(__FILE__), '', $_SERVER['SCRIPT_NAME']);
    $baseURL = rtrim($baseURL, '/');
    $installerURL = $baseURL . '/' . $basePath . '/index.php';
    if (strpos($_SERVER['REQUEST_URI'], $basePath) === false) {
        header('Location: ' . $installerURL);
        exit;
    }
}

if (is_file(dirname(__DIR__) . '/files/update/update.json') || is_dir(dirname(__DIR__) . '/update-assets')) {
    header('Content-type: text/html; charset=utf-8', true, 503);
    header('Status: 503 Service Temporarily Unavailable');
    header('Retry-After: 1200');
    if (file_exists(__DIR__ . '/maintenance.html')) {
        readfile(__DIR__ . '/maintenance.html');
    } else {
        readfile(__DIR__ . '/recovery/update/maintenance.html');
    }

    return;
}

$projectRoot = dirname(__DIR__);
if (class_exists(Dotenv::class) && (file_exists($projectRoot . '/.env.local.php') || file_exists($projectRoot . '/.env') || file_exists($projectRoot . '/.env.dist'))) {
    (new Dotenv())->usePutenv()->bootEnv(dirname(__DIR__) . '/.env');
}

$appEnv = $_SERVER['APP_ENV'] ?? $_ENV['APP_ENV'] ?? 'dev';
$debug = (bool) ($_SERVER['APP_DEBUG'] ?? $_ENV['APP_DEBUG'] ?? ($appEnv !== 'prod' && $appEnv !== 'e2e'));

if ($debug) {
    umask(0000);

    Debug::enable();
}

$trustedProxies = $_SERVER['TRUSTED_PROXIES'] ?? $_ENV['TRUSTED_PROXIES'] ?? false;
if ($trustedProxies) {
    Request::setTrustedProxies(explode(',', $trustedProxies), Request::HEADER_X_FORWARDED_FOR | Request::HEADER_X_FORWARDED_PORT | Request::HEADER_X_FORWARDED_PROTO | Request::HEADER_FORWARDED | Request::HEADER_X_FORWARDED_AWS_ELB);
}

$trustedHosts = $_SERVER['TRUSTED_HOSTS'] ?? $_ENV['TRUSTED_HOSTS'] ?? false;
if ($trustedHosts) {
    Request::setTrustedHosts(explode(',', $trustedHosts));
}

$request = Request::createFromGlobals();

$kernel = new HttpKernel($appEnv, $debug, $classLoader);
$result = $kernel->handle($request);

$result->getResponse()->send();

$kernel->terminate($result->getRequest(), $result->getResponse());

i just cant point my finger at it

digitalkaoz commented 2 years ago

oh my, its even more f*cked up than i thought...

i pointend the ALB Healthcheck to /admin which in turn renders a Twig file to HTML (this twig file will be cached), sadly the ELB Healtcheck doesnt send any FORWARDED Headers, so the initial cached page isnt trusted and so on.

When pointing my Healtcheck to another url, the /admin url will be accessed by a human first, which in turn generates the correct twig cached response.

i considered it closed. im not sure if the branch/middleware shouldnt be included anyways @Baldinof ?

Baldinof commented 2 years ago

That is indeed f*cked up 😂

I'll keep the PR open and will merge it soon, because it implement a feature supported by Symfony and not here for now.