Rework SELinux labeling more

First, in the install code, acquire a proper policy object.

Add helpers for writing files/directories that take a policy
object and operate *solely* using fd-relative operations and
don't fork off helper processes.

This is a notable cleanup because we don't need to juggle
absolute file paths *and* fds, which avoids a lot of confusion.
Our usage of a wrapper for the cap-std-ext atomic write API
for generating files ensures that if the file is present,
it will always have the correct label without any race conditions.

Change the one place we now call `chcon` as a helper process
to be an explicit recursive selinux relabeling.  In the
future we should switch to using a direct API instead of
forking off `/usr/bin/chcon` - then everything would be fd-relative.

Signed-off-by: Colin Walters <[email protected]>

.github/workflows/ci.yml

@@ -132,16 +132,17 @@ jobs:
       - name: Integration tests
         run: |
           set -xeuo pipefail
+          image=quay.io/centos-bootc/centos-bootc-dev:stream9 
           echo 'ssh-ed25519 ABC0123 [email protected]' > test_authorized_keys
           sudo podman run --rm -ti --privileged -v ./test_authorized_keys:/test_authorized_keys --env RUST_LOG=debug -v /:/target -v /var/lib/containers:/var/lib/containers -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable \
-            quay.io/centos-bootc/centos-bootc-dev:stream9 bootc install to-filesystem \
+            ${image} bootc install to-filesystem \
             --karg=foo=bar --disable-selinux --replace=alongside --root-ssh-authorized-keys=/test_authorized_keys /target
+          sudo podman run --rm -ti --privileged -v /:/target -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable ${image} bootc internal-tests verify-selinux /target/ostree --warn
           ls -al /boot/loader/
           sudo grep foo=bar /boot/loader/entries/*.conf
           grep authorized_keys /ostree/deploy/default/deploy/*/etc/tmpfiles.d/bootc-root-ssh.conf
           # TODO fix https://github.com/containers/bootc/pull/137
           sudo chattr -i /ostree/deploy/default/deploy/*
           sudo rm /ostree/deploy/default -rf
           sudo podman run --rm -ti --privileged --env BOOTC_SKIP_SELINUX_HOST_CHECK=1 --env RUST_LOG=debug -v /:/target -v /var/lib/containers:/var/lib/containers -v ./usr/bin/bootc:/usr/bin/bootc --pid=host --security-opt label=disable \
-            quay.io/centos-bootc/centos-bootc-dev:stream9 bootc install to-existing-root
-          sudo ls -ldZ / /ostree/deploy/default/deploy/* /ostree/deploy/default/deploy/*/etc
+            ${image} bootc install to-existing-root

lib/src/cli.rs

@@ -150,6 +150,12 @@ pub(crate) enum TestingOpts {
         image: String,
         blockdev: Utf8PathBuf,
     },
+    #[clap(name = "verify-selinux")]
+    VerifySELinux {
+        root: String,
+        #[clap(long)]
+        warn: bool,
+    },
 }
 
 /// Deploy and transactionally in-place with bootable container images.

lib/src/install.rs

@@ -10,7 +10,6 @@ pub(crate) mod baseline;
 pub(crate) mod config;
 pub(crate) mod osconfig;
 
-use std::io::BufWriter;
 use std::io::Write;
 use std::os::fd::AsFd;
 use std::os::unix::process::CommandExt;
