Re: [pbs-devel] [PATCH v2 proxmox-backup 13/31] api types: define remote permissions and roles for push sync

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

after thinking about this some more - this is missing some more bits I think.

for a pull based sync, we have two users:
- local owner/user
- remote user via remote.cfg

we only ever query/read from the remote side, so having two privs is fine:
- one to see the remote in the first place
- one to read backups that the configured remote user sees

for the local side, we have the full set of privs/operations:
- create/delete namespaces (DATASTORE_MODIFY)
- delete snapshots or groups (DATASTORE_MODIFY or DATASTORE_PRUNE+owner)
- create snapshots or groups (DATASTORE_BACKUP+owner)

but for push based backups, we basically need to map the "local" privs from a
pull to the remote privs, since we possibly need to do the same set of
operations (while in the case of push, we only query/read the local side, so
the priv matrix there is simpler).

the owner semantics for push based backups are also rather weird/confusing:

- if owner has PRIV_DATASTORE_READ on the source, all groups will be synced
- if owner only has PRIV_DATASTORE_BACKUP only owned groups will be synced
- the backups on the target will always be owned by the remote.cfg user(!)

syncing back is thus only possible if the (source) owner and the remote.cfg
user are identical?

should we rename that parameter? owner makes sense for pulling (it is the user
that owns the synced groups after the sync), but not so much for pushing, where
something like "Local User" might be less misleading?

Quoting Fabian Grünbichler (2024-08-07 12:45:35)
> Quoting Christian Ebner (2024-08-01 09:43:45)
> > Adding the privileges to allow backup and prune on remote targets, to
> > be used for sync jobs in push direction.
> > 
> > Also adds a dedicated role collecting the required privileges.
> > 
> > Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> > ---
> > changes since version 1:
> > - no changes
> > 
> >  pbs-api-types/src/acl.rs | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/pbs-api-types/src/acl.rs b/pbs-api-types/src/acl.rs
> > index ef6398629..f644029fa 100644
> > --- a/pbs-api-types/src/acl.rs
> > +++ b/pbs-api-types/src/acl.rs
> > @@ -58,6 +58,12 @@ constnamedbitmap! {
> >          PRIV_REMOTE_MODIFY("Remote.Modify");
> >          /// Remote.Read allows reading data from a configured `Remote`
> >          PRIV_REMOTE_READ("Remote.Read");
> > +        /// Remote.Backup allows Remote.Read and creating new snapshots on a configured `Remote`,
> > +        /// but also requires backup ownership
> > +        PRIV_REMOTE_BACKUP("Remote.Backup");
> > +        /// Remote.Prune allows deleting snapshots on a configured `Remote`,
> > +        /// but also requires backup ownership
> > +        PRIV_REMOTE_PRUNE("Remote.Prune");
> >  
> >          /// Sys.Console allows access to the system's console
> >          PRIV_SYS_CONSOLE("Sys.Console");
> > @@ -151,6 +157,7 @@ pub const ROLE_REMOTE_AUDIT: u64 = 0
> >  pub const ROLE_REMOTE_ADMIN: u64 = 0
> >      | PRIV_REMOTE_AUDIT
> >      | PRIV_REMOTE_MODIFY
> > +    | PRIV_REMOTE_BACKUP
> >      | PRIV_REMOTE_READ;
> 
> according to the description of the role, this should also get PRIV_REMOTE_PRUNE?
> 
> >  
> >  #[rustfmt::skip]
> > @@ -160,6 +167,14 @@ pub const ROLE_REMOTE_SYNC_OPERATOR: u64 = 0
> >      | PRIV_REMOTE_AUDIT
> >      | PRIV_REMOTE_READ;
> 
> do we want to copy+rename this to make the "PULL" explicit, and deprecate the old name?
> 
> >  
> > +#[rustfmt::skip]
> > +#[allow(clippy::identity_op)]
> > +/// Remote.SyncPushOperator can do read, backup and prune on the remote.
> > +pub const ROLE_REMOTE_SYNC_PUSH_OPERATOR: u64 = 0
> > +    | PRIV_REMOTE_AUDIT
> > +    | PRIV_REMOTE_BACKUP
> > +    | PRIV_REMOTE_READ;
> 
> shouldn't just BACKUP be enough here? that implies having access to owned backups..
> 
> > +
> >  #[rustfmt::skip]
> >  #[allow(clippy::identity_op)]
> >  /// Tape.Audit can audit the tape backup configuration and media content
> > @@ -225,6 +240,8 @@ pub enum Role {
> >      RemoteAdmin = ROLE_REMOTE_ADMIN,
> >      /// Syncronisation Opertator
> >      RemoteSyncOperator = ROLE_REMOTE_SYNC_OPERATOR,
> > +    /// Syncronisation Opertator (push direction)
> 
> typo: Opertator
> 
> pre-existing typo: Synchronisation
> 
> > +    RemoteSyncPushOperator = ROLE_REMOTE_SYNC_PUSH_OPERATOR,
> >      /// Tape Auditor
> >      TapeAudit = ROLE_TAPE_AUDIT,
> >      /// Tape Administrator
> > -- 
> > 2.39.2
> > 
> > 
> > 
> > _______________________________________________
> > pbs-devel mailing list
> > pbs-devel@lists.proxmox.com
> > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> > 
> >


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