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 7D15A1FF136 for ; Mon, 20 Apr 2026 10:21:35 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5BE372016A; Mon, 20 Apr 2026 10:21:35 +0200 (CEST) Message-ID: <4397d618-6d48-4096-8cf9-ba51b57a3c9e@proxmox.com> Date: Mon, 20 Apr 2026 10:21:29 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup v3 13/30] api: config: allow encryption key manipulation for sync job To: Thomas Lamprecht References: <20260419210610.3915597-1-t.lamprecht@proxmox.com> <20260419210610.3915597-4-t.lamprecht@proxmox.com> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20260419210610.3915597-4-t.lamprecht@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776673206340 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.070 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: A237VYZFHFMIKTLHALDULZJHMAYWZ2UB X-Message-ID-Hash: A237VYZFHFMIKTLHALDULZJHMAYWZ2UB X-MailFrom: c.ebner@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 CC: pbs-devel@lists.proxmox.com 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: 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.