[pbs-devel] [PATCH v2 proxmox proxmox-backup 0/6] fix #6072: sync encrypted/verified snapshots only

[pbs-devel] [PATCH v2 proxmox proxmox-backup 0/6] fix #6072: sync encrypted/verified snapshots only

[Christian Ebner]

This patches implement the logic to selectively synchronize only backup
snapshots which are encrypted and/or have been verified.

The implementation exposes this as boolean flags, as the main usecase
for this feature is to only synchronize verified, therefore considered
fine backup snapshots or encrypted backup snapshots, to make sure to
only push encrypted contents to a possibly untrusted remote.

Link to bugtracker issue:
https://bugzilla.proxmox.com/show_bug.cgi?id=6072

proxmox:

Christian Ebner (1):
  pbs-api-types: sync: add sync encrypted/verified snapshots only flags

 pbs-api-types/src/jobs.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

proxmox-backup:

Christian Ebner (5):
  server: pull: refactor snapshot pull logic
  api: sync: honor sync jobs encrypted/verified only flags
  fix #6072: server: sync encrypted or verified snapshots only
  bin: manager: expose encrypted/verified only flags for cli
  www: expose encrypted/verified only flags in the sync job edit

 src/api2/config/sync.rs           |  18 +++++
 src/api2/pull.rs                  |  17 ++++-
 src/api2/push.rs                  |  15 ++++-
 src/bin/proxmox-backup-manager.rs |  39 ++++++++++-
 src/server/pull.rs                | 106 ++++++++++++++++++++++++------
 src/server/push.rs                |  48 ++++++++++++--
 src/server/sync.rs                |   2 +
 www/window/SyncJobEdit.js         |  24 +++++++
 8 files changed, 241 insertions(+), 28 deletions(-)

-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox 1/6] pbs-api-types: sync: add sync encrypted/verified snapshots only flags

[Christian Ebner]

Add optional sync job config options to allow to include only
encrypted and/or verified backup snapshots, excluding others from the
sync.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- Split of pbs api types into own patch, as they have been moved to
  proxmox repo since.

 pbs-api-types/src/jobs.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index 04631d92..a787ad33 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -522,6 +522,10 @@ impl std::fmt::Display for SyncDirection {
 pub const RESYNC_CORRUPT_SCHEMA: Schema =
     BooleanSchema::new("If the verification failed for a local snapshot, try to pull it again.")
         .schema();
+pub const SYNC_ENCRYPTED_ONLY_SCHEMA: Schema =
+    BooleanSchema::new("Only synchronize encrypted backup snapshots, exclude others.").schema();
+pub const SYNC_VERIFIED_ONLY_SCHEMA: Schema =
+    BooleanSchema::new("Only synchronize verified backup snapshots, exclude others.").schema();
 
 #[api(
     properties: {
@@ -581,6 +585,14 @@ pub const RESYNC_CORRUPT_SCHEMA: Schema =
             schema: RESYNC_CORRUPT_SCHEMA,
             optional: true,
         },
+        "encrypted-only": {
+            schema: SYNC_ENCRYPTED_ONLY_SCHEMA,
+            optional: true,
+        },
+        "verified-only": {
+            schema: SYNC_VERIFIED_ONLY_SCHEMA,
+            optional: true,
+        },
         "sync-direction": {
             type: SyncDirection,
             optional: true,
@@ -621,6 +633,10 @@ pub struct SyncJobConfig {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub resync_corrupt: Option<bool>,
     #[serde(skip_serializing_if = "Option::is_none")]
+    pub encrypted_only: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub verified_only: Option<bool>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub sync_direction: Option<SyncDirection>,
 }
 
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 2/6] server: pull: refactor snapshot pull logic

[Christian Ebner]

In preparation for skipping over snapshots when synchronizing with
encrypted/verified only flags set. In these cases, the manifest has
to be fetched from the remote and it's status checked. If the
snapshot should be skipped, the snapshot directory including the
temporary manifest file has to be cleaned up, given the snapshot
directory has been newly created. By reorganizing the current
snapshot pull logic, this can be achieved more easily.

The `corrupt` flag will be set to `false` in the snapshot
prefiltering, so the previous explicit distinction for newly created
snapshot directories must not be preserved.

No functional changes intended.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 src/server/pull.rs | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index 516abfe5d..2c0ad9e1e 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -338,7 +338,16 @@ async fn pull_snapshot<'a>(
     snapshot: &'a pbs_datastore::BackupDir,
     downloaded_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
     corrupt: bool,
+    is_new: bool,
 ) -> Result<SyncStats, Error> {
+    if is_new {
+        info!("sync snapshot {}", snapshot.dir());
+    } else if corrupt {
+        info!("re-sync snapshot {} due to corruption", snapshot.dir());
+    } else {
+        info!("re-sync snapshot {}", snapshot.dir());
+    }
+
     let mut sync_stats = SyncStats::default();
     let mut manifest_name = snapshot.full_path();
     manifest_name.push(MANIFEST_BLOB_NAME.as_ref());
@@ -456,11 +465,11 @@ async fn pull_snapshot_from<'a>(
         .datastore()
         .create_locked_backup_dir(snapshot.backup_ns(), snapshot.as_ref())?;
 
-    let sync_stats = if is_new {
-        info!("sync snapshot {}", snapshot.dir());
+    let result = pull_snapshot(reader, snapshot, downloaded_chunks, corrupt, is_new).await;
 
-        // this snapshot is new, so it can never be corrupt
-        match pull_snapshot(reader, snapshot, downloaded_chunks, false).await {
+    if is_new {
+        // Cleanup directory on error if snapshot was not present before
+        match result {
             Err(err) => {
                 if let Err(cleanup_err) = snapshot.datastore().remove_backup_dir(
                     snapshot.backup_ns(),
@@ -471,21 +480,11 @@ async fn pull_snapshot_from<'a>(
                 }
                 return Err(err);
             }
-            Ok(sync_stats) => {
-                info!("sync snapshot {} done", snapshot.dir());
-                sync_stats
-            }
+            Ok(_) => info!("sync snapshot {} done", snapshot.dir()),
         }
-    } else {
-        if corrupt {
-            info!("re-sync snapshot {} due to corruption", snapshot.dir());
-        } else {
-            info!("re-sync snapshot {}", snapshot.dir());
-        }
-        pull_snapshot(reader, snapshot, downloaded_chunks, corrupt).await?
-    };
+    }
 
-    Ok(sync_stats)
+    result
 }
 
 /// Pulls a group according to `params`.
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 3/6] api: sync: honor sync jobs encrypted/verified only flags

[Christian Ebner]

Extend the sync job config api to adapt the 'encrypted-only' and
'verified-only' flags, allowing to include only encrypted and/or
verified backup snapshots, excluding others from the sync.

Set these flags to the sync jobs push or pull parameters on job
invocation.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- Split of pbs api types into own patch, as they have been moved to
  proxmox repo since.

 src/api2/config/sync.rs | 18 ++++++++++++++++++
 src/api2/pull.rs        | 17 ++++++++++++++++-
 src/api2/push.rs        | 15 ++++++++++++++-
 src/server/pull.rs      | 10 ++++++++++
 src/server/push.rs      | 10 ++++++++++
 src/server/sync.rs      |  2 ++
 6 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
index a8ea93465..6194d8653 100644
--- a/src/api2/config/sync.rs
+++ b/src/api2/config/sync.rs
@@ -335,6 +335,10 @@ pub enum DeletableProperty {
     MaxDepth,
     /// Delete the transfer_last property,
     TransferLast,
+    /// Delete the encrypted_only property,
+    EncryptedOnly,
+    /// Delete the verified_only property,
+    VerifiedOnly,
     /// Delete the sync_direction property,
     SyncDirection,
 }
@@ -448,6 +452,12 @@ pub fn update_sync_job(
                 DeletableProperty::TransferLast => {
                     data.transfer_last = None;
                 }
+                DeletableProperty::EncryptedOnly => {
+                    data.encrypted_only = None;
+                }
+                DeletableProperty::VerifiedOnly => {
+                    data.verified_only = None;
+                }
                 DeletableProperty::SyncDirection => {
                     data.sync_direction = None;
                 }
@@ -491,6 +501,12 @@ pub fn update_sync_job(
     if let Some(resync_corrupt) = update.resync_corrupt {
         data.resync_corrupt = Some(resync_corrupt);
     }
+    if let Some(encrypted_only) = update.encrypted_only {
+        data.encrypted_only = Some(encrypted_only);
+    }
+    if let Some(verified_only) = update.verified_only {
+        data.verified_only = Some(verified_only);
+    }
     if let Some(sync_direction) = update.sync_direction {
         data.sync_direction = Some(sync_direction);
     }
@@ -665,6 +681,8 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSyncOperator
         schedule: None,
         limit: pbs_api_types::RateLimitConfig::default(), // no limit
         transfer_last: None,
+        encrypted_only: None,
+        verified_only: None,
         sync_direction: None, // use default
     };
 
diff --git a/src/api2/pull.rs b/src/api2/pull.rs
index d8ed1a734..4b1fd5e60 100644
--- a/src/api2/pull.rs
+++ b/src/api2/pull.rs
@@ -10,7 +10,8 @@ use pbs_api_types::{
     Authid, BackupNamespace, GroupFilter, RateLimitConfig, SyncJobConfig, DATASTORE_SCHEMA,
     GROUP_FILTER_LIST_SCHEMA, NS_MAX_DEPTH_REDUCED_SCHEMA, PRIV_DATASTORE_BACKUP,
     PRIV_DATASTORE_PRUNE, PRIV_REMOTE_READ, REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA,
-    RESYNC_CORRUPT_SCHEMA, TRANSFER_LAST_SCHEMA,
+    RESYNC_CORRUPT_SCHEMA, SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA,
+    TRANSFER_LAST_SCHEMA,
 };
 use pbs_config::CachedUserInfo;
 use proxmox_rest_server::WorkerTask;
@@ -87,6 +88,8 @@ impl TryFrom<&SyncJobConfig> for PullParameters {
             sync_job.group_filter.clone(),
             sync_job.limit.clone(),
             sync_job.transfer_last,
+            sync_job.encrypted_only,
+            sync_job.verified_only,
             sync_job.resync_corrupt,
         )
     }
