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

Update Dockerfile to let the app run without root permissions #200

Merged
merged 1 commit into from Mar 1, 2024

Conversation

juliu-s
Copy link

@juliu-s juliu-s commented Feb 28, 2024

Hello,

In this pull request the the Dockerfile is updated mainly to let the app/container run rootless.
This is handy if you want to run the app/container in a environment when running as root is not allowed.
For example a Kubernetes platform with Restricted Pod Security Standards[1] / Security Context[2]

There is also a modification to support arbitrary user ids [3].

Also the /data directory is created during the build with the correct permissions. This makes the -v planning-poker-data:/data option optional.

Finally the full registry path is used at the FROM instructions to clarify where the image comes from.

[1] Pod security standards
[2] Security Context
[3] Support arbitrary user ids

@axeleroy
Copy link
Owner

Hello,

Thanks for your contribution! It looks just right, I just have a little reservation about the change: does it require users to update the database's rights before upgrading?

Cheers,
Axel

@juliu-s
Copy link
Author

juliu-s commented Feb 28, 2024

Hi,

That is a good catch! Yes, this should considered as a breaking change. Since the current database file is owned by root:root. The (possible) new version does not have write permissions to the database file, you should:

  1. Stop the container
  2. Update the permissions of the database file in the mounted volume:chown 1001:0 <database file>
    a. If you are using Podman instead of Docker use: podman unshare chown 1001:0 <database file>
  3. Start the new container

Rollback should not be an issue since the app is running as root who can write to files with a different owner.

@axeleroy
Copy link
Owner

Okay, I'll have to find a way to inform users (which may not even know that this repository exists) of this breaking change 🤔

@juliu-s
Copy link
Author

juliu-s commented Feb 28, 2024

I did the following test:

  1. Run the current latest version (with a volume)
    $ podman run -d --name poker-root -v /Podmandata/poker:/data -p 8000:8000 docker.io/axeleroy/self-host-planning-poker:latest
  2. Stopped and removed the container
  3. Run the local build rootless version (using the same volume)
    $ podman run -d --name poker-rootless -v /Podmandata/poker:/data -p 8000:8000 localhost/poker:alpine-rootless

The container/app starts, but when clicking on Create new game the following error occurs:

$ podman logs poker-rootless
[2024-02-28 11:14:33 +0000] [1] [INFO] Starting gunicorn 21.2.0
[2024-02-28 11:14:33 +0000] [1] [INFO] Listening at: http://0.0.0.0:8000 (1)
[2024-02-28 11:14:33 +0000] [1] [INFO] Using worker: eventlet
[2024-02-28 11:14:33 +0000] [2] [INFO] Booting worker with pid: 2
[2024-02-28 11:15:00,422] ERROR in app: Exception on /create [POST]
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3251, in execute_sql
    cursor.execute(sql, params or ())
