[pve-devel] [PATCH pve-container 3/4] mountpoint_mount: disallow symlinks in bind mounts

[Wolfgang Bumiller]

symlinks in mount paths can cause security issues
assume the following setup:
mp1: local:X,mp=/disk2
mp2: /mnt/shared,mp=/shared
Now the container boots and executes this sequence:
ct:# ln -s /var/lib/lxc/$VMID/etc /disk2/shared
ct:# umount /disk2
ct:# ln -s /mnt /disk2
ct:# umount /shared
ct:# rmdir /shared
ct:# ln -s /etc /shared
ct:# poweroff
Now the owner waits for a stop-mode backup of the container
to be created:
mp1 will be mounted to the host's /mnt because the
container's /disk2 is a symlink to /mnt.
mp2 will now access the replaced /mnt/shared, which is a
symlink to the container's /etc, and mount that over the
container's /shared, which is a symlink to the host's /etc.
Now until the backup is finished the container's owner could
log into the host via ssh using his container's user
credentials.

We'll also unshare the mount namespace when performing such
backups, but it's still a bad idea to allow symlinks
modifying mount container paths.
---
 src/PVE/LXC.pm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index c7f7ef5..4b4dd2d 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -5,6 +5,8 @@ use warnings;
 use POSIX qw(EINTR);
 
 use File::Path;
+use File::Spec;
+use Cwd qw();
 use Fcntl ':flock';
 
 use PVE::Cluster qw(cfs_register_file cfs_read_file);
@@ -1960,6 +1962,15 @@ sub mountpoint_mount_path {
     return mountpoint_mount($mountpoint, undef, $storage_cfg, $snapname);
 }
 
+my $check_mount_path = sub {
+    my ($path) = @_;
+    $path = File::Spec->canonpath($path);
+    my $real = Cwd::realpath($path);
+    if ($real ne $path) {
+	die "mount path modified by symlink: $path != $real";
+    }
+};
+
 # use $rootdir = undef to just return the corresponding mount path
 sub mountpoint_mount {
     my ($mountpoint, $rootdir, $storage_cfg, $snapname) = @_;
@@ -1975,6 +1986,7 @@ sub mountpoint_mount {
 	$rootdir =~ s!/+$!!;
 	$mount_path = "$rootdir/$mount";
 	$mount_path =~ s!/+!/!g;
+	&$check_mount_path($mount_path);
 	File::Path::mkpath($mount_path);
     }
     
@@ -2031,6 +2043,7 @@ sub mountpoint_mount {
 	PVE::Tools::run_command(['mount', $volid, $mount_path]) if $mount_path;
 	return wantarray ? ($volid, 0) : $volid;
     } elsif ($volid !~ m|^/dev/.+| && $volid =~ m|^/.+| && -d $volid) {
+	&$check_mount_path($volid);
 	PVE::Tools::run_command(['mount', '-o', 'bind', $volid, $mount_path]) if $mount_path;
 	return wantarray ? ($volid, 0) : $volid;
     }
-- 
2.1.4