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

Minimise all root-owned files/directories in the running CKAN container #80

Merged
merged 14 commits into from
Nov 13, 2024

Conversation

kowh-ai
Copy link
Contributor

@kowh-ai kowh-ai commented Sep 9, 2024

Fixes: #79

To enhance the security of CKAN images and containers, it is recommended that all files and directories associated with the application be owned by a dedicated non-root user. Additionally, running CKAN processes under a separate non-root user account further reinforces system security by limiting the privileges available to any running service.

For CKAN 2.10, 2.11 and master images (base and dev)

User: ckan-sys (id=502)- owns the files/directories that are part of the application and supporting libraries

User: ckan (id=503) - runs the application processes, owns files and directories it needs write access to

There is also a ckan-docker update to be merged alongside with this change

@kowh-ai
Copy link
Contributor Author

kowh-ai commented Sep 10, 2024

@EricSoroos - your expertise would be very valuable here if you get a chance to look at this - thanks!

ckan-2.10/base/Dockerfile Outdated Show resolved Hide resolved
Comment on lines +74 to +75
useradd -rm -d /srv/app -s /bin/bash -g ckan-sys -u 502 ckan-sys && \
useradd -rm -d /srv/app -s /bin/bash -g ckan-sys -u 503 ckan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having home directories owned by the user is nice for keeping track of things like .bash_history when logging in to the container

Copy link
Contributor

@wardi wardi Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the ckan user has the same primary group as ckan-sys we'll need to make sure the group permissions on all the ckan source files prevent writing. I'm not used to setting things up this way but there may be other issues as well.

Copy link
Contributor Author

@kowh-ai kowh-ai Sep 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is set up this way by default. The ckan user has write permissions only on the files/directories it needs to write into. The ckan-sys user does not have write access. However I have made the ckan-sys group the primary group for both ckan and ckan-sys users in case further down the track we need group write access for more granularity

# Create ckan and ckan-sys users and the ckan-sys group plus set up the storage path
RUN groupadd -g 502 ckan-sys && \
useradd -rm -d /srv/app -s /bin/bash -g ckan-sys -u 502 ckan-sys && \
useradd -rm -d /srv/app -s /bin/bash -g ckan-sys -u 503 ckan
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment about home directories above

@kowh-ai
Copy link
Contributor Author

kowh-ai commented Sep 16, 2024

@wardi - I have been experimenting with bind mounts in the ckan dev images/containers and I have struggled with getting the ownership of files in the directory (/srv/app/src_extensions) in the ckan container to take the ownership of the files located in the source directory on the host machine. File permissions are fine and they are reflected through to the files in the container. I'm suspecting this is because I'm running Mac OS on the host computer.

@wardi
Copy link
Contributor

wardi commented Sep 25, 2024

docker bind mounts on Mac OS are certainly different than in Linux. IIRC there's a Linux VM in between that's used to run all the containers and I have no idea how permissions are mapped between the host directories and the VM. I do remember that Mac OS bind mounted directories were many many times slower to access than normal linux filesystems so there's probably some network filesystem and translation being used in between.

@EricSoroos
Copy link

EricSoroos commented Sep 26, 2024

The mac bind mounts for docker have changed dramatically over the last ~5 years or so -- originally they where super slow, at least for individual ops. (actual transfer wasn't that bad, but latency was roughly like spinning rust). They've gone through an intermediate, unstable alternative, and now they're using the built in virtfs/virtualization framework, which I haven't really noticed as being bad.

What I typically do if I need to shell in and match ids is:

docker compose exec -u `id -u`:`id -g` container  /bin/bash

(though that's a canned script in a makefile, so I don't have to remember how to type all that)

There's really no way to do this for the volume mounts themselves.

mjanez added a commit to mjanez/ckan-docker that referenced this pull request Oct 17, 2024
@kowh-ai
Copy link
Contributor Author

kowh-ai commented Nov 1, 2024

@wardi - this is good to go (again). After consultation with @amercader the "files/directories owned by a non-root user" changes will apply to the following images only:

  • CKAN 2.9 py3.9
  • CKAN 2.10 py3.10
  • CKAN 2.11
  • CKAN master

Any users still on the alpine images will be encouraged to use/migrate to the python slim-bookworm images instead

Iv'e tested all base/dev images/containers and all work file

Copy link
Contributor

@wardi wardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just need some readme updates on ckan-docker, see ckan/ckan-docker#172 (comment)

Copy link
Contributor

@wardi wardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some other issues with development mode.
There are errors when installing extensions or ckan from the src directory

eg:

ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: '/srv/app/.local'

and

[Errno 13] Permission denied: '/usr/local/lib/python3.10/site-packages/test-easy-install-41.write-test'

@amercader amercader mentioned this pull request Nov 13, 2024
@kowh-ai kowh-ai merged commit 11fd0d5 into main Nov 13, 2024
amercader added a commit that referenced this pull request Nov 15, 2024
amercader added a commit that referenced this pull request Nov 15, 2024
ckan-sys only exists in the Python based images (Dockerfile.py*)
after #80
@@ -200,7 +200,7 @@ def create_sysadmin():
# We're running as root before pivoting to uwsgi and dropping privs
data_dir = "%s/storage" % os.environ['CKAN_STORAGE_PATH']

command = ["chown", "-R", "ckan:ckan", data_dir]
command = ["chown", "-R", "ckan:ckan-sys", data_dir]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 2.10, the prerun.py script will still be used by both the alpine (Dockerfile) and Python (Dockerfile.py3.10) based images but the ckan-sys user has only been added to the python one, so this command will fail in the Alpine images. We need two prerun.py files or some magic here that uses the right user

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amercader added a commit to ckan/ckanext-dcat that referenced this pull request Nov 20, 2024
After changes in the CKAN Docker images (ckan/ckan-docker-base#80) the
images run in the test workflows does not use root by default anymore.
This is not supported in GitHub Actions so we need to pass `--user root`
option to avoid failures. To catch any permissions related issues, we
run the `ckan` and `pytest` commands with `sudo -u ckan`
amercader added a commit that referenced this pull request Nov 21, 2024
Although supervisor has been removed from 2.11 onwards, it is still used
in the 2.10 and 2.9 images. In #80 the supervisord.conf [was modified][1]
to run the process as the newly created `ckan-sys` user, but this same
conf file is used in the alpine image, where this user does not exist.
Conversely, the 2.9 Python image includes the same file as the alpine
one, so it doesn't use the new user.

This PR splits the files in two different versions so they use the
appropiate user.

[1]
https://github.com/ckan/ckan-docker-base/pull/80/files#diff-a9f3dc77dfb98b40d119163f7a8049572695f238c20380a6c9ca8e43c4a9daf3R4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a non-root user for all application files/directories, use another non-root user to run the CKAN processes
4 participants