From 3af1e06c9260614613e175b3a0b50d9b293ee5e1 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 22 Jun 2026 17:32:33 -0400 Subject: [PATCH] ostree-ext: Adapt to new containers policy config load order Closes: https://github.com/bootc-dev/bootc/issues/2258 The real fix here will be to migrate to `--require-signed` from skopeo in the future. Generated-by: AI (with only a few tweaks) Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 5 +- crates/lib/src/install.rs | 5 +- crates/ostree-ext/src/container/skopeo.rs | 172 +++++++++++++++++- crates/ostree-ext/src/container/store.rs | 11 +- docs/src/man/bootc-install-config.5.md | 3 +- docs/src/man/bootc-install-to-disk.8.md | 2 +- .../man/bootc-install-to-existing-root.8.md | 2 +- docs/src/man/bootc-install-to-filesystem.8.md | 2 +- 8 files changed, 189 insertions(+), 13 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 9a7c307ff..6b14fc901 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -165,8 +165,9 @@ pub(crate) struct SwitchOpts { /// This is the inverse of the previous `--target-no-signature-verification` (which is now /// a no-op). /// - /// Enabling this option enforces that `/etc/containers/policy.json` includes a - /// default policy which requires signatures. + /// Enabling this option enforces that `containers-policy.json` (see `man + /// containers-policy.json` for the full search path) includes a default + /// policy which requires signatures. #[clap(long)] pub(crate) enforce_container_sigpolicy: bool, diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index d49c6a350..cefd19a48 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -265,8 +265,9 @@ pub(crate) struct InstallTargetOpts { pub(crate) target_no_signature_verification: bool, /// This is the inverse of the previous `--target-no-signature-verification` (which is now - /// a no-op). Enabling this option enforces that `/etc/containers/policy.json` includes a - /// default policy which requires signatures. + /// a no-op). Enabling this option enforces that `containers-policy.json` (see `man + /// containers-policy.json` for the full search path) includes a default policy which + /// requires signatures. #[clap(long)] #[serde(default)] pub(crate) enforce_container_sigpolicy: bool, diff --git a/crates/ostree-ext/src/container/skopeo.rs b/crates/ostree-ext/src/container/skopeo.rs index 27c7ffea5..98de9e07f 100644 --- a/crates/ostree-ext/src/container/skopeo.rs +++ b/crates/ostree-ext/src/container/skopeo.rs @@ -2,6 +2,7 @@ use super::ImageReference; use anyhow::{Context, Result}; +use cap_std_ext::cap_std; use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds}; use containers_image_proxy::oci_spec::image as oci_image; use fn_error_context::context; @@ -17,9 +18,84 @@ use tokio::process::Command; // https://github.com/containers/image/blob/main/signature/policy_types.go // Ideally we add something like `skopeo pull --disallow-insecure-accept-anything` // but for now we parse the policy. -const POLICY_PATH: &str = "/etc/containers/policy.json"; const INSECURE_ACCEPT_ANYTHING: &str = "insecureAcceptAnything"; +/// The env var that overrides the policy path, matching the upstream Go +/// containers/image library behavior. +const POLICY_ENV_VAR: &str = "CONTAINERS_POLICY_JSON"; + +/// Well-known system paths for `containers-policy.json`, checked in order. +const SYSTEM_POLICY_PATHS: &[&str] = &[ + "etc/containers/policy.json", + "usr/share/containers/policy.json", +]; + +/// Suffix appended under `$XDG_CONFIG_HOME` (or `$HOME/.config`). +const USER_POLICY_SUFFIX: &str = "containers/policy.json"; + +/// Resolve the containers policy path using the same load order as the +/// upstream Go containers/image library, with all lookups relative to `root`: +/// +/// 1. `CONTAINERS_POLICY_JSON` env var (trusted, no existence check) +/// 2. `$XDG_CONFIG_HOME/containers/policy.json` (or `$HOME/.config/…`) +/// 3. `/etc/containers/policy.json` +/// 4. `/usr/share/containers/policy.json` +/// +/// For candidates 2–4 we only return a path when the file exists on disk. +/// +/// Absolute paths (from env vars) have their leading `/` stripped so they +/// resolve under `root`. Passing `root` opened on `/` gives normal behaviour; +/// tests can pass a cap-std `Dir` backed by a temporary directory. +fn resolve_policy_path( + root: &cap_std::fs::Dir, + env_override: Option<&Path>, + xdg_config_home: Option<&Path>, + home: Option<&Path>, +) -> Result { + // Helper: strip a leading `/` so the path is relative to root. + fn strip_abs(p: &Path) -> &Path { + p.strip_prefix("/").unwrap_or(p) + } + + // 1. Env var override – trust unconditionally (no existence check). + if let Some(raw) = env_override.filter(|v| !v.as_os_str().is_empty()) { + let relative = strip_abs(raw); + tracing::debug!("Using policy path from {POLICY_ENV_VAR}: {}", raw.display()); + return root.open(relative).with_context(|| { + format!( + "Opening policy file from {POLICY_ENV_VAR}={}", + raw.display() + ) + }); + } + + // 2. Per-user config dir. + let user_candidate = if let Some(xdg) = xdg_config_home { + Some(strip_abs(xdg).join(USER_POLICY_SUFFIX)) + } else { + home.map(|h| strip_abs(h).join(".config").join(USER_POLICY_SUFFIX)) + }; + if let Some(p) = &user_candidate { + if let Ok(f) = root.open(p) { + tracing::debug!("Using user policy path: {}", p.display()); + return Ok(f); + } + } + + // 3–4. System paths. + for candidate in SYSTEM_POLICY_PATHS { + if let Ok(f) = root.open(candidate) { + tracing::debug!("Using system policy path: {candidate}"); + return Ok(f); + } + } + + anyhow::bail!( + "No containers policy.json found; \ + checked ${POLICY_ENV_VAR}, user config dir, and system paths" + ) +} + #[derive(Deserialize)] struct PolicyEntry { #[serde(rename = "type")] @@ -43,8 +119,17 @@ impl ContainerPolicy { } } -pub(crate) fn container_policy_is_default_insecure() -> Result { - let r = std::io::BufReader::new(std::fs::File::open(POLICY_PATH)?); +pub(crate) fn container_policy_is_default_insecure(root: &cap_std::fs::Dir) -> Result { + let f = resolve_policy_path( + root, + std::env::var_os(POLICY_ENV_VAR).as_deref().map(Path::new), + std::env::var_os("XDG_CONFIG_HOME") + .as_deref() + .map(Path::new), + std::env::var_os("HOME").as_deref().map(Path::new), + ) + .context("Resolving containers policy path")?; + let r = std::io::BufReader::new(f); let policy: ContainerPolicy = serde_json::from_reader(r)?; Ok(policy.is_default_insecure()) } @@ -106,6 +191,7 @@ pub async fn copy( #[cfg(test)] mod tests { use super::*; + use cap_std_ext::cap_tempfile; // Default value as of the Fedora 34 containers-common-1-21.fc34.noarch package. const DEFAULT_POLICY: &str = indoc::indoc! {r#" @@ -155,4 +241,84 @@ mod tests { assert!(!p.is_default_insecure()); } } + + /// Create `/` with empty JSON content, creating parent dirs. + /// Returns the (dev, ino) of the created file for identity checks. + fn touch(dir: &cap_std::fs::Dir, path: &str) -> (u64, u64) { + use cap_std::fs::MetadataExt; + if let Some(parent) = Path::new(path).parent() { + dir.create_dir_all(parent).unwrap(); + } + dir.write(path, b"{}").unwrap(); + let m = dir.metadata(path).unwrap(); + (m.dev(), m.ino()) + } + + /// Return (dev, ino) for an open cap-std file. + fn file_id(f: &cap_std::fs::File) -> (u64, u64) { + use cap_std::fs::MetadataExt; + let m = f.metadata().unwrap(); + (m.dev(), m.ino()) + } + + #[test] + fn resolve_policy_path_cases() -> Result<()> { + let td = cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + let etc_id = touch(&td, "etc/containers/policy.json"); + let _usr_id = touch(&td, "usr/share/containers/policy.json"); + + // Env var override wins (trusted — errors if file missing) + let custom = Path::new("/custom/policy.json"); + assert!(resolve_policy_path(&td, Some(custom), None, None).is_err()); + let custom_id = touch(&td, "custom/policy.json"); + let f = resolve_policy_path(&td, Some(custom), None, None)?; + assert_eq!( + file_id(&f), + custom_id, + "env var should open the custom file" + ); + + // Empty env var is ignored, falls through to /etc + let f = resolve_policy_path(&td, Some(Path::new("")), None, None)?; + assert_eq!( + file_id(&f), + etc_id, + "empty env var should fall through to /etc" + ); + + // XDG_CONFIG_HOME wins when file exists + let xdg_id = touch(&td, "xdg/containers/policy.json"); + let f = resolve_policy_path(&td, None, Some(Path::new("/xdg")), None)?; + assert_eq!(file_id(&f), xdg_id, "XDG_CONFIG_HOME should win"); + + // XDG_CONFIG_HOME skipped when file missing, falls through to /etc + let f = resolve_policy_path(&td, None, Some(Path::new("/xdg-empty")), None)?; + assert_eq!(file_id(&f), etc_id, "missing XDG dir should fall through"); + + // HOME/.config fallback when XDG unset + let home_id = touch(&td, "home/.config/containers/policy.json"); + let f = resolve_policy_path(&td, None, None, Some(Path::new("/home")))?; + assert_eq!(file_id(&f), home_id, "HOME fallback should work"); + + // /etc preferred over /usr/share + let f = resolve_policy_path(&td, None, None, None)?; + assert_eq!( + file_id(&f), + etc_id, + "/etc should be preferred over /usr/share" + ); + + // Falls through to /usr/share when /etc missing + let td2 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + let usr2_id = touch(&td2, "usr/share/containers/policy.json"); + let f = resolve_policy_path(&td2, None, None, None)?; + assert_eq!(file_id(&f), usr2_id, "should fall through to /usr/share"); + + // Nothing found returns error + let td3 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + assert!(resolve_policy_path(&td3, None, None, None).is_err()); + + Ok(()) + } } diff --git a/crates/ostree-ext/src/container/store.rs b/crates/ostree-ext/src/container/store.rs index f810bb7ca..008ceeaa9 100644 --- a/crates/ostree-ext/src/container/store.rs +++ b/crates/ostree-ext/src/container/store.rs @@ -302,6 +302,8 @@ struct LayerRef { #[derive(Debug)] pub struct ImageImporter { repo: ostree::Repo, + /// The root filesystem directory, used for policy lookups. + root: Dir, pub(crate) proxy: ImageProxy, imgref: OstreeImageReference, target_imgref: Option, @@ -648,8 +650,11 @@ impl ImageImporter { let diffid_to_digest = Self::build_diffid_to_digest_map(&repo)?; + let root = Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + Ok(ImageImporter { repo, + root, proxy, target_imgref: None, no_imgref: false, @@ -935,7 +940,9 @@ impl ImageImporter { #[context("Fetching manifest")] pub(crate) async fn prepare_internal(&mut self, verify_layers: bool) -> Result { match &self.imgref.sigverify { - SignatureSource::ContainerPolicy if skopeo::container_policy_is_default_insecure()? => { + SignatureSource::ContainerPolicy + if skopeo::container_policy_is_default_insecure(&self.root)? => + { return Err(anyhow!( "containers-policy.json specifies a default of `insecureAcceptAnything`; refusing usage" )); @@ -1039,7 +1046,7 @@ impl ImageImporter { ) -> Result<()> { tracing::debug!("Fetching base"); if matches!(self.imgref.sigverify, SignatureSource::ContainerPolicy) - && skopeo::container_policy_is_default_insecure()? + && skopeo::container_policy_is_default_insecure(&self.root)? { return Err(anyhow!( "containers-policy.json specifies a default of `insecureAcceptAnything`; refusing usage" diff --git a/docs/src/man/bootc-install-config.5.md b/docs/src/man/bootc-install-config.5.md index 76fba5fc7..8ed61e8b0 100644 --- a/docs/src/man/bootc-install-config.5.md +++ b/docs/src/man/bootc-install-config.5.md @@ -40,7 +40,8 @@ The `install` section supports these subfields: needs the `bli` module (available in newer builds). Defaults to `true` when using systemd-boot, `false` otherwise. - `enforce-container-sigpolicy`: A boolean that controls whether to enforce that - `/etc/containers/policy.json` includes a default policy which requires signatures. + `containers-policy.json` (see `man containers-policy.json` for the full search + path) includes a default policy which requires signatures. When `true`, image pulls will be rejected if the policy file specifies `insecureAcceptAnything` as the default. Defaults to `false`. This is equivalent to the `--enforce-container-sigpolicy` CLI flag. diff --git a/docs/src/man/bootc-install-to-disk.8.md b/docs/src/man/bootc-install-to-disk.8.md index 5b4a7bd39..41a4033f3 100644 --- a/docs/src/man/bootc-install-to-disk.8.md +++ b/docs/src/man/bootc-install-to-disk.8.md @@ -111,7 +111,7 @@ set `discoverable-partitions = true` in their install configuration **--enforce-container-sigpolicy** - This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures + This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures **--run-fetch-check** diff --git a/docs/src/man/bootc-install-to-existing-root.8.md b/docs/src/man/bootc-install-to-existing-root.8.md index 4b4f8925d..705b5dd1d 100644 --- a/docs/src/man/bootc-install-to-existing-root.8.md +++ b/docs/src/man/bootc-install-to-existing-root.8.md @@ -157,7 +157,7 @@ of migrating the fstab entries. See the "Injecting kernel arguments" section abo **--enforce-container-sigpolicy** - This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures + This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures **--run-fetch-check** diff --git a/docs/src/man/bootc-install-to-filesystem.8.md b/docs/src/man/bootc-install-to-filesystem.8.md index f46892de3..d011591d5 100644 --- a/docs/src/man/bootc-install-to-filesystem.8.md +++ b/docs/src/man/bootc-install-to-filesystem.8.md @@ -65,7 +65,7 @@ is currently expected to be empty by default. **--enforce-container-sigpolicy** - This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `/etc/containers/policy.json` includes a default policy which requires signatures + This is the inverse of the previous `--target-no-signature-verification` (which is now a no-op). Enabling this option enforces that `containers-policy.json` (see `man containers-policy.json` for the full search path) includes a default policy which requires signatures **--run-fetch-check**