0.9.0

fprintd's API docs say that "retry" errors for verification
"the verification is still ongoing" and that "[the] user should retry
scanning their finger.

Unfortunately, retry errors are fatal in libfprint. Make fprintd restart
operations when "retry" is the error for either identification or
verification purposes.

We need to also make sure that a "*Stop" D-Bus call will return as
normal if called while we're stopping a verification or identification
in order to restart it.

Closes: #22

.gitlab-ci.yml

@@ -28,6 +28,8 @@ build_dev:
     - ninja -C _build
     - ninja -C _build install
     - cd ..
+    # So we don't get error about this libfprint file
+    - echo "libfprint/demo/gtk-libfprint-test.ui" >> po/POTFILES.skip
   script:
     - ./autogen.sh --disable-dependency-tracking
     - make

NEWS

@@ -1,6 +1,14 @@
 This file lists notable changes in each release. For the full history of all
 changes, see ChangeLog.
 
+version 0.9.0:
+- Fix hangs when there the verification error was "retry"
+- Update for fp_get_pollfds() changes
+- Fix "client_username" memory leak, fix memory leak when saving a file
+- Create the fingerprint storage directory at install time,
+  the storage path is now hard-coded as /var/lib/fprint and created by
+  systemd when the service is started
+
 version 0.8.1:
 - Fix build when builddir != srcdir
 - Fix possible crash on exit

README.transifex

@@ -4,7 +4,7 @@ Transifex.net Token Verification
 The list of tokens bellow guarantee the respective users to be able to enable 
 submission on components using the following repository url:
 
-ssh://git.freedesktop.org/git/libfprint/fprintd
+https://gitlab.freedesktop.org/libfprint/fprintd/
 
 Tokens:
 

autogen.sh

@@ -3,7 +3,8 @@
 srcdir=`dirname $0`
 test -z "$srcdir" && srcdir=.
 
-pushd $srcdir
+olddir=`pwd`
+cd "$srcdir"
 
 aclocal || exit 1
 autoheader || exit 1
@@ -13,7 +14,8 @@ intltoolize -c -f || exit 1
 libtoolize -c || exit 1
 autoconf || exit 1
 automake -a -c || exit 1
-popd
+
+cd "$olddir"
 
 if test -z "$NOCONFIGURE"; then
 	$srcdir/configure --enable-maintainer-mode $*

code-of-conduct.md

@@ -0,0 +1,3 @@
+This project and its community follow the [Freedesktop.org code of conduct]
+
+[Freedesktop.org code of conduct]: https://www.freedesktop.org/wiki/CodeOfConduct/

configure.ac

@@ -1,4 +1,4 @@
-AC_INIT([fprintd], [0.8.1])
+AC_INIT([fprintd], [0.9.0])
 AM_INIT_AUTOMAKE([1.11 dist-xz no-dist-gzip check-news])
 AC_CONFIG_SRCDIR([src/main.c])
 AC_CONFIG_HEADERS([config.h])