@@ -133,6 +136,14 @@ impl TryFrom<&SyncJobConfig> for PullParameters {
                 schema: TRANSFER_LAST_SCHEMA,
                 optional: true,
             },
+            "encrypted-only": {
+                schema: SYNC_ENCRYPTED_ONLY_SCHEMA,
+                optional: true,
+            },
+            "verified-only": {
+                schema: SYNC_VERIFIED_ONLY_SCHEMA,
+                optional: true,
+            },
             "resync-corrupt": {
                 schema: RESYNC_CORRUPT_SCHEMA,
                 optional: true,
@@ -161,6 +172,8 @@ async fn pull(
     group_filter: Option<Vec<GroupFilter>>,
     limit: RateLimitConfig,
     transfer_last: Option<usize>,
+    encrypted_only: Option<bool>,
+    verified_only: Option<bool>,
     resync_corrupt: Option<bool>,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<String, Error> {
@@ -199,6 +212,8 @@ async fn pull(
         group_filter,
         limit,
         transfer_last,
+        encrypted_only,
+        verified_only,
         resync_corrupt,
     )?;
 
diff --git a/src/api2/push.rs b/src/api2/push.rs
index bf846bb37..e5edc13e0 100644
--- a/src/api2/push.rs
+++ b/src/api2/push.rs
@@ -5,7 +5,8 @@ use pbs_api_types::{
     Authid, BackupNamespace, GroupFilter, RateLimitConfig, DATASTORE_SCHEMA,
     GROUP_FILTER_LIST_SCHEMA, 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, TRANSFER_LAST_SCHEMA,
+    REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, SYNC_ENCRYPTED_ONLY_SCHEMA,
+    SYNC_VERIFIED_ONLY_SCHEMA, TRANSFER_LAST_SCHEMA,
 };
 use proxmox_rest_server::WorkerTask;
 use proxmox_router::{Permission, Router, RpcEnvironment};
@@ -91,6 +92,14 @@ fn check_push_privs(
                 schema: GROUP_FILTER_LIST_SCHEMA,
                 optional: true,
             },
+            "encrypted-only": {
+                schema: SYNC_ENCRYPTED_ONLY_SCHEMA,
+                optional: true,
+            },
+            "verified-only": {
+                schema: SYNC_VERIFIED_ONLY_SCHEMA,
+                optional: true,
+            },
             limit: {
                 type: RateLimitConfig,
                 flatten: true,
@@ -120,6 +129,8 @@ async fn push(
     remove_vanished: Option<bool>,
     max_depth: Option<usize>,
     group_filter: Option<Vec<GroupFilter>>,
+    encrypted_only: Option<bool>,
+    verified_only: Option<bool>,
     limit: RateLimitConfig,
     transfer_last: Option<usize>,
     rpcenv: &mut dyn RpcEnvironment,
@@ -149,6 +160,8 @@ async fn push(
         remove_vanished,
         max_depth,
         group_filter,
+        encrypted_only,
+        verified_only,
         limit,
         transfer_last,
     )
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 2c0ad9e1e..616d45eb9 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -55,6 +55,10 @@ pub(crate) struct PullParameters {
     group_filter: Vec<GroupFilter>,
     /// How many snapshots should be transferred at most (taking the newest N snapshots)
     transfer_last: Option<usize>,
+    /// Only sync encrypted backup snapshots
+    encrypted_only: bool,
+    /// Only sync verified backup snapshots
+    verified_only: bool,
     /// Whether to re-sync corrupted snapshots
     resync_corrupt: bool,
 }
@@ -74,6 +78,8 @@ impl PullParameters {
         group_filter: Option<Vec<GroupFilter>>,
         limit: RateLimitConfig,
         transfer_last: Option<usize>,
+        encrypted_only: Option<bool>,
+        verified_only: Option<bool>,
         resync_corrupt: Option<bool>,
     ) -> Result<Self, Error> {
         if let Some(max_depth) = max_depth {
@@ -82,6 +88,8 @@ impl PullParameters {
         };
         let remove_vanished = remove_vanished.unwrap_or(false);
         let resync_corrupt = resync_corrupt.unwrap_or(false);
+        let encrypted_only = encrypted_only.unwrap_or(false);
+        let verified_only = verified_only.unwrap_or(false);
 
         let source: Arc<dyn SyncSource> = if let Some(remote) = remote {
             let (remote_config, _digest) = pbs_config::remote::config()?;
@@ -120,6 +128,8 @@ impl PullParameters {
             max_depth,
             group_filter,
             transfer_last,
+            encrypted_only,
+            verified_only,
             resync_corrupt,
         })
     }
diff --git a/src/server/push.rs b/src/server/push.rs
index 0db3dff30..1fb447b58 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -73,6 +73,10 @@ pub(crate) struct PushParameters {
     max_depth: Option<usize>,
     /// Filters for reducing the push scope
     group_filter: Vec<GroupFilter>,
+    /// Synchronize only encrypted backup snapshots
+    encrypted_only: bool,
+    /// Synchronize only verified backup snapshots
+    verified_only: bool,
     /// How many snapshots should be transferred at most (taking the newest N snapshots)
     transfer_last: Option<usize>,
 }
@@ -90,6 +94,8 @@ impl PushParameters {
         remove_vanished: Option<bool>,
         max_depth: Option<usize>,
         group_filter: Option<Vec<GroupFilter>>,
+        encrypted_only: Option<bool>,
+        verified_only: Option<bool>,
         limit: RateLimitConfig,
         transfer_last: Option<usize>,
     ) -> Result<Self, Error> {
@@ -98,6 +104,8 @@ impl PushParameters {
             remote_ns.check_max_depth(max_depth)?;
         };
         let remove_vanished = remove_vanished.unwrap_or(false);
+        let encrypted_only = encrypted_only.unwrap_or(false);
+        let verified_only = verified_only.unwrap_or(false);
         let store = DataStore::lookup_datastore(store, Some(Operation::Read))?;
 
         if !store.namespace_exists(&ns) {
@@ -149,6 +157,8 @@ impl PushParameters {
             remove_vanished,
             max_depth,
             group_filter,
+            encrypted_only,
+            verified_only,
             transfer_last,
         })
     }
diff --git a/src/server/sync.rs b/src/server/sync.rs
index 4dd46c5a0..63c5f1cd9 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -672,6 +672,8 @@ pub fn do_sync_job(
                             sync_job.remove_vanished,
                             sync_job.max_depth,
                             sync_job.group_filter.clone(),
+                            sync_job.encrypted_only,
+                            sync_job.verified_only,
                             sync_job.limit.clone(),
                             sync_job.transfer_last,
                         )
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 4/6] fix #6072: server: sync encrypted or verified snapshots only

[Christian Ebner]

Skip over snapshots which have not been verified or encrypted if the
sync jobs has set the flags accordingly.
A snapshot is considered as encrypted, if all the archives in the
manifest have `CryptMode::Encrypt`. A snapshot is considered as
verified, when the manifest's verify state is set to
`VerifyState::Ok`.

This allows to only synchronize a subset of the snapshots, which are
known to be fine (verified) or which are known to be encrypted. The
latter is of most interest for sync jobs in push direction to
untrusted or less trusted remotes, where it might be desired to not
expose unencrypted contents.

Link to the bugtracker issue:
https://bugzilla.proxmox.com/show_bug.cgi?id=6072

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 src/server/pull.rs | 65 +++++++++++++++++++++++++++++++++++++++++++---
 src/server/push.rs | 38 ++++++++++++++++++++++++---
 2 files changed, 95 insertions(+), 8 deletions(-)

diff --git a/src/server/pull.rs b/src/server/pull.rs
index 616d45eb9..c223e1b93 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -12,7 +12,7 @@ use tracing::info;
 
 use pbs_api_types::{
     print_store_and_ns, ArchiveType, Authid, BackupArchiveName, BackupDir, BackupGroup,
-    BackupNamespace, GroupFilter, Operation, RateLimitConfig, Remote, VerifyState,
+    BackupNamespace, CryptMode, GroupFilter, Operation, RateLimitConfig, Remote, VerifyState,
     CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_AUDIT,
     PRIV_DATASTORE_BACKUP,
 };
@@ -344,6 +344,7 @@ async fn pull_single_archive<'a>(
 ///   -- if not, pull it from the remote
 /// - Download log if not already existing
 async fn pull_snapshot<'a>(
+    params: &PullParameters,
     reader: Arc<dyn SyncSourceReader + 'a>,
     snapshot: &'a pbs_datastore::BackupDir,
     downloaded_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
@@ -402,6 +403,55 @@ async fn pull_snapshot<'a>(
 
     let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
 
+    if params.verified_only {
+        let mut snapshot_verified = false;
+        if let Ok(Some(verify_state)) = manifest.verify_state() {
+            if let VerifyState::Ok = verify_state.state {
+                snapshot_verified = true;
+            }
+        }
+
+        if !snapshot_verified {
+            info!(
+                "Snapshot {} not verified but verified-only set, snapshot skipped",
+                snapshot.dir(),
+            );
+            if is_new {
+                let path = snapshot.full_path();
+                // safe to remove as locked by caller
+                std::fs::remove_dir_all(&path).map_err(|err| {
+                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
+                })?;
+            }
+            return Ok(sync_stats);
+        }
+    }
+
+    if params.encrypted_only {
+        let mut snapshot_encrypted = true;
+        // Consider only encrypted if all files in the manifest are marked as encrypted
+        for file in manifest.files() {
+            if file.chunk_crypt_mode() != CryptMode::Encrypt {
+                snapshot_encrypted = false;
+            }
+        }
+
+        if !snapshot_encrypted {
+            info!(
+                "Snapshot {} not encrypted but encrypted-only set, snapshot skipped",
+                snapshot.dir(),
+            );
+            if is_new {
+                let path = snapshot.full_path();
+                // safe to remove as locked by caller
+                std::fs::remove_dir_all(&path).map_err(|err| {
+                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
+                })?;
+            }
+            return Ok(sync_stats);
+        }
+    }
+
     for item in manifest.files() {
         let mut path = snapshot.full_path();
         path.push(&item.filename);
@@ -466,6 +516,7 @@ async fn pull_snapshot<'a>(
 /// The `reader` is configured to read from the source backup directory, while the
 /// `snapshot` is pointing to the local datastore and target namespace.
 async fn pull_snapshot_from<'a>(
+    params: &PullParameters,
     reader: Arc<dyn SyncSourceReader + 'a>,
     snapshot: &'a pbs_datastore::BackupDir,
     downloaded_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
@@ -475,7 +526,7 @@ async fn pull_snapshot_from<'a>(
         .datastore()
         .create_locked_backup_dir(snapshot.backup_ns(), snapshot.as_ref())?;
 
-    let result = pull_snapshot(reader, snapshot, downloaded_chunks, corrupt, is_new).await;
+    let result = pull_snapshot(params, reader, snapshot, downloaded_chunks, corrupt, is_new).await;
 
     if is_new {
         // Cleanup directory on error if snapshot was not present before
@@ -621,8 +672,14 @@ async fn pull_group(
             .source
             .reader(source_namespace, &from_snapshot)
             .await?;
-        let result =
-            pull_snapshot_from(reader, &to_snapshot, downloaded_chunks.clone(), corrupt).await;
+        let result = pull_snapshot_from(
+            params,
+            reader,
+            &to_snapshot,
+            downloaded_chunks.clone(),
+            corrupt,
+        )
+        .await;
 
         progress.done_snapshots = pos as u64 + 1;
         info!("percentage done: {progress}");
diff --git a/src/server/push.rs b/src/server/push.rs
index 1fb447b58..a9a371771 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -11,10 +11,11 @@ use tracing::{info, warn};
 
 use pbs_api_types::{
     print_store_and_ns, ApiVersion, ApiVersionInfo, ArchiveType, Authid, BackupArchiveName,
-    BackupDir, BackupGroup, BackupGroupDeleteStats, BackupNamespace, GroupFilter, GroupListItem,
-    NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem, CLIENT_LOG_BLOB_NAME,
-    MANIFEST_BLOB_NAME, PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_BACKUP,
-    PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE,
+    BackupDir, BackupGroup, BackupGroupDeleteStats, BackupNamespace, CryptMode, GroupFilter,
+    GroupListItem, NamespaceListItem, Operation, RateLimitConfig, Remote, SnapshotListItem,
+    VerifyState, CLIENT_LOG_BLOB_NAME, MANIFEST_BLOB_NAME, PRIV_DATASTORE_BACKUP,
+    PRIV_DATASTORE_READ, PRIV_REMOTE_DATASTORE_BACKUP, PRIV_REMOTE_DATASTORE_MODIFY,
+    PRIV_REMOTE_DATASTORE_PRUNE,
 };
 use pbs_client::{BackupRepository, BackupWriter, HttpClient, MergedChunkInfo, UploadOptions};
 use pbs_config::CachedUserInfo;
@@ -810,6 +811,35 @@ pub(crate) async fn push_snapshot(
         }
     };
 
+    if params.verified_only {
+        let mut snapshot_verified = false;
+        if let Ok(Some(verify_state)) = source_manifest.verify_state() {
+            if let VerifyState::Ok = verify_state.state {
+                snapshot_verified = true;
+            }
+        }
+
+        if !snapshot_verified {
+            info!("Snapshot {snapshot} not verified but verified-only set, snapshot skipped");
+            return Ok(stats);
+        }
+    }
+
+    if params.encrypted_only {
+        let mut snapshot_encrypted = true;
+        // Consider only encrypted if all files in the manifest are marked as encrypted
+        for file in source_manifest.files() {
+            if file.chunk_crypt_mode() != CryptMode::Encrypt {
+                snapshot_encrypted = false;
+            }
+        }
+
+        if !snapshot_encrypted {
+            info!("Snapshot {snapshot} not encrypted but encrypted-only set, snapshot skipped");
+            return Ok(stats);
+        }
+    }
+
     // Writer instance locks the snapshot on the remote side
     let backup_writer = BackupWriter::start(
         &params.target.client,
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 5/6] bin: manager: expose encrypted/verified only flags for cli

[Christian Ebner]

Allow to perform a push/pull sync job including only encrypted and/or
verified backup snapshots via the command line.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 src/bin/proxmox-backup-manager.rs | 39 +++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index 02ca0d028..154c6d5da 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -14,8 +14,9 @@ use pbs_api_types::percent_encoding::percent_encode_component;
 use pbs_api_types::{
     BackupNamespace, GroupFilter, RateLimitConfig, SyncDirection, SyncJobConfig, DATASTORE_SCHEMA,
     GROUP_FILTER_LIST_SCHEMA, IGNORE_VERIFIED_BACKUPS_SCHEMA, NS_MAX_DEPTH_SCHEMA,
-    REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, RESYNC_CORRUPT_SCHEMA, TRANSFER_LAST_SCHEMA,
-    UPID_SCHEMA, VERIFICATION_OUTDATED_AFTER_SCHEMA,
+    REMOTE_ID_SCHEMA, REMOVE_VANISHED_BACKUPS_SCHEMA, RESYNC_CORRUPT_SCHEMA,
+    SYNC_ENCRYPTED_ONLY_SCHEMA, SYNC_VERIFIED_ONLY_SCHEMA, TRANSFER_LAST_SCHEMA, UPID_SCHEMA,
+    VERIFICATION_OUTDATED_AFTER_SCHEMA,
 };
 use pbs_client::{display_task_log, view_task_result};
 use pbs_config::sync;
@@ -308,6 +309,8 @@ async fn sync_datastore(
     limit: RateLimitConfig,
     transfer_last: Option<usize>,
     resync_corrupt: Option<bool>,
+    encrypted_only: Option<bool>,
+    verified_only: Option<bool>,
     param: Value,
     sync_direction: SyncDirection,
 ) -> Result<Value, Error> {
@@ -348,6 +351,14 @@ async fn sync_datastore(
         args["resync-corrupt"] = Value::from(resync);
     }
 
+    if let Some(encrypted_only) = encrypted_only {
+        args["encrypted-only"] = Value::from(encrypted_only);
+    }
+
+    if let Some(verified_only) = verified_only {
+        args["verified-only"] = Value::from(verified_only);
+    }
+
     let mut limit_json = json!(limit);
     let limit_map = limit_json
         .as_object_mut()
@@ -414,6 +425,14 @@ async fn sync_datastore(
                 schema: RESYNC_CORRUPT_SCHEMA,
                 optional: true,
             },
+            "encrypted-only": {
+                schema: SYNC_ENCRYPTED_ONLY_SCHEMA,
+                optional: true,
+            },
+            "verified-only": {
+                schema: SYNC_VERIFIED_ONLY_SCHEMA,
+                optional: true,
+            },
         }
    }
 )]
@@ -431,6 +450,8 @@ async fn pull_datastore(
     limit: RateLimitConfig,
     transfer_last: Option<usize>,
     resync_corrupt: Option<bool>,
+    encrypted_only: Option<bool>,
+    verified_only: Option<bool>,
     param: Value,
 ) -> Result<Value, Error> {
     sync_datastore(
@@ -445,6 +466,8 @@ async fn pull_datastore(
         limit,
         transfer_last,
         resync_corrupt,
+        encrypted_only,
+        verified_only,
         param,
         SyncDirection::Pull,
     )
@@ -495,6 +518,14 @@ async fn pull_datastore(
                 schema: TRANSFER_LAST_SCHEMA,
                 optional: true,
             },
+            "encrypted-only": {
+                schema: SYNC_ENCRYPTED_ONLY_SCHEMA,
+                optional: true,
+            },
+            "verified-only": {
+                schema: SYNC_VERIFIED_ONLY_SCHEMA,
+                optional: true,
+            },
         }
    }
 )]
@@ -511,6 +542,8 @@ async fn push_datastore(
     group_filter: Option<Vec<GroupFilter>>,
     limit: RateLimitConfig,
     transfer_last: Option<usize>,
+    encrypted_only: Option<bool>,
+    verified_only: Option<bool>,
     param: Value,
 ) -> Result<Value, Error> {
     sync_datastore(
@@ -525,6 +558,8 @@ async fn push_datastore(
         limit,
         transfer_last,
         None,
+        encrypted_only,
+        verified_only,
         param,
         SyncDirection::Push,
     )
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH v2 proxmox-backup 6/6] www: expose encrypted/verified only flags in the sync job edit

[Christian Ebner]

Allows the user to set the encrypted/verified only flags in the
advanced settings of a sync job edit window.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
changes since version 1:
- no changes

 www/window/SyncJobEdit.js | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/www/window/SyncJobEdit.js b/www/window/SyncJobEdit.js
index bcd2f2fb2..4b9922d3e 100644
--- a/www/window/SyncJobEdit.js
+++ b/www/window/SyncJobEdit.js
@@ -400,6 +400,30 @@ Ext.define('PBS.window.SyncJobEdit', {
 			value: false,
 		    },
 		],
+		advancedColumn2: [
+		    {
+			fieldLabel: gettext('Encrypted snapshots only'),
+			xtype: 'proxmoxcheckbox',
+			name: 'encrypted-only',
+			autoEl: {
+			    tag: 'div',
+			    'data-qtip': gettext('Sync only encrypted backup snapshots, exclude others.'),
+			},
+			uncheckedValue: false,
+			value: false,
+		    },
+		    {
+			fieldLabel: gettext('Verified snapshots only'),
+			xtype: 'proxmoxcheckbox',
+			name: 'verified-only',
+			autoEl: {
+			    tag: 'div',
+			    'data-qtip': gettext('Sync only verified backup snapshots, exclude others.'),
+			},
+			uncheckedValue: false,
+			value: false,
+		    },
+		],
 	    },
 	    {
 		xtype: 'inputpanel',
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 4/6] fix #6072: server: sync encrypted or verified snapshots only

[Thomas Lamprecht]

Am 18.03.25 um 12:39 schrieb Christian Ebner:
> @@ -402,6 +403,55 @@ async fn pull_snapshot<'a>(
>  
>      let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
>  
> +    if params.verified_only {
> +        let mut snapshot_verified = false;
> +        if let Ok(Some(verify_state)) = manifest.verify_state() {
> +            if let VerifyState::Ok = verify_state.state {
> +                snapshot_verified = true;
> +            }
> +        }

nit: IMO this would read slightly nicer as match, but no hard feelings.
E.g. like (untested):

let snapshot_verified = match source_manifest.verify_state() {
    Ok(Some(VerifyState::Ok)) => true,
    _ => false,
};

> +
> +        if !snapshot_verified {
> +            info!(
> +                "Snapshot {} not verified but verified-only set, snapshot skipped",
> +                snapshot.dir(),
> +            );
> +            if is_new {
> +                let path = snapshot.full_path();
> +                // safe to remove as locked by caller
> +                std::fs::remove_dir_all(&path).map_err(|err| {
> +                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
> +                })?;
> +            }
> +            return Ok(sync_stats);

Maybe it might be nicer to use an ignore_snapshot bool shared by this and the
encrypted-only branch and then move the early exit after that to a common if?

> +        }
> +    }
> +
> +    if params.encrypted_only {
> +        let mut snapshot_encrypted = true;
> +        // Consider only encrypted if all files in the manifest are marked as encrypted
> +        for file in manifest.files() {
> +            if file.chunk_crypt_mode() != CryptMode::Encrypt {
> +                snapshot_encrypted = false;

could use break after this, the value of snapshot_encrypted won't change after
this anymore.

> +            }
> +        }

Alternatively use a more functional style, e.g. something like (untested):

let snapshot_encrypted = source_manifest
    .files()
    .all(|&file| file.chunk_crypt_mode() == CryptMode::Encrypt);

> +
> +        if !snapshot_encrypted {
> +            info!(
> +                "Snapshot {} not encrypted but encrypted-only set, snapshot skipped",
> +                snapshot.dir(),
> +            );
> +            if is_new {
> +                let path = snapshot.full_path();
> +                // safe to remove as locked by caller
> +                std::fs::remove_dir_all(&path).map_err(|err| {
> +                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
> +                })?;
> +            }
> +            return Ok(sync_stats);
> +        }
> +    }
> +
>      for item in manifest.files() {
>          let mut path = snapshot.full_path();
>          path.push(&item.filename);


>  use pbs_client::{BackupRepository, BackupWriter, HttpClient, MergedChunkInfo, UploadOptions};
>  use pbs_config::CachedUserInfo;
> @@ -810,6 +811,35 @@ pub(crate) async fn push_snapshot(
>          }
>      };
>  
> +    if params.verified_only {
> +        let mut snapshot_verified = false;
> +        if let Ok(Some(verify_state)) = source_manifest.verify_state() {
> +            if let VerifyState::Ok = verify_state.state {
> +                snapshot_verified = true;
> +            }
> +        }

same as above w.r.t. code style nit.

> +
> +        if !snapshot_verified {
> +            info!("Snapshot {snapshot} not verified but verified-only set, snapshot skipped");
> +            return Ok(stats);
> +        }
> +    }
> +
> +    if params.encrypted_only {
> +        let mut snapshot_encrypted = true;
> +        // Consider only encrypted if all files in the manifest are marked as encrypted
> +        for file in source_manifest.files() {
> +            if file.chunk_crypt_mode() != CryptMode::Encrypt {
> +                snapshot_encrypted = false;

same as above w.r.t. code style nit.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH v2 proxmox-backup 2/6] server: pull: refactor snapshot pull logic

[Thomas Lamprecht]

Am 18.03.25 um 12:39 schrieb Christian Ebner:
> In preparation for skipping over snapshots when synchronizing with
> encrypted/verified only flags set. In these cases, the manifest has
> to be fetched from the remote and it's status checked. If the
> snapshot should be skipped, the snapshot directory including the
> temporary manifest file has to be cleaned up, given the snapshot
> directory has been newly created. By reorganizing the current
> snapshot pull logic, this can be achieved more easily.
> 
> The `corrupt` flag will be set to `false` in the snapshot
> prefiltering, so the previous explicit distinction for newly created
> snapshot directories must not be preserved.
> 
> No functional changes intended.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> changes since version 1:
> - no changes
> 
>  src/server/pull.rs | 33 ++++++++++++++++-----------------
>  1 file changed, 16 insertions(+), 17 deletions(-)
> 
>

applied this one already, thanks!

maybe it makes sense to combine the is_new and corrupt flags into a struct
for passing around, but even if, that is probably better done independent
of this series and with some hindsight.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 4/6] fix #6072: server: sync encrypted or verified snapshots only

[Christian Ebner]

On 4/2/25 15:29, Thomas Lamprecht wrote:
> Am 18.03.25 um 12:39 schrieb Christian Ebner:
>> @@ -402,6 +403,55 @@ async fn pull_snapshot<'a>(
>>   
>>       let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
>>   
>> +    if params.verified_only {
>> +        let mut snapshot_verified = false;
>> +        if let Ok(Some(verify_state)) = manifest.verify_state() {
>> +            if let VerifyState::Ok = verify_state.state {
>> +                snapshot_verified = true;
>> +            }
>> +        }
> 
> nit: IMO this would read slightly nicer as match, but no hard feelings.
> E.g. like (untested):
> 
> let snapshot_verified = match source_manifest.verify_state() {
>      Ok(Some(VerifyState::Ok)) => true,
>      _ => false,
> };

While that reads much nicer, destructuring does not work as the actual 
verify state is stored within the `verify_state.state`. The alternative 
would be to use a match guard, but that might be more confusing?

> 
>> +
>> +        if !snapshot_verified {
>> +            info!(
>> +                "Snapshot {} not verified but verified-only set, snapshot skipped",
>> +                snapshot.dir(),
>> +            );
>> +            if is_new {
>> +                let path = snapshot.full_path();
>> +                // safe to remove as locked by caller
>> +                std::fs::remove_dir_all(&path).map_err(|err| {
>> +                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
>> +                })?;
>> +            }
>> +            return Ok(sync_stats);
> 
> Maybe it might be nicer to use an ignore_snapshot bool shared by this and the
> encrypted-only branch and then move the early exit after that to a common if?

Acked, will adapt that according to your suggestion.

> 
>> +        }
>> +    }
>> +
>> +    if params.encrypted_only {
>> +        let mut snapshot_encrypted = true;
>> +        // Consider only encrypted if all files in the manifest are marked as encrypted
>> +        for file in manifest.files() {
>> +            if file.chunk_crypt_mode() != CryptMode::Encrypt {
>> +                snapshot_encrypted = false;
> 
> could use break after this, the value of snapshot_encrypted won't change after
> this anymore.
> 
>> +            }
>> +        }
> 
> Alternatively use a more functional style, e.g. something like (untested):
> 
> let snapshot_encrypted = source_manifest
>      .files()
>      .all(|&file| file.chunk_crypt_mode() == CryptMode::Encrypt);

This is more readable IMO, so will adapt to that!

> 
>> +
>> +        if !snapshot_encrypted {
>> +            info!(
>> +                "Snapshot {} not encrypted but encrypted-only set, snapshot skipped",
>> +                snapshot.dir(),
>> +            );
>> +            if is_new {
>> +                let path = snapshot.full_path();
>> +                // safe to remove as locked by caller
>> +                std::fs::remove_dir_all(&path).map_err(|err| {
>> +                    format_err!("removing temporary backup snapshot {path:?} failed - {err}")
>> +                })?;
>> +            }
>> +            return Ok(sync_stats);
>> +        }
>> +    }
>> +
>>       for item in manifest.files() {
>>           let mut path = snapshot.full_path();
>>           path.push(&item.filename);
> 
> 
>>   use pbs_client::{BackupRepository, BackupWriter, HttpClient, MergedChunkInfo, UploadOptions};
>>   use pbs_config::CachedUserInfo;
>> @@ -810,6 +811,35 @@ pub(crate) async fn push_snapshot(
>>           }
>>       };
>>   
>> +    if params.verified_only {
>> +        let mut snapshot_verified = false;
>> +        if let Ok(Some(verify_state)) = source_manifest.verify_state() {
>> +            if let VerifyState::Ok = verify_state.state {
>> +                snapshot_verified = true;
>> +            }
>> +        }
> 
> same as above w.r.t. code style nit.

Unfortunately same as above.

> 
>> +
>> +        if !snapshot_verified {
>> +            info!("Snapshot {snapshot} not verified but verified-only set, snapshot skipped");
>> +            return Ok(stats);
>> +        }
>> +    }
>> +
>> +    if params.encrypted_only {
>> +        let mut snapshot_encrypted = true;
>> +        // Consider only encrypted if all files in the manifest are marked as encrypted
>> +        for file in source_manifest.files() {
>> +            if file.chunk_crypt_mode() != CryptMode::Encrypt {
>> +                snapshot_encrypted = false;
> 
> same as above w.r.t. code style nit.
Acked, will adapt this.



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox-backup 4/6] fix #6072: server: sync encrypted or verified snapshots only

[Thomas Lamprecht]

Am 02.04.25 um 15:57 schrieb Christian Ebner:
> On 4/2/25 15:29, Thomas Lamprecht wrote:
>> Am 18.03.25 um 12:39 schrieb Christian Ebner:
>>> @@ -402,6 +403,55 @@ async fn pull_snapshot<'a>(
>>>   
>>>       let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
>>>   
>>> +    if params.verified_only {
>>> +        let mut snapshot_verified = false;
>>> +        if let Ok(Some(verify_state)) = manifest.verify_state() {
>>> +            if let VerifyState::Ok = verify_state.state {
>>> +                snapshot_verified = true;
>>> +            }
>>> +        }
>> nit: IMO this would read slightly nicer as match, but no hard feelings.
>> E.g. like (untested):
>>
>> let snapshot_verified = match source_manifest.verify_state() {
>>      Ok(Some(VerifyState::Ok)) => true,
>>      _ => false,
>> };
> While that reads much nicer, destructuring does not work as the actual 
> verify state is stored within the `verify_state.state`. The alternative 
> would be to use a match guard, but that might be more confusing?

Oh, right. Yeah that is not nice, and some more involved like doing an
.ok().flatten() for the result and then .map()'ing that is also not
really nice.

Maybe it would be better to add an is_verified() -> Result<bool, Error>
function added to manifest impl instead; but does not have to be part
of this series and I did not think that really through, just an idea.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] superseded: [PATCH v2 proxmox proxmox-backup 0/6] fix #6072: sync encrypted/verified snapshots only

[Christian Ebner]

superseded-by version 3:
https://lore.proxmox.com/pbs-devel/20250402152752.598501-1-c.ebner@proxmox.com/T/


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel