Copy default.settings.php to settings.php during install IFF webserver owns files (FTP on shared hosting)

Here's the original issue, for reference #99011: Remove sites/default/settings.php and ship with sites/default/default.settings.php instead

Would it be possible to have the d.o. packaging scripts include settings.php (copy of default.settings.php) automatically so that it would be available for people downloading the tarball?

That way we could keep the current behavior for CVS users, possibly (and keep settings.php out of people's patches).

Talked with Bojhan in IRC briefly to clarify, that he is proposing that the downloaded package comes with an existing settings.php. This is the only way to do this because of the issues with the early 6.x releases creating the settings.php for them and the webserver then owning the file (sorry I don't have the issue number here).

My only concern here is if the download tarball comes with a settings.php file, then upgrading may be more difficult when a user downloads a new version and attempts to copy it over their old files.

Must have cross posted......

So...

we used to have settings.php ship with Drupal - there were two issues with this:

1. People tend to commit changes to their settings.php, whoopsie.
2. When you upgrade there's a risk of overwriting your settings.php

Then we went to creating the settings.php ourselves - but webserver owned files on shared hosting are a pain.

Now we have the bizarre step of copying default.settings.php to settings.php

update.php can now regenerate settings.php database info, so #2 is dealt with. On #1 - I think Keith's idea of having settings.php excluded from a CVS checkout but included in tarballs by the packaging script makes sense. Those who can checkout from CVS can deal with the extra step of copying default.settings.php to settings.php. So really this issue is about a change to the packaging script, assuming we leave the default.settings.php step for CVS users, changes to core would be tiny.

sorry.

Is it within the realm of possibility that we add a CLI installation script? It could generate the settings.php and set the correct permissions. Might also consider adding one to finish the install by re-chmod'ing and removing write permission.

Updates obviously wouldn't require re-running the installation script. Thus we can avoid the settings overwriting problem.

Another possibility, if writing an installation script is an option, would be to write a script that initially did the creation and chmod of the file, paused until the user was finished with the web installer, and then finished by re-chmoding the settings.php.

This could actually be done automatically with a little bit of polling. And it would (I would argue) greatly increase the chances that settings was correctly locked down after the install.

Something chx came up with on IRC one time is leaving settings.php in core so that you didn't have to do the asinine step of *copying a file* to get installation to proceed. And instead, Drupal would write its database settings to something like db.php, which would not be included in core.

Advantages:
1. No more stupid renaming of files. YAY!
2. Would prevent people from accidentally committing their DB settings, since this would be kept in a separate file NOT in Drupal's CVS. HOORAY!
3. Would allow people using private version control systems such as SVN to manage their sites to check in settings.php, which might contain settings overrides, variable hard-coding, custom_url_rewrite implementations and lord knows what else which you do want under version control without also keeping the production site's database password in SVN. WIN!

From the usability side of things, users would probably need to chmod their sites directory (and back again) for this to work. But anyone who installs web apps on a regular basis is used to having to do chmodding. What they're not used to is the step where they need to copy a file and re-FTP it up at the first step of the installation, which creates a serious DrupalWTF.

Needs testing, though, as well as consideration for larger ramifications, if any, for people who have deployment scripts or whatever.

Oh, and incidentally, although #99011: Remove sites/default/settings.php and ship with sites/default/default.settings.php instead was where this issue originated, the file copy requirement didn't rear its ugly head until ~Drupal 6.2 as a security fix. We were teaching a Lullabot workshop at the time and I remember poor Gábor got a face full of rage from Addi and I about it because we got to see first-hand how much #fail it caused real life users. ;) Sorry, Gábor! ;D

So now instead of having one settings file, we have two? One for settings that apply to all servers, and one with settings that apply to only the current server?

I'm not sure I see how this solves any problems for the average user, and sites dealing with more robust configuration needs can accomplish this already using an include from settings.php.

What this *does* require is write access to an entire directory instead of just a file. That seems like a security step backwards without a usability step forward.

Maybe I'm just not clear on what the real issue is....

So we trade one critical UX issue (installing) for different, more severe UX issue (upgrading) ?

This does indeed seem like a step backward. A site is updated ~ 10 times for every 1 install. Every time I update a Drupal 5.x or 4.7.x site this bites me, and I've probably done it 100 times (not a hyperbole).

I'm not saying the issue shouldn't be addressed at all; but I just don't see re-adding settings.php to the package as helping.

Also, in my experience most beginners are running on shared hosts, which means file ownership isn't an issue. (Because they are running FastCGI or similar and the user account is the same as the httpd process owner.) So the installer automatically does the right thing for them by creating the settings.php file.

My problem with this system was "why can't I just rename default.settings.php to settings.php? why do I need both files for the installation to start?"
Question: wouldn't be more user friendly and easier to just ask to chmod default.settings.php and in the installation process to rename it to settings.php and when the installation is finished chmod it? This way the ownership of the file won't be a problem anymore and everything else will work exactly like it is now.

I did a impromptu survey of our projects.

Gallery http://gallery.menalto.com/ makes you edit a file (preinstaller.php) and add a password and change a directory's permissions.
Simple Machines forum http://docs.simplemachines.org/index.php?topic=5 allows you to enter your ftp username and password to chmod the directories correctly or the is a manual option during the installer.

Those are just two ways other projects have solved this problem.

What I do not understand is that drupal has this great multisite ability. Why don't we just leverage that?

Have a page that informs the site admin that they must chmod the "sites" directory (either via automated ftp method or manually) Then the install system will create (or ask you to create a directory) under the sites directory for your site. Then the install system will generate the "settings.php" (or allow you to download it to upload) for you.

The reason for the "()" is to get around the problem that you may lose access to you settings.php if the webserver creates the file for. (that has already been covered)

We just need to do a better job of walking users though the software install and maybe have more then one way to get the job done. The users most likly are not dumb, but they do not know "the Drupal way". Let inform them of "the Durpal Way" and the options.

I took me months of using drupal to understand that you could use the same core install for multiple websites, because the information was stuck down in a README file and not part of the installer.

A couple suggestions.

1) As mentioned above and through my experiences, most hosts and servers are not properly secure, so why not attempt to do all that is necessary automatically, and if pass, tell the user they should look into some better practices (though this could be confusing), or if this is not possible because of permissions, then do the manual way.

2) I like the idea of a separate db.php file, but I think the suggestion is kind of backwards. Keep settings.php as site specific and sensitive things like db passwords, then create a second settings file (like site-settings.php) that can be edited by advanced users where someone can change all the other stuff that is put into settings.php and put that in a place that will get versioned.

-1 to this.

By shipping settings.php we're changing a (one time) install problem for a (multiple times) upgrade problem.
Note that upgrades are sometimes done by lesser gods then the install, and also, overwriting the custom settings is never good.

I think the solution is, to first automatically create a test file, check if the test file is owned as the same user as the default setting, and delete the test file.

If the test file was owned by the same user, we can (I assume safely) create the settings.php automatically, and if not, the manual method must be used.

From an enduser standpoint I can say that creating the settings.php manually was manageable. But I was pretty inexperienced and therefore had read about the installation process carefully in advance and knew about the upcomming task. I guess that only users who are not prepared get into trouble and that might be those who use Drupal for the first time.

People having trouble with this step might either quit on Drupal immediatly or choose an auto installer what might create other problems.

I think that the current situation is managable and probably not worth the trouble we might run into afterwards. So unless a solution can be found that solves not only the initial problem but also all upcoming ones I would go with proper documentation and guidance during the installation process. It would be wise to create an updated how-to video because I personally feel pretty comfortable doing "new" and "complicated" stuff after preparing by watching a decent screencast.

First off I do not like shipping with a settings.php file, because of upgrade problems. I think the way to solve this problem is to take the instructions that are in the INSTALL.txt and put them as part of the install process. This can be done as part of the non-expert install profile.

Combine the 2 approaches outlined in this thread.
DB settings stored in db.php: http://drupal.org/node/418302#comment-1452848
Copy settings.php if it doesn't exist: http://drupal.org/node/418302#comment-1452660

Link to some simple chmod guide & directions like this if it doesn't work.
http://drupal.org/node/367081#comment-1504894

Is this really that hard to do?

if (!file_exists('settings.php')) {
  if (!copy('default.settings.php', 'settings.php')) {
    watchdog('drupal install', t('Your server sucks. Go here %link to fix this problem.', array('%link' => 'drupal.org')));
    drupal_set_message(t('Your server sucks. Go here %link to fix this problem.', array('%link' => 'drupal.org')));
  }
}

I like #18 2. a lot. I recently installed phpMyadmin and they have a default-settings.conf which is loaded unless a specific my.conf (or something similar, I forget the exact file names, you get the idea, though) is created in which case my.conf overrides default-settings.conf.

I think this system gives us the best of two worlds. We could ship with settings.php for unexperienced users and everything custom for advanced users could live in mysite.settings.php or something. The file names are arbitrary: we could actually go with default.settings.php (because we already have that) or with db.php as proposed above.

I'm just going to +1 and subscribe, as I don't entirely understand the problems that prevent us from making this better.

I've just installed a new test site with 6.12, and a directory sites/all/default/files got created: if Drupal can do this, surely it can create the settings.php file too?

Your right. Drupal can do that (and it did a matter of time ago), but the problem is that under certain conditions it is not safe enough to have the file created by the webserver, ergo the user must create it him/herself.

I would like to see an upgrade distribution. A separate distro for each upgrade can provide information on what changed, and also allow for people who need to change core to be able to upgrade more reliably. From removing CSS entries to optimizing functions to perform better on a case by case basis, this is important. One key module that requires core changes for better performance is Block Cache Alter

I think this also solves the main problem with including settings.php within the distro. If changes to settings.php need to be made, a separate distro will bring these changes to light, simply because the file would be included in the upgrade distro.

+1 for some kind of separate package for people who are upgrading vs. installing for the first time.

-1 for upgrade packages.

db.php is also not an option, as that will cheate the issue: ship with db.php

the solution to this problem is, are we able to determine in which cases it's (un)safe to write setting.php during install.
this is the road we should pursue, simplified install on "good" hosters, no changes on "bad" hosters.

if you hack core, you can simply diff the clean version you hacked and the new clean version to see (and appply) the changes...

--- drupal rule #1) you never hack core ;)

We discussed this yesterday in #drupal-usability, hopefully this is a reasonable summary of the issues:

1. Shipping with settings.php means that on upgrade you can overwrite and wipe out your database credentials, not good.
2. Having Drupal write a settings.php if it doesn't already exist means that if you're on certain shared hosting configurations and someone on that server gets control over apache, there's a decent chance they can execute arbitrary php by writing to your settings.php. It's also possible that users would be unable to edit their own settings.php if they needed to because it'd be owned by apache.
3. Not shipping with it (status quo) means every new install has to manually copy the file before it can proceed.

Out of the three options, #3 is the least worst for various reasons, but still annoying. We can't simply go back to #1 or try #2.

I think the most likely solution to this would be to use some of the code from #395472: Plugin Manager in Core: Part 1 (backend) - we still encourage people to copy/create the settings.php file, but if it's not there, then the plugin manager code can prompt for their ftp username and password, then write the file (using their hosting account user, not apache). That way it saves having to be on the command line or uploading files via ftp for this part of the installation, and from a file permissions issue it'll be the same as people creating it themselves.

Marking postponed on that issue.

catch, that sounds like a good solution. I just upgraded a WordPress plugin the other day by entering my FTP username & password into an admin screen. I think Joomla has a similar feature. This would bring Drupal closer to others in the industry on this important issue.

Breaking out of the install process to copy a file like we do now is feeble.
Using alternate credentials is a radical way of doing it. I guess it'll work.
Dodgy, but less dodgy than getting the webserver to write its own configs. Man it's hard to explain to people why letting web scripts write to their own code is a Bad Thing ^TM

That said, I've seen many more real server exploits from compromised logins than I have from compromised self-modifying code...

Every update I have to remove the core .css file, the .htaccess file, and the system.php file, before I update. On D5 I have to remove the javascript files too. I do not have the ability to do a diff on the files, so if any of these files change I will probably not know that they did. On D6, I will also have to remove the block module file. For me an update version is a viable solution to the original issue, and one that I like. Some people want it, others don't. However those who don't want to use an upgrade version of Drupal can always download the full version! The only question I have is, why don't we do this, and would the people who don't want to have settings.php shipped feel that this is a reasonable solution for there problem too?

