device: Fix possible race condition when checking claimed state

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.
This commit is contained in:
Benjamin Berg
2020-11-09 14:59:33 +01:00
parent 778a8540aa
commit 1e2f360ade

View File

@ -699,8 +699,14 @@ static gboolean fprint_device_claim (FprintDBusDevice *dbus_dev,
FprintDevice *rdev = FPRINT_DEVICE (dbus_dev); FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
g_autoptr(GMutexLocker) locked = NULL; g_autoptr(GMutexLocker) locked = NULL;
g_autoptr(GError) error = NULL;
char *sender, *user; char *sender, *user;
if (!_fprint_device_check_claimed (rdev, invocation, STATE_UNCLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
locked = g_mutex_locker_new (&priv->lock); locked = g_mutex_locker_new (&priv->lock);
g_assert_null (priv->session); g_assert_null (priv->session);
@ -756,9 +762,15 @@ static void dev_close_cb(FpDevice *dev, GAsyncResult *res, void *user_data)
static gboolean fprint_device_release (FprintDBusDevice *dbus_dev, static gboolean fprint_device_release (FprintDBusDevice *dbus_dev,
GDBusMethodInvocation *invocation) GDBusMethodInvocation *invocation)
{ {
g_autoptr(GError) error = NULL;
FprintDevice *rdev = FPRINT_DEVICE (dbus_dev); FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
if (priv->current_cancellable) { if (priv->current_cancellable) {
if (priv->current_action == ACTION_ENROLL) { if (priv->current_action == ACTION_ENROLL) {
g_warning("Enrollment was in progress, stopping it"); g_warning("Enrollment was in progress, stopping it");
@ -950,6 +962,11 @@ static gboolean fprint_device_verify_start (FprintDBusDevice *dbus_dev,
g_autoptr(GError) error = NULL; g_autoptr(GError) error = NULL;
guint finger_num = finger_name_to_num (finger_name); guint finger_num = finger_name_to_num (finger_name);
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
if (priv->current_action != ACTION_NONE) { if (priv->current_action != ACTION_NONE) {
if (priv->current_action == ACTION_ENROLL) { if (priv->current_action == ACTION_ENROLL) {
g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE, g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_ALREADY_IN_USE,
@ -1051,6 +1068,11 @@ static gboolean fprint_device_verify_stop (FprintDBusDevice *dbus_dev,
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
GError *error = NULL; GError *error = NULL;
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
if (priv->current_action == ACTION_NONE) { if (priv->current_action == ACTION_NONE) {
g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ACTION_IN_PROGRESS, g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_NO_ACTION_IN_PROGRESS,
"No verification in progress"); "No verification in progress");
@ -1260,6 +1282,11 @@ static gboolean fprint_device_enroll_start (FprintDBusDevice *dbus_dev,
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
int finger_num = finger_name_to_num (finger_name); int finger_num = finger_name_to_num (finger_name);
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
if (finger_num == -1) { if (finger_num == -1) {
g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INVALID_FINGERNAME, g_set_error(&error, FPRINT_ERROR, FPRINT_ERROR_INVALID_FINGERNAME,
"Invalid finger name"); "Invalid finger name");
@ -1304,7 +1331,12 @@ static gboolean fprint_device_enroll_stop (FprintDBusDevice *dbus_dev,
{ {
FprintDevice *rdev = FPRINT_DEVICE (dbus_dev); FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
GError *error = NULL; g_autoptr(GError) error = NULL;
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
if (priv->current_action != ACTION_ENROLL) { if (priv->current_action != ACTION_ENROLL) {
if (priv->current_action == ACTION_NONE) { if (priv->current_action == ACTION_NONE) {
@ -1319,7 +1351,6 @@ static gboolean fprint_device_enroll_stop (FprintDBusDevice *dbus_dev,
} else } else
g_assert_not_reached (); g_assert_not_reached ();
g_dbus_method_invocation_return_gerror (invocation, error); g_dbus_method_invocation_return_gerror (invocation, error);
g_error_free (error);
return TRUE; return TRUE;
} }
@ -1514,6 +1545,12 @@ static gboolean fprint_device_delete_enrolled_fingers2 (FprintDBusDevice *dbus_d
FprintDevice *rdev = FPRINT_DEVICE (dbus_dev); FprintDevice *rdev = FPRINT_DEVICE (dbus_dev);
FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev); FprintDevicePrivate *priv = fprint_device_get_instance_private(rdev);
g_autoptr(GMutexLocker) locked = NULL; g_autoptr(GMutexLocker) locked = NULL;
g_autoptr(GError) error = NULL;
if (!_fprint_device_check_claimed (rdev, invocation, STATE_CLAIMED, &error)) {
g_dbus_method_invocation_return_gerror (invocation, error);
return TRUE;
}
locked = g_mutex_locker_new (&priv->lock); locked = g_mutex_locker_new (&priv->lock);
@ -1595,6 +1632,9 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
g_assert_not_reached (); 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, if (!_fprint_device_check_claimed (rdev, invocation, required_state,
&error)) { &error)) {
return handle_unauthorized_access (rdev, invocation, error); return handle_unauthorized_access (rdev, invocation, error);
@ -1613,14 +1653,6 @@ action_authorization_handler (GDBusInterfaceSkeleton *interface,
return handle_unauthorized_access (rdev, invocation, error); return handle_unauthorized_access (rdev, invocation, error);
} }
/* By this time some other invocation might have beaten the one that
* arrived earlier, as an user might slower in authenticating, so
* we need to check again wheter the device is claimed */
if (!_fprint_device_check_claimed (rdev, invocation, required_state,
&error)) {
return handle_unauthorized_access (rdev, invocation, error);
}
g_debug ("Authorization granted to %s for device %s!", g_debug ("Authorization granted to %s for device %s!",
fp_device_get_name (priv->dev), fp_device_get_name (priv->dev),
g_dbus_method_invocation_get_sender (invocation)); g_dbus_method_invocation_get_sender (invocation));