[PATCH proxmox{,-backup} 00/28] append-only sync jobs and snapshot retention timespan

[PATCH proxmox{,-backup} 00/28] append-only sync jobs and snapshot retention timespan

[Christian Ebner]

Currently sync jobs cannot be configured to be fully append-only
since namespace creation requires datastore modify privileges to do
so. Further, permissions would also allow to restore or modify owned
content.

This patch series therefore extends the current permissions
and roles to allow for append only sync jobs, by only allowing
the minimally required permissions and roles.

In particular, for push the sync jobs local user on the source
requires RemoteSyncAppendOperator as well as DatastoreReader on the
source datastore, with DatastoreAppend and DatastoreAudit (latter for
listing privs of pre-existing contents without restore) permissions
for the user on the remote instance used for connection.

For pull, the user on the target must be able to append to the
datastore via DatastoreAppend and able to read from the remote source
by the respective RemoteSyncOperator permissions on the remote and
by either DatastoreBackup or DatastoreReader permissions to access the
contents.

Further, sync jobs are extended to allow setting a retention timespan
for which synced snapshots cannot be pruned, neither by the sync job,
nor by prune jobs. Only root@pam is allowed to change the retention
period. After the retention period, snapshots behave like regular
snapshots again and can be pruned.

To protect from sync jobs setting unintended retention timespans,
it is now also possible to configure a maximum allowed reteniton time
on the datastore.

Sending this as RFC for some initial feedback on the overall
implementation approach, plan to further have a look into object
locking and retention on s3 object stores [0] and changes required
for immutable storage [1].

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=6780
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=4293


proxmox:

Christian Ebner (5):
  pbs-api-types: add append only permission and role
  pbs-api-types: add remote datastore append privs and role
  pbs-api-types: extend snapshot list items by retention timestamp
  pbs-api-types: extend sync job config by retention-timespan parameter
  pbs-api-types: add maximum retention timespan property to datastore

 pbs-api-types/src/acl.rs       | 24 +++++++++++++
 pbs-api-types/src/datastore.rs | 16 +++++++--
 pbs-api-types/src/jobs.rs      | 64 +++++++++++++++++++++++++++++++++-
 3 files changed, 101 insertions(+), 3 deletions(-)


proxmox-backup:

Christian Ebner (23):
  api: config: extend sync job config by new retention-timespan
  api: admin: improve code style for status endpoint
  client: avoid error in status if user lacks permissions
  server: allow iterating contents for Datastore.Append permissions
  api: backup: fix possible information leak in multi-tenant datastores
  api: backup: allow backup for user/token with append permission
  api: allow namespace creation on append permissions
  api: sync: allow pull to target for user/token with append permission
  sync: pull: allow pulling for user/token with append permission
  sync: push: allow push and ns creation on Remote.DatastoreAppend
  datastore: conditionally treat missing manifest as error or bening
  api: backup: provide retain-until timestamp for extended prune
    protection
  tools: include retain-until timestamp in snapshot list items
  client: backup writer: allow to send retain-until timestamp on backup
  sync: push: allow to set retention timestamp for synced snapshots
  sync: pull: allow to set retention timestamp for synced snapshots
  sync: pull: protect retained snapshot from being overwritten
  api: config: allow to set or delete reteniton timespan for sync jobs
  ui: add retention timespan form and use it for sync job edit window
  datastore/config: parse and enforce maximum retention timespan
  ui: allow datastore wide max retention timespan configuration
  api: admin: allow to update snapshot retention for root user
  ui: show retention in datastore contents

 examples/upload-speed.rs               |  1 +
 pbs-client/src/backup_writer.rs        |  5 ++
 pbs-datastore/src/backup_info.rs       | 63 ++++++++++++++++---
 pbs-datastore/src/datastore.rs         | 10 ++-
 pbs-datastore/src/manifest.rs          | 27 ++++++++
 proxmox-backup-client/src/benchmark.rs |  1 +
 proxmox-backup-client/src/main.rs      | 19 +++---
 src/api2/admin/datastore.rs            | 85 ++++++++++++++++++++------
 src/api2/admin/namespace.rs            |  8 +--
 src/api2/backup/environment.rs         |  4 ++
 src/api2/backup/mod.rs                 | 35 +++++++++--
 src/api2/config/datastore.rs           |  9 +++
 src/api2/config/sync.rs                | 22 +++++--
 src/api2/pull.rs                       | 19 ++++--
 src/api2/push.rs                       |  8 ++-
 src/api2/tape/restore.rs               |  6 +-
 src/backup/hierarchy.rs                | 23 +++++--
 src/server/pull.rs                     | 69 +++++++++++++++++----
 src/server/push.rs                     | 30 +++++++--
 src/server/sync.rs                     |  1 +
 src/tools/mod.rs                       | 23 +++----
 www/Makefile                           |  2 +
 www/datastore/Content.js               | 67 ++++++++++++++++++++
 www/datastore/OptionView.js            |  8 +++
 www/form/RetentionTimespan.js          | 21 +++++++
 www/window/MaxRetentionTimespanEdit.js | 27 ++++++++
 www/window/SyncJobEdit.js              |  9 +++
 27 files changed, 510 insertions(+), 92 deletions(-)
 create mode 100644 www/form/RetentionTimespan.js
 create mode 100644 www/window/MaxRetentionTimespanEdit.js


Summary over all repositories:
  30 files changed, 611 insertions(+), 95 deletions(-)

-- 
Generated by murpp 0.11.0

[PATCH proxmox 01/28] pbs-api-types: add append only permission and role

[Christian Ebner]

While allowing to perform backups like Datastore.Backup, the new
Datastore.Append allows further to create new namespaces, but does
not allow to list, restore or otherwise modify datastore contents,
not even if owned.

The role is intended to allow user configuration for immutable backup
and sync jobs and to be set on the user/token with target datastore
ACL path.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/acl.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/pbs-api-types/src/acl.rs b/pbs-api-types/src/acl.rs
index 691af305..9055dfab 100644
--- a/pbs-api-types/src/acl.rs
+++ b/pbs-api-types/src/acl.rs
@@ -42,6 +42,9 @@ constnamedbitmap! {
         /// Allows verifying a datastore
         PRIV_DATASTORE_VERIFY("Datastore.Verify");
 
+        /// Datastore.Append allows to create new snapshots or namespaces,
+        /// but also requires backup ownership
+        PRIV_DATASTORE_APPEND("Datastore.Append");
         /// Datastore.Backup allows Datastore.Read|Verify and creating new snapshots,
         /// but also requires backup ownership
         PRIV_DATASTORE_BACKUP("Datastore.Backup");
@@ -126,6 +129,13 @@ pub const ROLE_DATASTORE_READER: u64 = 0
     | PRIV_DATASTORE_VERIFY
     | PRIV_DATASTORE_READ;
 
+#[rustfmt::skip]
+#[allow(clippy::identity_op)]
+/// Datastore.Append can only add backups and namespaces, but not
+/// list, restore or prune backups and cannot delete namespaces.
+pub const ROLE_DATASTORE_APPEND: u64 = 0
+    | PRIV_DATASTORE_APPEND;
+
 #[rustfmt::skip]
 #[allow(clippy::identity_op)]
 /// Datastore.Backup can do backup and restore, but no prune.
@@ -241,6 +251,8 @@ pub enum Role {
     Audit = ROLE_AUDIT,
     /// Disable Access
     NoAccess = ROLE_NO_ACCESS,
+    /// Datastore Append (append new backups and create namespaces)
+    DatastoreAppend = ROLE_DATASTORE_APPEND,
     /// Datastore Administrator
     DatastoreAdmin = ROLE_DATASTORE_ADMIN,
     /// Datastore Reader (inspect datastore content and do restores)
-- 
2.47.3

[PATCH proxmox 02/28] pbs-api-types: add remote datastore append privs and role

[Christian Ebner]

While allowing to push/backup to remotes like Remote.DatastoreBackup,
Remote.DatastoreAppend also allows creation of namespaces, but never
deletion/modification as Remote.DatastoreModify would imply, not even
for owned contents.

The role is intended to allow local (source) user configuration for
immutable push sync jobs and is to be set on the user/token on the
remote's datastore ACL path.

This is intended to be used with a remote user on the push target
having Datastore.Audit and Datastore.Append on the target datastore
or sub-namespace.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/acl.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/pbs-api-types/src/acl.rs b/pbs-api-types/src/acl.rs
index 9055dfab..f467db8d 100644
--- a/pbs-api-types/src/acl.rs
+++ b/pbs-api-types/src/acl.rs
@@ -61,6 +61,8 @@ constnamedbitmap! {
         PRIV_REMOTE_MODIFY("Remote.Modify");
         /// Remote.Read allows reading data from a configured `Remote`
         PRIV_REMOTE_READ("Remote.Read");
+        /// Remote.DatastoreAppend allows creating new snapshots and namespaces on remote datastores
+        PRIV_REMOTE_DATASTORE_APPEND("Remote.DatastoreAppend");
         /// Remote.DatastoreBackup allows creating new snapshots on remote datastores
         PRIV_REMOTE_DATASTORE_BACKUP("Remote.DatastoreBackup");
         /// Remote.DatastoreModify allows to modify remote datastores
@@ -183,6 +185,14 @@ pub const ROLE_REMOTE_SYNC_PUSH_OPERATOR: u64 = 0
     | PRIV_REMOTE_AUDIT
     | PRIV_REMOTE_DATASTORE_BACKUP;
 
+#[rustfmt::skip]
+#[allow(clippy::identity_op)]
+/// Remote.SyncAppendOperator can read remote datastores, as well as push snapshots and create
+/// namespaces on the remote.
+pub const ROLE_REMOTE_SYNC_APPEND_OPERATOR: u64 = 0
+    | PRIV_REMOTE_AUDIT
+    | PRIV_REMOTE_DATASTORE_APPEND;
+
 #[rustfmt::skip]
 #[allow(clippy::identity_op)]
 /// Remote.DatastorePowerUser can read and push snapshots to the remote, and prune owned snapshots
@@ -271,6 +281,8 @@ pub enum Role {
     RemoteSyncOperator = ROLE_REMOTE_SYNC_OPERATOR,
     /// Synchronisation Operator (push direction)
     RemoteSyncPushOperator = ROLE_REMOTE_SYNC_PUSH_OPERATOR,
+    /// Synchronisation Operator (append push direction)
+    RemoteSyncAppendOperator = ROLE_REMOTE_SYNC_APPEND_OPERATOR,
     /// Remote Datastore Prune
     RemoteDatastorePowerUser = ROLE_REMOTE_DATASTORE_POWERUSER,
     /// Remote Datastore Admin
-- 
2.47.3

[PATCH proxmox 03/28] pbs-api-types: extend snapshot list items by retention timestamp

[Christian Ebner]

Return the retention timestamp as part of the snapshot list items,
for it to be shown in the UI/CLI outputs just like the protected
flag.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 93ccaf07..53634a7d 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -1526,6 +1526,9 @@ pub struct SnapshotListItem {
     /// Protection from prunes
     #[serde(default)]
     pub protected: bool,
+    /// Retention timestamp (unix epoch)
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub retain_until: Option<i64>,
 }
 
 #[api(
-- 
2.47.3

[PATCH proxmox 04/28] pbs-api-types: extend sync job config by retention-timespan parameter

[Christian Ebner]

Allows to configure a sync job which sets a retention timestamp on
synced snapshots based on the start time of the sync.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/jobs.rs | 64 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 63 insertions(+), 1 deletion(-)

diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 9a5d2b77..6aa4e3a0 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -1,7 +1,8 @@
-use std::str::FromStr;
+use std::{fmt, str::FromStr};
 
 use anyhow::bail;
 use const_format::concatcp;
+use proxmox_time::TimeSpan;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
 
@@ -94,6 +95,61 @@ pub const SYNC_WORKER_THREADS_SCHEMA: Schema = threads_schema(
     1,
 );
 
+pub const RETENTION_TIMESPAN_FORMAT: ApiStringFormat = ApiStringFormat::VerifyFn(|s| {
+    RetentionTimespan::from_str(s)?;
+    Ok(())
+});
+
+pub const RETENTION_TIMESPAN_SCHEMA: Schema = StringSchema::new("Retention timespan")
+    .format(&RETENTION_TIMESPAN_FORMAT)
+    .schema();
+
+#[derive(Clone, Debug, PartialEq, UpdaterType)]
+pub struct RetentionTimespan {
+    timespan: TimeSpan,
+}
+
+impl std::str::FromStr for RetentionTimespan {
+    type Err = anyhow::Error;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let timespan: TimeSpan = s.parse()?;
+        if timespan.subsec_nanos() > 0 {
+            bail!("Sub-second precision not supported by retention timespan");
+        }
+        Ok(Self { timespan })
+    }
+}
+
+impl fmt::Display for RetentionTimespan {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.timespan)
+    }
+}
+
+impl ApiType for RetentionTimespan {
+    const API_SCHEMA: Schema = RETENTION_TIMESPAN_SCHEMA;
+}
+
+impl RetentionTimespan {
+    /// Provide the retention timespan as plain timespan
+    pub fn as_timespan(&self) -> &TimeSpan {
+        &self.timespan
+    }
+
+    /// Calculate the unix epoch for given retenition timespan relative to now from system time.
+    pub fn to_timestamp_from_systemtime(&self) -> Result<i64, anyhow::Error> {
+        // no sub-second precision allowed by parser, safe to skip
+        match proxmox_time::epoch_i64().checked_add_unsigned(self.timespan.as_secs()) {
+            Some(timestamp) => Ok(timestamp),
+            None => bail!("retention timestamp calculation failed with overflow"),
+        }
+    }
+}
+
+serde_plain::derive_serialize_from_display!(RetentionTimespan);
+serde_plain::derive_deserialize_from_fromstr!(RetentionTimespan, "retention timespan");
+
 #[api(
     properties: {
         "next-run": {
@@ -690,6 +746,10 @@ pub const UNMOUNT_ON_SYNC_DONE_SCHEMA: Schema =
             },
             optional: true,
         },
+        "retention-timespan": {
+            type: RetentionTimespan,
+            optional: true,
+        },
     }
 )]
 #[derive(Serialize, Deserialize, Clone, Updater, PartialEq)]
