Skip to content

Commit

Permalink
feat: go deeper when whitelisting with TemporaryFileSystem
Browse files Browse the repository at this point in the history
  • Loading branch information
desbma committed Feb 5, 2025
1 parent 191fb61 commit d8b6ac5
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 14 deletions.
9 changes: 8 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ fn main() -> anyhow::Result<()> {

#[cfg(not(feature = "gen-man-pages"))]
fn main() -> anyhow::Result<()> {
use std::env;

// Init logger
simple_logger::SimpleLogger::new()
.with_level(if cfg!(debug_assertions) {
Expand Down Expand Up @@ -100,9 +102,14 @@ fn main() -> anyhow::Result<()> {
}
});

// Get & parse PATH env var
let env_paths: Vec<_> = env::var_os("PATH")
.map(|ev| env::split_paths(&ev).collect())
.unwrap_or_default();

// Summarize actions
let logs = st.log_lines()?;
let actions = summarize::summarize(logs)?;
let actions = summarize::summarize(logs, &env_paths)?;
log::debug!("{actions:?}");

if let Some(profile_data_path) = profile_data_path {
Expand Down
27 changes: 22 additions & 5 deletions src/summarize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ struct ProgramState {
cur_dir: Option<PathBuf>,
}

pub(crate) fn summarize<I>(syscalls: I) -> anyhow::Result<Vec<ProgramAction>>
pub(crate) fn summarize<I>(syscalls: I, env_paths: &[PathBuf]) -> anyhow::Result<Vec<ProgramAction>>
where
I: IntoIterator<Item = anyhow::Result<Syscall>>,
{
Expand Down Expand Up @@ -637,6 +637,9 @@ where
// Create single action with all syscalls for efficient handling of seccomp filters
actions.push(ProgramAction::Syscalls(stats.keys().cloned().collect()));

// Directories in PATH env var need to be accessible, otherwise systemd errors
actions.extend(env_paths.iter().cloned().map(ProgramAction::Read));

// Report stats
let mut syscall_names = stats.keys().collect::<Vec<_>>();
syscall_names.sort_unstable();
Expand All @@ -661,6 +664,11 @@ mod tests {
fn test_relative_rename() {
let _ = simple_logger::SimpleLogger::new().init();

let env_paths = [
PathBuf::from("/path/from/env/1"),
PathBuf::from("/path/from/env/2"),
];

let temp_dir_src = tempfile::tempdir().unwrap();
let temp_dir_dst = tempfile::tempdir().unwrap();
let syscalls = [Ok(Syscall {
Expand Down Expand Up @@ -695,13 +703,15 @@ mod tests {
},
})];
assert_eq!(
summarize(syscalls).unwrap(),
summarize(syscalls, &env_paths).unwrap(),
vec![
ProgramAction::Read(temp_dir_src.path().join("a")),
ProgramAction::Write(temp_dir_src.path().join("a")),
ProgramAction::Create(temp_dir_dst.path().join("b")),
ProgramAction::Write(temp_dir_dst.path().join("b")),
ProgramAction::Syscalls(["renameat".to_owned()].into())
ProgramAction::Syscalls(["renameat".to_owned()].into()),
ProgramAction::Read("/path/from/env/1".into()),
ProgramAction::Read("/path/from/env/2".into()),
]
);
}
Expand All @@ -710,6 +720,11 @@ mod tests {
fn test_connect_uds() {
let _ = simple_logger::SimpleLogger::new().init();

let env_paths = [
PathBuf::from("/path/from/env/1"),
PathBuf::from("/path/from/env/2"),
];

let syscalls = [Ok(Syscall {
pid: 598056,
rel_ts: 0.000036,
Expand Down Expand Up @@ -746,10 +761,12 @@ mod tests {
},
})];
assert_eq!(
summarize(syscalls).unwrap(),
summarize(syscalls, &env_paths).unwrap(),
vec![
ProgramAction::Read("/run/user/1000/systemd/private".into()),
ProgramAction::Syscalls(["connect".to_owned()].into())
ProgramAction::Syscalls(["connect".to_owned()].into()),
ProgramAction::Read("/path/from/env/1".into()),
ProgramAction::Read("/path/from/env/2".into()),
]
);
}
Expand Down
57 changes: 49 additions & 8 deletions src/systemd/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use std::{
borrow::ToOwned,
collections::{HashMap, HashSet},
env, fmt, fs, iter,
fmt, fs, iter,
num::NonZeroUsize,
os::unix::ffi::OsStrExt as _,
path::{Path, PathBuf},
Expand Down Expand Up @@ -1386,16 +1386,10 @@ pub(crate) fn build_options(
.and_then(|v| {
// Don't make those inaccessible, systemd won't be able to start anything
let excluded_run_dirs = [Path::new("/run"), Path::new("/proc")];
// Directories in PATH env var need to be accessible, otherwise systemd errors
let env_paths: Vec<_> = env::var_os("PATH")
.map(|ev| env::split_paths(&ev).collect())
.unwrap_or_default();
v.into_iter()
.filter(|e| !excluded_run_dirs.contains(&e.path().as_ref()))
// systemd follows symlinks when applying option, so exclude them
.filter(|e| e.file_type().is_ok_and(|t| !t.is_symlink()))
// exclude paths wich are below paths in PATH
.filter(|e| !env_paths.iter().any(|ep| ep.starts_with(e.path())))
.map(|e| e.path().to_str().map(ToOwned::to_owned))
.collect()
})
Expand Down Expand Up @@ -1445,7 +1439,7 @@ pub(crate) fn build_options(
base,
exceptions,
}) => {
// This will be reached only when first transforming an initial InacessiblePaths option (RemovePath) into
// This will be reached only when first transforming an initial InaccessiblePaths option (RemovePath) into
// less restrictive EmptyPaths + exceptions
assert!(exceptions.is_empty());
let new_exception_path = if action_path
Expand Down Expand Up @@ -1476,6 +1470,53 @@ pub(crate) fn build_options(
exceptions_rw,
}))
}
OptionValueEffect::EmptyPath(EmptyPathDescription {
base,
base_ro,
exceptions_ro,
exceptions_rw,
}) => {
let mut new_exceptions_ro =
Vec::with_capacity(exceptions_ro.len() + usize::from(action_ro));
new_exceptions_ro.extend(exceptions_ro.iter().cloned());
let mut new_exceptions_rw =
Vec::with_capacity(exceptions_rw.len() + usize::from(!action_ro));
new_exceptions_rw.extend(exceptions_rw.iter().cloned());
let mut base_ro = *base_ro;
let new_exception_path = if action_path
.symlink_metadata()
.is_ok_and(|m| m.file_type().is_symlink())
{
// systemd follows symlinks, so won't bind mount the symlink,
// add exception for parent instead
action_path
.parent()
.map(Path::to_path_buf)
.unwrap_or(action_path)
} else {
action_path
};
if matches!(action, ProgramAction::Create(_))
&& new_exception_path == *base
{
base_ro = false;
} else {
if base.starts_with(&new_exception_path) {
return None;
}
if action_ro {
new_exceptions_ro.push(new_exception_path);
} else {
new_exceptions_rw.push(new_exception_path);
}
}
Some(OptionValueEffect::EmptyPath(EmptyPathDescription {
base: base.to_owned(),
base_ro,
exceptions_ro: new_exceptions_ro,
exceptions_rw: new_exceptions_rw,
}))
}
_ => None,
}
},
Expand Down

0 comments on commit d8b6ac5

Please sign in to comment.