Merging upstream version 1.72.1+dfsg1.

Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>

6 files changed, 607 insertions, 319 deletions

diff --git a/vendor/redox_users/.cargo-checksum.json b/vendor/redox_users/.cargo-checksum.json
index 9898804be..4a85c2e99 100644
--- a/vendor/redox_users/.cargo-checksum.json
+++ b/vendor/redox_users/.cargo-checksum.json
@@ -1 +1 @@
-{"files":{"Cargo.toml":"2924323f7d907818842bd62777c75b0f8055cebd6cd0e5d26909d4a536d5a946","LICENSE":"e6a8ae2d796083783efc94b1e66271aa2929dc4dfb231d34239aa9c7db8396db","README.md":"9c3a6fd2a798bd1e105c5ee72f7b475471e3a9abec89b5be2b5263d654085d22","src/lib.rs":"8e6e1a2bffc3f63c2839b6a41ddeaa21da3134f6c937f16d6eacddad111b7fe2","tests/etc/group":"175d89e8d03e2976d3f37110050372163fcafd07a20c899ade3a9690b2cb4526","tests/etc/passwd":"aee0d4bd2abf55846683cc2e5daaa03641636a262519623d59b1ab8e1eb1db32","tests/etc/shadow":"ca7c1a6f96eaef3bd26da4faeae55e78cad04822c191e3263b539ee687de4d0a"},"package":"528532f3d801c87aec9def2add9ca802fe569e44a544afe633765267840abe64"}
\ No newline at end of file
+{"files":{"Cargo.toml":"330ea31605d1c46fdad019ad5020a3650b8997e58eb7104bda47229d4635ecaf","LICENSE":"e6a8ae2d796083783efc94b1e66271aa2929dc4dfb231d34239aa9c7db8396db","README.md":"9c3a6fd2a798bd1e105c5ee72f7b475471e3a9abec89b5be2b5263d654085d22","src/lib.rs":"e1bba5f2359ea6bdc5909c3fa0569a7fa08f7427dd63b65943cdd426b1f89dcb","tests/etc/group":"af4c6ad49fd74135c4c040087d2a8a89564c1a8e9fb79644fcc24aedd0e77552","tests/etc/passwd":"3728274286c9e78a8a3f17c0973930eb112e9cc7959de2310510ba0d6b60bf49","tests/etc/shadow":"aecc1cba1a7ab65612abc049e2ea32c99401bacefb1259e23f75c004fb75896d"},"package":"b033d837a7cf162d7993aded9304e30a83213c648b6e389db233191f891e5c2b"}
\ No newline at end of file
diff --git a/vendor/redox_users/Cargo.toml b/vendor/redox_users/Cargo.toml
index 5a21c8fa6..3ac606b01 100644
--- a/vendor/redox_users/Cargo.toml
+++ b/vendor/redox_users/Cargo.toml
@@ -3,17 +3,16 @@
 # When uploading crates to the registry Cargo will automatically
 # "normalize" Cargo.toml files for maximal compatibility
 # with all versions of Cargo and also rewrite `path` dependencies
-# to registry (e.g., crates.io) dependencies
+# to registry (e.g., crates.io) dependencies.
 #
-# If you believe there's an error in this file please file an
-# issue against the rust-lang/cargo repository. If you're
-# editing this file be aware that the upstream Cargo.toml
-# will likely look very different (and much more reasonable)
+# If you are reading this file be aware that the original Cargo.toml
+# will likely look very different (and much more reasonable).
+# See Cargo.toml.orig for the original contents.
 
 [package]
 edition = "2018"
 name = "redox_users"
-version = "0.4.0"
+version = "0.4.3"
 authors = ["Jose Narvaez <goyox86@gmail.com>", "Wesley Hershberger <mggmugginsmc@gmail.com>"]
 description = "A Rust library to access Redox users and groups functionality"
 documentation = "https://docs.rs/redox_users"
@@ -26,12 +25,20 @@ version = "0.2"
 features = ["std"]
 
 [dependencies.redox_syscall]
-version = "0.2"
+version = "0.2.11"
 
 [dependencies.rust-argon2]
 version = "0.8"
 optional = true
 
+[dependencies.thiserror]
+version = "1.0"
+
+[dependencies.zeroize]
+version = "1.4"
+features = ["zeroize_derive"]
+optional = true
+
 [features]
-auth = ["rust-argon2"]
+auth = ["rust-argon2", "zeroize"]
 default = ["auth"]
diff --git a/vendor/redox_users/src/lib.rs b/vendor/redox_users/src/lib.rs
index c64a9d6b4..04e055bc1 100644
--- a/vendor/redox_users/src/lib.rs
+++ b/vendor/redox_users/src/lib.rs
@@ -28,11 +28,9 @@
 //! software.
 
 use std::convert::From;
-use std::error::Error;
-use std::fmt::{self, Debug};
+use std::fmt::Debug;
 use std::fs::{File, OpenOptions};
 use std::io::{Read, Seek, SeekFrom, Write};
-use std::marker::PhantomData;
 #[cfg(target_os = "redox")]
 use std::os::unix::fs::OpenOptionsExt;
 #[cfg(not(target_os = "redox"))]
@@ -46,12 +44,15 @@ use std::slice::{Iter, IterMut};
 use std::thread;
 use std::time::Duration;
 
+use thiserror::Error;
+#[cfg(feature = "auth")]
+use zeroize::Zeroize;
+
 //#[cfg(not(target_os = "redox"))]
 //use nix::fcntl::{flock, FlockArg};
 
 #[cfg(target_os = "redox")]
 use syscall::flag::{O_EXLOCK, O_SHLOCK};
-use syscall::Error as SyscallError;
 
 const PASSWD_FILE: &'static str = "/etc/passwd";
 const GROUP_FILE: &'static str = "/etc/group";
@@ -67,70 +68,80 @@ const MIN_ID: usize = 1000;
 const MAX_ID: usize = 6000;
 const DEFAULT_TIMEOUT: u64 = 3;
 
-#[cfg(feature = "auth")]
-const USER_AUTH_FULL_EXPECTED_HASH: &str = "A User<auth::Full> had no hash";
-
-pub type Result<T> = std::result::Result<T, Box<dyn Error + Send + Sync>>;
+const USERNAME_LEN_MIN: usize = 3;
+const USERNAME_LEN_MAX: usize = 32;
 
 /// Errors that might happen while using this crate
-#[derive(Debug, PartialEq)]
-pub enum UsersError {
-    Os { reason: String },
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum Error {
+    #[error("os error: {reason}")]
+    Os { reason: &'static str },
+
+    #[error(transparent)]
+    Io(#[from] std::io::Error),
+
+    #[error("failed to generate seed: {0}")]
+    Getrandom(#[from] getrandom::Error),
+
+    #[cfg(feature = "auth")]
+    #[error("")]
+    Argon(#[from] argon2::Error),
+
+    #[error("parse error line {line}: {reason}")]
     Parsing { reason: String, line: usize },
-    NotFound,
-    AlreadyExists
-}
 
-impl fmt::Display for UsersError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            UsersError::Os { reason } => write!(f, "os error: code {}", reason),
-            UsersError::Parsing { reason, line } => {
-                write!(f, "parse error line {}: {}", line, reason)
-            },
-            UsersError::NotFound => write!(f, "user/group not found"),
-            UsersError::AlreadyExists => write!(f, "user/group already exists")
-        }
-    }
-}
+    #[error(transparent)]
+    ParseInt(#[from] std::num::ParseIntError),
 
-impl Error for UsersError {
-    fn description(&self) -> &str { "UsersError" }
+    #[error("user not found")]
+    UserNotFound,
 
-    fn cause(&self) -> Option<&dyn Error> { None }
+    #[error("group not found")]
+    GroupNotFound,
+
+    #[error("user already exists")]
+    UserAlreadyExists,
+
+    #[error("group already exists")]
+    GroupAlreadyExists,
+
+    #[error("invalid name '{name}'")]
+    InvalidName { name: String },
+
+    /// Used for invalid string field values of [`User`]
+    #[error("invalid entry element '{data}'")]
+    InvalidData { data: String },
 }
 
 #[inline]
-fn parse_error(line: usize, reason: &str) -> UsersError {
-    UsersError::Parsing {
+fn parse_error(line: usize, reason: &str) -> Error {
+    Error::Parsing {
         reason: reason.into(),
         line,
     }
 }
 
-#[inline]
-fn os_error(reason: &str) -> UsersError {
-    UsersError::Os {
-        reason: reason.into()
-    }
-}
-
-impl From<SyscallError> for UsersError {
-    fn from(syscall_error: SyscallError) -> UsersError {
-        UsersError::Os {
-            reason: format!("{}", syscall_error)
-        }
+impl From<syscall::Error> for Error {
+    fn from(syscall_error: syscall::Error) -> Error {
+        Error::Os { reason: syscall_error.text() }
     }
 }
 
-#[derive(Clone, Copy)]
-#[allow(dead_code)]
+#[derive(Clone, Copy, Debug)]
 enum Lock {
     Shared,
     Exclusive,
 }
 
 impl Lock {
+    fn can_write(&self) -> bool {
+        match self {
+            Lock::Shared => false,
+            Lock::Exclusive => true,
+        }
+    }
+
     #[cfg(target_os = "redox")]
     fn as_olock(self) -> i32 {
         (match self {
@@ -138,7 +149,7 @@ impl Lock {
             Lock::Exclusive => O_EXLOCK,
         }) as i32
     }
-    
+
     /*#[cfg(not(target_os = "redox"))]
     fn as_flock(self) -> FlockArg {
         match self {
@@ -150,7 +161,7 @@ impl Lock {
 
 /// Naive semi-cross platform file locking (need to support linux for tests).
 #[allow(dead_code)]
-fn locked_file(file: impl AsRef<Path>, _lock: Lock) -> Result<File> {
+fn locked_file(file: impl AsRef<Path>, lock: Lock) -> Result<File, Error> {
     #[cfg(test)]
     println!("Open file: {}", file.as_ref().display());
 
@@ -158,8 +169,8 @@ fn locked_file(file: impl AsRef<Path>, _lock: Lock) -> Result<File> {
     {
         Ok(OpenOptions::new()
             .read(true)
-            .write(true)
-            .custom_flags(_lock.as_olock())
+            .write(lock.can_write())
+            .custom_flags(lock.as_olock())
             .open(file)?)
     }
     #[cfg(not(target_os = "redox"))]
@@ -167,34 +178,213 @@ fn locked_file(file: impl AsRef<Path>, _lock: Lock) -> Result<File> {
     {
         let file = OpenOptions::new()
             .read(true)
-            .write(true)
+            .write(lock.can_write())
             .open(file)?;
         let fd = file.as_raw_fd();
         eprintln!("Fd: {}", fd);
-        //flock(fd, _lock.as_flock())?;
+        //flock(fd, lock.as_flock())?;
         Ok(file)
     }
 }
 
 /// Reset a file for rewriting (user/group dbs must be erased before write-out)
-fn reset_file(fd: &mut File) -> Result<()> {
+fn reset_file(fd: &mut File) -> Result<(), Error> {
     fd.set_len(0)?;
     fd.seek(SeekFrom::Start(0))?;
     Ok(())
 }
 
+/// Is a string safe to write to `/etc/group` or `/etc/passwd`?
+fn is_safe_string(s: &str) -> bool {
+    !s.contains(';')
+}
+
+const PORTABLE_FILE_NAME_CHARS: &str =
+    "0123456789._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
+
+/// This function is used by [`UserBuilder`] and [`GroupBuilder`] to determine
+/// if a name for a user/group is valid. It is provided for convenience.
+///
+/// Usernames must match the [POSIX standard
+/// for usernames](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_437)
+/// . The "portable filename character set" is defined as `A-Z`, `a-z`, `0-9`,
+/// and `._-` (see [here](https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_282)).
+///
+/// Usernames may not be more than 32 or less than 3 characters in length.
+pub fn is_valid_name(name: &str) -> bool {
+    if name.len() < USERNAME_LEN_MIN || name.len() > USERNAME_LEN_MAX {
+        false
+    } else if let Some(first) = name.chars().next() {
+        first != '-' &&
+            name.chars().all(|c| {
+                PORTABLE_FILE_NAME_CHARS.contains(c)
+            })
+    } else {
+        false
+    }
+}
+
 /// Marker types for [`User`] and [`AllUsers`].
 pub mod auth {
+    #[cfg(feature = "auth")]
+    use std::fmt;
+
+    #[cfg(feature = "auth")]
+    use zeroize::Zeroize;
+
+    #[cfg(feature = "auth")]
+    use crate::Error;
+
     /// Marker type indicating that a `User` only has access to world-readable
     /// user information, and cannot authenticate.
-    #[derive(Debug)]
+    #[derive(Debug, Default)]
     pub struct Basic {}
-    
+
     /// Marker type indicating that a `User` has access to all user
     /// information, including password hashes.
     #[cfg(feature = "auth")]
-    #[derive(Debug)]
-    pub struct Full {}
+    #[derive(Default, Zeroize)]
+    #[zeroize(drop)]
+    pub struct Full {
+        pub(crate) hash: String,
+    }
+
+    #[cfg(feature = "auth")]
+    impl Full {
+        pub(crate) fn empty() -> Full {
+            Full { hash: "".into() }
+        }
+
+        pub(crate) fn is_empty(&self) -> bool {
+            &self.hash == ""
+        }
+
+        pub(crate) fn unset() -> Full {
+            Full { hash: "!".into() }
+        }
+
+        pub(crate) fn is_unset(&self) -> bool {
+            &self.hash == "!"
+        }
+
+        pub(crate) fn passwd(pw: &str) -> Result<Full, Error> {
+            Ok(if pw != "" {
+                let mut buf = [0u8; 8];
+                getrandom::getrandom(&mut buf)?;
+                let mut salt = format!("{:X}", u64::from_ne_bytes(buf));
+
+                let config = argon2::Config::default();
+                let hash: String = argon2::hash_encoded(
+                    pw.as_bytes(),
+                    salt.as_bytes(),
+                    &config
+                )?;
+
+                buf.zeroize();
+                salt.zeroize();
+                Full { hash } // note that move == shallow copy in Rust
+            } else {
+                Full::empty()
+            })
+        }
+
+        pub(crate) fn verify(&self, pw: &str) -> bool {
+            match self.hash.as_str() {
+                "" => pw == "",
+                "!" => false,
+                //TODO: When does this panic? Should this function return
+                // Result? Or does it need to simply fail to verify if
+                // verify_encoded() fails?
+                hash => argon2::verify_encoded(&hash, pw.as_bytes())
+                    .expect("failed to verify hash"),
+            }
+        }
+    }
+
+    #[cfg(feature = "auth")]
+    impl fmt::Debug for Full {
+        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+            f.debug_struct("Full")
+                .finish()
+        }
+    }
+}
+
+/// A builder pattern for adding [`User`]s to [`AllUsers`]. Fields are verified
+/// when the group is built via [`AllUsers::add_user`]. See the documentation
+/// of that function for default values.
+///
+/// Note that this builder is not available when the `auth` feature of the
+/// crate is disabled.
+///
+/// # Example
+/// ```no_run
+/// # use redox_users::{AllGroups, Config, GroupBuilder, UserBuilder};
+/// let mut allgs = AllGroups::new(Config::default()).unwrap();
+///
+/// let g = GroupBuilder::new("foobar")
+///     .user("foobar");
+/// let foobar_g = allgs.add_group(g).unwrap();
+///
+/// let u = UserBuilder::new("foobar")
+///     .gid(foobar_g.gid)
+///     .name("Foo Bar")
+///     // Note that this directory will not be created
+///     .home("file:/home/foobar");
+/// ```
+#[cfg(feature = "auth")]
+pub struct UserBuilder {
+    user: String,
+    uid: Option<usize>,
+    gid: Option<usize>,
+    name: Option<String>,
+    home: Option<String>,
+    shell: Option<String>,
+}
+
+#[cfg(feature = "auth")]
+impl UserBuilder {
+    /// Create a new `UserBuilder` with the login name for the new user.
+    pub fn new(user: impl AsRef<str>) -> UserBuilder {
+        UserBuilder {
+            user: user.as_ref().to_string(),
+            uid: None,
+            gid: None,
+            name: None,
+            home: None,
+            shell: None,
+        }
+    }
+
+    /// Set the user id for this user.
+    pub fn uid(mut self, uid: usize) -> UserBuilder {
+        self.uid = Some(uid);
+        self
+    }
+
+    /// Set the primary group id for this user.
+    pub fn gid(mut self, gid: usize) -> UserBuilder {
+        self.gid = Some(gid);
+        self
+    }
+
+    /// Set the GECOS field for this user.
+    pub fn name(mut self, name: impl AsRef<str>) -> UserBuilder {
+        self.name = Some(name.as_ref().to_string());
+        self
+    }
+
+    /// Set the home directory for this user.
+    pub fn home(mut self, home: impl AsRef<str>) -> UserBuilder {
+        self.home = Some(home.as_ref().to_string());
+        self
+    }
+
+    /// Set the login shell for this user.
+    pub fn shell(mut self, shell: impl AsRef<str>) -> UserBuilder {
+        self.shell = Some(shell.as_ref().to_string());
+        self
+    }
 }
 
 /// A struct representing a Redox user.
@@ -214,6 +404,7 @@ pub mod auth {
 /// most commonly used hash for an unset password is `"!"`, but
 /// this crate makes no distinction. The most common way to unset
 /// the password is to use [`User::unset_passwd`].
+#[derive(Debug)]
 pub struct User<A> {
     /// Username (login name)
     pub user: String,
@@ -227,17 +418,15 @@ pub struct User<A> {
     pub home: String,
     /// Shell path
     pub shell: String,
-    
-    // Stored password hash text and an indicator to determine if the text is a
-    // hash.
-    #[cfg(feature = "auth")]
-    hash: Option<(String, bool)>,
+
     // Failed login delay duration
     auth_delay: Duration,
-    auth: PhantomData<A>,
+
+    #[allow(dead_code)]
+    auth: A,
 }
 
-impl<A> User<A> {
+impl<A: Default> User<A> {
     /// Get a Command to run the user's default shell (see [`User::login_cmd`]
     /// for more docs).
     pub fn shell_cmd(&self) -> Command { self.login_cmd(&self.shell) }
@@ -270,8 +459,8 @@ impl<A> User<A> {
             .env("SHELL", &self.shell);
         command
     }
-    
-    fn from_passwd_entry(s: &str, line: usize) -> Result<Self> {
+
+    fn from_passwd_entry(s: &str, line: usize) -> Result<User<A>, Error> {
         let mut parts = s.split(';');
 
         let user = parts
@@ -302,52 +491,29 @@ impl<A> User<A> {
             name: name.into(),
             home: home.into(),
             shell: shell.into(),
-            #[cfg(feature = "auth")]
-            hash: None,
-            auth: PhantomData,
+            auth: A::default(),
             auth_delay: Duration::default(),
         })
     }
-
-    /// Format this user as an entry in `/etc/passwd`.
-    fn passwd_entry(&self) -> String {
-        #[cfg_attr(rustfmt, rustfmt_skip)]
-        format!("{};{};{};{};{};{}\n",
-            self.user, self.uid, self.gid, self.name, self.home, self.shell
-        )
-    }
 }
 
-/// Additional methods for if this `User` is authenticatable.
 #[cfg(feature = "auth")]
 impl User<auth::Full> {
     /// Set the password for a user. Make **sure** that `password`
     /// is actually what the user wants as their password (this doesn't).
     ///
     /// To set the password blank, pass `""` as `password`.
-    pub fn set_passwd(&mut self, password: impl AsRef<str>) -> Result<()> {
-        let password = password.as_ref();
-
-        self.hash = if password != "" {
-            let mut buf = [0u8; 8];
-            getrandom::getrandom(&mut buf)?;
-            let salt = format!("{:X}", u64::from_ne_bytes(buf));
-            let config = argon2::Config::default();
-            let hash = argon2::hash_encoded(
-                password.as_bytes(),
-                salt.as_bytes(),
-                &config
-            )?;
-            Some((hash, true))
-        } else {
-            Some(("".into(), false))
-        };
+    ///
+    /// Note that `password` is taken as a reference, so it is up to the caller
+    /// to properly zero sensitive memory (see `zeroize` on crates.io).
+    pub fn set_passwd(&mut self, password: impl AsRef<str>) -> Result<(), Error> {
+        self.auth = auth::Full::passwd(password.as_ref())?;
         Ok(())
     }
 
     /// Unset the password ([`User::verify_passwd`] always returns `false`).
     pub fn unset_passwd(&mut self) {
-        self.hash = Some(("!".into(), false));
+        self.auth = auth::Full::unset();
     }
 
     /// Verify the password. If the hash is empty, this only returns `true` if
@@ -355,17 +521,11 @@ impl User<auth::Full> {
     ///
     /// Note that this is a blocking operation if the password is incorrect.
     /// See [`Config::auth_delay`] to set the wait time. Default is 3 seconds.
+    ///
+    /// Note that `password` is taken as a reference, so it is up to the caller
+    /// to properly zero sensitive memory (see `zeroize` on crates.io).
     pub fn verify_passwd(&self, password: impl AsRef<str>) -> bool {
-        let &(ref hash, ref encoded) = self.hash.as_ref()
-            .expect(USER_AUTH_FULL_EXPECTED_HASH);
-        let password = password.as_ref();
-
-        let verified = if *encoded {
-            argon2::verify_encoded(&hash, password.as_bytes()).unwrap()
-        } else {
-            hash == "" && password == ""
-        };
-
+        let verified = self.auth.verify(password.as_ref());
         if !verified {
             #[cfg(not(test))] // Make tests run faster
             thread::sleep(self.auth_delay);
@@ -376,36 +536,39 @@ impl User<auth::Full> {
     /// Determine if the hash for the password is blank ([`User::verify_passwd`]
     /// returns `true` *only* when the password is blank).
     pub fn is_passwd_blank(&self) -> bool {
-        let &(ref hash, ref encoded) = self.hash.as_ref()
-            .expect(USER_AUTH_FULL_EXPECTED_HASH);
-        hash == "" && ! encoded
+        self.auth.is_empty()
     }
 
     /// Determine if the hash for the password is unset
     /// ([`User::verify_passwd`] returns `false` regardless of input).
     pub fn is_passwd_unset(&self) -> bool {
-        let &(ref hash, ref encoded) = self.hash.as_ref()
-            .expect(USER_AUTH_FULL_EXPECTED_HASH);
-        hash != "" && ! encoded
+        self.auth.is_unset()
     }
 
-    fn shadow_entry(&self) -> String {
-        let hashstring = match self.hash {
-            Some((ref hash, _)) => hash,
-            None => panic!(USER_AUTH_FULL_EXPECTED_HASH)
-        };
-        format!("{};{}\n", self.user, hashstring)
+    /// Format this user as an entry in `/etc/passwd`.
+    fn passwd_entry(&self) -> Result<String, Error> {
+        if !is_safe_string(&self.user) {
+            Err(Error::InvalidName { name: self.user.to_string() })
+        } else if !is_safe_string(&self.name) {
+            Err(Error::InvalidData { data: self.name.to_string() })
+        } else if !is_safe_string(&self.home) {
+            Err(Error::InvalidData { data: self.home.to_string() })
+        } else if !is_safe_string(&self.shell) {
+            Err(Error::InvalidData { data: self.shell.to_string() })
+        } else {
+            #[cfg_attr(rustfmt, rustfmt_skip)]
+            Ok(format!("{};{};{};{};{};{}\n",
+                self.user, self.uid, self.gid, self.name, self.home, self.shell
+            ))
+        }
     }
 
-    /// Give this a hash string (not a shadowfile entry!!!)
-    fn populate_hash(&mut self, hash: &str) -> Result<()> {
-        let encoded = match hash {
-            "" => false,
-            "!" => false,
-            _ => true,
-        };
-        self.hash = Some((hash.to_string(), encoded));
-        Ok(())
+    fn shadow_entry(&self) -> Result<String, Error> {
+        if !is_safe_string(&self.user) {
+            Err(Error::InvalidName { name: self.user.to_string() })
+        } else {
+            Ok(format!("{};{}\n", self.user, self.auth.hash))
+        }
     }
 }
 
@@ -421,17 +584,50 @@ impl<A> Id for User<A> {
     }
 }
 
-impl<A> Debug for User<A> {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("User")
-            .field("user", &self.user)
-            .field("uid", &self.uid)
-            .field("gid", &self.gid)
-            .field("name", &self.name)
-            .field("home", &self.home)
-            .field("shell", &self.shell)
-            .field("auth_delay", &self.auth_delay)
-            .finish()
+/// A builder pattern for adding [`Group`]s to [`AllGroups`]. Fields are
+/// verified when the `Group` is built, via [`AllGroups::add_group`].
+///
+/// # Example
+/// ```
+/// # use redox_users::GroupBuilder;
+/// // When added, this group will use the first available group id
+/// let mygroup = GroupBuilder::new("group_name");
+///
+/// // A little more stuff:
+/// let other = GroupBuilder::new("special")
+///     .gid(9055)
+///     .user("some_username");
+/// ```
+pub struct GroupBuilder {
+    // Group name
+    group: String,
+
+    gid: Option<usize>,
+
+    users: Vec<String>,
+}
+
+impl GroupBuilder {
+    /// Create a new `GroupBuilder` with the given group name.
+    pub fn new(group: impl AsRef<str>) -> GroupBuilder {
+        GroupBuilder {
+            group: group.as_ref().to_string(),
+            gid: None,
+            users: vec![],
+        }
+    }
+
+    /// Set the group id of this group.
+    pub fn gid(mut self, gid: usize) -> GroupBuilder {
+        self.gid = Some(gid);
+        self
+    }
+
+    /// Add a user to this group. Call this function multiple times to add more
+    /// users.
+    pub fn user(mut self, user: impl AsRef<str>) -> GroupBuilder {
+        self.users.push(user.as_ref().to_string());
+        self
     }
 }
 
@@ -448,7 +644,7 @@ pub struct Group {
 }
 
 impl Group {
-    fn from_group_entry(s: &str, line: usize) -> Result<Self> {
+    fn from_group_entry(s: &str, line: usize) -> Result<Group, Error> {
         let mut parts = s.trim()
             .split(';');
 
@@ -476,16 +672,23 @@ impl Group {
         })
     }
 
-    /// Format this group as an entry in `/etc/group`. This
-    /// is an implementation detail, do NOT rely on this trait
-    /// being implemented in future.
-    fn group_entry(&self) -> String {
-        #[cfg_attr(rustfmt, rustfmt_skip)]
-        format!("{};{};{}\n",
-            self.group,
-            self.gid,
-            self.users.join(",").trim_matches(',')
-        )
+    fn group_entry(&self) -> Result<String, Error> {
+        if !is_safe_string(&self.group) {
+            Err(Error::InvalidName { name: self.group.to_string() })
+        } else {
+            for username in self.users.iter() {
+                if !is_safe_string(&username) {
+                    return Err(Error::InvalidData { data: username.to_string() });
+                }
+            }
+
+            #[cfg_attr(rustfmt, rustfmt_skip)]
+            Ok(format!("{};{};{}\n",
+                self.group,
+                self.gid,
+                self.users.join(",").trim_matches(',')
+            ))
+        }
     }
 }
 
@@ -514,11 +717,9 @@ impl Id for Group {
 /// # use redox_users::get_euid;
 /// let euid = get_euid().unwrap();
 /// ```
-pub fn get_euid() -> Result<usize> {
-    match syscall::geteuid() {
-        Ok(euid) => Ok(euid),
-        Err(syscall_error) => Err(From::from(os_error(syscall_error.text())))
-    }
+pub fn get_euid() -> Result<usize, Error> {
+    syscall::geteuid()
+        .map_err(From::from)
 }
 
 /// Gets the current process real user ID.
@@ -534,11 +735,9 @@ pub fn get_euid() -> Result<usize> {
 /// # use redox_users::get_uid;
 /// let uid = get_uid().unwrap();
 /// ```
-pub fn get_uid() -> Result<usize> {
-    match syscall::getuid() {
-        Ok(uid) => Ok(uid),
-        Err(syscall_error) => Err(From::from(os_error(syscall_error.text())))
-    }
+pub fn get_uid() -> Result<usize, Error> {
+    syscall::getuid()
+        .map_err(From::from)
 }
 
 /// Gets the current process effective group ID.
@@ -554,11 +753,9 @@ pub fn get_uid() -> Result<usize> {
 /// # use redox_users::get_egid;
 /// let egid = get_egid().unwrap();
 /// ```
-pub fn get_egid() -> Result<usize> {
-    match syscall::getegid() {
-        Ok(egid) => Ok(egid),
-        Err(syscall_error) => Err(From::from(os_error(syscall_error.text())))
-    }
+pub fn get_egid() -> Result<usize, Error> {
+    syscall::getegid()
+        .map_err(From::from)
 }
 
 /// Gets the current process real group ID.
@@ -574,11 +771,9 @@ pub fn get_egid() -> Result<usize> {
 /// # use redox_users::get_gid;
 /// let gid = get_gid().unwrap();
 /// ```
-pub fn get_gid() -> Result<usize> {
-    match syscall::getgid() {
-        Ok(gid) => Ok(gid),
-        Err(syscall_error) => Err(From::from(os_error(syscall_error.text())))
-    }
+pub fn get_gid() -> Result<usize, Error> {
+    syscall::getgid()
+        .map_err(From::from)
 }
 
 /// A generic configuration that allows fine control of an [`AllUsers`] or
@@ -606,6 +801,7 @@ pub struct Config {
     auth_delay: Duration,
     min_id: usize,
     max_id: usize,
+    lock: Lock,
 }
 
 impl Config {
@@ -636,6 +832,16 @@ impl Config {
         self
     }
 
+    /// Allow writes to group, passwd, and shadow files
+    pub fn writeable(mut self, writeable: bool) -> Config {
+        self.lock = if writeable {
+            Lock::Exclusive
+        } else {
+            Lock::Shared
+        };
+        self
+    }
+
     // Prepend a path with the scheme in this Config
     fn in_scheme(&self, path: impl AsRef<Path>) -> PathBuf {
         let mut canonical_path = PathBuf::from(&self.scheme);
@@ -662,6 +868,7 @@ impl Default for Config {
             auth_delay: Duration::new(DEFAULT_TIMEOUT, 0),
             min_id: MIN_ID,
             max_id: MAX_ID,
+            lock: Lock::Shared,
         }
     }
 }
@@ -826,15 +1033,17 @@ pub trait All: AllInner {
 pub struct AllUsers<A> {
     users: Vec<User<A>>,
     config: Config,
-    
+
     // Hold on to the locked fds to prevent race conditions
+    #[allow(dead_code)]
     passwd_fd: File,
+    #[allow(dead_code)]
     shadow_fd: Option<File>,
 }
 
-impl<A> AllUsers<A> {
-    fn new(config: Config) -> Result<AllUsers<A>> {
-        let mut passwd_fd = locked_file(config.in_scheme(PASSWD_FILE), Lock::Exclusive)?;
+impl<A: Default> AllUsers<A> {
+    pub fn new(config: Config) -> Result<AllUsers<A>, Error> {
+        let mut passwd_fd = locked_file(config.in_scheme(PASSWD_FILE), config.lock)?;
         let mut passwd_cntnt = String::new();
         passwd_fd.read_to_string(&mut passwd_cntnt)?;
 
@@ -844,7 +1053,7 @@ impl<A> AllUsers<A> {
             user.auth_delay = config.auth_delay;
             passwd_entries.push(user);
         }
-        
+
         Ok(AllUsers::<A> {
             users: passwd_entries,
             config,
@@ -857,7 +1066,7 @@ impl<A> AllUsers<A> {
 impl AllUsers<auth::Basic> {
     /// Provide access to all user information on the system except
     /// authentication. This is adequate for almost all uses of `AllUsers`.
-    pub fn basic(config: Config) -> Result<AllUsers<auth::Basic>> {
+    pub fn basic(config: Config) -> Result<AllUsers<auth::Basic>, Error> {
         Self::new(config)
     }
 }
@@ -866,15 +1075,15 @@ impl AllUsers<auth::Basic> {
 impl AllUsers<auth::Full> {
     /// If access to password related methods for the [`User`]s yielded by this
     /// `AllUsers` is required, use this constructor.
-    pub fn authenticator(config: Config) -> Result<AllUsers<auth::Full>> {
-        let mut shadow_fd = locked_file(config.in_scheme(SHADOW_FILE), Lock::Exclusive)?;
+    pub fn authenticator(config: Config) -> Result<AllUsers<auth::Full>, Error> {
+        let mut shadow_fd = locked_file(config.in_scheme(SHADOW_FILE), config.lock)?;
         let mut shadow_cntnt = String::new();
         shadow_fd.read_to_string(&mut shadow_cntnt)?;
         let shadow_entries: Vec<&str> = shadow_cntnt.lines().collect();
-        
+
         let mut new = Self::new(config)?;
         new.shadow_fd = Some(shadow_fd);
-        
+
         for (indx, entry) in shadow_entries.iter().enumerate() {
             let mut entry = entry.split(';');
             let name = entry.next().ok_or(parse_error(indx,
@@ -888,59 +1097,91 @@ impl AllUsers<auth::Full> {
                 .find(|user| user.user == name)
                 .ok_or(parse_error(indx,
                     "error parsing shadowfile: unkown user"
-                ))?
-                .populate_hash(hash)?;
+                ))?.auth.hash = hash.to_string();
         }
-        
+
+        shadow_cntnt.zeroize();
         Ok(new)
     }
-    
-    /// Adds a user with the specified attributes to the `AllUsers`
-    /// instance. Note that the user's password is set unset (see
-    /// [Unset vs Blank Passwords](struct.User.html#unset-vs-blank-passwords))
-    /// during this call.
+
+    /// Consumes a builder, adding a new user to this `AllUsers`. Returns a
+    /// reference to the created user.
     ///
     /// Make sure to call [`AllUsers::save`] in order for the new user to be
     /// applied to the system.
-    //TODO: Take uid/gid as Option<usize> and if none, find an unused ID.
-    pub fn add_user(
-        &mut self,
-        login: &str,
-        uid: usize,
-        gid: usize,
-        name: &str,
-        home: &str,
-        shell: &str
-    ) -> Result<()> {
-        if self.iter()
-            .any(|user| user.user == login || user.uid == uid)
-        {
-            return Err(From::from(UsersError::AlreadyExists))
+    ///
+    /// Note that the user's password is set unset (see
+    /// [Unset vs Blank Passwords](struct.User.html#unset-vs-blank-passwords))
+    /// during this call.
+    ///
+    /// Also note that the user is not added to any groups when this builder is
+    /// consumed. In order to keep the system in a consistent state, it is
+    /// reccomended to also use an instance of [`AllGroups`] to update group
+    /// information when creating new users.
+    ///
+    /// # Defaults
+    /// Fields not passed to the builder before calling this function are as
+    /// follows:
+    /// - `uid`: [`AllUsers::get_unique_id`] is called on self to get the next
+    ///   available id.
+    /// - `gid`: `99`. This is the default UID for the group `nobody`. Note
+    ///   that the user is NOT added to this group in `/etc/groups`.
+    /// - `name`: The login name passed to [`UserBuilder::new`].
+    /// - `home`: `"/"`
+    /// - `shell`: `file:/bin/ion`
+    pub fn add_user(&mut self, builder: UserBuilder) -> Result<&User<auth::Full>, Error> {
+        if !is_valid_name(&builder.user) {
+            return Err(Error::InvalidName { name: builder.user });
         }
 
-        self.users.push(User {
-            user: login.into(),
-            uid,
-            gid,
-            name: name.into(),
-            home: home.into(),
-            shell: shell.into(),
-            hash: Some(("!".into(), false)),
-            auth: PhantomData,
-            auth_delay: self.config.auth_delay
-        });
-        Ok(())
+        let uid = builder.uid.unwrap_or_else(||
+            self.get_unique_id()
+                .expect("no remaining unused user ids")
+        );
+
+        if self.iter().any(|user| user.user == builder.user || user.uid == uid) {
+            Err(Error::UserAlreadyExists)
+        } else {
+            self.users.push(User {
+                user: builder.user.clone(),
+                uid,
+                gid: builder.gid.unwrap_or(99),
+                name: builder.name.unwrap_or(builder.user),
+                home: builder.home.unwrap_or("/".to_string()),
+                shell: builder.shell.unwrap_or("file:/bin/ion".to_string()),
+                auth: auth::Full::unset(),
+                auth_delay: self.config.auth_delay
+            });
+            Ok(&self.users[self.users.len() - 1])
+        }
     }
 
     /// Syncs the data stored in the `AllUsers` instance to the filesystem.
     /// To apply changes to the system from an `AllUsers`, you MUST call this
     /// function!
-    pub fn save(&mut self) -> Result<()> {
+    pub fn save(&mut self) -> Result<(), Error> {
         let mut userstring = String::new();
-        let mut shadowstring = String::new();
+
+        // Need to be careful to prevent allocations here so that
+        // shadowstring can be zeroed when this process is complete.
+        // 1 is suppossedly parallelism, not sure exactly what this means.
+        // 16 is the max length of a u64, which is used as the salt.
+        // 2 accounts for the semicolon separator and newline
+        let acfg = argon2::Config::default();
+        let argon_len = argon2::encoded_len(
+            acfg.variant, acfg.mem_cost, acfg.time_cost,
+            1, 16, acfg.hash_length) as usize;
+        let mut shadowstring = String::with_capacity(
+            self.users.len() * (USERNAME_LEN_MAX + argon_len + 2)
+        );
+
         for user in &self.users {
-            userstring.push_str(&user.passwd_entry());
-            shadowstring.push_str(&user.shadow_entry());
+            userstring.push_str(&user.passwd_entry()?);
+
+            let mut shadow_entry = user.shadow_entry()?;
+            shadowstring.push_str(&shadow_entry);
+
+            shadow_entry.zeroize();
         }
 
         let mut shadow_fd = self.shadow_fd.as_mut()
@@ -951,6 +1192,8 @@ impl AllUsers<auth::Full> {
 
         reset_file(&mut shadow_fd)?;
         shadow_fd.write_all(shadowstring.as_bytes())?;
+
+        shadowstring.zeroize();
         Ok(())
     }
 }
@@ -994,14 +1237,14 @@ impl<A> Drop for AllUsers<A> {
 pub struct AllGroups {
     groups: Vec<Group>,
     config: Config,
-    
+
     group_fd: File,
 }
 
 impl AllGroups {
     /// Create a new `AllGroups`.
-    pub fn new(config: Config) -> Result<AllGroups> {
-        let mut group_fd = locked_file(config.in_scheme(GROUP_FILE), Lock::Exclusive)?;
+    pub fn new(config: Config) -> Result<AllGroups, Error> {
+        let mut group_fd = locked_file(config.in_scheme(GROUP_FILE), config.lock)?;
         let mut group_cntnt = String::new();
         group_fd.read_to_string(&mut group_cntnt)?;
 
@@ -1018,43 +1261,58 @@ impl AllGroups {
         })
     }
 
-    /// Adds a group with the specified attributes to this `AllGroups`.
+    /// Consumes a builder, adding a new group to this `AllGroups`. Returns a
+    /// reference to the created `Group`.
     ///
     /// Make sure to call [`AllGroups::save`] in order for the new group to be
     /// applied to the system.
-    //TODO: Take Option<usize> for gid and find unused ID if None
-    pub fn add_group(
-        &mut self,
-        name: &str,
-        gid: usize,
-        users: &[&str]
-    ) -> Result<()> {
-        if self.iter()
-            .any(|group| group.group == name || group.gid == gid)
-        {
-            return Err(From::from(UsersError::AlreadyExists))
-        }
-
-        //Might be cleaner... Also breaks...
-        //users: users.iter().map(String::to_string).collect()
-        self.groups.push(Group {
-            group: name.into(),
-            gid,
-            users: users
-                .iter()
-                .map(|user| user.to_string())
-                .collect()
-        });
+    ///
+    /// # Defaults
+    /// If a builder is not passed a group id ([`GroupBuilder::gid`]) before
+    /// being passed to this function, [`AllGroups::get_unique_id`] is used.
+    ///
+    /// If the builder is not passed any users ([`GroupBuilder::user`]), the
+    /// group will still be created.
+    pub fn add_group(&mut self, builder: GroupBuilder) -> Result<&Group, Error> {
+        let group_exists = self.iter()
+            .any(|group| {
+                let gid_taken = if let Some(gid) = builder.gid {
+                    group.gid == gid
+                } else {
+                    false
+                };
+                group.group == builder.group || gid_taken
+            });
+
+        if group_exists {
+            Err(Error::GroupAlreadyExists)
+        } else if !is_valid_name(&builder.group) {
+            Err(Error::InvalidName { name: builder.group })
+        } else {
+            for username in builder.users.iter() {
+                if !is_valid_name(username) {
+                    return Err(Error::InvalidName { name: username.to_string() });
+                }
+            }
 
-        Ok(())
+            self.groups.push(Group {
+                group: builder.group,
+                gid: builder.gid.unwrap_or_else(||
+                    self.get_unique_id()
+                        .expect("no remaining unused group IDs")
+                ),
+                users: builder.users,
+            });
+            Ok(&self.groups[self.groups.len() - 1])
+        }
     }
 
     /// Syncs the data stored in this `AllGroups` instance to the filesystem.
     /// To apply changes from an `AllGroups`, you MUST call this function!
-    pub fn save(&mut self) -> Result<()> {
+    pub fn save(&mut self) -> Result<(), Error> {
         let mut groupstring = String::new();
         for group in &self.groups {
-            groupstring.push_str(&group.group_entry());
+            groupstring.push_str(&group.group_entry()?);
         }
 
         reset_file(&mut self.group_fd)?;
@@ -1102,25 +1360,48 @@ mod test {
         complete
     }
 
+    #[test]
+    fn test_safe_string() {
+        assert!(is_safe_string("Hello\\$!"));
+        assert!(!is_safe_string("semicolons are awesome; yeah!"));
+    }
+
+    #[test]
+    fn test_portable_filename() {
+        let valid = |s| {
+            assert!(is_valid_name(s));
+        };
+        let invld = |s| {
+            assert!(!is_valid_name(s));
+        };
+        valid("valid");
+        valid("vld.io");
+        valid("hyphen-ated");
+        valid("under_scores");
+        valid("1334");
+
+        invld("-no_flgs");
+        invld("invalid!");
+        invld("also:invalid");
+        invld("coolie-o?");
+        invld("sh");
+        invld("avery_very_very_very_loooooooonnggg-username");
+    }
+
     fn test_cfg() -> Config {
         Config::default()
             // Since all this really does is prepend `sheme` to the consts
             .scheme(TEST_PREFIX.to_string())
+            .writeable(true)
     }
 
-    fn read_locked_file(file: impl AsRef<Path>) -> Result<String> {
-        let mut fd = locked_file(file, Lock::Exclusive)?;
+    fn read_locked_file(file: impl AsRef<Path>) -> Result<String, Error> {
+        let mut fd = locked_file(file, Lock::Shared)?;
         let mut cntnt = String::new();
         fd.read_to_string(&mut cntnt)?;
         Ok(cntnt)
     }
 
-    fn write_locked_file(file: impl AsRef<Path>, cntnt: impl AsRef<[u8]>) -> Result<()> {
-        locked_file(file, Lock::Exclusive)?
-            .write_all(cntnt.as_ref())?;
-        Ok(())
-    }
-
     // *** struct.User ***
     #[cfg(feature = "auth")]
     #[test]
@@ -1165,48 +1446,33 @@ mod test {
 
         let root = users.get_by_id(0).expect("'root' user missing");
         assert_eq!(root.user, "root".to_string());
-        let &(ref hashstring, ref encoded) = root.hash.as_ref().expect("'root' hash is None");
-        assert_eq!(hashstring,
-            &"$argon2i$m=4096,t=10,p=1$Tnc4UVV0N00$ML9LIOujd3nmAfkAwEcSTMPqakWUF0OUiLWrIy0nGLk".to_string());
+        assert_eq!(root.auth.hash.as_str(),
+            "$argon2i$m=4096,t=10,p=1$Tnc4UVV0N00$ML9LIOujd3nmAfkAwEcSTMPqakWUF0OUiLWrIy0nGLk");
         assert_eq!(root.uid, 0);
         assert_eq!(root.gid, 0);
         assert_eq!(root.name, "root".to_string());
         assert_eq!(root.home, "file:/root".to_string());
         assert_eq!(root.shell, "file:/bin/ion".to_string());
-        match encoded {
-            true => (),
-            false => panic!("Expected encoded argon hash!")
-        }
 
         let user = users.get_by_name("user").expect("'user' user missing");
         assert_eq!(user.user, "user".to_string());
-        let &(ref hashstring, ref encoded) = user.hash.as_ref().expect("'user' hash is None");
-        assert_eq!(hashstring, &"".to_string());
+        assert_eq!(user.auth.hash.as_str(), "");
         assert_eq!(user.uid, 1000);
         assert_eq!(user.gid, 1000);
         assert_eq!(user.name, "user".to_string());
         assert_eq!(user.home, "file:/home/user".to_string());
         assert_eq!(user.shell, "file:/bin/ion".to_string());
-        match encoded {
-            true => panic!("Should not be an argon hash!"),
-            false => ()
-        }
         println!("{:?}", users);
 
-        let li = users.get_by_name("li").expect("'li' user missing");
-        println!("got li");
-        assert_eq!(li.user, "li");
-        let &(ref hashstring, ref encoded) = li.hash.as_ref().expect("'li' hash is None");
-        assert_eq!(hashstring, &"!".to_string());
+        let li = users.get_by_name("loip").expect("'loip' user missing");
+        println!("got loip");
+        assert_eq!(li.user, "loip");
+        assert_eq!(li.auth.hash.as_str(), "!");
         assert_eq!(li.uid, 1007);
         assert_eq!(li.gid, 1007);
         assert_eq!(li.name, "Lorem".to_string());
         assert_eq!(li.home, "file:/home/lorem".to_string());
         assert_eq!(li.shell, "file:/bin/ion".to_string());
-        match encoded {
-            true => panic!("Should not be an argon hash!"),
-            false => ()
-        }
     }
 
     #[cfg(feature = "auth")]
@@ -1215,9 +1481,17 @@ mod test {
         let mut users = AllUsers::authenticator(test_cfg()).unwrap();
         // NOT testing `get_unique_id`
         let id = 7099;
+
+        let fb = UserBuilder::new("fbar")
+            .uid(id)
+            .gid(id)
+            .name("Foo Bar")
+            .home("/home/foob")
+            .shell("/bin/zsh");
+
         users
-            .add_user("fb", id, id, "Foo Bar", "/home/foob", "/bin/zsh")
-            .expect("failed to add user 'fb'");
+            .add_user(fb)
+            .expect("failed to add user 'fbar'");
         //                                            weirdo ^^^^^^^^ :P
         users.save().unwrap();
         let p_file_content = read_locked_file(test_prefix(PASSWD_FILE)).unwrap();
@@ -1226,21 +1500,22 @@ mod test {
             concat!(
                 "root;0;0;root;file:/root;file:/bin/ion\n",
                 "user;1000;1000;user;file:/home/user;file:/bin/ion\n",
-                "li;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n",
-                "fb;7099;7099;Foo Bar;/home/foob;/bin/zsh\n"
+                "loip;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n",
+                "fbar;7099;7099;Foo Bar;/home/foob;/bin/zsh\n"
             )
         );
         let s_file_content = read_locked_file(test_prefix(SHADOW_FILE)).unwrap();
         assert_eq!(s_file_content, concat!(
             "root;$argon2i$m=4096,t=10,p=1$Tnc4UVV0N00$ML9LIOujd3nmAfkAwEcSTMPqakWUF0OUiLWrIy0nGLk\n",
             "user;\n",
-            "li;!\n",
-            "fb;!\n"
+            "loip;!\n",
+            "fbar;!\n"
         ));
 
         {
             println!("{:?}", users);
-            let fb = users.get_mut_by_name("fb").expect("'fb' user missing");
+            let fb = users.get_mut_by_name("fbar")
+                .expect("'fbar' user missing");
             fb.shell = "/bin/fish".to_string(); // That's better
             fb.set_passwd("").unwrap();
         }
@@ -1251,16 +1526,16 @@ mod test {
             concat!(
                 "root;0;0;root;file:/root;file:/bin/ion\n",
                 "user;1000;1000;user;file:/home/user;file:/bin/ion\n",
-                "li;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n",
-                "fb;7099;7099;Foo Bar;/home/foob;/bin/fish\n"
+                "loip;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n",
+                "fbar;7099;7099;Foo Bar;/home/foob;/bin/fish\n"
             )
         );
         let s_file_content = read_locked_file(test_prefix(SHADOW_FILE)).unwrap();
         assert_eq!(s_file_content, concat!(
             "root;$argon2i$m=4096,t=10,p=1$Tnc4UVV0N00$ML9LIOujd3nmAfkAwEcSTMPqakWUF0OUiLWrIy0nGLk\n",
             "user;\n",
-            "li;!\n",
-            "fb;\n"
+            "loip;!\n",
+            "fbar;\n"
         ));
 
         users.remove_by_id(id);
@@ -1271,7 +1546,7 @@ mod test {
             concat!(
                 "root;0;0;root;file:/root;file:/bin/ion\n",
                 "user;1000;1000;user;file:/home/user;file:/bin/ion\n",
-                "li;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n"
+                "loip;1007;1007;Lorem;file:/home/lorem;file:/bin/ion\n"
             )
         );
     }
@@ -1281,10 +1556,10 @@ mod test {
     fn empty_groups() {
         let group_trailing = Group::from_group_entry("nobody;2066; ", 0).unwrap();
         assert_eq!(group_trailing.users.len(), 0);
-        
+
         let group_no_trailing = Group::from_group_entry("nobody;2066;", 0).unwrap();
         assert_eq!(group_no_trailing.users.len(), 0);
-        
+
         assert_eq!(group_trailing.group, group_no_trailing.group);
         assert_eq!(group_trailing.gid, group_no_trailing.gid);
         assert_eq!(group_trailing.users, group_no_trailing.users);
@@ -1307,11 +1582,15 @@ mod test {
 
     #[test]
     fn manip_group() {
-        let mut groups = AllGroups::new(test_cfg()).unwrap();
-        // NOT testing `get_unique_id`
         let id = 7099;
+        let mut groups = AllGroups::new(test_cfg()).unwrap();
 
-        groups.add_group("fb", id, &["fb"]).unwrap();
+        let fb = GroupBuilder::new("fbar")
+            // NOT testing `get_unique_id`
+            .gid(id)
+            .user("fbar");
+
+        groups.add_group(fb).unwrap();
         groups.save().unwrap();
         let file_content = read_locked_file(test_prefix(GROUP_FILE)).unwrap();
         assert_eq!(
@@ -1320,13 +1599,13 @@ mod test {
                 "root;0;root\n",
                 "user;1000;user\n",
                 "wheel;1;user,root\n",
-                "li;1007;li\n",
-                "fb;7099;fb\n"
+                "loip;1007;loip\n",
+                "fbar;7099;fbar\n"
             )
         );
 
         {
-            let fb = groups.get_mut_by_name("fb").unwrap();
+            let fb = groups.get_mut_by_name("fbar").unwrap();
             fb.users.push("user".to_string());
         }
         groups.save().unwrap();
@@ -1337,8 +1616,8 @@ mod test {
                 "root;0;root\n",
                 "user;1000;user\n",
                 "wheel;1;user,root\n",
-                "li;1007;li\n",
-                "fb;7099;fb,user\n"
+                "loip;1007;loip\n",
+                "fbar;7099;fbar,user\n"
             )
         );
 
@@ -1351,16 +1630,18 @@ mod test {
                 "root;0;root\n",
                 "user;1000;user\n",
                 "wheel;1;user,root\n",
-                "li;1007;li\n"
+                "loip;1007;loip\n"
             )
         );
     }
-    
+
     #[test]
     fn empty_group() {
         let mut groups = AllGroups::new(test_cfg()).unwrap();
-        
-        groups.add_group("nobody", 2260, &[]).unwrap();
+        let nobody = GroupBuilder::new("nobody")
+            .gid(2260);
+
+        groups.add_group(nobody).unwrap();
         groups.save().unwrap();
         let file_content = read_locked_file(test_prefix(GROUP_FILE)).unwrap();
         assert_eq!(
@@ -1369,17 +1650,17 @@ mod test {
                 "root;0;root\n",
                 "user;1000;user\n",
                 "wheel;1;user,root\n",
-                "li;1007;li\n",
+                "loip;1007;loip\n",
                 "nobody;2260;\n",
             )
         );
-        
+
         drop(groups);
         let mut groups = AllGroups::new(test_cfg()).unwrap();
-        
+
         groups.remove_by_name("nobody");
         groups.save().unwrap();
-        
+
         let file_content = read_locked_file(test_prefix(GROUP_FILE)).unwrap();
         assert_eq!(
             file_content,
@@ -1387,7 +1668,7 @@ mod test {
                 "root;0;root\n",
                 "user;1000;user\n",
                 "wheel;1;user,root\n",
-                "li;1007;li\n"
+                "loip;1007;loip\n"
             )
         );
     }
diff --git a/vendor/redox_users/tests/etc/group b/vendor/redox_users/tests/etc/group
index 89cb5e61b..8b8665f53 100644
--- a/vendor/redox_users/tests/etc/group
+++ b/vendor/redox_users/tests/etc/group
@@ -1,4 +1,4 @@
 root;0;root
 user;1000;user
 wheel;1;user,root
-li;1007;li
+loip;1007;loip
diff --git a/vendor/redox_users/tests/etc/passwd b/vendor/redox_users/tests/etc/passwd
index 0679ecfdc..5c7118b51 100644
--- a/vendor/redox_users/tests/etc/passwd
+++ b/vendor/redox_users/tests/etc/passwd
@@ -1,3 +1,3 @@
 root;0;0;root;file:/root;file:/bin/ion
 user;1000;1000;user;file:/home/user;file:/bin/ion
-li;1007;1007;Lorem;file:/home/lorem;file:/bin/ion
+loip;1007;1007;Lorem;file:/home/lorem;file:/bin/ion
diff --git a/vendor/redox_users/tests/etc/shadow b/vendor/redox_users/tests/etc/shadow
index e0bab42a8..ed6cc2d40 100644
--- a/vendor/redox_users/tests/etc/shadow
+++ b/vendor/redox_users/tests/etc/shadow
@@ -1,3 +1,3 @@
 root;$argon2i$m=4096,t=10,p=1$Tnc4UVV0N00$ML9LIOujd3nmAfkAwEcSTMPqakWUF0OUiLWrIy0nGLk
 user;
-li;!
+loip;!