imgstorage: Set selinux labels for imgstorage

[ckyrouac]

Running some containers (e.g. mssql) requires the imgstorage labels to
be identical to the /var/lib/containers/storage. The ideal long term
solution to this is to ship a policy with bootc to be included in each
bootc image that handles the mapping. As a near term solution, this
commit creates each subdirectory of /sysroot/ostree/bootc/storage, sets
the label of each directory directly, then runs podman --root ... images to populate the storage directory with all the correct labels
set. The reason for directly setting the labels rather than inferring
them from /var/lib/containers in an existing policy, is to avoid
requiring a policy when initializing the storage directory in other
places than the install to filesystem path (e.g. anaconda).

---

this ballooned into more changes than I expected. I'm not sure if directly setting the label is what we want, but I was having a hard time finding a way to access a policy to use in the anaconda installation path, or the image cli stuff.

[cgwalters]

It's quite suboptimal to hardcode specific labels for a few reasons. A simple one is that these things can change over time inside the policy. But a larger reason is that there are actually multiple SELinux policies out there. For example Android has a totally different one (not that we're likely to run there but you never know). More realistically though there are definitely people who want to deploy custom hardened policies too (selinux-policy-targeted is too "loose" for some use cases). See e.g. https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/using_selinux/using-multi-level-security-mls_using-selinux#ref_selinux-roles-in-mls_using-multi-level-security-mls

(Tangent on selinux-policy-mls but a related and very topical one; look at those installation instructions targeting a live system - a giant pile of hacks! Whereas if you build a bootc container with the package, you should get a transactional in-place install (or upgrade) to the new state...)

Anyways my belief here is that it should work to use what is now ensure_dir_labeled_as_path and pass /var/lib/containers etc here. The way this would work is it looks up /var/lib/containers in the file context regexps list and uses it for our target path. Did you try that and it failed?

Now you mention Anaconda...I think making this work will require that we have container-selinux (and selinux-policy-targeted) in the Anaconda environment.

Actually more generally, it will mean we use the versions of those - which is different from the ones in the bootc container in the general case. This isn't a new problem at all - when installing RPMs with Anaconda, the /usr/bin/rpm comes from Anaconda today. But for bootc we can do something a bit nicer because the target for installation is a container image - a well-defined self-contained entity. And that is "tracked" in rhinstaller/anaconda#5197 to be sure we always use the bootc (and policy versions, etc.) from the target system, not whatever is in the ISO.

---

Anyways...again can you say if the change to use ensure_dir_labeled_as_path() was actually failing?

Mmmmm. The more I think about this the more I'm thinking that in the near term we should build on the recently added install finalize.

• Change Anaconda to invoke it
• Fix up the labels and such based on the installed target system using it

[ckyrouac]

It's quite suboptimal to hardcode specific labels for a few reasons.

Agreed, just thought this might be a valid temporary approach if we plan to ship a policy in the base images sooner rather than later.

Anyways my belief here is that it should work to use what is now ensure_dir_labeled_as_path and pass /var/lib/containers etc here. The way this would work is it looks up /var/lib/containers in the file context regexps list and uses it for our target path. Did you try that and it failed?

The ImgStorage::init_dirs function in this PR worked using ensure_dir_labeled_as_path in the install-to-filesystem path. However, since ensure_dir_labeled_as_path looks up the label from the ostree.SePolicy, it doesn't work when the storage is initialized in other places of the code, e.g. here, here, here, etc.

Mmmmm. The more I think about this the more I'm thinking that in the near term we should build on the recently added #1157.

Change Anaconda to invoke it
Fix up the labels and such based on the installed target system using it

I think this will work for the anaconda and install-to-filesystem cases. A problem I was running into was setting the labels after the storage is initialized, since it's not enough to just set the top level directory (and sub-directories). I guess since the update path will need to recursively set all the labels anyways, we can just do the same thing in install_finalize?

The other issue is the CLI commands that currently initialize the storage if it doesn't exists, e.g. bootc image pull-from-default-storage. I guess if we make sure the storage is initialized on install and update, then we can change the CLI code that needs storage to fail if it isn't initialized?

[cgwalters]

I'm OK with this change but the rationale behind the more complex code that was there before was it was attempting to do an atomic initialization of the target directory, i.e. either everything there is created or not. As is right now if the command dies partway through I am not sure that the c/storage stack will handle completing initialization.

