all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [RFC common 2/4] tools: add 'check_for_root' helper
Date: Mon, 10 Jan 2022 14:45:18 +0100	[thread overview]
Message-ID: <1641819247.ufpnev051m.astroid@nora.none> (raw)
In-Reply-To: <20220105152215.1307583-3-o.bektas@proxmox.com>

On January 5, 2022 4:22 pm, Oguz Bektas wrote:
> checks if the rpcenv user is 'root@pam' or has the 'Sys.Root' privilege
> on a given API path
> 
> Signed-off-by: Oguz Bektas <o.bektas@proxmox.com>
> ---
>  src/PVE/Tools.pm | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/PVE/Tools.pm b/src/PVE/Tools.pm
> index 787942a..3e2e7f9 100644
> --- a/src/PVE/Tools.pm
> +++ b/src/PVE/Tools.pm
> @@ -2017,4 +2017,13 @@ sub get_file_hash {
>      return lc($digest);
>  }
>  
> +sub check_for_root {
> +    my ($path_to_check) = @_;
> +
> +    my $rpcenv = PVE::RPCEnvironment::get();
> +    my $authuser = $rpcenv->get_user();
> +    my $is_root = $authuser eq 'root@pam' || $rpcenv->check($authuser, $path_to_check, ['Sys.Root'], 1);
> +    return $is_root;
> +}

this is in the wrong place (pve-common is not concerned with 
access-control), move it to RPCEnvironment if kept.

this is only needed for the short-cut of not actually checking privs for 
the 'root@pam' user, since 'root@pam' by definition has 'Sys.Root' on 
all paths - so not sure whether the helper is necessary at all.

we don't have that many places limited to 'root@pam', so not sure 
whether simply doing:

# no short-cut
$rpcenv->check($authuser, $path_to_check, ['Sys.Root']);

or

# short-cut for user if in hot path somehow
$rpcenv->check($authuser, $path_to_check, ['Sys.Root'])
    if $authuser ne 'root@pam';

is not sufficient? it seems to be very short compared to the other 
helpers we have in PVE::RPCEnvironment ;)

> +
>  1;
> -- 
> 2.30.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 




  reply	other threads:[~2022-01-10 13:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05 15:22 [pve-devel] [RFC access-control common container qemu-server 0/4] #2582: Sys.Root privilege Oguz Bektas
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 [this message]
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=1641819247.ufpnev051m.astroid@nora.none \
    --to=f.gruenbichler@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