mirror of
https://github.com/kata-containers/kata-containers.git
synced 2026-07-16 08:35:04 +00:00
Merge pull request #12971 from burgerdev/pathrs
agent: constrain copy_file to shared directory
This commit is contained in:
manuelh-dev
GitHub
19
Cargo.lock
generated
19
Cargo.lock
generated
@@ -3575,6 +3575,7 @@ dependencies = [
|
||||
"nix 0.31.3",
|
||||
"oci-spec 0.10.0",
|
||||
"opentelemetry 0.17.0",
|
||||
"pathrs",
|
||||
"procfs 0.12.0",
|
||||
"prometheus",
|
||||
"protobuf",
|
||||
@@ -5237,6 +5238,24 @@ dependencies = [
|
||||
"once_cell",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pathrs"
|
||||
version = "0.2.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2fb2311801201fc6fd2e8a9f4841b41eee565e992fbe713731e29e367b8e3f17"
|
||||
dependencies = [
|
||||
"bitflags 2.11.1",
|
||||
"itertools 0.14.0",
|
||||
"libc",
|
||||
"memchr",
|
||||
"once_cell",
|
||||
"rustix 1.1.4",
|
||||
"rustversion",
|
||||
"static_assertions",
|
||||
"tempfile",
|
||||
"thiserror 2.0.18",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pci-ids"
|
||||
version = "0.2.6"
|
||||
|
||||
@@ -183,6 +183,7 @@ netns-rs = "0.1.0"
|
||||
nix = { version = "0.31.3", features = ["fs", "mount", "sched", "process", "ioctl", "signal", "socket", "feature", "user", "hostname", "term", "event", "mman", "reboot"] }
|
||||
oci-spec = { version = "0.10.0", features = ["runtime"] }
|
||||
opentelemetry = { version = "0.17.0", features = ["rt-tokio"] }
|
||||
pathrs = "0.2.4"
|
||||
procfs = "0.12.0"
|
||||
prometheus = { version = "0.14.0", features = ["process"] }
|
||||
protobuf = "3.7.2"
|
||||
|
||||
@@ -81,6 +81,7 @@ safe-path.workspace = true
|
||||
# to be modified at runtime.
|
||||
logging.workspace = true
|
||||
vsock-exporter.workspace = true
|
||||
pathrs.workspace = true
|
||||
|
||||
# Initdata
|
||||
base64 = { workspace = true }
|
||||
|
||||
@@ -6,6 +6,7 @@
|
||||
use async_trait::async_trait;
|
||||
#[cfg(feature = "agent-policy")]
|
||||
use kata_agent_policy::policy::PolicyCopyFileRequest;
|
||||
use pathrs::flags::OpenFlags;
|
||||
use rustjail::{pipestream::PipeStream, process::StreamType};
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt, ReadHalf};
|
||||
use tokio::sync::Mutex;
|
||||
@@ -64,7 +65,6 @@ use nix::unistd::{self, Pid};
|
||||
use rustjail::process::ProcessOperations;
|
||||
#[cfg(all(test, not(target_arch = "powerpc64")))]
|
||||
use std::os::fd::AsRawFd;
|
||||
use std::os::fd::BorrowedFd;
|
||||
|
||||
#[cfg(target_arch = "s390x")]
|
||||
use crate::ccw;
|
||||
@@ -113,7 +113,7 @@ use std::os::unix::prelude::PermissionsExt;
|
||||
use std::process::{Command, Stdio};
|
||||
|
||||
use nix::unistd::{Gid, Uid};
|
||||
use std::fs::{File, OpenOptions};
|
||||
use std::fs::File;
|
||||
use std::io::{BufRead, BufReader, Write};
|
||||
use std::os::unix::fs::FileExt;
|
||||
use std::path::PathBuf;
|
||||
@@ -146,6 +146,14 @@ const ERR_NO_SANDBOX_PIDNS: &str = "Sandbox does not have sandbox_pidns";
|
||||
// not available.
|
||||
const IPTABLES_RESTORE_WAIT_SEC: u64 = 5;
|
||||
|
||||
/// This mask is applied to parent directories implicitly created for CopyFile requests.
|
||||
const IMPLICIT_DIRECTORY_PERMISSION_MASK: u32 = 0o777;
|
||||
|
||||
/// This mask is applied to files and directories created for CopyFile requests.
|
||||
/// In addition to the permissions, it allows setuid/setgid/sticky bits.
|
||||
/// Note that the setuid bit does not have an effect on Linux, though.
|
||||
const FILE_PERMISSION_MASK: u32 = 0o7777;
|
||||
|
||||
// Convenience function to obtain the scope logger.
|
||||
fn sl() -> slog::Logger {
|
||||
slog_scope::logger()
|
||||
@@ -1680,7 +1688,9 @@ impl agent_ttrpc::AgentService for AgentService {
|
||||
#[cfg(not(feature = "agent-policy"))]
|
||||
is_allowed(&req).await?;
|
||||
|
||||
do_copy_file(&req).map_ttrpc_err(same)?;
|
||||
// Potentially untrustworthy data from the host needs to go into the shared dir.
|
||||
let root_path = PathBuf::from(KATA_GUEST_SHARE_DIR);
|
||||
do_copy_file(&req, &root_path).map_ttrpc_err(same)?;
|
||||
|
||||
Ok(Empty::new())
|
||||
}
|
||||
@@ -2240,127 +2250,173 @@ fn do_set_guest_date_time(sec: i64, usec: i64) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn do_copy_file(req: &CopyFileRequest) -> Result<()> {
|
||||
let path = PathBuf::from(req.path.as_str());
|
||||
/// do_copy_file creates a file, directory or symlink beneath the provided directory.
|
||||
///
|
||||
/// The function guarantees that no content is written outside of the directory. However, a symlink
|
||||
/// created by this function might point outside the shared directory. Other users of that
|
||||
/// directory need to consider whether they trust the host, or handle the directory with the same
|
||||
/// care as do_copy_file.
|
||||
///
|
||||
/// Parent directories are created, if they don't exist already. For these implicit operations, the
|
||||
/// permissions are set with req.dir_mode. The actual target is created with permissions from
|
||||
/// req.file_mode, even if it's a directory.
|
||||
///
|
||||
/// If req.file_mode requests a symbolic link, the link is created pointing to the path in
|
||||
/// req.data. In that case, req.file_mode is ignored because symlinks don't have permissions on
|
||||
/// Linux.
|
||||
///
|
||||
/// If this function returns an error, the filesystem may be in an unexpected state. This is not
|
||||
/// significant for the caller, since errors are almost certainly not retriable. The runtime should
|
||||
/// abandon this VM instead.
|
||||
fn do_copy_file(req: &CopyFileRequest, shared_dir: &PathBuf) -> Result<()> {
|
||||
let insecure_full_path = PathBuf::from(req.path.as_str());
|
||||
let path = insecure_full_path
|
||||
.strip_prefix(shared_dir)
|
||||
.context(format!(
|
||||
"removing {:?} prefix from {}",
|
||||
shared_dir, req.path
|
||||
))?;
|
||||
|
||||
if !path.starts_with(CONTAINER_BASE) {
|
||||
return Err(anyhow!(
|
||||
"Path {:?} does not start with {}",
|
||||
path,
|
||||
CONTAINER_BASE
|
||||
));
|
||||
}
|
||||
// The shared directory might not exist yet, but we need to create it in order to open the root.
|
||||
std::fs::create_dir_all(shared_dir)?;
|
||||
let root = pathrs::Root::open(shared_dir)?;
|
||||
|
||||
// Create parent directories if missing
|
||||
if let Some(parent) = path.parent() {
|
||||
if !parent.exists() {
|
||||
let dir = parent.to_path_buf();
|
||||
// Attempt to create directory, ignore AlreadyExists errors
|
||||
if let Err(e) = fs::create_dir_all(&dir) {
|
||||
if e.kind() != std::io::ErrorKind::AlreadyExists {
|
||||
return Err(e.into());
|
||||
}
|
||||
}
|
||||
let dir = root
|
||||
.mkdir_all(
|
||||
parent,
|
||||
&std::fs::Permissions::from_mode(req.dir_mode & IMPLICIT_DIRECTORY_PERMISSION_MASK),
|
||||
)
|
||||
.context("mkdir_all parent")?
|
||||
.reopen(OpenFlags::O_DIRECTORY)
|
||||
.context("reopen parent")?;
|
||||
|
||||
// Set directory permissions and ownership
|
||||
std::fs::set_permissions(&dir, std::fs::Permissions::from_mode(req.dir_mode))?;
|
||||
unistd::chown(
|
||||
&dir,
|
||||
Some(Uid::from_raw(req.uid as u32)),
|
||||
Some(Gid::from_raw(req.gid as u32)),
|
||||
)?;
|
||||
}
|
||||
// TODO(burgerdev): why are we only applying this to the immediate parent?
|
||||
unistd::fchown(
|
||||
dir,
|
||||
Some(Uid::from_raw(req.uid as u32)),
|
||||
Some(Gid::from_raw(req.gid as u32)),
|
||||
)
|
||||
.context("fchown parent")?
|
||||
}
|
||||
|
||||
let sflag = stat::SFlag::from_bits_truncate(req.file_mode);
|
||||
|
||||
if sflag.contains(stat::SFlag::S_IFDIR) {
|
||||
// Remove existing non-directory file if present
|
||||
if path.exists() && !path.is_dir() {
|
||||
fs::remove_file(&path)?;
|
||||
}
|
||||
|
||||
fs::create_dir(&path).or_else(|e| {
|
||||
if e.kind() != std::io::ErrorKind::AlreadyExists {
|
||||
return Err(e);
|
||||
// Directories are somewhat special: for backwards compatibility, we need to preserve an
|
||||
// existing directory at path, so we can't just remove_all. Instead, we try to remove a
|
||||
// file and just don't propagate the error if it's a directory or doesn't exist.
|
||||
root.remove_file(path).or_else(|e| match e.kind() {
|
||||
pathrs::error::ErrorKind::OsError(Some(errno))
|
||||
if errno == libc::ENOENT || errno == libc::EISDIR =>
|
||||
{
|
||||
Ok(())
|
||||
}
|
||||
Ok(())
|
||||
_ => Err(e),
|
||||
})?;
|
||||
|
||||
std::fs::set_permissions(&path, std::fs::Permissions::from_mode(req.file_mode))?;
|
||||
// mkdir_all does not support the setuid/setgid/sticky bits, so we first create the
|
||||
// directory with the stricter mask and then change permissions with the correct mask.
|
||||
let dir = root
|
||||
.mkdir_all(
|
||||
path,
|
||||
&std::fs::Permissions::from_mode(
|
||||
req.file_mode & IMPLICIT_DIRECTORY_PERMISSION_MASK,
|
||||
),
|
||||
)
|
||||
.context("mkdir_all dir")?
|
||||
.reopen(OpenFlags::O_DIRECTORY)
|
||||
.context("reopen dir")?;
|
||||
dir.set_permissions(std::fs::Permissions::from_mode(
|
||||
req.file_mode & FILE_PERMISSION_MASK,
|
||||
))?;
|
||||
|
||||
unistd::chown(
|
||||
&path,
|
||||
unistd::fchown(
|
||||
dir,
|
||||
Some(Uid::from_raw(req.uid as u32)),
|
||||
Some(Gid::from_raw(req.gid as u32)),
|
||||
)?;
|
||||
)
|
||||
.context("fchown dir")?;
|
||||
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Remove any existing file if we're not resuming a chunked upload.
|
||||
if req.offset == 0 {
|
||||
// Remove anything that might already exist at the target location.
|
||||
// This is safe even for a symlink leaf, remove_all removes the named inode in its parent dir.
|
||||
root.remove_all(path).or_else(|e| match e.kind() {
|
||||
pathrs::error::ErrorKind::OsError(Some(errno)) if errno == libc::ENOENT => Ok(()),
|
||||
_ => Err(e),
|
||||
})?;
|
||||
}
|
||||
|
||||
// Handle symlink creation
|
||||
if sflag.contains(stat::SFlag::S_IFLNK) {
|
||||
// Clean up existing path (whether symlink, dir, or file)
|
||||
if path.exists() || path.is_symlink() {
|
||||
// Use appropriate removal method based on path type
|
||||
if path.is_symlink() {
|
||||
unistd::unlink(&path)?;
|
||||
} else if path.is_dir() {
|
||||
fs::remove_dir_all(&path)?;
|
||||
} else {
|
||||
fs::remove_file(&path)?;
|
||||
}
|
||||
}
|
||||
|
||||
// Create new symbolic link
|
||||
let symlink_target = PathBuf::from(OsStr::from_bytes(&req.data));
|
||||
// Use BorrowedFd to wrap AT_FDCWD for symlinkat
|
||||
let cwd_fd = unsafe { BorrowedFd::borrow_raw(libc::AT_FDCWD) };
|
||||
unistd::symlinkat(&symlink_target, cwd_fd, &path)?;
|
||||
root.create(path, &pathrs::InodeType::Symlink(symlink_target))
|
||||
.context("create symlink")?;
|
||||
|
||||
// Set symlink ownership (permissions not supported for symlinks)
|
||||
let path_str = CString::new(path.as_os_str().as_bytes())?;
|
||||
|
||||
let ret = unsafe { libc::lchown(path_str.as_ptr(), req.uid as u32, req.gid as u32) };
|
||||
Errno::result(ret).map(drop)?;
|
||||
// Set symlink ownership.
|
||||
// At the time of writing this, there was no API for creating the symlink and opening a
|
||||
// handle to the created inode. Best we can do is to resolve it again under the root and
|
||||
// hope that its still the same inode, but at least we guarantee that we're changing
|
||||
// ownership only within the shared directory.
|
||||
nix::unistd::fchownat(
|
||||
root,
|
||||
path,
|
||||
Some(Uid::from_raw(req.uid as u32)),
|
||||
Some(Gid::from_raw(req.gid as u32)),
|
||||
nix::fcntl::AtFlags::AT_SYMLINK_NOFOLLOW,
|
||||
)
|
||||
.context("fchownat")?;
|
||||
|
||||
// Symlinks don't have permissions on Linux!
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let mut tmpfile = path.clone();
|
||||
let mut tmpfile = path.to_path_buf();
|
||||
tmpfile.set_extension("tmp");
|
||||
|
||||
let file = OpenOptions::new()
|
||||
.write(true)
|
||||
.create(true)
|
||||
.truncate(req.offset == 0) // Only truncate when offset is 0
|
||||
.open(&tmpfile)?;
|
||||
// Write file content.
|
||||
let flags = if req.offset == 0 {
|
||||
OpenFlags::O_RDWR | OpenFlags::O_CREAT | OpenFlags::O_TRUNC
|
||||
} else {
|
||||
OpenFlags::O_RDWR | OpenFlags::O_CREAT
|
||||
};
|
||||
let file = root
|
||||
.create_file(
|
||||
&tmpfile,
|
||||
flags,
|
||||
&std::fs::Permissions::from_mode(req.file_mode & FILE_PERMISSION_MASK),
|
||||
)
|
||||
.context("create_file")?;
|
||||
file.write_all_at(req.data.as_slice(), req.offset as u64)
|
||||
.context("write_all_at")?;
|
||||
|
||||
file.write_all_at(req.data.as_slice(), req.offset as u64)?;
|
||||
let st = stat::stat(&tmpfile)?;
|
||||
// Check whether we're waiting for more data.
|
||||
|
||||
let st = nix::sys::stat::fstat(&file).context("fstat")?;
|
||||
if st.st_size != req.file_size {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
file.set_permissions(std::fs::Permissions::from_mode(req.file_mode))?;
|
||||
// Things like umask can change the permissions after create, make sure that they stay
|
||||
file.set_permissions(std::fs::Permissions::from_mode(
|
||||
req.file_mode & FILE_PERMISSION_MASK,
|
||||
))
|
||||
.context("set_permissions")?;
|
||||
|
||||
unistd::chown(
|
||||
&tmpfile,
|
||||
unistd::fchown(
|
||||
file,
|
||||
Some(Uid::from_raw(req.uid as u32)),
|
||||
Some(Gid::from_raw(req.gid as u32)),
|
||||
)?;
|
||||
)
|
||||
.context("fchown")?;
|
||||
|
||||
// Remove existing target path before rename
|
||||
if path.exists() || path.is_symlink() {
|
||||
if path.is_dir() {
|
||||
fs::remove_dir_all(&path)?;
|
||||
} else {
|
||||
fs::remove_file(&path)?;
|
||||
}
|
||||
}
|
||||
|
||||
fs::rename(tmpfile, path)?;
|
||||
nix::fcntl::renameat(&root, &tmpfile, &root, path).context("renameat")?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -2651,6 +2707,7 @@ mod tests {
|
||||
|
||||
use super::*;
|
||||
use crate::{namespace::Namespace, protocols::agent_ttrpc_async::AgentService as _};
|
||||
use anyhow::{bail, ensure};
|
||||
use nix::mount;
|
||||
use nix::sched::{unshare, CloneFlags};
|
||||
use oci::{
|
||||
@@ -3744,4 +3801,366 @@ COMMIT
|
||||
ids.sort();
|
||||
assert_eq!(ids, vec!["container-1", "container-2"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_do_copy_file() {
|
||||
let temp_dir = tempdir().expect("creating temp dir failed");
|
||||
// We start one directory deeper such that we catch problems when the shared directory does
|
||||
// not exist yet.
|
||||
let base = temp_dir.path().join("shared");
|
||||
|
||||
type Assertions = Box<dyn Fn(&Path) -> Result<()>>;
|
||||
struct TestCase {
|
||||
name: String,
|
||||
request: CopyFileRequest,
|
||||
assertions: Assertions,
|
||||
should_fail: bool,
|
||||
}
|
||||
|
||||
// Attention: these test cases depend on each other and can't be reordered.
|
||||
// The first few cases build up a directory structure that the subsequent tests then rely
|
||||
// on or try to exploit.
|
||||
// TODO(burgerdev): define a common directory structure for all tests up front.
|
||||
let tests = [
|
||||
TestCase {
|
||||
name: "Create a top-level file".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("f").to_string_lossy().into(),
|
||||
file_mode: 0o644 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let f = base.join("f");
|
||||
let f_stat = fs::metadata(&f).context("stat ./f failed")?;
|
||||
ensure!(f_stat.is_file());
|
||||
ensure!(0o644 == f_stat.permissions().mode() & 0o777);
|
||||
let content = std::fs::read_to_string(&f).context("read ./f failed")?;
|
||||
ensure!(content.is_empty());
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Writing a file onto an existing file replaces it".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("f").to_string_lossy().into(),
|
||||
file_mode: 0o600 | libc::S_IFREG,
|
||||
data: b"Hello!".to_vec(),
|
||||
file_size: 6,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let f = base.join("f");
|
||||
let f_stat = fs::metadata(&f).context("stat ./f failed")?;
|
||||
ensure!(f_stat.is_file());
|
||||
ensure!(0o600 == f_stat.permissions().mode() & 0o777);
|
||||
let content = std::fs::read_to_string(&f).context("read ./f failed")?;
|
||||
ensure!("Hello!" == content);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a file implicitly creates parent directories".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/b").to_string_lossy().into(),
|
||||
dir_mode: 0o755 | libc::S_IFDIR,
|
||||
file_mode: 0o644 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let a_stat = fs::metadata(base.join("a")).context("stat ./a failed")?;
|
||||
ensure!(a_stat.is_dir());
|
||||
ensure!(0o755 == a_stat.permissions().mode() & 0o777);
|
||||
let b_stat = fs::metadata(base.join("a/b")).context("stat ./a/b failed")?;
|
||||
ensure!(b_stat.is_file());
|
||||
ensure!(0o644 == b_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Create a file within an existing directory".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/c").to_string_lossy().into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that existing directories are not touched - we expect this to stay 0o755.
|
||||
file_mode: 0o621 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let a_stat = fs::metadata(base.join("a")).context("stat ./a failed")?;
|
||||
ensure!(a_stat.is_dir());
|
||||
ensure!(0o755 == a_stat.permissions().mode() & 0o777);
|
||||
let c_stat = fs::metadata(base.join("a/c")).context("stat ./a/c failed")?;
|
||||
ensure!(c_stat.is_file());
|
||||
ensure!(0o621 == c_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Create a directory".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/d").to_string_lossy().into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that the permissions are taken from file_mode.
|
||||
file_mode: 0o755 | libc::S_IFDIR,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let a_stat = fs::metadata(base.join("a")).context("stat ./a failed")?;
|
||||
ensure!(a_stat.is_dir());
|
||||
ensure!(0o755 == a_stat.permissions().mode() & 0o777);
|
||||
let d_stat = fs::metadata(base.join("a/d")).context("stat ./a/d failed")?;
|
||||
ensure!(d_stat.is_dir());
|
||||
ensure!(0o755 == d_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a dir onto an existing file replaces the file".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/b").to_string_lossy().into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that the permissions are taken from file_mode.
|
||||
file_mode: 0o755 | libc::S_IFDIR,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let b_stat = fs::metadata(base.join("a/b")).context("stat ./a/b failed")?;
|
||||
ensure!(b_stat.is_dir());
|
||||
ensure!(0o755 == b_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a file onto an existing dir replaces the dir".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/b").to_string_lossy().into(),
|
||||
dir_mode: 0o755 | libc::S_IFDIR,
|
||||
file_mode: 0o644 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let b_stat = fs::metadata(base.join("a/b")).context("stat ./a/b failed")?;
|
||||
ensure!(b_stat.is_file());
|
||||
ensure!(0o644 == b_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a dir onto an existing dir does not replace that dir".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a").to_string_lossy().into(),
|
||||
dir_mode: 0o755 | libc::S_IFDIR,
|
||||
file_mode: 0o751 | libc::S_IFDIR,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
// Check that a/b still exists
|
||||
let b_stat = fs::metadata(base.join("a/b")).context("stat ./a/b failed")?;
|
||||
ensure!(b_stat.is_file());
|
||||
let a_stat = fs::metadata(base.join("a")).context("stat ./a failed")?;
|
||||
ensure!(0o751 == a_stat.permissions().mode() & 0o777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Create a symlink".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/link").to_string_lossy().into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that the permissions are taken from file_mode.
|
||||
file_mode: 0o755 | libc::S_IFLNK,
|
||||
data: b"/etc/passwd".to_vec(),
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let link = base.join("a/link");
|
||||
let link_stat = nix::sys::stat::lstat(&link).context("stat ./a/link failed")?;
|
||||
// Linux symlinks have no permissions!
|
||||
ensure!(0o777 | libc::S_IFLNK == link_stat.st_mode);
|
||||
let target = fs::read_link(&link).context("read_link ./a/link failed")?;
|
||||
ensure!(target.to_string_lossy() == "/etc/passwd");
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Create a directory with setgid and sticky bit".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("x/y").to_string_lossy().into(),
|
||||
dir_mode: 0o3755 | libc::S_IFDIR,
|
||||
file_mode: 0o3770 | libc::S_IFDIR,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
// Implicitly created directories should not get a sticky bit.
|
||||
let x_stat = fs::metadata(base.join("x")).context("stat ./x failed")?;
|
||||
ensure!(x_stat.is_dir());
|
||||
ensure!(0o755 == x_stat.permissions().mode() & 0o7777);
|
||||
// Explicitly created directories should.
|
||||
let y_stat = fs::metadata(base.join("x/y")).context("stat ./x/y failed")?;
|
||||
ensure!(y_stat.is_dir());
|
||||
ensure!(0o3770 == y_stat.permissions().mode() & 0o7777);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Chunked upload 1".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("x/chunked").to_string_lossy().into(),
|
||||
dir_mode: 0o755 | libc::S_IFDIR,
|
||||
file_mode: 0o644 | libc::S_IFREG,
|
||||
offset: 0,
|
||||
file_size: 11,
|
||||
data: b"Hello ".to_vec(),
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
ensure!(
|
||||
!(fs::exists(base.join("x/chunked"))
|
||||
.context("exists ./x/chunked failed")?)
|
||||
);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Chunked upload 2".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("x/chunked").to_string_lossy().into(),
|
||||
dir_mode: 0o755 | libc::S_IFDIR,
|
||||
file_mode: 0o644 | libc::S_IFREG,
|
||||
offset: 6,
|
||||
file_size: 11,
|
||||
data: b"World".to_vec(),
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let content = std::fs::read(base.join("x/chunked"))?;
|
||||
println!("{:?}", content);
|
||||
ensure!(b"Hello World".to_vec() == content);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
// =================================
|
||||
// Below are some adversarial tests.
|
||||
// =================================
|
||||
TestCase {
|
||||
name: "Malicious intermediate directory is a symlink".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base
|
||||
.join("a/link/this-could-just-be-shadow-but-I-am-not-risking-it")
|
||||
.to_string_lossy()
|
||||
.into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that the permissions are taken from file_mode.
|
||||
file_mode: 0o755 | libc::S_IFLNK,
|
||||
data: b"root:password:19000:0:99999:7:::\n".to_vec(),
|
||||
file_size: 33,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: true,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
let link_stat = nix::sys::stat::lstat(&base.join("a/link"))
|
||||
.context("stat ./a/link failed")?;
|
||||
ensure!(0o777 | libc::S_IFLNK == link_stat.st_mode);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a symlink onto an existing symlink should replace the symlink, not follow it".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/link").to_string_lossy().into(),
|
||||
dir_mode: 0o700 | libc::S_IFDIR, // Test that the permissions are taken from file_mode.
|
||||
file_mode: 0o755 | libc::S_IFLNK,
|
||||
data: b"/etc".to_vec(),
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
// The symlink should be created at the same place (not followed), with the new content.
|
||||
let a_stat = fs::metadata(base.join("a")).context("stat ./a failed")?;
|
||||
ensure!(a_stat.is_dir());
|
||||
ensure!(0o751 == a_stat.permissions().mode() & 0o777);
|
||||
let link = base.join("a/link");
|
||||
let link_stat = nix::sys::stat::lstat(&link).context("stat ./a/link failed")?;
|
||||
// Linux symlinks have no permissions!
|
||||
ensure!(0o777 | libc::S_IFLNK == link_stat.st_mode);
|
||||
let target = fs::read_link(&link).context("read_link ./a/link failed")?;
|
||||
ensure!(target.to_string_lossy() == "/etc");
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Creating a file at an existing symlink replaces the link and does not follow it".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("a/link").to_string_lossy().into(),
|
||||
file_mode: 0o600 | libc::S_IFREG,
|
||||
data: b"Hello!".to_vec(),
|
||||
file_size: 6,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: false,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
// The symlink itself should be replaced with the file, not followed.
|
||||
let link = base.join("a/link");
|
||||
let link_stat = nix::sys::stat::lstat(&link).context("stat ./a/link failed")?;
|
||||
ensure!(0o600 | libc::S_IFREG == link_stat.st_mode);
|
||||
let content = std::fs::read_to_string(&link).context("read ./a/link failed")?;
|
||||
ensure!("Hello!" == content);
|
||||
Ok(())
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Writing outside the shared directory is rejected".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.parent().unwrap().join("not-shared").to_string_lossy().into(),
|
||||
file_mode: 0o600 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: true,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
match fs::metadata(base.parent().unwrap().join("not-shared")) {
|
||||
Ok(_) => bail!("successful write outside shared directory"),
|
||||
Err(_) => Ok(())
|
||||
}
|
||||
}),
|
||||
},
|
||||
TestCase {
|
||||
name: "Traversal outside shared directory is rejected".into(),
|
||||
request: CopyFileRequest {
|
||||
path: base.join("../not-shared").to_string_lossy().into(),
|
||||
file_mode: 0o600 | libc::S_IFREG,
|
||||
..Default::default()
|
||||
},
|
||||
should_fail: true,
|
||||
assertions: Box::new(|base| -> Result<()> {
|
||||
match fs::metadata(base.join("../not-shared")) {
|
||||
Ok(_) => bail!("successful write outside shared directory"),
|
||||
Err(_) => Ok(())
|
||||
}
|
||||
}),
|
||||
},
|
||||
];
|
||||
|
||||
let uid = unistd::getuid().as_raw() as i32;
|
||||
let gid = unistd::getgid().as_raw() as i32;
|
||||
|
||||
for mut tc in tests {
|
||||
println!("Running test case: {}", tc.name);
|
||||
// Since we're in a unit test, using root ownership causes issues with cleaning the temp dir.
|
||||
tc.request.uid = uid;
|
||||
tc.request.gid = gid;
|
||||
|
||||
let res = do_copy_file(&tc.request, &base);
|
||||
if tc.should_fail != res.is_err() {
|
||||
panic!("{}: unexpected do_copy_file result: {:?}", tc.name, res)
|
||||
}
|
||||
(tc.assertions)(&base).context(tc.name).unwrap()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,32 +11,32 @@ setup_file() {
|
||||
info "setup"
|
||||
}
|
||||
|
||||
@test "Test CopyFile API: Copy a file to /run/kata-containers" {
|
||||
info "Copy file to /run/kata-containers"
|
||||
@test "Test CopyFile API: Copy a file to /run/kata-containers/shared/containers" {
|
||||
info "Copy file to /run/kata-containers/shared/containers"
|
||||
src_file=$(mktemp)
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/foo\"}'")
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/shared/containers/foo\"}'")
|
||||
run_agent_ctl "${cmds[@]}"
|
||||
rm $src_file
|
||||
}
|
||||
|
||||
@test "Test CopyFile API: Copy a symlink to /run/kata-containers" {
|
||||
info "Copy symlink to /run/kata-containers"
|
||||
@test "Test CopyFile API: Copy a symlink to /run/kata-containers/shared/containers" {
|
||||
info "Copy symlink to /run/kata-containers/shared/containers"
|
||||
src_file=$(mktemp)
|
||||
link_file="/tmp/link"
|
||||
ln -s $src_file $link_file
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$link_file\", \"dest\":\"/run/kata-containers/link\"}'")
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$link_file\", \"dest\":\"/run/kata-containers/shared/containers/link\"}'")
|
||||
run_agent_ctl "${cmds[@]}"
|
||||
unlink $link_file
|
||||
rm $src_file
|
||||
}
|
||||
|
||||
@test "Test CopyFile API: Copy a directory to /run/kata-containers" {
|
||||
info "Copy directory to /run/kata-containers"
|
||||
@test "Test CopyFile API: Copy a directory to /run/kata-containers/shared/containers" {
|
||||
info "Copy directory to /run/kata-containers/shared/containers"
|
||||
src_dir=$(mktemp -d)
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_dir\", \"dest\":\"/run/kata-containers/dir\"}'")
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_dir\", \"dest\":\"/run/kata-containers/shared/containers/dir\"}'")
|
||||
run_agent_ctl "${cmds[@]}"
|
||||
rmdir $src_dir
|
||||
}
|
||||
@@ -51,17 +51,30 @@ setup_file() {
|
||||
rm $src_file
|
||||
}
|
||||
|
||||
@test "Test CopyFile API: Copy a large file to /run/kata-containers" {
|
||||
info "Copy large file to /run/kata-containers"
|
||||
@test "Test CopyFile API: Copy a file to an unallowed destination beneath /run/kata-containers" {
|
||||
# This is a regression test, copying files to /run/kata-containers used to be allowed, but the
|
||||
# implementation is more strict now and requires paths to be under
|
||||
# /run/kata-containers/shared/containers.
|
||||
info "Copy file to /tmp"
|
||||
src_file=$(mktemp)
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/shared/foo\"}'")
|
||||
run run_agent_ctl "${cmds[@]}"
|
||||
[ "$status" -ne 0 ]
|
||||
rm $src_file
|
||||
}
|
||||
|
||||
@test "Test CopyFile API: Copy a large file to /run/kata-containers/shared/containers" {
|
||||
info "Copy large file to /run/kata-containers/shared/containers"
|
||||
src_file="/tmp/large_file_2M.txt"
|
||||
truncate -s 2M $src_file
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/large_file_2M.txt\"}'")
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/shared/containers/large_file_2M.txt\"}'")
|
||||
run_agent_ctl "${cmds[@]}"
|
||||
rm $src_file
|
||||
}
|
||||
|
||||
teardown_file() {
|
||||
info "teardown"
|
||||
sudo rm -r /run/kata-containers/ || echo "Failed to clean /run/kata-containers"
|
||||
sudo rm -r /run/kata-containers/shared/containers/ || echo "Failed to clean /run/kata-containers/shared/containers"
|
||||
}
|
||||
|
||||
@@ -30,7 +30,7 @@ setup_file() {
|
||||
|
||||
src_file=$(mktemp)
|
||||
local cmds=()
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/foo\"}'")
|
||||
cmds+=("-c 'CopyFile json://{\"src\": \"$src_file\", \"dest\":\"/run/kata-containers/shared/containers/foo\"}'")
|
||||
run run_agent_ctl "${cmds[@]}"
|
||||
[ "$status" -ne 0 ]
|
||||
|
||||
|
||||
Reference in New Issue
Block a user