From ba9cfb6a276df439b57ddbbc9816073d9d783f98 Mon Sep 17 00:00:00 2001 From: Omer Tuchfeld Date: Mon, 9 Dec 2024 17:04:09 +0100 Subject: [PATCH] install: Guide user towards the correct podman flags Modified the error / root checking code a bit to better guide the user towards the correct bootc invocation. Issue BIFROST-552 [1] ``` [omer@hal9000 ~]$ podman run -it quay.io/otuchfel/bootc:comfy bootc install to-existing-root ERROR Installing to filesystem: Querying root privilege: The container must be executed with full privileges (e.g. --privileged flag) [omer@hal9000 ~]$ podman run -it --privileged quay.io/otuchfel/bootc:comfy bootc install to-existing-root ERROR Installing to filesystem: This command must be run with the podman --pid=host flag [omer@hal9000 ~]$ podman run -it --pid=host --privileged quay.io/otuchfel/bootc:comfy bootc install to-existing-root ERROR Installing to filesystem: /proc/1 is owned by 65534, not zero; this command must be run in the root user namespace (e.g. not rootless podman) [omer@hal9000 ~]$ sudo podman run -it --privileged --pid=host quay.io/otuchfel/bootc:comfy bootc install to-existing-root Installing image: docker://quay.io/otuchfel/bootc:comfy ... ``` [1] https://issues.redhat.com/browse/BIFROST-552 Signed-off-by: Omer Tuchfeld --- lib/src/cli.rs | 32 ++++++++++++++++++++++---------- lib/src/install.rs | 20 ++++++++++---------- lib/src/install/completion.rs | 4 ++-- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index d1cf0d9c..bd896f4f 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -7,7 +7,7 @@ use std::io::Seek; use std::os::unix::process::CommandExt; use std::process::Command; -use anyhow::{Context, Result}; +use anyhow::{ensure, Context, Result}; use camino::Utf8PathBuf; use cap_std_ext::cap_std; use cap_std_ext::cap_std::fs::Dir; @@ -576,15 +576,27 @@ pub(crate) async fn get_storage() -> Result { } #[context("Querying root privilege")] -pub(crate) fn require_root() -> Result<()> { - let uid = rustix::process::getuid(); - if !uid.is_root() { - anyhow::bail!("This command requires root privileges"); - } - if !rustix::thread::capability_is_in_bounding_set(rustix::thread::Capability::SystemAdmin)? { - anyhow::bail!("This command requires full root privileges (CAP_SYS_ADMIN)"); - } +pub(crate) fn require_root(is_container: bool) -> Result<()> { + ensure!( + rustix::process::getuid().is_root(), + if is_container { + "The user inside the container from which you are running this command must be root" + } else { + "This command must be executed as the root user" + } + ); + + ensure!( + rustix::thread::capability_is_in_bounding_set(rustix::thread::Capability::SystemAdmin)?, + if is_container { + "The container must be executed with full privileges (e.g. --privileged flag)" + } else { + "This command requires full root privileges (CAP_SYS_ADMIN)" + } + ); + tracing::trace!("Verified uid 0 with CAP_SYS_ADMIN"); + Ok(()) } @@ -616,7 +628,7 @@ fn prepare_for_write() -> Result<()> { ostree_booted()?, "This command requires an ostree-booted host system" ); - crate::cli::require_root()?; + crate::cli::require_root(false)?; ensure_self_unshared_mount_namespace()?; if crate::lsm::selinux_enabled()? && !crate::lsm::selinux_ensure_install()? { tracing::warn!("Do not have install_t capabilities"); diff --git a/lib/src/install.rs b/lib/src/install.rs index 368794ef..60814683 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -1003,7 +1003,7 @@ pub(crate) fn finalize_filesystem( /// A heuristic check that we were invoked with --pid=host fn require_host_pidns() -> Result<()> { if rustix::process::getpid().is_init() { - anyhow::bail!("This command must be run with --pid=host") + anyhow::bail!("This command must be run with the podman --pid=host flag") } tracing::trace!("OK: we're not pid 1"); Ok(()) @@ -1019,9 +1019,7 @@ fn require_host_userns() -> Result<()> { .uid(); // We must really be in a rootless container, or in some way // we're not part of the host user namespace. - if pid1_uid != 0 { - anyhow::bail!("{proc1} is owned by {pid1_uid}, not zero; this command must be run in the root user namespace (e.g. not rootless podman)"); - } + ensure!(pid1_uid == 0, "{proc1} is owned by {pid1_uid}, not zero; this command must be run in the root user namespace (e.g. not rootless podman)"); tracing::trace!("OK: we're in a matching user namespace with pid1"); Ok(()) } @@ -1154,8 +1152,6 @@ async fn prepare_install( target_opts: InstallTargetOpts, ) -> Result> { tracing::trace!("Preparing install"); - // We need full root privileges, i.e. --privileged in podman - crate::cli::require_root()?; let rootfs = cap_std::fs::Dir::open_ambient_dir("/", cap_std::ambient_authority()) .context("Opening /")?; @@ -1163,9 +1159,10 @@ async fn prepare_install( let external_source = source_opts.source_imgref.is_some(); let source = match source_opts.source_imgref { None => { - if !host_is_container { - anyhow::bail!("Either --source-imgref must be defined or this command must be executed inside a podman container.") - } + ensure!(host_is_container, "Either --source-imgref must be defined or this command must be executed inside a podman container."); + + crate::cli::require_root(true)?; + require_host_pidns()?; // Out of conservatism we only verify the host userns path when we're expecting // to do a self-install (e.g. not bootc-image-builder or equivalent). @@ -1187,7 +1184,10 @@ async fn prepare_install( SourceInfo::from_container(&rootfs, &container_info)? } - Some(source) => SourceInfo::from_imageref(&source, &rootfs)?, + Some(source) => { + crate::cli::require_root(false)?; + SourceInfo::from_imageref(&source, &rootfs)? + } }; // Parse the target CLI image reference options and create the *target* image diff --git a/lib/src/install/completion.rs b/lib/src/install/completion.rs index 1cc33345..e37d2865 100644 --- a/lib/src/install/completion.rs +++ b/lib/src/install/completion.rs @@ -197,7 +197,7 @@ pub(crate) async fn run_from_anaconda(rootfs: &Dir) -> Result<()> { // unshare our mount namespace, so any *further* mounts aren't leaked. // Note that because this does a re-exec, anything *before* this point // should be idempotent. - crate::cli::require_root()?; + crate::cli::require_root(false)?; crate::cli::ensure_self_unshared_mount_namespace()?; if std::env::var_os(ANACONDA_ENV_HINT).is_none() { @@ -245,7 +245,7 @@ pub(crate) async fn run_from_anaconda(rootfs: &Dir) -> Result<()> { /// From ostree-rs-ext, run through the rest of bootc install functionality pub async fn run_from_ostree(rootfs: &Dir, sysroot: &Utf8Path, stateroot: &str) -> Result<()> { - crate::cli::require_root()?; + crate::cli::require_root(false)?; // Load sysroot from the provided path let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(sysroot))); sysroot.load(gio::Cancellable::NONE)?;