From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 13DC464721 for ; Fri, 30 Oct 2020 12:37:17 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 11BF01303D for ; Fri, 30 Oct 2020 12:37:17 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 31FBC13006 for ; Fri, 30 Oct 2020 12:37:13 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F2F1C45F90 for ; Fri, 30 Oct 2020 12:37:12 +0100 (CET) From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= To: pbs-devel@lists.proxmox.com Date: Fri, 30 Oct 2020 12:36:42 +0100 Message-Id: <20201030113644.2044947-7-f.gruenbichler@proxmox.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20201030113644.2044947-1-f.gruenbichler@proxmox.com> References: <20201030113644.2044947-1-f.gruenbichler@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.027 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [job.store, remote.rs, remote.read, sync.rs, data.store, remote.name] Subject: [pbs-devel] [PATCH proxmox-backup 6/8] sync: allow sync for non-superusers 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: , X-List-Received-Date: Fri, 30 Oct 2020 11:37:17 -0000 by requiring - Datastore.Backup permission for target datastore - Remote.Read permission for source remote/datastore - Datastore.Prune if vanished snapshots should be removed - Datastore.Modify if another user should own the freshly synced snapshots reading a sync job entry only requires knowing about both the source remote and the target datastore. note that this does not affect the Authid used to authenticate with the remote, which of course also needs permissions to access the source datastore. Signed-off-by: Fabian Grünbichler --- src/api2/admin/sync.rs | 30 +++++++-- src/api2/config/remote.rs | 15 ++++- src/api2/config/sync.rs | 138 +++++++++++++++++++++++++++++++++++--- src/config/sync.rs | 16 ++++- 4 files changed, 182 insertions(+), 17 deletions(-) diff --git a/src/api2/admin/sync.rs b/src/api2/admin/sync.rs index f7032a3a..c9b9e145 100644 --- a/src/api2/admin/sync.rs +++ b/src/api2/admin/sync.rs @@ -1,12 +1,15 @@ -use anyhow::{format_err, Error}; +use anyhow::{bail, format_err, Error}; use serde_json::Value; -use proxmox::api::{api, ApiMethod, Router, RpcEnvironment}; +use proxmox::api::{api, ApiMethod, Permission, Router, RpcEnvironment}; use proxmox::api::router::SubdirMap; use proxmox::{list_subdirs_api_method, sortable}; use crate::api2::types::*; use crate::api2::pull::do_sync_job; +use crate::api2::config::sync::{check_sync_job_modify_access, check_sync_job_read_access}; + +use crate::config::cached_user_info::CachedUserInfo; use crate::config::sync::{self, SyncJobStatus, SyncJobConfig}; use crate::server::UPID; use crate::server::jobstate::{Job, JobState}; @@ -27,6 +30,10 @@ use crate::tools::systemd::time::{ type: Array, items: { type: sync::SyncJobStatus }, }, + access: { + description: "Limited to sync jobs where user has Datastore.Audit on target datastore, and Remote.Audit on source remote.", + permission: &Permission::Anybody, + }, )] /// List all sync jobs pub fn list_sync_jobs( @@ -35,6 +42,9 @@ pub fn list_sync_jobs( mut rpcenv: &mut dyn RpcEnvironment, ) -> Result, Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; + let (config, digest) = sync::config()?; let mut list: Vec = config @@ -46,6 +56,10 @@ pub fn list_sync_jobs( } else { true } + }) + .filter(|job: &SyncJobStatus| { + let as_config: SyncJobConfig = job.clone().into(); + check_sync_job_read_access(&user_info, &auth_id, &as_config) }).collect(); for job in &mut list { @@ -89,7 +103,11 @@ pub fn list_sync_jobs( schema: JOB_ID_SCHEMA, } } - } + }, + 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", + permission: &Permission::Anybody, + }, )] /// Runs the sync jobs manually. fn run_sync_job( @@ -97,11 +115,15 @@ fn run_sync_job( _info: &ApiMethod, rpcenv: &mut dyn RpcEnvironment, ) -> Result { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let (config, _digest) = sync::config()?; let sync_job: SyncJobConfig = config.lookup("sync", &id)?; - let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) { + bail!("permission check failed"); + } let job = Job::new("syncjob", &id)?; diff --git a/src/api2/config/remote.rs b/src/api2/config/remote.rs index 96869695..ffbba1d2 100644 --- a/src/api2/config/remote.rs +++ b/src/api2/config/remote.rs @@ -6,6 +6,7 @@ use proxmox::api::{api, ApiMethod, Router, RpcEnvironment, Permission}; use proxmox::tools::fs::open_file_locked; use crate::api2::types::*; +use crate::config::cached_user_info::CachedUserInfo; use crate::config::remote; use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY}; @@ -22,7 +23,8 @@ use crate::config::acl::{PRIV_REMOTE_AUDIT, PRIV_REMOTE_MODIFY}; }, }, access: { - permission: &Permission::Privilege(&["remote"], PRIV_REMOTE_AUDIT, false), + description: "List configured remotes filtered by Remote.Audit privileges", + permission: &Permission::Anybody, }, )] /// List all remotes @@ -31,16 +33,25 @@ pub fn list_remotes( _info: &ApiMethod, mut rpcenv: &mut dyn RpcEnvironment, ) -> Result, Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let (config, digest) = remote::config()?; let mut list: Vec = config.convert_to_typed_array("remote")?; - // don't return password in api for remote in &mut list { remote.password = "".to_string(); } + let list = list + .into_iter() + .filter(|remote| { + let privs = user_info.lookup_privs(&auth_id, &["remote", &remote.name]); + privs & PRIV_REMOTE_AUDIT != 0 + }) + .collect(); + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); Ok(list) } diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs index 758b4a02..0d67b033 100644 --- a/src/api2/config/sync.rs +++ b/src/api2/config/sync.rs @@ -2,13 +2,72 @@ use anyhow::{bail, Error}; use serde_json::Value; use ::serde::{Deserialize, Serialize}; -use proxmox::api::{api, Router, RpcEnvironment}; +use proxmox::api::{api, Permission, Router, RpcEnvironment}; use proxmox::tools::fs::open_file_locked; use crate::api2::types::*; + +use crate::config::acl::{ + PRIV_DATASTORE_AUDIT, + PRIV_DATASTORE_BACKUP, + PRIV_DATASTORE_MODIFY, + PRIV_DATASTORE_PRUNE, + PRIV_REMOTE_AUDIT, + PRIV_REMOTE_READ, +}; + +use crate::config::cached_user_info::CachedUserInfo; use crate::config::sync::{self, SyncJobConfig}; -// fixme: add access permissions +pub fn check_sync_job_read_access( + user_info: &CachedUserInfo, + auth_id: &Authid, + job: &SyncJobConfig, +) -> bool { + let datastore_privs = user_info.lookup_privs(&auth_id, &["datastore", &job.store]); + if datastore_privs & PRIV_DATASTORE_AUDIT == 0 { + return false; + } + + let remote_privs = user_info.lookup_privs(&auth_id, &["remote", &job.remote]); + remote_privs & PRIV_REMOTE_AUDIT != 0 +} +// user can run the corresponding pull job +pub fn check_sync_job_modify_access( + user_info: &CachedUserInfo, + auth_id: &Authid, + job: &SyncJobConfig, +) -> bool { + let datastore_privs = user_info.lookup_privs(&auth_id, &["datastore", &job.store]); + if datastore_privs & PRIV_DATASTORE_BACKUP == 0 { + return false; + } + + if let Some(true) = job.remove_vanished { + if datastore_privs & PRIV_DATASTORE_PRUNE == 0 { + return false; + } + } + + let correct_owner = match job.owner { + Some(ref owner) => { + owner == auth_id + || (owner.is_token() + && !auth_id.is_token() + && owner.user() == auth_id.user()) + }, + // default sync owner + None => auth_id == Authid::backup_auth_id(), + }; + + // same permission as changing ownership after syncing + if !correct_owner && datastore_privs & PRIV_DATASTORE_MODIFY == 0 { + return false; + } + + let remote_privs = user_info.lookup_privs(&auth_id, &["remote", &job.remote, &job.remote_store]); + remote_privs & PRIV_REMOTE_READ != 0 +} #[api( input: { @@ -19,12 +78,18 @@ use crate::config::sync::{self, SyncJobConfig}; type: Array, items: { type: sync::SyncJobConfig }, }, + access: { + description: "Limited to sync job entries where user has Datastore.Audit on target datastore, and Remote.Audit on source remote.", + permission: &Permission::Anybody, + }, )] /// List all sync jobs pub fn list_sync_jobs( _param: Value, mut rpcenv: &mut dyn RpcEnvironment, ) -> Result, Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let (config, digest) = sync::config()?; @@ -32,7 +97,11 @@ pub fn list_sync_jobs( rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); - Ok(list) + let list = list + .into_iter() + .filter(|sync_job| check_sync_job_read_access(&user_info, &auth_id, &sync_job)) + .collect(); + Ok(list) } #[api( @@ -69,13 +138,25 @@ 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", + permission: &Permission::Anybody, + }, )] /// Create a new sync job. -pub fn create_sync_job(param: Value) -> Result<(), Error> { +pub fn create_sync_job( + param: Value, + rpcenv: &mut dyn RpcEnvironment, +) -> Result<(), Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; let sync_job: sync::SyncJobConfig = serde_json::from_value(param.clone())?; + if !check_sync_job_modify_access(&user_info, &auth_id, &sync_job) { + bail!("permission check failed"); + } let (mut config, _digest) = sync::config()?; @@ -104,15 +185,26 @@ pub fn create_sync_job(param: Value) -> Result<(), Error> { description: "The sync job configuration.", type: sync::SyncJobConfig, }, + access: { + description: "Limited to sync job entries where user has Datastore.Audit on target datastore, and Remote.Audit on source remote.", + permission: &Permission::Anybody, + }, )] /// Read a sync job configuration. pub fn read_sync_job( id: String, mut rpcenv: &mut dyn RpcEnvironment, ) -> Result { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; + let (config, digest) = sync::config()?; let sync_job = config.lookup("sync", &id)?; + if !check_sync_job_read_access(&user_info, &auth_id, &sync_job) { + bail!("permission check failed"); + } + rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into(); Ok(sync_job) @@ -183,6 +275,10 @@ 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", + }, )] /// Update sync job config. pub fn update_sync_job( @@ -196,7 +292,10 @@ pub fn update_sync_job( schedule: Option, delete: Option>, digest: Option, + rpcenv: &mut dyn RpcEnvironment, ) -> Result<(), Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; @@ -233,11 +332,15 @@ pub fn update_sync_job( if let Some(store) = store { data.store = store; } if let Some(remote) = remote { data.remote = remote; } if let Some(remote_store) = remote_store { data.remote_store = remote_store; } - if let Some(owner) = owner { data.owner = owner; } + if let Some(owner) = owner { data.owner = Some(owner); } if schedule.is_some() { data.schedule = schedule; } if remove_vanished.is_some() { data.remove_vanished = remove_vanished; } + if !check_sync_job_modify_access(&user_info, &auth_id, &data) { + bail!("permission check failed"); + } + config.set_data(&id, "sync", &data)?; sync::save_config(&config)?; @@ -258,9 +361,19 @@ 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", + }, )] /// Remove a sync job configuration -pub fn delete_sync_job(id: String, digest: Option) -> Result<(), Error> { +pub fn delete_sync_job( + id: String, + digest: Option, + rpcenv: &mut dyn RpcEnvironment, +) -> Result<(), Error> { + let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?; + let user_info = CachedUserInfo::new()?; let _lock = open_file_locked(sync::SYNC_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; @@ -271,10 +384,15 @@ pub fn delete_sync_job(id: String, digest: Option) -> Result<(), Error> crate::tools::detect_modified_configuration_file(&digest, &expected_digest)?; } - match config.sections.get(&id) { - Some(_) => { config.sections.remove(&id); }, - None => bail!("job '{}' does not exist.", id), - } + match config.lookup("sync", &id) { + Ok(job) => { + if !check_sync_job_modify_access(&user_info, &auth_id, &job) { + bail!("permission check failed"); + } + config.sections.remove(&id); + }, + Err(_) => { bail!("job '{}' does not exist.", id) }, + }; sync::save_config(&config)?; diff --git a/src/config/sync.rs b/src/config/sync.rs index d007570c..d2e945a1 100644 --- a/src/config/sync.rs +++ b/src/config/sync.rs @@ -21,7 +21,6 @@ lazy_static! { static ref CONFIG: SectionConfig = init(); } - #[api( properties: { id: { @@ -72,6 +71,21 @@ pub struct SyncJobConfig { pub schedule: Option, } +impl From<&SyncJobStatus> for SyncJobConfig { + fn from(job_status: &SyncJobStatus) -> Self { + Self { + id: job_status.id.clone(), + store: job_status.store.clone(), + owner: job_status.owner.clone(), + remote: job_status.remote.clone(), + remote_store: job_status.remote_store.clone(), + remove_vanished: job_status.remove_vanished.clone(), + comment: job_status.comment.clone(), + schedule: job_status.schedule.clone(), + } + } +} + // FIXME: generate duplicate schemas/structs from one listing? #[api( properties: { -- 2.20.1