Problem/Motivation

  • An increasing number of operating systems and hosts have tightened up their security settings and now forbids +FollowSymLinks option in the .htaccess file that comes with Drupal core. This causes an error 500 when accessing the site. When they introduced this policy they automatically converted +FollowSymLinks to +SymLinksIfOwnerMatch. A Drupal upgrade overwrote this change.

Affected platforms and systems

Proposed resolution

Don't set Options +FollowSymLinks in our .htaccess files.

Remaining tasks

None

Why is this an RC target?

If a server 500's because of this setting then people can not even install Drupal.

Alleged +FollowSymlinks weakness which leads to security exploits

Contrib module that support SymLinksIfOwnerMatch

Files: 
CommentFileSizeAuthor
#152 output.txt3.36 KBtuandungb
#143 drupal_remove_followsymlinks_htaccess_drupal7-1269780-143.patch898 bytesdamien_vancouver
PASSED: [[SimpleTest]]: [MySQL] 41,754 pass(es). View
#54 Configure_site-step_1.png973.3 KBFrancewhoa
#54 Configure_site-step_2.png83.18 KBFrancewhoa
#54 500 Internal Server Error with Drupal 8.png16.43 KBFrancewhoa
#60 Internal_Server_Error.png15.01 KBFrancewhoa
#42 drupal-7.x-symlinksifownermatch-1269780-42.patch759 bytesAgileware
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-42.patch. Unable to apply patch. See the log in the details link for more information. View
#91 drupal-use_symlinksifownermatch_instead_of_followsymlinks-1269780-91.patch7.09 KBdamien_vancouver
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch drupal-use_symlinksifownermatch_instead_of_followsymlinks-1269780-91.patch. Unable to apply patch. See the log in the details link for more information. View
#115 drupal-1269780-115-Use-SymLinksIfOwnerMatch.patch6.52 KBaxel.rutz
#129 1269780.129.patch3.82 KBalexpott
#136 2015-11-07---before_patch_1269780.129.png17.03 KBFrancewhoa
#136 2015-11-07---after_patch_1269780.129.png435.32 KBFrancewhoa
#136 2015-11-07---after_patch_1269780.129-successful_installation.png44.38 KBFrancewhoa

Comments

aspilicious’s picture

Status: Active » Closed (won't fix)

It's a comman case yoiu have to disable the "options" in the htacess file. You do this by placing a '#' in front of that line. When upgrading you need to back up your own htaccess file.

This is not a bug.

AFowle’s picture

Status: Closed (won't fix) » Active

It is a bug - it caused an error. It IS fixable.
I realise that I can save and restore the .htaccess file, but that is not the entire answer. I think the change I suggested would be the better option. At the very least it needs mentioning in the documentation (it's not in the install guide). Before I write a section for the documentation, we should discuss changing the distributed file if the change would be better for the majority.

mdupont’s picture

Title: +FollowSymLinks option in .htaccess blocked by host, causing error 500 » Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess
Version: 7.8 » 8.x-dev
Category: bug » feature

As per #2 it seems to me that it is a feature request against standard .htaccess file. Advantages seem to be a better out-of-the-box compatibility with some hosting providers and maybe better security. Drawbacks would be less choice given to the user (what if he/she actually wants to use a different owner for some linked folders?) and SymLinksIfOwnerMatch being slower (more system calls needed to stat the file).

AFowle’s picture

I can live with the analysis and resolution in #3, thanks. Do you know how much slower SymLinksIfOwnerMatch would be? If this is accepted by the community / core maintainers, would it be back-ported to the next update of D7?

mdupont’s picture

@AFowle: I don't know more about performance hit than what is said on http://httpd.apache.org/docs/2.2/misc/perf-tuning.html#symlinks

Wherever in your URL-space you do not have an Options FollowSymLinks, or you do have an Options SymLinksIfOwnerMatch Apache will have to issue extra system calls to check up on symlinks. One extra call per filename component.

The policy for Drupal bugs and feature requests is to commit the fix firstly to the latest development version of Drupal (D8), then backport it to previous version (D7 and D6) to ensure the development version always have all the fixes.

Ankabout’s picture

Just ran into this myself, but I think it's worth an investigation. Is this going to be a new 'standard' for web hosts, where they disable symlinks? If so, and it will affect a large majority of Drupal users, I think considering a change in the .htaccess is worth it.

In both cases documentation should be updated I think.

axel.rutz’s picture

a big +10 for this.
had big issues with this on 2 hosts and it really seems this gets hoster "standard".

giorgio79’s picture

Title: Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess » Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess - Security
Issue tags: +security

The latest upgrade of my Virtualmin (security announcement ) control panel alerted me to change FollowSymLinks to SymLinksIfOwnerMatch to protect against malicious links into other domain's directories..

Drupal works just as fine with this htaccess setting.

yngens’s picture

Status: Needs review » Active

Quote in #5 from http://httpd.apache.org/docs/2.2/misc/perf-tuning.html#symlinks means Apache will have to issue extra system calls to check up on symlinks for both Options FollowSymLinks and Options SymLinksIfOwnerMatch, but I am little bit concerned by mdupont's assirtion that SymLinksIfOwnerMatch is slower. Is it really so?

And what we should do with kind of suggestions as on http://drupal.org/node/1722250

set FollowSymLinks everywhere and never set SymLinksIfOwnerMatch

?

And here is contravening opinion from https://www.virtualmin.com/node/24493#comment-110641--

It would seem to me that the Drupal guys doesn't overly care about security, if they instruct users to apply the insecure FollowSymlinks everywhere.

They should be made aware of this potentially serious issue and make their software work with SymlinksIfOwnerMatch.

hswong3i’s picture

Status: Active » Needs review
FileSize
4.42 KB
PASSED: [[SimpleTest]]: [MySQL] 49,422 pass(es). View
2.57 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-10.patch. Unable to apply patch. See the log in the details link for more information. View

Totally understand yngens's concern (as I am also mainly focus on performance improvement, indeed), but I need to agree with both axel.rutz and giorgio79 point of view: most modern hosting provider give +SymLinksIfOwnerMatch as their default setup, even as they already understand its performance impact and willing to waste more CPU/IO cycle for better security measurement. For most end-user, out-of-the-box default setup in high security should always be a good idea.

IMHO, if performance is the primary goal for corresponding site owner, we may tweak overall setup in more detail style, e.g. varnish + memcache + cdn + mod_php + php_apc + MySQL master/slave + Apache cluster + etc (including using +FollowSymLinks). For power user, we should already have enough knowledge to handle our requirement.

Patch attached for both Drupal core 8.x and 7.x. Simply search with find . | xargs fgrep -nH FollowSymLinks and replace all +FollowSymLinks as +SymLinksIfOwnerMatch. Will apply to DruStack for indeed testing.

giorgio79’s picture

Hey hswon3i,

Patch looks good, will try to test it. Meanwhile, I started an htaccess module and seeing your patch, I made you comaintainer as well :)
http://drupal.org/sandbox/giorgio79/1872392

hswong3i’s picture

giorgio79 thanks for mark me as co-maintainer, but I would like to suggest support this change from Drupal core point of view ;-)

Patch from #10 just tested with Virtualmin 3.97.gpl GPL + DruStack 7.x-18.x-dev, works fine as expected.

yngens’s picture

Status: Active » Needs review

Drupion have been making available the patched version of Drupal 6, Drupal 7 and Presslow for a while now on http://drupion.com/resources/downloads/drupal

nothinghere’s picture

@yngens : "drush @all-alias up" didn't download from drupion.com. This is usless for me... but thanks anyway.

yngens’s picture

Oh sorry, but you have to wget in SSH.

Agileware’s picture

FileSize
1.04 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-6.x-symlinksifownermatch-1269780-17.patch. Unable to apply patch. See the log in the details link for more information. View

Patch attached for Drupal core 6.x

Status: Needs review » Needs work

The last submitted patch, drupal-6.x-symlinksifownermatch-1269780-17.patch, failed testing.

yngens’s picture

Just so you know Boost module is affected by this change and also should be patched.

hswong3i’s picture

Assigned: Unassigned » hswong3i
FileSize
4.37 KB
FAILED: [[SimpleTest]]: [MySQL] 58,853 pass(es), 1 fail(s), and 0 exception(s). View
2.56 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-20.patch. Unable to apply patch. See the log in the details link for more information. View
1.45 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-6.x-symlinksifownermatch-1269780-20.patch. Unable to apply patch. See the log in the details link for more information. View

Patch(es) reroll via all latest dev.

hswong3i’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-8.x-symlinksifownermatch-1269780-20.patch, failed testing.

Agileware’s picture

FileSize
2.07 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-23.patch. Unable to apply patch. See the log in the details link for more information. View

Updated patch for Drupal core 7.x

ricardoamaro’s picture

This is vastly important to apply in order to have better security over symlinks.

hswong3i’s picture

Status: Needs work » Needs review
Issue tags: -security

Status: Needs review » Needs work
Issue tags: +security

The last submitted patch, drupal-7.x-symlinksifownermatch-1269780-23.patch, failed testing.

ricardoamaro’s picture

FileSize
3.72 KB
FAILED: [[SimpleTest]]: [MySQL] 58,789 pass(es), 1 fail(s), and 0 exception(s). View

Patch for Drupal 8.x

ricardoamaro’s picture

FileSize
2.07 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-24.patch. Unable to apply patch. See the log in the details link for more information. View

Patch for Drupal 7.x

ricardoamaro’s picture

Status: Needs work » Needs review
FileSize
1.04 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-6.x-symlinksifownermatch-1269780-21.patch. Unable to apply patch. See the log in the details link for more information. View

Patch for Drupal 6.x

Status: Needs review » Needs work
Issue tags: -security

The last submitted patch, drupal-6.x-symlinksifownermatch-1269780-21.patch, failed testing.

ricardoamaro’s picture

Status: Needs work » Needs review
Issue tags: +security
ricardoamaro’s picture

Patches for Drupal 6.x and Drupal 7.x passed on issues #2106057 & #2106273 respectively

scor’s picture

It would help to get this committed if you were able to show how this improve security on various distro/platform/providers and generally link to others who have done the same change. I can see some comments mentioning external references, but it would be good to have a good overview in the summary of this issue. Also, does it impact performance in any way? Does this increase the Apache requirements for D7 or even D6? (in which case it would not be compatible)

The current summary indicates that this still needs to be tested on other systems (in remaining tasks).

scor’s picture

Issue summary: View changes

added security concern

christefano’s picture

Issue summary: View changes

It would help to get this committed if you were able to show how this improve security on various distro/platform/providers and generally link to others who have done the same change.

I've added an "Affected platforms and systems" section to this issue.

christefano’s picture

Issue summary: View changes
christefano’s picture

Issue summary: View changes

I also updated the issue with a link to the Sucuri blog post about this security problem.

I don't think that running arbitrary PHP code (as mentioned in the article) is the only attack vector. Can't someone simply upload a symlink via a vulnerable application's frontend interface and therefore have access to wherever the symlink points to?

christefano’s picture

I also updated the issue with a link to the Sucuri blog post about this security problem.

I don't think that running arbitrary PHP code (as mentioned in the article) is the only attack vector. Can't someone simply upload a symlink via a vulnerable application's frontend interface and therefore have access to wherever the symlink points to?

hswong3i’s picture

FileSize
4.38 KB
FAILED: [[SimpleTest]]: [MySQL] 59,115 pass(es), 1 fail(s), and 0 exception(s). View

Update for 8.x via GIT, after 7.24 released.

Status: Needs review » Needs work

The last submitted patch, 38: drupal-8.x-symlinksifownermatch-1269780-38.patch, failed testing.

AFowle’s picture

As the original poster, I am grateful to you all for your efforts over the last 2yrs. I note, with sadness, that the core update issued 2 days ago for D6 and D7 advises hand editing of .htaccess files but does not cover this issue!

Agileware’s picture

Updated patch for Drupal 7

Agileware’s picture

FileSize
759 bytes
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch drupal-7.x-symlinksifownermatch-1269780-42.patch. Unable to apply patch. See the log in the details link for more information. View

Updated patch for Drupal 7, without the .txt extension this time.

hswong3i’s picture

Issue summary: View changes
Related issues: +#2106273: Testing patch for +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess - Drupal 6.x - Security, +#2106057: Testing path for +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess - Drupal 7.x - Security
hswong3i’s picture

Status: Needs work » Needs review

For failed test on #38, the report show as below:

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 536870912 bytes) in /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Database/Connection.php on line 336
FATAL Drupal\system\Tests\Common\UrlTest: test runner returned a non-zero error code (255).