@@ -60,6 +60,8 @@ AC_ARG_WITH([systemdsystemunitdir],
             [],
             [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
 AS_IF([test "x$with_systemdsystemunitdir" != "xno"], [
+  # StateDirectory was introduced in systemd 235
+  PKG_CHECK_MODULES(SYSTEMD, systemd >= 235)
   AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
 ])
 AM_CONDITIONAL(HAVE_SYSTEMD, [test -n "$systemdsystemunitdir"])
@@ -70,7 +72,8 @@ DBUS_SERVICES_DIR="$DATADIR/dbus-1/services"
 AC_SUBST(DBUS_SERVICES_DIR)
 AC_DEFINE_UNQUOTED(DBUS_SERVICES_DIR, "$DBUS_SERVICES_DIR", [Where services dir for DBUS is])
 
-AC_DEFINE_UNQUOTED(SYSCONFDIR, "$sysconfdir", [Where the configuration file will be located])
+AS_AC_EXPAND(SYSCONFDIR, $sysconfdir)
+AC_DEFINE_UNQUOTED(SYSCONFDIR, "$SYSCONFDIR", [Where the configuration file will be located])
 
 GNOME_COMPILE_WARNINGS
 

data/Makefile.am

@@ -15,7 +15,7 @@ if HAVE_SYSTEMD
 systemdservicedir       = $(systemdsystemunitdir)
 systemdservice_DATA     = $(systemdservice_in_files:.service.in=.service)
 $(systemdservice_DATA): $(systemdservice_in_files) Makefile
-	@sed -e "s|\@libexecdir\@|$(libexecdir)|" -e "s|\@localstatedir\@|$(localstatedir)|" $< > $@
+	@sed -e "s|\@libexecdir\@|$(libexecdir)|" $< > $@
 endif
 
 polkitdir = $(datadir)/polkit-1/actions

data/fprintd.service.in

@@ -11,7 +11,8 @@ ExecStart=@libexecdir@/fprintd
 ProtectSystem=strict
 ProtectKernelTunables=true
 ProtectControlGroups=true
-ReadWritePaths=@localstatedir@/lib/fprint
+# This always corresponds to /var/lib/fprint
+StateDirectory=fprint
 ProtectHome=true
 PrivateTmp=true
 

pam/pam_fprintd.c

@@ -356,7 +356,7 @@ static int do_verify(GMainLoop *loop, pam_handle_t *pamh, DBusGProxy *dev, gbool
 				g_free (data->result);
 				break;
 			} else {
-				send_info_msg (data->pamh, "An unknown error occured");
+				send_info_msg (data->pamh, "An unknown error occurred");
 				ret = PAM_AUTH_ERR;
 				g_free (data->result);
 				break;
@@ -476,8 +476,13 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, int flags, int argc,
 					   G_TYPE_NONE, G_TYPE_STRING, G_TYPE_BOOLEAN, G_TYPE_INVALID);
 
 	pam_get_item(pamh, PAM_RHOST, (const void **)(const void*) &rhost);
-	if (rhost != NULL && strlen(rhost) > 0) {
-		/* remote login (e.g. over SSH) */
+
+	/* NULL or empty rhost if the host information is not available or set.
+	 * "localhost" if the host is local.
+	 * We want to not run for known remote hosts */
+	if (rhost != NULL &&
+	    rhost != '\0' &&
+	    strcmp (rhost, "localhost") != 0) {
 		return PAM_AUTHINFO_UNAVAIL;
 	}
 

src/Makefile.am

@@ -13,8 +13,7 @@ AM_CFLAGS =						\
 	$(DAEMON_CFLAGS)				\
 	-DG_LOG_DOMAIN=\""fprintd"\"			\
 	-DLOCALEDIR=\""$(datadir)/locale"\"		\
-	-DPLUGINDIR=\""$(libdir)/fprintd/modules"\"	\
-	-DFILE_STORAGE_PATH=\""$(localstatedir)/lib/fprint"\"
+	-DPLUGINDIR=\""$(libdir)/fprintd/modules"\"
 
 libfprintd_la_SOURCES =				\
 	manager.c device.c			\
@@ -50,3 +49,7 @@ fprintd-marshal.h: fprintd-marshal.list
 fprintd-marshal.c: fprintd-marshal.h
 	( $(GLIB_GENMARSHAL) --prefix=fprintd_marshal $(srcdir)/fprintd-marshal.list --body --header > fprintd-marshal.c )
 
+install-data-hook:
+	if test -w $(DESTDIR)$(prefix)/; then \
+		mkdir -p $(DESTDIR)$(localstatedir)/lib/fprint; \
+	fi

src/device.c

@@ -469,7 +469,6 @@ _fprint_device_check_for_username (FprintDevice *rdev,
 	char *sender;
 	unsigned long uid;
 	struct passwd *user;
-	char *client_username;
 
 	/* Get details about the current sender, and username/uid */
 	conn = dbus_g_connection_get_connection (fprintd_dbus_conn);
@@ -490,17 +489,16 @@ _fprint_device_check_for_username (FprintDevice *rdev,
 			    "Failed to get information about user UID %lu", uid);
 		return NULL;
 	}
-	client_username = g_strdup (user->pw_name);
 
 	/* The current user is usually allowed to access their
 	 * own data, this should be followed by PolicyKit checks
 	 * anyway */
-	if (username == NULL || *username == '\0' || g_str_equal (username, client_username)) {
+	if (username == NULL || *username == '\0' || g_str_equal (username, user->pw_name)) {
 		if (ret_sender != NULL)
 			*ret_sender = sender;
 		else
 			g_free (sender);
-		return client_username;
+		return g_strdup (user->pw_name);
 	}
 
 	/* If we're not allowed to set a different username,
@@ -751,8 +749,30 @@ static void fprint_device_release(FprintDevice *rdev,
 	}
 }
 
-static void verify_cb(struct fp_dev *dev, int r, struct fp_img *img,
-		      void *user_data)
+static void verify_cb(struct fp_dev *dev,
+		      int            r,
+		      struct fp_img *img,
+		      void          *user_data);
+
+static void
+verify_restart_cb(struct fp_dev *dev,
+		  void          *user_data)
+{
+	struct FprintDevice *rdev = user_data;
+	FprintDevicePrivate *priv = DEVICE_GET_PRIVATE(rdev);
+
+	if (priv->current_action != ACTION_VERIFY)
+		return;
+
+	g_debug("verify_restart_cb: restarting verification");
+	fp_async_verify_start(priv->dev, priv->verify_data, verify_cb, rdev);
+}
+
+static void
+verify_cb(struct fp_dev *dev,
+	  int            r,
+	  struct fp_img *img,
+	  void          *user_data)
 {
 	struct FprintDevice *rdev = user_data;
 	FprintDevicePrivate *priv = DEVICE_GET_PRIVATE(rdev);
@@ -763,8 +783,20 @@ static void verify_cb(struct fp_dev *dev, int r, struct fp_img *img,
 
 	g_debug("verify_cb: result %s (%d)", name, r);
 
-	if (r == FP_VERIFY_NO_MATCH || r == FP_VERIFY_MATCH || r < 0)
-		priv->action_done = TRUE;
+	if (r == FP_VERIFY_RETRY ||
+	    r == FP_VERIFY_RETRY_TOO_SHORT ||
+	    r == FP_VERIFY_RETRY_CENTER_FINGER ||
+	    r == FP_VERIFY_RETRY_REMOVE_FINGER) {
+		g_debug ("verify_cb: stopping current verification to retry");
+		fp_img_free(img);
+		if (fp_async_verify_stop(priv->dev, verify_restart_cb, user_data) == 0) {
+			g_signal_emit(rdev, signals[SIGNAL_VERIFY_STATUS], 0, name, priv->action_done);
+			return;
+		}
+		/* fallthrough to error out if restarting failed */
+	}
+
+	priv->action_done = TRUE;
 	set_disconnected (priv, name);
 	g_signal_emit(rdev, signals[SIGNAL_VERIFY_STATUS], 0, name, priv->action_done);
 	fp_img_free(img);
@@ -775,8 +807,32 @@ static void verify_cb(struct fp_dev *dev, int r, struct fp_img *img,
 	}
 }
 
-static void identify_cb(struct fp_dev *dev, int r,
-			 size_t match_offset, struct fp_img *img, void *user_data)
+static void identify_cb(struct fp_dev *dev,
+			int            r,
+			size_t         match_offset,
+			struct fp_img *img,
+			void          *user_data);
+
+static void
+identify_restart_cb(struct fp_dev *dev,
+		    void          *user_data)
+{
+	struct FprintDevice *rdev = user_data;
+	FprintDevicePrivate *priv = DEVICE_GET_PRIVATE(rdev);
+
+	if (priv->current_action != ACTION_IDENTIFY)
+		return;
+
+	g_debug("identify_restart_cb: restarting identification");
+	fp_async_identify_start (priv->dev, priv->identify_data, identify_cb, rdev);
+}
+
+static void
+identify_cb(struct fp_dev *dev,
+	    int            r,
+	    size_t         match_offset,
+	    struct fp_img *img,
+	    void          *user_data)
 {
 	struct FprintDevice *rdev = user_data;
 	FprintDevicePrivate *priv = DEVICE_GET_PRIVATE(rdev);
@@ -787,8 +843,20 @@ static void identify_cb(struct fp_dev *dev, int r,
 
 	g_debug("identify_cb: result %s (%d)", name, r);
 
-	if (r == FP_VERIFY_NO_MATCH || r == FP_VERIFY_MATCH || r < 0)
-		priv->action_done = TRUE;
+	if (r == FP_VERIFY_RETRY ||
+	    r == FP_VERIFY_RETRY_TOO_SHORT ||
+	    r == FP_VERIFY_RETRY_CENTER_FINGER ||
+	    r == FP_VERIFY_RETRY_REMOVE_FINGER) {
+		g_debug ("identify_cb: stopping current identification to retry");
+		fp_img_free(img);
+		if (fp_async_identify_stop(priv->dev, identify_restart_cb, user_data) == 0) {
+			g_signal_emit(rdev, signals[SIGNAL_VERIFY_STATUS], 0, name, priv->action_done);
+			return;
+		}
+		/* fallthrough to error out if restarting failed */
+	}
+
+	priv->action_done = TRUE;
 	set_disconnected (priv, name);
 	g_signal_emit(rdev, signals[SIGNAL_VERIFY_STATUS], 0, name, priv->action_done);
 	fp_img_free(img);
@@ -992,6 +1060,13 @@ static void fprint_device_verify_stop(FprintDevice *rdev,
 		return;
 	}
 
+	if (r == -EINPROGRESS) {
+		g_debug ("%s stop already in progress",
+			 priv->current_action == ACTION_VERIFY ? "verification" : "identification");
+		dbus_g_method_return(context);
+		r = 0;
+	}
+
 	if (r < 0) {
 		g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INTERNAL,
 			"Verify stop failed with error %d", r);

src/file_storage.c

@@ -39,6 +39,7 @@
 
 #include "file_storage.h"
 
+#define FILE_STORAGE_PATH "/var/lib/fprint"
 #define DIR_PERMS 0700
 
 #define FP_FINGER_IS_VALID(finger) \
@@ -82,12 +83,9 @@ static char *get_path_to_print_dscv(struct fp_dscv_dev *dev, enum fp_finger fing
 		fp_dscv_dev_get_devtype(dev), finger, base_store);
 }
 
-static int file_storage_get_basestore_for_username(const char *username, char **base_store)
+static char *file_storage_get_basestore_for_username(const char *username)
 {
-	char *dirpath = FILE_STORAGE_PATH;
-
-	*base_store = g_build_filename(dirpath, username, NULL);
-	return 0;
+	return g_build_filename(FILE_STORAGE_PATH, username, NULL);
 }
 
 /* if username == NULL function will use current username */
@@ -95,16 +93,13 @@ int file_storage_print_data_save(struct fp_print_data *data,
 	enum fp_finger finger, const char *username)
 {
 	GError *err = NULL;
-	char *path, *dirpath, *buf;
+	char *path, *dirpath;
 	size_t len;
 	int r;
 	char *base_store = NULL;
+	char *buf = NULL;
 
-	r = file_storage_get_basestore_for_username(username, &base_store);
-
-	if (r < 0) {
-		return r;
-	}
+	base_store = file_storage_get_basestore_for_username(username);
 
 	len = fp_print_data_get_data(data, (guchar **) &buf);
 	if (!len) {
@@ -116,26 +111,30 @@ int file_storage_print_data_save(struct fp_print_data *data,
 	dirpath = g_path_get_dirname(path);
 	r = g_mkdir_with_parents(dirpath, DIR_PERMS);
 	if (r < 0) {
-		g_free(base_store);
-		g_free(path);
+		g_debug("file_storage_print_data_save(): could not mkdir(\"%s\"): %s",
+			dirpath, g_strerror(r));
 		g_free(dirpath);
-		return r;
+		g_free(path);
+		goto out;
 	}
+	g_free(dirpath);
 
 	//fp_dbg("saving to %s", path);
 	g_file_set_contents(path, buf, len, &err);
-	free(buf);
-	g_free(dirpath);
 	g_free(path);
 	if (err) {
 		r = err->code;
-		//fp_err("save failed: %s", err->message);
+		g_debug("file_storage_print_data_save(): could not save '%s': %s",
+			path, err->message);
 		g_error_free(err);
 		/* FIXME interpret error codes */
-		return r;
+		goto out;
 	}
 
-	return 0;
+out:
+	g_clear_pointer(&buf, free);
+	g_clear_pointer(&base_store, g_free);
+	return r;
 }
 
 static int load_from_file(char *path, struct fp_print_data **data)
@@ -173,14 +172,12 @@ int file_storage_print_data_load(struct fp_dev *dev,
 	int r;
 	char *base_store = NULL;
 
-	r = file_storage_get_basestore_for_username(username, &base_store);
-
-	if (r < 0) {
-		return r;
-	}
+	base_store = file_storage_get_basestore_for_username(username);
 
 	path = get_path_to_print(dev, finger, base_store);
 	r = load_from_file(path, &fdata);
+	g_debug ("file_storage_print_data_load(): loaded '%s' %s",
+		 path, g_strerror(r));
 	g_free(path);
 	g_free(base_store);
 	if (r)
@@ -199,17 +196,14 @@ int file_storage_print_data_delete(struct fp_dscv_dev *dev,
 	enum fp_finger finger, const char *username)
 {
 	int r;
-	char *base_store;
+	char *base_store, *path;
 
-	r = file_storage_get_basestore_for_username(username, &base_store);
-
-	if (r < 0) {
-		return r;
-	}
-
-	gchar *path = get_path_to_print_dscv(dev, finger, base_store);
+	base_store = file_storage_get_basestore_for_username(username);
+	path = get_path_to_print_dscv(dev, finger, base_store);
 
 	r = g_unlink(path);
+	g_debug("file_storage_print_data_delete(): unlink(\"%s\") %s",
+		path, g_strerror(r));
 	g_free(path);
 	g_free(base_store);
 
@@ -225,7 +219,7 @@ static GSList *scan_dev_storedir(char *devpath, uint16_t driver_id,
 
 	GDir *dir = g_dir_open(devpath, 0, &err);
 	if (!dir) {
-		//fp_err("opendir %s failed: %s", devpath, err->message);
+		g_debug("scan_dev_storedir(): opendir(\"%s\") failed: %s", devpath, err->message);
 		g_error_free(err);
 		return list;
 	}
@@ -240,7 +234,7 @@ static GSList *scan_dev_storedir(char *devpath, uint16_t driver_id,
 
 		val = g_ascii_strtoull(ent, &endptr, 16);
 		if (endptr == ent || !FP_FINGER_IS_VALID(val)) {
-			//fp_dbg("skipping print file %s", ent);
+			g_debug("scan_dev_storedir(): skipping print file '%s'", ent);
 			continue;
 		}
 
@@ -256,18 +250,15 @@ GSList *file_storage_discover_prints(struct fp_dscv_dev *dev, const char *userna
 	GSList *list = NULL;
 	char *base_store = NULL;
 	char *storedir = NULL;
-	int r;
 
-	
-	r = file_storage_get_basestore_for_username(username, &base_store);
-
-	if (r < 0) {
-		return NULL;
-	}
+	base_store = file_storage_get_basestore_for_username(username);
 
 	storedir = get_path_to_storedir(fp_driver_get_driver_id(fp_dscv_dev_get_driver(dev)), 
 		fp_dscv_dev_get_devtype(dev), base_store);
 
+	g_debug ("file_storage_discover_prints() for user '%s' in '%s'",
+		 username, storedir);
+
 	list = scan_dev_storedir(storedir, fp_driver_get_driver_id(fp_dscv_dev_get_driver(dev)), 
 		fp_dscv_dev_get_devtype(dev), list);
 

src/loop.c

@@ -168,7 +168,7 @@ static void pollfd_removed_cb(int fd)
 
 int setup_pollfds(void)
 {
-	size_t numfds;
+	ssize_t numfds;
 	size_t i;
 	struct fp_pollfd *fpfds;
 	GSource *gsource;

src/main.c

@@ -87,8 +87,9 @@ load_conf (void)
 
 	filename = g_build_filename (SYSCONFDIR, "fprintd.conf", NULL);
 	file = g_key_file_new ();
+	g_debug("About to load configuration file '%s'", filename);
 	if (!g_key_file_load_from_file (file, filename, G_KEY_FILE_NONE, &error)) {
-		g_print ("Could not open fprintd.conf: %s\n", error->message);
+		g_warning ("Could not open \"%s\": %s\n", filename, error->message);
 		goto bail;
 	}
 
@@ -148,7 +149,7 @@ int main(int argc, char **argv)
 #endif
 
 	if (g_option_context_parse (context, &argc, &argv, &error) == FALSE) {
-		g_print ("couldn't parse command-line options: %s\n", error->message);
+		g_warning ("couldn't parse command-line options: %s\n", error->message);
 		g_error_free (error);
 		return 1;
 	}
@@ -199,7 +200,7 @@ int main(int argc, char **argv)
 
 	r = setup_pollfds();
 	if (r < 0) {
-		g_print("pollfd setup failed\n");
+		g_warning("pollfd setup failed\n");
 		goto err;
 	}