PrivateNetwork=true is a bad idea, as it means that udev events cannot
be delivered. Remove it, we already restrict the address families
sufficiently anyway.
Closes: #119
This was never really used and it's breaking meson 60.
While this may just become a warning in 60.1, it's just better to avoid
using it.
See: https://github.com/mesonbuild/meson/issues/9441
If the user specified "any" finger, then we would mirror this back even
if there is only one finger available. Change it so that we act as if
that finger was passed explicitly, meaning we use the "verify" method
and also send the signal for the selected finger accordingly.
While the delay inhibitor is grabbed almost immediately, this can be
slow enough to not have happened immediately after the bus name has been
registered. Add a generous timeout to prevent issues.
If a print we have stored locally is not available in device anymore, we
need to cleanup the local database.
We do not get a proper DATA_NOT_FOUND error for most devices (indeed, at
this point no device does this properly). As such, do this when we see a
DATA_NOT_FOUND error and the first time that we get a verify-no-match
results on a device which is capable of listing all known prints.
Co-Authored-by: Marco Trevisan (Treviño) <mail@3v1n0.net>
In case we got a data-not-found error, it means that the device has not
such prints stored, and thus the verification failed, and there's no
need to expose the internal reasons to fprintd clients.
If the client disconnected while the release call was stopping the
current action, then the disconnect will be processed. This means that
the device can be closed already and the session is destroyed.
Add a check for this, in the same way that the vanished handler deals
with this corner case.
This renames the internal "in-use" property to "busy" and redefines the
value to be TRUE either if a client is connected or if the device is
considered WARM or HOT.
This prevents fprintd shutdown while devices are warm in order to ensure
that the libfprint hardware protection is functional.
For verify-match, the PAM module should simply drop off the bus. In
other cases it should correctly run VerifyStop and Release the device.
Verify this for verify-match and verify-no-match.
In the verify-match case, this means disconnecting from the bus rather
than stopping the verification. This is the only way to make sure that
the result is immediately reported and we do not wait for the device to
be idle again (which generally means waiting for finger removal).
In the verify-no-match case we simply send the string first before the
operation is stopped. An exceeded retry limit is only reported after
VerifyStop has finished.
Before VerifyStart can be called again the current verify operation
needs to be completed. This requires waiting for VerifyStop to happen.
As such, remove the test, which is expected to fail randomly.
We just need large enough samples to tell them apart correctly. For this
a 128x128 area from the center of each image is sufficient.
This speeds up the test run considerably. Other ways of achieving this
could be to also lower the number of enroll steps for the image device.
The test suite ran into a very rare error where it seemed to get stuck
during authorization. A possible explanation is that the priv->_session
pointer re-fetching is optimized away and the pointer just continues to
contain the invalid placeholder rather than an updated value.
Use g_atomic_pointer_get in order to avoid this possibility entirely.
While debugging the g-s-d testsuite a few more issues in the
OutputChecker code came up. Pull in these fixes ensuring that EOF and
the read side FD are handled correctly.
gnome-control-center expects to be able to re-enroll an existing print
when calling EnrollStart without deleting it first. As such, implicitly
delete the existing print rather than throwing an error.
Ideally, we'll change the API, but we need to give API users time to
adjust to the world.
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.
fprintd only needs very few syscalls. Mainly normal IO operations and
ioctl for USB access. All of this is covered by @system-service, we
could likely restrict it quite a bit more though.
If the device supports listing prints, then we can do more targeted
deletes once the storage runs out. As such, do not try to clear the
storage on first enroll (therefore allowing dual boot setups to work to
a limited degree).
Clear the device storage before we enroll the first print. At that
point, we know that the storage should be completely empty and we have
no way of deleting "garbage" prints later if the device does not support
listing prints.
This makes little sense. Users should delete the finger before trying to
enroll the same one again. So throw an error at them from EnrollStart
right away.
Fixes: #95
Always do an identify step before starting an enroll. If we find an
existing print, delete or throw an error depending on what is
appropriate.
Doing this ensures that we should not get duplicate prints system wide.
This means we will be able to identify the user that is trying to log
in. But more importantly, we need to do these checks for MoC devices,
which always run "identify" against all device stored prints rather than
the passed gallery.
During VerifyStart we may return early if there are no enrolled prints.
In such case we don't require the verification to be stopped if we're
using identification, but in the verification case we may leave the
device into the verification state.
So ensure we only set the device current state only when we're about to
start it.
Add tests ensuring those cases
The user may have some invalid prints saved (like the ones enrolled with
fprintd 1) in the storage, this lead to list such prints as enrolled but
they're actually not valid.
So load the prints to ensure that those are of the valid type instead of
just discovering them.
We may make just store.discover_prints to be aware of this, but this
would break some assumptions we do in tests, so better to go this way.
We may want to be able to load the user prints to check whether they
are usable, so add an utility function for this.
And use it also in load_all_prints().
It might make sense to push this into the storage layer. But, overall,
it is OK to live here, and if we do make changes on the storage layer we
probably want to change more than just this.
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.
When saving the prints we use g_file_set_contents under the hood and in
case return its error code that is a positive value.
So in such case we don't fail if we have a write failure at the end of
the enrollment.
While we could ensure in file storage to always return a negative value,
it's always better to ensure that is has to be 0 when we didn't get an
error.
Add a test checking for this case.
Since we so far we had no duplicated-check for prints in fprintd an user
may have enrolled the same print for multiple accounts or even for
different fingers, so we need to simulate this case.
Given that fprintd may not allow to enroll duplicated prints soon, it's
better to just copy the storage value so that we simulate a duplicated
enrollment in the past.
Allow to enroll multiple data in a single shot so that we don't have to
do it for each user, and add a test that uses it to match each possible
combination.
Some tests were delaying VerifyStop by not reporting the finger status
in the image driver, we can do the same using sleeps in the virtual
device driver, so let's reimplement such calls
Now that FPrintdVirtualStorageDeviceBaseTest is a
FPrintdVirtualDeviceBaseTest we can implement the needed `send_*`
functions that we use in the tests in order to get easily an interface
that can be used to repeat all the tests we've already written with the
new virtual device.
To do this, we've only to create new test classes that use the
FPrintdVirtualStorageDeviceBaseTest as main base class and that
also implement the test class.
When claiming a device for delete operation we'd not get an error in
case we can claim it but it's not already claimed, so in such case we
should explicitly check that the device has been opened.
When a device has reported the verification status the client should
call VerifyStop to stop the device, however this under the hood may lead
to a premature cancellation, causing the device not to react as expected
in case the finger is still on the sensor or in case it may return to us
some errors that we may want to handle (like the data-missing one).
So, in case we are about to stop the verification and the operation is
still in process, wait for a maximum timeout before proceed to the
cancellation.
However, while waiting, the action may be also cancelled because of a
call to Release() or because the client vanished, and in such case we
have to ensure that the current invocation is saved for being invoked by
stoppable_action_completed() when callback will return. That will also
unset it, and that's a clear indication for us that it has been already
consumed, and thus that we can just return doing nothing else.
Fixes: #100
Some tests may have different behaviors depending on the CPU load when
using parallel tests, so async results could arrive in different order.
Then we need to accept multiple results:
- A specific exception has to happen all the times
- One of the accepted exception has to happen all the times
- No exception or one of the allowed exception may happen
Depending on the test, use one of the possible strategies.
As per GDBus we can now get async calls (from the same client) happening
while we're still processing a previous request, so we need to ensure
that we react properly.
In case we get concurrent requests on EnrollStart/EnrollStop we'd just
continue with the operation, making the first processed request to start
the process and the second to hang (in code before the introduction of
stoppable_action_stop()) or to crash (in the current code).
So in such case we should always check that we're not handling already
the request, by checking priv->current_cancel_invocation value.
Add tests to verify the race.
Add a method to delete only a Fingerprint for a device, this is required
by they g-c-c UI design and at the same time it reflects the libfprint
API, where so far only a fingerprint at time can be deleted.
libfprint v1.90.4 introduced a new finger status API to expose to the UI
the finger status on sensor.
Add two new properties to the Device interface that represent the
possible values.
Add new tests.
The device DBus skeleton interface already implements caching for the
properties and can smartly handle their update sending (batched) dbus
events on changes.
Even if the default properties are only read only and we don't care, we
are going to introduce properties that will change values, and so having
the skeleton to handle this for us is quite convenient.
Given that we don't really need to override those properties, we can
just set them at start and leave the skeleton cache to handle the rest.
In case we'd ever need to override them, however the skeleton also
provides a way to override all the properties and to get a reference of
the number of properties it defines, ensuring to keep the order they are
defined.
This would allow us to get back the parent's properties IDs and to use
this to implement ours properties getters/setters using the parent one
as fallback.
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.
This makes garbage collection a bit more predictable overall. Note that
we'll first delete prints that we do not know the age of.
If we cannot sort them by age, then randomize the order so that we don't
end up deleting in the order that the device returned the prints.
The stoppable actions (Verify/Enroll) have the same logic during
completion. Create a common function to share this logic instead of
copying it in each of the handlers.
Fixes: #97
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.
In the garbage collection code we always ended up to load the first
enrolled print, and this may lead to removing from device storage prints
that are actually in use.
We have a condition where a client vanishing instead of cleaning up the
operation using VerifyStop would cause fprintd to hang. This only
happens if the underlying enroll/verify/identify operation has already
finished when the client vanishes.
Fix this by correctly interpreting current_cancellable as a flag for
these operations.
Fixes: #97
This is useful in the functions where we have to unset the device's
current action but we may use early-return to handle multiple conditions
such as in open, close and delete functions.
The latest also currently is a bit buggy as it won't reset the state on
some failures.
In the usual test we cancel the operation immediately by calling
VerifyStop. This (often) tests the case where we don't end up restarting
the Verify operation internally.
We can easily force fprintd to have restarted already internally, so add
a test that does so by sleeping a bit. This should give us a slightly
higher branch coverage in the verify_cb/identify_cb tests.
It seems that GLib may process multiple DBus signals in one mainloop
iteration. This could cause messages to be re-ordered, which in turn
caused a race condition in the CI that could trigger random failures.
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
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.
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
We had a race that was causing the events to be handled even if we were
not ready to accept them, causing a potential non-authentication.
So simulate this case, by sending a 'verify-match' event before we
started the verification and ensure that we ignore it.
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
This is both in case in we start the authentication and in the absurd
but (hey, testing!) situation in which prints gets deleted in between
the device claiming and the verification start.
To handle this second scenario we need to instruct fprintd mock to raise
an error on some special command
This now allows:
* Sending signals before and after method return
* Exiting the daemon
* Emulating NoEnrolledPrints DBus method error
Co-authored-by: Benjamin Berg <bberg@redhat.com>
The verify script would start an async routine. However, this blocks the
dbus return, which really is needed.
Also, we should only return one item of the script for each VerifyStart
run. So, fix things by pop'ing the first item and putting it on the bus
from a GLib.add_timeout handler.
Signals like VerifyResult may be received from unrelated Verify
operations. To avoid races, we need to ignore any VerifyResult that
happenes before the DBus method returns.
The only way to do this race-free is to use the async version of the
VerifyStart method.
In order to be race free, clients need to ignore all signals until after
the DBus method to start verification has returned. So the signal must
be emitted later than it currently is.
If someone has started an operation, then we don't really need to
confirm they are permitted to stop it again. Not doing this has the
advantage that we cannot run into a second interactive authorization
step accidentally.
Delete needs to operate on the device, so no other actions are permitted
at the same time. And using the libfprint _sync methods does not
guard against reentrance.
This way we can avoid repeating the same checks multiple times, and
we have a single point where we check the permissions needed for method
invocation.
Given that we could do operations where at least one permission, is
requested, we should give more priority to the weaker ones that are
acceptable and in case raise the level at later points.
We may have a case where the sender matches with the
session's sender but have a session invocation already set.
In such case we set an error, but still return TRUE.
But instead only wait for name to appear and do the tests in the main
function so that we can properly check the exception and depending on
its type skip the test or raise it so that it can be caught by the test
suite
If we rely on CI_PROJECT_NAME being set, then the test will fail in
similar environments outside of the fprintd main CI. So just add a
os.stat call afterwards to check whether the permission changes took
effect, and if not, then skip.
So, instead try to create a file and check that this fails.
The code to pick up the utilities from the test environment would fail
if the environment variables are not set. In that case, we can just use
the binary name and rely on PATH though.
This makes the meson output nicer, as it will correctly display that the
test has been skipped. It only happens if all tests in the run were
skipped, but meson always does one test a time.
This is nicer than adding -Wall and gives users more control.
Add -Wno-unused-parameter for now as there are lot of places where
this would need to be changed and it is reasonable in most cases.
Add -Wno-pedantic because it conflicts with
g_signal_handlers_disconnect_*
It seems that meson will not always apply the CFLAGS as defined through
the environment if "c_args=" is used in the default_options array for
the project() call.
Switching to add_project_arguments solves this problem.
See https://github.com/mesonbuild/meson/issues/8037
In the way the rule is currently set it would allow clients to send
messages with the fprintd interface to any other service, while we only
allow them to be redirected to fprintd itself.
This was causing a debian linter failure [1].
[1] https://lintian.debian.org/tags/dbus-policy-without-send-destination.html
PAM wrapper creates /tmp/pam.X files during its execution (strictly as
it does not follow $TMPDIR either), however given the low number of
combinations, we may end up in re-using the same pam.* folder during
meson test, causing a failure.
As per this, remove these temporary files on tearDown so that we won't
try reusing the same folder multiple times.
In case we're using an old polkit version that does not support
auto-pointers, we need to re-define such functions manually or fprintd
won't compile.
Given that polkit doesn't provide us version informations in headers we
need to get that from pkg-config
Disconnecting the g-authorize-method handler is not really needed, as it
is a signal from the same object. This basically reverts 6eb9f263fd
(device: Disconnect authorization callback and remove clients) but keeps
the code to clear known clients in the dispose handler.
Closes: #91
On finalization, the device should always be cleaned up properly (no
data associated with an action may be left). Show a critical warning if
this is not the case, as it indicates a programming error.
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.
When user is requested for enrolling, we should ask for password as
anyone who has physical access to the machine could otherwise enroll
its own fingers, and have access to it.
Fixes#5
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
Simulate the case in which multiple users are trying to access a device
at the same time, verifying that the access is granted only to the one
that first completes the authorization phase and that no other client is
then allowed.
Added various methods that allow to make methods to delay to return a
value, both by using timing functions and using a way to manually
stop and restart the calls.
This is mostly done using async callbacks in dbus methods
We need to be able to hack this to be an async daemon to perform some
multi-thread tests, so replacing default implementation with a simple
one that for now just does same of default
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
The current lockdown rules prevent USB devices from being accessed and
cause threading to not work.
As such, revert them until it is clear on how/if we can apply these
measures. It is primarily not clear on how to prevent fork/clone as
fprintd does not need those.
This reverts commit 2fd86624e5.
See: #82
Use error messages to be consistent, and avoid checking for a returned
value when dbus-glib function to fetch it returned false, as it's
implicit that we had a failure.
Otherwise if didn't fail we are sure that we got the requested value.
fprintd supports "any" finger parameter for the VerifyStart call, and it's
up to the daemon to pick the first known if the device doesn't support
identification.
So remove the check to verify utility and add a test to verify this is
respected.
This avoids addCleanup ordering errors and also errors when we may try to
print an invalid stdout pipe (like when we have processed it all), as python
might fail with something like:
======================================================================
ERROR: test_fprintd_multiple_verify_fails (__main__.TestFprintdUtilsVerify)
----------------------------------------------------------------------
Traceback (most recent call last):
File "~/GNOME/fprintd/tests/test_fprintd_utils.py", line 102, in <lambda>
self.addCleanup(lambda: print(process.stdout.read()))
File "/usr/lib/python3.8/codecs.py", line 321, in decode
data = self.buffer + input
TypeError: can't concat NoneType to bytes
unittest addCleanup calls are called in reverse order, so we need to reverse
the order of the calls as well, otherwise we won't correctly terminate the
subprocess children
No need to repeat the action in every unit test, but move the tests to a
different class to easily allow adding another class with tests with no
such initialization
An assertion that is raised within a callback will not be swallowed by
the C code that called the function. To ensure that errors will be
noticable, pass the result back to the surrounding scope and check it
there.
This excercises the path where we early-report a result and the
VerifyStop call must wait for the operation to complete intenernally.
Note that this test cannot fail right now due to the FpImageDevice
internal code still trying to hide the deactivation delay internally.
See libfprint!174
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
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.
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.
An application terminating because of a signal like SIGSEGV, SIGABRT and
friends, will exit with a signal number that is 128 + $SIGNAL_NUMBER, so
let's ensure that the daemon has not been terminated because of a such error
This makes even more sense with address sanitizer builds, as the daemon
would exit with abort.
Make possible to run tests with address sanitizer to quickly check for
memory errors, although we have to disable the error exit code in case of
leaks because we have some which are due to something else down in the stack
(and LSAN suppression files doesn't allow to define the stack to ignore
as we can in valgrind).
However, we'd abort in case of memory errors anyways, so this still helps
to prevent major problems, while still logging the leaks.
In order to run pam module tests with ASAN we need to manually pass the
library to LD_PRELOAD, as we do for the wrapper.
In order to run pam module tests we need to pass the libraries via
LD_PRELOAD, this supports a list of library paths, so use the compiler in
order to find their full paths (with soname) and check their presence.
In order to support linker scripts we need to introduce a workaround.
See meson issue https://github.com/mesonbuild/meson/issues/6880
Meson supports checking for default arguments natively without having to
do this for each one, so just use this feature.
Not doing this will become a warning as per meson 0.52.0 [1].
[1] https://github.com/mesonbuild/meson/pull/5627
The test doesn't need any assertion because we're calling DeleteEnrolledFingers
and in case it fails a net.reactivated.Fprint.Error.PermissionDenied error
would be thrown, and thus an exception would be raised at python level, making
the test to fail.
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
test_claim_from_other_client_is_released_when_vanished would fail on
the CI but work on a local system because we wouldn't want long enough
for the "vanished" code path to be taken into account. Add a small
timeout to make sure it works on the CI as well.
enroll_image() was always waiting for enroll-completed rather than for
what the caller expected as the result.
======================================================================
FAIL: test_enroll_invalid_storage_dir (__main__.FPrintdVirtualDeviceClaimedTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/hadess/Projects/jhbuild/fprintd/tests/fprintd.py", line 661, in test_enroll_invalid_storage_dir
self.enroll_image('whorl', expected_result='enroll-failed')
File "/home/hadess/Projects/jhbuild/fprintd/tests/fprintd.py", line 384, in enroll_image
self.wait_for_result('enroll-completed')
File "/home/hadess/Projects/jhbuild/fprintd/tests/fprintd.py", line 373, in wait_for_result
self.assertEqual(self._last_result, expected)
AssertionError: 'enroll-failed' != 'enroll-completed'
- enroll-failed
+ enroll-completed
We run a certain number of tests right now, without being able to easily
run them separated or to check which one failed.
So add a script to inspect all the available unittests per each python
script and use it to figure out the tests we can run in meson.
As per this, define a global 'python_tests' variable in meson that allows
to register new python tests easily without having to repeat the settings
for all the tests.
For each test we have, we check if we can fetch a list of unit tests, and
if possible we create a meson test for each one.
Otherwise we just fallback to normal behavior.
This is something that can be hopefully implemented into upstream meson [1].
[1] https://github.com/mesonbuild/meson/issues/6851
The unit tests files are provided as python files, adding a tool that
allows to list them in order to be able to run them easily via
python3 -m unittest module.Class.test_method
The test file calls self.daemon_start() in order to start fprintd and
locate the virtual image device that's needed for the test to run.
However, since the virtual image driver is not available on all
libfprint installations, the test should be skipped if the driver is not
available.
The skipping mechanism used to work, by checking if self.device is None.
This is based on the assumption that self.device would be set to None in
cases where the driver is not available. However, even in the past
self.device is only set to None in the tearDown method and not in setUp,
so presumably in edge cases it didn't entirely work.
However, since 0fb4f3b021 which
consistently removes the self.device attribute rather than setting it to
None, the "self.device is None" check no longer works. In particular,
the following error message is shown:
test_manager_get_default_device (__main__.FPrintdManagerTests) ...
Did not find virtual device! Probably libfprint was build without
the corresponding driver!
ERROR
After this patch, the following is shown:
test_manager_get_default_device (__main__.FPrintdManagerTests) ...
Did not find virtual device! Probably libfprint was build without
the corresponding driver!
skipped 'Need virtual_image device to run the test'
We fix this bug by consistently setting self.device to None, in both the
setUp method before daemon_start gets called, and in tearDown. We also
make the same change to self.manager for consistency.
The issue was not caught on CI, as the CI configuration always installs
a libfprint version that has the virtual_image device explicitly enabled
in the preparation phase.
Avoid using a temporary file for reading the utilities output, so we can
read it as it comes, ignoring the previous one, and avoiding open/closing
the file.
To keep the output printing on cleanup working, adding an utility function
that reads the output and save it for later printing
Avoid repeating the same operation to launch the utilities all the times,
but provide instead a function that allows to start a process and saves its
output without having to handle this in every test.
Simplify the operation when we just want the final output, still reusing
the same code.
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.
When we run tests in a system with real devices, we may try to initialize
the real ones, while we can just ignore them all in tests.
We do it in setUp instead of setUpClass to allow tests to change this if
they need to, but just for temporary.
When creating a new unit we used to get the system bus via Gio.bus_get_sync,
however this has a singleton implementation, and so would always return the
same connection, creating issues in tests when a new test suite is added
because the newly got connection would be already closed.
So, just manually create a new bus connection, also close the bus and
cleanup the test bus in dbus.
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.
Instead of automatically replying with the 'whorl' image for every enroll
state signal with result 'enroll-stage-passed', only perform the number
of required enroll stages and ensure that we get the expected results.
This also will allow to manually perform enroll steps in other tests.
Fix CI syntax error:
container_fedora_build: unknown keys in `extends` (.fedora@container-build)
Caused by changes in the wayland CI templates:
4a73f030d0
If we get a `NoEnrolledPrints` error while list, we don't consider it an
hard error and in such case we proceed to releasing the device, but without
clearing the previously set error first.
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.
If we get a `NoEnrolledPrints` error during delete, we don't consider it an
hard error and in such case we proceed to releasing the device, but without
clearing the previously set error first.
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.
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>
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.
Given that finger_name is set by GOptionEntry, make sure it's always
using allocated memory.
../utils/enroll.c:38:28: error: initialization discards ‘const’ qualifier
from pointer target type [-Werror=discarded-qualifiers]
38 | static char *finger_name = "right-index-finger";
| ^~~~~~~~~~~~~~~~~~~~
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;
Like for the fprintd test, run the tests under valgrind if the `VALGRIND`
environment variable is set, and use the contents of the variable as the
path to the suppression file.
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.
Allocate SessionData using g_malloc(). There are no benefits to using
GSlice for a seldom used structure. This also allows use to use
g_clear_pointer() to free the struct.
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
Fix linking error as the "store" global variable gets redeclared in
each C file that includes the header. Move the actual declaration to
main.c.
Fixes:
/usr/bin/ld: ./.libs/libfprintd.a(device.o):/builds/libfprint/fprintd/src/storage.h:51: multiple definition of `store'; main.o:/builds/libfprint/fprintd/src/storage.h:51: first defined here
Add scripting capabilities to the verification process so that the mock
daemon can send its own results without needing the client to prod it to
do that. This is incredibly useful when the client is single threaded
and blocking.
Note that there are barely any safeguards, so the scripting task is not
cancelled if an error occurs, or the VerifyStatus signals are sent out
of order.
This API was added to libfprint to allow drivers to report the match
result early before the operation has been completed. No driver makes
use of this facility yet and instead drivers try to finish the
operation early for quick result reporting. This primarily means not
waiting for finger removal.
Once drivers are updated, fprintd reactivity will regress unless the
early match callback is implemented as they would only get an operation
finished callback when the whole of the operation was finished,
including finger removal and finishing up USB communications.
See: https://gitlab.freedesktop.org/libfprint/fprintd/issues/35
The dependency list of libfprint used to be a direct copy of the
libfprint CI list. However, many of the dependencies are not needed as
only a minimal version of libfprint is built for testing purposes.
The state directory will generally be the same as the hardcoded one.
However, being able to override it is important for testing purposes, so
add the option.
These utilities are generally useful beyond only testing purproses. And,
since it is desirable to have automated tests inside the tests
subdirecty, it makes sense to move them elsewhere.
Some devices require storing the print on the device, to support this,
try deleting prints from the device before deleting them from local
storage.
To handle these devices, add a new API that requires the device to be
claimed rather than allowing deletion without claiming the device first.
Also add appropriate fallbacks so that the old API will continue to
work, but warn about its use.
When creating the FprintManager object the devices will be enumerated.
This operation calls the mainloop recursively. We do not want to receive
any client requests before the initial enumeration has happened. Because
of this, move the registration of the common name to happen after the
enumeration has finished.
The new libfprint version has support for devices that store data on the
sensor. In that case, the on-sensor storage might fill up when the user
tries to enroll a new print.
The strategy introduced here to handle this is to try and delete prints
from the device that we do not know about (assuming, it is e.g. from an
old installation and unusable).
It can also happen that we are not able to garbage collect old prints.
If that happens, a new error code "enroll-data-full" will be returned
signalling the situation to the enrolling application.
For sensors with internal storage we may want to garbage collect prints.
Adding this API means we can list all local prints, allowing us to find
out whether there are prints on the device's storage with no
corresponding print on the host.
The libfprint master branch will soon contain the v2 API. So change to
use the libfprint-1-0 which will mean that the CI will continue to work.
Note that the build_stable target will need to be removed when the new
libfprint version reaches fedora rawhide.
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
Fix incorrect configuration path when the sysconfdir is relative to the
prefix argument:
fprintd-WARNING **: 12:22:38.816: Could not open "${prefix}/etc/fprintd.conf": No such file or directory
The path needs to be expanded before it's substituted.
As written in the "Linux-PAM Application Developers' Guide"
at http://www.linux-pam.org/Linux-PAM-html/adg-security-user-identity.html:
"
As a general rule, the following convention for its value can be
assumed: NULL = unknown; localhost = invoked directly from the
local system; other.place.xyz = some component of the user's
connection originates from this remote/requesting host.
"
So also exit early if the hostname isn't localhost as it should be.
Closes: #21
If the directory referred to by ReadWritePaths= does not exist, the
service fails to start:
systemd[1]: Starting Fingerprint Authentication Daemon...
systemd[9736]: fprintd.service: Failed to set up mount namespacing: No such file or directory
systemd[9736]: fprintd.service: Failed at step NAMESPACE spawning /usr/lib/fprintd/fprintd: No such file or directory
systemd[1]: fprintd.service: Main process exited, code=exited, status=226/NAMESPACE
systemd[1]: fprintd.service: Failed with result 'exit-code'.
systemd[1]: Failed to start Fingerprint Authentication Daemon.
This may happen when booting with an empty /var filesystem.
For a system service, "StateDirectory=fprint" causes /var/lib/fprint and
any parent directories to be created if missing (with mode 0755 by
default, owned by the user and group of the service, which in this case
is root). In combination with ProtectSystem=strict, this state
directory will be mounted read-write. StateDirectory was introduced in
systemd 235, so require at least this version.
The /var/lib prefix is hardcoded in systemd. (Since systemd 240, the
full path(s) to StateDirectory are provided as $STATE_DIRECTORY, but
since it is always /var/lib, we continue to just hardcode that path.)
On non-systemd systems, since fprintd runs as root with no confinement,
it can create its state directory as needed (with g_mkdir_with_parents()
in file_storage_print_data_save()).
--localstatedir (and --prefix) will now be ignored in favour of this
hardcoded path. This is in preparation for a change to use systemd's
StateDirectory feature.
Otherwise you could get into a state where the daemon could not start
because the directory listed as a ReadWritePaths in the .service file is
missing.
Spotted by Will Thompson.
See: !5
The gettext in fprintd would be getting confused by a new file in
libfprint that looks like it should have been translated. Ignore this
file in our build.
Fix a possible crash when an fprintd client disappears. If the client
requested for the device to be released, then, without waiting for the
reply of that release, disappeared from the bus, we would try to close
it a second time, accessing a function pointer that didn't exist
anymore.
See https://bugzilla.redhat.com/show_bug.cgi?id=1515720
If not all the functions in the header are used, we'd get warnings about
them being unused. Mark all the functions as unused, so we can avoid
warnings.
Before claiming the device and therefore potentially activating
the actual hardware, make a call to see if the user has any
prints registered at all.
https://bugs.freedesktop.org/show_bug.cgi?id=99811
For some operations, i.e. listing the enrolled prints, the device
does not need to be claimed. Therefore the claiming can be delayed
until we actually start the verification process, allowing us to
query the fingerprint system if the user has any prints enrolled.
https://bugs.freedesktop.org/show_bug.cgi?id=99811
Give read-write access to USB devices in /dev, and the location of the
fingerprints, access to Unix sockets for D-Bus and
close everything else down.
See systemd.exec(5) for details about the options.
tx supports a single "--root" argument to pass both where it expects to
read the created data file (the .pot file) and the configuration. As
those are two separate directories with builddir != srcdir, copy the .tx
file from srcdir to builddir to upload the updated .pot file.
In file included from tests/enroll.c:29:0:
pam/fingerprint-strings.h: In function ‘finger_str_to_msg’:
pam/fingerprint-strings.h:99:6: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
return g_strdup_printf (TR (fingers[i].place_str_specific), driver_name);
^~~~~~
pam/fingerprint-strings.h:104:6: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
return g_strdup_printf (TR (fingers[i].swipe_str_specific), driver_name);
^~~~~~
This happens on hardware without fingerprint readers on every boot; we
don't need to log anything about it, it's totally normal.
This patch is part of an initiative to reduce logging spew in GNOME
so that actual errors and important messages are more visible.
https://bugs.freedesktop.org/show_bug.cgi?id=71889
This commit makes pam_fprintd return PAM_UNKNOWN_USER when
the user has not enrolled a fingerprint.
This lets the administrator set up pam_fprintd as a required
authentication, method, but only for users that have enrolled a
fingerprint, as such:
auth [success=ok user_unknown=ignore default=die] pam_fprintd.so max_tries=1 timeout=-1
auth [success=1 default=ignore] pam_unix.so nullok_secure
auth requisite pam_deny.so
With this config, users w/o an enrolled fingerprint will just be
asked for a password. Users with an enrolled fingerprint will
required to login using both their fingerprint and a password.
https://bugs.freedesktop.org/show_bug.cgi?id=64781
the dbus activation machinery depends on daemons taking a name on
the bus to complete activation without timeouts.
fprintd fails prematurely if there is USB bus (as found in some
virtual machine setups).
This commit moves the bus-name-acquisition code to happen before
the fail-from-no-usb-bus code to keep callers from timing out
when activating fprintd.
https://bugs.freedesktop.org/show_bug.cgi?id=57025
The PAM module uses dbus-glib, static gobject types, etc,
so it really can't get unloaded.
This commit adds some linker-fu to keep it resident even
after the pam module closes.
<warning><para><literal><xsl:value-ofselect="$name"/></literal> is deprecated since version <xsl:value-ofselect="@version"/> and should not be used in newly-written code. Use
<doc:doc><doc:summary>The username for whom to list the enrolled fingerprints. See <doc:reftype="description"to="usernames">Usernames</doc:ref>.</doc:summary></doc:doc>
<doc:doc><doc:summary>An array of strings representing the enrolled fingerprints. See <doc:reftype="description"to="fingerprint-names">Fingerprint names</doc:ref>.</doc:summary></doc:doc>
<doc:doc><doc:summary>The username for whom to delete the enrolled fingerprints. See <doc:reftype="description"to="usernames">Usernames</doc:ref>.</doc:summary></doc:doc>
<doc:doc><doc:summary>The username for whom to claim the device. See <doc:reftype="description"to="usernames">Usernames</doc:ref>.</doc:summary></doc:doc>
<doc:doc><doc:summary>A string representing the finger to verify. See <doc:reftype="description"to="fingerprint-names">Fingerprint names</doc:ref>.</doc:summary></doc:doc>
A string representing the status of the enrollment.
</doc:summary>
</doc:doc>
</arg>
<argtype="b"name="done">
<doc:doc>
<doc:summary>
Whether the enrollment finished and can be stopped.
</doc:summary>
</doc:doc>
</arg>
<doc:doc>
<doc:seealso>
<doc:reftype="description"to="enroll-statuses">Enrollment Statuses</doc:ref> and <doc:reftype="method"to="Device.EnrollStop">Device.EnrollStop</doc:ref>.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
