The attached patch adds X-Forwarded-Proto and Front-End-Https support to Drupal.

Drupal detects whether it's using HTTPS by looking at $_SERVER['HTTPS']. This variable is of course not set to "YES" if Drupal runs in HTTP. If your Drupal site is behind an reverse proxy or load balancer that serves HTTPS to clients but communicates with the web server via HTTP (i.e. Internet --HTTPS--> ReverseProxy --HTTP--> Apache), Drupal infers an incorrect $base_root.

I've found two conventions for passing protocol information back to the web server from the load balancer. The more common strategy is to use an X-Forwarded-Proto (XFP) header as part of the request sent to the back-end web server, set to the protocol (HTTP or HTTPS). Pound (http://www.apsis.ch/pound) includes this natively, and the practice is growing to add this header line to Apache servers running mod_proxy_http so that back-end applications know what protocol to use.

The less common method is a Front-End-Https header used apparently only by "Microsoft Internet Security and Acceleration Server", and is documented generally in conjunction with getting Outlook Web Access to function properly (presumably since it also requires AJAX callbacks to have a correct URL).

Benefit:

Without a correct $base_root, various things break -- most visibly, AJAX including and the Drupal installer, because callback URLs are built on an incorrect $base_url.

Cost:

The patch introduces one to three extra IF conditions to be evaluated for every HTTP request, unless $base_url has been overridden in settings.php. For the largest portion of users, who are not using a load balancer, the patch introduces only two conditions to be evaluated, one for each header variable.

Note:

It is possible to set $base_root manually in settings.php. But using this patch will allow Drupal to work in this sort of environment as it is already documented, without special care being taken to create settings.php ahead of time.

Also, I should point out that neither of the headers this patch detects are standard parts of HTTP. For details on Front-End-Https, see MS KB document Q307347 for details.

It's possible that protocol detection can be made in other ways as well. If you know of one (because you use a load balancer that does this differently), please comment!

Files: 
CommentFileSizeAuthor
#75 313145-75.patch914 bytesneclimdul
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]
#38 x-forward-proto-313145-38.patch738 bytesghoti
PASSED: [[SimpleTest]]: [MySQL] 23,288 pass(es).
[ View ]
#30 x-forward-proto-313145-30.patch1.2 KBpwolanin
Passed: 14713 passes, 0 fails, 0 exceptions
[ View ]
#24 bootstrap.inc_.patch2.07 KBseanr
Failed: 12845 passes, 2 fails, 0 exceptions
[ View ]
#21 bootstrap.inc_313145.patch1.47 KBghoti
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bootstrap.inc_313145_1.patch.
[ View ]
#20 bootstrap.inc_313145.patch1.55 KBghoti
Failed: Failed to apply patch.
[ View ]
#11 bootstrap.inc_313145.patch1.72 KBghoti
Failed: 11815 passes, 18 fails, 1 exception
[ View ]
#7 bootstrap.inc_proxy_3.patch1.77 KBghoti
Failed: 7199 passes, 4 fails, 0 exceptions
[ View ]
#6 bootstrap.inc_proxy_3.patch1.52 KBghoti
Failed: Failed to apply patch.
[ View ]
#4 bootstrap.inc_proxy_2.patch1.45 KBghoti
Failed: Failed to install HEAD.
[ View ]
bootstrap.inc_.patch1.09 KBghoti
Failed: Failed to install HEAD.
[ View ]

Comments

The patch looks good to me but I can't test it.

Status:Needs review» Needs work

<?php
+    elseif (isset($_SERVER['HTTP_X_FORWARDED_PROTO'])) {
+     
// We're behind a proxy that talks to the web server via HTTP.
+      $base_root = $_SERVER['HTTP_X_FORWARDED_PROTO'];
+    }
?>

X-Forwarded-Proto could potentially be set by a user, check if it is really "http" or "https" and not something malicious.

Also, we have a "reverse_proxy" setting. It would be logical to only attempt protocol detection when the admin enabled reverse proxy support.

Good point about the security issue; I'll think about that. Would it be too costly to compare against a regexp, or would it be better merely to use a few IFs comparing simple strings? I'm highly cognizant of the fact that this is in bootstrap.inc and will be executed for every hit.

As for having the admin enable something ... how does that help the person installing for the first time? There is no admin option until that has completed.

How could trusting X-Forwarded-Proto to reflect proxy requirement represent a security risk? I'm trying to imagine a way, and I can't come up with a mechanism.

Status:Needs work» Needs review
StatusFileSize
new1.45 KB
Failed: Failed to install HEAD.
[ View ]

Patch updated to foil malicious header padding and respect $conf['reverse_proxy'] if it is set. This should allow the header to be respected during install.

I haven't actually tested this yet....

You should use variable_get() rather than accessing $conf directly.

If you check for reverse_proxy with X-Forwarded-Proto, you should check it for Front-End-Https as well. I cannot see a security risk in trusting X-Forwarded-Proto either, but when it comes to security-related stuff it is generally better to be safe than sorry. Checking reverse_proxy takes about 0.00 seconds and IMO even makes the code slightly easier to read.

What if the user talks HTTP with the reverse proxy, but the reverse proxy talks HTTPS with the webserver? AFAICT this isn't supported by this patch - is that in any way relevant?

StatusFileSize
new1.52 KB
Failed: Failed to apply patch.
[ View ]
You should use variable_get() rather than accessing $conf directly.

I'd like to, but variable_get() requires a default, and can't be used to see *whether* a variable has been set. Except perhaps through inelegant use of $default, which I'd like to avoid. Note that the changes this patch applies are to function conf_init(), which is the thing that actually *reads* settings.php and sets the $conf global in the first place. So we're guaranteed to have $conf close-by. :)

you should check it for Front-End-Https as well.

You're absolutely right. Thanks! I've updated the patch.

What if the user talks HTTP with the reverse proxy, but the reverse proxy talks HTTPS with the webserver?

I don't think this is an issue. I can't see why you'd set that up in real life, and I know it's not even possible using Pound or Varnish. Pound does not support back-end HTTPS, and Varnish has no HTTPS support anywhere (one of the ways Varnish docs suggest for adding HTTPS support is to put it behind Pound).

StatusFileSize
new1.77 KB
Failed: 7199 passes, 4 fails, 0 exceptions
[ View ]

Meh, wrong diff format. :)

Status:Needs review» Needs work

The last submitted patch failed testing.

I'll be able to start testing this and am very interested in it. Won't be right away (awaiting PHP 5.2 upgrade in progress), though, so just subscribing for the time being.

We used F5 BIG-IP where I worked before (not together with Drupal). I did some research and it seems that BIG-IP does not send such a header by default but can easily be configured to send any header you want. So it should be fine if we document which headers are supported.

Status:Needs work» Needs review
StatusFileSize
new1.72 KB
Failed: 11815 passes, 18 fails, 1 exception
[ View ]

Updated for current HEAD

Status:Needs review» Needs work

Inline comment: I looked "spoor" up in the dictionary, since I wasn't sure what it meant (even though it's Dutch and I speak German). I even first googled to see if it was a specific term associated with HTTPS. The dictionary says that, in English, the word is chiefly used for animal droppings. Even if it can mean a generic trail, surely it would be better to use a word that doesn't make developers reach for the dictionary? ;)

Arancaytar,

http://drupal.org/coding-standards#comment seems to contain no restrictions on language. Are you suggesting that being imprecise is preferable if it keeps the language more simple? "Trail" is a path. "Spoor" is an indicator of something's existence or presence.

If there are criteria for selecting which English words are appropriate for use in PHP comments, I'll be happy to follow them. If not, and if nobody else objects, I'll move this back to "needs review".

<?php
if (!isset($conf['reverse_proxy']) || $conf['reverse_proxy'] === TRUE)
?>

Erm? That condition makes no sense. It should probably be:

<?php
if (isset($conf['reverse_proxy']) && $conf['reverse_proxy'])
?>

We don't want to use the block of code below if reverse_proxy is not set.

The whole block of code could be simplified this way:

<?php
+    if (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] == 'on') {
+     
// We're running HTTPS natively in the web server.
+      $base_root = 'https';
+    }
+    elseif (isset(
$conf['reverse_proxy']) && $conf['reverse_proxy']) {
+     
// Only trust this header if reverse_proxy is on or unset.  Note that
+      // this header is provided by the client and therefore can't be trusted.
+      if (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && strtolower($_SERVER['HTTP_X_FORWARDED_PROTO']) == "https") {
+       
// We appear to be behind a proxy.
+        $base_root = "https";
+      }
+      elseif (isset(
$_SERVER['HTTP_FRONT_END_HTTPS']) && $_SERVER['HTTP_FRONT_END_HTTPS'] == 'on') {
+       
// The proxy follows the Microsoft convention for passing protocol
+        // information back to the web server per MS KB document Q307347.
+        $base_root = 'https';
+      }
+    }
+
+    if (!isset(
$base_root)) {
+     
// Defaults to HTTP if we have not found a clue that this connection is using HTTPS.
+      $base_root = 'http';
+    }
?>

Damien, thanks for your input.

Note that the original purpose of this patch was to make it possible to install Drupal on HTTP web servers behind an HTTPS reverse proxy. I'm suggesting that we trust $_SERVER['HTTP_X_FORWARDED_PROTO'] if reverse_proxy is TRUE, or unset.

Your suggestion has an entirely different meaning, and will fail on a new installation where reverse_proxy has not been set because settings.php does not yet exist. If you're going to reverse the logic, you need to update the comments accordingly and provide another "else" that deals with cases where reverse_proxy is unset. If an administrator wants to "protect" their server from malicious HTTP headers, they can set $reverse_proxy in settings.php. (Perhaps proxy detection as part of the setup would also be useful, but that would be a different patch.)

Also, while removing the embedded elses makes the code easier to read, adding an isset() to the bottom will add a few ms of processing time to each hit. A faster approach might be to set $base_root to its default before the set of ifs, then overwrite its value, but I have the sense that overwriting variables is a little lazy, so I tend to avoid it.

What are your thoughts? Do we mind the extra CPU for an isset() on each hit? Seems like an avoidable waste to me. I haven't done any real performance testing to determine just how much extra CPU it would use.

1. We don't trust HTTP headers. Never. You have to create the settings.php file before the installation, so you should be able to set $conf['reverse_proxy'] there if you want to.

2. While the performance difference is not measurable, setting the default before the big if blocks makes sense. So please go ahead the way you are suggesting.

Damien,

We "trust" that the "Host:" header is correct, because it provides vital information regarding what web site is to be served. How is this any different? As far as I can tell, a forged "Host:" line puts us at just as much risk as a forged "X-Forwarded-Proto:" line. The only possible impact of adding that line on a server without HTTP is that AJAX callbacks and other functions that required URLs to be generated will stop working.

Per discussion above, nobody has suggested a way that a forged header might cause a problem or vulnerability, but without knowledge of the front-end protocol on a load balancer, all Drupal's AJAX callbacks, including those in the installer, will fail. If the community has decided that we WILL NOT support HTTPS-only installation behind load balancers, it should be documented somewhere. My impression over the last few years is that Drupal is leaning towards support of larger scale, enterprise environments.

@ghoti: Nothing prevent you from configuring your settings.php properly before launching the installation process. You already have to copy default.settings.php to settings.php anyway.

And no, we obviously don't trust the Host header.

Actually, if the directory is writable, Drupal will take care of writing a settings file by itself. For a single-site installation, this is the quickest and easiest way to get things running. IIRC, this change was made in response to complaints that Drupal was too difficult or cumbersome to install, especially for those who aren't comfortable editing .php files.

Would you be comfortable if we trusted a syntactically valid X-Forwarded-Proto header only in cases where settings.php does not already exist? Then at least the install would not fail.

As for the Host header ... we USE the Host header, and we certainly trust it to tell us what web site is being requested, because that's the only place we can get such information via HTTP. As long as it passes validation checks, we trust its content. If we didn't trust Host in this way, we'd have to go back to numbered virtualhosts. All I'm suggesting is that we do the same thing for another line in the header which is also the only place where certain other vital information is located.

Status:Needs work» Needs review
Issue tags:+reverse proxy, +https
StatusFileSize
new1.55 KB
Failed: Failed to apply patch.
[ View ]

Here's an update which implements Damien's suggestion to tighten up the code. It certainly is easier to read!

The previous version trusted the X-Forwarded-Proto header if 'reverse_proxy' was TRUE or unset.

