Task #52668
closed
Install Tool: Remove permission checking and fixing code from "folder structure"
100%
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
- 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
- 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
Updated by Gerrit Code Review about 11 years ago
- Status changed from Accepted to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Gerrit Code Review about 11 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Ernesto Baschny about 11 years ago
- Assignee changed from Christian Kuhn to Ernesto Baschny
- % Done changed from 0 to 90
- Complexity set to medium
Updated by Gerrit Code Review about 11 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Gerrit Code Review almost 11 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Michiel Roos almost 11 years ago
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.
Updated by Gerrit Code Review almost 11 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Michiel Roos almost 11 years ago
- File chmodTest.php chmodTest.php added
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.
Updated by Michiel Roos almost 11 years ago
- File chmodTest.php chmodTest.php added
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?
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.
Updated by Gerrit Code Review almost 11 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Gerrit Code Review over 10 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Gerrit Code Review over 10 years ago
Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Gerrit Code Review over 10 years ago
Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/25416
Updated by Ernesto Baschny over 10 years ago
- Status changed from Under Review to Resolved
- % Done changed from 90 to 100
Applied in changeset 18a8f44414593cccd3469ff63afe6d90ebd7cab1.
Updated by Jan Radecker over 10 years ago
- 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.
Updated by Riccardo De Contardi about 7 years ago
- Status changed from Resolved to Closed