all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Oguz Bektas <o.bektas@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC access-control common container qemu-server 0/4] #2582: Sys.Root privilege
Date: Wed,  5 Jan 2022 16:22:11 +0100	[thread overview]
Message-ID: <20220105152215.1307583-1-o.bektas@proxmox.com> (raw)

sending this in as RFC, because i think it needs a bit of ironing
out and discussing the quirky bits ;)

i'm not done with pve-manager patch so that'll come with the v1, though API
should work for testing the changes.

it probably makes sense to also add a helper there, since at the moment
we only check if Proxmox.UserName === 'root@pam' or in some cases
specific permissions for storage and so forth, in order to decide
whether to show/enable some GUI elements.

container API already works pretty well. VM API should also work but i haven't
tested this extensively w.r.t. storage and migration.

some questions that popped up in my head:

+ should adding 'Sys.Root' privilege to a user give them all available
privileges on that path? this would make sense as having a
root-equivalent privilege should be already enough (otherwise it doesn't
have much of a point?). since at some places we have things like:

-------
        } elsif ($target_vmid) {
            $rpcenv->check_vm_perm($authuser, $target_vmid, undef, ['VM.Config.Disk'])
-               if $authuser ne 'root@pam';
+               if !$is_root;
-------

where one could theoretically have root-eq privs but not the 'VM.Config.Disk' for the target vm...


+ $authuser could also be an (optional?) parameter for the helper in
PVE::Tools, so that we could check arbitrary users and not only the current
one. also on most of these we already call $rpcenv->get_user() before doing the
check, so we could spare that call inside the helper if we do that
consistently.

+ would it make sense to be able to give 'Sys.Root' on a single node (like on
/nodes/foo instead of the whole cluster)? this seemed like a rabbithole to me
since we'd have to lock down quite a bit of stuff to limit movement from one
cluster member to the other, without any(?) worthwhile benefits? or might make
sense to just allow 'Sys.Root' to be given on '/' (since it should be
equivalent to root@pam anyway)

+ should root@pam have 'Sys.Root' by default? or does it make sense to still
differentiate the "real" root user and the "impersonated" one?





 pve-access-control:

 Oguz Bektas (1):
  add Sys.Root privilege

 src/PVE/AccessControl.pm | 1 +
 1 file changed, 1 insertion(+)

 pve-common:

 Oguz Bektas (1):
  tools: add 'check_for_root' helper

 src/PVE/Tools.pm | 9 +++++++++
 1 file changed, 9 insertions(+)

 pve-container:

 Oguz Bektas (1):
  fix #2582: api: use common helper for checking root privileges

 src/PVE/API2/LXC.pm | 5 ++---
 src/PVE/LXC.pm      | 8 +++++---
 2 files changed, 7 insertions(+), 6 deletions(-)

 qemu-server:

 Oguz Bektas (1):
  api: use common helper for checking root privileges

 PVE/API2/Qemu.pm | 74 ++++++++++++++++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 25 deletions(-)


-- 
2.30.2




             reply	other threads:[~2022-01-05 15:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 15:22 Oguz Bektas [this message]
2022-01-05 15:22 ` [pve-devel] [RFC access-control 1/4] add " Oguz Bektas
2022-01-05 15:22 ` [pve-devel] [RFC common 2/4] tools: add 'check_for_root' helper Oguz Bektas
2022-01-10 13:45   ` Fabian Grünbichler
2022-01-05 15:22 ` [pve-devel] [PATCH container 3/4] fix #2582: api: use common helper for checking root privileges Oguz Bektas
2022-01-05 15:22 ` [pve-devel] [RFC qemu-server 4/4] " Oguz Bektas
2022-01-10 13:45   ` Fabian Grünbichler
2022-01-10 13:45 ` [pve-devel] [RFC access-control common container qemu-server 0/4] #2582: Sys.Root privilege Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220105152215.1307583-1-o.bektas@proxmox.com \
    --to=o.bektas@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal