Update Dockerfile and docker compose to follow OWASP best practices for Docker where possible

[zaniyah]

If you are self-hosting, you have to make substantial changes to get the Docker container to run correctly if you do not want it to run insecurely.

OWASP Docker cheatsheet

There are a number of changes that could be made relatively easily to the current Dockerfile and docker compose file, to ensure that the app does not run as root in the Docker container, and things like that. This would help to make OpenCarwings less vulnerable and would be greatly appreciated by those who decide to self-host.

[zaniyah]

First and easiest is probably to get it running the Docker containers as non-root users, and adjusting the capabilities they have.

I would make updates to the wiki but I couldn't find a way to contribute without having write access to the repository.

[bartowl]

I would also point out, that the image should contain all of the application code that is subjected to change with next releases. Only things to be mounted from outside should be things like config or database, so things that might change or need to be overriden.

Currently, the docker-compose file still recommends mounting staticfiles which requires to keep local directory tree in sync with image version being deployed. I have checked and the image seems to have its staticfiles built-in so this requirements seems obsolete.
On the other hand, requiring to mount entire carwings folder instead just settings.py is the same problem - when urls.py changes, it not only requires image update but also manual git pull on the mounted volumes otherwise strange and unexpected things will happen...

The best recommendation is to have a minimal working setup inside the image and an option to mount specific parts from outside to provide persistency for database of configs... all the rest should be built-in and replacable by simple image pull.

[developerfromjokela]

I would also point out, that the image should contain all of the application code that is subjected to change with next releases. Only things to be mounted from outside should be things like config or database, so things that might change or need to be overriden.

Currently, the docker-compose file still recommends mounting staticfiles which requires to keep local directory tree in sync with image version being deployed. I have checked and the image seems to have its staticfiles built-in so this requirements seems obsolete. On the other hand, requiring to mount entire carwings folder instead just settings.py is the same problem - when urls.py changes, it not only requires image update but also manual git pull on the mounted volumes otherwise strange and unexpected things will happen...

The best recommendation is to have a minimal working setup inside the image and an option to mount specific parts from outside to provide persistency for database of configs... all the rest should be built-in and replacable by simple image pull.

I fully agree with everything, but staticfiles and mediafiles folder is the directory meant for serving static files (the directory structure), not to be mixed with any other static directory inside the code. staticfiles folder has also static resources for django admin and other UI elements from different modules. These folders are auto-generated on every startup, so this is taken care of by django.

settings.py would need a refactoring, to take in configuration only from .env file. this way we can keep these files in docker image. It's become a bad habit for me, passing down the project folder as volume :(

[zaniyah]

I think the first priority is to simply have the containers execute with the lowest level of privilege that they need. I have forked the repository so that I can look at this but unfortunately I don't have a lot of free time at the moment so I don't know if I will manage it. I'd be very happy if someone else got there first.

[developerfromjokela]

I think the first priority is to simply have the containers execute with the lowest level of privilege that they need. I have forked the repository so that I can look at this but unfortunately I don't have a lot of free time at the moment so I don't know if I will manage it. I'd be very happy if someone else got there first.

I will try in a moment 👍

[developerfromjokela]

Got it! Had to replace cron daemon with supercronic, otherwise everything works out-of-the-box

[zaniyah]

I can see the changes to the Dockerfile, adding a user & group, and the other things to make it work. This is great, thanks.

I think the next thing is to be able to run docker exec without being root, and to drop privileges & capabilities from the container. This will mean a bit more work though, as I expect some things will break, and it needs some minor changes to the docker-compose.yml.

[developerfromjokela]

I can see the changes to the Dockerfile, adding a user & group, and the other things to make it work. This is great, thanks.

I think the next thing is to be able to run docker exec without being root, and to drop privileges & capabilities from the container. This will mean a bit more work though, as I expect some things will break, and it needs some minor changes to the docker-compose.yml.

If there's some modification to docker compose that would keep compatibility with normal docker, happy to help. My guess is it should go smoothly, especially if db and redis container works already without root.

[zaniyah]

I think it needs to look something like this, but restarting after messing around I have different problems (unrelated to these changes) with django not finding installed python modules, and cannot properly test it yet. Probably some stale cache somewhere but django is not something I've ever used before.

https://github.com/zaniyah/opencarwings/blob/main/docker-compose.yml

[zaniyah]

It is missing a line for the server section, where you specify what user to run as. Probably user: "5000:5000"

[developerfromjokela]

If you have some time, please do open a PR, once something is working. Draft PR would also be great.

I will get back to this once I have a bit of time.

[zaniyah]

If you have some time, please do open a PR, once something is working. Draft PR would also be great.

I will get back to this once I have a bit of time.

I've merged the changes in my fork, but have not had time yet to test it. I think it should work with minimal tweaking, but as I haven't tested it I haven't wanted to unleash it on others. If I get a chance to test it I will of course get on to opening a PR.

[bartowl]

current fix for docker-compose.yml is not sufficient. I tried mounting just settings.py but it shows up that /app/carwings is an empty directory inside the image and it produces such errors when running without mounting the entire directory contents from a cloned git repo:

ModuleNotFoundError: No module named 'carwings.urls'

also inside the container:

528356f0212c:/app$ ls -la carwings
total 16
drwxr-xr-x    2 root     root          4096 Sep  1 10:53 .
drwxr-xr-x    1 ocw      ocw           4096 Sep  1 10:53 ..
-rw-r--r--    1 ocw      ocw           7089 Aug 11 09:55 settings.py
528356f0212c:/app$ git status
Refresh index: 100% (434/434), done.
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add/rm <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
        deleted:    carwings/__init__.py
        deleted:    carwings/asgi.py
        deleted:    carwings/settings.docker.py
        deleted:    carwings/settings.example.py
        deleted:    carwings/urls.py
        deleted:    carwings/wsgi.py
        deleted:    docs/ACP_245_V_1_2_2.pdf