Skip to content

Commit 12024bf

Browse files
committed
move umask from policy to Restrictions (since it affects the after-authentication context)
1 parent d75adcc commit 12024bf

File tree

7 files changed

+21
-22
lines changed

7 files changed

+21
-22
lines changed

src/common/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ impl Context {
9696
process: Process::new(),
9797
use_pty: true,
9898
noexec: false,
99-
umask: policy.umask(),
99+
umask: Umask::Preserve,
100100
})
101101
}
102102

src/exec/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ impl SpawnNoexecHandler {
5050
}
5151

5252
#[derive(Debug, Copy, Clone)]
53+
#[cfg_attr(test, derive(PartialEq))]
5354
#[repr(u32)]
5455
pub enum Umask {
5556
/// Keep the umask of the parent process.

src/sudo/env/environment.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@ mod tests {
280280
chdir: crate::sudoers::DirChange::Strict(None),
281281
trust_environment: false,
282282
use_pty: true,
283+
umask: crate::exec::Umask::Preserve,
283284
#[cfg(feature = "apparmor")]
284285
apparmor_profile: None,
285286
noexec: false,

src/sudo/env/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ fn test_environment_variable_filtering() {
172172
use_pty: true,
173173
chdir: crate::sudoers::DirChange::Strict(None),
174174
trust_environment: false,
175+
umask: crate::exec::Umask::Preserve,
175176
#[cfg(feature = "apparmor")]
176177
apparmor_profile: None,
177178
noexec: false,

src/sudo/pipeline.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ fn apply_policy_to_context(
238238
// could be needed, but here we copy these -- perhaps we should split up the Context type
239239
context.use_pty = controls.use_pty;
240240
context.noexec = controls.noexec;
241+
context.umask = controls.umask;
241242

242243
Ok(())
243244
}

src/sudoers/mod.rs

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use std::path::{Path, PathBuf};
1515

1616
use crate::common::resolve::resolve_path;
1717
use crate::defaults;
18-
use crate::exec::Umask;
1918
use crate::log::auth_warn;
2019
use crate::system::interface::{GroupId, UnixGroup, UnixUser, UserId};
2120
use crate::system::{self, can_execute};
@@ -318,26 +317,6 @@ impl Sudoers {
318317

319318
None
320319
}
321-
322-
pub(crate) fn umask(&self) -> Umask {
323-
if self.settings.umask() == 0o777 {
324-
Umask::Preserve
325-
} else if self.settings.umask_override() {
326-
Umask::Override(
327-
self.settings
328-
.umask()
329-
.try_into()
330-
.expect("the umask parser should have prevented overflow"),
331-
)
332-
} else {
333-
Umask::Extend(
334-
self.settings
335-
.umask()
336-
.try_into()
337-
.expect("the umask parser should have prevented overflow"),
338-
)
339-
}
340-
}
341320
}
342321

343322
// a `take_while` variant that does not consume the first non-matching item

src/sudoers/policy.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use super::Judgement;
44
use crate::common::{
55
SudoPath, HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2,
66
};
7+
use crate::exec::Umask;
78
use crate::sudoers::ast::{ExecControl, Tag};
89
use crate::system::{time::Duration, Hostname, User};
910
/// Data types and traits that represent what the "terms and conditions" are after a succesful
@@ -59,6 +60,7 @@ pub struct Restrictions<'a> {
5960
pub env_check: &'a HashSet<String>,
6061
pub chdir: DirChange<'a>,
6162
pub path: Option<&'a str>,
63+
pub umask: Umask,
6264
#[cfg(feature = "apparmor")]
6365
pub apparmor_profile: Option<String>,
6466
}
@@ -109,6 +111,20 @@ impl Judgement {
109111
Some(super::ChDir::Path(path)) => DirChange::Strict(Some(path)),
110112
},
111113
path: self.settings.secure_path(),
114+
umask: {
115+
let mask = self
116+
.settings
117+
.umask()
118+
.try_into()
119+
.expect("the umask parser should have prevented overflow");
120+
if self.settings.umask() == 0o777 {
121+
Umask::Preserve
122+
} else if self.settings.umask_override() {
123+
Umask::Override(mask)
124+
} else {
125+
Umask::Extend(mask)
126+
}
127+
},
112128
#[cfg(feature = "apparmor")]
113129
apparmor_profile: tag
114130
.apparmor_profile

0 commit comments

Comments
 (0)