all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC manager 3/3] node console: lift root@pam restriction for commands
Date: Wed, 14 Jun 2023 12:42:15 +0200	[thread overview]
Message-ID: <20230614104215.359768-4-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20230614104215.359768-1-f.gruenbichler@proxmox.com>

instead, fallback to a plain login shell if the current user is not already
root. both current custom commands are effectively a root shell, so it's not
possible to allow them for regular users.

note that the non-login commands via xtermjs already had the fallback behaviour
(i.e., no check for $param->{cmd}) previous to this commit, it was just not
exposed via our web UI, since the corresponding button/wizard was only enabled
for root@pam.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    RFC because for a nice UX we probably want to somehow display or inject the
    command that should be executed once the user is (effectively) root in the
    console, instead of just opening a login prompt without any indication what the
    user should do with it..
    
    some possible options/suggestions offered so far:
    - let the API return the command in case of fallback, let the UI display it
    -- probably would work best if upgrade is converted to an inline xtermjs
       console, since that supports copy+paste
    - pass FAKE_SHELL to login, point it at a shell wrapper that echos a note with
      the command and then executes the real shell
    - pass FAKE_SHELL to login, point it at a wrapper that runs the command (or the
      command with sudo, in case the logged in console user is not root) with the
      user's real shell
    - wrap/reimplement the login binary to support passing a command to the
      shell (meh)
    
    the first one has the smallest chance of weird side-effects, but it also is
    less convenient (especially compared to the last two)

 PVE/API2/Nodes.pm           | 16 +++-------------
 www/manager6/Utils.js       |  5 +++--
 www/manager6/node/Config.js |  2 +-
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/PVE/API2/Nodes.pm b/PVE/API2/Nodes.pm
index 649735115..2595a0e52 100644
--- a/PVE/API2/Nodes.pm
+++ b/PVE/API2/Nodes.pm
@@ -948,7 +948,7 @@ __PACKAGE__->register_method ({
 	    node => get_standard_option('pve-node'),
 	    cmd => {
 		type => 'string',
-		description => "Run specific command or default to login (requires 'root\@pam')",
+		description => "Run specific command or default to login (only honored for 'root\@pam')",
 		enum => [keys %$shell_cmd_map],
 		optional => 1,
 		default => 'login',
@@ -997,11 +997,6 @@ __PACKAGE__->register_method ({
 	my $rpcenv = PVE::RPCEnvironment::get();
 	my ($user, undef, $realm) = PVE::AccessControl::verify_username($rpcenv->get_user());
 
-
-	if (defined($param->{cmd}) && $param->{cmd} ne 'login' && $user ne 'root@pam') {
-	    raise_perm_exc('user != root@pam');
-	}
-
 	my $node = $param->{node};
 
 	my $authpath = "/nodes/$node";
@@ -1086,7 +1081,7 @@ __PACKAGE__->register_method ({
 	    node => get_standard_option('pve-node'),
 	    cmd => {
 		type => 'string',
-		description => "Run specific command or default to login (requires 'root\@pam')",
+		description => "Run specific command or default to login (only honored for 'root\@pam')",
 		enum => [keys %$shell_cmd_map],
 		optional => 1,
 		default => 'login',
@@ -1216,7 +1211,7 @@ __PACKAGE__->register_method ({
 	    proxy => get_standard_option('spice-proxy', { optional => 1 }),
 	    cmd => {
 		type => 'string',
-		description => "Run specific command or default to login (requires 'root\@pam')",
+		description => "Run specific command or default to login (only honored for 'root\@pam')",
 		enum => [keys %$shell_cmd_map],
 		optional => 1,
 		default => 'login',
@@ -1239,11 +1234,6 @@ __PACKAGE__->register_method ({
 
 	my ($user, undef, $realm) = PVE::AccessControl::verify_username($authuser);
 
-
-	if (defined($param->{cmd}) && $param->{cmd} ne 'login' && $user ne 'root@pam') {
-	    raise_perm_exc('user != root@pam');
-	}
-
 	my $node = $param->{node};
 	my $proxy = $param->{proxy};
 
diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index a150e848f..c95873701 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1701,7 +1701,8 @@ Ext.define('PVE.Utils', {
 
     showCephInstallOrMask: function(container, msg, nodename, callback) {
 	if (msg.match(/not (installed|initialized)/i)) {
-	    if (Proxmox.UserName === 'root@pam') {
+	    var caps = Ext.state.Manager.get('GuiCap');
+	    if (caps.nodes['Sys.Console'] && caps.dc['Sys.Modify']) {
 		container.el.mask();
 		if (!container.down('pveCephInstallWindow')) {
 		    var isInstalled = !!msg.match(/not initialized/i);
@@ -1718,7 +1719,7 @@ Ext.define('PVE.Utils', {
 		}
 	    } else {
 		container.mask(Ext.String.format(gettext('{0} not installed.') +
-		    ' ' + gettext('Log in as root to install.'), 'Ceph'), ['pve-static-mask']);
+		    ' ' + gettext('Requires "Sys.Console" and "Sys.Modify" to install.'), 'Ceph'), ['pve-static-mask']);
 	    }
 	    return true;
 	} else {
diff --git a/www/manager6/node/Config.js b/www/manager6/node/Config.js
index 6ed2172aa..a4d10ce0d 100644
--- a/www/manager6/node/Config.js
+++ b/www/manager6/node/Config.js
@@ -248,7 +248,7 @@ Ext.define('PVE.node.Config', {
 		    itemId: 'apt',
 		    upgradeBtn: {
 			xtype: 'pveConsoleButton',
-			disabled: Proxmox.UserName !== 'root@pam',
+			disabled: !caps.nodes['Sys.Console'],
 			text: gettext('Upgrade'),
 			consoleType: 'upgrade',
 			nodename: nodename,
-- 
2.39.2





  parent reply	other threads:[~2023-06-14 10:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14 10:42 [pve-devel] [PATCH manager 0/3] rework node console permission checks Fabian Grünbichler
2023-06-14 10:42 ` [pve-devel] [PATCH manager 1/3] node console: restrict all non-login commands to root@pam Fabian Grünbichler
2023-11-06 14:38   ` [pve-devel] applied: " Thomas Lamprecht
2023-06-14 10:42 ` [pve-devel] [PATCH manager 2/3] node console: allow usage for non-pam realms Fabian Grünbichler
2023-11-06 14:38   ` [pve-devel] applied: " Thomas Lamprecht
2023-06-14 10:42 ` Fabian Grünbichler [this message]
2023-11-06 14:46   ` [pve-devel] [RFC manager 3/3] node console: lift root@pam restriction for commands Thomas Lamprecht
2023-11-06  7:36 ` [pve-devel] [PATCH manager 0/3] rework node console permission checks 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=20230614104215.359768-4-f.gruenbichler@proxmox.com \
    --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