Prevent interface lockups during startup with multiple tabs

Fixes #11998

Avoids UI lockups by removing several unnecessary mutex blocks  and avoiding redundant key detection calls.

Detect Yubikeys dynamically when challenging:

Prevents issue where correct key cannot be found if the internal state was reset prior to saving

This can occur if a user has multiple tabs open and multiple keys connected. Then switches to a locked tab without their DB key inserted which resets detection state.

Side Benefit - ensures proper cascade between USB and PC/SC interfaces so users can switch between the two modes seamlessly.

share/translations/keepassxc_en.ts

@@ -10590,11 +10590,7 @@ Example: JBSWY3DPEHPK3PXP</source>
 <context>
     <name>YubiKey</name>
     <message>
-        <source>General: </source>
+        <source>Could not find hardware key with serial number %1. Please connect it to continue.</source>
-        <translation type="unfinished"></translation>
-    </message>
-    <message>
-        <source>Could not find interface for hardware key with serial number %1. Please connect it to continue.</source>
         <translation type="unfinished"></translation>
     </message>
 </context>
@@ -10655,10 +10651,6 @@ Example: JBSWY3DPEHPK3PXP</source>
 </context>
 <context>
     <name>YubiKeyInterfacePCSC</name>
-    <message>
-        <source>Could not find or access hardware key with serial number %1. Please present it to continue. </source>
-        <translation type="unfinished"></translation>
-    </message>
     <message>
         <source>Hardware key is locked or timed out. Unlock or re-present it to continue.</source>
         <translation type="unfinished"></translation>

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);
+    return m_error;
-
-    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;
 }
 
 /**
@@ -175,25 +147,31 @@ bool YubiKey::testChallenge(YubiKeySlot slot, bool* wouldBlock)
 YubiKey::ChallengeResult
 YubiKey::challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector<char>& response)
 {
-    QMutexLocker lock(&s_interfaceMutex);
-
     m_error.clear();
 
-    // Make sure we tried to find available keys
+    // Prevent re-entrant access to hardware keys
-    if (m_usbKeys.isEmpty() && m_pcscKeys.isEmpty()) {
+    QMutexLocker lock(&s_interfaceMutex);
-        findValidKeys(lock);
+
+    // 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)) {
+    // If a USB key was not found, try PC/SC interface
-        return YubiKeyInterfaceUSB::instance()->challenge(slot, challenge, response);
+    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)) {
+    if (ret == ChallengeResult::YCR_KEYNOTFOUND) {
-        return YubiKeyInterfacePCSC::instance()->challenge(slot, challenge, response);
+        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.")
+    return ret;
-                  .arg(slot.first);
-
-    return ChallengeResult::YCR_ERROR;
 }

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;

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,

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;