Repository: memorysafety/sudo-rs
Pull request #819 (2024-01-16 22:54:43)

Use a ring buffer to pipe data between IO objects
This PR supersedes #617



Patch:
diff --git a/README.md b/README.md
index e5a34ea6..15b379ef 100644
--- a/README.md
+++ b/README.md
@@ -23,16 +23,6 @@ Linux distribution does not package that version (or a later one), you can alway
 install the most recent version through [rustup]. You also need the C development
 files for PAM (`libpam0g-dev` on Debian, `pam-devel` on Fedora).
 
-On Ubuntu or Debian-based systems, use the following command to install the PAM development library:
-```
-sudo apt-get install libpam0g-dev
-```
-
-On CentOS or Red Hat-based systems, you can use the following command:
-```
-sudo yum install pam-devel
-```
-
 With dependencies installed, building sudo-rs is a simple matter of:
 ```
 cargo build --release
diff --git a/src/common/context.rs b/src/common/context.rs
index c9412063..b7b5be45 100644
--- a/src/common/context.rs
+++ b/src/common/context.rs
@@ -1,11 +1,11 @@
 use crate::common::{HARDENED_ENUM_VALUE_0, HARDENED_ENUM_VALUE_1, HARDENED_ENUM_VALUE_2};
-use crate::system::{Group, Hostname, Process, User};
+use crate::system::{hostname, Group, Process, User};
 
-use super::resolve::CurrentUser;
+use super::path::SudoPath;
 use super::{
     command::CommandAndArguments,
-    resolve::{resolve_launch_and_shell, resolve_target_user_and_group},
-    Error, SudoPath, SudoString,
+    resolve::{resolve_current_user, resolve_launch_and_shell, resolve_target_user_and_group},
+    Error, SudoString,
 };
 
 #[derive(Clone, Copy)]
@@ -41,8 +41,8 @@ pub struct Context {
     pub non_interactive: bool,
     pub use_session_records: bool,
     // system
-    pub hostname: Hostname,
-    pub current_user: CurrentUser,
+    pub hostname: String,
+    pub current_user: User,
     pub process: Process,
     // policy
     pub use_pty: bool,
@@ -61,8 +61,8 @@ impl Context {
         sudo_options: OptionsForContext,
         path: String,
     ) -> Result<Context, Error> {
-        let hostname = Hostname::resolve();
-        let current_user = CurrentUser::resolve()?;
+        let hostname = hostname();
+        let current_user = resolve_current_user()?;
         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);
@@ -95,7 +95,7 @@ impl Context {
 
 #[cfg(test)]
 mod tests {
-    use crate::{sudo::SudoAction, system::Hostname};
+    use crate::{sudo::SudoAction, system::hostname};
     use std::collections::HashMap;
 
     use super::Context;
@@ -115,7 +115,7 @@ mod tests {
 
         assert_eq!(context.command.command.to_str().unwrap(), "/usr/bin/echo");
         assert_eq!(context.command.arguments, ["hello"]);
-        assert_eq!(context.hostname, Hostname::resolve());
+        assert_eq!(context.hostname, hostname());
         assert_eq!(context.target_user.uid, 0);
     }
 }
diff --git a/src/common/error.rs b/src/common/error.rs
index f30d19a1..97829e6b 100644
--- a/src/common/error.rs
+++ b/src/common/error.rs
@@ -1,4 +1,4 @@
-use crate::{pam::PamError, system::Hostname};
+use crate::pam::PamError;
 use std::{borrow::Cow, fmt, path::PathBuf};
 
 use super::{SudoPath, SudoString};
@@ -9,7 +9,7 @@ pub enum Error {
     NotAllowed {
         username: SudoString,
         command: Cow<'static, str>,
-        hostname: Hostname,
+        hostname: String,
         other_user: Option<SudoString>,
     },
     SelfCheck,
diff --git a/src/common/resolve.rs b/src/common/resolve.rs
index 118e3b5c..ac2810e7 100644
--- a/src/common/resolve.rs
+++ b/src/common/resolve.rs
@@ -1,9 +1,7 @@
 use crate::system::{Group, User};
-use core::fmt;
+use std::ffi::CStr;
 use std::{
-    env,
-    ffi::CStr,
-    fs, io, ops,
+    env, fs, io,
     os::unix::prelude::MetadataExt,
     path::{Path, PathBuf},
     str::FromStr,
@@ -33,42 +31,8 @@ impl<'a, T: FromStr> NameOrId<'a, T> {
     }
 }
 
-#[derive(Clone)]
-pub struct CurrentUser {
-    inner: User,
-}
-
-impl From<CurrentUser> for User {
-    fn from(value: CurrentUser) -> Self {
-        value.inner
-    }
-}
-
-impl fmt::Debug for CurrentUser {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("CurrentUser").field(&self.inner).finish()
-    }
-}
-
-impl ops::Deref for CurrentUser {
-    type Target = User;
-
-    fn deref(&self) -> &Self::Target {
-        &self.inner
-    }
-}
-
-impl CurrentUser {
-    #[cfg(test)]
-    pub fn fake(user: User) -> Self {
-        Self { inner: user }
-    }
-
-    pub fn resolve() -> Result<Self, Error> {
-        Ok(Self {
-            inner: User::real()?.ok_or(Error::UserNotFound("current user".to_string()))?,
-        })
-    }
+pub fn resolve_current_user() -> Result<User, Error> {
+    User::real()?.ok_or(Error::UserNotFound("current user".to_string()))
 }
 
 type Shell = Option<PathBuf>;
@@ -94,7 +58,7 @@ pub(super) fn resolve_launch_and_shell(
 pub(crate) fn resolve_target_user_and_group(
     target_user_name_or_id: &Option<SudoString>,
     target_group_name_or_id: &Option<SudoString>,
-    current_user: &CurrentUser,
+    current_user: &User,
 ) -> Result<(User, Group), Error> {
     // resolve user name or #<id> to a user
     let mut target_user =
@@ -107,7 +71,7 @@ pub(crate) fn resolve_target_user_and_group(
     match (&target_user_name_or_id, &target_group_name_or_id) {
         // when -g is specified, but -u is not specified default -u to the current user
         (None, Some(_)) => {
-            target_user = Some(current_user.clone().into());
+            target_user = Some(current_user.clone());
         }
         // when -u is specified but -g is not specified, default -g to the primary group of the specified user
         (Some(_), None) => {
@@ -222,9 +186,10 @@ pub(crate) fn resolve_path(command: &Path, path: &str) -> Option<PathBuf> {
 mod tests {
     use std::path::PathBuf;
 
-    use crate::common::resolve::CurrentUser;
-
-    use super::{is_valid_executable, resolve_path, resolve_target_user_and_group, NameOrId};
+    use super::{
+        is_valid_executable, resolve_current_user, resolve_path, resolve_target_user_and_group,
+        NameOrId,
+    };
 
     #[test]
     fn test_resolve_path() {
@@ -269,7 +234,7 @@ mod tests {
 
     #[test]
     fn test_resolve_target_user_and_group() {
-        let current_user = CurrentUser::resolve().unwrap();
+        let current_user = resolve_current_user().unwrap();
 
         // fallback to root
         let (user, group) = resolve_target_user_and_group(&None, &None, &current_user).unwrap();
diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs
index 31b70aae..3b10bb27 100644
--- a/src/exec/use_pty/parent.rs
+++ b/src/exec/use_pty/parent.rs
@@ -266,7 +266,7 @@ fn get_pty() -> io::Result<Pty> {
 
     let pty = Pty::open().map_err(|err| {
         dev_error!("cannot allocate pty: {err}");
-        io::Error::new(io::ErrorKind::NotFound, "unable to open pty")
+        err
     })?;
 
     let euid = User::effective_uid();
diff --git a/src/exec/use_pty/pipe.rs b/src/exec/use_pty/pipe/mod.rs
similarity index 69%
rename from src/exec/use_pty/pipe.rs
rename to src/exec/use_pty/pipe/mod.rs
index 5f3447ca..7f02f067 100644
--- a/src/exec/use_pty/pipe.rs
+++ b/src/exec/use_pty/pipe/mod.rs
@@ -1,3 +1,5 @@
+mod ring_buffer;
+
 use std::{
     io::{self, Read, Write},
     marker::PhantomData,
@@ -6,6 +8,8 @@ use std::{
 
 use crate::exec::event::{EventHandle, EventRegistry, PollEvent, Process};
 
+use self::ring_buffer::RingBuffer;
+
 // A pipe able to stream data bidirectionally between two read-write types.
 pub(super) struct Pipe<L, R> {
     left: L,
@@ -100,16 +104,9 @@ impl<L: Read + Write + AsRawFd, R: Read + Write + AsRawFd> Pipe<L, R> {
     }
 }
 
-/// The size of the internal buffer of the pipe.
-const BUFSIZE: usize = 6 * 1024;
-
 /// A buffer that stores the bytes read from `R` before they are written to `W`.
 struct Buffer<R, W> {
-    buffer: [u8; BUFSIZE],
-    /// The start of the busy section of the buffer.
-    start: usize,
-    /// The end of the busy section of the buffer.
-    end: usize,
+    internal: RingBuffer,
     /// The handle for the event of the reader.
     read_handle: EventHandle,
     /// The handle for the event of the writer.
@@ -128,27 +125,13 @@ impl<R: Read, W: Write> Buffer<R, W> {
         write_handle.ignore(registry);
 
         Self {
-            buffer: [0; BUFSIZE],
-            start: 0,
-            end: 0,
+            internal: RingBuffer::new(),
             read_handle,
             write_handle,
             marker: PhantomData,
         }
     }
 
-    /// Return true if the buffer is empty.
-    fn is_empty(&self) -> bool {
-        self.start == self.end
-    }
-
-    /// Return true if the buffer is full.
-    fn is_full(&self) -> bool {
-        // FIXME: This doesn't really mean that the buffer is full but it cannot be used for writes
-        // anyway.
-        self.end == BUFSIZE
-    }
-
     /// Read bytes into the buffer.
     ///
     /// Calling this function will block until `read` is ready to be read.
@@ -157,23 +140,17 @@ impl<R: Read, W: Write> Buffer<R, W> {
         read: &mut R,
         registry: &mut EventRegistry<T>,
     ) -> io::Result<()> {
-        // Don't read if the buffer is full.
-        if self.is_full() {
+        // If the buffer is full, there is nothing to be read.
+        if self.internal.is_full() {
             self.read_handle.ignore(registry);
             return Ok(());
         }
 
-        // This is the remaining free section that follows the busy section of the buffer.
-        let buffer = &mut self.buffer[self.end..];
+        // Read bytes and insert them into the buffer.
+        let inserted_len = self.internal.insert(read)?;
 
-        // Read `len` bytes from `read` into the buffer.
-        let len = read.read(buffer)?;
-
-        // Mark the `len` bytes after the busy section as busy too.
-        self.end += len;
-
-        // If we read something, the buffer is not empty anymore and we can resume writing.
-        if len > 0 {
+        // If we inserted something, the buffer is not empty anymore and we can resume writing.
+        if inserted_len > 0 {
             self.write_handle.resume(registry);
         }
 
@@ -188,29 +165,17 @@ impl<R: Read, W: Write> Buffer<R, W> {
         write: &mut W,
         registry: &mut EventRegistry<T>,
     ) -> io::Result<()> {
-        // Don't write if the buffer is empty.
-        if self.is_empty() {
+        // If the buffer is empty, there is nothing to be written.
+        if self.internal.is_empty() {
             self.write_handle.ignore(registry);
             return Ok(());
         }
 
-        // This is the busy section of the buffer.
-        let buffer = &self.buffer[self.start..self.end];
+        // Remove bytes from the buffer and write them.
+        let removed_len = self.internal.remove(write)?;
 
-        // Write the first `len` bytes of the busy section to `write`.
-        let len = write.write(buffer)?;
-
-        if len == buffer.len() {
-            // If we were able to write all the busy section, we can mark the whole buffer as free.
-            self.start = 0;
-            self.end = 0;
-        } else {
-            // Otherwise we just free the first `len` bytes of the busy section.
-            self.start += len;
-        }
-
-        // If we wrote something, the buffer is not full anymore and we can resume reading.
-        if len > 0 {
+        // If we removed something, the buffer is not full anymore and we can resume reading.
+        if removed_len > 0 {
             self.read_handle.resume(registry);
         }
 
@@ -219,15 +184,8 @@ impl<R: Read, W: Write> Buffer<R, W> {
 
     /// Flush this buffer, ensuring that all the contents of its internal buffer are written.
     fn flush(&mut self, write: &mut W) -> io::Result<()> {
-        // This is the busy section of the buffer.
-        let buffer = &self.buffer[self.start..self.end];
-
-        // Write the complete busy section to `write`.
-        write.write_all(buffer)?;
-
-        // If we were able to write all the busy section, we can mark the whole buffer as free.
-        self.start = 0;
-        self.end = 0;
+        // Remove bytes from the buffer and write them.
+        self.internal.remove(write)?;
 
         write.flush()
     }
diff --git a/src/exec/use_pty/pipe/ring_buffer.rs b/src/exec/use_pty/pipe/ring_buffer.rs
new file mode 100644
index 00000000..f87555ec
--- /dev/null
+++ b/src/exec/use_pty/pipe/ring_buffer.rs
@@ -0,0 +1,288 @@
+use std::io::{self, IoSlice, IoSliceMut, Read, Write};
+
+pub(super) struct RingBuffer {
+    storage: Box<[u8; Self::LEN]>,
+    // The start index of the non-empty section of the buffer.
+    start: usize,
+    // The length of the non-empty section of the buffer.
+    len: usize,
+}
+
+impl RingBuffer {
+    /// The size of the internal storage of the ring buffer.
+    const LEN: usize = 8 * 1024;
+
+    /// Create a new, empty buffer.
+    pub(super) fn new() -> Self {
+        Self {
+            storage: Box::new([0; Self::LEN]),
+            start: 0,
+            len: 0,
+        }
+    }
+
+    pub(super) fn is_full(&self) -> bool {
+        self.len == self.storage.len()
+    }
+
+    pub(super) fn insert<R: Read>(&mut self, read: &mut R) -> io::Result<usize> {
+        let inserted_len = if self.is_empty() {
+            // Case 1.1. The buffer is empty, meaning that there are two unfilled slices in
+            // `storage`:`start..` and `..start`.
+            let (second_slice, first_slice) = self.storage.split_at_mut(self.start);
+            read.read_vectored(&mut [first_slice, second_slice].map(IoSliceMut::new))?
+        } else {
+            let &mut Self { start, len, .. } = self;
+            let end = start + len;
+            if end >= self.storage.len() {
+                // Case 1.2. The buffer is not empty and the filled section wraps around `storage`.
+                // Meaning that there is only one unfilled slice in `storage`: `end..start`.
+                let end = end % self.storage.len();
+                read.read(&mut self.storage[end..start])?
+            } else {
+                // Case 1.3. The buffer is not empty and the filled section is a contiguous slice
+                // of `storage`. Meaning that there are two unfilled slices in `storage`: `..start`
+                // and `end..`.
+                let (mid, first_slice) = self.storage.split_at_mut(end);
+                let second_slice = &mut mid[..start];
+                read.read_vectored(&mut [first_slice, second_slice].map(IoSliceMut::new))?
+            }
+        };
+
+        self.len += inserted_len;
+
+        debug_assert!(self.start < Self::LEN);
+        debug_assert!(self.len <= Self::LEN);
+
+        Ok(inserted_len)
+    }
+
+    pub(super) fn is_empty(&self) -> bool {
+        self.len == 0
+    }
+
+    pub(super) fn remove<W: Write>(&mut self, write: &mut W) -> io::Result<usize> {
+        let removed_len = if self.is_full() {
+            // Case 2.1. The buffer is full, meaning that there are two filled slices in `storage`:
+            // `start..` and `..start`.
+            let (second_slice, first_slice) = self.storage.split_at(self.start);
+            write.write_vectored(&[first_slice, second_slice].map(IoSlice::new))?
+        } else {
+            let end = self.start + self.len;
+            if end >= self.storage.len() {
+                // Case 2.2. The buffer is not full and the filled section wraps around `storage`.
+                // Meaning that there are two non-empty slices in `storage`: `start..` and `..end`.
+                let end = end % self.storage.len();
+                let first_slice = &self.storage[self.start..];
+                let second_slice = &self.storage[..end];
+                write.write_vectored(&[first_slice, second_slice].map(IoSlice::new))?
+            } else {
+                // Case 2.3. The buffer is not full and the filled section is a contiguous slice
+                // of `storage.` Meaning that there is only one filled slice in `storage`:
+                // `start..end`.
+                write.write(&self.storage[self.start..end])?
+            }
+        };
+
+        self.start += removed_len;
+        self.start %= Self::LEN;
+
+        self.len -= removed_len;
+
+        debug_assert!(self.start < Self::LEN);
+        debug_assert!(self.len <= Self::LEN);
+
+        Ok(removed_len)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::RingBuffer;
+
+    #[test]
+    fn empty_buffer_is_empty() {
+        let buf = RingBuffer::new();
+
+        assert!(buf.is_empty());
+    }
+
+    #[test]
+    fn full_buffer_is_full() {
+        let mut buf = RingBuffer::new();
+
+        let inserted_len = buf.insert(&mut [0x45; RingBuffer::LEN].as_slice()).unwrap();
+        assert_eq!(inserted_len, RingBuffer::LEN);
+
+        assert!(buf.is_full());
+    }
+
+    #[test]
+    fn buffer_is_fifo() {
+        let mut buf = RingBuffer::new();
+
+        let expected = (0..=u8::MAX).collect::<Vec<u8>>();
+        let inserted_len = buf.insert(&mut expected.as_slice()).unwrap();
+        assert_eq!(inserted_len, expected.len());
+
+        let mut found = vec![];
+        let removed_len = buf.remove(&mut found).unwrap();
+        assert_eq!(removed_len, expected.len());
+
+        assert_eq!(expected, found);
+    }
+
+    #[test]
+    fn insert_into_empty_buffer_with_offset() {
+        const HALF_LEN: usize = RingBuffer::LEN / 2;
+        let mut buf = RingBuffer::new();
+
+        // This should leave the buffer empty but with the start field pointing to the middle of
+        // the buffer.
+        // ┌───────────────────┐
+        // │                   │
+        // └───────────────────┘
+        //           ▲
+        //           │
+        //         start
+        buf.insert(&mut [0u8; HALF_LEN].as_slice()).unwrap();
+        buf.remove(&mut vec![]).unwrap();
+
+        assert_eq!(buf.start, HALF_LEN);
+        assert_eq!(buf.len, 0);
+
+        // Then we fill the first half of the buffer with ones and the second one with twos in a
+        // single insertion. This tests case 1.1.
+        // ┌─────────┬─────────┐
+        // │    2    │    1    │
+        // └─────────┴─────────┘
+        //           ▲
+        //           │
+        //         start
+        let mut expected = vec![1; HALF_LEN];
+        expected.extend_from_slice(&[2; HALF_LEN]);
+        buf.insert(&mut expected.as_slice()).unwrap();
+
+        assert_eq!(buf.start, HALF_LEN);
+        assert_eq!(buf.len, RingBuffer::LEN);
+
+        // When we remove all the elements of the buffer we should find them in the same order we
+        // inserted them. This tests case 2.1.
+        let mut found = vec![];
+        let removed_len = buf.remove(&mut found).unwrap();
+        assert_eq!(removed_len, expected.len());
+
+        assert_eq!(expected, found);
+
+        assert_eq!(buf.start, HALF_LEN);
+        assert_eq!(buf.len, 0);
+    }
+
+    #[test]
+    fn insert_into_non_empty_wrapping_buffer() {
+        const QUARTER_LEN: usize = RingBuffer::LEN / 4;
+        let mut buf = RingBuffer::new();
+
+        // This should leave the buffer empty but with the start field pointing to the middle of
+        // the buffer.
+        // ┌───────────────────────┐
+        // │                       │
+        // └───────────────────────┘
+        //             ▲
+        //             │
+        //           start
+        buf.insert(&mut [0; 2 * QUARTER_LEN].as_slice()).unwrap();
+        buf.remove(&mut vec![]).unwrap();
+
+        assert_eq!(buf.start, 2 * QUARTER_LEN);
+        assert_eq!(buf.len, 0);
+
+        // Then we fill one quarter of the buffer with ones. This gives us a non-empty buffer whose
+        // empty section is not contiguous.
+        // ┌───────────┬─────┬─────┐
+        // │           │  1  │     │
+        // └───────────┴─────┴─────┘
+        //             ▲
+        //             │
+        //           start
+        let mut expected = vec![1; QUARTER_LEN];
+        buf.insert(&mut expected.as_slice()).unwrap();
+
+        assert_eq!(buf.start, 2 * QUARTER_LEN);
+        assert_eq!(buf.len, QUARTER_LEN);
+
+        // Then we fill one quarter of the buffer with twos and another quarter of the buffer with
+        // threes in a single insertion. This tests case 1.2.
+        // ┌─────┬─────┬─────┬─────┐
+        // │  3  │     │  1  │  2  │
+        // └─────┴─────┴─────┴─────┘
+        //             ▲
+        //             │
+        //           start
+        let mut second_half = vec![2; QUARTER_LEN];
+        second_half.extend_from_slice(&[3; QUARTER_LEN]);
+        buf.insert(&mut second_half.as_slice()).unwrap();
+
+        expected.extend(second_half);
+
+        assert_eq!(buf.start, 2 * QUARTER_LEN);
+        assert_eq!(buf.len, 3 * QUARTER_LEN);
+
+        // When we remove all the elements of the buffer we should find them in the same order we
+        // inserted them. This tests case 2.2.
+        let mut found = vec![];
+        let removed_len = buf.remove(&mut found).unwrap();
+        assert_eq!(removed_len, expected.len());
+
+        assert_eq!(expected, found);
+
+        assert_eq!(buf.start, QUARTER_LEN);
+        assert_eq!(buf.len, 0);
+    }
+
+    #[test]
+    fn insert_into_non_empty_non_wrapping_buffer() {
+        const QUARTER_LEN: usize = RingBuffer::LEN / 4;
+        let mut buf = RingBuffer::new();
+
+        // We fill one quarter of the buffer with ones. This gives us a non-empty buffer whose
+        // empty section is contiguous.
+        // ┌─────┬────────────────┐
+        // │  1  │                │
+        // └─────┴────────────────┘
+        // ▲
+        // │
+        // └ start
+        let mut expected = vec![1; QUARTER_LEN];
+        buf.insert(&mut expected.as_slice()).unwrap();
+
+        assert_eq!(buf.start, 0);
+        assert_eq!(buf.len, QUARTER_LEN);
+
+        // Then we fill one quarter of the buffer with twos. This tests case 1.3.
+        // ┌─────┬─────┬──────────┐
+        // │  1  │  2  │          │
+        // └─────┴─────┴──────────┘
+        // ▲
+        // │
+        // └ start
+        let second_half = vec![2; QUARTER_LEN];
+        buf.insert(&mut second_half.as_slice()).unwrap();
+
+        expected.extend(second_half);
+
+        assert_eq!(buf.start, 0);
+        assert_eq!(buf.len, 2 * QUARTER_LEN);
+
+        // When we remove all the elements of the buffer we should find them in the same order we
+        // inserted them. This tests case 2.3.
+        let mut found = vec![];
+        let removed_len = buf.remove(&mut found).unwrap();
+        assert_eq!(removed_len, expected.len());
+
+        assert_eq!(expected, found);
+
+        assert_eq!(buf.start, 2 * QUARTER_LEN);
+        assert_eq!(buf.len, 0);
+    }
+}
diff --git a/src/su/context.rs b/src/su/context.rs
index f2083fc6..5ee00140 100644
--- a/src/su/context.rs
+++ b/src/su/context.rs
@@ -5,8 +5,11 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use crate::common::{error::Error, resolve::CurrentUser, Environment};
-use crate::common::{resolve::is_valid_executable, SudoPath};
+use crate::common::{error::Error, Environment};
+use crate::common::{
+    resolve::{is_valid_executable, resolve_current_user},
+    SudoPath,
+};
 use crate::exec::RunOptions;
 use crate::log::user_warn;
 use crate::system::{Group, Process, User};
@@ -27,7 +30,7 @@ pub(crate) struct SuContext {
     options: SuRunOptions,
     pub(crate) environment: Environment,
     user: User,
-    requesting_user: CurrentUser,
+    requesting_user: User,
     group: Group,
     pub(crate) process: Process,
 }
@@ -72,7 +75,7 @@ impl SuContext {
             }
         }
 
-        let requesting_user = CurrentUser::resolve()?;
+        let requesting_user = resolve_current_user()?;
 
         // resolve target user
         let mut user = User::from_name(options.user.as_cstr())?
diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs
index 61888734..e1e686ae 100644
--- a/src/sudo/mod.rs
+++ b/src/sudo/mod.rs
@@ -1,7 +1,6 @@
 #![forbid(unsafe_code)]
 
-use crate::common::resolve::CurrentUser;
-use crate::common::{Context, Error};
+use crate::common::{resolve::resolve_current_user, Context, Error};
 use crate::log::dev_info;
 use crate::system::timestamp::RecordScope;
 use crate::system::User;
@@ -60,7 +59,7 @@ impl PolicyPlugin for SudoersPolicy {
         context: &Context,
     ) -> Result<Self::Policy, Error> {
         Ok(pre.check(
-            &*context.current_user,
+            &context.current_user,
             &context.hostname,
             crate::sudoers::Request {
                 user: &context.target_user,
@@ -96,17 +95,17 @@ fn sudo_process() -> Result<(), Error> {
                 std::process::exit(0);
             }
             SudoAction::RemoveTimestamp(_) => {
-                let user = CurrentUser::resolve()?;
+                let user = resolve_current_user()?;
                 let mut record_file =
-                    SessionRecordFile::open_for_user(&user, Duration::seconds(0))?;
+                    SessionRecordFile::open_for_user(user.uid, Duration::seconds(0))?;
                 record_file.reset()?;
                 Ok(())
             }
             SudoAction::ResetTimestamp(_) => {
                 if let Some(scope) = RecordScope::for_process(&Process::new()) {
-                    let user = CurrentUser::resolve()?;
+                    let user = resolve_current_user()?;
                     let mut record_file =
-                        SessionRecordFile::open_for_user(&user, Duration::seconds(0))?;
+                        SessionRecordFile::open_for_user(user.uid, Duration::seconds(0))?;
                     record_file.disable(scope, None)?;
                 }
                 Ok(())
diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs
index 949970f4..0fddd2d5 100644
--- a/src/sudo/pipeline.rs
+++ b/src/sudo/pipeline.rs
@@ -3,7 +3,6 @@ use std::process::exit;
 
 use super::cli::{SudoRunOptions, SudoValidateOptions};
 use crate::common::context::OptionsForContext;
-use crate::common::resolve::CurrentUser;
 use crate::common::{Context, Environment, Error};
 use crate::exec::{ExecOutput, ExitReason};
 use crate::log::{auth_info, auth_warn};
@@ -146,7 +145,7 @@ impl<Policy: PolicyPlugin, Auth: AuthPlugin> Pipeline<Policy, Auth> {
             context.use_session_records,
             scope,
             context.current_user.uid,
-            &context.current_user,
+            context.current_user.uid,
             prior_validity,
         );
         self.authenticator.init(context)?;
@@ -217,7 +216,7 @@ fn determine_auth_status(
     use_session_records: bool,
     record_for: Option<RecordScope>,
     auth_uid: UserId,
-    current_user: &CurrentUser,
+    current_user: UserId,
     prior_validity: Duration,
 ) -> AuthStatus {
     if !must_policy_authenticate {
diff --git a/src/sudo/pipeline/list.rs b/src/sudo/pipeline/list.rs
index da1f1d9a..f1216d85 100644
--- a/src/sudo/pipeline/list.rs
+++ b/src/sudo/pipeline/list.rs
@@ -79,7 +79,7 @@ impl Pipeline<SudoersPolicy, PamAuthenticator<CLIConverser>> {
             target_group: &context.target_group,
         };
         let judgement =
-            sudoers.check_list_permission(&*context.current_user, &context.hostname, list_request);
+            sudoers.check_list_permission(&context.current_user, &context.hostname, list_request);
         match judgement.authorization() {
             Authorization::Allowed(auth) => {
                 self.auth_and_update_record_file(context, auth)?;
diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs
index cb57509a..45ff5243 100644
--- a/src/sudoers/mod.rs
+++ b/src/sudoers/mod.rs
@@ -15,8 +15,8 @@ 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::{self, can_execute};
 use ast::*;
 use tokens::*;
 
@@ -77,7 +77,7 @@ impl Sudoers {
     pub fn check<User: UnixUser + PartialEq<User>, Group: UnixGroup>(
         &self,
         am_user: &User,
-        on_host: &system::Hostname,
+        on_host: &str,
         request: Request<User, Group>,
     ) -> Judgement {
         // exception: if user is root or does not switch users, NOPASSWD is implied
@@ -100,7 +100,7 @@ impl Sudoers {
     pub fn check_list_permission<User: UnixUser + PartialEq<User>, Group: UnixGroup>(
         &self,
         invoking_user: &User,
-        hostname: &system::Hostname,
+        hostname: &str,
         request: ListRequest<User, Group>,
     ) -> Judgement {
         // exception: if user is root or does not switch users, NOPASSWD is implied
@@ -142,7 +142,7 @@ impl Sudoers {
     fn matching_user_specs<'a: 'b + 'c, 'b: 'c, 'c, User: UnixUser + PartialEq<User>>(
         &'a self,
         invoking_user: &'b User,
-        hostname: &'c system::Hostname,
+        hostname: &'c str,
     ) -> impl Iterator<Item = impl Iterator<Item = (Option<&'a RunAs>, (Tag, &'a Spec<Command>))> + 'b>
            + 'c {
         let Self { rules, aliases, .. } = self;
@@ -166,7 +166,7 @@ impl Sudoers {
     pub fn matching_entries<'a, User: UnixUser + PartialEq<User>>(
         &'a self,
         invoking_user: &User,
-        hostname: &system::Hostname,
+        hostname: &str,
     ) -> Vec<Entry<'a>> {
         // NOTE this method MUST NOT perform any filtering that `Self::check` does not do to
         // ensure `sudo $command` and `sudo --list` use the same permission checking logic
@@ -276,7 +276,7 @@ fn open_subsudoers(path: &Path) -> io::Result<Vec<basic_parser::Parsed<Sudo>>> {
 #[derive(Default)]
 pub(super) struct AliasTable {
     user: VecOrd<Def<UserSpecifier>>,
-    host: VecOrd<Def<tokens::Hostname>>,
+    host: VecOrd<Def<Hostname>>,
     cmnd: VecOrd<Def<Command>>,
     runas: VecOrd<Def<UserSpecifier>>,
 }
@@ -299,7 +299,7 @@ fn elems<T>(vec: &VecOrd<T>) -> impl Iterator<Item = &T> {
 fn check_permission<User: UnixUser + PartialEq<User>, Group: UnixGroup>(
     sudoers: &Sudoers,
     am_user: &User,
-    on_host: &system::Hostname,
+    on_host: &str,
     request: Request<User, Group>,
 ) -> Option<Tag> {
     let cmdline = (request.command, request.arguments);
diff --git a/src/system/mod.rs b/src/system/mod.rs
index a929bfed..07cc0190 100644
--- a/src/system/mod.rs
+++ b/src/system/mod.rs
@@ -1,11 +1,9 @@
-use core::fmt;
 // TODO: remove unused attribute when system is cleaned up
 use std::{
     collections::BTreeSet,
     ffi::{c_uint, CStr, CString},
     io,
     mem::MaybeUninit,
-    ops,
     os::{
         fd::AsRawFd,
         unix::{self, prelude::OsStrExt},
@@ -149,63 +147,25 @@ pub fn setsid() -> io::Result<ProcessId> {
     cerr(unsafe { libc::setsid() })
 }
 
-#[derive(Clone)]
-#[cfg_attr(test, derive(PartialEq))]
-pub struct Hostname {
-    inner: String,
-}
-
-impl fmt::Debug for Hostname {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.debug_tuple("Hostname").field(&self.inner).finish()
-    }
-}
-
-impl fmt::Display for Hostname {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        f.write_str(&self.inner)
-    }
-}
-
-impl ops::Deref for Hostname {
-    type Target = str;
+pub fn hostname() -> String {
+    // see `man 2 gethostname`
+    const MAX_HOST_NAME_SIZE_ACCORDING_TO_SUSV2: libc::c_long = 255;
 
-    fn deref(&self) -> &str {
-        &self.inner
-    }
-}
+    // POSIX.1 systems limit hostnames to `HOST_NAME_MAX` bytes
+    // not including null-byte in the count
+    let max_hostname_size =
+        sysconf(libc::_SC_HOST_NAME_MAX).unwrap_or(MAX_HOST_NAME_SIZE_ACCORDING_TO_SUSV2) as usize;
 
-impl Hostname {
-    #[cfg(test)]
-    pub fn fake(hostname: &str) -> Self {
-        Self {
-            inner: hostname.to_string(),
-        }
-    }
+    let buffer_size = max_hostname_size + 1 /* null byte delimiter */ ;
+    let mut buf = vec![0; buffer_size];
 
-    pub fn resolve() -> Self {
-        // see `man 2 gethostname`
-        const MAX_HOST_NAME_SIZE_ACCORDING_TO_SUSV2: libc::c_long = 255;
+    match cerr(unsafe { libc::gethostname(buf.as_mut_ptr(), buffer_size) }) {
+        Ok(_) => unsafe { string_from_ptr(buf.as_ptr()) },
 
-        // POSIX.1 systems limit hostnames to `HOST_NAME_MAX` bytes
-        // not including null-byte in the count
-        let max_hostname_size = sysconf(libc::_SC_HOST_NAME_MAX)
-            .unwrap_or(MAX_HOST_NAME_SIZE_ACCORDING_TO_SUSV2)
-            as usize;
-
-        let buffer_size = max_hostname_size + 1 /* null byte delimiter */ ;
-        let mut buf = vec![0; buffer_size];
-
-        match cerr(unsafe { libc::gethostname(buf.as_mut_ptr(), buffer_size) }) {
-            Ok(_) => Self {
-                inner: unsafe { string_from_ptr(buf.as_ptr()) },
-            },
-
-            // ENAMETOOLONG is returned when hostname is greater than `buffer_size`
-            Err(_) => {
-                // but we have chosen a `buffer_size` larger than `max_hostname_size` so no truncation error is possible
-                panic!("Unexpected error while retrieving hostname, this should not happen");
-            }
+        // ENAMETOOLONG is returned when hostname is greater than `buffer_size`
+        Err(_) => {
+            // but we have chosen a `buffer_size` larger than `max_hostname_size` so no truncation error is possible
+            panic!("Unexpected error while retrieving hostname, this should not happen");
         }
     }
 }
diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs
index c5d5542d..c8b9c426 100644
--- a/src/system/timestamp.rs
+++ b/src/system/timestamp.rs
@@ -4,10 +4,7 @@ use std::{
     path::PathBuf,
 };
 
-use crate::{
-    common::resolve::CurrentUser,
-    log::{auth_info, auth_warn},
-};
+use crate::log::{auth_info, auth_warn};
 
 use super::{
     audit::secure_open_cookie_file,
@@ -47,11 +44,10 @@ pub struct SessionRecordFile {
 impl SessionRecordFile {
     const BASE_PATH: &'static str = "/var/run/sudo-rs/ts";
 
-    pub fn open_for_user(user: &CurrentUser, timeout: Duration) -> io::Result<Self> {
-        let uid = user.uid;
+    pub fn open_for_user(user: UserId, timeout: Duration) -> io::Result<Self> {
         let mut path = PathBuf::from(Self::BASE_PATH);
-        path.push(uid.to_string());
-        SessionRecordFile::new(uid, secure_open_cookie_file(&path)?, timeout)
+        path.push(user.to_string());
+        SessionRecordFile::new(user, secure_open_cookie_file(&path)?, timeout)
     }
 
     const FILE_VERSION: u16 = 1;


Test patch:
diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs
index a0028be0..4824fe5e 100644
--- a/src/sudo/env/tests.rs
+++ b/src/sudo/env/tests.rs
@@ -1,10 +1,9 @@
-use crate::common::resolve::CurrentUser;
 use crate::common::{CommandAndArguments, Context, Environment};
 use crate::sudo::{
     cli::{SudoAction, SudoRunOptions},
     env::environment::get_target_environment,
 };
-use crate::system::{Group, Hostname, Process, User};
+use crate::system::{Group, Process, User};
 use std::collections::{HashMap, HashSet};
 
 const TESTS: &str = "
@@ -81,7 +80,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
     let command =
         CommandAndArguments::build_from_args(None, sudo_options.positional_args.clone(), &path);
 
-    let current_user = CurrentUser::fake(User {
+    let current_user = User {
         uid: 1000,
         gid: 1000,
 
@@ -91,7 +90,7 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
         shell: "/bin/sh".into(),
         passwd: String::new(),
         groups: vec![],
-    });
+    };
 
     let current_group = Group {
         gid: 1000,
@@ -119,11 +118,11 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context {
     };
 
     Context {
-        hostname: Hostname::fake("test-ubuntu"),
+        hostname: "test-ubuntu".to_string(),
         command,
         current_user: current_user.clone(),
         target_user: if sudo_options.user.as_deref() == Some("test") {
-            current_user.into()
+            current_user
         } else {
             root_user
         },
diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs
index 79606007..21482cf8 100644
--- a/src/sudoers/test/mod.rs
+++ b/src/sudoers/test/mod.rs
@@ -94,7 +94,7 @@ fn permission_test() {
             let (Sudoers { rules,aliases,settings }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]);
             let cmdvec = $command.split_whitespace().map(String::from).collect::<Vec<_>>();
             let req = Request { user: $req.0, group: $req.1, command: &realpath(cmdvec[0].as_ref()), arguments: &cmdvec[1..].to_vec() };
-            assert_eq!(Sudoers { rules, aliases, settings }.check(&Named($user), &system::Hostname::fake($server), req).flags, None);
+            assert_eq!(Sudoers { rules, aliases, settings }.check(&Named($user), $server, req).flags, None);
         }
     }
 
@@ -103,7 +103,7 @@ fn permission_test() {
             let (Sudoers { rules,aliases,settings }, _) = analyze(Path::new("/etc/fakesudoers"), sudoer![$($sudo),*]);
             let cmdvec = $command.split_whitespace().map(String::from).collect::<Vec<_>>();
             let req = Request { user: $req.0, group: $req.1, command: &realpath(cmdvec[0].as_ref()), arguments: &cmdvec[1..].to_vec() };
-            let result = Sudoers { rules, aliases, settings }.check(&Named($user), &system::Hostname::fake($server), req).flags;
+            let result = Sudoers { rules, aliases, settings }.check(&Named($user), $server, req).flags;
             assert!(!result.is_none());
             $(
                 let result = result.unwrap();