[pbs-devel] [PATCH v3 proxmox-backup 1/2] access: limit editing pam credentials to superuser

* [pbs-devel] [PATCH v3 proxmox-backup 1/2] access: limit editing pam credentials to superuser
@ 2021-01-13 16:26 Oguz Bektas
  2021-01-13 16:26 ` [pbs-devel] [PATCH v3 proxmox-backup 2/2] access: restrict password changes on @pam realm " Oguz Bektas
  2021-01-15  7:52 ` [pbs-devel] applied: [PATCH v3 proxmox-backup 1/2] access: limit editing pam credentials " Fabian Grünbichler
  0 siblings, 2 replies; 4+ messages in thread
From: Oguz Bektas @ 2021-01-13 16:26 UTC (permalink / raw)
  To: pbs-devel

modifying @pam users credentials should be only possible for root@pam,
otherwise it can have unintended consequences.

also enforce the same limit on user creation (except self_service check,
since it makes no sense during user creation)

Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
---
v2->v3:
* apply restrictions for 'create_user' as well
* make if condition more readable with variables
* fix issue with regular pam users being unable to edit their own
passwords (self_service)


 src/api2/access/user.rs | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 484919bf..d9458524 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -218,7 +218,11 @@ pub fn list_users(
     },
 )]
 /// Create new user.
-pub fn create_user(password: Option<String>, param: Value) -> Result<(), Error> {
+pub fn create_user(
+    password: Option<String>,
+    param: Value,
+    rpcenv: &mut dyn RpcEnvironment
+) -> Result<(), Error> {
 
     let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
 
@@ -230,14 +234,19 @@ pub fn create_user(password: Option<String>, param: Value) -> Result<(), Error>
         bail!("user '{}' already exists.", user.userid);
     }
 
-    let authenticator = crate::auth::lookup_authenticator(&user.userid.realm())?;
-
     config.set_data(user.userid.as_str(), "user", &user)?;
 
     user::save_config(&config)?;
 
     if let Some(password) = password {
-        authenticator.store_password(user.userid.name(), &password)?;
+        let user_info = CachedUserInfo::new()?;
+        let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        let target_realm = &user.userid.realm();
+        if *target_realm == "pam" && !user_info.is_superuser(&current_auth_id) {
+            bail!("only superuser can edit pam credentials!");
+        }
+        let authenticator = crate::auth::lookup_authenticator(target_realm)?;
+        authenticator.store_password(&user.userid.name(), &password)?;
     }
 
     Ok(())
@@ -350,6 +359,7 @@ pub fn update_user(
     email: Option<String>,
     delete: Option<Vec<DeletableProperty>>,
     digest: Option<String>,
+    rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<(), Error> {
 
     let _lock = open_file_locked(user::USER_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?;
@@ -392,6 +402,13 @@ pub fn update_user(
     }
 
     if let Some(password) = password {
+        let user_info = CachedUserInfo::new()?;
+        let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+        let self_service = current_auth_id.user() == &userid;
+        let target_realm = userid.realm();
+        if !self_service && target_realm == "pam" && !user_info.is_superuser(&current_auth_id) {
+            bail!("only superuser can edit pam credentials!");
+        }
         let authenticator = crate::auth::lookup_authenticator(userid.realm())?;
         authenticator.store_password(userid.name(), &password)?;
     }
-- 
2.20.1




^ permalink raw reply	[flat|nested] 4+ messages in thread