priv checks: use priv_to_priv_names and include path

where appropriate. these should never leak anything sensitive, as we
check privs before checking existence or existence is already known at
that point via other privileges.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
This commit is contained in:
Fabian Grünbichler 2022-05-24 11:00:27 +02:00 committed by Thomas Lamprecht
parent efa62d44d4
commit 7d0dbaa013
2 changed files with 23 additions and 14 deletions

View File

@ -32,14 +32,14 @@ use pxar::accessor::aio::Accessor;
use pxar::EntryKind; use pxar::EntryKind;
use pbs_api_types::{ use pbs_api_types::{
print_ns_and_snapshot, Authid, BackupContent, BackupNamespace, BackupType, Counts, CryptMode, print_ns_and_snapshot, privs_to_priv_names, Authid, BackupContent, BackupNamespace, BackupType,
DataStoreListItem, DataStoreStatus, DatastoreWithNamespace, GarbageCollectionStatus, Counts, CryptMode, DataStoreListItem, DataStoreStatus, DatastoreWithNamespace,
GroupListItem, Operation, PruneOptions, RRDMode, RRDTimeFrame, SnapshotListItem, GarbageCollectionStatus, GroupListItem, Operation, PruneOptions, RRDMode, RRDTimeFrame,
SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA, SnapshotListItem, SnapshotVerifyState, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA,
BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, BACKUP_NAMESPACE_SCHEMA, BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, DATASTORE_SCHEMA,
MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, IGNORE_VERIFIED_BACKUPS_SCHEMA, MAX_NAMESPACE_DEPTH, NS_MAX_DEPTH_SCHEMA, PRIV_DATASTORE_AUDIT,
PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_DATASTORE_VERIFY, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA, PRIV_DATASTORE_VERIFY, UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
}; };
use pbs_client::pxar::{create_tar, create_zip}; use pbs_client::pxar::{create_tar, create_zip};
use pbs_config::CachedUserInfo; use pbs_config::CachedUserInfo;
@ -110,7 +110,13 @@ fn check_ns_privs(
return Ok(true); return Ok(true);
} }
proxmox_router::http_bail!(FORBIDDEN, "permission check failed"); let priv_names = privs_to_priv_names(full_access_privs | partial_access_privs).join("|");
let path = format!("/{}", store_with_ns.acl_path().join("/"));
proxmox_router::http_bail!(
FORBIDDEN,
"permission check failed - missing {priv_names} on {path}"
);
} }
// helper to unify common sequence of checks: // helper to unify common sequence of checks:

View File

@ -16,9 +16,9 @@ use proxmox_router::HttpError;
use proxmox_sys::task_log; use proxmox_sys::task_log;
use pbs_api_types::{ use pbs_api_types::{
Authid, BackupNamespace, DatastoreWithNamespace, GroupFilter, GroupListItem, NamespaceListItem, privs_to_priv_names, Authid, BackupNamespace, DatastoreWithNamespace, GroupFilter,
Operation, RateLimitConfig, Remote, SnapshotListItem, MAX_NAMESPACE_DEPTH, GroupListItem, NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem,
PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY,
}; };
use pbs_client::{ use pbs_client::{
@ -800,10 +800,13 @@ fn check_ns_privs(
// TODO re-sync with API, maybe find common place? // TODO re-sync with API, maybe find common place?
let user_privs = user_info.lookup_privs(owner, &store_with_ns.acl_path()); let path = &store_with_ns.acl_path();
let user_privs = user_info.lookup_privs(owner, path);
if (user_privs & privs) == 0 { if (user_privs & privs) == 0 {
bail!("no permission to modify parent/datastore."); let priv_names = privs_to_priv_names(privs).join("|");
let path = path.join("/");
bail!("privilege(s) {priv_names} missing on /{path}");
} }
Ok(()) Ok(())
} }