Re: [PATCH proxmox-backup v2 5/5] sync: push: gracefully handle previous manifest signature mismatches

[Christian Ebner <c.ebner@proxmox.com>]

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.