From ba5ad72f2af2a8dd8b48ac35f6270419a88eae75 Mon Sep 17 00:00:00 2001
From: Dominik George <dominik.george@teckids.org>
Date: Sat, 8 May 2021 20:16:58 +0200
Subject: [PATCH] [Cache] Add comments and debug logging
Fixed a security issue on the go which would write the user cache file
as root if dropping privileges failed.
---
src/cache.rs | 101 +++++++++++++++++++++++++++++++++------------------
1 file changed, 66 insertions(+), 35 deletions(-)
diff --git a/src/cache.rs b/src/cache.rs
index 951747f..6c25d66 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -89,17 +89,27 @@ impl Cache {
};
let target_euid = (*getpwnam(nam.as_ptr())).pw_uid;
+
if target_euid == self.original_euid {
+ debug!("No need to drop privileges, already running as {}", username);
return Ok(self.original_euid);
} else if self.original_euid == 0 {
- seteuid(target_euid);
- return Ok(target_euid);
+ let res = seteuid(target_euid);
+ if res == 0 {
+ debug!("Successfully dropped privileges to {}", username);
+ return Ok(target_euid);
+ } else {
+ error!("Could not drop privileges to {}", username);
+ return Err("Failed to drop privileges".to_string());
+ }
}
+ error!("Not running as root or target user, cannot drop privileges");
return Err("Dropping privileges not supported".to_string());
}
fn restore_privileges(&self) {
+ debug!("Restoring privileges");
seteuid(self.original_euid);
}
@@ -112,6 +122,7 @@ impl Cache {
};
let user_home = CString::from_raw((*getpwnam(nam.as_ptr())).pw_dir).to_str().unwrap();
env::set_var("HOME", user_home);
+ debug!("Home directory for {} is {}", username, user_home);
let base_dirs = BaseDirectories::with_prefix(BASE_NAME)?;
@@ -122,7 +133,16 @@ impl Cache {
}
match self.drop_privileges(username) {
- Ok(_) => base_dirs.create_cache_directory(BASE_NAME).ok(),
+ Ok(_) => match base_dirs.create_cache_directory(BASE_NAME) {
+ Ok(v) => {
+ info!("Created XDG cache directory for user {}", username);
+ Some(v)
+ },
+ Err(e) => {
+ error!("Error creating XDG cache directory for user {}: {}", username, e);
+ None
+ }
+ }
Err(_) => None
};
self.restore_privileges();
@@ -138,56 +158,66 @@ impl Cache {
}
pub fn load_user_token(&self, owner: String) -> Option<&UserToken> {
- match self.user_tokens.get(&owner) {
- Some(t) => {
- if t.is_expired() {
- // Try to load token from file
- self.drop_privileges(owner).ok();
- let new_token = match self.place_user_cache_file(owner, USER_TOKEN_FILENAME) {
- Ok(path) => match load_json(path) {
- Ok(read_token) => {
- self.user_tokens.insert(owner, read_token);
- Some(&read_token)
- },
- Err(e) => {
- self.user_tokens.remove(&owner);
- None
- }
- },
- Err(e) => {
- self.user_tokens.remove(&owner);
+ let token = self.user_tokens.get(&owner);
+ if token.is_none() || token.unwrap().is_expired() {
+ debug!("No recent token for {} in memory, trying to load from file", owner);
+ self.user_tokens.remove(&owner);
+
+ self.drop_privileges(owner).ok();
+ let new_token = match self.place_user_cache_file(owner, USER_TOKEN_FILENAME) {
+ Ok(path) => match load_json::<UserToken>(path) {
+ Ok(read_token) => {
+ if !read_token.is_expired() {
+ debug!("Found valid token for {} in file", owner);
+ self.user_tokens.insert(owner, read_token);
+ Some(&read_token)
+ } else {
+ debug!("Token in file for {} is expired.", owner);
None
}
- };
- self.restore_privileges();
- new_token
- } else {
- Some(t)
- }
- },
- None => None
+ },
+ Err(e) => None
+ },
+ Err(e) => None
+ };
+ self.restore_privileges();
+ new_token
+ } else {
+ debug!("Found valid token for {} in memory", owner);
+ token
}
}
- pub fn save_user_token(&self, owner: String, token: UserToken) {
+ pub fn save_user_token(&self, owner: String, token: UserToken) -> Result<(), io::Error> {
self.user_tokens.insert(owner, token);
+ debug!("Saved token for {} in memory", owner);
// Try to write user's token cache file
- self.drop_privileges(owner).ok();
- match self.place_user_cache_file(owner, USER_TOKEN_FILENAME) {
- Ok(path) => save_json(path, token),
- Err(e) => Err(e)
+ let res = match self.drop_privileges(owner) {
+ Ok(_) => match self.place_user_cache_file(owner, USER_TOKEN_FILENAME) {
+ Ok(path) => {
+ debug!("Storing token for {} in cache file", owner);
+ save_json(path, token)
+ },
+ Err(e) => Err(e)
+ },
+ Err(e) => Err(io::Error::new(io::ErrorKind::PermissionDenied, e))
};
self.restore_privileges();
+ return res;
}
pub fn delete_user_token(&self, owner: String) {
self.user_tokens.remove(&owner);
+ debug!("Token for {} removed from memory", owner);
// Try to remove user's token cache file
self.drop_privileges(owner).ok();
match self.place_user_cache_file(owner, USER_TOKEN_FILENAME) {
- Ok(path) => fs::remove_file(path),
+ Ok(path) => {
+ debug!("Deleting cache file for {}", owner);
+ fs::remove_file(path)
+ },
Err(e) => Err(e)
};
self.restore_privileges();
@@ -197,6 +227,7 @@ impl Cache {
for (owner, token) in self.user_tokens {
if token.is_expired() {
self.delete_user_token(owner);
+ debug!("Deleted expired token for {}", owner);
}
}
}
--
GitLab