@@ -741,6 +801,8 @@ pub struct SyncJobConfig {
     pub active_encryption_key: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub associated_key: Option<Vec<String>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub retention_timespan: Option<RetentionTimespan>,
 }
 
 impl SyncJobConfig {
-- 
2.47.3

[PATCH proxmox 05/28] pbs-api-types: add maximum retention timespan property to datastore

[Christian Ebner]

Intended to allow administrators to configure a maximum retention
timespan which can be set on snapshots for given datastore, as
otherwise users might be able to prevent snapshots from ever being
cleaned up. Default is no limit.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index 53634a7d..4efecb67 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -24,8 +24,8 @@ use crate::{
     Authid, BACKUP_ID_RE, BACKUP_NS_RE, BACKUP_TIME_RE, BACKUP_TYPE_RE, CryptMode,
     DATASTORE_NOTIFY_STRING_SCHEMA, Fingerprint, GC_SCHEDULE_SCHEMA,
     GROUP_OR_SNAPSHOT_PATH_REGEX_STR, GroupFilter, MaintenanceMode, MaintenanceType,
-    PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX_STR, PRUNE_SCHEDULE_SCHEMA, SHA256_HEX_REGEX,
-    SINGLE_LINE_COMMENT_SCHEMA, SNAPSHOT_PATH_REGEX_STR, UPID, Userid,
+    PROXMOX_SAFE_ID_FORMAT, PROXMOX_SAFE_ID_REGEX_STR, PRUNE_SCHEDULE_SCHEMA, RetentionTimespan,
+    SHA256_HEX_REGEX, SINGLE_LINE_COMMENT_SCHEMA, SNAPSHOT_PATH_REGEX_STR, UPID, Userid,
     VERIFY_JOB_READ_THREADS_SCHEMA, VERIFY_JOB_VERIFY_THREADS_SCHEMA,
 };
 
@@ -509,6 +509,10 @@ pub const COUNTER_RESET_SCHEDULE_SCHEMA: Schema =
             optional: true,
             schema: COUNTER_RESET_SCHEDULE_SCHEMA,
         },
+        "max-retention-timespan": {
+            type: RetentionTimespan,
+            optional: true,
+        },
     }
 )]
 #[derive(Serialize, Deserialize, Updater, Clone, PartialEq)]
@@ -577,6 +581,10 @@ pub struct DataStoreConfig {
     /// Notification threshold related counter reset schedule
     #[serde(skip_serializing_if = "Option::is_none")]
     pub counter_reset_schedule: Option<String>,
+
+    /// Maximum retention timespan which can be set on this datastore
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub max_retention_timespan: Option<RetentionTimespan>,
 }
 
 #[api]
@@ -619,6 +627,7 @@ impl DataStoreConfig {
             backend: None,
             notification_thresholds: None,
             counter_reset_schedule: None,
+            max_retention_timespan: None,
         }
     }
 
-- 
2.47.3

[PATCH proxmox-backup 06/28] api: config: extend sync job config by new retention-timespan

[Christian Ebner]

The sync job config gained an optional retention-timespan parameter,
add it for the test to work with the bumped API types.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/sync.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index 068e2619c..d0e678220 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -829,6 +829,7 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
         worker_threads: None,
         active_encryption_key: None,
         associated_key: None,
+        retention_timespan: None,
     };
 
     // should work without ACLs
-- 
2.47.3

[PATCH proxmox-backup 07/28] api: admin: improve code style for status endpoint

[Christian Ebner]

Reduces intendation level and makes the if branches more concise.

