From c8dc51e41f5c0cd02d82478c21a3b9743c7a4a29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= Date: Tue, 24 May 2022 11:02:51 +0200 Subject: [PATCH] api: namespace: check privs directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit instead of doing a manual lookup and check - this changes the returned error slightly since check_privs will include the checked ACL path, but that is okay here, checks are before we even lookup the namespace/store, so no chance to leak anything. Signed-off-by: Fabian Grünbichler --- src/api2/admin/namespace.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs index a126ba59..8541f248 100644 --- a/src/api2/admin/namespace.rs +++ b/src/api2/admin/namespace.rs @@ -2,7 +2,7 @@ use anyhow::{bail, Error}; use serde_json::Value; use pbs_config::CachedUserInfo; -use proxmox_router::{http_bail, ApiMethod, Permission, Router, RpcEnvironment}; +use proxmox_router::{http_err, ApiMethod, Permission, Router, RpcEnvironment}; use proxmox_schema::*; use pbs_api_types::{ @@ -14,10 +14,16 @@ use pbs_api_types::{ use pbs_datastore::DataStore; // TODO: move somewhere we can reuse it from (datastore has its own copy atm.) -fn get_ns_privs(store_with_ns: &DatastoreWithNamespace, auth_id: &Authid) -> Result { +fn check_ns_privs( + store_with_ns: &DatastoreWithNamespace, + auth_id: &Authid, + privs: u64, +) -> Result<(), Error> { let user_info = CachedUserInfo::new()?; - Ok(user_info.lookup_privs(auth_id, &store_with_ns.acl_path())) + user_info + .check_privs(auth_id, &store_with_ns.acl_path(), privs, true) + .map_err(|err| http_err!(FORBIDDEN, "{err}")) } #[api( @@ -61,9 +67,7 @@ pub fn create_namespace( ns: parent.clone(), }; - if get_ns_privs(&store_with_parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { - proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); - } + check_ns_privs(&store_with_parent, &auth_id, PRIV_DATASTORE_MODIFY)?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?; @@ -111,9 +115,8 @@ pub fn list_namespaces( ns: parent.clone(), }; - if get_ns_privs(&store_with_parent, &auth_id)? & PRIVS_OK == 0 { - proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); - } + check_ns_privs(&store_with_parent, &auth_id, PRIVS_OK)?; + let user_info = CachedUserInfo::new()?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Read))?; @@ -172,9 +175,7 @@ pub fn delete_namespace( store: store.clone(), ns: parent.clone(), }; - if get_ns_privs(&store_with_parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { - http_bail!(FORBIDDEN, "permission check failed"); - } + check_ns_privs(&store_with_parent, &auth_id, PRIV_DATASTORE_MODIFY)?; let datastore = DataStore::lookup_datastore(&store, Some(Operation::Write))?;