Implement support for NOEXEC (take 2)

Cargo.toml

@@ -29,7 +29,7 @@ name = "visudo"
 path = "bin/visudo.rs"
 
 [dependencies]
-libc = "0.2.149"
+libc = "0.2.152"
 glob = "0.3.0"
 log = { version = "0.4.11", features = ["std"] }
 

src/common/context.rs

@@ -31,6 +31,7 @@ pub struct Context {
     pub process: Process,
     // policy
     pub use_pty: bool,
+    pub noexec: bool,
 }
 
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
@@ -93,6 +94,7 @@ impl Context {
             non_interactive: sudo_options.non_interactive,
             process: Process::new(),
             use_pty: true,
+            noexec: false,
         })
     }
 
@@ -117,6 +119,7 @@ impl Context {
             non_interactive: sudo_options.non_interactive,
             process: Process::new(),
             use_pty: true,
+            noexec: false,
         })
     }
 
@@ -161,6 +164,7 @@ impl Context {
             non_interactive: sudo_options.non_interactive,
             process: Process::new(),
             use_pty: true,
+            noexec: false,
         })
     }
 
@@ -179,6 +183,7 @@ impl Context {
             group: &self.target_group,
 
             use_pty: self.use_pty,
+            noexec: self.noexec,
         })
     }
 }

src/defaults/mod.rs

