Release 1.90.6

We already use FpFinger for storage operations and prints management,
but internally we keep still using the old finger number, that uses
different values for invalid data.

Let's be consistent, and always use FpFinger everywhere.

NEWS

@@ -1,6 +1,35 @@
 This file lists notable changes in each release. For the full history of all
 changes, see ChangeLog.
 
+Version 1.90.6:
+
+The 1.90.5 release was unusable due to a number of inter-related issues
+with the DBus interface and authorization. We also found a number of
+problems with possible security implications.
+
+Currently fprintd will do interactive authorization even if this was not
+requested using the correct DBus method call flag. All API users MUST be
+updated to set the flag as it will be enabled in the future!
+
+Highlights:
+ - Fix fprintd DBus configuration
+ - Change details of what requires authorization
+ - Fix various race conditions in pam_fprintd
+ - Permit interactive authorization from fprintd utilities
+ - Do not allow deletion while another operation is ongoing
+
+
+Version 1.90.5:
+
+The 1.90.4 release contained some bad errors, this release addresses those.
+
+Highlights:
+ - Permit building with polkit older than 0.114
+ - Fix possible issues with PAM test
+ - Fix incorrect DBus policy
+ - Fix build so that CFLAGS enviroment is correctly used
+ - Skip hotplug test with older libfprint (which times out otherwise)
+
 Version 1.90.4:
 
 This fprintd release contains major core reworkings and improved testing.

config.h.in

@@ -9,3 +9,6 @@
 
 /* Define to the version of this package. */
 #mesondefine VERSION
+
+/* Whether current polkit version supports autopointers */
+#mesondefine POLKIT_HAS_AUTOPOINTERS

data/net.reactivated.Fprint.conf

@@ -12,8 +12,18 @@
 
   <!-- Anyone can talk to the service -->
   <policy context="default">
-    <allow send_destination="net.reactivated.Fprint"/>
-    <allow send_interface="net.reactivated.Fprint"/>
+    <allow send_destination="net.reactivated.Fprint"
+           send_interface="net.reactivated.Fprint.Manager"/>
+    <allow send_destination="net.reactivated.Fprint"
+           send_interface="net.reactivated.Fprint.Device"/>
+
+    <!-- Basic D-Bus API stuff -->
+    <allow send_destination="net.reactivated.Fprint"
+           send_interface="org.freedesktop.DBus.Introspectable"/>
+    <allow send_destination="net.reactivated.Fprint"
+           send_interface="org.freedesktop.DBus.Properties"/>
+    <allow send_destination="net.reactivated.Fprint"
+           send_interface="org.freedesktop.DBus.ObjectManager"/>
   </policy>
 
 </busconfig>

meson.build

@@ -1,49 +1,10 @@
 project('fprintd', 'c',
-    version: '1.90.4',
+    version: '1.90.6',
     license: 'GPLv2+',
     default_options: [
         'buildtype=debugoptimized',
-        'warning_level=1',
+        'warning_level=3',
         'c_std=gnu99',
-        'c_args=' + ' '.join([
-            '-fno-strict-aliasing',
-            '-Wall',
-            '-Wcast-align',
-            '-Werror=address',
-            '-Werror=array-bounds',
-            '-Werror=empty-body',
-            '-Werror=implicit',
-            '-Werror=init-self',
-            '-Werror=int-to-pointer-cast',
-            '-Werror=main',
-            '-Werror=missing-braces',
-            '-Werror=nonnull',
-            '-Werror=pointer-to-int-cast',
-            '-Werror=redundant-decls',
-            '-Werror=return-type',
-            '-Werror=sequence-point',
-            '-Werror=trigraphs',
-            '-Werror=write-strings',
-            '-Wformat-nonliteral',
-            '-Wformat-security',
-            '-Wformat=2',
-            '-Wignored-qualifiers',
-            '-Wimplicit-function-declaration',
-            '-Wlogical-op',
-            '-Wmissing-declarations',
-            '-Wmissing-format-attribute',
-            '-Wmissing-include-dirs',
-            '-Wmissing-noreturn',
-            '-Wmissing-prototypes',
-            '-Wnested-externs',
-            '-Wold-style-definition',
-            '-Wpointer-arith',
-            '-Wshadow',
-            '-Wstrict-prototypes',
-            '-Wtype-limits',
-            '-Wundef',
-            '-Wunused',
-        ]),
     ],
     meson_version: '>= 0.50.0')
 
@@ -51,7 +12,57 @@ gnome = import('gnome')
 i18n = import('i18n')
 
 cc = meson.get_compiler('c')
+common_cflags = cc.get_supported_arguments([
+    '-fno-strict-aliasing',
+    '-Wcast-align',
+    '-Werror=address',
+    '-Werror=array-bounds',
+    '-Werror=empty-body',
+    '-Werror=implicit',
+    '-Werror=init-self',
+    '-Werror=int-to-pointer-cast',
+    '-Werror=main',
+    '-Werror=missing-braces',
+    '-Werror=nonnull',
+    '-Werror=pointer-to-int-cast',
+    '-Werror=redundant-decls',
+    '-Werror=return-type',
+    '-Werror=sequence-point',
+    '-Werror=trigraphs',
+    '-Werror=write-strings',
+    '-Wformat-nonliteral',
+    '-Wformat-security',
+    '-Wformat=2',
+    '-Wignored-qualifiers',
+    '-Wimplicit-function-declaration',
+    '-Wlogical-op',
+    '-Wmissing-declarations',
+    '-Wmissing-format-attribute',
+    '-Wmissing-include-dirs',
+    '-Wmissing-noreturn',
+    '-Wmissing-prototypes',
+    '-Wnested-externs',
+    '-Wold-style-definition',
+    '-Wpointer-arith',
+    '-Wshadow',
+    '-Wstrict-prototypes',
+    '-Wtype-limits',
+    '-Wundef',
+    '-Wunused',
+])
+add_project_arguments(common_cflags, language: 'c')
+
+common_cflags = cc.get_supported_arguments([
+    # The stub passes a lot of params that we do not use, maybe a good idea to
+    # mark it appropriately, but this works well for now.
+    '-Wno-unused-parameter',
+    # We use g_signal_handlers_disconnect_* which is not compatible with -Wpedantic
+    '-Wno-pedantic',
+])
+add_project_arguments(common_cflags, language: 'c')
+
 host_system = host_machine.system()
+# NOTE: Bump gdbus-codegen min version once we can depend on 2.64!
 glib_min_version = '2.56'
 libfprint_min_version = '1.90.1'
 
@@ -139,6 +150,7 @@ cdata.set_quoted('GETTEXT_PACKAGE', meson.project_name())
 cdata.set_quoted('PACKAGE_VERSION', meson.project_version())
 cdata.set_quoted('VERSION', meson.project_version())
 cdata.set_quoted('SYSCONFDIR', sysconfdir)
+cdata.set('POLKIT_HAS_AUTOPOINTERS', polkit_gobject_dep.version().version_compare('>= 0.114'))
 
 config_h = configure_file(
   input: 'config.h.in',

pam/pam_fprintd.c

@@ -43,6 +43,7 @@
 #define N_(s) (s)
 
 #include "fingerprint-strings.h"
+#include "pam_fprintd_autoptrs.h"
 
 #define DEFAULT_MAX_TRIES 3
 #define DEFAULT_TIMEOUT 30
@@ -83,10 +84,8 @@ static bool send_msg(pam_handle_t *pamh, const char *msg, int style)
 	const struct pam_message *msgp = &mymsg;
 	const struct pam_conv *pc;
 	struct pam_response *resp;
-	int r;
 
-	r = pam_get_item(pamh, PAM_CONV, (const void **) &pc);
-	if (r != PAM_SUCCESS)
+	if (pam_get_item(pamh, PAM_CONV, (const void **) &pc) != PAM_SUCCESS)
 		return false;
 
 	if (!pc || !pc->conv)
@@ -110,39 +109,35 @@ open_device (pam_handle_t    *pamh,
 	     sd_bus          *bus,
 	     bool            *has_multiple_devices)
 {
-	sd_bus_error error = SD_BUS_ERROR_NULL;
-	sd_bus_message *m = NULL;
+	pf_auto(sd_bus_error) error = SD_BUS_ERROR_NULL;
+	pf_autoptr(sd_bus_message) m = NULL;
 	size_t num_devices;
 	const char *path = NULL;
-	char *ret;
 	const char *s;
 	int r;
 
 	*has_multiple_devices = false;
 
-	r = sd_bus_call_method (bus,
+	if (sd_bus_call_method (bus,
 				"net.reactivated.Fprint",
 				"/net/reactivated/Fprint/Manager",
 				"net.reactivated.Fprint.Manager",
 				"GetDevices",
 				&error,
 				&m,
-				NULL);
-	if (r < 0) {
+				NULL) < 0) {
 		pam_syslog (pamh, LOG_ERR, "GetDevices failed: %s", error.message);
-		sd_bus_error_free (&error);
 		return NULL;
 	}
 
 	r = sd_bus_message_enter_container (m, 'a', "o");
 	if (r < 0) {
 		pam_syslog (pamh, LOG_ERR, "Failed to parse answer from GetDevices(): %d", r);
-		goto out;
+		return NULL;
 	}
 
-	r = sd_bus_message_read_basic (m, 'o', &path);
-	if (r < 0)
-		goto out;
+	if (sd_bus_message_read_basic (m, 'o', &path) < 0)
+		return NULL;
 
 	num_devices = 1;
 	while ((r = sd_bus_message_read_basic(m, 'o', &s)) > 0)
@@ -153,22 +148,35 @@ open_device (pam_handle_t    *pamh,
 
 	sd_bus_message_exit_container (m);
 
-out:
-	ret = path ? strdup (path) : NULL;
-	sd_bus_message_unref (m);
-	return ret;
+	return strdup (path);
 }
 
 typedef struct {
+	char *dev;
+	bool has_multiple_devices;
+
 	unsigned max_tries;
 	char *result;
 	bool timed_out;
 	bool is_swipe;
+	bool verify_started;
+	int verify_ret;
 	pam_handle_t *pamh;
 
 	char *driver;
 } verify_data;
 
+static void
+verify_data_free (verify_data *data)
+{
+	free (data->result);
+	free (data->driver);
+	free (data->dev);
+	free (data);
+}
+
+PF_DEFINE_AUTOPTR_CLEANUP_FUNC (verify_data, verify_data_free)
+
 static int
 verify_result (sd_bus_message *m,
 	       void           *userdata,
@@ -193,15 +201,29 @@ verify_result (sd_bus_message *m,
 		return 0;
 	}
 
+	if (!data->verify_started) {
+		pam_syslog (data->pamh, LOG_ERR, "Unexpected VerifyResult '%s', %"PRIu64" signal", result, done);
+		return 0;
+	}
+
 	if (debug)
 		pam_syslog (data->pamh, LOG_DEBUG, "Verify result: %s (done: %d)", result, done ? 1 : 0);
 
+	if (data->result) {
+		free (data->result);
+		data->result = NULL;
+	}
+
 	if (done) {
 		data->result = strdup (result);
 		return 0;
 	}
 
 	msg = verify_result_str_to_msg (result, data->is_swipe);
+	if (!msg) {
+		data->result = strdup ("Protocol error with fprintd!");
+		return 0;
+	}
 	send_err_msg (data->pamh, msg);
 
 	return 0;
@@ -214,18 +236,26 @@ verify_finger_selected (sd_bus_message *m,
 {
 	verify_data *data = userdata;
 	const char *finger_name = NULL;
-	char *msg;
+	pf_autofree char *msg = NULL;
 
 	if (sd_bus_message_read_basic (m, 's', &finger_name) < 0) {
-		pam_syslog (data->pamh, LOG_ERR, "Failed to parse VerifyFingerSelected signal: %m");
+		pam_syslog (data->pamh, LOG_ERR, "Failed to parse VerifyFingerSelected signal: %d", errno);
+		return 0;
+	}
+
+	if (!data->verify_started) {
+		pam_syslog (data->pamh, LOG_ERR, "Unexpected VerifyFingerSelected %s signal", finger_name);
 		return 0;
 	}
 
 	msg = finger_str_to_msg(finger_name, data->driver, data->is_swipe);
+	if (!msg) {
+		data->result = strdup ("Protocol error with fprintd!");
+		return 0;
+	}
 	if (debug)
 		pam_syslog (data->pamh, LOG_DEBUG, "verify_finger_selected %s", msg);
 	send_info_msg (data->pamh, msg);
-	free (msg);
 
 	return 0;
 }
@@ -240,7 +270,7 @@ get_property_string (sd_bus *bus,
 		     sd_bus_error *error,
 		     char **ret) {
 
-	sd_bus_message *reply = NULL;
+	pf_autoptr(sd_bus_message) reply = NULL;
 	const char *s;
 	char *n;
 	int r;
@@ -251,121 +281,133 @@ get_property_string (sd_bus *bus,
 
 	r = sd_bus_message_enter_container(reply, 'v', "s");
 	if (r < 0)
-		goto fail;
+		return sd_bus_error_set_errno(error, r);
 
 	r = sd_bus_message_read_basic(reply, 's', &s);
 	if (r < 0)
-		goto fail;
+		return sd_bus_error_set_errno(error, r);
 
 	n = strdup(s);
 	if (!n) {
-		r = -ENOMEM;
-		goto fail;
+		return sd_bus_error_set_errno(error, -ENOMEM);
 	}
 
-	sd_bus_message_unref (reply);
-
 	*ret = n;
 	return 0;
+}
 
-fail:
-	if (reply != NULL)
-		sd_bus_message_unref (reply);
-	return sd_bus_error_set_errno(error, r);
+
+static int verify_started_cb (sd_bus_message *m,
+			      void           *userdata,
+			      sd_bus_error   *ret_error) {
+	const sd_bus_error *error = sd_bus_message_get_error (m);
+	verify_data *data = userdata;
+
+	if (error) {
+		if (sd_bus_error_has_name (error, "net.reactivated.Fprint.Error.NoEnrolledPrints")) {
+			pam_syslog (data->pamh, LOG_DEBUG, "No prints enrolled");
+			data->verify_ret = PAM_USER_UNKNOWN;
+		} else {
+			data->verify_ret = PAM_AUTH_ERR;
+		}
+
+		if (debug)
+			pam_syslog (data->pamh, LOG_DEBUG, "VerifyStart failed: %s", error->message);
+
+		return 1;
+	}
+
+	if (debug)
+		pam_syslog (data->pamh, LOG_DEBUG, "VerifyStart completed successfully");
+
+	data->verify_started = true;
+
+	return 1;
 }
 
 static int
-do_verify (pam_handle_t *pamh,
-	   sd_bus       *bus,
-	   const char   *dev,
-	   bool          has_multiple_devices)
+do_verify (sd_bus       *bus,
+	   verify_data  *data)
 {
-	verify_data *data;
-	sd_bus_slot *verify_status_slot, *verify_finger_selected_slot;
-	char *scan_type = NULL;
-	int ret;
+	pf_autoptr(sd_bus_slot) verify_status_slot = NULL;
+	pf_autoptr(sd_bus_slot) verify_finger_selected_slot = NULL;
+	pf_autofree char *scan_type = NULL;
 	int r;
 
-	data = calloc (1, sizeof(verify_data));
-	data->max_tries = max_tries;
-	data->pamh = pamh;
-
 	/* Get some properties for the device */
 	r = get_property_string (bus,
 				 "net.reactivated.Fprint",
-				 dev,
+				 data->dev,
 				 "net.reactivated.Fprint.Device",
 				 "scan-type",
 				 NULL,
 				 &scan_type);
 	if (r < 0)
-		pam_syslog (data->pamh, LOG_ERR, "Failed to get scan-type for %s: %d", dev, r);
+		pam_syslog (data->pamh, LOG_ERR, "Failed to get scan-type for %s: %d", data->dev, r);
 	if (debug)
-		pam_syslog (data->pamh, LOG_DEBUG, "scan-type for %s: %s", dev, scan_type);
+		pam_syslog (data->pamh, LOG_DEBUG, "scan-type for %s: %s", data->dev, scan_type);
 	if (str_equal (scan_type, "swipe"))
 		data->is_swipe = true;
-	free (scan_type);
 
-	if (has_multiple_devices) {
+	if (data->has_multiple_devices) {
 		get_property_string (bus,
 				     "net.reactivated.Fprint",
-				     dev,
+				     data->dev,
 				     "net.reactivated.Fprint.Device",
 				     "name",
 				     NULL,
 				     &data->driver);
 		if (r < 0)
-			pam_syslog (data->pamh, LOG_ERR, "Failed to get driver name for %s: %d", dev, r);
+			pam_syslog (data->pamh, LOG_ERR, "Failed to get driver name for %s: %d", data->dev, r);
 		if (debug && r == 0)
-			pam_syslog (data->pamh, LOG_DEBUG, "driver name for %s: %s", dev, data->driver);
+			pam_syslog (data->pamh, LOG_DEBUG, "driver name for %s: %s", data->dev, data->driver);
 	}
 
-	verify_status_slot = NULL;
 	sd_bus_match_signal (bus,
 			     &verify_status_slot,
 			     "net.reactivated.Fprint",
-			     dev,
+			     data->dev,
 			     "net.reactivated.Fprint.Device",
 			     "VerifyStatus",
 			     verify_result,
 			     data);
 
-	verify_finger_selected_slot = NULL;
 	sd_bus_match_signal (bus,
 			     &verify_finger_selected_slot,
 			     "net.reactivated.Fprint",
-			     dev,
+			     data->dev,
 			     "net.reactivated.Fprint.Device",
 			     "VerifyFingerSelected",
 			     verify_finger_selected,
 			     data);
 
-	ret = PAM_AUTH_ERR;
-
-	while (ret == PAM_AUTH_ERR && data->max_tries > 0) {
+	while (data->max_tries > 0) {
 		uint64_t verification_end = now () + (timeout * USEC_PER_SEC);
-		sd_bus_message *m = NULL;
-		sd_bus_error error = SD_BUS_ERROR_NULL;
 
 		data->timed_out = false;
+		data->verify_started = false;
+		data->verify_ret = PAM_INCOMPLETE;
 
-		r = sd_bus_call_method (bus,
-					"net.reactivated.Fprint",
-					dev,
-					"net.reactivated.Fprint.Device",
-					"VerifyStart",
-					&error,
-					&m,
-					"s",
-					"any");
+		free (data->result);
+		data->result = NULL;
+
+		if (debug)
+			pam_syslog (data->pamh, LOG_DEBUG, "About to call VerifyStart");
+
+		r = sd_bus_call_method_async (bus,
+					      NULL,
+					      "net.reactivated.Fprint",
+					      data->dev,
+					      "net.reactivated.Fprint.Device",
+					      "VerifyStart",
+					      verify_started_cb,
+					      data,
+					      "s",
+					      "any");
 
 		if (r < 0) {
-			if (sd_bus_error_has_name (&error, "net.reactivated.Fprint.Error.NoEnrolledPrints"))
-				ret = PAM_USER_UNKNOWN;
-
 			if (debug)
-				pam_syslog (pamh, LOG_DEBUG, "VerifyStart failed: %s", error.message);
-			sd_bus_error_free (&error);
+				pam_syslog (data->pamh, LOG_DEBUG, "VerifyStart call failed: %d", r);
 			break;
 		}
 
@@ -379,29 +421,38 @@ do_verify (pam_handle_t *pamh,
 			r = sd_bus_process (bus, NULL);
 			if (r < 0)
 				break;
+			if (data->verify_ret != PAM_INCOMPLETE)
+				break;
+			if (!data->verify_started)
+				continue;
 			if (data->result != NULL)
 				break;
 			if (r == 0) {
 				if (debug) {
-					pam_syslog(pamh, LOG_DEBUG, "Waiting for %"PRId64" seconds (%"PRId64" usecs)",
+					pam_syslog(data->pamh, LOG_DEBUG,
+						   "Waiting for %"PRId64" seconds (%"PRId64" usecs)",
 						   wait_time / USEC_PER_SEC,
 						   wait_time);
 				}
-				r = sd_bus_wait (bus, wait_time);
-				if (r < 0)
+				if (sd_bus_wait (bus, wait_time) < 0)
 					break;
 			}
 		}
 
+		if (data->verify_ret != PAM_INCOMPLETE) {
+			return data->verify_ret;
+		}
+
 		if (now () >= verification_end) {
 			data->timed_out = true;
 			send_info_msg (data->pamh, _("Verification timed out"));
 		}
 
 		/* Ignore errors from VerifyStop */
+		data->verify_started = false;
 		sd_bus_call_method (bus,
 				    "net.reactivated.Fprint",
-				    dev,
+				    data->dev,
 				    "net.reactivated.Fprint.Device",
 				    "VerifyStop",
 				    NULL,
@@ -410,43 +461,28 @@ do_verify (pam_handle_t *pamh,
 				    NULL);
 
 		if (data->timed_out) {
-			ret = PAM_AUTHINFO_UNAVAIL;
-			break;
+			return PAM_AUTHINFO_UNAVAIL;
 		} else {
 			if (str_equal (data->result, "verify-no-match")) {
 				send_err_msg (data->pamh, "Failed to match fingerprint");
-				ret = PAM_AUTH_ERR;
 			} else if (str_equal (data->result, "verify-match")) {
-				ret = PAM_SUCCESS;
-				break;
+				return PAM_SUCCESS;
 			} else if (str_equal (data->result, "verify-unknown-error")) {
-				ret = PAM_AUTHINFO_UNAVAIL;
+				return PAM_AUTHINFO_UNAVAIL;
 			} else if (str_equal (data->result, "verify-disconnected")) {
-				ret = PAM_AUTHINFO_UNAVAIL;
-				break;
+				return PAM_AUTHINFO_UNAVAIL;
 			} else {
 				send_err_msg (data->pamh, _("An unknown error occurred"));
-				ret = PAM_AUTH_ERR;
-				break;
+				return PAM_AUTH_ERR;
 			}
-			free (data->result);
-			data->result = NULL;
 		}
 		data->max_tries--;
 	}
 
 	if (data->max_tries == 0)
-		ret = PAM_MAXTRIES;
+		return PAM_MAXTRIES;
 
-	sd_bus_slot_unref (verify_status_slot);
-	sd_bus_slot_unref (verify_finger_selected_slot);
-
-	if (data->result)
-		free (data->result);
-	free (data->driver);
-	free (data);
-
-	return ret;
+	return PAM_AUTH_ERR;
 }
 
 static bool
@@ -455,8 +491,8 @@ user_has_prints (pam_handle_t *pamh,
 		 const char   *dev,
 		 const char   *username)
 {
-	sd_bus_error error = SD_BUS_ERROR_NULL;
-	sd_bus_message *m = NULL;
+	pf_auto(sd_bus_error) error = SD_BUS_ERROR_NULL;
+	pf_autoptr(sd_bus_message) m = NULL;
 	size_t num_fingers = 0;
 	const char *s;
 	int r;
@@ -478,23 +514,19 @@ user_has_prints (pam_handle_t *pamh,
 			pam_syslog (pamh, LOG_DEBUG, "ListEnrolledFingers failed for %s: %s",
 				    username, error.message);
 		}
-		sd_bus_error_free (&error);
 		return false;
 	}
 
 	r = sd_bus_message_enter_container (m, 'a', "s");
 	if (r < 0) {
 		pam_syslog (pamh, LOG_ERR, "Failed to parse answer from ListEnrolledFingers(): %d", r);
-		goto out;
+		return false;
 	}
 
-	num_fingers = 0;
 	while ((r = sd_bus_message_read_basic(m, 's', &s)) > 0)
 		num_fingers++;
 	sd_bus_message_exit_container (m);
 
-out:
-	sd_bus_message_unref (m);
 	return (num_fingers > 0);
 }
 
@@ -503,10 +535,9 @@ release_device (pam_handle_t *pamh,
 		 sd_bus       *bus,
 		 const char   *dev)
 {
-	sd_bus_error error = SD_BUS_ERROR_NULL;
-	int r;
+	pf_auto(sd_bus_error) error = SD_BUS_ERROR_NULL;
 
-	r = sd_bus_call_method (bus,
+	if (sd_bus_call_method (bus,
 				"net.reactivated.Fprint",
 				dev,
 				"net.reactivated.Fprint.Device",
@@ -514,10 +545,8 @@ release_device (pam_handle_t *pamh,
 				&error,
 				NULL,
 				NULL,
-				NULL);
-	if (r < 0) {
+				NULL) < 0) {
 		pam_syslog (pamh, LOG_ERR, "ReleaseDevice failed: %s", error.message);
-		sd_bus_error_free (&error);
 	}
 }
 
@@ -527,10 +556,9 @@ claim_device (pam_handle_t *pamh,
 	      const char   *dev,
 	      const char   *username)
 {
-	sd_bus_error error = SD_BUS_ERROR_NULL;
-	int r;
+	pf_auto(sd_bus_error) error = SD_BUS_ERROR_NULL;
 
-	r = sd_bus_call_method (bus,
+	if (sd_bus_call_method (bus,
 				"net.reactivated.Fprint",
 				dev,
 				"net.reactivated.Fprint.Device",
@@ -538,53 +566,88 @@ claim_device (pam_handle_t *pamh,
 				&error,
 				NULL,
 				"s",
-				username);
-	if (r < 0) {
+				username) < 0) {
 		if (debug)
 			pam_syslog (pamh, LOG_DEBUG, "failed to claim device %s", error.message);
-		sd_bus_error_free (&error);
 		return false;
 	}
 
 	return true;
 }
 
+static int
+name_owner_changed (sd_bus_message *m,
+		    void           *userdata,
+		    sd_bus_error   *ret_error)
+{
+	verify_data *data = userdata;
+	const char *name = NULL;
+	const char *old_owner = NULL;
+	const char *new_owner = NULL;
+
+	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);
+		return 0;
+	}
+
+	if (strcmp (name, "net.reactivated.Fprint") != 0)
+		return 0;
+
+	/* Name owner for fprintd changed, give up as we might start listening
+	 * to events from a new name owner otherwise. */
+	data->verify_ret = PAM_AUTHINFO_UNAVAIL;
+
+	if (debug)
+		pam_syslog (data->pamh, LOG_ERR, "fprintd name owner changed during operation!\n");
+
+	return 0;
+}
+
 static int do_auth(pam_handle_t *pamh, const char *username)
 {
-	char *dev;
 	bool have_prints;
-	bool has_multiple_devices;
-	int ret = PAM_AUTHINFO_UNAVAIL;
-	sd_bus *bus = NULL;
+	pf_autoptr(verify_data) data = NULL;
+	pf_autoptr(sd_bus) bus = NULL;
+	pf_autoptr(sd_bus_slot) name_owner_changed_slot = NULL;
+
+	data = calloc (1, sizeof(verify_data));
+	data->max_tries = max_tries;
+	data->pamh = pamh;
 
 	if (sd_bus_open_system (&bus) < 0) {
-		pam_syslog (pamh, LOG_ERR, "Error with getting the bus: %m");
+		pam_syslog (pamh, LOG_ERR, "Error with getting the bus: %d", errno);
 		return PAM_AUTHINFO_UNAVAIL;
 	}
 
-	dev = open_device (pamh, bus, &has_multiple_devices);
-	if (dev == NULL) {
-		sd_bus_unref (bus);
+	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;
 	}
 
-	have_prints = user_has_prints (pamh, bus, dev, username);
+	have_prints = user_has_prints (pamh, bus, data->dev, username);
 	if (debug)
 		pam_syslog (pamh, LOG_DEBUG, "prints registered: %s\n", have_prints ? "yes" : "no");
 
 	if (!have_prints)
-		goto out;
+		return PAM_AUTHINFO_UNAVAIL;
 
-	if (claim_device (pamh, bus, dev, username)) {
-		ret = do_verify (pamh, bus, dev, has_multiple_devices);
-		release_device (pamh, bus, dev);
+	if (claim_device (pamh, bus, data->dev, username)) {
+		int ret = do_verify (bus, data);
+		release_device (pamh, bus, data->dev);
+		return ret;
 	}
 
-out:
-	free (dev);
-	sd_bus_unref (bus);
-
-	return ret;
+	return PAM_AUTHINFO_UNAVAIL;
 }
 
 static bool
@@ -613,8 +676,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
 				   const char **argv)
 {
 	const char *username;
-	unsigned i;
-	int r;
+	int i;
 
 	bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR);
 	bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8");
@@ -622,8 +684,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
 	if (is_remote (pamh))
 		return PAM_AUTHINFO_UNAVAIL;
 
-	r = pam_get_user(pamh, &username, NULL);
-	if (r != PAM_SUCCESS)
+	if (pam_get_user(pamh, &username, NULL) != PAM_SUCCESS)
 		return PAM_AUTHINFO_UNAVAIL;
 
 	for (i = 0; i < argc; i++) {
@@ -674,9 +735,7 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
 		}
 	}
 
-	r = do_auth(pamh, username);
-
-	return r;
+	return do_auth(pamh, username);
 }
 
 PAM_EXTERN int pam_sm_setcred(pam_handle_t *pamh, int flags, int argc,

pam/pam_fprintd_autoptrs.h

@@ -0,0 +1,60 @@
+/*
+ * pam_fprint: PAM module for fingerprint authentication through fprintd
+ * Copyright (C) 2020 Marco Trevisan <marco.trevisan@canonical.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#pragma once
+
+#include <stdlib.h>
+
+/* Define auto-pointers functions, based on GLib Macros */
+
+#define _CLEANUP_FUNC(func) __attribute__((cleanup(func)))
+
+#define _PF_AUTOPTR_FUNC_NAME(TypeName) pf_autoptr_cleanup_##TypeName
+#define _PF_AUTOPTR_TYPENAME(TypeName)  TypeName##_pf_autoptr
+
+#define PF_DEFINE_AUTOPTR_CLEANUP_FUNC(TypeName, cleanup) \
+	typedef TypeName *_PF_AUTOPTR_TYPENAME (TypeName);    \
+	static __attribute__((__unused__)) inline void        \
+	_PF_AUTOPTR_FUNC_NAME (TypeName) (TypeName **_ptr)    \
+	{ if (_ptr) (cleanup) (*_ptr); }
+
+#define PF_DEFINE_AUTO_CLEAN_FUNC(TypeName, cleanup)  \
+	static __attribute__((__unused__)) inline void    \
+	_PF_AUTOPTR_FUNC_NAME (TypeName) (TypeName *_ptr) \
+	{ cleanup (_ptr); }
+
+static inline void
+autoptr_cleanup_generic_free (void *p)
+{
+  void **pp = (void**)p;
+  free (*pp);
+}
+
+#define pf_autofree _CLEANUP_FUNC (autoptr_cleanup_generic_free)
+#define pf_autoptr(TypeName) \
+	_CLEANUP_FUNC (_PF_AUTOPTR_FUNC_NAME (TypeName)) \
+	_PF_AUTOPTR_TYPENAME (TypeName)
+#define pf_auto(TypeName) \
+	_CLEANUP_FUNC (_PF_AUTOPTR_FUNC_NAME (TypeName)) TypeName
+
+PF_DEFINE_AUTOPTR_CLEANUP_FUNC (sd_bus, sd_bus_unref)
+PF_DEFINE_AUTOPTR_CLEANUP_FUNC (sd_bus_message, sd_bus_message_unref)
+PF_DEFINE_AUTOPTR_CLEANUP_FUNC (sd_bus_slot, sd_bus_slot_unref)
+
+PF_DEFINE_AUTO_CLEAN_FUNC (sd_bus_error, sd_bus_error_free)

src/dbus-interactive-auth.patch

@@ -0,0 +1,110 @@
+--- a/src/fprintd-dbus.c	2020-12-04 16:38:28.527712626 +0100
++++ b/src/fprintd-dbus.c	2020-12-04 16:40:03.561692619 +0100
+@@ -1149,7 +1149,7 @@
+     "ListEnrolledFingers",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1213,7 +1213,7 @@
+     "ListEnrolledFingers",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);
+@@ -1253,7 +1253,7 @@
+     "DeleteEnrolledFingers",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1312,7 +1312,7 @@
+     "DeleteEnrolledFingers",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);
+@@ -1348,7 +1348,7 @@
+   g_dbus_proxy_call (G_DBUS_PROXY (proxy),
+     "DeleteEnrolledFingers2",
+     g_variant_new ("()"),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1404,7 +1404,7 @@
+   _ret = g_dbus_proxy_call_sync (G_DBUS_PROXY (proxy),
+     "DeleteEnrolledFingers2",
+     g_variant_new ("()"),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);
+@@ -1443,7 +1443,7 @@
+     "Claim",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1502,7 +1502,7 @@
+     "Claim",
+     g_variant_new ("(s)",
+                    arg_username),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);
+@@ -1633,7 +1633,7 @@
+     "VerifyStart",
+     g_variant_new ("(s)",
+                    arg_finger_name),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1692,7 +1692,7 @@
+     "VerifyStart",
+     g_variant_new ("(s)",
+                    arg_finger_name),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);
+@@ -1823,7 +1823,7 @@
+     "EnrollStart",
+     g_variant_new ("(s)",
+                    arg_finger_name),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     callback,
+@@ -1882,7 +1882,7 @@
+     "EnrollStart",
+     g_variant_new ("(s)",
+                    arg_finger_name),
+-    G_DBUS_CALL_FLAGS_NONE,
++    G_DBUS_CALL_FLAGS_ALLOW_INTERACTIVE_AUTHORIZATION,
+     -1,
+     cancellable,
+     error);

src/device.c

@@ -33,17 +33,17 @@
 #include "storage.h"
 
 static const char *FINGERS_NAMES[] = {
-	[FP_FINGER_UNKNOWN] = "unknown",
-	"left-thumb",
-	"left-index-finger",
-	"left-middle-finger",
-	"left-ring-finger",
-	"left-little-finger",
-	"right-thumb",
-	"right-index-finger",
-	"right-middle-finger",
-	"right-ring-finger",
-	"right-little-finger"
+	[FP_FINGER_UNKNOWN]      = "unknown",
+	[FP_FINGER_LEFT_THUMB]   = "left-thumb",
+	[FP_FINGER_LEFT_INDEX]   = "left-index-finger",
+	[FP_FINGER_LEFT_MIDDLE]  = "left-middle-finger",
+	[FP_FINGER_LEFT_RING]    = "left-ring-finger",
+	[FP_FINGER_LEFT_LITTLE]  = "left-little-finger",
+	[FP_FINGER_RIGHT_THUMB]  = "right-thumb",
+	[FP_FINGER_RIGHT_INDEX]  = "right-index-finger",
+	[FP_FINGER_RIGHT_MIDDLE] = "right-middle-finger",
+	[FP_FINGER_RIGHT_RING]   = "right-ring-finger",
+	[FP_FINGER_RIGHT_LITTLE] = "right-little-finger"
 };
 
 static void fprint_device_dbus_skeleton_iface_init (FprintDBusDeviceIface *);
@@ -60,12 +60,14 @@ typedef enum {
 	ACTION_ENROLL,
 	ACTION_OPEN,
 	ACTION_CLOSE,
+	ACTION_DELETE,
 } FprintDeviceAction;
 
 typedef enum {
 	STATE_CLAIMED,
 	STATE_UNCLAIMED,
-	STATE_IGNORED,
+	STATE_AUTO_CLAIM,
+	STATE_ANYTIME,
 } FprintDeviceClaimState;
 
 typedef struct {
@@ -129,6 +131,12 @@ enum fprint_device_signals {
 static guint32 last_id = ~0;
 static guint signals[NUM_SIGNALS] = { 0, };
 
+#ifndef POLKIT_HAS_AUTOPOINTERS
+/* FIXME: Remove this once we're fine to depend on polkit 0.114 */
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (PolkitAuthorizationResult, g_object_unref)
+G_DEFINE_AUTOPTR_CLEANUP_FUNC (PolkitSubject, g_object_unref)
+#endif
+
 static void
 session_data_unref(SessionData *session)
 {
@@ -360,22 +368,22 @@ guint32 _fprint_device_get_id(FprintDevice *rdev)
 }
 
 static const char *
-finger_num_to_name (int finger_num)
+fp_finger_to_name (FpFinger finger)
 {
-	if (finger_num == -1)
+	if (finger == FP_FINGER_UNKNOWN)
 		return "any";
-	if (!FP_FINGER_IS_VALID (finger_num))
+	if (!FP_FINGER_IS_VALID (finger))
 		return NULL;
-	return FINGERS_NAMES[finger_num];
+	return FINGERS_NAMES[finger];
 }
 
-static int
-finger_name_to_num (const char *finger_name)
+static FpFinger
+finger_name_to_fp_finger (const char *finger_name)
 {
-	guint i;
+	FpFinger i;
 
 	if (finger_name == NULL || *finger_name == '\0' || g_str_equal (finger_name, "any"))
-		return -1;
+		return FP_FINGER_UNKNOWN;
 
 	for (i = FP_FINGER_FIRST; i <= FP_FINGER_LAST; i++) {
 		if (g_str_equal (finger_name, FINGERS_NAMES[i]))
@@ -383,7 +391,7 @@ finger_name_to_num (const char *finger_name)
 	}
 
 	/* Invalid, let's try that */
-	return -1;
+	return FP_FINGER_UNKNOWN;
 }
 
 static const char *
@@ -458,21 +466,78 @@ enroll_result_to_name (gboolean completed, gboolean enrolled, GError *error)
 	}
 }
 
+static FprintDevicePermission
+get_permissions_for_invocation (GDBusMethodInvocation *invocation)
+{
+	FprintDevicePermission required_perms;
+	const char *method_name;
+
+	required_perms = FPRINT_DEVICE_PERMISSION_NONE;
+	method_name = g_dbus_method_invocation_get_method_name (invocation);
+
+	if (g_str_equal (method_name, "Claim")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
+		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
+	} else if (g_str_equal (method_name, "DeleteEnrolledFingers")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
+	} else if (g_str_equal (method_name, "DeleteEnrolledFingers2")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
+	} else if (g_str_equal (method_name, "EnrollStart")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
+	} else if (g_str_equal (method_name, "ListEnrolledFingers")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
+	} else if (g_str_equal (method_name, "VerifyStart")) {
+		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
+	} else if (g_str_equal (method_name, "Release")) {
+	} else if (g_str_equal (method_name, "EnrollStop")) {
+	} else if (g_str_equal (method_name, "VerifyStop")) {
+		/* Don't require permissiong for for release/stop operations.
+		 * We are authenticated already if we could start, and we don't
+		 * want to end up authorizing interactively again.
+		 */
+	} else {
+		g_assert_not_reached ();
+	}
+
+	return required_perms;
+}
+
+static FprintDeviceClaimState
+get_claim_state_for_invocation (GDBusMethodInvocation *invocation)
+{
+	const char *method_name;
+
+	method_name = g_dbus_method_invocation_get_method_name (invocation);
+
+	if (g_str_equal (method_name, "Claim")) {
+		return STATE_UNCLAIMED;
+	} else if (g_str_equal (method_name, "DeleteEnrolledFingers")) {
+		return STATE_AUTO_CLAIM;
+	} else if (g_str_equal (method_name, "ListEnrolledFingers")) {
+		return STATE_ANYTIME;
+	}
+
+	return STATE_CLAIMED;
+}
+
 static gboolean
 _fprint_device_check_claimed (FprintDevice *rdev,
 			      GDBusMethodInvocation *invocation,
-			      FprintDeviceClaimState requested_state,
 			      GError **error)
 {
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
 	g_autoptr(SessionData) session = NULL;
+	FprintDeviceClaimState requested_state;
 	const char *sender;
-	gboolean retval;
 
-	if (requested_state == STATE_IGNORED)
+	requested_state = get_claim_state_for_invocation (invocation);
+
+	if (requested_state == STATE_ANYTIME)
 		return TRUE;
 
 	session = session_data_get (priv);
+	if (requested_state == STATE_AUTO_CLAIM)
+		requested_state = session ? STATE_CLAIMED : STATE_UNCLAIMED;
 
 	if (requested_state == STATE_UNCLAIMED) {
 		/* Is it already claimed? */
@@ -495,14 +560,14 @@ _fprint_device_check_claimed (FprintDevice *rdev,
 	}
 
 	sender = g_dbus_method_invocation_get_sender (invocation);
-	retval = g_str_equal (sender, session->sender);
 
-	if (retval == FALSE || session->invocation != NULL) {
+	if (!g_str_equal (sender, session->sender) || session->invocation != NULL) {
 		g_set_error (error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
 			     _("Device already in use by another user"));
+		return FALSE;
 	}
 
-	return retval;
+	return TRUE;
 }
 
 static gboolean
@@ -513,7 +578,7 @@ _fprint_device_check_polkit_for_action (FprintDevice *rdev,
 {
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
 	const char *sender;
-	g_autoptr(GError) _error = NULL;
+	g_autoptr(GError) local_error = NULL;
 	g_autoptr(PolkitAuthorizationResult) result = NULL;
 	g_autoptr(PolkitSubject) subject = NULL;
 
@@ -526,11 +591,11 @@ _fprint_device_check_polkit_for_action (FprintDevice *rdev,
                                                             action,
 							    NULL,
                                                             POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
-					                    NULL, &_error);
+					                    NULL, &local_error);
 	if (result == NULL) {
 		g_set_error (error, FPRINT_ERROR,
 			     FPRINT_ERROR_PERMISSION_DENIED,
-			     "Not Authorized: %s", _error->message);
+			     "Not Authorized: %s", local_error->message);
 		return FALSE;
 	}
 
@@ -773,21 +838,20 @@ static gboolean fprint_device_claim (FprintDBusDevice *dbus_dev,
 	g_autoptr(GError) error = NULL;
 	char *sender, *user;
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_UNCLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
 	user = g_object_steal_qdata (G_OBJECT (invocation), quark_auth_user);
 	g_assert (user);
+	g_assert (g_str_equal (username, "") || g_str_equal (user, username));
 
 	sender = g_strdup (g_dbus_method_invocation_get_sender (invocation));
 	_fprint_device_add_client (rdev, sender);
 
 	session = session_data_set_new (priv, g_steal_pointer (&sender), g_steal_pointer(&user));
 	session->invocation = g_object_ref (invocation);
-	username = g_steal_pointer (&user);
-	sender = g_steal_pointer (&sender);
 
 	g_debug ("user '%s' claiming the device: %d", session->username, priv->id);
 
@@ -834,7 +898,7 @@ static gboolean fprint_device_release (FprintDBusDevice *dbus_dev,
 	FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
@@ -845,6 +909,8 @@ static gboolean fprint_device_release (FprintDBusDevice *dbus_dev,
 		} else if (priv->current_action == ACTION_IDENTIFY ||
 			   priv->current_action == ACTION_VERIFY) {
 			g_warning("Verification was in progress, stopping it");
+		} else if (priv->current_action == ACTION_DELETE) {
+			g_warning("Deletion was in progress, stopping it");
 		}
 
 		g_cancellable_cancel (priv->current_cancellable);
@@ -889,6 +955,48 @@ static void report_verify_status (FprintDevice *rdev,
 		session->verify_status_reported = TRUE;
 }
 
+static gboolean can_start_action(FprintDevice *rdev, GError **error) {
+	FprintDevicePrivate *priv = fprint_device_get_instance_private (rdev);
+
+	switch (priv->current_action) {
+		case ACTION_NONE:
+			return TRUE;
+		case ACTION_ENROLL:
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Enrollment already in progress");
+			break;
+		case ACTION_IDENTIFY:
+		case ACTION_VERIFY:
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Enrollment already in progress");
+			break;
+		case ACTION_OPEN:
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Claim already in progress");
+			break;
+		case ACTION_CLOSE:
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Release already in progress");
+			break;
+		case ACTION_DELETE:
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Delete already in progress");
+			break;
+		default: /* Fallback only. */
+			g_assert_not_reached();
+			g_set_error (error,
+				     FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
+				     "Another operation is already in progress");
+	}
+
+	return FALSE;
+}
+
 static void match_cb (FpDevice *device,
 		      FpPrint *match,
 		      FpPrint *print,
@@ -1026,28 +1134,21 @@ static gboolean fprint_device_verify_start (FprintDBusDevice *dbus_dev,
 	g_autoptr(FpPrint) print = NULL;
 	g_autoptr(SessionData) session = NULL;
 	g_autoptr(GError) error = NULL;
-	guint finger_num = finger_name_to_num (finger_name);
+	FpFinger finger = finger_name_to_fp_finger (finger_name);
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
 	session = session_data_get (priv);
 
-	if (priv->current_action != ACTION_NONE) {
-		if (priv->current_action == ACTION_ENROLL) {
-			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				    "Enrollment in progress");
-		} else {
-			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				    "Verification already in progress");
-		}
+	if (!can_start_action (rdev, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	if (finger_num == -1) {
+	if (finger == FP_FINGER_UNKNOWN) {
 		g_autoptr(GSList) prints = NULL;
 
 		prints = store.discover_prints(priv->dev, session->username);
@@ -1063,19 +1164,19 @@ static gboolean fprint_device_verify_start (FprintDBusDevice *dbus_dev,
 			gallery = g_ptr_array_new_with_free_func (g_object_unref);
 
 			for (l = prints; l != NULL; l = l->next) {
-				g_debug ("adding finger %d to the gallery", GPOINTER_TO_INT (l->data));
-				store.print_data_load(priv->dev, GPOINTER_TO_INT (l->data),
+				g_debug ("adding finger %u to the gallery", GPOINTER_TO_UINT (l->data));
+				store.print_data_load(priv->dev, GPOINTER_TO_UINT (l->data),
 						      session->username, &print);
 
 				if (print)
 					g_ptr_array_add (gallery, g_steal_pointer (&print));
 			}
 		} else {
-			finger_num = GPOINTER_TO_INT (prints->data);
+			finger = GPOINTER_TO_UINT (prints->data);
 		}
 	}
 
-	if (fp_device_supports_identify (priv->dev) && finger_num == -1) {
+	if (fp_device_supports_identify (priv->dev) && finger == FP_FINGER_UNKNOWN) {
 		if (gallery->len == 0) {
 			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ENROLLED_PRINTS,
 				    "No fingerprints on that device");
@@ -1093,14 +1194,14 @@ static gboolean fprint_device_verify_start (FprintDBusDevice *dbus_dev,
 	} else {
 		priv->current_action = ACTION_VERIFY;
 
-		g_debug("start verification device %d finger %d", priv->id, finger_num);
+		g_debug("start verification device %d finger %d", priv->id, finger);
 
-		store.print_data_load(priv->dev, finger_num,
+		store.print_data_load(priv->dev, finger,
 				      session->username, &print);
 
 		if (!print) {
 			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ENROLLED_PRINTS,
-				    "No such print %d", finger_num);
+				    "No such print %d", finger);
 			g_dbus_method_invocation_return_gerror (invocation,
 								error);
 			return TRUE;
@@ -1113,12 +1214,12 @@ static gboolean fprint_device_verify_start (FprintDBusDevice *dbus_dev,
 		                  (GAsyncReadyCallback) verify_cb, rdev);
 	}
 
+	fprint_dbus_device_complete_verify_start  (dbus_dev, invocation);
+
 	/* Emit VerifyFingerSelected telling the front-end which finger
 	 * we selected for auth */
 	g_signal_emit(rdev, signals[SIGNAL_VERIFY_FINGER_SELECTED],
-		      0, finger_num_to_name (finger_num));
-
-	fprint_dbus_device_complete_verify_start  (dbus_dev, invocation);
+		      0, fp_finger_to_name (finger));
 
 	return TRUE;
 }
@@ -1131,23 +1232,27 @@ static gboolean fprint_device_verify_stop (FprintDBusDevice *dbus_dev,
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
 	g_autoptr(GError) error = NULL;
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	if (priv->current_action == ACTION_NONE) {
-		g_dbus_method_invocation_return_error_literal (invocation,
-							       FPRINT_ERROR,
-							       FPRINT_ERROR_NO_ACTION_IN_PROGRESS,
-							       "No verification in progress");
-		return TRUE;
-	} else if (priv->current_action == ACTION_ENROLL) {
-		g_dbus_method_invocation_return_error_literal (invocation,
-							       FPRINT_ERROR,
-							       FPRINT_ERROR_ALREADY_IN_USE,
-							       "Enrollment in progress");
-		return TRUE;
+	switch (priv->current_action) {
+		case ACTION_VERIFY:
+		case ACTION_IDENTIFY:
+			break;
+		case ACTION_NONE:
+			g_dbus_method_invocation_return_error_literal (
+				invocation, FPRINT_ERROR,
+				FPRINT_ERROR_NO_ACTION_IN_PROGRESS,
+				"No verification in progress");
+			return TRUE;
+		default:
+			g_dbus_method_invocation_return_error_literal (
+				invocation, FPRINT_ERROR,
+				FPRINT_ERROR_ALREADY_IN_USE,
+				"Another operation is already in progress");
+			return TRUE;
 	}
 
 	if (priv->current_cancellable) {
@@ -1209,7 +1314,7 @@ static gboolean try_delete_print(FprintDevice *rdev)
 			guint index;
 
 			store.print_data_load (priv->dev,
-			                       GPOINTER_TO_INT (fingers->data),
+			                       GPOINTER_TO_UINT (fingers->data),
 			                       username,
 			                       &print);
 
@@ -1253,7 +1358,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC (GDate, g_date_free);
 #endif
 
 static FpPrint*
-fprint_device_create_enroll_template(FprintDevice *rdev, gint finger_num)
+fprint_device_create_enroll_template(FprintDevice *rdev, FpFinger finger)
 {
 	g_autoptr(SessionData) session = NULL;
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
@@ -1265,7 +1370,7 @@ fprint_device_create_enroll_template(FprintDevice *rdev, gint finger_num)
 	session = session_data_get (priv);
 
 	template = fp_print_new (priv->dev);
-	fp_print_set_finger (template, finger_num);
+	fp_print_set_finger (template, finger);
 	fp_print_set_username (template, session->username);
 	datetime = g_date_time_new_now_local ();
 	g_date_time_get_ymd (datetime, &year, &month, &day);
@@ -1343,36 +1448,29 @@ static gboolean fprint_device_enroll_start (FprintDBusDevice *dbus_dev,
 	g_autoptr(GError) error = NULL;
 	FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
-	int finger_num = finger_name_to_num (finger_name);
+	FpFinger finger = finger_name_to_fp_finger (finger_name);
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	if (finger_num == -1) {
+	if (finger == FP_FINGER_UNKNOWN) {
 		g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INVALID_FINGERNAME,
 			    "Invalid finger name");
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	if (priv->current_action != ACTION_NONE) {
-		if (priv->current_action == ACTION_ENROLL) {
-			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				    "Enrollment already in progress");
-		} else {
-			g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				    "Verification in progress");
-		}
+	if (!can_start_action (rdev, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	g_debug("start enrollment device %d finger %d", priv->id, finger_num);
+	g_debug("start enrollment device %d finger %d", priv->id, finger);
 
 	priv->current_cancellable = g_cancellable_new ();
-	priv->enroll_data = finger_num;
+	priv->enroll_data = finger;
 	fp_device_enroll (priv->dev,
 	                  fprint_device_create_enroll_template (rdev, priv->enroll_data),
 	                  priv->current_cancellable,
@@ -1396,25 +1494,26 @@ static gboolean fprint_device_enroll_stop (FprintDBusDevice *dbus_dev,
 	FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
 	g_autoptr(GError) error = NULL;
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
-	if (priv->current_action != ACTION_ENROLL) {
-		if (priv->current_action == ACTION_NONE) {
-			g_set_error (&error, FPRINT_ERROR, FPRINT_ERROR_NO_ACTION_IN_PROGRESS,
-				     "No enrollment in progress");
-		} else if (priv->current_action == ACTION_VERIFY) {
-			g_set_error (&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				     "Verification in progress");
-		} else if (priv->current_action == ACTION_IDENTIFY) {
-			g_set_error (&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
-				     "Identification in progress");
-		} else
-			g_assert_not_reached ();
-		g_dbus_method_invocation_return_gerror (invocation, error);
-		return TRUE;
+	switch (priv->current_action) {
+		case ACTION_ENROLL:
+			break;
+		case ACTION_NONE:
+			g_dbus_method_invocation_return_error_literal (
+				invocation, FPRINT_ERROR,
+				FPRINT_ERROR_NO_ACTION_IN_PROGRESS,
+				"No enrollment in progress");
+			return TRUE;
+		default:
+			g_dbus_method_invocation_return_error_literal (
+				invocation, FPRINT_ERROR,
+				FPRINT_ERROR_ALREADY_IN_USE,
+				"Another operation is already in progress");
+			return TRUE;
 	}
 
 	if (priv->current_cancellable) {
@@ -1457,8 +1556,8 @@ static gboolean fprint_device_list_enrolled_fingers (FprintDBusDevice *dbus_dev,
 
 	ret = g_ptr_array_new ();
 	for (item = prints; item; item = item->next) {
-		int finger_num = GPOINTER_TO_INT (item->data);
-		g_ptr_array_add (ret, (char *) finger_num_to_name (finger_num));
+		FpFinger finger = GPOINTER_TO_UINT (item->data);
+		g_ptr_array_add (ret, (char *) fp_finger_to_name (finger));
 	}
 	g_ptr_array_add (ret, NULL);
 
@@ -1487,7 +1586,7 @@ static void delete_enrolled_fingers(FprintDevice *rdev, const char *user)
 			g_autoptr(FpPrint) print = NULL;
 
 			store.print_data_load(priv->dev,
-			                      GPOINTER_TO_INT (l->data),
+			                      GPOINTER_TO_UINT (l->data),
 			                      user,
 			                      &print);
 
@@ -1566,11 +1665,14 @@ static gboolean fprint_device_delete_enrolled_fingers (FprintDBusDevice *dbus_de
 	log_offending_client (invocation);
 #endif
 
-	user = g_object_steal_qdata (G_OBJECT (invocation), quark_auth_user);
-	g_assert (user);
+	if (!can_start_action(rdev, &error)) {
+		g_dbus_method_invocation_return_gerror (invocation, error);
+		return TRUE;
+	}
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED,
-					   &error)) {
+	priv->current_action = ACTION_DELETE;
+
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		/* Return error for anything but FPRINT_ERROR_CLAIM_DEVICE */
 		if (!g_error_matches (error, FPRINT_ERROR, FPRINT_ERROR_CLAIM_DEVICE)) {
 			g_dbus_method_invocation_return_gerror (invocation,
@@ -1589,11 +1691,17 @@ static gboolean fprint_device_delete_enrolled_fingers (FprintDBusDevice *dbus_de
 	if (!opened && fp_device_has_storage (priv->dev))
 		fp_device_open_sync (priv->dev, NULL, NULL);
 
+	user = g_object_steal_qdata (G_OBJECT (invocation), quark_auth_user);
+	g_assert (user);
+	g_assert (g_str_equal (username, "") || g_str_equal (user, username));
+
 	delete_enrolled_fingers (rdev, user);
 
 	if (!opened && fp_device_has_storage (priv->dev))
 		fp_device_close_sync (priv->dev, NULL, NULL);
 
+	priv->current_action = ACTION_NONE;
+
 	fprint_dbus_device_complete_delete_enrolled_fingers (dbus_dev,
 							     invocation);
 	return TRUE;
@@ -1607,14 +1715,24 @@ static gboolean fprint_device_delete_enrolled_fingers2 (FprintDBusDevice *dbus_d
 	g_autoptr(SessionData) session = NULL;
 	g_autoptr(GError) error = NULL;
 
-	if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		g_dbus_method_invocation_return_gerror (invocation, error);
 		return TRUE;
 	}
 
+	if (!can_start_action(rdev, &error)) {
+		g_dbus_method_invocation_return_gerror (invocation, error);
+		return TRUE;
+	}
+
+	priv->current_action = ACTION_DELETE;
+
 	session = session_data_get (priv);
 
 	delete_enrolled_fingers (rdev, session->username);
+
+	priv->current_action = ACTION_NONE;
+
 	fprint_dbus_device_complete_delete_enrolled_fingers2 (dbus_dev,
 							      invocation);
 	return TRUE;
@@ -1646,8 +1764,7 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
 	FprintDBusDevice *dbus_dev = FPRINT_DBUS_DEVICE (interface);
 	FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
 	FprintDevicePrivate *priv = fprint_device_get_instance_private (rdev);
-	FprintDeviceClaimState required_state = STATE_IGNORED;
-	FprintDevicePermission required_perms = FPRINT_DEVICE_PERMISSION_NONE;
+	FprintDevicePermission required_perms;
 	gboolean needs_user_auth = FALSE;
 	g_autoptr(GError) error = NULL;
 	const gchar *method_name;
@@ -1660,43 +1777,16 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
 
 	if (g_str_equal (method_name, "Claim")) {
 		needs_user_auth = TRUE;
-		required_state = STATE_UNCLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
 	} else if (g_str_equal (method_name, "DeleteEnrolledFingers")) {
 		needs_user_auth = TRUE;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
-	} else if (g_str_equal (method_name, "DeleteEnrolledFingers2")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
-	} else if (g_str_equal (method_name, "EnrollStart")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
-	} else if (g_str_equal (method_name, "EnrollStop")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
 	} else if (g_str_equal (method_name, "ListEnrolledFingers")) {
 		needs_user_auth = TRUE;
-		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
-	} else if (g_str_equal (method_name, "Release")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
-		required_perms |= FPRINT_DEVICE_PERMISSION_ENROLL;
-	} else if (g_str_equal (method_name, "VerifyStart")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
-	} else if (g_str_equal (method_name, "VerifyStop")) {
-		required_state = STATE_CLAIMED;
-		required_perms |= FPRINT_DEVICE_PERMISSION_VERIFY;
-	} else {
-		g_assert_not_reached ();
 	}
 
 	/* This is just a quick check in order to avoid authentication if
 	 * the user cannot make the call at this time anyway.
 	 * The method handler itself is required to check again! */
-	if (!_fprint_device_check_claimed (rdev, invocation, required_state,
-					   &error)) {
+	if (!_fprint_device_check_claimed (rdev, invocation, &error)) {
 		return handle_unauthorized_access (rdev, invocation, error);
 	}
 
@@ -1705,6 +1795,8 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
 		return handle_unauthorized_access (rdev, invocation, error);
 	}
 
+	required_perms = get_permissions_for_invocation (invocation);
+
 	/* This may possibly block the invocation till the user has not
 	 * provided an authentication method, so other calls could arrive */
 	if (!fprint_device_check_polkit_for_permissions (rdev, invocation,

src/file_storage.c

@@ -251,7 +251,7 @@ static GSList *scan_dev_storedir(char *devpath,
 			continue;
 		}
 
-		list = g_slist_prepend(list, GINT_TO_POINTER(val));
+		list = g_slist_prepend (list, GUINT_TO_POINTER (val));
 	}
 
 	g_dir_close(dir);

src/fprintd.h

@@ -53,11 +53,16 @@ typedef enum {
 	FPRINT_ERROR_NO_SUCH_DEVICE, /*< nick=net.reactivated.Fprint.Error.NoSuchDevice >*/
 } FprintError;
 
+/* Enum of possible permissions, orders and nick matter here:
+   - The order controls the priority of a required permission when various are
+     accepted: the lowest the value, the more priorty it has.
+   - Nick must match the relative polkit rule.
+*/
 typedef enum {
 	FPRINT_DEVICE_PERMISSION_NONE = 0,
-	FPRINT_DEVICE_PERMISSION_ENROLL = (1 << 0), /*< nick=net.reactivated.fprint.device.enroll >*/
-	FPRINT_DEVICE_PERMISSION_SETUSERNAME = (1 << 1), /*< nick=net.reactivated.fprint.device.setusername >*/
-	FPRINT_DEVICE_PERMISSION_VERIFY = (1 << 2), /*< nick=net.reactivated.fprint.device.verify >*/
+	FPRINT_DEVICE_PERMISSION_VERIFY = (1 << 0), /*< nick=net.reactivated.fprint.device.verify >*/
+	FPRINT_DEVICE_PERMISSION_ENROLL = (1 << 1), /*< nick=net.reactivated.fprint.device.enroll >*/
+	FPRINT_DEVICE_PERMISSION_SETUSERNAME = (1 << 2), /*< nick=net.reactivated.fprint.device.setusername >*/
 } FprintDevicePermission;
 
 /* Manager */

src/meson.build

@@ -14,7 +14,8 @@ foreach interface_name: dbus_interfaces
     )
 endforeach
 
-fprintd_dbus_sources = gnome.gdbus_codegen('fprintd-dbus',
+# NOTE: We should pass "--glib-min-required 2.64" but cannot
+fprintd_dbus_sources_base = gnome.gdbus_codegen('fprintd-dbus',
     sources: dbus_interfaces_files,
     autocleanup: 'all',
     interface_prefix: 'net.reactivated.Fprint.',
@@ -22,6 +23,23 @@ fprintd_dbus_sources = gnome.gdbus_codegen('fprintd-dbus',
     object_manager: true,
 )
 
+# FIXME: remove this and just use fprintd_dbus_sources when we're on glib 2.64
+fprintd_dbus_sources = [
+    fprintd_dbus_sources_base[1] # header file
+]
+
+fprintd_dbus_sources += custom_target('fprintd-dbus-interactive',
+    input: fprintd_dbus_sources_base[0], # c file,
+    output: 'fprintd-dbus-interactive.c',
+    command: [
+        find_program('patch'),
+        '-p1',
+        '--merge',
+        '@INPUT@',
+        files('dbus-interactive-auth.patch'),
+        '-o', '@OUTPUT@',
+    ])
+
 fprintd_enum_files = gnome.mkenums_simple('fprintd-enums',
   sources: 'fprintd.h',
 )

tests/dbusmock/fprintd.py

@@ -18,6 +18,8 @@ __email__ = 'hadess@hadess.net'
 __copyright__ = '(c) 2020 Red Hat Inc.'
 __license__ = 'LGPL 3+'
 
+import sys
+from gi.repository import GLib
 import dbus
 import asyncio
 
@@ -213,13 +215,26 @@ def can_verify_finger(device, finger_name):
         return True
     return False
 
-async def send_verify_script(device, script):
-    for [result, done, timeout] in device.verify_script:
-        await asyncio.sleep(timeout)
-        device.EmitSignal(DEVICE_IFACE, 'VerifyStatus', 'sb', [
-                            result,
-                            done
-                          ])
+def glib_sleep(timeout):
+    waiting = True
+
+    def done_waiting():
+        nonlocal waiting
+        waiting = False
+
+    GLib.timeout_add(timeout, done_waiting)
+    while (waiting):
+        GLib.main_context_default().iteration(True)
+
+def device_run_script(device, result, done):
+    if result == 'MOCK: quit':
+        sys.exit(0)
+
+    # Emit signal
+    device.EmitSignal(DEVICE_IFACE, 'VerifyStatus', 'sb', [
+                        result,
+                        done
+                      ])
 
 @dbus.service.method(DEVICE_IFACE,
                      in_signature='s', out_signature='')
@@ -249,13 +264,44 @@ def VerifyStart(device, finger_name):
     if finger_name == 'any' and not device.has_identification:
         finger_name = device.fingers[device.claimed_user][0]
     device.selected_finger = finger_name
-    device.EmitSignal(DEVICE_IFACE, 'VerifyFingerSelected', 's', [
-                          finger_name
-                      ])
+    # Needs to happen after method return
+    GLib.idle_add(device.EmitSignal,
+                  DEVICE_IFACE, 'VerifyFingerSelected', 's', [
+                    device.selected_finger
+                  ])
 
-    if device.verify_script is not None and len(device.verify_script) > 0:
-        asyncio.run(send_verify_script(device, device.verify_script))
+    error = None
+    base_delay = 0
+    while device.verify_script is not None and len(device.verify_script) > 0:
+        result, done, timeout = device.verify_script.pop(0)
 
+        # We stop when "timeout >= 0 and done"
+        if result == 'MOCK: no-prints':
+            # Special case to change return value of DBus call, ignores timeout
+            error = dbus.exceptions.DBusException(
+                'No enrolled prints for user \'%s\'' % device.claimed_user,
+                name='net.reactivated.Fprint.Error.NoEnrolledPrints')
+
+        elif timeout < 0:
+            # Negative timeouts mean emitting before the DBus call returns
+            device_run_script(device, result, done)
+            glib_sleep(-timeout)
+
+        else:
+            # Positive or zero means emitting afterwards the given timeout
+            base_delay += timeout
+            GLib.timeout_add(base_delay,
+                             device_run_script,
+                             device,
+                             result,
+                             done)
+
+            # Stop processing commands when the done flag is set
+            if done:
+                break
+
+    if error:
+        raise error
 
 @dbus.service.method(DEVICE_MOCK_IFACE,
                      in_signature='sb', out_signature='')
@@ -383,3 +429,11 @@ def SetVerifyScript(device, script):
     '''
 
     device.verify_script = script
+
+@dbus.service.method(DEVICE_MOCK_IFACE,
+                     in_signature='s', out_signature='')
+def SetClaimed(device, user):
+    if user == '':
+        device.claimed_user = None
+    else:
+        device.claimed_user = user

tests/fprintd.py

@@ -138,6 +138,8 @@ class FPrintdTest(dbusmock.DBusTestCase):
         fprintd = None
         cls._polkitd = None
 
+        cls._has_hotplug = FPrint.Device.find_property("removed") is not None
+
         if 'FPRINT_BUILD_DIR' in os.environ:
             print('Testing local build')
             build_dir = os.environ['FPRINT_BUILD_DIR']
@@ -491,32 +493,55 @@ class FPrintdManagerPreStartTests(FPrintdTest):
             self.manager.GetDefaultDevice()
 
     def test_manager_get_devices_on_name_appeared(self):
-        self._appeared_res = []
+        self._appeared_name = None
 
         def on_name_appeared(connection, name, name_owner):
-            self._appeared_res.append(connection.call_sync('net.reactivated.Fprint',
-                '/net/reactivated/Fprint/Manager',
-                'net.reactivated.Fprint.Manager',
-                'GetDefaultDevice', None, None,
-                Gio.DBusCallFlags.NO_AUTO_START, 500, None))
+            self._appeared_name = name
+
+        def on_name_vanished(connection, name):
+            self._appeared_name = 'NAME_VANISHED'
 
         id = Gio.bus_watch_name_on_connection(self.dbus,
             'net.reactivated.Fprint', Gio.BusNameWatcherFlags.NONE,
-            on_name_appeared, None)
+            on_name_appeared, on_name_vanished)
+        self.addCleanup(Gio.bus_unwatch_name, id)
 
         self.daemon_start()
-        while not self._appeared_res:
+        while not self._appeared_name:
             ctx.iteration(True)
 
-        self.assertIsNotNone(self._appeared_res[0])
-        dev_path = self._appeared_res[0][0]
-        self.assertTrue(dev_path.startswith('/net/reactivated/Fprint/Device/'))
+        self.assertEqual(self._appeared_name, 'net.reactivated.Fprint')
 
-        Gio.bus_unwatch_name(id)
+        try:
+            appeared_device = self.dbus.call_sync(
+                'net.reactivated.Fprint',
+                '/net/reactivated/Fprint/Manager',
+                'net.reactivated.Fprint.Manager',
+                'GetDefaultDevice', None, None,
+                Gio.DBusCallFlags.NO_AUTO_START, 500, None)
+        except GLib.GError as e:
+            if 'net.reactivated.Fprint.Error.NoSuchDevice' in e.message:
+                self.skipTest("Need virtual_image device to run the test")
+            raise(e)
+
+        self.assertIsNotNone(appeared_device)
+        [dev_path] = appeared_device
+        self.assertTrue(dev_path.startswith('/net/reactivated/Fprint/Device/'))
 
 
 class FPrintdVirtualDeviceTest(FPrintdVirtualDeviceBaseTest):
 
+    def test_name_property(self):
+        self.assertEqual(self.device.get_cached_property('name').unpack(),
+            'Virtual image device for debugging')
+
+    def test_enroll_stages_property(self):
+        self.assertEqual(self.device.get_cached_property('num-enroll-stages').unpack(), 5)
+
+    def test_scan_type(self):
+        self.assertEqual(self.device.get_cached_property('scan-type').unpack(),
+            'swipe')
+
     def test_allowed_claim_release_enroll(self):
         self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.setusername',
                                       'net.reactivated.fprint.device.enroll'])
@@ -578,6 +603,34 @@ class FPrintdVirtualDeviceTest(FPrintdVirtualDeviceBaseTest):
         with self.assertFprintError('PermissionDenied'):
             self.device.Claim('(s)', 'testuser')
 
+    def test_unallowed_enroll_with_verify_claim(self):
+        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.verify'])
+        self.device.Claim('(s)', '')
+
+        with self.assertFprintError('PermissionDenied'):
+            self.enroll_image('whorl', finger='right-thumb')
+
+    def test_unallowed_delete_with_verify_claim(self):
+        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.verify'])
+        self.device.Claim('(s)', '')
+
+        with self.assertFprintError('PermissionDenied'):
+            self.device.DeleteEnrolledFingers('(s)', 'testuser')
+
+    def test_unallowed_delete2_with_verify_claim(self):
+        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.verify'])
+        self.device.Claim('(s)', '')
+
+        with self.assertFprintError('PermissionDenied'):
+            self.device.DeleteEnrolledFingers2()
+
+    def test_unallowed_verify_with_enroll_claim(self):
+        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.enroll'])
+        self.device.Claim('(s)', '')
+
+        with self.assertFprintError('PermissionDenied'):
+            self.device.VerifyStart('(s)', 'any')
+
     def test_unallowed_claim_current_user(self):
         self._polkitd_obj.SetAllowed([''])
 
@@ -595,20 +648,11 @@ class FPrintdVirtualDeviceTest(FPrintdVirtualDeviceBaseTest):
 
         self.device.Release()
 
-    def test_unallowed_release(self):
+    def test_always_allowed_release(self):
         self.device.Claim('(s)', 'testuser')
 
         self._polkitd_obj.SetAllowed([''])
 
-        with self.assertFprintError('PermissionDenied'):
-            self.device.Release()
-
-        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.setusername'])
-
-        with self.assertFprintError('PermissionDenied'):
-            self.device.Release()
-
-        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.enroll'])
         self.device.Release()
 
     def test_unclaimed_release(self):
@@ -688,6 +732,9 @@ class FPrintdVirtualDeviceTest(FPrintdVirtualDeviceBaseTest):
         time.sleep(1)
 
     def test_removal_during_enroll(self):
+        if not self._has_hotplug:
+            self.skipTest("libfprint is too old for hotplug")
+
         self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.setusername',
                                       'net.reactivated.fprint.device.enroll'])
         self.device.Claim('(s)', 'testuser')
@@ -732,6 +779,11 @@ class FPrintdVirtualDeviceClaimedTest(FPrintdVirtualDeviceBaseTest):
             self.device.VerifyStart('(s)', 'any')
 
     def test_enroll_verify_list_delete(self):
+        # This test can trigger a race in older libfprint, only run if we have
+        # hotplug support, which coincides with the fixed release.
+        if not self._has_hotplug:
+            self.skipTest("libfprint is too old for hotplug")
+
         with self.assertFprintError('NoEnrolledPrints'):
             self.device.ListEnrolledFingers('(s)', 'testuser')
 
@@ -770,6 +822,7 @@ class FPrintdVirtualDeviceClaimedTest(FPrintdVirtualDeviceBaseTest):
         self.wait_for_result()
         self.assertTrue(self._verify_stopped)
         self.assertEqual(self._last_result, 'verify-match')
+        self.device.VerifyStop()
 
         self.assertEqual(self.device.ListEnrolledFingers('(s)', 'testuser'), ['right-index-finger'])
 
@@ -792,18 +845,29 @@ class FPrintdVirtualDeviceClaimedTest(FPrintdVirtualDeviceBaseTest):
         self.assertFalse(os.path.exists(os.path.join(self.state_dir, 'testuser/virtual_image/0/7')))
 
     def test_enroll_invalid_storage_dir(self):
-        if 'CI_PROJECT_NAME' in os.environ:
-            self.skipTest('Permissions aren\'t respected in CI environment')
+        # Directory wil not exist yet
         os.makedirs(self.state_dir, mode=0o500)
         self.addCleanup(os.chmod, self.state_dir, mode=0o700)
+
+        try:
+            os.open(os.path.join(self.state_dir, "testfile"), os.O_CREAT | os.O_WRONLY)
+            self.skipTest('Permissions aren\'t respected (CI environment?)')
+        except PermissionError:
+            pass
+
         self.enroll_image('whorl', expected_result='enroll-failed')
 
     def test_verify_invalid_storage_dir(self):
-        if 'CI_PROJECT_NAME' in os.environ:
-            self.skipTest('Permissions aren\'t respected in CI environment')
         self.enroll_image('whorl')
         os.chmod(self.state_dir, mode=0o000)
         self.addCleanup(os.chmod, self.state_dir, mode=0o700)
+
+        try:
+            os.open(os.path.join(self.state_dir, "testfile"), os.O_CREAT | os.O_WRONLY)
+            self.skipTest('Permissions aren\'t respected (CI environment?)')
+        except PermissionError:
+            pass
+
         with self.assertFprintError('NoEnrolledPrints'):
             self.device.VerifyStart('(s)', 'any')
 
@@ -938,15 +1002,11 @@ class FPrintdVirtualDeviceClaimedTest(FPrintdVirtualDeviceBaseTest):
         self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.enroll'])
         self.enroll_image('whorl')
 
-    def test_unallowed_enroll_stop(self):
+    def test_always_allowed_enroll_stop(self):
         self.device.EnrollStart('(s)', 'right-index-finger')
 
         self._polkitd_obj.SetAllowed([''])
 
-        with self.assertFprintError('PermissionDenied'):
-            self.device.EnrollStop()
-
-        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.enroll'])
         self.device.EnrollStop()
 
     def test_unallowed_verify_start(self):
@@ -955,15 +1015,11 @@ class FPrintdVirtualDeviceClaimedTest(FPrintdVirtualDeviceBaseTest):
         with self.assertFprintError('PermissionDenied'):
             self.device.VerifyStart('(s)', 'any')
 
-    def test_unallowed_verify_stop(self):
+    def test_always_allowed_verify_stop(self):
         self.enroll_image('whorl')
         self.device.VerifyStart('(s)', 'any')
 
         self._polkitd_obj.SetAllowed([''])
-        with self.assertFprintError('PermissionDenied'):
-            self.device.VerifyStop()
-
-        self._polkitd_obj.SetAllowed(['net.reactivated.fprint.device.verify'])
         self.device.VerifyStop()
 
     def test_list_enrolled_fingers_current_user(self):
@@ -1419,6 +1475,10 @@ class FPrintdUtilsTest(FPrintdVirtualDeviceBaseTest):
                 print('Testing JHBuild version')
                 jhbuild_prefix = os.environ['JHBUILD_PREFIX']
                 path = os.path.join(jhbuild_prefix, 'bin', util_bin)
+            else:
+                # Assume it is in path
+                utils[util] = util_bin
+                continue
 
             assert os.path.exists(path), 'failed to find {} in {}'.format(util, path)
             utils[util] = path
@@ -1480,4 +1540,12 @@ if __name__ == '__main__':
             print("%s %s" % (machine, human), end="\n")
         sys.exit(0)
 
-    unittest.main(verbosity=2)
+    prog = unittest.main(verbosity=2, exit=False)
+    if prog.result.errors or prog.result.failures:
+        sys.exit(1)
+
+    # Translate to skip error
+    if prog.result.testsRun == len(prog.result.skipped):
+        sys.exit(77)
+
+    sys.exit(0)

tests/output_checker.py

@@ -0,0 +1,148 @@
+#! /usr/bin/env python3
+# Copyright © 2020, RedHat Inc.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2.1 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library. If not, see <http://www.gnu.org/licenses/>.
+# Authors:
+#       Benjamin Berg <bberg@redhat.com>
+
+import os
+import sys
+import fcntl
+import io
+import re
+import time
+import threading
+
+class OutputChecker(object):
+
+    def __init__(self, out=sys.stdout):
+        self._output = out
+        self._pipe_fd_r, self._pipe_fd_w = os.pipe()
+        self._partial_buf = b''
+        self._lines_sem = threading.Semaphore()
+        self._lines = []
+        self._reader_io = io.StringIO()
+
+        # Just to be sure, shouldn't be a problem even if we didn't set it
+        fcntl.fcntl(self._pipe_fd_r, fcntl.F_SETFL,
+                    fcntl.fcntl(self._pipe_fd_r, fcntl.F_GETFL) | os.O_CLOEXEC)
+        fcntl.fcntl(self._pipe_fd_w, fcntl.F_SETFL,
+                    fcntl.fcntl(self._pipe_fd_w, fcntl.F_GETFL) | os.O_CLOEXEC)
+
+        # Start copier thread
+        self._thread = threading.Thread(target=self._copy)
+        self._thread.start()
+
+    def _copy(self):
+        while True:
+            r = os.read(self._pipe_fd_r, 1024)
+            if not r:
+                return
+
+            l = r.split(b'\n')
+            l[0] = self._partial_buf + l[0]
+            self._lines.extend(l[:-1])
+            self._partial_buf = l[-1]
+
+            self._lines_sem.release()
+
+            os.write(self._output.fileno(), r)
+
+    def check_line_re(self, needle_re, timeout=0, failmsg=None):
+        deadline = time.time() + timeout
+
+        if isinstance(needle_re, str):
+            needle_re = needle_re.encode('ascii')
+
+        r = re.compile(needle_re)
+        ret = []
+
+        while True:
+            try:
+                l = self._lines.pop(0)
+            except IndexError:
+                # Check if should wake up
+                if not self._lines_sem.acquire(timeout = deadline - time.time()):
+                    if failmsg:
+                        raise AssertionError(failmsg)
+                    else:
+                        raise AssertionError('Timed out waiting for needle %s (timeout: %0.2f)' % (str(needle_re), timeout))
+                continue
+
+            ret.append(l)
+            if r.search(l):
+                return ret
+
+    def check_line(self, needle, timeout=0, failmsg=None):
+        if isinstance(needle, str):
+            needle = needle.encode('ascii')
+
+        needle_re = re.escape(needle)
+
+        return self.check_line_re(needle_re, timeout=timeout, failmsg=failmsg)
+
+    def check_no_line_re(self, needle_re, wait=0, failmsg=None):
+        deadline = time.time() + wait
+
+        if isinstance(needle_re, str):
+            needle_re = needle_re.encode('ascii')
+
+        r = re.compile(needle_re)
+        ret = []
+
+        while True:
+            try:
+                l = self._lines.pop(0)
+            except IndexError:
+                # Check if should wake up
+                if not self._lines_sem.acquire(timeout = deadline - time.time()):
+                    # Timed out, so everything is good
+                    break
+                continue
+
+            ret.append(l)
+            if r.search(l):
+                if failmsg:
+                    raise AssertionError(failmsg)
+                else:
+                    raise AssertionError('Found needle %s but shouldn\'t have been there (timeout: %0.2f)' % (str(needle_re), timeout))
+
+        return ret
+
+    def check_no_line(self, needle, wait=0, failmsg=None):
+        if isinstance(needle, str):
+            needle = needle.encode('ascii')
+
+        needle_re = re.escape(needle)
+
+        return self.check_no_line_re(needle_re, wait=wait, failmsg=failmsg)
+
+    def clear(self):
+        ret = self._lines
+        self._lines = []
+        return ret
+
+    def assert_closed(self, timeout=1):
+        self._thread.join(timeout)
+        if self._thread.is_alive() != False:
+            raise AssertionError("OutputCheck: Write side has not been closed yet!")
+
+    @property
+    def fd(self):
+        return self._pipe_fd_w
+
+    def writer_attached(self):
+        os.close(self._pipe_fd_w)
+        self._pipe_fd_w = -1
+

tests/pam/meson.build

@@ -39,6 +39,7 @@ foreach t: tests
         {
             'name': t,
             'file': files(meson.current_source_dir() / t + '.py')[0],
+            'is_parallel': false,
             'env': [
                 'TOPBUILDDIR=' + meson.build_root(),
                 'TOPSRCDIR=' + meson.source_root(),

tests/pam/test_pam_fprintd.py

@@ -17,8 +17,9 @@ import sys
 import subprocess
 import dbus
 import dbusmock
-import fcntl
+import glob
 import os
+import shutil
 import time
 import pypamtest
 
@@ -68,12 +69,12 @@ class TestPamFprintd(dbusmock.DBusTestCase):
     def tearDownClass(klass):
         klass.stop_monitor()
 
+        # Remove pam wrapper files, as they may break other tests
+        [shutil.rmtree(i) for i in glob.glob('/tmp/pam.[0-9A-z]')]
+
     def setUp(self):
         (self.p_mock, self.obj_fprintd_manager) = self.spawn_server_template(
-            self.template_name, {}, stdout=subprocess.PIPE)
-        # set log to nonblocking
-        flags = fcntl.fcntl(self.p_mock.stdout, fcntl.F_GETFL)
-        fcntl.fcntl(self.p_mock.stdout, fcntl.F_SETFL, flags | os.O_NONBLOCK)
+            self.template_name, {})
         self.obj_fprintd_mock = dbus.Interface(self.obj_fprintd_manager, 'net.reactivated.Fprint.Manager.Mock')
 
     def tearDown(self):
@@ -138,6 +139,66 @@ class TestPamFprintd(dbusmock.DBusTestCase):
         self.assertRegex(res.info[0], r'Swipe your left little finger across the fingerprint reader')
         self.assertEqual(len(res.errors), 0)
 
+    def test_pam_fprintd_no_fingers(self):
+        self.setup_device()
+        self.device_mock.SetEnrolledFingers('toto', dbus.Array(set([]), signature='s'))
+        script = [
+            ( 'verify-match', True, 1 )
+        ]
+        self.device_mock.SetVerifyScript(script)
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_AUTHINFO_UNAVAIL)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+
+    def test_pam_fprintd_no_fingers_while_verifying(self):
+        self.setup_device()
+        script = [
+            ( 'MOCK: no-prints', True, 1),
+            ( 'verify-match', True, 1 )
+        ]
+        self.device_mock.SetVerifyScript(script)
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_USER_UNKNOWN)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+
+    def test_pam_fprintd_blocks_unexpected_auth(self):
+        self.setup_device()
+        script = [
+            ( 'verify-match', True, -500 ), # This one is sent before VerifyStart has completed
+            ( 'verify-no-match', True, 1 ),
+            ( 'verify-no-match', True, 1 ),
+            ( 'verify-no-match', True, 1 ),
+        ]
+        self.device_mock.SetVerifyScript(script)
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_MAXTRIES)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+
+        self.assertRegex(res.info[0], r'Swipe your left little finger across the fingerprint reader')
+        self.assertEqual(len(res.errors), 3)
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+
+    def test_pam_fprintd_blocks_unexpected_auth2(self):
+        self.setup_device()
+        script = [
+            ( 'verify-no-match', True, 1 ),
+            ( 'verify-match', True, -500 ), # This one is sent before VerifyStart has completed
+            ( 'verify-no-match', True, 1 ),
+            ( 'verify-no-match', True, 1 ),
+        ]
+        self.device_mock.SetVerifyScript(script)
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_MAXTRIES)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+
+        self.assertRegex(res.info[0], r'Swipe your left little finger across the fingerprint reader')
+        self.assertEqual(len(res.errors), 3)
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+        self.assertRegex(res.errors[0], r'Failed to match fingerprint')
+
     def test_pam_fprintd_dual_reader_auth(self):
         device_path = self.obj_fprintd_mock.AddDevice('FDO Sandpaper Reader', 3, 'press')
         sandpaper_device_mock = self.dbus_con.get_object('net.reactivated.Fprint', device_path)
@@ -191,6 +252,20 @@ class TestPamFprintd(dbusmock.DBusTestCase):
         self.assertRegex(res.errors[1], r'Failed to match fingerprint')
         self.assertRegex(res.errors[2], r'Failed to match fingerprint')
 
+    def test_pam_already_claimed(self):
+        self.setup_device()
+        script = [
+            ( 'verify-match', True, 2 )
+        ]
+        self.device_mock.SetVerifyScript(script)
+        self.device_mock.SetClaimed('toto')
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_AUTHINFO_UNAVAIL)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+
+        self.assertEqual(len(res.info), 0)
+        self.assertEqual(len(res.errors), 0)
+
     def test_pam_timeout(self):
         self.setup_device()
 
@@ -198,6 +273,19 @@ class TestPamFprintd(dbusmock.DBusTestCase):
         res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
         self.assertRegex(res.info[1], r'Verification timed out')
 
+    def test_pam_notices_fprintd_disappearing(self):
+        self.setup_device()
+
+        script = [
+            ( 'MOCK: quit', True, 0 ),
+        ]
+        self.device_mock.SetVerifyScript(script)
+
+        tc = pypamtest.TestCase(pypamtest.PAMTEST_AUTHENTICATE, expected_rv=PAM_AUTHINFO_UNAVAIL)
+        res = pypamtest.run_pamtest("toto", "fprintd-pam-test", [tc], [ 'unused' ])
+        self.assertEqual(len(res.errors), 0)
+        self.assertEqual(len(res.info), 0)
+
 if __name__ == '__main__':
     if 'PAM_WRAPPER_SERVICE_DIR' not in os.environ:
         print('Cannot run test without environment set correctly, run "meson test" instead')

tests/test_fprintd_utils.py

@@ -18,9 +18,9 @@ import subprocess
 import dbus
 import dbus.mainloop.glib
 import dbusmock
-import fcntl
 import os
 import time
+from output_checker import OutputChecker
 
 
 VALID_FINGER_NAMES = [
@@ -77,10 +77,8 @@ class TestFprintdUtilsBase(dbusmock.DBusTestCase):
     def setUp(self):
         super().setUp()
         (self.p_mock, self.obj_fprintd_manager) = self.spawn_server_template(
-            self.template_name, {}, stdout=subprocess.PIPE)
+            self.template_name, {})
         # set log to nonblocking
-        flags = fcntl.fcntl(self.p_mock.stdout, fcntl.F_GETFL)
-        fcntl.fcntl(self.p_mock.stdout, fcntl.F_SETFL, flags | os.O_NONBLOCK)
         self.obj_fprintd_mock = dbus.Interface(self.obj_fprintd_manager, 'net.reactivated.Fprint.Manager.Mock')
 
     def tearDown(self):
@@ -102,22 +100,21 @@ class TestFprintdUtilsBase(dbusmock.DBusTestCase):
 
     def start_utility_process(self, utility_name, args=[], sleep=True):
         utility = [ os.path.join(self.tools_prefix, 'fprintd-{}'.format(utility_name)) ]
+        output = OutputChecker()
         process = subprocess.Popen(self.wrapper_args + utility + args,
-                                   stdout=subprocess.PIPE,
-                                   stderr=subprocess.STDOUT,
-                                   universal_newlines=True)
-        flags = fcntl.fcntl(process.stdout, fcntl.F_GETFL)
-        fcntl.fcntl(process.stdout, fcntl.F_SETFL, flags | os.O_NONBLOCK)
+                                   stdout=output.fd,
+                                   stderr=subprocess.STDOUT)
+        output.writer_attached()
 
+        self.addCleanup(output.assert_closed)
         self.addCleanup(self.try_stop_utility_process, process)
 
         if sleep:
             time.sleep(self.sleep_time)
 
-        return process
+        return process, output
 
     def stop_utility_process(self, process):
-        print(process.stdout.read())
         process.terminate()
         process.wait()
 
@@ -127,17 +124,12 @@ class TestFprintdUtilsBase(dbusmock.DBusTestCase):
         except:
             pass
 
-    def get_process_output(self, process):
-        out = process.stdout.read()
-        self.addCleanup(print, out)
-        return out
-
     def run_utility_process(self, utility_name, args=[], sleep=True, timeout=None):
-        proc = self.start_utility_process(utility_name, args=args, sleep=sleep)
+        proc, output = self.start_utility_process(utility_name, args=args, sleep=sleep)
         ret = proc.wait(timeout=timeout if timeout is not None else self.sleep_time * 4)
         self.assertLessEqual(ret, 128)
 
-        return self.get_process_output(proc), ret
+        return b''.join(output.clear()), ret
 
 
 class TestFprintdUtils(TestFprintdUtilsBase):
@@ -146,65 +138,62 @@ class TestFprintdUtils(TestFprintdUtilsBase):
         self.setup_device()
 
     def test_fprintd_enroll(self):
-        process = self.start_utility_process('enroll', ['-f', 'right-index-finger', 'toto'])
+        process, out = self.start_utility_process('enroll', ['-f', 'right-index-finger', 'toto'])
 
-        out = self.get_process_output(process)
-        self.assertRegex(out, r'right-index-finger')
+        out.check_line(rb'right-index-finger', 0)
 
         self.device_mock.EmitEnrollStatus('enroll-completed', True)
-        time.sleep(self.sleep_time)
 
-        out = self.get_process_output(process)
-        self.assertRegex(out, 'Enroll result: enroll-completed')
+        out.check_line(rb'Enroll result: enroll-completed', self.sleep_time)
 
     def test_fprintd_list(self):
         # Rick has no fingerprints enrolled
         out, ret = self.run_utility_process('list', ['rick'])
-        self.assertRegex(out, r'has no fingers enrolled for')
+        self.assertRegex(out, rb'has no fingers enrolled for')
         self.assertEqual(ret, 0)
 
         # Toto does
         out, ret = self.run_utility_process('list', ['toto'])
-        self.assertRegex(out, r'right-little-finger')
+        self.assertRegex(out, rb'right-little-finger')
         self.assertEqual(ret, 0)
 
     def test_fprintd_delete(self):
         # Has fingerprints enrolled
         out, ret = self.run_utility_process('list', ['toto'])
-        self.assertRegex(out, r'left-little-finger')
+        self.assertRegex(out, rb'left-little-finger')
         self.assertEqual(ret, 0)
-        self.assertRegex(out, r'right-little-finger')
+        self.assertRegex(out, rb'right-little-finger')
 
         # Delete fingerprints
         out, ret = self.run_utility_process('delete', ['toto'])
-        self.assertRegex(out, r'Fingerprints deleted')
+        self.assertRegex(out, rb'Fingerprints deleted')
         self.assertEqual(ret, 0)
 
         # Doesn't have fingerprints
         out, ret = self.run_utility_process('list', ['toto'])
-        self.assertRegex(out, r'has no fingers enrolled for')
+        self.assertRegex(out, rb'has no fingers enrolled for')
         self.assertEqual(ret, 0)
 
 
 class TestFprintdUtilsNoDeviceTests(TestFprintdUtilsBase):
     def test_fprintd_enroll(self):
         out, ret = self.run_utility_process('enroll', ['toto'])
-        self.assertIn('No devices available', out)
+        self.assertIn(b'No devices available', out)
         self.assertEqual(ret, 1)
 
     def test_fprintd_list(self):
         out, ret = self.run_utility_process('list', ['toto'])
-        self.assertIn('No devices available', out)
+        self.assertIn(b'No devices available', out)
         self.assertEqual(ret, 1)
 
     def test_fprintd_delete(self):
         out, ret = self.run_utility_process('delete', ['toto'])
-        self.assertIn('No devices available', out)
+        self.assertIn(b'No devices available', out)
         self.assertEqual(ret, 1)
 
     def test_fprintd_verify(self):
         out, ret = self.run_utility_process('verify', ['toto'])
-        self.assertIn('No devices available', out)
+        self.assertIn(b'No devices available', out)
         self.assertEqual(ret, 1)
 
 
@@ -213,24 +202,20 @@ class TestFprintdUtilsVerify(TestFprintdUtilsBase):
         super().setUp()
         self.setup_device()
 
-    def start_verify_process(self, user='toto', finger=None, checkEnrolled=True):
+    def start_verify_process(self, user='toto', finger=None, nowait=False):
         args = [user]
         if finger:
             args += ['-f', finger]
 
-        self.process = self.start_utility_process('verify', args)
-        out = self.get_process_output(self.process)
+        self.process, self.output = self.start_utility_process('verify', args)
+        if nowait:
+            return
 
-        self.assertNotRegex(out, r'Device already in use by [A-z]+')
-        self.assertNotIn('Verify result:', out)
+        preamble = self.output.check_line(b'Verify started!')
 
-        if checkEnrolled and finger:
-            self.assertNotIn('''Finger '{}' not enrolled for user {}'''.format(
-                finger, user), out)
+        out = b''.join(preamble)
 
-        if checkEnrolled:
-            for f in self.enrolled_fingers:
-                self.assertIn(f, out)
+        self.assertNotIn(b'Verify result:', out)
 
         if finger:
             expected_finger = finger
@@ -239,9 +224,8 @@ class TestFprintdUtilsVerify(TestFprintdUtilsBase):
             self.assertEqual(self.device_mock.GetSelectedFinger(), expected_finger)
 
     def assertVerifyMatch(self, match):
-        self.assertIn('Verify result: {} (done)'.format(
-            'verify-match' if match else 'verify-no-match'),
-            self.get_process_output(self.process))
+        self.output.check_line(r'Verify result: {} (done)'.format(
+            'verify-match' if match else 'verify-no-match'))
 
     def test_fprintd_verify(self):
         self.start_verify_process()
@@ -280,14 +264,16 @@ class TestFprintdUtilsVerify(TestFprintdUtilsBase):
 
     def test_fprintd_verify_not_enrolled_fingers(self):
         for finger in [f for f in VALID_FINGER_NAMES if f not in self.enrolled_fingers]:
+            self.start_verify_process(finger=finger, nowait=True)
             regex = r'Finger \'{}\' not enrolled'.format(finger)
-            with self.assertRaisesRegex(AssertionError, regex):
-                self.start_verify_process(finger=finger)
+            self.output.check_line_re(regex, timeout=self.sleep_time)
+
             self.device_mock.Release()
 
     def test_fprintd_verify_no_enrolled_fingers(self):
         self.set_enrolled_fingers([])
-        self.start_verify_process()
+        self.start_verify_process(nowait=True)
+        self.output.check_line(b'No fingers enrolled for this device.', timeout=self.sleep_time)
         self.assertEqual(self.process.poll(), 1)
 
     def test_fprintd_list_all_fingers(self):
@@ -299,16 +285,17 @@ class TestFprintdUtilsVerify(TestFprintdUtilsBase):
             ( 'verify-match', True, 2 )
         ]
         self.device_mock.SetVerifyScript(script)
+        time.sleep(2)
 
         self.start_verify_process()
-        time.sleep(self.sleep_time * 4)
+        time.sleep(2 + self.sleep_time)
         self.assertVerifyMatch(True)
 
     def test_fprintd_multiple_verify_fails(self):
         self.start_verify_process()
 
-        with self.assertRaisesRegex(AssertionError, r'Device already in use'):
-            self.start_verify_process()
+        self.start_verify_process(nowait=True)
+        self.output.check_line_re(rb'Device already in use by [A-z]+', timeout=self.sleep_time)
 
 if __name__ == '__main__':
     # avoid writing to stderr

utils/verify.c

@@ -120,12 +120,18 @@ static void find_finger (FprintDBusDevice *dev, const char *username)
 	}
 }
 
+struct VerifyState {
+	GError *error;
+	gboolean started;
+	gboolean completed;
+};
+
 static void verify_result(GObject *object, const char *result, gboolean done, void *user_data)
 {
-	gboolean *verify_completed = user_data;
+	struct VerifyState *verify_state = user_data;
 	g_print("Verify result: %s (%s)\n", result, done ? "done" : "not done");
 	if (done != FALSE)
-		*verify_completed = TRUE;
+		verify_state->completed = TRUE;
 }
 
 static void verify_finger_selected(GObject *object, const char *name, void *user_data)
@@ -133,12 +139,27 @@ static void verify_finger_selected(GObject *object, const char *name, void *user
 	g_print("Verifying: %s\n", name);
 }
 
+static void verify_started_cb (GObject *obj,
+			       GAsyncResult *res,
+			       gpointer user_data)
+{
+	struct VerifyState *verify_state = user_data;
+
+	if (fprint_dbus_device_call_verify_start_finish (FPRINT_DBUS_DEVICE (obj), res, &verify_state->error))
+		verify_state->started = TRUE;
+}
+
 static void proxy_signal_cb (GDBusProxy *proxy,
 			     const gchar *sender_name,
 			     const gchar *signal_name,
 			     GVariant *parameters,
 			     gpointer user_data)
 {
+	struct VerifyState *verify_state = user_data;
+
+	if (!verify_state->started)
+		return;
+
 	if (g_str_equal (signal_name, "VerifyStatus")) {
 		const gchar *result;
 		gboolean done;
@@ -156,23 +177,44 @@ static void proxy_signal_cb (GDBusProxy *proxy,
 static void do_verify (FprintDBusDevice *dev)
 {
 	g_autoptr(GError) error = NULL;
-	gboolean verify_completed = FALSE;
+	struct VerifyState verify_state = { 0 };
+
+	/* This one is funny. We connect to the signal immediately to avoid
+	 * race conditions. However, we must ignore any authentication results
+	 * that happen before our start call returns.
+	 * This is because the verify call itself may internally try to verify
+	 * against fprintd (possibly using a separate account).
+	 *
+	 * To do so, we *must* use the async version of the verify call, as the
+	 * sync version would cause the signals to be queued and only processed
+	 * after it returns.
+	 */
 
 	g_signal_connect (dev, "g-signal", G_CALLBACK (proxy_signal_cb),
-			  &verify_completed);
+			  &verify_state);
 
-	if (!fprint_dbus_device_call_verify_start_sync (dev, finger_name, NULL,
-							&error)) {
-		g_print("VerifyStart failed: %s\n", error->message);
+	fprint_dbus_device_call_verify_start (dev, finger_name, NULL,
+					      verify_started_cb,
+					      &verify_state);
+
+	/* Wait for verify start while discarding any VerifyStatus signals */
+	while (!verify_state.started && !verify_state.error)
+		g_main_context_iteration(NULL, TRUE);
+
+	if (verify_state.error) {
+		g_print("VerifyStart failed: %s\n", verify_state.error->message);
+		g_clear_error (&verify_state.error);
 		exit (1);
 	}
+	g_print("Verify started!\n");
 
-	while (!verify_completed)
+	/* VerifyStatus signals are processing, wait for completion. */
+	while (!verify_state.completed)
 		g_main_context_iteration(NULL, TRUE);
 
 
 	g_signal_handlers_disconnect_by_func (dev, proxy_signal_cb,
-					      &verify_completed);
+					      &verify_state);
 
 	if (!fprint_dbus_device_call_verify_stop_sync (dev, NULL, &error)) {
 		g_print("VerifyStop failed: %s\n", error->message);