This reverts commit ecf6b7c323.
The idea of the commit was to make device failures less fatal to the
system. Unfortunately, we can fail quickly in this case, and returning
PAM_AUTH_ERR means that the user might run into a retry limit due to
this.
Go back to reporting PAM_AUTHINFO_UNAVAIL, it appears as the lesser evil
right now. Ideally we want to a way to tell the upper stack to retry
authentication whenever there is a good opportunity, but to not consider
it as an authentication failure.
If something under the hood failed with a generic device error we'd just
mark the PAM module not available, this is probably too much as it may
just be due to a device temporary error.
So make it stop but allow the loading system to retry with it
Loading saved prints may lead to an error if they were stored long time
ago and so they're using a wrong format.
In such case we list the prints as available even though they are really
not, so the PAM module won't return PAM_AUTHINFO_UNAVAIL as in the
no-prints case but PAM_USER_UNKNOWN.
This will lead some auth systems (such as gdm) to keep retrying using
PAM fprintd module, even if it's not really available.
It makes sense to allow interrupting fingerprint authentication, but PAM
does not provide a way to define an interruptable operation.
We can work around this somewhat though by at least reacting to SIGINT
in an interactive terminal. Obviously, we shouldn't override the signal
handler, because that would be too intrusive. But creating a signalfd is
easy enough and doesn't affect the rest of the process state as much.
When multiple devices are available PAM module will just pick the first
one, even if it has not enrolled fingers.
Since this can't be user configured (yet) we can be a bit smarter and
select the device that has more fingerprints configured for the user.
We must ignore NameOwnerChanged that happen due to automatic startup.
The easy way to do so is to just register it only when we get to the
point that a name owner change has security implications.
While add it, change it to always log at a warning level.
Fixes: #94
Implement simple auto-pointers for the types we use in pam_fprintd with
a basic implementation based on GLib one so that we can have the same
features without having neither an header-dependency on it.
If fprintd disappears or is replaced, then we might be getting signals
from another daemon/verifcation session.
As such we must give up at that point.
Related: #47
In case fprintd is emitting a verify signal for another request that is
still going on while we're about to start a new verification, we'd just
accept such signal, so potentially allowing a log-in because another
concurrent request succeeded.
To avoid this, use async call to VerifyStart and open a verify window
(during which we accept the verification related signals) that is kept
open just once the VerifyStart call has been completed and before
stopping the verification again. As that's the only moment in which we
can be sure that we've control of the daemon events for such device.
Thanks to Benjamin to find out the race.
Fixes: #47
The data->result was free'ed both in the loop (before breaking) and
afterwards. As the first case did not set the pointer to NULL, this
could result in a double free.
Fix this by simply removing the free that is in the loop and relying on
the cleanup later on.
And avoid treating "libdir" as an absolute path, the documentation
clearly states that it is "relative to the prefix".
Based on patch by Timothy Gu <timothygu99@gmail.com>
We need to make sure that the max_tries variable isn't decremented
further when we have success in the verification loop. Add missing break
to do that.
Fixes: affffaf134Closes: #40
Remove "nodelete" linker flag now that we use sd-bus and not dbus-glib,
so that libraries that pam_fprintd links to can be unloaded.
This was added because GLib's type system expects to be initialised
once and only once per process, and re-loading this type system when it
had already been initialised caused crashes.
This pam plugin never used GDBus because it transparently uses threads
which do not work well with a lot of PAM applications. But even settling
on the "still better to use than plain dbus library" dbus-glib wasn't
without problems, as any use or initialisation of GIO sockets would
modify signal handler for signals such as SIGPIPE (see gio/gsocket.c).
Many years later, sd-bus is a more modern alternative to the bare dbus
library with a better API.
This includes:
- Removing use of gboolean, guint, g_new0() and many glib string helpers
- Simplifying debug logging
- Marking user-facing messages to be translated