Seems not directly related to this patch itself but other else issue?

hswong3i’s picture

FileSize
2.98 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch drupal-8.x-symlinksifownermatch-1269780-45.patch. Unable to apply patch. See the log in the details link for more information. View

Re-roll via latest drupal-8.x-dev

christefano’s picture

Issue summary: View changes

Adding a note that Boost has been supporting SymLinksIfOwnerMatch for about a month now.

Status: Needs review » Needs work

The last submitted patch, 45: drupal-8.x-symlinksifownermatch-1269780-45.patch, failed testing.

yannickoo’s picture

Assigned: hswong3i » Unassigned
Status: Needs work » Needs review
FileSize
2.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,673 pass(es). View
Vayira’s picture

What is the state of this issue for drupal 6? I'm trying to move my site to a virtualmin server & am finding it difficult to get it working.

axel.rutz’s picture

Category: Feature request » Bug report
Status: Needs review » Reviewed & tested by the community

d8 patch #49 applies and fixes the issue for me.
d7 patch #42 applies and fixes the issue for me.

so both rtbc.

as symlinksifownermatch has security issues (using symlink to get RW privileges of webserver) and now is deactivated on any responsible shared hoster, changed to bug.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/.htaccess
@@ -11,7 +11,7 @@
 # Follow symbolic links in this directory.
-Options +FollowSymLinks
+Options +SymLinksIfOwnerMatch

Let's improve the comments - I guess there might be situations where this would break sites - where the owners don't match - so I'm not sure if this is backportable.

axel.rutz’s picture

Sorry, is there any known real-world use case for this? To me this sounds like a weird setup.
When we change code we do not care about legacy code that might break, but is pathologic in the first place.

As for backport: Shared hosting is still a mass market and as more and more sharehosters disable +FollowSymLinks for good reasons.
So in fact
* +SymLinksIfOwnerMatch is the evolving option you can safely rely on
* +FollowSymLinks you can not rely on and if you need it you know what to do.

So my 5 cent: yes let's improve the comment (i'll roll a new patch later) and backport this with a suitable changenote.

Francewhoa’s picture

Priority: Normal » Critical
FileSize
973.3 KB
83.18 KB
16.43 KB

Agileware and yannickoo, thanks for the patches :)

All, I suggest to set this ticket to priority "Critical" because of the following two reasons. What do you think?

  1. Expose security vulnerabilities: As you know FollowSymlinks does not protect against malicious links into other domain's directories;
  2. /.htaccess files brake the Drupal installation process which renders it unusable: Details and steps to reproduce below.

Steps to reproduce the second reason

  1. Edit the /.htaccess file. Replace Options +FollowSymLinks with Options +SymLinksIfOwnerMatch
  2. Go to the base URL of your website. Start the Drupal installer. So far it works. Yay :)
  3. During the Drupal installation process. At "Configure site" step fill the form. Click on "Save and continue" button.
    Screenshot of Configure site step 1
  4. On the next page the user is presented with a broken Drupal theme. Which renders it unusable. Resuming the installation and using Drupal range from difficult or impossible depending on the user' skills level. To clarify find attached screenshot "Configure_site-step_2.png".

    Screenshot of Configure site step 2

  5. Expected result is user should be able to install Drupal without being presented with a broken Drupal theme.

Notes

  • As you know this issue affect multiple .htaccess files. Up to four and maybe more depending on the core installer, preferences, or which Drupal installer profile.
    1. /public_html/.htaccess
    2. /public_html/sites/default/files/.htaccess
      • Note: That file is the source of the issue that brakes the Drupal theme during Drupal installation process
    3. /public_html/sites/default/files/private/.htaccess
    4. /tmp/.htaccess
      • Note: That file is created by Drupal installer. It is located outside the Drupal directory.

Using

  • Drupal: 8.0.0-beta9 (2015-Mar-25)
  • PHP: 5.4.39-0+deb7u2
  • Apache: 2.2.22
  • MySQL: 5.5.41
  • Operating system: Debian Linux Wheezy 7.8 at 64 bits
  • Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz, 1 cores
  • PHP memory limit: 256M
  • PHP API: 20100412
  • PHP Extension: 20100525
  • PDO drivers: mysql
  • Virtualmin: 4.15.gpl
xjm’s picture

Priority: Critical » Major

Thanks @Francewhoa for taking a close look at this issue.

A security hardening with no actual vulnerability is at most major; we would not block release on it except under exceptional circumstances. Also, the bug with the installer under a specific unusual configuration also matches the definition of a major.

Reference: https://www.drupal.org/core/issue-priority#critical

Francewhoa’s picture

xjm, thanks for your message :)
I'll let the community's documentation for priority decide about what priority level is appropriate

I disagree with the unusual configuration though. Debian is one of the most widely use operating system. And Ubuntu is based on Debian. Together they are the most widely use Linux operating system. According to W3techs roughly 58% of all the websites who use Linux are affected (26.5% + 32.3%).

We were able to reproduce this error with a stock Debian 7.8 and a stock Drupal 8 install. Maybe I'm misunderstanding the meaning of unusual configuration? Maybe this issue was not clear about how many Drupal users are affected? Per above statistics this error is not a rare circumstance and affects a large percentage of all Drupal users.

xjm’s picture

@Francewhoa, the .htaccess configuration you are proposing is the unusual part. Drupal 8 HEAD does not show this error on Debian; you have to modify the existing .htaccess file for it to occur. If this patch went in without addressing that, then we'd be introducing a critical regression by breaking Debian sites. But as it is, Drupal 8 itself works fine on Debian. Thanks!

Francewhoa’s picture

xjm, thanks for clarifying :)

We would be happy to contribute testing and quality assurance (QA) for Drupal 8 HEAD. Where can we find it? Maybe we QA the wrong version? The version we QA and were able to reproduce that error is with Drupal: 8.0.0-beta9 (2015-Mar-25).

We were able to reproduce that error with a stock Drupal 8 and stock Debian 7.8. By "stock" we mean no modification to the existing Drupal core .htaccess file, no modification to Drupal 8, no modification to Debian 7.8. All of them fresh installs.

By ".htaccess" file we mean the file that comes with Drupal 8 core. One of them is located at /.htaccess

As for the contributed patches from Agileware and yannickoo, they said they still need work. They do fix the issue but they said they are not yet ready to be committed. I'm not a Dev so I trust their judgment and skills on that. I agree with you that committing those patches now seems premature. I was not suggesting that. I'm suggesting to set the priority to "Critical" as it seems to be a closer fit to that description.

Again I'll let the community's documentation for priority decide.

To whoever owns the decision about what priority level is appropriate I suggest to consider the above information when making your decision. There is maybe some new information in there.

xjm’s picture

Thanks @Francewhoa. I'm confused by #58 as it doesn't seem to be the same as #54 or the issue summary. Your step 1 in #54 states that the .htaccess file needs to be modified.

The workaround would presumably be to install Drupal first (or on a different environment) and then modify the .htaccess file afterward. I'm not saying that we shouldn't fix the issue -- just that if 58% of people were unable to use Drupal at all, that would be worth blocking the release of Drupal. But if 58% people can't install Drupal using a non-standard (but recommended, or required by some hosts) .htaccess configuration, then that's a major. :)

Francewhoa’s picture

FileSize
15.01 KB

Thanks @Francewhoa. I'm confused by #58 as it doesn't seem to be the same as #54 or the issue summary. Your step 1 in #54 states that the .htaccess file needs to be modified.

xjm :) #54 and #58 are different. #54 is the result of testing the latest patch. That comment was meant for the patch Developers not for you ;) Sorry for the confusion. #58 is the result of testing a standard Drupal 8 and standard Debian. Without any modification to Drupal or Debian. #58 was meant for you. If this is confusing I suggest to ignore both #54 and #58 and focus on the below steps to reproduce issue. It's the same as #58 but re-worded.

Summary

Issue is that ~58%* of the most widely use Linux operating system are unable to use a standard Drupal 8 at all

Steps to reproduce issue

  1. Install the following standard and fresh items. Do not make any modification. Do not apply any patch.
    1. Operating system: Debian Linux Wheezy 7.8 at 64 bits
    2. Drupal: 8.0.0-beta9 (2015-Mar-25)
    3. Apache: 2.2.22
    4. MySQL: 5.5.41
    5. For details and versions of all items use find below "Using" section
  2. Run Drupal 8 install script. To do so go to the base URL of your website. For example http://www.example.com
  3. The following error message is return: "Internal Server Error"

    Screenshot of error
  4. User is not able to install or use Drupal 8 at all
  5. Expected result is the user should be guided through several screens to install Drupal 8

Notes

  • *Debian is one of the most widely use operating system. And Ubuntu is based on Debian. Together they are the most widely use Linux operating system. According to W3techs stats roughly 58% of all the websites who use Linux are affected (26.5% + 32.3%).
  • That issue was reproduced with a standard and fresh Drupal 8, Debian 7.8. No modification. No patch applied.
  • The issue seems to be cause by the .htaccess files that comes with Drupal 8 core. Such as the file located in the Drupal root directory at /.htaccess Other /.htaccess files are created by Drupal 8 core during Drupal 8 installation. All those /.htaccess files seem to be related to the issue.

Using

  • Drupal: 8.0.0-beta9 (2015-Mar-25)
  • PHP: 5.4.39-0+deb7u2
  • Apache: 2.2.22
  • MySQL: 5.5.41
  • Operating system: Debian Linux Wheezy 7.8 at 64 bits
  • Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz, 1 cores
  • PHP memory limit: 256M
  • PHP API: 20100412
  • PHP Extension: 20100525
  • PDO drivers: mysql
  • Virtualmin: 4.15.gpl
nothinghere’s picture

@xjm : I'm following this post since +2 years.
It's a major issue for me when I need to update each drupal website I host (actually 12 websites, more soon).

Debian is a very stable platform for web hosting, I'm using it for personal and professionnal web hosting since 10 years.
Drupal (with drush update) is very good... but this .htaccess compatibility is a major problem for me.

