From 2263ab4e015c82cd6b8286d48626f81e59c72b50 Mon Sep 17 00:00:00 2001 From: desbma Date: Fri, 17 Jan 2025 19:09:25 +0100 Subject: [PATCH] feat: filesystem exception whitelist merging --- src/cl.rs | 15 ++++- src/main.rs | 4 +- src/systemd/options.rs | 126 ++++++++++++++++++++++++++++++++++++---- src/systemd/resolver.rs | 68 +++++++++++++--------- 4 files changed, 169 insertions(+), 44 deletions(-) diff --git a/src/cl.rs b/src/cl.rs index 7434c7b..9c8e336 100644 --- a/src/cl.rs +++ b/src/cl.rs @@ -1,6 +1,6 @@ //! Command line interface -use std::path::PathBuf; +use std::{num::NonZeroUsize, path::PathBuf}; use clap::Parser; @@ -35,6 +35,10 @@ pub(crate) struct HardeningOptions { /// Enable whitelist-based filesystem hardening #[arg(short = 'w', long, default_value_t)] pub filesystem_whitelisting: bool, + /// When using whitelist-based filesystem hardening, if path whitelist is longer than this value, + /// try to merge paths with the same parent + #[arg(long, default_value = "5")] + pub merge_paths_threshold: NonZeroUsize, } impl HardeningOptions { @@ -45,6 +49,8 @@ impl HardeningOptions { mode: HardeningMode::Safe, network_firewalling: false, filesystem_whitelisting: false, + #[expect(clippy::unwrap_used)] + merge_paths_threshold: NonZeroUsize::new(1).unwrap(), } } @@ -54,19 +60,22 @@ impl HardeningOptions { mode: HardeningMode::Aggressive, network_firewalling: true, filesystem_whitelisting: true, + #[expect(clippy::unwrap_used)] + merge_paths_threshold: NonZeroUsize::new(usize::MAX).unwrap(), } } pub(crate) fn to_cmdline(&self) -> String { format!( - "-m {}{}{}", + "-m {}{}{} --merge-paths-threshold {}", self.mode, if self.network_firewalling { " -n" } else { "" }, if self.filesystem_whitelisting { " -w" } else { "" - } + }, + self.merge_paths_threshold ) } } diff --git a/src/main.rs b/src/main.rs index af1eec7..921bfa6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -112,7 +112,7 @@ fn main() -> anyhow::Result<()> { bincode::serialize_into(file, &actions)?; } else { // Resolve - let resolved_opts = systemd::resolve(&sd_opts, &actions); + let resolved_opts = systemd::resolve(&sd_opts, &actions, &hardening_opts); // Report systemd::report_options(resolved_opts); @@ -136,7 +136,7 @@ fn main() -> anyhow::Result<()> { log::debug!("{actions:?}"); // Resolve - let resolved_opts = systemd::resolve(&sd_opts, &actions); + let resolved_opts = systemd::resolve(&sd_opts, &actions, &hardening_opts); // Report systemd::report_options(resolved_opts); diff --git a/src/systemd/options.rs b/src/systemd/options.rs index fb27035..e8e3f13 100644 --- a/src/systemd/options.rs +++ b/src/systemd/options.rs @@ -6,6 +6,7 @@ use std::{ borrow::ToOwned, collections::{HashMap, HashSet}, fmt, iter, + num::NonZeroUsize, os::unix::ffi::OsStrExt as _, path::{Path, PathBuf}, str::FromStr, @@ -27,9 +28,10 @@ use crate::{ #[derive(Debug)] pub(crate) struct OptionUpdater { /// Generate a new option effect compatible with the previously incompatible action - pub effect: fn(&OptionValueEffect, &ProgramAction) -> Option, + pub effect: + fn(&OptionValueEffect, &ProgramAction, &HardeningOptions) -> Option, /// Generate new options from the new effect - pub options: fn(&OptionValueEffect) -> Vec, + pub options: fn(&OptionValueEffect, &HardeningOptions) -> Vec, } /// Systemd option with its possibles values, and their effect @@ -812,6 +814,49 @@ static SYSCALL_CLASSES: LazyLock>> = ]) }); +fn merge_similar_paths(paths: &[PathBuf], threshold: NonZeroUsize) -> Vec { + if paths.len() <= threshold.get() { + paths.to_vec() + } else { + let mut children: HashMap> = HashMap::new(); + for path in paths { + let ancestors: Vec<_> = path.ancestors().map(Path::to_path_buf).collect(); + let mut parent: Option = None; + for dir in ancestors.into_iter().rev() { + if let Some(parent) = parent.as_ref() { + children + .entry(parent.to_owned()) + .or_default() + .insert(dir.clone()); + } + parent = Some(dir); + } + } + let initial_candidates = vec![PathBuf::from("/")]; + let mut candidates = initial_candidates.clone(); + loop { + let mut advancing = false; + let mut new_candidates = Vec::with_capacity(candidates.len()); + for candidate in &candidates { + if let Some(candidate_children) = children.get(candidate) { + new_candidates.extend(candidate_children.iter().cloned()); + advancing |= !candidate_children.is_empty(); + } + } + if !advancing || new_candidates.len() > threshold.get() { + break; + } + candidates = new_candidates; + } + if candidates == initial_candidates { + paths.to_vec() + } else { + candidates.sort_unstable(); + candidates + } + } +} + #[expect(clippy::too_many_lines)] pub(crate) fn build_options( systemd_version: &SystemdVersion, @@ -1121,7 +1166,7 @@ pub(crate) fn build_options( })), }], updater: Some(OptionUpdater { - effect: |effect, action| match effect { + effect: |effect, action, _hopts| match effect { OptionValueEffect::DenyWrite(PathDescription::Base { base, exceptions }) => { let new_exception = match action { ProgramAction::Write(action_path) => Some(action_path.to_owned()), @@ -1145,7 +1190,7 @@ pub(crate) fn build_options( } _ => None, }, - options: |effect| match effect { + options: |effect, hopts| match effect { OptionValueEffect::DenyWrite(PathDescription::Base { base, exceptions }) => { vec![ OptionWithValue { @@ -1156,10 +1201,13 @@ pub(crate) fn build_options( OptionWithValue { name: "ReadWritePaths".to_owned(), value: OptionValue::List { - values: exceptions - .iter() - .filter_map(|p| p.to_str().map(ToOwned::to_owned)) - .collect(), + values: merge_similar_paths( + exceptions, + hopts.merge_paths_threshold, + ) + .iter() + .filter_map(|p| p.to_str().map(ToOwned::to_owned)) + .collect(), value_if_empty: None, prefix: "", repeat_option: false, @@ -1330,7 +1378,7 @@ pub(crate) fn build_options( ), }], updater: hardening_opts.network_firewalling.then_some(OptionUpdater { - effect: |e, a| { + effect: |e, a, _| { let OptionValueEffect::DenyAction(ProgramAction::NetworkActivity(effect_na)) = e else { unreachable!(); @@ -1353,7 +1401,7 @@ pub(crate) fn build_options( }), )) }, - options: |e| { + options: |e, _| { let OptionValueEffect::DenyAction(ProgramAction::NetworkActivity(denied_na)) = e else { unreachable!(); @@ -1640,3 +1688,61 @@ pub(crate) fn build_options( log::debug!("{options:#?}"); options } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_merge_similar_paths() { + assert_eq!( + merge_similar_paths( + &[ + PathBuf::from("/a/ab/ab1"), + PathBuf::from("/a/ab/ab2"), + PathBuf::from("/a/ab/ab3"), + PathBuf::from("/a/ab/ab4/abc") + ], + NonZeroUsize::new(2).unwrap() + ), + vec![PathBuf::from("/a/ab")] + ); + assert_eq!( + merge_similar_paths( + &[ + PathBuf::from("/a1/ab/ab1"), + PathBuf::from("/a2/ab/ab2"), + PathBuf::from("/a3/ab/ab3") + ], + NonZeroUsize::new(2).unwrap() + ), + vec![ + PathBuf::from("/a1/ab/ab1"), + PathBuf::from("/a2/ab/ab2"), + PathBuf::from("/a3/ab/ab3") + ] + ); + assert_eq!( + merge_similar_paths( + &[ + PathBuf::from("/a/aa/ab1"), + PathBuf::from("/a/ab/ab2"), + PathBuf::from("/a/ac/ab3") + ], + NonZeroUsize::new(2).unwrap() + ), + vec![PathBuf::from("/a")] + ); + assert_eq!( + merge_similar_paths( + &[ + PathBuf::from("/a/aa/ab1"), + PathBuf::from("/a/aa/ab2"), + PathBuf::from("/a/ab/ab3") + ], + NonZeroUsize::new(2).unwrap() + ), + vec![PathBuf::from("/a/aa"), PathBuf::from("/a/ab")] + ); + } +} diff --git a/src/systemd/resolver.rs b/src/systemd/resolver.rs index 5abb0b4..3c7ef8e 100644 --- a/src/systemd/resolver.rs +++ b/src/systemd/resolver.rs @@ -3,6 +3,7 @@ use itertools::Itertools as _; use crate::{ + cl::HardeningOptions, summarize::{NetworkActivity, ProgramAction}, systemd::options::{ ListMode, OptionDescription, OptionEffect, OptionValue, OptionValueEffect, OptionWithValue, @@ -17,6 +18,7 @@ impl OptionValueEffect { action: &ProgramAction, prev_actions: &[ProgramAction], updater: Option<&OptionUpdater>, + hardening_opts: &HardeningOptions, ) -> ActionOptionEffectCompatibility { let compatible = match self { OptionValueEffect::DenyAction(denied) => match denied { @@ -70,23 +72,21 @@ impl OptionValueEffect { true } } - OptionValueEffect::Multiple(effects) => { - effects - .iter() - .all(|e| match e.compatible(action, prev_actions, None) { - ActionOptionEffectCompatibility::Compatible => true, - ActionOptionEffectCompatibility::CompatibleIfChanged(_) => todo!(), - ActionOptionEffectCompatibility::Incompatible => false, - }) - } + OptionValueEffect::Multiple(effects) => effects.iter().all(|e| { + match e.compatible(action, prev_actions, None, hardening_opts) { + ActionOptionEffectCompatibility::Compatible => true, + ActionOptionEffectCompatibility::CompatibleIfChanged(_) => todo!(), + ActionOptionEffectCompatibility::Incompatible => false, + } + }), }; if compatible { ActionOptionEffectCompatibility::Compatible } else if let Some(updater) = updater { - if let Some(new_eff) = (updater.effect)(self, action) { + if let Some(new_eff) = (updater.effect)(self, action, hardening_opts) { ActionOptionEffectCompatibility::CompatibleIfChanged( ChangedOptionValueDescription { - new_options: (updater.options)(&new_eff), + new_options: (updater.options)(&new_eff, hardening_opts), effect: new_eff, }, ) @@ -127,11 +127,12 @@ pub(crate) fn actions_compatible( eff: &OptionValueEffect, actions: &[ProgramAction], updater: Option<&OptionUpdater>, + hardening_opts: &HardeningOptions, ) -> ActionOptionEffectCompatibility { let mut changed_desc: Option = None; for i in 0..actions.len() { let cur_eff = changed_desc.as_ref().map_or(eff, |d| &d.effect); - match cur_eff.compatible(&actions[i], &actions[..i], updater) { + match cur_eff.compatible(&actions[i], &actions[..i], updater, hardening_opts) { ActionOptionEffectCompatibility::Compatible => {} ActionOptionEffectCompatibility::CompatibleIfChanged(new_desc) => { log::debug!( @@ -163,6 +164,7 @@ pub(crate) fn actions_compatible( pub(crate) fn resolve( opts: &Vec, actions: &[ProgramAction], + hardening_opts: &HardeningOptions, ) -> Vec { let mut candidates = Vec::new(); for opt in opts { @@ -178,7 +180,8 @@ pub(crate) fn resolve( break; } OptionEffect::Simple(effect) => { - match actions_compatible(effect, actions, opt.updater.as_ref()) { + match actions_compatible(effect, actions, opt.updater.as_ref(), hardening_opts) + { ActionOptionEffectCompatibility::Compatible => { candidates.push(OptionWithValue { name: opt.name.to_owned(), @@ -206,8 +209,12 @@ pub(crate) fn resolve( debug_assert_eq!(values.len(), effects.len()); let mut cur_effects = effects.clone(); for (optv, opte) in values.iter().zip(&mut cur_effects) { - let compatible = - actions_compatible(opte, actions, opt.updater.as_ref()); + let compatible = actions_compatible( + opte, + actions, + opt.updater.as_ref(), + hardening_opts, + ); let mut cur_opt_vals = vec![optv.to_owned()]; let enable_opt = match mode { ListMode::WhiteList => matches!( @@ -220,7 +227,7 @@ pub(crate) fn resolve( nd, ) => { *opte = nd.effect; - match actions_compatible(opte, actions, None) { + match actions_compatible(opte, actions, None, hardening_opts) { ActionOptionEffectCompatibility::Compatible => { match nd.new_options.iter().at_most_one() { Ok(Some(OptionWithValue { name, value: OptionValue::List { values: new_vals, .. } })) if name == opt.name => { @@ -286,52 +293,54 @@ mod tests { #[test] fn test_resolve_protect_system() { let _ = simple_logger::SimpleLogger::new().init(); + let hardening_opts = HardeningOptions::safe(); let opts = test_options(&["ProtectSystem"]); let actions = vec![]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectSystem=strict"); let actions = vec![ProgramAction::Write("/sys/whatever".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectSystem=strict"); let actions = vec![ProgramAction::Write("/var/cache/whatever".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectSystem=full"); let actions = vec![ProgramAction::Write("/etc/plop.conf".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectSystem=true"); let actions = vec![ProgramAction::Write("/usr/bin/false".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 0); } #[test] fn test_resolve_protect_home() { let _ = simple_logger::SimpleLogger::new().init(); + let hardening_opts = HardeningOptions::safe(); let opts = test_options(&["ProtectHome"]); let actions = vec![]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectHome=tmpfs"); let actions = vec![ProgramAction::Write("/home/user/data".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectHome=true"); let actions = vec![ProgramAction::Read("/home/user/data".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectHome=read-only"); @@ -339,7 +348,7 @@ mod tests { ProgramAction::Create("/home/user/data".into()), ProgramAction::Read("/home/user/data".into()), ]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "ProtectHome=true"); } @@ -347,28 +356,29 @@ mod tests { #[test] fn test_resolve_private_tmp() { let _ = simple_logger::SimpleLogger::new().init(); + let hardening_opts = HardeningOptions::safe(); let opts = test_options(&["PrivateTmp"]); let actions = vec![]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "PrivateTmp=true"); let actions = vec![ProgramAction::Write("/tmp/data".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "PrivateTmp=true"); let actions = vec![ProgramAction::Read("/tmp/data".into())]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 0); let actions = vec![ ProgramAction::Create("/tmp/data".into()), ProgramAction::Read("/tmp/data".into()), ]; - let candidates = resolve(&opts, &actions); + let candidates = resolve(&opts, &actions, &hardening_opts); assert_eq!(candidates.len(), 1); assert_eq!(format!("{}", candidates[0]), "PrivateTmp=true"); }