public inbox for pve-devel@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] [PATCH container 2/3] create/restore: require Sys.Modify for privileged containers
Date: Wed, 30 Jul 2025 17:00:11 +0200	[thread overview]
Message-ID: <20250730150013.901666-3-f.gruenbichler@proxmox.com> (raw)
In-Reply-To: <20250730150013.901666-1-f.gruenbichler@proxmox.com>

except for in-place restore where both the current and the backed-up config are
already privileged.

this covers the following cases:
- creating a fresh container: defaults to unprivileged, requires Sys.Modify if set to privileged
- restoring with explicit override of unprivileged value to make the container privileged
- in-place restoring of privileged backup over unprivileged config
- restoring of privileged backup into new container

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/API2/LXC.pm | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index a247b80..951b1c7 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -139,7 +139,8 @@ __PACKAGE__->register_method({
         description =>
             "You need 'VM.Allocate' permission on /vms/{vmid} or on the VM pool /pool/{pool}. "
             . "For restore, it is enough if the user has 'VM.Backup' permission and the VM already exists. "
-            . "You also need 'Datastore.AllocateSpace' permissions on the storage.",
+            . "You also need 'Datastore.AllocateSpace' permissions on the storage. "
+            . "For privileged containers, 'Sys.Modify' permissions on '/' are required.",
     },
     protected => 1,
     proxyto => 'node',
@@ -254,6 +255,7 @@ __PACKAGE__->register_method({
             # fixme: limit allowed parameters
         } else {
             $unprivileged = 1 if !defined($unprivileged);
+            $rpcenv->check($authuser, '/', ['Sys.Modify']) if !$unprivileged;
         }
 
         my $force = extract_param($param, 'force');
@@ -289,12 +291,11 @@ __PACKAGE__->register_method({
             # since the user is lacking permission to configure the container's FW
             $skip_fw_config_restore = 1;
 
-            # error out if a user tries to change from unprivileged to privileged
+            # error out if a user tries to change from unprivileged to privileged without required privileges
             # explicit change is checked here, implicit is checked down below or happening in root-only paths
             my $conf = PVE::LXC::Config->load_config($vmid);
             if ($conf->{unprivileged} && defined($unprivileged) && !$unprivileged) {
-                raise_perm_exc(
-                    "cannot change from unprivileged to privileged without VM.Allocate");
+                $rpcenv->check($authuser, '/', ['Sys.Modify']);
             }
         } else {
             raise_perm_exc();
@@ -442,9 +443,12 @@ __PACKAGE__->register_method({
                         assert_not_restore_from_external($archive, $storage_cfg)
                             if !$conf->{unprivileged};
 
-                        # implicit privileged change is checked here
-                        if ($old_conf->{unprivileged} && !$conf->{unprivileged}) {
-                            $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Allocate']);
+                        # implicit privileged change, or creating a new privileged container is checked here
+                        if (
+                            (!$same_container_exists || $old_conf->{unprivileged})
+                            && !$conf->{unprivileged}
+                        ) {
+                            $rpcenv->check($authuser, '/', ['Sys.Modify']);
                         }
                     }
                 }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

  parent reply	other threads:[~2025-07-30 14:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-30 15:00 [pve-devel] [PATCH container/manager 0/4] restrict " Fabian Grünbichler
2025-07-30 15:00 ` [pve-devel] [PATCH container 1/3] api: create: default to unprivileged containers Fabian Grünbichler
2025-07-30 23:59   ` [pve-devel] applied: " Thomas Lamprecht
2025-07-30 15:00 ` Fabian Grünbichler [this message]
2025-07-30 23:59   ` [pve-devel] applied: [PATCH container 2/3] create/restore: require Sys.Modify for privileged containers Thomas Lamprecht
2025-07-30 15:00 ` [pve-devel] [PATCH container 3/3] migration: require Sys.Modify for incoming " Fabian Grünbichler
2025-07-30 23:59   ` [pve-devel] applied: " Thomas Lamprecht
2025-07-30 15:00 ` [pve-devel] [PATCH manager 1/1] lxc: create: always submit unprivileged field Fabian Grünbichler
2025-07-30 23:21   ` [pve-devel] applied: " Thomas Lamprecht

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=20250730150013.901666-3-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal