[pbs-devel] applied: [PATCH v3 proxmox-backup 1/2] access: limit editing pam credentials to superuser
Fabian Grünbichler
f.gruenbichler at proxmox.com
Fri Jan 15 08:52:11 CET 2021
with fixup removing unnecessary ref/deref and moving the realm lookup
back before saving the config, as it is our safeguard against bogus
realms:
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index d9458524..f03389f8 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -236,17 +236,20 @@ pub fn create_user(
config.set_data(user.userid.as_str(), "user", &user)?;
+ let realm = user.userid.realm();
+
+ // Fails if realm does not exist!
+ let authenticator = crate::auth::lookup_authenticator(realm)?;
+
user::save_config(&config)?;
if let Some(password) = password {
let user_info = CachedUserInfo::new()?;
let current_auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let target_realm = &userZZ.userid.realm();
- if *target_realm == "pam" && !user_info.is_superuser(¤t_auth_id) {
+ if realm == "pam" && !user_info.is_superuser(¤t_auth_id) {
bail!("only superuser can edit pam credentials!");
}
- let authenticator = crate::auth::lookup_authenticator(target_realm)?;
- authenticator.store_password(&user.userid.name(), &password)?;
+ authenticator.store_password(user.userid.name(), &password)?;
}
Ok(())
On January 13, 2021 5:26 pm, Oguz Bektas wrote:
> modifying @pam users credentials should be only possible for root at 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 at 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(¤t_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(¤t_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
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
More information about the pbs-devel
mailing list