[pbs-devel] [PATCH proxmox-backup 09/16] 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 | 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(&param, "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(&param, "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(&param, "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(&param, "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