The correct way to unexport the object again is to unexported it on the
manager rather than on the interface skeleton. This fixes notifications
about device removal on DBus.
Add a dispose function to disconnect the authorization callback and
remove all clients (i.e. unwatch their bus names) before destroying the
hash table.
If a device is unplugged/destroyed while a client is using it, then we
would still end up watching the name. The vanish notification will then
access the destroyed FprintDevice object.
Fix this by unwatching the bus name when removing the client entry from
the dictionary.
The tests cannot currently parse the logs of fprintd. This means we need
to rely on fprintd aborting when a condition is hit that needs to be
tested.
This makes certain possible races when clients vanish testable.
Add a scheme that allows getting and referencing the current session
data while also adding a reference at the same time. This allows getting
the session and using the constant attributes from outside the main
thread without worrying about it being destroyed.
Implement the getter/setter in a safe way by marking the pointer as
invalid while we get the reference.
We already check the claimed state in advance during authorization. This
makes sense, as we can avoid authorization if the API has been used
incorrectly. However, as the mainloop is running and handling other
request the claimed state might change at any point until the method
handler is actually running.
As such, check the claimed state again in each method. Doing so fixes
the possible race condition.
Given that mk_genenum already parses FprintError, add the nick metadata
to the errors so that it matches the wanted DBus error and automatically
generate the errors list.
In this way we'll have to only touch one definition to get everything
updated
GDBus generated interface skeletons support natively an authorization
method that allows to filter calls before we get into the method
callback and that gets called into the call thread, before we go back
to main thread.
As per this, we can move all the polkit and other authorization checks
into this callback so that method handlers are now just assuming they're
the right to perform the requested operation.
As per the fact we'll share some data between another thread and the
callbacks, we will need to introduce some locking mechanism to ensure
safe data access.
This might be reduced by moving the claiming checks back to the method,
but would lead errors to be handled in different ordering, and so the
user to be requested for a password, and then - in case fail.
This can still happen now, but only if there are concurrent requests.
We now can get an invocation-owned sender at any moment with GDBus, so
there's no point of getting it as optional return-out value from the
username check function.
This is not used right now in all its full possibilities, but will make
devices hotplug support easier to implement and handle at client-side
level.
As per this we can stop doing the manual tracking of the devices.
Fprintd is dependent on the deprecated dbus-glib, also this doesn't provide
various features we can take advantage of, like the ones for async
authentication mechanism.
So, remove all the dbus-glib dependencies and simplify the code, but without
any further refactor, and keeping everything as it used to work, while this
will give room for further improvements in subsequent commits.
Internally, we just use dbus-codegen to generate the skeletons, and we
use the generated FprintdDBusManager with composition, while we
implement the device skeleton interface in FprintDevice, so that we
don't have to use it as a proxy, and keep being closer to what it used
to be with dbus-glib.
Fixes: #61
libfprint 1.90.1 supports early match result report which allows to inform
the clients as soon as the driver detected a match/no-match without having
to wait the whole device driver deactivation.
Use this feature in fprintd, by emitting the "VerifyStatus" signal as soon
as we've an usable result we can inform the client about.
As per this, ignore any possible error we may get afterwards, due to device
issues or late user cancellation, as the point of the verification action
(i.e. getting a match/no-match) can be considered accomplished.
Fixes https://gitlab.freedesktop.org/libfprint/fprintd/issues/35
If a client vanishes while we are opening or closing then fprintd would
uncoditionally try to close the device immediately. This should not be
done, it instead needs to wait for the open/close to complete.
Add open/close to the current action enum and keep track of the fact
that they are ongoing. Also check, whether the device has been closed in
the meantime (after the nested mainloop is done).
Fixes: #65
We were saving the state of a disconnected device because we used a
workaround to figure it out, but now libfprint would provide us a proper
GError in such case, and so we'd handle it without the need of saving the
state, given it's never used anyways.
As per systemd's documentation:
If multiple directories are set, then in the environment variable the
paths are concatenated with colon (":").
Handle that case by splitting the paths if there's a ":" in the
variable.
Closes: #50
When starting an enroll when verification is in progress (and vice-versa) we
emit an AlreadyInUse error, however when calling VerifyStop() during an
enrollment (and vice-versa) we just return a NoActionInProgress error, which
is not the case.
So let's be consistent and change the error type.
If a device is currently verifying, identifying or enrolling we may want the
user to stop the operation before we actually release the device.
Otherwise we may end-up in trying to close (failing) the internal device,
while fprintd is still considering the device active, causing a dead-lock
(the device can't be released, but neither claimed again or stop the current
action).
In fact calling Claim() -> EnrollStart() -> Release(), we would fail with
the error
net.reactivated.Fprint.Error.Internal:
Release failed with error: The device is still busy with another
operation, please try again later. (36)"
However, if we try to call VerifyStop, after this error, we'd fail because
for the fprintd logic, the device is not claimed anymore, but actually
closed, and we'd need to claim it again, but... That would still cause an
internal error.
To avoid this, in case Relase() is called cancel the ongoing operation,
and wait until it's done before completing the release call.
When starting an identify operation we allocate a gallery of prints from the
gallery, although if we match one of them we get that back in the finish
callback but with a further reference added.
So, in order to clean it up, use an auto-pointer or we'd end up in leaking
it, and the address sanitizer was catching this in our tests already:
Indirect leak of 12020 byte(s) in 5 object(s) allocated from:
#0 0x7fe8bc638ce6 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0x10dce6)
#1 0x7fe8bc37ffd0 in g_malloc0 ../../glib/glib/gmem.c:132
#2 0x55d100635c01 in load_from_file ../src/file_storage.c:159
#3 0x55d100635c01 in file_storage_print_data_load ../src/file_storage.c:182
#4 0x55d10063e950 in fprint_device_verify_start ../src/device.c:882
#5 0x55d10064036b in dbus_glib_marshal_fprint_device_VOID__STRING_POINTER src/device-dbus-glue.h:96
#6 0x7fe8bc50f6f5 (/usr/lib/x86_64-linux-gnu/libdbus-glib-1.so.2+0xd6f5)
When using the delete method we check if the device was claimed, if this
fails because the device is already in use we return an error, but we don't
free the user.
While this could be fixed by just a further g_free call, let's just remove
remove the other manual free calls, and use an auto-pointer instead for this
function.
During delete enrolled fingers2 call, if the check-claimed control fails, we
would return the error without freeing it.
While this could be fixed by just a further g_error_free call, let's just
remove the other manual free call, and use an auto-pointer instead for this
function.
During delete enrolled fingers call, if the check-claimed control fails, and
we get an error different from FPRINT_ERROR_CLAIM_DEVICE, we would return
the error without freeing it.
While this could be fixed by just a further g_error_free call, let's just
remove all the manual free calls, and use an auto-pointer instead for this
function.
In case of early return we may not free them consistently, while this is not
a big problem in a main function, is better to have a cleaner management,
and we did get valgrind reports.
When coverage is enabled fprintd test won't generate any .gcda file and so
apparently no data, this happens because gcov doesn't handle properly the
process termination when SIGTERM is used, and so when in fprintd.py we
terminate the process no coverage data is reported.
To avoid this, quit the main loop cleanly on SIGTERM, so that we will exit
from the main function cleanly, making libc to perform a gcov flush when we
exit the program.
There's no need to declare it as extern in the header as it is already
declared in the source files where it's used.
Fixes:
../src/device.c:51:25: error: redundant redeclaration of ‘fprintd_dbus_conn’ [-Werror=redundant-decls]
51 | extern DBusGConnection *fprintd_dbus_conn;
| ^~~~~~~~~~~~~~~~~
In file included from ../src/device.c:34:
../src/fprintd.h:29:25: note: previous declaration of ‘fprintd_dbus_conn’ was here
29 | extern DBusGConnection *fprintd_dbus_conn;
| ^~~~~~~~~~~~~~~~~
With the stronger warnings enabled when building with meson, we get a
warning for all the fingers definitions:
../src/device.c:38:24: warning: initialization discards ‘const’ qualifier
from pointer target type [-Wdiscarded-qualifiers]
38 | [FP_FINGER_UNKNOWN] = "unknown",
As the `fingers` array name was shadowed in another file:
../src/device.c:1000:11: warning: declaration of ‘fingers’ shadows a
global declaration [-Wshadow]
1000 | GSList *fingers, *finger;
Use the device session data to store all the informations we care about
while a device is claimed, and make its cleanup easier.
Keep just one instance of the current context, given we use it only during
claim and release, and those are mutually exclusive operations.
