From c06b7c8b907f5cdece397ef0d9663f07d6fcf394 Mon Sep 17 00:00:00 2001
From: Mira Limbeck <m.limbeck@proxmox.com>
Date: Mon, 11 Nov 2024 16:01:03 +0100
Subject: [PATCH] iscsi: verify volume disks are part of target
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We build the disk path by appending the last part of the volname to
/dev/disk/by-id. These could in theory be any other disk found under
there instead of a LUN provided by the target configured.

This patch adds a way to verify the disk used is actually provided by
the target. To do so `udevadm` is used to get the devpath
(/devices/...). This can then be checked under `/sys` for a session.
With the session the targetname can be looked up under /sys/class and
compared with the configured target of the storage.
In case of multipath, all disks backing the multipath device are checked
recursively (in case of nested device mapper devices), and verification
succeeds if at least one backing disk is part of the iSCSI target.
Mixing disks from different iSCSI targets is allowed as long as one
corresponds to the right target.

udevadm input is limited to `/dev/` paths since we only pass those either
explicitly, or via Cwd::realpath on a /dev/disk/by-id path returned by
filesystem_path.

According to [0] /sys/subsystems should be preferred over /sys/class if
available, but neither kernel 6.8 nor kernel 6.11 provided it. It is
mentioned that in the future this will be moved to /sys/subsystems. So
this has to be kept in mind for future kernels.

[0] https://www.kernel.org/doc/html/v6.11/admin-guide/sysfs-rules.html

Reported-by: Friedrich Weber <f.weber@proxmox.com>
Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
Tested-by: Friedrich Weber <f.weber@proxmox.com>
Reviewed-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 src/PVE/Storage/ISCSIPlugin.pm | 94 ++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/src/PVE/Storage/ISCSIPlugin.pm b/src/PVE/Storage/ISCSIPlugin.pm
index 5f5cbf7..6de7610 100644
--- a/src/PVE/Storage/ISCSIPlugin.pm
+++ b/src/PVE/Storage/ISCSIPlugin.pm
@@ -463,6 +463,100 @@ sub deactivate_storage {
     }
 }
 
+my $check_devices_part_of_target = sub {
+    my ($device_paths, $target) = @_;
+
+    my $found = 0;
+    for my $path (@$device_paths) {
+	if ($path =~ m!^/devices/platform/host\d+/session(\d+)/target\d+:\d:\d!) {
+	    my $session_id = $1;
+
+	    my $targetname = file_read_firstline(
+		"/sys/class/iscsi_session/session$session_id/targetname",
+	    );
+	    if ($targetname && ($targetname eq $target)) {
+		$found = 1;
+		last;
+	    }
+	}
+    }
+    return $found;
+};
+
+my $udev_query_path = sub {
+    my ($dev) = @_;
+
+    # only accept device names (see `man udevadm`)
+    ($dev) = $dev =~ m!^(/dev/.+)$!; # untaint
+    die "invalid device for udevadm path query\n" if !defined($dev);
+
+    my $device_path;
+    my $cmd = [
+	'udevadm',
+	'info',
+	'--query=path',
+	$dev,
+    ];
+    eval {
+	run_command($cmd, outfunc => sub {
+	    $device_path = shift;
+	});
+    };
+    die "failed to query device path for '$dev': $@\n" if $@;
+
+    ($device_path) = $device_path =~ m!^(/devices/.+)$!; # untaint
+    die "invalid resolved device path\n" if !defined($device_path);
+
+    return $device_path;
+};
+
+my $resolve_virtual_devices;
+$resolve_virtual_devices = sub {
+    my ($dev, $visited) = @_;
+
+    $visited = {} if !defined($visited);
+
+    my $resolved = [];
+    if ($dev =~ m!^/devices/virtual/block/!) {
+	dir_glob_foreach("/sys/$dev/slaves", '([^.].+)', sub {
+	    my ($slave) = @_;
+
+	    # don't check devices multiple times
+	    return if $visited->{$slave};
+	    $visited->{$slave} = 1;
+
+	    my $path;
+	    eval { $path = $udev_query_path->("/dev/$slave"); };
+	    return if $@;
+
+	    my $nested_resolved = $resolve_virtual_devices->($path, $visited);
+
+	    push @$resolved, @$nested_resolved;
+	});
+    } else {
+	push @$resolved, $dev;
+    }
+
+    return $resolved;
+};
+
+sub activate_volume {
+    my ($class, $storeid, $scfg, $volname, $snapname, $cache) = @_;
+
+    my $path = $class->filesystem_path($scfg, $volname, $snapname);
+    my $real_path = Cwd::realpath($path);
+    die "failed to get realpath for '$path': $!\n" if !$real_path;
+    # in case $path does not exist or is not a symlink, check if the returned
+    # $real_path is a block device
+    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 $found = $check_devices_part_of_target->($resolved_paths, $scfg->{target});
+    die "volume '$volname' not part of target '$scfg->{target}'\n" if !$found;
+}
+
 sub check_connection {
     my ($class, $storeid, $scfg) = @_;