From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 40F641FF13F for ; Thu, 07 May 2026 15:02:37 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1A10E1BA2B; Thu, 7 May 2026 15:02:37 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2 5/5] sync: push: gracefully handle previous manifest signature mismatches Date: Thu, 7 May 2026 15:01:35 +0200 Message-ID: <20260507130135.589100-6-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260507130135.589100-1-c.ebner@proxmox.com> References: <20260507130135.589100-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1778158810568 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.070 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 Message-ID-Hash: K6S65ZBJMJ47NTG5YP6OKPUKBCEAPEYL X-Message-ID-Hash: K6S65ZBJMJ47NTG5YP6OKPUKBCEAPEYL X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: During push sync jobs with a given active encryption key, the key is loaded for the backup writer, used to encrypt the source snapshot on the fly. As optimization, the backup writer deduplicates chunks already present in the previous snapshot by loading them from the index files as stored in the previous manifest. Reuse of the previous manifest and index must however only happen if: - The previous manifest is not encrypted, neither will push encrypt. - The previous manifest is encrypted and passes signature verification using the same key to be used during push encryption. Since these checks are now performed when loading the previous manifest, drop redundant and now useless checks performed before loading the chunks from index files. Signed-off-by: Christian Ebner --- src/server/push.rs | 81 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/src/server/push.rs b/src/server/push.rs index afa78b751..4495b68ed 100644 --- a/src/server/push.rs +++ b/src/server/push.rs @@ -979,25 +979,12 @@ pub(crate) async fn push_group( } async fn load_previous_snapshot_known_chunks( - params: &PushParameters, upload_options: &UploadOptions, backup_writer: &BackupWriter, archive_name: &BackupArchiveName, known_chunks: Arc>>, ) { if let Some(manifest) = upload_options.previous_manifest.as_ref() { - if let Some((_id, crypt_config)) = ¶ms.crypt_config { - // no fingerprint in previous manifest -> no reuse possible - let Ok(Some(fingerprint)) = manifest.fingerprint() else { - return; - }; - - if *fingerprint.bytes() != crypt_config.fingerprint() { - // key mismatch -> no reuse possible - return; - } - } - // Add known chunks, ignore errors since archive might not be present and it is better // to proceed on unrelated errors than to fail here. match archive_name.archive_type() { @@ -1128,18 +1115,73 @@ pub(crate) async fn push_snapshot( .await .with_context(|| prefix.to_string())?; - let mut previous_manifest = None; // Use manifest of previous snapshots in group on target for chunk upload deduplication - if fetch_previous_manifest { - match backup_writer.download_previous_manifest(false).await { - Ok(manifest) => previous_manifest = Some(Arc::new(manifest)), + let previous_manifest = if !fetch_previous_manifest { + None + } else { + let result = backup_writer + .download_previous_manifest(false) + .await + .map_err(|err| err.context("failed to download")) + .and_then(|manifest| { + let fingerprint = manifest + .fingerprint() + .context("failed getting fingerprint")?; + + let Some(manifest_key_fp) = fingerprint else { + if encrypt_using_key.is_none() { + return Ok(Arc::new(manifest)); + } + bail!("manifest not encrypted"); + }; + + let signed_only = manifest + .files() + .iter() + .all(|f| f.chunk_crypt_mode() == CryptMode::None); + + let Some((_key_id, crypt_config)) = &encrypt_using_key else { + if signed_only { + return Ok(Arc::new(manifest)); + } + bail!("manifest encrypted but no encryption key configured"); + }; + + if *manifest_key_fp.bytes() == crypt_config.fingerprint() { + if let Some(expected_signature) = &manifest.signature { + let calculated_signature = manifest + .signature(crypt_config) + .context("signature calculation failed")?; + if hex::encode(calculated_signature) == *expected_signature { + return Ok(Arc::new(manifest)); + } + bail!("signature mismatch with configured encryption key"); + } + bail!("manifest not signed with configured encryption key"); + } + bail!( + "encryption key of previous snapshot does not match configured encryption key" + ); + }); + + match result { + Ok(prev) => { + log_sender + .log( + Level::INFO, + format!("{prefix}: Reusing previous manifest for deduplication"), + ) + .await?; + Some(prev) + } Err(err) => { log_sender .log( Level::INFO, - format!("{prefix}: Could not download previous manifest - {err}"), + format!("{prefix}: Skip reuse of previous manifest - {err:#}"), ) - .await? + .await?; + None } } }; @@ -1184,7 +1226,6 @@ pub(crate) async fn push_snapshot( }; load_previous_snapshot_known_chunks( - params, &upload_options, &backup_writer, &archive_name, -- 2.47.3