From f45f0f0350e629268ea17fa0315313bf01e20d8e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 24 Jan 2024 17:16:13 -0500 Subject: [PATCH 1/3] lsm: Make setenforce 0 fallback require `BOOTC_SETENFORCE0_FALLBACK` We shouldn't perform global system mutation without an opt-in. As painful as it is. Signed-off-by: Colin Walters --- lib/src/lsm.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/src/lsm.rs b/lib/src/lsm.rs index e70994cf4..740d32efe 100644 --- a/lib/src/lsm.rs +++ b/lib/src/lsm.rs @@ -105,9 +105,13 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result Date: Wed, 24 Jan 2024 18:26:37 -0500 Subject: [PATCH 2/3] lsm: Test if we have install_t capability Hardcoding `install_t` is a bit ugly; maybe at some point things change so that `spc_t` has `install_t` privileges. Let's do a runtime check if we can set an invalid label; if so then we're good. Signed-off-by: Colin Walters --- lib/src/cli.rs | 4 +++- lib/src/lsm.rs | 40 ++++++++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 520c6d511..da4441bab 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -293,7 +293,9 @@ pub(crate) async fn prepare_for_write() -> Result<()> { } ensure_self_unshared_mount_namespace().await?; if crate::lsm::selinux_enabled()? { - crate::lsm::selinux_ensure_install()?; + if !crate::lsm::selinux_ensure_install()? { + tracing::warn!("Do not have install_t capabilities"); + } } Ok(()) } diff --git a/lib/src/lsm.rs b/lib/src/lsm.rs index 740d32efe..2760e146f 100644 --- a/lib/src/lsm.rs +++ b/lib/src/lsm.rs @@ -41,8 +41,18 @@ fn context_is_install_t(context: &str) -> bool { context.contains(":install_t:") } +#[context("Testing install_t")] +fn test_install_t() -> Result { + let tmpf = tempfile::NamedTempFile::new()?; + let st = Command::new("chcon") + .args(["-t", "invalid_bootcinstall_testlabel_t"]) + .arg(tmpf.path()) + .status()?; + Ok(st.success()) +} + #[context("Ensuring selinux install_t type")] -pub(crate) fn selinux_ensure_install() -> Result<()> { +pub(crate) fn selinux_ensure_install() -> Result { let guardenv = "_bootc_selinuxfs_mounted"; let current = get_current_security_context()?; tracing::debug!("Current security context is {current}"); @@ -54,9 +64,13 @@ pub(crate) fn selinux_ensure_install() -> Result<()> { } else { tracing::debug!("Assuming we now have a privileged (e.g. install_t) label"); } - return Ok(()); + return test_install_t(); + } + if test_install_t()? { + tracing::debug!("We have install_t"); + return Ok(true); } - tracing::debug!("Copying self to temporary file for re-exec"); + tracing::debug!("Lacking install_t capabilities; copying self to temporary file for re-exec"); // OK now, we always copy our binary to a tempfile, set its security context // to match that of /usr/bin/ostree, and then re-exec. This is really a gross // hack; we can't always rely on https://github.com/fedora-selinux/selinux-policy/pull/1500/commits/67eb283c46d35a722636d749e5b339615fe5e7f5 @@ -102,18 +116,16 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result Date: Wed, 24 Jan 2024 21:12:27 -0500 Subject: [PATCH 3/3] lsm: Make a not-`nosuid` `/tmp` This was the thing that was breaking our `unconfined_t` -> `install_t` transition; the host `/tmp` is `nosuid`. It simplifies things here to just make our own, so do that. Signed-off-by: Colin Walters --- lib/src/install.rs | 21 +++++++++++++++++---- lib/src/lsm.rs | 13 +------------ 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/src/install.rs b/lib/src/install.rs index dfaa2634e..fa97452c4 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -873,9 +873,22 @@ fn ensure_var() -> Result<()> { /// We can't bind mount though - we need to symlink it so that each calling process /// will traverse the link. #[context("Linking tmp mounts to host")] -pub(crate) fn propagate_tmp_mounts_to_host() -> Result<()> { - // Point our /tmp and /var/tmp at the host, via the /proc/1/root magic link - for path in ["/tmp", "/var/tmp"].map(Utf8Path::new) { +pub(crate) fn setup_tmp_mounts() -> Result<()> { + let st = rustix::fs::statfs("/tmp")?; + if st.f_type == libc::TMPFS_MAGIC { + tracing::trace!("Already have tmpfs /tmp") + } else { + // Note we explicitly also don't want a "nosuid" tmp, because that + // suppresses our install_t transition + Task::new_and_run( + "Mounting tmpfs /tmp", + "mount", + ["tmpfs", "-t", "tmpfs", "/tmp"], + )?; + } + + // Point our /var/tmp at the host, via the /proc/1/root magic link + for path in ["/var/tmp"].map(Utf8Path::new) { if path.try_exists()? { let st = rustix::fs::statfs(path.as_std_path()).context(path)?; if st.f_type != libc::OVERLAYFS_SUPER_MAGIC { @@ -999,7 +1012,7 @@ async fn prepare_install( } ensure_var()?; - propagate_tmp_mounts_to_host()?; + setup_tmp_mounts()?; // Even though we require running in a container, the mounts we create should be specific // to this process, so let's enter a private mountns to avoid leaking them. diff --git a/lib/src/lsm.rs b/lib/src/lsm.rs index 2760e146f..f2271e762 100644 --- a/lib/src/lsm.rs +++ b/lib/src/lsm.rs @@ -33,14 +33,6 @@ fn get_current_security_context() -> Result { std::fs::read_to_string(SELF_CURRENT).with_context(|| format!("Reading {SELF_CURRENT}")) } -/// Determine if a security context is the "install_t" type which can -/// write arbitrary labels. -fn context_is_install_t(context: &str) -> bool { - // TODO: we shouldn't actually hardcode this...it's just ugly though - // to figure out whether we really can gain CAP_MAC_ADMIN. - context.contains(":install_t:") -} - #[context("Testing install_t")] fn test_install_t() -> Result { let tmpf = tempfile::NamedTempFile::new()?; @@ -112,10 +104,6 @@ impl Drop for SetEnforceGuard { #[cfg(feature = "install")] pub(crate) fn selinux_ensure_install_or_setenforce() -> Result> { // If the process already has install_t, exit early - let current = get_current_security_context()?; - if context_is_install_t(¤t) { - return Ok(None); - } // Note that this may re-exec the entire process if selinux_ensure_install()? { return Ok(None); @@ -125,6 +113,7 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result