Add newtypes for UserId, GroupId and ProcessId

src/common/context.rs

@@ -95,7 +95,10 @@ impl Context {
 
 #[cfg(test)]
 mod tests {
-    use crate::{sudo::SudoAction, system::Hostname};
+    use crate::{
+        sudo::SudoAction,
+        system::{interface::UserId, Hostname},
+    };
     use std::collections::HashMap;
 
     use super::Context;
@@ -121,6 +124,6 @@ mod tests {
         }
         assert_eq!(context.command.arguments, ["hello"]);
         assert_eq!(context.hostname, Hostname::resolve());
-        assert_eq!(context.target_user.uid, 0);
+        assert_eq!(context.target_user.uid, UserId::ROOT);
     }
 }

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,8 @@ 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 +59,7 @@ impl RunOptions for Context {
         &self.target_group
     }
 
-    fn pid(&self) -> i32 {
+    fn pid(&self) -> ProcessId {
         self.process.pid
     }
 

src/exec/no_pty.rs

@@ -148,7 +148,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.is_valid() {
             if Some(signaler_pid) == self.command_pid {
                 return true;
             }

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::new(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.inner(),
             ParentMessage::CommandStatus(status) => match status {
                 CommandStatus::Exit(data)
                 | CommandStatus::Term(data)

src/exec/use_pty/monitor.rs

@@ -199,9 +199,9 @@ fn exec_command(
     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::new(std::process::id() as i32);
 
-    setpgid(0, command_pid).ok();
+    setpgid(ProcessId::new(0), command_pid).ok();
 
     // Wait for the monitor to set us as the foreground group for the pty if we are in the
     // foreground.
@@ -415,7 +415,7 @@ fn is_self_terminating(
     command_pid: ProcessId,
     command_pgrp: ProcessId,
 ) -> bool {
-    if signaler_pid != 0 {
+    if signaler_pid.is_valid() {
         if signaler_pid == command_pid {
             return true;
         }

src/exec/use_pty/parent.rs

@@ -433,7 +433,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.is_valid() {
             if Some(signaler_pid) == self.command_pid {
                 return true;
             }

src/su/context.rs

@@ -5,11 +5,17 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use crate::common::{error::Error, resolve::CurrentUser, Environment};
-use crate::common::{resolve::is_valid_executable, SudoPath};
 use crate::exec::RunOptions;
 use crate::log::user_warn;
 use crate::system::{Group, Process, User};
+use crate::{
+    common::{error::Error, resolve::CurrentUser, Environment},
+    system::interface::ProcessId,
+};
+use crate::{
+    common::{resolve::is_valid_executable, SudoPath},
+    system::interface::UserId,
+};
 
 use super::cli::SuRunOptions;
 
@@ -79,7 +85,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() == UserId::ROOT;
         let is_target_root = options.user == "root";
 
         // only root can set a (additional) group
@@ -237,7 +243,7 @@ impl RunOptions for SuContext {
         &self.group
     }
 
-    fn pid(&self) -> i32 {
+    fn pid(&self) -> ProcessId {
         self.process.pid
     }
 

src/sudo/env/tests.rs

@@ -4,6 +4,7 @@ use crate::sudo::{
     cli::{SudoAction, SudoRunOptions},
     env::environment::get_target_environment,
 };
+use crate::system::interface::{GroupId, UserId};
 use crate::system::{Group, Hostname, Process, User};
 use std::collections::{HashMap, HashSet};
 
@@ -82,8 +83,8 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
         CommandAndArguments::build_from_args(None, sudo_options.positional_args.clone(), &path);
 
     let current_user = CurrentUser::fake(User {
-        uid: 1000,
-        gid: 1000,
+        uid: UserId::new(1000),
+        gid: GroupId::new(1000),
 
         name: "test".into(),
         gecos: String::new(),
@@ -94,13 +95,13 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
     });
 
     let current_group = Group {
-        gid: 1000,
+        gid: GroupId::new(1000),
         name: "test".to_string(),
     };
 
     let root_user = User {
-        uid: 0,
-        gid: 0,
+        uid: UserId::ROOT,
+        gid: GroupId::new(0),
         name: "root".into(),
         gecos: String::new(),
         home: "/root".into(),
@@ -110,7 +111,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
     };
 
     let root_group = Group {
-        gid: 0,
+        gid: GroupId::new(0),
         name: "root".to_string(),
     };
 

src/sudo/mod.rs

@@ -3,6 +3,7 @@
 use crate::common::resolve::CurrentUser;
 use crate::common::{Context, Error};
 use crate::log::dev_info;
+use crate::system::interface::UserId;
 use crate::system::kernel::kernel_check;
 use crate::system::timestamp::RecordScope;
 use crate::system::User;
@@ -143,10 +144,8 @@ fn sudo_process() -> Result<(), Error> {
 }
 
 fn self_check() -> Result<(), Error> {
-    const ROOT: u32 = 0;
-
     let euid = User::effective_uid();
-    if euid == ROOT {
+    if euid == UserId::ROOT {
         Ok(())
     } else {
         Err(Error::SelfCheck)

src/sudo/pipeline/list.rs

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

src/sudoers/mod.rs

@@ -15,7 +15,7 @@ use std::{io, mem};
 
 use crate::common::resolve::resolve_path;
 use crate::log::auth_warn;
-use crate::system::interface::{UnixGroup, UnixUser};
+use crate::system::interface::{GroupId, UnixGroup, UnixUser, UserId};
 use crate::system::{self, can_execute};
 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::new(*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::new(*num),
         Identifier::Name(name) => group.try_as_name().map_or(false, |s| name == s),
     }
 }
@@ -508,7 +508,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::new(*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 {
+        UserId::new(dummy_cksum(self.0)) == uid
     }
 
     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 {
+        GroupId::new(dummy_cksum(self.0)) == gid
     }
 
     fn is_root(&self) -> bool {
@@ -38,8 +38,8 @@ impl UnixUser for Named {
 }
 
 impl UnixGroup for Named {
-    fn as_gid(&self) -> crate::system::interface::GroupId {
-        dummy_cksum(self.0)
+    fn as_gid(&self) -> GroupId {
+        GroupId::new(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::new(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

@@ -15,6 +15,6 @@ impl Chown for File {
 
         // SAFETY: `fchown` is passed a proper file descriptor; and even if the user/group id
         // is invalid, it will not cause UB.
-        cerr(unsafe { libc::fchown(fd, owner, group) }).map(|_| ())
+        cerr(unsafe { libc::fchown(fd, owner.inner(), group.inner()) }).map(|_| ())
     }
 }