roadrunner-server / roadrunner

🤯 High-performance PHP application server, process manager written in Go and powered with plugins
https://docs.roadrunner.dev
MIT License
7.86k stars 409 forks source link

[RR2, QUESTION] Does RoadRunner respect X-Forwarded-Proto and X-Forwarded-Port? #644

Closed denisvmedia closed 3 years ago

denisvmedia commented 3 years ago

I created this issue but maybe the problem is deeper. I used nginx-proxy which allows running apps locally in docker using https. And it works without any issue. Then I tried to use RoadRunner and the relevant Symfony bundle. The thing is that Spiral\RoadRunner\Http\Request::$uri contains the url with http scheme instead of https and as such Symfony generates links with http scheme instead of https, which leads to further problems.

Is there something I miss? How can I configure RoadRunner to treat the forwarded https connection, while serving http itself?

rustatian commented 3 years ago

Hey @denisvmedia . At the moment RR2 handles only X-Forwarded-For header. But, if I understand correctly, you need to specify redirect in the http config: https://github.com/spiral/roadrunner-binary/blob/master/.rr.yaml#L355

denisvmedia commented 3 years ago

Thank you for your answer @48d90782. Unfortunately, it doesn't solve my problem. redirect: true is placed under the ssl section, but roadrunner in my setup is behind a reverse proxy, and rr itself doesn't run ssl (and should not do that). Here is my setup:

image

Before I had a different setup (without rr) and it worked:

image

In general, I don't want RR to handle SSL for end-user itself (nginx will do it for me) and I don't want my internal communication to be done via https (i.e. proxy_pass should work via unencrypted plain http connection).

denisvmedia commented 3 years ago

To give you more details, nginx that acts as a reverse proxy sets the following headers:

proxy_set_header X-Real-IP $remote_addr;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Proto $proxy_x_forwarded_proto;
proxy_set_header X-Forwarded-Ssl $proxy_x_forwarded_ssl;
proxy_set_header X-Forwarded-Port $proxy_x_forwarded_port;

Where the proxy_set_header X-Forwarded-Proto will have a value https and X-Forwarded-Port will have a value 443. So, nginx gives this information to RR, but the latter seems to be not using it.

rustatian commented 3 years ago

Thanks for the great explanation @denisvmedia . If you don't mind, I'm not familiar with the PHP, could you please create a gist or test repo with the app.php, your Nginx config, and few lines of instructions on how to run all these parts together? It would be very helpful to me to renew the RR2 HTTP module.

denisvmedia commented 3 years ago

@48d90782 here is an example: https://github.com/denisvmedia/rr-problem-demo

Please note, although the project demonstrates the problem using local https certificates, the same applies to production, where we run using a similar configuration (HTTPS:443 nginx reverse proxy that proxies HTTP:80 connections to the internal infrastructure).

denisvmedia commented 3 years ago

For me, the ideal solution would be RR to respect the X-Forwarded-Proto and X-Forwarded-Port headers. In this case the bundle would have received https://rr-problem.home.test instead of http://rr-problem.home.test (i.e. https instead of http, and potentially proper port if a custom port is used). But of course, if for some reason RR will not be able to do that, then RR Symfony Bundle would need to have a feature to re-define the proto and scheme (but this would be hacky).

cc @Baldinof

Baldinof commented 3 years ago

I think there is no need to investigate here, the problem seems to be on the symfony bundle integration side, the roadrunner worker receive the headers:

image
denisvmedia commented 3 years ago

Maybe you are right. However, I still have a question about your screenshot: is it expected to have uri with the http scheme instead of https?

rustatian commented 3 years ago

@Baldinof Thanks, would be great if you can help here 👍🏻

denisvmedia commented 3 years ago

@48d90782 I'm still not sure that this is right:

image

And this comes from RR. So, RR is able to detect the domain (via Host header), but not able to detect the protocol and port.

rustatian commented 3 years ago

@denisvmedia Sure, as I said above, RR does not handle X...Proto and X...Port headers. Let's wait for the @Baldinof answer to your question and then move forward 😃

Baldinof commented 3 years ago

We see http:// in the request received by RR because it's actually receiving HTTP and doing its job well: sending all headers to the app.

The app then, can use X-Forwarded headers to know the client is communicating via HTTPS with the frontend proxy (nginx). The app should trust those headers only if you know that the request is really coming from the proxy (the trusted_proxies configuration in Symfony).

IMO I don't think RR should handle X-Forwarded headers. Trusting proxies is well handled in major PHP frameworks, and I expect to receive in PHP the same request RR received.

denisvmedia commented 3 years ago

Well, then why does RR already handle X-Forwarded-For?

rustatian commented 3 years ago

I quite agree with @Baldinof . RR handles only a few cases, such as trusted IPs, can set some headers, but mostly just passes the payload to the underlying PHP worker. X-Forwarded-For used in the internal RR transport layer to set response address.

denisvmedia commented 3 years ago

Ok, I still think it's kind a inconsistent in RR. But 2 vs. 1, you win guys :D Thanks for your efforts and help.

rustatian commented 3 years ago

It's ok @denisvmedia. If you think, that we can make RR better, you can create an RFC and describe your approach, and discuss with more people involved.

denisvmedia commented 3 years ago

@48d90782 sure. I will think of it better, and if I don't change my mind in the end, I will come with an RFC proposal :)

rustatian commented 3 years ago

Great :) Thanks @Baldinof , @denisvmedia 👍

digitalkaoz commented 2 years ago

@denisvmedia we also ran into this problem but its a little different:

we skipped NGINX alltogether for now.

We try to run roadrunner in Docker in AWS ECS and behind an ALB.

The Problem is that roadrunner thinks its running on http://172.12.2.2:8000 (some internal ECS Service IP) but in reality its running on https://xyz.com (which somehow doesnt is respected by roadrunner). So basically not respecting the FORWARDED header sent by AWS ALB (with everything in it), see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Forwarded

Is there any workaround or solution to this?

In our Symfony Application we had to do this, to properly use this header:

$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);
}
wolfy-j commented 2 years ago

I think we have a config option to specify trusted networks for ip forwarding.

rustatian commented 2 years ago

Hey @digitalkaoz. You need to open the bug or feature request with the information about the needed behavior. The best will be to point to some RFC describing how to handle that.

rustatian commented 2 years ago

Because RR at the moment has some logic to handle X-Forwarded-For headers, it uses the first IP address specified in the X-Forwarded-For header.

rustatian commented 2 years ago

I think we have a config option to specify trusted networks for ip forwarding.

Yeah, we have such logic, but the trusted networks check applied before the X-Forwarded-For header parse.

rustatian commented 2 years ago

@digitalkaoz I guess I understand your problem. RR parses the X-Forwarded-For header, which is kinda old, but not parsed the new one - Forwarded specified by this RFC. I'm also guessing, that AWS ALB doesn't send the XFF header and only sends the new one, am I right?

digitalkaoz commented 2 years ago

@rustatian Yap, AWS ALB only sends the new one

rustatian commented 2 years ago

@digitalkaoz got u, would you mind creating an issue with the description of the problem?