Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specific Defaults #897

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
32 changes: 14 additions & 18 deletions src/common/context.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use crate::common::resolve::AuthUser;
use crate::common::{HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
use crate::sudoers::AuthenticatingUser;
use crate::system::{Group, Hostname, Process, User};

use super::resolve::CurrentUser;
Expand Down Expand Up @@ -45,7 +43,6 @@ pub struct Context {
// system
pub hostname: Hostname,
pub current_user: CurrentUser,
pub auth_user: AuthUser,
pub process: Process,
// policy
pub use_pty: bool,
Expand All @@ -63,15 +60,10 @@ pub enum LaunchType {
impl Context {
pub fn build_from_options(
sudo_options: OptionsForContext,
path: String,
auth_user: AuthenticatingUser,
secure_path: Option<&str>,
) -> Result<Context, Error> {
let hostname = Hostname::resolve();
let current_user = CurrentUser::resolve()?;
let auth_user = match auth_user {
AuthenticatingUser::InvokingUser => AuthUser::from_current_user(current_user.clone()),
AuthenticatingUser::Root => AuthUser::resolve_root_for_rootpw()?,
};
let (target_user, target_group) =
resolve_target_user_and_group(&sudo_options.user, &sudo_options.group, &current_user)?;
let (launch, shell) = resolve_launch_and_shell(&sudo_options, &current_user, &target_user);
Expand All @@ -82,14 +74,24 @@ impl Context {
// FIXME `Default` is being used as `Option::None`
Default::default()
}
_ => CommandAndArguments::build_from_args(shell, sudo_options.positional_args, &path),
_ => {
let system_path;

let path = if let Some(path) = secure_path {
path
} else {
system_path = std::env::var("PATH").unwrap_or_default();
system_path.as_ref()
};

CommandAndArguments::build_from_args(shell, sudo_options.positional_args, path)
}
};

Ok(Context {
hostname,
command,
current_user,
auth_user,
target_user,
target_group,
use_session_records: !sudo_options.reset_timestamp,
Expand All @@ -108,7 +110,6 @@ impl Context {
mod tests {
use crate::{
sudo::SudoAction,
sudoers::AuthenticatingUser,
system::{interface::UserId, Hostname},
};
use std::collections::HashMap;
Expand All @@ -124,12 +125,7 @@ mod tests {
.unwrap();
let path = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin";
let (ctx_opts, _pipe_opts) = options.into();
let context = Context::build_from_options(
ctx_opts,
path.to_string(),
AuthenticatingUser::InvokingUser,
)
.unwrap();
let context = Context::build_from_options(ctx_opts, Some(path)).unwrap();

let mut target_environment = HashMap::new();
target_environment.insert("SUDO_USER".to_string(), context.current_user.name.clone());
Expand Down
4 changes: 2 additions & 2 deletions src/defaults/settings_dsl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn FnOnce(&mut Settings)>
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<dyn FnOnce(&mut Settings)>
Box::new(move |obj: &mut Settings| obj.$id = i) as SettingsModifier
})
})
};
Expand Down
56 changes: 22 additions & 34 deletions src/sudo/env/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{
};

use crate::common::{CommandAndArguments, Context, Error};
use crate::sudoers::Policy;
use crate::sudoers::Restrictions;
use crate::system::PATH_MAX;

use super::wildcard_match::wildcard_match;
Expand Down Expand Up @@ -47,7 +47,7 @@ fn format_command(command_and_arguments: &CommandAndArguments) -> OsString {
/// Construct sudo-specific environment variables
fn add_extra_env(
context: &Context,
cfg: &impl Policy,
cfg: &Restrictions,
sudo_ps1: Option<OsString>,
environment: &mut Environment,
) {
Expand Down Expand Up @@ -101,7 +101,7 @@ fn add_extra_env(
}

// Overwrite PATH when secure_path is set
if let Some(secure_path) = cfg.secure_path() {
if let Some(secure_path) = &cfg.path {
// assign path by env path or secure_path configuration
environment.insert("PATH".into(), secure_path.into());
}
Expand Down Expand Up @@ -168,25 +168,25 @@ fn in_table(needle: &OsStr, haystack: &HashSet<String>) -> bool {
}

/// Determine whether a specific environment variable should be kept
fn should_keep(key: &OsStr, value: &OsStr, cfg: &impl Policy) -> bool {
fn should_keep(key: &OsStr, value: &OsStr, cfg: &Restrictions) -> bool {
if value.as_bytes().starts_with("()".as_bytes()) {
return false;
}

if cfg.secure_path().is_some() && key == "PATH" {
if cfg.path.is_some() && key == "PATH" {
return false;
}

if key == "TZ" {
return in_table(key, cfg.env_keep())
|| (in_table(key, cfg.env_check()) && is_safe_tz(value.as_bytes()));
return in_table(key, cfg.env_keep)
|| (in_table(key, cfg.env_check) && is_safe_tz(value.as_bytes()));
}

if in_table(key, cfg.env_check()) {
if in_table(key, cfg.env_check) {
return !value.as_bytes().iter().any(|c| *c == b'%' || *c == b'/');
}

in_table(key, cfg.env_keep())
in_table(key, cfg.env_keep)
}

/// Construct the final environment from the current one and a sudo context
Expand All @@ -207,7 +207,7 @@ pub fn get_target_environment(
additional_env: impl IntoIterator<Item = (OsString, OsString)>,
user_override: Vec<(String, String)>,
context: &Context,
settings: &impl Policy,
settings: &Restrictions,
) -> Result<Environment, Error> {
// retrieve SUDO_PS1 value to set a PS1 value as additional environment
let sudo_ps1 = current_env.get(OsStr::new("SUDO_PS1")).cloned();
Expand Down Expand Up @@ -254,7 +254,6 @@ where
#[cfg(test)]
mod tests {
use super::{is_safe_tz, should_keep, PATH_ZONEINFO};
use crate::sudoers::Policy;
use std::{collections::HashSet, ffi::OsStr};

struct TestConfiguration {
Expand All @@ -263,32 +262,21 @@ mod tests {
path: Option<String>,
}

impl Policy for TestConfiguration {
fn env_keep(&self) -> &HashSet<String> {
&self.keep
}

fn env_check(&self) -> &HashSet<String> {
&self.check
}

fn secure_path(&self) -> Option<String> {
self.path.clone()
}

fn use_pty(&self) -> bool {
true
}

fn pwfeedback(&self) -> bool {
false
}
}

impl TestConfiguration {
pub fn check_should_keep(&self, key: &str, value: &str, expected: bool) {
assert_eq!(
should_keep(OsStr::new(key), OsStr::new(value), self),
should_keep(
OsStr::new(key),
OsStr::new(value),
&crate::sudoers::Restrictions {
env_keep: &self.keep,
env_check: &self.check,
path: self.path.as_deref(),
chdir: crate::sudoers::DirChange::Strict(None),
trust_environment: false,
use_pty: true,
}
),
expected,
"{} should {}",
key,
Expand Down
16 changes: 10 additions & 6 deletions src/sudo/env/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::common::resolve::{AuthUser, CurrentUser};
use crate::common::resolve::CurrentUser;
use crate::common::{CommandAndArguments, Context};
use crate::sudo::{
cli::{SudoAction, SudoRunOptions},
Expand Down Expand Up @@ -94,8 +94,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
groups: vec![],
});

let auth_user = AuthUser::from_current_user(current_user.clone());

let current_group = Group {
gid: GroupId::new(1000),
name: Some("test".to_string()),
Expand All @@ -121,7 +119,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
hostname: Hostname::fake("test-ubuntu"),
command,
current_user: current_user.clone(),
auth_user,
target_user: if sudo_options.user.as_deref() == Some("test") {
current_user.into()
} else {
Expand Down Expand Up @@ -162,14 +159,21 @@ fn test_environment_variable_filtering() {
.try_into_run()
.ok()
.unwrap();
let settings = crate::sudoers::Judgement::default();
let settings = crate::defaults::Settings::default();
let context = create_test_context(&options);
let resulting_env = get_target_environment(
initial_env.clone(),
HashMap::new(),
Vec::new(),
&context,
&settings,
&crate::sudoers::Restrictions {
env_keep: settings.env_keep(),
env_check: settings.env_check(),
path: settings.secure_path(),
use_pty: true,
chdir: crate::sudoers::DirChange::Strict(None),
trust_environment: false,
},
)
.unwrap();

Expand Down
49 changes: 2 additions & 47 deletions src/sudo/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![forbid(unsafe_code)]

use crate::common::resolve::CurrentUser;
use crate::common::{Context, Error};
use crate::common::Error;
use crate::log::dev_info;
use crate::system::interface::UserId;
use crate::system::kernel::kernel_check;
Expand All @@ -14,7 +14,7 @@ pub use cli::SudoAction;
#[cfg(not(test))]
use cli::SudoAction;
use pam::PamAuthenticator;
use pipeline::{Pipeline, PolicyPlugin};
use pipeline::Pipeline;
use std::path::Path;

mod cli;
Expand Down Expand Up @@ -65,50 +65,6 @@ pub(crate) fn candidate_sudoers_file() -> &'static Path {
file
}

#[derive(Default)]
pub(crate) struct SudoersPolicy {}

impl PolicyPlugin for SudoersPolicy {
type PreJudgementPolicy = crate::sudoers::Sudoers;
type Policy = crate::sudoers::Judgement;

fn init(&mut self) -> Result<Self::PreJudgementPolicy, Error> {
let sudoers_path = candidate_sudoers_file();

let (sudoers, syntax_errors) = crate::sudoers::Sudoers::open(sudoers_path)
.map_err(|e| Error::Configuration(format!("{e}")))?;

for crate::sudoers::Error {
source,
location,
message,
} in syntax_errors
{
let path = source.as_deref().unwrap_or(sudoers_path);
diagnostic::diagnostic!("{message}", path @ location);
}

Ok(sudoers)
}

fn judge(
&mut self,
pre: Self::PreJudgementPolicy,
context: &Context,
) -> Result<Self::Policy, Error> {
Ok(pre.check(
&*context.current_user,
&context.hostname,
crate::sudoers::Request {
user: &context.target_user,
group: &context.target_group,
command: &context.command.command,
arguments: &context.command.arguments,
},
))
}
}

fn sudo_process() -> Result<(), Error> {
crate::log::SudoLogger::new("sudo: ").into_global_logger();

Expand All @@ -118,7 +74,6 @@ fn sudo_process() -> Result<(), Error> {
kernel_check()?;

let pipeline = Pipeline {
policy: SudoersPolicy::default(),
authenticator: PamAuthenticator::new_cli(),
};

Expand Down
13 changes: 7 additions & 6 deletions src/sudo/pam.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
use std::ffi::OsString;

use crate::common::context::LaunchType;
use crate::common::resolve::AuthUser;
use crate::common::{error::Error, Context};
use crate::log::{dev_info, user_warn};
use crate::pam::{CLIConverser, Converser, PamContext, PamError, PamErrorType, PamResult};
use crate::system::term::current_tty_name;

use super::pipeline::AuthPlugin;

type PamBuilder<C> = dyn Fn(&Context) -> PamResult<PamContext<C>>;
type PamBuilder<C> = dyn Fn(&Context, AuthUser) -> PamResult<PamContext<C>>;

pub struct PamAuthenticator<C: Converser> {
builder: Box<PamBuilder<C>>,
Expand All @@ -17,7 +18,7 @@ pub struct PamAuthenticator<C: Converser> {

impl<C: Converser> PamAuthenticator<C> {
fn new(
initializer: impl Fn(&Context) -> PamResult<PamContext<C>> + 'static,
initializer: impl Fn(&Context, AuthUser) -> PamResult<PamContext<C>> + 'static,
) -> PamAuthenticator<C> {
PamAuthenticator {
builder: Box::new(initializer),
Expand All @@ -28,23 +29,23 @@ impl<C: Converser> PamAuthenticator<C> {

impl PamAuthenticator<CLIConverser> {
pub fn new_cli() -> PamAuthenticator<CLIConverser> {
PamAuthenticator::new(|context| {
PamAuthenticator::new(|context, auth_user| {
init_pam(
matches!(context.launch, LaunchType::Login),
matches!(context.launch, LaunchType::Shell),
context.stdin,
context.non_interactive,
context.password_feedback,
&context.auth_user.name,
&auth_user.name,
&context.current_user.name,
)
})
}
}

impl<C: Converser> AuthPlugin for PamAuthenticator<C> {
fn init(&mut self, context: &Context) -> Result<(), Error> {
self.pam = Some((self.builder)(context)?);
fn init(&mut self, context: &Context, auth_user: AuthUser) -> Result<(), Error> {
self.pam = Some((self.builder)(context, auth_user)?);
Ok(())
}

Expand Down
Loading
Loading