dman makes a good point, login credentials are a big issue. Having Drupal do FTP correctly seems like a whole new area of security. Also, does plugin manager only use FTP, not SFTP? Doesn't that seem insecure? Also, transmitting your login credentials is insecure without a secure connection. There is also the issue of how the credentials are stored.

Personally, this seems like FTP adds a whole new layer of security concerns.

plugin manager does ssh / sftp / ftp afaik, and doesn't store credentials. Please read the other issue where these questions are already answered more than once.

@#32
you might want to start using Kdiff3, a three way diff utility with auto-patch support.
- A = old drupal version (clean)
- B = old drupal version (hacked)
- C = new drupal version (clean)
output = new drupal version, with hacks added.

also, CSS is cascading, edit your theme, not your core CSS.

--- drupal rule #1) you never hack core ;)

Though I respect and know that the CSS can be overridden, when you have over-ridden more then 50% of the file then its a waste of time to download (on d5). D6 CSS compression should eliminate this duplication. But I regress talking about rule #1 is not the topic of the post.

This post was marked as a dup (I didn't know about this thread) so here it is again (in the right spot though).
---------------------------------------------------------------------------------------------

I was going to use this "patch" as part of my dissertation for getting a CVS account, but I was advised to do otherwise... Either way I'd like to know everyone's opinion... In a nutshell, the following explains a potential core upgrade that would eliminate the "Verify Requirements" step of a fresh installation, making the Drupal platform much more "user-friendly" because the creation of a separate settings.php file now becomes automated.

I have to say I've grown tired of manually creating a settings.php file with every single install I do, so I came up with a (possible) way to automate that process. I didn't see anything in the documentation about doing so being a 'negative' thing (as far as security issues, at least) so I decided to try a few tweaks. I'm using a loaner-computer right now, so I don't have access to "diff" to create a patch (which again, otherwise I'd be applying for my CVS account, but whatever). What I did instead though was download the 6.x and 7.x dev tarballs & applied the code directly. I was primarily testing for 7.x (because of the whole "D7UX" thing) but I also did test it on D6 and had no problems. Please review the following and as usual, any feedback would be appreciated.

Here's what the patch would look similar to (if I did it correctly...):

--- install.php
+++ install.php
@@ -911,9 +911,11 @@
         $writable = drupal_verify_install_file($settings_file, FILE_READABLE|FILE_WRITABLE);
         $exists = TRUE;
       }
-    }
-
+}
     if (!$exists) {
+      file_put_contents("$conf_path/settings.php", '');
+    }
+       elseif (!$exists) {
       $requirements['settings file exists'] = array(
         'title'       => st('Settings file'),
         'value'       => st('The settings file does not exist.'),
@@ -926,10 +928,14 @@
         'title'       => st('Settings file'),
         'value'       => st('The %file file exists.', array('%file' => $file)),
       );
-      if (!$writable) {
+     }
+      if (!$writable) {
+           chmod("$conf_path/settings.php",0644);
+         }
+      elseif (!$writable) {
         $requirements['settings file writable'] = array(
           'title'       => st('Settings file'),
-          'value'       => st('The settings file is not writable.'),
+          'value'       => st('The settings file either does not exist or it is not writable.'),
           'severity'    => REQUIREMENT_ERROR,
           'description' => st('The @drupal installer requires write permissions to %file during the installation process. If you are unsure how to grant file permissions, please consult the <a href="@handbook_url">online handbook</a>.', array('@drupal' => drupal_install_profile_name(), '%file' => $file, '@handbook_url' => 'http://drupal.org/server-permissions')),
         );
@@ -941,7 +947,6 @@
         );
       }
     }
-  }
   return $requirements;
 }

(Sorry if that's way off...) I also attached a snippet of the actual code from the 'enhanced' version of install.php which, when used, makes the core automatically creates the settings file and then, if necessary, configures the permissions for that file to make it writeable. Replace everything below (and including) line 897 until with the following, until you reach the point where the bottom line of this snippet matches back up:

  // If Drupal is not set up already, we need to create a settings file.
  if (!$verify) {
    $writable = FALSE;
    $conf_path = './' . conf_path(FALSE, TRUE);
    $settings_file = $conf_path . '/settings.php';
    $file = $conf_path;
    $exists = FALSE;
    // Verify that the directory exists.
    if (drupal_verify_install_file($conf_path, FILE_EXIST, 'dir')) {
      // Check to make sure a settings.php already exists.
      $file = $settings_file;
      if (drupal_verify_install_file($settings_file, FILE_EXIST)) {
        $exists = TRUE;
        // If it does, make sure it is writable.
        $writable = drupal_verify_install_file($settings_file, FILE_READABLE|FILE_WRITABLE);
        $exists = TRUE;
      }
    }
    if (!$exists) {
      file_put_contents("$conf_path/settings.php", '');
      }
	elseif (!$exists) {
      $requirements['settings file exists'] = array(
        'title'       => st('Settings file'),
        'value'       => st('The settings file does not exist.'),
        'severity'    => REQUIREMENT_ERROR,
        'description' => st('The @drupal installer requires that you create a settings file as part of the installation process. Copy the %default_file file to %file. More details about installing Drupal are available in <a href="@install_txt">INSTALL.txt</a>.', array('@drupal' => drupal_install_profile_name(), '%file' => $file, '%default_file' => $conf_path . '/default.settings.php', '@install_txt' => base_path() . 'INSTALL.txt')),
      );
    }
    else {
      $requirements['settings file exists'] = array(
        'title'       => st('Settings file'),
        'value'       => st('The %file file exists.', array('%file' => $file)),
      );
    }
	if (!$writable) {
      chmod("$conf_path/settings.php",0644);
    }
    elseif (!$writable) {
      $requirements['settings file writable'] = array(
        'title'       => st('Settings file'),
        'value'       => st('The settings file either does not exist or it is not writable.'),
        'severity'    => REQUIREMENT_ERROR,
        'description' => st('The @drupal installer requires write permissions to %file during the installation process. If you are unsure how to grant file permissions, please consult the <a href="@handbook_url">online handbook</a>.', array('@drupal' => drupal_install_profile_name(), '%file' => $file, '@handbook_url' => 'http://drupal.org/server-permissions')),
        );
      }
    else {
      $requirements['settings file'] = array(
        'title'       => st('Settings file'),
        'value'       => st('Settings file is writable.'),
        );
      }
    }

  return $requirements;
}

/**
 * Add the installation task list to the current page.
 */

(Also sorry for the messiness... Been a LONG day). :)

If I understood correctly this approach again requests letting the webserver write settings.php, which is not an option as has been said before (see #29).

Actually this was postponed on #395472: Plugin Manager in Core: Part 1 (backend). Since that is in now, setting to active for discussion about a proper approach to this problem.

What if we shipped with settings.php pointing to a SQLite database install of Drupal?

See #41

Regarding #42:
#332303: bootstrap from sqlite
Seems to be stalled, but not a won't fix.

Even after reading the documentation, I'm still not quite clear why my suggestion (#37) would pose a security threat but I'll take everyone's word for it.

On the other hand, I understand that what I wrote was having the server write the setting.php file; but what would be wrong with shipping a completely blank file that pointed to (literally) nothing? Settings.php does not need to be filled in/populated manually -- the core already does that part (D6 as well as D7). It doesn't care if the file is empty or if it has the Bible written in it; all it wants is a file named settings.php to exist whose perms are 644 and it'll take care of the rest. If we supply an empty file, it should be no different than having the user upload a file named settings.php with the exception that the user won't get an annoying red form_set_error() message on their screen telling them to do something that might as well already be done.

If I lost you somewhere, please see the following example: http://docs.google.com/Present?docid=ddfnfxfh_0dmvx62g4&skipauth=true

+1 for shipping with settings.php

It was there in the past and not that millions of site got unsafe with it being shipped.
If there is problem with security, solve the security issue please do NOT stop shipping with settings.php

first of all, let me make some thing clear.

- ANY option where the webserver/installer writes a new file is NOT AN OPTION, since this'll lead to (webserver) permission problems later on.
- ANY option where the file is shipped together with drupal is NOT AN OPTION, since this'll lead to upgrade overwrite options.

now, taking these two restrictions in account, i see two possibilities:
1) Keep Current Situation, Provide better documentation.
2) Provide multiple packages:
- an Install Package, containing all files
- an Upgrade Package containing all files except the sites directory

In my opinion, 2) is a step backwards.

Solution is simple IMHO. Using the latest 7.x toy (#395472: Plugin Manager in Core: Part 1 (backend)) write the settings.php to the server via FTP if it doesn't exist.

you mean: #395478: Plugin Manager in Core: Part 3 (integration with installation system) and #395480: Plugin Manager in Core: Part 4 (installation profiles)

none of the above issues mention settings.php in them. Why did you change it to won't fix?

I didn't really follow this whole thread, but #42 make me feel like #50 misses a third option to discuss: Ship with a file, and RENAME that file from installer. Does that make sense, or is there a problem with that? It should avoid webserver-owned file, as well as overwrite-on-upgrade risk.

I was thinking more of the early 6.x pattern of default.settings.php, turned into settings.php automatically by the installer - only just replace copying by renaming. Renaming works, at least in the Category module's 'wrappers' it always did well for me, without raising the problem of webserver-owned files.

EDIT: The above is how my idea goes, the below is my opinion on the other (just to make it clear)

Having version number in the filename will add an extra step to even minor update - if I upgrade from, say, 7.1 to 7.2, the existing settings-7-1.php won't work with 7.2, so I'll need to go and manually copy/rename the file (although there's no change in the file), or else there needs to be extra code in update.php to adjust the name (which will run into issues of doing that early and yet being able to deal with repeated upade.php runs, and not having sufficient permissions due to settings.php being secured/chmod-ed after install per status-report page suggestion, so asking the user to chmod settings.php at the start of update.php, but only when the version actually changed, without having database access due to the file not renamed yet, and... and... This looks scary to me. EDIT: Also the update.php access switch is inside settings.php, so some users won't even get as far as seeing the advice, unles they rename version number by hand in advance.)

Or am I missing something? I consider the version number in filename unneeded and ugly, sorry.

Subscribing.

changing title to better reflect the issue

okay, i think i've got the idea:
1) require default.settings.php OR settings.php (instead of AND) during install.
2) if settings.php found, assume credentials already set unless content is default.
3) verify if default.settings.php (or settings.php) is properly writable (or make it so), else fallback to current situation.
4) rename default.settings.php to settings.php (in not already existing)
5) request credentials
6) store credentials in settings.php
7) use chmod to make files secure again

Yes, I like that very much.

Just to point out: We are creating a minor WTF because now some people will have a default.settings.php and some won't. But default.settings.php isn't really used for anything, unless you messed up your settings.php (BY HAND!) and in that case, it shouldn't be too hard to get a new one.

@alexanderpas: I'm against 4), It's handy to have a clean default.settings.php when creating multisites, and renaming a file needs write permissions on the directory anyway. Is the 'default' directory coming with write access by default?

If write permissions is needed on the 'default' directory to install then the installation process could be as it's now but letting drupal copy defaults.settings.php to settings.php. This would introduce the additional security step of removing write permissions from the default directory, otherwise any user could change permissions of files under that directory.

In short I don't think the planned process is going to be more intuitive for users and could be more insecure.

@Pedro Lozano: as has been said multiple times in this issue it does not work on some webhosts that Drupal creates the settings file because then you cannot edit it later on due to ownership problems. That's why you have to create it by yourself currently and that's why the plan is to rename the file.

How would this be more insecure?

To be able to rename the file you need write permissions on the directory. You don't know how users and groups will be set on the webhost so you need either to ship the 'default' directory with write permissions for all users by default or require the user to grant that permission. I find having write permission for all users on the 'default' directory pretty insecure, so in both cases the user will have to remove those permissions after installation. In the end we are changing a copy operation (default.settings.php -> settings.php) by a chmod operation and I don't clearly see whether that improves the usability of the installation process which is supposedly the purpose of this issue.

@ #62-63: The Drupal-created copy from default.settings.php to settings.php was already there (in early 6.x releases), and it was removed (for a reason, I'm sure, webserver owning the file is no fun on some hosts). Using rename operation, we can avoid both the mentioned problem of file ownership AND the awkward step of the user being required to create settings.php manually (initial problem in this issue).

I'm not sure about multisites, but you're supposed to have settings.php in a place other than 'default' directory there, so you need to copy the file manually anyway. Am I right?

