(mostly) iSCSI iBFT fixes

modules/iscsi/data/org.freedesktop.UDisks2.iscsi.xml

@@ -162,6 +162,9 @@
         <parameter>reverse-password</parameter> will be used for CHAP
         authentication.
 
+        Firmware-discovered nodes retain their authentication info unless
+        overriden with specified credentials (see above).
+
         All the additional options are transformed into the interface
         parameters.  For example, if an automatic node startup is desired, the
         <parameter>node.startup</parameter> needs to be set to

modules/iscsi/udisksiscsiutil.c

@@ -264,6 +264,31 @@ iscsi_params_pop_chap_data (GVariant      *params,
   return g_variant_dict_end (&dict);
 }
 
+static gboolean
+is_auth_required (struct libiscsi_context   *ctx,
+                  struct libiscsi_node      *node,
+                  struct libiscsi_auth_info *auth_info)
+{
+  char val[LIBISCSI_VALUE_MAXLEN + 1] = {'\0',};
+  int ret;
+
+  /* TODO: No way to distinguish between the "no auth requested" and
+   *       "retain discovered auth info" scenarios from the D-Bus API.
+   */
+
+  /* In case CHAP auth is requested, let's use it unconditionally */
+  if (auth_info->method != libiscsi_auth_none)
+    return TRUE;
+
+  /* Avoid auth override on firmware-discovered nodes */
+  ret = libiscsi_node_get_parameter (ctx, node, "node.discovery_type", val);
+  if (ret == 0 && g_strcmp0 (val, "fw") == 0)
+    return FALSE;
+
+  /* Not a firmware-discovered node, maintain legacy rules */
+  return TRUE;
+}
+
 gint
 iscsi_login (UDisksLinuxModuleISCSI *module,
              const gchar            *name,
@@ -317,7 +342,7 @@ iscsi_login (UDisksLinuxModuleISCSI *module,
       err = iscsi_perform_login_action (module,
                                         ACTION_LOGIN,
                                         &node,
-                                        &auth_info,
+                                        is_auth_required (ctx, &node, &auth_info) ? &auth_info : NULL,
                                         errorstr);
     }
 

modules/iscsi/udiskslinuxiscsisessionobject.c

@@ -49,6 +49,8 @@ struct _UDisksLinuxISCSISessionObject {
   UDisksLinuxModuleISCSI *module;
   gchar                  *session_id;
 
+  GHashTable             *sysfs_paths;
+
   /* Interface(s) */
   UDisksLinuxISCSISession *iface_iscsi_session;
 };
@@ -151,6 +153,7 @@ udisks_linux_iscsi_session_object_finalize (GObject *object)
 
   g_free (session_object->session_id);
   g_object_unref (session_object->module);
+  g_hash_table_destroy (session_object->sysfs_paths);
 
   if (G_OBJECT_CLASS (udisks_linux_iscsi_session_object_parent_class)->finalize)
     G_OBJECT_CLASS (udisks_linux_iscsi_session_object_parent_class)->finalize (object);
@@ -205,6 +208,7 @@ udisks_linux_iscsi_session_object_init (UDisksLinuxISCSISessionObject *session_o
   g_return_if_fail (UDISKS_IS_LINUX_ISCSI_SESSION_OBJECT (session_object));
 
   session_object->module = NULL;
+  session_object->sysfs_paths = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
 }
 
 /**
@@ -336,7 +340,7 @@ udisks_linux_iscsi_session_object_update_iface (UDisksLinuxISCSISessionObject *s
   if (libiscsi_get_session_info_by_id (ctx, &session_info, session_object->session_id) != 0)
     {
       udisks_linux_module_iscsi_unlock_libiscsi_context (session_object->module);
-      udisks_critical ("Can not retrieve session information for %s", session_object->session_id);
+      udisks_warning ("Cannot retrieve session information for %s", session_object->session_id);
       return;
     }
 
@@ -359,7 +363,7 @@ udisks_linux_iscsi_session_object_update_iface (UDisksLinuxISCSISessionObject *s
   g_dbus_interface_skeleton_flush (G_DBUS_INTERFACE_SKELETON (iface));
 }
 
-static gboolean
+gboolean
 udisks_linux_iscsi_session_object_process_uevent (UDisksModuleObject *module_object,
                                                   const gchar        *action,
                                                   UDisksLinuxDevice  *device,
@@ -382,12 +386,14 @@ udisks_linux_iscsi_session_object_process_uevent (UDisksModuleObject *module_obj
       g_free (session_id);
       if (g_strcmp0 (action, "remove") == 0)
         {
-          *keep = FALSE;
+          g_warn_if_fail (g_hash_table_remove (session_object->sysfs_paths, sysfs_path));
+          *keep = g_hash_table_size (session_object->sysfs_paths) > 0;
           return TRUE;
         }
       else
         {
           *keep = TRUE;
+          g_hash_table_add (session_object->sysfs_paths, g_strdup (sysfs_path));
           return TRUE;
         }
     }

modules/iscsi/udiskslinuxiscsisessionobject.h

@@ -21,6 +21,7 @@
 #define __UDISKS_LINUX_ISCSI_SESSION_OBJECT_H__
 
 #include <src/udisksdaemontypes.h>
+#include <src/udisksmoduleobject.h>
 #include "udisksiscsitypes.h"
 #include "udiskslinuxmoduleiscsi.h"
 
@@ -36,12 +37,14 @@ G_BEGIN_DECLS
 GType                           udisks_linux_iscsi_session_object_get_type  (void) G_GNUC_CONST;
 UDisksLinuxISCSISessionObject  *udisks_linux_iscsi_session_object_new (UDisksLinuxModuleISCSI *module,
                                                                        const gchar            *session_id);
-UDisksLinuxModuleISCSI         *udisks_linux_iscsi_session_object_get_module
-                                                            (UDisksLinuxISCSISessionObject *session_object);
-const gchar                    *udisks_linux_iscsi_session_object_get_session_id
-                                                            (UDisksLinuxISCSISessionObject *session_object);
-gchar                          *udisks_linux_iscsi_session_object_get_object_path
-                                                            (UDisksLinuxISCSISessionObject *session_object);
+UDisksLinuxModuleISCSI         *udisks_linux_iscsi_session_object_get_module      (UDisksLinuxISCSISessionObject *session_object);
+const gchar                    *udisks_linux_iscsi_session_object_get_session_id  (UDisksLinuxISCSISessionObject *session_object);
+gchar                          *udisks_linux_iscsi_session_object_get_object_path (UDisksLinuxISCSISessionObject *session_object);
+
+gboolean                        udisks_linux_iscsi_session_object_process_uevent (UDisksModuleObject *module_object,
+                                                                                  const gchar        *action,
+                                                                                  UDisksLinuxDevice  *device,
+                                                                                  gboolean           *keep);
 
 gchar                          *udisks_linux_iscsi_session_object_make_object_path
                                                             (const gchar *session_id);

modules/iscsi/udiskslinuxmoduleiscsi.c

@@ -210,38 +210,24 @@ udisks_linux_module_iscsi_new_object (UDisksModule      *module,
 #ifdef HAVE_LIBISCSI_GET_SESSION_INFOS
   GDBusObjectSkeleton **objects;
   UDisksLinuxISCSISessionObject *session_object = NULL;
-  GDBusObjectManagerServer *object_manager_server;
-  GDBusObject *object;
-  UDisksDaemon *daemon;
   const gchar *sysfs_path;
   gchar *session_id;
-  gchar *object_path;
+  gboolean keep = FALSE;
 
   g_return_val_if_fail (UDISKS_IS_LINUX_MODULE_ISCSI (module), NULL);
 
   /* Session ID */
   sysfs_path = g_udev_device_get_sysfs_path (device->udev_device);
   session_id = udisks_linux_iscsi_session_object_get_session_id_from_sysfs_path (sysfs_path);
-  daemon = udisks_module_get_daemon (module);
 
   if (session_id)
     {
-      /* check if such object exists */
-      object_manager_server = udisks_daemon_get_object_manager (daemon);
-      object_path = udisks_linux_iscsi_session_object_make_object_path (session_id);
-      object = g_dbus_object_manager_get_object (G_DBUS_OBJECT_MANAGER (object_manager_server),
-                                                 object_path);
-      g_free (object_path);
-
-      if (! object)
-        {
-          /* create a new DBus object */
-          session_object = udisks_linux_iscsi_session_object_new (UDISKS_LINUX_MODULE_ISCSI (module), session_id);
-        }
+       session_object = udisks_linux_iscsi_session_object_new (UDISKS_LINUX_MODULE_ISCSI (module), session_id);
+       udisks_linux_iscsi_session_object_process_uevent (UDISKS_MODULE_OBJECT (session_object), "add", device, &keep);
+       g_warn_if_fail (keep == TRUE);
+       g_free (session_id);
     }
 
-  g_free (session_id);
-
   if (session_object)
     {
       objects = g_new0 (GDBusObjectSkeleton *, 2);

src/tests/dbus-tests/test_19_lsm.py

@@ -163,7 +163,8 @@ def test_drive_lsm_local(self):
             # now call each of the LED control methods and let them fail
             for method_name in UdisksLSMTestCase._LED_CONTROL_METHOD_NAMES:
                 method = drive_lsm_local.get_dbus_method(method_name)
-                with six.assertRaisesRegex(self, dbus.exceptions.DBusException, r'Specified disk does not support this action'):
+                msg = r'(Specified disk does not support this action|Unable to find block device for drive)'
+                with six.assertRaisesRegex(self, dbus.exceptions.DBusException, msg):
                     method(self.no_options)
 
     def test_drive_lsm(self):

src/tests/dbus-tests/test_30_iscsi.py

@@ -374,3 +374,57 @@ def test_login_noauth_badauth(self):
         udisks = self.get_object('')
         objects = udisks.GetManagedObjects(dbus_interface='org.freedesktop.DBus.ObjectManager')
         self.assertNotIn(dbus_path, objects.keys())
+
+    @udiskstestcase.tag_test(udiskstestcase.TestTags.UNSAFE)
+    def test_ibft(self):
+        """
+        Test iBFT discovery and login if available
+        """
+        if not os.path.exists('/sys/firmware/acpi/tables/iBFT'):
+            udiskstestcase.UdisksTestCase.tearDownClass()
+            self.skipTest('No iBFT ACPI table detected')
+        ret, out = udiskstestcase.run_command('modprobe iscsi_ibft')
+        if ret != 0:
+            udiskstestcase.UdisksTestCase.tearDownClass()
+            self.skipTest('iscsi_ibft kernel module unavailable')
+
+        manager = self.get_object('/Manager')
+        nodes, _ = manager.DiscoverFirmware(self.no_options,
+                                            dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator',
+                                            timeout=self.iscsi_timeout)
+        self.addCleanup(self._clean_iscsid_node_dir)
+
+        # let's try to connect in whatever was discovered
+        for node in nodes:
+            (iqn, tpg, host, port, iface) = node
+            self.assertIsNotNone(iqn)
+            self.assertIsNotNone(host)
+            self.assertIsNotNone(port)
+
+            self.addCleanup(self._force_lougout, iqn)
+
+            # no password
+            options = dbus.Dictionary(signature='sv')
+            options['node.session.auth.chap_algs'] = 'SHA3-256,SHA256,SHA1'  # disallow MD5
+            manager.Login(iqn, tpg, host, port, iface, options,
+                          dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator',
+                          timeout=self.iscsi_timeout)
+
+            devs = glob.glob('/dev/disk/by-path/*%s*' % iqn)
+            self.assertGreater(len(devs), 0)
+
+            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)
+
+            # second attempt - wrong password
+            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['username'] = 'nonsenseuser'
+                options['password'] = '12345'
+                manager.Login(iqn, tpg, host, port, iface, options,
+                              dbus_interface=self.iface_prefix + '.Manager.ISCSI.Initiator',
+                              timeout=self.iscsi_timeout)

src/udiskslinuxmanager.c

@@ -965,15 +965,20 @@ handle_enable_modules (UDisksManager         *object,
       return TRUE;
     }
 
-  if (! udisks_daemon_get_disable_modules (manager->daemon))
+  if (udisks_daemon_get_disable_modules (manager->daemon))
     {
-      data = g_new0 (EnableModulesData, 1);
-      data->object = g_object_ref (object);
-      data->invocation = g_object_ref (invocation);
-      /* push to idle, process in main thread */
-      g_idle_add (load_modules_in_idle_cb, data);
+      g_dbus_method_invocation_return_error_literal (invocation,
+                                                     G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
+                                                     "Modules are disabled by a commandline switch.");
+      return TRUE;
     }
 
+  data = g_new0 (EnableModulesData, 1);
+  data->object = g_object_ref (object);
+  data->invocation = g_object_ref (invocation);
+  /* push to idle, process in main thread */
+  g_idle_add (load_modules_in_idle_cb, data);
+
   return TRUE; /* returning TRUE means that we handled the method invocation */
 }
 