I need to run manualy this script after each drush update command : my websites are down for ~10 min each time (debian update, drush update and .htaccess update).
If it helps someone, here is some code for people who has this problem with FollowSymLinks / SymLinksIfOwnerMatch :

#!/bin/bash
echo  "Start patching of .htaccess files"
find /home -name ".htaccess" -type f -exec sed -i 's/FollowSymLinks/SymLinksIfOwnerMatch/g' {} ";"
echo "End patch of SymLinksIfOwnerMatch."
echo "Patching exemple.com"
chown -R exemple.com:exemple.com /home/exemple.com/public_html/
echo "Patching subdomain.exemple.com"
chown -R exemple.com:exemple.com /home/exemple.com/domains/subdomain.exemple.com/public_html/
echo "Errors : $?. Script end."
exit 0
giorgio79’s picture

@xjm a bit of a background. Many webhosts and linux distros started tightegning from FolowSymlinks to SymlinksIfOwnerMatch due to security exploits. More info here

Francewhoa’s picture

All, hope the following helps :)

Francewhoa’s picture

Issue summary: View changes
Francewhoa’s picture

Issue summary: View changes
Francewhoa’s picture

Issue summary: View changes
Francewhoa’s picture

Issue summary: View changes
Francewhoa’s picture

Issue summary: View changes
xaa’s picture

I need to run manualy this script after each drush update command : my websites are down for ~10 min each time (debian update, drush update and .htaccess update).

I've also created a basic bash interactive script to reduce the downtime due to the htaccess modifications. You can find it on github

After $ druh up drupal I run :
$ bash htaccess.sh xxx
where xxx is the folder you want (/home/xxx).

The script ask for some inputs such as :
- specify the folder you want to adjust ($1)
- confirm the "root folder" (/home by default)
- list the folders available (in case you are not sure of the foldername /home/xxx you want to to adjust)

then the script will replace recursively +FollowSymlinks by +SymLinksIfOwnerMatch and list the files adjusted.
The downtime due to the htaccess is less than 20sec here.

David_Rothstein’s picture

Title: Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess - Security » Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess
Issue tags: -Security improvements, -security

This issue was recently brought to the attention of the Drupal security team.

It's my understanding that there is no security-related issue here at all. On a shared hosting setup that is vulnerable to this issue, an attacker who already had control of someone's account could just add +FollowSymlinks to an .htaccess file on their own; it doesn't matter if Drupal does it for them by default or not. That is why hosts are blocking this at the server level, right?

If I'm mistaken and if including +FollowSymlinks in Drupal's .htaccess file actually makes the Drupal installation itself vulnerable to attack, that would be a security issue that should be reported to the security team and handled privately, not discussed here.

Francewhoa’s picture

Thanks for your comment David :) If after reviewing all the information currently available the Drupal security team feels it's not a security-related issue at all, then speaking for myself I really trust the security team skills and judgment on that :)

About the alleged +FollowSymlinks weakness which leads to security exploits, I'll let other contributors reply to your above comment #70 if they want to. As I don't have an extensive security experience. So I prefer trusting experienced folks like you guys on that. I'm happy to contribute quality assurance, documentation, and agile project management services.

I'll update this public issue description shortly to reflect the Drupal security team current assessment

Please let us know if you have any other questions or need anything else

Francewhoa’s picture

Issue summary: View changes

Done. I updated this issue description to reflect the Drupal security team current assessment. And added a summary section title "Alleged +FollowSymlinks weakness which leads to security exploits"

Francewhoa’s picture

Issue summary: View changes

Clarified issue summary

David_Rothstein’s picture

Thanks Francewhoa :)

damien_vancouver’s picture

Hello, having just spent the day cleaning up after someone hacked the database credentials on a server using the FollowSymLinks vulnerability, I can explain how it is in fact a huge security risk in some environments.

In #70 above, David_Rothstein asked:

On a shared hosting setup that is vulnerable to this issue, an attacker who already had control of someone's account could just add +FollowSymlinks to an .htaccess file on their own; it doesn't matter if Drupal does it for them by default or not. That is why hosts are blocking this at the server level, right?

That is incorrect actually, a properly secured shared hosting environment would grant +SymLinksIfOwnerMatch and then use the AllowOverride directive with an options list, so as to prevent individual sites from overriding FollowSymLinks.

Virtualmin since 2013 has wanted to set up new virtual hosts like so:

Options -Indexes +IncludesNOEXEC +SymLinksIfOwnerMatch +ExecCGI
AllowOverride All Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch

Note that the list of allowed overrideable options is provided, and FollowSymLinks is not one of them. On my server that was hacked, I had re-added FollowSymLinks to that AllowOverride options list, specifically so that Drupal would not 500 error every time a customer attempted to do a core upgrade or an initial install. As Drupal stands now, this .htaccess setting is incompatible with these more secure setups.

The problem with +FollowSymLinks

The problem occurs if the parent user of the Apache server is in the UNIX group of web users. I use FastCGI so that each website's PHP processes runs as a separate user, and filesystem permissions are hardened so that users cannot explore other users files. I tested that this was the case using PHP test scripts, and sure enough there was no ability as PHP to explore other users' files.

Unfortunately though, symbolic links are processed using Apache's user credentials, not the user and group PHP runs as.

If you don't use +SymLinksIfOwnerMatch (which ensures by comparing the directories that the two symlinks are not between users) then Apache doesn't check any permissions and just serves up the file, even if it's securely owned by another user and not readable via the shell.

The hacker can:
- obtain usernames from the host system via a variety of methods
- construct symbolic links to potential settings.php (or wp-config.php) file locations using a .txt extension for the filename. e.g. guessing the path to username1's settings.php:
ln -s /home/username1/public_html/sites/default.settings.php username1-settings.txt
- Override a few things in .htaccess (I'm not sure which of these exactly was my downfall but one of them did the trick)

Options Indexes FollowSymLinks 
ForceType text/plain 
AddType text/plain .php  
AddType text/plain .html 
AddType text/html .shtml 
AddType txt .php 
AddHandler server-parsed .php 
AddHandler txt .php 

- Visit the URL of the symbolic link over HTTP, which causes Apache to incorrectly follow the link and return the contents of the private settings.php file belonging to another user as text.

At that point they have the database credentials for the other site (if not its URL or ability to list its files), but that's enough to connect via MySQL and proceed via the database to inject malware and own user accounts, then they can move onto the next site, and the next etc.

Mitigation

I found two ways only one way to mitigate the problem:

Edit: This #1 does not work, it breaks PHP for the other site as Apache is unable to get in there at all.
#1 is to remove the Apache user from the website owner's group. This configuration is not required in a FastCGI/SuExec setup as PHP runs as the files' owner alread. Putting www-data in the user's group is a throwback to the days of mod_php when that was required. I didn't think there was any harm in that as PHP was the correct user, but I was wrong. Nonetheless it's a default configuration in a lot of places.

#2 Is to modify the Drupal install's .htaccess file to use Options +SymLinksIfOwnerMatch, and then I was able to restore the default secure Virtualmin setting in the VirtualHost directive where +FollowSymLinks cannot be turned back on.

Conclusion: It's a security issue

I am of course biased after spending the day cleaning up after one of these hacks. It was not at all pleasant and if I hadn't had to downgrade Virtualmin's default security because I didn't want to fight with every core update, it would not have happened. I should have sucked up the extra work, but the real solution is to fix the dangerous configuration in Drupal's .htaccess. I now have to hack every site's .htaccess and continue doing so each time core is updated in order to remain secure.

In light of all that, I believe this is actually a Drupal security issue. While not a vulnerability, it is an extra mitigation step that could be taken where the security benefits outweigh the benefit of not making any changes. Especially given widespread documentation as referenced in this issue already and its use in other products for years now without problems. SymLinksIfOwnerMatch support goes back at least as far as Apache 2.2 which is far enough back to cover Debian 6 LTS and Ubuntu 12.04 LTS, RHEL5 and probably any other long term support old versions of Apache out there. The change can be documented in a comment in .htaccess, along with what to change it to if you need to symlink between directories with different owners.

I can create a patch for current HEAD, but what is one to do regarding writing tests for a .htaccess change?
Setting up a test environment for the vulnerability requires Apache configuration changes before and after.

It would be really nice to get this committed and backported so that next feature release users were protected from this potential vulnerability. It would would also reduce new user frustration by avoiding HTTP 500 errors by novice users. Those users won't realize their host is using the secure configuration, and that's why Drupal isn't loading after they first try to bring up a page, and that hacking .htaccess is the solution. All these issues could be avoided by a simple one line change.

damien_vancouver’s picture

If anyone remains doubtful that this is a real security issue, I recommend they watch this video I just found, which was posted by the hacker I just cleaned up after.

I've linked to the spot (5:10) where the attacker uses the FollowSymLink weakness. The links must be accessed over HTTP so Apache provides access to another user's files. One site after another is owned on the server in the minute and a half of that video. For each site, once the configuration file is shown successfully in a browser tab, the site URL is found on Google and then the finally the symbolic link path is pasted into the PHP hack Script which uses fetches the config again over HTTP and connects to MySQL to change the password.

https://www.youtube.com/watch?v=QLVwSQIyFeQ&t=5m10s see 5:10 through 7:00 or so. There's no audio other than the music, so you can mute that.

ciss’s picture

ciss’s picture

Moved the most recent 8.x and 7.x patch to the top of the files list.

David_Rothstein’s picture

In #70 above, David_Rothstein asked:

On a shared hosting setup that is vulnerable to this issue, an attacker who already had control of someone's account could just add +FollowSymlinks to an .htaccess file on their own; it doesn't matter if Drupal does it for them by default or not. That is why hosts are blocking this at the server level, right?

That is incorrect actually, a properly secured shared hosting environment would grant +SymLinksIfOwnerMatch and then use the AllowOverride directive with an options list, so as to prevent individual sites from overriding FollowSymLinks.

That is exactly what I meant by "hosts are blocking this at the server level"....

For this issue, the bottom line (as far as I understand it) is:

  1. If the server is configured securely, there is nothing Drupal's .htaccess is currently doing (or can do) to make it less secure.
  2. If the server is configured insecurely, there is nothing Drupal's .htaccess can do to fix it.

Hence, not a security issue from Drupal's point of view. If people are making their shared hosting servers less secure in order to avoid breaking Drupal (rather than changing Drupal's .htaccess to work with the server) that is unfortunate, but still not a security issue with Drupal :)

We should still try to fix this because of the HTTP 500 errors, of course. It is too bad that Apache doesn't provide a way for an .htaccess file to programmatically check if FollowSymlinks overrides are allowed before using them (at least I don't think it does?) since that would make this easier to fix...

damien_vancouver’s picture

2. If the server is configured insecurely, there is nothing Drupal's .htaccess can do to fix it.

However if Drupal's .htaccess used +SymLinksIfOwnerMatch instead of +FollowSymLinks, the server would not necessarily need to be configured insecurely. Hacking every single core of every Drupal on a server and re-hacking it every update is an inconvenient waste of time that is multiplied linearly by every sysadmin who has to do that extra work. When you consider the effort involved in explaining that process to less experienced developers who have no idea what is going on, that is a whole lot of effort and core hacking that could be totally avoided by just switching to the more secure .htaccess option.

