Some of sites use symlinks within the site folder to hold content elsewhere. It is useful to have provision-backup support following symlinks as an option (off by default, to maintain current behaviour) - especially for full backups when doing remote imports.

Note: we're still using 6.x-2.x, so this patch applies to that. When I have some more time, I'd happy be happy to roll a similar patch for 7.x-3.x. I'm not really expecting this patch to get comitted given that 6.x-2.x is no longer support, but at least it'll be here for folks to look at or hack on.

CommentFileSizeAuthor
#2 provision-backup_follow_symlinks_option-2719577.patch1.64 KBkienan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kienan created an issue. See original summary.

kienan’s picture

kienan CreditAttribution: kienan commented
FileSize
provision-backup_follow_symlinks_option-2719577.patch1.64 KB
omega8cc’s picture

omega8cc CreditAttribution: omega8cc commented

What if someone will create (or even upload) there a symlink to /var/aegir/.drush/server_master.alias.drushrc.php to get Aegir "root" password to all hosted sites databases?

gboudrias’s picture

gboudrias CreditAttribution: gboudrias at Praxis Labs Coop commented
Status: Active » Closed (duplicate)
Related issues: +#2226759: Add -h to tar command for backups

This is a good patch for those who want it it, but we've already had this discussion:
#2226759: Add -h to tar command for backups
#837960: Tar command to dereference symlinks
#2185627: Add a storage service: Allow configurable file_directory_path per site.

I don't know if this patch addresses Anarcat's comment in the last close: https://www.drupal.org/node/2226759#comment-9338693

I'm afraid this is a duplicate. As far as I can tell people are trying to resolve this in #2185627: Add a storage service: Allow configurable file_directory_path per site.

Feel free to reopen if I'm mistaken, this has been mentioned a lot so I'm having a hard time keeping track.

kienan’s picture

kienan CreditAttribution: kienan commented

gboudrias, I hadn't realized there had already been an extended discussion about this, my apologies and thanks for bringing those to light.

This patch does nothing to prevent any of the scenarios described by omega8cc or anarcat. However, it only presents the option to the user doing remote site imports (with the patched hosting_remote_import module) - an action that is not necessarily accessible by default, and is by no means the default course of action for provision-backup. This lines up with the criterion proposed by anarcat.

Certainly https://www.drupal.org/node/2185627 may end up being a more engineered solution that could try and prevent the two attacks by meticulously having administrators specify which folders have what kind of backup schemes.

Another option could be control how symlinks are created on the system by the webserver / users in some manner. Such as:

* chrooted sftp (if you give access to any users in such a way) (can't link outside of their "/")
* disabling the creation of symlinks by php code (affects Archive_Tar in system.tar.inc, backup_migrate possibly, a VBO action, etc.)

I also feel it's important to note that drush ard already supports this option through --tar-options, and it's accessible in much the same way as provision-backup is in aegir installations.

(Note: given https://www.drupal.org/node/2185627 renders this patch is way more abusable, but unless things have changed - the entire system only remains intact in tightly controlled scenarios as it is).

Edit: missed a verb

omega8cc’s picture

omega8cc CreditAttribution: omega8cc commented

@kienan Just a quick feedback on symlinks restrictions either via SFTP chroot, or PHP -- none of those restrictions prevent user with access restricted to his site files directory from adding a symlink. All he needs to do is to create that symlink on his local machine and then hide it inside simple tar or zip archive, then upload the archive and extract it in the files directory:

$ ln -s /var/aegir/.drush/server_localhost.alias.drushrc.php imgage12.jpg
$ ls -la imgage12.jpg
lrwxr-xr-x  1 albert  staff  52 May 11 22:04 imgage12.jpg -> /var/aegir/.drush/server_localhost.alias.drushrc.php
$ tar cpf - imgage12.jpg | gzip -c > archive.tar.gz
$ ls -la archive.tar.gz
-rw-r--r--  1 albert  staff  168 May 11 22:06 archive.tar.gz

Then on the server just:

tar -xzf archive.tar.gz

Remember, the symlink will be followed by Aegir backend user, with full privileges to /var/aegir/.drush

kienan’s picture

kienan CreditAttribution: kienan commented

omeg8cc, that's a good point - I hadn't thought of that. Perhaps it's not possible to prevent that kind of attack at that level then.

If one takes a step back to look at where, and when this code is called:

1. provision-backup called by hand (or exec, etc.) with --follow-symlinks
2. arbitrary code that sets the follow-symlinks drush option before calling the provision backup

A user who doesn't already have access to run the command or arbitrary code can't really do anything with this. Regular provision-backup tasks are unchanged, so symlinks aren't followed and can't be abused with a user that has access to start backups via the interface, and then to download such backups.

The prime motivator of this patch, is so that a complete site can be sent with hosting_remote_import. If remote_import & hosting_remote_import are patched as such then a user with access to start a remote import can set the option to trigger a backup that follows symlinks.

As such, this option doesn't provide a new way for someone to execute arbitrary code, but does have some traps in case an unwary administrator runs this and ships full backups without checking where all the symlinks go.

When I imagine an aegir admin having to make a full dump of a site, if provision-backup isn't sufficient the logical alternative is drush ard and if the site is known to use symlinks legitimatey through.ly, then running that would clearly lead to similar situations as described by symlinking to /var/aegir/.drush/*, another site, or having a recursive links inside the site.

There are certainly things that could be added here to improve the patch:

  • dereference only one level (files, private, themes, modules, etc.)
  • enforce that symlinks point to folders not files or devices
  • that the symlink doesn't point to $AEGIR_HOME/.drush

  • if not derefering only a single level, have a recursion limit check (tar doesn't support this, so it would have to be done by something in provision-backup)
  • the symlink doesn't point to another site that's hosted in aegir (+ check the aliases)

Are there other checks that can be added here? In any case, if those are all added then the solution starts to look like a mutated version of the provision_storage/hosting_storage with fewer advantages.

In summary, while it's conceivable to safety pad this option in a number of ways, my opinion is that unless a user has already gained the ability to execute arbitrary code run by the aegir user (in which case they can steal all the data or destroy everything anyway), this option presents only new attacks (which can be done anyway with drush and tar as it is) and not new points of entry.

However, the way in which I use aegir is surely not representative of all the ways in which it's used. I'm interested in hearing how provision-backup will be called (with this option) by users who don't already have a heightened level of access.