Support HTTPS environment variable via x-forwarded-proto header

[stt]

When generating fully-qualified links and redirect URLs, php apps typically use server environment variable "HTTPS" to check whether to redirect to https or http. With apache this variable is set by mod_ssl (http://httpd.apache.org/docs/current/mod/mod_ssl.html#envvars).

If on other hand, SSL termination happens before the http request reaches the server where php is running then this information is lost, unless it's carried over, typically by x-forwarded-* headers.

With this change if request is e.g. forwarded to the docker container via mod_proxy in another apache:

ProxyPass / http://localhost4:9080/
RequestHeader set X-Forwarded-Proto "https"

Then dockerized php sees that $_SERVER[HTTPS]='on'.

In my usecase this was required for dockerized Drupal 8 to stop redirecting the user to http which wasn't even open in the firewall. (Typically proposed fix for this on SO and drupal forums is to add redirects to .htaccess which 1) wouldn't work unless port 80 was open, 2) is a potential security issue due to requests being made in cleartext, 3) POST request breaks if it receives a redirect response, 4) it's unnecessary and ugly.)

[stt]

Looks like there's an issue with test 338.6, it fails with:

gpg: requesting key 31CBD89E from hkp server ha.pool.sks-keyservers.net
gpgkeys: key 528995BFEDFBA7191D46839EF9BA0ADA31CBD89E not found on keyserver
gpg: no valid OpenPGP data found.
gpg: Total number processed: 0
....
The command "docker build -t "$image" "${VARIANT:-.}"" exited with 2.
$ ~/official-images/test/run.sh "$image"
testing php:7.1-zts 
    image does not exist!

[tianon]

Hmm, I'm a bit on the fence with this one -- on the one hand, it'd make PHP applications transparently able to detect this sort of thing, but on the other hand, it'd be a further divergence between the Apache and FPM variants and it should be fairly trivial to add this to a local .htaccess file, right?

I think the "correct" solution would be for PHP applications to also check $_SERVER['HTTP_X_FORWARDED_PROTO'], but that's asking a lot too. 😞

Are there any official recommendations regarding this sort of thing from PHP upstream?

[stt]

.htaccess would require exposed docroot and many apps, like drupal, already have stuff in .htaccess that they need so overwriting it with a custom one introduces maintenance challenges.

You are correct that it is more of an application issue but given how long this issue has persisted, checking both $_SERVER['HTTPS'] for locally terminated SSL and $_SERVER['HTTP_X_FORWARDED_PROTO'] for remotely terminated SSL every time full URL needs to be generated, a secure cookie flag set etc, is indeed not too likely to catch on. :)

There is dmikusa/cf-php-apache-buildpack#6 about this same thing and they submitted https://bugs.php.net/bug.php?id=66398 that through documentation would've suggested developers to handle the header consistently but the issue is still open after 3y.

For the time being my concern is that most people after they do docker pull php-app and proxy their SSL terminated traffic to the exposed port expecting it to just work. When it doesn't, they then go to google and find these redirect hacks that may superficially solve few things but in long run only add to the collective headaches of users, developers, admins and customers alike.

[stt]

I'm not really familiar with FPM, but as I understand fastcgi is a binary protocol that browsers don't speak so it requires an httpd to act as a middleman. In a setup where webapp is served by php:fpm container, is it typically accessed via a proxy to the container's port 9000?

It seems that at least with mod_proxy_fcgi requests also inherit environment variables from httpd, at least I could pass HTTPS flag from apache to the dockerized php like this:

<FilesMatch \.php$>
  SetEnv HTTPS on
  SetHandler "proxy:fcgi://127.0.0.1:9000"
</FilesMatch>

If I understood correctly then no changes to the docker container configuration is required to support HTTPS envvar with FPM?

Though it would be nice to provide documentation in docker hub readme preferably, so that people know to set that envvar in FPM's case, or otherwise pass a X-Forwarded-Proto header, unless they are using 3rd party SSL termination that already provides it (AWS ELB for example).

[tianon]

Sorry for the long delay.

I think http://codex.wordpress.org/Administration_Over_SSL#Using_a_Reverse_Proxy is a good example from a relatively large and popular PHP application, whose documentation suggests that manually checking the value of $_SERVER['HTTP_X_FORWARDED_PROTO'] is indeed the appropriate solution here, and barring PHP upstream updating their logic for how they set that HTTPS variable to automatically take that into account, I'm of the opinion that we shouldn't do anything extra to either facilitate or get in the way of users who want this behavior (because I think it is conceivable for users to want to be able to differentiate from PHP/Apache itself terminating SSL vs a load balancer doing so, which is likely why the WordPress devs haven't added that if statement directly in the WordPress core).

I'm going to close, but I think this discussion will serve as a good reference point for future travelers! Thanks! ❤️

[sathieu]

Please reopen this one.
Two reasons:

• This directive affects Apache too. For example when apache redirects /directory to /directory/
• This will make apache2 flavor behave like the fpm flavor (i.e. using previous hop infos)

[mildsunrise]

I think http://codex.wordpress.org/Administration_Over_SSL#Using_a_Reverse_Proxy is a good example from a relatively large and popular PHP application, whose documentation suggests that manually checking the value of $_SERVER['HTTP_X_FORWARDED_PROTO'] is indeed the appropriate solution here

I wouldn't trust Wordpress on this. The code snippet they suggest right now is incorrect, as it trusts values that may come from the client.

In the end, the snippet fakes HTTPS to make the app work. They probably don't want to suggest server-specific solutions, but IMO an Apache directive is preferable to patching PHP or .htaccess.

and barring PHP upstream updating their logic for how they set that HTTPS variable to automatically take that into account, I'm of the opinion that we shouldn't do anything extra to either facilitate or get in the way of users who want this behavior

As said in #943, I don't think PHP should ever add this kind of logic.

Also, I'm pretty sure most of the users would prefer a patched behaviour. If you're an advanced user who needs the original variables, mentioning a2disconf reverse-proxy.conf in the docs should be fine IMHO.

(because I think it is conceivable for users to want to be able to differentiate from PHP/Apache itself terminating SSL vs a load balancer doing so

I have yet to see an example of this...

which is likely why the WordPress devs haven't added that if statement directly in the WordPress core).

I don't think that's an excuse, they could have put it behind a config option like Drupal 😞

it'd be a further divergence between the Apache and FPM variants

Not really, FPM does not include a webserver... Configuring a webserver is their task, and if they want the same behaviour as php:apache they should make sure to do it correctly.

[nyov]

Not sure why I comment on this, but heh. :)

Don't trust HTTP_* headers in $SERVER, unless you sanitize them! They are insecure as they can be injected by a client. Makes no difference whether they're checked by PHP or Apache's SetEnvIf.

I agree it's a lot better to set this here (by the admin), than if braindead application does check XPP itself, but enabling it by default may be fatal on the unsuspecting admin. Because even if their app is secure by default and only trusts $SERVER['HTTPS'] (which can't be faked, sending a HTTPS header would end up as HTTP_HTTPS), you just made it potentially insecure by elevating an untrusted X-Forwarded-Proto header into the trusted server ENV.
So in fact it would probably be safer to unset XPP by default, because of said braindead applications, who check X-Forwarded-Proto with no idea about their runtime environment. (But people probably won't appreciate the added safety when it stops their sites from working.)

See, if there is no reverse proxy (or it doesn't disallow port 80), I only have to send X-Forwarded-Proto: https with a client to port 80, to set $_SERVER['HTTP_X_FORWARDED_PROTO'] and fake a secure connection for such an app this way.
And this SetEnvIf-line would elevate it into HTTPS and even fool an application that wouldn't usually be vulnerable and isn't prepared for this. Which isn't a problem, if the admin is aware of this, and secures their config against this, like redirect to HTTPS always. But otherwise?

Let's assume this completely made up scenario for a second:

An application is accessible via both Port 80, and through TLS on 443 as mysite.com.
It's using a configuration like this:

RewriteCond %{THE_REQUEST} ^[A-Z]{3,9}\ /(.*)\ HTTP/ [NC]
RewriteCond %{HTTPS} !=on [NC]
RewriteRule ^/?(wp-admin/|wp-login\.php) https://mysite.com%{REQUEST_URI}%{QUERY_STRING} [R=301,QSA,L]

So it allows logins only over HTTPS (if server env HTTPS=on), to safeguard the user from credential theft. But because of said SetEnvIf X-Forwarded-Proto "https" HTTPS=on, $badguy only has to send X-Forwarded-Proto: https with the request, to disable this redirect and also have the application give out nicely formatted https:// URLs in the page output.

So how is this bad? All they achieved is getting a broken page and possibly some https-only cookies, right?
Yes.

Don't bother with the rest here, it got trashed. ;)

Well, yes, unless $badguy is really keen on getting your site login. Then they could for example register the domain mys1te.com, even grab a valid lets-encrypt certificate for it, and run their own reverse-proxy that serves mysite.com:80 with X-Forwarded-Proto: https on their own port 443. And because of the nice https://-links, there aren't even any mixed-content problems ;)

Then they add a little regex that defaces the front-page when loaded, and send you this email: "Dear $siteadmin, I just noticed your website on <a href="https://mys1te.com">mysite.com</a> is broken, did you get hacked?"

So of course you click the link, and indeed, that is your site, with the valid LE cert, and the page is broken! All flustered, you log in to /admin.php to check what's going on and .... yeah, your insecure login just got phished.

But that's just wordpress, and a dumb example, so who cares.
However, you probably don't want to expose some hundred forum users to such phishing, because unsuspecting server admin was unaware of a little config setting ("I don't know what happened! I ran this container for 4 years, it worked well all this time! All I did was upgrade!").

Again, I agree it makes sense to have the option here, if the user is made aware of it and has the choice to disable or enable it.
But something like SetEnvIf Remote_Addr "HTTPS_PROXY_IP_ADDRESS" HTTPS=on might arguably be safer. And btw, SetEnvIf also requires a2enmod setenvif, so the PR may be incomplete.

[mildsunrise]

Don't trust HTTP_* headers in $SERVER, unless you sanitize them! They are insecure as they can be injected by a client

That is correct in theory, but not in practice (see first point below)

I agree it's a lot better to set this here (by the admin), than if braindead application does check XPP itself, but enabling it by default may be fatal on the unsuspecting admin. Because even if their app is secure by default and only trusts $SERVER['HTTPS'] (which can't be faked, sending a HTTPS header would end up as HTTP_HTTPS), you just made it potentially insecure by elevating an untrusted X-Forwarded-Proto header into the trusted server ENV.

This attack only works if there's no proxy (or a misconfigured proxy that doesn't send the header). But then, the app will not work correctly because it will perceive all requests as http. This will force the user to setup the proxy (which makes this attack impossible, since the client no longer controls the header).

Second: This requires the attacker to have the capability to add arbitrary headers to the request. If the attacker has this privilege, why wouldn't they be able to send an https request in the first place?

Third: why would the server need to validate the protocol for security purposes? protocol validation is meaningful to the client (which needs to authenticate itself to the server)

Given all this, it's really hard for me to come up with an example where this would open a vulnerability; even the official snippet from Wordpress allows the client to forge $SERVER['HTTPS'].

So in the end, I think it would be okay to enable this by default & add a warning to the documentation explaining what it does, and how to disable it.

Well, yes, unless $badguy is really keen on getting your site login. Then they could for example register the domain mys1te.com, even grab a valid lets-encrypt certificate for it, and run their own reverse-proxy that serves mysite.com:80 with X-Forwarded-Proto: https on their own port 443. And because of the nice https://-links, there aren't even any mixed-content problems ;)

This doesn't make sense 😞 The attacker could just serve mysite.com:443, it would work the same...

[nyov]

Best things first ;)

This doesn't make sense 😞 The attacker could just serve mysite.com:443, it would work the same...

Hahaha, dang, girl! Dismantled that completely there! ;) You are quite correct, on reflection I can just re-wrap the TLS connection for the same results.
Here I thought I had a reasonably safe use-case to present and then catgirl came and hacked it. Meow. 🤟
I'll edit my post to hide that bogus argument, so not to confuse anyone else with that.

This attack only works if there's no proxy (or a misconfigured proxy that doesn't send the header).

Yes, exactly so. Or possibly, if it doesn't strip the header from a client.

But then, the app will not work correctly because it will perceive all requests as http.

This I do not get. Do you talk about a CGI environment here, as opposed to http requests? Or a PROXY protocol connection to Apache?
In this given case CGI will be the Apache<->FPM talking to each other. But Apache sets the SetEnvIf here, and it talks plain http on port 80 (excepting perhaps fancy mod_proxy magic).
And I don't see any mention that this docker image requires another proxy in front, so assume we have no proxy? Or the user might just run a TLS terminator like Pound or Hitch on port 443 and nothing in front of Port 80. Or they might run a reverse proxy in front of apache, but not use PROXY protocol. In any of these events we talk plain HTTP to apache.

Given all this, it's really hard for me to come up with an example where this would open a vulnerability

I'll say. It seems I have trouble myself ;) Given this one is a boolean setting, it's relatively safe from direct exploiting.

But imagine, if you will, an application that interprets some X_SSL_* headers from a TLS proxy about the client connection, only when it believes to be safe behind such a proxy; then an attacker
who knows this and can pretend to be that proxy (as explained) might have a string injection vector into the application that they would not have had otherwise.

And if you won't, I'll rest my case. But I'll stand by my point that elevating insecure input to a usually trusted environment is not a good idea, even if it looks safe enough at a glance.
PHP has had enough examples of this in it's colorful past. This should be at the admin's discretion, on a case-by-case basis.

even the official snippet from Wordpress allows the client to forge $SERVER['HTTPS'].

And it doesn't seem to be a default enabled setting there either, though they at least know their application and how that setting will affect it. Here on the other hand, we're packaging only an interpreter (environment), knowing neither what applications will run in it, nor in which environment the container will be run. Then making predictions on both counts might not be the best idea.

So in the end, I think it would be okay to enable this by default & add a warning to the documentation explaining what it does, and how to disable it.

I might just have to vote for you there, seeing how you took down my argument. But no, I'll leave that up to y'all as I have no beef with that either way.
From a sysadmin standpoint, I'd say a prominent info would be prudent, as unexpected change tends to be not well received.

[mildsunrise]

Or possibly, if it doesn't strip the header from a client

No; the attack is not possible even if the proxy doesn't strip the header from the client (and just appends its own). We are checking for an exact https value.

And it doesn't seem to be a default enabled setting there either

Yes, but anyone who follows this advice (including the official wordpress container, IIRC) is vulnerable.

But imagine, if you will, an application that interprets some X_SSL_* headers from a TLS proxy about the client connection, only when it believes to be safe behind such a proxy; then an attacker

For obvious reasons, an application won't do that until it confirms that a reverse proxy has been setup correctly. Once this is done, the attack is not possible...

From a sysadmin standpoint, I'd say a prominent info would be prudent, as unexpected change tends to be not well received

True; we can first add the option (disabled by default) and mention in the documentation that it might be enabled by default someday. @tianon would that be accepted? if I'm not missing anything, it's just a file at /etc/apache2/conf-available

[mildsunrise]

Update: the wordpress image is not vulnerable, they're using an older version of the snippet