Significantly enhance hardware key robustness

* Significantly improve user experience when using hardware keys on databases in both GUI and CLI modes. Prevent locking up the YubiKey USB interface for prolonged periods of time. Allows for other apps to use the key concurrently with KeePassXC.

* Improve messages displayed to user when finding keys and when user interaction is required. Output specific error messages when handling hardware keys during database read/write.

* Only poll for keys when previously used or upon user request. Prevent continuously polling keys when accessing the UI such as switching tabs and minimize/maximize.

* Add support for using multiple hardware keys simultaneously. Keys are identified by their serial number which prevents using the wrong key during open and save operations.

* Fixes #4400
* Fixes #4065
* Fixes #1050
* Fixes #1215
* Fixes #3087
* Fixes #1088
* Fixes #1869

src/cli/Command.cpp

@@ -19,6 +19,7 @@
 #include <cstdlib>
 #include <utility>
 
+#include <QFileInfo>
 #include <QMap>
 
 #include "Command.h"
@@ -72,8 +73,8 @@ const QCommandLineOption Command::NoPasswordOption =
 const QCommandLineOption Command::YubiKeyOption =
     QCommandLineOption(QStringList() << "y"
                                      << "yubikey",
-                       QObject::tr("Yubikey slot used to encrypt the database."),
+                       QObject::tr("Yubikey slot and optional serial used to access the database (e.g., 1:7370001)."),
-                       QObject::tr("slot"));
+                       QObject::tr("slot[:serial]"));
 
 namespace
 {
@@ -121,7 +122,15 @@ QString Command::getDescriptionLine()
 
 QString Command::getHelpText()
 {
-    return buildParser(this)->helpText().replace("[options]", name + " [options]");
+    auto help = buildParser(this)->helpText();
+    // Fix spacing of options parameter
+    help.replace(QStringLiteral("[options]"), name + QStringLiteral(" [options]"));
+    // Remove application directory from command line example
+    auto appname = QFileInfo(QCoreApplication::applicationFilePath()).fileName();
+    auto regex = QRegularExpression(QStringLiteral(" .*%1").arg(QRegularExpression::escape(appname)));
+    help.replace(regex, appname.prepend(" "));
+
+    return help;
 }
 
 QSharedPointer<QCommandLineParser> Command::getCommandLineParser(const QStringList& arguments)

src/cli/Utils.cpp

@@ -17,6 +17,10 @@
 
 #include "Utils.h"
 
+#ifdef WITH_XC_YUBIKEY
+#include "keys/YkChallengeResponseKeyCLI.h"
+#endif
+
 #ifdef Q_OS_WIN
 #include <windows.h>
 #else
@@ -141,25 +145,28 @@ namespace Utils
 
 #ifdef WITH_XC_YUBIKEY
         if (!yubiKeySlot.isEmpty()) {
+            unsigned int serial = 0;
+            int slot;
+
             bool ok = false;
-            int slot = yubiKeySlot.toInt(&ok, 10);
+            auto parts = yubiKeySlot.split(":");
+            slot = parts[0].toInt(&ok);
+
             if (!ok || (slot != 1 && slot != 2)) {
-                err << QObject::tr("Invalid YubiKey slot %1").arg(yubiKeySlot) << endl;
+                err << QObject::tr("Invalid YubiKey slot %1").arg(parts[0]) << endl;
                 return {};
             }
 
-            QString errorMessage;
+            if (parts.size() > 1) {
-            bool blocking = YubiKey::instance()->checkSlotIsBlocking(slot, errorMessage);
+                serial = parts[1].toUInt(&ok, 10);
-            if (!errorMessage.isEmpty()) {
+                if (!ok) {
-                err << errorMessage << endl;
+                    err << QObject::tr("Invalid YubiKey serial %1").arg(parts[1]) << endl;
-                return {};
+                    return {};
+                }
             }
 
             auto key = QSharedPointer<YkChallengeResponseKeyCLI>(new YkChallengeResponseKeyCLI(
-                slot,
+                {serial, slot}, QObject::tr("Please touch the button on your YubiKey to continue…"), err));
-                blocking,
-                QObject::tr("Please touch the button on your YubiKey to unlock %1").arg(databaseFilename),
-                err.device()));
             compositeKey->addChallengeResponseKey(key);
         }
 #else

src/cli/Utils.h

@@ -26,12 +26,6 @@
 #include "keys/PasswordKey.h"
 #include <QtCore/qglobal.h>
 
-#ifdef WITH_XC_YUBIKEY
-#include "keys/YkChallengeResponseKey.h"
-#include "keys/YkChallengeResponseKeyCLI.h"
-#include "keys/drivers/YubiKey.h"
-#endif
-
 namespace Utils
 {
     extern QTextStream STDOUT;

src/core/Alloc.cpp

@@ -79,6 +79,8 @@ void operator delete[](void* ptr) noexcept
     ::operator delete(ptr);
 }
 