This version trusts the header only if 'reverse_proxy' is unset due to the non-existence of settings.php, as mentioned in #19. This means that a completed installation has a 'reverse_proxy' that defaults to FALSE, and new installations will not fail.

StatusFileSize
new1.47 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch bootstrap.inc_313145_1.patch.
[ View ]

Slightly refactored version of the previous patch. This one checks for 'reverse_proxy' before checking for settings.php, in the hope that we can avoid accessing the filesystem more often than necessary.

+

Status:Needs review» Needs work

Only for code readability I would move the $base_root = "http"; // default to "else {}". Additionally // default is not the way how we write core documentation. Also do not surround a string with double quotes if not absolutely required. if ( (is... - this blank is not core code style... remove the blank, please.

Status:Needs work» Needs review
StatusFileSize
new2.07 KB
Failed: 12845 passes, 2 fails, 0 exceptions
[ View ]

I took the liberty of re-rolling the patch with your recommendations. Please see attached.

Status:Needs review» Needs work

The last submitted patch failed testing.

Not sure why that's failing - nothing has really changed from the previous patch which succeeded. When I ran the tests on my own server I get "Login form action is secure" in session.test line 303, and "Page has been cached" in bootstrap.test line 266, but I can't figure out why either one is failing. The login form seems to work fine and I stay on HTTPS, so what else is it expecting? And I have no clue about the other one.

subscribe

We require settings.php to exist in order to install Drupal - so this part of the patch seems wrong:

+    elseif (!file_exists(DRUPAL_ROOT . '/' . conf_path() . '/settings.php')) {

@DamZ - unless the installer has changed - we actually overwrite all contents of the original settings.php - in fact you can just you touch to make an empty file, you don't need to copy the default.settings.php.

Still - you can install with a customized settings.php by manually populating the database settings - I did it recently for D7 behind a reverse-proxy, since with the request going across multiple web nodes, unless you have you docroot in a netowrk file system you can't have Drupal write the settings anyhow.

Status:Needs work» Needs review
StatusFileSize
new1.2 KB
Passed: 14713 passes, 0 fails, 0 exceptions
[ View ]

why does it need to be more than this?

I can imagine some possible security issues with this - e.g. if you are trying to force all authenticated traffic (e.g. admins) over SSL, but allow non-SSL for anonymous users for performance reasons, this header in combination with a CSRF attack might be able to expose via non-SSL the session cookie for the user. If you are behind a reverse proxy it seems like http://www.php.net/manual/en/session.configuration.php#ini.session.cooki... could not be used, so you'd have to be relying on this header (for example).

A bit of an edge case, but worth considering. Clearly the easiest way to prevent it is to block these headers at the reverse proxy.

any reviews?

I can't test it on the server I actually need it on since we haven't been able to get php5.2 installed for Drupal 7 (fark RedHat!), but on my personal server (no reverse proxy) it seems to run just fine with this patch.

#21: bootstrap.inc_313145.patch queued for re-testing.

pwolanin, with your patch, how will bootstrap.inc know that a web server is running behind a HTTPS reverse proxy?

Note the original purpose of this issue: "The attached patch adds X-Forwarded-Proto and Front-End-Https support to Drupal."

Until a configuration has been created (i.e. during initial installation), variable_get() will not return accurate data. That's why in http://drupal.org/node/313145#comment-1709094 I had the patch check $conf itself rather than use the function with a default.

(I'm still trying to figure out why that patch failed...)

@ghoti - you have to manually wset it via settings.php - as you always do.

@pwolanin, the original point of this issue was to make it possible to install Drupal without pre-setting any variables in settings.php.

The security issues that were brought up were due to the concern that client HTTP headers could not be trusted, and that by basing our protocol on the available headers we might introduce an unknown risk.

I still think the patch at #21 is on-target for that goal, while introducing negligible risk.

Your simplification of most of this in #30 is great, but would you consider it unduly risky to support fresh Drupal installations by "believing" X-Forwarded-Proto headers if settings.php hasn't yet been created?

Priority:Normal» Major
StatusFileSize
new738 bytes
PASSED: [[SimpleTest]]: [MySQL] 23,288 pass(es).
[ View ]

Re-cut for today's -dev.

Note that this is now a one-liner, since the previous performance concerns I had about checking for the existence of settings.php at each hit is moot, since we're now doing it anyway.

The "security risk" of trusting this header are not addressed in this patch because:

  1. filtering non-legitimate headers at the proxy is the responsibility of the proxy administrator,
  2. adding this header can only reduce availability of content, notwithstanding CSRF attacks, and
  3. if we trust this header only when settings.php does not exist, we need also to modify install.inc so that that the rest of the install (after settings.php is created) will work. That makes this a much bigger patch....

@pwolanin, the original point of this issue was to make it possible to install Drupal without pre-setting any variables in settings.php.

Is it really something people actually do, i.e. install Drupal from scratch using the installer on a server that is behind a reverse proxy that does HTTPS-HTTP mapping? That doesn't sound like the typical development environment but rather like a place where you deploy your site afterwords.

Status:Needs review» Needs work

This is won't fix except if this is governed by an hidden variable that defaults to FALSE.

c960657, I've installed quite a few sites that way. And I certainly believe that there are IT managers and webmasters using large-scale server environments who may as yet be unfamiliar with Drupal but might wish to install it using the web UI method. Wouldn't you expect things to "just work", rather than just fail?

Damien, per our discussion, I'll work on the patch to add reverse_proxy to settings.php if proxy indicators exist at install time.

I may be asking for help. :-)

Status:Needs work» Needs review
StatusFileSize
new3.38 KB
PASSED: [[SimpleTest]]: [MySQL] 24,765 pass(es).
[ View ]

Here's the latest incarnation.

Per discussion with Damien, we continue to by-default assume client-supplied headers are untrustworthy, but that if reverse_proxy is on, headers are received from the proxy, and are therefore trustworthy.

Then, in order to make setup work, I've added a line to the install.php initial summary showing proxy status, and told install.core.inc to set the reverse_proxy variable if it sees proxy indicators during setup. I've included a link to additional documentation, which I'll write well before D7 is released.

Last thing I've done is to remove the function drupal_detect_baseurl() from install.inc, as it's not being (and should not be) used.

StatusFileSize
new3.38 KB
PASSED: [[SimpleTest]]: [MySQL] 24,759 pass(es).
[ View ]

It turns out, I *did* start writing this documentation already.

The attached file is identical to the previous one except that it points to existing documentation, which I will expand.

Can you do this as a unified diff? cvs diff -up > example.patch See here for more info: http://drupal.org/patch/create (scroll down to "Patch Readability"). Thanks.

StatusFileSize
new4.74 KB
PASSED: [[SimpleTest]]: [MySQL] 24,807 pass(es).
[ View ]

Here it is, including the link to http://drupal.org/node/425990 .

Status:Needs review» Needs work

Several code style issues, and at a "XXX" code comment that looks like it's missing it s intended content.

Several code style issues, and at a "XXX" code comment that looks like it's missing it s intended content.

Ah, I see the XXX - yes, that was just in for debugging; no code is missing. I'll remove it.

Can you point out the style issues, or how I might detect them? I believed I was following our coding style fairly well, based on what I read at http://drupal.org/coding-standards . Aside from the debugging line which I'll remove, what did I miss?

Thanks for your help.

Status:Needs work» Needs review
StatusFileSize
new4.7 KB
PASSED: [[SimpleTest]]: [MySQL] 24,819 pass(es).
[ View ]

Debug line removed.

Just tested #48, doesn't look like the reverse proxy variable was set in settings.php where I expected it. Is there somewhere else I should look to determine that it worked correctly? Nothing broke, but I can't tell whether it actually did what it was supposed to.

It furthermore is getting the load balancer IP in the watchdog instead of the correct one. Looks like this isn't actually doing anything.

Category:bug» task
Status:Needs review» Needs work

Sounds like 1) needs work and 2) a task (actually a feature request).

Please note that Squid sets the HTTP_FRONT_END_HTTPS variable to "On" instead of "on", so the

$_SERVER['HTTP_FRONT_END_HTTPS'] == 'on'

should be case-insensitive.

See: Squid cache_peer settings.

@george - we are already using strtolower() in the patch, probably for that reason.

@seanr - what in the patch do you expect to be adding something to settings.php?

@pwolanin actually, it looks like that patch forgets to wrap HTTP_FRONT_END_HTTPS in a strtolower. And I assume seanr is referring to the $conf['reverse_proxy'] = TRUE; to the settings file if you're behind a proxy. Personally, I see support as more important than seamless install but that's a nice to have I'm sure.

Couple feedback points:
- There seem to be extra spaces around parenthesis in if statements.
- drupal_detect_baseurl is removed but I don't see why. Its lack of use seems a separate issue.
- $_SERVER['HTTP_FRONT_END_HTTPS'] isn't wrapped in strtolower as mentioned above.
- Seems like we could wrap the "from proxy protocol" logic up into one function and call that so we don't have this giant string of compares spread out and repeated all over the place. Maybe I'm wrong though.

Are there any risks in also having this set $_SERVER['HTTPS'] to 'on'?
The patches in this issue seem to work for us ( Squid terminating SSL and forwarding to haproxy and then finally Apache) but the problem I've hit is that other code such as PHP libraries will just check for the $_SERVER variable. Rather than doing an equivalent fix for every third-party piece of software which comes along it would be great to have $_SERVER['HTTPS'] set once we've come out of bootstrap.

Version:7.x-dev» 8.x-dev
Issue tags:+needs backport to D7

This needs to be fixed in d8 first.

Subscribe.

Priority:Major» Normal

This really isn't major.

Hello All,

I am stuck at a point where I could not be able to get $_SERVER['HTTPS'] value from the Apache environment which has SSL on it. Could any body please tell me how can I decide from the $_SERVER that the server is HTTPS or not ???

--
PLN

I stuck this variant at the end of my settings.php file to get something working with a Zeus Load Balancer configured as the SSL end-point:

$is_https = (isset($_SERVER['HTTPS']) && strtolower($_SERVER['HTTPS']) == 'on') ||
            (isset($_SERVER['HTTP_X_FORWARDED_PROTO']) && strtolower($_SERVER['HTTP_X_FORWARDED_PROTO']) == "https") ||
            (isset($_SERVER['HTTP_FRONT_END_HTTPS']) && $_SERVER['HTTP_FRONT_END_HTTPS'] == 'on') ||
            (isset($_SERVER['HTTP_SSLSESSIONID']) && !empty($_SERVER['HTTP_SSLSESSIONID']));
$_SERVER['HTTPS'] = $is_https ? 'on' : 'off';

I have Zeus setup with 'ssl_headers' set to true, but this doesn't give anything like HTTP_X_FORWARDED_PROTO. I've had it key off the existence of HTTP_SSLSESSIONID for the moment since it only exists during an SSL session. If anyone has a better way to get the protocol from Zeus I'd love to hear it.

This should be fixed by using Symfony2's HTTP Foundation component.

http://api.drupal.org/api/drupal/core--includes--Symfony--Component--Htt...

As for the FRONT-END-HTTPS header. That's seems to generally be for configuring SSL offloading proxies to workaround Outlook Web Access not reading the X-FORWARDED-PROTO header. See BIG-IP's documentation and Microsoft's report.

The Zeus Load Balancer problem seems like way more of an edge case and should probably go in a separate issue.

I think the Zeus stuff is still valid in this issue because it demonstrates an edge case. Specifically, there will be edge cases and we need to make sure that there is a good way to handle them in the future, such as with Symphony.

So we agree that Zeus Load Balancer is an edge case and you have proven you can fix it with your settings.php. Then I don't really see the merit in bloating core further. Do we really want every Drupal site to check for Zeus headers on every page load?

However a write-up about how you fixed it for Zeus would be a nice addition to the documentation. There are probably other load balancers out there with their respective, proprietary header to convey the edge protocol.

@tobiassjosten: Yes and no. As you said this can get into the realm of Symfony2 once we get past D7, and this issue is no longer listed as D7 only. I think it's important to identify these types of edge-cases so that any solution brought to the table does not make it more difficult than it needs to be to get around said edge-case. It does not need to check for Zeus in core if there is an easy documented solution. I have a hack (I wouldn't call it more than that) for D7, but I want to make sure I have a solution moving forward.

Then I think we agree? :)

Documentation is still needed to cover SSL proxying with proprietary headers. Maybe you want to create a documentation page for this?

Btw, I found this excellent write-up that covers the solution very nicely: http://www.metaltoad.com/blog/running-drupal-secure-pages-behind-proxy.

