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: Guide user towards the correct podman flags #953

Merged
merged 1 commit into from
Dec 9, 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
32 changes: 22 additions & 10 deletions lib/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -576,15 +576,27 @@ pub(crate) async fn get_storage() -> Result<crate::store::Storage> {
}

#[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(())
}

Expand Down Expand Up @@ -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");
Expand Down
20 changes: 10 additions & 10 deletions lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand All @@ -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(())
}
Expand Down Expand Up @@ -1154,18 +1152,17 @@ async fn prepare_install(
target_opts: InstallTargetOpts,
) -> Result<Arc<State>> {
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 /")?;

let host_is_container = crate::containerenv::is_container(&rootfs);
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).
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions lib/src/install/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-blocking: One larger cleanup I think we could do is have an Environment struct that gathers global state like this (uid, whether we have cap_sys_admin, whether /run/ostree-booted exists, whether we appear to be in a container), and then query requirements on it.

e.g. here we're passing false for is_container but we don't actually know that's false or not.

Not a blocker, just an optional followup.

// Load sysroot from the provided path
let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(sysroot)));
sysroot.load(gio::Cancellable::NONE)?;
Expand Down
Loading