From a5f3d4a21cc3c4c222d06d9e5763023d51dc722c Mon Sep 17 00:00:00 2001 From: Hannes Laimer Date: Tue, 26 Nov 2024 15:28:40 +0100 Subject: [PATCH] api: removable datastores: require Sys.Modify permission on /system/disks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A lot of removable datastore actions can alter the system state (mounting, unmounting), so require Sys.Modify for lack of better alternative. Suggested-by: Fabian Grünbichler Signed-off-by: Hannes Laimer [ TL: improve commit subject and add access-description for create, and delete, where we do a dynamic access check ] Signed-off-by: Thomas Lamprecht --- src/api2/admin/datastore.rs | 12 +++++++++--- src/api2/config/datastore.rs | 16 +++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs index 1c939bc2..cae7eb89 100644 --- a/src/api2/admin/datastore.rs +++ b/src/api2/admin/datastore.rs @@ -45,7 +45,7 @@ use pbs_api_types::{ BACKUP_TYPE_SCHEMA, CATALOG_NAME, CLIENT_LOG_BLOB_NAME, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, - PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, UPID, UPID_SCHEMA, + PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, UPID, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA, }; use pbs_client::pxar::{create_tar, create_zip}; @@ -2512,7 +2512,10 @@ pub fn do_mount_device(datastore: DataStoreConfig) -> Result<(), Error> { schema: UPID_SCHEMA, }, access: { - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_AUDIT, false), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), }, )] /// Mount removable datastore. @@ -2625,7 +2628,10 @@ fn do_unmount_device( schema: UPID_SCHEMA, }, access: { - permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), + permission: &Permission::And(&[ + &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true), + &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false) + ]), } )] /// Unmount a removable device that is associated with the datastore diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs index 88ccd200..24d3f630 100644 --- a/src/api2/config/datastore.rs +++ b/src/api2/config/datastore.rs @@ -14,7 +14,8 @@ use proxmox_uuid::Uuid; use pbs_api_types::{ Authid, DataStoreConfig, DataStoreConfigUpdater, DatastoreNotify, DatastoreTuning, KeepOptions, MaintenanceMode, PruneJobConfig, PruneJobOptions, DATASTORE_SCHEMA, PRIV_DATASTORE_ALLOCATE, - PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, UPID_SCHEMA, + PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_MODIFY, PRIV_SYS_MODIFY, PROXMOX_CONFIG_DIGEST_SCHEMA, + UPID_SCHEMA, }; use pbs_config::BackupLockGuard; use pbs_datastore::chunk_store::ChunkStore; @@ -172,6 +173,7 @@ pub(crate) fn do_create_datastore( }, }, access: { + description: "Requires Datastore.Allocate and, for a backing-device, Sys.Modfiy on '/system/disks'.", permission: &Permission::Privilege(&["datastore"], PRIV_DATASTORE_ALLOCATE, false), }, )] @@ -203,6 +205,11 @@ pub fn create_datastore( let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI; + if config.backing_device.is_some() { + let user_info = CachedUserInfo::new()?; + user_info.check_privs(&auth_id, &["system", "disks"], PRIV_SYS_MODIFY, false)?; + } + let mut prune_job_config = None; if config.keep.keeps_something() || !has_prune_job(&config.name)? { prune_job_config = config.prune_schedule.as_ref().map(|schedule| { @@ -550,6 +557,7 @@ pub fn update_datastore( }, }, access: { + description: "Requires Datastore.Allocate and, for a backing-device, Sys.Modfiy on '/system/disks'.", permission: &Permission::Privilege(&["datastore", "{name}"], PRIV_DATASTORE_ALLOCATE, false), }, returns: { @@ -579,6 +587,12 @@ pub async fn delete_datastore( let store_config: DataStoreConfig = config.lookup("datastore", &name)?; + if store_config.backing_device.is_some() { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; + user_info.check_privs(&auth_id, &["system", "disks"], PRIV_SYS_MODIFY, false)?; + } + if destroy_data && get_datastore_mount_status(&store_config) == Some(false) { http_bail!( BAD_REQUEST,