Release 1.90.8

It seems that GLib may process multiple DBus signals in one mainloop
iteration. This could cause messages to be re-ordered, which in turn
caused a race condition in the CI that could trigger random failures.

NEWS

@@ -1,6 +1,18 @@
 This file lists notable changes in each release. For the full history of all
 changes, see ChangeLog.
 
+Version 1.90.8:
+
+It seems that we are finally reaching the end of the tunnel with regard
+to regressions. One more issue that cropped up was that a pam_fprintd fix
+to avoid a possible authentication bypass caused issues when fprintd was
+just started on demand.
+
+Highlights:
+ - pam: Only listen to NameOwnerChanged after fprintd is known to run (#94)
+ - Place new ObjectManager DBus API at /net/reactivated/Fprint
+
+
 Version 1.90.7:
 
 While 1.90.6 fixed a number of issues, we did have a bad regression due

data/meson.build

@@ -11,6 +11,7 @@ configure_file(
     install_dir: dbus_service_dir,
 )
 
+if get_option('systemd')
     configure_file(
         configuration: configuration_data({
             'libexecdir': fprintd_installdir,
@@ -20,6 +21,7 @@ configure_file(
         install: true,
         install_dir: systemd_unit_dir,
     )
+endif
 
 polkit_policy = 'net.reactivated.fprint.device.policy'
 polkit_policy_target = i18n.merge_file(polkit_policy,

meson.build

@@ -1,5 +1,5 @@
 project('fprintd', 'c',
-    version: '1.90.7',
+    version: '1.90.8',
     license: 'GPLv2+',
     default_options: [
         'buildtype=debugoptimized',
@@ -94,13 +94,17 @@ pod2man = find_program('pod2man', required: get_option('man'))
 xsltproc = find_program('xsltproc', required: get_option('gtk_doc'))
 
 # StateDirectory was introduced in systemd 235
-systemd_dep = dependency('systemd', version: '>= 235')
+systemd_dep = dependency('systemd', version: '>= 235', required: false)
 systemd_unit_dir = get_option('systemd_system_unit_dir')
 
-if systemd_unit_dir == ''
+if systemd_unit_dir == '' and systemd_dep.found()
     systemd_unit_dir = systemd_dep.get_pkgconfig_variable('systemdsystemunitdir')
 endif
 
+if get_option('systemd') and systemd_unit_dir == ''
+    error('systemd development files or systemd_system_unit_dir is needed for systemd support.')
+endif
+
 dbus_service_dir = get_option('dbus_service_dir')
 dbus_data_dir = datadir
 dbus_interfaces_dir = ''

meson_options.txt

@@ -6,6 +6,10 @@ option('man',
     description: 'Generate the man files',
     type: 'boolean',
     value: true)
+option('systemd',
+    description: 'Install system service files',
+    type: 'boolean',
+    value: true)
 option('systemd_system_unit_dir',
     description: 'Directory for systemd service files',
     type: 'string')

pam/pam_fprintd.c

@@ -208,6 +208,7 @@ verify_result (sd_bus_message *m,
   if ((r = sd_bus_message_read (m, "sb", &result, &done)) < 0)
     {
       pam_syslog (data->pamh, LOG_ERR, "Failed to parse VerifyResult signal: %d", r);
+      data->verify_ret = PAM_AUTHINFO_UNAVAIL;
       return 0;
     }
 
@@ -255,6 +256,7 @@ verify_finger_selected (sd_bus_message *m,
   if (sd_bus_message_read_basic (m, 's', &finger_name) < 0)
     {
       pam_syslog (data->pamh, LOG_ERR, "Failed to parse VerifyFingerSelected signal: %d", errno);
+      data->verify_ret = PAM_AUTHINFO_UNAVAIL;
       return 0;
     }
 
@@ -630,6 +632,7 @@ name_owner_changed (sd_bus_message *m,
   if (sd_bus_message_read (m, "sss", &name, &old_owner, &new_owner) < 0)
     {
       pam_syslog (data->pamh, LOG_ERR, "Failed to parse NameOwnerChanged signal: %d", errno);
+      data->verify_ret = PAM_AUTHINFO_UNAVAIL;
       return 0;
     }
 
@@ -640,8 +643,7 @@ name_owner_changed (sd_bus_message *m,
    * to events from a new name owner otherwise. */
   data->verify_ret = PAM_AUTHINFO_UNAVAIL;
 
-  if (debug)
+  pam_syslog (data->pamh, LOG_WARNING, "fprintd name owner changed during operation!");
-    pam_syslog (data->pamh, LOG_ERR, "fprintd name owner changed during operation!\n");
 
   return 0;
 }
@@ -665,16 +667,6 @@ do_auth (pam_handle_t *pamh, const char *username)
       return PAM_AUTHINFO_UNAVAIL;
     }
 
-  name_owner_changed_slot = NULL;
-  sd_bus_match_signal (bus,
-                       &name_owner_changed_slot,
-                       "org.freedesktop.DBus",
-                       "/org/freedesktop/DBus",
-                       "org.freedesktop.DBus",
-                       "NameOwnerChanged",
-                       name_owner_changed,
-                       data);
-
   data->dev = open_device (pamh, bus, &data->has_multiple_devices);
   if (data->dev == NULL)
     return PAM_AUTHINFO_UNAVAIL;
@@ -686,6 +678,19 @@ do_auth (pam_handle_t *pamh, const char *username)
   if (!have_prints)
     return PAM_AUTHINFO_UNAVAIL;
 
+  /* Only connect to NameOwnerChanged when needed. In case of automatic startup
+   * we rely on the fact that we never see those signals.
+   */
+  name_owner_changed_slot = NULL;
+  sd_bus_match_signal (bus,
+                       &name_owner_changed_slot,
+                       "org.freedesktop.DBus",
+                       "/org/freedesktop/DBus",
+                       "org.freedesktop.DBus",
+                       "NameOwnerChanged",
+                       name_owner_changed,
+                       data);
+
   if (claim_device (pamh, bus, data->dev, username))
     {
       int ret = do_verify (bus, data);

src/device.c

@@ -717,7 +717,7 @@ _fprint_device_check_for_username (FprintDevice          *rdev,
                                    GError               **error)
 {
   g_autoptr(GVariant) ret = NULL;
-  g_autoptr(GError) err = NULL;
+  g_autoptr(GError) local_error = NULL;
   GDBusConnection *connection;
   const char *sender;
   struct passwd *user;
@@ -734,15 +734,13 @@ _fprint_device_check_for_username (FprintDevice          *rdev,
                                      "GetConnectionUnixUser",
                                      g_variant_new ("(s)", sender),
                                      NULL, G_DBUS_CALL_FLAGS_NONE, -1,
-                                     NULL, &err);
+                                     NULL, &local_error);
 
   if (!ret)
     {
-      g_autoptr(GError) e = NULL;
-
       g_set_error (error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL,
                    "Could not get conection unix user ID: %s",
-                   err->message);
+                   local_error->message);
       return NULL;
     }
 
@@ -1925,8 +1923,9 @@ handle_unauthorized_access (FprintDevice          *rdev,
 
   g_assert (error);
 
-  g_warning ("Client %s not authorized for device %s: %s",
+  g_warning ("Client %s not authorized to call method '%s' for device %s: %s",
              g_dbus_method_invocation_get_sender (invocation),
+             g_dbus_method_invocation_get_method_name (invocation),
              fp_device_get_name (priv->dev),
              error->message);
   g_dbus_method_invocation_return_gerror (invocation, error);
@@ -1980,8 +1979,9 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
                                                    &error))
     return handle_unauthorized_access (rdev, invocation, error);
 
-  g_debug ("Authorization granted to %s for device %s!",
+  g_debug ("Authorization granted to %s to call method '%s' for device %s!",
            fp_device_get_name (priv->dev),
+           g_dbus_method_invocation_get_method_name (invocation),
            g_dbus_method_invocation_get_sender (invocation));
 
   return TRUE;

src/manager.c

@@ -295,7 +295,7 @@ fprint_manager_constructed (GObject *object)
   GDBusObjectManagerServer *object_manager_server;
 
   object_manager_server =
-    g_dbus_object_manager_server_new (FPRINT_SERVICE_PATH "/Device");
+    g_dbus_object_manager_server_new (FPRINT_SERVICE_PATH);
 
   priv->object_manager = G_DBUS_OBJECT_MANAGER (object_manager_server);
   priv->dbus_manager = fprint_dbus_manager_skeleton_new ();

utils/verify.c

@@ -160,8 +160,11 @@ verify_started_cb (GObject      *obj,
   struct VerifyState *verify_state = user_data;
 
   if (fprint_dbus_device_call_verify_start_finish (FPRINT_DBUS_DEVICE (obj), res, &verify_state->error))
+    {
+      g_print ("Verify started!\n");
       verify_state->started = TRUE;
     }
+}
 
 static void
 proxy_signal_cb (GDBusProxy  *proxy,
@@ -226,7 +229,6 @@ do_verify (FprintDBusDevice *dev)
       g_clear_error (&verify_state.error);
       exit (1);
     }
-  g_print ("Verify started!\n");
 
   /* VerifyStatus signals are processing, wait for completion. */
   while (!verify_state.completed)