So this is a major can of worms, but a frequently asked feature (faf?!) is to allow people to edit their ".htaccess" (their virtual host, in effect) to add (for example) rewrite rules, php settings or other stuff.

This can be a major security issues, on multiple vectors:

* if unchecked, people could inject new vhosts and therefore inspect any file on the system using a blob like this:

</VirtualHost>
<VirtualHost *:80>
 ServerName hahahagotcha.com
 DocumentRoot /etc
 <Directory /etc>
  allow from any
 </Directory>

* even if we blacklist VirtualHost directives, there are a lot of other directives that can be potentially harmful (load module?!)
* a whitelist would need to be very precise to control well potential attacks

That's all I can think of right now, but there's probably way more nasty things that could kick in here.

What I would suggest to begin with would be only to allow RewriteRule directives, and be very strict about the allowed flags.

Additional directives could be allowed after thorough considerations.

The implementation would require an extra field in the site form that would get passed to the task, and suck that in the vhost template in the backend. Should be fairly simple to implement.

Comments

j0nathan’s picture

j0nathan commented

Subscribing.

omega8cc’s picture

omega8cc commented

Hmm.. I would never open this for end-users (sites owners). They could create vhosts to access anything they want also in other sites, if they are creative enough. Too big security hole, imo, even with strict filtering in the front-end. Note: you would need to use/create filtering for Apache and Nginx directives.

But it could be still useful (and I have it already almost ready for Nginx) to have an extra include file for every vhost (initially created empty) so the server admin could edit it per site, just like it is possible with local.settings.php now, but without .htaccess (for performance reasons and compatibility with Nginx).

anarcat’s picture

anarcat commented

I had the same concerns as you, omega8cc. However, as I described in the original post, i think it would be possible to have a whitelist of allowed line patterns that would make it safe to allow arbitrary apache/nginx code. Of course, the whitelist would be server-specific. In apache, out of the box, I can think of this regex:

^RewriteRule\s+[\w/._\?-]+\s+[\w/._\?:-]+(\s+\[(L|QSA|R(=\d+)(,L|QSA|R=\d+)*\]$

The above pattern wouldn't allow some regex metacharacters or backslashes, but it's a start, and I think it's secure and flexible enough for basic use cases..

steven jones’s picture

steven jones commented

This could be handled quite easily by 'contrib' right? You'd need a frontend module and a backend service to store the data, then a little implementation of the hook that gets called when the vhost is re-created.
Given the security implications of the feature, it might be good that Aegir admins would need to go through the extra step of installing and extra module + drush plugin.

anarcat’s picture

anarcat commented

There seems to be work in this direction here: https://github.com/EugenMayer/hosting_vhost_extra

From #1063146: UI: Add an option to add extra vhost settings to sites, marked as a dupe of this ticket.

EugenMayer’s picture

EugenMayer commented
Status: Active » Needs review

repo is online again please test the version. For now you have to move the vhost_extra.drush.inc into your aegir ~/.drush folder. Maybe that can be solved differently, migt5

crea’s picture

crea commented

Subs

anarcat’s picture

anarcat commented
Status: Needs review » Needs work

Okay, that's a good start. Unfortunately, it does absolutely no validation of the extra content added to the vhost. My guess is that some validation should occur in _hosting_vhost_extra_vhost_settings() but i'm having trouble finding my way around the code.

I'm also unsure as to why we would need SSL or non-SSL settings, if the extras are per site anyways...

Did I read the code right? :)

Thank you for your contribution.

steven jones’s picture

steven jones commented
Status: Needs work » Closed (won't fix)

Seems like we're agreed that contrib is the solution here.

helmo’s picture

helmo commented
Issue summary: View changes

For reference, one contrib solution is in https://www.drupal.org/project/hosting_injections