We might include some backup.default.settings.php file (mirroring default.settings.php) in the worst case, so that something stays after renaming, but I consider that unnecessary and ugly - after all, default.settings.php was only introduced to keep settings.php from being overwritten by carelessly done upgrades (or by carelessly rolled patches), or am I missing something? There's really nothing weird about downloading the Drupal tarball, if a fresh copy of some file is needed.

@ #64: CHMODing files is much simpler and more common requirement, plus in several cases it boils down to only just ONE such operation post-installation (for example I'm on a shared host, where all files uploaded through FTP are writable by the webserver by default, as both are run under the same user), so it avoids the web-based install process being interrupted.

While it might fail on some hosts, it's still worth a try, as it may improve the installer usability for a part of the userbase at least.

Completely agree with JirkaRybka - when I first started using ftp/shared hosting, chmod was one of the first things I learned. I never had to, and probably had no access to, chown - and it's not being able to get back ownership of files from the webserver which is the main issue we want to avoid.

Drupal 7 - Install - Dreamer's Error 001 (Where dreamer=end_user_only.)

D7 Tech: Please create file "settings.php" during installation.

I agree that this "renaming" step is a huge wall for the users I class as "Dreamers"; as opposed to the other side of my brain, the "tech".

I remain hopeful that you all will change this as it will greatly assist in my instructions to help every great-grandma get content on her website.

Thanking you in advance for your time and help.

- Chris Rodgers

PS: I welcomed seeing the following as I hit my first d7 error.

# 2009.09.22

Settings file

The settings file does not exist.

The Drupal installer requires that you
create a settings file as part
of the installation process.
Copy the ./sites/default/default.settings.php file
to ./sites/default/settings.php.
More details about installing Drupal
are available in INSTALL.txt.

### Thee End - Technician's Dream Instructions. - AKA: Dreamer's First Nightmare.

Easily doable:

Change the packaging scripts to copy default.settings.php into settings.php when packaging Drupal.

Users know about CHMOD. Most should know that from other CMS already. Manually copying means downloading, altering, and uploading a file with the proper permissions, which many (S)FTP GUI-clients do not support (or users don't know that their client may support that).

Renaming won't fly: default.settings.php == settings.api.php. Users are free to alter settings.php to nuke all docs they know already. There should always be a file to diff against when updating or upgrading Drupal.

Renaming won't fly: default.settings.php == settings.api.php.

I disagree. default.settings.php was AFAIK introduced, to avoid settings.php changes being rolled into patches and committed accidentally, and to avoid settings.php being overwritten on upgrades accidentally (if someone is so dumb, to copy entire tarball over existing production site code without reviewing the contents first, which I won't ever do, but community generally thinks this is recommended workflow, ick...)

If you added settings.php back to the package, that fix will be undone (except for commits or patches rolled against CVS, if that's a special case in packaging script). People will overwrite their settings.php on upgrades again.

If someone needs a fresh copy of settings.php for some reason, they can always download a copy of the Drupal package version in use. We have them all on Drupal.org, and that's generally what CVS is for. I don't think this is the primary purpose of default.settings.php, nor a purpose so significant to block an important installer usability fix. (Following your logic, we might include a copy of *all* core files, just in case someone hacks them and then needs to reference the original.)

Or, we might just include another copy of settings.php that won't get changed by the installer, like backup.settings.php (either commit, or in the packaging script as you suggest), if we need a copy so badly. All the same, a copy for developer's reference is less important than error-less install IMO, and rename is, to me, the only alternative to manual work by the (newbie) user.

JirkaRibka: a clean copy of default.settings.php is sorta neccessary to have handy when multisiting! Not going to go back to CVS for that.

Then add another copy rather than blocking installer usability, I would say ;-)

BTW we didn't have a copy on 5.x and below, too.

Hm, thinking a bit more - what about this:

- Ship with two files in the tarball:
* default.settings.php - just like we have now
* dummy.settings.php - An empty file (maybe containing a plain text explanation like 'This file will be modified by the installation process to become settings.php, if applicable.')
* (Note: No settings.php in the tarball, nor in CVS, to avoid re-introducing the problems default.settings.php initially solved.)

- The installer will attempt to do the following:
* Rename dummy.settings.php to settings.php (to avoid file ownership problems on freshly created files), if settings.php doesn't exist already
* Copy the contents from default.settings.php to (now renamed) settings.php (to avoid the need of maintaining two copies of the same file in CVS, and to keep default.settings.php unchanged for those who need to keep it for some other purpose)
* Save necessary changes to settings.php.

- If the above failed, ask the user for setting write permissions or creating the settings.php file manually.

So, it's pretty much what we had back on early 6.x, except that we'll have dummy.settings.php and a rename instead of creation of a new (webserver owned) file.

Now how is #74-75 not Off Topic here? This is about the usability problem in the installer. Are we going to fix it, postpone it, or say the very unpopular words "won't fix"? I can't see how discussing the D7 release strategy helps with this bug, unless I'm missing some hidden specific connection between the two.

#74 is referring to a specific host setup (his host?) so they don't influence this issue.

a recap of the current situattion:
- We can't create the file during installation, due to ownership problems on certain webhosts.
- We can't ship the file inside the package due to overwrite on upgrade problems.

that means the only thing we can do is to rename default.settings.php

I'm willing to trade in a (slightly) less DX for a (much) more UX

(really, those needing to refer to default.settings.php for multisites can grab it from the tarball, or make a copy beforehand (when using CVS) like we currently do.)

and to increase DX again, I would like to see a settings.api.php (in sites/all) documenting _all_ availble options you can set in settings.php

Did everyone miss the point? Let's step back and review settings.php. What do most of our users change in there? The database credentials. There is a lengthy explanation right now which only advanced people need. The rest is variables documentation which should not be here (Edit: #145164: DX: Use hook_variable_info to declare variables and defaults is where it should be) and some ini_set commands which can be moved to bootstrap.inc to be overridden by some settings as necessary.

So. We remove default.settings.php and not add settings.php. The installer asks for write permissions on sites/default. If you want multisite, yay, you are advanced and can plonk down a settings.php file (even empty) in the directory you want.

Piece of cake.

btw, a very similar discussion about the same issue more or less:
http://drupal.org/node/538660#comment-2065738

Fundamentally, we have to decide if it is okay for the user to have a file created by the webserver which they cannot delete.

I see users in 3 groups:

1. Devs:
SSH, FTP, Butterflies, whatever they know all the tricks, not really a concern here.
2. Techies:
That person who is forced to run the website, or that CEO who used to program in Smalltalk. They can deal with FTP, don't really get perms. So they can upload / download no problem as long as it looks like the finder, but don't know what chmod is.
3. Reluctant Typewriter Converts:
You know what I mean, white out on the screen, etc. They will follow instructions, use FTP built into their OS, and use it to upload Drupal, or get their hosting company to dump it on for them.

So for Group 2: The proposed solution presents the problem of them not being able to delete or move their site. It also doesn't really help them install, because they still have to set the perms of sites/default :(

For Group 3: They will probably want to upload it via step-by-step instructions and then get the hell out of there. Sadly, we can't do much more to serve them given the nature of the web. There is no way they will learn to chmod. I think they are still stuck.

So I don't see how this will *really* help either group. I think we should use the FileTransfer classes in core (which now have chmod functionality), and the developments in the Plugin Manager to let them set perms upon install, or even copy files (as their own user) during install if need be. This wins on all counts. The settings file is owned by the user, the perms are safe.

However, it's more complicated in terms of integration right now, and probably will delay this.

SO while I think we should plan for that, I think that at least making the user set the perms in sites/default and creating an un-deletable file is better than making them copy the file to a new name (but only slightly).

Best,
Jacob

Sub

#78: However we might want to change (default.)settings.php contents (and frankly, I see removal of the basic documentation from there as a bad move, as it turns simple edits into a research on remote documentation, so making usability worse in fact), still the points in #77 are valid, and the extra WTF step in simple, single-site default installation is not addressed at all. Removing both files instead of fixing the workflow as proposed in this issue, means that we still keep one of the usability flaws unfixed: Either the user have to create a file manually, web-based installation being interrupted, or the file is created with webserver ownership (which is a big problem on some hosts). Really, what exactly is inside the file is other issue IMO.

I'm sorry that I have to repeat the previously mentioned points, but the situation, or users grouping is IMO like this (sorry for yet another recap, but there seems to be some misunderstanding going on, still):

Group one - Whoever have full control over the environment. Advanced users, can do just anything, no concern here.

Group two - Less experienced users, who most likely use shared hosting accounts. This is where most (I believe) Drupal installs are, and this is where the problem lies.
* Significant part of these environments have issues with webserver owned files. You cannot chown, you cannot delete files created by webserver, and doing so through a dedicated php script is too advanced for such users. So no webserver-created settings file, please.
* Also significant part of these environments set FTP-uploaded files as webserver-writable by default. So no need to interrupt the web-based installer at all, if we hit such a case. There's a _chance_ that it'll work without any manual work on files, and we should leverage that if possible. Actually, they sometimes run both FTP and webserver as the same user, so even chmod done by the installer might (or might not) work.
* Otherwise, chmod is a thing that the user on shared host likely needs to do later anyway, somehow (usually through FTP), as we ask for the settings.php file being secured (and it would be nice to secure the whole codebase, for that matter). Chmod, once undestood, is acceptable for user's experience as an understandable security measure. Error message about a file "missing" in the tarball, and need to create the file manually, is not acceptable as a part of installer user experience.

I believe that we don't want to discuss whether this or that setup is correct for security, and we can't rely on any of them either, as the typical shared host user have absolutely no control over that (and knowledge neither). We want to attempt a flawless run of the installer if at all possible, while avoiding the two problems summarized in #77.

So, I believe that our options in this issue currently are either #60, or #73.

I came across http://www.vtiger.com/ this -- it generates a settings.php you can upload. This augments my solution above: ship with an empty sites/default directory, generate the file and ask the user to upload it.

@chx:
if we ship with an empty default.settings.php
which we'll rename to settings.php,
and write the stuff in there.
voila, same result, improved UX.

still, i say we should implent #60

also, from now on, multi-site issues with the availability of the unedited file are non-issues!! (where are your backups? grab it from a tarball, or grab it from CVS again.)

@#79
it has been determined before, that CHMOD is one of the first things you'll learn when running websites (and it is already needed for the files directory etc.) so it is _not_ an added obstruction.

99% of people running, creating, installing, using, clicking, licking a Drupal site do not know what CHMOD is, let alone how to use it correctly. I have been doing this a while and I don't know it completely.

I would LOVE to see how that was determined? Maybe back in 1990.

Okay, this is likely not ready for "prime-time", but if someone is interested in coding it up:

See #538660: Move update manager upgrade process into new authorize.php file (and make it actually work).

If you apply this patch, and then do something like this:

$batch = array (
'operations' => array(
array('upload_new_settings_php', $contents);
.....
);

$_SESSION['plugin_op'] = $batch;

Then redirect the user to plugin.php, it should be almost there. There is some hardcoded stuff in plugin.php for adding the FileTransfer object to a certain batch operation.

Probably a better idea is to just use install.php and rip out the needed bits.

In this case, just show the filetransfer form, get the results back, copy from plugin.php, and then make the upload like this:

$filetransfer->copyFile(DRUPAL_ROOT . '/' . conf_path() . '/default.settings.php', DRUPAL_ROOT . '/' . conf_path() . 'settings.php');

Or, you could create one in tpm and then copy it. Either way, not that hard.

Best,
Jacob

> also, from now on, multi-site issues with the availability of the unedited file are non-issues!! (where are your backups? grab it from a tarball, or grab it from CVS again.)

Or we just add a template.settings.php, which is there for admins to copy for multisite setup.

Let's not make multisite harder at the same time as we make regular install easier!

I'd be fine with shipping with default.settings.php and template.settings.php - this leaves only the issue of having to sync changes between them during development, but they don't change often, so that's a tiny price to pay.

I suggested a workaround in #73, to avoid the need of keeping two files in sync in CVS.

Otherwise I don't see how plugin manager fits into this (didn't really read that other issue), but seems like a serious overkill, and a risk of introducing flaws and problems given that we're very early in the installation process (having even no working settings.php file yet). I would prefer to have these early stages of installation (i.e. pre-database) self-contained in the installer.

@JirkaRybka: In one sentence, you said you didn't understand what I was suggesting, and then that it was a bad idea :)

I agree, it's probably not popular to take this thread in another direction, and sounds more complicated, but I don't think there is much of a resolution here on how to make anything more usable anyway.

It's not about PM per se, but can re-use some functionality it required, namely, the ability to use FTP on behalf of a user. Through this, we could upload a settings file for them, thereby solving this entire problem.

Best,
Jacob

Jacob pointed me to this issue in IRC. We just discussed how this could be solved, given all the work we did to get update manager and authorize.php into core (see #538660: Move update manager upgrade process into new authorize.php file (and make it actually work) for more). If settings.php doesn't already exist and contain the DB credentials, the installer should prompt the user for their file manipulation credentials so that it can copy settings.default.inc to settings.inc as the real user, not the web user. That's the only real solution here that satisfies all the concerns:

7) use chmod to make files secure again

Wow, most people really don't understand file permissions, do they? If the file is owned by the UID that the webserver runs as, and therefore the webserver user has permission to chmod() the file to make it "secure", obviously that UID has permission to chmod() the file back to some other mode so the webserver can write to it. Doesn't matter what mode it's currently in, if it was created via copy, rename, or write, etc. So long as the file is owned by the webserver UID, it's fundamentally NOT secure...

- Shipping settings.php, either in tarballs or CVS, breaks upgrades. No thanks.

- Status quo is a major UX WTF.

- Writing the DB credentials to a separate file is probably a good idea in its own right, but we should do that as the "FTP user" (the one that's ideally *not* the same UID as the webserver that owns all the files), too. Even with the separate file, we need a way to create it securely.

So, here's how it should work from a technical standpoint, given the technology available in D7 core now:

A) authorize.php itself isn't going to help us much, since it's depending on bootstrapping Drupal at least to the SESSION, which requires the DB, which requires DB credentials, which we don't have yet. However, all we really need is a FileTransfer object. Most of the form code to prompt the user for their credentials lives in includes/authorize.inc, so we can just re-use nearly all of that to instantiate a FileTransfer object, and then use that to do the file operations with the elevated privileges.

B) authorize.inc isn't quite cleanly separated from authorize.php enough to accomplish (A) yet, so we'll need a wee bit of refactoring to make authorize.inc not care about the SESSION in various places. Should be easy, and a good move. Any API (in this case, authorize.inc) needs at least two things trying to use it to be at all viable as an API. The current split was done very hastily, in the midst of a whirlwind of stress and lack of sleep. So, stepping back a little and seeing how it should really be organized to support being invoked from both authorize.php and install.php would be great.

C) Ideally, once install.php has this FileTransfer object for dealing with settings.php (and potentially db.php), it should pass it off to the install profile in case the install profile wants to use it for anything (e.g. fetching and installing 3rd party code during the install -- (see #594704: Allow packaged install profiles on d.o to pull in code from other sources + sites for more). This is not a hard-requirement. An install profile could also solve this problem itself in contrib, but it'd involve prompting the user for all this information twice, which is clumsy for the UX, for no good technical reason.

I think this is totally do-able, and would be a big UX win.

That's right, except we should probably remove #3 - because that causes major issues on shared hosts where the user is unable to move or edit files written by the web server.

Agreed. If our wished-for e) can work, then trying c) is unnecessary.

Anyone want to speculate on a turnkey host setup that allows users to create a new Drupal site yet not give you even FTP access to it? I imagine such a thing could be built - but I'd also imagine that system should be able to (required to) set the database credentials for you at that time also. So a non-issue, but possibly worth documenting.

I would rather not remove c) - All the time I installed early 6.x Drupal (once on shared host, many times on localhost) it always worked. I have a live site on a host, where FTP and Apache runs as the same user, so there's quite no point in requesting some credentials from the user, to do the same thing in the end. Requiring FTP credentials is an extra step, and I would rather avoid extra steps where unnecessary. Also from user's point of view, I would feel really uneasy about providing my FTP credentials to a script I'm not experienced with yet (case of installation phase). On localhost, FTP most probably won't work (why set that up on a testing local server?), so I would need to take the extra step of manual file creation, am I right?

In other words, I have no problem with _adding_ FTP/whatever as an additional way how to deal with stuff, but I don't like the idea of _removal_ the simple way without FTP, which is the most straightforward where it works. The previous part of this thread (around #60-#70) was about how to make it work.

Read through #90 and #91. Sounds great, tentatively, and I obviously would love to see this complete usability WTF fixed (I think our usability test data is skewed, in fact, by the fact that we never have people encounter red errors of death). If we start getting into major refactoring, though, obviously Dries and I need to have a discussion, since we're technically in code freeze as of two days ago. So I would plead to try and keep such refactorings sane. ;)

I am concerned about the usability issues Jirka outlines in #95, however. Let's be mindful of our localhost developers because they are the ones who do 99.9999% of the patches around here. ;)

Bojhan's #90 is a summary of an IRC conversation I had with him explaining what I meant by #90. My only clarifications are:

- we're calling it "Update manager", not "Plugin manager" (see #602582: Change name and description in update.info to reflect update manager functionality)

- the UI we'd present in the installer isn't really the "Update manager" UI per se, but just one step in that workflow, the form you see when you're redirected to authorize.php. So, d) and e) would be more accurately written as:

d) If we can't write it, we present them with the authorize UI (asking for FTP/SSH credentials)
e) Installer writes the correct DB credentials using the authorized file access

@catch #92 and @dman #93: I think you miss my point. If you happen to be on a crappy shared host (or, as JirkaRybka points out, in some cases on localhost) where the webserver runs as the same user as the "FTP user" (I really hate that terminology, but everyone knows what I'm talking about that way, so I'll keep using it), it's pointless (and yes, a possible phishing attack vector) to prompt for their SSH credentials during the install. Re: " If our wished-for e) can work, then trying c) is unnecessary." -- not at all. If the installer can already write to the file (which we can easily test automatically) we save an entire extra page/step during installation. Ideally, we need that other step, but if we don't, we shouldn't take the step anyway, since it just slows down the process for the end-user, and is another opportunity for failure when trying to get started with Drupal. Of all the places where we could spend energy on UX, the installer is probably the most important.

If we *can* write to the file directly, we should warn them "Looks like the webserver can write to your files. This configuration is inherently less secure, and discouraged. See [some handbook page] for more details." (or something).

If we *can't* write to the file, only then do we do the extra step of presenting a page with the authorize UI. Also, at this step in the process, I'd be fine leaving a bit of fine print about "if you don't trust me, log in yourself and copy this file, etc..." like we have now, and a "retry" button or whatever. Maybe that's just extra confusion and we should leave this as simple as possible.

Also, see #607654: Create an SSH key-pair FileTransfer class about providing another means to authorize that doesn't involve typing your password. And if we're going to be typing passwords, see #607008: Fix bugs in https support and force using https for authorize.php if available...

Well the reason we ask people to copy the file, rather than writing it ourselves, is less about security, and more that if your 'FTP user' and webserver user are different, then your FTP user can't write to, move, or delete files written by your web user. This becomes a massive problem for people when they want to upgrade Drupal later on, since they'll generally have to file support requests to their webhost to get the file permissions fixed, which is a pain in the arse. However we've got a trade-off betwen initial pain, and pain later, as long as we admit its a trade-off I don't really mind which on we do.

Thanks to those clarifications, I agree totally that there is a place for server-side file-write.
I was thinking of step e) and step c) being about the same impact on the user, and didn't think about how it actually takes a full new form submission step.

I would indeed be disturbed if a brand new web script I put on my site asked me up front my for my hosting account/ssh username and password! Over a non-secure http, yet!

And yes, localhost developers shouldn't have things made difficult for them.

But still ...
Remaining issue is the absolute insecurity of the file that the webserver has now created, and the ownership of that file if the ftp user can't reclaim it. This dichotomy is why this problem has gone around in circles :-}

Should the UI offer the CHOICE? Or a security fix-up (option e) that can be applied even after c) was done in the first place?
Gah. It's hard.

the reason we ask people to copy the file ... is ... that if your 'FTP user' and webserver user are different, then your FTP user can't write to, move, or delete files written by your web user.

Remaining issue is the absolute insecurity of the file that the webserver has now created, and the ownership of that file if the ftp user can't reclaim it. This dichotomy is why this problem has gone around in circles :-}

I hate to repeat myself, but unless I'm missing something fundamental, this leads back to my proposal to create that file by RENAMING (and modifying the contents of) a dummy file, that was distributed inside the Drupal tarball, and so was already uploaded by the FTP user, and therefore is owned by that user too (and may well be secured by FTP user later). Or at least we might try if the installer can do that - as for my personal case, it works on both localhost and shared hosting [FTP user = Apache user].

UX wise, I think there is a fairly clear path, although it could be messy in regards to the current install.php process.

Here's what I see:

1. We don't check for files and private, etc at the begining. We wait until we've got the DB information (and are ready to write settings.php).

IF the WS can write, just do it. ELSE, User sees a message:

"Your file permissions are not setup for installation.

If you would like to attempt auto-configuration, click here.

If you would like to try to do it yourself, click here for instructions."

2. If they choose the first one, we ask for creds, login as them and create the directories, and create settings.php

3. If they do it themselves, they will have a link to "re-check", that they have given sufficient perms to the webserver.

Summary
* Drupal doesn't ship with settings.php, but a default to copy.
* settings.php is created by the FTP user if they choose to do it that way, else by the WS.
* Setting the perms of sites/default to 555 is kinda stupid IMO as I and dww have said earlier, this isn't any more secure if it is owned by the webserver.

Is there some problem here I'm not seeing? This would not change localhost users process at all AFAICT, just progressive enhancement. Also, they should be using CLI installs, as they are much superior for this type of thing.

Setting the perms of sites/default to 555 is kinda stupid IMO as I and dww have said earlier, this isn't any more secure if it is owned by the webserver.

It is a bit more secure, as it requires a potential malicious script to do one more operation, not just a generic file write, to take over the settings. (BTW I don't really see why the webserver access to the files is so much of an issue - if someone can run malicious scripts on your account, you're in trouble already, aren't you? But that's off-topic here, so don't bother to discuss ;-))

@JirkaRybka: Personally, I agree that WS writable files is not a HUGE deal. However, good luck getting that one through! If people felt that was okay, the Update Manager project (Plugin Manager) would have been A LOT simpler :)

I guess one possible reason is that in poorly setup shared hosting, if someone is executing PHP using the same UID as you, they can write to *your* files. Also, say you run another system (besides Drupal) on your account. If the WS can write to your Drupal install, they could spread it, else, probably not.

-J

I know that there's no point in discussing the opinions on WS writable files. I already learnt a few unpleasant lessons about having a different opinion than majority on certain matters. (Although I might have additional points regarding files directory, we better stop this tangent...)

My point in #102 actually was, that chmod is an additional barrier to possible attack (however small), so it still makes a bit of sense to do it.

I'm also in favor of c) If not, we try to write it (insecure server)
Can anyone provide an explanation of the security risk here? I presume the objection is writable files offer an attacker persistent storage of malicious code, should a remote code execution vulnerability exist. (Personally I believe that if you have remote code execution, you have already lost everything. But of course there's something to be said for defense in depth, and perhaps an attack could less severe if there's no persistent storage available.)

Unfortunately, it would seem that any benefit derived from denying write access is completely lost by Show if the following PHP code returns TRUE (PHP-mode, experts only). (among others). If I were to write shellcode for Drupal, I wouldn't bother with the filesystem, I'd go straight for block visibility. And one little XSS to deliver the payload. :-\

Sorry to continue to drive this thread into the weeds. But the debate about (c) seems to hinge on trading some ease of use for better security, and if so, we should be confident the security benefits are real.

I hope I'm wrong! If so please explain it to me. I'd love to learn something new here!

Reading through this issue, I really think we should be doing @JirkaRybka's proposal in #73 (simply moving the file) as a first step, and make sure that gets in.

The plugin manager integration would certainly be a nice bonus, but seems to make things more complicated, without a huge benefit.... Unless I'm really mistaken, @JirkaRybka's proposal is all that is needed to solve the usability problem on the vast majority of shared webhosts. On these hosts, you already don't need to play around with chmod (Drupal already does that for you). The only stumbling block that is currently in place is the need to manually copy settings.php, and @JirkaRybka's proposal would solve that completely.

