[pve-devel] [PATCH container/manager 0/4] restrict privileged containers

[pve-devel] [PATCH container/manager 0/4] restrict privileged containers

[Fabian Grünbichler]

this series
- defaults to unprivileged containers in the backend (already the
  default in the UI for a while)
- requires Sys.Modify when creating a new privileged container, or
  converting and existing unprivileged one to a privileged one via
  in-place restore

pve-container technically breaks old pve-manager, insofar as privileged
container creation via the UI is not honored.

pve-container:

Fabian Grünbichler (3):
  api: create: default to unprivileged containers
  create/restore: require Sys.Modify for privileged containers
  migration: require Sys.Modify for incoming privileged containers

 src/PVE/API2/LXC.pm | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

pve-manager:

Fabian Grünbichler (1):
  lxc: create: always submit unprivileged field

 www/manager6/lxc/CreateWizard.js | 1 +
 1 file changed, 1 insertion(+)

-- 
2.39.5



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

[pve-devel] [PATCH container 1/3] api: create: default to unprivileged containers

[Fabian Grünbichler]

restore keeps what is in the backup config, but allows switching to
unprivileged. switching from unprivileged to privileged requires VM.Allocate at
the moment.

the config schema default cannot easily be changed to unprivileged, as that
would break existing configs.

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

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index a56c441..a247b80 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -252,6 +252,8 @@ __PACKAGE__->register_method({
 
         if ($restore) {
             # fixme: limit allowed parameters
+        } else {
+            $unprivileged = 1 if !defined($unprivileged);
         }
 
         my $force = extract_param($param, 'force');
-- 
2.39.5



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

[pve-devel] [PATCH container 2/3] create/restore: require Sys.Modify for privileged containers

[Fabian Grünbichler]

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

[pve-devel] [PATCH container 3/3] migration: require Sys.Modify for incoming privileged containers

[Fabian Grünbichler]

an incoming remote migration is akin to a container creation, so treat it the same.

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

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 951b1c7..2574739 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -3036,6 +3036,7 @@ __PACKAGE__->register_method({
                             unprivileged => $unprivileged,
                             arch => $arch,
                         };
+                        $rpcenv->check($authuser, '/', ['Sys.Modify']) if !$unprivileged;
                         PVE::LXC::check_ct_modify_config_perm(
                             $rpcenv,
                             $authuser,
-- 
2.39.5



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

[pve-devel] [PATCH manager 1/1] lxc: create: always submit unprivileged field

[Fabian Grünbichler]

even if unchecked, since the backend now defaults to unprivileged if not
defined at all.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 www/manager6/lxc/CreateWizard.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/www/manager6/lxc/CreateWizard.js b/www/manager6/lxc/CreateWizard.js
index 2971991e2..0f6fcce8d 100644
--- a/www/manager6/lxc/CreateWizard.js
+++ b/www/manager6/lxc/CreateWizard.js
@@ -65,6 +65,7 @@ Ext.define('PVE.lxc.CreateWizard', {
                     xtype: 'proxmoxcheckbox',
                     name: 'unprivileged',
                     value: true,
+                    uncheckedValue: 0,
                     bind: {
                         value: '{unprivileged}',
                     },
-- 
2.39.5



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

[pve-devel] applied: [PATCH manager 1/1] lxc: create: always submit unprivileged field

[Thomas Lamprecht]

On Wed, 30 Jul 2025 17:00:13 +0200, Fabian Grünbichler wrote:
> even if unchecked, since the backend now defaults to unprivileged if not
> defined at all.
> 
> 

Applied this one already, thanks!

[1/1] lxc: create: always submit unprivileged field
      commit: ffd144b299c03ea340b2e9f5f3fead40b30e373e


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

[pve-devel] applied: [PATCH container 1/3] api: create: default to unprivileged containers

[Thomas Lamprecht]

On Wed, 30 Jul 2025 17:00:10 +0200, Fabian Grünbichler wrote:
> restore keeps what is in the backup config, but allows switching to
> unprivileged. switching from unprivileged to privileged requires VM.Allocate at
> the moment.
> 
> the config schema default cannot easily be changed to unprivileged, as that
> would break existing configs.
> 
> [...]

Applied, thanks!

[1/3] api: create: default to unprivileged containers
      commit: 103068aa6449a0d5768715269b0b7946dd9b40e1


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

[pve-devel] applied: [PATCH container 2/3] create/restore: require Sys.Modify for privileged containers

[Thomas Lamprecht]

On Wed, 30 Jul 2025 17:00:11 +0200, Fabian Grünbichler wrote:
> 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
> 
> [...]

Applied, thanks!

[2/3] create/restore: require Sys.Modify for privileged containers
      commit: 4cc73b7b7566ac6a9f87f995919d4e562c8c0db7


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

[pve-devel] applied: [PATCH container 3/3] migration: require Sys.Modify for incoming privileged containers

[Thomas Lamprecht]

On Wed, 30 Jul 2025 17:00:12 +0200, Fabian Grünbichler wrote:
> an incoming remote migration is akin to a container creation, so treat it the same.
> 
> 

Applied, thanks!

[3/3] migration: require Sys.Modify for incoming privileged containers
      commit: 9466f341c1e869f0a65f44ecdebcb21a7b6f9311


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