From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D28171FF16B for ; Thu, 17 Oct 2024 15:28:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3E130105BA; Thu, 17 Oct 2024 15:28:40 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Thu, 17 Oct 2024 15:27:01 +0200 Message-Id: <20241017132716.385234-17-c.ebner@proxmox.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20241017132716.385234-1-c.ebner@proxmox.com> References: <20241017132716.385234-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.025 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH v4 proxmox 16/31] api: config: extend read access check by sync direction X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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 --- 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