diff --git a/src/defaults/settings_dsl.rs b/src/defaults/settings_dsl.rs index 8924d82f2..c5db17076 100644 --- a/src/defaults/settings_dsl.rs +++ b/src/defaults/settings_dsl.rs @@ -72,14 +72,14 @@ macro_rules! modifier_of { .ok() .filter(|val| ($first ..= $last).contains(val)) .map(|i| { - Box::new(move |obj: &mut Settings| obj.$id = i) as Box + Box::new(move |obj: &mut Settings| obj.$id = i) as SettingsModifier }) }) }; ($id:ident, =int $fn: expr; $value: expr) => { $crate::defaults::SettingKind::Integer(|text| { $fn(&text).map(|i| { - Box::new(move |obj: &mut Settings| obj.$id = i) as Box + Box::new(move |obj: &mut Settings| obj.$id = i) as SettingsModifier }) }) }; diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 1f62821e0..9b87e5b95 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -48,7 +48,7 @@ fn read_sudoers() -> Result { Ok(sudoers) } -fn judge(policy: Sudoers, context: &Context) -> Result { +fn judge(mut policy: Sudoers, context: &Context) -> Result { Ok(policy.check( &*context.current_user, &context.hostname, @@ -63,7 +63,7 @@ fn judge(policy: Sudoers, context: &Context) -> Result { impl Pipeline { pub fn run(mut self, cmd_opts: SudoRunOptions) -> Result<(), Error> { - let policy = read_sudoers()?; + let mut policy = read_sudoers()?; let (ctx_opts, pipe_opts) = cmd_opts.into(); @@ -73,7 +73,7 @@ impl Pipeline { ) } - let mut context = Context::build_from_options(ctx_opts, policy.secure_path())?; + let mut context = Context::build_from_options(ctx_opts, policy.search_path())?; let policy = judge(policy, &context)?; @@ -136,8 +136,8 @@ impl Pipeline { } pub fn run_validate(mut self, cmd_opts: SudoValidateOptions) -> Result<(), Error> { - let policy = read_sudoers()?; - let mut context = Context::build_from_options(cmd_opts.into(), policy.secure_path())?; + let mut policy = read_sudoers()?; + let mut context = Context::build_from_options(cmd_opts.into(), policy.search_path())?; match policy.validate_authorization() { Authorization::Forbidden => { diff --git a/src/sudo/pipeline/list.rs b/src/sudo/pipeline/list.rs index c6e9262f2..42acba547 100644 --- a/src/sudo/pipeline/list.rs +++ b/src/sudo/pipeline/list.rs @@ -23,9 +23,9 @@ impl Pipeline { let original_command = cmd_opts.positional_args.first().cloned(); - let sudoers = super::read_sudoers()?; + let mut sudoers = super::read_sudoers()?; - let mut context = Context::build_from_options(cmd_opts.into(), sudoers.secure_path())?; + let mut context = Context::build_from_options(cmd_opts.into(), sudoers.search_path())?; if original_command.is_some() && !context.command.resolved { return Err(Error::CommandNotFound(context.command.command)); @@ -43,7 +43,7 @@ impl Pipeline { } if let Some(original_command) = original_command { - check_sudo_command_perms(&original_command, &context, &other_user, &sudoers)?; + check_sudo_command_perms(&original_command, &context, &other_user, &mut sudoers)?; } else { let invoking_user = other_user.as_ref().unwrap_or(&context.current_user); println_ignore_io_error!( @@ -148,7 +148,7 @@ fn check_sudo_command_perms( original_command: &str, context: &Context, other_user: &Option, - sudoers: &Sudoers, + sudoers: &mut Sudoers, ) -> Result<(), Error> { let user = other_user.as_ref().unwrap_or(&context.current_user); diff --git a/src/sudoers/ast.rs b/src/sudoers/ast.rs index 93d55e8fe..43b9cda5f 100644 --- a/src/sudoers/ast.rs +++ b/src/sudoers/ast.rs @@ -32,6 +32,22 @@ impl Qualified { pub type Spec = Qualified>; pub type SpecList = Vec>; +/// A generic mapping function (only used for turning `Spec` into `Spec`) +impl Spec { + pub fn map(self, f: impl Fn(T) -> U) -> Spec { + let transform = |meta| match meta { + Meta::All => Meta::All, + Meta::Alias(alias) => Meta::Alias(alias), + Meta::Only(x) => Meta::Only(f(x)), + }; + + match self { + Qualified::Allow(x) => Qualified::Allow(transform(x)), + Qualified::Forbid(x) => Qualified::Forbid(transform(x)), + } + } +} + /// An identifier is a name or a #number #[cfg_attr(test, derive(Clone, Debug, PartialEq, Eq))] #[repr(u32)] @@ -120,7 +136,19 @@ pub enum Directive { HostAlias(Defs) = HARDENED_ENUM_VALUE_1, CmndAlias(Defs) = HARDENED_ENUM_VALUE_2, RunasAlias(Defs) = HARDENED_ENUM_VALUE_3, - Defaults(Vec) = HARDENED_ENUM_VALUE_4, + Defaults(Vec, ConfigScope) = HARDENED_ENUM_VALUE_4, +} + +/// AST object for the 'context' (host, user, cmnd, runas) of a Defaults directive +#[repr(u32)] +pub enum ConfigScope { + // "Defaults entries are parsed in the following order: + // generic, host and user Defaults first, then runas Defaults and finally command defaults." + Generic = HARDENED_ENUM_VALUE_0, + Host(SpecList) = HARDENED_ENUM_VALUE_1, + User(SpecList) = HARDENED_ENUM_VALUE_2, + RunAs(SpecList) = HARDENED_ENUM_VALUE_3, + Command(SpecList) = HARDENED_ENUM_VALUE_4, } /// The Sudoers file can contain permissions and directives @@ -522,7 +550,7 @@ impl Parse for Sudo { if let Some(users) = maybe(try_nonterminal::>(stream))? { // element 1 always exists (parse_list fails on an empty list) let key = &users[0]; - if let Some(directive) = maybe(get_directive(key, stream))? { + if let Some(directive) = maybe(get_directive(key, stream, start_pos))? { if users.len() != 1 { unrecoverable!(pos = start_pos, stream, "invalid user name list"); } @@ -612,9 +640,15 @@ impl Many for Def { const SEP: char = ':'; } +// NOTE: This function is a bit of a hack, since it relies on the fact that all directives +// occur in the spot of a username, and are of a form that would otherwise be a legal user name. +// I.e. after a valid username has been parsed, we check if it isn't actually a valid start of a +// directive. A more robust solution would be to use the approach taken by the `MetaOrTag` above. + fn get_directive( perhaps_keyword: &Spec, stream: &mut CharStream, + begin_pos: (usize, usize), ) -> Parsed { use super::ast::Directive::*; use super::ast::Meta::*; @@ -629,7 +663,30 @@ fn get_directive( "Host_Alias" => make(HostAlias(expect_nonterminal(stream)?)), "Cmnd_Alias" | "Cmd_Alias" => make(CmndAlias(expect_nonterminal(stream)?)), "Runas_Alias" => make(RunasAlias(expect_nonterminal(stream)?)), - "Defaults" => make(Defaults(expect_nonterminal(stream)?)), + "Defaults" => { + //HACK: this avoids having to add "Defaults@" etc as separate tokens; but relying + //on positional information during parsing is of course, cheating. + let allow_scope_modifier = stream.get_pos().0 == begin_pos.0 + && stream.get_pos().1 - begin_pos.1 == "Defaults".len(); + + let scope = if allow_scope_modifier { + if is_syntax('@', stream)? { + ConfigScope::Host(expect_nonterminal(stream)?) + } else if is_syntax(':', stream)? { + ConfigScope::User(expect_nonterminal(stream)?) + } else if is_syntax('!', stream)? { + ConfigScope::Command(expect_nonterminal(stream)?) + } else if is_syntax('>', stream)? { + ConfigScope::RunAs(expect_nonterminal(stream)?) + } else { + ConfigScope::Generic + } + } else { + ConfigScope::Generic + }; + + make(Defaults(expect_nonterminal(stream)?, scope)) + } _ => reject(), } } diff --git a/src/sudoers/ast_names.rs b/src/sudoers/ast_names.rs index a7b5d2602..f8ddd7ea8 100644 --- a/src/sudoers/ast_names.rs +++ b/src/sudoers/ast_names.rs @@ -40,6 +40,10 @@ mod names { const DESCRIPTION: &'static str = "path to binary (or sudoedit)"; } + impl UserFriendly for tokens::SimpleCommand { + const DESCRIPTION: &'static str = "path to binary (or sudoedit)"; + } + impl UserFriendly for ( SpecList, diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs index ff528d499..482272c73 100644 --- a/src/sudoers/mod.rs +++ b/src/sudoers/mod.rs @@ -34,11 +34,20 @@ pub struct Error { pub message: String, } +/// A "Customiser" represents a "Defaults" setting that has 'late binding'; i.e. +/// cannot be determined simply by reading a sudoers configuration. This is used +/// for Defaults@host, Defaults:user, Defaults>runas and Defaults!cmd. +/// +/// I.e. the Setting modifications in the second part of the tuple only apply for +/// items explicitly matched by the first part of the tuple. +type Customiser = (SpecList, Vec); + #[derive(Default)] pub struct Sudoers { rules: Vec, aliases: AliasTable, settings: Settings, + customisers: CustomiserTable, } /// A structure that represents what the user wants to do @@ -84,12 +93,55 @@ impl Sudoers { Ok(analyze(path.as_ref(), sudoers)) } + fn specify_host_user_runas>( + &mut self, + hostname: &system::Hostname, + requesting_user: &User, + target_user: &User, + ) { + let host_customisers = std::mem::take(&mut self.customisers.host); + let user_customisers = std::mem::take(&mut self.customisers.user); + specialise_setting( + &mut self.settings, + host_customisers, + &match_token(hostname), + &self.aliases.host, + ); + specialise_setting( + &mut self.settings, + user_customisers, + &match_user(requesting_user), + &self.aliases.user, + ); + + let runas_customisers = std::mem::take(&mut self.customisers.runas); + specialise_setting( + &mut self.settings, + runas_customisers, + &match_user(target_user), + &self.aliases.runas, + ); + } + + fn specify_command(&mut self, command: &Path, arguments: &[String]) { + let customisers = std::mem::take(&mut self.customisers.cmnd); + specialise_setting( + &mut self.settings, + customisers, + &match_command((command, arguments)), + &self.aliases.cmnd, + ); + } + pub fn check, Group: UnixGroup>( - &self, + &mut self, am_user: &User, on_host: &system::Hostname, request: Request, ) -> Judgement { + self.specify_host_user_runas(on_host, am_user, request.user); + self.specify_command(request.command, request.arguments); + // exception: if user is root or does not switch users, NOPASSWD is implied let skip_passwd = am_user.is_root() || (request.user == am_user && in_group(am_user, request.group)); @@ -103,7 +155,7 @@ impl Sudoers { Judgement { flags, - settings: self.settings.clone(), // this is wasteful, but in the future this will not be a simple clone and it avoids a lifetime + settings: self.settings.clone(), } } @@ -264,14 +316,24 @@ fn open_subsudoers(path: &Path) -> io::Result>> { read_sudoers(source) } +// note: trying to DRY using GAT's is tempting but doesn't make the code any shorter + #[derive(Default)] -pub(super) struct AliasTable { +struct AliasTable { user: VecOrd>, - host: VecOrd>, + host: VecOrd>, cmnd: VecOrd>, runas: VecOrd>, } +#[derive(Default)] +struct CustomiserTable { + user: Vec>, + host: Vec>, + cmnd: Vec>, + runas: Vec>, +} + /// A vector with a list defining the order in which it needs to be processed struct VecOrd(Vec, Vec); @@ -399,7 +461,7 @@ where } /// A interface to access optional "satellite data" -trait WithInfo: Clone { +trait WithInfo { type Item; type Info; fn as_inner(&self) -> Self::Item; @@ -429,6 +491,23 @@ impl<'a> WithInfo for (Tag, &'a Spec) { } } +/// Apply a specialization to the Settings object, used for "specific" Defaults +fn specialise_setting( + settings: &mut Settings, + customisers: impl IntoIterator>, + matcher: &impl Fn(&T) -> bool, + alias_defs: &VecOrd>, +) { + let aliases = get_aliases(alias_defs, matcher); + for (list, modifiers) in customisers { + if find_item(&list, matcher, &aliases).is_some() { + for modifier in modifiers { + modifier(settings); + } + } + } +} + /// Now follow a collection of functions used as closures for `find_item` fn match_user(user: &impl UnixUser) -> impl Fn(&UserSpecifier) -> bool + '_ { move |spec| match spec { @@ -585,16 +664,28 @@ fn analyze( Sudo::Spec(permission) => cfg.rules.push(permission), - Sudo::Decl(UserAlias(mut def)) => cfg.aliases.user.1.append(&mut def), Sudo::Decl(HostAlias(mut def)) => cfg.aliases.host.1.append(&mut def), - Sudo::Decl(CmndAlias(mut def)) => cfg.aliases.cmnd.1.append(&mut def), + Sudo::Decl(UserAlias(mut def)) => cfg.aliases.user.1.append(&mut def), Sudo::Decl(RunasAlias(mut def)) => cfg.aliases.runas.1.append(&mut def), + Sudo::Decl(CmndAlias(mut def)) => cfg.aliases.cmnd.1.append(&mut def), - Sudo::Decl(Defaults(params)) => { - for modifier in params { - modifier(&mut cfg.settings); + Sudo::Decl(Defaults(params, scope)) => match scope { + ConfigScope::Generic => { + for modifier in params { + modifier(&mut cfg.settings) + } } - } + ConfigScope::Host(specs) => cfg.customisers.host.push((specs, params)), + ConfigScope::User(specs) => cfg.customisers.user.push((specs, params)), + ConfigScope::RunAs(specs) => cfg.customisers.runas.push((specs, params)), + ConfigScope::Command(specs) => cfg.customisers.cmnd.push(( + specs + .into_iter() + .map(|spec| spec.map(|simple_command| (simple_command, None))) + .collect(), + params, + )), + }, Sudo::Include(path, span) => include( cfg, diff --git a/src/sudoers/policy.rs b/src/sudoers/policy.rs index 28dde0089..f5d2d2b06 100644 --- a/src/sudoers/policy.rs +++ b/src/sudoers/policy.rs @@ -98,7 +98,7 @@ impl Judgement { } impl Sudoers { - pub fn secure_path(&self) -> Option<&str> { + pub fn search_path(&mut self) -> Option<&str> { self.settings.secure_path() } diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs index 4714c0762..8483eb9a6 100644 --- a/src/sudoers/test/mod.rs +++ b/src/sudoers/test/mod.rs @@ -92,19 +92,19 @@ fn permission_test() { macro_rules! FAIL { ([$($sudo:expr),*], $user:expr => $req:expr, $server:expr; $command:expr) => { - let (Sudoers { rules,aliases,settings }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]); + let (Sudoers { rules,aliases,settings, customisers }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]); let cmdvec = $command.split_whitespace().map(String::from).collect::>(); let req = Request { user: $req.0, group: $req.1, command: &realpath(cmdvec[0].as_ref()), arguments: &cmdvec[1..].to_vec() }; - assert_eq!(Sudoers { rules, aliases, settings }.check(&Named($user), &system::Hostname::fake($server), req).flags, None); + assert_eq!(Sudoers { rules, aliases, settings, customisers }.check(&Named($user), &system::Hostname::fake($server), req).flags, None); } } macro_rules! pass { ([$($sudo:expr),*], $user:expr => $req:expr, $server:expr; $command:expr $(=> [$($key:ident : $val:expr),*])?) => { - let (Sudoers { rules,aliases,settings }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]); + let (Sudoers { rules,aliases,settings, customisers }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]); let cmdvec = $command.split_whitespace().map(String::from).collect::>(); let req = Request { user: $req.0, group: $req.1, command: &realpath(cmdvec[0].as_ref()), arguments: &cmdvec[1..].to_vec() }; - let result = Sudoers { rules, aliases, settings }.check(&Named($user), &system::Hostname::fake($server), req).flags; + let result = Sudoers { rules, aliases, settings, customisers }.check(&Named($user), &system::Hostname::fake($server), req).flags; assert!(!result.is_none()); $( let result = result.unwrap(); @@ -215,6 +215,7 @@ fn permission_test() { pass!(["user ALL=(root) NOPASSWD: /bin/ls, (sudo) /bin/true"], "user" => request! { sudo }, "server"; "/bin/true" => [authenticate: Authenticate::Nopasswd]); FAIL!(["user ALL=(root) /bin/ls, (sudo) /bin/true"], "user" => request! { sudo }, "server"; "/bin/ls"); FAIL!(["user ALL=(root) /bin/ls, (sudo) /bin/true"], "user" => request! { root }, "server"; "/bin/true"); + pass!(["user ALL=(root) NOPASSWD: /bin/ls, (sudo) /bin/ls, /bin/true"], "user" => request! { sudo }, "server"; "/bin/true"); SYNTAX!(["User_Alias, marc ALL = ALL"]); @@ -421,6 +422,94 @@ fn defaults_regression() { assert!(try_parse_line("Defaults .mymachine=ALL").is_none()) } +#[test] +fn specific_defaults() { + assert!(parse_line("Defaults !use_pty").is_decl()); + assert!(try_parse_line("Defaults!use_pty").is_none()); + assert!(parse_line("Defaults!/bin/bash !use_pty").is_decl()); + assert!(try_parse_line("Defaults!/bin/bash!use_pty").is_none()); + assert!(try_parse_line("Defaults !/bin/bash !use_pty").is_none()); + assert!(try_parse_line("Defaults !/bin/bash").is_none()); + assert!(parse_line("Defaults@host !use_pty").is_decl()); + assert!(parse_line("Defaults@host!use_pty").is_decl()); + assert!(try_parse_line("Defaults @host!use_pty").is_none()); + assert!(try_parse_line("Defaults @host !use_pty").is_none()); + assert!(parse_line("Defaults:user !use_pty").is_decl()); + assert!(parse_line("Defaults:user!use_pty").is_decl()); + assert!(try_parse_line("Defaults :user!use_pty").is_none()); + assert!(try_parse_line("Defaults :user !use_pty").is_none()); + assert!(parse_line("Defaults>user !use_pty").is_decl()); + assert!(parse_line("Defaults>user!use_pty").is_decl()); + assert!(try_parse_line("Defaults >user!use_pty").is_none()); + assert!(try_parse_line("Defaults >user !use_pty").is_none()); +} + +#[test] +fn default_specific_test() { + let sudoers = || { + analyze( + Path::new("/etc/fakesudoers"), + sudoer![ + "Defaults env_editor", + "Defaults@host !env_editor", + "Defaults:user use_pty", + "Defaults !use_pty", + "Defaults>runas secure_path=\"/bin\"", + "Defaults !secure_path", + "Defaults!/bin/foo !env_keep", + "Defaults!RR use_pty", + "Cmnd_Alias RR=/usr/bin/rr twice" + ], + ) + }; + + let (Sudoers { settings, .. }, _) = sudoers(); + assert!(settings.env_editor()); + assert!(!settings.use_pty()); + assert!(settings.env_keep().contains("COLORS")); + assert_eq!(settings.secure_path(), None); + + let (mut mod_sudoers, _) = sudoers(); + mod_sudoers.specify_host_user_runas( + &system::Hostname::fake("host"), + &Named("user"), + &Named("root"), + ); + assert!(!mod_sudoers.settings.env_editor()); + assert!(mod_sudoers.settings.use_pty()); + assert!(mod_sudoers.settings.env_keep().contains("COLORS")); + assert_eq!(mod_sudoers.settings.secure_path(), None); + + let (mut mod_sudoers, _) = sudoers(); + mod_sudoers.specify_host_user_runas( + &system::Hostname::fake("machine"), + &Named("admin"), + &Named("runas"), + ); + assert!(mod_sudoers.settings.env_editor()); + assert!(!mod_sudoers.settings.use_pty()); + assert!(mod_sudoers.settings.env_keep().contains("COLORS")); + assert_eq!(mod_sudoers.settings.secure_path(), Some("/bin")); + mod_sudoers.specify_command(Path::new("/bin/foo"), &["".to_string(), "a".to_string()]); + assert!(mod_sudoers.settings.env_keep().is_empty()); + + let (mut mod_sudoers, _) = sudoers(); + mod_sudoers.specify_host_user_runas( + &system::Hostname::fake("machine"), + &Named("admin"), + &Named("self"), + ); + mod_sudoers.specify_command(Path::new("/usr/bin/rr"), &["thrice".to_string()]); + assert!(settings.env_editor()); + assert!(!settings.use_pty()); + assert!(settings.env_keep().contains("COLORS")); + assert_eq!(settings.secure_path(), None); + + let (mut mod_sudoers, _) = sudoers(); + mod_sudoers.specify_command(Path::new("/usr/bin/rr"), &["twice".to_string()]); + assert!(mod_sudoers.settings.use_pty()); +} + #[test] fn useralias_underscore_regression() { let sudo = parse_line("FOO_BAR ALL=ALL"); diff --git a/src/sudoers/tokens.rs b/src/sudoers/tokens.rs index a67a42596..19c699fda 100644 --- a/src/sudoers/tokens.rs +++ b/src/sudoers/tokens.rs @@ -152,21 +152,23 @@ impl Token for AliasName { /// A struct that represents valid command strings; this can contain escape sequences and are /// limited to 1024 characters. -pub type Command = (glob::Pattern, Option>); +pub type Command = (SimpleCommand, Option>); + +/// A type that is specific to 'only commands', that can only happen in "Defaults!command" contexts; +/// which is essentially a subset of "Command" +pub type SimpleCommand = glob::Pattern; impl Token for Command { const MAX_LEN: usize = 1024; fn construct(s: String) -> Result { - let cvt_err = |pat: Result<_, glob::PatternError>| { - pat.map_err(|err| format!("wildcard pattern error {err}")) - }; - // the tokenizer should not give us a token that consists of only whitespace let mut cmd_iter = s.split_whitespace(); - let mut cmd = cmd_iter.next().unwrap().to_string(); + let cmd = cmd_iter.next().unwrap().to_string(); let mut args = cmd_iter.map(String::from).collect::>(); + let command = SimpleCommand::construct(cmd)?; + let argpat = if args.is_empty() { // if no arguments are mentioned, anything is allowed None @@ -178,6 +180,32 @@ impl Token for Command { Some(args.into_boxed_slice()) }; + Ok((command, argpat)) + } + + // all commands start with "/" except "sudoedit" + fn accept_1st(c: char) -> bool { + SimpleCommand::accept_1st(c) + } + + fn accept(c: char) -> bool { + SimpleCommand::accept(c) || c == ' ' + } + + const ALLOW_ESCAPE: bool = SimpleCommand::ALLOW_ESCAPE; + fn escaped(c: char) -> bool { + SimpleCommand::escaped(c) + } +} + +impl Token for SimpleCommand { + const MAX_LEN: usize = 1024; + + fn construct(mut cmd: String) -> Result { + let cvt_err = |pat: Result<_, glob::PatternError>| { + pat.map_err(|err| format!("wildcard pattern error {err}")) + }; + // record if the cmd ends in a slash and remove it if it does let is_dir = cmd.ends_with('/') && { cmd.pop(); @@ -197,7 +225,7 @@ impl Token for Command { cmd.push_str("/*"); } - Ok((cvt_err(glob::Pattern::new(&cmd))?, argpat)) + cvt_err(glob::Pattern::new(&cmd)) } // all commands start with "/" except "sudoedit" @@ -211,11 +239,12 @@ impl Token for Command { const ALLOW_ESCAPE: bool = true; fn escaped(c: char) -> bool { - matches!(c, '\\' | ',' | ':' | '=' | '#') + matches!(c, '\\' | ',' | ':' | '=' | '#' | ' ') } } impl Many for Command {} +impl Many for SimpleCommand {} pub struct DefaultName(pub String);