No functional changes.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 32 +++++++++++++-------------------
 1 file changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index bc2b2436e..bb829cd3f 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -718,27 +718,21 @@ pub async fn status(
     let backend_type = datastore.backend_type();
     let s3_statistics = datastore.s3_statistics();
 
-    Ok(if store_stats {
+    let (total, used, avail) = if store_stats {
         let storage = crate::tools::fs::fs_info(datastore.base_path()).await?;
-        DataStoreStatus {
-            total: storage.total,
-            used: storage.used,
-            avail: storage.available,
-            gc_status,
-            counts,
-            backend_type,
-            s3_statistics,
-        }
+        (storage.total, storage.used, storage.available)
     } else {
-        DataStoreStatus {
-            total: 0,
-            used: 0,
-            avail: 0,
-            gc_status,
-            counts,
-            backend_type,
-            s3_statistics,
-        }
+        (0, 0, 0)
+    };
+
+    Ok(DataStoreStatus {
+        total,
+        used,
+        avail,
+        gc_status,
+        counts,
+        backend_type,
+        s3_statistics,
     })
 }
 
-- 
2.47.3

[PATCH proxmox-backup 08/28] client: avoid error in status if user lacks permissions

[Christian Ebner]

In case of missing permissions the total storage space will be
returned as zero, leading to the status output not being rendered
since protected from zero division.

This is however neither user friendly as the error does not reflect
there being a lack of permissions, nor is it the correct behaviour,
since the user still might have access to other information such as
snapshot count (although currently not exposed in the CLI).

Fix this by showing the values as returned by the API and skipping
the percentage display if this would lead to zero division.

While at it, rename the closure to reflect this behaviour and convert
value parsing issues to errors instead of panics.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 proxmox-backup-client/src/main.rs | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index e77661b22..ab77f0bbf 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -2082,23 +2082,27 @@ async fn status(param: Value) -> Result<Value, Error> {
 
     record_repository(&repo);
 
-    let render_total_percentage = |v: &Value, record: &Value| -> Result<String, Error> {
-        let v = v.as_u64().unwrap();
-        let total = record["total"].as_u64().unwrap();
+    let total_with_opt_percentage = |v: &Value, record: &Value| -> Result<String, Error> {
+        let v = v
+            .as_u64()
+            .ok_or_else(|| format_err!("value is not a number"))?;
+        let total = record["total"]
+            .as_u64()
+            .ok_or_else(|| format_err!("total is not a number"))?;
         let roundup = total / 200;
         if let Some(per) = ((v + roundup) * 100).checked_div(total) {
             let info = format!(" ({per} %)");
             Ok(format!("{v} {info:>8}"))
         } else {
-            bail!("Cannot render total percentage: denominator is zero");
+            Ok(v.to_string())
         }
     };
 
     let options = default_table_format_options()
         .noheader(true)
-        .column(ColumnConfig::new("total").renderer(render_total_percentage))
-        .column(ColumnConfig::new("used").renderer(render_total_percentage))
-        .column(ColumnConfig::new("avail").renderer(render_total_percentage));
+        .column(ColumnConfig::new("total").renderer(total_with_opt_percentage))
+        .column(ColumnConfig::new("used").renderer(total_with_opt_percentage))
+        .column(ColumnConfig::new("avail").renderer(total_with_opt_percentage));
 
     let return_type = &API_METHOD_STATUS.returns;
 
-- 
2.47.3

[PATCH proxmox-backup 09/28] server: allow iterating contents for Datastore.Append permissions

[Christian Ebner]

Appending backups to groups in namespaces must be aware of
pre-existing contents, so it must be allowed to iterate those.

Further, sort the privs list alphabetically to improve readability.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/backup/hierarchy.rs | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/backup/hierarchy.rs b/src/backup/hierarchy.rs
index ef8efd323..d81dc3565 100644
--- a/src/backup/hierarchy.rs
+++ b/src/backup/hierarchy.rs
@@ -3,8 +3,8 @@ use std::sync::Arc;
 use anyhow::{Error, bail};
 
 use pbs_api_types::{
-    Authid, BackupNamespace, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY,
-    PRIV_DATASTORE_READ, privs_to_priv_names,
+    Authid, BackupNamespace, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
+    PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_READ, privs_to_priv_names,
 };
 use pbs_config::CachedUserInfo;
 use pbs_datastore::{DataStore, ListGroups, ListNamespacesRecursive, backup_info::BackupGroup};
@@ -151,8 +151,11 @@ impl<'a> ListAccessibleBackupGroups<'a> {
     }
 }
 
-pub static NS_PRIVS_OK: u64 =
-    PRIV_DATASTORE_MODIFY | PRIV_DATASTORE_READ | PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_AUDIT;
+pub static NS_PRIVS_OK: u64 = PRIV_DATASTORE_APPEND
+    | PRIV_DATASTORE_AUDIT
+    | PRIV_DATASTORE_BACKUP
+    | PRIV_DATASTORE_READ
+    | PRIV_DATASTORE_MODIFY;
 
 impl Iterator for ListAccessibleBackupGroups<'_> {
     type Item = Result<BackupGroup, Error>;
-- 
2.47.3

[PATCH proxmox-backup 10/28] api: backup: fix possible information leak in multi-tenant datastores

[Christian Ebner]

Do not leak the owner of the group in permission checks on backup, as
a user with Datastore.Backup only permissions should never be able to
see other users. This is especially of interest if users gained
unintended access to a namespace they should not be allowed to, e.g.
due to misconfiguration.

Rather include the namespace and group information.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/backup/mod.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index 4d3db731b..fe01636d6 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -155,8 +155,13 @@ fn upgrade_to_backup_protocol(
         let correct_owner =
             owner == auth_id || (owner.is_token() && Authid::from(owner.user().clone()) == auth_id);
         if !correct_owner && worker_type != "benchmark" {
+            let namespace_and_group = if !backup_group.backup_ns().is_root() {
+                format!("[{}]:{}", backup_group.backup_ns(), backup_group.group())
+            } else {
+                backup_group.group().to_string()
+            };
             // only the owner is allowed to create additional snapshots
-            bail!("backup owner check failed ({} != {})", auth_id, owner);
+            bail!("backup owner check failed: {namespace_and_group} not owned by {auth_id}");
         }
 
         let last_backup = {
-- 
2.47.3

[PATCH proxmox-backup 11/28] api: backup: allow backup for user/token with append permission

[Christian Ebner]

In addition to these with PRIV_DATASTORE_BACKUP, also users/tokens
with PRIV_DATASTORE_APPEND should be able to create new backup groups
and snapshots.

Since only one of them is required to be set, change the priv check to
be partial only.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/backup/mod.rs | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index fe01636d6..cb54d397c 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -23,7 +23,8 @@ use proxmox_sortable_macro::sortable;
 use pbs_api_types::{
     ArchiveType, Authid, BACKUP_ARCHIVE_NAME_SCHEMA, BACKUP_ID_SCHEMA, BACKUP_NAMESPACE_SCHEMA,
     BACKUP_TIME_SCHEMA, BACKUP_TYPE_SCHEMA, BackupArchiveName, BackupNamespace, BackupType,
-    CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, Operation, PRIV_DATASTORE_BACKUP, VerifyState,
+    CHUNK_DIGEST_SCHEMA, DATASTORE_SCHEMA, Operation, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_BACKUP,
+    VerifyState,
 };
 use pbs_config::CachedUserInfo;
 use pbs_datastore::index::IndexFile;
@@ -94,8 +95,8 @@ fn upgrade_to_backup_protocol(
             .check_privs(
                 &auth_id,
                 &backup_ns.acl_path(&store),
-                PRIV_DATASTORE_BACKUP,
-                false,
+                PRIV_DATASTORE_APPEND | PRIV_DATASTORE_BACKUP,
+                true,
             )
             .map_err(|err| http_err!(FORBIDDEN, "{err}"))?;
 
-- 
2.47.3

[PATCH proxmox-backup 12/28] api: allow namespace creation on append permissions

[Christian Ebner]

Only require full modify permissions for moves and namespace
destruction, allow namespace creation also with append permissions.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/namespace.rs |  8 ++++----
 src/api2/tape/restore.rs    |  6 +++---
 src/backup/hierarchy.rs     | 12 +++++++++++-
 src/server/pull.rs          | 10 +++++-----
 4 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/src/api2/admin/namespace.rs b/src/api2/admin/namespace.rs
index 19e1e8cd0..f874f8129 100644
--- a/src/api2/admin/namespace.rs
+++ b/src/api2/admin/namespace.rs
@@ -56,7 +56,7 @@ pub fn create_namespace(
     let mut ns = parent.clone();
     ns.push(name.clone())?;
 
-    check_ns_modification_privs(&store, &ns, &auth_id)?;
+    check_ns_modification_privs(&store, &ns, &auth_id, false)?;
 
     let lookup = crate::tools::lookup_with(&store, Operation::Write);
     let datastore = DataStore::lookup_datastore(lookup)?;
@@ -166,7 +166,7 @@ pub fn delete_namespace(
 ) -> Result<BackupGroupDeleteStats, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
-    check_ns_modification_privs(&store, &ns, &auth_id)?;
+    check_ns_modification_privs(&store, &ns, &auth_id, true)?;
 
     let lookup = crate::tools::lookup_with(&store, Operation::Write);
     let datastore = DataStore::lookup_datastore(lookup)?;
@@ -247,8 +247,8 @@ pub fn move_namespace(
 ) -> Result<Value, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
 
-    check_ns_modification_privs(&store, &ns, &auth_id)?;
-    check_ns_modification_privs(&store, &target_ns, &auth_id)?;
+    check_ns_modification_privs(&store, &ns, &auth_id, true)?;
+    check_ns_modification_privs(&store, &target_ns, &auth_id, true)?;
 
     let datastore =
         DataStore::lookup_datastore(crate::tools::lookup_with(&store, Operation::Write))?;
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index bb3825aae..df383a810 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -254,9 +254,9 @@ fn check_and_create_namespaces(
         for comp in ns.components() {
             tmp_ns.push(comp.to_string())?;
             if !store.namespace_exists(&tmp_ns) {
-                check_ns_modification_privs(store.name(), &tmp_ns, auth_id).map_err(|_err| {
-                    format_err!("no permission to create namespace '{}'", tmp_ns)
-                })?;
+                check_ns_modification_privs(store.name(), &tmp_ns, auth_id, false).map_err(
+                    |_err| format_err!("no permission to create namespace '{}'", tmp_ns),
+                )?;
 
                 store.create_namespace(&tmp_ns.parent(), comp.to_string())?;
             }
diff --git a/src/backup/hierarchy.rs b/src/backup/hierarchy.rs
index d81dc3565..5cc8fd778 100644
--- a/src/backup/hierarchy.rs
+++ b/src/backup/hierarchy.rs
@@ -20,10 +20,14 @@ pub fn check_ns_privs(
 }
 
 /// Asserts that `privs` for creating/destroying namespace in datastore are fulfilled.
+///
+/// If `needs_full_modify_priv` is not set, `Datastore.Append` is sufficient which must
+/// allow namespace creation only, never deletion.
 pub fn check_ns_modification_privs(
     store: &str,
     ns: &BackupNamespace,
     auth_id: &Authid,
+    needs_full_modify_priv: bool,
 ) -> Result<(), Error> {
     // we could allow it as easy purge-whole datastore, but lets be more restrictive for now
     if ns.is_root() {
@@ -33,7 +37,13 @@ pub fn check_ns_modification_privs(
 
     let parent = ns.parent();
 
-    check_ns_privs(store, &parent, auth_id, PRIV_DATASTORE_MODIFY)
+    let required_privs = if needs_full_modify_priv {
+        PRIV_DATASTORE_MODIFY
+    } else {
+        PRIV_DATASTORE_APPEND | PRIV_DATASTORE_MODIFY
+    };
+
+    check_ns_privs(store, &parent, auth_id, required_privs)
 }
 
 /// Asserts that either either `full_access_privs` or `partial_access_privs` are fulfilled on
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 4eb5bcf11..8c6253604 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -22,8 +22,8 @@ use tokio::io::AsyncWriteExt;
 use pbs_api_types::{
     ArchiveType, Authid, BackupDir, BackupGroup, BackupNamespace, CLIENT_LOG_BLOB_NAME, CryptMode,
     Fingerprint, GroupFilter, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, Operation,
-    PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, RateLimitConfig, Remote, SnapshotListItem,
-    VerifyState, print_store_and_ns,
+    PRIV_DATASTORE_APPEND, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, RateLimitConfig, Remote,
+    SnapshotListItem, VerifyState, print_store_and_ns,
 };
 use pbs_client::BackupRepository;
 use pbs_config::CachedUserInfo;
@@ -1426,7 +1426,7 @@ fn check_and_create_ns(params: &PullParameters, ns: &BackupNamespace) -> Result<
     let store_ns_str = print_store_and_ns(params.target.store.name(), ns);
 
     if !ns.is_root() && !params.target.store.namespace_path(ns).exists() {
-        check_ns_modification_privs(params.target.store.name(), ns, &params.owner)
+        check_ns_modification_privs(params.target.store.name(), ns, &params.owner, false)
             .map_err(|err| format_err!("Creating {ns} not allowed - {err}"))?;
 
         let name = match ns.components().last() {
@@ -1446,7 +1446,7 @@ fn check_and_create_ns(params: &PullParameters, ns: &BackupNamespace) -> Result<
         params.target.store.name(),
         ns,
         &params.owner,
-        PRIV_DATASTORE_BACKUP,
+        PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_APPEND,
     )
     .map_err(|err| format_err!("sync into {store_ns_str} not allowed - {err}"))?;
 
@@ -1454,7 +1454,7 @@ fn check_and_create_ns(params: &PullParameters, ns: &BackupNamespace) -> Result<
 }
 
 fn check_and_remove_ns(params: &PullParameters, local_ns: &BackupNamespace) -> Result<bool, Error> {
-    check_ns_modification_privs(params.target.store.name(), local_ns, &params.owner)
+    check_ns_modification_privs(params.target.store.name(), local_ns, &params.owner, true)
         .map_err(|err| format_err!("Removing {local_ns} not allowed - {err}"))?;
 
     // The outer loop (check_and_remove_vanished_ns) iterates children first, so we only need
-- 
2.47.3

[PATCH proxmox-backup 13/28] api: sync: allow pull to target for user/token with append permission

[Christian Ebner]

Allows appending new backup groups/snapshots to the target datastore
on syncs in pull direction if the user/token has append permissions
and is owner of the backups.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/sync.rs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index d0e678220..ac1039064 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -7,10 +7,10 @@ use proxmox_router::{Permission, Router, RpcEnvironment, http_bail};
 use proxmox_schema::{api, param_bail};
 
 use pbs_api_types::{
-    Authid, JOB_ID_SCHEMA, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY,
-    PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_REMOTE_AUDIT, PRIV_REMOTE_DATASTORE_BACKUP,
-    PRIV_REMOTE_DATASTORE_PRUNE, PRIV_REMOTE_READ, PROXMOX_CONFIG_DIGEST_SCHEMA, SyncJobConfig,
-    SyncJobConfigUpdater,
+    Authid, JOB_ID_SCHEMA, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP,
+    PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ, PRIV_REMOTE_AUDIT,
+    PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_PRUNE, PRIV_REMOTE_READ,
+    PROXMOX_CONFIG_DIGEST_SCHEMA, SyncJobConfig, SyncJobConfigUpdater,
 };
 use pbs_config::sync;
 
@@ -99,7 +99,9 @@ pub fn check_sync_job_modify_access(
             }
 
             // creating backups on target check
-            if ns_anchor_privs & PRIV_DATASTORE_BACKUP == 0 {
+            if ns_anchor_privs & PRIV_DATASTORE_BACKUP == 0
+                && ns_anchor_privs & PRIV_DATASTORE_APPEND == 0
+            {
                 return false;
             }
 
-- 
2.47.3

[PATCH proxmox-backup 14/28] sync: pull: allow pulling for user/token with append permission

[Christian Ebner]

Extends the access checks for syncs in pull direction to allow
appending contents when user has Datastore.Append permissions.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/pull.rs   | 10 +++++-----
 src/server/pull.rs | 17 ++++++++++-------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 17c1e4f34..6b53c55b7 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -8,9 +8,9 @@ use proxmox_schema::api;
 
 use pbs_api_types::{
     Authid, BackupNamespace, CRYPT_KEY_ID_SCHEMA, DATASTORE_SCHEMA, GROUP_FILTER_LIST_SCHEMA,
-    GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_PRUNE,
-    PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, RESYNC_CORRUPT_SCHEMA,
-    RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
+    GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_BACKUP,
+    PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA,
+    RESYNC_CORRUPT_SCHEMA, RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
     SYNC_WORKER_THREADS_SCHEMA, SyncJobConfig, TRANSFER_LAST_SCHEMA,
 };
 use pbs_config::CachedUserInfo;
@@ -36,8 +36,8 @@ pub fn check_pull_privs(
     user_info.check_privs(
         auth_id,
         &local_store_ns_acl_path,
-        PRIV_DATASTORE_BACKUP,
-        false,
+        PRIV_DATASTORE_APPEND | PRIV_DATASTORE_BACKUP,
+        true,
     )?;
 
     if let Some(remote) = remote {
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 8c6253604..856e0fc5a 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -1442,13 +1442,16 @@ fn check_and_create_ns(params: &PullParameters, ns: &BackupNamespace) -> Result<
         created = true;
     }
 
-    check_ns_privs(
-        params.target.store.name(),
-        ns,
-        &params.owner,
-        PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_APPEND,
-    )
-    .map_err(|err| format_err!("sync into {store_ns_str} not allowed - {err}"))?;
+    let privs = if params.resync_corrupt {
+        PRIV_DATASTORE_BACKUP
+    } else {
+        PRIV_DATASTORE_BACKUP | PRIV_DATASTORE_APPEND
+    };
+
+    check_ns_privs(params.target.store.name(), ns, &params.owner, privs).map_err(|err| {
+        let prefix = if params.resync_corrupt { "re-" } else { "" };
+        format_err!("{prefix}sync into {store_ns_str} not allowed - {err}")
+    })?;
 
     Ok(created)
 }
-- 
2.47.3

[PATCH proxmox-backup 15/28] sync: push: allow push and ns creation on Remote.DatastoreAppend

[Christian Ebner]

A local sync job user with Remote.DatastoreAppend permissions is
intended to be able to append backups on the remote just like
Remote.DatastoreBackup, but also create namespaces not present on
the remote, which would otherwise require Remote.DatastoreModify.
The latter would however also allow for namespace destruction, so
cannot be used for this.

These are push job source checks only, the user connecting to the
remote as configured in the remote config must be setup on the
target PBS instance accordingly (i.e. with Datastore.Audit and
Datastore.Append on the target datastore and sub-namespace).

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/push.rs | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index 9a69f5ce8..83e4ea22d 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -15,8 +15,9 @@ use pbs_api_types::{
     ApiVersion, ApiVersionInfo, ArchiveType, Authid, BackupArchiveName, BackupDir, BackupGroup,
     BackupGroupDeleteStats, BackupNamespace, CLIENT_LOG_BLOB_NAME, CryptMode, GroupFilter,
     GroupListItem, MANIFEST_BLOB_NAME, NamespaceListItem, Operation, PRIV_DATASTORE_BACKUP,
-    PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_MODIFY,
-    PRIV_REMOTE_DATASTORE_PRUNE, RateLimitConfig, Remote, SnapshotListItem, print_store_and_ns,
+    PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_APPEND, PRIV_REMOTE_DATASTORE_BACKUP,
+    PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE, RateLimitConfig, Remote,
+    SnapshotListItem, print_store_and_ns,
 };
 use pbs_client::{
     BackupRepository, BackupStats, BackupWriter, BackupWriterOptions, HttpClient, IndexType,
@@ -211,7 +212,7 @@ fn check_ns_remote_datastore_privs(
     let acl_path =
         target_namespace.remote_acl_path(&params.target.remote.name, params.target.repo.store());
 
-    user_info.check_privs(&params.local_user, &acl_path, privs, false)?;
+    user_info.check_privs(&params.local_user, &acl_path, privs, true)?;
 
     Ok(())
 }
@@ -353,7 +354,8 @@ async fn check_or_create_target_namespace(
         // Namespace not present on target, create namespace.
         // Sub-namespaces have to be created by creating parent components first.
 
-        check_ns_remote_datastore_privs(params, target_namespace, PRIV_REMOTE_DATASTORE_MODIFY)
+        let privs = PRIV_REMOTE_DATASTORE_MODIFY | PRIV_REMOTE_DATASTORE_APPEND;
+        check_ns_remote_datastore_privs(params, target_namespace, privs)
             .context("Creating remote namespace not allowed")?;
 
         let mut parent = BackupNamespace::root();
@@ -550,7 +552,8 @@ pub(crate) async fn push_namespace(
 ) -> Result<(StoreProgress, SyncStats, bool), Error> {
     let target_namespace = params.map_to_target(namespace)?;
     // Check if user is allowed to perform backups on remote datastore
-    check_ns_remote_datastore_privs(&params, &target_namespace, PRIV_REMOTE_DATASTORE_BACKUP)
+    let privs = PRIV_REMOTE_DATASTORE_BACKUP | PRIV_REMOTE_DATASTORE_APPEND;
+    check_ns_remote_datastore_privs(&params, &target_namespace, privs)
         .context("Pushing to remote namespace not allowed")?;
 
     let mut list: Vec<BackupGroup> = params
-- 
2.47.3

[PATCH proxmox-backup 16/28] datastore: conditionally treat missing manifest as error or bening

[Christian Ebner]

A backup snapshot without a manifest is generally a transient state
during ongoing backups. In other cases it must however be treated
as error.

Therefore, refactor the manifest and blob loading logic to
conditionally allow for missing manifest blob files. By this the
caller is in control whether to treat the missing file as error or
regular operation.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 52 +++++++++++++++++++++++++++-----
 src/tools/mod.rs                 | 13 ++------
 2 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index be4ec8b3e..f476d4469 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -648,11 +648,26 @@ impl BackupDir {
         let mut path = self.full_path();
         path.push(filename);
 
-        proxmox_lang::try_block!({
-            let mut file = std::fs::File::open(&path)?;
-            DataBlob::load_from_reader(&mut file)
-        })
-        .map_err(|err| format_err!("unable to load blob '{:?}' - {}", path, err))
+        self.load_blob_conditionally(&path, true)
+            .map_err(|err| format_err!("unable to load blob '{path:?}' - {err}"))?
+            .ok_or(format_err!("unable to load missing blob '{path:?}'"))
+    }
+
+    /// Load the `DataBlob` from given full path, to be constructed and verified by the caller.
+    ///
+    /// If the blob file does not exist and `not_found_is_error` is `true`, propagates the error
+    /// while otherwise `Ok(None)` is returned.
+    fn load_blob_conditionally(
+        &self,
+        full_path: &Path,
+        not_found_is_error: bool,
+    ) -> Result<Option<DataBlob>, Error> {
+        let opt_data_blob = match std::fs::File::open(full_path) {
+            Ok(mut file) => Some(DataBlob::load_from_reader(&mut file)?),
+            Err(err) if err.kind() == std::io::ErrorKind::NotFound && !not_found_is_error => None,
+            Err(err) => return Err(err.into()),
+        };
+        Ok(opt_data_blob)
     }
 
     /// Generate the full archive file path with given archive name including server side type
@@ -989,11 +1004,34 @@ impl BackupDir {
     }
 
     /// Load the manifest without a lock. Must not be written back.
+    ///
+    /// A missing manifest file for the snapshot is treated as error.
     pub fn load_manifest(&self) -> Result<(BackupManifest, u64), Error> {
-        let blob = self.load_blob(MANIFEST_BLOB_NAME.as_ref())?;
+        self.load_manifest_optionally(true)?.ok_or_else(|| {
+            format_err!(
+                "unable to load missing manifest '{:?}'",
+                self.full_path().join(MANIFEST_BLOB_NAME.as_ref()),
+            )
+        })
+    }
+
+    /// Load the manifest without a lock. Must not be written back.
+    ///
+    /// If the manifest file for the snapshot is missing and `missing_manifest_is_error` is
+    /// `true`, propagates the error while otherwise `Ok(None)` is returned.
+    pub fn load_manifest_optionally(
+        &self,
+        missing_manifest_is_error: bool,
+    ) -> Result<Option<(BackupManifest, u64)>, Error> {
+        let mut full_path = self.full_path();
+        full_path.push(MANIFEST_BLOB_NAME.as_ref());
+        let Some(blob) = self.load_blob_conditionally(&full_path, missing_manifest_is_error)?
+        else {
+            return Ok(None);
+        };
         let raw_size = blob.raw_size();
         let manifest = BackupManifest::try_from(blob)?;
-        Ok((manifest, raw_size))
+        Ok(Some((manifest, raw_size)))
     }
 
     /// Update the manifest of the specified snapshot. Never write a manifest directly,
diff --git a/src/tools/mod.rs b/src/tools/mod.rs
index ab624d7a6..4e2dd38e8 100644
--- a/src/tools/mod.rs
+++ b/src/tools/mod.rs
@@ -47,17 +47,8 @@ pub fn setup_safe_path_env() {
 pub(crate) fn read_backup_index(
     backup_dir: &BackupDir,
 ) -> Result<Option<(BackupManifest, Vec<BackupContent>)>, Error> {
-    let (manifest, index_size) = match backup_dir.load_manifest() {
-        Ok((manifest, index_size)) => (manifest, index_size),
-        Err(err) => {
-            let mut manifest_path = backup_dir.full_path();
-            manifest_path.push(MANIFEST_BLOB_NAME.as_ref());
-            if !manifest_path.exists() {
-                return Ok(None);
-            } else {
-                return Err(err);
-            }
-        }
+    let Some((manifest, index_size)) = backup_dir.load_manifest_optionally(false)? else {
+        return Ok(None);
     };
 
     let mut result = Vec::new();
-- 
2.47.3

[PATCH proxmox-backup 17/28] api: backup: provide retain-until timestamp for extended prune protection

[Christian Ebner]

Allows to protect snapshots from being pruned by providing a retention
timestamp during backup.

Pruning must now also parse the manifest if present, which adds some
additional overhead on snapshot forget operations. This is however not
a performance critical code path as prunes run either in a job anyways,
where a slight delay for each prune is acceptable or a single snapshot
forget via API/CLI.

However, as a side effect manifests which cannot be parsed can no
longer be pruned without manual intervention.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/backup_info.rs | 11 +++++++++++
 pbs-datastore/src/manifest.rs    | 27 +++++++++++++++++++++++++++
 src/api2/backup/environment.rs   |  4 ++++
 src/api2/backup/mod.rs           | 12 ++++++++++++
 4 files changed, 54 insertions(+)

diff --git a/pbs-datastore/src/backup_info.rs b/pbs-datastore/src/backup_info.rs
index f476d4469..13eb0cdf6 100644
--- a/pbs-datastore/src/backup_info.rs
+++ b/pbs-datastore/src/backup_info.rs
@@ -924,6 +924,17 @@ impl BackupDir {
             bail!("cannot remove protected snapshot"); // use special error type?
         }
 
+        if let Some((manifest, _size)) = self.load_manifest_optionally(false)? {
+            if let Some(retain_until) = manifest.retention_timestamp()? {
+                if retain_until > proxmox_time::epoch_i64() {
+                    bail!(
+                        "snapshot protected by retention until {}",
+                        proxmox_time::epoch_to_rfc3339(retain_until)?
+                    );
+                }
+            }
+        }
+
         if let DatastoreBackend::S3(s3_client) = backend {
             let path = self.relative_path();
             let snapshot_prefix = path
diff --git a/pbs-datastore/src/manifest.rs b/pbs-datastore/src/manifest.rs
index 11de1085b..a31c3ea7d 100644
--- a/pbs-datastore/src/manifest.rs
+++ b/pbs-datastore/src/manifest.rs
@@ -82,6 +82,33 @@ impl BackupManifest {
         Ok(())
     }
 
+    /// Set a retention timestamp for given manifest. Snapshot destruction is only allowed after
+    /// this time, the caller must assure a valid timestamp is passed and the caller has privileges
+    /// to do so. Providing `None` will clear the timestamp.
+    pub fn add_retention_timestamp(&mut self, timestamp: Option<i64>) {
+        match timestamp {
+            Some(timestamp) => self.unprotected["retain-until"] = timestamp.into(),
+            None => {
+                self.unprotected["retain-until"].take();
+            }
+        }
+    }
+
+    /// Parse the retention timestamp for given manifest, if set.
+    /// Returns `Ok(None)` if not set and errors when set but parsing failed.
+    pub fn retention_timestamp(&self) -> Result<Option<i64>, Error> {
+        match &self.unprotected["retain-until"] {
+            Value::Number(timestamp) => {
+                let retain_until = timestamp.as_i64().ok_or_else(|| {
+                    format_err!("unexpected value for retain-until timestamp: {timestamp}")
+                })?;
+                Ok(Some(retain_until))
+            }
+            Value::Null => Ok(None),
+            _ => bail!("unexpected value variant for retain-until timestamp"),
+        }
+    }
+
     pub fn files(&self) -> &[FileInfo] {
         &self.files[..]
     }
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index be70d74f8..ce2be070c 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -154,6 +154,7 @@ pub struct BackupEnvironment {
     pub backup_dir: BackupDir,
     pub last_backup: Option<BackupInfo>,
     pub backend: DatastoreBackend,
+    pub retain_until: Option<i64>,
     state: Arc<Mutex<SharedBackupState>>,
 }
 
@@ -166,6 +167,7 @@ impl BackupEnvironment {
         backup_dir: BackupDir,
         no_cache: bool,
         backup_lock_guards: BackupLockGuards,
+        retain_until: Option<i64>,
     ) -> Result<Self, Error> {
         let state = SharedBackupState {
             finished: BackupState::Active,
@@ -195,6 +197,7 @@ impl BackupEnvironment {
             last_backup: None,
             backend,
             state: Arc::new(Mutex::new(state)),
+            retain_until,
         })
     }
 
@@ -797,6 +800,7 @@ impl BackupEnvironment {
 
         // check for valid manifest and store stats
         manifest.unprotected["chunk_upload_stats"] = stats;
+        manifest.add_retention_timestamp(self.retain_until);
 
         for file_info in manifest.files() {
             let archive_name = file_info.filename.as_ref();
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index cb54d397c..cfce13943 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -52,6 +52,7 @@ pub const API_METHOD_UPGRADE_BACKUP: ApiMethod = ApiMethod::new(
             ("backup-type", false, &BACKUP_TYPE_SCHEMA),
             ("backup-id", false, &BACKUP_ID_SCHEMA),
             ("backup-time", false, &BACKUP_TIME_SCHEMA),
+            ("retain-until", true, &BACKUP_TIME_SCHEMA),
             ("debug", true, &BooleanSchema::new("Enable verbose debug logging.").schema()),
             ("benchmark", true, &BooleanSchema::new("Job is a benchmark (do not keep data).").schema()),
             ("no-cache", true, &BooleanSchema::new("Disable local datastore cache for network storages").schema()),
@@ -88,6 +89,16 @@ fn upgrade_to_backup_protocol(
         let store = required_string_param(&param, "store")?.to_owned();
         let backup_ns = optional_ns_param(&param)?;
         let backup_dir_arg = pbs_api_types::BackupDir::deserialize(&param)?;
+        let retain_until = match &param["retain-until"] {
+            Value::Number(timestamp) => {
+                let timestamp = timestamp
+                    .as_i64()
+                    .ok_or_else(|| format_err!("invalid retain-until timestamp"))?;
+                Some(timestamp)
+            }
+            Value::Null => None,
+            _ => bail!("unexpected retain-until timestamp variant"),
+        };
 
         let user_info = CachedUserInfo::new()?;
 
@@ -233,6 +244,7 @@ fn upgrade_to_backup_protocol(
                     backup_dir,
                     no_cache,
                     backup_lock_guards,
+                    retain_until,
                 )?;
 
                 env.debug = debug;
-- 
2.47.3

[PATCH proxmox-backup 18/28] tools: include retain-until timestamp in snapshot list items

[Christian Ebner]

Include the parsed unix epoch timestamp in the listing, for it to be
shown in the UI/CLI output analogous to the protected flag.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/tools/mod.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/tools/mod.rs b/src/tools/mod.rs
index 4e2dd38e8..2bfe71f35 100644
--- a/src/tools/mod.rs
+++ b/src/tools/mod.rs
@@ -133,6 +133,14 @@ pub(crate) fn backup_info_to_snapshot_list_item(
 
             let size = Some(files.iter().map(|x| x.size.unwrap_or(0)).sum());
 
+            let retain_until = match manifest.retention_timestamp() {
+                Ok(opt_timestamp) => opt_timestamp,
+                Err(err) => {
+                    eprintln!("error parsing retain-until timestamp: '{err}'");
+                    None
+                }
+            };
+
             return SnapshotListItem {
                 backup,
                 comment,
@@ -142,6 +150,7 @@ pub(crate) fn backup_info_to_snapshot_list_item(
                 size,
                 owner,
                 protected,
+                retain_until,
             };
         }
         Ok(None) => (),
@@ -169,6 +178,7 @@ pub(crate) fn backup_info_to_snapshot_list_item(
         size: None,
         owner,
         protected,
+        retain_until: None,
     }
 }
 
-- 
2.47.3

[PATCH proxmox-backup 19/28] client: backup writer: allow to send retain-until timestamp on backup

[Christian Ebner]

Extend the client options to allow configurartion of a retain-until
timestamp for the to be create backup snapshot on the PBS datastore.

If the server api supports it, the timestamp will be picked up and
processed.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 examples/upload-speed.rs               | 1 +
 pbs-client/src/backup_writer.rs        | 5 +++++
 proxmox-backup-client/src/benchmark.rs | 1 +
 proxmox-backup-client/src/main.rs      | 1 +
 4 files changed, 8 insertions(+)

diff --git a/examples/upload-speed.rs b/examples/upload-speed.rs
index 5a63e0f09..09a5f2042 100644
--- a/examples/upload-speed.rs
+++ b/examples/upload-speed.rs
@@ -27,6 +27,7 @@ async fn upload_speed() -> Result<f64, Error> {
             debug: false,
             benchmark: true,
             no_cache: false,
+            retain_until: None,
         },
     )
     .await?;
diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index b0b6a5133..1e32994ca 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -98,6 +98,8 @@ pub struct BackupWriterOptions<'a> {
     pub benchmark: bool,
     /// Skip datastore cache
     pub no_cache: bool,
+    /// Set retention timestamp for snapshot on PBS
+    pub retain_until: Option<i64>,
 }
 
 impl BackupWriter {
@@ -125,6 +127,9 @@ impl BackupWriter {
         if writer_options.no_cache {
             param["no-cache"] = serde_json::to_value(writer_options.no_cache)?;
         }
+        if let Some(timestamp) = writer_options.retain_until {
+            param["retain-until"] = serde_json::to_value(timestamp)?;
+        }
 
         if !writer_options.ns.is_root() {
             param["ns"] = serde_json::to_value(writer_options.ns)?;
diff --git a/proxmox-backup-client/src/benchmark.rs b/proxmox-backup-client/src/benchmark.rs
index a937bacb9..89bc2396b 100644
--- a/proxmox-backup-client/src/benchmark.rs
+++ b/proxmox-backup-client/src/benchmark.rs
@@ -246,6 +246,7 @@ async fn test_upload_speed(
             debug: false,
             benchmark: true,
             no_cache,
+            retain_until: None,
         },
     )
     .await?;
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index ab77f0bbf..c9b89368f 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1046,6 +1046,7 @@ async fn create_backup(
             debug: true,
             benchmark: false,
             no_cache,
+            retain_until: None,
         },
     )
     .await?;
-- 
2.47.3

[PATCH proxmox-backup 20/28] sync: push: allow to set retention timestamp for synced snapshots

[Christian Ebner]

Allow the push sync job to present the retention timestamp when
pushing the snapshot to the remote, if the remote supports it.

The timestamp is calculated based on the start time of the sync job
by push parameter construction.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/push.rs   |  8 +++++++-
 src/server/push.rs | 19 ++++++++++++++++++-
 src/server/sync.rs |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/api2/push.rs b/src/api2/push.rs
index 16bcdf85b..e74bf78c3 100644
--- a/src/api2/push.rs
+++ b/src/api2/push.rs
@@ -5,7 +5,7 @@ use pbs_api_types::{
     Authid, BackupNamespace, CRYPT_KEY_ID_SCHEMA, DATASTORE_SCHEMA, GROUP_FILTER_LIST_SCHEMA,
     GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ,
     PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_PRUNE, REMOTE_ID_SCHEMA,
-    REMOVE_VANISHED_BACKUPS_SCHEMA, RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA,
+    REMOVE_VANISHED_BACKUPS_SCHEMA, RateLimitConfig, RetentionTimespan, SYNC_ENCRYPTED_ONLY_SCHEMA,
     SYNC_VERIFIED_ONLY_SCHEMA, SYNC_WORKER_THREADS_SCHEMA, TRANSFER_LAST_SCHEMA,
 };
 use proxmox_rest_server::WorkerTask;
@@ -116,6 +116,10 @@ fn check_push_privs(
                 schema: CRYPT_KEY_ID_SCHEMA,
                 optional: true,
             },
+            "retention-timespan": {
+                type: RetentionTimespan,
+                optional: true,
+            },
         },
     },
     access: {
@@ -143,6 +147,7 @@ async fn push(
     transfer_last: Option<usize>,
     worker_threads: Option<usize>,
     encryption_key: Option<String>,
+    retention_timespan: Option<RetentionTimespan>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -176,6 +181,7 @@ async fn push(
         transfer_last,
         worker_threads,
         encryption_key,
+        retention_timespan,
     )
     .await?;
 
diff --git a/src/server/push.rs b/src/server/push.rs
index 83e4ea22d..617b2ba9b 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -17,7 +17,7 @@ use pbs_api_types::{
     GroupListItem, MANIFEST_BLOB_NAME, NamespaceListItem, Operation, PRIV_DATASTORE_BACKUP,
     PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_APPEND, PRIV_REMOTE_DATASTORE_BACKUP,
     PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE, RateLimitConfig, Remote,
-    SnapshotListItem, print_store_and_ns,
+    RetentionTimespan, SnapshotListItem, print_store_and_ns,
 };
 use pbs_client::{
     BackupRepository, BackupStats, BackupWriter, BackupWriterOptions, HttpClient, IndexType,
@@ -98,6 +98,9 @@ pub(crate) struct PushParameters {
     /// Encryption key to use for pushing unencrypted backup snapshots. Does not affect
     /// already encrypted snapshots.
     crypt_config: Option<(String, Arc<CryptConfig>)>,
+    /// Retention timestamp to be set for newly pushed snapshots on sync target,
+    /// overwrites existing retentions if present on source manifest.
+    retain_until: Option<i64>,
 }
 
 impl PushParameters {
@@ -119,6 +122,7 @@ impl PushParameters {
         transfer_last: Option<usize>,
         worker_threads: Option<usize>,
         active_encryption_key: Option<String>,
+        retention_timespan: Option<RetentionTimespan>,
     ) -> Result<Self, Error> {
         if let Some(max_depth) = max_depth {
             ns.check_max_depth(max_depth)?;
@@ -161,6 +165,17 @@ impl PushParameters {
             bail!("Unsupported remote api version, minimum v2.2 required");
         }
 
+        let retain_until = retention_timespan
+            .map(|timespan| {
+                if api_version < ApiVersion::new(4, 2, 6) {
+                    bail!(
+                        "Unsupported 'retain-until' by remote api version, minimum v4.2.6 required"
+                    );
+                }
+                timespan.to_timestamp_from_systemtime()
+            })
+            .transpose()?;
+
         let supports_prune_delete_stats = api_version >= ApiVersion::new(3, 2, 11);
 
         let target = PushTarget {
@@ -192,6 +207,7 @@ impl PushParameters {
             transfer_last,
             worker_threads,
             crypt_config,
+            retain_until,
         })
     }
 
@@ -1113,6 +1129,7 @@ pub(crate) async fn push_snapshot(
             debug: false,
             benchmark: false,
             no_cache: false,
+            retain_until: params.retain_until,
         },
     )
     .await
diff --git a/src/server/sync.rs b/src/server/sync.rs
index 11f30d318..7b9510be2 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -752,6 +752,7 @@ pub fn do_sync_job(
                             sync_job.transfer_last,
                             sync_job.worker_threads,
                             sync_job.active_encryption_key,
+                            sync_job.retention_timespan,
                         )
                         .await?;
                         push_store(push_params).await?
-- 
2.47.3

[PATCH proxmox-backup 21/28] sync: pull: allow to set retention timestamp for synced snapshots

[Christian Ebner]

Allow the pull sync job to set the retention timestamp when pulling
the snapshot from the remote.

The timestamp is calculated based on the start time of the sync job
by pull parameter construction.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/pull.rs   | 11 +++++++++--
 src/server/pull.rs | 24 ++++++++++++++++++++++--
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index 6b53c55b7..c197c161f 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -10,8 +10,8 @@ use pbs_api_types::{
     Authid, BackupNamespace, CRYPT_KEY_ID_SCHEMA, DATASTORE_SCHEMA, GROUP_FILTER_LIST_SCHEMA,
     GroupFilter, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_APPEND, PRIV_DATASTORE_BACKUP,
     PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA,
-    RESYNC_CORRUPT_SCHEMA, RateLimitConfig, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
-    SYNC_WORKER_THREADS_SCHEMA, SyncJobConfig, TRANSFER_LAST_SCHEMA,
+    RESYNC_CORRUPT_SCHEMA, RateLimitConfig, RetentionTimespan, SYNC_ENCRYPTED_ONLY_SCHEMA,
+    SYNC_VERIFIED_ONLY_SCHEMA, SYNC_WORKER_THREADS_SCHEMA, SyncJobConfig, TRANSFER_LAST_SCHEMA,
 };
 use pbs_config::CachedUserInfo;
 use proxmox_rest_server::WorkerTask;
@@ -93,6 +93,7 @@ impl TryFrom<&SyncJobConfig> for PullParameters {
             sync_job.resync_corrupt,
             sync_job.worker_threads,
             sync_job.associated_key.clone(),
+            sync_job.retention_timespan.clone(),
         )
     }
 }
@@ -162,6 +163,10 @@ impl TryFrom<&SyncJobConfig> for PullParameters {
                 },
                 optional: true,
             },
+            "retention-timespan": {
+                type: RetentionTimespan,
+                optional: true,
+            },
         },
     },
     access: {
@@ -191,6 +196,7 @@ async fn pull(
     resync_corrupt: Option<bool>,
     worker_threads: Option<usize>,
     decryption_keys: Option<Vec<String>>,
+    retention_timespan: Option<RetentionTimespan>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
     let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
@@ -233,6 +239,7 @@ async fn pull(
         resync_corrupt,
         worker_threads,
         decryption_keys,
+        retention_timespan,
     )?;
 
     // fixme: set to_stdout to false?
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 856e0fc5a..816e64ce7 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -23,7 +23,7 @@ use pbs_api_types::{
     ArchiveType, Authid, BackupDir, BackupGroup, BackupNamespace, CLIENT_LOG_BLOB_NAME, CryptMode,
     Fingerprint, GroupFilter, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, Operation,
     PRIV_DATASTORE_APPEND, PRIV_DATASTORE_AUDIT, PRIV_DATASTORE_BACKUP, RateLimitConfig, Remote,
-    SnapshotListItem, VerifyState, print_store_and_ns,
+    RetentionTimespan, SnapshotListItem, VerifyState, print_store_and_ns,
 };
 use pbs_client::BackupRepository;
 use pbs_config::CachedUserInfo;
@@ -77,6 +77,9 @@ pub(crate) struct PullParameters {
     worker_threads: Option<usize>,
     /// Decryption key ids and configs to decrypt snapshots with matching key fingerprint
     crypt_configs: Vec<(String, Arc<CryptConfig>)>,
+    // Retention timestamp to be set for newly pulled snapshots on sync target, overwrites
+    // existing retentions if present on source manifest.
+    retain_until: Option<i64>,
 }
 
 impl PullParameters {
@@ -99,6 +102,7 @@ impl PullParameters {
         resync_corrupt: Option<bool>,
         worker_threads: Option<usize>,
         decryption_keys: Option<Vec<String>>,
+        retention_timespan: Option<RetentionTimespan>,
     ) -> Result<Self, Error> {
         if let Some(max_depth) = max_depth {
             ns.check_max_depth(max_depth)?;
@@ -140,6 +144,10 @@ impl PullParameters {
 
         let group_filter = group_filter.unwrap_or_default();
 
+        let retain_until = retention_timespan
+            .map(|timespan| timespan.to_timestamp_from_systemtime())
+            .transpose()?;
+
         let crypt_configs = if let Some(key_ids) = &decryption_keys {
             let mut crypt_configs = Vec::with_capacity(key_ids.len());
             for key_id in key_ids {
@@ -165,6 +173,7 @@ impl PullParameters {
             resync_corrupt,
             worker_threads,
             crypt_configs,
+            retain_until,
         })
     }
 }
@@ -768,7 +777,14 @@ async fn pull_snapshot<'a>(
     )
     .await?
     {
-        (None, false) => (None, None), // regular pull without decryption
+        (None, false) => {
+            let new_manifest = if params.retain_until.is_some() {
+                Some(Arc::new(Mutex::new(BackupManifest::new(snapshot.into()))))
+            } else {
+                None
+            };
+            (None, new_manifest)
+        }
         (Some(crypt_config), false) => {
             // decrypt while pull
             let new_manifest = Arc::new(Mutex::new(BackupManifest::new(snapshot.into())));
@@ -875,6 +891,10 @@ async fn pull_snapshot<'a>(
             new_manifest.set_sync_source_signature(expected.bytes())?;
         }
 
+        if params.retain_until.is_some() {
+            new_manifest.add_retention_timestamp(params.retain_until);
+        }
+
         // keep signature
         let manifest_blob = new_manifest.to_data_blob(None)?;
         // update contents to be uploaded to backend
-- 
2.47.3

[PATCH proxmox-backup 22/28] sync: pull: protect retained snapshot from being overwritten

[Christian Ebner]

During a resync, a protected snapshot might be overwritten, which
must not be allowed if it is still protected by a retention timespan.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/pull.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index 816e64ce7..893b642d4 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -768,6 +768,17 @@ async fn pull_snapshot<'a>(
         return Ok(None);
     }
 
+    if let Some(manifest) = &existing_target_manifest {
+        if let Some(retain_until) = manifest.retention_timestamp()? {
+            if retain_until > proxmox_time::epoch_i64() {
+                bail!(
+                    "Cannot resync target manifest since retention locked until '{}'",
+                    proxmox_time::epoch_to_rfc3339(retain_until)?,
+                );
+            }
+        }
+    }
+
     let (crypt_config, new_manifest) = match optionally_use_decryption_key(
         Arc::clone(&params),
         &manifest,
-- 
2.47.3

[PATCH proxmox-backup 23/28] api: config: allow to set or delete reteniton timespan for sync jobs

[Christian Ebner]

Extend the sync job config API handler to allow for a retention
timespan to be configured or removed.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/config/sync.rs | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index ac1039064..71ade1792 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -400,6 +400,8 @@ pub enum DeletableProperty {
     ActiveEncryptionKey,
     /// Delete associated key property,
     AssociatedKey,
+    /// Delete retention-timespan property
+    RetentionTimespan,
 }
 
 #[api(
@@ -540,6 +542,9 @@ pub fn update_sync_job(
                     // Previous active encryption key might be added as associated below.
                     data.associated_key = None;
                 }
+                DeletableProperty::RetentionTimespan => {
+                    data.retention_timespan = None;
+                }
             }
         }
         keep_previous_key_as_associated(
@@ -692,6 +697,10 @@ pub fn update_sync_job(
         }
     }
 
+    if update.retention_timespan.is_some() {
+        data.retention_timespan = update.retention_timespan;
+    }
+
     if !check_sync_job_modify_access(&user_info, &auth_id, &data) {
         bail!("permission check failed");
     }
-- 
2.47.3

[PATCH proxmox-backup 24/28] ui: add retention timespan form and use it for sync job edit window

[Christian Ebner]

Based on the calender event form, the retention timespan form
provides some useful template values for snapshot retention to
be set on sync jobs.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Makefile                  |  1 +
 www/form/RetentionTimespan.js | 21 +++++++++++++++++++++
 www/window/SyncJobEdit.js     |  9 +++++++++
 3 files changed, 31 insertions(+)
 create mode 100644 www/form/RetentionTimespan.js

diff --git a/www/Makefile b/www/Makefile
index 568836455..e14a17507 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -56,6 +56,7 @@ JSSRC=							\
 	form/GroupFilter.js				\
 	form/VerifyOutdatedAfter.js			\
 	form/EncryptionKeySelector.js			\
+	form/RetentionTimespan.js			\
 	data/RunningTasksStore.js			\
 	button/TaskButton.js				\
 	panel/PrunePanel.js				\
diff --git a/www/form/RetentionTimespan.js b/www/form/RetentionTimespan.js
new file mode 100644
index 000000000..9a84c59a2
--- /dev/null
+++ b/www/form/RetentionTimespan.js
@@ -0,0 +1,21 @@
+Ext.define('PBS.data.RetentionTimespanTemplates', {
+    extend: 'Ext.data.Store',
+    alias: 'store.retentionTimespanTemplates',
+
+    field: ['value', 'text'],
+    data: [
+        { value: '1 day', text: Ext.String.format(gettext('Retain for {0} day'), 1) },
+        { value: '4 weeks', text: Ext.String.format(gettext('Retain for {0} weeks'), 4) },
+        { value: '1 month', text: Ext.String.format(gettext('Retain for {0} month'), 1) },
+        { value: '1 year', text: Ext.String.format(gettext('Retain for {0} year'), 1) },
+    ],
+});
+
+Ext.define('PBS.form.RetentionTimespan', {
+    extend: 'PBS.form.CalendarEvent',
+    alias: 'widget.pbsRetentionTimespan',
+
+    store: {
+        type: 'retentionTimespanTemplates',
+    },
+});
diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index 961ce2951..91519f95a 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -231,6 +231,15 @@ Ext.define('PBS.window.SyncJobEdit', {
                         // NOTE: handle deleteEmpty in onGetValues due to bandwidth field having a cbind too,
                         // further hide rate limit field depending on sync direction in controller init.
                     },
+                    {
+                        xtype: 'pbsRetentionTimespan',
+                        name: 'retention-timespan',
+                        fieldLabel: gettext('Retention Timespan'),
+                        emptyText: gettext('keep retention from snapshot'),
+                        cbind: {
+                            deleteEmpty: '{!isCreate}',
+                        },
+                    },
                 ],
 
                 column2: [
-- 
2.47.3

[PATCH proxmox-backup 25/28] datastore/config: parse and enforce maximum retention timespan

[Christian Ebner]

Allows to limit the maximum retention timespan which can be set on
snapshots, so users cannot block contents for unintended extended
periods of time.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 10 +++++++++-
 src/api2/backup/mod.rs         |  9 +++++++++
 src/api2/config/datastore.rs   |  9 +++++++++
 src/server/pull.rs             | 13 +++++++++++--
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index c3db52269..cf68efe9a 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -34,7 +34,7 @@ use pbs_api_types::{
     ArchiveType, Authid, BackupGroupDeleteStats, BackupNamespace, BackupType, ChunkOrder,
     DataStoreConfig, DatastoreBackendConfig, DatastoreBackendType, GarbageCollectionCacheStats,
     GarbageCollectionStatus, MAX_NAMESPACE_DEPTH, MaintenanceMode, MaintenanceType, Operation,
-    S3Statistics, UPID,
+    RetentionTimespan, S3Statistics, UPID,
 };
 use pbs_config::s3::S3_CFG_TYPE_ID;
 use pbs_config::{BackupLockGuard, ConfigVersionCache};
@@ -212,6 +212,7 @@ pub struct DataStoreImpl {
     config_generation: Option<usize>,
     request_counters: Option<Arc<SharedRequestCounters>>,
     thresholds_exceeded_callback: Option<ThresholdsExceededCallback>,
+    max_retention_timespan: Option<RetentionTimespan>,
 }
 
 impl DataStoreImpl {
@@ -230,6 +231,7 @@ impl DataStoreImpl {
             config_generation: None,
             request_counters: None,
             thresholds_exceeded_callback: None,
+            max_retention_timespan: None,
         })
     }
 }
@@ -804,6 +806,7 @@ impl DataStore {
             config_generation: generation,
             request_counters,
             thresholds_exceeded_callback,
+            max_retention_timespan: config.max_retention_timespan,
         })
     }
 
@@ -849,6 +852,11 @@ impl DataStore {
         Ok(())
     }
 
+    /// Return the configured maximum retenition timespan
+    pub fn max_retention_timespan(&self) -> Option<&RetentionTimespan> {
+        self.inner.max_retention_timespan.as_ref()
+    }
+
     // Requires obtaining a shared chunk store lock beforehand
     pub fn create_fixed_writer<P: AsRef<Path>>(
         &self,
diff --git a/src/api2/backup/mod.rs b/src/api2/backup/mod.rs
index cfce13943..a6c06f8ff 100644
--- a/src/api2/backup/mod.rs
+++ b/src/api2/backup/mod.rs
@@ -156,6 +156,15 @@ fn upgrade_to_backup_protocol(
             "backup"
         };
 
+        if let Some(retain_until) = &retain_until {
+            if let Some(max_timespan) = datastore.max_retention_timespan() {
+                let max_timestamp = max_timespan.to_timestamp_from_systemtime()?;
+                if *retain_until > max_timestamp {
+                    bail!("requested retention timestamp exceeds datastore limit")
+                }
+            }
+        }
+
         // lock backup group to only allow one backup per group at a time
         let (owner, group_guard) = datastore.create_locked_backup_group(
             backup_group.backup_ns(),
diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs
index e7028480c..347d0b9b6 100644
--- a/src/api2/config/datastore.rs
+++ b/src/api2/config/datastore.rs
@@ -440,6 +440,8 @@ pub enum DeletableProperty {
     NotificationThresholds,
     /// Delete the counter reset schedule.
     CounterResetSchedule,
+    /// Delete the max-retention-timespan
+    MaxRetentionTimespan,
 }
 
 #[api(
@@ -545,6 +547,9 @@ pub fn update_datastore(
                 DeletableProperty::CounterResetSchedule => {
                     data.counter_reset_schedule = None;
                 }
+                DeletableProperty::MaxRetentionTimespan => {
+                    data.max_retention_timespan = None;
+                }
             }
         }
     }
@@ -646,6 +651,10 @@ pub fn update_datastore(
         data.counter_reset_schedule = update.counter_reset_schedule;
     }
 
+    if update.max_retention_timespan.is_some() {
+        data.max_retention_timespan = update.max_retention_timespan;
+    }
+
     config.set_data(&name, "datastore", &data)?;
 
     pbs_config::datastore::save_config(&config)?;
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 893b642d4..bafb1fe21 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -140,14 +140,23 @@ impl PullParameters {
         let lookup = crate::tools::lookup_with(store, Operation::Write);
         let store = DataStore::lookup_datastore(lookup)?;
         let backend = store.backend()?;
-        let target = PullTarget { store, ns, backend };
 
         let group_filter = group_filter.unwrap_or_default();
 
         let retain_until = retention_timespan
-            .map(|timespan| timespan.to_timestamp_from_systemtime())
+            .map(|timespan| {
+                if let Some(max_timespan) = (*store).max_retention_timespan() {
+                    // sub-second precision not permitted by api type, fine to ignore
+                    if timespan.as_timespan().as_secs() > max_timespan.as_timespan().as_secs() {
+                        bail!("provided timespan '{timespan}' exceeds datastore limit of '{max_timespan}'");
+                    }
+                }
+                timespan.to_timestamp_from_systemtime()
+            })
             .transpose()?;
 
+        let target = PullTarget { store, ns, backend };
+
         let crypt_configs = if let Some(key_ids) = &decryption_keys {
             let mut crypt_configs = Vec::with_capacity(key_ids.len());
             for key_id in key_ids {
-- 
2.47.3

[PATCH proxmox-backup 26/28] ui: allow datastore wide max retention timespan configuration

[Christian Ebner]

Expose the datastore config property in the UI for ease of adaption.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Makefile                           |  1 +
 www/datastore/OptionView.js            |  8 ++++++++
 www/window/MaxRetentionTimespanEdit.js | 27 ++++++++++++++++++++++++++
 3 files changed, 36 insertions(+)
 create mode 100644 www/window/MaxRetentionTimespanEdit.js

diff --git a/www/Makefile b/www/Makefile
index e14a17507..42691d466 100644
--- a/www/Makefile
+++ b/www/Makefile
@@ -91,6 +91,7 @@ JSSRC=							\
 	window/RemoteEdit.js				\
 	window/TrafficControlEdit.js			\
 	window/ThresholdResetScheduleEdit.js		\
+	window/MaxRetentionTimespanEdit.js		\
 	window/NotifyOptions.js				\
 	window/SyncJobEdit.js				\
 	window/PruneJobEdit.js				\
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index 42a6104dc..b5ef14eb1 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -384,5 +384,13 @@ Ext.define('PBS.Datastore.Options', {
                 xtype: 'pbsThresholdResetScheduleEdit',
             },
         },
+        'max-retention-timespan': {
+            required: true,
+            header: gettext('Maximum Retention'),
+            renderer: (schedule) => schedule ?? gettext('None'),
+            editor: {
+                xtype: 'pbsMaxRetentionTimespanEdit',
+            },
+        },
     },
 });
diff --git a/www/window/MaxRetentionTimespanEdit.js b/www/window/MaxRetentionTimespanEdit.js
new file mode 100644
index 000000000..dc8aebe28
--- /dev/null
+++ b/www/window/MaxRetentionTimespanEdit.js
@@ -0,0 +1,27 @@
+Ext.define('PBS.window.MaxRetentionTimespanEdit', {
+    extend: 'Proxmox.window.Edit',
+    alias: 'widget.pbsMaxRetentionTimespanEdit',
+    mixins: ['Proxmox.Mixin.CBind'],
+
+    userid: undefined,
+    isAdd: false,
+
+    subject: gettext('Maximum Retention Timespan'),
+
+    cbindData: function (initial) {
+        let me = this;
+
+        me.datastore = encodeURIComponent(me.datastore);
+        me.url = `/api2/extjs/config/datastore/${me.datastore}`;
+        me.method = 'PUT';
+        me.autoLoad = true;
+        return {};
+    },
+
+    items: {
+        xtype: 'pbsRetentionTimespan',
+        name: 'max-retention-timespan',
+        fieldLabel: gettext('Retention Timespan'),
+        emptyText: gettext('none'),
+    },
+});
-- 
2.47.3

[PATCH proxmox-backup 27/28] api: admin: allow to update snapshot retention for root user

[Christian Ebner]

Only root user should be able to ever modify the retention setting
set by the sync/backup api.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/api2/admin/datastore.rs | 53 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
index bb829cd3f..244320989 100644
--- a/src/api2/admin/datastore.rs
+++ b/src/api2/admin/datastore.rs
@@ -45,7 +45,7 @@ use pbs_api_types::{
     MaintenanceMode, MaintenanceType, NS_MAX_DEPTH_SCHEMA, Operation, PRIV_DATASTORE_AUDIT,
     PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_MODIFY, PRIV_DATASTORE_PRUNE, PRIV_DATASTORE_READ,
     PRIV_DATASTORE_VERIFY, PRIV_SYS_MODIFY, PruneJobOptions, SnapshotListItem, SyncJobConfig, UPID,
-    UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA, VERIFY_JOB_READ_THREADS_SCHEMA,
+    UPID_SCHEMA, Userid, VERIFICATION_OUTDATED_AFTER_SCHEMA, VERIFY_JOB_READ_THREADS_SCHEMA,
     VERIFY_JOB_VERIFY_THREADS_SCHEMA, print_ns_and_snapshot, print_store_and_ns,
 };
 use pbs_client::pxar::{create_tar, create_zip};
@@ -2319,6 +2319,56 @@ pub async fn set_protection(
     .await?
 }
 
+#[api(
+    input: {
+        properties: {
+            store: { schema: DATASTORE_SCHEMA },
+            ns: {
+                type: BackupNamespace,
+                optional: true,
+            },
+            backup_dir: {
+                type: pbs_api_types::BackupDir,
+                flatten: true,
+            },
+            "retain-until": {
+                type: Number,
+                description: "Set retention timestamp (clears retention if not given)",
+                optional: true,
+            },
+        },
+    },
+    access: {
+        permission: &Permission::Anybody,
+        description: "Only root@pam is allowed to modify retentions",
+    },
+)]
+/// Set retention for a specific already existing backup
+pub fn set_retention(
+    store: String,
+    ns: Option<BackupNamespace>,
+    backup_dir: pbs_api_types::BackupDir,
+    retain_until: Option<i64>,
+    rpcenv: &mut dyn RpcEnvironment,
+) -> Result<(), Error> {
+    let auth_id: Authid = rpcenv.get_auth_id().unwrap().parse()?;
+    if auth_id.is_token() || auth_id.user() != Userid::root_userid() {
+        bail!("only root can modifiy snapshot retention");
+    }
+
+    let datastore = DataStore::lookup_datastore(lookup_with(&store, Operation::Write))?;
+    let backend = datastore.backend()?;
+    let ns = ns.unwrap_or_default();
+    let backup_dir = datastore.backup_dir(ns, backup_dir)?;
+    backup_dir
+        .update_manifest(&backend, |manifest| {
+            manifest.add_retention_timestamp(retain_until);
+        })
+        .map_err(|err| format_err!("failed to set retention timestamp on manifest: {err}"))?;
+
+    Ok(())
+}
+
 #[api(
     input: {
         properties: {
@@ -2994,6 +3044,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap = &[
         "pxar-file-download",
         &Router::new().download(&API_METHOD_PXAR_FILE_DOWNLOAD),
     ),
+    ("retention", &Router::new().put(&API_METHOD_SET_RETENTION)),
     ("rrd", &Router::new().get(&API_METHOD_GET_RRD_STATS)),
     ("s3-refresh", &Router::new().put(&API_METHOD_S3_REFRESH)),
     (
-- 
2.47.3

[PATCH proxmox-backup 28/28] ui: show retention in datastore contents

[Christian Ebner]

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/datastore/Content.js | 67 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/www/datastore/Content.js b/www/datastore/Content.js
index bd989e660..bf8b23187 100644
--- a/www/datastore/Content.js
+++ b/www/datastore/Content.js
@@ -46,6 +46,11 @@ Ext.define('pbs-data-store-snapshots', {
             type: 'boolean',
             defaultValue: true,
         },
+        {
+            name: 'retain-until',
+            type: 'date',
+            dateFormat: 'timestamp',
+        },
     ],
 });
 
@@ -842,6 +847,50 @@ Ext.define('PBS.DataStoreContent', {
             });
         },
 
+        onRetentionChange: function (table, rI, cI, item, e, { data }) {
+            let me = this;
+            let view = me.getView();
+
+            if ((data.ty !== 'group' && data.ty !== 'dir' && data.ty !== 'ns') || !view.datastore) {
+                return;
+            }
+
+            Ext.Msg.show({
+                title: gettext('Confirm'),
+                icon: Ext.Msg.WARNING,
+                message: Ext.String.format(
+                    gettext('Are you sure you want to clear retention for snapshot {0}'),
+                    `'${data.text}'`,
+                ),
+                buttons: Ext.Msg.YESNO,
+                defaultFocus: 'no',
+                callback: function (btn) {
+                    if (btn !== 'yes') {
+                        return;
+                    }
+                    let params = {
+                        'backup-type': data['backup-type'],
+                        'backup-id': data['backup-id'],
+                        'backup-time': (data['backup-time'].getTime() / 1000).toFixed(0),
+                    };
+                    if (view.namespace && view.namespace !== '') {
+                        params.ns = view.namespace;
+                    }
+
+                    Proxmox.Utils.API2Request({
+                        url: `/api2/extjs/admin/datastore/${view.datastore}/retention`,
+                        params,
+                        method: 'PUT',
+                        waitMsgTarget: view,
+                        failure: function (response, opts) {
+                            Ext.Msg.alert(gettext('Error'), response.htmlStatus);
+                        },
+                        callback: me.reload.bind(me),
+                    });
+                },
+            });
+        },
+
         onForget: function (table, rI, cI, item, e, { data }) {
             let me = this;
             let view = this.getView();
@@ -1036,6 +1085,7 @@ Ext.define('PBS.DataStoreContent', {
                     onCopy: createControllerCallback('onCopy'),
                     onVerify: createControllerCallback('onVerify'),
                     onProtectionChange: createControllerCallback('onProtectionChange'),
+                    onRetentionChange: createControllerCallback('onRetentionChange'),
                     onForget: createControllerCallback('onForget'),
                 });
             }
@@ -1196,6 +1246,23 @@ Ext.define('PBS.DataStoreContent', {
                     },
                     isActionDisabled: (v, r, c, i, rec) => rec.data.ty !== 'dir',
                 },
+                {
+                    handler: 'onRetentionChange',
+                    getTip: (v, m, rec) => {
+                        if (!rec.data['retain-until']) {
+                            return;
+                        }
+                        return Ext.String.format(gettext("Clear Retention: '{0}'"), rec.data['retain-until']);
+                    },
+                    getClass: (v, m, rec) => {
+                        if (rec.data.ty === 'dir' && !!rec.data['retain-until'] > 0) {
+                            return `fa fa-hourglass`;
+                        }
+                        return 'pmx-hidden';
+                    },
+                    isActionDisabled: (v, r, c, i, rec) =>
+                        rec.data.ty !== 'dir' || !rec.data['retain-until'],
+                },
                 {
                     handler: 'onForget',
                     getTip: (v, m, { data }) => {
-- 
2.47.3