Update for 1.90.2

This is based on the patch and observation from Bastien that some
URU4000B devices do not use encryption by default (it is a configuration
stored within the firmware). As such, it makes sense to always detect
whether encryption is in use by inspecting the image.

The encryption option would disable flipping of the image for the
URU400B device. Retain this behaviour for backward compatibility.

.gitlab-ci.yml

@@ -69,6 +69,7 @@ test:
     paths:
       - _build/meson-logs
     expire_in: 1 week
+  coverage: '/^TOTAL.*\s+(\d+\%)$/'
 
 test_valgrind:
   stage: test

NEWS

@@ -1,6 +1,16 @@
 This file lists notable changes in each release. For the full history of all
 changes, see ChangeLog.
 
+2020-06-08: v1.90.2 release
+
+This release contains a large amount of bug and regression fixes. These
+are not listed explicitly, but affect the majority of drivers.
+
+Highlights:
+ * A patch for nbis required for some sensors was accidentally dropped in
+   an earlier release. Users of these sensors/drivers (aes1610, aes2501,
+   aes2550, aes1660, aes2660, elan, upektc_img) need to re-enroll (#142).
+
 2019-11-20: v1.90.1 release
 
 This release fixes a lot of the regressions introduced in 1.90.0. Please note

libfprint/drivers/elan.c

@@ -1015,7 +1015,10 @@ dev_change_state (FpImageDevice *dev, FpiImageDeviceState state)
     state = FPI_IMAGE_DEVICE_STATE_INACTIVE;
 
   if (self->dev_state_next == state)
-    fp_dbg ("change to state %d already queued", state);
+    {
+      fp_dbg ("change to state %d already queued", state);
+      return;
+    }
 
   switch (state)
     {

libfprint/drivers/upeksonly.c

@@ -47,17 +47,11 @@ enum {
 enum sonly_kill_transfers_action {
   NOT_KILLING = 0,
 
-  /* abort a SSM with an error code */
-  ABORT_SSM,
-
   /* report an image session error */
   IMG_SESSION_ERROR,
 
   /* iterate a SSM to the next state */
   ITERATE_SSM,
-
-  /* call a callback */
-  EXEC_CALLBACK,
 };
 
 enum sonly_fs {
@@ -97,13 +91,9 @@ struct _FpiDeviceUpeksonly
 
   enum sonly_kill_transfers_action killing_transfers;
   GError                          *kill_error;
-  union
+  FpiSsm                          *kill_ssm;
-  {
-    FpiSsm *kill_ssm;
-    void    (*kill_cb)(FpImageDevice *dev);
-  };
 
-  struct fpi_line_asmbl_ctx assembling_ctx;
+  struct fpi_line_asmbl_ctx        assembling_ctx;
 };
 G_DECLARE_FINAL_TYPE (FpiDeviceUpeksonly, fpi_device_upeksonly, FPI,
                       DEVICE_UPEKSONLY, FpImageDevice);
@@ -176,11 +166,6 @@ last_transfer_killed (FpImageDevice *dev)
 
   switch (self->killing_transfers)
     {
-    case ABORT_SSM:
-      fp_dbg ("abort ssm error %s", self->kill_error->message);
-      fpi_ssm_mark_failed (self->kill_ssm, g_steal_pointer (&self->kill_error));
-      return;
-
     case ITERATE_SSM:
       fp_dbg ("iterate ssm");
       fpi_ssm_next_state (self->kill_ssm);
@@ -531,6 +516,14 @@ img_data_cb (FpiUsbTransfer *transfer, FpDevice *device,
       return;
     }
 
+  /* NOTE: The old code assume 4096 bytes are received each time
+   * but there is no reason we need to enforce that. However, we
+   * always need full lines. */
+  if (transfer->actual_length % 64 != 0)
+    error = fpi_device_error_new_msg (FP_DEVICE_ERROR_PROTO,
+                                      "Data packets need to be multiple of 64 bytes, got %zi bytes",
+                                      transfer->actual_length);
+
   if (error)
     {
       fp_warn ("bad status %s, terminating session", error->message);
@@ -551,7 +544,7 @@ img_data_cb (FpiUsbTransfer *transfer, FpDevice *device,
    * the first 2 bytes are a sequence number
    * then there are 62 bytes for image data
    */
-  for (i = 0; i < 4096; i += 64)
+  for (i = 0; i + 64 <= transfer->actual_length; i += 64)
     {
       if (!is_capturing (self))
         return;
@@ -560,7 +553,7 @@ img_data_cb (FpiUsbTransfer *transfer, FpDevice *device,
 
   if (is_capturing (self))
     {
-      fpi_usb_transfer_submit (transfer,
+      fpi_usb_transfer_submit (fpi_usb_transfer_ref (transfer),
                                0,
                                self->img_cancellable,
                                img_data_cb,
@@ -588,6 +581,8 @@ write_regs_finished (struct write_regs_data *wrdata, GError *error)
     fpi_ssm_next_state (wrdata->ssm);
   else
     fpi_ssm_mark_failed (wrdata->ssm, error);
+
+  g_free (wrdata);
 }
 
 static void write_regs_iterate (struct write_regs_data *wrdata);
@@ -634,9 +629,9 @@ write_regs_iterate (struct write_regs_data *wrdata)
                                  1);
   transfer->short_is_error = TRUE;
   transfer->ssm = wrdata->ssm;
-  fpi_usb_transfer_submit (transfer, CTRL_TIMEOUT, NULL, write_regs_cb, NULL);
-
   transfer->buffer[0] = regwrite->value;
+
+  fpi_usb_transfer_submit (transfer, CTRL_TIMEOUT, NULL, write_regs_cb, wrdata);
 }
 
 static void
@@ -675,10 +670,10 @@ sm_write_reg (FpiSsm        *ssm,
                                  1);
   transfer->short_is_error = TRUE;
   transfer->ssm = ssm;
+  transfer->buffer[0] = value;
+
   fpi_usb_transfer_submit (transfer, CTRL_TIMEOUT, NULL,
                            fpi_ssm_usb_transfer_cb, NULL);
-
-  transfer->buffer[0] = value;
 }
 
 static void
@@ -908,7 +903,7 @@ capsm_fire_bulk (FpiSsm   *ssm,
   self->img_cancellable = g_cancellable_new ();
   for (i = 0; i < self->img_transfers->len; i++)
     {
-      fpi_usb_transfer_submit (g_ptr_array_index (self->img_transfers, i),
+      fpi_usb_transfer_submit (fpi_usb_transfer_ref (g_ptr_array_index (self->img_transfers, i)),
                                0,
                                self->img_cancellable,
                                img_data_cb,
@@ -1406,8 +1401,12 @@ dev_activate (FpImageDevice *dev)
   self->capturing = FALSE;
 
   self->num_flying = 0;
+  self->img_transfers = g_ptr_array_new_with_free_func ((GFreeFunc) fpi_usb_transfer_unref);
 
-  for (i = 0; i < self->img_transfers->len; i++)
+  /* This might seem odd, but we do need multiple in-flight URBs so that
+   * we never stop polling the device for more data.
+   */
+  for (i = 0; i < NUM_BULK_TRANSFERS; i++)
     {
       FpiUsbTransfer *transfer;
 

libfprint/drivers/uru4000.c

@@ -81,7 +81,7 @@ static const struct uru4k_dev_profile
 {
   const char *name;
   gboolean    auth_cr;
-  gboolean    encryption;
+  gboolean    image_not_flipped;
 } uru4k_dev_info[] = {
   [MS_KBD] = {
     .name = "Microsoft Keyboard with Fingerprint Reader",
@@ -106,7 +106,7 @@ static const struct uru4k_dev_profile
   [DP_URU4000B] = {
     .name = "Digital Persona U.are.U 4000B",
     .auth_cr = FALSE,
-    .encryption = TRUE,
+    .image_not_flipped = TRUE, /* See comment in the code where it is used. */
   },
 };
 
@@ -680,17 +680,17 @@ imaging_run_state (FpiSsm *ssm, FpDevice *_dev)
           fpi_ssm_jump_to_state (ssm, IMAGING_CAPTURE);
           return;
         }
-      if (!self->profile->encryption)
+
+      /* Detect whether image is encrypted (by checking how noisy it is) */
+      dev2 = calc_dev2 (img);
+      fp_dbg ("dev2: %d", dev2);
+      if (dev2 < ENC_THRESHOLD)
         {
-          dev2 = calc_dev2 (img);
+          fpi_ssm_jump_to_state (ssm, IMAGING_REPORT_IMAGE);
-          fp_dbg ("dev2: %d", dev2);
+          return;
-          if (dev2 < ENC_THRESHOLD)
-            {
-              fpi_ssm_jump_to_state (ssm, IMAGING_REPORT_IMAGE);
-              return;
-            }
-          fp_info ("image seems to be encrypted");
         }
+      fp_info ("image seems to be encrypted");
+
       buf[0] = img->key_number;
       buf[1] = self->img_enc_seed;
       buf[2] = self->img_enc_seed >> 8;
@@ -769,7 +769,13 @@ imaging_run_state (FpiSsm *ssm, FpDevice *_dev)
         }
 
       fpimg->flags = FPI_IMAGE_COLORS_INVERTED;
-      if (!self->profile->encryption)
+      /* NOTE: For some reason all but U4000B (or rather U4500?) flipped the
+       * image, we retain this behaviour here, but it is not clear whether it
+       * is correct.
+       * It may be that there are different models with the same USB ID that
+       * behave differently.
+       */
+      if (self->profile->image_not_flipped)
         fpimg->flags |= FPI_IMAGE_V_FLIPPED | FPI_IMAGE_H_FLIPPED;
       fpi_image_device_image_captured (dev, fpimg);
 

libfprint/drivers/vfs0050.c

@@ -117,9 +117,10 @@ async_abort_callback (FpiUsbTransfer *transfer, FpDevice *device,
   int ep = transfer->endpoint;
 
   /* In normal case endpoint is empty */
-  if (g_error_matches (error, G_USB_DEVICE_ERROR, G_USB_DEVICE_ERROR_TIMED_OUT))
+  if (g_error_matches (error, G_USB_DEVICE_ERROR, G_USB_DEVICE_ERROR_TIMED_OUT) ||
+      (g_strcmp0 (g_getenv ("FP_DEVICE_EMULATION"), "1") == 0 && transfer->actual_length == 0))
     {
-      g_error_free (error);
+      g_clear_error (&error);
       fpi_ssm_next_state (transfer->ssm);
       return;
     }

libfprint/drivers/vfs301_proto.c

@@ -438,7 +438,7 @@ img_process_data (int first_block, FpDeviceVfs301 *dev, const guint8 *buf, int l
     usb_send (dev, data, len, NULL); \
   }
 
-#define RAW_DATA(x) x, sizeof (x)
+#define RAW_DATA(x) g_memdup (x, sizeof (x)), sizeof (x)
 
 #define IS_VFS301_FP_SEQ_START(b) ((b[0] == 0x01) && (b[1] == 0xfe))
 

libfprint/fp-image-device.c

@@ -62,17 +62,34 @@ static gboolean
 pending_activation_timeout (gpointer user_data)
 {
   FpImageDevice *self = FP_IMAGE_DEVICE (user_data);
+  FpDevice *device = FP_DEVICE (self);
   FpImageDevicePrivate *priv = fp_image_device_get_instance_private (self);
+  FpiDeviceAction action = fpi_device_get_current_action (device);
+  GError *error;
 
   priv->pending_activation_timeout_id = 0;
 
   if (priv->pending_activation_timeout_waiting_finger_off)
-    fpi_device_action_error (FP_DEVICE (self),
+    error = fpi_device_retry_new_msg (FP_DEVICE_RETRY_REMOVE_FINGER,
-                             fpi_device_retry_new_msg (FP_DEVICE_RETRY_REMOVE_FINGER,
+                                      "Remove finger before requesting another scan operation");
-                                                       "Remove finger before requesting another scan operation"));
   else
-    fpi_device_action_error (FP_DEVICE (self),
+    error = fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL);
-                             fpi_device_retry_new (FP_DEVICE_RETRY_GENERAL));
+
+  if (action == FPI_DEVICE_ACTION_VERIFY)
+    {
+      fpi_device_verify_report (device, FPI_MATCH_ERROR, NULL, error);
+      fpi_device_verify_complete (device, NULL);
+    }
+  else if (action == FPI_DEVICE_ACTION_IDENTIFY)
+    {
+      fpi_device_identify_report (device, NULL, NULL, error);
+      fpi_device_identify_complete (device, NULL);
+    }
+  else
+    {
+      /* Can this happen for enroll? */
+      fpi_device_action_error (device, error);
+    }
 
   return G_SOURCE_REMOVE;
 }

meson.build

@@ -1,5 +1,5 @@
 project('libfprint', [ 'c', 'cpp' ],
-    version: '1.90.1',
+    version: '1.90.2',
     license: 'LGPLv2.1+',
     default_options: [
         'buildtype=debugoptimized',

tests/README.md

@@ -0,0 +1,94 @@
+`umockdev` Tests
+================
+`umockdev` tests use fingerprint devices mocked by [`umockdev`
+toolchain][umockdev].
+
+This document describes how to create a 'capture' test: a test that
+captures a picture of a fingerprint from the device (mocked by
+`umockdev`) and compares it with the standard one.
+
+Other kinds of `umockdev` tests could be created in a similar manner.
+
+
+'Capture' Test Creation
+-----------------------
+A new 'capture' test is created by means of `capture.py` script:
+
+1. Create (if needed) a directory for the driver under `tests`
+   directory:
+
+   `mkdir DRIVER`
+
+2. Prepare your execution environment.
+
+   In the next step a working and up to date libfprint is needed. This can be
+   achieved by installing it into your system. Alternatively, you can set
+   the following environment variables to run a local build:
+   - `export LD_PRELOAD=<meson-build-dir>/libfprint/libfprint-2.so`
+   - `export GI_TYPELIB_PATH=<meson-build-dir>/libfprint`
+
+   Also, sometimes the driver must be adopted to the emulated environment
+   (mainly if it uses random numbers, see `synaptics.c` for an example).
+   Set the following environment variable to enable this adaptation:
+   - `export FP_DEVICE_EMULATION=1`
+
+   Run the next steps in the same terminal.
+
+3. Find the real USB fingerprint device with `lsusb`, e.g.:
+
+   `Bus 001 Device 005: ID 138a:0090 Validity Sensors, Inc. VFS7500 Touch Fingerprint Sensor`
+
+   The following USB device is used in the example above:
+   `/dev/bus/usb/001/005`.
+
+4. Record information about this device:
+
+   `umockdev-record /dev/bus/usb/001/005 > DRIVER/device`
+
+5. Record interaction of `capture.py` (or other test) with the device:
+
+   `umockdev-record -i /dev/bus/usb/001/005=DRIVER/capture.ioctl -- python3 ./capture.py DRIVER/capture.png`
+
+   Files `capture.ioctl` and `capture.png` will be created as the
+   result of this command.
+
+6. Add driver's name to `drivers_tests` in the `meson.build`.
+7. Check whether everything works as expected.
+
+**Note.** To avoid submitting a real fingerprint, the side of finger,
+arm, or anything else producing an image with the device can be used.
+
+
+Possible Issues
+---------------
+`umockdev-record` aggressively groups URBs. In most cases, manual
+intervention is unfortunately required. Often, drivers do a chain of
+commands like: A then B each with a different reply. However,
+`umockdev-record` could create a file like this:
+
+    A
+     reply 1
+     reply 2
+    B
+     reply 1
+     reply 2
+
+In that case, records must be re-ordered:
+
+    A
+     reply 1
+    B
+     reply 1
+    A
+     reply 2
+    B
+     reply 2
+
+Other changes may be needed to get everything working. For example the
+`elan` driver relies on a timeout that is not reported correctly. In
+this case the driver works around it by interpreting the protocol
+error differently in the virtual environment (by means of
+`FP_DEVICE_EMULATION` environment variable).
+
+
+[umockdev]: https://github.com/martinpitt/umockdev

tests/meson.build

@@ -18,8 +18,9 @@ envs.set('NO_AT_BRIDGE', '1')
 
 drivers_tests = [
     'elan',
-    'vfs5011',
     'synaptics',
+    'vfs0050',
+    'vfs5011',
 ]
 
 if get_option('introspection')

tests/vfs0050/device

@@ -0,0 +1,83 @@
+P: /devices/pci0000:00/0000:00:14.0/usb1/1-9
+N: bus/usb/001/004=12011001FF10FF088A13500060000000010109022E00010100A0320904000004FF00000007050102400000070581024000000705820240000007058303080004
+E: DEVNAME=/dev/bus/usb/001/004
+E: DEVTYPE=usb_device
+E: DRIVER=usb
+E: PRODUCT=138a/50/60
+E: TYPE=255/16/255
+E: BUSNUM=001
+E: DEVNUM=004
+E: MAJOR=189
+E: MINOR=3
+E: SUBSYSTEM=usb
+E: ID_VENDOR=138a
+E: ID_VENDOR_ENC=138a
+E: ID_VENDOR_ID=138a
+E: ID_MODEL=0050
+E: ID_MODEL_ENC=0050
+E: ID_MODEL_ID=0050
+E: ID_REVISION=0060
+E: ID_SERIAL=138a_0050_6d1900a1a0c0
+E: ID_SERIAL_SHORT=6d1900a1a0c0
+E: ID_BUS=usb
+E: ID_USB_INTERFACES=:ff0000:
+E: ID_VENDOR_FROM_DATABASE=Validity Sensors, Inc.
+E: ID_MODEL_FROM_DATABASE=Swipe Fingerprint Sensor
+E: ID_PATH=pci-0000:00:14.0-usb-0:9
+E: ID_PATH_TAG=pci-0000_00_14_0-usb-0_9
+E: LIBFPRINT_DRIVER=Validity VFS0050
+A: authorized=1
+A: avoid_reset_quirk=0
+A: bConfigurationValue=1
+A: bDeviceClass=ff
+A: bDeviceProtocol=ff
+A: bDeviceSubClass=10
+A: bMaxPacketSize0=8
+A: bMaxPower=100mA
+A: bNumConfigurations=1
+A: bNumInterfaces= 1
+A: bcdDevice=0060
+A: bmAttributes=a0
+A: busnum=1
+A: configuration=
+H: descriptors=12011001FF10FF088A13500060000000010109022E00010100A0320904000004FF00000007050102400000070581024000000705820240000007058303080004
+A: dev=189:3
+A: devnum=4
+A: devpath=9
+L: driver=../../../../../bus/usb/drivers/usb
+A: idProduct=0050
+A: idVendor=138a
+A: ltm_capable=no
+A: maxchild=0
+L: port=../1-0:1.0/usb1-port9
+A: power/active_duration=19312
+A: power/async=enabled
+A: power/autosuspend=2
+A: power/autosuspend_delay_ms=2000
+A: power/connected_duration=1037732
+A: power/control=auto
+A: power/level=auto
+A: power/persist=1
+A: power/runtime_active_kids=0
+A: power/runtime_active_time=19151
+A: power/runtime_enabled=enabled
+A: power/runtime_status=active
+A: power/runtime_suspended_time=1018305
+A: power/runtime_usage=0
+A: power/wakeup=disabled
+A: power/wakeup_abort_count=
+A: power/wakeup_active=
+A: power/wakeup_active_count=
+A: power/wakeup_count=
+A: power/wakeup_expire_count=
+A: power/wakeup_last_time_ms=
+A: power/wakeup_max_time_ms=
+A: power/wakeup_total_time_ms=
+A: quirks=0x0
+A: removable=fixed
+A: rx_lanes=1
+A: serial=6d1900a1a0c0
+A: speed=12
+A: tx_lanes=1
+A: urbnum=8
+A: version= 1.10