Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update uri.php #8690

Merged
merged 7 commits into from Nov 1, 2016
Merged

Update uri.php #8690

merged 7 commits into from Nov 1, 2016

Conversation

bpeterson69
Copy link
Contributor

@bpeterson69 bpeterson69 commented Dec 15, 2015

Added additional validation code to support reverse proxies. Without this additional validation, a Joomla site set to force SSL on the entire site will go into an infinite redirect loop because the reverse proxy is sending the information to confirm SSL in the $_SERVER variable key 'HTTP_X_FORWARDED_PROTO', not the key 'HTTPS'.

Added additional validation code to support reverse proxies.  Without this additional validation, a Joomla site set to force SSL on the entire site will go into an infinite redirect loop because the reverse proxy is sending the information to confirm SSL in the $_SERVER variable key 'HTTP_X_FORWARDED_PROTO', not the key 'HTTPS'.
@PhilETaylor

This comment was marked as abuse.

@Hackwar
Copy link
Member

Hackwar commented Feb 19, 2016

@bpeterson69 will you fix this PR?

@bpeterson69
Copy link
Contributor Author

I believe I have fixed the PR

@bpeterson69
Copy link
Contributor Author

What do I need to do to get this patch moved into the base branch?

@PhilETaylor

This comment was marked as abuse.

@bpeterson69
Copy link
Contributor Author

The reason behind the PR is to prevent client browsers going into an infinite redirect loop when Joomla is set to require SSL. The problems presents itself when Apache is configured as a reverse proxy (Apple is deploying Apache as a reverse proxy in Server 5.0.15 and later). The $_SERVER array that Apache returns in this case does not have the key 'HTTPS'. The patch adds additional sanity checking to validate against a different key in the $_SERVER (the key is 'HTTP_X_FORWARDED_PROTO ') array to prevent an infinite redirect loop.

The way the problem manifests itself is as follows:

  1. Install Joomla on an Apple Server running Server 5.0.15 or later.
  2. In the Administration interface, goto Global Configuration->Server.
  3. Set Force SSL to Entire Site.
  4. Now when you try to browse to the site, you will get a "to many redirects" error in your browser.

The reason you get this error is the current version of uri.php checks for the value of $_SERVER['HTTPS'] to verify if the client is connecting via SSL. Here is what essentially happens at this point:

The user loads the website, doesn't matter if it is HTTPS or HTTP. The uri.php file checks the variable $_SERVER['HTTPS'] to validate if the user has arrived at the site over SSL or not. Since the $_SERVER array does not have the key "'HTTPS', uri.php decides that the user has not arrived via HTTPS and forces the user to reload the page over SSL, again, the $_SERVER array does not have the key 'HTTPS' so again, Joomla assumes the user has not arrived via HTTPS and does the redirect again. This continues until the browser gives up and gives the error "To many redirects".

The patch I have created leaves the original code in place for the validation of the $_SERVER['HTTPS']. It adds additional validation against the key the reverse proxy is returning in the $_SERVER array that indicated SSL or not. You can look at the contents of the $_SERVER array in the comment I made on Dec 22.

I have received reports that Cloudflare uses reverse proxy as well and this patch resolves the issue.

Please let me know if you need more information.

@bpeterson69
Copy link
Contributor Author

