Re: [PATCH proxmox-backup 1/2] client: allow skipping signature check on previous manifest fetching

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

On 4/29/26 1:41 PM, Fabian Grünbichler wrote:
> On April 29, 2026 11:28 am, Christian Ebner wrote:
>> Extends the BackupWriter implementation by a method which allows to
>> download the previous manifest from a PBS instance, but skipping the
>> signature check even if the backup writer has a crypt_config set.
>>
>> This will be used to silence misleading logs during encrypting push
>> sync jobs, by performing the signature check on the call site
>> instead.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>>   pbs-client/src/backup_writer.rs | 22 +++++++++++++++++++---
>>   1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
>> index 49aff3fdd..ea990dbde 100644
>> --- a/pbs-client/src/backup_writer.rs
>> +++ b/pbs-client/src/backup_writer.rs
>> @@ -735,8 +735,25 @@ impl BackupWriter {
>>           })
>>       }
>>   
>> -    /// Download backup manifest (index.json) of last backup
>> +    /// Download backup manifest (index.json) of last backup, checking the signature
>> +    /// using the backup writer's crypt config.
>>       pub async fn download_previous_manifest(&self) -> Result<BackupManifest, Error> {
> 
> since we only have two callsites for this (one for each variant ;)), couldn't we just switch to
> 
> download_previous_manifest(&self, check_signature: bool)
> 
> ? would make the writer interface a little less bloated..

Okay, will move it to a boolean flag instead.

> 
>> +        let manifest = self
>> +            .download_previous_manifest_without_signature_check()
>> +            .await?;
>> +
>> +        if let Some(crypt_config) = &self.crypt_config {
>> +            manifest.check_signature(crypt_config)?;
>> +        }
>> +
>> +        Ok(manifest)
>> +    }
>> +
>> +    /// Download backup manifest (index.json) of last backup, but skips the signature
>> +    /// check by not providing the crypt config when parsing the manifest from the data blob.
>> +    pub async fn download_previous_manifest_without_signature_check(
>> +        &self,
>> +    ) -> Result<BackupManifest, Error> {
>>           let mut raw_data = Vec::with_capacity(64 * 1024);
>>   
>>           let param = json!({ "archive-name": MANIFEST_BLOB_NAME.to_string() });
>> @@ -748,8 +765,7 @@ impl BackupWriter {
>>           // no expected digest available
>>           let data = blob.decode(self.crypt_config.as_ref().map(Arc::as_ref), None)?;
> 
> pre-existing, but this makes no sense - manifests are never encrypted,
> so passing the key here is just misleading?

The DataBlob::decode() does however fallback to use the crypt config's 
compute_digest implementation instead of the fallback sha256sum. Not 
sure about the implementation details, must have a closer look if it is 
actually fine to drop this.

> 
>>   
>> -        let manifest =
>> -            BackupManifest::from_data(&data[..], self.crypt_config.as_ref().map(Arc::as_ref))?;
>> +        let manifest = BackupManifest::from_data(&data[..], None)?;
>>   
>>           Ok(manifest)
>>       }
>> -- 
>> 2.47.3
>>
>>
>>
>>
>>
>>