On a test Fedora 16 system with PHP 5.3, Drupal cannot move a file to the php/service_container/.php directory on a FUSE-mounted file system unless the directory is also readable. I'm not sure if the file system being FUSE makes a difference here, but I'm guessing the current code worked for someone with a local files/ directory when they tested it.
Bumping the permissions to 0700 does the trick. If a site has no support for .htaccess, no alternative configured directory for generating these (which chx said will be an option), and no other web server restriction, this might open up access very briefly until the permissions get reduced down again to 0100. But, I'm not sure what the alternative might be.
Comment | File | Size | Author |
---|---|---|---|
fix-generated-php-permissions.patch | 724 bytes | David Strauss | |
Comments
Comment #1
David StraussComment #2
kotnik CreditAttribution: kotnik commentedTests can not be cleaned up because of those permissions too: #1829960: Failure to delete unreadable and unwritable directories.
Comment #3
sunI think this also affects me on Windows. I'm getting tons of access denied errors caused by phpstorage on every full cache flush.
Comment #4
webchickSeems like this could use security team sign-off.
Comment #5
gregglesIf I understand: the directory it's already writable, this just makes it readable/listable too. In general I think the security concern is around writing files, so I don't see a problem with this change but don't consider my knowledge to be complete.
Comment #6
sunThat's my understanding, too.
Assigning to @chx to potentially object, but otherwise, this still looks RTBC to me.
Comment #7
chx CreditAttribution: chx commentedYou are weakening the system by an extremely small amount and I won't object to it -- there now exist a very small period of time when the secret hash can be learned provided but only if the server is already breached and you would still need to chmod the file and rewrite it to breach the security. So, go ahead.
Also, really, we should make the php storage profile dependent and move to filestorage in minimal and add a question to the installer which adds a directory outside of docroot. Followup, someone please do it.
Comment #8
sunDoesn't that translate into a installer widget/screen for configuring the location of the
private://
stream wrapper filesystem location in reality?Happy to whip that up as a patch (even though that's a Novice task after #1798732: Convert install_task, install_time and install_current_batch to use the state system).
Comment #9
chx CreditAttribution: chx commentedNo, it doesn't and it should be minimal only to make the standard installer as lean as possible.
Comment #10
Dries CreditAttribution: Dries commentedCommitted to 8.x. We can always follow-up if necessary.
Comment #12
sunNot completely fixed for Windows yet: #1885796: PHP warnings in PhpStorage\FileStorage::save()