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 2FE741FF143 for ; Sat, 11 Apr 2026 10:51:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DC27B517D; Sat, 11 Apr 2026 10:52:41 +0200 (CEST) From: Thomas Lamprecht To: c.ebner@proxmox.com Subject: Re: [PATCH proxmox-backup v2 18/27] fix #7251: api: push: encrypt snapshots using configured encryption key Date: Sat, 11 Apr 2026 10:02:11 +0200 Message-ID: <20260411085154.1961287-7-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260410165454.1578501-19-c.ebner@proxmox.com> References: <20260411085154.1961287-1-t.lamprecht@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775897455073 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.002 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. [push.rs] Message-ID-Hash: HPR4LPHDI35UVZXCXF3QJBVASLUQMCMC X-Message-ID-Hash: HPR4LPHDI35UVZXCXF3QJBVASLUQMCMC X-MailFrom: t.lamprecht@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 CC: pbs-devel@lists.proxmox.com 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: Am 10.04.26 um 18:54 schrieb Christian Ebner: > diff --git a/src/server/push.rs b/src/server/push.rs > @@ -886,6 +886,27 @@ pub(crate) async fn push_snapshot( > > + if params.crypt_config.is_some() { > + let mut contains_unencrypted_file = false; > + // Check if snapshot is fully encrypted or not encrypted at all: > + // refuse progress otherwise to upload partially unencrypted contents or mix encryption key. > + if source_manifest.files().iter().all(|file| { > + if file.chunk_crypt_mode() == CryptMode::None { > + contains_unencrypted_file = true; > + true > + } else { > + false > + } > + }) { nit: this is hard to follow - .all() returning true here means "every file is unencrypted" because the closure returns true for None and false for encrypted. Maybe just spell out the three states, e.g. something like: let all_none = files.iter().all(|f| f.chunk_crypt_mode() == CryptMode::None); let any_none = files.iter().any(|f| f.chunk_crypt_mode() == CryptMode::None); if all_none { encrypt } else if any_none { warn partial, skip } else { push as-is } > + } else if contains_unencrypted_file { > + warn!("Encountered partially encrypted snapshot, refuse to re-encrypt and skip"); nit: I'd probably include the snapshot name here, otherwise the admin cannot tell which snapshot was skipped in a multi-snapshot sync run as e.g. getting that from context, e.g. log lines above might feel like guess work to some admins, especially if we do parallel syncing. OTOH. if we add a prefix for all log message on parallel syncing, this nit might not matter anymore. btw: still has log::warn!/log::info! on lines 873-874 and log::info! on line 931 while everything else uses tracing macros.