Re: [PATCH proxmox-backup v3 19/30] fix #7251: api: push: encrypt snapshots using configured encryption key

Thomas Lamprecht
2026-04-19T21:08:27Z

Am 14.04.26 um 14:59 schrieb Christian Ebner:
> diff --git a/src/server/push.rs b/src/server/push.rs
>
> + if all_unencrypted {
> + encrypt_using_key = params.crypt_config.clone();
> + info!("Encrypt and push unencrypted snapshot '{snapshot}'");
> + } else if any_unencrypted {
> + warn!("Encountered partially encrypted snapshot '{snapshot}', refuse to re-encrypt and skip");
> + return Ok(stats);
> + } else {
> + info!("Pushing already encrypted snapshot '{snapshot}' without re-encryption");
> + }

The "already fully encrypted" case pushes client-encrypted content
through unchanged, regardless of whether the configured sync key matches
the client key. That's the right behavior (re-encrypting client-
encrypted content is impossible without the client key), but from the
user side this is surprising: they assigned a sync key and the log only
says "without re-encryption" - nothing indicates that the snapshot is
*not* using the configured sync key at all.

Please document this explicitly in the docs patch (30), and consider
making the log message say something like "already encrypted with client
key, configured sync key is not applied". Also, does a push job with an
`encrypted-only=true` filter + a configured sync key make sense at all?
If yes, the interaction is worth mentioning too.

> - target_manifest.unprotected = source_manifest.unprotected;
> - target_manifest.signature = source_manifest.signature;
> + target_manifest.unprotected = source_manifest.unprotected.clone();
> + target_manifest.signature = source_manifest.signature.clone();

When encrypt_using_key is Some, the target manifest's per-file csums
differ from the source (new ciphertexts), so a copied-over `signature`
won't verify on the target anymore. PBS-to-PBS sync rarely carries
client signatures in practice, but when it does, this produces a
snapshot that fails signature verification. Probably safest to drop
signature in the encrypt path and let the target manifest be unsigned.

Minor observation on commit hygiene: patch 16 adds `let mut
encrypt_using_key = None;` which is never assigned in that commit (only
starting here in patch 19), triggering an unused_mut warning on the
intermediate commit. Same pattern for `let mut new_manifest = None;` in
patch 26 vs its assignment in patch 28. Not critical, but it means
bisect-compiling each commit with -D warnings trips. Easy fix is to
introduce the `mut` in the same patch that first assigns it, or to use
`#[allow(unused_mut)]` on the intermediate.