Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Created files/directories on local storage ignore web server umask and therefore have incorrect permissions set #29041

Open
tjwood100 opened this issue Oct 3, 2021 · 2 comments
Open

Created files/directories on local storage ignore web server umask and therefore have incorrect permissions set #29041

tjwood100 opened this issue Oct 3, 2021 · 2 comments
Labels
0. Needs triage bug

Comments

@tjwood100
Copy link

@tjwood100 tjwood100 commented Oct 3, 2021 •
edited

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

  1. Set Apache umask to 0007 in /etc/apache2/envvars or equivalent
  2. Create a file or directory on local storage

Expected behaviour

Created files/directories should observe Apache's umask,
i.e. if Apache's umask is 0007 and if I have a directory with permissions
drwxrwsr-x me mygroup

And I create a file within that directory in nextcloud, it should have the permissions
-rw-rw---- www-data mygroup

and I as member of mygroup should be able to write to the file.

My use case is that I access files in local storage externally as well as via Nextcloud. It should be possible for me (as a member of the group the files are created under) to locally edit a file or write to a directory created via Nextcloud.

Actual behaviour

Apache's umask is ignored and the file is created with rw-r--r-- permissions therefore I can't write to it from outside of nextcloud.

Note that as well as not being group writeable, they are also other-readable, which could be unexpected.

This issue didn't happen in earlier versions of nextcloud - it appears to have been introduced by this commit e5dc1a8 which forces umask to be 022. Also this replaced an earlier commit d182043 which forcibly set permissions on created directories to 755.

Server configuration

Operating system: Ubuntu 20.04.3

Web server: Apache

Database: mariadb

PHP version: 7.4.3

Nextcloud version: Nextcloud 22.2.0

Updated from an older Nextcloud/ownCloud or fresh install: Fresh install

Where did you install Nextcloud from: tarball

Signing status:

Signing status No errors have been found.

List of activated apps:

App list Enabled: - accessibility: 1.8.0 - activity: 2.15.0 - bruteforcesettings: 2.2.0 - circles: 22.1.1 - cloud_federation_api: 1.5.0 - comments: 1.12.0 - contactsinteraction: 1.3.0 - dashboard: 7.2.0 - dav: 1.19.0 - federatedfilesharing: 1.12.0 - federation: 1.12.0 - files: 1.17.0 - files_external: 1.13.0 - files_pdfviewer: 2.3.0 - files_rightclick: 1.1.0 - files_sharing: 1.14.0 - files_trashbin: 1.12.0 - files_versions: 1.15.0 - files_videoplayer: 1.11.0 - firstrunwizard: 2.11.0 - logreader: 2.7.0 - lookup_server_connector: 1.10.0 - nextcloud_announcements: 1.11.0 - notifications: 2.10.1 - oauth2: 1.10.0 - password_policy: 1.12.0 - photos: 1.4.0 - privacy: 1.6.0 - provisioning_api: 1.12.0 - recommendations: 1.1.0 - serverinfo: 1.12.0 - settings: 1.4.0 - sharebymail: 1.12.0 - support: 1.5.0 - survey_client: 1.10.0 - systemtags: 1.12.0 - text: 3.3.0 - theming: 1.13.0 - twofactor_backupcodes: 1.11.0 - updatenotification: 1.12.0 - user_status: 1.2.0 - viewer: 1.6.0 - weather_status: 1.2.0 - workflowengine: 2.4.0 Disabled: - admin_audit - encryption - user_ldap

Nextcloud configuration:

Config report 'dbtype' => 'mysql', 'version' => '22.2.0.2',
the rest is not relevant

Are you using external storage, if yes which one: yes - local

Are you using encryption: no but not relevant

Are you using an external user-backend, if yes which one: no

Client configuration

Browser: not relevant

Operating system: not relevant

Logs

not relevant
@tjwood100 tjwood100 mentioned this issue Oct 3, 2021
Closed
@tjwood100
Copy link
Author

@tjwood100 tjwood100 commented Oct 3, 2021 •
edited

I have verified that commenting out the ten lines that call umask() in Local.php fixes this issue, so it was definitely a regression introduced by e5dc1a8 (and it looks like d182043 was a previous attempt that would have done similar).
It's not clear what issue either of these were trying to fix - they aren't linked to bug reports as far as I can tell.
(The commit message on the earlier of these says "this works around any umask that might be set and limiting the folder permissions", but it's overlooked that these changes themselves are limiting the folder/file permissions, which is a problem in this case)

@tjwood100 tjwood100 changed the title Created files/directories on local storage ignore Apache umask and have incorrect permissions set. Created files/directories on local storage ignore web server umask and therefore have incorrect permissions set Oct 3, 2021
@tjwood100
Copy link
Author

@tjwood100 tjwood100 commented Oct 3, 2021 •
edited

Judging by the tests added in commit e5dc1a8 it seems that the intent of the change is to make sure that if Nextcloud creates a file or directory, it is then able to write it. That seems like a reasonable aim.
But it seems that explicitly setting the umask to 022 is a very crude way of doing that, which inadvertently changes the group and other permissions.

A better pattern might be something like:

$oldMask = umask();
umask($oldMask & 077); // clear owner mask, but keep group and others parts of mask intact
$result = // do whatever
umask($oldMask);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
0. Needs triage bug
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
@tjwood100