dmikusa / cf-php-apache-buildpack

CloudFoundry PHP & Apache HTTPD Buildpack
Apache License 2.0
4 stars 11 forks source link

$_SERVER['HTTPS'] not set when php page accessed from HTTPS #6

Closed gberche-orange closed 10 years ago

gberche-orange commented 10 years ago

As described into http://fr2.php.net/manual/en/reserved.variables.server.php when a php script is handling an HTTPS request, the $_SERVER['HTTPS'] is expec ted to be set.

This is not currently the case as shown on the sample php-info running from the cf-php-apache-buildpack:

https://php-info.cfapps.io/info.php

When an app is running within the cf-php-apache-buildpack, the load balancer before the gorouter (AWS ELB for run.pivotal.io or HAProxy or Nginx such as in https://github.com/cloudfoundry-community/sslproxy-boshrelease) converts the received HTTPS traffic into HTTP and adds the x-forwarded-proto header and x-forwarded-for.

A possible workaround for php apps is to perform the test themselves:

if(isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && $_SERVER['HTTP_X_FORWARDED_PROTO'] == 'https'){
    $_SERVER['HTTPS']='on'; 
}

Some standard apps already perform such tests (e.g. https://drupal.org/node/313145 ), however others may break when accessed from https as they will detect that incoming traffic comes from HTTP, and may try to redirect to HTTPS urls, resulting into infinite redirection loops.

It would be great to have the cf-php-apache-buildpack automatically process the HTTP_X_FORWARDED_PROTO, in a similar way the java-buildpack does it with the RemoteIpValve cf https://github.com/cloudfoundry/java-buildpack/blob/master/resources/tomcat/conf/server.xml and http://tomcat.apache.org/tomcat-7.0-doc/api/org/apache/catalina/valves/RemoteIpValve.html

I searched for a similar apache module but only found so far:

If some php instructions could be included by the php interpreter prio to every HTTP request handling then the php snipnet above could be added.

dmikusa commented 10 years ago

This is an interesting problem. While I understand what you're saying with your comparison to the Java build pack, I feel like the circumstances are different between the two.

In Java, the Servlet spec dictates that "ServletRequest.isSecure()" can be used to determine if the channel is secure. Thus the RemoteIpValve needs to be set so that this method works as defined by the Servlet spec. As far as I understand, the same cannot be said about PHP and _SERVER['HTTPS']. In fact the documentation that you linked to specifically says "There is no guarantee that every web server will provide any of these; servers may omit some, or provide others not listed here." This leads me to believe that this is more of an application issue, in that applications need to be robust enough to work in environments that don't set these values.

Regardless, there are some easy workarounds. Here are the two that I like.

1.) Use the option auto_prepend_file in php.ini to added the code that you referenced. This will execute before every request.

http://www.php.net/manual/en/ini.core.php#ini.auto-prepend-file

To do this, simply include the code you mentioned in a PHP file. I put mine in "lib/https_fix.php" (note the "lib" folder is special to the build pack and is automatically added to the include_path). Then copy the php.ini file from the build pack to your application. Put it at "config/php.ini" and set auto_prepend_file as follows.

auto_prepend_file = https_fix.php

2.) Use a SetEnvIf directive to set HTTPS to on based on the x-forwarded-proto header.

To do this in your app, just copy these two files, httpd-php.conf and httpd-modules.conf, from the build pack to your application. Put them in the "config/httpd/extra" folder of your application.

In httpd-modules.conf, uncomment this line.

LoadModule setenvif_module modules/mod_setenvif.so

In httpd-php.conf, add this line.

#
# Set HTTPS environment variable if we came in over secure
#  channel.
SetEnvIf x-forwarded-proto https HTTPS=on

The same solution will work if you're using HTTPD 2.4, you'll just want to copy the HTTPD 2.4 config files from the build pack instead, httpd-php.conf and httpd-modules.conf.

As a note, these suggestions only address the issue mentioned here, which is setting _SERVER['HTTPS']. A similar issue exists for REMOTE_ADDR. That is more complicated though because X-FORWARDED-FOR is a list of IP addresses and needs to be handled with care so as to not introduce an opportunity for the client to set REMOTE_ADDR to an arbitrary value. If you could open a second issue for that, I'll put some more thoughts down there.

Thanks!

gberche-orange commented 10 years ago

The documentation at http://fr2.php.net/manual/en/reserved.variables.server.php states:

'HTTPS'
    Set to a non-empty value if the script was queried through the HTTPS protocol. 

I've submitted a documentation bug to have this refined to explicitly state requests received from SSL termination load balancer should have their $SERVER['HTTPS'] return true, see https://bugs.php.net/bug.php?id=66398

In the meantime, I believe that it won't harm anyone to have the buildpack set the HTTPS value automatically when X-FORWARDED-PROTO header is present. This would contribute to the "it just works" cloudfoundry experience, and provide more compatibility for legacy php apps.

Most recent web frameworks seem to now directly inspect $SERVER['X-FORWARDED-PROTO'] to detect HTTPS support, as a workaround for weak implementation of $SERVER['HTTPS'] e.g. http://symfony.com/doc/current/components/http_foundation/trusting_proxies.html

In any case, thanks a lot for the suggested workarounds for legacy php apps!

dmikusa commented 10 years ago

I'm going to recant here. You make a good point in that it should just work. I've added the SetEnvIf fix I listed above to the build pack.

gberche-orange commented 10 years ago

Thanks!

nueverest commented 7 years ago

@gberche-orange This may be a little off-topic, but I used the if-statement you referenced in functions.php on a wordpress site and now the frontend of the site works, but the wp-admin section is no longer accessible. Do you know why this is?

You can see the details here: http://wordpress.stackexchange.com/questions/250240/setting-serverhttps-on-prevents-access-to-wp-admin

gberche-orange commented 7 years ago

@nueverest sorry, I'm not so familiar with wordpress, your stackexchange topic looks promising though, sorry I can't help on this.