From 1f1f1953fd868b99ec06c35d5e72b9a43e1920da Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 03:26:45 +0800 Subject: [PATCH 1/7] Fixed issues#769 : Add newtypes for UserId, GroupId and ProcessId --- src/common/command.rs | 2 +- src/exec/interface.rs | 5 +- src/exec/no_pty.rs | 2 +- src/exec/use_pty/backchannel.rs | 5 +- src/exec/use_pty/monitor.rs | 11 ++- src/exec/use_pty/parent.rs | 2 +- src/su/context.rs | 5 +- src/sudo/mod.rs | 3 +- src/sudo/pipeline/list.rs | 3 +- src/sudoers/mod.rs | 8 +- src/system/file/chown.rs | 2 +- src/system/interface.rs | 136 ++++++++++++++++++++++++++++---- src/system/mod.rs | 58 +++++++------- src/system/signal/info.rs | 2 +- src/system/term/mod.rs | 4 +- src/system/timestamp.rs | 17 ++-- src/system/wait.rs | 4 +- 17 files changed, 194 insertions(+), 75 deletions(-) diff --git a/src/common/command.rs b/src/common/command.rs index fd3b8a0db..14b0ab96c 100644 --- a/src/common/command.rs +++ b/src/common/command.rs @@ -63,7 +63,7 @@ impl CommandAndArguments { arguments = vec!["-c".to_string(), escaped(arguments)] } } else { - command = arguments.get(0).map(|s| s.into()).unwrap_or_default(); + command = arguments.first().map(|s| s.into()).unwrap_or_default(); arguments.remove(0); // remember the original binary name before resolving symlinks; this is not diff --git a/src/exec/interface.rs b/src/exec/interface.rs index d04c83423..8a5284507 100644 --- a/src/exec/interface.rs +++ b/src/exec/interface.rs @@ -2,6 +2,7 @@ use std::io::{self, ErrorKind}; use std::path::PathBuf; use crate::common::SudoPath; +use crate::system::interface::ProcessId; use crate::{ common::{context::LaunchType, Context}, system::{Group, User}, @@ -16,7 +17,7 @@ pub trait RunOptions { fn user(&self) -> &User; fn requesting_user(&self) -> &User; fn group(&self) -> &Group; - fn pid(&self) -> i32; + fn pid(&self) -> ProcessId; fn use_pty(&self) -> bool; } @@ -57,7 +58,7 @@ impl RunOptions for Context { &self.target_group } - fn pid(&self) -> i32 { + fn pid(&self) -> ProcessId { self.process.pid } diff --git a/src/exec/no_pty.rs b/src/exec/no_pty.rs index 68ff6543d..5ecd86c41 100644 --- a/src/exec/no_pty.rs +++ b/src/exec/no_pty.rs @@ -147,7 +147,7 @@ impl ExecClosure { /// - is the same PID of the command, or /// - is in the process group of the command and either sudo or the command is the leader. fn is_self_terminating(&self, signaler_pid: ProcessId) -> bool { - if signaler_pid != 0 { + if signaler_pid != ProcessId(0) { if Some(signaler_pid) == self.command_pid { return true; } diff --git a/src/exec/use_pty/backchannel.rs b/src/exec/use_pty/backchannel.rs index fda2c3905..565fac707 100644 --- a/src/exec/use_pty/backchannel.rs +++ b/src/exec/use_pty/backchannel.rs @@ -73,7 +73,7 @@ impl ParentMessage { Self::CMD_STAT_EXIT => Self::CommandStatus(CommandStatus::Exit(data)), Self::CMD_STAT_TERM => Self::CommandStatus(CommandStatus::Term(data)), Self::CMD_STAT_STOP => Self::CommandStatus(CommandStatus::Stop(data)), - Self::CMD_PID => Self::CommandPid(data), + Self::CMD_PID => Self::CommandPid(ProcessId(data)), Self::SHORT_READ => Self::ShortRead, _ => unreachable!(), } @@ -90,7 +90,8 @@ impl ParentMessage { }; let data = match self { - ParentMessage::IoError(data) | ParentMessage::CommandPid(data) => *data, + ParentMessage::IoError(data) => *data, + ParentMessage::CommandPid(data) => data.0, ParentMessage::CommandStatus(status) => match status { CommandStatus::Exit(data) | CommandStatus::Term(data) diff --git a/src/exec/use_pty/monitor.rs b/src/exec/use_pty/monitor.rs index 057247346..98c3737d4 100644 --- a/src/exec/use_pty/monitor.rs +++ b/src/exec/use_pty/monitor.rs @@ -198,14 +198,17 @@ fn exec_command( original_set: Option, ) -> io::Error { // FIXME (ogsudo): Do any additional configuration that needs to be run after `fork` but before `exec` - let command_pid = std::process::id() as ProcessId; + let command_pid = std::process::id() as i32; - setpgid(0, command_pid).ok(); + setpgid(ProcessId(0), ProcessId(command_pid)).ok(); // Wait for the monitor to set us as the foreground group for the pty if we are in the // foreground. if foreground { - while !pty_follower.tcgetpgrp().is_ok_and(|pid| pid == command_pid) { + while !pty_follower + .tcgetpgrp() + .is_ok_and(|pid| pid == ProcessId(command_pid)) + { std::thread::sleep(std::time::Duration::from_micros(1)); } } @@ -413,7 +416,7 @@ fn is_self_terminating( command_pid: ProcessId, command_pgrp: ProcessId, ) -> bool { - if signaler_pid != 0 { + if signaler_pid != ProcessId(0) { if signaler_pid == command_pid { return true; } diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index 3b10bb27e..104cf527b 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -432,7 +432,7 @@ impl ParentClosure { /// - is the same PID of the command, or /// - is in the process group of the command and either sudo or the command is the leader. fn is_self_terminating(&self, signaler_pid: ProcessId) -> bool { - if signaler_pid != 0 { + if signaler_pid != ProcessId(0) { if Some(signaler_pid) == self.command_pid { return true; } diff --git a/src/su/context.rs b/src/su/context.rs index 5ee001403..c152da752 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -12,6 +12,7 @@ use crate::common::{ }; use crate::exec::RunOptions; use crate::log::user_warn; +use crate::system::interface::{ProcessId, ROOT}; use crate::system::{Group, Process, User}; use super::cli::SuRunOptions; @@ -82,7 +83,7 @@ impl SuContext { .ok_or_else(|| Error::UserNotFound(options.user.clone().into()))?; // check the current user is root - let is_current_root = User::real_uid() == 0; + let is_current_root = User::real_uid() == ROOT; let is_target_root = options.user == "root"; // only root can set a (additional) group @@ -240,7 +241,7 @@ impl RunOptions for SuContext { &self.group } - fn pid(&self) -> i32 { + fn pid(&self) -> ProcessId { self.process.pid } diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs index e1e686aee..bb41f9eb9 100644 --- a/src/sudo/mod.rs +++ b/src/sudo/mod.rs @@ -2,6 +2,7 @@ use crate::common::{resolve::resolve_current_user, Context, Error}; use crate::log::dev_info; +use crate::system::interface::ROOT; use crate::system::timestamp::RecordScope; use crate::system::User; use crate::system::{time::Duration, timestamp::SessionRecordFile, Process}; @@ -134,8 +135,6 @@ fn sudo_process() -> Result<(), Error> { } fn self_check() -> Result<(), Error> { - const ROOT: u32 = 0; - let euid = User::effective_uid(); if euid == ROOT { Ok(()) diff --git a/src/sudo/pipeline/list.rs b/src/sudo/pipeline/list.rs index f1216d85f..b2504cb4c 100644 --- a/src/sudo/pipeline/list.rs +++ b/src/sudo/pipeline/list.rs @@ -5,6 +5,7 @@ use crate::{ pam::CLIConverser, sudo::{cli::SudoListOptions, pam::PamAuthenticator, SudoersPolicy}, sudoers::{Authorization, ListRequest, Policy, Request, Sudoers}, + system::interface::ROOT, system::User, }; @@ -87,7 +88,7 @@ impl Pipeline> { } Authorization::Forbidden => { - if context.current_user.uid == 0 { + if context.current_user.uid == ROOT { if original_command.is_some() { return Err(Error::Silent); } diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs index 45ff5243d..6f501db7d 100644 --- a/src/sudoers/mod.rs +++ b/src/sudoers/mod.rs @@ -16,7 +16,7 @@ use std::{io, mem}; use crate::common::resolve::resolve_path; use crate::log::auth_warn; use crate::system::can_execute; -use crate::system::interface::{UnixGroup, UnixUser}; +use crate::system::interface::{GroupId, UnixGroup, UnixUser, UserId}; use ast::*; use tokens::*; @@ -432,7 +432,7 @@ fn match_user(user: &impl UnixUser) -> impl Fn(&UserSpecifier) -> bool + '_ { move |spec| match spec { UserSpecifier::User(id) => match_identifier(user, id), UserSpecifier::Group(Identifier::Name(name)) => user.in_group_by_name(name.as_cstr()), - UserSpecifier::Group(Identifier::ID(num)) => user.in_group_by_gid(*num), + UserSpecifier::Group(Identifier::ID(num)) => user.in_group_by_gid(GroupId(*num)), _ => todo!(), // nonunix-groups, netgroups, etc. } } @@ -443,7 +443,7 @@ fn in_group(user: &impl UnixUser, group: &impl UnixGroup) -> bool { fn match_group(group: &impl UnixGroup) -> impl Fn(&Identifier) -> bool + '_ { move |id| match id { - Identifier::ID(num) => group.as_gid() == *num, + Identifier::ID(num) => group.as_gid() == GroupId(*num), Identifier::Name(name) => group.try_as_name().map_or(false, |s| name == s), } } @@ -510,7 +510,7 @@ where fn match_identifier(user: &impl UnixUser, ident: &ast::Identifier) -> bool { match ident { Identifier::Name(name) => user.has_name(name), - Identifier::ID(num) => user.has_uid(*num), + Identifier::ID(num) => user.has_uid(UserId(*num)), } } diff --git a/src/system/file/chown.rs b/src/system/file/chown.rs index 83cce5f3b..d3ac35b0e 100644 --- a/src/system/file/chown.rs +++ b/src/system/file/chown.rs @@ -13,6 +13,6 @@ impl Chown for File { fn chown(&self, owner: UserId, group: GroupId) -> io::Result<()> { let fd = self.as_raw_fd(); - cerr(unsafe { libc::fchown(fd, owner, group) }).map(|_| ()) + cerr(unsafe { libc::fchown(fd, owner.id(), group.id()) }).map(|_| ()) } } diff --git a/src/system/interface.rs b/src/system/interface.rs index 9b12603da..5e3eb7e8b 100644 --- a/src/system/interface.rs +++ b/src/system/interface.rs @@ -1,9 +1,116 @@ +use core::convert::Infallible; +use std::convert::TryFrom; use std::ffi::CStr; +use std::fmt; +use std::str::FromStr; -pub type GroupId = libc::gid_t; -pub type UserId = libc::uid_t; -pub type ProcessId = libc::pid_t; -pub type DeviceId = libc::dev_t; +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) struct GroupId(pub(crate) libc::gid_t); +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) struct UserId(pub(crate) libc::uid_t); +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) struct ProcessId(pub(crate) libc::pid_t); +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub(crate) struct DeviceId(pub(crate) libc::dev_t); + +pub(crate) const ROOT_ID: u32 = 0; +pub(crate) const ROOT: UserId = UserId(ROOT_ID); + +impl UserId { + pub(crate) fn id(&self) -> u32 { + self.0 + } +} + +impl ProcessId { + pub(crate) fn id(&self) -> i32 { + self.0 + } +} + +impl GroupId { + pub(crate) fn id(&self) -> u32 { + self.0 + } +} + +impl DeviceId { + pub(crate) fn id(&self) -> u64 { + self.0 + } +} + +impl FromStr for UserId { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + let uid = s.parse::()?; + Ok(UserId(uid)) + } +} + +impl fmt::Display for UserId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let uid_str = format!("{}", self.0); + f.write_str(&uid_str) + } +} + +impl FromStr for ProcessId { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + let pid = s.parse::()?; + Ok(ProcessId(pid)) + } +} + +impl fmt::Display for ProcessId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let pid_str = format!("{}", self.0); + f.write_str(&pid_str) + } +} + +impl TryFrom for i64 { + type Error = Infallible; + + fn try_from(value: ProcessId) -> Result { + value.0.try_into() + } +} + +impl FromStr for GroupId { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + let gid = s.parse::()?; + Ok(GroupId(gid)) + } +} + +impl fmt::Display for GroupId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let gid_str = format!("{}", self.0); + f.write_str(&gid_str) + } +} + +impl FromStr for DeviceId { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + let did = s.parse::()?; + Ok(DeviceId(did)) + } +} + +impl fmt::Display for DeviceId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let did_str = format!("{}", self.0); + f.write_str(&did_str) + } +} /// This trait/module is here to not make this crate independent (at the present time) in the idiosyncracies of user representation details /// (which we may decide over time), as well as to make explicit what functionality a user-representation must have; this @@ -12,7 +119,7 @@ pub trait UnixUser { fn has_name(&self, _name: &str) -> bool { false } - fn has_uid(&self, _uid: libc::uid_t) -> bool { + fn has_uid(&self, _uid: UserId) -> bool { false } fn is_root(&self) -> bool { @@ -35,11 +142,12 @@ impl UnixUser for super::User { fn has_name(&self, name: &str) -> bool { self.name == name } - fn has_uid(&self, uid: GroupId) -> bool { + fn has_uid(&self, uid: UserId) -> bool { self.uid == uid } fn is_root(&self) -> bool { - self.has_uid(0) + let root_id = UserId(ROOT_ID); + self.has_uid(root_id) } fn in_group_by_name(&self, name_c: &CStr) -> bool { if let Ok(Some(group)) = super::Group::from_name(name_c) { @@ -69,7 +177,7 @@ mod test { use super::*; - fn test_user(user: impl UnixUser, name_c: &CStr, uid: libc::uid_t) { + fn test_user(user: impl UnixUser, name_c: &CStr, uid: UserId) { let name = name_c.to_str().unwrap(); assert!(user.has_name(name)); assert!(user.has_uid(uid)); @@ -77,7 +185,7 @@ mod test { assert_eq!(user.is_root(), name == "root"); } - fn test_group(group: impl UnixGroup, name: &str, gid: libc::gid_t) { + fn test_group(group: impl UnixGroup, name: &str, gid: GroupId) { assert_eq!(group.as_gid(), gid); assert_eq!(group.try_as_name(), Some(name)); } @@ -85,22 +193,22 @@ mod test { #[test] fn test_unix_user() { let user = |name| User::from_name(name).unwrap().unwrap(); - test_user(user(cstr!("root")), cstr!("root"), 0); - test_user(user(cstr!("daemon")), cstr!("daemon"), 1); + test_user(user(cstr!("root")), cstr!("root"), UserId(ROOT_ID)); + test_user(user(cstr!("daemon")), cstr!("daemon"), UserId(1)); } #[test] fn test_unix_group() { let group = |name| Group::from_name(name).unwrap().unwrap(); - test_group(group(cstr!("root")), "root", 0); - test_group(group(cstr!("daemon")), "daemon", 1); + test_group(group(cstr!("root")), "root", GroupId(ROOT_ID)); + test_group(group(cstr!("daemon")), "daemon", GroupId(1)); } #[test] fn test_default() { impl UnixUser for () {} assert!(!().has_name("root")); - assert!(!().has_uid(0)); + assert!(!().has_uid(UserId(ROOT_ID))); assert!(!().is_root()); assert!(!().in_group_by_name(cstr!("root"))); } diff --git a/src/system/mod.rs b/src/system/mod.rs index 07cc01907..b73355752 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -120,7 +120,7 @@ unsafe fn inner_fork() -> io::Result { if pid == 0 { Ok(ForkResult::Child) } else { - Ok(ForkResult::Parent(pid)) + Ok(ForkResult::Parent(ProcessId(pid))) } } @@ -144,7 +144,7 @@ pub(crate) unsafe fn fork() -> io::Result { } pub fn setsid() -> io::Result { - cerr(unsafe { libc::setsid() }) + cerr(unsafe { ProcessId(libc::setsid()) }) } pub fn hostname() -> String { @@ -201,10 +201,10 @@ pub fn set_target_user( cmd.pre_exec(move || { cerr(libc::setgroups( target_user.groups.len(), - target_user.groups.as_ptr(), + &(*target_user.groups.as_ptr()).0, ))?; - cerr(libc::setgid(target_group.gid))?; - cerr(libc::setuid(target_user.uid))?; + cerr(libc::setgid(target_group.gid.id()))?; + cerr(libc::setuid(target_user.uid.id()))?; Ok(()) }); @@ -215,30 +215,30 @@ pub fn set_target_user( pub fn kill(pid: ProcessId, signal: SignalNumber) -> io::Result<()> { // SAFETY: This function cannot cause UB even if `pid` is not a valid process ID or if // `signal` is not a valid signal code. - cerr(unsafe { libc::kill(pid, signal) }).map(|_| ()) + cerr(unsafe { libc::kill(pid.id(), signal) }).map(|_| ()) } /// Send a signal to a process group with the specified ID. pub fn killpg(pgid: ProcessId, signal: SignalNumber) -> io::Result<()> { // SAFETY: This function cannot cause UB even if `pgid` is not a valid process ID or if // `signal` is not a valid signal code. - cerr(unsafe { libc::killpg(pgid, signal) }).map(|_| ()) + cerr(unsafe { libc::killpg(pgid.id(), signal) }).map(|_| ()) } /// Get the process group ID of the current process. pub fn getpgrp() -> ProcessId { - unsafe { libc::getpgrp() } + unsafe { ProcessId(libc::getpgrp()) } } /// Get a process group ID. pub fn getpgid(pid: ProcessId) -> io::Result { // SAFETY: This function cannot cause UB even if `pid` is not a valid process ID - cerr(unsafe { libc::getpgid(pid) }) + cerr(unsafe { ProcessId(libc::getpgid(pid.id())) }) } /// Set a process group ID. pub fn setpgid(pid: ProcessId, pgid: ProcessId) -> io::Result<()> { - cerr(unsafe { libc::setpgid(pid, pgid) }).map(|_| ()) + cerr(unsafe { libc::setpgid(pid.id(), pgid.id()) }).map(|_| ()) } pub fn chown>( @@ -250,7 +250,7 @@ pub fn chown>( let uid = uid.into(); let gid = gid.into(); - cerr(unsafe { libc::chown(path, uid, gid) }).map(|_| ()) + cerr(unsafe { libc::chown(path, uid.id(), gid.id()) }).map(|_| ()) } #[derive(Debug, Clone, PartialEq)] @@ -271,15 +271,15 @@ impl User { /// (It can cause UB if any of `pwd`'s pointed-to strings does not have a null-terminator.) unsafe fn from_libc(pwd: &libc::passwd) -> Result { let mut buf_len: libc::c_int = 32; - let mut groups_buffer: Vec; + let mut groups_buffer: Vec; while { - groups_buffer = vec![0; buf_len as usize]; + groups_buffer = vec![GroupId(0); buf_len as usize]; let result = unsafe { libc::getgrouplist( pwd.pw_name, pwd.pw_gid, - groups_buffer.as_mut_ptr(), + &mut (*groups_buffer.as_mut_ptr()).0, &mut buf_len, ) }; @@ -298,8 +298,8 @@ impl User { }); Ok(User { - uid: pwd.pw_uid, - gid: pwd.pw_gid, + uid: UserId(pwd.pw_uid), + gid: GroupId(pwd.pw_gid), name: SudoString::new(string_from_ptr(pwd.pw_name))?, gecos: string_from_ptr(pwd.pw_gecos), home: SudoPath::new(os_string_from_ptr(pwd.pw_dir).into())?, @@ -316,7 +316,7 @@ impl User { let mut pwd_ptr = std::ptr::null_mut(); cerr(unsafe { libc::getpwuid_r( - uid, + uid.id(), pwd.as_mut_ptr(), buf.as_mut_ptr(), buf.len(), @@ -332,19 +332,19 @@ impl User { } pub fn effective_uid() -> UserId { - unsafe { libc::geteuid() } + unsafe { UserId(libc::geteuid()) } } pub fn effective_gid() -> GroupId { - unsafe { libc::getegid() } + unsafe { GroupId(libc::getegid()) } } pub fn real_uid() -> UserId { - unsafe { libc::getuid() } + unsafe { UserId(libc::getuid()) } } pub fn real_gid() -> GroupId { - unsafe { libc::getgid() } + unsafe { GroupId(libc::getgid()) } } pub fn real() -> Result, Error> { @@ -404,7 +404,7 @@ impl Group { } Group { - gid: grp.gr_gid, + gid: GroupId(grp.gr_gid), name: string_from_ptr(grp.gr_name), passwd: string_from_ptr(grp.gr_passwd), members, @@ -418,7 +418,7 @@ impl Group { let mut grp_ptr = std::ptr::null_mut(); cerr(unsafe { libc::getgrgid_r( - gid, + gid.id(), grp.as_mut_ptr(), buf.as_mut_ptr(), buf.len(), @@ -504,29 +504,29 @@ impl Process { pub fn process_id() -> ProcessId { // NOTE libstd casts the `i32` that `libc::getpid` returns into `u32` // here we cast it back into `i32` (`ProcessId`) - std::process::id() as ProcessId + ProcessId(std::process::id() as i32) } /// Return the parent process identifier for the current process pub fn parent_id() -> Option { // NOTE libstd casts the `i32` that `libc::getppid` returns into `u32` // here we cast it back into `i32` (`ProcessId`) - let pid = unix::process::parent_id() as ProcessId; + let pid = unix::process::parent_id() as i32; if pid == 0 { None } else { - Some(pid) + Some(ProcessId(pid)) } } /// Return the process group id for the current process pub fn group_id() -> ProcessId { - unsafe { libc::getpgid(0) } + unsafe { ProcessId(libc::getpgid(0)) } } /// Get the session id for the current process pub fn session_id() -> ProcessId { - unsafe { libc::getsid(0) } + unsafe { ProcessId(libc::getsid(0)) } } /// Returns the device identifier of the TTY device that is currently @@ -542,7 +542,7 @@ impl Process { // int. We convert via u32 because a direct conversion to DeviceId // would use sign extension, which would result in a different bit // representation - Ok(Some(data as u32 as DeviceId)) + Ok(Some(DeviceId(data as u64))) } } diff --git a/src/system/signal/info.rs b/src/system/signal/info.rs index 31c31dbd2..8cca3a492 100644 --- a/src/system/signal/info.rs +++ b/src/system/signal/info.rs @@ -21,7 +21,7 @@ impl SignalInfo { /// Gets the PID that sent the signal. pub(crate) fn pid(&self) -> ProcessId { // FIXME: some signals don't set si_pid. - unsafe { self.info.si_pid() } + unsafe { ProcessId(self.info.si_pid()) } } /// Gets the signal number. diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index f23cc8b85..9b97ca2b0 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -150,11 +150,11 @@ pub(crate) trait Terminal: sealed::Sealed { impl Terminal for F { /// Get the foreground process group ID associated with this terminal. fn tcgetpgrp(&self) -> io::Result { - cerr(unsafe { libc::tcgetpgrp(self.as_raw_fd()) }) + cerr(unsafe { ProcessId(libc::tcgetpgrp(self.as_raw_fd())) }) } /// Set the foreground process group ID associated with this terminalto `pgrp`. fn tcsetpgrp(&self, pgrp: ProcessId) -> io::Result<()> { - cerr(unsafe { libc::tcsetpgrp(self.as_raw_fd(), pgrp) }).map(|_| ()) + cerr(unsafe { libc::tcsetpgrp(self.as_raw_fd(), pgrp.id()) }).map(|_| ()) } /// Make the given terminal the controlling terminal of the calling process. diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs index c8b9c4265..900b3f9db 100644 --- a/src/system/timestamp.rs +++ b/src/system/timestamp.rs @@ -420,8 +420,8 @@ impl RecordScope { if let Ok(Some(tty_device)) = tty { if let Ok(init_time) = Process::starting_time(WithProcess::Other(process.session_id)) { Some(RecordScope::Tty { - tty_device, - session_pid: process.session_id, + tty_device: tty_device.id(), + session_pid: process.session_id.id(), init_time, }) } else { @@ -431,7 +431,7 @@ impl RecordScope { } else if let Some(parent_pid) = process.parent_pid { if let Ok(init_time) = Process::starting_time(WithProcess::Other(parent_pid)) { Some(RecordScope::Ppid { - group_pid: parent_pid, + group_pid: parent_pid.id(), init_time, }) } else { @@ -484,7 +484,7 @@ impl SessionRecord { ) -> SessionRecord { SessionRecord { scope, - auth_user, + auth_user: auth_user.id(), timestamp, enabled, } @@ -533,7 +533,12 @@ impl SessionRecord { } }; - Ok(SessionRecord::init(scope, auth_user, enabled, timestamp)) + Ok(SessionRecord::init( + scope, + UserId(auth_user), + enabled, + timestamp, + )) } /// Convert the record to a vector of bytes for storage. @@ -561,7 +566,7 @@ impl SessionRecord { /// Returns true if this record matches the specified scope and is for the /// specified target auth user. pub fn matches(&self, scope: &RecordScope, auth_user: UserId) -> bool { - self.scope == *scope && self.auth_user == auth_user + self.scope == *scope && self.auth_user == auth_user.id() } /// Returns true if this record was written somewhere in the time range diff --git a/src/system/wait.rs b/src/system/wait.rs index ecb081d09..b1c5fbdc7 100644 --- a/src/system/wait.rs +++ b/src/system/wait.rs @@ -27,14 +27,14 @@ impl Wait for ProcessId { fn wait(self, options: WaitOptions) -> Result<(ProcessId, WaitStatus), WaitError> { let mut status: c_int = 0; - let pid = cerr(unsafe { libc::waitpid(self, &mut status, options.flags) }) + let pid = cerr(unsafe { libc::waitpid(self.id(), &mut status, options.flags) }) .map_err(WaitError::Io)?; if pid == 0 && options.flags & WNOHANG != 0 { return Err(WaitError::NotReady); } - Ok((pid, WaitStatus { status })) + Ok((ProcessId(pid), WaitStatus { status })) } } From 7d74b096d1cac77b96df0bb848434f44d5f89df5 Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 04:19:01 +0800 Subject: [PATCH 2/7] Fixed issues#769 : modify tests --- src/common/context.rs | 3 ++- src/sudo/env/tests.rs | 13 +++++++------ src/sudoers/test/mod.rs | 12 ++++++------ src/system/mod.rs | 22 +++++++++++++--------- src/system/term/mod.rs | 5 +++-- src/system/timestamp.rs | 22 +++++++++++----------- src/system/wait.rs | 6 +++--- 7 files changed, 45 insertions(+), 38 deletions(-) diff --git a/src/common/context.rs b/src/common/context.rs index b7b5be458..a631ce290 100644 --- a/src/common/context.rs +++ b/src/common/context.rs @@ -95,6 +95,7 @@ impl Context { #[cfg(test)] mod tests { + use crate::system::interface::UserId; use crate::{sudo::SudoAction, system::hostname}; use std::collections::HashMap; @@ -116,6 +117,6 @@ mod tests { assert_eq!(context.command.command.to_str().unwrap(), "/usr/bin/echo"); assert_eq!(context.command.arguments, ["hello"]); assert_eq!(context.hostname, hostname()); - assert_eq!(context.target_user.uid, 0); + assert_eq!(context.target_user.uid, UserId(0)); } } diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index 4824fe5ed..151542565 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -3,6 +3,7 @@ use crate::sudo::{ cli::{SudoAction, SudoRunOptions}, env::environment::get_target_environment, }; +use crate::system::interface::{GroupId, UserId}; use crate::system::{Group, Process, User}; use std::collections::{HashMap, HashSet}; @@ -81,8 +82,8 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { CommandAndArguments::build_from_args(None, sudo_options.positional_args.clone(), &path); let current_user = User { - uid: 1000, - gid: 1000, + uid: UserId(1000), + gid: GroupId(1000), name: "test".into(), gecos: String::new(), @@ -93,15 +94,15 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { }; let current_group = Group { - gid: 1000, + gid: GroupId(1000), name: "test".to_string(), passwd: String::new(), members: Vec::new(), }; let root_user = User { - uid: 0, - gid: 0, + uid: UserId(0), + gid: GroupId(0), name: "root".into(), gecos: String::new(), home: "/root".into(), @@ -111,7 +112,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { }; let root_group = Group { - gid: 0, + gid: GroupId(0), name: "root".to_string(), passwd: String::new(), members: Vec::new(), diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs index 21482cf84..038dfedd7 100644 --- a/src/sudoers/test/mod.rs +++ b/src/sudoers/test/mod.rs @@ -20,16 +20,16 @@ impl UnixUser for Named { self.0 == name } - fn has_uid(&self, uid: u32) -> bool { - dummy_cksum(self.0) == uid + fn has_uid(&self, uid: UserId) -> bool { + dummy_cksum(self.0) == uid.id() } fn in_group_by_name(&self, name: &CStr) -> bool { self.has_name(name.to_str().unwrap()) } - fn in_group_by_gid(&self, gid: u32) -> bool { - dummy_cksum(self.0) == gid + fn in_group_by_gid(&self, gid: GroupId) -> bool { + dummy_cksum(self.0) == gid.id() } fn is_root(&self) -> bool { @@ -39,7 +39,7 @@ impl UnixUser for Named { impl UnixGroup for Named { fn as_gid(&self) -> crate::system::interface::GroupId { - dummy_cksum(self.0) + GroupId(dummy_cksum(self.0)) } fn try_as_name(&self) -> Option<&str> { Some(self.0) @@ -175,7 +175,7 @@ fn permission_test() { pass!(["user ALL=(ALL:ALL) /bin/foo"], "user" => request! { user, user }, "server"; "/bin/foo" => [authenticate: Authenticate::Nopasswd]); pass!(["user ALL=(ALL:ALL) /bin/foo"], "user" => request! { user, root }, "server"; "/bin/foo" => [authenticate: Authenticate::None]); - assert_eq!(Named("user").as_gid(), 1466); + assert_eq!(Named("user").as_gid(), GroupId(1466)); pass!(["#1466 server=(ALL:ALL) ALL"], "user" => root(), "server"; "/bin/hello"); pass!(["%#1466 server=(ALL:ALL) ALL"], "user" => root(), "server"; "/bin/hello"); FAIL!(["#1466 server=(ALL:ALL) ALL"], "root" => root(), "server"; "/bin/hello"); diff --git a/src/system/mod.rs b/src/system/mod.rs index b73355752..6eba9cafc 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -640,6 +640,10 @@ mod tests { process::exit, }; + use crate::system::GroupId; + use crate::system::ProcessId; + use crate::system::UserId; + use libc::SIGKILL; use super::{ @@ -668,13 +672,13 @@ mod tests { fn test_get_user_and_group_by_id() { let fixed_users = &[(0, "root"), (1, "daemon")]; for &(id, name) in fixed_users { - let root = User::from_uid(id).unwrap().unwrap(); - assert_eq!(root.uid, id as libc::uid_t); + let root = User::from_uid(UserId(id)).unwrap().unwrap(); + assert_eq!(root.uid, UserId(id as libc::uid_t)); assert_eq!(root.name, name); } for &(id, name) in fixed_users { - let root = Group::from_gid(id).unwrap().unwrap(); - assert_eq!(root.gid, id as libc::gid_t); + let root = Group::from_gid(GroupId(id)).unwrap().unwrap(); + assert_eq!(root.gid, GroupId(id as libc::gid_t)); assert_eq!(root.name, name); } } @@ -708,7 +712,7 @@ mod tests { Group { name: name.to_string(), passwd: passwd.to_string(), - gid, + gid: GroupId(gid), members: mem.iter().map(|s| s.to_string()).collect(), } ) @@ -737,8 +741,8 @@ mod tests { use super::{getpgid, setpgid}; let pgrp = getpgrp(); - assert_eq!(getpgid(0).unwrap(), pgrp); - assert_eq!(getpgid(std::process::id() as i32).unwrap(), pgrp); + assert_eq!(getpgid(ProcessId(0)).unwrap(), pgrp); + assert_eq!(getpgid(ProcessId(std::process::id() as i32)).unwrap(), pgrp); match super::fork().unwrap() { ForkResult::Child => { @@ -747,7 +751,7 @@ mod tests { } ForkResult::Parent(child_pid) => { // The child should be in our process group. - assert_eq!(getpgid(child_pid).unwrap(), getpgid(0).unwrap(),); + assert_eq!(getpgid(child_pid).unwrap(), getpgid(ProcessId(0)).unwrap(),); // Move the child to its own process group setpgid(child_pid, child_pid).unwrap(); // The process group of the child should have changed. @@ -761,7 +765,7 @@ mod tests { .arg("1") .spawn() .unwrap(); - super::kill(child.id() as i32, SIGKILL).unwrap(); + super::kill(ProcessId(child.id() as i32), SIGKILL).unwrap(); assert!(!child.wait().unwrap().success()); } #[test] diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index 9b97ca2b0..fe7fb1bba 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -213,6 +213,7 @@ mod tests { process::exit, }; + use crate::system::interface::ProcessId; use crate::system::{fork, getpgid, setsid, term::*, ForkResult}; #[test] @@ -235,13 +236,13 @@ mod tests { // Open a new pseudoterminal. let leader = Pty::open().unwrap().leader; // The pty leader should not have a foreground process group yet. - assert_eq!(leader.tcgetpgrp().unwrap(), 0); + assert_eq!(leader.tcgetpgrp().unwrap(), ProcessId(0)); // Create a new session so we can change the controlling terminal. setsid().unwrap(); // Set the pty leader as the controlling terminal. leader.make_controlling_terminal().unwrap(); // Set us as the foreground process group of the pty leader. - let pgid = getpgid(0).unwrap(); + let pgid = getpgid(ProcessId(0)).unwrap(); leader.tcsetpgrp(pgid).unwrap(); // Check that we are in fact the foreground process group of the pty leader. assert_eq!(pgid, leader.tcgetpgrp().unwrap()); diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs index 900b3f9db..3cef47f2f 100644 --- a/src/system/timestamp.rs +++ b/src/system/timestamp.rs @@ -583,7 +583,7 @@ mod tests { use crate::system::tests::tempfile; - const TEST_USER_ID: UserId = 1000; + const TEST_USER_ID: UserId = UserId(1000); impl SetLength for Cursor> { fn set_len(&mut self, new_len: usize) -> io::Result<()> { @@ -613,7 +613,7 @@ mod tests { session_pid: 42, init_time: SystemTime::now().unwrap() - Duration::seconds(150), }, - 999, + UserId(999), ) .unwrap(); @@ -633,7 +633,7 @@ mod tests { group_pid: 42, init_time: SystemTime::now().unwrap(), }, - 123, + UserId(123), ) .unwrap(); let bytes = ppid_sample.as_bytes().unwrap(); @@ -650,24 +650,24 @@ mod tests { init_time, }; - let tty_sample = SessionRecord::new(scope, 675).unwrap(); + let tty_sample = SessionRecord::new(scope, UserId(675)).unwrap(); - assert!(tty_sample.matches(&scope, 675)); - assert!(!tty_sample.matches(&scope, 789)); + assert!(tty_sample.matches(&scope, UserId(675))); + assert!(!tty_sample.matches(&scope, UserId(789))); assert!(!tty_sample.matches( &RecordScope::Tty { tty_device: 20, session_pid: 1234, init_time }, - 675 + UserId(675) )); assert!(!tty_sample.matches( &RecordScope::Ppid { group_pid: 42, init_time }, - 675 + UserId(675) )); // make sure time is different @@ -678,7 +678,7 @@ mod tests { session_pid: 1234, init_time: SystemTime::now().unwrap() }, - 675 + UserId(675) )); } @@ -690,7 +690,7 @@ mod tests { session_pid: 1234, init_time: some_time, }; - let sample = SessionRecord::init(scope, 1234, true, some_time); + let sample = SessionRecord::init(scope, UserId(1234), true, some_time); let dur = Duration::seconds(30); @@ -755,7 +755,7 @@ mod tests { session_pid: 0, init_time: SystemTime::new(0, 0), }; - let auth_user = 2424; + let auth_user = UserId(2424); let res = srf.create(tty_scope, auth_user).unwrap(); let CreateResult::Created { time } = res else { panic!("Expected record to be created"); diff --git a/src/system/wait.rs b/src/system/wait.rs index b1c5fbdc7..8aae14766 100644 --- a/src/system/wait.rs +++ b/src/system/wait.rs @@ -168,7 +168,7 @@ mod tests { .spawn() .unwrap(); - let command_pid = command.id() as ProcessId; + let command_pid = ProcessId(command.id() as i32); let (pid, status) = command_pid.wait(WaitOptions::new()).unwrap(); assert_eq!(command_pid, pid); @@ -195,7 +195,7 @@ mod tests { .spawn() .unwrap(); - let command_pid = command.id() as ProcessId; + let command_pid = ProcessId(command.id() as i32); kill(command_pid, SIGSTOP).unwrap(); @@ -224,7 +224,7 @@ mod tests { .spawn() .unwrap(); - let command_pid = command.id() as ProcessId; + let command_pid = ProcessId(command.id() as i32); let mut count = 0; let (pid, status) = loop { From 986364e832ea013e24323b98d34acf61816daff7 Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 04:31:52 +0800 Subject: [PATCH 3/7] Fixed issues#769 : modify ci --- src/common/error.rs | 6 +++--- src/sudo/pipeline.rs | 2 +- src/system/interface.rs | 27 ++++++++++++--------------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/common/error.rs b/src/common/error.rs index b022e02a5..b4193fc13 100644 --- a/src/common/error.rs +++ b/src/common/error.rs @@ -25,7 +25,7 @@ pub enum Error { Configuration(String), Options(String), Pam(PamError), - IoError(Option, std::io::Error), + IoErr(Option, std::io::Error), MaxAuthAttempts(usize), PathValidation(PathBuf), StringValidation(String), @@ -64,7 +64,7 @@ impl fmt::Display for Error { Error::Configuration(e) => write!(f, "invalid configuration: {e}"), Error::Options(e) => write!(f, "{e}"), Error::Pam(e) => write!(f, "PAM error: {e}"), - Error::IoError(location, e) => { + Error::IoErr(location, e) => { if let Some(path) = location { write!(f, "cannot execute '{}': {e}", path.display()) } else { @@ -98,7 +98,7 @@ impl From for Error { impl From for Error { fn from(err: std::io::Error) -> Self { - Error::IoError(None, err) + Error::IoErr(None, err) } } diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 56890e75f..a052ad685 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -86,7 +86,7 @@ impl Pipeline { log_command_execution(&context); crate::exec::run_command(&context, target_env) - .map_err(|io_error| Error::IoError(Some(context.command.command), io_error)) + .map_err(|io_error| Error::IoErr(Some(context.command.command), io_error)) } else { Err(Error::CommandNotFound(context.command.command)) }; diff --git a/src/system/interface.rs b/src/system/interface.rs index 5e3eb7e8b..e8670c4a3 100644 --- a/src/system/interface.rs +++ b/src/system/interface.rs @@ -1,41 +1,40 @@ -use core::convert::Infallible; -use std::convert::TryFrom; +use std::convert::From; use std::ffi::CStr; use std::fmt; use std::str::FromStr; #[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(crate) struct GroupId(pub(crate) libc::gid_t); +pub struct GroupId(pub libc::gid_t); #[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(crate) struct UserId(pub(crate) libc::uid_t); +pub struct UserId(pub libc::uid_t); #[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(crate) struct ProcessId(pub(crate) libc::pid_t); +pub struct ProcessId(pub libc::pid_t); #[derive(Copy, Clone, Debug, PartialEq, Eq)] -pub(crate) struct DeviceId(pub(crate) libc::dev_t); +pub struct DeviceId(pub libc::dev_t); pub(crate) const ROOT_ID: u32 = 0; pub(crate) const ROOT: UserId = UserId(ROOT_ID); impl UserId { - pub(crate) fn id(&self) -> u32 { + pub fn id(&self) -> u32 { self.0 } } impl ProcessId { - pub(crate) fn id(&self) -> i32 { + pub fn id(&self) -> i32 { self.0 } } impl GroupId { - pub(crate) fn id(&self) -> u32 { + pub fn id(&self) -> u32 { self.0 } } impl DeviceId { - pub(crate) fn id(&self) -> u64 { + pub fn id(&self) -> u64 { self.0 } } @@ -72,11 +71,9 @@ impl fmt::Display for ProcessId { } } -impl TryFrom for i64 { - type Error = Infallible; - - fn try_from(value: ProcessId) -> Result { - value.0.try_into() +impl From for i64 { + fn from(value: ProcessId) -> Self { + value.0.into() } } From 7fb43915b9c77f019025e3cb9a6377f1a72e0bc6 Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 15:10:39 +0800 Subject: [PATCH 4/7] Fixed issues#769 : add test --- src/system/interface.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/system/interface.rs b/src/system/interface.rs index e8670c4a3..66121aa7d 100644 --- a/src/system/interface.rs +++ b/src/system/interface.rs @@ -174,6 +174,42 @@ mod test { use super::*; + #[test] + fn test_user_id() { + let uid = UserId(1000); + assert_eq!(uid.id(), 1000); + let parsed_uid: UserId = "1000".parse().unwrap(); + assert_eq!(parsed_uid, uid); + assert_eq!(format!("{}", uid), "1000"); + } + + #[test] + fn test_group_id() { + let gid = GroupId(1000); + assert_eq!(gid.id(), 1000); + let parsed_gid: GroupId = "1000".parse().unwrap(); + assert_eq!(parsed_gid, gid); + assert_eq!(format!("{}", gid), "1000"); + } + + #[test] + fn test_process_id() { + let pid = ProcessId(1000); + assert_eq!(pid.id(), 1000); + let parsed_pid: ProcessId = "1000".parse().unwrap(); + assert_eq!(parsed_pid, pid); + assert_eq!(format!("{}", pid), "1000"); + } + + #[test] + fn test_device_id() { + let did = DeviceId(1000); + assert_eq!(did.id(), 1000); + let parsed_did: DeviceId = "1000".parse().unwrap(); + assert_eq!(parsed_did, did); + assert_eq!(format!("{}", did), "1000"); + } + fn test_user(user: impl UnixUser, name_c: &CStr, uid: UserId) { let name = name_c.to_str().unwrap(); assert!(user.has_name(name)); From 4e4bfe1d92f672f550fdfb295402845c855f9cd9 Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 15:45:47 +0800 Subject: [PATCH 5/7] Fixed issues#769 : modify --- src/exec/use_pty/monitor.rs | 9 +++------ src/system/mod.rs | 18 ++++++++---------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/exec/use_pty/monitor.rs b/src/exec/use_pty/monitor.rs index 98c3737d4..c64330cc4 100644 --- a/src/exec/use_pty/monitor.rs +++ b/src/exec/use_pty/monitor.rs @@ -198,17 +198,14 @@ fn exec_command( original_set: Option, ) -> io::Error { // FIXME (ogsudo): Do any additional configuration that needs to be run after `fork` but before `exec` - let command_pid = std::process::id() as i32; + let command_pid = ProcessId(std::process::id() as i32); - setpgid(ProcessId(0), ProcessId(command_pid)).ok(); + setpgid(ProcessId(0), command_pid).ok(); // Wait for the monitor to set us as the foreground group for the pty if we are in the // foreground. if foreground { - while !pty_follower - .tcgetpgrp() - .is_ok_and(|pid| pid == ProcessId(command_pid)) - { + while !pty_follower.tcgetpgrp().is_ok_and(|pid| pid == command_pid) { std::thread::sleep(std::time::Duration::from_micros(1)); } } diff --git a/src/system/mod.rs b/src/system/mod.rs index 6eba9cafc..e2ce1a3c9 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -116,11 +116,11 @@ pub(crate) enum ForkResult { } unsafe fn inner_fork() -> io::Result { - let pid = cerr(unsafe { libc::fork() })?; - if pid == 0 { + let pid = cerr(unsafe { ProcessId(libc::fork()) })?; + if pid == ProcessId(0) { Ok(ForkResult::Child) } else { - Ok(ForkResult::Parent(ProcessId(pid))) + Ok(ForkResult::Parent(pid)) } } @@ -201,7 +201,7 @@ pub fn set_target_user( cmd.pre_exec(move || { cerr(libc::setgroups( target_user.groups.len(), - &(*target_user.groups.as_ptr()).0, + &(*target_user.groups.as_ptr()).id(), ))?; cerr(libc::setgid(target_group.gid.id()))?; cerr(libc::setuid(target_user.uid.id()))?; @@ -511,12 +511,10 @@ impl Process { pub fn parent_id() -> Option { // NOTE libstd casts the `i32` that `libc::getppid` returns into `u32` // here we cast it back into `i32` (`ProcessId`) - let pid = unix::process::parent_id() as i32; - if pid == 0 { - None - } else { - Some(ProcessId(pid)) - } + let pid = ProcessId(unix::process::parent_id() as i32); + + // Return None if the parent process ID is 0, else wrap in Some + (pid != ProcessId(0)).then_some(pid) } /// Return the process group id for the current process From 2f7ff647633491828a89f9421ae9b5d31f9d87ae Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 16:11:03 +0800 Subject: [PATCH 6/7] Fixed issues#769 : modify --- src/exec/interface.rs | 7 +++---- src/exec/mod.rs | 8 +++++--- src/su/context.rs | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/exec/interface.rs b/src/exec/interface.rs index 8a5284507..49013d36d 100644 --- a/src/exec/interface.rs +++ b/src/exec/interface.rs @@ -2,7 +2,6 @@ use std::io::{self, ErrorKind}; use std::path::PathBuf; use crate::common::SudoPath; -use crate::system::interface::ProcessId; use crate::{ common::{context::LaunchType, Context}, system::{Group, User}, @@ -17,7 +16,7 @@ pub trait RunOptions { fn user(&self) -> &User; fn requesting_user(&self) -> &User; fn group(&self) -> &Group; - fn pid(&self) -> ProcessId; + fn pid(&self) -> i32; fn use_pty(&self) -> bool; } @@ -58,8 +57,8 @@ impl RunOptions for Context { &self.target_group } - fn pid(&self) -> ProcessId { - self.process.pid + fn pid(&self) -> i32 { + self.process.pid.id() } fn use_pty(&self) -> bool { diff --git a/src/exec/mod.rs b/src/exec/mod.rs index 7d1bbc093..15d2b3709 100644 --- a/src/exec/mod.rs +++ b/src/exec/mod.rs @@ -106,16 +106,18 @@ fn run_command_internal(options: &impl RunOptions, env: Environment) -> io::Resu } } + let pid = ProcessId(options.pid()); + if options.use_pty() { match UserTerm::open() { - Ok(user_tty) => exec_pty(options.pid(), command, user_tty), + Ok(user_tty) => exec_pty(pid, command, user_tty), Err(err) => { dev_info!("Could not open user's terminal, not allocating a pty: {err}"); - exec_no_pty(options.pid(), command) + exec_no_pty(pid, command) } } } else { - exec_no_pty(options.pid(), command) + exec_no_pty(pid, command) } } diff --git a/src/su/context.rs b/src/su/context.rs index c152da752..42093a9b8 100644 --- a/src/su/context.rs +++ b/src/su/context.rs @@ -12,7 +12,7 @@ use crate::common::{ }; use crate::exec::RunOptions; use crate::log::user_warn; -use crate::system::interface::{ProcessId, ROOT}; +use crate::system::interface::ROOT; use crate::system::{Group, Process, User}; use super::cli::SuRunOptions; @@ -241,8 +241,8 @@ impl RunOptions for SuContext { &self.group } - fn pid(&self) -> ProcessId { - self.process.pid + fn pid(&self) -> i32 { + self.process.pid.id() } fn use_pty(&self) -> bool { From eb5f4888b172c19b525318094aaa255973eb1d50 Mon Sep 17 00:00:00 2001 From: blackanger Date: Fri, 29 Dec 2023 22:06:47 +0800 Subject: [PATCH 7/7] Fixed issues#769 : modify --- src/system/interface.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/system/interface.rs b/src/system/interface.rs index 66121aa7d..8e163401d 100644 --- a/src/system/interface.rs +++ b/src/system/interface.rs @@ -177,7 +177,7 @@ mod test { #[test] fn test_user_id() { let uid = UserId(1000); - assert_eq!(uid.id(), 1000); + assert_eq!(uid.id(), 1000u32); let parsed_uid: UserId = "1000".parse().unwrap(); assert_eq!(parsed_uid, uid); assert_eq!(format!("{}", uid), "1000"); @@ -186,7 +186,7 @@ mod test { #[test] fn test_group_id() { let gid = GroupId(1000); - assert_eq!(gid.id(), 1000); + assert_eq!(gid.id(), 1000u32); let parsed_gid: GroupId = "1000".parse().unwrap(); assert_eq!(parsed_gid, gid); assert_eq!(format!("{}", gid), "1000"); @@ -195,7 +195,7 @@ mod test { #[test] fn test_process_id() { let pid = ProcessId(1000); - assert_eq!(pid.id(), 1000); + assert_eq!(pid.id(), 1000i32); let parsed_pid: ProcessId = "1000".parse().unwrap(); assert_eq!(parsed_pid, pid); assert_eq!(format!("{}", pid), "1000"); @@ -204,7 +204,7 @@ mod test { #[test] fn test_device_id() { let did = DeviceId(1000); - assert_eq!(did.id(), 1000); + assert_eq!(did.id(), 1000u64); let parsed_did: DeviceId = "1000".parse().unwrap(); assert_eq!(parsed_did, did); assert_eq!(format!("{}", did), "1000");