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

manifest: label {,u}mount as install_exec_t to avoid selinux denials #1

Closed
wants to merge 1 commit into from

Conversation

mvo5
Copy link

@mvo5 mvo5 commented Nov 27, 2023

I explored the ideas around how to make osbuild-deploy-container selinux denial free a bit Friday/Monday and here is one way of doing it that avoids us having to implement a native ctypes based mount/unmount. Please double check my reasoning below @ondrejbudai and @achilleas-k


When running osbuild inside a container created via the new osbuild-deploy-container the selinux setup is interesting (and different from a normal host).

Because the / inside the contains is mounted nosuid the main osbuild binary is labeled with install_exec_t because with nosuid the transition from osbuild_t to install_t is not allowed. This works around the limitations of the container.

However when in install_t the transition to mount_t is not allowed which leads to an selinux denial in the logs. The install_t has all the privs needed so even with this transition failing mount still works.

This commit labels {,u}mount with install_exec_t in the buildroot now as well to avoid this error in the logs.

Open questions:

  • is this really strictly only for the buildroot or could these hacked permissions somehow escape into a real image?
  • why is the error from install_t -> mount_t not fatal?

Missing:

  • some sort of integration test that ensures we can automatically test that the contains is free of denials.

When running osbuild inside a container created via the new
`osbuild-deploy-container` the selinux setup is interesting (and
different from a normal host).

Because the `/` inside the contains is mounted `nosuid` the main
osbuild binary is labeled with `install_exec_t` because with
`nosuid` the transition from `osbuild_t` to `install_t` is not
allowed. This works around the limitations of the container.

However when in `install_t` the transition to `mount_t` is not
allowed which leads to an selinux denial in the logs. The `install_t`
has all the privs needed so even with this transition failing
mount still works.

This commit labels `{,u}mount` with `install_exec_t` in the
buildroot now as well to avoid this error in the logs.

Open questions:
- is this *really* strictly only for the buildroot or could these
  hacked permissions somehow escape into a real image?
- why is the error from `install_t` -> `mount_t` not fatal?

Missing:
- some sort of integration test that ensures we can automatically
  test that the contains is free of denials.
@achilleas-k
Copy link
Owner

achilleas-k commented Nov 27, 2023

  • is this really strictly only for the buildroot or could these hacked permissions somehow escape into a real image?

It should be. I don't see how this might affect the image unless we copy things from the build root to the image. Which we do sometimes for EFI binaries (we should figure out how to avoid that btw).

@achilleas-k
Copy link
Owner

We should move this to the real images repo. Since we only need to set it in certain cases (the bifrost builder), we can add it as an optional toggle on the build pipeline that we only set when needed. Here for example
https://github.com/osbuild/osbuild-deploy-container/pull/11/files#diff-895717f8731ac528a1e229abd5c4b2d22eb2c471412ea30c1df61b88dd90e4f5R49
we could set an option on img (img.ContainerBuild? Something better) that gets propagated to the Build pipeline.

@mvo5 mvo5 changed the title manifest: label {,u}mount as install_exec_t to avoid apparmor denials manifest: label {,u}mount as install_exec_t to avoid selinux denials Nov 27, 2023
@ondrejbudai
Copy link

* why is the error from `install_t` -> `mount_t` not fatal?

I believe that when there's an SELinux transition denial, the transition just doesn't happen. And because the stage has CAP_SYS_ADMIN (neither install_t, nor bwrap drops it) mount(1) just works fine.

we could set an option on img (img.ContainerBuild? Something better) that gets propagated to the Build pipeline.

I agree!

@mvo5
Copy link
Author

mvo5 commented Nov 27, 2023

Closing and will be reworked and opened against the right repo

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.

3 participants