sqlite3.OperationalError: attempt to write a readonly database

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 2190, in wsgi_app
    response = self.full_dispatch_request()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 1486, in full_dispatch_request
    rv = self.handle_user_exception(e)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 1484, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 1469, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/app.py", line 43, in create
    return gm.create(game_name, game_deck)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/app/gamestate/game_manager.py", line 21, in create
    StoredGame.create(uuid=game_uuid, name=name, deck=deck_name)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 6596, in create
    inst.save(force_insert=True)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 6806, in save
    pk = self.insert(**field_dict).execute()
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 1971, in inner
    return method(self, database, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2042, in execute
    return self._execute(database)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2847, in _execute
    return super(Insert, self)._execute(database)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 2560, in _execute
    cursor = database.execute(self)
             ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3259, in execute
    return self.execute_sql(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3249, in execute_sql
    with __exception_wrapper__:
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3019, in __exit__
    reraise(new_type, new_type(exc_value, *exc_args), traceback)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 192, in reraise
    raise value.with_traceback(tb)
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3251, in execute_sql
    cursor.execute(sql, params or ())
peewee.OperationalError: attempt to write a readonly database

A few ideas:

  1. Add basic logging displaying general info (or / and a link to the changelog / a Wiki entry in this repo) before starting the app.
  2. Catch the above error and display instructions or a link to the changelog / a Wiki entry in this repo.

@axeleroy
Copy link
Owner

I think I'll check permissions on the database at startup, print a message explaining the issue with remediation steps and then exit.

I will then look into merging this.

@axeleroy axeleroy merged commit d370092 into axeleroy:main Mar 1, 2024
4 checks passed
@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

Hello,

I have been trying to test the lack of permissions after upgrading on my laptop with Docker, both in normal and rootless mode but I wasn't able to reproduce the issue after building the latest commit locally, is it an issue only on Podman?

@juliu-s juliu-s deleted the rootless branch March 1, 2024 07:19
@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

Hi Axel,

Hmm, that's interesting. Did you use your local built version? I don't see a new tag on DockerHub since the merge.

I approached the problem from Linux permissions: First the database file was owned by root (0) since the app/container was running under root. After the update it was running under user default (1001). Since the file was created to only be writable by the user. That should be enough to get a write permission error.

Or maby Docker does some checks and applies some ownership magic before starting the container?

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

I built it locally, yes (I disabled the CI on the main branch since I wasn't able to stop it from pushing on PR runs).

Okay, so I was testing on another machine and I realized that the ORM wasn't trying to write to the database until I create or update a game... I'll have to devise another way to detect if the database is writable.

@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

I created an script which can be enhanced / simplified:

#!/usr/bin/env python3

import os
import sys

# Get current process user and group
current_user_id = os.getuid()
current_group_id = os.getgid()

print(f"Current UID: {current_user_id}")
print(f"Current GID: {current_group_id}")

# Define file path
file_path = '~/Podmandata/poker/database.db'

# Expand home directory in the file path using os.path.expanduser()
expanded_file_path = os.path.expanduser(file_path)

# Get file permissions using os.stat()
file_stats = os.stat(expanded_file_path)

owner = file_stats.st_uid
group = file_stats.st_gid

print(f"File owner: {owner}")
print(f"File group: {group}")

try:
    f = open(expanded_file_path, "a")
    print("The file is writeable for the current user")
except PermissionError:
    print("The file is not writeable!")
    sys.exit(1)

Testing:

❯ ls -l ~/Podmandata/poker/database.db
-rw-r--r--  1 julius  staff  12288 Feb 28 12:12 /Users/julius/Podmandata/poker/database.db
❯ python3 test.py
Current UID: 501
Current GID: 20
File owner: 501
File group: 20
The file is writeable for the current user

❯ sudo chown root /Users/julius/Podmandata/poker/database.db
❯ ls -l ~/Podmandata/poker/database.db
-rw-r--r--  1 root  staff  12288 Feb 28 12:12 /Users/julius/Podmandata/poker/database.db
❯ python3 test.py
Current UID: 501
Current GID: 20
File owner: 0
File group: 20
The file is not writeable!
❯ echo $?
1

❯ sudo chown julius /Users/julius/Podmandata/poker/database.db
❯ chmod 444 /Users/julius/Podmandata/poker/database.db
❯ ls -l ~/Podmandata/poker/database.db
-r--r--r--  1 julius  staff  12288 Feb 28 12:12 /Users/julius/Podmandata/poker/database.db
❯ python3 test.py
Current UID: 501
Current GID: 20
File owner: 501
File group: 20
The file is not writeable!
❯ echo $?
1

❯ chmod 644 /Users/julius/Podmandata/poker/database.db
❯ ls -l ~/Podmandata/poker/database.db
-rw-r--r--  1 julius  staff  12288 Feb 28 12:12 /Users/julius/Podmandata/poker/database.db
❯ python3 test.py
Current UID: 501
Current GID: 20
File owner: 501
File group: 20
The file is writeable for the current user

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

Thanks for your code, I figured what I got wrong: I was expecting an IOError instead of PermissionError and had the wrong mode set.

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

Well, I may have spoken too soon: my code now detects when root owns the database, but event after changing ownership the app errors out when writing to the database 😕

root@axel-XPS-13-9370:/var/lib/docker/volumes/shpp-data/_data# ls -l
total 12
-rw-r--r-- 1 root root 12288 mars   1 10:57 database.db
root@axel-XPS-13-9370:/var/lib/docker/volumes/shpp-data/_data# chown 1001:0 database.db 
root@axel-XPS-13-9370:/var/lib/docker/volumes/shpp-data/_data# ls -l
total 12
-rw-r--r-- 1 1001 root 12288 mars   1 10:57 database.db
[2024-03-01 15:24:54,266] ERROR in app: Exception on /create [POST]
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/peewee.py", line 3251, in execute_sql
    cursor.execute(sql, params or ())
sqlite3.OperationalError: attempt to write a readonly database

@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

And what is the output when you listing the /data directory inside the container?

❯ podman exec -it poker sh -c 'ls -n /data'
total 12
-rw-r--r--    1 1001     0            12288 Mar  1 15:39 database.db

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

$ docker exec fb8ac4c6fe8e sh -c 'ls -n /data'
total 12
-rw-r--r--    1 1001     0            12288 Mar  1 09:57 database.db

I don't get it...

@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

And the output of:

$ docker exec fb8ac4c6fe8e id

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

$ docker exec fb8ac4c6fe8e id
uid=1001(default) gid=0(root) groups=0(root)

@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

And the permissions of the /data directory?

$ docker exec fb8ac4c6fe8e sh -c "ls -ln /"

@axeleroy
Copy link
Owner

axeleroy commented Mar 1, 2024

The issue comes from the folder's permissions... Extremely weird behavior from Python. I'll update the check and instructions accordingly.

@juliu-s
Copy link
Author

juliu-s commented Mar 1, 2024

I also seem to have an issue with Podman with the new version. It is all good when not mounting a folder. But when I do the /data inside the container changes to root:root with 750 permissions. I have to take a look...

@axeleroy
Copy link
Owner

axeleroy commented Mar 4, 2024

Hum, I haven't checked with folder mounts either. Let me know what you find on your side.

@juliu-s
Copy link
Author

juliu-s commented Mar 4, 2024

So, I came to the conclusion that supporting all platforms is not easy ;).

For Podman I found that you can use the :U volume mount option. From the podman-run man page at the Chowning Volume Mounts section at --volume [1]:

The :U suffix tells Podman to use the correct host UID and GID based on the UID and GID within the container, to change recursively the owner and group of the source volume.

$ podman run --rm -it --name poker-rootless -v ~/Podmandata/poker:/data:U localhost/self-host-planning-poker:rootless sh
/app $ ls -l /
total 64
drwxrwxr-x    1 default  root          4096 Mar  4 08:03 app
drwxr-xr-x    1 root     root          4096 Jan 27 07:22 bin
drwxr-xr-x    2 default  root          4096 Mar  4 11:55 data
..snip..
/app $ touch /data/testfile1
/app $ ls -l /data
total 0
-rw-r--r--    1 default  root             0 Mar  4 11:55 testfile1

Update: The option above works for Podman on Linux for Podman on macOS (via Podman machine) you can use the --userns=keep-id:uid=1001 option:

$ podman run --rm -it --name poker-rootless -v ~/Podmandata/poker:/data --userns=keep-id:uid=1001 localhost/self-host-planning-poker:rootless sh
/app $ ls -l /
total 12
drwxrwxr-x    1 default  root           143 Feb 28 08:05 app
drwxr-xr-x    1 root     root          4096 Jan 27 07:00 bin
drwxr-xr-x    2 default  nobody          64 Mar  5 08:03 data
..snip..
/app $ touch /data/testfile1
/app $ ls -l /data
total 0
-rw-r--r--    1 default  nobody           0 Mar  5 08:10 testfile1

For Docker there is no such option, there is an issue from 2013 to support this[2]. The current "workaround" is to chown the directory on the host before starting the container:

# Inside an Docker in Docker container
/ # id
uid=0(root) gid=0(root)
/ # docker --version
Docker version 25.0.3, build 4debf41
/ # id 1001
id: unknown user 1001
/ # mkdir -p /opt/dockerdata/poker
/ # chown -R 1001:0 /opt/dockerdata/poker
/ # docker run --rm -it -v /opt/dockerdata/poker:/data localhost/self-host-planning-poker:rootless sh
/app $ id
uid=1001(default) gid=0(root) groups=0(root)
/app $ ls -l /
total 64
drwxrwxr-x    1 default  root          4096 Mar  4 08:24 app
drwxr-xr-x    1 root     root          4096 Jan 27 07:22 bin
drwxr-xr-x    2 default  root          4096 Mar  4 11:32 data
..snip..
/app $ touch /data/testfile1
/app $ ls -l /data
total 0
-rw-r--r--    1 default  root             0 Mar  4 11:33 testfile1
/app $ exit
/ # docker run -d --name poker-rootless -v /opt/dockerdata/poker:/data -p 8000:8000 localhost/self-host-planning-poker:rootless 
1043d1e9b2e6ecc638b6a3eff30293d6bde24483341f7a0185ea6dd16a08ce98
/ # docker logs poker-rootless
[2024-03-04 11:44:49 +0000] [1] [INFO] Starting gunicorn 21.2.0
[2024-03-04 11:44:49 +0000] [1] [INFO] Listening at: http://0.0.0.0:8000 (1)
[2024-03-04 11:44:49 +0000] [1] [INFO] Using worker: eventlet
[2024-03-04 11:44:49 +0000] [7] [INFO] Booting worker with pid: 7
/ # ls -l /opt/dockerdata/poker/
total 12
-rw-r--r--    1 1001     root         12288 Mar  4 11:44 database.db
-rw-r--r--    1 1001     root             0 Mar  4 11:33 testfile1
/ # curl -I http://localhost:8000
HTTP/1.1 200 OK
Server: gunicorn
Date: Mon, 04 Mar 2024 11:46:08 GMT
Connection: keep-alive
Content-Type: text/html; charset=utf-8
Content-Length: 1385

I did not test it with rootless Docker since the Docker in Docker rootless container would not start. But in the logs I saw that the same mechanism is used as with Podman (mapping of UID/GID. But since Docker doesn't have the :U option, it could be an issue.

[rootlesskit:parent] error: failed to setup UID/GID map: newuidmap 63 [0 1000 1 1 100000 65536] failed: newuidmap: write to uid_map failed: Operation not permitted

To summarize, before this PR the app/container would run fine with Docker, Podman and K8s where you are allowed to run privileged containers.
If you decide to not revert this PR you should update the README.md with the extra instructions. Or I can do it for you in another PR?
An third option could be to create an second Docker/Containerfile, update the pipeline to push 2 (root and non-root) images to Dockerhub.

[1] https://docs.podman.io/en/latest/markdown/podman-run.1.html#volume-v-source-volume-host-dir-container-dir-options
[2] moby/moby#2259

@juliu-s juliu-s mentioned this pull request Mar 13, 2024
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.

None yet

2 participants