From c53f7bfb85abc0313f879739285980ad65a80341 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 15 May 2025 14:47:19 -0400 Subject: [PATCH] Dedup sepolicy handling For historical reasons the ostree sepolicy API can exist as a no-op even if it didn't find a policy, one has to query `.csum()` or `.name()` to verify it's present. In our code just map that case to None. Followup to 99d30dfd556866091c558559810647bdde4e1ee1 to ensure we consistently handle this case. Signed-off-by: Colin Walters --- lib/src/install.rs | 8 +++----- lib/src/install/completion.rs | 6 ++---- lib/src/lsm.rs | 13 +++++++++++++ lib/src/store/mod.rs | 17 ++++++----------- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index 395293c19..6fc9a0ef6 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -385,16 +385,14 @@ pub(crate) struct State { impl State { #[context("Loading SELinux policy")] pub(crate) fn load_policy(&self) -> Result> { - use std::os::fd::AsRawFd; if !self.selinux_state.enabled() { return Ok(None); } // We always use the physical container root to bootstrap policy - let r = ostree::SePolicy::new_at(self.container_root.as_raw_fd(), gio::Cancellable::NONE)?; - let csum = r - .csum() + let r = lsm::new_sepolicy_at(&self.container_root)? .ok_or_else(|| anyhow::anyhow!("SELinux enabled, but no policy found in root"))?; - tracing::debug!("Loaded SELinux policy: {csum}"); + // SAFETY: Policy must have a checksum here + tracing::debug!("Loaded SELinux policy: {}", r.csum().unwrap()); Ok(Some(r)) } diff --git a/lib/src/install/completion.rs b/lib/src/install/completion.rs index 9aab012c8..6c1dd9e2d 100644 --- a/lib/src/install/completion.rs +++ b/lib/src/install/completion.rs @@ -13,7 +13,6 @@ 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; @@ -294,13 +293,12 @@ pub(crate) async fn impl_completion( 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)?; + let sepolicy = crate::lsm::new_sepolicy_at(deployment_fd)?; // 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, Some(sepolicy))?; + &crate::imgstorage::Storage::create(&sysroot_dir, &rundir, sepolicy.as_ref())?; crate::boundimage::pull_images_impl(imgstorage, bound_images) .await .context("pulling bound images")?; diff --git a/lib/src/lsm.rs b/lib/src/lsm.rs index a3b76482f..ca33fa044 100644 --- a/lib/src/lsm.rs +++ b/lib/src/lsm.rs @@ -169,6 +169,19 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result Result> { + let fd = fd.as_fd(); + let cancellable = gio::Cancellable::NONE; + let sepolicy = ostree::SePolicy::new_at(fd.as_raw_fd(), cancellable)?; + let r = if sepolicy.csum().is_none() { + None + } else { + Some(sepolicy) + }; + Ok(r) +} + #[context("Setting SELinux permissive mode")] #[allow(dead_code)] pub(crate) fn selinux_set_permissive(permissive: bool) -> Result<()> { diff --git a/lib/src/store/mod.rs b/lib/src/store/mod.rs index 62cb1c1dc..9c52a7b4f 100644 --- a/lib/src/store/mod.rs +++ b/lib/src/store/mod.rs @@ -8,13 +8,13 @@ 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::lsm; use crate::spec::ImageStatus; use crate::utils::deployment_fd; @@ -94,25 +94,20 @@ impl Storage { // this should only happen during cleanup of a broken install tracing::trace!("falling back to container root's selinux policy"); let container_root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; - &ostree::SePolicy::new_at(container_root.as_raw_fd(), gio::Cancellable::NONE)? + lsm::new_sepolicy_at(&container_root)? } else { // load the sepolicy from the booted ostree deployment so the imgstorage can be // properly labeled with /var/lib/container/storage labels tracing::trace!("loading sepolicy from booted ostree deployment"); let dep = self.sysroot.booted_deployment().unwrap(); let dep_fs = deployment_fd(&self.sysroot, &dep)?; - &ostree::SePolicy::new_at(dep_fs.as_raw_fd(), gio::Cancellable::NONE)? - }; - - let sepolicy = if sepolicy.csum().is_none() { - None - } else { - Some(sepolicy) + lsm::new_sepolicy_at(&dep_fs)? }; tracing::trace!("sepolicy in get_ensure_imgstore: {sepolicy:?}"); - let imgstore = crate::imgstorage::Storage::create(&sysroot_dir, &self.run, sepolicy)?; + let imgstore = + crate::imgstorage::Storage::create(&sysroot_dir, &self.run, sepolicy.as_ref())?; Ok(self.imgstore.get_or_init(|| imgstore)) }