Merge pull request #1198 from ckyrouac/storage-selinux-fix

imgstorage: Set selinux labels for imgstorage

Author: ckyrouac

lib/src/boundimage.rs

@@ -148,11 +148,11 @@ pub(crate) async fn pull_images(
     sysroot: &Storage,
     bound_images: Vec<crate::boundimage::BoundImage>,
 ) -> Result<()> {
-    // Only do work like initializing the image storage if we have images to pull.
+    // Always initialize the img store to ensure labels are set when upgrading
+    let imgstore = sysroot.get_ensure_imgstore()?;
     if bound_images.is_empty() {
         return Ok(());
     }
-    let imgstore = sysroot.get_ensure_imgstore()?;
     pull_images_impl(imgstore, bound_images).await
 }
 

lib/src/imgstorage.rs

@@ -20,6 +20,7 @@ use cap_std_ext::cap_tempfile::TempDir;
 use cap_std_ext::cmdext::CapStdExtCommandExt;
 use cap_std_ext::dirext::CapStdExtDirExt;
 use fn_error_context::context;
+use ostree_ext::ostree::{self};
 use std::os::fd::OwnedFd;
 use tokio::process::Command as AsyncCommand;
 
@@ -35,6 +36,8 @@ pub(crate) const STORAGE_ALIAS_DIR: &str = "/run/bootc/storage";
 /// We pass this via /proc/self/fd to the child process.
 const STORAGE_RUN_FD: i32 = 3;
 
+const LABELED: &str = ".bootc_labeled";
+
 /// The path to the image storage, relative to the bootc root directory.
 pub(crate) const SUBPATH: &str = "storage";
 /// The path to the "runroot" with transient runtime state; this is
@@ -161,24 +164,68 @@ impl Storage {
         Ok(())
     }
 
+    #[context("Labeling imgstorage dirs")]
+    fn label_dirs(root: &Dir, sepolicy: Option<&ostree::SePolicy>) -> Result<()> {
+        if root.try_exists(LABELED)? {
+            return Ok(());
+        }
+        let Some(sepolicy) = sepolicy else {
+            return Ok(());
+        };
+
+        // recursively set the labels because they were previously set to usr_t,
+        // and there is no policy defined to set them to the c/storage labels
+        crate::lsm::ensure_dir_labeled_recurse_policy(
+            &root,
+            ".",
+            Some(Utf8Path::new("/var/lib/containers/storage")),
+            0o755.into(),
+            Some(sepolicy),
+        )
+        .context("labeling storage root")?;
+
+        let paths = ["overlay", "overlay-images", "overlay-layers", "volumes"];
+        for p in paths {
+            let full_label_path = format!("/var/lib/containers/storage/{}", p);
+            crate::lsm::ensure_dir_labeled_recurse_policy(
+                &root,
+                p,
+                Some(Utf8Path::new(full_label_path.as_str())),
+                0o755.into(),
+                Some(sepolicy),
+            )
+            .context(format!("labeling storage subpath: {}", p))?;
+        }
+
+        root.create(LABELED)?;
+
+        Ok(())
+    }
+
     #[context("Creating imgstorage")]