We agree that a specific check for Zeus does not need to be in core, but I don't think you get the rest of what I'm saying. The symphony code, as it stands now, covers the major cases. Unfortunately, I still don't see a "clean" way to handle the edge cases in D8 and beyond.

A good example of what I want is the 'reverse_proxy_header' variable. It was added to provide an easy way to handle when something other than 'X-Forwarded-For' was used by an upstream proxy. It makes it so we don't have to do a hack like I did with Zeus. It would be nice if we also provided a variable to override the use of 'X_FORWARDED_PROTO' in Symphony.

I get what you're saying. Only I disagree with implementing it in Drupal core.

What it all boils down to is getting $_SERVER['HTTPS'] to say "on", before the bootstrap kicks in. A clean way of achieving this with proprietary proxy headers is to implement the logic in your settings.php - the file dedicated to holding your environment specific configurations.

// Zeus Load Balancer proxies SSL.
if (!empty($_SERVER['HTTP_SSLSESSIONID'])) {
  $_SERVER['HTTPS'] = 'on';
}

Done. This way no one but those with these edge cases will be bogged down by the extra checks and balances.

However, code speaks louder than words so if you have a core implementation in mind - please share and we can talk more specifically.

Agreed. I don't want it in core. I just want to make sure I can do it easily with 1-3 lines of code, rather than 10+, as this moves forward.

Status:Needs work» Needs review
StatusFileSize
new1.02 KB
PASSED: [[SimpleTest]]: [MySQL] 63,617 pass(es).
[ View ]

I'm not so sure about the use-case of needing to install Drupal through https, behind a reverse proxy.

I do know that if you don't have $_SERVER['HTTPS'] set to 'on' then you run the risk of a site full of insecure/mixed content warnings for image styles generated with an incorrect $base_path - at least, this is the case in D7.

Here is an attempt at doing something for D8 as simply as possible, without messing with the installer and ignoring the Microsoft headers that were questioned earlier.

The reason for setting $_SERVER['HTTPS'] to 'on' instead of working with $is_https inside drupal_settings_initialize() is that, as @dave_robinson pointed out in #55, and I did in my duplicate issue, other PHP code (like in vendor/) generally expects $_SERVER['HTTPS'] so setting this ensures that 3rd party code is more likely to work too.

Modifying $_SERVER is a Wrong Thing To Do(tm) on many levels.

Tell that to Acquia >.<

"Handling incoming HTTPS requests"
https://docs.acquia.com/cloud/configure/https

And what is "The Right Way" to fix this while supporting 3rd party PHP libraries? I'll roll a patch if you tell me :)

function settings() was removed. Used Settings::get() instead

StatusFileSize
new914 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Drupal installation failed.
[ View ]

Generally I'd be against this but here I agree, the HTTPS environment variable actually has to be set. As has been said repeatedly, this is because this variable is expected by convention to be set correctly outside of our or Symfony's code including in libraries we're using in core. https://github.com/zendframework/zf2/blob/master/library/Zend/Feed/PubSu...
I can't find where that code would actually be called in our code base, but it serves as an example of why we need to do this.

Let me put it this way. You could be handling setting the environment correctly outside of PHP code, for example in your .htaccess. That might even be the best way. However, the entire point of our reverse proxy code is to do that in software based on user configured values. So if we're going to do it we need to do it right and have it just work.

Here's a little bit tighter version that very well may fail by building a request too early. However after #2016629: Refactor bootstrap to better utilize the kernel goes in there's a handy place in the new bootstrap that already had the request object available that's a perfect place for this code.

Status:Needs review» Needs work

The last submitted patch, 75: 313145-75.patch, failed testing.

Status:Needs work» Needs review

75: 313145-75.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 75: 313145-75.patch, failed testing.

This is a simple enough patch that's been going back/forth now since 2008. @neclimdul your patch looks good but I don't know how to debug the problem here:

Fatal error: Call to a member function get() on a non-object in /home/s9f53f623164402c/www/core/lib/Drupal.php on line 187

@Crell, would be really good if you could give us some direction about what you think the way forward should be.

This is a potential security problem for a lot of sites. It shouldn't sit this long without a fix.

It's currently a big note on the Using a load balancer or reverse proxy Handbook page.