From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id BCFFB1FF13B for ; Wed, 22 Apr 2026 17:18:54 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 782E62400D; Wed, 22 Apr 2026 17:18:54 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Wed, 22 Apr 2026 17:18:15 +0200 Message-Id: Subject: Re: [PATCH proxmox-backup v4 13/30] api: config: allow encryption key manipulation for sync job From: =?utf-8?q?Michael_K=C3=B6ppl?= To: "Christian Ebner" , Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0 References: <20260420161533.1055484-1-c.ebner@proxmox.com> <20260420161533.1055484-14-c.ebner@proxmox.com> In-Reply-To: <20260420161533.1055484-14-c.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776871008072 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.099 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: DHNYW5AZUTWI5SVV5VSRVLYYMZJRKJIV X-Message-ID-Hash: DHNYW5AZUTWI5SVV5VSRVLYYMZJRKJIV X-MailFrom: m.koeppl@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Noticed a problem when updating sync jobs. Tried to explain it below. Hope it's comprehensible and I didn't miss anything. On Mon Apr 20, 2026 at 6:15 PM CEST, Christian Ebner wrote: [snip] > if let Some(group_filter) =3D update.group_filter { > data.group_filter =3D Some(group_filter); > @@ -555,6 +603,34 @@ pub fn update_sync_job( > data.sync_direction =3D Some(sync_direction); > } > =20 > + if let Some(active_encryption_key) =3D update.active_encryption_key = { > + // owner updated above already, so can use the one in data > + let owner =3D data > + .owner > + .as_ref() > + .unwrap_or_else(|| Authid::root_auth_id()); > + sync_user_can_access_optional_key(Some(&active_encryption_key), = owner, true)?; > + > + keep_previous_key_as_associated( > + data.active_encryption_key.as_ref(), > + &mut update.associated_key, > + ); > + data.active_encryption_key =3D Some(active_encryption_key); Suppose the user has a sync job configured with the following keys: - Active key: key-a - Associated keys: key-b Now the user updates the sync job with proxmox-backup-manager sync-job update "${SYNC_JOB_ID}" \ --active-encryption-key "key-c" Note that there's explicitly no associated keys set as part of the update. Then `update.associated_key` is None when passed into `keep_previous_key_as_associated`, therefore following the match arm ``` ... None =3D> *associated_keys =3D Some(vec![prev.clone()]), ... ``` putting the previously active key as the only element of the list of associated keys. continue in the next comment... > + } > + > + if let Some(associated_key) =3D update.associated_key { > + // owner updated above already, so can use the one in data > + let owner =3D data > + .owner > + .as_ref() > + .unwrap_or_else(|| Authid::root_auth_id()); > + // Don't allow associating keys the local user/owner can't acces= s > + for key in &associated_key { > + sync_user_can_access_optional_key(Some(key), owner, false)?; > + } > + data.associated_key =3D Some(associated_key); Since above `update.associated key` is set to a vector containing only the previously active key, this point is reached and the list of associated keys is overwritten. The resulting sync job will then have the following: - Active key: key-c - Associated keys: key-a key-b is gone without the user explicitly removing it or updating the list of associated keys. > + } > + > if update.limit.rate_in.is_some() { > data.limit.rate_in =3D update.limit.rate_in; > } > @@ -727,6 +803,8 @@ acl:1:/remote/remote1/remotestore1:write@pbs:RemoteSy= ncOperator > run_on_mount: None, > unmount_on_done: None, > sync_direction: None, // use default > + active_encryption_key: None, > + associated_key: None, > }; > =20 > // should work without ACLs