For a summary of the security issues (which @JirkaRybka's proposal also seems to solve), see here: #225880-57: non-writablilty of settings.php when created by webserver

Regarding UI proposed in #101, I think that makes a lot of sense - note that we've been having similar discussions at #534556: Fix double appearance of the settings.php message in the installer (an issue which I'd really like to find time to get back to). In this issue, I'm not sure we need to deal much with the UI. I think it would be best to focus on making sure that the proposal in #73 can be implemented, unless someone has a really good reason why it shouldn't be.

The plugin manager integration would certainly be a nice bonus, but seems to make things more complicated, without a huge benefit....

The huge benifit is outlined by webchick and others earlier. If we can get it working, then a user never needs to learn chmod if they are setting up their site. Potentially, they wouldn't even have to know FTP if someone would dump the directory there. Since the PM (now UpgradeM) solves this technical hurdle in updating (and will for core at some point), when we require it at install, we kinda make a "this tall to get in", but "this tall to ride scenario" where saving them from learning what CHMOD is later doesn't help if only CHMOD knowledgeable users can install the system.

The only stumbling block that is currently in place is the need to manually copy settings.php, and @JirkaRybka's proposal would solve that completely.

The problem is that if sites/default is owned by the FTP user (as it commonly is), then the WS can *not* do the job and the WS must write it.

Maybe I'm missing something, but the proposal in #73 is no different from what we have now. As opposed to copy default.settings.php -> settings.php, it will mv dummy.settings.php (what is this) -> settings.php and then insert the contents of default.settings.php > settings.php? Isn't this the same as cp default.settings.php settings.php? I think chx's proposal to have default.settings.php contain defaults and be included along with settings.php is sensible, bu that doesn't change the permissions problem.

@Bohjan: Go for it :)

The huge benifit is outlined by webchick and others earlier. If we can get it working, then a user never needs to learn chmod if they are setting up their site.

The settings.php issue and the chmod issue are two separate things. Every single Drupal user has to deal with the settings.php problem, which is why it's a major problem and the focus of the current issue.

Most already do not have have to deal with chmod, because Drupal already does it for them automatically. At least, that was certainly my experience the last time I tried to install Drupal on shared hosting (it was Drupal 5). Someone please correct me if I'm wrong but I would think that the same should be true for any kind of reputable shared hosting provider, since they run PHP as CGI rather than via mod_php.

The people who do actually have to deal with chmod are generally people running Drupal on a local machine or on a VPS, and they probably should know what they are doing anyway ... plus, if they don't, it's not clear to me that Update Manager will even be able to help most people with those setups?

Maybe I'm missing something, but the proposal in #73 is no different from what we have now. As opposed to copy default.settings.php -> settings.php, it will mv dummy.settings.php (what is this) -> settings.php and then insert the contents of default.settings.php > settings.php? Isn't this the same as cp default.settings.php settings.php?

Because the proposal involves moving files rather than copying them, that makes all the difference - the file ownership/etc is preserved. (At least, that's how it works on Linux - I wonder if the same is true for all operating systems?)

Yeah, good point about mod_cgi. I'm not sure that we can assume it is a trivial amount of users who get stuck on perms as managed VPSs are become so much more popular and cheap, but you're right it might not be as large a base as I and others are expecting.

Regarding the mv vs copy thing, If we are copying the file ourselves without the FileTransfer class, then we have rights to chmod the file anyway. So I don't really get the intention here. Moving a file which is in CVS is just bound to confuse, plus it isn't idempotent.

The code to copy and then find-and-replace settings.php is pretty gnarly IMO. I like chx's idea of including settings.default.php and layering settings.php on top of it to clean that up.

Regarding the mv vs copy thing, If we are copying the file ourselves without the FileTransfer class, then we have rights to chmod the file anyway. So I don't really get the intention here.

We need to chmod the settings.php file so that it's not world-writable (security...), and since FTP user may be different than webserver, FTP user is then denied access to manage the file later if it was copied (i.e. created -> owned) by the webserver. In the other hand, if we rename a previously uploaded file, it's still owned by the FTP user and may be managed. The webserver process on a shared host may be either the same as FTP user (i.e. we can do it all from installer), or different but FTP uploaded files writable by default (so we can do it too, avoiding change of ownership on a copy), or different and FTP uploaded files not writable by default (this is the place for Plugin Manager, manual work, or extra chmod from the user). It's about supporting various setups if possible, without breaking any (as the copy did in early 6.x regarding file ownership), and without unneeded extra user-facing complexity (typing FTP passwords where the webserver already have direct access).

" In the other hand, if we rename a previously uploaded file, it's still owned by the FTP user and may be managed."

Huh? That means that the sites/default dir (so that settings.php can be created) and the dummy.settings.php file must be 777. I guess this is a small improvement on asking the user to copy settings.php and make sure it is 777, but only slightly. It will still require setting permissions.

The key distinction in my mind is people who know how to chmod and people who don't. There are A LOT of developers who don't properly understand chmod. Especially the windows set, but the designer oriented ones as well.

Still, my objection that it doesn't solve the problem shouldn't reclude others from making this change, it is still a little better. Ultimately, I think that the system should use UM or PHP's chmod() to set it up depending on the needs of the user if we really want to squash this.

I wish there was a great solution here :( FTP kinda sucks I'm afraid.

-J

File ownership and permission lesson #2: You can't mv a file unless you have write permission to the directory that file lives in.

File ownership and permission lesson #3: To be able to chmod a file, you must either own that file or be root.

So, assuming the drupal.tar.gz was unpacked as the FTP user, and therefore all the files and directories are owned by the FTP user, all this talk of having the web user that the installer is running as moving and chmod'ing files around is sheer fantasy.

The only cases where that could work are cases where FTP user == web user, which is inherently less secure, but unfortunately quite common.

Therefore, to actually work in the more secure configuration, we have to do the mv/cp/chmod as the FTP user, which is where the authorize API and FileTransfer class come in.

@Bojhan: "Lets stop talking, and patch making please?" -- I refuse to spend time on a patch for this until we're in general agreement on the direction and scope of that patch. If you want to start writing something now, feel free, but I have way to many other patches I need to get in to waste a few hours coding something up that isn't going to get committed because people object to the approach.

The only cases where that could work are cases where FTP user == web user, which is inherently less secure, but unfortunately quite common.

Right, the keyword there is "quite common" :) I know for a fact that Bluehost does this. And I assume the other major shared hosting providers do it as well (Dreamhost, GoDaddy, etc) - I know they are running PHP as CGI, at any rate. It's not totally secure, but for shared hosting, it ought to be more secure than running mod_php (which I assume is why they do it).

Jacob brings up a good point about managed VPS hosting though... hadn't thought about that.

So can we agree on these goals?

1. People on shared hosting (e.g. Bluehost/etc) should be able to run install.php without having to touch their filesystem at all, and without having to type in their FTP credentials during the installation (just like Drupal 5 used to work).

2a. People on a managed VPS should not have to go into their filesystem and manually create settings.php (again, just like Drupal 5 used to work).
2b. They also should not have to go into their filesystem and manually chmod things; however, in that case they may need to type in their FTP credentials (this would be totally new for Drupal 7).

If so, that's why I suggested doing #73 first - it should be relatively simple to write and immediately takes care of 1 and 2a. 2b would need the Update Manager, and certainly seems like a good goal as well, but also seems like a secondary goal.

I'm completely at a loss to understand the point of dummy.settings.php and renaming the file. Given how file system permissions actually work (see #114) can someone explain to me what that gains us?

I propose:

1. Ship default.settings.php like we do now.
2. Installer attempts to copy that file and update the DB credentials.
3. If success: print a message about potentially insecure hosting configuration with a link to a handbook page with more info, but otherwise proceed with no more manual input from the end-user.
4. If fail: print a landing page that says: "Cool, your installer can't write to your Drupal files. That's a Good Thing(tm), but now I need a little help. Either I need you to let me write some files as you, using your SSH/FTP credentials, or I need you to do that for me".
   • If they click on the "go ahead, I'll give you my SSH/FTP credentials", we use the authorize UI and a FileTransfer object to perform #2 as the FTP user.
   • If they click on "I'll do it for you, hold on", we print them a landing page with instructions much like we have now, and a "Click here when you're done" that retries -- basically, our existing installer workflow.

Disclaimer, in case it's not obvious: that's not the actual UI text I'm proposing... ;)

Note 1: the UX problem of files written as the webserver which the FTP user can no longer move later -- all the more reason to be writing this file as the FTP user via a FileTransfer object, instead of doing it as the web user.

Note 2: I still have some stuff at Dreamhost, and they in fact allow your web server processes to run as separate UIDs from the user owning your files. Granted, you have to know what you're doing to set this up securely in the first place, but it's definitely possible (dare I say, easy) to do so...

Again, AFAIK, this satisfies all the possible concerns from the UI perspective, and is based on the reality of how the filesystem actually works. Does anyone have an objection to this proposal, other than "wow, it sounds hard to make that work right"? If that's the only objection, I'm not afraid. ;) I'd rather solve this problem correctly than do another half-assed "solution" just because we're scared to do it right.

dww: That sounds awesome, actually.

Would anyone CNW a patch implementing #116? I guess now's the time to speak up. Otherwise, I'm looking forward to reviewing this patch!

#116 sounds great to me, bearing in mind the disclaimer. ;) I'd like to see someone from the UX team chime in on this, though.

Sorry, I'm still not getting it :)

With step 2 of #116, how do you know if you can safely copy default.settings.php? Just because the webserver is able to copy it doesn't mean it's safe to do -- all it means is that sites/default is writable by the webserver at the time Drupal is being installed. There are many, many ways that can happen, and therefore, you can still easily wind up in a situation like #225880: non-writablilty of settings.php when created by webserver - where settings.php is stuck in the filesystem with the webserver as the owner and the user has no way to edit that file ever again - which is exactly what we want to avoid.

All these problems go away if you attempt to move the file rather than copying it.

So basically, I'd support #116 completely, if you replace "attempt to copy the file" with "attempt to move the file" in step 2.

replace "attempt to copy the file" with "attempt to move the file" in step 2.

How about this? Attempt to copy the file, then check if the file owner matches the original. If the owners don't match, delete the copy, and fall back to 4.

@all: I had an IRC chat with David_Rothstein and pwolanin about all this.

"... all it means is that sites/default is writable by the webserver at the time Drupal is being installed. There are many, many ways that can happen ..."

I got David to admit this was hyperbole. ;) By "many, many ways", he really means only 3, one of which isn't true:

1) FTP user is the same as httpd user.
2) The FTP user went out of their way before starting install.php to make sites/defaults world-writable.
3) the sites/defaults directory starts life world-writable (not true)

So, all this talk of renaming an existing file is to handle a fairly obscure edge case that I'm not convinced we should even bother trying to catch. *If* we're dealing with a situation where the FTP user != the httpd user, and the FTP user manually made sites/default world-writable before visiting install.php, my proposal in #116 would result in settings.php being owned by the httpd user after install, which is a little too bad (although I'm not sure we really need to get crazy about this IMHO obscure case).

If we want to prevent this case, there are lots of ways we could potentially test for this before we proceed, and even though we *could* write settings.php as the httpd user, we fall back to #116.4 to do it as the httpd user, anyway...

@grendzy: Sure, something like that would work:

A) Copy default.settings.php to settings.php, stat the two files, and compare the UIDs.

Other possibilities include:

B) Attempt to touch a dummy temp file in sites/default and compare the ownership on that file to the UID that owns default.settings.php.

C) Try to do some logic with attempting to open both settings.php and default.settings.php for appending, and see if we can write to both or not (although this is going to fail if the FTP user ran "chmod -R a+w sites/default" to recursively make everything world-writable (woe is them)). Therefore, (A) or (B) would be better.

... There are probably many other equivalent tests we could try. (A) has the benefit of being an I/O operation we'd perform, anyway. So, basically, this would just involve adding step 2.5 to my proposal in #116, which would be:

2.5. Compare the UIDs that own the new settings.php and the default.settings.php. If they're the same, go to step #3. If they're different, remove the new settings.php file, and go to a new landing page that says:

Hrm, you made your sites/default directory and all its contents world-writable. #FAIL! You really want your settings.php file owned by YOU, not by me. See [some handbook page]. I can fix all this for you if you give me your password, otherwise, you're going to have to take these steps to recover from your current state so you end up with a secure installation: a) copy default.settings.php to settings.php and make it (temporarily) world-writable, b) make sites/default itself not world writable, c) ... After I install, I'll have you lock down settings.php again so only you can edit it. Thanks.

Frankly, I'm not concerned it's worth complicating the installer and holding up this patch for step 2.5 here, but if people are convinced users are going to get nailed by this, we can certainly add it...

Yes, 'twas a good IRC discussion and a good summary...