Perhaps a theoretical concern but

• It's important to me that bootc gives transactional upgrades, i.e. we should be robust to having e.g. the kernel freeze or the laptop battery fall out or being randomly struck by the OOM killer or whatever.
• While what we're talking about here is for the installation path, this code can also get invoked for old bootc systems that predate LBIs (or just systems previously pure ostree) that don't have the c/storage instance.

I guess in the end we should just make it a c/storage bug if it doesn't handle partial initialization.

[ckyrouac]

👍 makes sense, should be simple to preserve the atomic initialization. c/storage does seem to handle at least some amount of partial initialization since I am able to create and label the sub-directories then let c/storage create the rest after.

[cgwalters]

We discussed this for Anaconda

diff --git a/lib/src/install/completion.rs b/lib/src/install/completion.rs
index 85983558..81c941b9 100644
--- a/lib/src/install/completion.rs
+++ b/lib/src/install/completion.rs
@@ -14,6 +14,8 @@ use ostree_ext::{gio, ostree};
 use rustix::fs::Mode;
 use rustix::fs::OFlags;
 
+use crate::utils::deployment_fd;
+
 use super::config;
 
 /// An environment variable set by anaconda that hints
@@ -277,6 +279,9 @@ pub(crate) async fn impl_completion(
         .merge_deployment(stateroot)
         .ok_or_else(|| anyhow::anyhow!("Failed to find deployment (stateroot={stateroot:?}"))?;
     let sysroot_dir = crate::utils::sysroot_dir(&sysroot)?;
+    let deployment_fd = &deployment_fd(sysroot, deployment)?;
+
+    let sepolicy = &ostree::SePolicy::new_at(deployment_fd.as_raw_fd(), gio::Cancellable::NONE)?;
 
     // Create a subdir in /run
     let rundir = "run/bootc-install";

[cgwalters]

Discussing:

• Add a stamp file like .bootc-labeled and if that isn't present then we do a recursive relabel; this will cover upgrades and installs too. The install path is a bit ugly here because we'll label it usr_t first but seems OK. This will need to take a SePolicy (in the install path we want to use the one from the deployment/target, but in a booted system we can just load from /)

[ckyrouac]

How do we want to handle upgrades? I initially thought I'd do the relabel in stage, using the policy from the staged deployment, but that has the potential to break the current deployment. e.g. if the labels changed in the staged deployment, currently running LBIs might break until rebooting into the new deployment. Or, if the upgrade is aborted, the labels will be incorrect indefinitely. I guess if we're linking the bootc container storage to a deployment via its selinux policy, to prevent this we need to have two storages (staged/booted)?

[ckyrouac]

@cgwalters still need to do some testing, but I pushed the latest code that at least works on the install path so you can take a look.

[cgwalters]

This looks pretty good to me overall! I'm fine to land as is, all this stuff is optional followup overall.

[cgwalters]

I think in order to add a bit more crash resilence here it'd be a good idea to do e.g.:

rustix::fs::fsync(root.as_fd());
root.create(LABELED)?;
rustix::fs::fsync(root.as_fd());

or so - that should help ensure that we don't end up in a situation where (on a system crash) the LABELED file exists but the pending writes to do the relabeling didn't land.

(arg, in a quick test this fails because a cap-std Dir is an O_PATH fd and we can't fsync on that, I will look at a little helper for this)

[cgwalters]

I think this is totally fine as is, but personally I lean towards making functions like this idempotent - IOW we do inside Self::label_dirs:

let Some(policy) = sepolicy else {
    return Ok(());
};
if storage_root.try_exists(LABELED)? {
    return Ok(());
}

or so

[cgwalters]

The booted deployment's directory is going to be equivalent (mostly) to /...actually where it won't is if there are locally modified policy in /etc - which we probably want to use?

If we did that it'd argue to just open up the / directory...basically everywhere instead of passing e.g. self.run above we just pass a fd for the root. OR we could pass a SePolicy instance.

But...eh, this can all be followup.

[cgwalters]

One other thing actually, it should be straightforward to add a test case under tests/booted/readonly/ that does e.g. getfattr -n security.selinux /usr/lib/bootc/storage/overlay | grep container_ro_file_t or so?

[ckyrouac]

thanks! Added a test and cleaned up label_dirs per your suggestion. Will file issues for followups.

[cgwalters]

EDIT: Fallout in #1219