From 2ed97d4cf1e7e718240de5e25e6ef636705fa438 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 7 Feb 2025 08:40:26 -0500 Subject: [PATCH] lints: Check for invalid /etc/hostname and /etc/resolv.conf Detect problems from https://github.com/containers/buildah/issues/4242 or similar. As part of this, add new infrastructure logic for lints that only operate on non-running roots (we expect these are mounted/written at runtime). Signed-off-by: Colin Walters --- lib/src/cli.rs | 8 +++++- lib/src/lints.rs | 64 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index adb7740d..46ffb1f3 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -1053,8 +1053,14 @@ async fn run_from_opt(opt: Opt) -> Result<()> { } else { lints::WarningDisposition::AllowWarnings }; + let root_type = if rootfs == "/" { + lints::RootType::Running + } else { + lints::RootType::Alternative + }; + let root = &Dir::open_ambient_dir(rootfs, cap_std::ambient_authority())?; - lints::lint(root, warnings, std::io::stdout().lock())?; + lints::lint(root, warnings, root_type, std::io::stdout().lock())?; Ok(()) } }, diff --git a/lib/src/lints.rs b/lib/src/lints.rs index fa27ff0c..1935524b 100644 --- a/lib/src/lints.rs +++ b/lib/src/lints.rs @@ -76,6 +76,12 @@ pub(crate) enum WarningDisposition { FatalWarnings, } +#[derive(Debug, Copy, Clone, Serialize, PartialEq, Eq)] +pub(crate) enum RootType { + Running, + Alternative, +} + #[derive(Debug, Serialize)] #[serde(rename_all = "kebab-case")] struct Lint { @@ -85,6 +91,9 @@ struct Lint { #[serde(skip)] f: LintFn, description: &'static str, + // Set if this only applies to a specific root type. + #[serde(skip_serializing_if = "Option::is_none")] + root_type: Option, } impl Lint { @@ -98,6 +107,7 @@ impl Lint { ty: LintType::Fatal, f: f, description: description, + root_type: None, } } @@ -111,6 +121,7 @@ impl Lint { ty: LintType::Warning, f: f, description: description, + root_type: None, } } } @@ -128,13 +139,22 @@ pub(crate) fn lint_list(output: impl std::io::Write) -> Result<()> { pub(crate) fn lint( root: &Dir, warning_disposition: WarningDisposition, + root_type: RootType, mut output: impl std::io::Write, ) -> Result<()> { let mut fatal = 0usize; let mut warnings = 0usize; let mut passed = 0usize; + let mut skipped = 0usize; for lint in LINTS { let name = lint.name; + + if let Some(lint_root_type) = lint.root_type { + if lint_root_type != root_type { + skipped += 1; + } + } + let r = match (lint.f)(&root) { Ok(r) => r, Err(e) => anyhow::bail!("Unexpected runtime error running lint {name}: {e}"), @@ -158,6 +178,9 @@ pub(crate) fn lint( } } writeln!(output, "Checks passed: {passed}")?; + if skipped > 0 { + writeln!(output, "Checks skipped: {skipped}")?; + } let fatal = if matches!(warning_disposition, WarningDisposition::FatalWarnings) { fatal + warnings } else { @@ -187,6 +210,30 @@ fn check_var_run(root: &Dir) -> LintResult { lint_ok() } +#[distributed_slice(LINTS)] +static LINT_BUILDAH_INJECTED: Lint = Lint { + name: "buildah-injected", + description: indoc::indoc! { " + Check for an invalid /etc/hostname or /etc/resolv.conf that may have been injected by + a container build system." }, + ty: LintType::Warning, + f: check_buildah_injected, + // This one doesn't make sense to run looking at the running root, + // because we do expect /etc/hostname to be injected as + root_type: Some(RootType::Alternative), +}; +fn check_buildah_injected(root: &Dir) -> LintResult { + const RUNTIME_INJECTED: &[&str] = &["etc/hostname", "etc/resolv.conf"]; + for ent in RUNTIME_INJECTED { + if let Some(meta) = root.symlink_metadata_optional(ent)? { + if meta.is_file() && meta.size() == 0 { + return lint_err(format!("/{ent} is an empty file; this may have been synthesized by a container runtime.")); + } + } + } + lint_ok() +} + #[distributed_slice(LINTS)] static LINT_ETC_USRUSETC: Lint = Lint::new_fatal( "etc-usretc", @@ -462,10 +509,11 @@ mod tests { let root = &passing_fixture()?; let mut out = Vec::new(); let warnings = WarningDisposition::FatalWarnings; - lint(root, warnings, &mut out).unwrap(); + let root_type = RootType::Running; + lint(root, warnings, root_type, &mut out).unwrap(); root.create_dir_all("var/run/foo")?; let mut out = Vec::new(); - assert!(lint(root, warnings, &mut out).is_err()); + assert!(lint(root, warnings, root_type, &mut out).is_err()); Ok(()) } @@ -639,6 +687,18 @@ mod tests { Ok(()) } + #[test] + fn test_buildah_injected() -> Result<()> { + let td = fixture()?; + td.create_dir("etc")?; + assert!(check_buildah_injected(&td).unwrap().is_ok()); + td.write("etc/hostname", b"")?; + assert!(check_buildah_injected(&td).unwrap().is_err()); + td.write("etc/hostname", b"some static hostname")?; + assert!(check_buildah_injected(&td).unwrap().is_ok()); + Ok(()) + } + #[test] fn test_list() { let mut r = Vec::new();