Fixed issues#769 : Add newtypes for UserId, GroupId and ProcessId

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

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));
     }
 }

src/common/error.rs

@@ -25,7 +25,7 @@
     Configuration(String),
     Options(String),
     Pam(PamError),
-    IoError(Option<PathBuf>, std::io::Error),
+    IoErr(Option<PathBuf>, std::io::Error),
     MaxAuthAttempts(usize),
     PathValidation(PathBuf),
     StringValidation(String),
@@ -64,7 +64,7 @@
             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<std::io::Error> for Error {
     fn from(err: std::io::Error) -> Self {
-        Error::IoError(None, err)
+        Error::IoErr(None, err)
     }
 }
 

src/exec/interface.rs

@@ -58,7 +58,7 @@
     }
 
     fn pid(&self) -> i32 {
-        self.process.pid
+        self.process.pid.id()
     }
 
     fn use_pty(&self) -> bool {

src/exec/mod.rs

@@ -106,16 +106,18 @@
         }
     }
 
+    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)
     }
 }
 

src/exec/no_pty.rs

@@ -147,7 +147,7 @@
     /// - 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;
             }

src/exec/use_pty/backchannel.rs

@@ -73,7 +73,7 @@
             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 @@
         };
 
         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)

src/exec/use_pty/monitor.rs

@@ -198,9 +198,9 @@
     original_set: Option<SignalSet>,
 ) -> 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 = ProcessId(std::process::id() as i32);
 
-    setpgid(0, 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.
@@ -413,7 +413,7 @@
     command_pid: ProcessId,
     command_pgrp: ProcessId,
 ) -> bool {
-    if signaler_pid != 0 {
+    if signaler_pid != ProcessId(0) {
         if signaler_pid == command_pid {
             return true;
         }

src/exec/use_pty/parent.rs

@@ -432,7 +432,7 @@
     /// - 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;
             }

src/su/context.rs

@@ -12,6 +12,7 @@
 };
 use crate::exec::RunOptions;
 use crate::log::user_warn;
+use crate::system::interface::ROOT;
 use crate::system::{Group, Process, User};
 
 use super::cli::SuRunOptions;
@@ -82,7 +83,7 @@
             .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
@@ -241,7 +242,7 @@
     }
 
     fn pid(&self) -> i32 {
-        self.process.pid
+        self.process.pid.id()
     }
 
     fn use_pty(&self) -> bool {

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(),

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(())

src/sudo/pipeline.rs

@@ -86,7 +86,7 @@
             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))
         };

src/sudo/pipeline/list.rs

@@ -5,6 +5,7 @@
     pam::CLIConverser,
     sudo::{cli::SudoListOptions, pam::PamAuthenticator, SudoersPolicy},
     sudoers::{Authorization, ListRequest, Policy, Request, Sudoers},
+    system::interface::ROOT,
     system::User,
 };
 
@@ -87,7 +88,7 @@
             }
 
             Authorization::Forbidden => {
-                if context.current_user.uid == 0 {
+                if context.current_user.uid == ROOT {
                     if original_command.is_some() {
                         return Err(Error::Silent);
                     }

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)),
     }
 }
 

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");

src/system/file/chown.rs

@@ -13,6 +13,6 @@
     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(|_| ())
     }
 }