Local.settings.php can stop working due to file ownership/permissions

I disagree - the local.settings.php is managed by the user, so the user should make sure the permissions are OK. Keep in mind that local.settings.php could be created with an ownership that would make aegir incapable of changing the mode (ie. if local.settings.php, which would seem to be a logical use for it), which would then make provision-verify fail.

I would rather leave this up to the user. No?

I guess I see the point, thanks for updating this.

I am not sure I understand why people would have security-sensitive stuff in local.settings.php when the DB password is already in the settings.php, but I guess it can happen.

We can probably fix perms on verify, but note that if the user screws up the permissions when they edit the file, there's always going to be a window when the file will be vulnerable, so this may create a false sense of security..

Subscribing

I observe the following behavior: after migration local.settings.php group ownership resets from www-data to aegir, leading to webserver not being able to access the file, so local settings stop working after migration. I consider this a bug, not a feature

What happens here: tar extracts files ignoring ownership, so group ownership resets to running user (aegir).
Would making tar keep ownership add any security risks ?
I.e. change to
$command = 'gunzip -c %s | tar --same-owner -p -x -f -';

The workaround is to set local.settings.php world-readable, but it adds security implications (you can't place secure credentials there, for example)

subbing

We have this happening on some aegir installs of ours, but not others. The local.settings.php becomes owned by aegir:aegir and so the web server can't read it, which screws things up nicely. Needs some investigation.

I think it is rather docs candidate and admin's responsibility to create this file world-readable, as there are no passwords etc stored probably, so it doesn't matter if the file ownership will change.

However, I never experienced that, even for sites cloned/migrated - Aegir never touched this file for me and it was always moved as-is.

Seeing this problem on our install as well. Particularly frustrating when permissions get changed while migrating a site and thus botching the whole genius of Aegir.

Setting world readable is not really an option, as this has security implications if things like API keys for external services (Facebook, AWS, etc) are stored in local.settings.php It seems to me to be the appropriate place to store stuff like that, but if anyone has any suggestions as to where to store that sort of info, I'm always open to ideas!

Here's a quick fix:

Add the following code to a MYFILE.drush.inc file in /var/aegir/.drush

function drush_MYFILE_post_provision_deploy() {
  //Make the local.settings.php file readable by the webserver
    provision_file()->chgrp(d()->site_path . '/local.settings.php', d('@server_master')->web_group, TRUE)
      ->succeed('Changed group ownership of file @path to @gid')
      ->fail('Could not change group ownership of file @path to @gid');
}

Tested and working with Apache.

If we're interested in updating Provision to include this code, should be as simple as adding this code somewhere in deploy.provision.inc

I think this would make sense, *if* this will not fail if local.settings.php is absent or if aegir can't change the perms. Can you roll out a patch that would do that?

Thanks.

Sure.

Here's a patch, doesn't try to do anything with the permissions, but keeps the group correct, so the end user is free to set the permissions as they need.

Or we could create an empty local.settings.php file by default, with correct ownership/group and permissions and then always check/set them only in function provision_drupal_drush_exit(), so only once on successful verify. Note that attached example patch is for 6.x-2.x

The patch from #18 is designed to break the site, as it never sets correct gid when the file exists. Fixed patch for 6.x-2.x attached.

Thanks for the patch, but I think that we should make sure that the permissions are at least enough to not cause any errors, but we shouldn't force the permissions to 0440 unless we have good reason to?

settings.php gets 0440.
As that was done as a security measure why not for local.settings.php as well?

The reason against making it read-only it is convenience ... ?

Well, local.settings.php is supposed to not really be managed by Aegir at all, it's just for people to pop their special config in anyway. So we should aim to ensure that it has the permissions that we need at least, but we shouldn't then start taking whatever permissions it has away if people have set them like that.

thanks. this is nice.

I agree that having it be the same settings as settings.php is a good idea. As an admin if I want to go in and edit, I just need to take the extra step.

Up until now I have constantly had to go in and do the chown/chmod dance. Thanks for this patch.

Still applies to 7.x-3.x and still sounds reasonable to add... Any objections left?

the patch works for me

The chmod and chgrp part was identical between the if and else blocks... so this new patch simplifies it a bit.

Committing.