From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 3/8] verify: introduce & use new Datastore.Verify privilege
Date: Fri, 30 Oct 2020 12:36:39 +0100 [thread overview]
Message-ID: <20201030113644.2044947-4-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20201030113644.2044947-1-f.gruenbichler@proxmox.com>
for verifying a whole datastore. Datastore.Backup now allows verifying
only backups owned by the triggering user.
Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
Notes:
this will need a rebase post-token, sending anyway for review of the idea itself.
src/api2/admin/datastore.rs | 24 ++++++++++++++++++++----
src/backup/verify.rs | 29 ++++++++++++++++++++++++++++-
src/config/acl.rs | 5 ++++-
src/server/verify_job.rs | 2 +-
4 files changed, 53 insertions(+), 7 deletions(-)
diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index b9412ba6..220f06ae 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -42,6 +42,7 @@ use crate::config::acl::{
PRIV_DATASTORE_READ,
PRIV_DATASTORE_PRUNE,
PRIV_DATASTORE_BACKUP,
+ PRIV_DATASTORE_VERIFY,
};
fn check_priv_or_backup_owner(
@@ -537,7 +538,7 @@ pub fn status(
schema: UPID_SCHEMA,
},
access: {
- permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_READ | PRIV_DATASTORE_BACKUP, true), // fixme
+ permission: &Permission::Privilege(&["datastore", "{store}"], PRIV_DATASTORE_VERIFY | PRIV_DATASTORE_BACKUP, true),
},
)]
/// Verify backups.
@@ -553,6 +554,7 @@ pub fn verify(
) -> Result<Value, Error> {
let datastore = DataStore::lookup_datastore(&store)?;
+ let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let worker_id;
let mut backup_dir = None;
@@ -563,12 +565,18 @@ pub fn verify(
(Some(backup_type), Some(backup_id), Some(backup_time)) => {
worker_id = format!("{}:{}/{}/{:08X}", store, backup_type, backup_id, backup_time);
let dir = BackupDir::new(backup_type, backup_id, backup_time)?;
+
+ check_priv_or_backup_owner(&datastore, dir.group(), &auth_id, PRIV_DATASTORE_VERIFY)?;
+
backup_dir = Some(dir);
worker_type = "verify_snapshot";
}
(Some(backup_type), Some(backup_id), None) => {
worker_id = format!("{}:{}/{}", store, backup_type, backup_id);
let group = BackupGroup::new(backup_type, backup_id);
+
+ check_priv_or_backup_owner(&datastore, &group, &auth_id, PRIV_DATASTORE_VERIFY)?;
+
backup_group = Some(group);
worker_type = "verify_group";
}
@@ -578,13 +586,12 @@ pub fn verify(
_ => bail!("parameters do not specify a backup group or snapshot"),
}
- let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
let to_stdout = if rpcenv.env_type() == RpcEnvironmentType::CLI { true } else { false };
let upid_str = WorkerTask::new_thread(
worker_type,
Some(worker_id.clone()),
- auth_id,
+ auth_id.clone(),
to_stdout,
move |worker| {
let verified_chunks = Arc::new(Mutex::new(HashSet::with_capacity(1024*16)));
@@ -617,7 +624,16 @@ pub fn verify(
)?;
failed_dirs
} else {
- verify_all_backups(datastore, worker.clone(), worker.upid(), None)?
+ let privs = CachedUserInfo::new()?
+ .lookup_privs(&auth_id, &["datastore", &store]);
+
+ let owner = if privs & PRIV_DATASTORE_VERIFY == 0 {
+ Some(auth_id)
+ } else {
+ None
+ };
+
+ verify_all_backups(datastore, worker.clone(), worker.upid(), owner, None)?
};
if failed_dirs.len() > 0 {
worker.log("Failed to verify following snapshots:");
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index bb39f7d8..e0e28ee9 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -482,7 +482,7 @@ pub fn verify_backup_group(
Ok((count, errors))
}
-/// Verify all backups inside a datastore
+/// Verify all (owned) backups inside a datastore
///
/// Errors are logged to the worker log.
///
@@ -493,14 +493,41 @@ pub fn verify_all_backups(
datastore: Arc<DataStore>,
worker: Arc<dyn TaskState + Send + Sync>,
upid: &UPID,
+ owner: Option<Authid>,
filter: Option<&dyn Fn(&BackupManifest) -> bool>,
) -> Result<Vec<String>, Error> {
let mut errors = Vec::new();
+ if let Some(owner) = &owner {
+ task_log!(
+ worker,
+ "verify datastore {} - limiting to backups owned by {}",
+ datastore.name(),
+ owner
+ );
+ }
+
+ let filter_by_owner = |group: &BackupGroup| {
+ if let Some(owner) = &owner {
+ match datastore.get_owner(group) {
+ Ok(ref group_owner) => {
+ group_owner == owner
+ || (group_owner.is_token()
+ && !owner.is_token()
+ && group_owner.user() == owner.user())
+ },
+ Err(_) => false,
+ }
+ } else {
+ true
+ }
+ };
+
let mut list = match BackupGroup::list_groups(&datastore.base_path()) {
Ok(list) => list
.into_iter()
.filter(|group| !(group.backup_type() == "host" && group.backup_id() == "benchmark"))
+ .filter(filter_by_owner)
.collect::<Vec<BackupGroup>>(),
Err(err) => {
task_log!(
diff --git a/src/config/acl.rs b/src/config/acl.rs
index f82d5903..7345adea 100644
--- a/src/config/acl.rs
+++ b/src/config/acl.rs
@@ -30,6 +30,7 @@ constnamedbitmap! {
PRIV_DATASTORE_ALLOCATE("Datastore.Allocate");
PRIV_DATASTORE_MODIFY("Datastore.Modify");
PRIV_DATASTORE_READ("Datastore.Read");
+ PRIV_DATASTORE_VERIFY("Datastore.Verify");
/// Datastore.Backup also requires backup ownership
PRIV_DATASTORE_BACKUP("Datastore.Backup");
@@ -64,12 +65,14 @@ pub const ROLE_DATASTORE_ADMIN: u64 =
PRIV_DATASTORE_AUDIT |
PRIV_DATASTORE_MODIFY |
PRIV_DATASTORE_READ |
+PRIV_DATASTORE_VERIFY |
PRIV_DATASTORE_BACKUP |
PRIV_DATASTORE_PRUNE;
-/// Datastore.Reader can read datastore content an do restore
+/// Datastore.Reader can read/verify datastore content and do restore
pub const ROLE_DATASTORE_READER: u64 =
PRIV_DATASTORE_AUDIT |
+PRIV_DATASTORE_VERIFY |
PRIV_DATASTORE_READ;
/// Datastore.Backup can do backup and restore, but no prune.
diff --git a/src/server/verify_job.rs b/src/server/verify_job.rs
index a8f532ac..c98cd5b2 100644
--- a/src/server/verify_job.rs
+++ b/src/server/verify_job.rs
@@ -65,7 +65,7 @@ pub fn do_verification_job(
task_log!(worker,"task triggered by schedule '{}'", event_str);
}
- let result = verify_all_backups(datastore, worker.clone(), worker.upid(), Some(&filter));
+ let result = verify_all_backups(datastore, worker.clone(), worker.upid(), None, Some(&filter));
let job_result = match result {
Ok(ref errors) if errors.is_empty() => Ok(()),
Ok(_) => Err(format_err!("verification failed - please check the log for details")),
--
2.20.1
next prev parent reply other threads:[~2020-10-30 11:37 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-30 11:36 [pbs-devel] [PATCH proxmox-backup 0/8] permission improvements Fabian Grünbichler
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 1/8] privs: allow reading notes with Datastore.Audit Fabian Grünbichler
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 2/8] privs: use Datastore.Modify|Backup to set backup notes Fabian Grünbichler
2020-10-30 11:36 ` Fabian Grünbichler [this message]
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 4/8] verify jobs: add permissions Fabian Grünbichler
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 5/8] fix #2864: add owner option to sync Fabian Grünbichler
2020-11-02 6:37 ` [pbs-devel] applied: " Dietmar Maurer
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 6/8] sync: allow sync for non-superusers Fabian Grünbichler
2020-11-02 6:39 ` [pbs-devel] applied: " Dietmar Maurer
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 7/8] privs: remove PRIV_REMOVE_PRUNE Fabian Grünbichler
2020-10-30 11:36 ` [pbs-devel] [PATCH proxmox-backup 8/8] privs: add some more comments explaining privileges Fabian Grünbichler
2020-10-30 15:44 ` [pbs-devel] partially-applied: [PATCH proxmox-backup 0/8] permission improvements Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201030113644.2044947-4-f.gruenbichler@proxmox.com \
--to=f.gruenbichler@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox