Skip to content

Commit 6ae6669

Browse files
policy: Handle absolute symlinks in system policy path resolution
cap-std refuses to follow absolute symlinks since they would escape the sandbox, returning PermissionDenied. When /etc/containers/policy.json is a symlink to e.g. /usr/share/containers/policy.json, this caused policy resolution to silently skip the file. Switch to `cap_std_ext::RootDir` that handles the above semantics by itself Test written by Claude Fixes: #2339 Signed-off-by: Pragyan Poudyal <pragyanpoudyal41999@gmail.com>
1 parent a2bc817 commit 6ae6669

1 file changed

Lines changed: 84 additions & 17 deletions

File tree

‎crates/ostree-ext/src/container/skopeo.rs‎

Lines changed: 84 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use super::ImageReference;
44
use anyhow::{Context, Result};
5+
use cap_std_ext::RootDir;
56
use cap_std_ext::cap_std;
67
use cap_std_ext::cmdext::{CapStdExtCommandExt, CmdFds};
78
use containers_image_proxy::oci_spec::image as oci_image;
@@ -46,12 +47,14 @@ const USER_POLICY_SUFFIX: &str = "containers/policy.json";
4647
/// Absolute paths (from env vars) have their leading `/` stripped so they
4748
/// resolve under `root`. Passing `root` opened on `/` gives normal behaviour;
4849
/// tests can pass a cap-std `Dir` backed by a temporary directory.
50+
///
51+
/// We use `RootDir` to handle absolute symlinks
4952
fn resolve_policy_path(
50-
root: &cap_std::fs::Dir,
53+
root: &RootDir,
5154
env_override: Option<&Path>,
5255
xdg_config_home: Option<&Path>,
5356
home: Option<&Path>,
54-
) -> Result<cap_std::fs::File> {
57+
) -> Result<std::fs::File> {
5558
// Helper: strip a leading `/` so the path is relative to root.
5659
fn strip_abs(p: &Path) -> &Path {
5760
p.strip_prefix("/").unwrap_or(p)
@@ -84,9 +87,15 @@ fn resolve_policy_path(
8487

8588
// 3–4. System paths.
8689
for candidate in SYSTEM_POLICY_PATHS {
87-
if let Ok(f) = root.open(candidate) {
88-
tracing::debug!("Using system policy path: {candidate}");
89-
return Ok(f);
90+
match root.open(candidate) {
91+
Ok(f) => {
92+
tracing::debug!("Using system policy path: {candidate}");
93+
return Ok(f);
94+
}
95+
Err(e) => {
96+
tracing::debug!("Opening {candidate}: {e:?}");
97+
continue;
98+
}
9099
}
91100
}
92101

@@ -120,6 +129,7 @@ impl ContainerPolicy {
120129
}
121130

122131
pub(crate) fn container_policy_is_default_insecure(root: &cap_std::fs::Dir) -> Result<bool> {
132+
let root = &RootDir::new(root, ".").context("Opening RootDir")?;
123133
let f = resolve_policy_path(
124134
root,
125135
std::env::var_os(POLICY_ENV_VAR).as_deref().map(Path::new),
@@ -254,33 +264,38 @@ mod tests {
254264
(m.dev(), m.ino())
255265
}
256266

257-
/// Return (dev, ino) for an open cap-std file.
258-
fn file_id(f: &cap_std::fs::File) -> (u64, u64) {
259-
use cap_std::fs::MetadataExt;
267+
/// Return (dev, ino) for an open file.
268+
fn file_id(f: &std::fs::File) -> (u64, u64) {
269+
use std::os::unix::fs::MetadataExt;
260270
let m = f.metadata().unwrap();
261271
(m.dev(), m.ino())
262272
}
263273

274+
fn to_root_dir(dir: &cap_std::fs::Dir) -> RootDir {
275+
RootDir::new(dir, ".").unwrap()
276+
}
277+
264278
#[test]
265279
fn resolve_policy_path_cases() -> Result<()> {
266280
let td = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
281+
let root = to_root_dir(&td);
267282

268283
let etc_id = touch(&td, "etc/containers/policy.json");
269284
let _usr_id = touch(&td, "usr/share/containers/policy.json");
270285

271286
// Env var override wins (trusted — errors if file missing)
272287
let custom = Path::new("/custom/policy.json");
273-
assert!(resolve_policy_path(&td, Some(custom), None, None).is_err());
288+
assert!(resolve_policy_path(&root, Some(custom), None, None).is_err());
274289
let custom_id = touch(&td, "custom/policy.json");
275-
let f = resolve_policy_path(&td, Some(custom), None, None)?;
290+
let f = resolve_policy_path(&root, Some(custom), None, None)?;
276291
assert_eq!(
277292
file_id(&f),
278293
custom_id,
279294
"env var should open the custom file"
280295
);
281296

282297
// Empty env var is ignored, falls through to /etc
283-
let f = resolve_policy_path(&td, Some(Path::new("")), None, None)?;
298+
let f = resolve_policy_path(&root, Some(Path::new("")), None, None)?;
284299
assert_eq!(
285300
file_id(&f),
286301
etc_id,
@@ -289,20 +304,20 @@ mod tests {
289304

290305
// XDG_CONFIG_HOME wins when file exists
291306
let xdg_id = touch(&td, "xdg/containers/policy.json");
292-
let f = resolve_policy_path(&td, None, Some(Path::new("/xdg")), None)?;
307+
let f = resolve_policy_path(&root, None, Some(Path::new("/xdg")), None)?;
293308
assert_eq!(file_id(&f), xdg_id, "XDG_CONFIG_HOME should win");
294309

295310
// XDG_CONFIG_HOME skipped when file missing, falls through to /etc
296-
let f = resolve_policy_path(&td, None, Some(Path::new("/xdg-empty")), None)?;
311+
let f = resolve_policy_path(&root, None, Some(Path::new("/xdg-empty")), None)?;
297312
assert_eq!(file_id(&f), etc_id, "missing XDG dir should fall through");
298313

299314
// HOME/.config fallback when XDG unset
300315
let home_id = touch(&td, "home/.config/containers/policy.json");
301-
let f = resolve_policy_path(&td, None, None, Some(Path::new("/home")))?;
316+
let f = resolve_policy_path(&root, None, None, Some(Path::new("/home")))?;
302317
assert_eq!(file_id(&f), home_id, "HOME fallback should work");
303318

304319
// /etc preferred over /usr/share
305-
let f = resolve_policy_path(&td, None, None, None)?;
320+
let f = resolve_policy_path(&root, None, None, None)?;
306321
assert_eq!(
307322
file_id(&f),
308323
etc_id,
@@ -311,13 +326,65 @@ mod tests {
311326

312327
// Falls through to /usr/share when /etc missing
313328
let td2 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
329+
let root2 = to_root_dir(&td2);
314330
let usr2_id = touch(&td2, "usr/share/containers/policy.json");
315-
let f = resolve_policy_path(&td2, None, None, None)?;
331+
let f = resolve_policy_path(&root2, None, None, None)?;
316332
assert_eq!(file_id(&f), usr2_id, "should fall through to /usr/share");
317333

318334
// Nothing found returns error
319335
let td3 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
320-
assert!(resolve_policy_path(&td3, None, None, None).is_err());
336+
let root3 = to_root_dir(&td3);
337+
assert!(resolve_policy_path(&root3, None, None, None).is_err());
338+
339+
Ok(())
340+
}
341+
342+
/// Create an absolute symlink inside a cap-std Dir. We need to go via
343+
/// procfs because cap-std (by design) refuses to create symlinks with
344+
/// absolute targets.
345+
fn symlink_absolute(dir: &cap_std::fs::Dir, target: &str, link: &str) {
346+
use std::os::fd::AsRawFd;
347+
let real_dir = format!("/proc/self/fd/{}", dir.as_raw_fd());
348+
let link_path = std::path::PathBuf::from(real_dir).join(link);
349+
std::os::unix::fs::symlink(target, &link_path).unwrap();
350+
}
351+
352+
#[test]
353+
fn resolve_policy_path_absolute_symlink() -> Result<()> {
354+
let td = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
355+
let root = to_root_dir(&td);
356+
357+
// Create the real file at a different location
358+
let real_id = touch(&td, "real/policy.json");
359+
360+
// RootDir uses RESOLVE_IN_ROOT, so absolute symlinks are followed
361+
// within the root automatically.
362+
td.create_dir_all("etc/containers")?;
363+
symlink_absolute(&td, "/real/policy.json", "etc/containers/policy.json");
364+
365+
let f = resolve_policy_path(&root, None, None, None)?;
366+
assert_eq!(
367+
file_id(&f),
368+
real_id,
369+
"absolute symlink should be resolved relative to root"
370+
);
371+
372+
// Symlink whose target doesn't exist should fall through
373+
let td2 = cap_tempfile::TempDir::new(cap_std::ambient_authority())?;
374+
let root2 = to_root_dir(&td2);
375+
td2.create_dir_all("etc/containers")?;
376+
symlink_absolute(
377+
&td2,
378+
"/nonexistent/policy.json",
379+
"etc/containers/policy.json",
380+
);
381+
let fallback_id = touch(&td2, "usr/share/containers/policy.json");
382+
let f = resolve_policy_path(&root2, None, None, None)?;
383+
assert_eq!(
384+
file_id(&f),
385+
fallback_id,
386+
"broken symlink should fall through to next candidate"
387+
);
321388

322389
Ok(())
323390
}

0 commit comments

Comments
 (0)