[pve-devel] [RFC container 3/3] implement permission checks for feature flags

[Wolfgang Bumiller]

To disable a feature it is enough to be generally allowed
to edit the configuration. Enabling a feature requires more
privileges. For now: root at pam.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---
 src/PVE/API2/LXC.pm        |  4 ++--
 src/PVE/API2/LXC/Config.pm |  5 ++---
 src/PVE/LXC.pm             | 42 +++++++++++++++++++++++++++++++++++++++++-
 src/PVE/LXC/Config.pm      |  2 +-
 4 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 55e6310..dcdca3e 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -244,7 +244,7 @@ __PACKAGE__->register_method({
 	my $ostemplate = extract_param($param, 'ostemplate');
 	my $storage = extract_param($param, 'storage') // 'local';
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, $param, []);
+	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, $pool, {}, $param, []);
 
 	my $storage_cfg = cfs_read_file("storage.cfg");
 
@@ -1567,7 +1567,7 @@ __PACKAGE__->register_method({
 
 	die "no options specified\n" if !scalar(keys %$param);
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
+	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, $param, []);
 
 	my $storage_cfg = cfs_read_file("storage.cfg");
 
diff --git a/src/PVE/API2/LXC/Config.pm b/src/PVE/API2/LXC/Config.pm
index 2b622b3..bfe6bb1 100644
--- a/src/PVE/API2/LXC/Config.pm
+++ b/src/PVE/API2/LXC/Config.pm
@@ -112,7 +112,7 @@ __PACKAGE__->register_method({
 	my $delete_str = extract_param($param, 'delete');
 	my @delete = PVE::Tools::split_list($delete_str);
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, [@delete]);
+	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, {}, {}, [@delete]);
 
 	foreach my $opt (@delete) {
 	    raise_param_exc({ delete => "you can't use '-$opt' and " .
@@ -124,8 +124,6 @@ __PACKAGE__->register_method({
 	    }
 	}
 
-	PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $param, []);
-
 	my $storage_cfg = cfs_read_file("storage.cfg");
 
 	my $repl_conf = PVE::ReplicationConfig->new();
@@ -156,6 +154,7 @@ __PACKAGE__->register_method({
 
 	    my $conf = PVE::LXC::Config->load_config($vmid);
 	    PVE::LXC::Config->check_lock($conf);
+	    PVE::LXC::check_ct_modify_config_perm($rpcenv, $authuser, $vmid, undef, $conf, $param, []);
 
 	    PVE::Tools::assert_if_modified($digest, $conf->{digest});
 
diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 46222ba..d9ecbed 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -1026,8 +1026,41 @@ sub template_create {
     PVE::LXC::Config->write_config($vmid, $conf);
 }
 
+sub check_feature_modify_perm {
+    my ($rpcenv, $authuser, $vmid, $pool, $old, $new) = @_;
+
+    my $features_desc = $PVE::LXC::Config::features_desc;
+
+    my $old_features = PVE::LXC::Config->parse_features($old);
+    my $new_features = PVE::LXC::Config->parse_features($new);
+
+    foreach my $feature (keys %$new_features) {
+	my $desc = $features_desc->{$feature};
+	my $type = $desc->{type};
+	my $new_value = $new_features->{$feature};
+	# Explicitly disabling a feature is like removing a feature (Also see
+	# the end of the function.)
+	next if !defined($new_value) ||
+		($type eq 'boolean' && !$new_value) ||
+		!length($new_value);
+
+	my $old_value = $old_features->{$feature};
+	# If we're not changing the feature's value, that's fine, too
+	next if defined($old_value) && $new_value eq $old_value;
+
+	# Otherwise, we can have feature-specific checks here.
+	# For now: require root at pam
+	my $action = defined($old_value) ? 'modify' : 'enable';
+	raise_perm_exc("only root\@pam can $action feature '$feature'")
+	    if $authuser ne 'root at pam';
+    }
+
+    # Whatever was in the old feature set abut not in the new feature set is
+    # being removed, which requires no special permissions.
+}
+
 sub check_ct_modify_config_perm {
-    my ($rpcenv, $authuser, $vmid, $pool, $newconf, $delete) = @_;
+    my ($rpcenv, $authuser, $vmid, $pool, $oldconf, $newconf, $delete) = @_;
 
     return 1 if $authuser eq 'root at pam';
 
@@ -1044,6 +1077,13 @@ sub check_ct_modify_config_perm {
 		if $data->{type} ne 'volume';
 	} elsif ($opt eq 'memory' || $opt eq 'swap') {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Memory']);
+	} elsif ($opt eq 'features') {
+	    # In any case we need the usual VM.Config.Options
+	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Options']);
+	    # But *adding* features requires special permissions:
+	    if (!$delete) {
+		check_feature_modify_perm($rpcenv, $authuser, $vmid, $pool, $oldconf->{$opt}, $newconf->{$opt});
+	    }
 	} elsif ($opt =~ m/^net\d+$/ || $opt eq 'nameserver' ||
 		 $opt eq 'searchdomain' || $opt eq 'hostname') {
 	    $rpcenv->check_vm_perm($authuser, $vmid, $pool, ['VM.Config.Network']);
diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 9f6765e..61c3126 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -272,7 +272,7 @@ PVE::JSONSchema::register_standard_option('pve-lxc-snapshot-name', {
     maxLength => 40,
 });
 
-my $features_desc = {
+our $features_desc = {
     mount => {
 	optional => 1,
 	type => 'string',
-- 
2.11.0