Repository navigation
Allow root owned config. #11075
Description
It would be good to fully support read-only configs, in order to be able to support a setup where the config is owned by root in /etc/nextcloud/ for example. This increases security, and better supports packaging Nextcloud into Linux distributions.
With some minor tweaks to the code, I have successfully managed to run Nextcloud as described, and have managed to perform an upgrade successfully by manually making changes to the config.
In order to accomplish this properly, these are the changes I believe are needed:
- In order to run Nextcloud normally with a root owned config, Nextcloud should check for the owner of the datadirectory instead of the config file (given this is the only thing it should be editing). (Check datadirectory owner, not config owner. #27613, feat: allow to configure php.user #45307)
- Regression in v24+: fix: regression with updating read-only config #44039
- For upgrades, Nextcloud should use the database to store non-user options during the upgrade process, rather than editing the config file.
- The loglevel could be set in the DB, and then perhaps the lowest value between the config and the DB is the level used. This way the upgrade process can set debuglevel to 0 in the DB, and then unset it after completion, leaving the user's setting alone.
- maintenance could be set in the DB, with maintenance mode being enabled if the flag is found in either the config or DB. Again, the upgrade can then set this in the DB, and unset it after upgrade.
- version could be set in the DB. Surely the release number is primarily used to ensure the DB is upgraded? Therefore the value belongs in the DB and not with the user config. (This makes me wonder what might go wrong if someone imports their database from a backup, but are still using the current config file with a newer release number and the imported DB is then in the wrong state.)
- Additionally, Nextcloud should avoid writing default settings to the config. When upgrading, it wanted to write "'theme' = ''", which is the default value if not present.
- If Nextcloud really does need to update a value in the config, it should pause the upgrade and tell the user what value needs to be changed/added and then allow the user to continue the upgrade after manually updating the config.
Activity
nextcloud-bot commentedon Sep 5, 2018
GitMate.io thinks possibly related issues are #6550 (Config 'trusted_proxies'. Allow networks with CIDR), #5117 (unsafe nginx config), #10120 (allow to disable encryption), #10121 (allow to disable encryption), and #10018 (Update root.vue).
ct-zen commentedon Sep 10, 2018
Neither config nor apps should NEED to be writable except when you want to make changes. At all other times those areas should be locked down to prevent the webserver software from making unwanted/unauthorized changes. This way you are protected from unauthorized changes that may come about through weaknesses in any of the systems involved (code, webserver, php, leaked/weak credentials, etc.).
flokli commentedon Nov 18, 2018
Also very interested in support for a read-only config through the whole lifecycle of nextcloud (installation, running, upgrading).
We currently run into unwanted side-effects with having the
occ maintenance:installand nextcloud itself writingconfig.php, while overriding some parts inoverride.config.php(see NixOS/nixpkgs#49783 ).@Dreamsorcerer As for the issues you mentioned, I think it might help breaking them down to PRs, linking to this issue.
Thinking your ideas further, this would also imply the
occ maintenance:installcommand by default reading database options etc. from the config file instead of command line arguments provided (and not writing to the config file on its own, as it doesn't change when options are being read from there obviously).Dreamsorcerer commentedon Nov 18, 2018
Of course, but the 4 line patch I linked is the only work I've put into this so far. Certainly doing a separate PR for each step will be useful if someone does start to work on this seriously.
Kixunil commentedon Jan 28, 2021
I don't think any checks of owners are correct. Checking scripts should only check read
access(). (But why check it at all and not just try to read it?) There should be no need for write access.Dreamsorcerer commentedon Jan 28, 2021
Write access is needed to complete the upgrade, right? Because it will try to update the apps installed.
Kixunil commentedon Jan 28, 2021
I mean writes should go to DB. Besides upgrades could be handled by package managers instead. Then write should not be needed.
Dreamsorcerer commentedon Jan 28, 2021
But an app is a set of files downloaded on the system (in the datadir). To upgrade them, you must overwrite those files with the new app...
Kixunil commentedon Jan 29, 2021
Apps can be stored in another directory and/or can also be handled by package managers.
Dreamsorcerer commentedon Jan 29, 2021
It would still need write access to that directory in order to complete the upgrade, how would that be any different from how it works right now?
Package managers sounds like a bad idea, as now you need someone to package and update everything in the Nextcloud app store and keep up to date with all new submissions. You would also need to disable the app store in the Nextcloud interface as it would result in broken behaviour. My proposal is to make it easier to package Nextcloud (e.g. in Debian), not harder.
Kixunil commentedon Jan 29, 2021
The question is what exactly does it need to write to finish the upgrade? Why is replacing the files (by package managers) not sufficient? Can the code be modified to no longer need it? If yes, then it gives the users/packagers more options, to choose from. They can still do it without packaging. It's not forcing people into it.
You enumerated disadvantages of packaging but forgot about advantages. A very similar situation arises with Python PIP. The software outside system package management can not have system-level dependencies or they must be installed manually. This is already the case with Nextcloud video conversion app which requires you to install
ffmpegmanually. If it were Debian package for instance, this would be solved trivially.Updates are obviously easier to perform using one tool rather than two tools.
System packaging also often gives you some guarantees about the packaged software: it's reviewed and signed. I didn't check Nextcloud apps deeply enough to tell but I'd be surprised if Nextcloud app store has security and reliability guarantees comparable to those of Debian.
Finally, the problems you describe can be mitigated by automating packaging (as is already done in Debian in cases of Python, Rust...) Disabling app store is already possible in config and it's non-issue if the administrator installed Nextcloud using package manger - he can install apps too.
I would personally like to see it the same as with Python PIP - package apps as
.debbut also keep the option to use app store.3 remaining items
Kixunil commentedon Jan 29, 2021
Oh, you were talking about writing to data directory (
/var/lib/nextcloud-server?), which must obviously be allowed for other reasons. I was thinking about/appsdirectory inside the source code (presumably/usr/share/nextcloud-server/apps). Ideally the default would be for the two to not conflict. Just as PIP doesn't conflict with system packages. (Although it does lead to surprising results so I wouldn't recommend it using same package from both.)Dreamsorcerer commentedon Jan 29, 2021
Ah, I forgot that this is not the default behaviour. The way I have the config setup (which would be required for Debian packages) is with an app path in the data directory. It still picks up system installed apps just fine, but user installed apps go into the data directory.
Kixunil commentedon Jan 29, 2021
Yes, you can have multiple paths and it should be used in packaging. :)
Not sure what happens if you install same app twice.
Dreamsorcerer commentedon Jan 29, 2021
But, basically, it's that path which needs to be writable, because Nextcloud will try to upgrade incompatible apps.
System apps in the read-only path are fine, they won't be touched.
With a couple of hacks I have this working already, with me manually updating the core nextcloud code as root, and then running the usual upgrade step in a browser.
At some point, I want to come back to this issue so I can implement it properly without hacks and make it user-friendly.
Kixunil commentedon Jan 29, 2021
Cool! I was looking into making
maintenance:installread the config so that passwords don't have to be on command line. (Useful fordbconfigin Debian.) I have something somewhat working but I got stuck at login redirect loop (I tried passing various pathinfo/script names, nothing seems to help.). By digging into the code a lot it seems that/loginroute is lost somewhere but I can't understand where. If you happen to have an idea that'd be great. (I realize that this is off-topic but if anyone helps me, it frees up my time to look at the topic of this issue. :))Dreamsorcerer commentedon Jan 29, 2021
No idea, sorry.
szaimen commentedon Jun 22, 2021
Do you mind creating a PR with this patch for further discussion?
Thank you!
Dreamsorcerer commentedon Feb 29, 2024
First step has finally been merged. Next up is storing some of the options used during upgrade in the DB.
Kixunil commentedon Mar 1, 2024
Cool! FYI I have some initial packaging work done. It works mostly well except upgrades of NC versions are now broken. Also I've automated the apps packaging as I mentioned, it was pretty easy.
Dreamsorcerer commentedon Mar 6, 2024