Allow groups_exempted and groups_enforced versions of their gids_ cou…

…nterparts

sudo_pair/README.md

@@ -130,15 +130,19 @@ The full list of options are as follows:
 
   This is the path where this plugin will store sockets for sessions that are pending approval. This directory must be owned by root and only writable by root, or the plugin will abort.
 
-* `gids_enforced` (default: `0`)
+* `gids_enforced`, `groups_enforced` (default: `0`)
 
-  This is a comma-separated list of gids that sudo_pair will gate access to. If a user is `sudo`ing to a user that is a member of one of these groups, they will be required to have a pair approve their session.
+  These are comma-separated lists of gids and groupnames, respectively, that sudo_pair will gate acces to. If a user is `sudo`ing to a user that is a member of one of these groups, they will be required to have a pair approve their session. If either of these options are provided, the default will not be used.
 
-* `gids_exempted` (default: none)
+  If a particular groupname cannot be resolved into a gid, it will be silently ignored. If both options are provided, the effective value is their logical union after groupname to gid resolution.
 
-  This is a comma-separated list of gids whose users will be exempted from the requirements of sudo_pair. Note that this is not the opposite of the `gids_enforced` flag. Whereas `gids_enforced` gates access *to* groups, `gids_exempted` exempts users sudoing *from* groups. For instance, this setting can be used to ensure that oncall sysadmins can respond to outages without needing to find a pair.
+* `gids_exempted`, `groups_exempted` (default: none)
 
-  Note that root is *always* exempt.
+  These are comma-separated lists of gids and groupnames, respectively, whose users will be exempted from the requirements of sudo_pair. *Note that this is not the opposite of the `gids_enforced` flag.* Whereas `gids_enforced` gates access *to* groups, `gids_exempted` exempts users sudoing *from* groups. For instance, this setting can be used to ensure that oncall sysadmins can respond to outages without needing to find a pair.
+
+  Note that uid `0` (root) is *always* exempt.
+
+  If a particular groupname cannot be resolved into a gid, it will be silently ignored. If both options are provided, the effective value is their logical union after groupname to gid resolution.
 
 ## Prompts
 

sudo_pair/src/lib.rs

@@ -64,6 +64,7 @@ use crate::template::Spec;
 use crate::socket::Socket;
 
 use std::collections::HashSet;
+use std::ffi::CString;
 use std::fs::File;
 use std::io::{Read, Write};
 use std::os::unix::ffi::OsStrExt;
@@ -608,6 +609,39 @@ impl PluginOptions {
 #[allow(single_use_lifetimes)]
 impl<'a> From<&'a OptionMap> for PluginOptions {
     fn from(map: &'a OptionMap) -> Self {
+        let groups_enforced : HashSet<String> = map.get("groups_enforced")
+            .unwrap_or_default();
+
+        let mut gids_enforced : HashSet<gid_t> = groups_enforced
+            .iter()
+            .filter_map(|groupname| groupname_to_gid(groupname))
+            .chain(map.get("gids_enforced"))
+            .collect();
+
+        // TODO: I feel like this is a bit of a hack, but we want to use the
+        // value of `DEFAULT_GIDS_ENFORCED` if neither `gids_enforced` nor
+        // `groups_enforced` were specified in the plugin options. There's
+        // probably a cleaner way to do this, but at least this approach does
+        // work so we can always clean it up later.
+        if !map.contains_key("gids_enforced") && !map.contains_key("groups_enforced") {
+            // if neither of these is provided, the `gids_enforced` shouldn't
+            // have any elements inside it
+            debug_assert!(gids_enforced.is_empty());
+
+            for gid in &DEFAULT_GIDS_ENFORCED {
+                let _ = gids_enforced.insert(*gid);
+            }
+        }
+
+        let groups_exempted : HashSet<String> = map.get("groups_exempted")
+            .unwrap_or_default();
+
+        let gids_exempted : HashSet<gid_t> = groups_exempted
+            .iter()
+            .filter_map(|groupname| groupname_to_gid(groupname))
+            .chain(map.get("gids_exempted"))
+            .collect();
+
         Self {
             binary_path: map.get("binary_path")
                 .unwrap_or_else(|_| DEFAULT_BINARY_PATH.into()),
@@ -621,11 +655,16 @@ impl<'a> From<&'a OptionMap> for PluginOptions {
             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().cloned().collect()),
-
-            gids_exempted: map.get("gids_exempted")
-                .unwrap_or_default(),
+            gids_enforced,
+            gids_exempted,
         }
     }
 }
+
+/// Converts a POSIX `groupname` into its equivalent `gid` using `getgrnam(3)`.
+/// If the groupname cannot be found, returns `None`.
+fn groupname_to_gid(groupname: &str) -> Option<gid_t> {
+    CString::new(groupname).ok()
+        .and_then(|g| unsafe { libc::getgrnam(g.as_ptr()).as_ref() })
+        .map(|g| g.gr_gid)
+}

sudo_plugin/src/plugin/option_map.rs

@@ -121,6 +121,18 @@ impl OptionMap {
     pub fn get_bytes(&self, k: &[u8]) -> Option<&[u8]> {
         self.0.get(k).map(Vec::as_slice)
     }
+
+    /// Returns whether or not the OptionMap contains the given named
+    /// key.
+    pub fn contains_key(&self, k: &str) -> bool {
+        self.contains_key_bytes(k.as_bytes())
+    }
+
+    /// Returns whether or not the OptionMap contains the given bytes
+    /// as a key.
+    pub fn contains_key_bytes(&self, k: &[u8]) -> bool {
+        self.0.contains_key(k)
+    }
 }
 
 #[cfg(test)]
@@ -131,10 +143,6 @@ mod tests {
     use std::path::PathBuf;
     use std::ptr;
 
-    impl FromSudoOptionList for String {
-        const SEPARATOR: char = '|';
-    }
-
     #[test]
     fn new_parses_string_keys() {
         let map = unsafe { OptionMap::from_raw([
@@ -245,21 +253,20 @@ mod tests {
     fn get_parses_lists() {
         let map = unsafe { OptionMap::from_raw([
             b"ints=1,2,3\0".as_ptr() as _,
-            b"strs=a|b|c\0".as_ptr() as _,
-            b"str=a,b,c\0" .as_ptr() as _,
+            b"strs=a,b,c\0".as_ptr() as _,
+            b"str=a|b|c\0" .as_ptr() as _,
             ptr::null(),
         ].as_ptr()) };
 
         assert_eq!(vec![1, 2, 3],       map.get::<Vec<u8>>("ints")    .unwrap());
         assert_eq!(vec!["a", "b", "c"], map.get::<Vec<String>>("strs").unwrap());
-        assert_eq!(vec!["a,b,c"],       map.get::<Vec<String>>("str") .unwrap());
+        assert_eq!(vec!["a|b|c"],       map.get::<Vec<String>>("str") .unwrap());
     }
 
     #[test]
     fn get_parses_hashsets() {
-
         let map = unsafe { OptionMap::from_raw([
-            b"nums=1|2|3\0".as_ptr() as _,
+            b"nums=1,2,3\0".as_ptr() as _,
             ptr::null(),
         ].as_ptr()) };
 

sudo_plugin/src/plugin/traits.rs

@@ -164,3 +164,4 @@ impl FromSudoOptionList for u32 {}
 impl FromSudoOptionList for i64 {}
 impl FromSudoOptionList for u64 {}
 impl FromSudoOptionList for PathBuf {}
+impl FromSudoOptionList for String {}