[PATCH proxmox-backup 0/2] gracefully handle signature mismatch from previous manifest load during encrypting push sync

[PATCH proxmox-backup 0/2] gracefully handle signature mismatch from previous manifest load during encrypting push sync

[Christian Ebner]

This patches more gracefully handle an otherwise rather alerting log
message during push sync jobs with configured active encryption key,
stating a key mismatch in the manifest signature check error.

During push, the previous snapshot of the backup group on the remote
target is being used for chunk deduplicaton on upload. When fetching,
the previous manifest's signature is checked against the backup
writer's key, which may however not match if the previous snapshot is
encrypted by a different key.
This being normal operation, log a less alerting log message stating
that deduplication using the previous snapshot is skipped instead.

Christian Ebner (2):
  client: allow skipping signature check on previous manifest fetching
  sync: push: gracefully handle previous manifest signature mismatches

 pbs-client/src/backup_writer.rs | 22 +++++++++++++++++++---
 src/server/push.rs              | 24 ++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 5 deletions(-)

-- 
2.47.3

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

[Christian Ebner]

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> {
+        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)?;
 
-        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

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

[Christian Ebner]

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.

If the previous backup snapshot in the same group (on the remote
target) is however encrypted or signed using a different key, the
signature check will fail, logging a rather alerting signature
mismatch, which is however benign.

Improve the logging behaviour by loading the manifest without
implicit signature check, check it on the call site and log
a more telling `Skip chunk deduplication from previous manifest`,
as that is the intended behaviour for this case.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/push.rs | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index dac62c84a..f4b68701d 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -1131,8 +1131,28 @@ pub(crate) async fn push_snapshot(
     let mut previous_manifest = None;
     // Use manifest of previous snapshots in group on target for chunk upload deduplication
     if fetch_previous_manifest {
-        match backup_writer.download_previous_manifest().await {
-            Ok(manifest) => previous_manifest = Some(Arc::new(manifest)),
+        match backup_writer
+            .download_previous_manifest_without_signature_check()
+            .await
+        {
+            Ok(manifest) => {
+                if let Some((_id, crypt_config)) = &encrypt_using_key {
+                    if manifest.check_signature(crypt_config).is_ok() {
+                        previous_manifest = Some(Arc::new(manifest));
+                    } else {
+                        log_sender
+                            .log(
+                                Level::INFO,
+                                format!(
+                                    "{prefix}: Skip chunk deduplication from previous manifest"
+                                ),
+                            )
+                            .await?
+                    }
+                } else {
+                    previous_manifest = Some(Arc::new(manifest));
+                }
+            }
             Err(err) => {
                 log_sender
                     .log(
-- 
2.47.3

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

[Fabian Grünbichler]

On April 29, 2026 11:28 am, Christian Ebner 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.
> 
> If the previous backup snapshot in the same group (on the remote
> target) is however encrypted or signed using a different key, the
> signature check will fail, logging a rather alerting signature
> mismatch, which is however benign.
> 
> Improve the logging behaviour by loading the manifest without
> implicit signature check, check it on the call site and log
> a more telling `Skip chunk deduplication from previous manifest`,
> as that is the intended behaviour for this case.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  src/server/push.rs | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/src/server/push.rs b/src/server/push.rs
> index dac62c84a..f4b68701d 100644
> --- a/src/server/push.rs
> +++ b/src/server/push.rs
> @@ -1131,8 +1131,28 @@ pub(crate) async fn push_snapshot(
>      let mut previous_manifest = None;
>      // Use manifest of previous snapshots in group on target for chunk upload deduplication
>      if fetch_previous_manifest {
> -        match backup_writer.download_previous_manifest().await {
> -            Ok(manifest) => previous_manifest = Some(Arc::new(manifest)),
> +        match backup_writer
> +            .download_previous_manifest_without_signature_check()
> +            .await
> +        {
> +            Ok(manifest) => {
> +                if let Some((_id, crypt_config)) = &encrypt_using_key {
> +                    if manifest.check_signature(crypt_config).is_ok() {
> +                        previous_manifest = Some(Arc::new(manifest));

see below, but this is incomplete - if there is no signature, this will
also return ok!

> +                    } else {
> +                        log_sender
> +                            .log(
> +                                Level::INFO,
> +                                format!(
> +                                    "{prefix}: Skip chunk deduplication from previous manifest"
> +                                ),
> +                            )
> +                            .await?
> +                    }

this part should check if the signature was made with the configured
key first, before attempting to validate the signature..

we basically have a matrix here of previous manifest state and whether
we ware doing a pushing encrypt or not..

if we don't have a key, we only need to skip the previous manifest if
it's encryption state doesn't match the currently pushed snapshot. this
is missing (but maybe handled somewhere else?).

if we have a key, we want to
- check if the previous snapshot was encrypted, if not, we cannot use it
  but this is fine.
- if it was encrypted, we want to check if it was encrypted using our
  key - if not, we cannot use it, but this is fine.
- if it was encrypted using our key, we want to validate the signature -
  if that fails, we cannot use it and should probably emit a warning,
  something is very wrong (either the fingerprint or the signature is
  invalid, which should never happen)

> +                } else {
> +                    previous_manifest = Some(Arc::new(manifest));
> +                }
> +            }
>              Err(err) => {
>                  log_sender
>                      .log(
> -- 
> 2.47.3
> 
> 
> 
> 
> 
> 

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

[Fabian Grünbichler]

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..

> +        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?

>  
> -        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
> 
> 
> 
> 
> 
> 

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

[Christian Ebner]

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
>>
>>
>>
>>
>>
>>