This patch blocks http access to files named CHANGELOG.txt COPYRIGHT.txt INSTALL.txt LICENSE.txt MAINTAINERS.txt UPGRADE.txt and README.txt. This is a security improvement because it makes it considerably more difficult for an ill-intending site visitor to find what version of drupal and/or modules the site is using and what security vulnerabilities the website is likely to be open to. All the above files have CVS version numbers and other subtle differences that help identify the drupal version in use.

This patch also implements handling of file access denied errors in drupal. For example, if a user tries to access example.com/modules or example.com/CHANGELOG.txt the user is presented drupal's 403 access denied page, instead of the web server's 403 access denied page. It does this by implementing ErrorDocument 403 /index.php in .htaccess and by checking for $_SERVER['REDIRECT_STATUS'] == 403 in system_init(). This is a usability and security improvement. Usability, because the user still feels like they are on the site if they got there accidentally. Security, because it helps obfuscate the reason why access is denied by revealing less information about the file system the site is running on. This change may have side effects and if there are concerns I'll split it into a separate patch.

There are also minor changes to .htaccess RewriteRule flags that should have no change on behaviour, but better document apache's handling of the .htaccess file.

CommentFileSizeAuthor
#6 217921.patch3.14 KBBevan
#3 217921.patch1.6 KBBevan
deny_access_files.patch3.13 KBBevan

Comments

BioALIEN’s picture

Subscribing, I see merit in the problem this issue is resolving. Displaying the server's 403 could stand better against DDoS attacks on a site with cache = off as it bypasses Drupal and the heavy load that comes with it.

Bevan’s picture

It also moves the ID CVS keyword to the top, for consistency

Bevan’s picture

StatusFileSize
new1.6 KB

This is a drupal 5 version with only the changes that modify behaviour.

meba’s picture

Status: Needs review » Needs work

Did not apply to HEAD. Otherwise, it works as expected. I like it, but is there a way to detect a 403 without an additional if() in init?

pwolanin’s picture

The concern about .txt files was addressed in this issue: http://drupal.org/node/79018 So to the extent that this is only about making .txt files non-readable, this is a duplicate issue.

However, the 403 redirect part might be a useful addition.

Bevan’s picture

Status: Needs work » Needs review
StatusFileSize
new3.14 KB

Rerolled for head. No changes.

#79018 seems to have a lot of conflict, bias, opinion and subjectiveness. I didn't read it to the end. It also seemed important. Is there any conclusion on the whether this is a good approach or not?

pwolanin’s picture

Status: Needs review » Needs work

The conclusion from the other issue was that trying to block the .txt files in .htaccess is unacceptable (for a variety of reasons), and that the effect on security is really theatrical, not substantive. So, however you feel about that, I'd suggest limiting this patch to the 403 part.

Bevan’s picture

Thanks. I'll do that.

lilou’s picture

bump.

sun’s picture

not_Dries_Buytaert’s picture

Version: 7.x-dev » 6.12

Why not (have a Drupal update):
1) move the txt files into an 'admin documentation'-subdirectory,
2) put a htaccess file in that directory (disallowing access to those txt files),
3) chmod so the directory and its content become unaccessible to web visitors?
It's a simple AND portable solution without negative impact on performance or security, yet making Drupal more secure out-of-the-box.
No solid objections have been raised against such solution, so far.
After all the debating about this since 16 August 2006 (in http://drupal.org/node/79018 which has been locked), visitors can still browse the txt files (i.e. easily fingerprint).

By default, Apache (2.2.8) includes a file called "error\include\bottom.html" at the bottom of http error messages. So, the redirect ErrorDocument patch should be applied to all http error messages (not only 403).

Status: Needs work » 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.