From abc8ed660843b3ad85e72c0002996dabd61778dd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 19 Jul 2023 17:34:48 -0400 Subject: [PATCH] repair: New functionality to detect (future: fix) inodes Initial code to detect the situation resulting from https://github.com/ostreedev/ostree/pull/2874/commits/de6fddc6adee09a93901243dc7074090828a1912 --- lib/src/cli.rs | 73 ++++++++++++ lib/src/container/store.rs | 226 +++++++++++++++++++++++++++++++++++- lib/src/diff.rs | 2 +- lib/src/lib.rs | 2 + lib/src/repair.rs | 227 +++++++++++++++++++++++++++++++++++++ 5 files changed, 527 insertions(+), 3 deletions(-) create mode 100644 lib/src/repair.rs diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 62a56cad..be17ac4a 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -8,10 +8,12 @@ use anyhow::{Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; use clap::{Parser, Subcommand}; +use fn_error_context::context; use ostree::{cap_std, gio, glib}; use std::collections::BTreeMap; use std::ffi::OsString; use std::path::PathBuf; +use std::process::Command; use tokio::sync::mpsc::Receiver; use crate::commit::container_commit; @@ -345,6 +347,34 @@ pub(crate) enum ContainerImageOpts { }, } +/// Options for deployment repair. +#[derive(Debug, Subcommand)] +pub(crate) enum ProvisionalRepairOpts { + AnalyzeInodes { + /// Path to the repository + #[clap(long, value_parser)] + repo: Utf8PathBuf, + + /// Print additional information + #[clap(long)] + verbose: bool, + }, + + Repair { + /// Path to the sysroot + #[clap(long, value_parser)] + sysroot: Utf8PathBuf, + + /// Do not mutate any system state + #[clap(long)] + dry_run: bool, + + /// Print additional information + #[clap(long)] + verbose: bool, + }, +} + /// Options for the Integrity Measurement Architecture (IMA). #[derive(Debug, Parser)] pub(crate) struct ImaSignOpts { @@ -410,6 +440,8 @@ pub(crate) enum Opt { #[clap(hide(true))] #[cfg(feature = "docgen")] Man(ManOpts), + #[clap(hide = true, subcommand)] + ProvisionalRepair(ProvisionalRepairOpts), } #[allow(clippy::from_over_into)] @@ -739,6 +771,22 @@ async fn testing(opts: &TestingOpts) -> Result<()> { } } +// Quick hack; TODO dedup this with the code in bootc or lower here +#[context("Remounting sysroot writable")] +fn container_remount_sysroot(sysroot: &Utf8Path) -> Result<()> { + if !Utf8Path::new("/run/.containerenv").exists() { + return Ok(()); + } + println!("Running in container, assuming we can remount {sysroot} writable"); + let st = Command::new("mount") + .args(["-o", "remount,rw", sysroot.as_str()]) + .status()?; + if !st.success() { + anyhow::bail!("Failed to remount {sysroot}: {st:?}"); + } + Ok(()) +} + /// Parse the provided arguments and execute. /// Calls [`structopt::clap::Error::exit`] on failure, printing the error message and aborting the program. pub async fn run_from_iter(args: I) -> Result<()> @@ -978,5 +1026,30 @@ where Opt::InternalOnlyForTesting(ref opts) => testing(opts).await, #[cfg(feature = "docgen")] Opt::Man(manopts) => crate::docgen::generate_manpages(&manopts.directory), + Opt::ProvisionalRepair(opts) => match opts { + ProvisionalRepairOpts::AnalyzeInodes { repo, verbose } => { + let repo = parse_repo(&repo)?; + match crate::repair::check_inode_collision(&repo, verbose)? { + crate::repair::InodeCheckResult::Okay => { + println!("OK: No colliding objects found."); + } + crate::repair::InodeCheckResult::PotentialCorruption(n) => { + eprintln!("warning: {} potentially colliding inodes found", n.len()); + } + } + Ok(()) + } + ProvisionalRepairOpts::Repair { + sysroot, + verbose, + dry_run, + } => { + container_remount_sysroot(&sysroot)?; + let sysroot = &ostree::Sysroot::new(Some(&gio::File::for_path(&sysroot))); + sysroot.load(gio::Cancellable::NONE)?; + let sysroot = &SysrootLock::new_from_sysroot(sysroot).await?; + crate::repair::auto_repair_inode_collision(sysroot, dry_run, verbose) + } + }, } } diff --git a/lib/src/container/store.rs b/lib/src/container/store.rs index 3b72c29a..913b0167 100644 --- a/lib/src/container/store.rs +++ b/lib/src/container/store.rs @@ -8,14 +8,17 @@ use super::*; use crate::logging::system_repo_journal_print; use crate::refescape; +use crate::sysroot::SysrootLock; use crate::utils::ResultExt; use anyhow::{anyhow, Context}; +use camino::{Utf8Path, Utf8PathBuf}; use containers_image_proxy::{ImageProxy, OpenedImage}; use fn_error_context::context; use futures_util::TryFutureExt; use oci_spec::image::{self as oci_image, Descriptor, History, ImageConfiguration, ImageManifest}; -use ostree::prelude::{Cast, ToVariant}; +use ostree::prelude::{Cast, FileEnumeratorExt, FileExt, ToVariant}; use ostree::{gio, glib}; +use rustix::fs::MetadataExt; use std::collections::{BTreeSet, HashMap}; use std::iter::FromIterator; use tokio::sync::mpsc::{Receiver, Sender}; @@ -37,7 +40,7 @@ const IMAGE_PREFIX: &str = "ostree/container/image"; pub const BASE_IMAGE_PREFIX: &str = "ostree/container/baseimage"; /// The key injected into the merge commit for the manifest digest. -const META_MANIFEST_DIGEST: &str = "ostree.manifest-digest"; +pub(crate) const META_MANIFEST_DIGEST: &str = "ostree.manifest-digest"; /// The key injected into the merge commit with the manifest serialized as JSON. const META_MANIFEST: &str = "ostree.manifest"; /// The key injected into the merge commit with the image configuration serialized as JSON. @@ -1262,3 +1265,222 @@ pub fn remove_images<'a>( } Ok(()) } + +#[derive(Debug, Default)] +struct CompareState { + verified: BTreeSet, + inode_corrupted: BTreeSet, + unknown_corrupted: BTreeSet, +} + +impl CompareState { + fn is_ok(&self) -> bool { + self.inode_corrupted.is_empty() && self.unknown_corrupted.is_empty() + } +} + +fn compare_file_info(src: &gio::FileInfo, target: &gio::FileInfo) -> bool { + if src.file_type() != target.file_type() { + return false; + } + if src.size() != target.size() { + return false; + } + for attr in ["unix::uid", "unix::gid", "unix::mode"] { + if src.attribute_uint32(attr) != target.attribute_uint32(attr) { + return false; + } + } + true +} + +#[context("Querying object inode")] +fn inode_of_object(repo: &ostree::Repo, checksum: &str) -> Result { + let repodir = repo.dfd_as_dir()?; + let (prefix, suffix) = checksum.split_at(2); + let objpath = format!("objects/{}/{}.file", prefix, suffix); + let metadata = repodir.symlink_metadata(objpath)?; + Ok(metadata.ino()) +} + +fn compare_commit_trees( + repo: &ostree::Repo, + root: &Utf8Path, + target: &ostree::RepoFile, + expected: &ostree::RepoFile, + exact: bool, + colliding_inodes: &BTreeSet, + state: &mut CompareState, +) -> Result<()> { + let cancellable = gio::Cancellable::NONE; + let queryattrs = "standard::name,standard::type"; + let queryflags = gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS; + let expected_iter = expected.enumerate_children(queryattrs, queryflags, cancellable)?; + + while let Some(expected_info) = expected_iter.next_file(cancellable)? { + let expected_child = expected_iter.child(&expected_info); + let name = expected_info.name(); + let name = name.to_str().expect("UTF-8 ostree name"); + let path = Utf8PathBuf::from(format!("{root}{name}")); + let target_child = target.child(name); + let target_info = crate::diff::query_info_optional(&target_child, queryattrs, queryflags) + .context("querying optional to")?; + let is_dir = matches!(expected_info.file_type(), gio::FileType::Directory); + if let Some(target_info) = target_info { + let to_child = target_child + .downcast::() + .expect("downcast"); + to_child.ensure_resolved()?; + let from_child = expected_child + .downcast::() + .expect("downcast"); + from_child.ensure_resolved()?; + + if is_dir { + let from_contents_checksum = from_child.tree_get_contents_checksum(); + let to_contents_checksum = to_child.tree_get_contents_checksum(); + if from_contents_checksum != to_contents_checksum { + let subpath = Utf8PathBuf::from(format!("{}/", path)); + compare_commit_trees( + repo, + &subpath, + &from_child, + &to_child, + exact, + colliding_inodes, + state, + )?; + } + } else { + let from_checksum = from_child.checksum(); + let to_checksum = to_child.checksum(); + let matches = if exact { + from_checksum == to_checksum + } else { + compare_file_info(&target_info, &expected_info) + }; + if !matches { + let from_inode = inode_of_object(repo, &from_checksum)?; + let to_inode = inode_of_object(repo, &to_checksum)?; + if colliding_inodes.contains(&from_inode) + || colliding_inodes.contains(&to_inode) + { + state.inode_corrupted.insert(path); + } else { + state.unknown_corrupted.insert(path); + } + } else { + state.verified.insert(path); + } + } + } else { + eprintln!("Missing {path}"); + state.unknown_corrupted.insert(path); + } + } + Ok(()) +} + +#[context("Verifying container image state")] +pub(crate) fn verify_container_image( + sysroot: &SysrootLock, + imgref: &ImageReference, + colliding_inodes: &BTreeSet, + verbose: bool, +) -> Result { + let cancellable = gio::Cancellable::NONE; + let repo = &sysroot.repo(); + let state = + query_image_ref(repo, imgref)?.ok_or_else(|| anyhow!("Expected present image {imgref}"))?; + let merge_commit = state.merge_commit.as_str(); + let merge_commit_root = repo.read_commit(merge_commit, gio::Cancellable::NONE)?.0; + let merge_commit_root = merge_commit_root + .downcast::() + .expect("downcast"); + merge_commit_root.ensure_resolved()?; + + // This shouldn't happen anymore + let config = state + .configuration + .ok_or_else(|| anyhow!("Missing configuration for image {imgref}"))?; + let (commit_layer, _component_layers, remaining_layers) = + parse_manifest_layout(&state.manifest, &config)?; + + let mut comparison_state = CompareState::default(); + + let query = |l: &Descriptor| query_layer(repo, l.clone()); + + let base_tree = repo + .read_commit(&state.base_commit, cancellable)? + .0 + .downcast::() + .expect("downcast"); + println!( + "Verifying with base ostree layer {}", + ref_for_layer(commit_layer)? + ); + compare_commit_trees( + repo, + "/".into(), + &merge_commit_root, + &base_tree, + true, + colliding_inodes, + &mut comparison_state, + )?; + + let remaining_layers = remaining_layers + .into_iter() + .map(query) + .collect::>>()?; + + println!("Image has {} derived layers", remaining_layers.len()); + + for layer in remaining_layers.iter().rev() { + let layer_ref = layer.ostree_ref.as_str(); + let layer_commit = layer + .commit + .as_deref() + .ok_or_else(|| anyhow!("Missing layer {layer_ref}"))?; + let layer_tree = repo + .read_commit(layer_commit, cancellable)? + .0 + .downcast::() + .expect("downcast"); + compare_commit_trees( + repo, + "/".into(), + &merge_commit_root, + &layer_tree, + false, + colliding_inodes, + &mut comparison_state, + )?; + } + + let n_verified = comparison_state.verified.len(); + if comparison_state.is_ok() { + println!("OK image {imgref} (verified={n_verified})"); + println!(); + } else { + let n_inode = comparison_state.inode_corrupted.len(); + let n_other = comparison_state.unknown_corrupted.len(); + eprintln!("warning: Found corrupted merge commit"); + eprintln!(" inode clashes: {n_inode}"); + eprintln!(" unknown: {n_other}"); + eprintln!(" ok: {n_verified}"); + if verbose { + eprintln!("Mismatches:"); + for path in comparison_state.inode_corrupted { + eprintln!(" inode: {path}"); + } + for path in comparison_state.unknown_corrupted { + eprintln!(" other: {path}"); + } + } + eprintln!(); + return Ok(false); + } + + Ok(true) +} diff --git a/lib/src/diff.rs b/lib/src/diff.rs index a66c17a5..655adc38 100644 --- a/lib/src/diff.rs +++ b/lib/src/diff.rs @@ -14,7 +14,7 @@ use std::collections::BTreeSet; use std::fmt; /// Like `g_file_query_info()`, but return None if the target doesn't exist. -fn query_info_optional( +pub(crate) fn query_info_optional( f: &gio::File, queryattrs: &str, queryflags: gio::FileQueryInfoFlags, diff --git a/lib/src/lib.rs b/lib/src/lib.rs index c9a424b3..71956400 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -39,6 +39,8 @@ pub mod ima; pub mod keyfileext; pub(crate) mod logging; pub mod refescape; +#[doc(hidden)] +pub mod repair; pub mod sysroot; pub mod tar; pub mod tokio_util; diff --git a/lib/src/repair.rs b/lib/src/repair.rs new file mode 100644 index 00000000..5adddf45 --- /dev/null +++ b/lib/src/repair.rs @@ -0,0 +1,227 @@ +//! System repair functionality + +use std::{ + collections::{BTreeMap, BTreeSet}, + process::Command, +}; + +use anyhow::{anyhow, Context, Result}; +use cap_std::fs::Dir; +use cap_std_ext::prelude::CapStdExtCommandExt; +use cap_tempfile::cap_std; +use fn_error_context::context; +use ostree::{gio, glib}; +use std::os::unix::fs::MetadataExt; + +use crate::sysroot::SysrootLock; + +// Find the inode numbers for objects +fn gather_inodes( + prefix: &str, + dir: &Dir, + little_inodes: &mut BTreeMap, + big_inodes: &mut BTreeMap, +) -> Result<()> { + for child in dir.entries()? { + let child = child?; + let metadata = child.metadata()?; + if !(metadata.is_file() || metadata.is_symlink()) { + continue; + } + let name = child.file_name(); + let name = name + .to_str() + .ok_or_else(|| anyhow::anyhow!("Invalid {name:?}"))?; + let object_rest = name + .split_once('.') + .ok_or_else(|| anyhow!("Invalid object {name}"))? + .0; + let checksum = format!("{prefix}{object_rest}"); + let inode = metadata.ino(); + if let Ok(little) = u32::try_from(inode) { + little_inodes.insert(little, checksum); + } else { + big_inodes.insert(inode, checksum); + } + } + Ok(()) +} + +#[context("Analyzing commit for derivation")] +fn commit_is_derived(commit: &glib::Variant) -> Result { + let commit_meta = &glib::VariantDict::new(Some(&commit.child_value(0))); + if commit_meta + .lookup::(crate::container::store::META_MANIFEST_DIGEST)? + .is_some() + { + return Ok(true); + } + if commit_meta + .lookup::("rpmostree.clientlayer")? + .is_some() + { + return Ok(true); + } + Ok(false) +} + +/// The result of a check_repair operation +#[derive(Debug, PartialEq, Eq)] +pub enum InodeCheckResult { + /// Problems are unlikely. + Okay, + /// There is potential corruption + PotentialCorruption(BTreeSet), +} + +#[context("Checking inodes")] +#[doc(hidden)] +/// Detect if any commits are potentially incorrect due to inode truncations. +pub fn check_inode_collision(repo: &ostree::Repo, verbose: bool) -> Result { + let repo_dir = repo.dfd_as_dir()?; + let objects = repo_dir.open_dir("objects")?; + + println!( + r#"Attempting analysis of ostree state for files that may be incorrectly linked. +For more information, see https://github.com/ostreedev/ostree/pull/2874/commits/de6fddc6adee09a93901243dc7074090828a1912 +"# + ); + + println!("Gathering inodes for ostree objects..."); + let mut little_inodes = BTreeMap::new(); + let mut big_inodes = BTreeMap::new(); + + for child in objects.entries()? { + let child = child?; + if !child.file_type()?.is_dir() { + continue; + } + let name = child.file_name(); + if name.len() != 2 { + continue; + } + let name = name + .to_str() + .ok_or_else(|| anyhow::anyhow!("Invalid {name:?}"))?; + let objdir = child.open_dir()?; + gather_inodes(name, &objdir, &mut little_inodes, &mut big_inodes) + .with_context(|| format!("Processing {name:?}"))?; + } + + let mut colliding_inodes = BTreeMap::new(); + for (big_inum, big_inum_checksum) in big_inodes.iter() { + let truncated = *big_inum as u32; + if let Some(small_inum_object) = little_inodes.get(&truncated) { + // Don't output each collision unless verbose mode is enabled. It's actually + // quite interesting to see data, but only for development and deep introspection + // use cases. + if verbose { + eprintln!( + r#"collision: + inode (>32 bit): {big_inum} + object: {big_inum_checksum} + inode (truncated): {truncated} + object: {small_inum_object} +"# + ); + } + colliding_inodes.insert(big_inum, big_inum_checksum); + } + } + + let n_big = big_inodes.len(); + let n_small = little_inodes.len(); + println!("Analyzed {n_big} objects with > 32 bit inode numbers and {n_small} objects with <= 32 bit inode numbers"); + if !colliding_inodes.is_empty() { + return Ok(InodeCheckResult::PotentialCorruption( + colliding_inodes + .keys() + .map(|&&v| v) + .collect::>(), + )); + } + + Ok(InodeCheckResult::Okay) +} + +/// Attempt to automatically repair any corruption from inode collisions. +#[doc(hidden)] +pub fn auto_repair_inode_collision( + sysroot: &SysrootLock, + dry_run: bool, + verbose: bool, +) -> Result<()> { + use crate::container::store as container_store; + let repo = &sysroot.repo(); + let repo_dir = repo.dfd_as_dir()?; + + let mut derived_commits = BTreeSet::new(); + for (_refname, digest) in repo.list_refs(None, gio::Cancellable::NONE)? { + let commit = repo.load_commit(&digest)?.0; + if commit_is_derived(&commit)? { + if verbose { + eprintln!("Found derived commit: {commit}"); + } + derived_commits.insert(digest); + } + } + + // This is not an ironclad guarantee...however, I am pretty confident that there's + // no exposure without derivation today. + if derived_commits.is_empty() { + println!("OK no derived commits found."); + return Ok(()); + } + let n_derived = derived_commits.len(); + println!("Found {n_derived} derived commits"); + println!("Backing filesystem information:"); + { + let st = Command::new("stat") + .args(["-f", "."]) + .cwd_dir(repo_dir.try_clone()?) + .status()?; + if !st.success() { + eprintln!("failed to spawn stat: {st:?}"); + } + } + + match check_inode_collision(repo, verbose)? { + InodeCheckResult::Okay => { + println!("OK no colliding inodes found"); + Ok(()) + } + InodeCheckResult::PotentialCorruption(colliding_inodes) => { + eprintln!( + "warning: {} potentially colliding inodes found", + colliding_inodes.len() + ); + let all_images = container_store::list_images(repo)?; + let all_images = all_images + .into_iter() + .map(|img| crate::container::ImageReference::try_from(img.as_str())) + .collect::>>()?; + println!("Verifying {} ostree-container images", all_images.len()); + let mut corrupted_images = Vec::new(); + for imgref in all_images { + if !container_store::verify_container_image( + sysroot, + &imgref, + &colliding_inodes, + verbose, + )? { + eprintln!("warning: Corrupted image {imgref}"); + corrupted_images.push(imgref); + } + } + if corrupted_images.is_empty() { + println!("OK no corrupted images found"); + return Ok(()); + } + if dry_run { + anyhow::bail!("Found potential corruption, dry-run mode enabled"); + } + container_store::remove_images(repo, corrupted_images.iter())?; + Ok(()) + } + } +}