Skip to content

Commit

Permalink
Fix issues with Hardware Key auto detection
Browse files Browse the repository at this point in the history
* Fix #10656 - Add a small delay when before auto-polling hardware keys to all them to settle immediately after plugging in. This resolves an issue where the key's serial number could not be resolved due to hardware timeout.
* Also fix use of uninitialized variable if polling serial number fails for whatever reason.

* Fix typo in macOS key registration code

* Prevent registering duplicate listeners on window focus. These were not de-registered because we didn't trigger on unfocus. Show/Hide are sufficient triggers to add and remove listeners.
  • Loading branch information
droidmonkey committed May 5, 2024
1 parent 83623c8 commit 3ace4c6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 33 deletions.
56 changes: 29 additions & 27 deletions src/gui/DatabaseOpenWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,51 +159,50 @@ void DatabaseOpenWidget::toggleHardwareKeyComponent(bool state)
bool DatabaseOpenWidget::event(QEvent* event)
{
bool ret = DialogyWidget::event(event);
auto type = event->type();

switch (event->type()) {
case QEvent::Show:
case QEvent::WindowActivate: {
if (type == QEvent::Show || type == QEvent::WindowActivate) {
if (isOnQuickUnlockScreen() && (m_db.isNull() || !canPerformQuickUnlock())) {
resetQuickUnlock();
}
toggleQuickUnlockScreen();
m_hideTimer.stop();

if (type == QEvent::Show) {
#ifdef WITH_XC_YUBIKEY
#ifdef Q_OS_WIN
m_deviceListener->registerHotplugCallback(true,
true,
YubiKeyInterfaceUSB::YUBICO_USB_VID,
DeviceListener::MATCH_ANY,
&DeviceListenerWin::DEV_CLS_KEYBOARD);
m_deviceListener->registerHotplugCallback(true,
true,
YubiKeyInterfaceUSB::ONLYKEY_USB_VID,
DeviceListener::MATCH_ANY,
&DeviceListenerWin::DEV_CLS_KEYBOARD);
m_deviceListener->registerHotplugCallback(true,
true,
YubiKeyInterfaceUSB::YUBICO_USB_VID,
DeviceListener::MATCH_ANY,
&DeviceListenerWin::DEV_CLS_KEYBOARD);
m_deviceListener->registerHotplugCallback(true,
true,
YubiKeyInterfaceUSB::ONLYKEY_USB_VID,
DeviceListener::MATCH_ANY,
&DeviceListenerWin::DEV_CLS_KEYBOARD);
#else
m_deviceListener->registerHotplugCallback(true, true, YubiKeyInterfaceUSB::YUBICO_USB_VID);
m_deviceListener->registerHotplugCallback(true, true, YubiKeyInterfaceUSB::ONLYKEY_USB_VID);
m_deviceListener->registerHotplugCallback(true, true, YubiKeyInterfaceUSB::YUBICO_USB_VID);
m_deviceListener->registerHotplugCallback(true, true, YubiKeyInterfaceUSB::ONLYKEY_USB_VID);
#endif
#endif
m_hideTimer.stop();
pollHardwareKey();
}

return true;
}

case QEvent::Hide: {
ret = true;
} else if (type == QEvent::Hide || type == QEvent::WindowDeactivate) {
// Schedule form clearing if we are hidden
if (!isVisible()) {
if (!m_hideTimer.isActive()) {
m_hideTimer.start();
}

#ifdef WITH_XC_YUBIKEY
m_deviceListener->deregisterAllHotplugCallbacks();
if (type == QEvent::Hide) {
m_deviceListener->deregisterAllHotplugCallbacks();
}
#endif

return true;
}

default:;
ret = true;
}

return ret;
Expand Down Expand Up @@ -501,7 +500,10 @@ void DatabaseOpenWidget::pollHardwareKey(bool manualTrigger)
m_pollingHardwareKey = true;
m_manualHardwareKeyRefresh = manualTrigger;

YubiKey::instance()->findValidKeysAsync();
// Add a delay, if this is an automatic trigger, to allow the USB device to settle as
// the device may not report a valid serial number immediately after plugging in
int delay = manualTrigger ? 0 : 500;
QTimer::singleShot(delay, this, [] { YubiKey::instance()->findValidKeysAsync(); });
}

void DatabaseOpenWidget::hardwareKeyResponse(bool found)
Expand Down
2 changes: 1 addition & 1 deletion src/gui/osutils/macutils/DeviceListenerMac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void DeviceListenerMac::registerHotplugCallback(bool arrived, bool left, int ven
CFRelease(vid);
}
if (productId > 0) {
auto pid = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &vendorId);
auto pid = CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &productId);
CFDictionaryAddValue(matchingDict, CFSTR(kIOHIDProductIDKey), pid);
CFRelease(pid);
}
Expand Down
19 changes: 14 additions & 5 deletions src/keys/drivers/YubiKeyInterfaceUSB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,22 @@ namespace
yk_close_key(key);
}

void printError()
{
if (yk_errno == YK_EUSBERR) {
qWarning("Hardware key USB error: %s", yk_usb_strerror());
} else {
qWarning("Hardware key error: %s", yk_strerror(yk_errno));
}
}

unsigned int getSerial(YK_KEY* key)
{
unsigned int serial;
yk_get_serial(key, 1, 0, &serial);
if (!yk_get_serial(key, 1, 0, &serial)) {
printError();
return 0;
}
return serial;
}

Expand All @@ -70,11 +82,8 @@ namespace
} 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));
}
printError();
}
return nullptr;
}
Expand Down

0 comments on commit 3ace4c6

Please sign in to comment.