I don't really care what we call this, security issue or something that causes 500 errors. I'm willing to pretend that it's not what it glaringly is (which is a weak configuration option that forces admins running shared servers to reduce their server security or otherwise face a whole pile of extra or impossible work herding cats with the people using the server). I will prepare a patch, again, if it will get anywhere. Are we all satisfied at what it is and isn't called and categorized at, and will a newly rolled patch get anywhere? Or will we discuss what it is and isn't to death in a Kafkaesque nightmare while years more pass?

David_Rothstein’s picture

Editing .htaccess is not hacking core - it's a normal thing that a lot of site admins have to do. Drupal cannot ship with an .htaccess file that works for everyone.

The last patch review was in #52 - as far as I can see the main reason this issue hasn't moved since then is just that no one has done anything in response to that?

I think I'd probably vote for backporting this, although it would definitely need a very prominent mention in the release notes. Ultimately it seems like more people out there need SymLinksIfOwnerMatch than need FollowSymLinks (and the people who need FollowSymLinks are probably, on average, more likely to know how to edit .htaccess themselves) so it's an overall win. But there is no question this could really break some people's sites if they just update to a new version of core and blindly take the new .htaccess along with it. One thing I'm particularly concerned about is that this is the kind of thing that could work fine on someone's staging environment but then break horribly in production (e.g. if the files directory is a symlink from the user's account to a webserver-owned shared directory?)... So it needs some discussion.

damien_vancouver’s picture

FileSize
5.15 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,101 pass(es). View

Hmm, semantics I guess. I consider any changes to the Drupal codebase that aren't in a custom module or theme, and require re-patching each time the core is updated, to be hacking the core files. I have hosted lots of Drupal sites since 4.7 without ever having to hack .htaccess. Instead I add extra configuration to the Apache Virtualhost configuration instead, where it is not clobbered by core updates. The default rules work fine for me as a base for all this and have never required file changes (other than this particular problem). It's a very safe way to do things I hope to keep using.

Regarding a backport, I hear what you are saying about old sites. The change may break existing sites out of the blue, so a backport would likely be best as an optional patch that could be included in a make file. Then it could at least be easily automated into a workflow for those who desire it without the risk of suddenly breaking existing sites. That being said, it'd be great to get this change into Drupal 8 somehow without having to wait for 9. Is that possible this close to release, since we haven't released yet?

I've attached a newly rolled patch for Drupal 8 HEAD with a few lines of comments added to the .htaccess files. I tried to match the tone of the other comments in .htaccess with the new text and include the relevant info.

damien_vancouver’s picture

Status: Needs work » Needs review

The last submitted patch, 42: drupal-7.x-symlinksifownermatch-1269780-42.patch, failed testing.

jonhattan’s picture

re #81:

One thing I'm particularly concerned about is that this is the kind of thing that could work fine on someone's staging environment but then break horribly in production (e.g. if the files directory is a symlink from the user's account to a webserver-owned shared directory?)... So it needs some discussion.

Indeed, if the owner of the symlink is not the same as the owner of the destination (as stated in the patch), files won't be served, and it's relatively hard to understand what's going on, if you haven't read the release notes.

I suffered this myself when moving to production a a client's site that had applied the patch here in order to run the staging site under virtualmin.

For the record, a symlinked files folder with chown SYMLINKOWNER:www-data and chmod 2775 (s bit) is fine.

damien_vancouver’s picture

FileSize
6.5 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 98,165 pass(es). View

I have improved the help text to include performance considerations of having +SymLinksIfOwnerMatch enabled, as documented here:
http://httpd.apache.org/docs/current/misc/perf-tuning.html#symlinks

I considered even spelling out their recommendation of setting +FollowSymLinks on but decided that was too much info.

Here is the text I came up with, so you don't have to dig in the patch:

# There are two options available for following symbolic links,
# SymLinksIfOwnerMatch and FollowSymLinks.
#
# Options +SymLinksIfOwnerMatch is the default and will only follow symbolic
# links for which the target file or directory is owned by the same user id
# as the link.  This check provides important security on shared servers, and
# without it users may be able to access private files belonging to other users.
# However, there is a performance cost for checking the security of each link.
# See the Apache Documentation on this subject for a recommended configuration:
# http://httpd.apache.org/docs/current/misc/perf-tuning.html#symlinks
Options +SymLinksIfOwnerMatch

# Options +FollowSymLinks will follow all symbolic links regardless of owner.
# Use this option instead if you need to link to files owned by another user,
# or if you are on a private server and are tuning for maximum performance.
# Some web hosts do not allow FollowSymLinks and enabling the following line
# may cause all site pages to return a HTTP 500 Error.
#Options +FollowSymLinks

Regardless of which default we end up choosing, this writeup in .htaccess should provide all the info someone needs to make an informed decision. If we default to +SymLinksIfOwnerMatch then it's defaulting to the more secure option that won't 500.

Then for the backport, we could just add these extra comments, leaving the default to +FollowSymLinks as to not break any sites or make unexpected changes.

New patch with these changes attached, and I've removed the older patches including the D7 one that was failing testing, so hopefully it gets a clean pass.

ciss’s picture

Please do not hide the 7.x patch, and keep it within the five top files.

masipila’s picture

Hi,

Just to make sure that the .htaccess for public, private & temp files are covered with the patch...

I checked the patch in #86 and I gues that core/lib/Drupal/Component/PhpStorage/FileStorage.php might be doing .htaccess magic for these directories but is this assumption correct? My shared host does not allow FollowSymlinks so I have found that I have to edit the .htaccess in these directories in addition to Drupal root to avoid HTTP 500 errors.

Cheers,
Markus

damien_vancouver’s picture

@masipila: I checked the patch in #86 and I gues that core/lib/Drupal/Component/PhpStorage/FileStorage.php might be doing .htaccess magic for these directories but is this assumption correct?

That is correct, the changes to FileStorage.php are in public static function htaccess_lines, which is used to create the .htaccess file for a public or a private files directory.

There is one other Options +FollowSymLinks in the codebase, in core/vendor/.htaccess, and that is also patched.

@ciss: Please do not hide the 7.x patch, and keep it within the five top files.

OK, does the failing 7.x patch need to be updated as well with the latest changes, or does that happen after the 8.x is fully worked out?

Patch in #86 has the extra documentation requested after the old #52. Is anything else needed on the patches to move this issue forward?

David_Rothstein’s picture

Looks good to me overall.

  1. # links for which the target file or directory is owned by the same user id
    

    "id" should be "ID"

  2. # as the link.  This check provides important security on shared servers, and
    # without it users may be able to access private files belonging to other users.
    

    I think this is somewhat confusing; it's going to scare people into not editing this .htaccess setting, but there's no security risk to them in editing it. (I mean, the only risk would be if you're in the habit of leaving symlinks around your own docroot that point to sensitive files you don't want visitors to see, but you should never do something like that either way :) I think we want to emphasize instead that on some servers this is the only possible one to use.

  3. # See the Apache Documentation on this subject for a recommended configuration:
    

    "Documentation" should be "documentation"

So putting it together, plus a couple other small edits to the last paragraph, I'd suggest this:

# There are two options available for following symbolic links,
# SymLinksIfOwnerMatch and FollowSymLinks.
#
# Options +SymLinksIfOwnerMatch is the default and will only follow symbolic
# links for which the target file or directory is owned by the same user ID as
# the link. On some shared hosting environments, this is the only allowed
# option, since it prevents users from trying to access private files belonging
# to other users on the system. However, there is a performance cost for
# checking the owner of each link. See the Apache documentation on this subject
# for a recommended configuration:
# http://httpd.apache.org/docs/current/misc/perf-tuning.html#symlinks
Options +SymLinksIfOwnerMatch

# Options +FollowSymLinks will follow all symbolic links regardless of owner.
# Use this option instead if you need to link to files owned by another user,
# or if you are on a private server and are tuning for maximum performance.
# As mentioned above, some shared hosting environments do not allow
# FollowSymLinks, so attempting to enable it in those environments may cause
# all site pages to return a HTTP 500 error. To enable FollowSymLinks,
# uncomment the following line (and comment out the SymLinksIfOwnerMatch line
# above):
# Options +FollowSymLinks

How does that look?

damien_vancouver’s picture

FileSize
7.09 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] Unable to apply patch drupal-use_symlinksifownermatch_instead_of_followsymlinks-1269780-91.patch. Unable to apply patch. See the log in the details link for more information. View

Hi David, thanks for the revisions, all of which look good to me.

Here's a new patch with the changes from #90, rolled against current 8.0.x.

Francewhoa’s picture

FileSize
11.17 KB

Thanks damien_vancouver and all contributors for the patch in #91 :)

We tested it. Here are the first results. The good news are the patch was successfully applied on the current Drupal 8.0.0-beta14. The patch did fix the first error message during a fresh Drupal 8 installation. The help text is useful and clear. It's easy to customize the settings.

But the server we tested on doesn't have the minimum PHP requirement for Drupal 8. Drupal 8 PHP requirement have recently been increase to at least PHP 5.5.9. We tested on Debian Wheezy 7. Which comes with PHP version 5.4.41.

Drupal 8 installer does start but then returns the following message:

Your PHP installation is too old. Drupal requires at least PHP 5.5.9. See the system requirements page for more information.

At least we know that the first part of the installation with +SymLinksIfOwnerMatch worked. Yay :) Otherwise the users would not see that message.

That message is an issue with the new PHP requirement. Not the patch. So we will try to resume the testing on Debian Jessie 8. Which comes with PHP 5.6.9. That might take a while though. As we need to setup & configure a new server.

Tested with

  • Debian: Wheezy 7 at 64 bit
  • Webmin: 1.760
  • Virtualmin:  4.18.gpl
  • PHP: 5.4.41
  • Drupal: 8.0.0-beta14
  • Apache: 2.2.22

Any volunteers to test that patch in #91 on other platforms and systems? Drupal 8 is not yet release. But it might be soon. It will be release shortly after those few critical issues are resolved. Would be great if that patch for +SymLinksIfOwnerMatch could be included in the first Drupal 8 recommended release.

amateescu’s picture

The documentation looks very good to me, but do we really have to repeat it everywhere?

Steve Hanson’s picture

Have tested this in our current D8 test hosting environment --- This is CentOS 6.7 with PHP5.5.21 installed, and a database server running MariaDB 5.5.41 (on CentOS 7)

Beta 14 with the patch #91 put in installs perfectly (our server configuration requires SymLinksifOwnerMatch).

Francewhoa’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for testing Steve :)

We also confirm the patch in #91 works. Yayaya :)

That server requires +SymLinksifOwnerMatch

Using

  • Drupal: 8.0.0-beta14
  • Debian: Jessie 8 at 64 bit
  • PHP: 5.6.9
  • Apache: 2.4.10
  • Webmin: 1.760
  • Virtualmin: 4.18.gpl
  • Kernel Linux: 4.1.0-x86_64

Francewhoa, note to myself: During Drupal 8 installation on the "Requirements review" page. Drupal returns the following message "Clean URLs. Disabled." We assumes this is cause by a server miss configuration. Not by the patch. The workaround is to scroll all the way down the "Requirements review" page. Then click on "continue anyway." link.