-    pub(crate) fn create(sysroot: &Dir, run: &Dir) -> Result<Self> {
+    pub(crate) fn create(
+        sysroot: &Dir,
+        run: &Dir,
+        sepolicy: Option<&ostree::SePolicy>,
+    ) -> Result<Self> {
         Self::init_globals()?;
         let subpath = &Self::subpath();
 
         // SAFETY: We know there's a parent
         let parent = subpath.parent().unwrap();
+        let tmp = format!("{subpath}.tmp");
         if !sysroot
             .try_exists(subpath)
             .with_context(|| format!("Querying {subpath}"))?
         {
-            let tmp = format!("{subpath}.tmp");
             sysroot.remove_all_optional(&tmp).context("Removing tmp")?;
             sysroot
                 .create_dir_all(parent)
                 .with_context(|| format!("Creating {parent}"))?;
             sysroot.create_dir_all(&tmp).context("Creating tmpdir")?;
             let storage_root = sysroot.open_dir(&tmp).context("Open tmp")?;
+            Self::label_dirs(&storage_root, sepolicy)?;
+
             // There's no explicit API to initialize a containers-storage:
             // root, simply passing a path will attempt to auto-create it.
             // We run "podman images" in the new root.
@@ -192,7 +239,12 @@ impl Storage {
                 .rename(&tmp, sysroot, subpath)
                 .context("Renaming tmpdir")?;
             tracing::debug!("Created image store");
+        } else {
+            // the storage already exists, make sure it has selinux labels
+            let storage_root = sysroot.open_dir(subpath).context("opening storage dir")?;
+            Self::label_dirs(&storage_root, sepolicy)?;
         }
+
         Self::open(sysroot, run)
     }
 

lib/src/install.rs

@@ -605,7 +605,10 @@ pub(crate) fn print_configuration() -> Result<()> {
 }
 
 #[context("Creating ostree deployment")]
-async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result<(Storage, bool)> {
+async fn initialize_ostree_root(
+    state: &State,
+    root_setup: &RootSetup,
+) -> Result<(Storage, bool, crate::imgstorage::Storage)> {
     let sepolicy = state.load_policy()?;
     let sepolicy = sepolicy.as_ref();
     // Load a fd for the mounted target physical root
@@ -671,14 +674,9 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
 
     state.tempdir.create_dir("temp-run")?;
     let temp_run = state.tempdir.open_dir("temp-run")?;
-    sysroot_dir
-        .create_dir_all(Utf8Path::new(crate::imgstorage::SUBPATH).parent().unwrap())
-        .context("creating bootc dir")?;
-    let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &temp_run)?;
-    // And drop it again - we'll reopen it after this
-    drop(imgstore);
 
     // Bootstrap the initial labeling of the /ostree directory as usr_t
+    // and create the imgstorage with the same labels as /var/lib/containers
     if let Some(policy) = sepolicy {
         let ostree_dir = rootfs_dir.open_dir("ostree")?;
         crate::lsm::ensure_dir_labeled(
@@ -690,9 +688,11 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result
         )?;
     }
 
+    let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &temp_run, sepolicy)?;
+
     sysroot.load(cancellable)?;
     let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?;
-    Ok((Storage::new(sysroot, &temp_run)?, has_ostree))
+    Ok((Storage::new(sysroot, &temp_run)?, has_ostree, imgstore))
 }
 
 #[context("Creating ostree deployment")]
@@ -1322,6 +1322,7 @@ async fn install_with_sysroot(
     boot_uuid: &str,
     bound_images: BoundImages,
     has_ostree: bool,
+    imgstore: &crate::imgstorage::Storage,
 ) -> Result<()> {
     // And actually set up the container in that root, returning a deployment and
     // the aleph state (see below).
@@ -1349,10 +1350,6 @@ async fn install_with_sysroot(
 
     tracing::debug!("Perfoming post-deployment operations");
 
-    // Note that we *always* initialize this container storage, even if there are no bound images
-    // today.
-    let imgstore = sysroot.get_ensure_imgstore()?;
-
     match bound_images {
         BoundImages::Skip => {}
         BoundImages::Resolved(resolved_bound_images) => {
@@ -1438,14 +1435,16 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re
 
     // Initialize the ostree sysroot (repo, stateroot, etc.)
     {
-        let (sysroot, has_ostree) = initialize_ostree_root(state, rootfs).await?;
+        let (sysroot, has_ostree, imgstore) = initialize_ostree_root(state, rootfs).await?;
+
         install_with_sysroot(
             state,
             rootfs,
             &sysroot,
             &boot_uuid,
             bound_images,
             has_ostree,
+            &imgstore,
         )
         .await?;
         // We must drop the sysroot here in order to close any open file

lib/src/install/completion.rs

@@ -13,6 +13,9 @@ use fn_error_context::context;
 use ostree_ext::{gio, ostree};
 use rustix::fs::Mode;
 use rustix::fs::OFlags;
+use std::os::fd::AsRawFd;
+
+use crate::utils::deployment_fd;
 
 use super::config;
 
@@ -289,9 +292,15 @@ pub(crate) async fn impl_completion(
     // ostree-ext doesn't do logically bound images
     let bound_images = crate::boundimage::query_bound_images_for_deployment(sysroot, deployment)?;
     if !bound_images.is_empty() {
+        // load the selinux policy from the target ostree deployment
+        let deployment_fd = deployment_fd(sysroot, deployment)?;
+        let sepolicy =
+            &ostree::SePolicy::new_at(deployment_fd.as_raw_fd(), gio::Cancellable::NONE)?;
+
         // When we're run through ostree, we only lazily initialize the podman storage to avoid
         // having a hard dependency on it.
-        let imgstorage = &crate::imgstorage::Storage::create(&sysroot_dir, &rundir)?;
+        let imgstorage =
+            &crate::imgstorage::Storage::create(&sysroot_dir, &rundir, Some(sepolicy))?;
         crate::boundimage::pull_images_impl(imgstorage, bound_images)
             .await
             .context("pulling bound images")?;

lib/src/lsm.rs

@@ -259,6 +259,36 @@ pub(crate) fn ensure_labeled(
     Ok(r)
 }
 
+pub(crate) fn ensure_dir_labeled_recurse_policy(
+    root: &Dir,
+    destname: impl AsRef<Utf8Path>,
+    as_path: Option<&Utf8Path>,
+    mode: rustix::fs::Mode,
+    policy: Option<&ostree::SePolicy>,
+) -> Result<()> {
+    ensure_dir_labeled(root, &destname, as_path, mode, policy)?;
+    let mut dest_path = destname.as_ref().to_path_buf();
+
+    for ent in root.read_dir(destname.as_ref())? {
+        let ent = ent?;
+        let metadata = ent.metadata()?;
+        let name = ent.file_name();
+        let name = name
+            .to_str()
+            .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF-8 filename: {name:?}"))?;
+        dest_path.push(name);
+
+        if metadata.is_dir() {
+            ensure_dir_labeled_recurse_policy(root, &dest_path, as_path, mode, policy)?;
+        } else {
+            ensure_dir_labeled(root, &dest_path, as_path, mode, policy)?
+        }
+        dest_path.pop();
+    }
+
+    Ok(())
+}
+
 /// A wrapper for creating a directory, also optionally setting a SELinux label.
 /// The provided `skip` parameter is a device/inode that we will ignore (and not traverse).
 pub(crate) fn ensure_dir_labeled_recurse(

lib/src/store/mod.rs

@@ -7,13 +7,15 @@ use cap_std_ext::cap_std::fs::Dir;
 use cap_std_ext::dirext::CapStdExtDirExt;
 use clap::ValueEnum;
 use fn_error_context::context;
+use std::os::fd::AsRawFd;
 
 use ostree_ext::container::OstreeImageReference;
 use ostree_ext::keyfileext::KeyFileExt;
-use ostree_ext::ostree;
 use ostree_ext::sysroot::SysrootLock;
+use ostree_ext::{gio, ostree};
 
 use crate::spec::ImageStatus;
+use crate::utils::deployment_fd;
 
 mod ostree_container;
 
@@ -85,7 +87,18 @@ impl Storage {
             return Ok(imgstore);
         }
         let sysroot_dir = crate::utils::sysroot_dir(&self.sysroot)?;
-        let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &self.run)?;
+
+        if self.sysroot.booted_deployment().is_none() {
+            anyhow::bail!("Not a bootc system (this shouldn't be possible)");
+        }
+
+        // load the sepolicy from the booted ostree deployment so the imgstorage can be
+        // properly labeled with /var/lib/container/storage labels
+        let dep = self.sysroot.booted_deployment().unwrap();
+        let dep_fs = deployment_fd(&self.sysroot, &dep)?;
+        let sepolicy = &ostree::SePolicy::new_at(dep_fs.as_raw_fd(), gio::Cancellable::NONE)?;
+
+        let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &self.run, Some(sepolicy))?;
         Ok(self.imgstore.get_or_init(|| imgstore))
     }
 

tests/booted/test-logically-bound-install.nu

@@ -35,7 +35,15 @@ def test_bootc_image_list [] {
     validate_images $images
 }
 
+def test_storage_labels [] {
+    let root_labeled = getfattr -n security.selinux /var/lib/containers/storage | grep container_var_lib_t | complete
+    assert equal $root_labeled.exit_code 0
+    let overlay_labeled = getfattr -n security.selinux /usr/lib/bootc/storage/overlay | grep container_ro_file_t | complete
+    assert equal $overlay_labeled.exit_code 0
+}
+
 test_logically_bound_images_in_storage
 test_bootc_image_list
+test_storage_labels
 
 tap ok