[pve-devel] [PATCH container v4 1/4] extract apparmor profile & namespace switch to a helper

[Filip Schauer <f.schauer@proxmox.com>]

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 src/PVE/LXC.pm | 75 ++++++++++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 30 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 741bb33..47fb86a 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -2125,15 +2125,42 @@ sub __mountpoint_mount {
     die "unsupported storage";
 }
 
+my $enter_mnt_ns_and_change_aa_profile = sub {
+    my ($mnt_ns, $aa_profile, $code_pre_changeprofile) = @_;
+
+    # Grab a file descriptor to our apparmor label file so we can change the profile
+    sysopen(my $aa_fd, "/proc/self/attr/current", O_WRONLY)
+        or die "failed to open '/proc/self/attr/current' for writing: $!\n";
+
+    # But switch namespaces first, to make sure the namespace switches aren't blocked by
+    # apparmor.
+    PVE::Tools::setns(fileno($mnt_ns), PVE::Tools::CLONE_NEWNS);
+    chdir('/')
+        or die "failed to change root directory within mount namespace: $!\n";
+
+    $code_pre_changeprofile->() if defined($code_pre_changeprofile);
+
+    # Now switch our apparmor profile:
+    my $data = "changeprofile $aa_profile";
+    my $data_written = syswrite($aa_fd, $data, length($data));
+    if (!defined($data_written) || $data_written != length($data)) {
+        die "failed to change apparmor profile: $!\n";
+    }
+    # Check errors on close as well:
+    close($aa_fd)
+        or die "failed to change apparmor profile (close() failed): $!\n";
+};
+
 sub mountpoint_hotplug : prototype($$$$$) {
     my ($vmid, $conf, $opt, $mp, $storage_cfg) = @_;
 
     my (undef, $root_uid, $root_gid) = PVE::LXC::parse_id_maps($conf);
 
-    # We do the rest in a fork with an unshared mount namespace, because:
-    #  -) change our papparmor profile to that of /usr/bin/lxc-start
-    #  -) we're now going to 'stage' # the mountpoint, then grab it, then move into the
-    #     container's namespace, then mount it.
+    # We do the rest in a fork with an unshared mount namespace:
+    #  -) change our apparmor profile to 'pve-container-mounthotplug', which is '/usr/bin/lxc-start'
+    #     with move_mount privileges on every mount.
+    #  -) we're now going to 'stage' the mountpoint, then grab it, then move into the container's
+    #     namespace, then mount it.
 
     PVE::Tools::run_fork(sub {
         # Pin the container pid longer, we also need to get its monitor/parent:
@@ -2146,32 +2173,20 @@ sub mountpoint_hotplug : prototype($$$$$) {
         my $ct_mnt_ns = $get_container_namespace->($vmid, $ct_pid, 'mnt');
         my $monitor_mnt_ns = $get_container_namespace->($vmid, $monitor_pid, 'mnt');
 
-        # Grab a file descriptor to our apparmor label file so we can change into the 'lxc-start'
-        # profile to lower our privileges to the same level we have in the start hook:
-        sysopen(my $aa_fd, "/proc/self/attr/current", O_WRONLY)
-            or die "failed to open '/proc/self/attr/current' for writing: $!\n";
-        # But switch namespaces first, to make sure the namespace switches aren't blocked by
-        # apparmor.
-
-        # Change into the monitor's mount namespace. We "pin" the mount into the monitor's
-        # namespace for it to remain active there since the container will be able to unmount
-        # hotplugged mount points and thereby potentially free up loop devices, which is a security
-        # concern.
-        PVE::Tools::setns(fileno($monitor_mnt_ns), PVE::Tools::CLONE_NEWNS);
-        chdir('/')
-            or die "failed to change root directory within the monitor's mount namespace: $!\n";
-
-        my $dir = get_staging_mount_path($opt);
-
-        # Now switch our apparmor profile before mounting:
-        my $data = 'changeprofile pve-container-mounthotplug';
-        my $data_written = syswrite($aa_fd, $data, length($data));
-        if (!defined($data_written) || $data_written != length($data)) {
-            die "failed to change apparmor profile: $!\n";
-        }
-        # Check errors on close as well:
-        close($aa_fd)
-            or die "failed to change apparmor profile (close() failed): $!\n";
+        # -) Change into the monitor's mount namespace. We "pin" the mount into the monitor's
+        #    namespace for it to remain active there since the container will be able to unmount
+        #    hotplugged mount points and thereby potentially free up loop devices, which is a
+        #    security concern.
+        # -) Prepare the staging directory.
+        # -) Switch to the 'pve-container-mounthotplug' apparmor profile.
+        my $dir;
+        $enter_mnt_ns_and_change_aa_profile->(
+            $monitor_mnt_ns,
+            "pve-container-mounthotplug",
+            sub {
+                $dir = get_staging_mount_path($opt);
+            },
+        );
 
         my $mount_fd = mountpoint_stage($mp, $dir, $storage_cfg, undef, $root_uid, $root_gid);
 
-- 
2.47.2



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