api: tape: restore: improve permission checks

no redundant store+namespace mapping, and synchronize namespace creation
check with that of manual creation and creation as part of sync.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler 2022-05-24 11:47:07 +02:00 committed by Thomas Lamprecht
parent 0aa5815fb6
commit 99e1399729
1 changed files with 36 additions and 38 deletions

View File

@ -18,9 +18,10 @@ use proxmox_uuid::Uuid;
use pbs_api_types::{ use pbs_api_types::{
parse_ns_and_snapshot, print_ns_and_snapshot, Authid, BackupDir, BackupNamespace, CryptMode, parse_ns_and_snapshot, print_ns_and_snapshot, Authid, BackupDir, BackupNamespace, CryptMode,
Operation, TapeRestoreNamespace, Userid, DATASTORE_MAP_ARRAY_SCHEMA, DATASTORE_MAP_LIST_SCHEMA, DatastoreWithNamespace, Operation, TapeRestoreNamespace, Userid, DATASTORE_MAP_ARRAY_SCHEMA,
DRIVE_NAME_SCHEMA, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, DATASTORE_MAP_LIST_SCHEMA, DRIVE_NAME_SCHEMA, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_BACKUP,
PRIV_TAPE_READ, TAPE_RESTORE_NAMESPACE_SCHEMA, TAPE_RESTORE_SNAPSHOT_SCHEMA, UPID_SCHEMA, PRIV_DATASTORE_MODIFY, PRIV_TAPE_READ, TAPE_RESTORE_NAMESPACE_SCHEMA,
TAPE_RESTORE_SNAPSHOT_SCHEMA, UPID_SCHEMA,
}; };
use pbs_config::CachedUserInfo; use pbs_config::CachedUserInfo;
use pbs_datastore::dynamic_index::DynamicIndexReader; use pbs_datastore::dynamic_index::DynamicIndexReader;
@ -198,22 +199,13 @@ impl DataStoreMap {
fn check_datastore_privs( fn check_datastore_privs(
user_info: &CachedUserInfo, user_info: &CachedUserInfo,
store: &str, store_with_ns: &DatastoreWithNamespace,
ns: &BackupNamespace,
auth_id: &Authid, auth_id: &Authid,
owner: Option<&Authid>, owner: Option<&Authid>,
) -> Result<(), Error> { ) -> Result<(), Error> {
let privs = if ns.is_root() { let privs = user_info.lookup_privs(auth_id, &store_with_ns.acl_path());
user_info.lookup_privs(auth_id, &["datastore", store])
} else {
user_info.lookup_privs(auth_id, &["datastore", store, &ns.to_string()])
};
if (privs & PRIV_DATASTORE_BACKUP) == 0 { if (privs & PRIV_DATASTORE_BACKUP) == 0 {
if ns.is_root() { bail!("no permissions on /{}", store_with_ns.acl_path().join("/"));
bail!("no permissions on /datastore/{}", store);
} else {
bail!("no permissions on /datastore/{}/{}", store, &ns.to_string());
}
} }
if let Some(ref owner) = owner { if let Some(ref owner) = owner {
@ -237,29 +229,33 @@ fn check_and_create_namespaces(
owner: Option<&Authid>, owner: Option<&Authid>,
) -> Result<(), Error> { ) -> Result<(), Error> {
// check normal restore privs first // check normal restore privs first
check_datastore_privs(user_info, store.name(), ns, auth_id, owner)?; let mut store_with_ns = DatastoreWithNamespace {
store: store.name().to_string(),
ns: ns.clone(),
};
check_datastore_privs(user_info, &store_with_ns, auth_id, owner)?;
// try create recursively if it does not exist // try create recursively if it does not exist
if !store.namespace_exists(ns) { if !store.namespace_exists(ns) {
let mut tmp_ns: BackupNamespace = Default::default(); let mut tmp_ns: BackupNamespace = Default::default();
let has_datastore_priv = user_info.lookup_privs(auth_id, &["datastore", store.name()])
& PRIV_DATASTORE_MODIFY
!= 0;
for comp in ns.components() { for comp in ns.components() {
tmp_ns.push(comp.to_string())?; tmp_ns.push(comp.to_string())?;
if !store.namespace_exists(&tmp_ns) { if !store.namespace_exists(&tmp_ns) {
if has_datastore_priv // check parent modification privs
|| user_info.lookup_privs( user_info
.check_privs(
auth_id, auth_id,
&["datastore", store.name(), &tmp_ns.parent().to_string()], &store_with_ns.acl_path(),
) & PRIV_DATASTORE_MODIFY PRIV_DATASTORE_MODIFY,
!= 0 false,
{ )
store.create_namespace(&tmp_ns.parent(), comp.to_string())?; .map_err(|_err| format_err!("no permission to create namespace '{tmp_ns}'"))?;
} else {
bail!("no permissions to create '{}'", tmp_ns); store.create_namespace(&tmp_ns.parent(), comp.to_string())?;
}
// update parent for next component
store_with_ns.ns = tmp_ns.clone();
} }
} }
} }
@ -350,8 +346,10 @@ pub fn restore(
for (target, namespaces) in used_datastores.values() { for (target, namespaces) in used_datastores.values() {
check_datastore_privs( check_datastore_privs(
&user_info, &user_info,
target.name(), &DatastoreWithNamespace {
&Default::default(), store: target.name().to_string(),
ns: Default::default(),
},
&auth_id, &auth_id,
owner.as_ref(), owner.as_ref(),
)?; )?;
@ -595,13 +593,13 @@ fn check_snapshot_restorable(
let mut can_restore_some = false; let mut can_restore_some = false;
for ns in namespaces { for ns in namespaces {
// only simple check, ns creation comes later // only simple check, ns creation comes later
if let Err(err) = check_datastore_privs( let store_with_ns = DatastoreWithNamespace {
user_info, store: datastore.name().to_string(),
datastore.name(), ns: ns.clone(),
&ns, };
auth_id, if let Err(err) =
Some(restore_owner), check_datastore_privs(user_info, &store_with_ns, auth_id, Some(restore_owner))
) { {
task_warn!(worker, "cannot restore {store}:{snapshot} to {ns}: '{err}'"); task_warn!(worker, "cannot restore {store}:{snapshot} to {ns}: '{err}'");
continue; continue;
} }