[pbs-devel] [PATCH proxmox-backup 1/2] reader: track index chunks and limit access

[pbs-devel] [PATCH proxmox-backup 1/2] reader: track index chunks and limit access

[Fabian Grünbichler]

a reader connection should not be allowed to read arbitrary chunks in
the datastore, but only those that were previously registered by opening
the corresponding index files.

this mechanism is needed to allow unprivileged users (that don't have
full READ permissions on the whole datastore) access to their own
backups via a reader environment.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    sync uses a separate reader per snapshot that gets pulled, so this works there as well.

 src/api2/reader.rs             | 26 ++++++++++++++++++++++++++
 src/api2/reader/environment.rs | 17 +++++++++++++----
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 0dd45940..5bdd81b7 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -194,6 +194,27 @@ fn download_file(
         path.push(&file_name);
 
         env.log(format!("download {:?}", path.clone()));
+ 
+        let index: Option<Box<dyn IndexFile + Send>> = match archive_type(&file_name)? {
+            ArchiveType::FixedIndex => {
+                let index = env.datastore.open_fixed_reader(&path)?;
+                Some(Box::new(index))
+            }
+            ArchiveType::DynamicIndex => {
+                let index = env.datastore.open_dynamic_reader(&path)?;
+                Some(Box::new(index))
+            }
+            _ => { None }
+        };
+
+        if let Some(index) = index {
+            env.log(format!("register chunks in '{}' as downloadable.", file_name));
+
+            for pos in 0..index.index_count() {
+                let info = index.chunk_info(pos).unwrap();
+                env.register_chunk(info.digest);
+            }
+        }
 
         helpers::create_download_response(path).await
     }.boxed()
@@ -224,6 +245,11 @@ fn download_chunk(
         let digest_str = tools::required_string_param(&param, "digest")?;
         let digest = proxmox::tools::hex_to_digest(digest_str)?;
 
+        if !env.check_chunk_access(digest) {
+            env.log(format!("attempted to download chunk {} which is not in registered chunk list", digest_str));
+            return Err(http_err!(UNAUTHORIZED, "download chunk {} not allowed", digest_str));
+        }
+
         let (path, _) = env.datastore.chunk_path(&digest);
         let path2 = path.clone();
 
diff --git a/src/api2/reader/environment.rs b/src/api2/reader/environment.rs
index 87bd5747..4c7db36f 100644
--- a/src/api2/reader/environment.rs
+++ b/src/api2/reader/environment.rs
@@ -1,5 +1,5 @@
-//use anyhow::{bail, format_err, Error};
-use std::sync::Arc;
+use std::sync::{Arc,RwLock};
+use std::collections::HashSet;
 
 use serde_json::{json, Value};
 
@@ -23,7 +23,7 @@ pub struct ReaderEnvironment {
     pub worker: Arc<WorkerTask>,
     pub datastore: Arc<DataStore>,
     pub backup_dir: BackupDir,
-    // state: Arc<Mutex<SharedBackupState>>
+    allowed_chunks: Arc<RwLock<HashSet<[u8;32]>>>,
 }
 
 impl ReaderEnvironment {
@@ -45,7 +45,7 @@ impl ReaderEnvironment {
             debug: false,
             formatter: &JSON_FORMATTER,
             backup_dir,
-            //state: Arc::new(Mutex::new(state)),
+            allowed_chunks: Arc::new(RwLock::new(HashSet::new())),
         }
     }
 
@@ -57,6 +57,15 @@ impl ReaderEnvironment {
         if self.debug { self.worker.log(msg); }
     }
 
+
+    pub fn register_chunk(&self, digest: [u8;32]) {
+        let mut allowed_chunks = self.allowed_chunks.write().unwrap();
+        allowed_chunks.insert(digest);
+    }
+
+    pub fn check_chunk_access(&self, digest: [u8;32]) -> bool {
+       self.allowed_chunks.read().unwrap().contains(&digest)
+    }
 }
 
 impl RpcEnvironment for ReaderEnvironment {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] reader: actually allow users to downlod their own backups

[Fabian Grünbichler]

via HTTP2/backup reader protocol. they already could do so via the plain
HTTP download-file/.. API calls that the GUI uses, but the reader
environment required READ permission on the whole datastore instead of
just BACKUP on the backup group itself.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/api2/reader.rs | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/api2/reader.rs b/src/api2/reader.rs
index 5bdd81b7..4c870eda 100644
--- a/src/api2/reader.rs
+++ b/src/api2/reader.rs
@@ -14,7 +14,7 @@ use crate::api2::types::*;
 use crate::backup::*;
 use crate::server::{WorkerTask, H2Service};
 use crate::tools;
-use crate::config::acl::PRIV_DATASTORE_READ;
+use crate::config::acl::{PRIV_DATASTORE_READ, PRIV_DATASTORE_BACKUP};
 use crate::config::cached_user_info::CachedUserInfo;
 use crate::api2::helpers;
 
@@ -58,7 +58,15 @@ fn upgrade_to_backup_reader_protocol(
         let store = tools::required_string_param(&param, "store")?.to_owned();
 
         let user_info = CachedUserInfo::new()?;
-        user_info.check_privs(&userid, &["datastore", &store], PRIV_DATASTORE_READ, false)?;
+        let privs = user_info.lookup_privs(&userid, &["datastore", &store]);
+
+        let priv_read = privs & PRIV_DATASTORE_READ != 0;
+        let priv_backup = privs & PRIV_DATASTORE_BACKUP != 0;
+
+        // priv_backup needs owner check further down below!
+        if !priv_read && !priv_backup {
+            bail!("no permissions on /datastore/{}", store);
+        }
 
         let datastore = DataStore::lookup_datastore(&store)?;
 
@@ -83,6 +91,13 @@ fn upgrade_to_backup_reader_protocol(
         let env_type = rpcenv.env_type();
 
         let backup_dir = BackupDir::new(backup_type, backup_id, backup_time)?;
+        if !priv_read {
+            let owner = datastore.get_owner(backup_dir.group())?;
+            if owner != userid {
+                bail!("backup owner check failed!");
+            }
+        }
+
         let path = datastore.base_path();
 
         //let files = BackupInfo::list_files(&path, &backup_dir)?;
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 1/2] reader: track index chunks and limit access

[Dietmar Maurer]

applied both patches