To expand on it a bit more, although what we are talking about is something of an edge case, it is also the reason #225880: non-writablilty of settings.php when created by webserver was filed in the first place, so I don't think we want to go all the way back to where we were in Drupal 6.0. The proposal in #116 is, I suppose, different from Drupal 6.0 in one important respect, which is that Drupal 6.0 actively told people to make sites/default world-writable, whereas #116 would not. However, the point I brought up was that there are tons of Drupal installation instructions floating around out there, many of which tell people they need to run chmod 777 on their sites/default directory, so I'm pretty sure there are people who will find themselves in this situation regardless. I guess that's what I meant by "many many ways", although I suppose they're all variations on the same thing :)

Note that we discussed something like #120 at one point in the previous issue, and I even tried to write some code for it at the time, although I didn't get it to work and gave up (in theory it should not be that hard though). But I'm not sure why it's better than simply moving the file, which seems like it's guaranteed to only work when we want it to, e.g.:

if (@rename('temporary.settings.php', 'settings.php')) {
  // Go to step 3 in #116.
}
else {
  // Go to step 4 in #116.
}

Whereas with the other methods, we first try to copy the file, then check file owners, then go back and delete the file if it isn't good, etc. Moving the file seems simpler, unless we are really opposed to the idea of shipping an extra file that exists just to be moved?

Anyway, here's a very rough start at a patch.

Writing this pointed out one problem with moving the file, which is that it is an annoyance for development (when rolling patches, etc)...

But anyway, it can be extended for #116, #120, #121, etc - this is just a start. It works fine at solving the initial problem brought up in the issue, so we're 90% there in that sense (well, except for a bit of a security hole, which is one of a few reasons why this is "needs work"...)

let the bot test your code...

Another solution (though it is postponed to D8) could be #22336: Move all core Drupal files under a /core folder to improve usability and upgrades as it would separate out core and contrib files, and not overwriting the settings file, as in an upgrade you would only copy the core files into the web folder.

@wretched sinner #126: that issue's a good idea on its own, but it won't help this problem at all. If you read and understand this issue, you'll see that it doesn't matter if the source copy is sites/default/settings.default.php (now) or core/wherever/settings.default.php, (post #22336) the problem is file ownership for the real settings.php file.

Subscribe. This needs a very motivated person to drive it home. Would be super nifty to resolve this for D7.

Just in case this issue is still in the books...

rerolled the patch from #123 against current head since the original patch failed.

took out 2 instances of '@drupal' from description strings since the associated 'array('@drupal' => drupal_install_profile_distribution_name(),' caused a crash because the function drupal_install_profile_distribution_name() was not found.

So far this installs very nicely on a shared hosting server which has the same user for ftp and apache (bad, I know, but there you are).

Results:
- temporary.settings.php gets moved to settings.php and updated with the right credentials. Permissions for new settings.php are 440 (good).
- directories default/files (775), default/files/styles (775), default/private (750), default/private/files (775), default/private/temp (775) get created. Not sure if the permissions for the default/private-directories are the desired ones.

Seems like a good starting point.

I'm not a great coder but would be willing to help out with testing and writing help-texts.

starting the bot

+1 if this is secure

Regarding security: I wonder how many D5 and D6 sites are out there with totally insecure sites/default directories and settings.php files just because non-expert site owners totally mess up their file permissions during the manual preparation for installing Drupal. Maybe a sensible default provided by the installer if we get his issue into core would be better.

Experts can then still fiddle around some more...

I am willing to test this out. Copying default.settings.php to settings.php was a bit of pain for every installation. +1 vote for this patch.

What should I do to make the status to 'reviewed and tested by community'?

I think this is the wrong approach. We should do what I said in #116 and #121.

@dww:
David said in #123 that his patch could be extended to accommodate #116 and #121. This would obviously have to be done still. My "renewing" the patch in #129 was in no way intended to completely solve this issue but rather an attempt at getting it going again (it had been stale for 3 weeks). So there's obviously still much work, which I am not really qualified to do :-(

I also really like the warning messages you proposed in #116 and #121 (with your caveats of course), since the messages that are shown right now (see attached screenshot) are badly structured (one links to INSTALL.txt, the other doesn't, the first one is grammatically wrong and should ideally show the missing directories in a list and so on...). Considering that these messages are among the first ones that Drupal shows to a new user when he is installing a site, this is really bad marketing. From my experience with customers, often when I try to convince them to use Drupal for their project, they do a quick local install themselves to try it out and almost invariable run across these issues with missing files and directories and wrong permissions. Bad vibes ensue.

I readily admit that I do not yet fully understand all the implications of this issue (security-wise and with regards to CVS) but would be really really happy to see it resolved.

Ok, I've reread the entire thread carefully.

Following dww's comment summarizing discussions on IRC in #121 here's an alternative patch with no need for a temporary.settings.php.

Refactored install.php so that it now just copies default.settings.php to settings.php.

This runs through seamlessly in the situation where the all files are owned by the http-user (which seems to be the case in most shared-hosting environments with identical ftp- and http-users). No problem with editing settings.php for these folks later on.

When file-ownership is not with the http-user (as it should be and is also regularly so for local installs), it still throws the errors shown as a screenshot in #135.

The whole mechanism of getting the user's ftp/ssh credentials and then writing the directories and settings.php is therefore still missing. Anybody wants to take a crack? I'm in way over my head with this. Nudge, nudge...

Also renewing my offer to work on testing and help-texts.

Also included in this patch is an addition to the help-text when the db-connection fails during install. There was no mention of the possibility that the db had not been created yet or its name was misspelt (which I think is a common hiccup - at least as common as misspelling the username or password).

Yes, we definitely want to start small and get something committed here - thanks for reviving this!

The "solve all problems at once" solution would be great if it materializes, but it's getting late to do that in Drupal 7, and it certainly doesn't have to all be done in one patch. The basic approach here is good and will help make things immensely easier for people on typical shared hosting (which is the exact population that struggles with this step of the installer the most). It's still a big win on its own.

As per the full discussion at #225880: non-writablilty of settings.php when created by webserver (which has been revived and revived here a million times, but really, that one is the definitive discussion), we need at a minimum to be checking the fileowner when we copy the settings file. In fact, I believe @dww got some code committed in the interim which already did this elsewhere, looks like in http://api.drupal.org/api/function/update_manager_install_form_submit/7:

  // If the owner of the directory we extracted is the same as the
  // owner of our configuration directory (e.g. sites/default) where we're
  // trying to install the code, there's no need to prompt for FTP/SSH
  // credentials. Instead, we instantiate a FileTransferLocal and invoke
  // update_authorize_run_install() directly.
  if (fileowner($project_real_location) == fileowner(conf_path())) {
    module_load_include('inc', 'update', 'update.authorize');
    $filetransfer = new FileTransferLocal(DRUPAL_ROOT);
    call_user_func_array('update_authorize_run_install', array_merge(array($filetransfer), $arguments));
  }

Ideally, I think we want to factor out that if() check into its own API function (so the security/other aspects are all handled in one place), and then reuse it here.

took out 2 instances of '@drupal' from description strings since the associated 'array('@drupal' => drupal_install_profile_distribution_name(),' caused a crash because the function drupal_install_profile_distribution_name() was not found.

Really? If so, I'm not sure how this patch would have introduced it - does it happen without this patch as well? If so, it should be an independent bug report. Ideally, we do not want to be removing @drupal here.

Also included in this patch is an addition to the help-text when the db-connection fails during install.

Unless it's 100% related to this issue, we should take it out of the patch (and file a separate issue for that improvement). To get this issue accepted for Drupal 7, I think the patch is going to need to be focused like a laser on the specific goal :)

Unless it's 100% related to this issue, we should take it out of the patch (and file a separate issue for that improvement). To get this issue accepted for Drupal 7, I think the patch is going to need to be focused like a laser on the specific goal :)

Moved this part of the patch to http://drupal.org/node/641008.

Ok, here is another patch that a) removes #138 and b) restores the @drupal to the description-strings.

More importantly, I've reread the entire thread as well as http://drupal.org/node/225880 and incorporated some logic into the patch to create the following workflow for the installer (see attached settingsphp-workflow2.png) that should take care of everything in #116 and #121:

1. We first try to copy default.settings.php to settings.php
2. If this fails ((1) in the graph), we give the user two alternatives:
   1. enter server-credentials and let Drupal copy the file with a FileTransfer object ((1a) from the graph, not implemented yet)
   2. copy settings.php yourself and make it writable ((1b) from the graph, traditional requirement)
3. If automatic copying succeeds, then check if it did because sites/default is world-writable (badly implemented)
   1. if yes, ((3) in the graph) indicating that the user chmodded sites/default, give him the options (1a) and (1b) again, since we do not like this
   2. if not, ((2) in the graph) then the httpd-user is identical with the ftp/ssh-user and we can safely proceed with the already copied settings.php

The result of this workflow would be:
A) Users on shared hosting servers can install Drupal without any user interaction with the file system (path (2) in the graph) after uploading Drupal (very good).
B) Users on safer servers (path (1) in the graph) will have the traditional option of manually creating settings.php or the better option of letting Drupal do it by merely entering their server credentials into a form (also good, see attached screenshot Requirements_different_users_standard_perms.png).
C) Smartypants users who chmodded sites/default to 777 before installing Drupal get stopped from harming themselves (see attached screenshot Requirements_different_users_conf_path_777.png). I am not sure if this is good. The alternative would be to just simply let the installer run through with those users, too.

Some caveats on the patch since I am at best an amateur coder:
- The mechanism for path (1a) is not in there at all. Somebody else would have to put it in there.
- My check for whether sites/default is world-writable is amateurish, I think. There should be a better way of doing this, but maybe we don't need it at all (see C) above and also dww's comment #121). Here it is in its full glory:

    $conf_path_perms = substr(sprintf('%o', fileperms($conf_path)), -4);
    if ($conf_path_perms == '0777') {
      $conf_path_worldwritable = TRUE;
    }

I also don't know if this will work on Windows machines.
- Warnings and help-texts need a lot of work still obviously.

The big advantages to the approach outlined here are:
- Users on many shared hosting servers where the httpd-user is he same as the ftp/ssh-user can now very simply install Drupal. Yeah!
- Users on safer servers with different users for httpd and ftp/ssh would be given the additional option of having Drupal create their settings.php for them. No ftp-ing or ssh-ing into the server anymore to copy a file and change its permissions. Yeah!
- Developers on local machines can achieve very quick installations by simply chmodding sites/default to 777 after unzipping Drupal.

ToDos:
- Write code that activates path (1a)
- Decide whether we really want to check for users who chmodded sites/default to 777 and if so write better code for this.
- Warnings, help-texts, landing pages etc.

