Improve Yubikey USB API

* Allow for multiple vendor ID's to be checked at once. This allows for the use of one tracking index, streamlining KPXC code.
* Remove support for libusb 0.x on Linux
* Better handling of USB errors during initial key query. Output warnings to console.

src/CMakeLists.txt

@@ -338,8 +338,7 @@ if(APPLE)
         target_link_libraries(keepassx_core Qt5::MacExtras)
     endif()
     if(WITH_XC_TOUCHID)
-        target_link_libraries(keepassx_core "-framework Security")
+        target_link_libraries(keepassx_core "-framework Security -framework LocalAuthentication")
-        target_link_libraries(keepassx_core "-framework LocalAuthentication")
     endif()
 endif()
 if(HAIKU)

src/keys/drivers/YubiKey.cpp

@@ -31,27 +31,22 @@ namespace
 {
     constexpr int MAX_KEYS = 4;
 
-    YK_KEY* openKey(int ykIndex, int okIndex, bool* onlyKey = nullptr)
+    YK_KEY* openKey(int index)
     {
-        YK_KEY* key = nullptr;
+        static const int vids[] = {YUBICO_VID, ONLYKEY_VID};
-        if (onlyKey) {
+        static const int pids[] = {YUBIKEY_PID,
-            *onlyKey = false;
+                                   NEO_OTP_PID,
-        }
+                                   NEO_OTP_CCID_PID,
-        // Only allow for the first found key to be used
+                                   NEO_OTP_U2F_PID,
-        if (ykIndex == 0) {
+                                   NEO_OTP_U2F_CCID_PID,
-            key = yk_open_first_key();
+                                   YK4_OTP_PID,
-        }
+                                   YK4_OTP_U2F_PID,
+                                   YK4_OTP_CCID_PID,
+                                   YK4_OTP_U2F_CCID_PID,
+                                   PLUS_U2F_OTP_PID,
+                                   ONLYKEY_PID};
 
-        // New fuction available in yubikey-personalization version >= 1.20.0 that allows
+        return yk_open_key_vid_pid(vids, sizeof(vids) / sizeof(vids[0]), pids, sizeof(pids) / sizeof(pids[0]), index);
-        // selecting device VID/PID (yk_open_key_vid_pid)
-        if (!key) {
-            static const int device_pids[] = {0x60fc}; // OnlyKey PID
-            key = yk_open_key_vid_pid(0x1d50, device_pids, 1, okIndex);
-            if (onlyKey) {
-                *onlyKey = true;
-            }
-        }
-        return key;
     }
 
     void closeKey(YK_KEY* key)
@@ -68,19 +63,21 @@ namespace
 
     YK_KEY* openKeySerial(unsigned int serial)
     {
-        bool onlykey;
+        for (int i = 0; i < MAX_KEYS; ++i) {
-        for (int i = 0, j = 0; i + j < MAX_KEYS;) {
+            auto* yk_key = openKey(i);
-            auto* yk_key = openKey(i, j, &onlykey);
             if (yk_key) {
-                onlykey ? ++j : ++i;
                 // If the provided serial number is 0, or the key matches the serial, return it
                 if (serial == 0 || getSerial(yk_key) == serial) {
                     return yk_key;
                 }
                 closeKey(yk_key);
-            } else {
+            } else if (yk_errno == YK_ENOKEY) {
                 // No more connected keys
                 break;
+            } else if (yk_errno == YK_EUSBERR) {
+                qWarning("Hardware key USB error: %s", yk_usb_strerror());
+            } else {
+                qWarning("Hardware key error: %s", yk_strerror(yk_errno));
             }
         }
         return nullptr;
@@ -143,12 +140,9 @@ void YubiKey::findValidKeys()
         m_foundKeys.clear();
 
         // Try to detect up to 4 connected hardware keys
-        for (int i = 0, j = 0; i + j < MAX_KEYS;) {
+        for (int i = 0; i < MAX_KEYS; ++i) {
-            bool onlyKey = false;
+            auto yk_key = openKey(i);
-            auto yk_key = openKey(i, j, &onlyKey);
             if (yk_key) {
-                onlyKey ? ++j : ++i;
-                auto vender = onlyKey ? QStringLiteral("OnlyKey") : QStringLiteral("YubiKey");
                 auto serial = getSerial(yk_key);
                 if (serial == 0) {
                     closeKey(yk_key);
@@ -160,6 +154,8 @@ void YubiKey::findValidKeys()
                 int vid, pid;
                 yk_get_key_vid_pid(yk_key, &vid, &pid);
 
+                auto vendor = vid == 0x1d50 ? QStringLiteral("OnlyKey") : QStringLiteral("YubiKey");
+
                 bool wouldBlock;
                 QList<QPair<int, QString>> ykSlots;
                 for (int slot = 1; slot <= 2; ++slot) {
@@ -172,12 +168,12 @@ void YubiKey::findValidKeys()
                     // if it is enabled for the slot resulting in failed detection
                     if (pid <= NEO_OTP_U2F_CCID_PID) {
                         auto display = tr("%1 [%2] Configured Slot - %3")
-                                           .arg(vender, QString::number(serial), QString::number(slot));
+                                           .arg(vendor, QString::number(serial), QString::number(slot));
                         ykSlots.append({slot, display});
                     } else if (performTestChallenge(yk_key, slot, &wouldBlock)) {
                         auto display =
                             tr("%1 [%2] Challenge-Response - Slot %3 - %4")
-                                .arg(vender,
+                                .arg(vendor,
                                      QString::number(serial),
                                      QString::number(slot),
                                      wouldBlock ? tr("Press", "Challenge-Response Key interaction request")
@@ -194,9 +190,13 @@ void YubiKey::findValidKeys()
                 closeKey(yk_key);
 
                 Tools::wait(100);
-            } else {
+            } else if (yk_errno == YK_ENOKEY) {
                 // No more keys are connected
                 break;
+            } else if (yk_errno == YK_EUSBERR) {
+                qWarning("Hardware key USB error: %s", yk_usb_strerror());
+            } else {
+                qWarning("Hardware key error: %s", yk_strerror(yk_errno));
             }
         }
 

src/thirdparty/ykcore/ykcore.c

@@ -84,9 +84,9 @@ YK_KEY *yk_open_first_key(void)
 	return yk_open_key(0);
 }
 
-YK_KEY *yk_open_key_vid_pid(int vid, const int* pids, size_t pids_len, int index)
+YK_KEY *yk_open_key_vid_pid(const int* vids, size_t vids_len, const int* pids, size_t pids_len, int index)
 {
-	YK_KEY *yk = _ykusb_open_device(vid, pids, pids_len, index);
+	YK_KEY *yk = _ykusb_open_device(vids, vids_len, pids, pids_len, index);
 	int rc = yk_errno;
 
 	if (yk) {
@@ -102,6 +102,7 @@ YK_KEY *yk_open_key_vid_pid(int vid, const int* pids, size_t pids_len, int index
 	return yk;
 }
 
+static const int yubico_vids[] = {YUBICO_VID};
 static const int yubico_pids[] = {YUBIKEY_PID, NEO_OTP_PID, NEO_OTP_CCID_PID,
 	NEO_OTP_U2F_PID, NEO_OTP_U2F_CCID_PID, YK4_OTP_PID,
 	YK4_OTP_U2F_PID, YK4_OTP_CCID_PID, YK4_OTP_U2F_CCID_PID,
@@ -109,7 +110,9 @@ static const int yubico_pids[] = {YUBIKEY_PID, NEO_OTP_PID, NEO_OTP_CCID_PID,
 
 YK_KEY *yk_open_key(int index)
 {
-	return yk_open_key_vid_pid(YUBICO_VID, yubico_pids, sizeof(yubico_pids) / sizeof(yubico_pids[0]), index);
+	return yk_open_key_vid_pid(yubico_vids, sizeof(yubico_vids) / sizeof(yubico_vids[0]),
+                               yubico_pids, sizeof(yubico_pids) / sizeof(yubico_pids[0]),
+                               index);
 }
 
 int yk_close_key(YK_KEY *yk)

src/thirdparty/ykcore/ykcore.h

@@ -81,7 +81,7 @@ extern int yk_release(void);
 /* opens first key available. For backwards compatability */
 extern YK_KEY *yk_open_first_key(void);
 extern YK_KEY *yk_open_key(int);	/* opens nth key available */
-extern YK_KEY *yk_open_key_vid_pid(int, const int*, size_t, int);
+extern YK_KEY *yk_open_key_vid_pid(const int*, size_t, const int*, size_t, int);
 extern int yk_close_key(YK_KEY *k);		/* closes a previously opened key */
 
 /*************************************************************************

src/thirdparty/ykcore/ykcore_backend.h

@@ -39,7 +39,7 @@
 int _ykusb_start(void);
 int _ykusb_stop(void);
 
-void * _ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len, int index);
+void * _ykusb_open_device(const int* vendor_ids, size_t vids_len, const int *product_ids, size_t pids_len, int index);
 int _ykusb_close_device(void *);
 
 int _ykusb_read(void *dev, int report_type, int report_number,

src/thirdparty/ykcore/ykcore_libusb-1.0.c

@@ -161,7 +161,7 @@ extern int _ykusb_stop(void)
 	return 0;
 }
 
-void *_ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len, int index)
+void *_ykusb_open_device(const int* vendor_ids, size_t vids_len, const int *product_ids, size_t pids_len, int index)
 {
 	libusb_device *dev = NULL;
 	libusb_device_handle *h = NULL;
@@ -177,21 +177,24 @@ void *_ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len,
 		ykl_errno = libusb_get_device_descriptor(list[i], &desc);
 		if (ykl_errno != 0)
 			goto done;
-
+        size_t k;
-		if (desc.idVendor == vendor_id) {
+        for (k = 0; k < vids_len; k++) {
+            if (desc.idVendor == vendor_ids[k]) {
                 size_t j;
                 for (j = 0; j < pids_len; j++) {
                     if (desc.idProduct == product_ids[j]) {
                         found++;
                         if (found - 1 == index) {
                             dev = list[i];
-						break;
+                            goto found;
+                        }
                     }
                 }
             }
         }
 	}
 
+ found:
 	if (dev) {
 		int current_cfg;
 		rc = YK_EUSBERR;

src/thirdparty/ykcore/ykcore_libusb.c

@@ -1,216 +0,0 @@
-/* -*- mode:C; c-file-style: "bsd" -*- */
-/*
- * Copyright (c) 2008-2014 Yubico AB
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *
- *     * Redistributions in binary form must reproduce the above
- *       copyright notice, this list of conditions and the following
- *       disclaimer in the documentation and/or other materials provided
- *       with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <usb.h>
-#include <stdio.h>
-#include <string.h>
-
-#include "ykcore.h"
-#include "ykdef.h"
-#include "ykcore_backend.h"
-
-#define HID_GET_REPORT			0x01
-#define HID_SET_REPORT			0x09
-
-/*************************************************************************
- **  function _ykusb_write						**
- **  Set HID report							**
- **									**
- **  int _ykusb_write(YUBIKEY *yk, int report_type, int report_number,	**
- **			char *buffer, int size)				**
- **									**
- **  Where:								**
- **  "yk" is handle to open Yubikey					**
- **  "report_type" is HID report type (in, out or feature)		**
- **  "report_number" is report identifier				**
- **  "buffer" is pointer to in buffer					**
- **  "size" is size of the buffer					**
- **									**
- **  Returns: Nonzero if successful, zero otherwise			**
- **									**
- *************************************************************************/
-
-int _ykusb_write(void *dev, int report_type, int report_number,
-		 char *buffer, int size)
-{
-	int rc = usb_claim_interface((usb_dev_handle *)dev, 0);
-
-	if (rc >= 0) {
-		int rc2;
-		rc = usb_control_msg((usb_dev_handle *)dev,
-				     USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_ENDPOINT_OUT,
-				     HID_SET_REPORT,
-				     report_type << 8 | report_number, 0,
-				     buffer, size,
-				     1000);
-		/* preserve a control message error over an interface
-		   release one */
-		rc2 = usb_release_interface((usb_dev_handle *)dev, 0);
-		if (rc >= 0 && rc2 < 0)
-			rc = rc2;
-	}
-	if (rc >= 0)
-		return 1;
-	yk_errno = YK_EUSBERR;
-	return 0;
-}
-
-/*************************************************************************
-**  function _ykusb_read						**
-**  Get HID report							**
-**                                                                      **
-**  int _ykusb_read(YUBIKEY *dev, int report_type, int report_number,	**
-**		       char *buffer, int size)				**
-**                                                                      **
-**  Where:                                                              **
-**  "dev" is handle to open Yubikey					**
-**  "report_type" is HID report type (in, out or feature)		**
-**  "report_number" is report identifier					**
-**  "buffer" is pointer to in buffer					**
-**  "size" is size of the buffer					**
-**									**
-**  Returns: Number of bytes read. Zero if failure			**
-**                                                                      **
-*************************************************************************/
-
-int _ykusb_read(void *dev, int report_type, int report_number,
-		char *buffer, int size)
-{
-	int rc = usb_claim_interface((usb_dev_handle *)dev, 0);
-
-	if (rc >= 0) {
-		int rc2;
-		rc = usb_control_msg((usb_dev_handle *)dev,
-				     USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_ENDPOINT_IN,
-				     HID_GET_REPORT,
-				     report_type << 8 | report_number, 0,
-				     buffer, size,
-				     1000);
-		/* preserve a control message error over an interface
-		   release one */
-		rc2 = usb_release_interface((usb_dev_handle *)dev, 0);
-		if (rc >= 0 && rc2 < 0)
-			rc = rc2;
-	}
-	if (rc >= 0)
-		return rc;
-	if(rc == 0)
-		yk_errno = YK_ENODATA;
-	else
-		yk_errno = YK_EUSBERR;
-	return 0;
-}
-
-int _ykusb_start(void)
-{
-	int rc;
-	usb_init();
-
-	rc = usb_find_busses();
-	if (rc >= 0)
-		rc = usb_find_devices();
-
-	if (rc >= 0)
-		return 1;
-	yk_errno = YK_EUSBERR;
-	return 0;
-}
-
-extern int _ykusb_stop(void)
-{
-	return 1;
-}
-
-void *_ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len, int index)
-{
-	struct usb_bus *bus;
-	struct usb_device *yk_device = NULL;
-	struct usb_dev_handle *h = NULL;
-	int rc = YK_EUSBERR;
-	int found = 0;
-
-	for (bus = usb_get_busses(); bus; bus = bus->next) {
-		struct usb_device *dev;
-		rc = YK_ENOKEY;
-		for (dev = bus->devices; dev; dev = dev->next) {
-			if (dev->descriptor.idVendor == vendor_id) {
-				size_t j;
-				for (j = 0; j < pids_len; j++) {
-					if (dev->descriptor.idProduct == product_ids[j]) {
-                                                found++;
-                                                if (found-1 == index) {
-                                                        yk_device = dev;
-                                                        break;
-                                                }
-					}
-				}
-			}
-		}
-	}
-	if(yk_device != NULL) {
-		rc = YK_EUSBERR;
-		h = usb_open(yk_device);
-#ifdef LIBUSB_HAS_DETACH_KERNEL_DRIVER_NP
-		if (h != NULL)
-			usb_detach_kernel_driver_np(h, 0);
-#endif
-		/* This is needed for yubikey-personalization to work inside virtualbox virtualization. */
-		if (h != NULL)
-			usb_set_configuration(h, 1);
-		goto done;
-	}
- done:
-	if (h == NULL)
-		yk_errno = rc;
-	return h;
-}
-
-int _ykusb_close_device(void *yk)
-{
-	int rc = usb_close((usb_dev_handle *) yk);
-
-	if (rc >= 0)
-		return 1;
-	yk_errno = YK_EUSBERR;
-	return 0;
-}
-
-int _ykusb_get_vid_pid(void *yk, int *vid, int *pid) {
-	struct usb_dev_handle *h = yk;
-	struct usb_device *dev = usb_device(h);
-	*vid = dev->descriptor.idVendor;
-	*pid = dev->descriptor.idProduct;
-	return 1;
-}
-
-const char *_ykusb_strerror(void)
-{
-	return usb_strerror();
-}

src/thirdparty/ykcore/ykcore_osx.c

@@ -120,8 +120,11 @@ static IOHIDDeviceRef _ykosx_getHIDDeviceMatching(CFArrayRef devices,
 	return matchingDevice;
 }
 
-void *_ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len, int index)
+void *_ykusb_open_device(const int* vendor_ids, size_t vids_len, const int *product_ids, size_t pids_len, int index)
 {
+    (void) vendor_ids;
+    (void) vids_len;
+
 	IOHIDDeviceRef yk = NULL;
 
 	int rc = YK_ENOKEY;

src/thirdparty/ykcore/ykcore_windows.c

@@ -49,7 +49,7 @@ int _ykusb_stop(void)
 	return 1;
 }
 
-void * _ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len, int index)
+void * _ykusb_open_device(const int* vendor_ids, size_t vids_len, const int *product_ids, size_t pids_len, int index)
 {
 	HDEVINFO hi;
 	SP_DEVICE_INTERFACE_DATA di;
@@ -61,8 +61,7 @@ void * _ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len
 
 	yk_errno = YK_EUSBERR;
 
-	hi = SetupDiGetClassDevs(&GUID_DEVINTERFACE_KEYBOARD, 0, 0,
+	hi = SetupDiGetClassDevs(&GUID_DEVINTERFACE_KEYBOARD, 0, 0, DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
-				 DIGCF_PRESENT | DIGCF_DEVICEINTERFACE);
 	if (hi == INVALID_HANDLE_VALUE)
 		return NULL;
 
@@ -85,40 +84,26 @@ void * _ykusb_open_device(int vendor_id, const int *product_ids, size_t pids_len
 		rc = SetupDiGetDeviceInterfaceDetail(hi, &di, pi, len, &len, 0);
 		if (rc) {
 			HANDLE m_handle;
-
+            HIDD_ATTRIBUTES devInfo;
 			m_handle = CreateFile(pi->DevicePath, GENERIC_WRITE,
 					      FILE_SHARE_READ | FILE_SHARE_WRITE, 0, OPEN_EXISTING, 0, 0);
-			if (m_handle != INVALID_HANDLE_VALUE) {
+			if (m_handle != INVALID_HANDLE_VALUE && HidD_GetAttributes(m_handle, &devInfo)) {
-				HIDD_ATTRIBUTES devInfo;
+                for (size_t k = 0; k < vids_len; k++) {
-
+                    bool vid_match = devInfo.VendorID == vendor_ids[k];
-				if (HidD_GetAttributes(m_handle, &devInfo)) {
+                    for (size_t j = 0; vid_match && j < pids_len; j++) {
-					if (devInfo.VendorID == vendor_id) {
+                        if (devInfo.ProductID == product_ids[j] && ++found == index + 1) {
-						size_t j;
-						for (j = 0; j < pids_len; j++) {
-							if (devInfo.ProductID == product_ids[j]) {
-								found++;
-								if (found-1 == index) {
                             ret_handle = m_handle;
-									break;
-								}
-							}
-						}
-					}
-				}
-			}
-			if(ret_handle == NULL) {
-				CloseHandle (m_handle);
-			} else {
-				break;
-			}
-		}
-
-		free (pi);
-	}
-	if(ret_handle != NULL) {
                             goto done;
                         }
+                    }
+                }
+			}
+            CloseHandle (m_handle);
+		}
+		free (pi);
+	}
 
+    // No key found
     yk_errno = YK_ENOKEY;
 
 done:

src/thirdparty/ykcore/ykdef.h

@@ -293,6 +293,9 @@ struct status_st {
 
 #define	PLUS_U2F_OTP_PID	0x0410	/* Yubikey plus - OTP+U2F */
 
+#define ONLYKEY_VID     0x1d50
+#define ONLYKEY_PID     0x60fc
+
 #define YK4_CAPA_TAG		0x01	/* TAG for capabilities */
 #define YK4_SERIAL_TAG		0x02	/* TAG for serial number */