+// clang-format versions less than 10.0 refuse to put a space before "noexcept"
+// clang-format off
 /**
  * Custom insecure delete operator that does not zero out memory before
  * freeing a buffer. Can be used for better performance.
@@ -87,6 +89,7 @@ void operator delete(void* ptr, bool) noexcept
 {
     std::free(ptr);
 }
+// clang-format on
 
 void operator delete[](void* ptr, bool) noexcept
 {

src/core/Database.cpp

@@ -659,10 +659,11 @@ QByteArray Database::challengeResponseKey() const
 
 bool Database::challengeMasterSeed(const QByteArray& masterSeed)
 {
+    m_keyError.clear();
     if (m_data.key) {
         m_data.masterSeed->setHash(masterSeed);
         QByteArray response;
-        bool ok = m_data.key->challenge(masterSeed, response);
+        bool ok = m_data.key->challenge(masterSeed, response, &m_keyError);
         if (ok && !response.isEmpty()) {
             m_data.challengeResponseKey->setHash(response);
         } else if (ok && response.isEmpty()) {
@@ -703,6 +704,7 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
                       bool transformKey)
 {
     Q_ASSERT(!m_data.isReadOnly);
+    m_keyError.clear();
 
     if (!key) {
         m_data.key.reset();
@@ -724,7 +726,7 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
 
     if (!transformKey) {
         transformedMasterKey = QByteArray(oldTransformedMasterKey.rawKey());
-    } else if (!key->transform(*m_data.kdf, transformedMasterKey)) {
+    } else if (!key->transform(*m_data.kdf, transformedMasterKey, &m_keyError)) {
         return false;
     }
 
@@ -743,25 +745,9 @@ bool Database::setKey(const QSharedPointer<const CompositeKey>& key,
     return true;
 }
 
-bool Database::verifyKey(const QSharedPointer<CompositeKey>& key) const
+QString Database::keyError()
 {
-    Q_ASSERT(!m_data.key->isEmpty());
+    return m_keyError;
-
-    if (!m_data.challengeResponseKey->rawKey().isEmpty()) {
-        QByteArray result;
-
-        if (!key->challenge(m_data.masterSeed->rawKey(), result)) {
-            // challenge failed, (YubiKey?) removed?
-            return false;
-        }
-
-        if (m_data.challengeResponseKey->rawKey() != result) {
-            // wrong response from challenged device(s)
-            return false;
-        }
-    }
-
-    return (m_data.key->rawKey() == key->rawKey());
 }
 
 QVariantMap& Database::publicCustomData()

src/core/Database.h

@@ -119,9 +119,9 @@ public:
                 bool updateChangedTime = true,
                 bool updateTransformSalt = false,
                 bool transformKey = true);
+    QString keyError();
     QByteArray challengeResponseKey() const;
     bool challengeMasterSeed(const QByteArray& masterSeed);
-    bool verifyKey(const QSharedPointer<CompositeKey>& key) const;
     const QUuid& cipher() const;
     void setCipher(const QUuid& cipher);
     Database::CompressionAlgorithm compressionAlgorithm() const;
@@ -210,6 +210,7 @@ private:
     QPointer<FileWatcher> m_fileWatcher;
     bool m_modified = false;
     bool m_emitModified;
+    QString m_keyError;
 
     QList<QString> m_commonUsernames;
 

src/format/Kdbx3Reader.cpp

@@ -55,7 +55,7 @@ bool Kdbx3Reader::readDatabaseImpl(QIODevice* device,
     }
 
     if (!db->challengeMasterSeed(m_masterSeed)) {
-        raiseError(tr("Unable to issue challenge-response."));
+        raiseError(tr("Unable to issue challenge-response: %1").arg(db->keyError()));
         return false;
     }
 

src/format/Kdbx3Writer.cpp

@@ -42,7 +42,7 @@ bool Kdbx3Writer::writeDatabase(QIODevice* device, Database* db)
     QByteArray endOfHeader = "\r\n\r\n";
 
     if (!db->challengeMasterSeed(masterSeed)) {
-        raiseError(tr("Unable to issue challenge-response."));
+        raiseError(tr("Unable to issue challenge-response: %1").arg(db->keyError()));
         return false;
     }
 

src/format/Kdbx4Reader.cpp

@@ -50,7 +50,7 @@ bool Kdbx4Reader::readDatabaseImpl(QIODevice* device,
 
     bool ok = AsyncTask::runAndWaitForFuture([&] { return db->setKey(key, false, false); });
     if (!ok) {
-        raiseError(tr("Unable to calculate master key"));
+        raiseError(tr("Unable to calculate master key: %1").arg(db->keyError()));
         return false;
     }
 

src/format/Kdbx4Writer.cpp

@@ -53,7 +53,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
     QByteArray endOfHeader = "\r\n\r\n";
 
     if (!db->setKey(db->key(), false, true)) {
-        raiseError(tr("Unable to calculate master key"));
+        raiseError(tr("Unable to calculate master key: %1").arg(db->keyError()));
         return false;
     }
 

src/gui/ApplicationSettingsWidget.cpp

@@ -391,6 +391,7 @@ void ApplicationSettingsWidget::saveSettings()
 
     if (!config()->get(Config::RememberLastKeyFiles).toBool()) {
         config()->remove(Config::LastKeyFiles);
+        config()->remove(Config::LastChallengeResponse);
         config()->remove(Config::LastDir);
     }
 

src/gui/DatabaseOpenWidget.cpp

@@ -37,7 +37,6 @@
 #include <QDesktopServices>
 #include <QFont>
 #include <QSharedPointer>
-#include <QtConcurrentRun>
 
 DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
     : DialogyWidget(parent)
@@ -73,18 +72,29 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
     connect(m_ui->keyFileClearIcon, SIGNAL(triggered(bool)), SLOT(clearKeyFileEdit()));
 
 #ifdef WITH_XC_YUBIKEY
-    m_ui->yubikeyProgress->setVisible(false);
+    m_ui->hardwareKeyProgress->setVisible(false);
-    QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy();
+    QSizePolicy sp = m_ui->hardwareKeyProgress->sizePolicy();
     sp.setRetainSizeWhenHidden(true);
-    m_ui->yubikeyProgress->setSizePolicy(sp);
+    m_ui->hardwareKeyProgress->setSizePolicy(sp);
 
-    connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey()));
+    connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollHardwareKey()));
+    connect(YubiKey::instance(), SIGNAL(detectComplete(bool)), SLOT(hardwareKeyResponse(bool)), Qt::QueuedConnection);
+
+    connect(YubiKey::instance(), &YubiKey::userInteractionRequest, this, [this] {
+        // Show the press notification if we are in an independent window (e.g., DatabaseOpenDialog)
+        if (window() != getMainWindow()) {
+            m_ui->messageWidget->showMessage(tr("Please touch the button on your YubiKey!"),
+                                             MessageWidget::Information,
+                                             MessageWidget::DisableAutoHide);
+        }
+    });
+    connect(YubiKey::instance(), &YubiKey::challengeCompleted, this, [this] { m_ui->messageWidget->hide(); });
 #else
     m_ui->hardwareKeyLabel->setVisible(false);
     m_ui->hardwareKeyLabelHelp->setVisible(false);
     m_ui->buttonRedetectYubikey->setVisible(false);
-    m_ui->comboChallengeResponse->setVisible(false);
+    m_ui->challengeResponseCombo->setVisible(false);
-    m_ui->yubikeyProgress->setVisible(false);
+    m_ui->hardwareKeyProgress->setVisible(false);
 #endif
 
 #ifndef WITH_XC_TOUCHID
@@ -104,37 +114,16 @@ void DatabaseOpenWidget::showEvent(QShowEvent* event)
 {
     DialogyWidget::showEvent(event);
     m_ui->editPassword->setFocus();
-
-#ifdef WITH_XC_YUBIKEY
-    // showEvent() may be called twice, so make sure we are only polling once
-    if (!m_yubiKeyBeingPolled) {
-        // clang-format off
-        connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection);
-        connect(YubiKey::instance(), SIGNAL(detectComplete()), SLOT(yubikeyDetectComplete()), Qt::QueuedConnection);
-        connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection);
-        // clang-format on
-
-        pollYubikey();
-        m_yubiKeyBeingPolled = true;
-    }
-#endif
 }
 
 void DatabaseOpenWidget::hideEvent(QHideEvent* event)
 {
     DialogyWidget::hideEvent(event);
 
-#ifdef WITH_XC_YUBIKEY
+    // Clear the forms if we are minimized
-    // Don't listen to any Yubikey events if we are hidden
+    if (!isVisible()) {
-    disconnect(YubiKey::instance(), nullptr, this, nullptr);
+        clearForms();
-    m_yubiKeyBeingPolled = false;
-#endif
-
-    if (isVisible()) {
-        return;
     }
-
-    clearForms();
 }
 
 void DatabaseOpenWidget::load(const QString& filename)
@@ -148,7 +137,7 @@ void DatabaseOpenWidget::load(const QString& filename)
     m_keyFileComboEdited = false;
 
     if (config()->get(Config::RememberLastKeyFiles).toBool()) {
-        QHash<QString, QVariant> lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
+        auto lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
         if (lastKeyFiles.contains(m_filename)) {
             m_ui->comboKeyFile->addItem(lastKeyFiles[m_filename].toString());
             m_ui->comboKeyFile->setCurrentIndex(1);
@@ -157,6 +146,16 @@ void DatabaseOpenWidget::load(const QString& filename)
 
     QHash<QString, QVariant> useTouchID = config()->get(Config::UseTouchID).toHash();
     m_ui->checkTouchID->setChecked(useTouchID.value(m_filename, false).toBool());
+
+#ifdef WITH_XC_YUBIKEY
+    // Only auto-poll for hardware keys if we previously used one with this database file
+    if (config()->get(Config::RememberLastKeyFiles).toBool()) {
+        auto lastChallengeResponse = config()->get(Config::LastChallengeResponse).toHash();
+        if (lastChallengeResponse.contains(m_filename)) {
+            pollHardwareKey();
+        }
+    }
+#endif
 }
 
 void DatabaseOpenWidget::clearForms()
@@ -176,6 +175,11 @@ QSharedPointer<Database> DatabaseOpenWidget::database()
     return m_db;
 }
 
+QString DatabaseOpenWidget::filename()
+{
+    return m_filename;
+}
+
 void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile)
 {
     m_ui->editPassword->setText(pw);
@@ -186,6 +190,8 @@ void DatabaseOpenWidget::enterKey(const QString& pw, const QString& keyFile)
 
 void DatabaseOpenWidget::openDatabase()
 {
+    m_ui->messageWidget->hide();
+
     QSharedPointer<CompositeKey> masterKey = databaseKey();
     if (!masterKey) {
         return;
@@ -223,11 +229,6 @@ void DatabaseOpenWidget::openDatabase()
 
         config()->set(Config::UseTouchID, useTouchID);
 #endif
-
-        if (m_ui->messageWidget->isVisible()) {
-            m_ui->messageWidget->animatedHide();
-        }
-
         emit dialogFinished(true);
         m_isOpeningDatabase = false;
         clearForms();
@@ -293,7 +294,7 @@ QSharedPointer<CompositeKey> DatabaseOpenWidget::databaseKey()
     }
 #endif
 
-    QHash<QString, QVariant> lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
+    auto lastKeyFiles = config()->get(Config::LastKeyFiles).toHash();
     lastKeyFiles.remove(m_filename);
 
     auto key = QSharedPointer<FileKey>::create();
@@ -315,14 +316,14 @@ QSharedPointer<CompositeKey> DatabaseOpenWidget::databaseKey()
             legacyWarning.setDefaultButton(QMessageBox::Ok);
             legacyWarning.setCheckBox(new QCheckBox(tr("Don't show this warning again")));
 
-            connect(legacyWarning.checkBox(), &QCheckBox::stateChanged, [](int state) {
+            connect(legacyWarning.checkBox(), &QCheckBox::stateChanged, this, [](int state) {
                 config()->set(Config::Messages_NoLegacyKeyFileWarning, state == Qt::CheckState::Checked);
             });
 
             legacyWarning.exec();
         }
         masterKey->addKey(key);
-        lastKeyFiles[m_filename] = keyFilename;
+        lastKeyFiles.insert(m_filename, keyFilename);
     }
 
     if (config()->get(Config::RememberLastKeyFiles).toBool()) {
@@ -330,19 +331,17 @@ QSharedPointer<CompositeKey> DatabaseOpenWidget::databaseKey()
     }
 
 #ifdef WITH_XC_YUBIKEY
-    QHash<QString, QVariant> lastChallengeResponse = config()->get(Config::LastChallengeResponse).toHash();
+    auto lastChallengeResponse = config()->get(Config::LastChallengeResponse).toHash();
     lastChallengeResponse.remove(m_filename);
 
-    int selectionIndex = m_ui->comboChallengeResponse->currentIndex();
+    int selectionIndex = m_ui->challengeResponseCombo->currentIndex();
     if (selectionIndex > 0) {
-        int comboPayload = m_ui->comboChallengeResponse->itemData(selectionIndex).toInt();
+        auto slot = m_ui->challengeResponseCombo->itemData(selectionIndex).value<YubiKeySlot>();
-
+        auto crKey = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey(slot));
-        // read blocking mode from LSB and slot index number from second LSB
-        bool blocking = comboPayload & 1;
-        int slot = comboPayload >> 1;
-        auto crKey = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey(slot, blocking));
         masterKey->addChallengeResponseKey(crKey);
-        lastChallengeResponse[m_filename] = true;
+
+        // Qt doesn't read custom types in settings so stuff into a QString
+        lastChallengeResponse.insert(m_filename, QStringLiteral("%1:%2").arg(slot.first).arg(slot.second));
     }
 
     if (config()->get(Config::RememberLastKeyFiles).toBool()) {
@@ -400,45 +399,62 @@ void DatabaseOpenWidget::handleKeyFileComboChanged()
     m_ui->keyFileClearIcon->setVisible(m_keyFileComboEdited);
 }
 
-void DatabaseOpenWidget::pollYubikey()
+void DatabaseOpenWidget::pollHardwareKey()
 {
-    m_ui->buttonRedetectYubikey->setEnabled(false);
+    if (m_pollingHardwareKey) {
-    m_ui->comboChallengeResponse->setEnabled(false);
+        return;
-    m_ui->comboChallengeResponse->clear();
+    }
-    m_ui->comboChallengeResponse->addItem(tr("Select slot..."), -1);
-    m_ui->yubikeyProgress->setVisible(true);
 
-    // YubiKey init is slow, detect asynchronously to not block the UI
+    m_ui->challengeResponseCombo->clear();
-    QtConcurrent::run(YubiKey::instance(), &YubiKey::detect);
+    m_ui->challengeResponseCombo->addItem(tr("Detecting hardware keys…"));
+
+    m_ui->buttonRedetectYubikey->setEnabled(false);
+    m_ui->challengeResponseCombo->setEnabled(false);
+    m_ui->hardwareKeyProgress->setVisible(true);
+    m_pollingHardwareKey = true;
+
+    YubiKey::instance()->findValidKeys();
 }
 
-void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking)
+void DatabaseOpenWidget::hardwareKeyResponse(bool found)
 {
-    YkChallengeResponseKey yk(slot, blocking);
+    m_ui->challengeResponseCombo->clear();
-    // add detected YubiKey to combo box and encode blocking mode in LSB, slot number in second LSB
+    m_ui->buttonRedetectYubikey->setEnabled(true);
-    m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1) | blocking));
+    m_ui->hardwareKeyProgress->setVisible(false);
+    m_pollingHardwareKey = false;
 
+    if (!found) {
+        m_ui->challengeResponseCombo->addItem(tr("No hardware keys detected"));
+        m_ui->challengeResponseCombo->setEnabled(false);
+        return;
+    } else {
+        m_ui->challengeResponseCombo->addItem(tr("Select hardware key…"));
+    }
+
+    YubiKeySlot lastUsedSlot;
     if (config()->get(Config::RememberLastKeyFiles).toBool()) {
-        QHash<QString, QVariant> lastChallengeResponse = config()->get(Config::LastChallengeResponse).toHash();
+        auto lastChallengeResponse = config()->get(Config::LastChallengeResponse).toHash();
         if (lastChallengeResponse.contains(m_filename)) {
-            m_ui->comboChallengeResponse->setCurrentIndex(1);
+            // Qt doesn't read custom types in settings so extract from QString
+            auto split = lastChallengeResponse.value(m_filename).toString().split(":");
+            if (split.size() > 1) {
+                lastUsedSlot = YubiKeySlot(split[0].toUInt(), split[1].toInt());
+            }
         }
     }
-}
 
-void DatabaseOpenWidget::yubikeyDetectComplete()
+    int selectedIndex = 0;
-{
+    for (auto& slot : YubiKey::instance()->foundKeys()) {
-    m_ui->comboChallengeResponse->setEnabled(true);
+        // add detected YubiKey to combo box
-    m_ui->buttonRedetectYubikey->setEnabled(true);
+        m_ui->challengeResponseCombo->addItem(YubiKey::instance()->getDisplayName(slot), QVariant::fromValue(slot));
-    m_ui->yubikeyProgress->setVisible(false);
+        // Select this YubiKey + Slot if we used it in the past
-    m_yubiKeyBeingPolled = false;
+        if (lastUsedSlot == slot) {
-}
+            selectedIndex = m_ui->challengeResponseCombo->count() - 1;
+        }
+    }
 
-void DatabaseOpenWidget::noYubikeyFound()
+    m_ui->challengeResponseCombo->setCurrentIndex(selectedIndex);
-{
+    m_ui->challengeResponseCombo->setEnabled(true);
-    m_ui->buttonRedetectYubikey->setEnabled(true);
-    m_ui->yubikeyProgress->setVisible(false);
-    m_yubiKeyBeingPolled = false;
 }
 
 void DatabaseOpenWidget::openHardwareKeyHelp()

src/gui/DatabaseOpenWidget.h

@@ -40,13 +40,11 @@ public:
     explicit DatabaseOpenWidget(QWidget* parent = nullptr);
     ~DatabaseOpenWidget();
     void load(const QString& filename);
+    QString filename();
     void clearForms();
     void enterKey(const QString& pw, const QString& keyFile);
     QSharedPointer<Database> database();
 
-public slots:
-    void pollYubikey();
-
 signals:
     void dialogFinished(bool accepted);
 
@@ -64,9 +62,8 @@ private slots:
     void clearKeyFileEdit();
     void handleKeyFileComboEdited();
     void handleKeyFileComboChanged();
-    void yubikeyDetected(int slot, bool blocking);
+    void pollHardwareKey();
-    void yubikeyDetectComplete();
+    void hardwareKeyResponse(bool found);
-    void noYubikeyFound();
     void openHardwareKeyHelp();
     void openKeyFileHelp();
 
@@ -77,7 +74,7 @@ protected:
     bool m_retryUnlockWithEmptyPassword = false;
 
 private:
-    bool m_yubiKeyBeingPolled = false;
+    bool m_pollingHardwareKey = false;
     bool m_keyFileComboEdited = false;
     bool m_isOpeningDatabase = false;
     Q_DISABLE_COPY(DatabaseOpenWidget)

src/gui/DatabaseOpenWidget.ui

@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>580</width>
+    <width>588</width>
-    <height>410</height>
+    <height>448</height>
    </rect>
   </property>
   <property name="accessibleName">
@@ -280,7 +280,7 @@
                      <number>0</number>
                     </property>
                     <item row="1" column="2">
-                     <widget class="QProgressBar" name="yubikeyProgress">
+                     <widget class="QProgressBar" name="hardwareKeyProgress">
                       <property name="maximumSize">
                        <size>
                         <width>16777215</width>
@@ -302,7 +302,7 @@
                      </widget>
                     </item>
                     <item row="0" column="2">
-                     <widget class="QComboBox" name="comboChallengeResponse">
+                     <widget class="QComboBox" name="challengeResponseCombo">
                       <property name="enabled">
                        <bool>false</bool>
                       </property>
@@ -338,7 +338,7 @@
                          <string>Hardware Key:</string>
                         </property>
                         <property name="buddy">
-                         <cstring>comboChallengeResponse</cstring>
+                         <cstring>challengeResponseCombo</cstring>
                         </property>
                        </widget>
                       </item>
@@ -606,7 +606,7 @@
   <tabstop>comboKeyFile</tabstop>
   <tabstop>buttonBrowseFile</tabstop>
   <tabstop>hardwareKeyLabelHelp</tabstop>
-  <tabstop>comboChallengeResponse</tabstop>
+  <tabstop>challengeResponseCombo</tabstop>
   <tabstop>checkTouchID</tabstop>
  </tabstops>
  <resources/>

src/gui/DatabaseWidget.cpp

@@ -1183,7 +1183,9 @@ void DatabaseWidget::switchToDatabaseSettings()
 
 void DatabaseWidget::switchToOpenDatabase()
 {
-    switchToOpenDatabase(m_db->filePath());
+    if (currentWidget() != m_databaseOpenWidget || m_databaseOpenWidget->filename() != m_db->filePath()) {
+        switchToOpenDatabase(m_db->filePath());
+    }
 }
 
 void DatabaseWidget::switchToOpenDatabase(const QString& filePath)

src/gui/MainWindow.cpp

@@ -68,6 +68,10 @@
 #include "fdosecrets/FdoSecretsPlugin.h"
 #endif
 
+#ifdef WITH_XC_YUBIKEY
+#include "keys/drivers/YubiKey.h"
+#endif
+
 #ifdef WITH_XC_BROWSER
 #include "browser/BrowserService.h"
 #include "browser/BrowserSettingsPage.h"
@@ -175,6 +179,11 @@ MainWindow::MainWindow()
     m_ui->settingsWidget->addSettingsPage(fdoSS);
 #endif
 
+#ifdef WITH_XC_YUBIKEY
+    connect(YubiKey::instance(), SIGNAL(userInteractionRequest()), SLOT(showYubiKeyPopup()), Qt::QueuedConnection);
+    connect(YubiKey::instance(), SIGNAL(challengeCompleted()), SLOT(hideYubiKeyPopup()), Qt::QueuedConnection);
+#endif
+
     setWindowIcon(resources()->applicationIcon());
     m_ui->globalMessageWidget->setHidden(true);
     // clang-format off

src/gui/masterkey/YubiKeyEditWidget.cpp

@@ -19,13 +19,12 @@
 #include "ui_YubiKeyEditWidget.h"
 
 #include "config-keepassx.h"
+#include "core/AsyncTask.h"
 #include "gui/MainWindow.h"
 #include "gui/MessageBox.h"
 #include "keys/CompositeKey.h"
 #include "keys/YkChallengeResponseKey.h"
 
-#include <QtConcurrent>
-
 YubiKeyEditWidget::YubiKeyEditWidget(QWidget* parent)
     : KeyComponentWidget(parent)
     , m_compUi(new Ui::YubiKeyEditWidget())
@@ -36,6 +35,8 @@ YubiKeyEditWidget::YubiKeyEditWidget(QWidget* parent)
            "for additional security.</p><p>The YubiKey requires one of its slots to be programmed as "
            "<a href=\"https://www.yubico.com/products/services-software/personalization-tools/challenge-response/\">"
            "HMAC-SHA1 Challenge-Response</a>.</p>"));
+
+    connect(YubiKey::instance(), SIGNAL(detectComplete(bool)), SLOT(hardwareKeyResponse(bool)), Qt::QueuedConnection);
 }
 
 YubiKeyEditWidget::~YubiKeyEditWidget()
@@ -44,24 +45,31 @@ YubiKeyEditWidget::~YubiKeyEditWidget()
 
 bool YubiKeyEditWidget::addToCompositeKey(QSharedPointer<CompositeKey> key)
 {
-    QSharedPointer<YkChallengeResponseKey> keyPtr;
+    if (!m_isDetected || !m_compEditWidget) {
-    if (!createCrKey(keyPtr, false)) {
         return false;
     }
-    key->addChallengeResponseKey(keyPtr);
 
+    int selectionIndex = m_compUi->comboChallengeResponse->currentIndex();
+    auto slot = m_compUi->comboChallengeResponse->itemData(selectionIndex).value<YubiKeySlot>();
+    key->addChallengeResponseKey(QSharedPointer<YkChallengeResponseKey>::create(slot));
     return true;
 }
 
 bool YubiKeyEditWidget::validate(QString& errorMessage) const
 {
-    QSharedPointer<YkChallengeResponseKey> keyPtr;
+    if (!m_isDetected) {
-    if (!createCrKey(keyPtr)) {
+        errorMessage = tr("Could not find any hardware keys!");
-        errorMessage = tr("No YubiKey detected, please ensure it's plugged in.");
         return false;
     }
 
-    return true;
+    // Perform a test challenge response
+    int selectionIndex = m_compUi->comboChallengeResponse->currentIndex();
+    auto slot = m_compUi->comboChallengeResponse->itemData(selectionIndex).value<YubiKeySlot>();
+    bool valid = AsyncTask::runAndWaitForFuture([&slot] { return YubiKey::instance()->testChallenge(slot); });
+    if (!valid) {
+        errorMessage = tr("Selected hardware key slot does not support challenge-response!");
+    }
+    return valid;
 }
 
 QWidget* YubiKeyEditWidget::componentEditWidget()
@@ -76,13 +84,6 @@ QWidget* YubiKeyEditWidget::componentEditWidget()
 
 #ifdef WITH_XC_YUBIKEY
     connect(m_compUi->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey()));
-
-    // clang-format off
-    connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection);
-    connect(YubiKey::instance(), SIGNAL(detectComplete()), SLOT(yubikeyDetectComplete()), Qt::QueuedConnection);
-    connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection);
-    // clang-format on
-
     pollYubikey();
 #endif
 
@@ -105,72 +106,36 @@ void YubiKeyEditWidget::pollYubikey()
 
     m_isDetected = false;
     m_compUi->comboChallengeResponse->clear();
+    m_compUi->comboChallengeResponse->addItem(tr("Detecting hardware keys…"));
     m_compUi->buttonRedetectYubikey->setEnabled(false);
     m_compUi->comboChallengeResponse->setEnabled(false);
     m_compUi->yubikeyProgress->setVisible(true);
 
-    // YubiKey init is slow, detect asynchronously to not block the UI
+    YubiKey::instance()->findValidKeys();
-    QtConcurrent::run(YubiKey::instance(), &YubiKey::detect);
 #endif
 }
 
-void YubiKeyEditWidget::yubikeyDetected(int slot, bool blocking)
+void YubiKeyEditWidget::hardwareKeyResponse(bool found)
 {
-#ifdef WITH_XC_YUBIKEY
     if (!m_compEditWidget) {
         return;
     }
-    YkChallengeResponseKey yk(slot, blocking);
-    // add detected YubiKey to combo box and encode blocking mode in LSB, slot number in second LSB
-    m_compUi->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1u) | blocking));
-    m_isDetected = true;
-#else
-    Q_UNUSED(slot);
-    Q_UNUSED(blocking);
-#endif
-}
 
-void YubiKeyEditWidget::yubikeyDetectComplete()
-{
-    m_compUi->comboChallengeResponse->setEnabled(true);
-    m_compUi->buttonRedetectYubikey->setEnabled(true);
-    m_compUi->yubikeyProgress->setVisible(false);
-}
-
-void YubiKeyEditWidget::noYubikeyFound()
-{
-#ifdef WITH_XC_YUBIKEY
-    if (!m_compEditWidget) {
-        return;
-    }
     m_compUi->comboChallengeResponse->clear();
-    m_compUi->comboChallengeResponse->setEnabled(false);
-    m_compUi->comboChallengeResponse->addItem(tr("No YubiKey inserted."));
     m_compUi->buttonRedetectYubikey->setEnabled(true);
     m_compUi->yubikeyProgress->setVisible(false);
-    m_isDetected = false;
+
-#endif
+    if (!found) {
-}
+        m_compUi->comboChallengeResponse->addItem(tr("No hardware keys detected"));
-
+        m_isDetected = false;
-bool YubiKeyEditWidget::createCrKey(QSharedPointer<YkChallengeResponseKey>& key, bool testChallenge) const
+        return;
-{
+    }
-    Q_ASSERT(m_compEditWidget);
+
-    if (!m_isDetected || !m_compEditWidget) {
+    for (auto& slot : YubiKey::instance()->foundKeys()) {
-        return false;
+        // add detected YubiKey to combo box and encode blocking mode in LSB, slot number in second LSB
-    }
+        m_compUi->comboChallengeResponse->addItem(YubiKey::instance()->getDisplayName(slot), QVariant::fromValue(slot));
-
+    }
-    int selectionIndex = m_compUi->comboChallengeResponse->currentIndex();
+
-    int comboPayload = m_compUi->comboChallengeResponse->itemData(selectionIndex).toInt();
+    m_isDetected = true;
-
+    m_compUi->comboChallengeResponse->setEnabled(true);
-    if (0 == comboPayload) {
-        return false;
-    }
-
-    auto blocking = static_cast<bool>(comboPayload & 1u);
-    int slot = comboPayload >> 1u;
-    key.reset(new YkChallengeResponseKey(slot, blocking));
-    if (testChallenge) {
-        return key->challenge(QByteArray("0000"));
-    }
-    return true;
 }

src/gui/masterkey/YubiKeyEditWidget.h

@@ -45,14 +45,10 @@ protected:
     void initComponentEditWidget(QWidget* widget) override;
 
 private slots:
-    void yubikeyDetected(int slot, bool blocking);
+    void hardwareKeyResponse(bool found);
-    void yubikeyDetectComplete();
-    void noYubikeyFound();
     void pollYubikey();
 
 private:
-    bool createCrKey(QSharedPointer<YkChallengeResponseKey>& key, bool testChallenge = true) const;
-
     const QScopedPointer<Ui::YubiKeyEditWidget> m_compUi;
     QPointer<QWidget> m_compEditWidget;
     bool m_isDetected = false;

src/keys/ChallengeResponseKey.h

@@ -29,18 +29,24 @@ public:
         : m_uuid(uuid)
     {
     }
-    Q_DISABLE_COPY(ChallengeResponseKey);
+    virtual ~ChallengeResponseKey() = default;
-    virtual ~ChallengeResponseKey()
+
-    {
-    }
     virtual QByteArray rawKey() const = 0;
     virtual bool challenge(const QByteArray& challenge) = 0;
     virtual QUuid uuid() const
     {
         return m_uuid;
     }
+    QString error() const
+    {
+        return m_error;
+    }
+
+protected:
+    QString m_error;
 
 private:
+    Q_DISABLE_COPY(ChallengeResponseKey);
     QUuid m_uuid;
 };
 

src/keys/CompositeKey.cpp

@@ -60,7 +60,7 @@ bool CompositeKey::isEmpty() const
  */
 QByteArray CompositeKey::rawKey() const
 {
-    return rawKey(nullptr, nullptr);
+    return rawKey(nullptr);
 }
 
 /**
@@ -73,7 +73,7 @@ QByteArray CompositeKey::rawKey() const
  * @param ok true if challenges were successful and all key components could be added to the composite key
  * @return key hash
  */
-QByteArray CompositeKey::rawKey(const QByteArray* transformSeed, bool* ok) const
+QByteArray CompositeKey::rawKey(const QByteArray* transformSeed, bool* ok, QString* error) const
 {
     CryptoHash cryptoHash(CryptoHash::Sha256);
 
@@ -87,7 +87,7 @@ QByteArray CompositeKey::rawKey(const QByteArray* transformSeed, bool* ok) const
 
     if (transformSeed) {
         QByteArray challengeResult;
-        bool challengeOk = challenge(*transformSeed, challengeResult);
+        bool challengeOk = challenge(*transformSeed, challengeResult, error);
         if (ok) {
             *ok = challengeOk;
         }
@@ -110,7 +110,7 @@ QByteArray CompositeKey::rawKey(const QByteArray* transformSeed, bool* ok) const
  * @param result transformed key hash
  * @return true on success
  */
-bool CompositeKey::transform(const Kdf& kdf, QByteArray& result) const
+bool CompositeKey::transform(const Kdf& kdf, QByteArray& result, QString* error) const
 {
     if (kdf.uuid() == KeePass2::KDF_AES_KDBX3) {
         // legacy KDBX3 AES-KDF, challenge response is added later to the hash
@@ -120,10 +120,10 @@ bool CompositeKey::transform(const Kdf& kdf, QByteArray& result) const
     QByteArray seed = kdf.seed();
     Q_ASSERT(!seed.isEmpty());
     bool ok = false;
-    return kdf.transform(rawKey(&seed, &ok), result) && ok;
+    return kdf.transform(rawKey(&seed, &ok, error), result) && ok;
 }
 
-bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
+bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result, QString* error) const
 {
     // if no challenge response was requested, return nothing to
     // maintain backwards compatibility with regular databases.
@@ -137,7 +137,10 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
     for (const auto& key : m_challengeResponseKeys) {
         // if the device isn't present or fails, return an error
         if (!key->challenge(seed)) {
-            qWarning("Failed to issue challenge");
+            if (error) {
+                *error = key->error();
+            }
+            qWarning() << "Failed to issue challenge: " << key->error();
             return false;
         }
         cryptoHash.addData(key->rawKey());

src/keys/CompositeKey.h

@@ -38,9 +38,9 @@ public:
     bool isEmpty() const;
 
     QByteArray rawKey() const override;
-    QByteArray rawKey(const QByteArray* transformSeed, bool* ok = nullptr) const;
+
-    Q_REQUIRED_RESULT bool transform(const Kdf& kdf, QByteArray& result) const;
+    Q_REQUIRED_RESULT bool transform(const Kdf& kdf, QByteArray& result, QString* error = nullptr) const;
-    bool challenge(const QByteArray& seed, QByteArray& result) const;
+    bool challenge(const QByteArray& seed, QByteArray& result, QString* error = nullptr) const;
 
     void addKey(const QSharedPointer<Key>& key);
     const QList<QSharedPointer<Key>>& keys() const;
@@ -49,6 +49,8 @@ public:
     const QList<QSharedPointer<ChallengeResponseKey>>& challengeResponseKeys() const;
 
 private:
+    QByteArray rawKey(const QByteArray* transformSeed, bool* ok = nullptr, QString* error = nullptr) const;
+
     QList<QSharedPointer<Key>> m_keys;
     QList<QSharedPointer<ChallengeResponseKey>> m_challengeResponseKeys;
 };

src/keys/YkChallengeResponseKey.cpp

@@ -23,7 +23,6 @@
 #include "core/Tools.h"
 #include "crypto/CryptoHash.h"
 #include "crypto/Random.h"
-#include "gui/MainWindow.h"
 
 #include <QApplication>
 #include <QEventLoop>
@@ -38,15 +37,10 @@
 
 QUuid YkChallengeResponseKey::UUID("e092495c-e77d-498b-84a1-05ae0d955508");
 
-YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking)
+YkChallengeResponseKey::YkChallengeResponseKey(YubiKeySlot keySlot)
     : ChallengeResponseKey(UUID)
-    , m_slot(slot)
+    , m_keySlot(keySlot)
-    , m_blocking(blocking)
 {
-    if (getMainWindow()) {
-        connect(this, SIGNAL(userInteractionRequired()), getMainWindow(), SLOT(showYubiKeyPopup()));
-        connect(this, SIGNAL(userConfirmed()), getMainWindow(), SLOT(hideYubiKeyPopup()));
-    }
 }
 
 YkChallengeResponseKey::~YkChallengeResponseKey()
@@ -63,60 +57,25 @@ QByteArray YkChallengeResponseKey::rawKey() const
     return QByteArray::fromRawData(m_key, static_cast<int>(m_keySize));
 }
 
-/**
+bool YkChallengeResponseKey::challenge(const QByteArray& challenge)
- * Assumes yubikey()->init() was called
- */
-bool YkChallengeResponseKey::challenge(const QByteArray& c)
 {
-    return challenge(c, 2);
+    m_error.clear();
-}
+    QByteArray key;
+    auto result =
+        AsyncTask::runAndWaitForFuture([&] { return YubiKey::instance()->challenge(m_keySlot, challenge, key); });
 
-bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned int retries)
+    if (result == YubiKey::SUCCESS) {
-{
+        if (m_key) {
-    do {
+            gcry_free(m_key);
-        --retries;
-
-        if (m_blocking) {
-            emit userInteractionRequired();
         }
+        m_keySize = static_cast<std::size_t>(key.size());
+        m_key = static_cast<char*>(gcry_malloc_secure(m_keySize));
+        std::memcpy(m_key, key.data(), m_keySize);
+        sodium_memzero(key.data(), static_cast<std::size_t>(key.capacity()));
+    } else {
+        // Record the error message
+        m_error = YubiKey::instance()->errorMessage();
+    }
 
-        QByteArray key;
+    return result == YubiKey::SUCCESS;
-        auto result = AsyncTask::runAndWaitForFuture(
-            [this, challenge, &key]() { return YubiKey::instance()->challenge(m_slot, true, challenge, key); });
-
-        if (m_blocking) {
-            emit userConfirmed();
-        }
-
-        if (result == YubiKey::SUCCESS) {
-            if (m_key) {
-                gcry_free(m_key);
-            }
-            m_keySize = static_cast<std::size_t>(key.size());
-            m_key = static_cast<char*>(gcry_malloc_secure(m_keySize));
-            std::memcpy(m_key, key.data(), m_keySize);
-            sodium_memzero(key.data(), static_cast<std::size_t>(key.capacity()));
-            return true;
-        }
-    } while (retries > 0);
-
-    return false;
-}
-
-QString YkChallengeResponseKey::getName() const
-{
-    unsigned int serial;
-    QString fmt(QObject::tr("%1[%2] Challenge Response - Slot %3 - %4"));
-
-    YubiKey::instance()->getSerial(serial);
-
-    return fmt.arg(YubiKey::instance()->getVendorName(),
-                   QString::number(serial),
-                   QString::number(m_slot),
-                   (m_blocking) ? QObject::tr("Press") : QObject::tr("Passive"));
-}
-
-bool YkChallengeResponseKey::isBlocking() const
-{
-    return m_blocking;
 }

src/keys/YkChallengeResponseKey.h

@@ -31,32 +31,16 @@ class YkChallengeResponseKey : public QObject, public ChallengeResponseKey
 public:
     static QUuid UUID;
 
-    explicit YkChallengeResponseKey(int slot = -1, bool blocking = false);
+    explicit YkChallengeResponseKey(YubiKeySlot keySlot = {});
     ~YkChallengeResponseKey() override;
 
     QByteArray rawKey() const override;
     bool challenge(const QByteArray& challenge) override;
-    bool challenge(const QByteArray& challenge, unsigned int retries);
-    QString getName() const;
-    bool isBlocking() const;
-
-signals:
-    /**
-     * Emitted whenever user interaction is required to proceed with the challenge-response protocol.
-     * You can use this to show a helpful dialog informing the user that his assistance is required.
-     */
-    void userInteractionRequired();
-
-    /**
-     * Emitted when the user has provided their required input.
-     */
-    void userConfirmed();
 
 private:
     char* m_key = nullptr;
     std::size_t m_keySize = 0;
-    int m_slot;
+    YubiKeySlot m_keySlot;
-    bool m_blocking;
 };
 
 #endif // KEEPASSX_YK_CHALLENGERESPONSEKEY_H

