Currently, accessing a site that uses the force-www RewriteRule defined in the default .htaccess without supplying a Host: header causes you to be redirected to "http://www./", as below:

GET / HTTP/1.0

HTTP/1.1 301 Moved Permanently
Date: Tue, 24 Jul 2012 13:10:29 GMT
Server: Apache/2.2.3 (CentOS)
Location: http://www./
Cache-Control: max-age=1209600
Expires: Tue, 07 Aug 2012 13:10:29 GMT
Vary: Accept-Encoding
Content-Length: 302
Connection: close
Content-Type: text/html; charset=iso-8859-1

As an additional issue, if you are accessing the site via HTTPS, you'll be downgraded to HTTP in the redirect.

I've attached three patches to address one or both of these issues, as follows:

v1: Disable redirect to "www." if no Host: header is present.
v2: If no Host: header is present, redirect to the ServerName defined in Apache for this VHost.
v3: v1, plus code to handle the http/https confusion.

Of these, v3 seems best. v2 is only included for completeness, but I'm willing to be convinced that it's the right approach.

This is a security issue because the code, as shipped (albeit commented out), is vulnerable to a MitM attack, because it redirects from secure to insecure. The security issue is demonstrated in this scenario:

Upstream MitM Linux router transparently forwards destination port 443, NATs dst_port 80 to localhost. Local port 80 is a proxy server that swaps any "https://" in the received reply from the server to be "http://", and eats the initial redirect onto "https://". Now, the client is permanently downgraded to HTTP, while the proxy server connects to the real server over HTTPS, thus keeping it happy (and oblivious to the fact that this MitM is happening).

Scenario where all content is intended to be encrypted (securepages in use, all pages are forced to HTTPS), and www. redirect is in effect:

Client manually types in: "https://drupal.org/user".
(Real) Server responds: HTTP-301, Location: http://www.drupal.org/user
Client issues request for http://www.drupal.org/user
Proxy issues request for http://www.drupal.org/user
Server responds: HTTP-301, Location: https://www.drupal.org/user
Proxy issues request for https://www.drupal.org/user
Server delivers content to proxy
Proxy delivers content to client, with s#https://#http://#g
Lather, rinse, repeat.

The client doesn't trust his network, and therefore properly typed in (or used a bookmark for) https://, and the SSL negotiation worked, since they really were hitting the intended destination site. But as soon as the connection is downgraded to HTTP, our attacker swoops in and makes sure it never gets pushed back onto HTTPS, even as, from the perspective of the server, it has been.

(see also https://security.drupal.org/node/76983)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

iwant2fly’s picture

Subscribing

greggles’s picture

Title: Update htaccess www/non-www RewriteRules to be more robust » Make default htaccess rules protocol sensitive to avoid man-in-the-middle-attacks if users don't fully customize the rule
Version: 7.x-dev » 8.x-dev
Status: Active » Needs review
FileSize
1.67 KB

Thanks for the detailed write-up and providing several options, BMDan.

I just re-read your advice and think that the v3 patch makes the most sense. I tested out drupal-htaccess.v3.patch and it works as designed.

This needs to go into 8.x first and then be backported, so tagging for that. It also didn't apply directly there, so I've attached a new patch that applies. The status should also have been at "needs review" since there's code here that needs review.

Status: Needs review » Needs work

The last submitted patch, 1733476_htaccess_protocol_sensitive.patch, failed testing.

greggles’s picture

Status: Needs work » Needs review
Issue tags: +Needs backport to D7

This tag is more popular.

greggles’s picture

My checkout wasn't fully updated for that patch. Let's try again.

The last submitted patch, 1733476_htaccess_protocol_sensitive_5.patch, failed testing.

greggles’s picture

Status: Needs work » Needs review
BMDan’s picture

greggles’s picture

Status: Needs review » Reviewed & tested by the community

Marking RTBC since I just manually re-applied BMDan's code. I'm using this on a site and it works as expected.

catch’s picture

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

Thanks! Committed/pushed to 8.x. Moving to 7.x for backport.

Is it worth trying to backport to D6 too? Don't remember how much the rules changed since then...

greggles’s picture

Status: Patch (to be ported) » Reviewed & tested by the community
FileSize
1.68 KB

It doesn't directly apply to 6.x, but a backport makes sense.

RTBC for similar reasons as #9.

greggles’s picture

Issue tags: +Needs backport to D6
David_Rothstein’s picture

Version: 7.x-dev » 6.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
+  RewriteCond %{HTTPS} on

I'm not convinced this works on older versions of Apache (for example, 1.3, which according to http://drupal.org/requirements/webserver we still support). However, I guess it won't break anything there either, and we aren't really obligated to add security improvements for older versions of webservers that aren't even supported by their own communities anymore...

Therefore, committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/ee22e04

Moving to 6.x for backport.

greggles’s picture

I did some quick research and it appears that while our page says we support Apache 1.3, the rest of the internet is no longer really supporting it. I filed #1862046: [no-code] Say that we no longer support Apache 1.3 for D8.

David_Rothstein’s picture

Drupal 7.18 was a security release only, so this issue is now scheduled for Drupal 7.19 instead.

Fixing tags accordingly.

David_Rothstein’s picture

Drupal 7.19 was a security release only, so this issue is now scheduled for Drupal 7.20 instead.

Fixing tags accordingly.

David_Rothstein’s picture

Drupal 7.20 was a security release only, so this issue is now scheduled for Drupal 7.21 instead. For real this time... I think :)

Fixing tags accordingly.

David_Rothstein’s picture

Issue tags: +7.22 release notes

Fixing tags since Drupal 7.21 only contained a fix to deal with fallout from the Drupal 7.20 security release. Hopefully this is really the last time I do this and this will be released in Drupal 7.22 for real :)

creatorwpy’s picture

Status: Patch (to be ported) » Needs review
Issue tags: -Needs backport to D6, -Security improvements, -Needs backport to D7, -7.22 release notes

drupal-htaccess.v1.patch queued for re-testing.

BMDan’s picture

Issue summary: View changes

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.