[pbs-devel] [PATCH v5 proxmox-backup 16/31] api: config: extend read access check by sync direction

[Christian Ebner <c.ebner@proxmox.com>]

Add the sync direction as additional parameter for the priv helper to
check for the required permissions in pull and push direction.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 4:
- no changes

changes since version 3:
- not present in previous version

 src/api2/admin/sync.rs  |  4 ++-
 src/api2/config/sync.rs | 80 +++++++++++++++++++++++++++++++++--------
 2 files changed, 68 insertions(+), 16 deletions(-)

diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs
index 1afe9fec2..7a4e38942 100644
--- a/src/api2/admin/sync.rs
+++ b/src/api2/admin/sync.rs
@@ -68,7 +68,9 @@ pub fn list_config_sync_jobs(
                 true
             }
         })
-        .filter(|job: &SyncJobConfig| check_sync_job_read_access(&user_info, &auth_id, job));
+        .filter(|job: &SyncJobConfig| {
+            check_sync_job_read_access(&user_info, &auth_id, job, sync_direction)
+        });
 
     let mut list = Vec::new();
 
diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index aed46aeb0..e0d96afe5 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -20,18 +20,35 @@ pub fn check_sync_job_read_access(
     user_info: &CachedUserInfo,
     auth_id: &Authid,
     job: &SyncJobConfig,
+    sync_direction: SyncDirection,
 ) -> bool {
+    // check for audit access on datastore/namespace, applies for pull and push direction
     let ns_anchor_privs = user_info.lookup_privs(auth_id, &job.acl_path());
     if ns_anchor_privs & PRIV_DATASTORE_AUDIT == 0 {
         return false;
     }
 
-    if let Some(remote) = &job.remote {
-        let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote]);
-        remote_privs & PRIV_REMOTE_AUDIT != 0
-    } else {
-        let source_ds_privs = user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
-        source_ds_privs & PRIV_DATASTORE_AUDIT != 0
+    match sync_direction {
+        SyncDirection::Pull => {
+            if let Some(remote) = &job.remote {
+                let remote_privs = user_info.lookup_privs(auth_id, &["remote", remote]);
+                remote_privs & PRIV_REMOTE_AUDIT != 0
+            } else {
+                let source_ds_privs =
+                    user_info.lookup_privs(auth_id, &["datastore", &job.remote_store]);
+                source_ds_privs & PRIV_DATASTORE_AUDIT != 0
+            }
+        }
+        SyncDirection::Push => {
+            // check for audit access on remote/datastore/namespace
+            if let Some(target_acl_path) = job.remote_acl_path() {
+                let remote_privs = user_info.lookup_privs(auth_id, &target_acl_path);
+                remote_privs & PRIV_REMOTE_AUDIT != 0
+            } else {
+                // Remote must always be present for sync in push direction, fail otherwise
+                false
+            }
+        }
     }
 }
 
@@ -115,7 +132,9 @@ pub fn list_sync_jobs(
 
     let list = list
         .into_iter()
-        .filter(|sync_job| check_sync_job_read_access(&user_info, &auth_id, sync_job))
+        .filter(|sync_job| {
+            check_sync_job_read_access(&user_info, &auth_id, sync_job, sync_direction)
+        })
         .collect();
     Ok(list)
 }
@@ -214,7 +233,7 @@ pub fn read_sync_job(
 
     let sync_direction = sync_direction.unwrap_or_default();
     let sync_job = config.lookup(sync_direction.as_config_type_str(), &id)?;
-    if !check_sync_job_read_access(&user_info, &auth_id, &sync_job) {
+    if !check_sync_job_read_access(&user_info, &auth_id, &sync_job, sync_direction) {
         bail!("permission check failed");
     }
 
@@ -573,14 +592,20 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
     };
 
     // should work without ACLs
-    assert!(check_sync_job_read_access(&user_info, root_auth_id, &job));
+    assert!(check_sync_job_read_access(
+        &user_info,
+        root_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
     assert!(check_sync_job_modify_access(&user_info, root_auth_id, &job));
 
     // user without permissions must fail
     assert!(!check_sync_job_read_access(
         &user_info,
         &no_perm_auth_id,
-        &job
+        &job,
+        SyncDirection::Pull,
     ));
     assert!(!check_sync_job_modify_access(
         &user_info,
@@ -589,16 +614,31 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
     ));
 
     // reading without proper read permissions on either remote or local must fail
-    assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
+    assert!(!check_sync_job_read_access(
+        &user_info,
+        &read_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
 
     // reading without proper read permissions on local end must fail
     job.remote = Some("remote1".to_string());
-    assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
+    assert!(!check_sync_job_read_access(
+        &user_info,
+        &read_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
 
     // reading without proper read permissions on remote end must fail
     job.remote = Some("remote0".to_string());
     job.store = "localstore1".to_string();
-    assert!(!check_sync_job_read_access(&user_info, &read_auth_id, &job));
+    assert!(!check_sync_job_read_access(
+        &user_info,
+        &read_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
 
     // writing without proper write permissions on either end must fail
     job.store = "localstore0".to_string();
@@ -624,7 +664,12 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
     job.remote = Some("remote1".to_string());
 
     // user with read permission can only read, but not modify/run
-    assert!(check_sync_job_read_access(&user_info, &read_auth_id, &job));
+    assert!(check_sync_job_read_access(
+        &user_info,
+        &read_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
     job.owner = Some(read_auth_id.clone());
     assert!(!check_sync_job_modify_access(
         &user_info,
@@ -645,7 +690,12 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
     ));
 
     // user with simple write permission can modify/run
-    assert!(check_sync_job_read_access(&user_info, &write_auth_id, &job));
+    assert!(check_sync_job_read_access(
+        &user_info,
+        &write_auth_id,
+        &job,
+        SyncDirection::Pull,
+    ));
     assert!(check_sync_job_modify_access(
         &user_info,
         &write_auth_id,
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel