diff options
32 files changed, 646 insertions, 663 deletions
diff --git a/src/cli/Command.cpp b/src/cli/Command.cpp index d352caace..260d9b2b1 100644 --- a/src/cli/Command.cpp +++ b/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("slot"));
+ QObject::tr("Yubikey slot and optional serial used to access the database (e.g., 1:7370001)."),
+ 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)
diff --git a/src/cli/Utils.cpp b/src/cli/Utils.cpp index e8561a1cd..e25ffe02d 100644 --- a/src/cli/Utils.cpp +++ b/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; - bool blocking = YubiKey::instance()->checkSlotIsBlocking(slot, errorMessage); - if (!errorMessage.isEmpty()) { - err << errorMessage << endl; - return {}; + if (parts.size() > 1) { + serial = parts[1].toUInt(&ok, 10); + if (!ok) { + err << QObject::tr("Invalid YubiKey serial %1").arg(parts[1]) << endl; + return {}; + } } auto key = QSharedPointer<YkChallengeResponseKeyCLI>(new YkChallengeResponseKeyCLI( - slot, - blocking, - QObject::tr("Please touch the button on your YubiKey to unlock %1").arg(databaseFilename), - err.device())); + {serial, slot}, QObject::tr("Please touch the button on your YubiKey to continue…"), err)); compositeKey->addChallengeResponseKey(key); } #else diff --git a/src/cli/Utils.h b/src/cli/Utils.h index 2e7ee8e1d..0cd5c0bea 100644 --- a/src/cli/Utils.h +++ b/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; diff --git a/src/core/Alloc.cpp b/src/core/Alloc.cpp index 743ffcdc9..6c798e728 100644 --- a/src/core/Alloc.cpp +++ b/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 { diff --git a/src/core/Database.cpp b/src/core/Database.cpp index a59bf03e9..e634cd754 100644 --- a/src/core/Database.cpp +++ b/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()); - - 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()); + return m_keyError; } QVariantMap& Database::publicCustomData() diff --git a/src/core/Database.h b/src/core/Database.h index 63a1f8cb6..8feb7e56b 100644 --- a/src/core/Database.h +++ b/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; diff --git a/src/format/Kdbx3Reader.cpp b/src/format/Kdbx3Reader.cpp index cce46deb4..002aa8af0 100644 --- a/src/format/Kdbx3Reader.cpp +++ b/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; } diff --git a/src/format/Kdbx3Writer.cpp b/src/format/Kdbx3Writer.cpp index 0728dc294..7d492e4e7 100644 --- a/src/format/Kdbx3Writer.cpp +++ b/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; } diff --git a/src/format/Kdbx4Reader.cpp b/src/format/Kdbx4Reader.cpp index ebdf634a1..f5e7382fe 100644 --- a/src/format/Kdbx4Reader.cpp +++ b/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; } diff --git a/src/format/Kdbx4Writer.cpp b/src/format/Kdbx4Writer.cpp index 03d549cf0..57211248e 100644 --- a/src/format/Kdbx4Writer.cpp +++ b/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; } diff --git a/src/gui/ApplicationSettingsWidget.cpp b/src/gui/ApplicationSettingsWidget.cpp index 9d1f45d49..72905c0b2 100644 --- a/src/gui/ApplicationSettingsWidget.cpp +++ b/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); } diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 1b1c74cd5..cef92d2d4 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/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); - QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); + m_ui->hardwareKeyProgress->setVisible(false); + 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->yubikeyProgress->setVisible(false); + m_ui->challengeResponseCombo->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 - // Don't listen to any Yubikey events if we are hidden - disconnect(YubiKey::instance(), nullptr, this, nullptr); - m_yubiKeyBeingPolled = false; -#endif - - if (isVisible()) { - return; + // Clear the forms if we are minimized + if (!isVisible()) { + clearForms(); } - - 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(); - - // 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)); + auto slot = m_ui->challengeResponseCombo->itemData(selectionIndex).value<YubiKeySlot>(); + auto crKey = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey(slot)); 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() { + if (m_pollingHardwareKey) { + return; + } + + m_ui->challengeResponseCombo->clear(); + m_ui->challengeResponseCombo->addItem(tr("Detecting hardware keys…")); + m_ui->buttonRedetectYubikey->setEnabled(false); - m_ui->comboChallengeResponse->setEnabled(false); - m_ui->comboChallengeResponse->clear(); - m_ui->comboChallengeResponse->addItem(tr("Select slot..."), -1); - m_ui->yubikeyProgress->setVisible(true); + m_ui->challengeResponseCombo->setEnabled(false); + m_ui->hardwareKeyProgress->setVisible(true); + m_pollingHardwareKey = true; - // YubiKey init is slow, detect asynchronously to not block the UI - QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + YubiKey::instance()->findValidKeys(); } -void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) +void DatabaseOpenWidget::hardwareKeyResponse(bool found) { - YkChallengeResponseKey yk(slot, blocking); - // add detected YubiKey to combo box and encode blocking mode in LSB, slot number in second LSB - m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1) | blocking)); + m_ui->challengeResponseCombo->clear(); + m_ui->buttonRedetectYubikey->setEnabled(true); + 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() -{ - m_ui->comboChallengeResponse->setEnabled(true); - m_ui->buttonRedetectYubikey->setEnabled(true); - m_ui->yubikeyProgress->setVisible(false); - m_yubiKeyBeingPolled = false; -} + int selectedIndex = 0; + for (auto& slot : YubiKey::instance()->foundKeys()) { + // add detected YubiKey to combo box + m_ui->challengeResponseCombo->addItem(YubiKey::instance()->getDisplayName(slot), QVariant::fromValue(slot)); + // Select this YubiKey + Slot if we used it in the past + if (lastUsedSlot == slot) { + selectedIndex = m_ui->challengeResponseCombo->count() - 1; + } + } -void DatabaseOpenWidget::noYubikeyFound() -{ - m_ui->buttonRedetectYubikey->setEnabled(true); - m_ui->yubikeyProgress->setVisible(false); - m_yubiKeyBeingPolled = false; + m_ui->challengeResponseCombo->setCurrentIndex(selectedIndex); + m_ui->challengeResponseCombo->setEnabled(true); } void DatabaseOpenWidget::openHardwareKeyHelp() diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index 61a220f43..487bc9d3c 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/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 yubikeyDetectComplete(); - void noYubikeyFound(); + void pollHardwareKey(); + void hardwareKeyResponse(bool found); 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) diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index f89e30ffd..278d45c47 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -6,8 +6,8 @@ <rect> <x>0</x> <y>0</y> - <width>580</width> - <height>410</height> + <width>588</width> + <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/> diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 2b30d6e01..d28f73714 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/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) diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index a94229376..3d129bb94 100644 --- a/src/gui/MainWindow.cpp +++ b/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 diff --git a/src/gui/masterkey/YubiKeyEditWidget.cpp b/src/gui/masterkey/YubiKeyEditWidget.cpp index 9ecd918ef..855bd7099 100644 --- a/src/gui/masterkey/YubiKeyEditWidget.cpp +++ b/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 (!createCrKey(keyPtr, false)) { + if (!m_isDetected || !m_compEditWidget) { 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 (!createCrKey(keyPtr)) { - errorMessage = tr("No YubiKey detected, please ensure it's plugged in."); + if (!m_isDetected) { + errorMessage = tr("Could not find any hardware keys!"); 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 - QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + YubiKey::instance()->findValidKeys(); #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 -} -bool YubiKeyEditWidget::createCrKey(QSharedPointer<YkChallengeResponseKey>& key, bool testChallenge) const -{ - Q_ASSERT(m_compEditWidget); - if (!m_isDetected || !m_compEditWidget) { - return false; + if (!found) { + m_compUi->comboChallengeResponse->addItem(tr("No hardware keys detected")); + m_isDetected = false; + return; } - int selectionIndex = m_compUi->comboChallengeResponse->currentIndex(); - int comboPayload = m_compUi->comboChallengeResponse->itemData(selectionIndex).toInt(); - - if (0 == comboPayload) { - return false; + for (auto& slot : YubiKey::instance()->foundKeys()) { + // 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)); } - 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; + m_isDetected = true; + m_compUi->comboChallengeResponse->setEnabled(true); } diff --git a/src/gui/masterkey/YubiKeyEditWidget.h b/src/gui/masterkey/YubiKeyEditWidget.h index 549f7f44f..d5be0a683 100644 --- a/src/gui/masterkey/YubiKeyEditWidget.h +++ b/src/gui/masterkey/YubiKeyEditWidget.h @@ -45,14 +45,10 @@ protected: void initComponentEditWidget(QWidget* widget) override; private slots: - void yubikeyDetected(int slot, bool blocking); - void yubikeyDetectComplete(); - void noYubikeyFound(); + void hardwareKeyResponse(bool found); 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; diff --git a/src/keys/ChallengeResponseKey.h b/src/keys/ChallengeResponseKey.h index 8d1fa5774..263b507e0 100644 --- a/src/keys/ChallengeResponseKey.h +++ b/src/keys/ChallengeResponseKey.h @@ -29,18 +29,24 @@ public: : m_uuid(uuid) { } - Q_DISABLE_COPY(ChallengeResponseKey); - virtual ~ChallengeResponseKey() - { - } + virtual ~ChallengeResponseKey() = default; + 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; }; diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index 3eb4691cd..23c830f9f 100644 --- a/src/keys/CompositeKey.cpp +++ b/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()); diff --git a/src/keys/CompositeKey.h b/src/keys/CompositeKey.h index c0c77f2f4..865864d16 100644 --- a/src/keys/CompositeKey.h +++ b/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; - bool challenge(const QByteArray& seed, QByteArray& result) const; + + Q_REQUIRED_RESULT bool transform(const Kdf& kdf, QByteArray& result, QString* error = nullptr) 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; }; diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index ecf11fe1c..4bf2ab196 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/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_blocking(blocking) + , m_keySlot(keySlot) { - 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)); } -/** - * Assumes yubikey()->init() was called - */ -bool YkChallengeResponseKey::challenge(const QByteArray& c) -{ - return challenge(c, 2); -} - -bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned int retries) +bool YkChallengeResponseKey::challenge(const QByteArray& challenge) { - do { - --retries; - - if (m_blocking) { - emit userInteractionRequired(); - } - - QByteArray key; - 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; + m_error.clear(); + QByteArray key; + auto result = + AsyncTask::runAndWaitForFuture([&] { return YubiKey::instance()->challenge(m_keySlot, challenge, key); }); + + if (result == YubiKey::SUCCESS) { + if (m_key) { + gcry_free(m_key); } - } 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")); -} + 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(); + } -bool YkChallengeResponseKey::isBlocking() const -{ - return m_blocking; + return result == YubiKey::SUCCESS; } diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index 5f7c40e72..ba213f489 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/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; - bool m_blocking; + YubiKeySlot m_keySlot; }; #endif // KEEPASSX_YK_CHALLENGERESPONSEKEY_H diff --git a/src/keys/YkChallengeResponseKeyCLI.cpp b/src/keys/YkChallengeResponseKeyCLI.cpp index 48420fb8c..4c78e4d0a 100644 --- a/src/keys/YkChallengeResponseKeyCLI.cpp +++ b/src/keys/YkChallengeResponseKeyCLI.cpp @@ -23,48 +23,30 @@ #include "crypto/Random.h" #include <QFile> -#include <QtCore/qglobal.h> QUuid YkChallengeResponseKeyCLI::UUID("e2be77c0-c810-417a-8437-32f41d00bd1d"); -YkChallengeResponseKeyCLI::YkChallengeResponseKeyCLI(int slot, - bool blocking, - QString messageInteraction, - QIODevice* out) +YkChallengeResponseKeyCLI::YkChallengeResponseKeyCLI(YubiKeySlot keySlot, QString interactionMessage, QTextStream& out) : ChallengeResponseKey(UUID) - , m_slot(slot) - , m_blocking(blocking) - , m_messageInteraction(messageInteraction) + , m_keySlot(keySlot) + , m_interactionMessage(interactionMessage) + , m_out(out.device()) { - m_out.setDevice(out); + connect(YubiKey::instance(), SIGNAL(userInteractionRequest()), SLOT(showInteractionMessage())); } -QByteArray YkChallengeResponseKeyCLI::rawKey() const +void YkChallengeResponseKeyCLI::showInteractionMessage() { - return m_key; + m_out << m_interactionMessage << "\n\n" << flush; } -/** - * Assumes yubikey()->init() was called - */ -bool YkChallengeResponseKeyCLI::challenge(const QByteArray& c) +QByteArray YkChallengeResponseKeyCLI::rawKey() const { - return challenge(c, 2); + return m_key; } -bool YkChallengeResponseKeyCLI::challenge(const QByteArray& challenge, unsigned int retries) +bool YkChallengeResponseKeyCLI::challenge(const QByteArray& challenge) { - 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; + auto result = YubiKey::instance()->challenge(m_keySlot, challenge, m_key); + return result == YubiKey::SUCCESS; } diff --git a/src/keys/YkChallengeResponseKeyCLI.h b/src/keys/YkChallengeResponseKeyCLI.h index e1e042d16..56025e7e1 100644 --- a/src/keys/YkChallengeResponseKeyCLI.h +++ b/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; - bool m_blocking; - QString m_messageInteraction; + YubiKeySlot m_keySlot; + QString m_interactionMessage; QTextStream m_out; }; diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 18cf8323a..82773bd56 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -30,17 +30,97 @@ #include "YubiKey.h" -// Cast the void pointer from the generalized class definition -// to the proper pointer type from the now included system headers -#define m_yk (static_cast<YK_KEY*>(m_yk_void)) -#define m_ykds (static_cast<YK_STATUS*>(m_ykds_void)) +#include <QtConcurrent> + +namespace +{ + 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_ykds_void(nullptr) - , m_onlyKey(false) - , m_mutex(QMutex::Recursive) + : 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(); - - // 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(); - } - } + return m_initialized; +} - if (!yk_init()) { - m_mutex.unlock(); - return false; +void YubiKey::findValidKeys() +{ + m_error.clear(); + if (!isInitialized()) { + return; } - // 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; - } + QtConcurrent::run([this] { + if (!m_mutex.tryLock(1000)) { + emit detectComplete(false); + return; + } - 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; - } + // Remove all known keys + m_foundKeys.clear(); + + // Try to detect up to 4 connected hardware keys + for (int i = 0, j = 0; i + j < MAX_KEYS;) { + bool onlyKey = false; + 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(); - return true; + m_mutex.unlock(); + emit detectComplete(!m_foundKeys.isEmpty()); + }); } -bool YubiKey::deinit() +QList<YubiKeySlot> YubiKey::foundKeys() { - m_mutex.lock(); - - if (m_yk) { - yk_close_key(m_yk); - m_yk_void = nullptr; - } - - if (m_ykds) { - ykds_free(m_ykds); - m_ykds_void = nullptr; + QList<YubiKeySlot> keys; + for (auto serial : m_foundKeys.uniqueKeys()) { + for (auto key : m_foundKeys.value(serial)) { + keys.append({serial, key.first}); + } } - - yk_release(); - - m_mutex.unlock(); - - return true; + return keys; } -void YubiKey::detect() +QString YubiKey::getDisplayName(YubiKeySlot slot) { - 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); + for (auto key : m_foundKeys.value(slot.first)) { + if (slot.second == key.first) { + return key.second; } - - // Wait between slots to let the yubikey settle. - Tools::sleep(150); } + return tr("%1 Invalid slot specified - %2").arg(QString::number(slot.first), QString::number(slot.second)); +} - if (!found) { - emit notFound(); - } else { - emit detectComplete(); - } +QString YubiKey::errorMessage() +{ + return m_error; } -bool YubiKey::checkSlotIsBlocking(int slot, QString& errorMessage) +/** + * 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) { - if (!init()) { - errorMessage = QString("Could not initialize YubiKey."); - return false; + bool ret = false; + auto* yk_key = openKeySerial(slot.first); + if (yk_key) { + ret = performTestChallenge(yk_key, slot.second, wouldBlock); } + return ret; +} - YubiKey::ChallengeResult result; - QByteArray rand = randomGen()->randomArray(1); +bool YubiKey::performTestChallenge(void* key, int slot, bool* wouldBlock) +{ + auto chall = randomGen()->randomArray(1); QByteArray resp; - - result = challenge(slot, false, rand, resp); - if (result == ALREADY_RUNNING) { - // Try this slot again after waiting - Tools::sleep(300); - result = challenge(slot, false, rand, resp); - } - - 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; + auto ret = performChallenge(static_cast<YK_KEY*>(key), slot, false, chall, resp); + if (ret == SUCCESS || ret == WOULDBLOCK) { + if (wouldBlock) { + *wouldBlock = ret == WOULDBLOCK; + } + return true; } - - 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(); - int result = yk_get_serial(m_yk, 1, 0, &serial); - m_mutex.unlock(); + m_error.clear(); + if (!m_initialized) { + m_error = tr("The YubiKey interface has not been initialized."); + 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; + } - if (!result) { - return false; + 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; } - return true; -} + emit challengeStarted(); + auto ret = performChallenge(yk_key, slot.second, true, challenge, response); -QString YubiKey::getVendorName() -{ - return m_onlyKey ? "OnlyKey" : "YubiKey"; + closeKey(yk_key); + emit challengeCompleted(); + m_mutex.unlock(); + + return ret; } -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response) +YubiKey::ChallengeResult +YubiKey::performChallenge(void* key, int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response) { - // ensure that YubiKey::init() succeeded - if (!init()) { - return ERROR; - } - + 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 - if (!m_mutex.tryLock(1000)) { - return ALREADY_RUNNING; - } + int ret = yk_challenge_response( + static_cast<YK_KEY*>(key), yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r); - int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r); - m_mutex.unlock(); + // actual HMAC-SHA1 response is only 20 bytes + response.resize(20); if (!ret) { if (yk_errno == YK_EWOULDBLOCK) { return WOULDBLOCK; - } else if (yk_errno == YK_ETIMEOUT) { - return ERROR; } else if (yk_errno) { - - /* Something went wrong, close the key, so that the next call to - * can try to re-open. - * - * Likely caused by the YubiKey being unplugged. - */ - - if (yk_errno == YK_EUSBERR) { - qWarning("USB error: %s", yk_usb_strerror()); + if (yk_errno == YK_ETIMEOUT) { + m_error = tr("Hardware key timed out waiting for user interaction."); + } else if (yk_errno == YK_EUSBERR) { + m_error = tr("A USB error ocurred when accessing the hardware key: %1").arg(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; } diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 7bb527a28..67eff5954 100644 --- a/src/keys/drivers/YubiKey.h +++ b/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, - SUCCESS = 0, - WOULDBLOCK, - ALREADY_RUNNING + ERROR, + SUCCESS, + WOULDBLOCK }; - /** - * @brief YubiKey::instance - get instance of singleton - * @return instance - */ static YubiKey* instance(); + bool isInitialized(); - /** - * @brief YubiKey::init - initialize yubikey library and hardware - * @return true on success - */ - bool init(); + void findValidKeys(); - /** - * @brief YubiKey::deinit - cleanup after init - * @return true on success - */ - bool deinit(); + QList<YubiKeySlot> foundKeys(); + QString getDisplayName(YubiKeySlot slot); - /** - * @brief YubiKey::challenge - issue a challenge - * - * 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); + ChallengeResult challenge(YubiKeySlot slot, const QByteArray& challenge, QByteArray& response); + bool testChallenge(YubiKeySlot slot, bool* wouldBlock = nullptr); - /** - * @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(); + QString errorMessage(); +signals: /** - * @brief YubiKey::detect - probe for attached YubiKeys + * Emitted when a detection process completes. Use the `detectedSlots` + * accessor function to get information on the available slots. + * + * @param found - true if a key was found */ - void detect(); + void detectComplete(bool found); /** - * @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 - * - * @slot is the slot number detected - * @blocking signifies if the YK is setup in passive mode or if requires - * the user to touch it for a response + * Emitted when user needs to interact with the hardware key to continue */ - void detected(int slot, bool blocking); + void userInteractionRequest(); /** - * Emitted when detection is complete + * Emitted before/after a challenge-response is performed */ - void detectComplete(); + void challengeStarted(); + void challengeCompleted(); /** - * Emitted when no Yubikey could be found. + * Emitted when an error occurred during challenge/response */ - void notFound(); + void challengeError(QString error); private: explicit YubiKey(); + ~YubiKey(); + static YubiKey* m_instance; - // Create void ptr here to avoid ifdef header include mess - void* m_yk_void; - void* m_ykds_void; - bool m_onlyKey; + ChallengeResult + performChallenge(void* key, int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response); + bool performTestChallenge(void* key, int slot, bool* wouldBlock); + + 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) }; diff --git a/src/keys/drivers/YubiKeyStub.cpp b/src/keys/drivers/YubiKeyStub.cpp index 1c2fcb8b6..8326974ac 100644 --- a/src/keys/drivers/YubiKeyStub.cpp +++ b/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() +QList<YubiKeySlot> YubiKey::foundKeys() { + return {}; } -bool YubiKey::getSerial(unsigned int& serial) +QString YubiKey::getDisplayName(YubiKeySlot slot) { - Q_UNUSED(serial); - - return false; + Q_UNUSED(slot); + return {}; } -QString YubiKey::getVendorName() +QString YubiKey::errorMessage() { - return "YubiKeyStub"; + return {}; } -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) +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; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3a74622cb..e1bfaac13 100644 --- a/tests/CMakeLists.txt +++ b/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 diff --git a/tests/TestCli.cpp b/tests/TestCli.cpp index 10bde7a08..348afb670 100644 --- a/tests/TestCli.cpp +++ b/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 - * should be configured as passive. + * This secret can be on either slot but must be passive. */ void TestCli::testYubiKeyOption() { - if (!YubiKey::instance()->init()) { - QSKIP("Unable to connect to YubiKey"); + if (!YubiKey::instance()->isInitialized()) { + QSKIP("Unable to initialize YubiKey interface."); } - QString errorMessage; - bool isBlocking = YubiKey::instance()->checkSlotIsBlocking(2, errorMessage); - if (isBlocking && errorMessage.isEmpty()) { - QSKIP("Skipping YubiKey in press mode."); + YubiKey::instance()->findValidKeys(); + + // Wait for the hardware to respond + 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; diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp index a4dd76270..05161544b 100644 --- a/tests/TestYkChallengeResponseKey.cpp +++ b/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()->init()) { - QSKIP("Unable to connect to YubiKey"); + if (!YubiKey::instance()->isInitialized()) { + QSKIP("Unable to initialize YubiKey interface."); } } -void TestYubiKeyChalResp::detectDevices() -{ - connect(YubiKey::instance(), SIGNAL(detected(int, bool)), SLOT(ykDetected(int, bool)), Qt::QueuedConnection); - QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); - - // need to wait for the hardware (that's hopefully plugged in)... - QTest::qWait(2000); - QVERIFY2(m_detected > 0, "Is a YubiKey attached?"); -} - -void TestYubiKeyChalResp::getSerial() -{ - unsigned int serial; - QVERIFY(YubiKey::instance()->getSerial(serial)); -} - -void TestYubiKeyChalResp::keyGetName() -{ - QVERIFY(m_key); - QVERIFY(m_key->getName().length() > 0); -} - -void TestYubiKeyChalResp::keyIssueChallenge() +void TestYubiKeyChallengeResponse::testDetectDevices() { - QVERIFY(m_key); - if (m_key->isBlocking()) { - /* Testing active mode in unit tests is unreasonable */ - QSKIP("YubiKey not in passive mode", SkipSingle); + YubiKey::instance()->findValidKeys(); + + // Wait for the hardware to respond + QSignalSpy detected(YubiKey::instance(), SIGNAL(detectComplete(bool))); + 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))); } - - QByteArray ba("UnitTest"); - QVERIFY(m_key->challenge(ba)); - - /* 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) +/** + * 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() { - Q_UNUSED(blocking); + auto keys = YubiKey::instance()->foundKeys(); + if (keys.isEmpty()) { + QSKIP("No YubiKey devices were detected."); + } - if (slot > 0) { - m_detected++; + // Find a key that is configured in passive mode + bool wouldBlock = false; + YubiKeySlot pKey(0, 0); + for (auto key : keys) { + if (YubiKey::instance()->testChallenge(key, &wouldBlock) && !wouldBlock) { + pKey = key; + break; + } + Tools::wait(100); } - /* Key used for later testing */ - if (!m_key) { - m_key.reset(new YkChallengeResponseKey(slot, blocking)); + if (pKey.first == 0) { + /* Testing active mode in unit tests is unreasonable */ + QSKIP("No YubiKey contains a slot in passive mode."); } -} -void TestYubiKeyChalResp::deinit() -{ - QVERIFY(YubiKey::instance()->deinit()); + QScopedPointer<YkChallengeResponseKey> key(new YkChallengeResponseKey(pKey)); + + QByteArray ba("UnitTest"); + QVERIFY(key->challenge(ba)); + QCOMPARE(key->rawKey().size(), 20); } diff --git a/tests/TestYkChallengeResponseKey.h b/tests/TestYkChallengeResponseKey.h index 81253cc90..63fcaf6ee 100644 --- a/tests/TestYkChallengeResponseKey.h +++ b/tests/TestYkChallengeResponseKey.h @@ -20,36 +20,16 @@ #define KEEPASSX_TESTYUBIKEYCHALRESP_H #include <QObject> -#include <QScopedPointer> -#include "keys/YkChallengeResponseKey.h" - -class TestYubiKeyChalResp : public QObject +class TestYubiKeyChallengeResponse : public QObject { Q_OBJECT private slots: void initTestCase(); - void init(); - - /* 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; + void testDetectDevices(); + void testKeyChallenge(); }; #endif // KEEPASSX_TESTYUBIKEYCHALRESP_H |