Re: [PATCH proxmox-backup v3 13/30] api: config: allow encryption key manipulation for sync job

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

On 4/19/26 11:06 PM, Thomas Lamprecht wrote:
> Am 14.04.26 um 14:59 schrieb Christian Ebner:
>> diff --git a/src/api2/config/sync.rs b/src/api2/config/sync.rs
>> @@ -586,6 +599,12 @@ pub fn update_sync_job(
>>
>> +    if let Some(associated_key) = update.associated_key {
>> +        data.associated_key = Some(associated_key);
>> +    }
> 
> AFAICT, the create path validates each associated key via
> sync_user_can_access_optional_key (around line 278), but the update path
> here just takes them as-is. Shouldn't the same validation loop be added
> here? Otherwise, afaict, a user could reference keys they don't have
> PRIV_SYS_MODIFY access to via an update, even though create correctly
> prevents it.

True, will add the checks for these as well, using the currently set 
owner. At this point, that is either the per-configured one or already 
the new owner if that is changed as well. So fine to check against 
data.owner.

> Related: the active_encryption_key access check only fires when the *key*
> itself is being updated, not when `owner` is updated. So reassigning
> ownership of a sync job to a user who lacks access to the already-set
> active_encryption_key passes validation here but then fails at job-run time.
> Probably worth re-validating the currently configured key(s) against the new
> owner whenever `owner` changes, but it's a bit odd situation either way.

True, will add checks for owner being changed, but not active encryption 
key or associated key. The other cases, where active encryption key or 
associated keys changed as well is then already covered by the 
respective checks as mentioned above.