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

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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