IS_SSL should check return true for SSL Terminated load balancing

[bretterer]

With cloud load balancing and load balancing in general, the option to do ssl termination at the load balancer and sending all traffic between the LB and nodes over plain http will cause issues with some WordPress Installs and Plugins. When you rely on is_ssl() to determine if you are secure so you can forward to secure site if you are not, it does not work on ssl terminated load balancing. The suggestion to do this in a server side configuration is not always an option but may still need to be used. My suggestion is we add a single check in the is_ssl function of checking the de facto standard for identifying the originating protocol of an HTTP request.

By relying on the HTTP_X_FORWARDED_PROTO for this case is very standard on the internet today to test if your application should respond like it was coming from an https request.

References for the standard
​http://tools.ietf.org/html/rfc7239
​http://docs.rackspace.com/loadbalancers/api/v1.0/clb-devguide/content/SSLTermination-d1e2479.html
​http://en.wikipedia.org/wiki/List_of_HTTP_header_fields#cite_ref-15

[chaoix]

I agree with this change. The X-Forwarded-For header is the de facto standard for load balancers running Linux or Windows at this point and should be supported in core.

There is always going to be edge cases for these types of problems, but this fix should apply to most people experiencing ssl termination issues.

[jond]

I don't know the status of RFC 7239 beyond the mentioned Proposed Standard as a Standards Track document waiting for approval by the IETF. This is much more progress than there was when the other tickets were decided wontfix (#15733, #20567).

Perhaps it is time to look at this again now that a standard is emerging?

[ocean90]

Duplicate of #15733.

[chaoix]

This issue needs to be looked at again since it was two years ago. That other ticket should serve as a historical reference, but since the server headers are more standardized now, we really should discuss this change again.

[dd32]

