From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id B1A748B57 for ; Wed, 16 Nov 2022 15:05:14 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8DCC220623 for ; Wed, 16 Nov 2022 15:04:43 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 16 Nov 2022 15:04:42 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4EB0C44D6C for ; Wed, 16 Nov 2022 15:04:39 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Date: Wed, 16 Nov 2022 15:04:34 +0100 Message-Id: <20221116140435.93067-6-f.ebner@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20221116140435.93067-1-f.ebner@proxmox.com> References: <20221116140435.93067-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: =?UTF-8?Q?0=0A=09?=AWL 0.027 Adjusted score from AWL reputation of From: =?UTF-8?Q?address=0A=09?=BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict =?UTF-8?Q?Alignment=0A=09?=SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF =?UTF-8?Q?Record=0A=09?=SPF_PASS -0.001 SPF: sender matches SPF =?UTF-8?Q?record=0A=09?=URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [backup.pm] Subject: [pve-devel] [PATCH manager 5/6] api: backup: require Datastore.Allocate on storage X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Nov 2022 14:05:14 -0000 In particular this ensures that the user is allowed to remove data on the storage, because configuring low retention results in removed older backups. Of course setting the storage itself also needs to require the same privilege then. This is a breaking API change, but it seems sensible to require permissions on the affected storage too. Jobs with a dumpdir setting can be configured by root only. Suggested-by: Thomas Lamprecht Signed-off-by: Fiona Ebner --- Not sure about the dumpdir jobs, should those rather require Sys.Modify on /? The dumpdir setting itself is still root-only. PVE/API2/Backup.pm | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm index 684a078e..74bf95ca 100644 --- a/PVE/API2/Backup.pm +++ b/PVE/API2/Backup.pm @@ -54,12 +54,39 @@ sub assert_param_permission_common { } } +my sub assert_param_permission_create { + my ($rpcenv, $user, $param) = @_; + return if $user eq 'root@pam'; # always OK + + assert_param_permission_common($rpcenv, $user, $param); + + if (!$param->{dumpdir}) { + my $storeid = $param->{storage} || 'local'; + $rpcenv->check($user, "/storage/$storeid", [ 'Datastore.Allocate' ]); + } # no else branch, because dumpdir is root-only +} + my sub assert_param_permission_update { - my ($rpcenv, $user, $update, $delete) = @_; + my ($rpcenv, $user, $update, $delete, $current) = @_; return if $user eq 'root@pam'; # always OK assert_param_permission_common($rpcenv, $user, $update); assert_param_permission_common($rpcenv, $user, $delete); + + if ($update->{storage}) { + $rpcenv->check($user, "/storage/$update->{storage}", [ 'Datastore.Allocate' ]) + } elsif ($delete->{storage}) { + $rpcenv->check($user, "/storage/local", [ 'Datastore.Allocate' ]); + } + + return if !$current; # early check done + + if ($current->{dumpdir}) { + die "only root\@pam may edit jobs with a 'dumpdir' option."; + } else { + my $storeid = $current->{storage} || 'local'; + $rpcenv->check($user, "/storage/$storeid", [ 'Datastore.Allocate' ]); + } } my $convert_to_schedule = sub { @@ -220,7 +247,7 @@ __PACKAGE__->register_method({ my $rpcenv = PVE::RPCEnvironment::get(); my $user = $rpcenv->get_user(); - assert_param_permission_common($rpcenv, $user, $param); + assert_param_permission_create($rpcenv, $user, $param); if (my $pool = $param->{pool}) { $rpcenv->check_pool_exist($pool); @@ -473,6 +500,8 @@ __PACKAGE__->register_method({ die "no such vzdump job\n" if !$job || $job->{type} ne 'vzdump'; } + assert_param_permission_update($rpcenv, $user, $param, $delete, $job); + my $deletable = { comment => 1, 'repeat-missed' => 1, -- 2.30.2