Goal

  • Decrease security checks and validations to a single point of failure.

Proposed solution

  1. Change the installer to expose a required setting for the private filesystem path.
  2. Store the private filesystem path in settings.php.
  3. Create config directories in the private filesystem path.

Comments

hass’s picture

Maybe it's a good idea to have a third setting named "Configuration directory". This way we are able to make sure this never clash with file management tools like IMCE.

Berdir’s picture

While we could initialize it based on a private:// sub-directory, I really think we should do this: #1887750: Use path relative to DRUPAL_ROOT in configuration directories.

sun’s picture

#1887750: Use path relative to DRUPAL_ROOT in configuration directories does not seem to change or improve the situation with regard to this issue in any way — the directories are still in the publicly accessible space and thus require manual futzing with security and verification that files are not world-readable.

But that's the whole point here:

  1. Use a private:// filesystem, which should not be world-readable in the first place.
  2. If you move private:// to somewhere else and don't protect it, your fault.
  3. If you can't move private:// outside of the world-readable document root, then core will try to protect it from prying eyes, but only for private:// itself, and only once, not for 1,234,523 different directories.
hass’s picture

Fully agree with sun. What do you think about config:// or configuration:// as an extra configuration option?

hass’s picture

Issue summary: View changes

A

sun’s picture

config:// would not be a tangible improvement compared to now, as it would still be a one-off filesystem location. The point of private:// is that it is a single location and bucket for many (different) files that share the need of a non-world-readable storage.

I can see your point with private:// though. When configured accordingly, all user-uploaded files are stored in there, and a potential file browser UI would have to cater for not exposing certain subdirectories in it.

We could investigate a system://

At minimum, we'd want to use that for the config active and staging directories, as well as the PHP storage.

hass’s picture

system:// sounds good, too. However it's named it must be located in a non-world-readable storage. I just thought it is a clean abstraction from private:// folder where the content could also get listed by file management tools like IMCE.

Berdir’s picture

Enforcing a path below private:// would make that required, do we really want that?

What #1887750: Use path relative to DRUPAL_ROOT in configuration directories does is making the configuration work the same way as the public/private/temporary path. If it's relative, then it starts from DRUPAL_ROOT and if not, then it's absolute. No need to have a separate flag for that anymore. Right now, the configuration path *looks* as if it would be below public:// but that's a lie, it just has the same default value. So if you e.g. move the public files directory, you must not move the config directory or change that as well and make it "absolute".

If we expose the private path setting in the UI then we could default the configuration directory to be below private, but those two settings should IMHO not overlap.

Weither we use or introduce config:// or something else like that doesn't matter, that's just a fancier way to access it than a helper function, both need configuration behind it.

sun’s picture

I just followed up on #1856766: Convert file_public_path to the new settings system (make it not configurable via UI or YAML), which is also closely related.

Yes, I understand that #1887750: Use path relative to DRUPAL_ROOT in configuration directories changes the config directory path names to be no longer "related" to the public files directory. What it does not change or improve in any way is:

  1. The config directories are still within the public files directory.
  2. We need to (re-)implement advanced security checks for each of those directories to make sure that they are not world-readable.

With system://, we imply a completely new, required filesystem location. It would be a hard-coded stream wrapper, which would target to settings('file_system_path'). That's where config directories live, and PHP code is dumped into. The filesystem path must exist and be writable for Drupal to operate. Drupal will only verify that this single filesystem path is secured.

sun’s picture

Issue summary: View changes

A

mtift’s picture

hass’s picture

Maybe less priority, but the issue still exists if you import/export your configuration.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.