-
Notifications
You must be signed in to change notification settings - Fork 24
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
Conversation
@EricSoroos - your expertise would be very valuable here if you get a chance to look at this - thanks! |
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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
@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 ( |
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. |
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:
(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. |
…figuration files - Until ckan#172 and ckan/ckan-docker-base#80 to ckan-docker
@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:
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 |
There was a problem hiding this 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)
There was a problem hiding this 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'
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] |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed this in #85 (612d0b916ec917c05bb31a67b5d5dff5d5f2e3fc)
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`
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
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