In Apache 2.4, the "Order" becomes deprecated

With Apache 2.4 the old Order get's deprecated in favor of Require.

If hosters by default have Access compat, then Order keeps working. Otherwise however, with our current .htaccess Apache 2.4 will not start after installing Drupal.

I tried to figure out a way to check which the current version of Apache is, but unfortunately IfVersion is only available in 2.4, so that will break 2.2. My stackoverflow question hasn't yielded anything that we can use yet. I asked on IRC, and got two suggestions.

  • Ship two files, one < 2.4 and one >= 2.4. Pick one during install. Not sure how feasable that is for us.
  • Find modules that are unique and frequently enabled in the different environments, and do IfModule on those to determine where we are. This seems like a total hack to be honest, but may actually be the best way to go if we can identify a reliable set of modules.
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

BTMash’s picture

Perhaps another solution might be that we have whatever we need inside the .htaccess file and if users are running 2.4, they comment out the 'Order' set of lines for the 'Require'. And if changing the htaccess file is considered hacking core, then it makes more sense for there to be an example.htaccess (or .htaccess.default as per #1117704: Move .htaccess to .htaccess.default) file anyways so a person can make the htaccess file as needed.

Letharion’s picture

Since I haven't been able to find anything close to a clean solution, it may just come down to that, but honestly, that is just _not_ a good solution. By the time we ship 8, 2.4 will be popping up all over the internet. By the time 8 takes over 7 in usage, 2.4 is likely to be a common server.

By requiring users to manually edit the .htaccess file, we're making an already complex installation worse. It's not that I mind myself, I don't use the file anyway, but since we're moving towards being more user friendly, we should be doing the opposite, that is, make installation easier.

marcingy’s picture

Status: Active » Needs review
FileSize
547 bytes

After some digging ifversion does work with 2.2 (it is available from 2.0 onwards) see http://httpd.apache.org/docs/current/mod/mod_version.html#ifversion

On xampp I had to load LoadModule version_module modules/mod_version.so to get it working so that could become part of the install process. Not sure how widely supported the module is on shared hosts though.

Obviously we need an update to install instructions for this approach for sites without the module installed.

Status: Needs review » Needs work

The last submitted patch, apache-2.4.patch, failed testing.

BTMash’s picture

I went through this with @marcingy (and am updating the issue as a result) by looking at our default apache environments when ubuntu gets installed. mod_versions is not enabled by default. At my workplace, we actually have a shared hosting environment so I was able to see what a shared env would have and unfortunately, same issue. Its a real shame because I like the patch quite a lot. I don't have an apache2.4 instance to test things out on...but is mod_access_compat enabled by default by any chance?

marcingy’s picture

Looking in xamppp mod_access_compat is enabled in 2.4 by default.

marcingy’s picture

I am assuming you are thinking of using ifmodule ! if so we should use http://httpd.apache.org/docs/trunk/mod/mod_authz_core.html as technical http://httpd.apache.org/docs/current/mod/mod_access_compat.html is just for 2.3.

BTMash’s picture

well...mod_access_compat is made for 2.3+ (so the settings from 2.2- can still continue to work). If it is enabled by default, then we know that someone intentionally went in and disabled it (I'd imagine if it is a shared hosting company, they would want the access_compat enabled or see a lot of angry emails but I am unsure on this). In which case, we can add some documentation for those that don't.

IfModule would be a nice solution if there was an directive to go with it.

marcingy’s picture

The issue I see that there is no way to determine if we on 2.2 where the module does not exist vs 2.3+ and the module has been disabled. Which why I am thinking this approach works better as the core module will be enabled and it gives us a simple option to switch, instead of having to have a multi layer ifModule structure.

<ifModule mod_authz_core>
     2.3+
</ifModule>
<ifModule !mod_authz_core>
     2.2
</ifModule>
BTMash’s picture

ah...that is what I was looking for (I did not understand what you meant by the ! until I looked it up). But yes...I'd imagine that is a fairly good workaround? I'll try and write up a patch if you're not able to get to it before then.

BTMash’s picture

Status: Needs work » Needs review
FileSize
565 bytes

I just realized...the patch wouldn't take too long. Attaching patch for review.

Letharion’s picture

The syntax used in the apache documentation http://httpd.apache.org/docs/2.4/mod/mod_authz_core.html#require looks like "Require all denied" instead of "Require allow,deny". Is there a reason to use the syntax of the current patch?

BTMash’s picture

Status: Needs review » Needs work

Ack, I had messed it up - you are right. What is the simplest rule we should use then? Would it be 'Require all granted'? It *seems* like the equivalent for 'Order allow, deny' but I'm not entirely sure.

bleen’s picture

... as long as you're making changes to the patch in #11, I would add a comment explaining why you checking for that module ... in an ideal world those if statements would be "if apache_version >=2.4 "... and there would be no confusion, but since we are using the existence of an apache module instead I think we need to explain why.

Damien Tournoud’s picture

<IfModule mod_authz_core.c>
</IfModule>

That looks very fragile. The existence of a mod_authz_core module is an implementation detail, not a contract we should rely on.

Given that it is 100% certain that no host in the world that is running Apache 2.4 is not going to run with mod_access_compat, I think we should just postpone this to after Drupal 8.

Letharion’s picture

@#13 Yes, require all granted (or all denied) is what the documentation does, and I've started using myself so I can say that it works as expected.

@#15 Yes, it's really fragile, I agree. However, I have spent quite some time trying to find out what a robust way would be, and that type of solution was the best one I could come up with as well. That said I have neither used all options to find a better method, nor am I an expert on apache config.

I initially assumed that there would be an environment variable hardcoded into apache, much like we do in bootstrap.inc, that tells us which version is being used, but if that is the case, then I haven't been able to find this info. Perhaps it would be a reasonable feature request against apache.

I agree that it's reasonble to assume that all generic hosts will run with mod_access_compat for a long time. We could definitely postpone this until around this time next year, yes.

Personally, I just want a useable patch in the queue, because I do run without compat myself, so that I can root out problems early and get the upgrade over with. Now that there's an issue and a suggestion for a patch, I can just stick that in my make-files and be happy.

catch’s picture

What about two sets of rules? One for each apache version, and then you'd be required to comment/uncomment depending on what you're running.

Letharion’s picture

That would be great, but I never even bothered suggestion it since that will make installation (even) more complicated, something I assumed wasn't gonna fly.

Perhaps stick with the 2.2 one by default, yet distribute an optional 2.4, requiring only 2.4 users to take extra action. Then switch those around when we release D9 or 10.

catch’s picture

Yeah shipping with 2.2 by default, then rules for 2.4 commented out I thought. Then we've got rules in core that can be tested by more people, and we can try to improve it as time goes on - or as you say swap the defaults if nothing much changes but 2.4 adoption picks up.

Letharion’s picture

@#19 It's probably one of the best ways of going forward with this.

The only issue I see with that is that technically, changing the .htaccess file will be "hacking core", something that I will find annoying since I move around and/or create new sites frequently.

I will probably just post a patch that does away with the 2.2 support and stick that patch in my makefile.

torvall’s picture

Gentoo has just started pushing Apache 2.4 in its stable release and the Drupal sites I have in a Gentoo box stopped working after the update. (Edit: It seems that this was a mistake by the ebuild maintainers and 2.4 was marked as unstable again. Reference: https://bugs.gentoo.org/show_bug.cgi?id=459264)

I had to edit .htaccess in all of them to get them working again. However, the Require directive syntax in #11 is not correct, should be "Require all granted" instead of "Require allow,deny" (see http://httpd.apache.org/docs/2.4/upgrading.html).

This will be a problem not only in D8 so, in my opinion, the directive should be included (as a comment, of course) in all versions.

FreekyMage’s picture

As #21 says, this should be fixed in D7 as well.

dellintosh’s picture

Might consider using this: http://httpd.apache.org/docs/2.2/mod/mod_version.html

A bit verbose, but our tests seem to work fine with it.


<IfVersion < 2.4>
... if less than 2.4
</IfVersion>

<IfVersion >= 2.4>
... 2.4-specific stuff
</IfVersion>

kristofferwiklund’s picture

It require the version module. And it seems that Ubuntu doesn't have it active as default. Read comment #5

I have the same problem with a update Gentoo version. Apache 2.4

kristofferwiklund’s picture

Here come a patch with updated syntax.

It has been tested on latest Gentoo with Apache 2.4 and Ubuntu 12.04 LTS with Apache 2.2.

And it works nice.

This is not a optimal solution but I could not find any other better. As the option of commenting and uncommenting will force Apache 2.4 users to first edit .htaccess before installation. This is not good for future OS upgrades. And also for every core update one needs to do the same thing. I have some idea about solving this but I needs to do some more testing.

And the other solution using IfVersion; this is not enabled by default. And also using the If syntax is not working in Apache 2.2. So that solution is not possible.

edit: typo

Sylvain Lecoy’s picture

Status: Needs work » Needs review

Works for RHE6 and Apache 2.4.

Sylvain Lecoy’s picture

I have issues however with the rewrite module. Maybe its not related but when I submit a login form from /user apache logs a 404, while when I submit a login form from /?q=user, apache logs a 200 OK.

kristofferwiklund’s picture

I have not seen that problem. For me it works with both url.

Which OS are you running under. And how is your server installed? I might be as you said an other problem that needs it's own issue.

Sylvain Lecoy’s picture

Yes probably, I encounter it on RHEL6, with Apache 2.4 built from sources with dynamic module Rewrite.so.

URLs works fine but for some reason the Clean URL test (in admin panel) fail to validate. I think this is definitely related to the Apache configuration and .htaccess configuration but may belong to another issue (or not).

kristofferwiklund’s picture

Updating the tags.

valthebald’s picture

Works well on Debian testing (Apache/PHP installed from packages, not manually compiled)

gnassar’s picture

Status: Needs review » Reviewed & tested by the community

Apache 2.4, FreeBSD 9.2. Worked perfectly. Clean URL test passed fine, both URLs listed in #27 work -- can't replicate.

Probably about time this gets bumped to RTBC.

gnassar’s picture

I should note: also rolled back to Apache 2.2 and tested. Still worked perfectly.

Xano’s picture

Morbus Iff’s picture

Why is it "Require all granted" and not "Require all denied"?

The docs for the old 2.2 "Order allow,deny" explain why it works without a "Deny from all": "First, all Allow directives are evaluated; at least one must match, or the request is rejected. Next, all Deny directives are evaluated. If any matches, the request is rejected. Last, any requests which do not match an Allow or a Deny directive are denied by default."

But, for 2.4, "Require all granted" just point blanks states: "Access is allowed unconditionally."

Thus, it appears something is wrong (in 2.4, we're allowing everything)
or something is tricky (in 2.4, what you think you see is wrong because...).

alexpott’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

file_save_htaccess() will not correctly create an htaccess for private files so this needs work.

The value of the MTimeProtectedFastFileStorage::HTACCESS constant is "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nDeny from all\nOptions None\nOptions +FollowSymLinks"

kristofferwiklund’s picture

Agree. And for Drupal 7 that´s in function file_create_htaccess($directory, $private = TRUE) in /includes/file.inc

antarchi’s picture

comment #35: which should it be? I've just had to manually edit .htaccess and used "Require all granted". Is this OK?? Is that the best solution currently? (I'm a noob)

Gorka Guridi’s picture

3: apache-2.4.patch queued for re-testing.

The last submitted patch, 3: apache-2.4.patch, failed testing.

longwave’s picture

#35 is correct, this should use "Require all denied". I am running Ubuntu 13.10, which uses Apache 2.4, but also has mod_access_compat enabled for backward compatibility.

Without any patch:

$ curl -I http://localhost/drupal8/drupal/core/modules/node/node.module
HTTP/1.1 403 Forbidden

With #25 applied:

$ curl -I http://localhost/drupal8/drupal/core/modules/node/node.module
HTTP/1.1 200 OK

With #25 applied, and modified to use "Require all denied":

$ curl -I http://localhost/drupal8/drupal/core/modules/node/node.module
HTTP/1.1 403 Forbidden
longwave’s picture

Status: Needs work » Needs review
FileSize
3.4 KB

Attached patch fixes the above, plus FileStorage::htaccessLines(), and adds some extra assertions.

sun’s picture

Title: Update the .htaccess to work with Apache 2.4 » .htaccess protections do not work on Apache 2.4 without mod_access_compat
Component: other » base system
Category: Feature request » Bug report
Issue tags: +Security

Not sure why this is categorized as feature. Also not sure whether the priority shouldn't actually be major or even critical, because unless I misunderstood, this incompatibility causes all of our .htaccess files in public but even more worrisome also private files directories to be dysfunctional — and no one is aware of it.

tstoeckler’s picture

Priority: Normal » Critical

I also do not see how this can not be critical, unless I'm completely missing something.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

agree this is critical, patch looks ready to me.

alexpott’s picture

Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to 7.x

Committed 9e72c8b and pushed to 8.x. Thanks!

  • Commit 9e72c8b on 8.x by alexpott:
    Issue #1599774 by longwave, kristofferwiklund, BTMash, marcingy |...
hgurol’s picture

There is a function somewhere that creates an .htaccess file on the fly for some certain folders; eg. the /tmp folder.
Does this patch takes care of that too?

PS: This is for D7 by the way...

hgurol’s picture

includes/file.inc
around lines 534

That "Deny from all" should be "Required something..." right?
or maybe more changes are needed.
I hope you have looked at the issue from all angles.

  if ($private) {
    $lines = "Deny from all\n\n" . $lines;
  }

  return $lines;
}
catch’s picture

That's https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Component%21PhpS... in Drupal 8, and it's covered in the patch.

marcingy’s picture

Version: 8.x-dev » 7.x-dev
longwave’s picture

I think this should move to a feature request for Drupal 7, not a bug. Drupal 7 was released before Apache 2.4 and so there was no way we could guarantee forward compatibility at the time. Trying to retrofit it now with the technique used in the Drupal 8 patch should work on most setups, but can we guarantee it works on all existing configurations out there?

Also, it looks like we might have a further difficult case to deal with: upgrading existing autogenerated .htaccess files that existed prior to the patch, to catch users that move their D7 site from Apache 2.2 to 2.4.

serundeputy’s picture

Status: Patch (to be ported) » Needs review
Issue tags: -
FileSize
1.18 KB

backport for D7.

valthebald’s picture

Issue tags: +Needs manual testing

Tagging for manual testing - someone needs to test this on both 2.2 and 2.4

widukind’s picture

I'm verifying patch #53 to be working with Apache 2.4.7 on Ubuntu 14.04 (access_compat turned off) and Apache 2.2.22 on Ubuntu 12.04.

quicksketch’s picture

+    $lines = <<<EOF

This line needs to be changed to use concatenation instead of replacing the entire $lines variable. If the entire variable is overridden, then the special string "Drupal_Security_Do_Not_Remove_See_SA_2006_003" goes missing, and then system_requirements() throws an error every time the Statue Report page is viewed.

+    $lines .= <<<EOF
quicksketch’s picture

Status: Needs review » Needs work
Rob C’s picture

Status: Needs work » Needs review
FileSize
1.18 KB

This fixes the missing dot, no other changes.

quicksketch’s picture

Thanks Rob C. One additional issue, the <<<EOF styntax doesn't add any new lines before/after, so the code comment # Deny all requests from Apache 2.4+. ends up squashed at the end of the line instead of on its own line. I'd recommend putting a new line above the comment, and for extra kudos, adding an extra line at the end of the first block of code. Here's the fix that we made to this in Backdrop: https://github.com/backdrop/backdrop/pull/997/files.

Rob C’s picture

Added 1 additional line before start and at the end.

ben.bunk’s picture

I've added an update hook to the system module to force regenerating the htaccess files in the files directories. This may cause problems for people if they modified these files but I think the benefit would outweigh the risk here for the majority of sites that exist and may not even realize the file was generated on their behalf.

Status: Needs review » Needs work

The last submitted patch, 61: 1599774-htaccess-apache-2.4-61-D7-backport.patch, failed testing.

ben.bunk’s picture

I updated the patch, it turns out that the overwrite flag for file_create_htaccess doesn't actually allow overwriting unless you first change the file permissions to allow it.

ben.bunk’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 63: 1599774-htaccess-apache-2.4-62-D7-backport.patch, failed testing.

joelpittet’s picture

Can we go back to 60? I disagree the benefits outweigh the risk and drush up will override the .htaccess.

Manuel Garcia’s picture

Patch on #60 still applies cleanly.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.19 KB

Patch from #60 re-uploaded for the bot.

Status: Needs review » Needs work

The last submitted patch, 68: 1599774-htaccess-apache-2.4-60-D7-backport.patch, failed testing.

DuneBL’s picture

Here is the re-rolled #60 patch for 7.42

longwave’s picture

Status: Needs work » Needs review
FileSize
1.19 KB

#70 is malformed. #68 looks like a random fail, let's try that again.

Status: Needs review » Needs work

The last submitted patch, 71: 1599774-htaccess-apache-2.4-60-D7-backport.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review
FileSize
1.21 KB

Reroll.

$ git checkout 405e861
$ git apply --index 1599774-htaccess-apache-2.4-60-D7-backport_0.patch
$ git rebase 7.x
First, rewinding head to replay your work on top of it...
Applying: 1599774-htaccess-apache-2.4-60-D7-backport_0.patch
Using index info to reconstruct a base tree...
M	.htaccess
Falling back to patching base and 3-way merge...
Auto-merging .htaccess
CONFLICT (content): Merge conflict in .htaccess
Failed to merge in the changes.
Patch failed at 0001 1599774-htaccess-apache-2.4-60-D7-backport_0.patch

Pretty simple hand merge to add the new FilesMatch pattern for composer.

Status: Needs review » Needs work

The last submitted patch, 73: drupal7-htaccess_protections-1599774-73.patch, failed testing.

mpdonadio’s picture

Status: Needs work » Needs review

OK, I ran the fails from #68 and #73 locally w/ Apache 2.2 and all was OK. I also queued up a few successive runs, and they came back OK. We should check to see if there is a problem with spurious fails w/ 7.x. If this last run comes up OK, someone should do the manual testing, and then we should be able to set RTBC if all is well.

rooby’s picture

Yeah I also did a bit of investigation and came to the conclusion that I didn't think this patch was the cause of the fail.

miranche’s picture

Heads up, Sabayon and Gentoo seem to have dropped support for mod_access_compat, so Drupal crashes on liftoff on a local apache 2.4 server due to this error.

  • alexpott committed 9e72c8b on 8.3.x
    Issue #1599774 by longwave, kristofferwiklund, BTMash, marcingy |...

  • alexpott committed 9e72c8b on 8.3.x
    Issue #1599774 by longwave, kristofferwiklund, BTMash, marcingy |...
J-Lee’s picture

I have applied successfully patch #73 at 7.x-dev and 7.50.
The patched Drupal 7.50 works well on Apache 2.2 and Apache 2.4.

RTBC for me.

stefan.r’s picture

Issue tags: -Needs manual testing +Drupal 7.52 target
joelpittet’s picture

Status: Needs review » Reviewed & tested by the community

This applies with 2 lines of fuzz.

yareckon’s picture

At least the main .htaccess patch must go in in the next release. Drupal has to work on apache for goodness sake.
If any more bugs come in on the autogenerated stuff, we can look at them, but this needs to go through.

yareckon’s picture

Title: .htaccess protections do not work on Apache 2.4 without mod_access_compat » Drupal fails to boot with 503 error and .htaccess protections do not work on Apache 2.4 without mod_access_compat
Fabianx’s picture

Issue tags: +Pending Drupal 7 commit

Marking for commit. I agree with this being critical.

We cannot port the tests, right?

Fabianx’s picture

I am happy to commit this, but don't we need an upgrade path to re-write already existing .htaccess files, which are written dynamically to the public files directory - if they only match the old version?

David_Rothstein’s picture

The patch looks good although unclear why it is moving those lines to the bottom of the .htaccess file? I think it would be cleaner if they stayed at the top where they currently are (and where the Drupal 8 patch left them - this should really just be a straight backport without any changes from what went into Drupal 8).

but don't we need an upgrade path to re-write already existing .htaccess files, which are written dynamically to the public files directory - if they only match the old version?

I think that could be left for a potential followup issue. If someone decides to deliberately change the Apache configuration/version to avoid using mod_access_compat for an existing site, it's reasonable that they might need to edit .htaccess files too. And we still haven't done an upgrade path for https://www.drupal.org/SA-CORE-2013-003 even :) (I remember it got difficult when looking into it in the private issue queue, and I think there's a public issue for it somewhere but I'm not sure.)

We cannot port the tests, right?

We could in theory, but we'd have to port the whole test. An overall backported test for file_create_htaccess() would definitely be useful, but I see little value in extending that to test this particular issue (it just comes down to asserting a particular set of lines exist in the .htaccess file, which is fragile and not actually testing much that's meaningful). So in my opinion we can skip it here.

  • alexpott committed 9e72c8b on 8.4.x
    Issue #1599774 by longwave, kristofferwiklund, BTMash, marcingy |...

  • alexpott committed 9e72c8b on 8.4.x
    Issue #1599774 by longwave, kristofferwiklund, BTMash, marcingy |...
stefan.r’s picture

Status: Reviewed & tested by the community » Needs review
Issue tags: -Drupal 7.52 target, -Pending Drupal 7 commit +Drupal bugfix target
FileSize
687 bytes
1.19 KB

Backport of the Drupal 8 version (untested!) keeping those lines at the top

J-Lee’s picture

#90 is nearly the same as #73. Applyed and working for me.
For me RTBC.

yareckon’s picture

Status: Needs review » Reviewed & tested by the community

Works. Lets get this into the next release please.

yareckon’s picture

Pinging release mgmt people!

mpdonadio’s picture

#93, updating issues to get attention doesn't usually work. The committers have a big RTBC queue that they are working through.

yareckon’s picture

There are only 3 RTBC criticals, and this is one of them. Cmon folks.
https://www.drupal.org/project/issues/drupal?text=&status=14&priorities=...

stefan.r’s picture

Assigned: Unassigned » David_Rothstein
David_Rothstein’s picture

The patch no longer applied, but it only required a trivial reroll, so I'm leaving this at RTBC. Everything looks good to me to get this in for the upcoming release.

While I was rerolling it, I fixed a minor issue - I put the \n\n that was there before back in (see attached interdiff) so that the spacing in the resulting .htaccess file is consistent throughout. I'll file a quick followup to do that for Drupal 8 too.

I will also create a followup issue to consider adding an update function.

stefan.r’s picture

Issue tags: +Pending Drupal 7 commit

Update function sounds good, but not blocking and the patch still looks OK, so marking for commit.

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -Pending Drupal 7 commit +7.55 release notes

Committed to 7.x - thanks!

  • David_Rothstein committed ff27da6 on 7.x
    Issue #1599774 by longwave, ben.bunk, Rob C, stefan.r, David_Rothstein,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.