@@ -36,6 +36,7 @@ defaults! {
     env_editor                = true
     rootpw                    = false
     targetpw                  = false
+    noexec                    = false
 
     apparmor_profile          = None (!= None)
     passwd_tries              = 3 [0..=1000]

src/exec/mod.rs

@@ -1,6 +1,8 @@
 mod event;
 mod io_util;
 mod no_pty;
+#[cfg(target_os = "linux")]
+mod noexec;
 mod use_pty;
 
 use std::{
@@ -16,15 +18,18 @@ use std::{
 };
 
 use crate::{
+    common::bin_serde::BinPipe,
     exec::no_pty::exec_no_pty,
     log::{dev_info, dev_warn, user_error},
     system::{
+        _exit,
         interface::ProcessId,
-        killpg,
-        signal::{consts::*, signal_name},
+        kill, killpg, set_target_user,
+        signal::{consts::*, signal_name, SignalNumber, SignalSet},
+        term::UserTerm,
         wait::{Wait, WaitError, WaitOptions},
+        FileCloser, Group, User,
     },
-    system::{kill, set_target_user, signal::SignalNumber, term::UserTerm, Group, User},
 };
 
 use self::{
@@ -43,6 +48,7 @@ pub struct RunOptions<'a> {
     pub group: &'a Group,
 
     pub use_pty: bool,
+    pub noexec: bool,
 }
 
 /// Based on `ogsudo`s `exec_pty` function.
@@ -53,6 +59,8 @@ pub fn run_command(
     options: RunOptions<'_>,
     env: impl IntoIterator<Item = (impl AsRef<OsStr>, impl AsRef<OsStr>)>,
 ) -> io::Result<ExitReason> {
+    let mut file_closer = FileCloser::new();
+
     // FIXME: should we pipe the stdio streams?
     let qualified_path = options.command;
     let mut command = Command::new(qualified_path);
@@ -74,6 +82,18 @@ pub fn run_command(
         command.arg0(OsStr::from_bytes(&process_name));
     }
 
+    let spawn_noexec_handler = if options.noexec {
+        #[cfg(not(target_os = "linux"))]
+        return Err(io::Error::other(
+            "NOEXEC is currently only supported on Linux",
+        ));
+
+        #[cfg(target_os = "linux")]
+        Some(noexec::add_noexec_filter(&mut command, &mut file_closer))
+    } else {
+        None
+    };
+
     // Decide if the pwd should be changed. `--chdir` takes precedence over `-i`.
     let path = options
         .chdir
@@ -108,14 +128,20 @@ pub fn run_command(
 
     if options.use_pty {
         match UserTerm::open() {
-            Ok(user_tty) => exec_pty(sudo_pid, command, user_tty),
+            Ok(user_tty) => exec_pty(
+                sudo_pid,
+                file_closer,
+                spawn_noexec_handler,
+                command,
+                user_tty,
+            ),
             Err(err) => {
                 dev_info!("Could not open user's terminal, not allocating a pty: {err}");
-                exec_no_pty(sudo_pid, command)
+                exec_no_pty(sudo_pid, file_closer, spawn_noexec_handler, command)
             }
         }
     } else {
-        exec_no_pty(sudo_pid, command)
+        exec_no_pty(sudo_pid, file_closer, spawn_noexec_handler, command)
     }
 }
 
@@ -126,6 +152,45 @@ pub enum ExitReason {
     Signal(i32),
 }
 
+fn exec_command(
+    file_closer: FileCloser,
+    mut command: Command,
+    original_set: Option<SignalSet>,
+    mut errpipe_tx: BinPipe<i32>,
+) -> ! {
+    // Restore the signal mask now that the handlers have been setup.
+    if let Some(set) = original_set {
+        if let Err(err) = set.set_mask() {
+            dev_warn!("cannot restore signal mask: {err}");
+        }
+    }
+
+    // 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() } {
+        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() {
+            errpipe_tx.write(&error_code).ok();
+        }
+
+        // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
+        _exit(1);
+    }
+
+    let err = command.exec();
+
+    dev_warn!("failed to execute command: {err}");
+    // If `exec` returns, it means that executing the command failed. Send the error to the
+    // monitor using the pipe.
+    if let Some(error_code) = err.raw_os_error() {
+        errpipe_tx.write(&error_code).ok();
+    }
+
+    // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
+    _exit(1);
+}
+
 // Kill the process with increasing urgency.
 //
 // Based on `terminate_command`.

src/exec/no_pty.rs

@@ -1,4 +1,4 @@
-use std::{ffi::c_int, io, os::unix::process::CommandExt, process::Command};
+use std::{ffi::c_int, io, process::Command};
 
 use super::{
     event::PollEvent,
@@ -14,10 +14,10 @@ use crate::{
     },
 };
 use crate::{
-    exec::{handle_sigchld, signal_fmt},
+    exec::{exec_command, handle_sigchld, noexec::SpawnNoexecHandler, signal_fmt},
     log::{dev_error, dev_info, dev_warn},
     system::{
-        _exit, fork, getpgid, getpgrp,
+        fork, getpgid, getpgrp,
         interface::ProcessId,
         kill, killpg,
         term::{Terminal, UserTerm},
@@ -26,7 +26,12 @@ use crate::{
     },
 };
 
-pub(super) fn exec_no_pty(sudo_pid: ProcessId, mut command: Command) -> io::Result<ExitReason> {
+pub(super) fn exec_no_pty(
+    sudo_pid: ProcessId,
+    mut file_closer: FileCloser,
+    spawn_noexec_handler: Option<SpawnNoexecHandler>,
+    command: Command,
+) -> io::Result<ExitReason> {
     // FIXME (ogsudo): Initialize the policy plugin's session here.
 
     // Block all the signals until we are done setting up the signal handlers so we don't miss
@@ -39,12 +44,10 @@ 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()?;
+    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.
@@ -56,39 +59,13 @@ pub(super) fn exec_no_pty(sudo_pid: ProcessId, mut command: Command) -> io::Resu
         err
     })?
     else {
-        // Restore the signal mask now that the handlers have been setup.
-        if let Some(set) = original_set {
-            if let Err(err) = set.set_mask() {
-                dev_warn!("cannot restore signal mask: {err}");
-            }
-        }
-
-        // 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() } {
-            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() {
-                errpipe_tx.write(&error_code).ok();
-            }
-
-            // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-            _exit(1);
-        }
-
-        let err = command.exec();
-
-        dev_warn!("failed to execute command: {err}");
-        // If `exec` returns, it means that executing the command failed. Send the error to the
-        // monitor using the pipe.
-        if let Some(error_code) = err.raw_os_error() {
-            errpipe_tx.write(&error_code).ok();
-        }
-
-        // We call `_exit` instead of `exit` to avoid flushing the parent's IO streams by accident.
-        _exit(1);
+        exec_command(file_closer, command, original_set, errpipe_tx);
     };
 
+    if let Some(spawner) = spawn_noexec_handler {
+        spawner.spawn();
+    }
+
     dev_info!("executed command with pid {command_pid}");
 
     let mut registry = EventRegistry::new();