Task #52668
closed
Install Tool: Remove permission checking and fixing code from "folder structure"
Added by Ernesto Baschny about 11 years ago.
Updated about 7 years ago.
Estimated time:
(Total: 0.00 h)
Description
We've got several complaints and issues with the new "folder permission" checking and fixing - In the Install Tool "Step Installer" and later in the "Folder Structure" menu entry.
So we decided to simply remove this functionality from the install tool for now, and leave the responsability of a "secure" and "working" environment with the administrator / hosting provider. There are just too many constelations and exceptional conditions to consider here.
Default permissions (for new folders / files) should be:
Folders: 2775
Files: 0664
In the "Folder Structure" checking section a warning could be displayed if these are the active settings, pointing to a Wiki page describing different methods on how to "secure your installation". The wiki page could be collaborative effort documenting the best practice setup for different PHP vs Webservers vs OS variants.
Streamline what is considered an "Error", "Warning" and Notice":
Errors are:
- Default permissions allowing world (other) writing (o+w)
- Directories that do not exist
- Directories that are not writeable (no files can be created within)
- Paths that are files but should be directories
- Paths that are not files but should be files
- Symlinks that should be present but do not exist
Warnings are:
- Default permissions allowing world (other) reading (o+r)
- Symlink that is not a symlink (i.e. typo3_src)
- Files that should be present but do not exist
Notices are:
- Default permissions allowing group (other) r/w (g+rw)
- Files that cannot be written (index.html, .htaccess)
- Files which not match the configured default permission
- Files that do not contain what we expect
Files
- Status changed from Accepted to Under Review
- Assignee changed from Christian Kuhn to Ernesto Baschny
- % Done changed from 0 to 90
- Complexity set to medium
Would it be possible to use only a umask instead of separate file and dir permissions? The umask could be set once per script invocation (or just outside of the loop doing the file operations). This would save a lot of octdec() calls and chmod() calls.
Just an idea. Have not done any tests yet. The file creation functions respect the current umask.
The PHP man page says:
Avoid using this function in multithreaded webservers. It is better to change the file permissions with chmod() after creating the file. Using umask() can lead to unexpected behavior of concurrently running scripts and the webserver itself because they all use the same umask.
So I guess this would be a nono for most cases. Implementing it as an option could be done, but would introduce unwanted complexity. So I guess we need to live with chmod for a while longer.
We should cache the octdec() calls in the calling classes though.
Please see this chmod test. All mode values are stored in a variable.
You can see that in the first case, where the mode is stored as an int, and prefixed with a 0, the permissions are set correctly without a call to octdec.
Here's a little test to figure out how PHP handles this whole chmod situation. We try both string values and plain int values to set the access controls.
It turns out that mkdir does not respect the sticky bit. An extra chmod is needed to add it. Not even ignoring the umask helps. Is this a PHP bug?
http://pastebin.com/RnHcFmJx
If we decide to keep on using the string values for the mode, the octdec() call should happen only once per class instance, when setting the mode property. The default property would have to be the 'dec' value then: 436 or 1533.
protected function setTargetPermission($permission) {
$this->targetPermission = octdec($permission);
}
If we change the mode property to be a plain integer, we can completely do away with the octdec() calls. But the direcotry mode must be prefixed by a 0 for that to work correctly.
- Status changed from Under Review to Resolved
- % Done changed from 90 to 100
- Estimated time set to 0.00 h
The fileCreateMask has been set to 0665 in the changeset, which is wrong.
As stated above 0664 should be the default setting.
- Estimated time deleted (
0.00 h)
- Status changed from Resolved to Closed
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by 
Updated by 
Updated by