nextcloud / docker

⛴ Docker image of Nextcloud
https://hub.docker.com/_/nextcloud/
GNU Affero General Public License v3.0
6.08k stars 1.83k forks source link

Introduced redis session handler does not work with non-root images #763

Open smueller18 opened 5 years ago

smueller18 commented 5 years ago

In commit 83ea69d54bea72e1df9a2647ee101c73deb86d44, a redis session handler was intrododuced. But the file /usr/local/etc/php/conf.d/redis-session.ini can not be written when using a non-root image, because the folder is owned by root and the default www-data user does not have write permission.

/var/www/html # ls -lha /usr/local/etc/php/conf.d
total 72
drwxr-xr-x    1 root     root        4.0K Feb 26 08:50 .
drwxr-xr-x    1 root     root        4.0K Feb 26 07:34 ..
-rw-r--r--    1 root     root          35 Feb 26 08:50 docker-php-ext-apcu.ini
-rw-r--r--    1 root     root          18 Feb 26 08:47 docker-php-ext-exif.ini
-rw-r--r--    1 root     root          16 Feb 26 08:47 docker-php-ext-gd.ini
-rw-r--r--    1 root     root          21 Feb 26 08:50 docker-php-ext-imagick.ini
-rw-r--r--    1 root     root          18 Feb 26 08:48 docker-php-ext-intl.ini
-rw-r--r--    1 root     root          18 Feb 26 08:48 docker-php-ext-ldap.ini
-rw-r--r--    1 root     root          23 Feb 26 08:50 docker-php-ext-memcached.ini
-rw-r--r--    1 root     root          82 Feb 26 08:48 docker-php-ext-opcache.ini
-rw-r--r--    1 root     root          19 Feb 26 08:48 docker-php-ext-pcntl.ini
-rw-r--r--    1 root     root          23 Feb 26 08:49 docker-php-ext-pdo_mysql.ini
-rw-r--r--    1 root     root          23 Feb 26 08:49 docker-php-ext-pdo_pgsql.ini
-rw-r--r--    1 root     root          19 Feb 26 08:50 docker-php-ext-redis.ini
-rw-r--r--    1 root     root          20 Feb 26 07:34 docker-php-ext-sodium.ini
-rw-r--r--    1 root     root          17 Feb 26 08:49 docker-php-ext-zip.ini
-rw-r--r--    1 root     root          18 Feb 26 08:50 memory-limit.ini
-rw-r--r--    1 root     root         189 Feb 26 08:50 opcache-recommended.ini

For running my nextcloud image, I use the default non-privileged user www-data.

/var/www/html $ id
uid=82(www-data) gid=82(www-data) groups=82(www-data)
smueller18 commented 5 years ago

Here is a command to reproduce this behavior:

» docker run --rm -it --user www-data nextcloud:fpm-alpine sh
/var/www/html $ NEXTCLOUD_ADMIN_USER=admin NEXTCLOUD_ADMIN_PASSWORD=admin NEXTCLOUD_UPDATE=1 REDIS_HOST=localhost /entrypoint.sh sh -c 'echo $?'
Configuring Redis as session handler
/entrypoint.sh: line 26: can't create /usr/local/etc/php/conf.d/redis-session.ini: Permission denied
Initializing nextcloud 16.0.1.1 ...
Initializing finished
New nextcloud instance
running web-based installer on first connect!
Unb0rn commented 5 years ago

Any workarounds? I have this problem too

J0WI commented 5 years ago

Thanks for reporting this. I can reproduce the issue, but I'm not very happy with the proposed permission change in #764. However, this should not break your NextCloud, you just have no Redis session handler. As a workaround, you can mount a redis-session.ini manually.

smueller18 commented 5 years ago

I know changing the permissions is kind of a security issue. But running the image as root is even more.

What definitely does not work:

  1. Writing configuration in .user.ini file. According to https://github.com/nextcloud/docker/issues/447#issuecomment-420679887, this does not work for CLI mode.

