From 092c7a8f49f179187e8d2a824f74372d141b255f Mon Sep 17 00:00:00 2001 From: Furisto <24721048+Furisto@users.noreply.github.com> Date: Sat, 16 Apr 2022 20:27:07 +0200 Subject: [PATCH] Ensure pid and root path are canonicalized Signed-off-by: Furisto <24721048+Furisto@users.noreply.github.com> --- crates/libcontainer/src/container/builder.rs | 64 +++++++++++++------- crates/libcontainer/src/utils.rs | 50 ++++++++++++++- 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/crates/libcontainer/src/container/builder.rs b/crates/libcontainer/src/container/builder.rs index c7d1c6ce..95a3989b 100644 --- a/crates/libcontainer/src/container/builder.rs +++ b/crates/libcontainer/src/container/builder.rs @@ -1,4 +1,4 @@ -use crate::syscall::Syscall; +use crate::{syscall::Syscall, utils::PathBufExt}; use anyhow::{Context, Result}; use std::path::PathBuf; @@ -108,8 +108,9 @@ impl<'a> ContainerBuilder<'a> { pub fn with_root_path>(mut self, path: P) -> Result { let path = path.into(); self.root_path = path - .canonicalize() - .with_context(|| format!("failed to canonicalize root path {:?}", path))?; + .canonicalize_safely() + .with_context(|| format!("failed to canonicalize root path {path:?}"))?; + Ok(self) } @@ -125,15 +126,17 @@ impl<'a> ContainerBuilder<'a> { /// .with_pid_file(Some("/var/run/docker.pid")).expect("invalid pid file"); /// ``` pub fn with_pid_file>(mut self, path: Option

) -> Result { - self.pid_file = if let Some(p) = path { - let path = p.into(); - Some( - path.canonicalize() - .with_context(|| format!("failed to canonicalize pid file path {:?}", path))?, - ) - } else { - None + self.pid_file = match path { + Some(path) => { + let p = path.into(); + Some( + p.canonicalize_safely() + .with_context(|| format!("failed to canonicalize pid file {p:?}"))?, + ) + } + None => None, }; + Ok(self) } @@ -179,7 +182,7 @@ mod tests { use std::path::PathBuf; #[test] - fn test_builder_failable_functions() -> Result<()> { + fn test_failable_functions() -> Result<()> { let root_path_temp_dir = TempDir::new("root_path").context("failed to create temp dir")?; let pid_file_temp_dir = TempDir::new("pid_file").context("failed to create temp dir")?; let syscall = create_syscall(); @@ -190,20 +193,37 @@ mod tests { .with_console_socket(Some("/var/run/docker/sock.tty")) .as_init("/var/run/docker/bundle"); - // Accept None pid file. + // accept None pid file. ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) .with_pid_file::(None)?; - let invalid_path_builder = - ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) - .with_root_path("/not/existing/path"); - assert!(invalid_path_builder.is_err()); + // accept absolute root path which does not exist + let abs_root_path = PathBuf::from("/not/existing/path"); + let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) + .with_root_path(&abs_root_path) + .context("build container")?; + assert_eq!(path_builder.root_path, abs_root_path); - let invalid_pid_file_builder = - ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) - .with_root_path(root_path_temp_dir.path())? - .with_pid_file(Some("/not/existing/path")); - assert!(invalid_pid_file_builder.is_err()); + // accept relative root path which does not exist + let cwd = std::env::current_dir().context("get current dir")?; + let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) + .with_root_path("./not/existing/path") + .context("build container")?; + assert_eq!(path_builder.root_path, cwd.join("not/existing/path")); + + // accept absolute pid path which does not exist + let abs_pid_path = PathBuf::from("/not/existing/path"); + let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) + .with_pid_file(Some(&abs_pid_path)) + .context("build container")?; + assert_eq!(path_builder.pid_file, Some(abs_pid_path)); + + // accept relative pid path which does not exist + let cwd = std::env::current_dir().context("get current dir")?; + let path_builder = ContainerBuilder::new("74f1a4cb3801".to_owned(), syscall.as_ref()) + .with_pid_file(Some("./not/existing/path")) + .context("build container")?; + assert_eq!(path_builder.pid_file, Some(cwd.join("not/existing/path"))); Ok(()) } diff --git a/crates/libcontainer/src/utils.rs b/crates/libcontainer/src/utils.rs index 2871d503..eb1e2092 100644 --- a/crates/libcontainer/src/utils.rs +++ b/crates/libcontainer/src/utils.rs @@ -14,11 +14,13 @@ use std::ops::Deref; use std::os::linux::fs::MetadataExt; use std::os::unix::fs::DirBuilderExt; use std::os::unix::prelude::{AsRawFd, OsStrExt}; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; pub trait PathBufExt { fn as_relative(&self) -> Result<&Path>; fn join_safely>(&self, p: P) -> Result; + fn canonicalize_safely(&self) -> Result; + fn normalize(&self) -> PathBuf; } impl PathBufExt for Path { @@ -42,6 +44,52 @@ impl PathBufExt for Path { .with_context(|| format!("failed to strip prefix from {}", path.display()))?; Ok(self.join(stripped)) } + + /// Canonicalizes existing and not existing paths + fn canonicalize_safely(&self) -> Result { + if self.exists() { + self.canonicalize() + .with_context(|| format!("failed to canonicalize path {:?}", self)) + } else { + if self.is_relative() { + let p = std::env::current_dir() + .context("could not get current directory")? + .join(self); + return Ok(p.normalize()); + } + + Ok(self.normalize()) + } + } + + /// Normalizes a path. In contrast to canonicalize the path does not need to exist. + // adapted from https://github.com/rust-lang/cargo/blob/fede83ccf973457de319ba6fa0e36ead454d2e20/src/cargo/util/paths.rs#L61 + fn normalize(&self) -> PathBuf { + let mut components = self.components().peekable(); + let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().cloned() { + components.next(); + PathBuf::from(c.as_os_str()) + } else { + PathBuf::new() + }; + + for component in components { + match component { + Component::Prefix(..) => unreachable!(), + Component::RootDir => { + ret.push(component.as_os_str()); + } + Component::CurDir => {} + Component::ParentDir => { + ret.pop(); + } + Component::Normal(c) => { + ret.push(c); + } + } + } + ret + } } pub fn parse_env(envs: &[String]) -> HashMap {