close_range improvements (#1114)

Author: squell

README.md

@@ -13,8 +13,7 @@ Suspected vulnerabilities can be reported on our [security page](https://github.
 An [audit of sudo-rs version 0.2.0](docs/audit/audit-report-sudo-rs.pdf) has been performed in August 2023.
 The findings from that audit are addressed in the current version.
 
-Sudo-rs currently is targeted for Linux-based operating systems only; Linux kernel 5.9
-or newer is necessary to run sudo-rs.
+Sudo-rs currently is targeted for FreeBSD and Linux-based operating systems only.
 
 ## Installing sudo-rs
 

src/common/error.rs

@@ -13,7 +13,6 @@ pub enum Error {
         other_user: Option<SudoString>,
     },
     SelfCheck,
-    KernelCheck,
     CommandNotFound(PathBuf),
     InvalidCommand(PathBuf),
     ChDirNotAllowed {
@@ -61,7 +60,6 @@ impl fmt::Display for Error {
             Error::SelfCheck => {
                 f.write_str("sudo must be owned by uid 0 and have the setuid bit set")
             }
-            Error::KernelCheck => f.write_str("sudo-rs needs a Linux kernel newer than v5.9"),
             Error::CommandNotFound(p) => write!(f, "'{}': command not found", p.display()),
             Error::InvalidCommand(p) => write!(f, "'{}': invalid command", p.display()),
             Error::UserNotFound(u) => write!(f, "user '{u}' not found"),

src/exec/no_pty.rs

@@ -19,10 +19,10 @@ use crate::{
     system::{
         _exit, fork, getpgid, getpgrp,
         interface::ProcessId,
-        kill, killpg,
+        kill, killpg, mark_fds_as_cloexec,
         term::{Terminal, UserTerm},
         wait::WaitOptions,
-        FileCloser, ForkResult,
+        ForkResult,
     },
 };
 
@@ -39,17 +39,11 @@ pub(super) fn exec_no_pty(sudo_pid: ProcessId, mut command: Command) -> io::Resu
         }
     };
 
-    let mut file_closer = FileCloser::new();
-
     // FIXME (ogsudo): Some extra config happens here if selinux is available.
 
     // Use a pipe to get the IO error if `exec` fails.
     let (mut errpipe_tx, errpipe_rx) = BinPipe::pair()?;
 
-    // Don't close the error pipe as we need it to retrieve the error code if the command execution
-    // fails.
-    file_closer.except(&errpipe_tx);
-
     // SAFETY: There should be no other threads at this point.
     let ForkResult::Parent(command_pid) = unsafe { fork() }.map_err(|err| {
         dev_warn!("unable to fork command process: {err}");
@@ -63,9 +57,7 @@ pub(super) fn exec_no_pty(sudo_pid: ProcessId, mut command: Command) -> io::Resu
             }
         }
 
-        // SAFETY: We immediately exec after this call and if the exec fails we only access stderr
-        // and errpipe before exiting without running atexit handlers using _exit
-        if let Err(err) = unsafe { file_closer.close_the_universe() } {
+        if let Err(err) = mark_fds_as_cloexec() {
             dev_warn!("failed to close the universe: {err}");
             // Send the error to the monitor using the pipe.
             if let Some(error_code) = err.raw_os_error() {

src/exec/use_pty/monitor.rs

@@ -19,7 +19,7 @@ use crate::{
         use_pty::{SIGCONT_BG, SIGCONT_FG},
     },
     log::{dev_error, dev_info, dev_warn},
-    system::FileCloser,
+    system::mark_fds_as_cloexec,
 };
 use crate::{
     exec::{handle_sigchld, terminate_process, HandleSigchld},
@@ -40,7 +40,6 @@ pub(super) fn exec_monitor(
     command: Command,
     foreground: bool,
     backchannel: &mut MonitorBackchannel,
-    mut file_closer: FileCloser,
     original_set: Option<SignalSet>,
 ) -> io::Result<Infallible> {
     // SIGTTIN and SIGTTOU are ignored here but the docs state that it shouldn't
@@ -69,10 +68,6 @@ pub(super) fn exec_monitor(
     // Use a pipe to get the IO error if `exec_command` fails.
     let (errpipe_tx, errpipe_rx) = BinPipe::pair()?;
 
-    // Don't close the error pipe as we need it to retrieve the error code if the command execution
-    // fails.
-    file_closer.except(&errpipe_tx);
-
     // Wait for the parent to give us green light before spawning the command. This avoids race
     // conditions when the command exits quickly.
     let event = retry_while_interrupted(|| backchannel.recv()).map_err(|err| {
@@ -93,14 +88,7 @@ pub(super) fn exec_monitor(
     else {
         drop(errpipe_rx);
 
-        match exec_command(
-            command,
-            foreground,
-            pty_follower,
-            file_closer,
-            errpipe_tx,
-            original_set,
-        ) {}
+        match exec_command(command, foreground, pty_follower, errpipe_tx, original_set) {}
     };
 
     // Send the command's PID to the parent.
@@ -192,7 +180,6 @@ fn exec_command(
     mut command: Command,
     foreground: bool,
     pty_follower: PtyFollower,
-    file_closer: FileCloser,
     mut errpipe_tx: BinPipe<i32, i32>,
     original_set: Option<SignalSet>,
 ) -> ! {
@@ -219,9 +206,7 @@ fn exec_command(
         }
     }
 
-    // SAFETY: We immediately exec after this call and if the exec fails we only access stderr
-    // and errpipe before exiting without running atexit handlers using _exit
-    if let Err(err) = unsafe { file_closer.close_the_universe() } {
+    if let Err(err) = mark_fds_as_cloexec() {
         dev_warn!("failed to close the universe: {err}");
         // Send the error to the monitor using the pipe.
         if let Some(error_code) = err.raw_os_error() {

src/exec/use_pty/parent.rs

@@ -19,9 +19,7 @@ use crate::system::signal::{
 };
 use crate::system::term::{Pty, PtyFollower, PtyLeader, TermSize, Terminal, UserTerm};
 use crate::system::wait::WaitOptions;
-use crate::system::{
-    chown, fork, getpgrp, kill, killpg, FileCloser, ForkResult, Group, User, _exit,
-};
+use crate::system::{chown, fork, getpgrp, kill, killpg, ForkResult, Group, User, _exit};
 use crate::system::{getpgid, interface::ProcessId};
 
 use super::pipe::Pipe;
@@ -58,18 +56,12 @@ pub(in crate::exec) fn exec_pty(
     // Fetch the parent process group so we can signals to it.
     let parent_pgrp = getpgrp();
 
-    let mut file_closer = FileCloser::new();
-
     // Set all the IO streams for the command to the follower side of the pty.
-    let mut clone_follower = || -> io::Result<PtyFollower> {
-        let follower = pty.follower.try_clone().map_err(|err| {
+    let clone_follower = || -> io::Result<PtyFollower> {
+        pty.follower.try_clone().map_err(|err| {
             dev_error!("cannot clone pty follower: {err}");
             err
-        })?;
-        // Don't close these as we will need them so they are dupped inside `Command::exec`.
-        file_closer.except(&follower);
-
-        Ok(follower)
+        })
     };
 
     command.stdin(clone_follower()?);
@@ -181,7 +173,6 @@ pub(in crate::exec) fn exec_pty(
             command,
             foreground && !pipeline && !exec_bg,
             &mut backchannels.monitor,
-            file_closer,
             original_set,
         ) {
             Ok(exec_output) => match exec_output {},

src/sudo/mod.rs

@@ -4,7 +4,6 @@ use crate::common::resolve::CurrentUser;
 use crate::common::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;
 use crate::system::{time::Duration, timestamp::SessionRecordFile, Process};
@@ -67,7 +66,6 @@ fn sudo_process() -> Result<(), Error> {
     dev_info!("development logs are enabled");
 
     self_check()?;
-    kernel_check()?;
 
     // parse cli options
     match SudoAction::from_env() {