Here are some options that I figured out:

  1. Change ownership of /usr/local/etc/php/conf.d/ to user www-data. The entrypoint creates redis-session.ini. Results in security concerns.
  2. Create placeholder file /usr/local/etc/php/conf.d/redis-session.ini in the Dockerfile with no content. Change ownership of only /usr/local/etc/php/conf.d/redis-session.ini to user www-data. Content can be written in the entrypoint. More secure than 1. but if redis is not used, there is an empty file.
  3. Create file /usr/local/etc/php/conf.d/session.ini in the Dockerfile with defaults from default config /usr/local/etc/php/php.ini-production. Change ownership of only /usr/local/etc/php/conf.d/session.ini to user www-data. Content can be replaced with sed in the entrypoint. Security equal to 2. but duplicate config for session.
  4. Only write /usr/local/etc/php/conf.d/redis-session.ini when running as root, otherwise log a warning that the file must be mounted manually instead of throwing the error like described above. No security concerns.

Currently I think version 3 is the best trade-off. @J0WI what do you think?

J0WI commented 5 years ago

Why would you prefer 3 over 2? 4 would also be an option for me, this is basically what happens now.

@tilosp what is your opinion?

smueller18 commented 5 years ago

@J0WI I prefer 3 over 2 because if redis host is not set, there is a file called redis-session.ini which might irritate users. Best practices of writing Docker images says

If a service can run without privileges, use USER to change to a non-root user. _https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#user_

All images in this project should run as non-root by default. Having this in mind, 4 is not an option. I will open a new PR that sets non-root users as default for all used images and examples. Not choosing 4 is a precondition for that.

tilosp commented 5 years ago

1 and 2 are not really an option because it wouldn't work for users other than root and www-data. Instead we would need to chmod 777 them.

It there any difference between 1, 2 and 3 in terms of security? In each case php can modify it's own config.

All images in this project should run as non-root by default. Having this in mind, 4 is not an option. I will open a new PR that sets non-root users as default for all used images and examples. Not choosing 4 is a precondition for that.

The web server already runs as a non-root user (www-data) by default. And i don't see how adding USER www-data to the Dockerfile will improve the security. If you don't trust the container to run apache as www-data you also can't trust the default user to be www-data.

J0WI commented 5 years ago

It there any difference between 1, 2 and 3 in terms of security?

Not it terms of security, but I'd avoid changing upstream files.

smueller18 commented 5 years ago

The web server already runs as a non-root user (www-data) by default.

I can't verify that, have a look at the commands:

» docker exec -i -t nextcloud ps aux
PID USER TIME COMMAND 1 root 0:00 php-fpm: master process (/usr/local/etc/php-fpm.conf) 35 www-data 0:00 php-fpm: pool www 36 www-data 0:00 php-fpm: pool www 37 root 0:00 ps aux


- Apache

» docker run --rm -it --name nextcloud -d nextcloud 7f64940a1f4db036e821432d4a53ba937a32ae5a25093a94ad09073348f29e16

» docker exec -i -t nextcloud ps aux
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND root 1 3.8 0.4 496432 35640 pts/0 Ss+ 20:07 0:00 apache2 -DFOREG www-data 45 0.0 0.1 496464 9380 pts/0 S+ 20:07 0:00 apache2 -DFOREG www-data 46 0.0 0.1 496464 9380 pts/0 S+ 20:07 0:00 apache2 -DFOREG www-data 47 0.0 0.1 496464 9380 pts/0 S+ 20:07 0:00 apache2 -DFOREG www-data 48 0.0 0.1 496464 9380 pts/0 S+ 20:07 0:00 apache2 -DFOREG www-data 49 0.0 0.1 496464 9380 pts/0 S+ 20:07 0:00 apache2 -DFOREG root 50 22.0 0.0 36632 2804 pts/1 Rs+ 20:07 0:00 ps aux



The PID 1 process always runs as root by using the defaults.

> And i don't see how adding USER www-data to the Dockerfile will improve the security. If you don't trust the container to run apache as www-data you also can't trust the default user to be www-data.

Have a look at the following blog post. There is a good explanation, why running images as root is not a good practice:

> Containers that run as root frequently have far more permissions than their workload requires which, in case of compromise, could help an attacker further their attack.
https://kubernetes.io/blog/2018/07/18/11-ways-not-to-get-hacked/#8-run-containers-as-a-non-root-user.

