From a6cf14d72da41553c9aaab21ddceb34989c295bc Mon Sep 17 00:00:00 2001
From: Dominik George <dominik.george@teckids.org>
Date: Fri, 14 May 2021 13:35:34 +0200
Subject: [PATCH] [Cache] Document module and remove some unnecessary NSS
 resolutions

---
 src/cache.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 92 insertions(+), 10 deletions(-)

diff --git a/src/cache.rs b/src/cache.rs
index cc2e347..6a77b70 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+//! This module encapsulates all data handling, both in-memory and
+//! backed by disk storage
+
 use crate::BASE_NAME;
 use crate::unix::{Passwd, getpwnam_safe, getpwuid_safe};
 
@@ -34,16 +37,31 @@ use serde::Serialize;
 use serde::de::DeserializeOwned;
 use serde_json;
 
+// FIXME move to config
 const USER_TOKEN_FILENAME: &str = "user_token.json";
 
+/// Holds (partial or full) information about a user
+/// This will mostly be the context user (see `Cache`), and is filled with
+/// as much detail about the user as is available
 pub struct UserInfo<'a> {
+    /// Numeric user ID
     uid: Option<uid_t>,
+    /// Username as used for authentication
     username: Option<String>,
+    /// Passwd struct (once full getpwnam/getpwuid resolution was possible)
     passwd: Option<Passwd<'a>>,
+    /// OAuth access token if freshly retrieved or known from disk backed storage
     access_token: Option<BasicTokenResponse>
 }
 
+/// In-memory structure to hold global process state
+/// Needed because the PAM and NSS components might be used chained (if NSS
+/// resolution is necessary to complete PAM authentication), and we need to
+/// remember state between the calls.
 pub struct Cache<'a> {
+    /// The user currently calling the library
+    /// For NSS, this will be the process owner; for PAM, this will be the user
+    /// logging in (after successful authentication)
     pub context_user: UserInfo<'a>
 }
 
@@ -64,14 +82,26 @@ impl Cache<'_> {
 }
 
 impl <'a>UserInfo<'a> {
+    /// Set the information of this user object to that of the process owner
+    // FIXME Move to Cache, with a from_current_user generator method here
     pub fn set_current_user(&mut self) {
         self.set_uid(get_original_euid());
     }
 
+    /// Returns `true` if any of the information slots is filled
     pub fn is_initialized(&self) -> bool {
-        self.uid.is_some() || self.username.is_some()
+        self.passwd.is_some() || self.uid.is_some() || self.username.is_some()
     }
 
+    /// Try to do `getpwnam`/`getpwuid` resolution for this user
+    ///
+    /// Will fill the `passwd` slot on success, or return an error if not successful.
+    /// This method will only attempt resolution if calling `getpwnam`/`getpwuid` is
+    /// currently considered safe, i.e. the `is_getpwnam_safe` flag has not been set
+    /// to `false`. It will be set to false if another resolution is currently running,
+    /// because libc will call back into our backend and we need to break the loop.
+    /// This means that e.g. home directory resolution is impossible during an NSS
+    /// backend call, because we cannot call NSS again.
     fn try_resolve(&mut self) -> Result<(), io::Error> {
         // If we already have a full passwd struct, do nothing
         if self.passwd.is_some() {
@@ -108,8 +138,12 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Return the numeric user ID from either the passwd struct or the uid slot,
+    /// attempting NSS resolution before doing so (in case only username is filled)
     pub fn get_uid(&mut self) -> Result<uid_t, io::Error> {
-        self.try_resolve();
+        if self.uid.is_none() && self.passwd.is_none() {
+            self.try_resolve();
+        }
         match &self.passwd {
             Some(passwd) => Ok(passwd.pw_uid),
             None => match self.uid {
@@ -119,15 +153,28 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Set the numeric user ID, clearing all mismatching fields and attepmting
+    /// resolution if necessary
     pub fn set_uid(&mut self, uid: uid_t) {
         self.uid = Some(uid);
-        self.username = None;
-        self.passwd = None;
-        self.try_resolve();
+
+        if self.passwd.is_some() && self.passwd.as_ref().unwrap().pw_uid != uid {
+            // Invalidate passwd because UID does not match anymore
+            self.passwd = None;
+            self.try_resolve();
+        }
+        self.username = match &self.passwd {
+            Some(p) => Some(p.pw_name.to_string()),
+            None => None
+        };
     }
 
+    /// Return the username from either the passwd struct or the username slot,
+    /// attempting NSS resolution before doing so (in case only uid is filled)
     pub fn get_username(&mut self) -> Result<String, io::Error> {
-        self.try_resolve();
+        if self.username.is_none() && self.passwd.is_none() {
+            self.try_resolve();
+        }
         match &self.passwd {
             Some(passwd) => Ok(passwd.pw_name.to_string()),
             None => match &self.username {
@@ -137,21 +184,35 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Set the username, clearing all mismatching fields and attepmting
+    /// resolution if necessary
     pub fn set_username(&mut self, username: String) {
         self.username = Some(username);
-        self.uid = None;
-        self.passwd = None;
-        self.try_resolve();
+
+        if self.passwd.is_some() && self.passwd.as_ref().unwrap().pw_name != self.username.as_ref().unwrap() {
+            // Invalidate passwd because UID does not match anymore
+            self.passwd = None;
+            self.try_resolve();
+        }
+        self.uid = match &self.passwd {
+            Some(p) => Some(p.pw_uid),
+            None => None
+        };
     }
 
+    /// Return the home directory from the passwd slot,
+    /// attempting NSS resolution before doing so
     pub fn get_home_directory(&mut self) -> Result<&str, io::Error> {
-        self.try_resolve();
+        if self.passwd.is_none() {
+            self.try_resolve();
+        }
         match &self.passwd {
             Some(passwd) => Ok(passwd.pw_dir),
             None => Err(io::Error::new(io::ErrorKind::InvalidInput, "foo"))
         }
     }
 
+    /// Attempt to drop privileges to this user, by setting EUID to their user ID
     fn drop_privileges(&mut self) -> Result<uid_t, io::Error> {
         let current_euid = unsafe {
             geteuid()
@@ -183,6 +244,8 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Restore privileges to the original process owner by setting EUID to their user ID
+    // FIXME Move to global scope
     fn restore_privileges(&self) {
         let current_euid = unsafe {
             geteuid()
@@ -201,13 +264,18 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Get the XDG base directories for this user
     fn get_user_xdg_base_directories(&mut self) -> Result<BaseDirectories, io::Error> {
         // Save original $HOME for later restore
         let saved_home = env::var_os("HOME");
 
         // Determine user ID to find out whether we should override $HOME
+        // For the current user, we rely on $HOME being set to avoid a bootstrapping
+        // issue to get the access token for NSS resolution
         let uid = self.get_uid()?;
         if uid != get_original_euid() {
+            // Determine home directory and override $HOME to make the XDG code return
+            // XDG directories for a different user
             let user_home = self.get_home_directory()?;
             env::set_var("HOME", user_home);
             debug!("Home directory for UID {} is {}", uid, user_home);
@@ -231,6 +299,7 @@ impl <'a>UserInfo<'a> {
         return Ok(base_dirs);
     }
 
+    /// Get the full path to a cache file under our prefix in this user's XDG cache directory
     fn place_user_cache_file(&mut self, filename: String) -> Result<PathBuf, io::Error> {
         match self.get_user_xdg_base_directories() {
             Ok(b) => b.place_cache_file(filename),
@@ -238,11 +307,16 @@ impl <'a>UserInfo<'a> {
         }
     }
 
+    /// Get a known access token for this user
+    ///
+    /// This will use the in-memory token from the `access_token` slot if it is filled,
+    /// or attempt to load a token from disk if not
     pub fn get_access_token(&mut self) -> &Option<BasicTokenResponse> {
         // Try to load our acess token if none is known
         if self.access_token.is_none() {
             debug!("No token in memory, trying to load from file");
             self.drop_privileges().ok();
+            // Trying to read even after failed privilege dropping is safe
             self.access_token = match self.place_user_cache_file(USER_TOKEN_FILENAME.to_string()) {
                 Ok(path) => match load_json(path) {
                     Ok(read_token) => Some(read_token),
@@ -256,11 +330,17 @@ impl <'a>UserInfo<'a> {
         return &self.access_token;
     }
 
+    /// Set the known access token for this user
+    ///
+    /// This will store the token in memory in the `access_token` slot, and attempt to
+    /// write the token to disk afterwards
     pub fn set_access_token(&mut self, token: BasicTokenResponse) -> Result<(), io::Error> {
         self.access_token = Some(token.clone());
         debug!("Saved token for in memory");
 
         // Try to write user's token cache file
+        // We need to ensure privileges were dropped successfully to avoid symlink attacks
+        // cf. https://capec.mitre.org/data/definitions/132.html
         let res = match self.drop_privileges() {
             Ok(_) => match self.place_user_cache_file(USER_TOKEN_FILENAME.to_string()) {
                 Ok(path) => {
@@ -276,6 +356,7 @@ impl <'a>UserInfo<'a> {
     }
 }
 
+/// Deserialize JSON stored in a file on disk
 fn load_json<O: DeserializeOwned>(path: PathBuf) -> Result<O, io::Error> {
     let file = fs::File::open(path)?;
     let reader = io::BufReader::new(file);
@@ -285,6 +366,7 @@ fn load_json<O: DeserializeOwned>(path: PathBuf) -> Result<O, io::Error> {
     }
 }
 
+/// Serialize JSON to a file on disk
 fn save_json<O: Serialize>(path: PathBuf, obj: O) -> Result<(), io::Error> {
     let json = match serde_json::to_string(&obj) {
         Ok(j) => j,
-- 
GitLab