src/keys/YkChallengeResponseKeyCLI.cpp

@@ -23,20 +23,21 @@
 #include "crypto/Random.h"
 
 #include <QFile>
-#include <QtCore/qglobal.h>
 
 QUuid YkChallengeResponseKeyCLI::UUID("e2be77c0-c810-417a-8437-32f41d00bd1d");
 
-YkChallengeResponseKeyCLI::YkChallengeResponseKeyCLI(int slot,
+YkChallengeResponseKeyCLI::YkChallengeResponseKeyCLI(YubiKeySlot keySlot, QString interactionMessage, QTextStream& out)
-                                                     bool blocking,
-                                                     QString messageInteraction,
-                                                     QIODevice* out)
     : ChallengeResponseKey(UUID)
-    , m_slot(slot)
+    , m_keySlot(keySlot)
-    , m_blocking(blocking)
+    , m_interactionMessage(interactionMessage)
-    , m_messageInteraction(messageInteraction)
+    , m_out(out.device())
 {
-    m_out.setDevice(out);
+    connect(YubiKey::instance(), SIGNAL(userInteractionRequest()), SLOT(showInteractionMessage()));
+}
+
+void YkChallengeResponseKeyCLI::showInteractionMessage()
+{
+    m_out << m_interactionMessage << "\n\n" << flush;
 }
 
 QByteArray YkChallengeResponseKeyCLI::rawKey() const
