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 203786AC3D for ; Thu, 17 Mar 2022 11:12:36 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 16DB61900 for ; Thu, 17 Mar 2022 11:12:36 +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 id D46E118F5 for ; Thu, 17 Mar 2022 11:12:34 +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 A58E24266F for ; Thu, 17 Mar 2022 11:12:34 +0100 (CET) Date: Thu, 17 Mar 2022 11:12:25 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20220311112504.595964-1-o.bektas@proxmox.com> <20220311112504.595964-4-o.bektas@proxmox.com> In-Reply-To: <20220311112504.595964-4-o.bektas@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1647511532.im0b20gme1.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.182 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [qemu.pm, proxmox.com] Subject: Re: [pve-devel] [PATCH v2 qemu-server 03/12] api: allow 'skiplock' option to be used by SU privileged users 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: Thu, 17 Mar 2022 10:12:36 -0000 On March 11, 2022 12:24 pm, Oguz Bektas wrote: > Signed-off-by: Oguz Bektas > --- > PVE/API2/Qemu.pm | 59 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 40 insertions(+), 19 deletions(-) >=20 > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index 21fc82b..95cc46d 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -1126,8 +1126,8 @@ my $update_vm_api =3D sub { > my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($auth= user, "/vms/$vmid", ['SuperUser'], 1); > =20 > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option= ." }) > + if $skiplock && !$is_superuser; > =20 > my $delete_str =3D extract_param($param, 'delete'); > =20 > @@ -1645,9 +1645,11 @@ __PACKAGE__->register_method({ > my $authuser =3D $rpcenv->get_user(); > my $vmid =3D $param->{vmid}; > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); nit: line too long > + > my $skiplock =3D $param->{skiplock}; > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > my $early_checks =3D sub { > # test if VM exists > @@ -2290,6 +2292,12 @@ __PACKAGE__->register_method({ > my $timeout =3D extract_param($param, 'timeout'); > my $machine =3D extract_param($param, 'machine'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); same > + > + my $skiplock =3D extract_param($param, 'skiplock'); > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > + > my $get_root_param =3D sub { > my $value =3D extract_param($param, $_[0]); > raise_param_exc({ "$_[0]" =3D> "Only root may use this option." }) > @@ -2298,7 +2306,6 @@ __PACKAGE__->register_method({ > }; > =20 a comment here that this are intentionally still root@pam because they=20 are only used for migration-internal flows (and marking them as such in=20 the parameter description) would be nice.. > my $stateuri =3D $get_root_param->('stateuri'); > - my $skiplock =3D $get_root_param->('skiplock'); > my $migratedfrom =3D $get_root_param->('migratedfrom'); > my $migration_type =3D $get_root_param->('migration_type'); > my $migration_network =3D $get_root_param->('migration_network'); > @@ -2436,9 +2443,11 @@ __PACKAGE__->register_method({ > my $node =3D extract_param($param, 'node'); > my $vmid =3D extract_param($param, 'vmid'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); same > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > my $keepActive =3D extract_param($param, 'keepActive'); > raise_param_exc({ keepActive =3D> "Only root may use this option." }) and same for these here (keepactive -> vzdump, migratedfrom ->=20 migration) > @@ -2513,9 +2522,11 @@ __PACKAGE__->register_method({ > =20 > my $vmid =3D extract_param($param, 'vmid'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); same > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid); > =20 > @@ -2580,9 +2591,11 @@ __PACKAGE__->register_method({ > my $node =3D extract_param($param, 'node'); > my $vmid =3D extract_param($param, 'vmid'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); same > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > my $keepActive =3D extract_param($param, 'keepActive'); > raise_param_exc({ keepActive =3D> "Only root may use this option." }) again, comment here and in schema description would be nice > @@ -2739,9 +2752,11 @@ __PACKAGE__->register_method({ > =20 > my $statestorage =3D extract_param($param, 'statestorage'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); again > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > die "VM $vmid not running\n" if !PVE::QemuServer::check_running($vmid); > =20 > @@ -2811,9 +2826,11 @@ __PACKAGE__->register_method({ > =20 > my $vmid =3D extract_param($param, 'vmid'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); same > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > my $nocheck =3D extract_param($param, 'nocheck'); > raise_param_exc({ nocheck =3D> "Only root may use this option." }) and comment here again (migration?) > @@ -2883,9 +2900,11 @@ __PACKAGE__->register_method({ > =20 > my $vmid =3D extract_param($param, 'vmid'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option." }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this option." = }) > + if $skiplock && !$is_superuser; > =20 > PVE::QemuServer::vm_sendkey($vmid, $skiplock, $param->{key}); > =20 > @@ -4114,9 +4133,11 @@ __PACKAGE__->register_method({ > =20 > my $sizestr =3D extract_param($param, 'size'); > =20 > + my $is_superuser =3D $authuser eq 'root@pam' || $rpcenv->check($authuse= r, "/vms/$vmid", ['SuperUser'], 1); > + > my $skiplock =3D extract_param($param, 'skiplock'); > - raise_param_exc({ skiplock =3D> "Only root may use this option."= }) > - if $skiplock && $authuser ne 'root@pam'; > + raise_param_exc({ skiplock =3D> "Only superusers may use this op= tion." }) > + if $skiplock && !$is_superuser; > =20 > my $storecfg =3D PVE::Storage::config(); > =20 > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20