Status: Reviewed & tested by the community » Needs work
alexpott’s picture

  1. +++ b/core/lib/Drupal/Component/PhpStorage/FileStorage.php
    @@ -80,7 +80,29 @@ public static function htaccessLines($private = TRUE) {
    +
    +# There are two options available for following symbolic links,
    +# SymLinksIfOwnerMatch and FollowSymLinks.
    +#
    +# Options +SymLinksIfOwnerMatch is the default and will only follow symbolic
    +# links for which the target file or directory is owned by the same user ID as
    +# the link. On some shared hosting environments, this is the only allowed
    +# option, since it prevents users from trying to access private files belonging
    +# to other users on the system. However, there is a performance cost for
    +# checking the owner of each link. See the Apache documentation on this subject
    +# for a recommended configuration:
    +# http://httpd.apache.org/docs/current/misc/perf-tuning.html#symlinks
    +Options +SymLinksIfOwnerMatch
    +
    +# Options +FollowSymLinks will follow all symbolic links regardless of owner.
    +# Use this option instead if you need to link to files owned by another user,
    +# or if you are on a private server and are tuning for maximum performance.
    +# As mentioned above, some shared hosting environments do not allow
    +# FollowSymLinks, so attempting to enable it in those environments may cause
    +# all site pages to return a HTTP 500 error. To enable FollowSymLinks,
    +# uncomment the following line (and comment out the SymLinksIfOwnerMatch line
    +# above):
    +# Options +FollowSymLinks
    

    Isn't this change a problem? Because unlike the root .htaccess users can't edit this version - it is automatically created by drupal in locations as an when it needs to.

  2. +++ b/core/vendor/.htaccess
    @@ -9,7 +9,29 @@
     # Turn off all options we don't need.
     Options None
    -Options +FollowSymLinks
    +
    +# There are two options available for following symbolic links,
    +# SymLinksIfOwnerMatch and FollowSymLinks.
    +#
    +# Options +SymLinksIfOwnerMatch is the default and will only follow symbolic
    +# links for which the target file or directory is owned by the same user ID as
    +# the link. On some shared hosting environments, this is the only allowed
    +# option, since it prevents users from trying to access private files belonging
    +# to other users on the system. However, there is a performance cost for
    +# checking the owner of each link. See the Apache documentation on this subject
    +# for a recommended configuration:
    +# http://httpd.apache.org/docs/current/misc/perf-tuning.html#symlinks
    +Options +SymLinksIfOwnerMatch
    +
    +# Options +FollowSymLinks will follow all symbolic links regardless of owner.
    +# Use this option instead if you need to link to files owned by another user,
    +# or if you are on a private server and are tuning for maximum performance.
    +# As mentioned above, some shared hosting environments do not allow
    +# FollowSymLinks, so attempting to enable it in those environments may cause
    +# all site pages to return a HTTP 500 error. To enable FollowSymLinks,
    +# uncomment the following line (and comment out the SymLinksIfOwnerMatch line
    +# above):
    +# Options +FollowSymLinks
    

    Rather than doing this here - how about we remove the Options settings from here, since doesn't it inherit from the .htaccess in the root? Then the user would only have one place to change. In fact is this not true of the .htaccess files written by core? So perhaps this more resolve problem in FileStorage?

Francewhoa’s picture

Hi damien_vancouver :) Do you want to re-roll your patch in comment #91 ? Your patch was successful and valid then. But maybe the code base has changed in the meantime.

Hi all :) If you're interested and available to re-roll that patch but you're a newcomer to re-rolling a patch you might be interested in that page https://www.drupal.org/patch/reroll

Hi alexpott :) Thanks for your suggestion. Users would only have one place to change does seem easier. I'll let interested Developers reply to your suggestion. I'm not a Dev. But I'm happy to contribute testing, quality assurance, documentation, and agile project management services if needed.

damien_vancouver’s picture

Status: Needs work » Needs review
FileSize
5.86 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal-use_symlinksifownermatch_instead_of_followsymlinks-1269780-99.patch. Unable to apply patch. See the log in the details link for more information. View

@alexpott, referring to the .htaccess lines written in public static function htaccessLines()

1. Isn't this change a problem? Because unlike the root .htaccess users can't edit this version - it is automatically created by drupal in locations as an when it needs to.

It is automatically created yes, but I don't see why users might not need to edit it after that happens, perhaps a while down the road when they need to relocate some or all of the directory contents.

re: the changes in core/vendor/.htaccess,

2. Rather than doing this here - how about we remove the Options settings from here, since doesn't it inherit from the .htaccess in the root? Then the user would only have one place to change. In fact is this not true of the .htaccess files written by core? So perhaps this more resolve problem in FileStorage?

It does inherit the +FollowSymLinks or +SymLinksIfOwnerMatch from the root, but the line before that is an Options None to turn off everything but the symbolic links, and that won't be inherited from the top level (as it's not present in that .htaccess file) . The idea is to disable all options and PHP running in the core/vendor folder. I wasn't sure if we needed the symlink directive in that folder but a quick find -type l shows that we do, there is a symlink in there: core/vendor/bin/phpunit -> ../phpunit/phpunit/phpunit. But there should never be a need for users to edit that directive, since all the files in the core/vendor should belong to the same user and files in that directory are not user-servicable. I've therefore removed the instructions from that spot in this latest patch.

@amateescu,

The documentation looks very good to me, but do we really have to repeat it everywhere?

I think it should be everywhere that the symlinks directive is specified and people might need to edit it (ie. in the root and in any private files directories that Drupal creates and puts .htaccess in). As per the above comment it's removed now from core/vendor/.htaccess file.

@Francewhoa, I've re-rolled #91's patch against current core.

Status: Needs review » Needs work

Francewhoa’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
29.92 KB
52.43 KB
153.33 KB

We are confirming the patch in #99 works. Ya :)

That server requires +SymLinksifOwnerMatch
No change to the directives in the default Apache ".conf" file: AllowOverride All Options=ExecCGI,Includes,IncludesNOEXEC,Indexes,MultiViews,SymLinksIfOwnerMatch

Using

  • Drupal: 8.0.0-beta14 (2015-Aug-03)
  • Debian: Jessie 8 at 64 bit
  • PHP: 5.6.9
  • Apache: 2.4.10
  • Kernel Linux: 4.1.0-x86_64
  • Webmin: 1.760
  • Virtualmin: 4.18.gpl

That patch might make it in time for Drupal 8 first recommended release. Currently only ~7 critical issues to resolve.

alexpott’s picture

Status: Reviewed & tested by the community » Needs review

l don't think the answer to #97.1 is okay. I mean what happens is a module creates directories on the fly as a result of user input and adds the .htaccess?

damien_vancouver’s picture

l don't think the answer to #97.1 is okay. I mean what happens is a module creates directories on the fly as a result of user input and adds the .htaccess?

In that case I don't think it's clear to me what the question for #97.1 is?

Is the problem (a) Drupal creating .htaccess with +FollowSymLinksIfOwnerMatch rather than +FollowSymLinks in directories created programatically? Or is the problem (b) the extra documentation and it taking up so much room? Or is it (c) that "users can't edit these .htaccess files"? or something else entirely?

If (a), why is it a problem for programatically created directories and how are they different from the main files directory or any other files directories Drupal is using? Any folder that might be in the public webroot and thus may expose a site to the +FollowSymLinks attack should have the secure directive, or it may cause 500 errors. Do we have some documented expectation that code-created folders will have +FollowSymLinks turned on? If the folder is created outside of the web root and never served by apache then the .htaccess directives are irrelevant anyway.

if (b), we could cut the documentation if it offends people for some reason or we're short on bytes... Perhaps we need different text for this file vs the main file, or just somehting more concise that references the main file as a source of help?

if (c), I don't really understand the objection properly... why couldn't someone edit the .htaccess file that was programmatically created by some module in some folder? Server Admins edit .htaccess files all the time.

alexpott’s picture

It is (a) - if a site needs +FollowSymLinks then if we make this change it'll have to update every automatic htaccess created. The site owner might not know when this occurs. And then at some point someone creates a symlink inside the folder and they have to spend ages debugging why it's not working.

damien_vancouver’s picture

OK thanks, that makes sense. But... if a hosting environment only supports +SymLinksIfOwnerMatch, then isn't it actually the opposite of what you're concerned of: each Drupal-created .htaccess file will need to be located and edited to avoid 500 errors when content is served from it... by anyone on shared hosting (which is a lot of people).

To further explain what I mean...

if a site needs +FollowSymLinks then if we make this change it'll have to update every automatic htaccess created.

The only use case I can think of where someone would need to switch the +SymLinksIfOwnerMatch option to +FollowSymLinks would be if they needed to split up content in an existing folder (perhaps because the filesystem the files are on is full), and then only if that other folder belonged to a different user ID than the source folder. Otherwise +SymLinksIfOwnerMatch will work fine in all scenarios.

Versus the opposite - using a shared hosting environment that for security reasons doesn't allow +FollowSymLinks, which is a common scenario affecting many thousands of site builders who have chosen inexpensive shared hosting. In that case then all the shared hosting users need to find and patch every single new .htaccess file that is created. This could be a task too daunting for novice users (likely to purchase cheap shared hosting).

Using the secure directive in all auto-created folders would avoid the 500 errors and the need to edit new folders entirely, it would default to working. The only drawback would be that you can't by default symbolic link to files owned by a different user ID than the symbolic link. (Which is something that cannot be done by default by PHP runtime, so that means an administrator would have had to set it up specifically, in which case they could set +FollowSymLinks at the same time). I think we should use a working, secure configuration by default if possible, given that the drawback is minor and an edge case.

Francewhoa’s picture

I agree with damien_vancouver point in #106

About the hedge case of users who would need +FollowSymLinks, I would add that another option is they could edit their Apache directive to allow +SymLinksIfOwnerMatch. It's just one edit per server. As a result all the .htaccess files automatically created by Drupal 8 would work as is with +SymLinksIfOwnerMatch in all scenarios. Without having to manually edit each .htaccess file. Yes it's additional work. But there would be a cost attach to holding to an older standard +FollowSymLinks. Maybe the past posts were not clear about that?

I suggest to consider embracing progress by shifting to +SymLinksIfOwnerMatch. As you know during the last years an increasing number of operating systems, platforms, and hosts have shifted from +FollowSymLinks to +SymLinksIfOwnerMatch. Details and motivations in above comments and links.

Another option could be to hold on to the decreasingly popular +FollowSymLinks. But there would be a few risks and challenges associated with that. Related to that Drupal as an history to not commit to backward compatibility. I agree with that. There are many benefits. Such as increase security, easier to embrace new standards, increase performance, and not having to drag legacy baggage. Related post from Dries and motivations at http://buytaert.net/backward-compatibility

In other words, both +SymLinksIfOwnerMatch and +FollowSymLinks would work for most scenarios. Users interested to embrace progress would have an automated path, less manual work. And users interested to hold to older standard could still do that but in some cases they would have more manual work to do.

damien_vancouver’s picture

About the hedge case of users who would need +FollowSymLinks, I would add that another option is they could edit their Apache directive to allow +SymLinksIfOwnerMatch. It's just one edit per server.

It's actually very unlikely they would need to edit anything at all to enable +SymLinksIfOwnerMatch. There is no reason for anyone to disable that yet leave +FollowSymLinks. By doing that you'd be disabling the secure option and leaving the insecure option.

Editing the .htaccess files is a valid concern, however the only reason one would need to edit the .htaccess files and change the directive is to support the scenario where one is symbolic linking to a directory/file owned by a different uid than the symbolic link itself. This is a very rare scenario for the auto-created folders we're worried about in Q#97.1. Otherwise the change will work as-is, +SymLinksIfOwnerMatch will follow symlinks in all normal circumstances.

