[pbs-devel] [RFC proxmox-backup 12/15] owner checks: handle backups owned by API tokens

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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@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(&param, "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(&param, "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(&param, "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(&param, "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