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 8BA6F9D921 for ; Mon, 5 Jun 2023 12:12:39 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 68CBF26521 for ; Mon, 5 Jun 2023 12:12:39 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 5 Jun 2023 12:12:38 +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 7627048A51 for ; Mon, 5 Jun 2023 12:12:38 +0200 (CEST) Date: Mon, 05 Jun 2023 12:12:31 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20230604233709.1340089-1-aderumier@odiso.com> <20230604233709.1340089-4-aderumier@odiso.com> In-Reply-To: <20230604233709.1340089-4-aderumier@odiso.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1685958788.q855jk8v8t.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.073 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 T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH v2 qemu-server 1/1] api2: add check_bridge_access for create/update vm 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: Mon, 05 Jun 2023 10:12:39 -0000 On June 5, 2023 1:37 am, Alexandre Derumier wrote: > test first if user have access to the full zone (any bridge/vlan) > if a tag is defined, test if user have a specific access to the vlan (or = propagate from full bridge acl) > if no tag, test if user have access to full bridge. (if trunks are define= d, it need also access to full bridge) >=20 > Signed-off-by: Alexandre Derumier > --- > PVE/API2/Qemu.pm | 38 +++++++++++++++++++++++++++++++++++++- > 1 file changed, 37 insertions(+), 1 deletion(-) >=20 > diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm > index 587bb22..4de7b32 100644 > --- a/PVE/API2/Qemu.pm > +++ b/PVE/API2/Qemu.pm > @@ -46,6 +46,12 @@ use PVE::SSHInfo; > use PVE::Replication; > use PVE::StorageTunnel; > =20 > +my $have_sdn; > +eval { > + require PVE::Network::SDN; > + $have_sdn =3D 1; > +}; > + > BEGIN { > if (!$ENV{PVE_GENERATING_DOCS}) { > require PVE::HA::Env::PVE2; > @@ -601,6 +607,34 @@ my $check_vm_create_usb_perm =3D sub { > return 1; > }; > =20 > +my $check_bridge_access =3D sub { > + my ($rpcenv, $authuser, $param) =3D @_; > + > + return 1 if $authuser eq 'root@pam'; > + > + foreach my $opt (keys %{$param}) { > + next if $opt !~ m/^net\d+$/; > + my $net =3D PVE::QemuServer::parse_net($param->{$opt}); > + my $bridge =3D $net->{bridge}; > + my $tag =3D $net->{tag}; > + my $zone =3D 'local'; > + > + if ($have_sdn) { > + my $vnet_cfg =3D PVE::Network::SDN::Vnets::config(); > + if (defined(my $vnet =3D PVE::Network::SDN::Vnets::sdn_vnets_config= ($vnet_cfg, $bridge, 1))) { > + $zone =3D $vnet->{zone}; > + } > + } > + # test first if user have access to the full zone (any bridge/vlan) > + return 1 if $rpcenv->check_any($authuser, "/sdn/zones/$zone", ['SDN.Aud= it', 'SDN.Allocate'], 1); > + # if a tag is defined, test if user have a specific access to the vlan = (or propagate from full bridge acl) > + return 1 if $tag && $rpcenv->check_any($authuser, "/sdn/vnets/$bridge/$= tag", ['SDN.Audit', 'SDN.Allocate'], 1); this one IMHO should be $rpcenv->check($authuser, "/sdn/vnets/$bridge/$tag", ['SDN.Use']) if $tag; see comment in cover letter and below. > + # if no tag, test if user have access to full bridge. (if trunks are de= fined, it need also access to full bridge) > + $rpcenv->check_any($authuser, "/sdn/vnets/$bridge", ['SDN.Audit', 'SDN.= Allocate']); trunks also allow (sub)ranges, in that case we could check for those tags? or do we really need the full bridge? these checks here should check for (a not yet existing) 'SDN.Use' privilege, so that we can differentiate between: - Audit: see the config, but neither change nor use its entries - Use: use the entries - Allocate: modify the entries similar to Dominik's series for PCI/USB (where there is 'Modify' to manage the entries, and 'Use' to reference them in guest configs). I agree with Thomas that it likely makes sense to have all of that after the parse_net call in a helper in pve-guest-common, like: sub check_vnet_access { my ($rpcenv, $authuser, $vnet, $tag) =3D @_; ... } for code reuse with pve-container. > + } > + return 1; > +}; > + > my $check_vm_modify_config_perm =3D sub { > my ($rpcenv, $authuser, $vmid, $pool, $key_list) =3D @_; > =20 > @@ -878,7 +912,7 @@ __PACKAGE__->register_method({ > =20 > &$check_vm_create_serial_perm($rpcenv, $authuser, $vmid, $pool, $pa= ram); > &$check_vm_create_usb_perm($rpcenv, $authuser, $vmid, $pool, $param= ); > - > + &$check_bridge_access($rpcenv, $authuser, $param); > &$check_cpu_model_access($rpcenv, $authuser, $param); > =20 > $check_drive_param->($param, $storecfg); > @@ -1578,6 +1612,8 @@ my $update_vm_api =3D sub { > =20 > &$check_storage_access($rpcenv, $authuser, $storecfg, $vmid, $param)= ; > =20 > + &$check_bridge_access($rpcenv, $authuser, $param); > + > my $updatefn =3D sub { > =20 > my $conf =3D PVE::QemuConfig->load_config($vmid); > --=20 > 2.30.2 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20