Toshaan Bharvani
6 months ago
8 changed files with 403 additions and 1 deletions
@ -0,0 +1,60 @@
@@ -0,0 +1,60 @@
|
||||
From eb917d346bc8592924c5f6566b01841176c53c8c Mon Sep 17 00:00:00 2001 |
||||
From: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Mon, 22 Aug 2022 16:27:11 +0200 |
||||
Subject: [PATCH] udiskslinuxblock: Only permit ATA Secure Erase during |
||||
Format() on a whole block device |
||||
|
||||
ATA Secure Erase requested as an option to the Format() method call used |
||||
to perform the actual erase on a whole drive object it looked up. When |
||||
Format() was called on a partition, this led to data loss on a whole drive. |
||||
This commit adds a safeguard to check that the Format() is requested |
||||
on a whole block device. |
||||
|
||||
Severity of this issue was slightly lowered by a failure to submit |
||||
the ATA Secure erase command in case some filesystem was mounted |
||||
at that point. |
||||
--- |
||||
src/udiskslinuxblock.c | 16 ++++++++++++++++ |
||||
1 file changed, 16 insertions(+) |
||||
|
||||
diff --git a/src/udiskslinuxblock.c b/src/udiskslinuxblock.c |
||||
index d1da94edf..db0ed2bf6 100644 |
||||
--- a/src/udiskslinuxblock.c |
||||
+++ b/src/udiskslinuxblock.c |
||||
@@ -2354,6 +2354,7 @@ erase_ata_device (UDisksBlock *block, |
||||
{ |
||||
gboolean ret = FALSE; |
||||
UDisksObject *drive_object = NULL; |
||||
+ UDisksLinuxBlockObject *block_object = NULL; |
||||
UDisksDriveAta *ata = NULL; |
||||
|
||||
drive_object = udisks_daemon_find_object (daemon, udisks_block_get_drive (block)); |
||||
@@ -2369,6 +2370,20 @@ erase_ata_device (UDisksBlock *block, |
||||
goto out; |
||||
} |
||||
|
||||
+ /* Reverse check to ensure we're erasing whole block device and not a partition */ |
||||
+ block_object = udisks_linux_drive_object_get_block (UDISKS_LINUX_DRIVE_OBJECT (drive_object), FALSE /* get_hw */); |
||||
+ if (block_object == NULL) |
||||
+ { |
||||
+ g_set_error (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, "Couldn't find a block device for the drive to erase"); |
||||
+ goto out; |
||||
+ } |
||||
+ if (g_strcmp0 (g_dbus_object_get_object_path (G_DBUS_OBJECT (object)), |
||||
+ g_dbus_object_get_object_path (G_DBUS_OBJECT (block_object))) != 0) |
||||
+ { |
||||
+ g_set_error (error, UDISKS_ERROR, UDISKS_ERROR_FAILED, "ATA secure erase needs to be performed on a whole block device"); |
||||
+ goto out; |
||||
+ } |
||||
+ |
||||
/* sleep a tiny bit here to avoid the secure erase code racing with |
||||
* programs spawned by udev |
||||
*/ |
||||
@@ -2382,6 +2397,7 @@ erase_ata_device (UDisksBlock *block, |
||||
out: |
||||
g_clear_object (&ata); |
||||
g_clear_object (&drive_object); |
||||
+ g_clear_object (&block_object); |
||||
return ret; |
||||
} |
||||
|
||||
@ -0,0 +1,29 @@
@@ -0,0 +1,29 @@
|
||||
From 9a6e6b700b19539465ab6b241f04b94d4b3769c4 Mon Sep 17 00:00:00 2001 |
||||
From: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Mon, 10 Oct 2022 13:55:29 +0200 |
||||
Subject: [PATCH] iscsi: Always set auth info |
||||
|
||||
In case of reusing a context auth info needs to be |
||||
always set to override previous data. |
||||
--- |
||||
modules/iscsi/udisksiscsiutil.c | 7 ++----- |
||||
1 file changed, 2 insertions(+), 5 deletions(-) |
||||
|
||||
diff --git a/modules/iscsi/udisksiscsiutil.c b/modules/iscsi/udisksiscsiutil.c |
||||
index 8fdae889c7..78890106f0 100644 |
||||
--- a/modules/iscsi/udisksiscsiutil.c |
||||
+++ b/modules/iscsi/udisksiscsiutil.c |
||||
@@ -171,11 +171,8 @@ iscsi_perform_login_action (UDisksLinuxModuleISCSI *module, |
||||
/* Get a libiscsi context. */ |
||||
ctx = udisks_linux_module_iscsi_get_libiscsi_context (module); |
||||
|
||||
- if (action == ACTION_LOGIN && |
||||
- auth_info && auth_info->method == libiscsi_auth_chap) |
||||
- { |
||||
- libiscsi_node_set_auth (ctx, node, auth_info); |
||||
- } |
||||
+ if (action == ACTION_LOGIN && auth_info) |
||||
+ libiscsi_node_set_auth (ctx, node, auth_info); |
||||
|
||||
/* Login or Logout */ |
||||
err = action == ACTION_LOGIN ? |
||||
@ -0,0 +1,75 @@
@@ -0,0 +1,75 @@
|
||||
commit fab797fcf5e4c8e09e4cde45647951acd764415e |
||||
Author: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Mon Oct 10 13:58:15 2022 +0200 |
||||
|
||||
tests: Add bad auth test for iscsi |
||||
|
||||
This tests that the auth info is properly set for each login call, |
||||
overriding previously set auth info with no trace. |
||||
|
||||
diff --git a/src/tests/dbus-tests/test_30_iscsi.py b/src/tests/dbus-tests/test_30_iscsi.py |
||||
index 34bdfc4b..6ac8386b 100644 |
||||
--- a/src/tests/dbus-tests/test_30_iscsi.py |
||||
+++ b/src/tests/dbus-tests/test_30_iscsi.py |
||||
@@ -284,3 +284,61 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
# make sure the session object is no longer on dbus |
||||
objects = udisks.GetManagedObjects(dbus_interface='org.freedesktop.DBus.ObjectManager') |
||||
self.assertNotIn(session_path, objects.keys()) |
||||
+ |
||||
+ def test_login_noauth_badauth(self): |
||||
+ """ |
||||
+ Test auth info override |
||||
+ """ |
||||
+ manager = self.get_object('/Manager') |
||||
+ nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
+ dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
+ timeout=self.iscsi_timeout) |
||||
+ |
||||
+ node = next((node for node in nodes if node[0] == self.noauth_iqn), None) |
||||
+ self.assertIsNotNone(node) |
||||
+ |
||||
+ (iqn, tpg, host, port, iface) = node |
||||
+ self.assertEqual(iqn, self.noauth_iqn) |
||||
+ self.assertEqual(host, self.address) |
||||
+ self.assertEqual(port, self.port) |
||||
+ |
||||
+ self.addCleanup(self._force_lougout, self.noauth_iqn) |
||||
+ |
||||
+ # first attempt - wrong password |
||||
+ options = dbus.Dictionary(signature='sv') |
||||
+ options['username'] = self.initiator |
||||
+ msg = 'Login failed: initiator reported error' |
||||
+ with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg): |
||||
+ options['password'] = '12345' |
||||
+ manager.Login(iqn, tpg, host, port, iface, options, |
||||
+ dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
+ timeout=self.iscsi_timeout) |
||||
+ |
||||
+ # second atttempt - no password |
||||
+ manager.Login(iqn, tpg, host, port, iface, self.no_options, |
||||
+ dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
+ timeout=self.iscsi_timeout) |
||||
+ |
||||
+ devs = glob.glob('/dev/disk/by-path/*%s*' % iqn) |
||||
+ self.assertEqual(len(devs), 1) |
||||
+ |
||||
+ # check if the block device have 'Symlinks' property updated |
||||
+ disk_name = os.path.realpath(devs[0]).split('/')[-1] |
||||
+ disk_obj = self.get_object('/block_devices/' + disk_name) |
||||
+ dbus_path = str(disk_obj.object_path) |
||||
+ self.assertIsNotNone(disk_obj) |
||||
+ |
||||
+ symlinks = self.get_property_raw(disk_obj, '.Block', 'Symlinks') |
||||
+ self.assertIn(self.str_to_ay(devs[0]), symlinks) |
||||
+ |
||||
+ manager.Logout(iqn, tpg, host, port, iface, self.no_options, |
||||
+ dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
+ timeout=self.iscsi_timeout) |
||||
+ |
||||
+ devs = glob.glob('/dev/disk/by-path/*%s*' % iqn) |
||||
+ self.assertEqual(len(devs), 0) |
||||
+ |
||||
+ # make sure the disk is no longer on dbus |
||||
+ udisks = self.get_object('') |
||||
+ objects = udisks.GetManagedObjects(dbus_interface='org.freedesktop.DBus.ObjectManager') |
||||
+ self.assertNotIn(dbus_path, objects.keys()) |
||||
@ -0,0 +1,51 @@
@@ -0,0 +1,51 @@
|
||||
commit 13a6a27eecdd1fb527b9151309366970b182a58d |
||||
Author: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Thu Oct 20 17:17:10 2022 +0200 |
||||
|
||||
tests: Fix LIO target config auth |
||||
|
||||
Linux kernel 6.0 brought number of the LIO target changes related to authentication |
||||
that made our tests fail. Turned out our target config was incorrect, e.g. |
||||
not requiring auth for CHAP tests, etc. The kernel 6.0 looks to be more strict |
||||
in this regard. |
||||
|
||||
diff --git a/src/tests/dbus-tests/targetcli_config.json b/src/tests/dbus-tests/targetcli_config.json |
||||
index 25d506b6..3be9eac2 100644 |
||||
--- a/src/tests/dbus-tests/targetcli_config.json |
||||
+++ b/src/tests/dbus-tests/targetcli_config.json |
||||
@@ -385,7 +385,7 @@ |
||||
"tpgs": [ |
||||
{ |
||||
"attributes": { |
||||
- "authentication": 0, |
||||
+ "authentication": 1, |
||||
"cache_dynamic_acls": 0, |
||||
"default_cmdsn_depth": 64, |
||||
"default_erl": 0, |
||||
@@ -432,7 +432,7 @@ |
||||
} |
||||
], |
||||
"parameters": { |
||||
- "AuthMethod": "CHAP,None", |
||||
+ "AuthMethod": "CHAP", |
||||
"DataDigest": "CRC32C,None", |
||||
"DataPDUInOrder": "Yes", |
||||
"DataSequenceInOrder": "Yes", |
||||
@@ -471,7 +471,7 @@ |
||||
"tpgs": [ |
||||
{ |
||||
"attributes": { |
||||
- "authentication": 0, |
||||
+ "authentication": 1, |
||||
"cache_dynamic_acls": 0, |
||||
"default_cmdsn_depth": 64, |
||||
"default_erl": 0, |
||||
@@ -520,7 +520,7 @@ |
||||
} |
||||
], |
||||
"parameters": { |
||||
- "AuthMethod": "CHAP,None", |
||||
+ "AuthMethod": "CHAP", |
||||
"DataDigest": "CRC32C,None", |
||||
"DataPDUInOrder": "Yes", |
||||
"DataSequenceInOrder": "Yes", |
||||
@ -0,0 +1,84 @@
@@ -0,0 +1,84 @@
|
||||
commit 68115b16181db7a38f852b101ec965b9fc3e59cb |
||||
Author: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Thu Oct 20 17:32:29 2022 +0200 |
||||
|
||||
tests: Clean the discovered test target iscsid node cache |
||||
|
||||
After each DiscoverSendTargets() and Login() calls iscsid caches |
||||
the node info in /var/lib/iscsi/nodes. That includes auth info and |
||||
passwords in plaintext. This might potentially lead to lingering |
||||
attributes sneaking into subsequent tests, affecting the results. |
||||
|
||||
Let's clean that after each test run. |
||||
|
||||
diff --git a/src/tests/dbus-tests/test_30_iscsi.py b/src/tests/dbus-tests/test_30_iscsi.py |
||||
index 6ac8386b..2b75462a 100644 |
||||
--- a/src/tests/dbus-tests/test_30_iscsi.py |
||||
+++ b/src/tests/dbus-tests/test_30_iscsi.py |
||||
@@ -6,6 +6,7 @@ import os |
||||
import re |
||||
import six |
||||
import time |
||||
+import shutil |
||||
import unittest |
||||
|
||||
|
||||
@@ -26,6 +27,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
chap_iqn = 'iqn.2003-01.udisks.test:iscsi-test-chap' |
||||
mutual_iqn = 'iqn.2003-01.udisks.test:iscsi-test-mutual' |
||||
|
||||
+ |
||||
# Define common D-Bus method call timeout that needs to be slightly longer |
||||
# than the corresponding timeout defined in libiscsi: |
||||
# #define ISCSID_REQ_TIMEOUT 1000 |
||||
@@ -61,6 +63,10 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
initiator = bytearray(data) |
||||
return initiator.strip().split(b"InitiatorName=")[1] |
||||
|
||||
+ def _clean_iscsid_node_dir(self): |
||||
+ for iqn in [self.noauth_iqn, self.chap_iqn, self.mutual_iqn]: |
||||
+ shutil.rmtree(os.path.join('/var/lib/iscsi/nodes/', iqn), ignore_errors=True) |
||||
+ |
||||
def test__manager_interface(self): |
||||
'''Test for module D-Bus Manager interface presence''' |
||||
|
||||
@@ -86,6 +92,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
timeout=self.iscsi_timeout) |
||||
+ self.addCleanup(self._clean_iscsid_node_dir) |
||||
|
||||
node = next((node for node in nodes if node[0] == self.noauth_iqn), None) |
||||
self.assertIsNotNone(node) |
||||
@@ -131,6 +138,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
timeout=self.iscsi_timeout) |
||||
+ self.addCleanup(self._clean_iscsid_node_dir) |
||||
|
||||
node = next((node for node in nodes if node[0] == self.chap_iqn), None) |
||||
self.assertIsNotNone(node) |
||||
@@ -190,6 +198,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
timeout=self.iscsi_timeout) |
||||
+ self.addCleanup(self._clean_iscsid_node_dir) |
||||
|
||||
node = next((node for node in nodes if node[0] == self.mutual_iqn), None) |
||||
self.assertIsNotNone(node) |
||||
@@ -246,6 +255,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
timeout=self.iscsi_timeout) |
||||
+ self.addCleanup(self._clean_iscsid_node_dir) |
||||
|
||||
node = next((node for node in nodes if node[0] == self.noauth_iqn), None) |
||||
self.assertIsNotNone(node) |
||||
@@ -293,6 +303,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
nodes, _ = manager.DiscoverSendTargets(self.address, self.port, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
timeout=self.iscsi_timeout) |
||||
+ self.addCleanup(self._clean_iscsid_node_dir) |
||||
|
||||
node = next((node for node in nodes if node[0] == self.noauth_iqn), None) |
||||
self.assertIsNotNone(node) |
||||
@ -0,0 +1,37 @@
@@ -0,0 +1,37 @@
|
||||
commit 1bf172603e4cc77da70d8fd13b6ba6c8b8c91600 |
||||
Author: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Thu Oct 20 17:53:20 2022 +0200 |
||||
|
||||
tests: Test iscsi noauth in test_login_chap_auth |
||||
|
||||
The other way is already tested in test_login_noauth_badauth. |
||||
|
||||
diff --git a/src/tests/dbus-tests/test_30_iscsi.py b/src/tests/dbus-tests/test_30_iscsi.py |
||||
index 2b75462a..f2594d99 100644 |
||||
--- a/src/tests/dbus-tests/test_30_iscsi.py |
||||
+++ b/src/tests/dbus-tests/test_30_iscsi.py |
||||
@@ -151,8 +151,14 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
options = dbus.Dictionary(signature='sv') |
||||
options['username'] = self.initiator |
||||
|
||||
+ msg = 'Login failed: initiator reported error \(24 - iSCSI login failed due to authorization failure\)' |
||||
+ # missing auth info |
||||
+ with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg): |
||||
+ manager.Login(iqn, tpg, host, port, iface, self.no_options, |
||||
+ dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator', |
||||
+ timeout=self.iscsi_timeout) |
||||
+ |
||||
# wrong password |
||||
- msg = 'Login failed: initiator reported error' |
||||
with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg): |
||||
options['password'] = '12345' |
||||
manager.Login(iqn, tpg, host, port, iface, options, |
||||
@@ -318,7 +324,7 @@ class UdisksISCSITest(udiskstestcase.UdisksTestCase): |
||||
# first attempt - wrong password |
||||
options = dbus.Dictionary(signature='sv') |
||||
options['username'] = self.initiator |
||||
- msg = 'Login failed: initiator reported error' |
||||
+ msg = r'Login failed: initiator reported error \((19 - encountered non-retryable iSCSI login failure|24 - iSCSI login failed due to authorization failure)\)' |
||||
with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg): |
||||
options['password'] = '12345' |
||||
manager.Login(iqn, tpg, host, port, iface, options, |
||||
@ -0,0 +1,42 @@
@@ -0,0 +1,42 @@
|
||||
From fbe970add68e6d9d998fb7f78377368c403e200d Mon Sep 17 00:00:00 2001 |
||||
From: Tomas Bzatek <tbzatek@redhat.com> |
||||
Date: Mon, 31 Oct 2022 15:15:31 +0100 |
||||
Subject: [PATCH] tests: Restart iscsid on every InitiatorName change |
||||
|
||||
The test LIO target config expects a specific initiator name as set |
||||
by the ACLs. However the iscsid daemon only seems to be reading |
||||
the InitiatorName string on startup and in case the service is running |
||||
with a different name, the auth tests will fail. |
||||
|
||||
As a workaround, restart the iscsid service after each change. |
||||
A proper way through libiscsi or libopeniscsiusr would be nice -> TODO. |
||||
--- |
||||
src/tests/dbus-tests/test_30_iscsi.py | 12 ++++++++++++ |
||||
1 file changed, 12 insertions(+) |
||||
|
||||
diff --git a/src/tests/dbus-tests/test_30_iscsi.py b/src/tests/dbus-tests/test_30_iscsi.py |
||||
index f2594d992..09e975f30 100644 |
||||
--- a/src/tests/dbus-tests/test_30_iscsi.py |
||||
+++ b/src/tests/dbus-tests/test_30_iscsi.py |
||||
@@ -48,9 +48,21 @@ def _force_lougout(self, target): |
||||
def _set_initiator_name(self): |
||||
manager = self.get_object('/Manager') |
||||
|
||||
+ # make backup of INITIATOR_FILE and restore it at the end |
||||
+ try: |
||||
+ initiatorname_backup = self.read_file(INITIATOR_FILE) |
||||
+ self.addCleanup(self.write_file, INITIATOR_FILE, initiatorname_backup) |
||||
+ except FileNotFoundError as e: |
||||
+ # no existing file, simply remove it once finished |
||||
+ self.addCleanup(self.remove_file, INITIATOR_FILE, True) |
||||
+ |
||||
manager.SetInitiatorName(self.initiator, self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator') |
||||
|
||||
+ # running iscsid needs to be restarted to reflect the change |
||||
+ self.run_command('systemctl try-reload-or-restart iscsid.service') |
||||
+ # ignore the return code in case of non-systemd distros |
||||
+ |
||||
init = manager.GetInitiatorName(self.no_options, |
||||
dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator') |
||||
self.assertEqual(init, self.initiator) |
||||
Loading…
Reference in new issue