Make the root-downloaded images available to non-root users #705
Conversation
|
Make sure to sign your commits. |
I'm not a huge fan of this, as with the current storage model it means all setuid binaries in those images become accessible too...this is a thing that gets fixed with composefs.
That seems...quite hacky? Also I guess what it's probably going to hit is I think we may be discarding xattrs when we end up filtering/rewriting the tar stream in the ostree-container side. That'd definitely be a bug. Anyways, how about a totally different approach: Create a systemd unit that invokes |
training/nvidia-bootc/Containerfile
Outdated
| /etc/containers/storage.conf && \ | ||
| sed -i -e 's/^# force_mask.*$/force_mask = "shared"/' \ | ||
| /etc/containers/storage.conf && \ | ||
| sed -i -e 's@^#mount_program = .*@mount_program = "/usr/bin/fuse-overlayfs"@' \ |
There was a problem hiding this comment.
But won't this force fuse-overlayfs for rootful containers stored in /var/lib/containers too? That seems quite suboptimal.
There was a problem hiding this comment.
agree it should be just storage options on the command line (I believe that is what he is doing in the current approach he is trying - more recent than this PR)
There was a problem hiding this comment.
I believe that is what he is doing in the current approach he is trying - more recent than this PR
Yes, pushed modified code
There was a problem hiding this comment.
If you chown the file system, it will require fuse-overlayfs in order to see the intended user permission flags.
I am still of the opinion that this is overkill and not needed until we fully understand the UI intended for RHEL AI.
If only one user is going to use this system, then going though these hoops rather then just sudo ilab is just a risky overhead.
There was a problem hiding this comment.
Yeah the more we progress the more I realize how far from trivial this is the more I feel like giving up and going sudo for now. Really hoped it would just work
IMO it's not hacky, since it's an intended use case (e.g. overlay.force_mask=shared). Plus fundamentally I should be able to construct a container image for rootless users
+1 fairly severe IMO. In addition to breaking this use case, it would also break the ability to include standard rootless containers constructed under their UID, and any other uses of xattrs in container files |
DRAFT: DOESN'T WORK YET, the files are not getting the xattr set because of bootc/ostree limitations # Background The RHEL-AI bootc image contains the ilab container image pre-pulled. This image is downloaded using the `root` user into a container image storage. # Issue Non-root users which try to run the `ilab` wrapper which launches a container using the `ilab` image face a very large pull because as of today, they're not even using the storage root pulls into as an additional image store # Solution Modify the wrapper such that podman will use the root storage configuration. This is achieved through the `--storage-opt" "additionalimagestore=/usr/lib/containers/storage"` flag # Issue 2 Since the storage is populated by the root user, the storage is readable/searchable only by root. # Solution This change runs `chmod a+rx -R /usr/lib/containers` to make the storage directory and its contents accessible to non-root users. However, this has the effect of modifying the permissions of all the in-container files to be world-readable and world-executable. This is not ideal. To overcome this, we use `--storage-opt 'overlay.force_mask=shared' --storage-opt 'overlay.mount_program=/usr/bin/fuse-overlayfs'` to modify the storage config to `force_mask` `shared` (alias for `0755`) and the `mount_program` to `/usr/bin/fuse-overlayfs`, so that the original in-image file permissions are recorded in the `user.containers.override_stat` xattr. Signed-off-by: Omer Tuchfeld <[email protected]>
I was more referring to involving fuse-overlayfs, which I guess is necessary to force the codepaths to be taken? I would agree that In the general case, personally I would do something like |
Yeah, this is pretty rare though. Hmm...in some quick testing actually I'm unable to get a
It's there at runtime but seems to be lost during the commit? |
| fi | ||
| IID=$(sudo podman --root /usr/lib/containers/storage --storage-opt 'overlay.force_mask=shared' --storage-opt 'overlay.mount_program=/usr/bin/fuse-overlayfs' pull ${INSTRUCTLAB_IMAGE}); \ | ||
| fi \ | ||
| && chmod a+rx -R /usr/lib/containers |
There was a problem hiding this comment.
Do you still need to do this, or does this happen automatically with the force_mask=shared flag?
There was a problem hiding this comment.
Hmm good question, I thought I still needed it but that was before I realized --root was causing my config to be ignored. I'll have another try without it see what happens
There was a problem hiding this comment.
It still seems to be partially required, as while the permissions inside the diff directory are a+rx'd as expected, the other surrounding directories are not:
bash-5.1# cd /usr/lib
bash-5.1# ls -lah containers
drwx------. 3 root root 46 Jan 1 1970 containers
bash-5.1# cd containers/
bash-5.1# ls -lah
total 12K
drwx------. 3 root root 46 Jan 1 1970 .
drwxr-xr-x. 40 root root 866 Jan 1 1970 ..
drwx------. 7 root root 244 Jan 1 1970 storage
bash-5.1# cd storage/
bash-5.1# ls -lah
total 148K
drwx------. 7 root root 244 Jan 1 1970 .
drwx------. 3 root root 46 Jan 1 1970 ..
-rw-r--r--. 1 root root 112K Jan 1 1970 db.sql
-rw-r--r--. 1 root root 8 Jan 1 1970 defaultNetworkBackend
drwx------. 2 root root 27 Jan 1 1970 libpod
drwx------. 4 root root 146 Jan 1 1970 overlay
drwx------. 2 root root 54 Jan 1 1970 overlay-containers
drwx------. 3 root root 149 Jan 1 1970 overlay-images
drwx------. 2 root root 162 Jan 1 1970 overlay-layers
-rw-r--r--. 1 root root 64 Jan 1 1970 storage.lock
-rw-r--r--. 1 root root 0 Jan 1 1970 userns.lock
bash-5.1# cd overlay
bash-5.1# cd 9fd35ed55c106b3cbc36bc3f0f34c5a2df71fef0969b1ea323fc9470497d8ba4/
bash-5.1# ls -lah
total 28K
drwx------. 6 root root 110 Jan 1 1970 .
drwx------. 4 root root 146 Jan 1 1970 ..
drwxr-xr-x. 20 root root 417 Jan 1 1970 diff
drwx------. 2 root root 27 Jan 1 1970 empty
-rw-r--r--. 1 root root 26 Jan 1 1970 link
drwx------. 2 root root 27 Jan 1 1970 merged
drwx------. 2 root root 27 Jan 1 1970 work
bash-5.1# cd diff/
bash-5.1# ls -lah
total 116K
drwxr-xr-x. 20 root root 417 Jan 1 1970 .
drwx------. 6 root root 110 Jan 1 1970 ..
-rwxr-xr-x. 1 root root 17K Jan 1 1970 NGC-DL-CONTAINER-LICENSE
drwxr-xr-x. 2 root root 27 Jan 1 1970 afs
lrwxrwxrwx. 1 root root 7 Jan 1 1970 bin -> usr/bin
drwxr-xr-x. 2 root root 27 Jan 1 1970 boot
drwxr-xr-x. 2 root root 27 Jan 1 1970 dev
drwxr-xr-x. 51 root root 4.0K Jan 1 1970 etc
drwxr-xr-x. 2 root root 27 Jan 1 1970 home
drwxr-xr-x. 2 root root 27 Jan 1 1970 instructlab
lrwxrwxrwx. 1 root root 7 Jan 1 1970 lib -> usr/lib
lrwxrwxrwx. 1 root root 9 Jan 1 1970 lib64 -> usr/lib64
drwxr-xr-x. 2 root root 27 Jan 1 1970 lost+found
drwxr-xr-x. 2 root root 27 Jan 1 1970 media
drwxr-xr-x. 2 root root 27 Jan 1 1970 mnt
drwxr-xr-x. 3 root root 45 Jan 1 1970 opt
drwxr-xr-x. 2 root root 27 Jan 1 1970 proc
drwxr-xr-x. 5 root root 308 Jan 1 1970 root
drwxr-xr-x. 14 root root 258 Jan 1 1970 run
lrwxrwxrwx. 1 root root 8 Jan 1 1970 sbin -> usr/sbin
drwxr-xr-x. 2 root root 27 Jan 1 1970 srv
drwxr-xr-x. 2 root root 27 Jan 1 1970 sys
drwxr-xr-x. 2 root root 27 Jan 1 1970 tmp
drwxr-xr-x. 12 root root 209 Jan 1 1970 usr
drwxr-xr-x. 18 root root 332 Jan 1 1970 var
There was a problem hiding this comment.
BTW with chmod there is a+rX (note the capitalization) which only sets the executable bit if it's already set elsewhere, which is definitely what we want here.
Also, I don't see a need to traverse into the container roots, we just need to chmod on the containers and overlay directories I believe.
Interesting. it looks like it needs a value else "" gets lost. Try with -v bar |
|
Temporarily superseded by #713 |
Make the root-downloaded images available to non-root users
DRAFT: DOESN'T WORK YET, the files are not getting the xattr set because of bootc/ostree limitations
Background
The RHEL-AI bootc image contains the ilab container image pre-pulled. This image is downloaded using the
rootuser into a container image storage.Issue
Non-root users which try to run the
ilabwrapper which launches a container using theilabimage face a very large pull because as of today, they're not even using the storage root pulls into as an additional image storeSolution
Modify the wrapper such that podman will use the root storage configuration. This is achieved through the
--storage-opt" "additionalimagestore=/usr/lib/containers/storage"flagIssue 2
Since the storage is populated by the root user, the storage is readable/searchable only by root.
Solution
This change runs
chmod a+rx -R /usr/lib/containersto make the storage directory and its contents accessible to non-root users.However, this has the effect of modifying the permissions of all the in-container files to be world-readable and world-executable. This is not ideal. To overcome this, we use
--storage-opt 'overlay.force_mask=shared' --storage-opt 'overlay.mount_program=/usr/bin/fuse-overlayfs'to modify the storage config toforce_maskshared(alias for0755) and themount_programto/usr/bin/fuse-overlayfs, so that the original in-image file permissions are recorded in theuser.containers.override_statxattr.