Skip to content

Commit

Permalink
(MODULES-2682) Update Apache Configuration to use FilesMatch instead …
Browse files Browse the repository at this point in the history
…of AddHandler for PHP Files.

The issue with this is that the extension handling behaviour of apache is
not well known by most php developers, and many php scripts are open to
security issues if this configuration is used (most commonly these scripts
handle upload forms which white list image extensions). For example
foo.php.jpg will be handled by php.

Many distro's no longer use AddHandler in their default config:
  http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/vivid/php5/vivid/view/head:/debian/php5.conf.

The PHP manual also recommends avoiding it:
  http://php.net/manual/en/install.unix.apache2.php#example-20

This is based on Alejandro Bednarik's <alejandro.bednarik@gmail.com> original
fix, I added proper regex escaping, and a changelog entry. All bugs are mine.
  • Loading branch information
DavidS committed Nov 26, 2015
1 parent bf86436 commit ddb6e4f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
@@ -1,3 +1,14 @@
## UNRELEASED
### Summary

TODO

###

#### Security

* apache::mod::php now uses FilesMatch to configure the php handler. This is following the recommended upstream configuration guidelines (http://php.net/manual/en/install.unix.apache2.php#example-20) and distribution's default config (e.g.: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/vivid/php5/vivid/view/head:/debian/php5.conf). It avoids inadvertently exposing the PHP handler to executing uploads with names like 'file.php.jpg', but might impact setups with unusual requirements.

## 2015-11-17 - Supported Release 1.7.0
### Summary
This release includes many new features and bugfixes. There are test, documentation and misc improvements.
Expand Down
2 changes: 1 addition & 1 deletion spec/classes/mod/php_spec.rb
Expand Up @@ -88,7 +88,7 @@
let :params do
{ :extensions => ['.php','.php5']}
end
it { is_expected.to contain_file("php5.conf").with_content(/AddHandler php5-script .php .php5\n/) }
it { is_expected.to contain_file("php5.conf").with_content(Regexp.new(Regexp.escape('<FilesMatch ".+(\.php|\.php5)$">'))) }
end
context "with specific version" do
let :pre_condition do
Expand Down
5 changes: 3 additions & 2 deletions templates/mod/php5.conf.erb
Expand Up @@ -14,8 +14,9 @@
#
# Cause the PHP interpreter to handle files with a .php extension.
#
AddHandler php5-script <%= @extensions.flatten.compact.join(' ') %>
AddType text/html .php
<FilesMatch ".+(<%= @extensions.flatten.compact.collect { |s| Regexp.escape(s) }.join('|') %>)$">
SetHandler php5-script
</FilesMatch>

#
# Add index.php to the list of files that will be served as directory
Expand Down

1 comment on commit ddb6e4f

@bruhadavid
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not let the admin choose which way to go? Most of our servers use Apache content negotiation (MultiViews option) with PHP and this change disables it.

Please sign in to comment.