P.S.
One more related issue is for users in scenarios A) and B) from above. For B) users the installer cannot create the directories sites/default/files, sites/default/private/files, and sites/default/private/temp. The wisdom of creating the sites/default/private/*-directories at install-time is being discussed at http://drupal.org/node/551658. For A) users, these directories will also be owned by the http-user which might turn out to problematic later. But anyways, let's keep focus on this issue. Anybody who cares for a streamlined install-process might lend a helping hand there.

the bot needs a nudge...

Just curious here, but what happens if your workflow doesn't use sites/default at all, and just has a bunch of explicit multisites?

Good question. Actually sites/default is never explicitly used but always the variable $conf_path. So AFAIK this should work for multisite as well since the value of $conf_path is set by Drupal when it searches subdirs of /sites for alternative dirs to sites/default. Example: you have a subdir sites/example.com and call your multisite with this url. Then $conf_path would be set to /sites/example.com and this installer would look for an existing settings.php in sites/example.com/settings.php. From there on everything should work just the same as before.

Haven't tested this though.

I was asked to chime in here via e-mail.

As much as I'd love to fix this since this workflow change has been the bane of my existence since it went in in ~Drupal 6.2, and it'd help get the new update manager stuff in front of lots and lots of people which would be a big win, AND as much as I really appreciate you jumping in here and trying to salvage things, regloge, I have some concerns:

1. I'm worried about trading one WTF (copy this file? why can't it just do it for me?) for another (why is it prompting me for my ftp password..?), so this definitely needs some testing in front of real people. As much of a pain in the ass as the current installation workflow is, we at least know that a quarter million sites were able to figure it out. ;P~ So unfortunately, until the patch is close to done, it's very hard to tell if it's a 7.x or an 8.x thing or even a won't fix thing, from my view.

2. I'm also concerned that as critical as this issue was, nothing happened on it at all for like 3 months, until Reinhold magically popped up. It's great to see some renewed energy around this, but it's hard for me to have confidence in this getting finished up in a timely manner, and there are definitely other pressing issues in the queue that are actual release-blockers.

3. My new "can this slide post-alpha?" measurement stick is "Is patch X worth potentially causing the 7.0 ship date past Drupalcon SF, thus tragically missing a tremendous opportunity for the entire project and community to laser-focus all of our core developers on the future of Drupal 8, rather than having them split between that and bug fixes and other clean-up?" And against that stick, I'm not really feeling this patch, esp. when we have over 250 issues currently blocking release.

So what seems safest for now is to bump this to Drupal 8.

Hmm... I wish I had time to put where my mouth is, but it is sad to watch this one go. I agree with webchick's point about risky at the moment, however I disagree that it is a WTF. Technically, it's the only thing possible AFAICT, and coming from the mind of a noob, it's a question that can be explained instead of "chmod +w... blah blah" which will never be understood.

Still, at least if someone sets up Drupal for you, you'll be able to update / install modules, that's still a big win.

-J

What if we worked on #312144: Install fails when default.settings.php is not present instead? That would still be a big usability improvement, and seems like it's finer-grained in scope.

webchick: I answered your call & re-rolled the patch for #312144: Install fails when default.settings.php is not present. Sorry it's so late in the game...

what a pain to see this one go...

subscribing

subscribing

I'm still in different about this. Drupal's multisite architecture is just plain awesome, and having default.settings.php go away after the first install pretty much removes any documentation the user has available on how to install another site on the same install using the multi-site architecture.... If someone has a good explanation for me for what I'm missing, I'd be glad to hear it, but I'm still on the edge with this one.

Until then, -1.

What you're missing is that the recent proposals involve copying default.settings.php rather than moving it :)

I still wonder if we have a way to do this in Drupal 7. @webchick moved this to Drupal 8 based on one possible solution (which would involve rewriting large parts of the installer and changing the UI, which we obviously don't want to do at this point). But I still think there may be a simpler solution that is "good enough for now" and doesn't involve any such rewrite. Maybe even simple enough to backport to Drupal 6. If I have time to look into this, I'll move it back to Drupal 7 if and only if I have a working patch :)

I'm relatively open to exploring something more "light-weight" + back-portable. I agree it's a complete UX #fail that's plagued us since Drupal 6.2 or so.

Just bear in mind that the reason it works the way it does now is SA-related. Any approach here needs security team sign-off.

@David_Rothstein: I remember you saying you had some ideas along those lines way back at DC SF. Thanks for your work on #312144: Install fails when default.settings.php is not present - I think you hinted that an easier way might build off that?

Since I am away for most of the next week, I'll post a quick patch now for review. This still needs some polish, but basically seems to work.

The attached patch just implements the "copy only when it's safe to" approach that we started talking about two years ago in the followups at #225880: non-writablilty of settings.php when created by webserver and also various times in this issue, in the simplest way possible :) It's helped by the fact that in the interim, @dww got some similar code committed to the update manager, so basically this patch is just taking code that's already in core and reusing it in the installer, not much more than that!

I think there was some confusion at various points above about whether an interim solution was worth doing, and I think it absolutely is. To clarify that, here's my recollection of how installation works in various versions of Drupal, listing what you need to do in your filesystem (outside of the web installer) in order for the installation to work:

So although this will not help all users of Drupal (we'll have to wait until Drupal 8 for that), it will massively help the Drupal users who are most likely to need it. If you're installing Drupal on your laptop or a VPS, you're likely to be pretty comfortable with the file system anyway. But if you're on a shared host, you're likely not to be, and for those users, this patch should restore things to the way it was in Drupal 5 - no manual steps needed. Especially for these users (who are probably using FTP programs where copying/renaming files is neither a simple nor intuitive operation), it seems to me like this would potentially be a huge step forward.

After documentation is added to the patch (#154), I would +1 it. Making D7 painless to install on shared (FTP) hosting lowers the bar and is a huge usability win. This is a good compromise and it would be a bad thing if it didn't get into 7 IMHO.

I like this approach a lot.

One minor nitpick: The naming of the function file_safe_to_copy() seems misleading to me. Actually we only check if the two files are owned by the same user. Shouldn't it be named like file_owners_identical() ?

I also took a stab at documenting the change. I added most documentation to install.core.inc however, because that is where the action really happens. The function file_owners_identical() only gets minimal documentation, because it might be reused by totally unrelated functions later on.

Regarding the @todo with stream wrappers: PHP documentation explicitly states that fileowner() does NOT work with stream wrapper uri's. I'm not sure about this however, so somebody with better PHP knowledge should chime in here.

The attached patch renames the function and adds some documentation.

I think this is a great direction to go in, and would make setup of Drupal easier in the majority of cases of its install. WordPress does something similar, I believe, where you only need to create the settings file if the files are not owned by the web server.

I looked at the code, but I did not test.

Well I'll test this on a testing server...
Will report back soon!

I think this title captures what we are trying to do here. IFF is math short hand for "if and only if"

Well I did some real world testing on my cheap host.
And it WORKS PERFECTLY.

This is SO much better than before :)
Please code/doc review this and get this one in!

*please*

just tested on windows (XAMPP); works great!

o_O I thought this didn't work on a localhost? (see #154)
(Just tested it and it actually works on my xampp stack)

Can someone explain this?

@aspilicious
windows xp and file permissions are kinda a joke. very similar environment to shared hosting.

Rerolled the patch against current HEAD (there were some offsets when applying) and added further documentation to INSTALL.txt and a clearer warning message when the automated copy fails. So back to needs review - sorry.

Otherwise I made no changes to the code.

@aspilicious, @mikeytown2: Can you test again, please? Simpletest can not test this.

Yes it still works (off course).
But I would like to wait until someone with a localhost (not windows) can produce the warning message. (and make a screenshot)

Here's the screenshot of the error message from a *real* localhost (Mac OS/X) where the Drupal files are not owned by Apache.

I like the patch, but a 1-line API function (file_owners_identical) seems rather silly. It would make more sense to me to just inline this test.

[Edit] - Also I'm not sure I agree with the failure notice saying "possibly due to a permissions problem". This sounds like something is wrong - when in fact a non-writeable docroot is a more secure and recommended configuration. See #116 for IMHO a better phrasing of this notice.

another screenshot for ubuntu

"possibly due to a permissions problem" from @grendzy. I agree this should be reworded.

+++ includes/install.core.inc	1 Sep 2010 20:31:49 -0000
@@ -1594,7 +1613,7 @@ function install_check_requirements($ins
-        'description' => st('The @drupal installer requires that you create a settings file as part of the installation process. Copy the %default_file file to %file. More details about installing Drupal are available in <a href="@install_txt">INSTALL.txt</a>.', array('@drupal' => drupal_install_profile_distribution_name(), '%file' => $file, '%default_file' => $default_settings_file, '@install_txt' => base_path() . 'INSTALL.txt')),
+        'description' => st('An automated attempt to create this file failed, possibly due to a permissions problem. The @drupal installer requires that you create a settings file as part of the installation process. Copy the %default_file file to %file. More details about installing Drupal are available in <a href="@install_txt">INSTALL.txt</a>.', array('@drupal' => drupal_install_profile_distribution_name(), '%file' => $file, '%default_file' => $default_settings_file, '@install_txt' => base_path() . 'INSTALL.txt')),

see #116 for ideas on better wording.
Powered by Dreditor.

I would almost say don't change this, keep it as is; OR remove "possibly due to a permissions problem" and go with that.

Here's a stripped-down patch:
- without the *silly* function $file_owners_identical()
- without the changed wording for the error message (#167 and #169 are right, of course)

The functionality remains the same - only leaner :-)

Either way is cool with me

Code looks good and it seems to work according to the comments. Could use another review though. Also, does this need security team, sign-off?

@tstoeckler: Regarding security-team signoff: webchick said so in #152.

On the other hand: David, who posted the patch reviving this issue in #154 is a member of the security team.

I happen to think that it's a security improvement, since with this patch we automate setting settings.php to read-only, which users on shared hosting had to to by themselves before this patch.

Users on more secure servers, i.e. servers where the file-owner is different from the http-user, are not affected by this patch and still have to to do the manual creation of sites/default/files and the manual copying of default.settings.php to settings.php.

Great!
Remember the DC CPH session about wordpress being better than drupal...?!
This is a point were we can gain much do doing very little, sind we really need this kind of ease-of-use for the end-user. And if this can be achieved with such a simple and forwards solution without sideeffects, then there's but one option: GO for it!

Regarding the @todo - I don't think file_unmanaged_copy is meant to be used for this kind of low-level operation. dopry has a good rundown at #876114-7: Normal stream schemes are not supported in Drupal. Why? (which probably should be in the handbook). @copy is fine to use here. So let's just remove the @todo.

I can also confirm that you get the warning message as expected on a regular Ubuntu Apache setup where the file owner is not the webserver.

New patch with the @todo removed according to #175.

Otherwise no changes.

Ok I think this one is ready :)

Thanks for picking up work on this. FYI, the reason I tried introducing file_safe_to_copy() was that we're using the fileowner technique as a safety check in 3 locations in core now, and could imagine adding further safety checks inside that function, so that's why we eventually might want to centralize it. I agree that for the purpose of this issue we don't necessarily need that, though.

In the patch, there seems to be a mismatch between using copy() to create the file but file_unmanaged_delete() to delete it - seems odd to use the Drupal APIs in one place but not the other? AFAIK, file_unmanaged_copy() would be reasonable here, and I assume the only reason it didn't work is that this part of the installer is defining all files with a "./" in front of them, which is odd to begin with and probably is what confuses that function... If it's easy to fix that, we should use the Drupal APIs.

I'd also want to flesh out some of the code comments more to explain exactly why we do things the way we do - i.e., add some more context based on this and the other issue.

Based on that I think this could probably use one more pass. I'm happy to take a look at it again on Monday :)

This also could probably benefit from one more review from a security perspective, as mentioned. AFAIK, this patch neither adds any security improvement nor security weakness, though; it should just be the same as before security-wise. Regarding this statement from above:

I happen to think that it's a security improvement, since with this patch we automate setting settings.php to read-only, which users on shared hosting had to to by themselves before this patch.

Unless I'm missing something Drupal does this already (before the patch), and the patch shouldn't affect it one way or another?

Yes, drupal alrdy changes the permissions

#179 and #180 regarding permissions: You are both right. My thinking was a little convoluted :-(

On the other hand, I have witnessed inexperienced users just setting their entire Drupal directory to 777 during installation because of the need to manually write-enable settings.php and sites/default. For these poor souls the patch would be an improvement, because we never ask them to do anything permissions-related during install. But that's really an edge case so overall, David is (of course) right when saying that the patch really doesn't change anything security-wise.

Here is a reroll as per above:

1. I looked into using file_unmanaged_copy() and indeed, it's not super-simple to make it work, plus we don't actually gain anything from it that we actually need, so instead I replaced file_unmanaged_delete() with drupal_unlink(), so that we are using the same "level" of function to create and delete the file.

2. I expanded the code comments a fair amount to discuss security, etc.

If someone who hasn't participated in writing the patch wants to give this a final review and make sure the security aspects make sense, I think this might be ready.

A) Personally, I'm not a fan of if conditionals with this many clauses and side effects:

+    elseif (!$exists && drupal_verify_install_file($conf_path, FILE_EXIST|FILE_WRITABLE, 'dir') && @copy($default_settings_file, $settings_file)) {

I'd rather this was unwound a bit and the logic tests were more clearly separated from the side effects like copying files, etc. It's also confusing since the giant comment right under this line is partly explaining the side effects from the if() and partially explaining the rest of the code block.

B) This comment is a bit misleading:

It is unsafe to keep a
+      // webserver-owned PHP file on a system where the webserver is running as
+      // its own user (distinct from the user who owns the Drupal files), since
+      // this would cause usability issues (site owners who do not have root
+      // access to the file system would not be able to edit their settings
+      // file later on) as well as security issues (a malicious webserver
+      // process could append arbitrary code to the settings file, even if the
+      // rest of the Drupal installation directory is protected against such
+      // threats).

It's *always* unsafe to have PHP files owned by the webserver's user. That part of the comment is independent of whether another user owns the rest of the Drupal files or not. Really, we're saying two things here:

1) It's unsafe to have PHP files owned by the webserver's user, but some hosting environments force this configuration and there's very little we can do in Drupal to protect you in this case. However, if you're already in the insecure configuration, we won't burden you with the extra (safety-providing) steps that users with a more secure configuration need to go through.

2) If the webserver and the user owning the Drupal files are different (yay for you!) it's a usability bug if we leave files lying around owned by the webserver, since the end user might not be able to edit or remove them.

So, I'd reorganize this comment to say both things as clearly and concisely as possible, but not to smoosh them together as if it's one point.

C) This line makes me nervous:

+        drupal_unlink($settings_file);

I'd have to look more closely at the rest of the code in install.core.inc to see where this variable is being set. Unfortunately, I don't have time for that right now. But, assuming that this isn't an exploit waiting to happen, I'm in general agreement that this change is okay from the security perspective. It's basically a usability optimization for sites that are in the insecure situation, and update manager already does this (it doesn't ask for your FTP credentials unless it actually needs them).

Generally, core should work really well if the webserver is securely configured, and we should go out of our way to support that configuration. However, if it's not a secure configuration in the first place, we shouldn't further punish users by making them do things themselves because it would be more secure if their webserver was differently configured.

Thanks. I won't have time to look at this again until the end of the week, but I'll reroll then to split up the if() statement and also try to reword the comment a bit to clarify the security issue. However, we have to be careful; it's true that webserver-owned files are an inherent security weakness themselves, but it's a difference of degree - the security issue that this code is protecting against and commenting about (i.e., the issue that existed in Drupal 6 prior to version 6.2) is more serious.

As a concrete example, I've at times in the past had to run a Drupal site on a corner of a large server shared by many people (all in the same organization). The Drupal codebase wasn't owned by Apache (good in theory), but since the Apache user was shared by so many people, that meant that at a minimum, any web application that some random person installed somewhere on the server could (a) write to my Drupal site's files directory, (b) read my settings.php and get my DB credentials, and (c) with Drupal 6.x prior to 6.2, write to the settings.php file also (and execute arbitrary code on my site visitors in a nice and hidden way); granted, you could say (b) is pretty bad as is, I guess, but I think (c) is still worse. In that situation, I would have felt much safer with a typical shared hosting environment (where the webserver owns the files - bad in theory - but the webserver also runs as my user and therefore is sandboxed to my account). The reason is that then, someone would have to hack my web application (Drupal) in order to cause trouble on my site, not be able to get in through some random back door in someone else's application that I didn't even know was on the server.

As for the drupal_unlink($settings_file), that $settings_file variable is basically defined as conf_path() . '/settings.php' which I think cannot be exploited. And also, the way the code works, we only ever delete the file stored in that variable if we just successfully created it ourselves a split second before, so I think that means it should be OK.

Rerolled the patch as per above.

Very good changes to the documentation. The patch works as advertised, automatically copying default.settings.php to settings.php in case the Drupal files are owned by the webserver. Copying fails when this is not the case and everything is as before.

comments and code look clear and useful. +1

Wow, this is great! Totally eliminates the red screen of death immediately greeting people during installation under MAMP, and does it with basically 5 lines of code and a bunch of comments! :)

I'm not sure if Moshe's comment was an explicit security team sign off. I would feel more comfortable committing this patch (and recommending it for backport) if this was done.

OK, I will sign off after this question: Is there a possibility that the copy() succeeds but the drupal_unlink() fails? It seems like we don't have error handling there.

Smart way to handle the UX problem for the audience that matters and I don't see any security regression. I'll defer to Moshe's question but otherwise I think it's RTBC.

Hm, are there any filesystem setups where a user has permission to create files but not to delete them? (I hope not.)

Beyond that, we'd be looking at some kind of transient failure. We could certainly check the return value of drupal_unlink() and do something if it's FALSE, but I guess the question is... what would we do?

Moshe's sent an email to the security team asking for review. I like this approach a lot, it's always embarrassing when you have to explain to a newbie why they have to connect to their server and mess with the files permissions etc.

I have no major concern with this patch. Minor question: have you thought of using a more robust comparison (===) between the two fileowner() calls? imagine the case where $default_settings_file is owned by root and the second fileowner($settings_file) fails and return FALSE (although it's likely the @copy() would have failed beforehand).

So what's the scenario where the webserver can write a settings.php file then has to unlink it? The sites/default directory must be group or world writable to the webserver?

I'd agree with scor in terms of using === to be safe, though I don't think it's truly needed given that the return value is an int (or FALSE):

"Returns the user ID of the owner of the file, or FALSE on failure."

@pwolanin: just to clarify, this is my point: with ==, if $default_settings_file is owned by root (fileowner($default_settings_file) will return 0) and fileowner($settings_file) returns FALSE, both files will be considered to be owned by the same user. I think === can do no harm here.

Security review:

I tested both use cases, with and without manual copying of the file. The patch works as expected. I also tested it with a multisite with no sites/default directory, and confirmed this does not allow the creation of new sites via a forged host header (the sites/foo directory must already exist).

Regarding Moshe's question "can copy() succeed but the drupal_unlink() fail" ?
I have not been able to devise a plausible** scenario where this could happen. Even if the process owner has a weird umask (like 0777), unlink() will override the permissions for the file owner.

(** If you are running FreeBSD, AND the sites/default directory has the setuid bit set, AND you have a weird umask that masks write permission from your own group.... then there might be a problem here. I think we can agree this is beyond an edge case.)

Still needs work for == vs. ===. I'm rerolling now, stay tuned.

Many of us on the sec team have signed off on this. I'd RTBC but it's my own patch. Fixed == vs. ===. Also fixed the comment a bit in the case where we have to unlink() as such:

+        // If settings.php and default.settings.php have different owners, this
+        // probably means the server is set up "securely" (with the webserver
+        // running as its own user, distinct from the user who owns all the
+        // Drupal PHP files), although with either a group or world writable
+        // sites directory. Keeping settings.php owned by the webserver would
+        // therefore introduce a security risk. It would also cause a usability
+        // problem, since site owners who do not have root access to the file
+        // system would be unable to edit their settings file later on. We
+        // therefore must delete this file and force the administrator to log
+        // on to the server and create it manually.

EDIT: sorry, crosspost. Ignore this.

Rerolled the patch from #185 with "===" between the two fileowner() functions.

removed the security tag.

#199 is good to go.

Incredibly minor cosmetic clarification to the comment above the unlink() clause to address something chx was confused about in IRC. Instead of this:

"... We therefore must delete this file and force the administrator to log on to the server and create it manually.

It now reads:

"... We therefore must delete the file we just created and force the administrator to log on to the server and create it manually.

Please wait for the bot to be sure, but this is slightly better than #199.

Excellent!! Thanks so much for your hard work on verifying this. I am so unbelievably excited to see this issue fixed at last.

Committed to HEAD. :D WOOOOO!

How about a D6 backport?

Also note that we might not want to backport this to D6, despite how awesome of a fix it is, because it would partially invalidate all installation documentation written in the past ~2.5 years. Gábor's call.

Yeah!

Just as a reminder: If Gábor doesn't want this backported to D6, please put this back on the D8 queue because there were ideas and lots of discussion in this issue on how to automate this process for users with *safe* servers too. Those were too late for D7 though.

@reglogge you might want to start fresh issue for D8, summarizing the ideas of these 200+ comments.

Yay - awesome!

Regarding a backport to D6, I don't think it would invalidate installation documentation exactly, just make some of the steps unnecessary for some users. But nothing would break if you continued to follow the old instructions and do those extra steps...

Before backporting, I wonder if we should think a bit more about that drupal_unlink() issue. It's true it's an extreme edge case (at least to have a server that does this by design), but I've certainly seen "blips" where a file system operation fails randomly - especially on shared network file systems, that kind of thing. It would probably be good if we had some kind of fallback code to handle that situation, even if it's not perfect.

Will think about that a bit more, but I think there might be a good way to do it. I'll try to get back to this in the next few days but probably not before then.

@reglogge in re: #206 --

I agree with David Rothstein on this. A new D8 issue should be created for how to automate this for users with safe servers. This one is already at > 200 comments, and will take a bit to get backported, probably.

I will do so, just not tonight :-)

D8 issue is here: #913796: Use authorize.inc to securely copy settings.php during the installer

For dealing with the case where drupal_unlink() fails, I think something like the attached should work.

It's not perfect, but does at least guarantee that the user gets the installation interrupted (with a one-time requirements error informing them of the problem) in the case where this happens. Given how rare we expect this event to be, that ought to be good enough.

+++ includes/install.core.inc	19 Sep 2010 20:56:32 -0000
@@ -1615,7 +1615,18 @@ function install_check_requirements($ins
+          $deleted = @drupal_unlink($settings_file);
+          // We expect deleting the file to be successful (since we just
+          // created it ourselves above), but if it fails somehow, we set a
+          // variable so we can display a one-time error message to the
+          // administrator at the bottom of the requirements list. We also try
+          // to make the file writable, to eliminate any conflicting error
+          // messages in the requirements list.
+          $exists = !$deleted;
+          if ($exists) {
+            $settings_file_ownership_error = TRUE;
+            $writable = drupal_verify_install_file($settings_file, FILE_READABLE|FILE_WRITABLE);
+          }

Seems like you could get rid of one of these variables $deleted OR $exists

Powered by Dreditor.

Well, we can't get rid of $exists because it's needed in the code below (not visible in the patch file), and I'd personally prefer to keep $deleted for purposes of code readability.

sounds good; I'm ok with the 2 variables.

That’s pretty excellent handling of a rare case.

Sorry, I have to disagree with Moshe. I know this is an edge case, but this will generally be misleading/wrong:

The @drupal installer failed to create a settings file with proper file ownership. Log on to your web server, remove the existing %file file, and create a new one by copying the %default_file file to %file.

This entire edge case is for handling the following situation:
- sites/default was writable by the httpd user
- the httpd user created settings.php and discovered the UID doesn't match the UID that owns settings.default.php
- the httpd user tried to remove the settings.php it just created and failed (presumably b/c of a temporary transient error).

So, at this point, we've got settings.php owned by the httpd UID which we *know* is different from the UID that owns settings.default.php. Therefore, when the end user tries to follow the above advice, logs into the webserver (presumably as the settings.default.php user), they're probably going to be unable to remove the file anyway since it's owned by someone else.

Granted, on UNIX file systems at least, assuming the end-user owns the sites/default directory itself, they should still be able to remove the file, even if it's owned by someone else (unless the sticky bit is set on the directory!).

I don't have a great suggestion on what else to say in this warning message, other than perhaps we need a d.o handbook page about this whole thing which can go into detail on the various edge cases and possible solutions and just point to that. But, I fear that the patch in #212 is just setting up the user for failure, given what we know about the situation they're in when the message is triggered...

Another thing to consider is to just retry the unlink() a few times before we give up? Maybe that's too much bloat for an edge case.

Anyway, I'm glad we're trying to handle the edge case, but I don't think telling the user to try something that's most likely going to fail is good. I'd rather we alerted them to the problem and linked them to a handbook page that can explain all the gory details and try to help them recover from the error.

Granted, on UNIX file systems at least, assuming the end-user owns the sites/default directory itself, they should still be able to remove the file, even if it's owned by someone else (unless the sticky bit is set on the directory!).

Right, I expected that to be the most common situation, which is why I explicitly had the instructions say remove the old file first, and THEN create a new one.

To me, the situation where even that doesn't work seems like an edge case within an edge case, so I don't see why we need to provide detailed instructions. We're giving them the correct set of steps to follow. If they don't have the right permissions for some reason, I think they could get help in a variety of ways (from their hosting provider, etc).

#200: 418302-copy-settings-197.patch queued for re-testing.

Re #217 and #218: I think David is right when pointing out that we are talking about a probably very rare edge case here. Nevertheless it might be good to point the user who experiences this to this Drupal handbook page: http://drupal.org/server-permissions. There is a lot of information on all kinds of problems and solutions there. We do this in other cases too.

The attached patch adds a link to this handbook page to the error message.

Best of both worlds?

messed up Component assignment...

Sure, that's fine. I revoke my concerns about being potentially misleading in an edge case of an edge case. We can't directly handle every possible case here, and this is *probably* going to be right most of the time. Ship it! ;)

Thanks @reglogge - that looks like a great compromise.

Awesome, thanks for the follow-up, folks!

Committed to HEAD.

Back to 6.x to see what Gábor has to say.

We're not backporting task patches to 6.x anymore, so closing as fixed in 7.x