If the `USER` directive is set to `www-data` the attack vector is minimized when running with defaults. And if anyone wants to run the image as root this is still possible by manually changing the user.

> 1 and 2 are not really an option because it wouldn't work for users other than root and www-data. Instead we would need to chmod 777 them.

It makes no sense to use another user than `root` or `www-data` to run fpm or apache, other users do not have sufficient permissions.

How should we go an with this issue?
tilosp commented 5 years ago

I can't verify that, have a look at the commands:

Yes the entrypoint scipt and apache/fpm master process run as root by default. I meant that the processes that handle the requests and run the php code are run by a different user.

If the USER directive is set to www-data the attack vector is minimized when running with defaults. And if anyone wants to run the image as root this is still possible by manually changing the user.

Yes it will reduce the attack surface in case there is a bug in apache/fpm that allows code execution in the master process. But it won't help in the case of a malicious docker image. To protect against a malicious docker image you would need to override to default user for example in the docker-compose file.

Something like this should be done upstream in the php base image, then all php based images get the benefit.

It makes no sense to use another user than root or www-data to run fpm or apache, other users do not have sufficient permissions.

This is valid use case and it is supported by the upstream image. It should work, have you tried it? If there is a permission problem it is a bug.

smueller18 commented 5 years ago

I was not aware of that in https://github.com/docker-library/php/pull/787 the permissions of folder /var/www/html were set to 777. So yes, every user can be used to run the php-fpm image.

I guess the upstream images will not set the user directive in the mid-term because that will break too many existing child images. So in my opinion, this should be treated proactive from the application side.

After the discussions above, how should we follow? In the meantime, I am just happy if the entrypoint does not throw an exception anymore. And all following changes of this project should work user independent so that updating the image is always possible without manual testing in a sandbox environment.

sethforprivacy commented 2 years ago

For anyone coming to this today -- you can easily work around this by creating a local redis-session.ini, chown it to the proper permissions (i.e. sudo chown 100:100 redis-session.ini), and then mount it as a bind mount into the Docker container, i.e.:

Docker CLI:

-v ./redis-session.ini:/usr/local/etc/php/conf.d/redis-session.ini

Or in docker-compose:

    volumes:
      - "./redis-session.ini:/usr/local/etc/php/conf.d/redis-session.ini"

Nextcloud can then properly write to the file and Redis caching works.

eloo commented 1 year ago

will this ever be fixed? or are we doomed to run nextcloud as root in kubernetes for ever?

stavros-k commented 1 year ago

will this ever be fixed?

or are we doomed to run nextcloud as root in kubernetes for ever?

Mounting a configmap works just fine. I run NC rootless

eloo commented 1 year ago

@stavros-k would you mind to share your config with us?

at least my config seems not to work

  # Extra mounts for the pods. Example shown is for connecting a legacy NFS volume
  # to NextCloud pods in Kubernetes. This can then be configured in External Storage
  extraVolumes:
    - name: redis
      configMap:
        name: nextcloud-redis
  extraVolumeMounts:
    - name: redis
      mountPath: /usr/local/etc/php/conf.d/redis-session.ini
      subPath: "redis-session.ini"

  # Set securityContext parameters for the nextcloud CONTAINER only (will not affect nginx container).
  # For example, you may need to define runAsNonRoot directive
  securityContext: 
    runAsNonRoot: true
    # nobody
    runAsUser: 65534
    runAsGroup: 65534
  #   readOnlyRootFilesystem: false

  # Set securityContext parameters for the entire pod. For example, you may need to define runAsNonRoot directive
  podSecurityContext: 
    fsGroup: 65534
  #   runAsGroup: 33
  #   runAsNonRoot: true
  #   readOnlyRootFilesystem: false

i guess an error like this

... read-only file system: unknown
stavros-k commented 1 year ago

@stavros-k would you mind to share your config with us?

