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 5CBD56B70D for ; Wed, 4 Aug 2021 10:45:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 52CA21C943 for ; Wed, 4 Aug 2021 10:45:56 +0200 (CEST) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id A8EBA1C938 for ; Wed, 4 Aug 2021 10:45:55 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 7819842DA0 for ; Wed, 4 Aug 2021 10:45:55 +0200 (CEST) Date: Wed, 4 Aug 2021 10:45:53 +0200 From: Wolfgang Bumiller To: Dominik Csapak Cc: pve-devel@lists.proxmox.com Message-ID: <20210804084553.nt5ajmlyr7p5jf2m@wobu-vie.proxmox.com> References: <20210803122954.2641138-1-d.csapak@proxmox.com> <20210803122954.2641138-2-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210803122954.2641138-2-d.csapak@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.649 Adjusted score from AWL reputation of From: address 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 Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [config.pm, lxc.pm] Subject: Re: [pve-devel] [PATCH container 1/2] add old config and unprivileged to check_ct_modify_config_perm 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, 04 Aug 2021 08:45:56 -0000 On Tue, Aug 03, 2021 at 02:29:51PM +0200, Dominik Csapak wrote: > we'll need that for checking the features more granularly > > Signed-off-by: Dominik Csapak > --- > src/PVE/API2/LXC.pm | 6 ++++-- > src/PVE/API2/LXC/Config.pm | 9 ++++++--- > src/PVE/LXC.pm | 2 +- > 3 files changed, 11 insertions(+), 6 deletions(-) > > diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm > index b929481..e16ce6c 100644 > --- a/src/PVE/API2/LXC.pm > +++ b/src/PVE/API2/LXC.pm > @@ -254,7 +254,7 @@ __PACKAGE__->register_method({ > my $ostemplate = extract_param($param, 'ostemplate'); > my $storage = extract_param($param, 'storage') // 'local'; > > - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, $param, []); > + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, undef, $param, [], $unprivileged); > > my $storage_cfg = cfs_read_file("storage.cfg"); > > @@ -1679,7 +1679,9 @@ __PACKAGE__->register_method({ > > die "no options specified\n" if !scalar(keys %$param); > > - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []); > + my $conf = PVE::LXC::Config->load_config($vmid); > + > + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, $param, [], $conf->{unprivileged}); 3 lines down we have a locked section where we load the config and use it for a digest check, so if we need the config for the check already, we should probably move the check itself down into the locked section now. Alternatively with this being the resize API call which shouldn't affect the config much, we could also add a comment and explicitly *not* pass the old config, but that's probably not as nice. > > my $storage_cfg = cfs_read_file("storage.cfg"); > > diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm > index 73fec36..ab136c0 100644 > --- a/src/PVE/API2/LXC/Config.pm > +++ b/src/PVE/API2/LXC/Config.pm > @@ -135,6 +135,9 @@ __PACKAGE__->register_method({ > my $node = extract_param($param, 'node'); > my $vmid = extract_param($param, 'vmid'); > > + my $conf = PVE::LXC::Config->load_config($vmid); Similar issue as above, but with a lot more work happening in between. AFAICT it's nothing too expensive though, so we could extend the locked section here, too. Alternatively remember this digest and add a 2nd digest check in the locked section below further down. > + my $unprivileged = $conf->{unprivileged}; > + > my $digest = extract_param($param, 'digest'); > > die "no options specified\n" if !scalar(keys %$param); > @@ -144,8 +147,8 @@ __PACKAGE__->register_method({ > my $revert_str = extract_param($param, 'revert'); > my @revert = PVE::Tools::split_list($revert_str); > > - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@delete]); > - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@revert]); > + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, {}, [@delete], $unprivileged); > + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, {}, [@revert], $unprivileged); > > foreach my $opt (@revert) { > raise_param_exc({ revert => "unknown option '$opt'" }) > @@ -166,7 +169,7 @@ __PACKAGE__->register_method({ > if grep(/^$opt$/, @revert); > } > > - PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []); > + PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, $param, [], $unprivileged); > > my $storage_cfg = PVE::Storage::config(); > > diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm > index 139f901..32a2127 100644 > --- a/src/PVE/LXC.pm > +++ b/src/PVE/LXC.pm > @@ -1242,7 +1242,7 @@ sub template_create { > } > > sub check_ct_modify_config_perm { > - my ($rpcenv, $authuser, $vmid, $pool, $newconf, $delete) = @_; > + my ($rpcenv, $authuser, $vmid, $pool, $oldconf, $newconf, $delete, $unprivileged) = @_; > > return 1 if $authuser eq 'root@pam'; > my $storage_cfg = PVE::Storage::config(); > -- > 2.30.2