[pbs-devel] [PATCH proxmox-backup 09/16] owner checks: handle backups owned by API tokens
Fabian Grünbichler
f.gruenbichler at proxmox.com
Wed Oct 28 12:36:32 CET 2020
a user should be allowed to read/list/overwrite backups owned by their
own tokens, but a token should not be able to read/list/overwrite
backups owned by their owning user.
when changing ownership of a backup group, a user should be able to
transfer ownership to/from their own tokens if the backup is owned by
them (or one of their tokens).
Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
src/api2/admin/datastore.rs | 128 ++++++++++++++++++++++--------------
src/api2/backup.rs | 5 +-
src/api2/reader.rs | 5 +-
3 files changed, 88 insertions(+), 50 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 8862637d..9d00ed8d 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -44,13 +44,29 @@ use crate::config::acl::{
PRIV_DATASTORE_BACKUP,
};
-fn check_backup_owner(
+fn check_priv_or_backup_owner(
store: &DataStore,
group: &BackupGroup,
auth_id: &Authid,
+ required_privs: u64,
+) -> Result<(), Error> {
+ let user_info = CachedUserInfo::new()?;
+ let privs = user_info.lookup_privs(&auth_id, &["datastore", store.name()]);
+
+ if privs & required_privs == 0 {
+ let owner = store.get_owner(group)?;
+ check_backup_owner(&owner, auth_id)?;
+ }
+ Ok(())
+}
+
+fn check_backup_owner(
+ owner: &Authid,
+ auth_id: &Authid,
) -> Result<(), Error> {
- let owner = store.get_owner(group)?;
- if &owner != auth_id {
+ let correct_owner = owner == auth_id
+ || (owner.is_token() && &Authid::from(owner.user().clone()) == auth_id);
+ if !correct_owner {
bail!("backup owner check failed ({} != {})", auth_id, owner);
}
Ok(())
@@ -171,7 +187,7 @@ fn list_groups(
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let owner = datastore.get_owner(group)?;
- if !list_all && owner != auth_id {
+ if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
continue;
}
@@ -231,15 +247,11 @@ pub fn list_snapshot_files(
) -> Result<Vec<BackupContent>, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
-
let datastore = DataStore::lookup_datastore(&store)?;
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & (PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)) != 0;
- if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)?;
let info = BackupInfo::new(&datastore.base_path(), snapshot)?;
@@ -283,15 +295,11 @@ fn delete_snapshot(
) -> Result<Value, Error> {
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let snapshot = BackupDir::new(backup_type, backup_id, backup_time)?;
-
let datastore = DataStore::lookup_datastore(&store)?;
- let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0;
- if !allowed { check_backup_owner(&datastore, snapshot.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &auth_id, PRIV_DATASTORE_MODIFY)?;
datastore.remove_backup_dir(&snapshot, false)?;
@@ -362,7 +370,7 @@ pub fn list_snapshots (
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let owner = datastore.get_owner(group)?;
- if !list_all && owner != auth_id {
+ if !list_all && check_backup_owner(&owner, &auth_id).is_err() {
continue;
}
@@ -702,8 +710,6 @@ fn prune(
let backup_id = tools::required_string_param(¶m, "backup-id")?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let dry_run = param["dry-run"].as_bool().unwrap_or(false);
@@ -711,8 +717,7 @@ fn prune(
let datastore = DataStore::lookup_datastore(&store)?;
- let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0;
- if !allowed { check_backup_owner(&datastore, &group, &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_MODIFY)?;
let prune_options = PruneOptions {
keep_last: param["keep-last"].as_u64(),
@@ -960,8 +965,6 @@ fn download_file(
let datastore = DataStore::lookup_datastore(store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -971,8 +974,7 @@ fn download_file(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
println!("Download {} from {} ({}/{})", file_name, store, backup_dir, file_name);
@@ -1033,8 +1035,6 @@ fn download_file_decoded(
let datastore = DataStore::lookup_datastore(store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -1044,8 +1044,7 @@ fn download_file_decoded(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
for file in files {
@@ -1158,7 +1157,8 @@ fn upload_backup_log(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- check_backup_owner(&datastore, backup_dir.group(), &auth_id)?;
+ let owner = datastore.get_owner(backup_dir.group())?;
+ check_backup_owner(&owner, &auth_id)?;
let mut path = datastore.base_path();
path.push(backup_dir.relative_path());
@@ -1228,13 +1228,10 @@ fn catalog(
let datastore = DataStore::lookup_datastore(&store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
let file_name = CATALOG_NAME;
@@ -1399,8 +1396,6 @@ fn pxar_file_download(
let datastore = DataStore::lookup_datastore(&store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
let filepath = tools::required_string_param(¶m, "filepath")?.to_owned();
@@ -1410,8 +1405,7 @@ fn pxar_file_download(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
let mut components = base64::decode(&filepath)?;
if components.len() > 0 && components[0] == '/' as u8 {
@@ -1578,13 +1572,9 @@ fn get_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
-
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
let (manifest, _) = datastore.load_manifest(&backup_dir)?;
@@ -1631,13 +1621,9 @@ fn set_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
-
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
- let allowed = (user_privs & PRIV_DATASTORE_READ) != 0;
- if !allowed { check_backup_owner(&datastore, backup_dir.group(), &auth_id)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &auth_id, PRIV_DATASTORE_READ)?;
datastore.update_manifest(&backup_dir,|manifest| {
manifest.unprotected["notes"] = notes.into();
@@ -1664,7 +1650,8 @@ fn set_notes(
},
},
access: {
- permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_MODIFY, true),
+ permission: &Permission::Anybody,
+ description: "Datastore.Modify on whole datastore, or changing ownership between user and a user's token for owned backups with Datastore.Backup"
},
)]
/// Change owner of a backup group
@@ -1673,15 +1660,60 @@ fn set_backup_owner(
backup_type: String,
backup_id: String,
new_owner: Authid,
- _rpcenv: &mut dyn RpcEnvironment,
+ rpcenv: &mut dyn RpcEnvironment,
) -> Result<(), Error> {
let datastore = DataStore::lookup_datastore(&store)?;
let backup_group = BackupGroup::new(backup_type, backup_id);
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+
let user_info = CachedUserInfo::new()?;
+ let privs = user_info.lookup_privs(&auth_id, &["datastore", &store]);
+
+ let allowed = if (privs & PRIV_DATASTORE_MODIFY) != 0 {
+ // High-privilege user/token
+ true
+ } else if (privs & PRIV_DATASTORE_BACKUP) != 0 {
+ let owner = datastore.get_owner(&backup_group)?;
+
+ match (owner.is_token(), new_owner.is_token()) {
+ (true, true) => {
+ // API token to API token, owned by same user
+ let owner = owner.user();
+ let new_owner = new_owner.user();
+ owner == new_owner && Authid::from(owner.clone()) == auth_id
+ },
+ (true, false) => {
+ // API token to API token owner
+ Authid::from(owner.user().clone()) == auth_id
+ && new_owner == auth_id
+ },
+ (false, true) => {
+ // API token owner to API token
+ owner == auth_id
+ && Authid::from(new_owner.user().clone()) == auth_id
+ },
+ (false, false) => {
+ // User to User, not allowed for unprivileged users
+ false
+ },
+ }
+ } else {
+ false
+ };
+
+ if !allowed {
+ return Err(http_err!(UNAUTHORIZED,
+ "{} does not have permission to change owner of backup group '{}' to {}",
+ auth_id,
+ backup_group,
+ new_owner,
+ ));
+ }
+
if !user_info.is_active_auth_id(&new_owner) {
bail!("{} '{}' is inactive or non-existent",
if new_owner.is_token() {
diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index e030d60d..ce9a34ae 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -108,7 +108,10 @@ async move {
let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &auth_id)?;
// permission check
- if owner != auth_id && worker_type != "benchmark" {
+ let correct_owner = owner == auth_id
+ || (owner.is_token()
+ && Authid::from(owner.user().clone()) == auth_id);
+ if !correct_owner && worker_type != "benchmark" {
// only the owner is allowed to create additional snapshots
bail!("backup owner check failed ({} != {})", auth_id, owner);
}
diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 3eeece52..010d73e3 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -94,7 +94,10 @@ fn upgrade_to_backup_reader_protocol(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
if !priv_read {
let owner = datastore.get_owner(backup_dir.group())?;
- if owner != auth_id {
+ let correct_owner = owner == auth_id
+ || (owner.is_token()
+ && Authid::from(owner.user().clone()) == auth_id);
+ if !correct_owner {
bail!("backup owner check failed!");
}
}
--
2.20.1
More information about the pbs-devel
mailing list