@@ -303,17 +302,17 @@ pub(crate) struct State {
 }
 
 impl State {
-    // Wraps core lsm labeling functionality, conditionalizing based on source state
-    pub(crate) fn lsm_label(
-        &self,
-        target: &Utf8Path,
-        as_path: &Utf8Path,
-        recurse: bool,
-    ) -> Result<()> {
-        if !self.source.selinux {
-            return Ok(());
+    #[context("Loading SELinux policy")]
+    pub(crate) fn load_policy(&self) -> Result<Option<ostree::SePolicy>> {
+        use std::os::fd::AsRawFd;
+        if !self.source.selinux || self.override_disable_selinux {
+            return Ok(None);
         }
-        crate::lsm::lsm_label(target, as_path, recurse)
+        // We always use the physical container root to bootstrap policy
+        let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?;
+        let r = ostree::SePolicy::new_at(rootfs.as_raw_fd(), gio::Cancellable::NONE)?;
+        tracing::debug!("Loaded SELinux policy: {}", r.name());
+        Ok(Some(r))
     }
 }
 
@@ -510,13 +509,17 @@ async fn initialize_ostree_root_from_self(
     state: &State,
     root_setup: &RootSetup,
 ) -> Result<InstallAleph> {
+    let sepolicy = state.load_policy()?;
+    let sepolicy = sepolicy.as_ref();
+
+    // Load a fd for the mounted target physical root
     let rootfs_dir = &root_setup.rootfs_fd;
     let rootfs = root_setup.rootfs.as_path();
     let cancellable = gio::Cancellable::NONE;
 
     // Ensure that the physical root is labeled.
     // Another implementation: https://github.com/coreos/coreos-assembler/blob/3cd3307904593b3a131b81567b13a4d0b6fe7c90/src/create_disk.sh#L295
-    state.lsm_label(rootfs, "/".into(), false)?;
+    crate::lsm::ensure_dir_labeled(rootfs_dir, "", Some("/".into()), 0o755.into(), sepolicy)?;
 
     // TODO: make configurable?
     let stateroot = STATEROOT_DEFAULT;
@@ -529,7 +532,7 @@ async fn initialize_ostree_root_from_self(
     // And also label /boot AKA xbootldr, if it exists
     let bootdir = rootfs.join("boot");
     if bootdir.try_exists()? {
-        state.lsm_label(&bootdir, "/boot".into(), false)?;
+        crate::lsm::ensure_dir_labeled(rootfs_dir, "boot", None, 0o755.into(), sepolicy)?;
     }
 
     // Default to avoiding grub2-mkconfig etc., but we need to use zipl on s390x.
@@ -557,8 +560,17 @@ async fn initialize_ostree_root_from_self(
         .cwd(rootfs_dir)?
         .run()?;
 
-    // Ensure everything in the ostree repo is labeled
-    state.lsm_label(&rootfs.join("ostree"), "/usr".into(), true)?;
+    // Bootstrap the initial labeling of the /ostree directory as usr_t
+    if let Some(policy) = sepolicy {
+        let ostree_dir = rootfs_dir.open_dir("ostree")?;
+        crate::lsm::ensure_dir_labeled(
+            &ostree_dir,
+            ".",
+            Some("/usr".into()),
+            0o755.into(),
+            Some(policy),
+        )?;
+    }
 
     let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs)));
     sysroot.load(cancellable)?;
@@ -620,8 +632,6 @@ async fn initialize_ostree_root_from_self(
     println!("Installed: {target_image}");
     println!("   Digest: {digest}");
 
-    // Write the entry for /boot to /etc/fstab.  TODO: Encourage OSes to use the karg?
-    // Or better bind this with the grub data.
     sysroot.load(cancellable)?;
     let deployment = sysroot
         .deployments()
@@ -633,28 +643,33 @@ async fn initialize_ostree_root_from_self(
     let root = rootfs_dir
         .open_dir(path.as_str())
         .context("Opening deployment dir")?;
-    let root_path = &rootfs.join(&path.as_str());
-    let mut f = {
-        let mut opts = cap_std::fs::OpenOptions::new();
-        root.open_with("etc/fstab", opts.append(true).write(true).create(true))
-            .context("Opening etc/fstab")
-            .map(BufWriter::new)?
-    };
-    if let Some(boot) = root_setup.boot.as_ref() {
-        writeln!(f, "{}", boot.to_fstab())?;
+
+    // And do another recursive relabeling pass over the entire physical root
+    // but avoid recursing into the deployment root (because that's a *distinct*
+    // logical root).
+    if let Some(policy) = sepolicy {
+        let deployment_root_meta = root.dir_metadata()?;
+        let deployment_root_devino = (deployment_root_meta.dev(), deployment_root_meta.ino());
+        let mut pathbuf = Utf8PathBuf::from("");
+        crate::lsm::ensure_dir_labeled_recurse(
+            rootfs_dir,
+            &mut pathbuf,
+            policy,
+            Some(deployment_root_devino),
+        )
+        .context("Recursive SELinux relabeling")?;
     }
-    f.flush()?;
 
-    let fstab_path = root_path.join("etc/fstab");
-    state.lsm_label(&fstab_path, "/etc/fstab".into(), false)?;
+    // Write the entry for /boot to /etc/fstab.  TODO: Encourage OSes to use the karg?
+    // Or better bind this with the grub data.
+    if let Some(boot) = root_setup.boot.as_ref() {
+        crate::lsm::atomic_replace_labeled(&root, "etc/fstab", 0o644.into(), sepolicy, |w| {
+            writeln!(w, "{}", boot.to_fstab()).map_err(Into::into)
+        })?;
+    }
 
     if let Some(contents) = state.root_ssh_authorized_keys.as_deref() {
-        osconfig::inject_root_ssh_authorized_keys(
-            &root,
-            &root_path,
-            |target, path, recurse| state.lsm_label(target, path, recurse),
-            contents,
-        )?;
+        osconfig::inject_root_ssh_authorized_keys(&root, sepolicy, contents)?;
     }
 
     let uname = rustix::system::uname();

lib/src/install/baseline.rs

@@ -195,6 +195,10 @@ pub(crate) fn install_create_rootfs(
         .transpose()
         .context("Parsing root size")?;
 
+    // Load the policy from the container root, which also must be our install root
+    let sepolicy = state.load_policy()?;
+    let sepolicy = sepolicy.as_ref();
+
     // Create a temporary directory to use for mount points.  Note that we're
     // in a mount namespace, so these should not be visible on the host.
     let rootfs = mntdir.join("rootfs");
@@ -368,15 +372,15 @@ pub(crate) fn install_create_rootfs(
         .collect::<Vec<_>>();
 
     mount::mount(&rootdev, &rootfs)?;
-    state.lsm_label(&rootfs, "/".into(), false)?;
+    let target_rootfs = Dir::open_ambient_dir(&rootfs, cap_std::ambient_authority())?;
+    crate::lsm::ensure_dir_labeled(&target_rootfs, "", Some("/".into()), 0o755.into(), sepolicy)?;
     let rootfs_fd = Dir::open_ambient_dir(&rootfs, cap_std::ambient_authority())?;
     let bootfs = rootfs.join("boot");
-    std::fs::create_dir(&bootfs).context("Creating /boot")?;
-    // The underlying directory on the root should be labeled
-    state.lsm_label(&bootfs, "/boot".into(), false)?;
+    // Create the underlying mount point directory, which should be labeled
+    crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;
     mount::mount(bootdev, &bootfs)?;
     // And we want to label the root mount of /boot
-    state.lsm_label(&bootfs, "/boot".into(), false)?;
+    crate::lsm::ensure_dir_labeled(&target_rootfs, "boot", None, 0o755.into(), sepolicy)?;
 
     // Create the EFI system partition, if applicable
     if let Some(esp_partno) = esp_partno {

lib/src/install/osconfig.rs

@@ -1,78 +1,51 @@
+use std::io::Write;
+
 use anyhow::Result;
-use camino::Utf8Path;
 use cap_std::fs::Dir;
-use cap_std_ext::{cap_std, dirext::CapStdExtDirExt};
+use cap_std_ext::cap_std;
 use fn_error_context::context;
+use ostree_ext::ostree;
 
 const ETC_TMPFILES: &str = "etc/tmpfiles.d";
 const ROOT_SSH_TMPFILE: &str = "bootc-root-ssh.conf";
 
 #[context("Injecting root authorized_keys")]
-pub(crate) fn inject_root_ssh_authorized_keys<F>(
+pub(crate) fn inject_root_ssh_authorized_keys(
     root: &Dir,
-    root_path: &Utf8Path,
-    lsm_label_fn: F,
+    sepolicy: Option<&ostree::SePolicy>,
     contents: &str,
-) -> Result<()>
-where
-    F: Fn(&Utf8Path, &Utf8Path, bool) -> Result<()>,
-{
+) -> Result<()> {
     // While not documented right now, this one looks like it does not newline wrap
     let b64_encoded = ostree_ext::glib::base64_encode(contents.as_bytes());
     // See the example in https://systemd.io/CREDENTIALS/
     let tmpfiles_content = format!("f~ /root/.ssh/authorized_keys 600 root root - {b64_encoded}\n");
 
-    let tmpfiles_dir = Utf8Path::new(ETC_TMPFILES);
-    root.create_dir_all(tmpfiles_dir)?;
-    let target = tmpfiles_dir.join(ROOT_SSH_TMPFILE);
-    root.atomic_write(&target, &tmpfiles_content)?;
-
-    let as_path = Utf8Path::new(ETC_TMPFILES).join(ROOT_SSH_TMPFILE);
-    lsm_label_fn(
-        &root_path.join(&as_path),
-        &Utf8Path::new("/").join(&as_path),
-        false,
+    crate::lsm::ensure_dir_labeled(root, ETC_TMPFILES, None, 0o755.into(), sepolicy)?;
+    let tmpfiles_dir = root.open_dir(ETC_TMPFILES)?;
+    crate::lsm::atomic_replace_labeled(
+        &tmpfiles_dir,
+        ROOT_SSH_TMPFILE,
+        0o644.into(),
+        sepolicy,
+        |w| w.write_all(tmpfiles_content.as_bytes()).map_err(Into::into),
     )?;
 
-    println!("Injected: {target}");
+    println!("Injected: {ETC_TMPFILES}/{ROOT_SSH_TMPFILE}");
     Ok(())
 }
 
 #[test]
 fn test_inject_root_ssh() -> Result<()> {
-    use camino::Utf8PathBuf;
-    use std::cell::Cell;
-
-    let fake_lsm_label_called = Cell::new(0);
-    let fake_lsm_label = |target: &Utf8Path, as_path: &Utf8Path, recurse: bool| -> Result<()> {
-        assert_eq!(
-            target,
-            format!("/root/path/etc/tmpfiles.d/{ROOT_SSH_TMPFILE}")
-        );
-        assert_eq!(as_path, format!("/etc/tmpfiles.d/{ROOT_SSH_TMPFILE}"));
-        assert_eq!(recurse, false);
-
-        fake_lsm_label_called.set(fake_lsm_label_called.get() + 1);
-        Ok(())
-    };
-
-    let root_path = &Utf8PathBuf::from("/root/path");
     let root = &cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
 
-    inject_root_ssh_authorized_keys(
-        root,
-        root_path,
-        fake_lsm_label,
-        "ssh-ed25519 ABCDE example@demo\n",
-    )
-    .unwrap();
+    // The code expects this to exist, reasonably so
+    root.create_dir("etc")?;
+    inject_root_ssh_authorized_keys(root, None, "ssh-ed25519 ABCDE example@demo\n").unwrap();
 
     let content = root.read_to_string(format!("etc/tmpfiles.d/{ROOT_SSH_TMPFILE}"))?;
     assert_eq!(
         content,
         "f~ /root/.ssh/authorized_keys 600 root root - c3NoLWVkMjU1MTkgQUJDREUgZXhhbXBsZUBkZW1vCg==\n"
     );
-    assert_eq!(fake_lsm_label_called, 1.into());
-
     Ok(())
 }