Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/keepassxreboot/keepassxc.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJonathan White <support@dmapps.us>2020-04-06 15:42:20 +0300
committerJonathan White <support@dmapps.us>2020-05-15 03:19:56 +0300
commit51429810189cf17501b925712920f9340ec4f4d1 (patch)
tree7ff60339527e885a1924cc525ceb4007f6596f22
parenta145bf91191f0a4630a7e31654aff8a8dfd09bf0 (diff)
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
-rw-r--r--src/cli/Command.cpp15
-rw-r--r--src/cli/Utils.cpp29
-rw-r--r--src/cli/Utils.h6
-rw-r--r--src/core/Alloc.cpp3
-rw-r--r--src/core/Database.cpp26
-rw-r--r--src/core/Database.h3
-rw-r--r--src/format/Kdbx3Reader.cpp2
-rw-r--r--src/format/Kdbx3Writer.cpp2
-rw-r--r--src/format/Kdbx4Reader.cpp2
-rw-r--r--src/format/Kdbx4Writer.cpp2
-rw-r--r--src/gui/ApplicationSettingsWidget.cpp1
-rw-r--r--src/gui/DatabaseOpenWidget.cpp166
-rw-r--r--src/gui/DatabaseOpenWidget.h11
-rw-r--r--src/gui/DatabaseOpenWidget.ui12
-rw-r--r--src/gui/DatabaseWidget.cpp4
-rw-r--r--src/gui/MainWindow.cpp9
-rw-r--r--src/gui/masterkey/YubiKeyEditWidget.cpp93
-rw-r--r--src/gui/masterkey/YubiKeyEditWidget.h6
-rw-r--r--src/keys/ChallengeResponseKey.h14
-rw-r--r--src/keys/CompositeKey.cpp17
-rw-r--r--src/keys/CompositeKey.h8
-rw-r--r--src/keys/YkChallengeResponseKey.cpp81
-rw-r--r--src/keys/YkChallengeResponseKey.h20
-rw-r--r--src/keys/YkChallengeResponseKeyCLI.cpp42
-rw-r--r--src/keys/YkChallengeResponseKeyCLI.h11
-rw-r--r--src/keys/drivers/YubiKey.cpp398
-rw-r--r--src/keys/drivers/YubiKey.h108
-rw-r--r--src/keys/drivers/YubiKeyStub.cpp39
-rw-r--r--tests/CMakeLists.txt4
-rw-r--r--tests/TestCli.cpp43
-rw-r--r--tests/TestYkChallengeResponseKey.cpp106
-rw-r--r--tests/TestYkChallengeResponseKey.h26
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