Re: [PATCH proxmox{,-backup} 00/20] fix #7251: implement server side encryption support for push sync jobs

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

On April 2, 2026 9:37 am, Christian Ebner wrote:
> On 4/2/26 2:24 AM, Thomas Lamprecht wrote:
>> Am 01.04.26 um 09:55 schrieb Christian Ebner:
>>> This patch series implements support for encrypting backup snapshots
>>> when pushing from a source PBS instance to an untrusted remote target
>>> PBS instance. Further, it adds support to decrypt snapshots being
>>> encrypted on the remote source PBS when pulling the contents to the
>>> local target PBS instance.
>> 

[..]

> 
>> When a server-side key is configured on push, all snapshots with any
>> encrypted content are silently skipped. That includes fully client-side
>> encrypted ones that could just be synced as-is - they are already encrypted
>> after all. Might be better to push those unchanged and only skip genuinely
>> partially-encrypted snapshots? Or if unsure, allow to user to chose what
>> they want. Wrapping encryption is probably never a good idea though,
>> maybe just any parts that are not yet encrypted?
> 
> Pushing fully encrypted ones as is makes sense, only log and skip 
> partially encrypted snapshots.

pushing fully encrypted snapshots *if they use the same key*, or all
fully encrypted snapshots? I think the former is okay, the latter might
be confusing/unexpected and lead to "I have all my backups on the remote
but can't decrypt them because of the wrong key"?

e.g.:

client A uses key A to backup to PBS X
client B uses plaintext to backup to PBS X
PBS X uses key B to sync to PBS Y

user thinks key B is enough to restore backups from Y (after all, it's
the encryption key used for syncing), and destroys clients A and B and
PBS X (all in the same location maybe?). now they can't recover client
A..

I know this is technically on the user (they didn't pay attention to
their key management), but seems like a potential foot gun. if we skip
the groups of client A because of a key mismatch (and warn about that?)
then it is at least obvious that those backups *are not on Y*. we could
have a flag for allowing key mismatches to support such scenarios
opt-in?

>> Nit: The same `encryption_key` field means "encrypt" for push and
>> "decrypt" for pull. Maybe be literal here and name it decryption_key for
>> the later. Or something generic that works for both.
> 
> Yes, that make sense... Was primed by the fact that the entities 
> themself are referred to as encryption keys, but renaming this to 
> decryption_key for the pull makes more sense for making the code better 
> readable. Will adapt that.
> 
>> A few key lifecycle things:
>> 
>> Deleting a key does not check whether sync jobs reference it; the next run
>> just fails. Might want to refuse deletion while references exist, or at
>> least warn.
> 
> Yes, also realized this just now, will add a check for this in the api 
> handler and not allow to remove it in that case.

key rotation/lifecycle management would be a question in any case - we
could have one "active" key per sync job, and a list of archived ones
that are only used for decryption? a key rotation would obviously break
deduplication chains.. such a scheme would also allow re-encryption on
the fly of client-encrypted backups, if a matching key is provided -
though the use case for that seems limited to me (why encrypt in the
first place if you trust this PBS with the plaintext data?).

or alternatively it would need to be documented that for rotating a key,
you want to create a new key and target namespace, then switch the sync
job over (syncing everything from scratch), and delete the old copies
encrypted with the previous key once the re-sync is done?