For example, let's say I write a module foobar.module and it creates an automatic directory somewhere in the web-accessible document root. This directory and the .htaccess file inside it will be owned by whatever uid that PHP is running as. Any files created by Drupal during runtime will also belong to that same uid. Any 3rd party libraries fetched and unpacked by Drupal will similarly belong to the same uid, including any symbolic links created. Therefore +SymLinksIfOwnerMatch will behave identically to +FollowSymLinks for any content inside these directories and there would never be a need to edit the .htaccess in normal circumstances.

Using a symbolic link to reference to an entire codebase (or files directory) owned by another user is a common way of securing Drupal and preventing writing by the PHP process to the codebase. Perhaps that is why there is concern about editing .htaccess files? However the 97.1 change is not that scenario and this change does not prevent or make more work for that scenario. It merely hardens the security inside these auto created directories, that can only contain content owned by the same uid anyway.

It could be that I am unaware of another, common scenario where this change would cause editing to need to happen, in which case could someone please provide any such scenario(s) so that we can weigh the pros and cons?

Francewhoa’s picture

Hi damien_vancouver :)

Interesting about the uid. I didn't know that.

It's actually very unlikely they would need to edit anything at all to enable +SymLinksIfOwnerMatch. There is no reason for anyone to disable that yet leave +FollowSymLinks. By doing that you'd be disabling the secure option and leaving the insecure option.

In our case we would usually not do that. Because many SysAdmins also confirmed and recommended to not change the Apache default directive. For the same security reason you mentioned. But in some rare cases when all other more secured options are not possible then we manually edited the Apache directive. Usually those rare cases happen with some rescue projects. When either the files and or Apache users permissions were miss-configured by previous service provider(s), plus the client doesn't have budget currently available to fix those. Assumes client made an informed decision about editing the Apache directive and its associated risk. But all of the above was for Drupal 7 and rare cases. I understand now that with that patch for Drupal 8 users would not need to edit anything at all. Easy is good.

I'm unaware of other scenario with that Drupal 8 patch were users would need to manually edit their Apache directive or .htaccess files

damien_vancouver’s picture

So where are we at with this issue now? By far the best time to make this change is along with a major version release, then there is no changed behavior for any existing sites. Is there any hope of making that window for Drupal 8?

Are there any other objections besides that it might break sites relying on symbolic links to files owned by another user, which is a non-issue if we change it for D8 before the final release ?

Are there any thoughts or pushback re: my clarifications in #106? As I explained there, not making this change results in a whole bunch of .htaccess patching by anyone in a secure shared environment. Whereas if we make the change it will just work without 500 errors and troublesome debugging. If we make the change now, for D8, then at least it will not be an entire major version cycle until we can safely make the change again without worrying about changing behaviour on existing sites relying on insecure symlinks.

Is there any hope even of fixing this in time for Drupal 8, or are we going to let this issue become seven years old before we get around to fixing it? Which would be a shame since it appears we have consensus that the change is required.

Francewhoa’s picture

Status: Needs review » Reviewed & tested by the community

Hi damien_vancouver :) Thanks for your comment

I'm assuming the next action is for someone from the drupal 8 core team to review the patch in above comment #99 and decide if it's committed or not. Any volunteer?

Hi all :) That patch has been reviewed and tested by other users. They confirmed that patch works. Find above comments #102 by Francewhoa and #105 by alexpott.

For those not familiar with the patches review and commit workflow. Patches need to be reviewed by someone other than the original author before being declared Reviewed and tested by the community. In this case someone else than damien_vancouver.
Documentation at:

Setting ticket to Reviewed and tested by the community. As I'm assuming others don't have questions or concerns. Feel free to set to "needs review" or "needs work" if any.

Francewhoa’s picture

Issue summary: View changes

Francois, note to myself: Added "Next action" section to ticket description section

Francewhoa’s picture

axel.rutz’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.52 KB

The vendor dir moved in #2380389: Use a single vendor directory in the root so we need a re-roll. Please re-test.

damien_vancouver’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +rc target triage

I tested #115 with a clean install of latest 8.0.x and a VirtualHost that doesn't allow +FollowSymLinks.

Before the patch, 500 error loading the installer page. After the patch the installer loads and Drupal operates properly (loading resources from vendor/)

I performed a fresh install and sites/default/files/.htaccess is created with the working +SymLinksIfOwnerMatch, along with the instructions on changing it if required. Uploaded files (site logo) work fine from that directory, no 500 errors.

Since the vendor folder was the only change, setting back to RTBC, and also adding "rc target triage" tag in hopes it can make it in before release.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

This at least needs a change record.

Also I think the issue underestimates how many people may have created symlinks with random owners in their files directories - I'm pretty sure I have in the past.

alexpott’s picture

I think we need to ensure that the site owner can set something in settings.php so that any new .htaccess files are written with the correct setting for the site. And use this in core/lib/Drupal/Component/PhpStorage/FileStorage.php to write the correct htaccess for the site.

axel.rutz’s picture

> #117: This at least needs a change record.

Yes indeed.

> #118: I think we need to ensure that the site owner can set something in settings.php so that any new .htaccess files are written with the correct setting for the site. And use this in core/lib/Drupal/Component/PhpStorage/FileStorage.php to write the correct htaccess for the site.

Yes but that's not the scope of this issue and imho would bikeshed this to oblivion. In this issue we replace an outdated default with a now more sensible one (knowing that any default will not fit for some people, but we try to minimize that).

To make that .htaccess line configurable, we should open a followup issue. And if we start making this line configurable, we surely will grow that into a .htaccess configurator. I support that, but imho that's outside of the scope of this issue.

alexpott’s picture

@axel.rutz I disagree we've got a situation where a percentage of users won't be able to use the current .htaccess file - and we're changing it so that another percentage won't be able to use the new .htaccess file. I see making this line configurable as part of the most optimal solution. Especially as we're in RC. In fact as we're in RC the default probably should be unchanged.

damien_vancouver’s picture

@axel.rutz I disagree we've got a situation where a percentage of users won't be able to use the current .htaccess file - and we're changing it so that another percentage won't be able to use the new .htaccess file.

I don't see how this can be true if we make this change before release. We aren't talking about changing it for existing Drupal 7 or 6 sites here, (though maybe a backport of some documentation into .htaccess would be in order, but that comes later). It's not going to break anybody's symbolic links to files/directories owned by other users - because those have not yet been created yet because D8 has not yet been released! Until Drupal 8 is released, no such sites can exist. And even so, that scenario is the exception not the rule. It requires custom actions using administrator permissions to implement. There is no way from within Drupal itself to create a folder owned by another user than who PHP runs as, in order to even get into the situation where it would break. So realistically, setting aside old Drupal 6/7 sites, we are talking about what percentage here? 0.1?% 0.0001?% pretty close to 0%

Versus the other percentage: anybody trying to install Drupal on shared hosting for the rest of time until Drupal 9 comes out and it's safe to make this change without breakage. If there are millions of Drupal sites out there and we can then expect millions of Drupal 8 sites, then it's fair to guesstimate that half of these sites will be on shared hosting (because it is the least expensive option). If even a quarter of them are on shared hosting, that's 25% of users (a quarter million sites) who are unable to even install because of a 500 error, and who will get a 500 error every core update if they don't manually patch .htaccess.

0.00001% vs. 25-50%? This change should be a no brainer from that perspective. Just think of how all those 500 errors will reflect poorly on Drupal to new users trying to install but stuck and having to edit files and re-edit them each update. It's not very user friendly to the beginners.

Besides the numbers, then there is also the fact we are replacing an insecure default directive with a secure one. If we are at a major version change, then this is the ideal time to replace the insecure directive with the secure version, as both work fine for standard Drupal installs. +FollowSymLinks is so dangerous on any server with more than one user on it, that the only excuse for shipping it in our code would be that we absolutely need it for Drupal to work. We do not, +SymLinksIfOwnerMatch works fine for running Drupal, is rarely restricted from AllowOverride because it's not a security hazard like +FollowSymLinks, and allows symlinks to work as expected as long as an admin user has not come along and altered the filesystem configuration in the OS.

So, can we please set aside this fear of breaking sites, because there are no Drupal 8 sites to break yet? We should consider this change based on its merits (removing 500 errors from the install / update process, and a more secure default configuration) and not based on hypothetical situations of broken configurations that don't exist yet.

Agreed on the need for a change record. I will look into how to do that and roll another patch for review.

effulgentsia’s picture

Until Drupal 8 is released, no such sites can exist.

https://www.drupal.org/project/usage/drupal shows that there are at least 44 thousand Drupal 8 sites. Most are probably development/testing ones, but some are real. It's very helpful to have some number of real sites running on release candidates: that helps find bugs that then lets us have a more solid 8.0.0 release. And one way that we encourage such early adoption is to do our best to not break those sites unnecessarily.

I haven't read up on the details of this issue, but if it's possible to solve it in a way that doesn't break existing 8.x RC sites (e.g., via a settings.php setting), I agree with #120 that that would be preferable.

damien_vancouver’s picture

Title: Use +SymLinksIfOwnerMatch instead of +FollowSymLinks option in .htaccess » Use SymLinksIfOwnerMatch instead of FollowSymLinks option in .htaccess

I created a draft Change Record here: https://www.drupal.org/node/2608772 but am having trouble linking to this issue, I think because of the + signs in the title. I've edited those out of the title and will try again.

damien_vancouver’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record

That worked - draft change record is now attached. Setting back to needs review as per instructions at https://www.drupal.org/contributor-tasks/draft-change-record.

damien_vancouver’s picture

@effulgentsia

https://www.drupal.org/project/usage/drupal shows that there are at least 44 thousand Drupal 8 sites. Most are probably development/testing ones, but some are real.

Agreed, although of those real sites, only a fraction of those will have symbolic links to files owned by another user. The change record and the fact we're pre-release should minimize the pain of this change. I see the 1M Drupal 7 number there, so we hopefully can expect that many future installs of D8. We are never going to be in a situation where that number is 0 (unless we wait for the day the 9.0.x branch is created).

Having Drupal being to update .htaccess and set the correct value is a great feature idea and is something we should work on in a followup issue ASAP. It is its own ball of wax though - if the wrong directive is set and the site is 500 erroring, then PHP won't run in order to even bootstrap and make any changes to .htaccess. We'd probably have to ship with no directive in .htaccess to start off with, and then perform some tests (similar to the old Clean URL test) to determine which directive is safe to place in .htaccess. Other than a few symbolic links in /core/vendor, there are no symbolic links in the core codebase, so shipping with no directive and having that configured in settings.php might be something that could work (but unlikely we could get this together in time for release day).

alexpott’s picture

Status: Needs review » Needs work

Thinking about this some more I'm not sure why we even bother to set this setting. For example, https://github.com/phpbb/phpbb/blob/3.1.x/phpBB/.htaccess has it commented out, https://github.com/laravel/laravel/blob/master/public/.htaccess doesn't bother, and https://github.com/symfony/symfony-standard/blob/2.8/web/.htaccess has it commented out.

Perhaps what we should do is remove it from all the htaccess files created in sub directories and just comment it out in the root .htaccess and add comments. This would mean we'd just inherit the default set by the Apache installation - which seems to me to be the right thing to do.

@damien_vancouver it is amazing that there are so many Drupal 7 sites on shared hosting working quite well. I think the percentages in #121 are way off.

