[PATCH v3 storage 11/76] iscsi: refuse virtual devices other than multipath maps as volumes

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

The target check of an iSCSI volume accepts a virtual device if any
disk below it belongs to the target. The rule is meant for multipath
maps, whose paths all lead to one LUN, possibly through different
target names. A linear map, a logical volume or a software RAID can
combine a disk of the target with disks outside of it and passes the
same check. Used as the base volume of an LVM storage, such a device
is initialized as a physical volume, which also writes to the disks
outside the target.

Accept a virtual device only if it is a multipath map or a partition
of one; any of its paths still proves membership. The volume list of
an iSCSI storage only offers SCSI disks and multipath maps, so only
hand-written volume names are affected.

Fixes: c06b7c8b ("iscsi: verify volume disks are part of target")
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---
 src/PVE/Storage/ISCSIPlugin.pm     | 11 +++-
 src/test/run_iscsi_volume_tests.pl | 81 ++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index 536b2e94..81f06979 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -617,6 +617,14 @@ $resolve_virtual_devices = sub {
 
     my $resolved = [];
     if ($dev =~ m!^/devices/virtual/block/!) {
+        # All paths of a multipath map lead to one LUN, possibly through other target names. Other
+        # virtual devices, such as linear maps or logical volumes, can include disks outside the
+        # target, so one disk of the target does not make them part of it.
+        my $uuid = file_read_firstline("/sys/$dev/dm/uuid") // '';
+        my ($name) = $dev =~ m!([^/]+)$!;
+        die "'$name' is neither a multipath map nor a partition of one\n"
+            if $uuid !~ m/^(?:part\d+-)?mpath-\S+$/;
+
         dir_glob_foreach(
             "/sys/$dev/slaves",
             '([^.].+)',
@@ -654,7 +662,8 @@ sub activate_volume {
     die "resolved realpath '$real_path' is not a block device\n" if !-b $real_path;
 
     my $device_path = $udev_query_path->($real_path);
-    my $resolved_paths = $resolve_virtual_devices->($device_path);
+    my $resolved_paths = eval { $resolve_virtual_devices->($device_path) };
+    die "volume '$volname' not part of target '$scfg->{target}' - $@" if $@;
 
     my $found = $check_devices_part_of_target->($resolved_paths, $scfg->{target});
     die "volume '$volname' not part of target '$scfg->{target}'\n" if !$found;
diff --git a/src/test/run_iscsi_volume_tests.pl b/src/test/run_iscsi_volume_tests.pl
index eb8a86cd..cc94fc3a 100755
--- a/src/test/run_iscsi_volume_tests.pl
+++ b/src/test/run_iscsi_volume_tests.pl
@@ -13,39 +13,94 @@ use PVE::Storage::ISCSIPlugin;
 my ($blockdev) = grep { -b $_ } map { s!^/sys/block/!/dev/!r } glob('/sys/block/*');
 plan skip_all => 'block device required for volume activation' if !$blockdev;
 
-my $device_path;
+my $pci = '/devices/pci0000:00/0000:00:03.0/0000:05:00.0';
+
+# What udev and sysfs report for the virtual devices of the cases below and the disks they use.
+my $udev_paths = {
+    '/dev/sda' => "$pci/ata1/host0/target0:0:0/0:0:0:0/block/sda",
+    '/dev/sdb' => '/devices/platform/host3/session1/target3:0:0/3:0:0:1/block/sdb',
+    '/dev/sdc' => '/devices/platform/host4/session2/target4:0:0/4:0:0:1/block/sdc',
+    '/dev/dm-3' => '/devices/virtual/block/dm-3',
+};
+my $dm_uuids = {
+    'dm-3' => 'mpath-3600a098038323536',
+    'dm-4' => 'part1-mpath-3600a098038323536',
+    'dm-5' => '',
+    'dm-6' => 'LVM-0hRrw7Rb3cd2WVzQ0ZjOhyAZB9KDGUj5x3wNPtbWZLQfx3m5lhIsm0dw7Js5Gs7v',
+    'dm-7' => 'mpath-3600a098038323537',
+};
+my $slaves = {
+    'dm-3' => ['sdb', 'sdc'],
+    'dm-4' => ['dm-3'],
+    'dm-5' => ['sda', 'sdb'],
+    'dm-6' => ['dm-3'],
+    'dm-7' => ['sdc'],
+    'md0' => ['sda', 'sdb'],
+};
+
+my ($device_path, $queried);
 my $plugin = Test::MockModule->new('PVE::Storage::ISCSIPlugin');
 $plugin->mock(
     filesystem_path => sub { $blockdev },
     run_command => sub {
         my ($cmd, %options) = @_;
-        $options{outfunc}->($device_path);
+        my $path = $queried++ ? $udev_paths->{ $cmd->[-1] } : $device_path;
+        die "no device path for '$cmd->[-1]'\n" if !defined($path);
+        $options{outfunc}->($path);
+    },
+    dir_glob_foreach => sub {
+        my ($dir, $regex, $func) = @_;
+        my ($name) = $dir =~ m!/([^/]+)/slaves$!;
+        $func->($_) for ($slaves->{$name} // [])->@*;
     },
     file_read_firstline => sub {
         my ($file) = @_;
+        return $dm_uuids->{$1} if $file =~ m!/([^/]+)/dm/uuid$!;
         return $file =~ m!/session1/targetname$! ? 'iqn.test' : 'iqn.other';
     },
 );
 
-my $pci = '/devices/pci0000:00/0000:00:03.0/0000:05:00.0';
+my $rejected = "volume '0.0.0.scsi-test' not part of target 'iqn.test'";
+my $not_multipath =
+    sub { "$rejected - '$_[0]' is neither a multipath map nor a partition of one\n" };
+
 for my $case (
-    ['software iSCSI', '/devices/platform/host3/session1/target3:0:0/3:0:0:1/block/sdb', 1],
-    ['iSER or offload', "$pci/host7/session1/target7:0:0/7:0:0:1/block/sdc", 1],
+    ['software iSCSI', '/devices/platform/host3/session1/target3:0:0/3:0:0:1/block/sdb', ''],
+    ['iSER or offload', "$pci/host7/session1/target7:0:0/7:0:0:1/block/sdc", ''],
     [
         'session of another target',
         '/devices/platform/host3/session2/target3:0:0/3:0:0:1/block/sdb',
-        0,
+        "$rejected\n",
     ],
     [
         'offload session of another target',
         "$pci/host7/session2/target7:0:0/7:0:0:1/block/sdc",
-        0,
+        "$rejected\n",
+    ],
+    ['local SATA disk', "$pci/ata1/host0/target0:0:0/0:0:0:0/block/sda", "$rejected\n"],
+    ['local NVMe disk', "$pci/nvme/nvme0/nvme0n1", "$rejected\n"],
+    ['multipath map with paths through two target names', '/devices/virtual/block/dm-3', ''],
+    ['partition of a multipath map', '/devices/virtual/block/dm-4', ''],
+    ['multipath map of another target', '/devices/virtual/block/dm-7', "$rejected\n"],
+    [
+        'linear map with a disk of the target',
+        '/devices/virtual/block/dm-5',
+        $not_multipath->('dm-5'),
+    ],
+    [
+        'logical volume on a multipath map',
+        '/devices/virtual/block/dm-6',
+        $not_multipath->('dm-6'),
+    ],
+    [
+        'software RAID with a disk of the target',
+        '/devices/virtual/block/md0',
+        $not_multipath->('md0'),
     ],
-    ['local SATA disk', "$pci/ata1/host0/target0:0:0/0:0:0:0/block/sda", 0],
-    ['local NVMe disk', "$pci/nvme/nvme0/nvme0n1", 0],
 ) {
-    my ($name, $path, $accepted) = $case->@*;
+    my ($name, $path, $error) = $case->@*;
     $device_path = $path;
+    $queried = 0;
 
     eval {
         PVE::Storage::ISCSIPlugin->activate_volume(
@@ -54,11 +109,7 @@ for my $case (
             '0.0.0.scsi-test',
         );
     };
-    is(
-        $@,
-        $accepted ? '' : "volume '0.0.0.scsi-test' not part of target 'iqn.test'\n",
-        "$name: " . ($accepted ? 'accepted' : 'rejected'),
-    );
+    is($@, $error, "$name: " . ($error eq '' ? 'accepted' : 'rejected'));
 }
 
 done_testing();
-- 
2.47.3