at least my config seems not to work


  # Extra mounts for the pods. Example shown is for connecting a legacy NFS volume

  # to NextCloud pods in Kubernetes. This can then be configured in External Storage

  extraVolumes:

    - name: redis

      configMap:

        name: nextcloud-redis

  extraVolumeMounts:

    - name: redis

      mountPath: /usr/local/etc/php/conf.d/redis-session.ini

      subPath: "redis-session.ini"

  # Set securityContext parameters for the nextcloud CONTAINER only (will not affect nginx container).

  # For example, you may need to define runAsNonRoot directive

  securityContext: 

    runAsNonRoot: true

    # nobody

    runAsUser: 65534

    runAsGroup: 65534

  #   readOnlyRootFilesystem: false

  # Set securityContext parameters for the entire pod. For example, you may need to define runAsNonRoot directive

  podSecurityContext: 

    fsGroup: 65534

  #   runAsGroup: 33

  #   runAsNonRoot: true

  #   readOnlyRootFilesystem: false

i guess an error like this


... read-only file system: unknown

https://github.com/truecharts/charts/tree/master/charts/stable/nextcloud Here is the whole chart

eloo commented 1 year ago

thats the truenas chart.. this is using a different docker container i guess this container is not using the same entrypoint as the one in this repository..

the redis-session is mounted directly as RO file with hardcoded values.. this is total different to the approach used in the official docker images here

so it looks like the official image are just not usable in a secure way

stavros-k commented 1 year ago

thats the truenas chart..

this is using a different docker container

i guess this container is not using the same entrypoint as the one in this repository..

the redis-session is mounted directly as RO file with hardcoded values..

this is total different to the approach used in the official docker images here

so it looks like the official image are just not usable in a secure way

TrueCharts*, but no, while the container is custom (also available to see the changed code), nothing is changed in terms of user or permissions, at least not that I can remember.

Just some extra utilities scripts are added and few packages. Some things are injected to the entry point right before the start of the app, but the original entry point runs as is.

eloo commented 1 year ago

this line would fail because the mounted CM is read-only (which is the default in k8s) https://github.com/nextcloud/docker/blob/master/27/apache/entrypoint.sh#L132C13-L132C13

maybe the config in truenas is different.. but for kubernetes mounted CMs are read-only https://github.com/kubernetes/kubernetes/issues/62099

stavros-k commented 1 year ago

Truecharts*

And no, it's kubernetes, still CM is readonly

https://github.com/nextcloud/docker/blob/2d39d9d190db9994187742b0577ea0a950b40be4/27/apache/entrypoint.sh#L108C10-L108C10

Don't set redis host. Configure it with occ

eloo commented 1 year ago

but this is automatically set by the official helm chart.

and also using occ in a cloud environment seems not be a good practice..

having a workaround for workaround to work does really not sound like a stable and futureproof solution.

i'm curious why this will not be fixed by the nextcloud image itself.

@smueller18 has provided some good examples how to solve this 4 years ago.

at least for me i have fixed this by fixing the official docker image with a single line

FROM nextcloud:27.1.1-apache

RUN touch /usr/local/etc/php/conf.d/redis-session.ini && chown 0:65534 /usr/local/etc/php/conf.d/redis-session.ini && chmod 770 /usr/local/etc/php/conf.d/redis-session.ini

this can easily be added to the official images without any "security concerns" because it is still more secure than running the whole container as root

J0WI commented 1 year ago
chmod 770 /usr/local/etc/php/conf.d/redis-session.ini

This makes the PHP config world writeable, including for the webserver itself. Apache and PHP-FPM both drop their privileges if the container is started as root.

jessebot commented 11 months ago

but this is automatically set by the official helm chart.

just a quick reminder that the helm chart is also community maintained, as is this docker image.

Don't set redis host. Configure it with occ

@stavros-k how do you set it with occ? I'm a collaborator over at the helm chart, and if there's a way to set this occ, I can work with the community and other maintainers there to suggest a solution for perhaps a post-install helm hook to run this occ command. :pray:

stavros-k commented 11 months ago

but this is automatically set by the official helm chart.

just a quite reminder that the helm chart is also community maintained, as is this docker image.

Don't set redis host. Configure it with occ

@stavros-k how do you set it with occ? I'm a collaborator over at the helm chart, and if there's a way to set this occ, I can work with the community and other maintainers there to suggest a solution for perhaps a post-install helm hook to run this occ command. 🙏

here is the redis script, you will also find other utils in this dir https://github.com/stavros-k/containers/blob/master/apps/nextcloud-fpm/configure-scripts/occ-redis.sh