@@ -1004,16 +1009,21 @@ handle_enable_module (UDisksManager         *object,
       return TRUE;
     }
 
-  if (! udisks_daemon_get_disable_modules (manager->daemon))
+  if (udisks_daemon_get_disable_modules (manager->daemon))
     {
-      data = g_new0 (EnableModulesData, 1);
-      data->object = g_object_ref (object);
-      data->invocation = g_object_ref (invocation);
-      data->module_name = g_strdup (arg_name);
-      /* push to idle, process in main thread */
-      g_idle_add (load_modules_in_idle_cb, data);
+      g_dbus_method_invocation_return_error_literal (invocation,
+                                                     G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT,
+                                                     "Modules are disabled by a commandline switch.");
+      return TRUE;
     }
 
+  data = g_new0 (EnableModulesData, 1);
+  data->object = g_object_ref (object);
+  data->invocation = g_object_ref (invocation);
+  data->module_name = g_strdup (arg_name);
+  /* push to idle, process in main thread */
+  g_idle_add (load_modules_in_idle_cb, data);
+
   return TRUE; /* returning TRUE means that we handled the method invocation */
 }
 

src/udiskslinuxprovider.c

@@ -1313,8 +1313,8 @@ handle_block_uevent_for_modules (UDisksLinuxProvider *provider,
               for (ll = instances_to_remove; ll; ll = ll->next)
                 {
                   object = ll->data;
-                  g_dbus_object_manager_server_unexport (udisks_daemon_get_object_manager (daemon),
-                                                         g_dbus_object_get_object_path (G_DBUS_OBJECT (object)));
+                  g_warn_if_fail (g_dbus_object_manager_server_unexport (udisks_daemon_get_object_manager (daemon),
+                                                                         g_dbus_object_get_object_path (G_DBUS_OBJECT (object))));
                   g_warn_if_fail (g_hash_table_remove (inst_table, object));
                 }
               if (g_hash_table_size (inst_table) == 0)
@@ -1328,7 +1328,7 @@ handle_block_uevent_for_modules (UDisksLinuxProvider *provider,
         }
 
       /* No module object claimed or was interested in this device, try creating new instance for the current module. */
-      if (! handled)
+      if (! handled && g_strcmp0 (action, "remove") != 0)
         {
           GDBusObjectSkeleton **objects, **ll;