[pbs-devel] [PATCH proxmox-backup 1/3] api: sync: restrict edit permissions for push sync jobs

[pbs-devel] [PATCH proxmox-backup 1/3] api: sync: restrict edit permissions for push sync jobs

[Christian Ebner]

Users require `Datastore.Audit` on the source datastore to read sync
jobs. Further restrict also the permissions to modify sync jobs in
push direction to include the `Datastore.Audit` permission on the
source, as otherwise a user is able to create or edit sync jobs in
push direction, but not able to see them.

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/sync.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index bc012744a..eb97ef940 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -129,6 +129,11 @@ pub fn check_sync_job_modify_access(
             }
 
             let source_privs = user_info.lookup_privs(auth_id, &job.acl_path());
+            // only allow to modify jobs the user is also allowed to read
+            if source_privs & PRIV_DATASTORE_AUDIT == 0 {
+                return false;
+            }
+
             // check user is allowed to read from (local) source datastore/namespace, independent
             // of job ownership
             if source_privs & PRIV_DATASTORE_READ != 0 {
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 2/3] api: sync: include required permissions for push direction

[Christian Ebner]

Sync jobs in push and pull direction require a different set of
privileges for the various api methods provided. Update the
descriptitons to include the push direction and list them
accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/sync.rs | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index eb97ef940..a8ea93465 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -167,7 +167,10 @@ pub fn check_sync_job_modify_access(
         items: { type: SyncJobConfig },
     },
     access: {
-        description: "Limited to sync job entries where user has Datastore.Audit on target datastore, and Remote.Audit on source remote.",
+        description: "Limited to sync job entries where user has Datastore.Audit on target \
+datastore, and Remote.Audit on source remote for sync jobs in pull direction.\n\
+For push direction the user requires RemoteDatastore.Audit on the remote datastore and \
+Datastore.Audit on the local datastore.",
         permission: &Permission::Anybody,
     },
 )]
@@ -208,7 +211,12 @@ pub fn list_sync_jobs(
         },
     },
     access: {
-        description: "User needs Datastore.Backup on target datastore, and Remote.Read on source remote. Additionally, remove_vanished requires Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify",
+        description: "For sync jobs in pull direction user needs Datastore.Backup on target \
+datastore, and Remote.Read on source remote. Additionally, remove_vanished requires \
+Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify.\n\
+For sync jobs in push direction, user needs RemoteDatastore.Backup on remote datastore, and \
+Datastore.Audit, Datastore.Read and Datastore.Modify on the source datastore, the latter not \
+required sync job owned by user. Additionally, remove vanished requires RemoteDatastore.Modify.",
         permission: &Permission::Anybody,
     },
 )]
@@ -269,7 +277,10 @@ pub fn create_sync_job(
     },
     returns: { type: SyncJobConfig },
     access: {
-        description: "Limited to sync job entries where user has Datastore.Audit on target datastore, and Remote.Audit on source remote.",
+        description: "Limited to sync job entries where user has Datastore.Audit on target \
+datastore, and Remote.Audit on source remote for sync jobs in pull direction.\n\
+For push direction the user requires RemoteDatastore.Audit on the remote datastore and \
+Datastore.Audit on the local datastore.",
         permission: &Permission::Anybody,
     },
 )]
@@ -355,7 +366,12 @@ pub enum DeletableProperty {
     },
     access: {
         permission: &Permission::Anybody,
-        description: "User needs Datastore.Backup on target datastore, and Remote.Read on source remote. Additionally, remove_vanished requires Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify",
+        description: "For sync jobs in pull direction user needs Datastore.Backup on target \
+datastore, and Remote.Read on source remote. Additionally, remove_vanished requires \
+Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify.\n\
+For sync jobs in push direction, user needs RemoteDatastore.Backup on remote datastore, and \
+Datastore.Audit, Datastore.Read and Datastore.Modify on the source datastore, the latter not \
+required sync job owned by user. Additionally, remove vanished requires RemoteDatastore.Modify.",
     },
 )]
 /// Update sync job config.
@@ -545,7 +561,12 @@ pub fn update_sync_job(
     },
     access: {
         permission: &Permission::Anybody,
-        description: "User needs Datastore.Backup on target datastore, and Remote.Read on source remote. Additionally, remove_vanished requires Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify",
+        description: "For sync jobs in pull direction user needs Datastore.Backup on target \
+datastore, and Remote.Read on source remote. Additionally, remove_vanished requires \
+Datastore.Prune, and any owner other than the user themselves requires Datastore.Modify.\n\
+For sync jobs in push direction, user needs RemoteDatastore.Backup on remote datastore, and \
+Datastore.Audit, Datastore.Read and Datastore.Modify on the source datastore, the latter not \
+required sync job owned by user. Additionally, remove vanished requires RemoteDatastore.Modify.",
     },
 )]
 /// Remove a sync job configuration
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 3/3] docs: mention required source audit permission for push sync jobs

[Christian Ebner]

To be in line with the updated permission requirements, as
Datastore.Audit is now required to read and edit sync jobs in push
direction.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 docs/managing-remotes.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/managing-remotes.rst b/docs/managing-remotes.rst
index 4a78a9310..f8012636e 100644
--- a/docs/managing-remotes.rst
+++ b/docs/managing-remotes.rst
@@ -256,9 +256,9 @@ The following permissions are required for a sync job in push direction:
 
 #. ``Remote.Audit`` on ``/remote/{remote}`` and ``Remote.DatastoreBackup`` on
    ``/remote/{remote}/{remote-store}/{remote-ns}`` path or subnamespace.
-#. At least ``Datastore.Read`` on the local source datastore namespace
-   (``/datastore/{store}/{ns}``) or ``Datastore.Backup`` if owner of the sync
-   job.
+#. At least ``Datastore.Read`` and ``Datastore.Audit`` on the local source
+   datastore namespace (``/datastore/{store}/{ns}``) or ``Datastore.Backup`` if
+   owner of the sync job.
 #. ``Remote.DatastorePrune`` on ``/remote/{remote}/{remote-store}/{remote-ns}``
    path to remove vanished snapshots and groups. Make sure to use a dedicated
    remote for each sync job in push direction as noted above.
-- 
2.39.5



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

[pbs-devel] applied-series: [PATCH proxmox-backup 1/3] api: sync: restrict edit permissions for push sync jobs

[Thomas Lamprecht]

Am 27.11.24 um 12:44 schrieb Christian Ebner:
> Users require `Datastore.Audit` on the source datastore to read sync
> jobs. Further restrict also the permissions to modify sync jobs in
> push direction to include the `Datastore.Audit` permission on the
> source, as otherwise a user is able to create or edit sync jobs in
> push direction, but not able to see them.
> 
> Reported-by: Friedrich Weber <f.weber@proxmox.com>
> Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  src/api2/config/sync.rs | 5 +++++
>  1 file changed, 5 insertions(+)
> 
>

applied-series, thanks!


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