From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 6813F1FF0E0 for ; Thu, 23 Jul 2026 11:45:15 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D61872138E; Thu, 23 Jul 2026 11:45:14 +0200 (CEST) Message-ID: <7df11e72-d872-4002-8609-84c4717185aa@proxmox.com> Date: Thu, 23 Jul 2026 11:45:10 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup v2 5/5] sync: push: gracefully handle previous manifest signature mismatches To: Robert Obkircher References: <20260507130135.589100-1-c.ebner@proxmox.com> <20260507130135.589100-6-c.ebner@proxmox.com> <178471986679.177215.10555006588074374450.b4-review@b4> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784799881397 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.165 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust 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: TAUKRYAB6HPXYDLGOW5HZRYZKLUJOVYI X-Message-ID-Hash: TAUKRYAB6HPXYDLGOW5HZRYZKLUJOVYI 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 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: On 7/23/26 11:32 AM, Robert Obkircher wrote: > > On 22.07.26 16:48, Christian Ebner wrote: >> On 7/22/26 1:30 PM, Robert Obkircher wrote: >>>> 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. >> [..] >>>> +            .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)); >>> What if encrypt_using_key is None because the local chunks are already >>> encrypted? Shouldn't we expect a fingerprint in that case? >> >> The fingerprint here is the one of the previous manifest on the >> remote, so if that has no fingerprint (not encrypted), and we have >> no key to encrypted (encrypt_using_key == None) it is fine to use >> the manifest for de-duplication during chunk upload of the contained >> matching index files, analog to an incremental backup. So if the >> local snapshot to be pushed is already encrypted (not on the fly >> encrypted), it will be allowed to reuse the manifest, but there will >> be no matching chunk digest. Just like for a regular push without >> any server side encryption. >> >> At least that is the intended behavior. > Isn't it technically always fine to use it for deduplication and just > inefficient when nothing matches? > > The commit message states that we must not reuse when going from > unencrypted to "push encrypted". Why is that forbidden while going > from unencrypted to before-push encrypted seems to be allowed? Is it > because we know for sure that "push encrypt" will encrypt everything, > while the other case could still reference unencrypted chunks? Technically yes, but there is no point in trying to, see suggestions in https://lore.proxmox.com/pbs-devel/1777462096.q4ecgbr8bo.astroid@yuna.none/ Tried to keep the previous behavior for regular (non-server-side-encrypted) push though.