containers / ai-lab-recipes

Examples for building and running LLM services and applications locally with Podman
Apache License 2.0
111 stars 110 forks source link

Make the root-downloaded images available to non-root users #705

Closed omertuc closed 3 months ago

omertuc commented 3 months ago

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 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.

rhatdan commented 3 months ago

Make sure to sign your commits.

cgwalters commented 3 months ago

This change runs chmod a+rx -R /usr/lib/containers to make the storage directory and its contents accessible to non-root users.

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.

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.

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 podman mount (and yes a dummy container for that) for these images at start into a well known location, then user units can use podman run --rootfs or so? (Actually, testing this out, it seems like --rootfs is super raw, even using :O to try to set up an overlayfs on top eventually fails due to run not being mounted?) I can easily set up a container with basic bwrap pointed at an underlying base rootfs)

n1hility commented 3 months ago

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.

That seems...quite hacky?

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

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.

+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

cgwalters commented 3 months ago

IMO it's not hacky, since it's an intended use case (e.g. overlay.force_mask=shared).

I was more referring to involving fuse-overlayfs, which I guess is necessary to force the codepaths to be taken?

I would agree that overlay.force_mask=shared was probably motivated by this, but also overriding the inaccessible dir perms as a prereq is also inherently ugly.

In the general case, personally I would do something like podman image set-permission <imagename> world-readable, and we could have a little (polkit enabled?) service that allowed mounting such an image in the desired mount namespace. Basically instead of a global modification to the storage config, allow opt-in per image and make permissions dynamic instead of static.

cgwalters commented 3 months ago

and any other uses of xattrs in container files

Yeah, this is pretty rare though. Hmm...in some quick testing actually I'm unable to get a user. xattr through the storage stack with:

RUN touch /usr/sometestfile && setfattr -n user.test /usr/sometestfile && getfattr -d -m . /usr/sometestfile

It's there at runtime but seems to be lost during the commit?

n1hility commented 3 months ago

and any other uses of xattrs in container files

Yeah, this is pretty rare though. Hmm...in some quick testing actually I'm unable to get a user. xattr through the storage stack with:

RUN touch /usr/sometestfile && setfattr -n user.test /usr/sometestfile && getfattr -d -m . /usr/sometestfile

It's there at runtime but seems to be lost during the commit?

Interesting. it looks like it needs a value else "" gets lost. Try with -v bar

omertuc commented 3 months ago

Temporarily superseded by #713