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

Can't run installer with unwritable apps directory #10243

Closed
gdamjan opened this issue Jul 15, 2018 · 15 comments
Closed

Can't run installer with unwritable apps directory #10243

gdamjan opened this issue Jul 15, 2018 · 15 comments

Comments

@gdamjan
Copy link
Contributor

gdamjan commented Jul 15, 2018

Nextcloud 13.0.4 is installed from an ArchLinux package in /usr/share/webapps/nextcloud. In my configuration, I'm running it in a constrained environment with uwsgi+php, and the service can only read the files in /usr/share/webapps/nextcloud and only write to /var/lib/nextcloud (using NEXTCLOUD_CONFIG_DIR=/var/lib/nextcloud env var). In this setup the data is stored in /var/lib/nextcloud/data.

Unfortunately the install fails before even showing the template to setup admin user/data dir with:

    Error

    Cannot write into "apps" directory

    This can usually be fixed by giving the webserver write access to the apps directory or disabling the appstore in the config file. See https://docs.nextcloud.com/server/13/go.php?to=admin-dir_permissions

It does create an almost bare config file (just instanceid), and if I add 'appstoreenabled' => false there the install will then work. Interestingly if I later remove appstoreenabled nextcloud will still work. So it only breaks the install process. (edit: it works when I'm logged in, if I logout it will be broken still).

Steps to reproduce

  1. fresh install of nextcloud
  2. setup nextcloud without permissions to write to its directory
  3. use NEXTCLOUD_CONFIG_DIR= so it can write a config file in another place
  4. try to configure it through its online wizard

Expected behaviour

Installed should ignore that the apps directory is not writable.

Actual behaviour

Installer fails with an error.

Server configuration

  • archlinux
  • nginx 1.14.0
  • uwsgi 2.0.17.1
  • sqlite3
  • php-7.2.7
  • nextcloud 13.0.4.0
  • fresh install from OS package
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #1965 (can't delete directory ), #6833 (Cannot run command line apps from command line), #3786 (Directory name), #6021 (Remember the current directory when switching apps (Files/Gallery)), and #2977 (can't active apps).

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 15, 2018

the code is the same in master too
https://github.com/nextcloud/server/blob/master/lib/private/legacy/util.php#L755-L760

it should probably be a warning at best (if not removed altogether)

@MorrisJobke
Copy link
Member

the code is the same in master too
https://github.com/nextcloud/server/blob/master/lib/private/legacy/util.php#L755-L760

it should probably be a warning at best (if not removed altogether)

No - this one is correct. if you allow to install apps from the appstore, then it needs to be able to write them somewhere. If you disable the appstore you can run a fully read-only install, but with the appstore enabled you need to have a writable apps directory.

@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 16, 2018

I've removed that part of the code and apart from the appstore (which I didn't try) everything else worked just fine.

No - this one is correct

I'm just saying that it shouldn't stop the install, it can emit a warning, and error out when I try to use the appstore. Or even, just setup appstoreenabled => false in the config and warn.

but with the appstore enabled you need to have a writable apps directory.

and Nextcloud already supports having a different writable apps directory, so I'm just saying don't stop the install. Warn, and suggest to setup writable apps directory.

ps.
I'd be willing to send some patches on the agreed fix.

@MorrisJobke
Copy link
Member

I'm just saying that it shouldn't stop the install, it can emit a warning, and error out when I try to use the appstore. Or even, just setup appstoreenabled => false in the config and warn.

But why? If you enable the appstore but can't use it? It is an hard failure if the web server can't do everything it is told to do. Disabling the appstore means, that it does not need to write to the apps directory and then Nextcloud is fine to have only read-only apps directories.

and Nextcloud already supports having a different writable apps directory, so I'm just saying don't stop the install. Warn, and suggest to setup writable apps directory.

Correct. Set it up and then the instance should work just fine as it only needs one apps folder where it can write to. So it works actually like it is.

@MorrisJobke MorrisJobke removed the bug label Jul 16, 2018
@gdamjan
Copy link
Contributor Author

gdamjan commented Jul 16, 2018

Correct. Set it up and then the instance should work just fine as it only needs one apps folder where it can write to. So it works actually like it is.

I would actually expect the installer to allow me to do that, instead of stopping with an error

@MorrisJobke
Copy link
Member

I would actually expect the installer to allow me to do that, instead of stopping with an error

cc @rullzer @nickvergessen Opinion on this one?

@rullzer
Copy link
Member

rullzer commented Jul 17, 2018

I don't have a strong opinion here.

But I think this is really a corner case. We only support setting up Nextcloud via our release tarballs and there the 99.9% of the people that follow our docs have a writable app dir.

I'm a bit hessitant to add it as it will be a code path that is hardly ever used I think and thus very likely to break at some point. Of course if there are a bunch of unit and acceptance tests that all would be fine.

@nickvergessen
Copy link
Member

I agree with rullzer here. Sounds like it is not worth the troubles.

@MorrisJobke
Copy link
Member

I'm a bit hessitant to add it as it will be a code path that is hardly ever used I think and thus very likely to break at some point. Of course if there are a bunch of unit and acceptance tests that all would be fine.

The requests for this are quite low. Currently there a no plans to implement this that way. Thus I will close this ticket for now. This does not mean we don't want this, but it is simply not on our roadmap for the near future and the maintenance of this has also taken into account. If somebody wants to implement and maintain this nevertheless we are happy to assist and help out. I would also like to see automated tests for this then.

@ct-zen
Copy link

ct-zen commented Sep 10, 2018

My reason for wanting this feature to be available is for security. As a normal matter of course, it isn't wise to trust that any code is 100% exploit free (or PHP for that matter) and solely rely upon the application's authentication system to keep attackers at bay (even with password complexity/strength). In production, except for those narrow times when we actually want to make changes, we don't want anything but the data directory to be writable. I can script the permission changes for apps and config during those times we actually want to make changes.

@nickvergessen
Copy link
Member

@ct-zen what I do is:
I added a writable folder as app directory, which is outside of the webroot, so it can not be accessed from the internet.

<?php
$CONFIG = array (
  …
  'apps_paths' => 
  array (
    0 => 
    array (
      'path' => '/var/www/nextcloud/apps',
      'url' => '/apps',
      'writable' => false,
    ),
    1 => 
    array (
      'path' => '/var/www/noapps',
      'url' => '/../noapps',
      'writable' => true,
    ),
  ),
…

So there is no writeable folder accessible from the web

@ct-zen
Copy link

ct-zen commented Sep 27, 2018

@ct-zen what I do is:
I added a writable folder as app directory, which is outside of the webroot, so it can not be accessed from the internet.

snip snip
So there is no writeable folder accessible from the web

Thanks for that bit of info. I'll certainly use the part of it to relocate apps, but it doesn't quite fit the security model we need to use. Config/apps needs to remain immutable (not just inaccessible to direct browsing) until we want to make changes. It's programmatically easier to change ownership and permissions to control this capability vs. having to edit code whenever we want to toggle "writable" as false, so we've simply modified the logic that tests those areas for writability. We chown/chmod to unlock and reverse that to lock. Leaving everything owned as a non-web user and running "occ" as that non-web user to manage things works very well for us. From the non-web user's perspective, everything is writable; from the web user's perspective, only uploads is writable.

@gdamjan
Copy link
Contributor Author

gdamjan commented Nov 25, 2018

would it be acceptable if I make a patch to optionally skip the aforementioned check which runs at first run time (before the config file is created)?

@dumblob
Copy link

dumblob commented Nov 25, 2019

I also ran into this problem with a similar setup as @ct-zen described. Modifying configuration is error prone when done manually in a self-updating system (and ownCloud/Nextcloud do break stuff still too often for this kind of automation).

Any chance of more graceful behavior? What about the patch from @gdamjan?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants