Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Mar 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
96 changes: 67 additions & 29 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>),
cgwalters marked this conversation as resolved.
Show resolved Hide resolved
/// 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
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand All @@ -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,
Expand All @@ -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());
}

Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clippy prefers if let here since it's only destructuring a single pattern... but there's also other preexisting things it's complaining about, so I'll just let a few accumulate and do a separate PR to clean them up all at once.

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(())
Expand Down
23 changes: 20 additions & 3 deletions lib/src/lsm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand All @@ -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");
Expand Down