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

[tjwood100]

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]

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]

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);

[Flowdalic]

I'd also like to add that it took me a while to figure out why nextcloud would not respect setting UMask in the corresponding systemd unit override. The culprit here is PR #25280. Besides they approach already mentioned, this change should not invoked umask() with cleared 'other' bits, as this allows for world-readable files. Sure, you can further protect access to nextcloud files by -x'ing the data directory, but when it comes to permissions and security, a defensive approach is always preferable.

[bgottschall]

How about putting a configuration parameter in the config file for such an important part of a cloud solution like setting file and folder permissions? I don't like that Nextcloud is forcing those permission when I want to have control who has access to my files. I'm not opposed to setting the defaults to umask(027), but giving the opportunity to change them accordingly would solve this issue.

[MartinBrugnara]

@bgottschall see if pr #32723 covers it.

[bgottschall]

@MartinBrugnara if one is pedantic it does not fix this issue as nextcloud would still ignore the umask given to it from the OS or the starting service. Normally umasks are configured where the process is started. Your pr gives one at least the opportunity to configure it within nextcloud. I don't really understand why the default umask needs to be always hardcoded when it is given from the system itself. I assume setting the default umask in the nextcloud config to umask() wouldn't work with some poeple since it is not a constant?!
Maybe that discussion should be placed in your pr. Feel free to copy it over if you like.

[MartinBrugnara]

@bgottschall I agree it does not fix the problem in general, it's just one step in the right direction.

As you said, my PR just does what you suggested in the comment, it exposes the umask setting via config file instead of hard-coding it.

I don't really understand why the default umask needs to be always hardcoded when it is given from the system itself. I assume setting the default umask in the nextcloud config to umask() wouldn't work with some poeple since it is not a constant?!

I didn't replicate the specific issue but the commit message from #25280 (451c06d) suggests that third-party scripts, extraneous from Nextcloud but running on the same workers, may tamper with umask and thus break folder/files creation. Note that in a well thought deployment this should never happen.

Nevertheless we may want to ensure that no matter what the ENV configuration is for umask, Nextcloud creates the files with minimal set of permissions to guarantee the correct execution. This could be achieved by the selecting proper max-umask e min-perm and then play with the bits as @tjwood100 suggested. At this point we could deal also with tampered umask values.

I am not sure what the correct solution is for all scenarios, only that having umask hard-coded was not working for my installation... so I tried to help =)

EDIT: we could also just put the "override umask" action behind a feature-flag and document it properly,

[bgottschall]

@MartinBrugnara I absolutely appreciate your pr as it also solves my problems. In fact I had another pr pending which got obsolete because of yours. Though I did only remove the fixed umask out of the local storage functions since I never understood what it was useful for. I never understood the commit message of #25280 fixing 'other php stuff', because its not the responsibility of the nextcloud code to alleviate any potential wrong doing of 'other php stuff'.
So after I studied this problem myself, I never got why one would want to fix or even change the umask that is given from the system. If the umask is wrong the system is configured wrong and not nextcloud. If the umask is changed by other stuff, the other stuff is responsible or should be fixed. I never got any explanation why things were done like they were done and the author of #25280 @icewind1991 is quite active on github, was supposed to review my pr and stayed silent. Maybe he can bring some light into this issue.

[flan7]

I am having difficulties with this issue, is it still hard coded? Odd behavior.

[bgottschall]

@toqov have you configured the umask in the nextcloud config? It is still hard coded basically. Changing it the default way like everyone would do it on a linux server doesn't work with nextcloud anymore, but you can now configure it in the nextcloud config.

'localstorage.umask' => 0000

[flan7]

@toqov have you configured the umask in the nextcloud config? It is still hard coded basically. Changing it the default way like everyone would do it on a linux server doesn't work with nextcloud anymore, but you can now configure it in the nextcloud config.

'localstorage.umask' => 0000

Even with setting the umask this way to 0002, nextcloud is still saving files as -rw-r--r--

This seems to cause permission errors with my unraid NFS share where my data dir is located.

[MartinBrugnara]

@toqov please verify which version you are running, afaik #32723 has been merged for NC25 but has not yet been backported for <= 24.

You could wait a few days for the release of 25
or manually apply the patch to your installation,
or, even better, help the project complete the backports ;)

[Shrizt]

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.

Finally Issue resolved in version 25+
Just add to config.php
'localstorage.umask' => 002,
it will make new files group writable.
or whatever umask you need. It is now not hardcoded (code var load added in www/nextcloud/lib/private/Files/Storage/Local.php )

[alexsmartens]

Would it be reasonable to allow changing permissions for creating new files/directories in Admin Settings?
I installed Nextcloud server via snap. All snap packages are installed on a read-only files system so that I cannot edit neither config.php nor Local.php.

[szaimen]

Hi, please update to 24.0.9 or better 25.0.3 and report back if it fixes the issue. Thank you!

My goal is to add a label like e.g. 25-feedback to this ticket of an up-to-date major Nextcloud version where the bug could be reproduced. However this is not going to work without your help. So thanks for all your effort!

If you don't manage to reproduce the issue in time and the issue gets closed but you can reproduce the issue afterwards, feel free to create a new bug report with up-to-date information by following this link: https://github.com/nextcloud/server/issues/new?assignees=&labels=bug%2C0.+Needs+triage&template=BUG_REPORT.yml&title=%5BBug%5D%3A+

[alexsmartens]

Still facing the same issue when copying files through the browser. Any chance someone could help?

Installation info:

snap list nextcloud
Name       Version      Rev    Tracking       Publisher   Notes
nextcloud  27.1.4snap1  39212  latest/stable  nextcloud✓  -

Nextcloud.forbidden.mov

[joshtrichards]

@alexsmartens A better place to post your query at this point would probably be the Nextcloud Help Forum - https://help.nextcloud.com

I installed Nextcloud server via snap. All snap packages are installed on a read-only files system so that I cannot edit neither config.php nor Local.php

https://github.com/nextcloud-snap/nextcloud-snap?tab=readme-ov-file#configuration

[alexsmartens]

In case anyone is facing the same issue, this is what helped me:

sudo snap disable nextcloud
sudo chown -R root:root /path/to/nextcloud_data_directory
sudo chmod -R 0770 /path/to/nextcloud_data_directory
sudo snap enable nextcloud
sudo snap run nextcloud.occ files:scan --all