Here are two messages I received on the patch (not sure why they aren't listed here).

The example code is kindly specified above by line number, in place of pre--- & post+++ change notation.

The issue raised is more frequent than suggested.

Test latest Joomla using CloudFlare and enable their SSL. Put the debugger on and review your site in HTTPS and you will see the lockups + database down issues with too many redirects = refused connection in some MySQL configs. Circular hits are throttled at the database connection level on some servers or the browser stops on multiple redirects. Also see the page reference javascript code being rejected then make this change to core as suggested ... (sorry folks, core change not recommended but to prove a point).

CloudFlare + SSL works after applying this amendment.

My site now runs in CloudFlare SSL: https://broadbandwithoutaphoneline.com/

SSL "free as in beer..."

My thanks to bpeterson69.

Anton
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7916.

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

and

Fixed the issue for me on J 3.6.2, also when using Cloudflare free SSL. Clearly something that needs implemented.

This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/7916.

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@brianteeman
Copy link
Contributor

Those comments are on your older issue of #7916

On 27 October 2016 at 00:02, bpeterson69 notifications@github.com wrote:

Here are two messages I received on the patch (not sure why they aren't
listed here).

The example code is kindly specified above by line number, in place of
pre--- & post+++ change notation.

The issue raised is more frequent than suggested.

Test latest Joomla using CloudFlare and enable their SSL. Put the debugger
on and review your site in HTTPS and you will see the lockups + database
down issues with too many redirects = refused connection in some MySQL
configs. Circular hits are throttled at the database connection level on
some servers or the browser stops on multiple redirects. Also see the page
reference javascript code being rejected then make this change to core as
suggested ... (sorry folks, core change not recommended but to prove a
point).

CloudFlare + SSL works after applying this amendment.

My site now runs in CloudFlare SSL: https://broadbandwithoutaphoneline.
com/

SSL "free as in beer..."

My thanks to bpeterson69.

Anton
This comment was created with the J!Tracker Application at
issues.joomla.org/joomla-cms/7916.

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

and

Fixed the issue for me on J 3.6.2, also when using Cloudflare free SSL.
Clearly something that needs implemented.

This comment was created with the J!Tracker Application at
issues.joomla.org/tracker/joomla-cms/7916.

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#8690 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8W3qwxYVCTjCuTrktdeKl47XzBh_ks5q39wFgaJpZM4G1TRr
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

@brianteeman
Copy link
Contributor

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@brianteeman
Copy link
Contributor

I was asking if the issue was the same more than is the solution the same

But not installed it etc - its late but here you are
http://i.tee.mn/cloudflareunissl_system.zip (I already had an account)

On 27 October 2016 at 00:08, Phil Taylor notifications@github.com wrote:

Is this the same thing as https://www.simbunch.com/
products/free-extensions/cloudflare-for-joomla

Dunno - was unable to register on their site to download the free
extension :(


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#8690 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8ZUM3D7RFX9eKKMj1iR3CTWnM8cgks5q3910gaJpZM4G1TRr
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@bpeterson69
Copy link
Contributor Author

I understand the concern. In this case, not implementing the patch protects people from creating a security issue that they my not be aware of if they don't deploy it properly.

@PhilETaylor

This comment was marked as abuse.

@bpeterson69
Copy link
Contributor Author

I have tested this item ✅ successfully on af3aad6

I have deployed this item across 5 Joomla Sites for several months (every time a new version of Joomla comes out). I understand the concern about security when using a reverse proxy, however, many software packages can be insecure if deployed improperly. There has to be a certain level of responsibility on the user to do his/her homework and Deploy there solution in a secure fashion.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@piute
Copy link

piute commented Oct 27, 2016

I have tested this item ✅ successfully on af3aad6

The test was successful with no errors.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@pogscomp
Copy link

I have tested this item ✅ successfully on af3aad6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@bpeterson69
Copy link
Contributor Author

Would this be better deployed in a plugin similar to the cloudflare plugin? That plugin could be modified very easily to cover the $_SERVER key "HTTP_X_FORWARDED_PROTO".


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@aprilddc
Copy link

I have tested this item ✅ successfully on af3aad6


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@brianteeman
Copy link
Contributor

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/8690.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 28, 2016
@brianteeman brianteeman added this to the Joomla 3.7.0 milestone Oct 28, 2016
@rdeutz rdeutz merged commit e782c49 into joomla:staging Nov 1, 2016
@zero-24 zero-24 removed the RTC This Pull Request is Ready To Commit label Nov 1, 2016
nvyush pushed a commit to nvyush/joomla-cms that referenced this pull request Nov 9, 2016
* Update uri.php

Added additional validation code to support reverse proxies.  Without this additional validation, a Joomla site set to force SSL on the entire site will go into an infinite redirect loop because the reverse proxy is sending the information to confirm SSL in the $_SERVER variable key 'HTTP_X_FORWARDED_PROTO', not the key 'HTTPS'.

* Update uri.php

* Update uri.php

* Update uri.php

* Update uri.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet