Allow groups_exempted, groups_enforced versions of their gids_ counterparts

sudo_pair/CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Added
+- New `groups_exempted` and `groups_enforced` alternatives to `gids_exempted` and `gids_enforced`.
+
 ### Fixed
 - The `-u` and `-g` flags can now both be used in exempt sessions.
 

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;
@@ -406,13 +407,13 @@ impl SudoPair {
     }
 
     fn is_sudoing_from_exempted_gid(&self) -> bool {
-        !self.options.gids_exempted.is_disjoint(
+        !self.options.gids_exempted().is_disjoint(
             &self.plugin.user_info.groups.iter().cloned().collect()
         )
     }
 
     fn is_sudoing_to_enforced_gid(&self) -> bool {
-        !self.options.gids_enforced.is_disjoint(
+        !self.options.gids_enforced().is_disjoint(
             &self.plugin.runas_gids()
         )
     }
@@ -581,7 +582,8 @@ struct PluginOptions {
     /// pair approve their session.
     ///
     /// Default: `[0]` (e.g., root)
-    gids_enforced: HashSet<gid_t>,
+    gids_enforced:   HashSet<gid_t>,
+    groups_enforced: HashSet<String>,
 
     /// `gids_exempted` is a comma-separated list of gids whose users
     /// will be exempted from the requirements of sudo_pair. Note that
@@ -592,7 +594,8 @@ struct PluginOptions {
     /// outages without needing to find a pair.
     ///
     /// Default: `[]` (however, root is *always* exempt)
-    gids_exempted: HashSet<gid_t>,
+    gids_exempted:   HashSet<gid_t>,
+    groups_exempted: HashSet<String>,
 }
 
 impl PluginOptions {
@@ -601,6 +604,28 @@ impl PluginOptions {
             self.binary_path.as_os_str()
         ).as_bytes()
     }
+
+    fn gids_enforced(&self) -> HashSet<gid_t> {
+        let gids_enforced : HashSet<gid_t> = self.groups_enforced
+            .iter()
+            .filter_map(|groupname| groupname_to_gid(groupname))
+            .chain(self.gids_enforced.clone())
+            .collect();
+
+        if gids_enforced.is_empty() {
+            return DEFAULT_GIDS_ENFORCED.iter().cloned().collect()
+        }
+
+        gids_enforced
+    }
+
+    fn gids_exempted(&self) -> HashSet<gid_t> {
+        self.groups_exempted
+            .iter()
+            .filter_map(|groupname| groupname_to_gid(groupname))
+            .chain(self.gids_exempted.clone())
+            .collect()
+    }
 }
 
 // TODO: single_use_lifetimes was committed, but I'm not sure there's
@@ -622,10 +647,24 @@ impl<'a> From<&'a OptionMap> for PluginOptions {
                 .unwrap_or_else(|_| DEFAULT_SOCKET_DIR.into()),
 
             gids_enforced: map.get("gids_enforced")
-                .unwrap_or_else(|_| DEFAULT_GIDS_ENFORCED.iter().cloned().collect()),
+                .unwrap_or_default(),
+
+            groups_enforced: map.get("groups_enforced")
+                .unwrap_or_default(),
 
             gids_exempted: map.get("gids_exempted")
                 .unwrap_or_default(),
+
+            groups_exempted: map.get("groups_exempted")
+                .unwrap_or_default(),
         }
     }
 }
+
+/// 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-sys/CHANGELOG.md

@@ -10,8 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Changed
 - Builds using Rust 2018
 - No longer fails to build on warnings, unless being run in CI
-- Bindgen-generated bindings are committed directly so we can remove
-  bindgen from the list of build dependencies
+- Bindgen-generated bindings are committed directly so we can remove bindgen from the list of build dependencies
 
 ## [1.1.0] - 2018-05-18
 

sudo_plugin/CHANGELOG.md

@@ -14,8 +14,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Changed
 - Builds using Rust 2018
 - No longer fails to build on warnings, unless being run in CI
-- Allows plugins to use any error library they wish, as long as the error
-  types returned in `Result`s implement `Into<sudo_plugin::errors::Error>`.
+- Allows plugins to use any error library they wish, as long as the error types returned in `Result`s implement `Into<sudo_plugin::errors::Error>`.
 
 ## [1.1.0] - 2018-05-18
 

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 {}