jessebot commented 11 months ago

@stavros-k awesome! Thanks!

For anyone in this repo trying to figure out the k8s side of things, as I've mentioned in https://github.com/nextcloud/helm/issues/187#issuecomment-1855491845 if you'd like, please submit a PR to the helm chart repo to remove the redis configMap and replace it with a job using a post-install hook annotation that runs the scripts that stavros-k has provided. As long as you test your code, we're happy to review it :blue_heart:

stavros-k commented 11 months ago

@stavros-k awesome! Thanks!

For anyone in this repo trying to figure out the k8s side of things, as I've mentioned in nextcloud/helm#187 (comment) if you'd like, please submit a PR to the helm chart repo to remove the redis configMap and replace it with a job using a post-install hook annotation that runs the scripts that stavros-k has provided. As long as you test your code, we're happy to review it 💙

Please also note this script which is required https://github.com/stavros-k/containers/blob/master/apps/nextcloud-fpm/scripts/occ https://github.com/stavros-k/containers/blob/50d9b1700ebdf98206b85ecb905a03e818492d09/apps/nextcloud-fpm/Dockerfile#L84


You should also not even need a helm hook. You can use those hooks mentioned here https://github.com/nextcloud/docker/blob/master/README.md#auto-configuration-via-hook-folders

As it will run before NC starts and run in the same container. (Not sure how wouold you run occ from another container, probably by sharing the config.php?)

jessebot commented 11 months ago

Thanks so much for all the info! :)

(Not sure how wouold you run occ from another container, probably by sharing the config.php?)

Yep, we mount the config directory as a persistence volume if you have persistence turned on in your values.yaml for the nextcloud/helm chart, so the job just needs to mount the same volume. That's how I do it in my personal post install apps job here.

That was before I knew about this handy post install hook folder though!

Thinking out loud... So now I'm thinking in nextcloud/helm we could just mount a ConfigMap data key like this to use if you have podSecurityContext set to anything other than 0:

data:
  configure-redis.sh: |
    echo '## Configuring Redis...'
    occ config:system:set redis host --value="${NC_REDIS_HOST:?"NC_REDIS_HOST is unset"}"
    occ config:system:set redis password --value="${NC_REDIS_PASS:?"NC_REDIS_PASS is unset"}"
    occ config:system:set redis port --value="${NC_REDIS_PORT:-6379}"
    occ config:system:set memcache.local --value="\\OC\\Memcache\\APCu"
    occ config:system:set memcache.distributed --value="\\OC\\Memcache\\Redis"
    occ config:system:set memcache.locking --value="\\OC\\Memcache\\Redis"

Then we just mount that configMap into the /docker-entrypoint-hooks.d/post-installation directory only if the security context runAsUser is not root. Otherwise, our current redis config system works fine.

jessebot commented 11 months ago

so the hooks don't seem to work if we use a configMap even though the file is there and executable. I mounted this configMap as a test:

---
apiVersion: v1
kind: ConfigMap
metadata:
  name: before-starting-scripts
data:
  install-apps.sh: |
    #!/bin/sh
    echo "Installing Nextcloud apps..."
    php /var/www/html/occ app:install oidc_login
    php /var/www/html/occ app:install notes
    php /var/www/html/occ app:install bookmarks
    php /var/www/html/occ app:install deck
    php /var/www/html/occ app:install side_menu
    echo "Nextcloud apps installation complete."

I'm using the helm chart, so I mounted the configmap via the values.yaml like this:

nextcloud:
  extraVolumes:
    - name: before-starting-scripts
      configMap:
        name: before-starting-scripts
        defaultMode: 0550

  extraVolumeMounts:
    - name: before-starting-scripts
      mountPath: /docker-entrypoint-hooks.d/before-starting

But the entrypoint.sh doesn't find the file. Here's the container logs:

=> Searching for scripts (*.sh) to run, located in the folder: /docker-entrypoint-hooks.d/before-starting

[14-Dec-2023 18:39:42] NOTICE: fpm is running, pid 1

[14-Dec-2023 18:39:42] NOTICE: ready to handle connections