@@ -44,27 +45,8 @@ QByteArray YkChallengeResponseKeyCLI::rawKey() const
     return m_key;
 }
 
-/**
+bool YkChallengeResponseKeyCLI::challenge(const QByteArray& challenge)
- * Assumes yubikey()->init() was called
- */
-bool YkChallengeResponseKeyCLI::challenge(const QByteArray& c)
 {
-    return challenge(c, 2);
+    auto result = YubiKey::instance()->challenge(m_keySlot, challenge, m_key);
-}
+    return result == YubiKey::SUCCESS;
-
-bool YkChallengeResponseKeyCLI::challenge(const QByteArray& challenge, unsigned int retries)
-{
-    do {
-        --retries;
-
-        if (m_blocking) {
-            m_out << m_messageInteraction << endl;
-        }
-        YubiKey::ChallengeResult result = YubiKey::instance()->challenge(m_slot, m_blocking, challenge, m_key);
-        if (result == YubiKey::SUCCESS) {
-            return true;
-        }
-    } while (retries > 0);
-
-    return false;
 }

src/keys/YkChallengeResponseKeyCLI.h

@@ -33,17 +33,18 @@ class YkChallengeResponseKeyCLI : public QObject, public ChallengeResponseKey
 public:
     static QUuid UUID;
 
-    explicit YkChallengeResponseKeyCLI(int slot, bool blocking, QString messageInteraction, QIODevice* out);
+    explicit YkChallengeResponseKeyCLI(YubiKeySlot keySlot, QString interactionMessage, QTextStream& out);
 
     QByteArray rawKey() const override;
     bool challenge(const QByteArray& challenge) override;
-    bool challenge(const QByteArray& challenge, unsigned int retries);
+
+private slots:
+    void showInteractionMessage();
 
 private:
     QByteArray m_key;
-    int m_slot;
+    YubiKeySlot m_keySlot;
-    bool m_blocking;
+    QString m_interactionMessage;
-    QString m_messageInteraction;
     QTextStream m_out;
 };
 

src/keys/drivers/YubiKey.cpp

@@ -30,17 +30,97 @@
 
 #include "YubiKey.h"
 
-// Cast the void pointer from the generalized class definition
+#include <QtConcurrent>
-// to the proper pointer type from the now included system headers
+
-#define m_yk (static_cast<YK_KEY*>(m_yk_void))
+namespace
-#define m_ykds (static_cast<YK_STATUS*>(m_ykds_void))
+{
+    constexpr int MAX_KEYS = 4;
+
+    YK_KEY* openKey(int ykIndex, int okIndex, bool* onlyKey = nullptr)
+    {
+        YK_KEY* key = nullptr;
+        if (onlyKey) {
+            *onlyKey = false;
+        }
+#if YKPERS_VERSION_NUMBER >= 0x011200
+        // This function is only available in ykcore >= 1.18.0
+        key = yk_open_key(ykIndex);
+#else
+        // Only allow for the first found key to be used
+        if (ykIndex == 0) {
+            key = yk_open_first_key();
+        }
+#endif
+#if YKPERS_VERSION_NUMBER >= 0x011400
+        // New fuction available in yubikey-personalization version >= 1.20.0 that allows
+        // 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;
+            }
+        }
+#else
+        Q_UNUSED(okIndex);
+#endif
+        return key;
+    }
+
+    void closeKey(YK_KEY* key)
+    {
+        yk_close_key(key);
+    }
+
+    unsigned int getSerial(YK_KEY* key)
+    {
+        unsigned int serial;
+        yk_get_serial(key, 1, 0, &serial);
+        return serial;
+    }
+
+    YK_KEY* openKeySerial(unsigned int serial)
+    {
+        bool onlykey;
+        for (int i = 0, j = 0; i + j < MAX_KEYS;) {
+            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 {
+                // No more connected keys
+                break;
+            }
+        }
+        return nullptr;
+    }
+} // namespace
 
 YubiKey::YubiKey()
-    : m_yk_void(nullptr)
+    : m_mutex(QMutex::Recursive)
-    , m_ykds_void(nullptr)
-    , m_onlyKey(false)
-    , m_mutex(QMutex::Recursive)
 {
+    m_interactionTimer.setSingleShot(true);
+    m_interactionTimer.setInterval(300);
+
+    if (!yk_init()) {
+        qDebug("YubiKey: Failed to initialize USB interface.");
+    } else {
+        m_initialized = true;
+        // clang-format off
+        connect(&m_interactionTimer, SIGNAL(timeout()), this, SIGNAL(userInteractionRequest()));
+        connect(this, &YubiKey::challengeStarted, this, [this] { m_interactionTimer.start(); }, Qt::QueuedConnection);
+        connect(this, &YubiKey::challengeCompleted, this, [this] { m_interactionTimer.stop(); }, Qt::QueuedConnection);
+        // clang-format on
+    }
+}
+
+YubiKey::~YubiKey()
+{
+    yk_release();
 }
 
 YubiKey* YubiKey::m_instance(Q_NULLPTR);
@@ -54,161 +134,190 @@ YubiKey* YubiKey::instance()
     return m_instance;
 }
 
-bool YubiKey::init()
+bool YubiKey::isInitialized()
 {
-    m_mutex.lock();
+    return m_initialized;
-
-    // previously initialized
-    if (m_yk != nullptr && m_ykds != nullptr) {
-
-        if (yk_get_status(m_yk, m_ykds)) {
-            // Still connected
-            m_mutex.unlock();
-            return true;
-        } else {
-            // Initialized but not connected anymore, re-init
-            deinit();
-        }
-    }
-
-    if (!yk_init()) {
-        m_mutex.unlock();
-        return false;
-    }
-
-    // TODO: handle multiple attached hardware devices
-    m_onlyKey = false;
-    m_yk_void = static_cast<void*>(yk_open_first_key());
-#if YKPERS_VERSION_NUMBER >= 0x011400
-    // New fuction available in yubikey-personalization version >= 1.20.0 that allows
-    // selecting device VID/PID (yk_open_key_vid_pid)
-    if (m_yk == nullptr) {
-        static const int device_pids[] = {0x60fc}; // OnlyKey PID
-        m_yk_void = static_cast<void*>(yk_open_key_vid_pid(0x1d50, device_pids, 1, 0));
-        m_onlyKey = true;
-    }
-#endif
-    if (m_yk == nullptr) {
-        yk_release();
-        m_mutex.unlock();
-        return false;
-    }
-
-    m_ykds_void = static_cast<void*>(ykds_alloc());
-    if (m_ykds == nullptr) {
-        yk_close_key(m_yk);
-        m_yk_void = nullptr;
-        yk_release();
-        m_mutex.unlock();
-        return false;
-    }
-
-    m_mutex.unlock();
-    return true;
 }
 
-bool YubiKey::deinit()
+void YubiKey::findValidKeys()
 {
-    m_mutex.lock();
+    m_error.clear();
-
+    if (!isInitialized()) {
-    if (m_yk) {
+        return;
-        yk_close_key(m_yk);
-        m_yk_void = nullptr;
     }
 
-    if (m_ykds) {
+    QtConcurrent::run([this] {
-        ykds_free(m_ykds);
+        if (!m_mutex.tryLock(1000)) {
-        m_ykds_void = nullptr;
+            emit detectComplete(false);
-    }
+            return;
-
-    yk_release();
-
-    m_mutex.unlock();
-
-    return true;
-}
-
-void YubiKey::detect()
-{
-    bool found = false;
-
-    // Check slot 1 and 2 for Challenge-Response HMAC capability
-    for (int i = 1; i <= 2; ++i) {
-        QString errorMsg;
-        bool isBlocking = checkSlotIsBlocking(i, errorMsg);
-        if (errorMsg.isEmpty()) {
-            found = true;
-            emit detected(i, isBlocking);
         }
 
-        // Wait between slots to let the yubikey settle.
+        // Remove all known keys
-        Tools::sleep(150);
+        m_foundKeys.clear();
-    }
 
-    if (!found) {
+        // Try to detect up to 4 connected hardware keys
-        emit notFound();
+        for (int i = 0, j = 0; i + j < MAX_KEYS;) {
-    } else {
+            bool onlyKey = false;
-        emit detectComplete();
+            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);
+                    continue;
+                }
+
+                auto st = ykds_alloc();
+                yk_get_status(yk_key, st);
+                int vid, pid;
+                yk_get_key_vid_pid(yk_key, &vid, &pid);
+
+                bool wouldBlock;
+                QList<QPair<int, QString>> ykSlots;
+                for (int slot = 1; slot <= 2; ++slot) {
+                    auto config = (i == 1 ? CONFIG1_VALID : CONFIG2_VALID);
+                    if (!(ykds_touch_level(st) & config)) {
+                        // Slot is not configured
+                        continue;
+                    }
+                    // Don't actually challenge a YubiKey Neo or below, they always require button press
+                    // 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));
+                        ykSlots.append({slot, display});
+                    } else if (performTestChallenge(yk_key, slot, &wouldBlock)) {
+                        auto display = tr("%1 [%2] Challenge Response - Slot %3 - %4")
+                                           .arg(vender,
+                                                QString::number(serial),
+                                                QString::number(slot),
+                                                wouldBlock ? tr("Press") : tr("Passive"));
+                        ykSlots.append({slot, display});
+                    }
+                }
+
+                if (!ykSlots.isEmpty()) {
+                    m_foundKeys.insert(serial, ykSlots);
+                }
+
+                ykds_free(st);
+                closeKey(yk_key);
+            } else {
+                // No more keys are connected
+                break;
+            }
+        }
+
+        m_mutex.unlock();
+        emit detectComplete(!m_foundKeys.isEmpty());
+    });
 }
 