Part of the problem with relying upon headers from untrusted downstreams (which is effectively what you're suggesting) is that it's then possible to bypass any SSL-only settings done in WordPress.
A MITM proxy could force it to HTTP, add the header, allow someone to login over HTTP (because is_ssl() is now true) on this SSL-only site, and then, well you get the picture..

The hard part is validating that the header is actually coming from a trusted source, which is why we suggest that this is a server configuration error.
Of course, it's becoming far more common for a LB/HTTPS proxy to be sitting in front of sites, the good news is that most of these commercial SSL proxies will rewrite the page on-the-fly to convert any HTTP resources to HTTPS.
Many private LB's which terminate SSL on the edge however, do not rewrite these things, and this is the exact scenario where some server configuration is needed to make the WordPress environment match your actual environment (see #19337 for some examples of code).

WordPress could of course add a No SSL / Could be SSL / Definitely SSL to the mix, but that then introduces complexity, as suddenly some SSL things work, but others which require SSL to be enabled don't, and it's a mystery to the admin as to why.

If there's a way to work around everything mentioned here, and being straight forward to use by all users, we'd have done it already.. but I'm all ears on how you think we can achieve it.

[bretterer]

Replying to dd32:

Part of the problem with relying upon headers from untrusted downstreams (which is effectively what you're suggesting) is that it's then possible to bypass any SSL-only settings done in WordPress.
A MITM proxy could force it to HTTP, add the header, allow someone to login over HTTP (because is_ssl() is now true) on this SSL-only site, and then, well you get the picture..

Isn't WordPress currently relying on headers anyway for the is_ssl() method. I could just as easily add a header to my request to set HTTPS to be true and bam, I have access to the site over HTTP because is_ssl() is now true. So this argument in my opinion is not a valid one. WordPress is still relying on headers in the current setup. I am just suggesting that we add a different header to look at.

[dd32]

Isn't WordPress currently relying on headers anyway for the is_ssl() method.

WordPress is currently using server-provided environmental headers - ie. Apache or nginx are saying "This is a HTTPS request", it doesn't trust anything coming from the client.
$_SERVER['HTTPS'] is server-provided, if a client sent a header of HTTPS: on, it'd come into PHP as $_SERVER['HTTP_HTTPS'].

[chaoix]

What about in addition to checking the X-Forwarded-Proto header we also check Remote-Addr, a server set header against a filtered array of whitelisted load balancer IP addresses since the issue here isn't whether to use the X-Forwarded-Proto header but verifying the identity of the server sending it and the ability to not have load balancer heading checks enabled by default.

I am not convinced header manipulation is a real concern though for this use case. Using the load balancer use case, the only traffic sent to the web server is through a VPN connection between the load balancers and the web servers. It is not possible for the web server to be access via port 80 directly.

[dd32]

Replying to chaoix:

What about in addition to checking the X-Forwarded-Proto header we also check Remote-Addr
...
It is not possible for the web server to be access via port 80 directly.

In some configurations, it's possible for the server to be accessed directly which is a valid angle, for example, a server using a cloudflare front-end proxy.

The ​cloudflare plugin for example correctly updates the REMOTE_ADDR field if it's coming from the CF ip ranges.

As has been rehashed in the many threads prior (and i'm doing again), the method to define it programmatically in a way that's useful for everyone, is almost certainly longer and more complex than simply fixing it according to the current environment, for example, to define it programmatically, it'd need something similar to this (with associated logic to handle all the wonderful edge cases)

define( 'HTTP_TRUSTED_HOSTS', '10.0.0.1' );
define( 'HTTP_TRUSTED_HOST_PROTOCOL_HEADERS', 'HTTP_HTTPS' ); // on | off
define( 'HTTP_TRUSTED_HOST_REMOTE_ADDR', 'HTTP_X_ORIGINATING_IP' ); // 1.2.3.4
or 
define( 'HTTP_TRUSTED_HOSTS', '10.0.0.5,10.0.0.6' );
define( 'HTTP_TRUSTED_HOST_PROTOCOL_HEADERS', 'HTTP_X_FORWARDED_PROTO' ); // http | https
define( 'HTTP_TRUSTED_HOST_REMOTE_ADDR', 'X_FORWARDED_FOR' ); // "1.2.3.4, 4.3.2.1, 10.0.0.5" (client, proxy, lb)

In contrast, for a server that isn't publicly accessible, these are the simple lines which you use in either wp-config.php or in nginx (to fix it reliable for every PHP application out there, WordPress included) (varied according to the environment of course, and you'd also have the downstream proxy setup to discard/overwrite conflicting headers)

$_SERVER['HTTPS'] = !empty( $_SERVER['HTTP_HTTPS'] ) ? $_SERVER['HTTP_HTTPS'] : 'off';
$_SERVER['REMOTE_ADDR'] = $_SERVER['HTTP_X_ORIGINATING_IP'];

or in nginx (which also provides modules to do this automatically when you're setting up a proxied environment - ngx_http_realip_module)

map $http_x_forwarded_proto $https {
    default off;
    https on;
}
fastcgi_param  HTTPS        $https;
fastcgi_param  REMOTE_ADDR  $http_x_forwarded_for;

[chaoix]

Given all that has been brought up in the ticket, I think the best solution that doesn't explicitly open up security concerns is adding a filter to the output of is_ssl so developers can handle the complex logic for their hosting environment in plugins. This would also help in situations where changing the proxy configuration is not an option.

[dd32]

I think the best solution that doesn't explicitly open up security concerns is adding a filter to the output of is_ssl so developers can handle the complex logic for their hosting environment in plugins

The thing is, they already can, all they need to do is have their plugin set $_SERVER['HTTPS'] = 1 as appropriate, no filtering needed. I realistically can't see any benefit to using a filter, and only possible downsides, the same conclusion we came to in the past: https://core.trac.wordpress.org/ticket/19654#comment:5

[ocean90]

#31439 was marked as a duplicate.

[dd32]

#32354 was marked as a duplicate.

[johnbillion]

I'm going to close this again, with particular reference to comment 11 and comment 13.

Trusting client-provided headers, even if they are de-facto standard for indicating a reverse proxy, is out of the question, and a filter on the return value of is_ssl() provides no benefit when plugins can already directly affect $_SERVER['HTTPS'] (and $_SERVER['REMOTE_ADDR'] as necessary).

At this point, we're just re-hashing previous discussion from #19654, #19337, #15733, and #20567.

[johnbillion]

#33730 was marked as a duplicate.

[webaware]

I second johnbillion above, this is not something you want to handle in core. It is too specific to the hosting environment, and it's too easy to forge headers.

FWIW, it's not as simple as supporting HTTP_X_FORWARDED_PROTO either, as different hosts present different variables to signify HTTPS, e.g.

• HTTP_X_FORWARDED_PROTO as 'https'
• HTTP_X_FORWARDED_SSL as 'on'
• HTTP_X_FORWARDED_SSL as '1'
• HTTP_CF_VISITOR as '{"scheme":"https"}' (CloudFlare Flexible SSL)

NB: I believe that CloudFlare Flexible SSL now also sets HTTP_X_FORWARDED_PROTO, which it transmits across the unsecured Internet over HTTP, thus demonstrating how easy it is to forge such headers.

There are existing plugins that handle these specific configurations already (e.g. ​https://wordpress.org/plugins/cloudflare-flexible-ssl/, my ​https://wordpress.org/plugins/ssl-insecure-content-fixer/) and it's very easy to create a site-specific fix either directly in wp-config.php or as a small plugin. Trying to handle it in core can, I believe, only complicate matters.

[ocean90]

#34298 was marked as a duplicate.

[johnbillion]

#34783 was marked as a duplicate.

[johnbillion]

#28443 was marked as a duplicate.

[dd32]

#28658 was marked as a duplicate.

[ocean90]

#39659 was marked as a duplicate.

[ocean90]

#40710 was marked as a duplicate.

[SergeyBiryukov]

#47288 was marked as a duplicate.

[SergeyBiryukov]

#49970 was marked as a duplicate.

[SergeyBiryukov]

#57125 was marked as a duplicate.

[joemcgill]

#61463 was marked as a duplicate.

[swissspidy]

#61915 was marked as a duplicate.