diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 3f2650cce..f974db170 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -10598,11 +10598,7 @@ Example: JBSWY3DPEHPK3PXP YubiKey - General: - - - - Could not find interface for hardware key with serial number %1. Please connect it to continue. + Could not find hardware key with serial number %1. Please connect it to continue. @@ -10663,10 +10659,6 @@ Example: JBSWY3DPEHPK3PXP YubiKeyInterfacePCSC - - Could not find or access hardware key with serial number %1. Please present it to continue. - - Hardware key is locked or timed out. Unlock or re-present it to continue. diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index dfc4b6b13..35544b709 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -73,31 +73,29 @@ bool YubiKey::isInitialized() bool YubiKey::findValidKeys() { + // Block operations on hardware keys while scanning QMutexLocker lock(&s_interfaceMutex); - findValidKeys(lock); + m_connectedKeys = 0; + m_findingKeys = true; + m_usbKeys = YubiKeyInterfaceUSB::instance()->findValidKeys(m_connectedKeys); + m_pcscKeys = YubiKeyInterfacePCSC::instance()->findValidKeys(m_connectedKeys); + m_findingKeys = false; return !m_usbKeys.isEmpty() || !m_pcscKeys.isEmpty(); } -void YubiKey::findValidKeys(const QMutexLocker& locker) -{ - // Check QMutexLocker since version 6.4 - Q_UNUSED(locker); - - m_connectedKeys = 0; - m_usbKeys = YubiKeyInterfaceUSB::instance()->findValidKeys(m_connectedKeys); - m_pcscKeys = YubiKeyInterfacePCSC::instance()->findValidKeys(m_connectedKeys); -} - void YubiKey::findValidKeysAsync() { - QtConcurrent::run([this] { emit detectComplete(findValidKeys()); }); + // Don't start another scan if we are already doing one + if (!m_findingKeys) { + m_findingKeys = true; + QtConcurrent::run([this] { emit detectComplete(findValidKeys()); }); + } } YubiKey::KeyMap YubiKey::foundKeys() { - QMutexLocker lock(&s_interfaceMutex); KeyMap foundKeys = m_usbKeys; foundKeys.unite(m_pcscKeys); @@ -106,38 +104,12 @@ YubiKey::KeyMap YubiKey::foundKeys() int YubiKey::connectedKeys() { - QMutexLocker lock(&s_interfaceMutex); - return m_connectedKeys; } QString YubiKey::errorMessage() { - QMutexLocker lock(&s_interfaceMutex); - - QString error; - error.clear(); - if (!m_error.isNull()) { - error += tr("General: ") + m_error; - } - - QString usb_error = YubiKeyInterfaceUSB::instance()->errorMessage(); - if (!usb_error.isNull()) { - if (!error.isNull()) { - error += " | "; - } - error += "USB: " + usb_error; - } - - QString pcsc_error = YubiKeyInterfacePCSC::instance()->errorMessage(); - if (!pcsc_error.isNull()) { - if (!error.isNull()) { - error += " | "; - } - error += "PCSC: " + pcsc_error; - } - - return error; + return m_error; } /** @@ -175,25 +147,31 @@ bool YubiKey::testChallenge(YubiKeySlot slot, bool* wouldBlock) YubiKey::ChallengeResult YubiKey::challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector& response) { - QMutexLocker lock(&s_interfaceMutex); - m_error.clear(); - // Make sure we tried to find available keys - if (m_usbKeys.isEmpty() && m_pcscKeys.isEmpty()) { - findValidKeys(lock); + // Prevent re-entrant access to hardware keys + QMutexLocker lock(&s_interfaceMutex); + + // Try finding key on the USB interface first + auto ret = YubiKeyInterfaceUSB::instance()->challenge(slot, challenge, response); + if (ret == ChallengeResult::YCR_ERROR) { + m_error = YubiKeyInterfaceUSB::instance()->errorMessage(); + return ret; } - if (m_usbKeys.contains(slot)) { - return YubiKeyInterfaceUSB::instance()->challenge(slot, challenge, response); + // If a USB key was not found, try PC/SC interface + if (ret == ChallengeResult::YCR_KEYNOTFOUND) { + ret = YubiKeyInterfacePCSC::instance()->challenge(slot, challenge, response); + if (ret == ChallengeResult::YCR_ERROR) { + m_error = YubiKeyInterfacePCSC::instance()->errorMessage(); + return ret; + } } - if (m_pcscKeys.contains(slot)) { - return YubiKeyInterfacePCSC::instance()->challenge(slot, challenge, response); + if (ret == ChallengeResult::YCR_KEYNOTFOUND) { + m_error = + tr("Could not find hardware key with serial number %1. Please connect it to continue.").arg(slot.first); } - m_error = tr("Could not find interface for hardware key with serial number %1. Please connect it to continue.") - .arg(slot.first); - - return ChallengeResult::YCR_ERROR; + return ret; } diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 5578fd8df..56fe6847b 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -44,7 +44,8 @@ public: { YCR_ERROR = 0, YCR_SUCCESS = 1, - YCR_WOULDBLOCK = 2 + YCR_WOULDBLOCK = 2, + YCR_KEYNOTFOUND = 3, }; static YubiKey* instance(); @@ -84,14 +85,14 @@ signals: private: explicit YubiKey(); - void findValidKeys(const QMutexLocker& locker); - static YubiKey* m_instance; QTimer m_interactionTimer; bool m_initialized = false; + bool m_findingKeys = false; QString m_error; + // Prevents multiple simultaneous operations on hardware keys static QMutex s_interfaceMutex; KeyMap m_usbKeys; diff --git a/src/keys/drivers/YubiKeyInterfacePCSC.cpp b/src/keys/drivers/YubiKeyInterfacePCSC.cpp index 6cec11b1a..10e4217e9 100644 --- a/src/keys/drivers/YubiKeyInterfacePCSC.cpp +++ b/src/keys/drivers/YubiKeyInterfacePCSC.cpp @@ -679,7 +679,7 @@ YubiKeyInterfacePCSC::challenge(YubiKeySlot slot, const QByteArray& challenge, B m_error.clear(); if (!m_initialized) { m_error = tr("The YubiKey PC/SC interface has not been initialized."); - return YubiKey::ChallengeResult::YCR_ERROR; + return YubiKey::ChallengeResult::YCR_KEYNOTFOUND; } // Try for a few seconds to find the key @@ -710,11 +710,8 @@ YubiKeyInterfacePCSC::challenge(YubiKeySlot slot, const QByteArray& challenge, B } } - m_error = tr("Could not find or access hardware key with serial number %1. Please present it to continue. ") - .arg(slot.first) - + m_error; emit challengeCompleted(); - return YubiKey::ChallengeResult::YCR_ERROR; + return YubiKey::ChallengeResult::YCR_KEYNOTFOUND; } YubiKey::ChallengeResult YubiKeyInterfacePCSC::performChallenge(void* key, diff --git a/src/keys/drivers/YubiKeyInterfaceUSB.cpp b/src/keys/drivers/YubiKeyInterfaceUSB.cpp index dba27d370..51fd82d8f 100644 --- a/src/keys/drivers/YubiKeyInterfaceUSB.cpp +++ b/src/keys/drivers/YubiKeyInterfaceUSB.cpp @@ -237,7 +237,7 @@ YubiKeyInterfaceUSB::challenge(YubiKeySlot slot, const QByteArray& challenge, Bo m_error.clear(); if (!m_initialized) { m_error = tr("The YubiKey USB interface has not been initialized."); - return YubiKey::ChallengeResult::YCR_ERROR; + return YubiKey::ChallengeResult::YCR_KEYNOTFOUND; } auto yk_key = openKeySerial(slot.first); @@ -245,12 +245,11 @@ YubiKeyInterfaceUSB::challenge(YubiKeySlot slot, const QByteArray& challenge, Bo // Key with specified serial number is not connected m_error = tr("Could not find hardware key with serial number %1. Please plug it in to continue.").arg(slot.first); - return YubiKey::ChallengeResult::YCR_ERROR; + return YubiKey::ChallengeResult::YCR_KEYNOTFOUND; } emit challengeStarted(); auto ret = performChallenge(yk_key.get(), slot.second, true, challenge, response); - emit challengeCompleted(); return ret;