-bool YubiKey::checkSlotIsBlocking(int slot, QString& errorMessage)
+QList<YubiKeySlot> YubiKey::foundKeys()
 {
-    if (!init()) {
+    QList<YubiKeySlot> keys;
-        errorMessage = QString("Could not initialize YubiKey.");
+    for (auto serial : m_foundKeys.uniqueKeys()) {
-        return false;
+        for (auto key : m_foundKeys.value(serial)) {
+            keys.append({serial, key.first});
+        }
     }
+    return keys;
+}
 
-    YubiKey::ChallengeResult result;
+QString YubiKey::getDisplayName(YubiKeySlot slot)
-    QByteArray rand = randomGen()->randomArray(1);
+{
+    for (auto key : m_foundKeys.value(slot.first)) {
+        if (slot.second == key.first) {
+            return key.second;
+        }
+    }
+    return tr("%1 Invalid slot specified - %2").arg(QString::number(slot.first), QString::number(slot.second));
+}
+
+QString YubiKey::errorMessage()
+{
+    return m_error;
+}
+
+/**
+ * Issue a test challenge to the specified slot to determine if challenge
+ * response is properly configured.
+ *
+ * @param slot YubiKey configuration slot
+ * @param wouldBlock return if the operation requires user input
+ * @return whether the challenge succeeded
+ */
+bool YubiKey::testChallenge(YubiKeySlot slot, bool* wouldBlock)
+{
+    bool ret = false;
+    auto* yk_key = openKeySerial(slot.first);
+    if (yk_key) {
+        ret = performTestChallenge(yk_key, slot.second, wouldBlock);
+    }
+    return ret;
+}
+
+bool YubiKey::performTestChallenge(void* key, int slot, bool* wouldBlock)
+{
+    auto chall = randomGen()->randomArray(1);
     QByteArray resp;
-
+    auto ret = performChallenge(static_cast<YK_KEY*>(key), slot, false, chall, resp);
-    result = challenge(slot, false, rand, resp);
+    if (ret == SUCCESS || ret == WOULDBLOCK) {
-    if (result == ALREADY_RUNNING) {
+        if (wouldBlock) {
-        // Try this slot again after waiting
+            *wouldBlock = ret == WOULDBLOCK;
-        Tools::sleep(300);
+        }
-        result = challenge(slot, false, rand, resp);
+        return true;
     }
-
-    if (result == SUCCESS || result == WOULDBLOCK) {
-        return result == WOULDBLOCK;
-    } else if (result == ALREADY_RUNNING) {
-        errorMessage = QString("YubiKey busy");
-        return false;
-    } else if (result == ERROR) {
-        errorMessage = QString("YubiKey error");
-        return false;
-    }
-
-    errorMessage = QString("Error while polling YubiKey");
     return false;
 }
 
-bool YubiKey::getSerial(unsigned int& serial)
+/**
+ * Issue a challenge to the specified slot
+ * This operation could block if the YubiKey requires a touch to trigger.
+ *
+ * @param slot YubiKey configuration slot
+ * @param challenge challenge input to YubiKey
+ * @param response response output from YubiKey
+ * @return challenge result
+ */
+YubiKey::ChallengeResult YubiKey::challenge(YubiKeySlot slot, const QByteArray& challenge, QByteArray& response)
 {
-    m_mutex.lock();
+    m_error.clear();
-    int result = yk_get_serial(m_yk, 1, 0, &serial);
+    if (!m_initialized) {
-    m_mutex.unlock();
+        m_error = tr("The YubiKey interface has not been initialized.");
-
-    if (!result) {
-        return false;
-    }
-
-    return true;
-}
-
-QString YubiKey::getVendorName()
-{
-    return m_onlyKey ? "OnlyKey" : "YubiKey";
-}
-
-YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response)
-{
-    // ensure that YubiKey::init() succeeded
-    if (!init()) {
         return ERROR;
     }
 
+    // Try to grab a lock for 1 second, fail out if not possible
+    if (!m_mutex.tryLock(1000)) {
+        m_error = tr("Hardware key is currently in use.");
+        return ERROR;
+    }
+
+    auto* yk_key = openKeySerial(slot.first);
+    if (!yk_key) {
+        // 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);
+        m_mutex.unlock();
+        return ERROR;
+    }
+
+    emit challengeStarted();
+    auto ret = performChallenge(yk_key, slot.second, true, challenge, response);
+
+    closeKey(yk_key);
+    emit challengeCompleted();
+    m_mutex.unlock();
+
+    return ret;
+}
+
+YubiKey::ChallengeResult
+YubiKey::performChallenge(void* key, int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response)
+{
+    m_error.clear();
     int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2;
     QByteArray paddedChallenge = challenge;
 
@@ -232,39 +341,28 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte
     c = reinterpret_cast<const unsigned char*>(paddedChallenge.constData());
     r = reinterpret_cast<unsigned char*>(response.data());
 
-    // Try to grab a lock for 1 second, fail out if not possible
+    int ret = yk_challenge_response(
-    if (!m_mutex.tryLock(1000)) {
+        static_cast<YK_KEY*>(key), yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r);
-        return ALREADY_RUNNING;
-    }
 
-    int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r);
+    // actual HMAC-SHA1 response is only 20 bytes
-    m_mutex.unlock();
+    response.resize(20);
 
     if (!ret) {
         if (yk_errno == YK_EWOULDBLOCK) {
             return WOULDBLOCK;
-        } else if (yk_errno == YK_ETIMEOUT) {
-            return ERROR;
         } else if (yk_errno) {
-
+            if (yk_errno == YK_ETIMEOUT) {
-            /* Something went wrong, close the key, so that the next call to
+                m_error = tr("Hardware key timed out waiting for user interaction.");
-             * can try to re-open.
+            } else if (yk_errno == YK_EUSBERR) {
-             *
+                m_error = tr("A USB error ocurred when accessing the hardware key: %1").arg(yk_usb_strerror());
-             * Likely caused by the YubiKey being unplugged.
-             */
-
-            if (yk_errno == YK_EUSBERR) {
-                qWarning("USB error: %s", yk_usb_strerror());
             } else {
-                qWarning("YubiKey core error: %s", yk_strerror(yk_errno));
+                m_error = tr("Failed to complete a challenge-response, the specific error was: %1")
+                              .arg(yk_strerror(yk_errno));
             }
 
             return ERROR;
         }
     }
 
-    // actual HMAC-SHA1 response is only 20 bytes
-    response.resize(20);
-
     return SUCCESS;
 }

src/keys/drivers/YubiKey.h

@@ -19,11 +19,16 @@
 #ifndef KEEPASSX_YUBIKEY_H
 #define KEEPASSX_YUBIKEY_H
 
+#include <QHash>
 #include <QMutex>
 #include <QObject>
+#include <QTimer>
+
+typedef QPair<unsigned int, int> YubiKeySlot;
+Q_DECLARE_METATYPE(YubiKeySlot);
 
 /**
- * Singleton class to manage the interface to the hardware
+ * Singleton class to manage the interface to hardware key(s)
  */
 class YubiKey : public QObject
 {
@@ -32,100 +37,65 @@ class YubiKey : public QObject
 public:
     enum ChallengeResult
     {
-        ERROR = -1,
+        ERROR,
-        SUCCESS = 0,
+        SUCCESS,
-        WOULDBLOCK,
+        WOULDBLOCK
-        ALREADY_RUNNING
     };
 
-    /**
-     * @brief YubiKey::instance - get instance of singleton
-     * @return instance
-     */
     static YubiKey* instance();
+    bool isInitialized();
 
-    /**
+    void findValidKeys();
-     * @brief YubiKey::init - initialize yubikey library and hardware
-     * @return true on success
-     */
-    bool init();
 
-    /**
+    QList<YubiKeySlot> foundKeys();
-     * @brief YubiKey::deinit - cleanup after init
+    QString getDisplayName(YubiKeySlot slot);
-     * @return true on success
-     */
-    bool deinit();
 
-    /**
+    ChallengeResult challenge(YubiKeySlot slot, const QByteArray& challenge, QByteArray& response);
-     * @brief YubiKey::challenge - issue a challenge
+    bool testChallenge(YubiKeySlot slot, bool* wouldBlock = nullptr);
-     *
-     * This operation could block if the YubiKey requires a touch to trigger.
-     *
-     * TODO: Signal to the UI that the system is waiting for challenge response
-     *       touch.
-     *
-     * @param slot YubiKey configuration slot
-     * @param mayBlock operation is allowed to block
-     * @param challenge challenge input to YubiKey
-     * @param response response output from YubiKey
-     * @return challenge result
-     */
-    ChallengeResult challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response);
 
-    /**
+    QString errorMessage();
-     * @brief YubiKey::getSerial - serial number of YubiKey
-     * @param serial serial number
-     * @return true on success
-     */
-    bool getSerial(unsigned int& serial);
 
-    /**
-     * @brief YubiKey::getVendorName - vendor name of token
-     * @return vendor name
-     */
-    QString getVendorName();
-
-    /**
-     * @brief YubiKey::detect - probe for attached YubiKeys
-     */
-    void detect();
-
-    /**
-     * @param slot the yubikey slot.
-     * @param errorMessage populated if an error occured.
-     *
-     * @return whether the key is blocking or not.
-     */
-    bool checkSlotIsBlocking(int slot, QString& errorMessage);
 signals:
-    /** Emitted in response to detect() when a device is found
+    /**
+     * Emitted when a detection process completes. Use the `detectedSlots`
+     * accessor function to get information on the available slots.
      *
-     * @slot is the slot number detected
+     * @param found - true if a key was found
-     * @blocking signifies if the YK is setup in passive mode or if requires
-     *           the user to touch it for a response
      */
-    void detected(int slot, bool blocking);
+    void detectComplete(bool found);
 
     /**
-     * Emitted when detection is complete
+     * Emitted when user needs to interact with the hardware key to continue
      */
-    void detectComplete();
+    void userInteractionRequest();
 
     /**
-     * Emitted when no Yubikey could be found.
+     * Emitted before/after a challenge-response is performed
      */
-    void notFound();
+    void challengeStarted();
+    void challengeCompleted();
+
+    /**
+     * Emitted when an error occurred during challenge/response
+     */
+    void challengeError(QString error);
 
 private:
     explicit YubiKey();
+    ~YubiKey();
+
     static YubiKey* m_instance;
 
-    // Create void ptr here to avoid ifdef header include mess
+    ChallengeResult
-    void* m_yk_void;
+    performChallenge(void* key, int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response);
-    void* m_ykds_void;
+    bool performTestChallenge(void* key, int slot, bool* wouldBlock);
-    bool m_onlyKey;
+
+    QHash<unsigned int, QList<QPair<int, QString>>> m_foundKeys;
 
     QMutex m_mutex;
+    QTimer m_interactionTimer;
+    bool m_initialized = false;
+    QString m_error;
 
     Q_DISABLE_COPY(YubiKey)
 };

src/keys/drivers/YubiKeyStub.cpp

@@ -16,20 +16,17 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <stdio.h>
-
-#include "core/Global.h"
-#include "crypto/Random.h"
-
 #include "YubiKey.h"
 
 YubiKey::YubiKey()
-    : m_yk_void(NULL)
-    , m_ykds_void(NULL)
 {
 }
 
-YubiKey* YubiKey::m_instance(Q_NULLPTR);
+YubiKey::~YubiKey()
+{
+}
+
+YubiKey* YubiKey::m_instance(nullptr);
 
 YubiKey* YubiKey::instance()
 {
@@ -40,45 +37,43 @@ YubiKey* YubiKey::instance()
     return m_instance;
 }
 
-bool YubiKey::init()
+bool YubiKey::isInitialized()
 {
     return false;
 }
 
-bool YubiKey::deinit()
+void YubiKey::findValidKeys()
-{
-    return false;
-}
-
-void YubiKey::detect()
 {
 }
 
-bool YubiKey::getSerial(unsigned int& serial)
+QList<YubiKeySlot> YubiKey::foundKeys()
 {
-    Q_UNUSED(serial);
+    return {};
-
-    return false;
 }
 
-QString YubiKey::getVendorName()
+QString YubiKey::getDisplayName(YubiKeySlot slot)
-{
+{
-    return "YubiKeyStub";
+    Q_UNUSED(slot);
-}
+    return {};
-
+}
-YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp)
+
+QString YubiKey::errorMessage()
+{
+    return {};
+}
+
+YubiKey::ChallengeResult YubiKey::challenge(YubiKeySlot slot, const QByteArray& chal, QByteArray& resp)
 {
     Q_UNUSED(slot);
-    Q_UNUSED(mayBlock);
     Q_UNUSED(chal);
     Q_UNUSED(resp);
 
     return ERROR;
 }
 
-bool YubiKey::checkSlotIsBlocking(int slot, QString& errorMessage)
+bool YubiKey::testChallenge(YubiKeySlot slot, bool* wouldBlock)
 {
     Q_UNUSED(slot);
-    Q_UNUSED(errorMessage);
+    Q_UNUSED(wouldBlock);
     return false;
 }

tests/CMakeLists.txt

@@ -207,9 +207,11 @@ add_unit_test(NAME testentrysearcher SOURCES TestEntrySearcher.cpp
 add_unit_test(NAME testcsvexporter SOURCES TestCsvExporter.cpp
         LIBS ${TEST_LIBRARIES})
 
-add_unit_test(NAME testykchallengeresponsekey
+if(WITH_XC_YUBIKEY)
+    add_unit_test(NAME testykchallengeresponsekey
         SOURCES TestYkChallengeResponseKey.cpp
         LIBS ${TEST_LIBRARIES})
+endif()
 
 if(WITH_XC_KEESHARE)
     add_unit_test(NAME testsharing SOURCES TestSharing.cpp

tests/TestCli.cpp

@@ -51,6 +51,9 @@
 
 #include <QClipboard>
 #include <QFuture>
+#include <QSet>
+#include <QSignalSpy>
+#include <QTextStream>
 #include <QtConcurrent>
 
 QTEST_MAIN(TestCli)
@@ -1711,26 +1714,46 @@ void TestCli::testInvalidDbFiles()
 /**
  * Secret key for the YubiKey slot used by the unit test is
  * 1c e3 0f d7 8d 20 dc fa 40 b5 0c 18 77 9a fb 0f 02 28 8d b7
- * This secret should be configured at slot 2, and the slot
+ * This secret can be on either slot but must be passive.
- * should be configured as passive.
  */
 void TestCli::testYubiKeyOption()
 {
-    if (!YubiKey::instance()->init()) {
+    if (!YubiKey::instance()->isInitialized()) {
-        QSKIP("Unable to connect to YubiKey");
+        QSKIP("Unable to initialize YubiKey interface.");
     }
 
-    QString errorMessage;
+    YubiKey::instance()->findValidKeys();
-    bool isBlocking = YubiKey::instance()->checkSlotIsBlocking(2, errorMessage);
+
-    if (isBlocking && errorMessage.isEmpty()) {
+    // Wait for the hardware to respond
-        QSKIP("Skipping YubiKey in press mode.");
+    QSignalSpy detected(YubiKey::instance(), SIGNAL(detectComplete(bool)));
+    QTRY_VERIFY_WITH_TIMEOUT(detected.count() > 0, 2000);
+
+    auto keys = YubiKey::instance()->foundKeys();
+    if (keys.isEmpty()) {
+        QSKIP("No YubiKey devices were detected.");
     }
 
+    bool wouldBlock = false;
     QByteArray challenge("CLITest");
     QByteArray response;
-    YubiKey::instance()->challenge(2, false, challenge, response);
     QByteArray expected("\xA2\x3B\x94\x00\xBE\x47\x9A\x30\xA9\xEB\x50\x9B\x85\x56\x5B\x6B\x30\x25\xB4\x8E", 20);
-    QVERIFY2(response == expected, "YubiKey Slot 2 is not configured with correct secret key.");
+
+    // Find a key that as configured for this test
+    YubiKeySlot pKey(0, 0);
+    for (auto key : keys) {
+        if (YubiKey::instance()->testChallenge(key, &wouldBlock) && !wouldBlock) {
+            YubiKey::instance()->challenge(key, challenge, response);
+            if (response == expected) {
+                pKey = key;
+                break;
+            }
+            Tools::wait(100);
+        }
+    }
+
+    if (pKey.first == 0 && pKey.second == 0) {
+        QSKIP("No YubiKey is properly configured to perform this test.");
+    }
 
     List listCmd;
     Add addCmd;

tests/TestYkChallengeResponseKey.cpp

@@ -19,82 +19,74 @@
 
 #include "TestYkChallengeResponseKey.h"
 #include "TestGlobal.h"
+
+#include "core/Tools.h"
 #include "crypto/Crypto.h"
+#include "keys/YkChallengeResponseKey.h"
 
-#include <QtConcurrentRun>
+#include <QScopedPointer>
+#include <QSignalSpy>
 
-QTEST_GUILESS_MAIN(TestYubiKeyChalResp)
+QTEST_GUILESS_MAIN(TestYubiKeyChallengeResponse)
 
-void TestYubiKeyChalResp::initTestCase()
+void TestYubiKeyChallengeResponse::initTestCase()
 {
     // crypto subsystem needs to be initialized for YubiKey testing
     QVERIFY(Crypto::init());
-}
 
-void TestYubiKeyChalResp::init()
+    if (!YubiKey::instance()->isInitialized()) {
-{
+        QSKIP("Unable to initialize YubiKey interface.");
-    if (!YubiKey::instance()->init()) {
-        QSKIP("Unable to connect to YubiKey");
     }
 }
 
-void TestYubiKeyChalResp::detectDevices()
+void TestYubiKeyChallengeResponse::testDetectDevices()
 {
-    connect(YubiKey::instance(), SIGNAL(detected(int, bool)), SLOT(ykDetected(int, bool)), Qt::QueuedConnection);
+    YubiKey::instance()->findValidKeys();
-    QtConcurrent::run(YubiKey::instance(), &YubiKey::detect);
 
-    // need to wait for the hardware (that's hopefully plugged in)...
+    // Wait for the hardware to respond
-    QTest::qWait(2000);
+    QSignalSpy detected(YubiKey::instance(), SIGNAL(detectComplete(bool)));
-    QVERIFY2(m_detected > 0, "Is a YubiKey attached?");
+    QTRY_VERIFY_WITH_TIMEOUT(detected.count() > 0, 2000);
+
+    // Look at the information retrieved from the key(s)
+    for (auto key : YubiKey::instance()->foundKeys()) {
+        auto displayName = YubiKey::instance()->getDisplayName(key);
+        QVERIFY(displayName.contains("Challenge Response - Slot") || displayName.contains("Configured Slot -"));
+        QVERIFY(displayName.contains(QString::number(key.first)));
+        QVERIFY(displayName.contains(QString::number(key.second)));
+    }
 }
 
-void TestYubiKeyChalResp::getSerial()
+/**
+ * Secret key for the YubiKey slot used by the unit test is
+ * 1c e3 0f d7 8d 20 dc fa 40 b5 0c 18 77 9a fb 0f 02 28 8d b7
+ * This secret can be on either slot but must be passive.
+ */
+void TestYubiKeyChallengeResponse::testKeyChallenge()
 {
-    unsigned int serial;
+    auto keys = YubiKey::instance()->foundKeys();
-    QVERIFY(YubiKey::instance()->getSerial(serial));
+    if (keys.isEmpty()) {
-}
+        QSKIP("No YubiKey devices were detected.");
+    }
 
-void TestYubiKeyChalResp::keyGetName()
+    // Find a key that is configured in passive mode
-{
+    bool wouldBlock = false;
-    QVERIFY(m_key);
+    YubiKeySlot pKey(0, 0);
-    QVERIFY(m_key->getName().length() > 0);
+    for (auto key : keys) {
-}
+        if (YubiKey::instance()->testChallenge(key, &wouldBlock) && !wouldBlock) {
+            pKey = key;
+            break;
+        }
+        Tools::wait(100);
+    }
 
-void TestYubiKeyChalResp::keyIssueChallenge()
+    if (pKey.first == 0) {
-{
-    QVERIFY(m_key);
-    if (m_key->isBlocking()) {
         /* Testing active mode in unit tests is unreasonable */
-        QSKIP("YubiKey not in passive mode", SkipSingle);
+        QSKIP("No YubiKey contains a slot in passive mode.");
     }
 
+    QScopedPointer<YkChallengeResponseKey> key(new YkChallengeResponseKey(pKey));
+
     QByteArray ba("UnitTest");
-    QVERIFY(m_key->challenge(ba));
+    QVERIFY(key->challenge(ba));
-
+    QCOMPARE(key->rawKey().size(), 20);
-    /* TODO Determine if it's reasonable to provide a fixed secret key for
-     *      verification testing.  Obviously simple technically, but annoying
-     *      if devs need to re-program their yubikeys or have a spare test key
-     *      for unit tests to pass.
-     *
-     *      Might be worth it for integrity verification though.
-     */
-}
-
-void TestYubiKeyChalResp::ykDetected(int slot, bool blocking)
-{
-    Q_UNUSED(blocking);
-
-    if (slot > 0) {
-        m_detected++;
-    }
-
-    /* Key used for later testing */
-    if (!m_key) {
-        m_key.reset(new YkChallengeResponseKey(slot, blocking));
-    }
-}
-
-void TestYubiKeyChalResp::deinit()
-{
-    QVERIFY(YubiKey::instance()->deinit());
 }

tests/TestYkChallengeResponseKey.h

@@ -20,36 +20,16 @@
 #define KEEPASSX_TESTYUBIKEYCHALRESP_H
 
 #include <QObject>
-#include <QScopedPointer>
 
-#include "keys/YkChallengeResponseKey.h"
+class TestYubiKeyChallengeResponse : public QObject
-
-class TestYubiKeyChalResp : public QObject
 {
     Q_OBJECT
 
 private slots:
     void initTestCase();
 
-    void init();
+    void testDetectDevices();
-
+    void testKeyChallenge();
-    /* Order is important!
-     * Need to init and detectDevices() before proceeding
-     */
-    void detectDevices();
-
-    void getSerial();
-    void keyGetName();
-    void keyIssueChallenge();
-
-    void deinit();
-
-    /* Callback for detectDevices() */
-    void ykDetected(int slot, bool blocking);
-
-private:
-    int m_detected = 0;
-    QScopedPointer<YkChallengeResponseKey> m_key;
 };
 
 #endif // KEEPASSX_TESTYUBIKEYCHALRESP_H