Merge pull request #420 from cgwalters/allow-no-selinux

install: Change no-SELinux -> SELinux to a warning && serialize to aleph

.github/workflows/ci.yml

@@ -143,6 +143,6 @@ jobs:
           # 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 \
+          sudo podman run --rm -ti --privileged --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 \
             ${image} bootc install to-existing-root
           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

lib/src/install.rs

@@ -288,9 +288,7 @@ pub(crate) struct SourceInfo {
 pub(crate) struct State {
     pub(crate) source: SourceInfo,
     /// Force SELinux off in target system
-    pub(crate) override_disable_selinux: bool,
-    #[allow(dead_code)]
-    pub(crate) setenforce_guard: Option<crate::lsm::SetEnforceGuard>,
+    pub(crate) selinux_state: SELinuxFinalState,
     #[allow(dead_code)]
     pub(crate) config_opts: InstallConfigOpts,
     pub(crate) target_imgref: ostree_container::OstreeImageReference,
@@ -303,7 +301,7 @@ impl State {
     #[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 {
+        if !self.selinux_state.enabled() {
             return Ok(None);
         }
         // We always use the physical container root to bootstrap policy
@@ -334,6 +332,8 @@ struct InstallAleph {
     timestamp: Option<chrono::DateTime<Utc>>,
     /// The `uname -r` of the kernel doing the installation
     kernel: String,
+    /// The state of SELinux at install time
+    selinux: String,
 }
 
 /// A mount specification is a subset of a line in `/etc/fstab`.
@@ -687,6 +687,7 @@ async fn initialize_ostree_root_from_self(
         version: imgstate.version().as_ref().map(|s| s.to_string()),
         timestamp,
         kernel: uname.release().to_str()?.to_string(),
+        selinux: state.selinux_state.to_aleph().to_string(),
     };
 
     Ok(aleph)
@@ -777,27 +778,53 @@ impl RootSetup {
     }
 }
 
+pub(crate) enum SELinuxFinalState {
+    /// Host and target both have SELinux, but user forced it off for target
+    ForceTargetDisabled,
+    /// Host and target both have SELinux
+    Enabled(Option<crate::lsm::SetEnforceGuard>),
+    /// Host has SELinux disabled, target is enabled.
+    HostDisabled,
+    /// Neither host or target have SELinux
+    Disabled,
+}
+
+impl SELinuxFinalState {
+    /// Returns true if the target system will have SELinux enabled.
+    pub(crate) fn enabled(&self) -> bool {
+        match self {
+            SELinuxFinalState::ForceTargetDisabled | SELinuxFinalState::Disabled => false,
+            SELinuxFinalState::Enabled(_) | SELinuxFinalState::HostDisabled => true,
+        }
+    }
+
+    /// Returns the canonical stringified version of self.  This is only used
+    /// for debugging purposes.
+    pub(crate) fn to_aleph(&self) -> &'static str {
+        match self {
+            SELinuxFinalState::ForceTargetDisabled => "force-target-disabled",
+            SELinuxFinalState::Enabled(_) => "enabled",
+            SELinuxFinalState::HostDisabled => "host-disabled",
+            SELinuxFinalState::Disabled => "disabled",
+        }
+    }
+}
+
 /// If we detect that the target ostree commit has SELinux labels,
 /// and we aren't passed an override to disable it, then ensure
 /// the running process is labeled with install_t so it can
 /// write arbitrary labels.
 pub(crate) fn reexecute_self_for_selinux_if_needed(
     srcdata: &SourceInfo,
     override_disable_selinux: bool,
-) -> Result<(bool, Option<crate::lsm::SetEnforceGuard>)> {
-    let mut ret_did_override = false;
+) -> Result<SELinuxFinalState> {
     // If the target state has SELinux enabled, we need to check the host state.
-    let mut g = None;
-    // We don't currently quite support installing SELinux enabled systems
-    // from SELinux disabled hosts, but this environment variable can be set
-    // to test it out anyways.
-    let skip_check_envvar = "BOOTC_SKIP_SELINUX_HOST_CHECK";
     if srcdata.selinux {
         let host_selinux = crate::lsm::selinux_enabled()?;
         tracing::debug!("Target has SELinux, host={host_selinux}");
-        if override_disable_selinux {
-            ret_did_override = true;
-            println!("notice: Target has SELinux enabled, overriding to disable")
+        let r = if override_disable_selinux {
+            println!("notice: Target has SELinux enabled, overriding to disable");
+            SELinuxFinalState::ForceTargetDisabled
         } else if host_selinux {
             // /sys/fs/selinuxfs is not normally mounted, so we do that now.
             // Because SELinux enablement status is cached process-wide and was very likely
@@ -806,21 +833,20 @@ pub(crate) fn reexecute_self_for_selinux_if_needed(
             // so let's just fall through to that.
             setup_sys_mount("selinuxfs", SELINUXFS)?;
             // This will re-execute the current process (once).
-            g = crate::lsm::selinux_ensure_install_or_setenforce()?;
-        } else if std::env::var_os(skip_check_envvar).is_some() {
-            eprintln!(
-                "Host kernel does not have SELinux support, but target enables it by default; {} is set, continuing anyways",
-                skip_check_envvar
-            );
+            let g = crate::lsm::selinux_ensure_install_or_setenforce()?;
+            SELinuxFinalState::Enabled(g)
         } else {
-            anyhow::bail!(
-                "Host kernel does not have SELinux support, but target enables it by default"
+            // This used to be a hard error, but is now a mild warning
+            crate::utils::medium_visibility_warning(
+                "Host kernel does not have SELinux support, but target enables it by default; this is less well tested.  See https://github.com/containers/bootc/issues/419",
             );
-        }
+            SELinuxFinalState::HostDisabled
+        };
+        Ok(r)
     } else {
         tracing::debug!("Target does not enable SELinux");
+        Ok(SELinuxFinalState::Disabled)
     }
-    Ok((ret_did_override, g))
 }
 
 /// Trim, flush outstanding writes, and freeze/thaw the target mounted filesystem;
@@ -1079,8 +1105,7 @@ async fn prepare_install(
     setup_sys_mount("efivarfs", EFIVARFS)?;
 
     // Now, deal with SELinux state.
-    let (override_disable_selinux, setenforce_guard) =
-        reexecute_self_for_selinux_if_needed(&source, config_opts.disable_selinux)?;
+    let selinux_state = reexecute_self_for_selinux_if_needed(&source, config_opts.disable_selinux)?;
 
     println!("Installing image: {:#}", &target_imgref);
     if let Some(digest) = source.digest.as_deref() {
@@ -1102,8 +1127,7 @@ async fn prepare_install(
     // so we can pass it to worker threads too. Right now this just
     // combines our command line options along with some bind mounts from the host.
     let state = Arc::new(State {
-        override_disable_selinux,
-        setenforce_guard,
+        selinux_state,
         source,
         config_opts,
         target_imgref,
@@ -1115,7 +1139,7 @@ async fn prepare_install(
 }
 
 async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Result<()> {
-    if state.override_disable_selinux {
+    if matches!(state.selinux_state, SELinuxFinalState::ForceTargetDisabled) {
         rootfs.kargs.push("selinux=0".to_string());
     }
 
@@ -1218,6 +1242,20 @@ pub(crate) async fn install_to_disk(mut opts: InstallToDiskOpts) -> Result<()> {
         loopback_dev.close()?;
     }
 
+    // At this point, all other threads should be gone.
+    if let Some(state) = Arc::into_inner(state) {
+        // If we had invoked `setenforce 0`, then let's re-enable it.
+        match state.selinux_state {
+            SELinuxFinalState::Enabled(Some(guard)) => {
+                guard.consume()?;
+            }
+            _ => {}
+        }
+    } else {
+        // This shouldn't happen...but we will make it not fatal right now
+        tracing::warn!("Failed to consume state Arc");
+    }
+
     installation_complete();
 
     Ok(())

lib/src/lsm.rs

@@ -99,12 +99,29 @@ pub(crate) fn selinux_ensure_install() -> Result<bool> {
 /// gain the `mac_admin` permission (install_t).
 #[cfg(feature = "install")]
 #[must_use]
-pub(crate) struct SetEnforceGuard;
+pub(crate) struct SetEnforceGuard(Option<()>);
+
+#[cfg(feature = "install")]
+impl SetEnforceGuard {
+    pub(crate) fn new() -> Self {
+        SetEnforceGuard(Some(()))
+    }
+
+    pub(crate) fn consume(mut self) -> Result<()> {
+        // SAFETY: The option cannot have been consumed until now
+        self.0.take().unwrap();
+        // This returns errors
+        selinux_set_permissive(false)
+    }
+}
 
 #[cfg(feature = "install")]
 impl Drop for SetEnforceGuard {
     fn drop(&mut self) {
-        let _ = selinux_set_permissive(false);
+        // A best-effort attempt to re-enable enforcement on drop (installation failure)
+        if let Some(()) = self.0.take() {
+            let _ = selinux_set_permissive(false);
+        }
     }
 }
 
@@ -121,7 +138,7 @@ pub(crate) fn selinux_ensure_install_or_setenforce() -> Result<Option<SetEnforce
     let g = if std::env::var_os("BOOTC_SETENFORCE0_FALLBACK").is_some() {
         tracing::warn!("Failed to enter install_t; temporarily setting permissive mode");
         selinux_set_permissive(true)?;
-        Some(SetEnforceGuard)
+        Some(SetEnforceGuard::new())
     } else {
         let current = get_current_security_context()?;
         anyhow::bail!("Failed to enter install_t (running as {current}) - use BOOTC_SETENFORCE0_FALLBACK=1 to override");