From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 502241FF17C
for <inbox@lore.proxmox.com>; Wed, 2 Apr 2025 17:28:21 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 2F9961E5FE;
Wed, 2 Apr 2025 17:28:08 +0200 (CEST)
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 2 Apr 2025 17:27:50 +0200
Message-Id: <20250402152752.598501-5-c.ebner@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250402152752.598501-1-c.ebner@proxmox.com>
References: <20250402152752.598501-1-c.ebner@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.031 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
DMARC_MISSING 0.1 Missing DMARC policy
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
Subject: [pbs-devel] [PATCH v3 proxmox-backup 4/6] fix #6072: server: sync
encrypted or verified snapshots only
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
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 2 (thanks @Thomas):
- factor out ignore snapshot logic into common
`ignore_not_verified_or_encrypted` helper
- use Iter::all in is encrypted check
- use match with guard over nested if blocks in is verified check
src/server/pull.rs | 34 +++++++++++++++++++++++++++++-----
src/server/push.rs | 13 +++++++++++--
src/server/sync.rs | 37 ++++++++++++++++++++++++++++++++++---
3 files changed, 74 insertions(+), 10 deletions(-)
diff --git a/src/server/pull.rs b/src/server/pull.rs
index 616d45eb9..8fb491cd4 100644
--- a/src/server/pull.rs
+++ b/src/server/pull.rs
@@ -28,8 +28,8 @@ use pbs_datastore::{check_backup_owner, DataStore, StoreProgress};
use pbs_tools::sha::sha256;
use super::sync::{
- check_namespace_depth_limit, LocalSource, RemoteSource, RemovedVanishedStats, SkipInfo,
- SkipReason, SyncSource, SyncSourceReader, SyncStats,
+ check_namespace_depth_limit, ignore_not_verified_or_encrypted, LocalSource, RemoteSource,
+ RemovedVanishedStats, SkipInfo, SkipReason, SyncSource, SyncSourceReader, SyncStats,
};
use crate::backup::{check_ns_modification_privs, check_ns_privs};
use crate::tools::parallel_handler::ParallelHandler;
@@ -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,22 @@ async fn pull_snapshot<'a>(
let manifest = BackupManifest::try_from(tmp_manifest_blob)?;
+ if ignore_not_verified_or_encrypted(
+ &manifest,
+ snapshot.dir(),
+ params.verified_only,
+ params.encrypted_only,
+ ) {
+ 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 +483,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 +493,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 +639,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..e71012ed8 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -26,8 +26,8 @@ use pbs_datastore::read_chunk::AsyncReadChunk;
use pbs_datastore::{DataStore, StoreProgress};
use super::sync::{
- check_namespace_depth_limit, LocalSource, RemovedVanishedStats, SkipInfo, SkipReason,
- SyncSource, SyncStats,
+ check_namespace_depth_limit, ignore_not_verified_or_encrypted, LocalSource,
+ RemovedVanishedStats, SkipInfo, SkipReason, SyncSource, SyncStats,
};
use crate::api2::config::remote;
@@ -810,6 +810,15 @@ pub(crate) async fn push_snapshot(
}
};
+ if ignore_not_verified_or_encrypted(
+ &source_manifest,
+ snapshot,
+ params.verified_only,
+ params.encrypted_only,
+ ) {
+ return Ok(stats);
+ }
+
// Writer instance locks the snapshot on the remote side
let backup_writer = BackupWriter::start(
¶ms.target.client,
diff --git a/src/server/sync.rs b/src/server/sync.rs
index d424a6b46..528e2054c 100644
--- a/src/server/sync.rs
+++ b/src/server/sync.rs
@@ -20,13 +20,13 @@ use proxmox_router::HttpError;
use pbs_api_types::{
Authid, BackupDir, BackupGroup, BackupNamespace, CryptMode, GroupListItem, SnapshotListItem,
- SyncDirection, SyncJobConfig, CLIENT_LOG_BLOB_NAME, MAX_NAMESPACE_DEPTH, PRIV_DATASTORE_BACKUP,
- PRIV_DATASTORE_READ,
+ SyncDirection, SyncJobConfig, VerifyState, CLIENT_LOG_BLOB_NAME, MAX_NAMESPACE_DEPTH,
+ PRIV_DATASTORE_BACKUP, PRIV_DATASTORE_READ,
};
use pbs_client::{BackupReader, BackupRepository, HttpClient, RemoteChunkReader};
use pbs_datastore::data_blob::DataBlob;
use pbs_datastore::read_chunk::AsyncReadChunk;
-use pbs_datastore::{DataStore, ListNamespacesRecursive, LocalChunkReader};
+use pbs_datastore::{BackupManifest, DataStore, ListNamespacesRecursive, LocalChunkReader};
use crate::backup::ListAccessibleBackupGroups;
use crate::server::jobstate::Job;
@@ -732,3 +732,34 @@ pub fn do_sync_job(
Ok(upid_str)
}
+
+pub(super) fn ignore_not_verified_or_encrypted(
+ manifest: &BackupManifest,
+ snapshot: &BackupDir,
+ verified_only: bool,
+ encrypted_only: bool,
+) -> bool {
+ if verified_only {
+ match manifest.verify_state() {
+ Ok(Some(verify_state)) if verify_state.state == VerifyState::Ok => (),
+ _ => {
+ info!("Snapshot {snapshot} not verified but verified-only set, snapshot skipped");
+ return true;
+ }
+ }
+ }
+
+ if encrypted_only {
+ // Consider only encrypted if all files in the manifest are marked as encrypted
+ if !manifest
+ .files()
+ .iter()
+ .all(|file| file.chunk_crypt_mode() == CryptMode::Encrypt)
+ {
+ info!("Snapshot {snapshot} not encrypted but encrypted-only set, snapshot skipped");
+ return true;
+ }
+ }
+
+ false
+}
--
2.39.5
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel