Proposal: Support for x-forwarded-proto

[mildsunrise]

Background

A lot of PHP apps (incorrectly) use HTTPS to determine the protocol that the browser made the request with. They then use it to construct the browser URL and determine when to redirect, etc.

Thus, when a reverse proxy is placed before the app, the URL isn't constructed correctly and you often get the dreaded "Too many redirects" error. Using reverse proxies is even more common in containers, so images like wordpress often patch the code so that the standard X-Forwarded-Proto header is obeyed and it behaves correctly behind a proxy:

			awk '
				/^\/\*.*stop editing.*\*\/$/ && c == 0 {
					c = 1
					system("cat")
					if (ENVIRON["WORDPRESS_CONFIG_EXTRA"]) {
						print "// WORDPRESS_CONFIG_EXTRA"
						print ENVIRON["WORDPRESS_CONFIG_EXTRA"] "\n"
					}
				}
				{ print }
			' wp-config-sample.php > wp-config.php <<'EOPHP'
// If we're behind a proxy server and using HTTPS, we need to alert WordPress of that fact
// see also http://codex.wordpress.org/Administration_Over_SSL#Using_a_Reverse_Proxy
if (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && $_SERVER['HTTP_X_FORWARDED_PROTO'] === 'https') {
	$_SERVER['HTTPS'] = 'on';
}
EOPHP

Solution

Ideally, every application should be modified to obey reverse proxies, but there appears to be a tendency in the PHP world to, umm, not do that (otherwise I don't understand why these patches are still needed for maintained apps like Wordpress and such).

Manually patching the PHP to set HTTPS (as shown above) is error prone, fragile and app-dependent. A better way is adding the following:

SetEnvIf x-forwarded-proto https HTTPS=on

to the Apache config. It achieves the same, but is much simpler, robust and application-independent. So, we could put it in /etc/apache2/conf-available/reverse-proxy.conf and then, a2enconf reverse-proxy.conf in the Dockerfile is all that's needed to make it work.

We could even enable it by default. It would be awesome, especially for new users, but some things should be taken into account first:

• Can it break existing images? Can it break apps that are proxy-aware? → Images/apps that are proxy-aware should obey X-Forwarded-Proto before HTTPS, so nothing should be broken.
• Does it pose a security risk? → I have to research this one further.
• Can it be unintuitive for users that write their own PHP with this image? → I think most users will expect the patched behaviour rather than the current one. If it's mentioned in the documentation, I don't see a big deal.

[tianon]

This is technically a duplicate of #369 -- I think the conversation there is very relevant.

This default $_SERVER['HTTPS'] variable is a documented feature of PHP, although I can't find where in the PHP source code it's actually set which is likely because PHP relies on the external server to send it (so IMO it seems odd for them to be documenting it).

On whether X-Forwarded-Proto poses a security risk, the answer is definitely that it depends on your deployment. See for example the RemoteIPTrustedProxy configuration parameter for Apache we had to use in order to use mod_remoteip safely:

https://github.com/docker-library/wordpress/blob/0df5de06a4f43f2790dfc3be92554a7e229115d9/apache-extras.template#L4-L15

I think where it poses a security risk is that it's essentially potentially untrusted input that a reverse proxy either needs to send with an appropriate value or scrub in order for an application to be able to trust the resulting value (which is more complicated when there isn't a reverse proxy, and the application is thus exposed directly to said "untrusted" input).

I imagine there have probably been discussions in either the PHP community or the Apache community (or both) predating this one where they've determined that the current state of affairs is the "best" they can do, but it's a bit of a hard thing to search for giving how generic the HTTPS search term is. 😅

[mildsunrise]

This is technically a duplicate of #369 -- I think the conversation there is very relevant.

Agree (why didn't that issue show up when I was searching for it? 🙁). Closing this and moving there.

This default $_SERVER['HTTPS'] variable is a documented feature of PHP, although I can't find where in the PHP source code it's actually set which is likely because PHP relies on the external server to send it (so IMO it seems odd for them to be documenting it).

Variables inside $_SERVER are not a PHP feature, they're defined by CGI and come straight from the webserver. PHP lists them, but only for orientative purposes, and makes it clear that variables may or may not be there. They shouldn't be set anywhere in PHP itself.

(Yes: while formally PHP isn't tied to CGI, there's still lots of legacy behaviour so the above isn't 100% true.) But still, PHP has nothing to do with this decision IMHO. This is either responsibility of the webserver (who could populate variables according to the proxy) or the application (who could handle the headers itself). PHP should limit itself to passing the variables from the webserver.

I imagine there have probably been discussions in either the PHP community or the Apache community (or both) predating this one where they've determined that the current state of affairs is the "best" they can do, but it's a bit of a hard thing to search for giving how generic the HTTPS search term is. sweat_smile

I think that ~95% of the time, PHP is deployed in a traditional hosting where there's no reverse proxy.
Docker is really the strange boy here, because in a containerized environment there's a lot more need for one.

On whether X-Forwarded-Proto poses a security risk, the answer is definitely that it depends on your deployment. See for example the RemoteIPTrustedProxy configuration parameter for Apache we had to use in order to use mod_remoteip safely

Of course, trusting X-Forwarded-For is a security issue. However in this case, if a client can forge the X-Forwarded-Proto, it can also issue the request through HTTPS. So it looks harmless, but I want to be sure.