From 210ded9803b39392fcc597ec98a789e4fc9edf4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= Date: Tue, 24 May 2022 10:46:37 +0200 Subject: [PATCH] priv handling: use DatastoreWithNamespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fabian Grünbichler --- src/api2/admin/datastore.rs | 20 ++++++++++++-------- src/api2/admin/namespace.rs | 33 ++++++++++++++++++++++----------- src/api2/backup/mod.rs | 17 +++++++++-------- src/backup/hierarchy.rs | 16 +++++++--------- 4 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index efa00893..6900c631 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -82,14 +82,10 @@ fn get_group_note_path( } // TODO: move somewhere we can reuse it from (namespace has its own copy atm.) -fn get_ns_privs(store: &str, ns: &BackupNamespace, auth_id: &Authid) -> Result { +fn get_ns_privs(store_with_ns: &DatastoreWithNamespace, auth_id: &Authid) -> Result { let user_info = CachedUserInfo::new()?; - Ok(if ns.is_root() { - user_info.lookup_privs(auth_id, &["datastore", store]) - } else { - user_info.lookup_privs(auth_id, &["datastore", store, &ns.to_string()]) - }) + Ok(user_info.lookup_privs(auth_id, &store_with_ns.acl_path())) } // asserts that either either `full_access_privs` or `partial_access_privs` are fulfilled, @@ -101,7 +97,11 @@ fn check_ns_privs( full_access_privs: u64, partial_access_privs: u64, ) -> Result { - let privs = get_ns_privs(store, ns, auth_id)?; + let store_with_ns = DatastoreWithNamespace { + store: store.to_string(), + ns: ns.clone(), + }; + let privs = get_ns_privs(&store_with_ns, auth_id)?; if full_access_privs != 0 && (privs & full_access_privs) != 0 { return Ok(false); @@ -1199,7 +1199,11 @@ fn can_access_any_ns(store: Arc, auth_id: &Authid, user_info: &Cached PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_READ | PRIV_DATASTORE_BACKUP; let name = store.name(); iter.any(|ns| -> bool { - let user_privs = user_info.lookup_privs(&auth_id, &["datastore", name, &ns.to_string()]); + let store_with_ns = DatastoreWithNamespace { + store: name.to_string(), + ns: ns, + }; + let user_privs = user_info.lookup_privs(&auth_id, &store_with_ns.acl_path()); user_privs & wanted != 0 }) } diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs index e3a9f77d..a126ba59 100644 --- a/src/api2/admin/namespace.rs +++ b/src/api2/admin/namespace.rs @@ -6,21 +6,18 @@ use proxmox_router::{http_bail, ApiMethod, Permission, Router, RpcEnvironment}; use proxmox_schema::*; use pbs_api_types::{ - Authid, BackupNamespace, NamespaceListItem, Operation, DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, - PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PROXMOX_SAFE_ID_FORMAT, + Authid, BackupNamespace, DatastoreWithNamespace, NamespaceListItem, Operation, + DATASTORE_SCHEMA, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, + PRIV_DATASTORE_MODIFY, PROXMOX_SAFE_ID_FORMAT, }; use pbs_datastore::DataStore; // TODO: move somewhere we can reuse it from (datastore has its own copy atm.) -fn get_ns_privs(store: &str, ns: &BackupNamespace, auth_id: &Authid) -> Result { +fn get_ns_privs(store_with_ns: &DatastoreWithNamespace, auth_id: &Authid) -> Result { let user_info = CachedUserInfo::new()?; - Ok(if ns.is_root() { - user_info.lookup_privs(auth_id, &["datastore", store]) - } else { - user_info.lookup_privs(auth_id, &["datastore", store, &ns.to_string()]) - }) + Ok(user_info.lookup_privs(auth_id, &store_with_ns.acl_path())) } #[api( @@ -59,7 +56,12 @@ pub fn create_namespace( let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let parent = parent.unwrap_or_default(); - if get_ns_privs(&store, &parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { + let store_with_parent = DatastoreWithNamespace { + store: store.clone(), + ns: parent.clone(), + }; + + if get_ns_privs(&store_with_parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); } @@ -104,7 +106,12 @@ pub fn list_namespaces( let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; const PRIVS_OK: u64 = PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_AUDIT; // first do a base check to avoid leaking if a NS exists or not - if get_ns_privs(&store, &parent, &auth_id)? & PRIVS_OK == 0 { + let store_with_parent = DatastoreWithNamespace { + store: store.clone(), + ns: parent.clone(), + }; + + if get_ns_privs(&store_with_parent, &auth_id)? & PRIVS_OK == 0 { proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); } let user_info = CachedUserInfo::new()?; @@ -161,7 +168,11 @@ pub fn delete_namespace( }; let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let parent = ns.parent(); // must have MODIFY permission on parent to allow deletion - if get_ns_privs(&store, &parent, &auth_id)? & PRIV_DATASTORE_MODIFY == 0 { + let store_with_parent = DatastoreWithNamespace { + 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"); } diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs index 7be975c2..d484cdb4 100644 --- a/src/api2/backup/mod.rs +++ b/src/api2/backup/mod.rs @@ -17,9 +17,10 @@ use proxmox_schema::*; use proxmox_sys::sortable; use pbs_api_types::{ - Authid, BackupNamespace, BackupType, Operation, SnapshotVerifyState, VerifyState, - BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, - BACKUP_TYPE_SCHEMA, CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, PRIV_DATASTORE_BACKUP, + Authid, BackupNamespace, BackupType, DatastoreWithNamespace, Operation, SnapshotVerifyState, + VerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, + BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, + PRIV_DATASTORE_BACKUP, }; use pbs_config::CachedUserInfo; use pbs_datastore::index::IndexFile; @@ -81,15 +82,15 @@ fn upgrade_to_backup_protocol( let store = required_string_param(¶m, "store")?.to_owned(); let backup_ns = optional_ns_param(¶m)?; + let store_with_ns = DatastoreWithNamespace { + store: store.clone(), + ns: backup_ns.clone(), + }; let backup_dir_arg = pbs_api_types::BackupDir::deserialize(¶m)?; let user_info = CachedUserInfo::new()?; - let privs = if backup_ns.is_root() { - user_info.lookup_privs(&auth_id, &["datastore", &store]) - } else { - user_info.lookup_privs(&auth_id, &["datastore", &store, &backup_ns.to_string()]) - }; + let privs = user_info.lookup_privs(&auth_id, &store_with_ns.acl_path()); if privs & PRIV_DATASTORE_BACKUP == 0 { proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); } diff --git a/src/backup/hierarchy.rs b/src/backup/hierarchy.rs index 95c32ee1..b5f9b937 100644 --- a/src/backup/hierarchy.rs +++ b/src/backup/hierarchy.rs @@ -3,8 +3,8 @@ use std::sync::Arc; use anyhow::Error; use pbs_api_types::{ - Authid, BackupNamespace, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, - PRIV_DATASTORE_READ, + Authid, BackupNamespace, DatastoreWithNamespace, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, + PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_READ, }; use pbs_config::CachedUserInfo; use pbs_datastore::{backup_info::BackupGroup, DataStore, ListGroups, ListNamespacesRecursive}; @@ -100,14 +100,12 @@ impl<'a> Iterator for ListAccessibleBackupGroups<'a> { let mut override_owner = false; if let Some(auth_id) = &self.auth_id { let info = &self.user_info; - let privs = if ns.is_root() { - info.lookup_privs(&auth_id, &["datastore", self.store.name()]) - } else { - info.lookup_privs( - &auth_id, - &["datastore", self.store.name(), &ns.to_string()], - ) + let store_with_ns = DatastoreWithNamespace { + store: self.store.name().to_string(), + ns: ns.clone(), }; + let privs = info.lookup_privs(&auth_id, &store_with_ns.acl_path()); + if privs & NS_PRIVS_OK == 0 { continue; }