Skip to content
This repository has been archived by the owner on Nov 7, 2024. It is now read-only.

Exporting OSTree commit to OCI does not maintain xattrs #655

Closed
antheas opened this issue Aug 21, 2024 · 10 comments · Fixed by #679
Closed

Exporting OSTree commit to OCI does not maintain xattrs #655

antheas opened this issue Aug 21, 2024 · 10 comments · Fixed by #679

Comments

@antheas
Copy link
Collaborator

antheas commented Aug 21, 2024

Part 2 of issue:
#654

As noted in the ostree-rs-ext readme, tar encoding used to have very poor characteristics for xattrs, which is why the format bare-split-xattrs exists for an OSTree repository, which maintains them.

This is not true anymore. The version of podman shipped in Ubuntu 2024 handles both SELinux and xattrs correctly.

The focus of this issue is that a quirk in the OSTree to OCI converter means that the xattrs of the files are not serialized to the tar stream.

This is not an issue during image deployment.

However, it does become an issue if I want to use rechunk on top of rechunk (e.g., extend a base image and then chunk it again), or deploy an OSTree image with future bootc.

As both will drop caps and ignore the xattrs in the OSTree repo of the image, which is discarded.

Therefore, ostree-rs-ext needs to encode those attributes into the tar stream, so that recent versions of podman can read and maintain them.

If a novice user extends an image with an old version of podman and drops the xattrs this is OK, as it is very unlikely that user will introduce files that need caps or special perms. Then, during deployment, rpm-ostree and bootc can reuse the xattrs and directory permissions from the bare-xattrs metadata. More advanced users will be aware of this requirement if they plan to rechunk their image.

@cgwalters
Copy link
Member

Dup of #654 right?

@cgwalters
Copy link
Member

Oh sorry, I see...you're actually arguing we should switch to encoding them in the tar stream; I was misled by the issue title.

@antheas
Copy link
Collaborator Author

antheas commented Aug 21, 2024

In addition to putting them in the OSTree repo.

Just verified the xattrs are not preserved.

Top image is the original bazzite image which installs gamescope that requires sys_nice. Bottom image is after passing through rechunk (different versions, but both have gamescope).

❯ sudo podman run -it --rm ghcr.io/ublue-os/bazzite-ally:40-20240427 bash -c "getcap /usr/bin/*"
/usr/bin/gamescope cap_sys_nice=eip
❯ sudo podman run -it --rm ghcr.io/hhd-dev/bazzite-automated-deck bash -c "getcap /usr/bin/*"
// no output

Mind the fact that both images miss e.g.,:

/usr/bin/newgidmap cap_setgid=ep
/usr/bin/newuidmap cap_setuid=ep

Which end up breaking rootless podman. If you were to deploy straight away. Of course, ostree-rs-ext will pull the caps from the repo, and I quirk the files as part of rechunk currently.

@cgwalters
Copy link
Member

This issue also relates to containers/podman#18543 among others.

The version of podman shipped in Ubuntu 2024 handles both SELinux and xattrs correctly.

One giant problem though is that you can't set security.selinux directly in a native container build.

@antheas
Copy link
Collaborator Author

antheas commented Aug 21, 2024

From my current understanding, we have yet to find a usecase for custom security.selinux. So it is fine for that to be discarded.

The problem with SELinux currently is that rpm-ostree derives the policy from the base commit. Which causes issues mainly with Waydroid and Cosmic at the moment.

@antheas
Copy link
Collaborator Author

antheas commented Aug 27, 2024

Seems like tar-rs does not support writing xattrs.

@cgwalters
Copy link
Member

@antheas
Copy link
Collaborator Author

antheas commented Aug 27, 2024

Indeed, the tar format supports xattrs.

But from looking at the tar-rs source code used in ostree-rs-ext it seems to only support reading them, not writing them.

https://github.com/alexcrichton/tar-rs/blob/97d5033eb80bf90c746a8b76dd31ca8a39326991/src/entry.rs#L872

Seems like SCHILY.xattr. is only wired up in entry.rs, which is used for extracting archives.

@antheas
Copy link
Collaborator Author

antheas commented Aug 30, 2024

You were right, I looked over this today again.

Then got lost trying to convert glib::Variant to the pax header using the code sample you referenced.

Looks like a simple addition though. I will probably give it another shot over the weekend.

@cgwalters
Copy link
Member

alexcrichton/tar-rs#382 will be helpful for this

cgwalters added a commit to cgwalters/ostree-rs-ext that referenced this issue Nov 1, 2024
This relies on the new tar API.

Closes: ostreedev#655

Signed-off-by: Colin Walters <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants