From f66c55f98e53a202988600820f9bc73a4826db0d Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 11:28:51 -0800 Subject: [PATCH 1/8] Run cargo fmt on sudo_pair --- sudo_pair/src/errors.rs | 26 +++-- sudo_pair/src/lib.rs | 211 +++++++++++++++++++++----------------- sudo_pair/src/socket.rs | 100 ++++++++++-------- sudo_pair/src/template.rs | 122 +++++++++------------- 4 files changed, 240 insertions(+), 219 deletions(-) diff --git a/sudo_pair/src/errors.rs b/sudo_pair/src/errors.rs index 996802d..3d32b02 100644 --- a/sudo_pair/src/errors.rs +++ b/sudo_pair/src/errors.rs @@ -12,13 +12,13 @@ // implied. See the License for the specific language governing // permissions and limitations under the License. +use std::error::Error as StdError; use std::fmt::{Display, Formatter, Result as FmtResult}; use std::result::Result as StdResult; -use std::error::Error as StdError; use failure::Context; -use sudo_plugin::prelude::{Error as PluginError, OpenStatus, LogStatus}; +use sudo_plugin::prelude::{Error as PluginError, LogStatus, OpenStatus}; pub(crate) type Result = StdResult; @@ -36,13 +36,19 @@ pub(crate) enum ErrorKind { impl ErrorKind { fn as_str(&self) -> &'static str { match self { - ErrorKind::CommunicationError => "couldn't establish communications with the pair", - ErrorKind::SessionDeclined => "pair declined the session", - ErrorKind::SessionTerminated => "pair ended the session", - ErrorKind::StdinRedirected => "redirection of stdin to paired sessions is prohibited", - ErrorKind::SudoToUserAndGroup => "the -u and -g options may not both be specified", - - ErrorKind::PluginError(_) => "the plugin failed to initialize", + ErrorKind::CommunicationError => { + "couldn't establish communications with the pair" + } + ErrorKind::SessionDeclined => "pair declined the session", + ErrorKind::SessionTerminated => "pair ended the session", + ErrorKind::StdinRedirected => { + "redirection of stdin to paired sessions is prohibited" + } + ErrorKind::SudoToUserAndGroup => { + "the -u and -g options may not both be specified" + } + + ErrorKind::PluginError(_) => "the plugin failed to initialize", } } } @@ -94,4 +100,4 @@ impl From for Error { } } -impl StdError for Error { } +impl StdError for Error {} diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 6ac49e8..8b80480 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -30,7 +30,6 @@ #![warn(rust_2018_compatibility)] #![warn(rust_2018_idioms)] #![warn(unused)] - #![warn(bare_trait_objects)] #![warn(missing_copy_implementations)] #![warn(missing_debug_implementations)] @@ -45,33 +44,28 @@ #![warn(unused_qualifications)] #![warn(unused_results)] #![warn(variant_size_differences)] - // this entire crate is unsafe code #![allow(unsafe_code)] - #![warn(rustdoc::all)] - #![warn(clippy::cargo)] #![warn(clippy::complexity)] #![warn(clippy::correctness)] #![warn(clippy::pedantic)] #![warn(clippy::perf)] #![warn(clippy::style)] - // this is triggered by dependencies #![allow(clippy::multiple_crate_versions)] - // FIXME: this appears to be triggering on non-Drop items like Result, // but this needs to be either investigated or reported upstream #![allow(clippy::let_underscore_drop)] mod errors; -mod template; mod socket; +mod template; use crate::errors::{Error, ErrorKind, Result}; -use crate::template::Spec; use crate::socket::Socket; +use crate::template::Spec; use std::cell::RefCell; use std::collections::HashSet; @@ -84,24 +78,24 @@ use libc::{gid_t, mode_t, uid_t}; use failure::ResultExt; -use sudo_plugin::prelude::*; use sudo_plugin::options::OptionMap; +use sudo_plugin::prelude::*; -const DEFAULT_BINARY_PATH : &str = "/usr/bin/sudo_approve"; -const DEFAULT_USER_PROMPT_PATH : &str = "/etc/sudo_pair.prompt.user"; -const DEFAULT_PAIR_PROMPT_PATH : &str = "/etc/sudo_pair.prompt.pair"; -const DEFAULT_SOCKET_DIR : &str = "/var/run/sudo_pair"; -const DEFAULT_GIDS_ENFORCED : [gid_t; 1] = [0]; +const DEFAULT_BINARY_PATH: &str = "/usr/bin/sudo_approve"; +const DEFAULT_USER_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.user"; +const DEFAULT_PAIR_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.pair"; +const DEFAULT_SOCKET_DIR: &str = "/var/run/sudo_pair"; +const DEFAULT_GIDS_ENFORCED: [gid_t; 1] = [0]; -const DEFAULT_USER_PROMPT : &[u8] = b"%B %u %p\n"; -const DEFAULT_PAIR_PROMPT : &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; +const DEFAULT_USER_PROMPT: &[u8] = b"%B %u %p\n"; +const DEFAULT_PAIR_PROMPT: &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; -sudo_io_plugin!{ sudo_pair : SudoPair } +sudo_io_plugin! { sudo_pair : SudoPair } struct SudoPair { - env: &'static IoEnv, + env: &'static IoEnv, options: PluginOptions, - socket: Option>, + socket: Option>, slog: slog::Logger, } @@ -116,18 +110,23 @@ impl IoPlugin for SudoPair { slog::debug!(slog, "plugin initializing"); - let args : Vec<_> = env.cmdline.iter() + let args: Vec<_> = env + .cmdline + .iter() .skip(1) - .map (|arg| arg.to_string_lossy()) + .map(|arg| arg.to_string_lossy()) .collect(); - slog = slog::Logger::new(&slog, slog::o!( - "uid" => &env.user_info.uid, - "runas_euid" => &env.command_info.runas_euid, - "runas_egid" => &env.command_info.runas_egid, - "command" => env.command_info.command.to_string_lossy().into_owned(), - "args" => format!("{:?}", args), - )); + slog = slog::Logger::new( + &slog, + slog::o!( + "uid" => &env.user_info.uid, + "runas_euid" => &env.command_info.runas_euid, + "runas_egid" => &env.command_info.runas_egid, + "command" => env.command_info.command.to_string_lossy().into_owned(), + "args" => format!("{:?}", args), + ), + ); let options = PluginOptions::from(&env.plugin_options); @@ -145,9 +144,12 @@ impl IoPlugin for SudoPair { }; if pair.is_exempt() { - slog::info!(pair.slog, "pair session exempt from pairing requirements"); + slog::info!( + pair.slog, + "pair session exempt from pairing requirements" + ); - return Ok(pair) + return Ok(pair); } slog::info!(pair.slog, "pair session required"); @@ -211,7 +213,7 @@ impl IoPlugin for SudoPair { } fn log_stdout(&self, log: &[u8]) -> Result<()> { - self.log_output(log) + self.log_output(log) } fn log_stderr(&self, log: &[u8]) -> Result<()> { @@ -235,9 +237,10 @@ impl IoPlugin for SudoPair { impl SudoPair { fn log_output(&self, log: &[u8]) -> Result<()> { // if we have a socket, write to it - self.socket.as_ref().map_or(Ok(()), |socket| { - socket.borrow_mut().write_all(log) - }).context(ErrorKind::SessionTerminated)?; + self.socket + .as_ref() + .map_or(Ok(()), |socket| socket.borrow_mut().write_all(log)) + .context(ErrorKind::SessionTerminated)?; slog::trace!(self.slog, "{{{} bytes sent}}", log.len()); @@ -247,9 +250,9 @@ impl SudoPair { fn local_pair_prompt(&self, template_spec: &Spec) { // read the template from the file; if there's an error, use the // default template instead - let template : Vec = File::open(&self.options.user_prompt_path) - .and_then(|file| file.bytes().collect() ) - .unwrap_or_else(|_| DEFAULT_USER_PROMPT.into() ); + let template: Vec = File::open(&self.options.user_prompt_path) + .and_then(|file| file.bytes().collect()) + .unwrap_or_else(|_| DEFAULT_USER_PROMPT.into()); slog::trace!(self.slog, "local prompt template loaded"); @@ -288,17 +291,23 @@ impl SudoPair { // improbable. For now, I'm ignoring the situation but hopefully // there's enough information here for someone (probably me) to // pick up where I left off. - let _ = self.env.tty().as_mut() - .and_then(|tty| tty.write_all(&prompt).ok() ) + let _ = self + .env + .tty() + .as_mut() + .and_then(|tty| tty.write_all(&prompt).ok()) .ok_or_else(|| self.env.stderr().write_all(&prompt)); slog::trace!(self.slog, "local prompt rendered"); } fn remote_pair_connect(&mut self) -> Result<()> { - let slog = slog::Logger::new(&self.slog, slog::o!( - "socket_path" => self.socket_path().to_string_lossy().into_owned(), - )); + let slog = slog::Logger::new( + &self.slog, + slog::o!( + "socket_path" => self.socket_path().to_string_lossy().into_owned(), + ), + ); slog::debug!(slog, "socket initializing"; "socket_uid" => self.socket_uid(), @@ -325,7 +334,8 @@ impl SudoPair { self.socket_uid(), self.socket_gid(), self.socket_mode(), - ).context(ErrorKind::CommunicationError)?; + ) + .context(ErrorKind::CommunicationError)?; self.socket = Some(RefCell::new(socket)); @@ -337,9 +347,9 @@ impl SudoPair { fn remote_pair_prompt(&mut self, template_spec: &Spec) -> Result<()> { // read the template from the file; if there's an error, use the // default template instead - let template : Vec = File::open(&self.options.pair_prompt_path) - .and_then(|file| file.bytes().collect() ) - .unwrap_or_else(|_| DEFAULT_PAIR_PROMPT.into() ); + let template: Vec = File::open(&self.options.pair_prompt_path) + .and_then(|file| file.bytes().collect()) + .unwrap_or_else(|_| DEFAULT_PAIR_PROMPT.into()); slog::trace!(self.slog, "remote prompt loaded"); @@ -347,17 +357,17 @@ impl SudoPair { slog::trace!(self.slog, "remote prompt evaluated"); - let mut socket = self.socket + let mut socket = self + .socket .as_ref() .ok_or(ErrorKind::CommunicationError)? .borrow_mut(); - socket.write_all(&prompt[..]) - .context(ErrorKind::CommunicationError)?; - - socket.flush() + socket + .write_all(&prompt[..]) .context(ErrorKind::CommunicationError)?; + socket.flush().context(ErrorKind::CommunicationError)?; slog::trace!(self.slog, "remote prompt rendered"); @@ -365,7 +375,7 @@ impl SudoPair { // `read` might return without actually having written anything; // this prevents us from being required to check the number of // bytes actually read from `read` - let mut response : [u8; 1] = [b'n']; + let mut response: [u8; 1] = [b'n']; slog::debug!(self.slog, "remote prompt awaiting response..."); @@ -374,7 +384,8 @@ impl SudoPair { // Ctrl-C and retry the read); we don't need to check the return // value because if the read was successful, we're guaranteed to // have read at least one byte - let _ = socket.read(&mut response) + let _ = socket + .read(&mut response) .context(ErrorKind::SessionDeclined)?; slog::debug!(self.slog, "remote pair responded"; @@ -388,7 +399,7 @@ impl SudoPair { match &response { b"y" | b"Y" => (), - _ => { + _ => { slog::warn!(self.slog, "remote pair declined session"); return Err(ErrorKind::SessionDeclined.into()); } @@ -495,10 +506,9 @@ impl SudoPair { /// facilities to log output for. /// fn is_exempted_from_logging(&self) -> bool { - if - !self.env.command_info.iolog_ttyout && - !self.env.command_info.iolog_stdout && - !self.env.command_info.iolog_stderr + if !self.env.command_info.iolog_ttyout + && !self.env.command_info.iolog_stdout + && !self.env.command_info.iolog_stderr { return true; } @@ -518,22 +528,24 @@ impl SudoPair { // to a new group which is fine, what we want to avoid is the // user explicitly providing a *different* group if self.is_sudoing_to_user() && self.is_sudoing_to_explicit_group() { - return true + return true; } false } fn is_sudoing_from_exempted_gid(&self) -> bool { - !self.options.gids_exempted.is_disjoint( - &self.env.user_info.groups.iter().copied().collect() - ) + !self + .options + .gids_exempted + .is_disjoint(&self.env.user_info.groups.iter().copied().collect()) } fn is_sudoing_to_enforced_gid(&self) -> bool { - !self.options.gids_enforced.is_disjoint( - &self.env.runas_gids() - ) + !self + .options + .gids_enforced + .is_disjoint(&self.env.runas_gids()) } fn is_sudoing_to_user(&self) -> bool { @@ -560,13 +572,10 @@ impl SudoPair { // note that we want the *`uid`* and not the `euid` here since // we want to know who the real user is and not the `uid` of the // owner of `sudo` - self.options.socket_dir.join( - format!( - "{}.{}.sock", - self.env.user_info.uid, - self.env.user_info.pid, - ) - ) + self.options.socket_dir.join(format!( + "{}.{}.sock", + self.env.user_info.uid, self.env.user_info.pid, + )) } fn socket_uid(&self) -> uid_t { @@ -723,9 +732,10 @@ struct PluginOptions { impl PluginOptions { fn binary_name(&self) -> &[u8] { - self.binary_path.file_name().unwrap_or_else(|| - self.binary_path.as_os_str() - ).as_bytes() + self.binary_path + .file_name() + .unwrap_or_else(|| self.binary_path.as_os_str()) + .as_bytes() } } @@ -735,29 +745,38 @@ impl PluginOptions { impl<'a> From<&'a OptionMap> for PluginOptions { fn from(map: &'a OptionMap) -> Self { Self { - binary_path: map.get("binary_path") + binary_path: map + .get("binary_path") .unwrap_or_else(|_| DEFAULT_BINARY_PATH.into()), - user_prompt_path: map.get("user_prompt_path") + user_prompt_path: map + .get("user_prompt_path") .unwrap_or_else(|_| DEFAULT_USER_PROMPT_PATH.into()), - pair_prompt_path: map.get("pair_prompt_path") + pair_prompt_path: map + .get("pair_prompt_path") .unwrap_or_else(|_| DEFAULT_PAIR_PROMPT_PATH.into()), - socket_dir: map.get("socket_dir") + socket_dir: map + .get("socket_dir") .unwrap_or_else(|_| DEFAULT_SOCKET_DIR.into()), - gids_enforced: map.get("gids_enforced") - .unwrap_or_else(|_| DEFAULT_GIDS_ENFORCED.iter().copied().collect()), + gids_enforced: map.get("gids_enforced").unwrap_or_else(|_| { + DEFAULT_GIDS_ENFORCED.iter().copied().collect() + }), - gids_exempted: map.get("gids_exempted") - .unwrap_or_default(), + gids_exempted: map.get("gids_exempted").unwrap_or_default(), } } } impl slog::Value for PluginOptions { - fn serialize(&self, _: &slog::Record<'_>, key: slog::Key, serializer: &mut dyn slog::Serializer) -> slog::Result { + fn serialize( + &self, + _: &slog::Record<'_>, + key: slog::Key, + serializer: &mut dyn slog::Serializer, + ) -> slog::Result { serializer.emit_str(key, &format!("{:?}", self)) } } @@ -765,7 +784,7 @@ impl slog::Value for PluginOptions { // TODO: can we only compile slog in when logging features are enabled? #[cfg(not(any(feature = "syslog", feature = "journald")))] fn slog(name: &str, version: &str) -> slog::Logger { - slog::Logger::root(slog::Discard, o!() ) + slog::Logger::root(slog::Discard, o!()) } #[cfg(all(feature = "syslog", not(feature = "journald")))] @@ -784,10 +803,13 @@ fn slog(name: &str, version: &str) -> slog::Logger { .ok(); match drain { - Some(d) => slog::Logger::root(d, slog::o!( - "plugin_name" => name .to_owned(), - "plugin_version" => version.to_owned() - )), + Some(d) => slog::Logger::root( + d, + slog::o!( + "plugin_name" => name .to_owned(), + "plugin_version" => version.to_owned() + ), + ), None => slog::Logger::root(slog::Discard, slog::o!()), } @@ -799,8 +821,11 @@ fn slog(name: &str, version: &str) -> slog::Logger { let drain = slog_journald::JournaldDrain.ignore_res(); - slog::Logger::root(drain, slog::o!( - "plugin_name" => name .to_owned(), - "plugin_version" => version.to_owned() - )) + slog::Logger::root( + drain, + slog::o!( + "plugin_name" => name .to_owned(), + "plugin_version" => version.to_owned() + ), + ) } diff --git a/sudo_pair/src/socket.rs b/sudo_pair/src/socket.rs index eaf45c6..6f9f4d1 100644 --- a/sudo_pair/src/socket.rs +++ b/sudo_pair/src/socket.rs @@ -19,11 +19,11 @@ use std::ffi::CString; use std::fs; -use std::io::{Read, Write, Result, Error, ErrorKind}; +use std::io::{Error, ErrorKind, Read, Result, Write}; +use std::mem; use std::net::Shutdown; -use std::os::unix::prelude::*; use std::os::unix::net::{UnixListener, UnixStream}; -use std::mem; +use std::os::unix::prelude::*; use std::path::Path; use std::ptr; @@ -37,8 +37,8 @@ pub(crate) struct Socket { impl Socket { pub(crate) fn open>( path: P, - uid: uid_t, - gid: gid_t, + uid: uid_t, + gid: gid_t, mode: mode_t, ) -> Result { let path = path.as_ref(); @@ -56,9 +56,7 @@ impl Socket { }; let socket = UnixListener::bind(&path).and_then(|listener| { - let cpath = CString::new( - path.as_os_str().as_bytes() - )?; + let cpath = CString::new(path.as_os_str().as_bytes())?; unsafe { if libc::chown(cpath.as_ptr(), uid, gid) == -1 { @@ -69,7 +67,7 @@ impl Socket { return Err(Error::last_os_error()); } - let fd = listener.as_raw_fd(); + let fd = listener.as_raw_fd(); let mut readfds = mem::MaybeUninit::::uninit(); libc::FD_ZERO(readfds.as_mut_ptr()); @@ -89,21 +87,27 @@ impl Socket { ptr::null_mut(), ptr::null_mut(), ) { - 1 => (), + 1 => (), -1 => return Err(Error::last_os_error()), - 0 => unreachable!("`select` returned 0 even though no timeout was set"), - _ => unreachable!("`select` indicated that more than 1 fd is ready"), + 0 => unreachable!( + "`select` returned 0 even though no timeout was set" + ), + _ => unreachable!( + "`select` indicated that more than 1 fd is ready" + ), }; // as a sanity check, confirm that the fd we're going to // `accept` is the one that `select` says is ready if !libc::FD_ISSET(fd, &readfds) { - unreachable!("`select` returned an unexpected file descriptor"); + unreachable!( + "`select` returned an unexpected file descriptor" + ); } } - listener.accept().map(|connection| { - Self { socket: connection.0 } + listener.accept().map(|connection| Self { + socket: connection.0, }) }); @@ -149,40 +153,44 @@ impl Socket { fn enforce_ownership(path: &Path) -> Result<()> { let parent = path.parent().ok_or_else(|| { - Error::new(ErrorKind::AlreadyExists, format!( + Error::new( + ErrorKind::AlreadyExists, + format!( "couldn't determine permissions of the parent directory for {}", path.to_string_lossy() - )) + ), + ) })?; - let parent = CString::new( - parent.as_os_str().as_bytes() - )?; + let parent = CString::new(parent.as_os_str().as_bytes())?; unsafe { let mut stat = mem::MaybeUninit::::uninit(); - if libc::stat( - parent.as_ptr(), - stat.as_mut_ptr() - ) == -1 { + if libc::stat(parent.as_ptr(), stat.as_mut_ptr()) == -1 { return Err(Error::last_os_error()); } let stat = stat.assume_init(); if stat.st_mode & libc::S_IFDIR == 0 { - return Err(Error::new(ErrorKind::Other, format!( - "the socket path {} is not a directory", - parent.to_string_lossy(), - ))); + return Err(Error::new( + ErrorKind::Other, + format!( + "the socket path {} is not a directory", + parent.to_string_lossy(), + ), + )); } if stat.st_uid != libc::geteuid() { - return Err(Error::new(ErrorKind::Other, format!( - "the socket directory {} is not owned by root", - parent.to_string_lossy(), - ))); + return Err(Error::new( + ErrorKind::Other, + format!( + "the socket directory {} is not owned by root", + parent.to_string_lossy(), + ), + )); } // TODO: temporarily disabled while I relearn everything I @@ -215,10 +223,13 @@ impl Socket { // } if stat.st_mode & (libc::S_IWGRP | libc::S_IWOTH) != 0 { - return Err(Error::new(ErrorKind::Other, format!( - "the socket directory {} has insecure permissions", - parent.to_string_lossy(), - ))); + return Err(Error::new( + ErrorKind::Other, + format!( + "the socket directory {} has insecure permissions", + parent.to_string_lossy(), + ), + )); } } @@ -238,17 +249,17 @@ impl Read for Socket { // of the socket, so we ensure that the signal handler for // Ctrl-C aborts the read instead of restarting it // automatically - ctrl_c_aborts_syscalls(|| self.socket.read(buf) )? + ctrl_c_aborts_syscalls(|| self.socket.read(buf))? } } impl Write for Socket { fn write(&mut self, buf: &[u8]) -> Result { - ctrl_c_aborts_syscalls(|| self.socket.write(buf) )? + ctrl_c_aborts_syscalls(|| self.socket.write(buf))? } fn flush(&mut self) -> Result<()> { - ctrl_c_aborts_syscalls(|| self.socket.flush() )? + ctrl_c_aborts_syscalls(|| self.socket.flush())? } } @@ -260,11 +271,12 @@ impl Write for Socket { /// will be terminated upon receipt on the signal instead of /// automatically resuming. fn ctrl_c_aborts_syscalls(func: F) -> Result - where F: FnOnce() -> T +where + F: FnOnce() -> T, { unsafe { - let mut sigaction_old = mem::MaybeUninit::::uninit(); - let sigaction_null = ::std::ptr::null_mut(); + let mut sigaction_old = mem::MaybeUninit::::uninit(); + let sigaction_null = ::std::ptr::null_mut(); // retrieve the existing handler sigaction(libc::SIGINT, sigaction_null, sigaction_old.as_mut_ptr())?; @@ -293,10 +305,10 @@ fn ctrl_c_aborts_syscalls(func: F) -> Result unsafe fn sigaction( sig: libc::c_int, new: *const libc::sigaction, - old: *mut libc::sigaction, + old: *mut libc::sigaction, ) -> Result<()> { if libc::sigaction(sig, new, old) == -1 { - return Err(Error::last_os_error()) + return Err(Error::last_os_error()); } Ok(()) diff --git a/sudo_pair/src/template.rs b/sudo_pair/src/template.rs index cf64849..14a0676 100644 --- a/sudo_pair/src/template.rs +++ b/sudo_pair/src/template.rs @@ -14,10 +14,10 @@ use std::collections::HashMap; -const DEFAULT_ESCAPE_BYTE : u8 = b'%'; +const DEFAULT_ESCAPE_BYTE: u8 = b'%'; pub(crate) struct Spec { - escape: u8, + escape: u8, expansions: HashMap>, } @@ -27,10 +27,17 @@ impl Spec { } pub(crate) fn with_escape(escape: u8) -> Self { - Self { escape, .. Self::new() } + Self { + escape, + ..Self::new() + } } - pub(crate) fn replace>>(&mut self, literal: u8, replacement: T) { + pub(crate) fn replace>>( + &mut self, + literal: u8, + replacement: T, + ) { drop(self.expansions.insert(literal, replacement.into())); } @@ -38,13 +45,11 @@ impl Spec { // the expanded result is likely to be at least as long as the // template; if we go a little over, it's not a big deal let mut result = Vec::with_capacity(template.len()); - let mut iter = template.iter().copied(); + let mut iter = template.iter().copied(); while iter.len() != 0 { // copy literally everything up to the next escape character - result.extend( - iter.by_ref().take_while(|b| *b != self.escape ) - ); + result.extend(iter.by_ref().take_while(|b| *b != self.escape)); // TODO: The above take_while consumes an extra byte in // the event that it finds the escape character; this is @@ -58,7 +63,7 @@ impl Spec { // line '%' character, we will silently eat it. let byte = match iter.next() { Some(b) => b, - None => break, + None => break, }; // if the spec contains an expansion for the escaped @@ -66,7 +71,7 @@ impl Spec { // literal match self.expansions.get(&byte) { Some(expansion) => result.extend_from_slice(expansion), - None => result.push(byte), + None => result.push(byte), }; } @@ -78,14 +83,17 @@ impl Default for Spec { fn default() -> Self { Self { expansions: HashMap::new(), - escape: DEFAULT_ESCAPE_BYTE, + escape: DEFAULT_ESCAPE_BYTE, } } } impl From>> for Spec { fn from(expansions: HashMap>) -> Self { - Self { expansions, .. Self::new() } + Self { + expansions, + ..Self::new() + } } } @@ -110,7 +118,7 @@ mod tests { #[test] fn from_hashmap() { let mut map = HashMap::new(); - let _ = map.insert(b'x', b"abc".to_vec()); + let _ = map.insert(b'x', b"abc".to_vec()); let spec = Spec::from(map.clone()); @@ -119,87 +127,69 @@ mod tests { #[test] fn no_expansions() { - let spec = Spec::new(); + let spec = Spec::new(); let template = b"this has no expansions"; - assert_eq!( - template[..], - spec.expand(template)[..] - ); + assert_eq!(template[..], spec.expand(template)[..]); } #[test] fn expansions() { - let mut spec = Spec::new(); - let template = b"a: %a, b: %b"; + let mut spec = Spec::new(); + let template = b"a: %a, b: %b"; let _ = spec.replace(b'a', &b"foo"[..]); let _ = spec.replace(b'b', &b"bar"[..]); - assert_eq!( - b"a: foo, b: bar"[..], - spec.expand(template)[..], - ); + assert_eq!(b"a: foo, b: bar"[..], spec.expand(template)[..],); } #[test] fn repeated_expansions() { - let mut spec = Spec::new(); - let template = b"%a%a%a%b%a%a%b"; + let mut spec = Spec::new(); + let template = b"%a%a%a%b%a%a%b"; let _ = spec.replace(b'a', &b"x"[..]); let _ = spec.replace(b'b', &b"y"[..]); - assert_eq!( - b"xxxyxxy"[..], - spec.expand(template)[..], - ); + assert_eq!(b"xxxyxxy"[..], spec.expand(template)[..],); } #[test] fn expansion_inserts_itself() { - let mut spec = Spec::new(); - let template = b"test %x test"; + let mut spec = Spec::new(); + let template = b"test %x test"; spec.replace(b'x', &b"x"[..]); - assert_eq!( - b"test x test"[..], - spec.expand(template)[..], - ); + assert_eq!(b"test x test"[..], spec.expand(template)[..],); } #[test] fn expansion_isnt_recursive() { - let mut spec = Spec::new(); - let template = b"test %x test"; + let mut spec = Spec::new(); + let template = b"test %x test"; spec.replace(b'x', &b"%x %y %z % %%"[..]); spec.replace(b'y', &b"BUG"[..]); - assert_eq!( - b"test %x %y %z % %% test"[..], - spec.expand(template)[..], - ); + assert_eq!(b"test %x %y %z % %% test"[..], spec.expand(template)[..],); } #[test] fn expansion_inserts_nothing() { - let mut spec = Spec::new(); - let template = b"test %X test"; + let mut spec = Spec::new(); + let template = b"test %X test"; spec.replace(b'X', &b""[..]); - assert_eq!( - b"test test"[..], - spec.expand(template)[..], - ); + assert_eq!(b"test test"[..], spec.expand(template)[..],); } #[test] fn unused_expansions() { - let mut spec = Spec::new(); - let template = b"only y should be expanded %y"; + let mut spec = Spec::new(); + let template = b"only y should be expanded %y"; spec.replace(b'y', &b"qwerty"[..]); spec.replace(b'n', &b"uiop["[..]); @@ -212,26 +202,20 @@ mod tests { #[test] fn literals() { - let mut spec = Spec::new(); - let template = b"a: %a, b: %b"; + let mut spec = Spec::new(); + let template = b"a: %a, b: %b"; spec.replace(b'b', &b"bar"[..]); - assert_eq!( - b"a: a, b: bar"[..], - spec.expand(template)[..], - ); + assert_eq!(b"a: a, b: bar"[..], spec.expand(template)[..],); } #[test] fn literal_escape_character() { - let spec = Spec::new(); + let spec = Spec::new(); let template = b"%%%%%%%%%%%%%%"; - assert_eq!( - b"%%%%%%%"[..], - spec.expand(template)[..], - ); + assert_eq!(b"%%%%%%%"[..], spec.expand(template)[..],); } // you can currently provide an expansion for the escape character, @@ -239,15 +223,12 @@ mod tests { // literal; this isn't worth fixing #[test] fn bug_wontfix_expand_escape_character() { - let mut spec = Spec::new(); - let template = b"|%%|"; + let mut spec = Spec::new(); + let template = b"|%%|"; spec.replace(b'%', &b"x"[..]); - assert_eq!( - b"|x|"[..], - spec.expand(template)[..] - ); + assert_eq!(b"|x|"[..], spec.expand(template)[..]); } // `take_while` silently eats one extra character off of the `Iter`, @@ -257,12 +238,9 @@ mod tests { // character; this isn't worth fixing #[test] fn bug_wontfix_swallow_trailing_escape_character() { - let spec = Spec::new(); + let spec = Spec::new(); let template = b"some text%"; - assert_eq!( - b"some text"[..], - spec.expand(template)[..] - ); + assert_eq!(b"some text"[..], spec.expand(template)[..]); } } From e72efaa74272531034c9aceeb58827292c09ca2c Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 12:01:57 -0800 Subject: [PATCH 2/8] Fix clippy lint errors --- sudo_plugin/src/options/settings.rs | 73 ++++++++++++++--------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/sudo_plugin/src/options/settings.rs b/sudo_plugin/src/options/settings.rs index 4115411..b2b3d13 100644 --- a/sudo_plugin/src/options/settings.rs +++ b/sudo_plugin/src/options/settings.rs @@ -12,9 +12,9 @@ // implied. See the License for the specific language governing // permissions and limitations under the License. -use crate::errors::{Result, Error}; -use crate::options::OptionMap; +use crate::errors::{Error, Result}; use crate::options::traits::{FromSudoOption, FromSudoOptionList}; +use crate::options::OptionMap; use std::convert::TryFrom; use std::net::{AddrParseError, IpAddr}; @@ -275,33 +275,37 @@ impl TryFrom for Settings { fn try_from(value: OptionMap) -> Result { Ok(Self { - plugin_dir: value.get("plugin_dir")?, + plugin_dir: value.get("plugin_dir")?, plugin_path: value.get("plugin_path")?, - progname: value.get("progname")?, - - bsd_auth_type: value.get("bsd_auth_type") .ok(), - close_from: value.get("closefrom") .ok(), - debug_flags: value.get("debug_flags") .ok(), - debug_level: value.get("debug_level") .ok(), - ignore_ticket: value.get("ignore_ticket") .unwrap_or(false), - implied_shell: value.get("implied_shell") .unwrap_or(false), - login_class: value.get("login_class") .ok(), - login_shell: value.get("login_shell") .unwrap_or(false), - max_groups: value.get("max_groups") .ok(), - network_addrs: value.get("network_addrs") .unwrap_or_else(|_| vec![]), - noninteractive: value.get("noninteractive") .unwrap_or(false), - preserve_environment: value.get("preserve_environment").unwrap_or(false), - preserve_groups: value.get("preserve_groups") .unwrap_or(false), - prompt: value.get("prompt") .ok(), - remote_host: value.get("remote_host") .ok(), - run_shell: value.get("run_shell") .unwrap_or(false), - runas_group: value.get("runas_group") .ok(), - runas_user: value.get("runas_user") .ok(), - selinux_role: value.get("selinux_role") .ok(), - selinux_type: value.get("selinux_type") .ok(), - set_home: value.get("set_home") .unwrap_or(false), - sudoedit: value.get("sudoedit") .unwrap_or(false), - timeout: value.get("timeout") .ok(), + progname: value.get("progname")?, + + bsd_auth_type: value.get("bsd_auth_type").ok(), + close_from: value.get("closefrom").ok(), + debug_flags: value.get("debug_flags").ok(), + debug_level: value.get("debug_level").ok(), + ignore_ticket: value.get("ignore_ticket").unwrap_or(false), + implied_shell: value.get("implied_shell").unwrap_or(false), + login_class: value.get("login_class").ok(), + login_shell: value.get("login_shell").unwrap_or(false), + max_groups: value.get("max_groups").ok(), + network_addrs: value + .get("network_addrs") + .unwrap_or_else(|_| vec![]), + noninteractive: value.get("noninteractive").unwrap_or(false), + preserve_environment: value + .get("preserve_environment") + .unwrap_or(false), + preserve_groups: value.get("preserve_groups").unwrap_or(false), + prompt: value.get("prompt").ok(), + remote_host: value.get("remote_host").ok(), + run_shell: value.get("run_shell").unwrap_or(false), + runas_group: value.get("runas_group").ok(), + runas_user: value.get("runas_user").ok(), + selinux_role: value.get("selinux_role").ok(), + selinux_type: value.get("selinux_type").ok(), + set_home: value.get("set_home").unwrap_or(false), + sudoedit: value.get("sudoedit").unwrap_or(false), + timeout: value.get("timeout").ok(), raw: value, }) @@ -323,17 +327,12 @@ impl FromSudoOption for NetAddr { // slice fn from_sudo_option(s: &str) -> ::std::result::Result { let bytes = s.as_bytes(); - let mid = bytes.iter() - .position(|b| *b == b'/' ) - .unwrap_or_else(|| bytes.len()); + let mid = bytes.iter().position(|b| *b == b'/').unwrap_or(bytes.len()); - let addr = s[ .. mid].parse()?; - let mask = s[mid + 1 .. ].parse()?; + let addr = s[..mid].parse()?; + let mask = s[mid + 1..].parse()?; - Ok(Self { - addr, - mask, - }) + Ok(Self { addr, mask }) } } From 68e580fe54f269831c17ebd79683abadbc0402a3 Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 12:21:05 -0800 Subject: [PATCH 3/8] Fix another lint --- sudo_pair/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 8b80480..04763c8 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -93,9 +93,9 @@ const DEFAULT_PAIR_PROMPT: &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; sudo_io_plugin! { sudo_pair : SudoPair } struct SudoPair { - env: &'static IoEnv, + env: &'static IoEnv, options: PluginOptions, - socket: Option>, + socket: Option>, slog: slog::Logger, } @@ -501,10 +501,8 @@ impl SudoPair { self.env.command_info.command == self.options.binary_path } - /// /// Returns true if the policy plugin has not given us any /// facilities to log output for. - /// fn is_exempted_from_logging(&self) -> bool { if !self.env.command_info.iolog_ttyout && !self.env.command_info.iolog_stdout @@ -734,7 +732,7 @@ impl PluginOptions { fn binary_name(&self) -> &[u8] { self.binary_path .file_name() - .unwrap_or_else(|| self.binary_path.as_os_str()) + .unwrap_or(self.binary_path.as_os_str()) .as_bytes() } } From 49843336fd9f20c942c1ea6dcabf26e0996f550b Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 16:24:54 -0800 Subject: [PATCH 4/8] Add some strategic rustfmt skip directives --- sudo_pair/src/lib.rs | 35 +++++++++------- sudo_plugin/src/options/settings.rs | 62 ++++++++++++++--------------- 2 files changed, 50 insertions(+), 47 deletions(-) diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 04763c8..8b0bc17 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -81,14 +81,19 @@ use failure::ResultExt; use sudo_plugin::options::OptionMap; use sudo_plugin::prelude::*; -const DEFAULT_BINARY_PATH: &str = "/usr/bin/sudo_approve"; -const DEFAULT_USER_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.user"; -const DEFAULT_PAIR_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.pair"; -const DEFAULT_SOCKET_DIR: &str = "/var/run/sudo_pair"; -const DEFAULT_GIDS_ENFORCED: [gid_t; 1] = [0]; - -const DEFAULT_USER_PROMPT: &[u8] = b"%B %u %p\n"; -const DEFAULT_PAIR_PROMPT: &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; +#[rustfmt::skip] +mod default { + use libc::gid_t; + + pub(crate) const BINARY_PATH: &str = "/usr/bin/sudo_approve"; + pub(crate) const USER_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.user"; + pub(crate) const PAIR_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.pair"; + pub(crate) const SOCKET_DIR: &str = "/var/run/sudo_pair"; + pub(crate) const GIDS_ENFORCED: [gid_t; 1] = [0]; + + pub(crate) const USER_PROMPT: &[u8] = b"%B %u %p\n"; + pub(crate) const PAIR_PROMPT: &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; +} sudo_io_plugin! { sudo_pair : SudoPair } @@ -252,7 +257,7 @@ impl SudoPair { // default template instead let template: Vec = File::open(&self.options.user_prompt_path) .and_then(|file| file.bytes().collect()) - .unwrap_or_else(|_| DEFAULT_USER_PROMPT.into()); + .unwrap_or_else(|_| default::USER_PROMPT.into()); slog::trace!(self.slog, "local prompt template loaded"); @@ -349,7 +354,7 @@ impl SudoPair { // default template instead let template: Vec = File::open(&self.options.pair_prompt_path) .and_then(|file| file.bytes().collect()) - .unwrap_or_else(|_| DEFAULT_PAIR_PROMPT.into()); + .unwrap_or_else(|_| default::PAIR_PROMPT.into()); slog::trace!(self.slog, "remote prompt loaded"); @@ -745,22 +750,22 @@ impl<'a> From<&'a OptionMap> for PluginOptions { Self { binary_path: map .get("binary_path") - .unwrap_or_else(|_| DEFAULT_BINARY_PATH.into()), + .unwrap_or_else(|_| default::BINARY_PATH.into()), user_prompt_path: map .get("user_prompt_path") - .unwrap_or_else(|_| DEFAULT_USER_PROMPT_PATH.into()), + .unwrap_or_else(|_| default::USER_PROMPT_PATH.into()), pair_prompt_path: map .get("pair_prompt_path") - .unwrap_or_else(|_| DEFAULT_PAIR_PROMPT_PATH.into()), + .unwrap_or_else(|_| default::PAIR_PROMPT_PATH.into()), socket_dir: map .get("socket_dir") - .unwrap_or_else(|_| DEFAULT_SOCKET_DIR.into()), + .unwrap_or_else(|_| default::SOCKET_DIR.into()), gids_enforced: map.get("gids_enforced").unwrap_or_else(|_| { - DEFAULT_GIDS_ENFORCED.iter().copied().collect() + default::GIDS_ENFORCED.iter().copied().collect() }), gids_exempted: map.get("gids_exempted").unwrap_or_default(), diff --git a/sudo_plugin/src/options/settings.rs b/sudo_plugin/src/options/settings.rs index b2b3d13..36ad467 100644 --- a/sudo_plugin/src/options/settings.rs +++ b/sudo_plugin/src/options/settings.rs @@ -273,39 +273,36 @@ impl Settings { impl TryFrom for Settings { type Error = Error; + #[rustfmt::skip] fn try_from(value: OptionMap) -> Result { Ok(Self { - plugin_dir: value.get("plugin_dir")?, + plugin_dir: value.get("plugin_dir")?, plugin_path: value.get("plugin_path")?, - progname: value.get("progname")?, - - bsd_auth_type: value.get("bsd_auth_type").ok(), - close_from: value.get("closefrom").ok(), - debug_flags: value.get("debug_flags").ok(), - debug_level: value.get("debug_level").ok(), - ignore_ticket: value.get("ignore_ticket").unwrap_or(false), - implied_shell: value.get("implied_shell").unwrap_or(false), - login_class: value.get("login_class").ok(), - login_shell: value.get("login_shell").unwrap_or(false), - max_groups: value.get("max_groups").ok(), - network_addrs: value - .get("network_addrs") - .unwrap_or_else(|_| vec![]), - noninteractive: value.get("noninteractive").unwrap_or(false), - preserve_environment: value - .get("preserve_environment") - .unwrap_or(false), - preserve_groups: value.get("preserve_groups").unwrap_or(false), - prompt: value.get("prompt").ok(), - remote_host: value.get("remote_host").ok(), - run_shell: value.get("run_shell").unwrap_or(false), - runas_group: value.get("runas_group").ok(), - runas_user: value.get("runas_user").ok(), - selinux_role: value.get("selinux_role").ok(), - selinux_type: value.get("selinux_type").ok(), - set_home: value.get("set_home").unwrap_or(false), - sudoedit: value.get("sudoedit").unwrap_or(false), - timeout: value.get("timeout").ok(), + progname: value.get("progname")?, + + bsd_auth_type: value.get("bsd_auth_type") .ok(), + close_from: value.get("closefrom") .ok(), + debug_flags: value.get("debug_flags") .ok(), + debug_level: value.get("debug_level") .ok(), + ignore_ticket: value.get("ignore_ticket") .unwrap_or(false), + implied_shell: value.get("implied_shell") .unwrap_or(false), + login_class: value.get("login_class") .ok(), + login_shell: value.get("login_shell") .unwrap_or(false), + max_groups: value.get("max_groups") .ok(), + network_addrs: value.get("network_addrs") .unwrap_or_default(), + noninteractive: value.get("noninteractive") .unwrap_or(false), + preserve_environment: value.get("preserve_environment").unwrap_or(false), + preserve_groups: value.get("preserve_groups") .unwrap_or(false), + prompt: value.get("prompt") .ok(), + remote_host: value.get("remote_host") .ok(), + run_shell: value.get("run_shell") .unwrap_or(false), + runas_group: value.get("runas_group") .ok(), + runas_user: value.get("runas_user") .ok(), + selinux_role: value.get("selinux_role") .ok(), + selinux_type: value.get("selinux_type") .ok(), + set_home: value.get("set_home") .unwrap_or(false), + sudoedit: value.get("sudoedit") .unwrap_or(false), + timeout: value.get("timeout") .ok(), raw: value, }) @@ -325,12 +322,13 @@ impl FromSudoOption for NetAddr { // rust to split a byte array on a delimiter, and the code below // selects the midpoint such that it's guaranteed to be within the // slice + #[rustfmt::skip] fn from_sudo_option(s: &str) -> ::std::result::Result { let bytes = s.as_bytes(); let mid = bytes.iter().position(|b| *b == b'/').unwrap_or(bytes.len()); - let addr = s[..mid].parse()?; - let mask = s[mid + 1..].parse()?; + let addr = s[ .. mid].parse()?; + let mask = s[mid + 1 .. ].parse()?; Ok(Self { addr, mask }) } From ded4e8a6ad8ce2f69431e42aa46b20aab43cc171 Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 16:39:52 -0800 Subject: [PATCH 5/8] Tweak rustfmt toml --- .rustfmt.toml | 3 +- sudo_pair/src/errors.rs | 12 ++------ sudo_pair/src/lib.rs | 65 ++++++++++----------------------------- sudo_pair/src/socket.rs | 36 ++++++---------------- sudo_pair/src/template.rs | 28 ++++------------- 5 files changed, 37 insertions(+), 107 deletions(-) diff --git a/.rustfmt.toml b/.rustfmt.toml index ffa996a..cb3d643 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -1,6 +1,6 @@ ### stable features ### -max_width = 80 +max_width = 100 normalize_comments = true reorder_imports = true @@ -12,3 +12,4 @@ wrap_comments = true unstable_features = true struct_field_align_threshold = 64 +use_small_heuristics = "Max" diff --git a/sudo_pair/src/errors.rs b/sudo_pair/src/errors.rs index 3d32b02..f6f568d 100644 --- a/sudo_pair/src/errors.rs +++ b/sudo_pair/src/errors.rs @@ -36,17 +36,11 @@ pub(crate) enum ErrorKind { impl ErrorKind { fn as_str(&self) -> &'static str { match self { - ErrorKind::CommunicationError => { - "couldn't establish communications with the pair" - } + ErrorKind::CommunicationError => "couldn't establish communications with the pair", ErrorKind::SessionDeclined => "pair declined the session", ErrorKind::SessionTerminated => "pair ended the session", - ErrorKind::StdinRedirected => { - "redirection of stdin to paired sessions is prohibited" - } - ErrorKind::SudoToUserAndGroup => { - "the -u and -g options may not both be specified" - } + ErrorKind::StdinRedirected => "redirection of stdin to paired sessions is prohibited", + ErrorKind::SudoToUserAndGroup => "the -u and -g options may not both be specified", ErrorKind::PluginError(_) => "the plugin failed to initialize", } diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 8b0bc17..580ebe5 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -115,12 +115,7 @@ impl IoPlugin for SudoPair { slog::debug!(slog, "plugin initializing"); - let args: Vec<_> = env - .cmdline - .iter() - .skip(1) - .map(|arg| arg.to_string_lossy()) - .collect(); + let args: Vec<_> = env.cmdline.iter().skip(1).map(|arg| arg.to_string_lossy()).collect(); slog = slog::Logger::new( &slog, @@ -140,19 +135,10 @@ impl IoPlugin for SudoPair { ); // TODO: convert all outgoing errors to be unauthorized errors - let mut pair = Self { - env, - options, - socket: None, - - slog, - }; + let mut pair = Self { env, options, socket: None, slog }; if pair.is_exempt() { - slog::info!( - pair.slog, - "pair session exempt from pairing requirements" - ); + slog::info!(pair.slog, "pair session exempt from pairing requirements"); return Ok(pair); } @@ -362,15 +348,9 @@ impl SudoPair { slog::trace!(self.slog, "remote prompt evaluated"); - let mut socket = self - .socket - .as_ref() - .ok_or(ErrorKind::CommunicationError)? - .borrow_mut(); + let mut socket = self.socket.as_ref().ok_or(ErrorKind::CommunicationError)?.borrow_mut(); - socket - .write_all(&prompt[..]) - .context(ErrorKind::CommunicationError)?; + socket.write_all(&prompt[..]).context(ErrorKind::CommunicationError)?; socket.flush().context(ErrorKind::CommunicationError)?; @@ -389,9 +369,7 @@ impl SudoPair { // Ctrl-C and retry the read); we don't need to check the return // value because if the read was successful, we're guaranteed to // have read at least one byte - let _ = socket - .read(&mut response) - .context(ErrorKind::SessionDeclined)?; + let _ = socket.read(&mut response).context(ErrorKind::SessionDeclined)?; slog::debug!(self.slog, "remote pair responded"; "response" => String::from_utf8_lossy(&response[..]).into_owned(), @@ -545,10 +523,7 @@ impl SudoPair { } fn is_sudoing_to_enforced_gid(&self) -> bool { - !self - .options - .gids_enforced - .is_disjoint(&self.env.runas_gids()) + !self.options.gids_enforced.is_disjoint(&self.env.runas_gids()) } fn is_sudoing_to_user(&self) -> bool { @@ -575,10 +550,9 @@ impl SudoPair { // note that we want the *`uid`* and not the `euid` here since // we want to know who the real user is and not the `uid` of the // owner of `sudo` - self.options.socket_dir.join(format!( - "{}.{}.sock", - self.env.user_info.uid, self.env.user_info.pid, - )) + self.options + .socket_dir + .join(format!("{}.{}.sock", self.env.user_info.uid, self.env.user_info.pid,)) } fn socket_uid(&self) -> uid_t { @@ -735,10 +709,7 @@ struct PluginOptions { impl PluginOptions { fn binary_name(&self) -> &[u8] { - self.binary_path - .file_name() - .unwrap_or(self.binary_path.as_os_str()) - .as_bytes() + self.binary_path.file_name().unwrap_or(self.binary_path.as_os_str()).as_bytes() } } @@ -748,9 +719,7 @@ impl PluginOptions { impl<'a> From<&'a OptionMap> for PluginOptions { fn from(map: &'a OptionMap) -> Self { Self { - binary_path: map - .get("binary_path") - .unwrap_or_else(|_| default::BINARY_PATH.into()), + binary_path: map.get("binary_path").unwrap_or_else(|_| default::BINARY_PATH.into()), user_prompt_path: map .get("user_prompt_path") @@ -760,13 +729,11 @@ impl<'a> From<&'a OptionMap> for PluginOptions { .get("pair_prompt_path") .unwrap_or_else(|_| default::PAIR_PROMPT_PATH.into()), - socket_dir: map - .get("socket_dir") - .unwrap_or_else(|_| default::SOCKET_DIR.into()), + socket_dir: map.get("socket_dir").unwrap_or_else(|_| default::SOCKET_DIR.into()), - gids_enforced: map.get("gids_enforced").unwrap_or_else(|_| { - default::GIDS_ENFORCED.iter().copied().collect() - }), + gids_enforced: map + .get("gids_enforced") + .unwrap_or_else(|_| default::GIDS_ENFORCED.iter().copied().collect()), gids_exempted: map.get("gids_exempted").unwrap_or_default(), } diff --git a/sudo_pair/src/socket.rs b/sudo_pair/src/socket.rs index 6f9f4d1..988d094 100644 --- a/sudo_pair/src/socket.rs +++ b/sudo_pair/src/socket.rs @@ -51,9 +51,7 @@ impl Socket { // by default, ensure no permissions on the created socket since // we're going to customize them immediately afterward - let umask = unsafe { - libc::umask(libc::S_IRWXU | libc::S_IRWXG | libc::S_IRWXO) - }; + let umask = unsafe { libc::umask(libc::S_IRWXU | libc::S_IRWXG | libc::S_IRWXO) }; let socket = UnixListener::bind(&path).and_then(|listener| { let cpath = CString::new(path.as_os_str().as_bytes())?; @@ -89,26 +87,18 @@ impl Socket { ) { 1 => (), -1 => return Err(Error::last_os_error()), - 0 => unreachable!( - "`select` returned 0 even though no timeout was set" - ), - _ => unreachable!( - "`select` indicated that more than 1 fd is ready" - ), + 0 => unreachable!("`select` returned 0 even though no timeout was set"), + _ => unreachable!("`select` indicated that more than 1 fd is ready"), }; // as a sanity check, confirm that the fd we're going to // `accept` is the one that `select` says is ready if !libc::FD_ISSET(fd, &readfds) { - unreachable!( - "`select` returned an unexpected file descriptor" - ); + unreachable!("`select` returned an unexpected file descriptor"); } } - listener.accept().map(|connection| Self { - socket: connection.0, - }) + listener.accept().map(|connection| Self { socket: connection.0 }) }); // once the connection has been made (or aborted due to ctrl-c), @@ -140,10 +130,7 @@ impl Socket { // file exists, is not a socket; abort Ok(false) => Err(Error::new( ErrorKind::AlreadyExists, - format!( - "{} exists and is not a socket", - path.to_string_lossy() - ), + format!("{} exists and is not a socket", path.to_string_lossy()), )), // file doesn't exist; nothing to do @@ -156,9 +143,9 @@ impl Socket { Error::new( ErrorKind::AlreadyExists, format!( - "couldn't determine permissions of the parent directory for {}", - path.to_string_lossy() - ), + "couldn't determine permissions of the parent directory for {}", + path.to_string_lossy() + ), ) })?; @@ -176,10 +163,7 @@ impl Socket { if stat.st_mode & libc::S_IFDIR == 0 { return Err(Error::new( ErrorKind::Other, - format!( - "the socket path {} is not a directory", - parent.to_string_lossy(), - ), + format!("the socket path {} is not a directory", parent.to_string_lossy(),), )); } diff --git a/sudo_pair/src/template.rs b/sudo_pair/src/template.rs index 14a0676..b896964 100644 --- a/sudo_pair/src/template.rs +++ b/sudo_pair/src/template.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; const DEFAULT_ESCAPE_BYTE: u8 = b'%'; pub(crate) struct Spec { - escape: u8, + escape: u8, expansions: HashMap>, } @@ -27,17 +27,10 @@ impl Spec { } pub(crate) fn with_escape(escape: u8) -> Self { - Self { - escape, - ..Self::new() - } + Self { escape, ..Self::new() } } - pub(crate) fn replace>>( - &mut self, - literal: u8, - replacement: T, - ) { + pub(crate) fn replace>>(&mut self, literal: u8, replacement: T) { drop(self.expansions.insert(literal, replacement.into())); } @@ -81,19 +74,13 @@ impl Spec { impl Default for Spec { fn default() -> Self { - Self { - expansions: HashMap::new(), - escape: DEFAULT_ESCAPE_BYTE, - } + Self { expansions: HashMap::new(), escape: DEFAULT_ESCAPE_BYTE } } } impl From>> for Spec { fn from(expansions: HashMap>) -> Self { - Self { - expansions, - ..Self::new() - } + Self { expansions, ..Self::new() } } } @@ -194,10 +181,7 @@ mod tests { spec.replace(b'y', &b"qwerty"[..]); spec.replace(b'n', &b"uiop["[..]); - assert_eq!( - b"only y should be expanded qwerty"[..], - spec.expand(template)[..], - ); + assert_eq!(b"only y should be expanded qwerty"[..], spec.expand(template)[..],); } #[test] From 15b788e456cafe46961dfdf1330760ba431037e1 Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 16:42:07 -0800 Subject: [PATCH 6/8] Remove small heuristics --- .rustfmt.toml | 3 +-- sudo_pair/src/lib.rs | 53 ++++++++++++++++++++++++++++++--------- sudo_pair/src/socket.rs | 9 +++++-- sudo_pair/src/template.rs | 20 ++++++++++++--- 4 files changed, 65 insertions(+), 20 deletions(-) diff --git a/.rustfmt.toml b/.rustfmt.toml index cb3d643..ceec86a 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -11,5 +11,4 @@ wrap_comments = true unstable_features = true -struct_field_align_threshold = 64 -use_small_heuristics = "Max" +struct_field_align_threshold = 64 \ No newline at end of file diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 580ebe5..906cc72 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -115,7 +115,12 @@ impl IoPlugin for SudoPair { slog::debug!(slog, "plugin initializing"); - let args: Vec<_> = env.cmdline.iter().skip(1).map(|arg| arg.to_string_lossy()).collect(); + let args: Vec<_> = env + .cmdline + .iter() + .skip(1) + .map(|arg| arg.to_string_lossy()) + .collect(); slog = slog::Logger::new( &slog, @@ -135,7 +140,12 @@ impl IoPlugin for SudoPair { ); // TODO: convert all outgoing errors to be unauthorized errors - let mut pair = Self { env, options, socket: None, slog }; + let mut pair = Self { + env, + options, + socket: None, + slog, + }; if pair.is_exempt() { slog::info!(pair.slog, "pair session exempt from pairing requirements"); @@ -348,9 +358,15 @@ impl SudoPair { slog::trace!(self.slog, "remote prompt evaluated"); - let mut socket = self.socket.as_ref().ok_or(ErrorKind::CommunicationError)?.borrow_mut(); + let mut socket = self + .socket + .as_ref() + .ok_or(ErrorKind::CommunicationError)? + .borrow_mut(); - socket.write_all(&prompt[..]).context(ErrorKind::CommunicationError)?; + socket + .write_all(&prompt[..]) + .context(ErrorKind::CommunicationError)?; socket.flush().context(ErrorKind::CommunicationError)?; @@ -369,7 +385,9 @@ impl SudoPair { // Ctrl-C and retry the read); we don't need to check the return // value because if the read was successful, we're guaranteed to // have read at least one byte - let _ = socket.read(&mut response).context(ErrorKind::SessionDeclined)?; + let _ = socket + .read(&mut response) + .context(ErrorKind::SessionDeclined)?; slog::debug!(self.slog, "remote pair responded"; "response" => String::from_utf8_lossy(&response[..]).into_owned(), @@ -523,7 +541,10 @@ impl SudoPair { } fn is_sudoing_to_enforced_gid(&self) -> bool { - !self.options.gids_enforced.is_disjoint(&self.env.runas_gids()) + !self + .options + .gids_enforced + .is_disjoint(&self.env.runas_gids()) } fn is_sudoing_to_user(&self) -> bool { @@ -550,9 +571,10 @@ impl SudoPair { // note that we want the *`uid`* and not the `euid` here since // we want to know who the real user is and not the `uid` of the // owner of `sudo` - self.options - .socket_dir - .join(format!("{}.{}.sock", self.env.user_info.uid, self.env.user_info.pid,)) + self.options.socket_dir.join(format!( + "{}.{}.sock", + self.env.user_info.uid, self.env.user_info.pid, + )) } fn socket_uid(&self) -> uid_t { @@ -709,7 +731,10 @@ struct PluginOptions { impl PluginOptions { fn binary_name(&self) -> &[u8] { - self.binary_path.file_name().unwrap_or(self.binary_path.as_os_str()).as_bytes() + self.binary_path + .file_name() + .unwrap_or(self.binary_path.as_os_str()) + .as_bytes() } } @@ -719,7 +744,9 @@ impl PluginOptions { impl<'a> From<&'a OptionMap> for PluginOptions { fn from(map: &'a OptionMap) -> Self { Self { - binary_path: map.get("binary_path").unwrap_or_else(|_| default::BINARY_PATH.into()), + binary_path: map + .get("binary_path") + .unwrap_or_else(|_| default::BINARY_PATH.into()), user_prompt_path: map .get("user_prompt_path") @@ -729,7 +756,9 @@ impl<'a> From<&'a OptionMap> for PluginOptions { .get("pair_prompt_path") .unwrap_or_else(|_| default::PAIR_PROMPT_PATH.into()), - socket_dir: map.get("socket_dir").unwrap_or_else(|_| default::SOCKET_DIR.into()), + socket_dir: map + .get("socket_dir") + .unwrap_or_else(|_| default::SOCKET_DIR.into()), gids_enforced: map .get("gids_enforced") diff --git a/sudo_pair/src/socket.rs b/sudo_pair/src/socket.rs index 988d094..01ca89d 100644 --- a/sudo_pair/src/socket.rs +++ b/sudo_pair/src/socket.rs @@ -98,7 +98,9 @@ impl Socket { } } - listener.accept().map(|connection| Self { socket: connection.0 }) + listener.accept().map(|connection| Self { + socket: connection.0, + }) }); // once the connection has been made (or aborted due to ctrl-c), @@ -163,7 +165,10 @@ impl Socket { if stat.st_mode & libc::S_IFDIR == 0 { return Err(Error::new( ErrorKind::Other, - format!("the socket path {} is not a directory", parent.to_string_lossy(),), + format!( + "the socket path {} is not a directory", + parent.to_string_lossy(), + ), )); } diff --git a/sudo_pair/src/template.rs b/sudo_pair/src/template.rs index b896964..e3859fa 100644 --- a/sudo_pair/src/template.rs +++ b/sudo_pair/src/template.rs @@ -27,7 +27,10 @@ impl Spec { } pub(crate) fn with_escape(escape: u8) -> Self { - Self { escape, ..Self::new() } + Self { + escape, + ..Self::new() + } } pub(crate) fn replace>>(&mut self, literal: u8, replacement: T) { @@ -74,13 +77,19 @@ impl Spec { impl Default for Spec { fn default() -> Self { - Self { expansions: HashMap::new(), escape: DEFAULT_ESCAPE_BYTE } + Self { + expansions: HashMap::new(), + escape: DEFAULT_ESCAPE_BYTE, + } } } impl From>> for Spec { fn from(expansions: HashMap>) -> Self { - Self { expansions, ..Self::new() } + Self { + expansions, + ..Self::new() + } } } @@ -181,7 +190,10 @@ mod tests { spec.replace(b'y', &b"qwerty"[..]); spec.replace(b'n', &b"uiop["[..]); - assert_eq!(b"only y should be expanded qwerty"[..], spec.expand(template)[..],); + assert_eq!( + b"only y should be expanded qwerty"[..], + spec.expand(template)[..], + ); } #[test] From acf07a7ba608541794253c25ebae2e5f8f212abc Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 16:47:47 -0800 Subject: [PATCH 7/8] More manual formatting --- .rustfmt.toml | 2 +- sudo_pair/src/lib.rs | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/.rustfmt.toml b/.rustfmt.toml index ceec86a..acd4547 100644 --- a/.rustfmt.toml +++ b/.rustfmt.toml @@ -11,4 +11,4 @@ wrap_comments = true unstable_features = true -struct_field_align_threshold = 64 \ No newline at end of file +struct_field_align_threshold = 64 diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index 906cc72..f1eb3e4 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -742,29 +742,26 @@ impl PluginOptions { // actually a way to satisfy the linter for the time being #[allow(single_use_lifetimes)] impl<'a> From<&'a OptionMap> for PluginOptions { + #[rustfmt::skip] fn from(map: &'a OptionMap) -> Self { Self { - binary_path: map - .get("binary_path") + binary_path: map.get("binary_path") .unwrap_or_else(|_| default::BINARY_PATH.into()), - user_prompt_path: map - .get("user_prompt_path") + user_prompt_path: map.get("user_prompt_path") .unwrap_or_else(|_| default::USER_PROMPT_PATH.into()), - pair_prompt_path: map - .get("pair_prompt_path") + pair_prompt_path: map.get("pair_prompt_path") .unwrap_or_else(|_| default::PAIR_PROMPT_PATH.into()), - socket_dir: map - .get("socket_dir") + socket_dir: map.get("socket_dir") .unwrap_or_else(|_| default::SOCKET_DIR.into()), - gids_enforced: map - .get("gids_enforced") + gids_enforced: map.get("gids_enforced") .unwrap_or_else(|_| default::GIDS_ENFORCED.iter().copied().collect()), - gids_exempted: map.get("gids_exempted").unwrap_or_default(), + gids_exempted: map.get("gids_exempted") + .unwrap_or_default(), } } } From ad08dbc9fdde5806d094381d73479a46fa11f72a Mon Sep 17 00:00:00 2001 From: John Wood Date: Mon, 13 Dec 2021 16:57:08 -0800 Subject: [PATCH 8/8] More manual formatting --- sudo_pair/src/lib.rs | 10 +++++----- sudo_plugin/src/options/settings.rs | 11 ++++++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/sudo_pair/src/lib.rs b/sudo_pair/src/lib.rs index f1eb3e4..24ea0cb 100644 --- a/sudo_pair/src/lib.rs +++ b/sudo_pair/src/lib.rs @@ -85,11 +85,11 @@ use sudo_plugin::prelude::*; mod default { use libc::gid_t; - pub(crate) const BINARY_PATH: &str = "/usr/bin/sudo_approve"; - pub(crate) const USER_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.user"; - pub(crate) const PAIR_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.pair"; - pub(crate) const SOCKET_DIR: &str = "/var/run/sudo_pair"; - pub(crate) const GIDS_ENFORCED: [gid_t; 1] = [0]; + pub(crate) const BINARY_PATH : &str = "/usr/bin/sudo_approve"; + pub(crate) const USER_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.user"; + pub(crate) const PAIR_PROMPT_PATH: &str = "/etc/sudo_pair.prompt.pair"; + pub(crate) const SOCKET_DIR : &str = "/var/run/sudo_pair"; + pub(crate) const GIDS_ENFORCED : [gid_t; 1] = [0]; pub(crate) const USER_PROMPT: &[u8] = b"%B %u %p\n"; pub(crate) const PAIR_PROMPT: &[u8] = b"%U@%h:%d$ %C\ny/n? [n]: "; diff --git a/sudo_plugin/src/options/settings.rs b/sudo_plugin/src/options/settings.rs index 36ad467..7c2491d 100644 --- a/sudo_plugin/src/options/settings.rs +++ b/sudo_plugin/src/options/settings.rs @@ -289,7 +289,7 @@ impl TryFrom for Settings { login_class: value.get("login_class") .ok(), login_shell: value.get("login_shell") .unwrap_or(false), max_groups: value.get("max_groups") .ok(), - network_addrs: value.get("network_addrs") .unwrap_or_default(), + network_addrs: value.get("network_addrs") .unwrap_or_else(|_| vec![]), noninteractive: value.get("noninteractive") .unwrap_or(false), preserve_environment: value.get("preserve_environment").unwrap_or(false), preserve_groups: value.get("preserve_groups") .unwrap_or(false), @@ -325,12 +325,17 @@ impl FromSudoOption for NetAddr { #[rustfmt::skip] fn from_sudo_option(s: &str) -> ::std::result::Result { let bytes = s.as_bytes(); - let mid = bytes.iter().position(|b| *b == b'/').unwrap_or(bytes.len()); + let mid = bytes.iter() + .position(|b| *b == b'/') + .unwrap_or(bytes.len()); let addr = s[ .. mid].parse()?; let mask = s[mid + 1 .. ].parse()?; - Ok(Self { addr, mask }) + Ok(Self { + addr, + mask + }) } }