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 E5E311FF140 for ; Fri, 24 Apr 2026 12:36:23 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C70D5131AA; Fri, 24 Apr 2026 12:36:23 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup 1/3] sync: pull decrypt: check if contents only signed or fully encrypted Date: Fri, 24 Apr 2026 12:36:05 +0200 Message-ID: <20260424103607.531400-2-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260424103607.531400-1-c.ebner@proxmox.com> References: <20260424103607.531400-1-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1777026888481 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [pull.rs] Message-ID-Hash: FI2DPZGNRQUVTQS7JGI5T3EH46JS5A2S X-Message-ID-Hash: FI2DPZGNRQUVTQS7JGI5T3EH46JS5A2S 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: The key-fingerprint stored on the source manifest might be present if the snapshot contents are encrypted or signed. Checking the key fingerprints presence on the source manifest is therefore not enough to determine whether the snapshot is fully encrypted, rather each of the registered files chunk crypt mode has to be checked. Only decrypt on pull with a matching key if the snapshots contains only contains encrypted files, warn and fallback to regular pull if not. Adapt error messages on key mismatch to reflect that snapshot might be signed but not encrypted. Reported-by: Fabian Grünbichler Signed-off-by: Christian Ebner --- src/server/pull.rs | 90 ++++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 38 deletions(-) diff --git a/src/server/pull.rs b/src/server/pull.rs index 7fa273edb..051405ae7 100644 --- a/src/server/pull.rs +++ b/src/server/pull.rs @@ -758,52 +758,66 @@ async fn pull_snapshot<'a>( let mut crypt_config = None; let mut new_manifest = None; if let Some(key_fp) = manifest.fingerprint().with_context(|| prefix.clone())? { - // source is encrypted, find matching key + // source got key fingerprint, expect contents to be signed or encrypted if let Some((key_id, config)) = params .crypt_configs .iter() .find(|(_id, crypt_conf)| crypt_conf.fingerprint() == *key_fp.bytes()) { - manifest - .check_signature(config) - .context("failed to check source manifest signature") - .with_context(|| prefix.clone())?; - - if let Some(existing_manifest) = existing_target_manifest { - if let Some(existing_fingerprint) = existing_manifest - .fingerprint() - .with_context(|| prefix.clone())? - { - if existing_fingerprint != key_fp { - bail!("Detected local encrypted snapshot with encryption key mismatch!"); - } - } else if let Some(source_fp) = manifest - .get_change_detection_fingerprint() - .context("failed to parse change detection fingerprint of source manifest") - .with_context(|| prefix.clone())? - { - // Stored fp is HMAC over the unencrypted source's protected fields; recompute - // over the locally decrypted manifest, not the fresh encrypted remote one. - let target_fp = existing_manifest - .signature(config) - .with_context(|| prefix.clone())?; - if target_fp == *source_fp.bytes() { - fetch_log().await?; - cleanup().await?; - return Ok(sync_stats); // nothing changed - } - - bail!("Change detection fingerprint mismatch, refuse to continue"); - } - } else { + // check if source is encrypted or contents signed + if !manifest + .files() + .iter() + .all(|f| f.chunk_crypt_mode() == CryptMode::Encrypt) + { log_sender .log( - Level::INFO, - format!("Found matching key '{key_id}' with fingerprint {key_fp}, decrypt on pull"), + Level::WARN, + format!("Snapshot not fully encrypted, sync as is despite matching key '{key_id}' with fingerprint {key_fp}"), ) .await?; - crypt_config = Some(Arc::clone(config)); - new_manifest = Some(Arc::new(Mutex::new(BackupManifest::new(snapshot.into())))); + } else { + manifest + .check_signature(config) + .context("failed to check source manifest signature") + .with_context(|| prefix.clone())?; + + if let Some(existing_manifest) = existing_target_manifest { + if let Some(existing_fingerprint) = existing_manifest + .fingerprint() + .with_context(|| prefix.clone())? + { + if existing_fingerprint != key_fp { + bail!("Detected local encrypted or signed snapshot with encryption key mismatch!"); + } + } else if let Some(source_fp) = manifest + .get_change_detection_fingerprint() + .context("failed to parse change detection fingerprint of source manifest") + .with_context(|| prefix.clone())? + { + // Stored fp is HMAC over the unencrypted source's protected fields; recompute + // over the locally decrypted manifest, not the fresh encrypted remote one. + let target_fp = existing_manifest + .signature(config) + .with_context(|| prefix.clone())?; + if target_fp == *source_fp.bytes() { + fetch_log().await?; + cleanup().await?; + return Ok(sync_stats); // nothing changed + } + + bail!("Change detection fingerprint mismatch, refuse to continue"); + } + } else { + log_sender + .log( + Level::INFO, + format!("Found matching key '{key_id}' with fingerprint {key_fp}, decrypt on pull"), + ) + .await?; + crypt_config = Some(Arc::clone(config)); + new_manifest = Some(Arc::new(Mutex::new(BackupManifest::new(snapshot.into())))); + } } } else if let Some(existing_target_manifest) = existing_target_manifest { if let Some(existing_fingerprint) = existing_target_manifest @@ -812,7 +826,7 @@ async fn pull_snapshot<'a>( { if existing_fingerprint != key_fp { // pre-existing local manifest for encrypted snapshot with key mismatch - bail!("Local encrypted snapshot with different key detected, refuse to sync"); + bail!("Local encrypted or signed snapshot with different key detected, refuse to sync"); } } else { // pre-existing local manifest without key-fingerprint was previously decrypted, -- 2.47.3