[pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens
Fabian Grünbichler
f.gruenbichler at proxmox.com
Mon Oct 19 09:39:16 CEST 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 | 124 +++++++++++++++++++++++-------------
src/api2/backup.rs | 3 +-
src/api2/reader.rs | 3 +-
3 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index 5c9902e1..a279d382 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -37,13 +37,29 @@ use crate::config::acl::{
PRIV_DATASTORE_BACKUP,
};
-fn check_backup_owner(
+fn check_priv_or_backup_owner(
store: &DataStore,
group: &BackupGroup,
userid: &Userid,
+ required_privs: u64,
) -> Result<(), Error> {
- let owner = store.get_owner(group)?;
- if &owner != userid {
+ let user_info = CachedUserInfo::new()?;
+ let user_privs = user_info.lookup_privs(&userid, &["datastore", store.name()]);
+
+ if user_privs & required_privs == 0 {
+ let owner = store.get_owner(group)?;
+ check_backup_owner(&owner, userid)?;
+ }
+
+ Ok(())
+}
+
+fn check_backup_owner(
+ owner: &Userid,
+ userid: &Userid,
+) -> Result<(), Error> {
+ let correct_owner = owner == userid || (owner.is_tokenid() && &owner.owner()? == userid);
+ if !correct_owner {
bail!("backup owner check failed ({} != {})", userid, owner);
}
Ok(())
@@ -164,7 +180,7 @@ fn list_groups(
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let owner = datastore.get_owner(group)?;
- if !list_all && owner != userid {
+ if !list_all && check_backup_owner(&owner, &userid).is_err() {
continue;
}
@@ -224,15 +240,11 @@ pub fn list_snapshot_files(
) -> Result<Vec<BackupContent>, Error> {
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &userid, PRIV_DATASTORE_AUDIT | PRIV_DATASTORE_READ)?;
let info = BackupInfo::new(&datastore.base_path(), snapshot)?;
@@ -276,15 +288,11 @@ fn delete_snapshot(
) -> Result<Value, Error> {
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, snapshot.group(), &userid, PRIV_DATASTORE_MODIFY)?;
datastore.remove_backup_dir(&snapshot, false)?;
@@ -355,7 +363,7 @@ pub fn list_snapshots (
let list_all = (user_privs & PRIV_DATASTORE_AUDIT) != 0;
let owner = datastore.get_owner(group)?;
- if !list_all && owner != userid {
+ if !list_all && check_backup_owner(&owner, &userid).is_err() {
continue;
}
@@ -637,8 +645,6 @@ fn prune(
let backup_id = tools::required_string_param(¶m, "backup-id")?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let dry_run = param["dry-run"].as_bool().unwrap_or(false);
@@ -646,8 +652,7 @@ fn prune(
let datastore = DataStore::lookup_datastore(&store)?;
- let allowed = (user_privs & PRIV_DATASTORE_MODIFY) != 0;
- if !allowed { check_backup_owner(&datastore, &group, &userid)?; }
+ check_priv_or_backup_owner(&datastore, &group, &userid, PRIV_DATASTORE_MODIFY)?;
let prune_options = PruneOptions {
keep_last: param["keep-last"].as_u64(),
@@ -894,8 +899,6 @@ fn download_file(
let datastore = DataStore::lookup_datastore(store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -905,8 +908,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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
println!("Download {} from {} ({}/{})", file_name, store, backup_dir, file_name);
@@ -967,8 +969,6 @@ fn download_file_decoded(
let datastore = DataStore::lookup_datastore(store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let file_name = tools::required_string_param(¶m, "file-name")?.to_owned();
@@ -978,8 +978,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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let (manifest, files) = read_backup_index(&datastore, &backup_dir)?;
for file in files {
@@ -1092,7 +1091,8 @@ fn upload_backup_log(
let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- check_backup_owner(&datastore, backup_dir.group(), &userid)?;
+ let owner = datastore.get_owner(backup_dir.group())?;
+ check_backup_owner(&owner, &userid)?;
let mut path = datastore.base_path();
path.push(backup_dir.relative_path());
@@ -1162,13 +1162,10 @@ fn catalog(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let file_name = CATALOG_NAME;
@@ -1273,8 +1270,6 @@ fn pxar_file_download(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
let filepath = tools::required_string_param(¶m, "filepath")?.to_owned();
@@ -1284,8 +1279,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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let mut components = base64::decode(&filepath)?;
if components.len() > 0 && components[0] == '/' as u8 {
@@ -1420,13 +1414,10 @@ fn get_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
let (manifest, _) = datastore.load_manifest(&backup_dir)?;
@@ -1473,13 +1464,10 @@ fn set_notes(
let datastore = DataStore::lookup_datastore(&store)?;
let userid: Userid = rpcenv.get_user().unwrap().parse()?;
- let user_info = CachedUserInfo::new()?;
- let user_privs = user_info.lookup_privs(&userid, &["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(), &userid)?; }
+ check_priv_or_backup_owner(&datastore, backup_dir.group(), &userid, PRIV_DATASTORE_READ)?;
datastore.update_manifest(&backup_dir,|manifest| {
manifest.unprotected["notes"] = notes.into();
@@ -1506,7 +1494,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
@@ -1515,15 +1504,58 @@ fn set_backup_owner(
backup_type: String,
backup_id: String,
new_owner: Userid,
- _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 userid:Userid = rpcenv.get_user().unwrap().parse()?;
+
let user_info = CachedUserInfo::new()?;
+ let user_privs = user_info.lookup_privs(&userid, &["datastore", &store]);
+
+ let allowed = if (user_privs & PRIV_DATASTORE_MODIFY) != 0 {
+ // High-privilege user/token
+ true
+ } else if (user_privs & PRIV_DATASTORE_BACKUP) != 0 {
+ let owner = datastore.get_owner(&backup_group)?;
+
+ match (owner.is_tokenid(), new_owner.is_tokenid()) {
+ (true, true) => {
+ // API token to API token, owned by same user
+ let owner = owner.owner().unwrap();
+ let new_owner = new_owner.owner().unwrap();
+ owner == new_owner && owner == userid
+ },
+ (true, false) => {
+ // API token to API token owner
+ owner.owner().unwrap() == userid && new_owner == userid
+ },
+ (false, true) => {
+ // API token owner to API token
+ owner == userid && new_owner.owner().unwrap() == userid
+ },
+ (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 {}",
+ userid,
+ backup_group,
+ new_owner,
+ ));
+ }
+
if !user_info.is_active_user(&new_owner) {
bail!("user '{}' is inactive or non-existent", new_owner);
}
diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index 8b2b0e1a..17da0558 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -108,7 +108,8 @@ async move {
let (owner, _group_guard) = datastore.create_locked_backup_group(&backup_group, &userid)?;
// permission check
- if owner != userid && worker_type != "benchmark" {
+ let correct_owner = owner == userid || (owner.is_tokenid() && owner.owner()? == userid);
+ if !correct_owner && worker_type != "benchmark" {
// only the owner is allowed to create additional snapshots
bail!("backup owner check failed ({} != {})", userid, owner);
}
diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 8f47047e..7ea066c4 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -94,7 +94,8 @@ 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 != userid {
+ let correct_owner = owner == userid || (owner.is_tokenid() && owner.owner()? == userid);
+ if !correct_owner {
bail!("backup owner check failed!");
}
}
--
2.20.1
More information about the pbs-devel
mailing list