127.0.0.1 -  14/Dec/2023:18:42:29 +0000 "GET /index.php" 200

127.0.0.1 -  14/Dec/2023:18:43:07 +0000 "GET /index.php" 200

here's some debug info from logging into the container:

$ /docker-entrypoint-hooks.d/before-starting/install-apps.sh
Installing Nextcloud apps...
oidc_login already installed
notes already installed
bookmarks already installed
deck already installed
side_menu already installed
Nextcloud apps installation complete.

$ whoami
www-data

$ export hook_folder_path="/docker-entrypoint-hooks.d/before-starting"
$ find "${hook_folder_path}" -type f -maxdepth 1 -iname '*.sh' -print | sort
$ ls $hook_folder_path
install-apps.sh

$ ls -hal /docker-entrypoint-hooks.d/before-starting
total 12K
drwxrwsrwx    3 root     www-data    4.0K Dec 14 18:39 .
drwxr-xr-x    7 root     root        4.0K Dec  1 08:30 ..
drwxr-sr-x    2 root     www-data    4.0K Dec 14 18:39 ..2023_12_14_18_39_40.2864972143
lrwxrwxrwx    1 root     www-data      32 Dec 14 18:39 ..data -> ..2023_12_14_18_39_40.2864972143
lrwxrwxrwx    1 root     www-data      22 Dec 14 18:39 install-apps.sh -> ..data/install-apps.sh

I guess the find can't follow links? I don't know how to fix that find command, but it renders the docker hooks unusable via the configmaps via k8s right now :( We'll have to use helm hooks with jobs till this is resolved. Did some more reading, but since the configMap file is mounted as a symlink, the find command needs to have a max-depth of 2 to fix this.

Example in the container to get this working:

$ find "${hook_folder_path}" -type f -maxdepth 1 -iname '*.sh' -print | sort
$ find "${hook_folder_path}" -type f -maxdepth 2 -iname '*.sh' -print | sort
/docker-entrypoint-hooks.d/before-starting/..2023_12_14_18_39_40.2864972143/install-apps.sh
stavros-k commented 11 months ago

Probably this:

https://github.com/nextcloud/docker/pull/2115/files

Lithimlin commented 10 months ago

Just wanted to update my Nextcloud docker setup (with redis and mariaDB) and can't start it anymore now due to this issue. The redis container seems to run fine, but Nextcloud doesn't start due to the permission error. I've tried the latest image as well as 26.0, no luck with either. I am using the user variable in my ansbile setup. Honestly, reading through all this just leaves me more confused since the permissions should be correct, but because the container does not start, I cannot check the file permissions inside it.

Since my last update before that was 25.0, I also cannot go back to the old version. Does the manual mount workaround (still) work if I am using redis in a docker container as well?

LorbusChris commented 10 months ago

I work around this by mounting redis-session.ini manually as a volume, i.e. touch ./nextcloud/redis/redis-session.ini and then add this flag to podman/docker on the nextcloud container:

--volume ./nextcloud/redis/redis-session.ini:/usr/local/etc/php/conf.d/redis-session.ini:Z
montana123 commented 9 months ago

I work around this by mounting redis-session.ini manually as a volume, i.e. touch ./nextcloud/redis/redis-session.ini and then add this flag to podman/docker on the nextcloud container:

--volume ./nextcloud/redis/redis-session.ini:/usr/local/etc/php/conf.d/redis-session.ini:z

Having the exact same problem. Thank you for the fix. Nextcloud should up their game on how to correctly use docker and providing images. The whole upgrade process with nextcloud is also horrendous. If there would be a viable alternative I would gladly take it.

hypery2k commented 7 months ago

any update on that issue?

LorbusChris commented 6 months ago

OT: You may want to check out https://github.com/LorbusChris/nextcloud-quadlet for an alternative way of running a rootless setup. The stack is a bit different, using Valkey instead of Redis, and Envoy as proxy. I've been running this setup for ~4 months with automatic upgrades and without any issues.

J0WI commented 6 months ago

will this ever be fixed?

Unlikely, due the reason mentioned in https://github.com/nextcloud/docker/issues/763#issuecomment-501279140 Making the server config world writeable would make the image less secure for everyone. Mounting a custom config is the way to go.