damien_vancouver’s picture

Perhaps what we should do is remove it from all the htaccess files created in sub directories and just comment it out in the root .htaccess and add comments. This would mean we'd just inherit the default set by the Apache installation - which seems to me to be the right thing to do.

That should work, though if neither +FollowSymLinks nor +SymLinksIfOwnerMatch is set then the one symbolic link in the core distro would not work properly (/vendor/bin/phpunit -> ../phpunit/phpunit/phpunit), potentially breaking phpunit.

@damien_vancouver it is amazing that there are so many Drupal 7 sites on shared hosting working quite well. I think the percentages in #121 are way off.

They aren't working quite well though, this issue goes back 4 years with people reporting that it's broken if they don't patch .htaccess each time. The percentages may be way off but even if they are we are still talking about huge numbers of affected hosts. One way or another we should ensure that patching .htaccess isn't necessary for them. I like the solution of removing the directive if that is viable.

alexpott’s picture

@damien_vancouver the phpunit symbolic is never accessed via the web. The whole vendor directory is locked down and is most secure when placed outside of webroot anyways.

alexpott’s picture

Status: Needs work » Needs review
FileSize
3.82 KB

Here's what I'm suggesting.

Oh and as for the alleged "security improvements" from this patch methinks people need to pay closer attention to the Apache documentation... https://httpd.apache.org/docs/2.2/mod/core.html#Options

It says, with respect to both symlink options:

This option should not be considered a security restriction, since symlink testing is subject to race conditions that make it circumventable.

catch’s picture

Issue tags: -rc target triage +rc target

Removing this makes sense to me. If we're just removing the directive, then this has no potential for disruption (or none that can't be fixed by updating apache configuration, which is an option for sites creating symlinks). So happy for this to be an RC target.

alexpott’s picture

+++ b/core/lib/Drupal/Component/PhpStorage/FileStorage.php
@@ -79,8 +79,7 @@ public function save($name, $code) {
-Options None
-Options +FollowSymLinks
+Options -Indexes -ExecCGI -Includes -MultiViews

This is the only part I'm not sure of. Reading the Apache documentation it would suggest that this is what we need to do.

alexpott’s picture

Issue summary: View changes
Issue tags: -Security improvements

I've updated the CR to reflect the simpler situation caused by the patch in #129. I've also removed the increased security claims from the CR since the Apache documentation claims this should not be relied on for security.

I've removed several sections from the issue summary and editted the Affected platforms and systems the numbers here are completely overblown since the Apache default setting for options is All in 2.2 and FollowSymLinks in 2.4

giorgio79’s picture

Thx alexpott #126 is genius :)

catch’s picture

Title: Use SymLinksIfOwnerMatch instead of FollowSymLinks option in .htaccess » Remove symlinks option from .htaccess
damien_vancouver’s picture

Patch #129 is working well for me in my test environment (Debian 8 / Apache 2.4.10 / PHP 5.6 / mod_fcgid)

Options -Indexes -ExecCGI -Includes -MultiViews works properly my files directory and appears to cover all the options we need to turn off as per the Apache 2.2 and 2.4 docs.

Oh and as for the alleged "security improvements" from this patch methinks people need to pay closer attention to the Apache documentation... https://httpd.apache.org/docs/2.2/mod/core.html#Options

Yes the Apache solution (like all symbolic links in Apache or even attempting to turn off symlinks entirely) is vulnerable to a race condition and yes, Apache chose to document that fact this rather than fix their broken implementation. However, +SymLinksIfOwnerMatch is still the more secure configuration directive, even if the default Apache implementation is flawed. The symlink race condition problem in Apache can be mitigated in a number of ways:

  • a kernel FS patch like grsecurity
  • jailing the apache process with something like mod_ruid2 (sadly does not support FastCGI PHP)
  • overriding the SymLinksIfOwnerMatch implementation using CloudLinux's mod_hostinglimits
  • hardening Apache to prevent race attacks with a patch like https://github.com/bluehost/apache-symlink-patch/blob/master/poc.c
  • Limiting PHP from being able to run any risky commands to make a hacktool (such as symlinking) using Suhosin
  • Setting permissions on all .php files to chmod 600, effectively blocking the target files of settings.php / wp-config.php

But in most of those workarounds, +SymLinksIfOwnerMatch still needs to be set in order to indicate you want that protection. It's a security configuration directive and we shouldn't refuse to set a more secure configuration just because its default implementation isn't perfect right now.

And even its current implementation provides additional (but not perfect) security: Not every user on the system can simply symlink to another user's files and serve them by default. Instead, they must use a hacktool (albeit a fairly trivial one) in order to create the race condition if they want to link to another user's file. That's undeniably more complicated than just "symlink a file and view it", so it does increase security by making the attack more difficult.

I am very pleased with the patch in #129 and the new approach, which it seems will work for everyone regardless of how they feel about numbers of affected sites, or the security-or-not security issue.

Francewhoa’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
44.38 KB
435.32 KB
17.03 KB

We are confirming the patch in comment #129 works. Yayaya :) Thanks alexpott and all :)

It's the easiest option we tried so far. As both shared and non-shared hosting users can install Drupal 8 as is. Without having to modify any file. Easy is good.

Using

  • Drupal: 8.0.0-rc3 from 2015-Nov-04
  • Debian: Jessie 8 at 64 bit
  • PHP: 5.6.14-0+deb8u1
  • Apache: 2.4.10
  • Database: MySQL 5.5.46-0+deb8u1
  • Kernel Linux: 4.1.5-x86_64
  • Webmin: 1.770
  • Virtualmin: 4.18.gpl
AFowle’s picture

Issue summary: View changes
AFowle’s picture

Well we have come a long way since my initial post 4 years ago!
#132 removed the details of the machine I actually had a problem with, so I have taken the liberty of adding it back.
It seems to have morphed into a D8 issue. I had problems with D6 and D7 sites. It's not clear to me that what is now proposed will suit them. Whilst D6 sites should, in theory, all have gone by 19th Feb 2016 I suspect that will not actually be the case. D7 sites will of course be around for much longer.

Please don't think I am being ungrateful. Much of the discussion has been beyond me, but it seems that we should perhaps fork this into a D7 (and prior) branch and a D8 branch and get both working. I'll leave that to the experts.

Everytime there is a version upgrade I upgrade and test locally and Drush the changes up to my live server like a good chap. 50% of the time I am then greeted with a 500 error because I have forgotten to amend the local copy of .htaccess.

Onwards and sideways

Francewhoa’s picture

It seems to have morphed into a D8 issue. I had problems with D6 and D7 sites. It's not clear to me that what is now proposed will suit them. Whilst D6 sites should, in theory, all have gone by 19th Feb 2016 I suspect that will not actually be the case. D7 sites will of course be around for much longer.

AFowle :) Indeed the same issue is still present with D6 and D7. My understanding is the present focus is to fix it for Drupal 8 (D8). It's a good timing as D8 will be released on November 19th 2015. Per above comment #8, after it's fixed and committed for D8 it is back-ported to D7 and D6. Assuming volunteer contributors are available and interested to do so. Me and the fellows I'm working with would be happy to contribute testing patches, quality assurance, documentation, and agile project management services if needed for both D7 and D6 back-port.

  • catch committed dc736e7 on 8.0.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...
catch’s picture

Version: 8.0.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 8.0.x, thanks!

Moving to 7.x for backport.

damien_vancouver’s picture

Status: Patch (to be ported) » Needs review
Issue tags: -Drupal 8, -rc target +needs backport to D7
FileSize
898 bytes
PASSED: [[SimpleTest]]: [MySQL] 41,754 pass(es). View

It is really great to see this committed in time for the release! Thank you to everyone who helped get this issue to the finish line.

I have backported the final patch to Drupal 7, patch attached for testing.

This change will only affect core's .htaccess file (not existing files directory .htaccess files that are already created). So from that perspective it should work because Drupal does not rely on symbolic links.

But... I have seen symbolic links used for "sites" directories (ie. the folders where settings.php live), in order to provide alternate domain names for a settings.php. This is supposed to be done in sites.php, but some people don't know about sites.php and use symbolic links, a technique that has worked back to 4.7 and probably before. (For example, a dev makes a sites/dev.example.com with a settings.php, and then later makes a symbolic link so that "dev.example.com" site also returns as "test.example.com" with: ln -s dev.example.com test.example.com). It's also possible there are symbolic links used in custom modules or even contrib modules. There may be cases like that where this change will break a site that was implicitly relying on that +FollowSymLinks to work.

If we do backport the patch into a maintenance release, then we need to clearly document the change. To do that we could put the old setting in .htaccess commented out, something like:

# Un-comment to explicitly follow symbolic links, restoring the behavior of Drupal <= 7.41
# Options +FollowSymLinks

Or maybe committing this change a backport is just too risky for D6 or D7?

Francewhoa’s picture

Committed/pushed to 8.0.x, thanks!

Good morning Catch :) Thanks

All contributors :) Congratulations and thanks for all your efforts

damien_vancouver :) We'll be happy to try to test that Drupal 7 patch. When done I'll post result here. I'll let others reply to your suggested approach. As you know I'm neither a Developer nor a SysAdmin.

tuandungb’s picture

I install Drupal 8 RC4 which include this issues.When access root folder I got error "access forbidden" and can not install it.After follow this issues and add Options +SymLinksIfOwnerMatch,this site back normal.
So I think why would we add it,or maybe have some another way to solve this.
I am use Opensuse 13.2 with Apache 2.4

cilefen’s picture

@tuandungb What symlinks do you have and why do you have them?

tuandungb’s picture

@cilefen : I just download Drupal 8 and install as normal.But I get "Access forbidden" error.Which I didn't get in RC3.So I track the change log and saw that thread.I just add that line and everything is fine.

Note that is fresh Drupal.

alexpott’s picture

@tuandungb you must be using symlinks somewhere.

tuandungb’s picture

Hmm.I am just setup another server to re-test in my laptop.Add mod_rewrite and here is config in default_server.conf.Everything is fresh,event Drupal download.I pushed Drupal in subfolder.I am sure I am not use any SymLinks

<Directory "/srv/www/htdocs">
        # Possible values for the Options directive are "None", "All",
        # or any combination of:
        #   Indexes Includes FollowSymLinks SymLinksifOwnerMatch ExecCGI MultiViews
        #
        # Note that "MultiViews" must be named *explicitly* --- "Options All"
        # doesn't give it to you.
        #
        # The Options directive is both complicated and important.  Please see
        # http://httpd.apache.org/docs/2.4/mod/core.html#options
        # for more information.
        Options None
        # AllowOverride controls what directives may be placed in .htaccess files.
        # It can be "All", "None", or any combination of the keywords:
        #   Options FileInfo AuthConfig Limit
        AllowOverride All
        # Controls who can get stuff from this server.
        <IfModule !mod_access_compat.c>
                Require all granted
        </IfModule>
        <IfModule mod_access_compat.c>
                Order allow,deny
                Allow from all
        </IfModule>
</Directory>

Can you tell me what's wrong here?

alexpott’s picture

@tuandungb and is Drupal installed in /srv/www/htdocs? And if so what is the output of ls -la /srv/www/htdocs?

damien_vancouver’s picture

I tested with the latest OpenSUSE and it worked for me:

- downloaded a fresh OpenSuse (latest version, "Leap 42") and installed into a VM.
- I then installed Apache 2.4 and PHP 5 (with "zypper in apache2 apache2-mod_php5"). I did not modify any Apache configuration files.
- downloaded and un-tarred Drupal 8.0 RC4 into /srv/www/htdocs.
- I browsed there (http://localhost in a browser) and saw the Drupal install screen.
- Next I moved the Drupal files to a subdirectory (/srv/www/htdocs/drupal) and when I went to http://localhost/drupal I also saw the drupal install screen.

@tuandungb As alexpott says, there must be a symbolic link somewhere in your path to Drupal, perhaps to the Drupal subfolder itself? In any case, you can avoid having to edit Drupal's .htaccess each time for your setup by putting: Options +SymLinksIfOwnerMatch or Options +FollowSymLinks inside your Apache config file (inside the is a good spot).

tuandungb’s picture

FileSize
3.36 KB

@alexpott : I attached output of "ls -la /srv/www/htdocs" command in attach file.I hope it can help.Drupal 8 located in drupal8 folder.
@damien_vancouver : Yes,your step is right in my side as I said above,except enabling mod_rewrite.Drupal 8 is okay until I start to enable mod_rewrite in Apache .I follow this tutorial : http://enarion.net/web/htaccess/mod_rewrite-on-suse/
- Add rewrite in Module apache list.
- Go to /etc/apache2/default-server.conf and Change AllowOverride None to AllowOverride All
- Restart apache
- Re-access to localhost/drupal8 and got "access forbidden".

Can you add mod_rewite and try out to see if this problem appear in your side? I test it all in both my destop and my laptop.One is Opensuse 13.2 ,and one is fresh opensuse 42.1 Leap.This problem happends in both.

damien_vancouver’s picture

Hi @tuandungb,

I've reproduced your problem, thanks for the extra information.

There is an explanation in the /var/log/apache2/error_log, and the problem is caused by your particular Apache configuration. You cannot have mod_rewrite enabled without one of either +FollowSymLinks or +SymLinksIfOwnerMatch, according to the error message:

AH00670: Options FollowSymLinks and SymLinksIfOwnerMatch are both off, so the RewriteRule directive is also forbidden due to its similar ability to circumvent directory restrictions : /srv/www/htdocs/drupal/core/install.php

If you're going to use mod_rewrite, you need to have one of those Symlinks directives set somewhere. In your default-server.conf would be a good spot:

        # The Options directive is both complicated and important.  Please see
        # http://httpd.apache.org/docs/2.4/mod/core.html#options
        # for more information.
        Options +SymLinksIfOwnerMatch
        # AllowOverride controls what directives may be placed in .htaccess files.
        # It can be "All", "None", or any combination of the keywords:
        #   Options FileInfo AuthConfig Limit
        AllowOverride All

Apache is behaving as expected and printing a sane error message. The error message does not show up until you enabled mod_rewrite because the RewriteRule directives it's upset about are inside an <IfModule mod_rewrite.c> ... </IfModule>.

It appears the problem is happening because of the default OpenSuSE Apache configuration. In Debian Linux, FollowSymLinks is set by default on the / directory (as is the most performant), and is the inherited by the default apache root.

From /etc/apache2/apache.conf in Debian 8:

  <Directory />
	Options FollowSymLinks
	AllowOverride None
	Require all denied
  </Directory>

Ubuntu and other deb-derivatives should behave the same way. We need to check the default situation in CentOS/RHEL/Fedora, if those also behave the same way. I will install a centos and a fedora today and try and reproduce the steps that lead to this problem and report back. If the upstream Apache 2.4 source sets Options None by default then we may need extra documentation or to consider another approach.

tuandungb’s picture

@damien_vancouver : Thank for that.I will add +SymLinksIfOwnerMatch to apache.I hope we have to give a notice in documentation to prevent confused like me

damien_vancouver’s picture

I fully tested that the Drupal 8 installer loads on default Apache/PHP installs for each of the following OS's:
- Ubuntu 14.04 LTS
- Redhat Enterprise Linux 7.1 (PHP is too old for Drupal 8 but the apache configuration works)
- Fedora Sever 31
- Debian 8

I also built the latest Apache 2.4 from source and confirmed that the httpd.conf you get after make install contains "Options +FollowSymLinks" for the default site's DocumentRoot. I noticed that all of the above configurations have mod_rewrite enabled by default, even the default Apache 2.4 source.

So, the configuration of "Options None" that is causing the problem for @tuandungb looks like it's an OpenSuSE specific configuration.

I hope we have to give a notice in documentation to prevent confused like me

Given this only affects OpenSuSE, I think the correct place for this documentation is on their side. Specifically, their modified instructions in default-server.conf where they set "Options None" should mention that mod_rewrite will not work without setting one of the Symlinks options.

I did some googling and there are several threads of people getting stuck enabling mod_rewrite on SuSE with this same problem. None of these threads have a good solution posted, so I will add some comments on them and will see if I can find where to properly ask for the documentation to be updated.

  • catch committed dc736e7 on 8.1.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...
damien_vancouver’s picture

@tuandungb,

I submitted a bug to OpenSuSE and it looks like they will add some documentation to help people avoid the 500 errors in the future. See: https://bugzilla.suse.com/show_bug.cgi?id=955701

@morenstrat and anyone else with 500 errors since Drupal 8.0.0,
We have a fix over at #2619250: Make .htaccess usage work for the widest possible configurations without relaxing security and document pitfalls where -MultiViews is no longer added to .htaccess files. You need to manually edit any sites/*/files/.htaccess files that have already been created so they no longer have Options -MultiViews.

sarathkm’s picture

Bitnami Lampstack has the same problem of not installing Drupal 8 installer under apps/ dir, though works fine under default apache2 htdocs as httpd.conf file has Options FollowSymLinks declared within it. The problem was resolved when I added Options Indexes MultiViews FollowSymLinks within conf folder residing in apps folder (apps/demo/conf/httpd-app.conf). The same worked good for Options Indexes MultiViews SymLinksIfOwnerMatch, though this one seems a little lazy to load, after reading all of the above comments I got to learn something new and came to know yes its slower.

This may be same for Bitnami WAMP, MAMP & ah.. maybe Bitnami XAMPP & that Bitnami Drupal Stack if we consider installing in some other folder other than apache2/htdocs/ folder after this change in current .htaccess folder.

AFowle’s picture

Title: Remove symlinks option from .htaccess » Change symlinks option in .htaccess to +SymLinksIfOwnerMatch
Version: 7.x-dev » 8.0.0
Status: Needs review » Active

The situation is now at least as bad as in my original post. I have just run into the D8 / OpenSuse issue myself, with the release version of D8 which has no option set in .htaccess. The error messages are unhelpful and it took me a long time to realise what I had done! We now have three possibilities for the symlink option at initial installation:

  • +FollowSymLinks - This will prevent numerous people on shared hosting from installing
  • Not set - This will prevent people using at least one major Linux distribution from installing
  • +SymLinksIfOwnerMatch - I don't think we have found anyone who cannot do the initial install with this option

This long thread has established that only a very few people with complex setups will need +FollowSymLinks. To turn the tables on comment #1 they should look after their own versions of .htaccess and backup / restore as needed. It is really disheartening to less skilled users to have severe errors when installing the bare application and we should prevent that happening. The complex setups will be built on the bare application by expert users. There may well be versions of Linux other than OpenSuse affected - we won't know in this thread because the number of users is small compared to those trying to install D8 worldwide.

We should use +SymLinksIfOwnerMatch in the supplied version of .htaccess and alter documentation to suit.

I have changed this back to a D8 issue as it is already marked as needing a D7 backport. It is not fixed for D8 IMHO.

cilefen’s picture

Title: Change symlinks option in .htaccess to +SymLinksIfOwnerMatch » Remove symlinks option from .htaccess
Version: 8.0.0 » 7.x-dev
Status: Active » Postponed

@AFowle That is a nice review of the situation. I think you more than anyone has a view of the situation. But this issue has already been committed to core. The usual procedure is to open a follow-up issue ("Followup to #1269780: etc..." to correct a regression because in core we usually try to avoid double-committing to a given branch on a core issue. Can you please do that?

I set the metadata on the issue back, with the exception that I marked it postponed, pending a decision or action on the follow-up issue.

AFowle’s picture

  • catch committed dc736e7 on 8.3.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...

  • catch committed dc736e7 on 8.3.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...
naught101’s picture

In d7, the status report still bitches about

Public files directory	Not fully protected
See http://drupal.org/SA-CORE-2013-003 for information about the recommended .htaccess file which should be added to the sites/default/files directory to help protect against arbitrary code execution.

if I change the sites/default/files/.htaccess file to use +SymlinksIfOwnerMatch instead of +FollowSymlinks (which breaks site functionality on my server).

Turamarth’s picture

I get this bug on drupal 8 last update, i need to delete the .htaccess to make my site work. I hope anyone can fix it.

damien_vancouver’s picture

@naught101,

if I change the sites/default/files/.htaccess file to use +SymlinksIfOwnerMatch instead of +FollowSymlinks (which breaks site functionality on my server).

Unfortunately the fix was not backported to D7. It was argued in this issue that the risk of breaking sites by removing this directive was greater than the benefit of backporting the change. It's frustrating if you have to patch every single Drupal core update, especially knowing that the directive isn't even used by anything in core, but there you have it... there is a real risk of breakage for large numbers of sites if we make changes to .htaccess. Fixing the problem for those of us who experience it would be great... but it must not be at the expense of an unknown number of other users suddenly having unexpected site breakage and possibly sending them down a rabbit hole of Apache troubleshooting without warning. Definitely not cool!

But, at least it is now fixed in Drupal 8 and it's possible to implement proper security in your webserver configuration without Drupal breaking it and 500 erroring every time you deploy a new core. (Before someone pipes up and claims Options +SymLinksIfOwnerMatch is not secure, you are wrong, please read https://en.wikibooks.org/wiki/Grsecurity/Appendix/Grsecurity_and_PaX_Con... which is just one of several ways this directive can be made secure).

Since the fix will not be backported then I guess the status report complaining is also technically not a bug, your .htaccess file does differ from the (ill-advisedly) recommended default.

@Turamarth,

I get this bug on drupal 8 last update, i need to delete the .htaccess to make my site work. I hope anyone can fix it.

I really strongly advise against deleting .htaccess! That file is required to make Clean URLs work, and also responsible for some of your Drupal site's security. You'll be exposing yourself to being hacked without it.

Instead of delete it, you should put it back and then figure out what is wrong with your Apache configuration. Check the error log for your site and you should see a message there explaining why the site isn't working. You should fix your Apache configuration so that the .htaccess file works properly.

Turamarth’s picture

Yeah i know that, i dont want delete it, but its a shared host and i almost cant do nothing there, thats the problem, in my localhost i dont have this problem and all works perfect.

EDIT: i was looking more deeper and i found this https://www.drupal.org/node/2620220 and i removed -ExecCGI option in .htaccess and now images are working, but im not full aware of the impact of remove this option.

Im also getting erro 500 when trying to clear the caches of sites from config/development/performance, im not sure if its related (but also this part works fine on localhost)

  • catch committed dc736e7 on 8.4.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...

  • catch committed dc736e7 on 8.4.x
    Issue #1269780 by hswong3i, damien_vancouver, Agileware, ricardoamaro,...