diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts
index ef75f3e30..f0b442bb0 100644
--- a/share/translations/keepassxc_en.ts
+++ b/share/translations/keepassxc_en.ts
@@ -1664,6 +1664,10 @@ Are you sure you want to continue with this file?.
<a href="#" style="text-decoration: underline">I have a key file</a>
+
+ Hardware keys found, but no slots are configured.
+
+
DatabaseSettingWidgetMetaData
@@ -10230,6 +10234,10 @@ Example: JBSWY3DPEHPK3PXP
<p>If you own a <a href="https://www.yubico.com/">YubiKey</a> or <a href="https://onlykey.io">OnlyKey</a>, you can use it for additional security.</p><p>The key requires one of its slots to be programmed with <a href="https://keepassxc.org/docs/#faq-yubikey-howto">Challenge-Response</a>.</p>
+
+ Hardware keys found, but no slots are configured
+
+
YubiKeyInterfacePCSC
diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp
index 7f633c7fd..11179da83 100644
--- a/src/gui/DatabaseOpenWidget.cpp
+++ b/src/gui/DatabaseOpenWidget.cpp
@@ -102,7 +102,7 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
m_ui->hardwareKeyProgress->setSizePolicy(sp);
#ifdef WITH_XC_YUBIKEY
- connect(m_deviceListener, SIGNAL(devicePlugged(bool, void*, void*)), this, SLOT(pollHardwareKey()));
+ connect(m_deviceListener, &DeviceListener::devicePlugged, this, [this] { pollHardwareKey(false, 500); });
connect(YubiKey::instance(), SIGNAL(detectComplete(bool)), SLOT(hardwareKeyResponse(bool)), Qt::QueuedConnection);
connect(YubiKey::instance(), &YubiKey::userInteractionRequest, this, [this] {
@@ -140,7 +140,12 @@ void DatabaseOpenWidget::toggleHardwareKeyComponent(bool state)
m_ui->hardwareKeyProgress->setVisible(false);
m_ui->hardwareKeyComponent->setVisible(state);
m_ui->hardwareKeyCombo->setVisible(state && m_ui->hardwareKeyCombo->count() != 1);
+
m_ui->noHardwareKeysFoundLabel->setVisible(!state && m_manualHardwareKeyRefresh);
+ m_ui->noHardwareKeysFoundLabel->setText(YubiKey::instance()->connectedKeys() > 0
+ ? tr("Hardware keys found, but no slots are configured.")
+ : tr("No hardware keys found."));
+
if (!state) {
m_ui->useHardwareKeyCheckBox->setChecked(false);
}
@@ -520,7 +525,7 @@ bool DatabaseOpenWidget::browseKeyFile()
return true;
}
-void DatabaseOpenWidget::pollHardwareKey(bool manualTrigger)
+void DatabaseOpenWidget::pollHardwareKey(bool manualTrigger, int delay)
{
if (m_pollingHardwareKey) {
return;
@@ -534,9 +539,6 @@ void DatabaseOpenWidget::pollHardwareKey(bool manualTrigger)
m_pollingHardwareKey = true;
m_manualHardwareKeyRefresh = manualTrigger;
- // Add a delay, if this is an automatic trigger, to allow the USB device to settle as
- // the device may not report a valid serial number immediately after plugging in
- int delay = manualTrigger ? 0 : 500;
QTimer::singleShot(delay, this, [] { YubiKey::instance()->findValidKeysAsync(); });
}
diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h
index f75e118de..e0a250e3d 100644
--- a/src/gui/DatabaseOpenWidget.h
+++ b/src/gui/DatabaseOpenWidget.h
@@ -79,7 +79,7 @@ protected slots:
private slots:
bool browseKeyFile();
void toggleHardwareKeyComponent(bool state);
- void pollHardwareKey(bool manualTrigger = false);
+ void pollHardwareKey(bool manualTrigger = false, int delay = 0);
void hardwareKeyResponse(bool found);
private:
diff --git a/src/gui/databasekey/YubiKeyEditWidget.cpp b/src/gui/databasekey/YubiKeyEditWidget.cpp
index d4215bd34..edac12af0 100644
--- a/src/gui/databasekey/YubiKeyEditWidget.cpp
+++ b/src/gui/databasekey/YubiKeyEditWidget.cpp
@@ -173,7 +173,9 @@ void YubiKeyEditWidget::hardwareKeyResponse(bool found)
if (!found) {
m_compUi->yubikeyProgress->setVisible(false);
- m_compUi->comboChallengeResponse->addItem(tr("No hardware keys detected"));
+ m_compUi->comboChallengeResponse->addItem(YubiKey::instance()->connectedKeys() > 0
+ ? tr("Hardware keys found, but no slots are configured")
+ : tr("No hardware keys detected"));
m_isDetected = false;
return;
}
diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp
index 7dce9903b..8d726727e 100644
--- a/src/keys/drivers/YubiKey.cpp
+++ b/src/keys/drivers/YubiKey.cpp
@@ -75,8 +75,9 @@ bool YubiKey::findValidKeys()
{
QMutexLocker lock(&s_interfaceMutex);
- m_usbKeys = YubiKeyInterfaceUSB::instance()->findValidKeys();
- m_pcscKeys = YubiKeyInterfacePCSC::instance()->findValidKeys();
+ m_connectedKeys = 0;
+ m_usbKeys = YubiKeyInterfaceUSB::instance()->findValidKeys(m_connectedKeys);
+ m_pcscKeys = YubiKeyInterfacePCSC::instance()->findValidKeys(m_connectedKeys);
return !m_usbKeys.isEmpty() || !m_pcscKeys.isEmpty();
}
@@ -89,19 +90,17 @@ void YubiKey::findValidKeysAsync()
YubiKey::KeyMap YubiKey::foundKeys()
{
QMutexLocker lock(&s_interfaceMutex);
- KeyMap foundKeys;
-
- for (auto i = m_usbKeys.cbegin(); i != m_usbKeys.cend(); ++i) {
- foundKeys.insert(i.key(), i.value());
- }
-
- for (auto i = m_pcscKeys.cbegin(); i != m_pcscKeys.cend(); ++i) {
- foundKeys.insert(i.key(), i.value());
- }
+ KeyMap foundKeys = m_usbKeys;
+ foundKeys.unite(m_pcscKeys);
return foundKeys;
}
+int YubiKey::connectedKeys()
+{
+ return m_connectedKeys;
+}
+
QString YubiKey::errorMessage()
{
QMutexLocker lock(&s_interfaceMutex);
diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h
index 0edd4c8a7..a36f8c30f 100644
--- a/src/keys/drivers/YubiKey.h
+++ b/src/keys/drivers/YubiKey.h
@@ -38,7 +38,8 @@ class YubiKey : public QObject
Q_OBJECT
public:
- typedef QMap KeyMap;
+ using KeyMap = QMap;
+
enum class ChallengeResult : int
{
YCR_ERROR = 0,
@@ -53,6 +54,7 @@ public:
void findValidKeysAsync();
KeyMap foundKeys();
+ int connectedKeys();
ChallengeResult challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector& response);
bool testChallenge(YubiKeySlot slot, bool* wouldBlock = nullptr);
@@ -93,6 +95,8 @@ private:
KeyMap m_usbKeys;
KeyMap m_pcscKeys;
+ int m_connectedKeys = 0;
+
Q_DISABLE_COPY(YubiKey)
};
diff --git a/src/keys/drivers/YubiKeyInterface.h b/src/keys/drivers/YubiKeyInterface.h
index 51b4ae846..276f520a6 100644
--- a/src/keys/drivers/YubiKeyInterface.h
+++ b/src/keys/drivers/YubiKeyInterface.h
@@ -32,7 +32,7 @@ class YubiKeyInterface : public QObject
public:
bool isInitialized() const;
- virtual YubiKey::KeyMap findValidKeys() = 0;
+ virtual YubiKey::KeyMap findValidKeys(int& connectedKeys) = 0;
virtual YubiKey::ChallengeResult
challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector& response) = 0;
virtual bool testChallenge(YubiKeySlot slot, bool* wouldBlock) = 0;
diff --git a/src/keys/drivers/YubiKeyInterfacePCSC.cpp b/src/keys/drivers/YubiKeyInterfacePCSC.cpp
index 8ae728fc2..765d3d0d8 100644
--- a/src/keys/drivers/YubiKeyInterfacePCSC.cpp
+++ b/src/keys/drivers/YubiKeyInterfacePCSC.cpp
@@ -20,6 +20,8 @@
#include "core/Tools.h"
#include "crypto/Random.h"
+#include
+
// MSYS2 does not define these macros
// So set them to the value used by pcsc-lite
#ifndef MAX_ATR_SIZE
@@ -511,7 +513,6 @@ namespace
return rv;
});
}
-
} // namespace
YubiKeyInterfacePCSC::YubiKeyInterfacePCSC()
@@ -542,7 +543,7 @@ YubiKeyInterfacePCSC* YubiKeyInterfacePCSC::instance()
return m_instance;
}
-YubiKey::KeyMap YubiKeyInterfacePCSC::findValidKeys()
+YubiKey::KeyMap YubiKeyInterfacePCSC::findValidKeys(int& connectedKeys)
{
m_error.clear();
if (!isInitialized()) {
@@ -578,6 +579,8 @@ YubiKey::KeyMap YubiKeyInterfacePCSC::findValidKeys()
continue;
}
+ auto finally = qScopeGuard([hCard]() { SCardDisconnect(hCard, SCARD_LEAVE_CARD); });
+
// Read the protocol and the ATR record
char pbReader[MAX_READERNAME] = {0};
SCUINT dwReaderLen = sizeof(pbReader);
@@ -605,6 +608,8 @@ YubiKey::KeyMap YubiKeyInterfacePCSC::findValidKeys()
unsigned int serial = 0;
getSerial(satr, serial);
+ ++connectedKeys;
+
/* This variable indicates that the key is locked / timed out.
When using the key via NFC, the user has to re-present the key to clear the timeout.
Also, the key can be programmatically reset (see below).
diff --git a/src/keys/drivers/YubiKeyInterfacePCSC.h b/src/keys/drivers/YubiKeyInterfacePCSC.h
index df4f25ba3..b91efe38e 100644
--- a/src/keys/drivers/YubiKeyInterfacePCSC.h
+++ b/src/keys/drivers/YubiKeyInterfacePCSC.h
@@ -52,7 +52,7 @@ class YubiKeyInterfacePCSC : public YubiKeyInterface
public:
static YubiKeyInterfacePCSC* instance();
- YubiKey::KeyMap findValidKeys() override;
+ YubiKey::KeyMap findValidKeys(int& connectedKeys) override;
YubiKey::ChallengeResult
challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector& response) override;
diff --git a/src/keys/drivers/YubiKeyInterfaceUSB.cpp b/src/keys/drivers/YubiKeyInterfaceUSB.cpp
index fde232d6f..a39b39dce 100644
--- a/src/keys/drivers/YubiKeyInterfaceUSB.cpp
+++ b/src/keys/drivers/YubiKeyInterfaceUSB.cpp
@@ -18,7 +18,6 @@
#include "YubiKeyInterfaceUSB.h"
-#include "core/Tools.h"
#include "crypto/Random.h"
#include "thirdparty/ykcore/ykcore.h"
#include "thirdparty/ykcore/ykstatus.h"
@@ -27,7 +26,15 @@ namespace
{
constexpr int MAX_KEYS = 4;
- YK_KEY* openKey(int index)
+ void closeKey(YK_KEY* key)
+ {
+ yk_close_key(key);
+ }
+
+ using Yubikey = std::unique_ptr;
+ using YubikeyStatus = std::unique_ptr;
+
+ Yubikey openKey(int index)
{
static const int vids[] = {YUBICO_VID, ONLYKEY_VID};
static const int pids[] = {YUBIKEY_PID,
@@ -42,12 +49,9 @@ namespace
PLUS_U2F_OTP_PID,
ONLYKEY_PID};
- return yk_open_key_vid_pid(vids, sizeof(vids) / sizeof(vids[0]), pids, sizeof(pids) / sizeof(pids[0]), index);
- }
-
- void closeKey(YK_KEY* key)
- {
- yk_close_key(key);
+ return Yubikey{
+ yk_open_key_vid_pid(vids, sizeof(vids) / sizeof(vids[0]), pids, sizeof(pids) / sizeof(pids[0]), index),
+ &closeKey};
}
void printError()
@@ -69,24 +73,25 @@ namespace
return serial;
}
- YK_KEY* openKeySerial(unsigned int serial)
+ Yubikey openKeySerial(unsigned int serial)
{
for (int i = 0; i < MAX_KEYS; ++i) {
- auto* yk_key = openKey(i);
- if (yk_key) {
+ auto key = openKey(i);
+ if (key) {
// 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;
+ if (serial == 0 || getSerial(key.get()) == serial) {
+ return key;
}
- closeKey(yk_key);
} else if (yk_errno == YK_ENOKEY) {
// No more connected keys
break;
}
printError();
}
- return nullptr;
+
+ return Yubikey{nullptr, &closeKey};
}
+
} // namespace
YubiKeyInterfaceUSB::YubiKeyInterfaceUSB()
@@ -114,7 +119,7 @@ YubiKeyInterfaceUSB* YubiKeyInterfaceUSB::instance()
return m_instance;
}
-YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys()
+YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys(int& connectedKeys)
{
m_error.clear();
if (!isInitialized()) {
@@ -127,29 +132,30 @@ YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys()
for (int i = 0; i < MAX_KEYS; ++i) {
auto yk_key = openKey(i);
if (yk_key) {
- auto serial = getSerial(yk_key);
+ auto serial = getSerial(yk_key.get());
if (serial == 0) {
- closeKey(yk_key);
continue;
}
- auto st = ykds_alloc();
- yk_get_status(yk_key, st);
+ ++connectedKeys;
+
+ YubikeyStatus st{ykds_alloc(), &ykds_free};
+ yk_get_status(yk_key.get(), st.get());
int vid, pid;
- yk_get_key_vid_pid(yk_key, &vid, &pid);
+ yk_get_key_vid_pid(yk_key.get(), &vid, &pid);
QString name = m_pid_names.value(pid, tr("Unknown"));
if (vid == ONLYKEY_VID) {
name = QStringLiteral("OnlyKey %ver");
}
if (name.contains("%ver")) {
- name = name.replace("%ver", QString::number(ykds_version_major(st)));
+ name = name.replace("%ver", QString::number(ykds_version_major(st.get())));
}
bool wouldBlock;
for (int slot = 1; slot <= 2; ++slot) {
auto config = (slot == 1 ? CONFIG1_VALID : CONFIG2_VALID);
- if (!(ykds_touch_level(st) & config)) {
+ if (!(ykds_touch_level(st.get()) & config)) {
// Slot is not configured
continue;
}
@@ -159,7 +165,7 @@ YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys()
auto display = tr("%1 [%2] - Slot %3", "YubiKey NEO display fields")
.arg(name, QString::number(serial), QString::number(slot));
keyMap.insert({serial, slot}, display);
- } else if (performTestChallenge(yk_key, slot, &wouldBlock)) {
+ } else if (performTestChallenge(yk_key.get(), slot, &wouldBlock)) {
auto display =
tr("%1 [%2] - Slot %3, %4", "YubiKey display fields")
.arg(name,
@@ -170,9 +176,6 @@ YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys()
keyMap.insert({serial, slot}, display);
}
}
-
- ykds_free(st);
- closeKey(yk_key);
} else if (yk_errno == YK_ENOKEY) {
// No more keys are connected
break;
@@ -197,11 +200,11 @@ YubiKey::KeyMap YubiKeyInterfaceUSB::findValidKeys()
bool YubiKeyInterfaceUSB::testChallenge(YubiKeySlot slot, bool* wouldBlock)
{
bool ret = false;
- auto* yk_key = openKeySerial(slot.first);
+ auto yk_key = openKeySerial(slot.first);
if (yk_key) {
- ret = performTestChallenge(yk_key, slot.second, wouldBlock);
+ ret = performTestChallenge(yk_key.get(), slot.second, wouldBlock);
}
- closeKey(yk_key);
+
return ret;
}
@@ -237,7 +240,7 @@ YubiKeyInterfaceUSB::challenge(YubiKeySlot slot, const QByteArray& challenge, Bo
return YubiKey::ChallengeResult::YCR_ERROR;
}
- auto* yk_key = openKeySerial(slot.first);
+ auto yk_key = openKeySerial(slot.first);
if (!yk_key) {
// Key with specified serial number is not connected
m_error =
@@ -246,9 +249,8 @@ YubiKeyInterfaceUSB::challenge(YubiKeySlot slot, const QByteArray& challenge, Bo
}
emit challengeStarted();
- auto ret = performChallenge(yk_key, slot.second, true, challenge, response);
+ auto ret = performChallenge(yk_key.get(), slot.second, true, challenge, response);
- closeKey(yk_key);
emit challengeCompleted();
return ret;
diff --git a/src/keys/drivers/YubiKeyInterfaceUSB.h b/src/keys/drivers/YubiKeyInterfaceUSB.h
index 07c8118b7..9d5835f07 100644
--- a/src/keys/drivers/YubiKeyInterfaceUSB.h
+++ b/src/keys/drivers/YubiKeyInterfaceUSB.h
@@ -35,7 +35,7 @@ public:
static constexpr int YUBICO_USB_VID = YUBICO_VID;
static constexpr int ONLYKEY_USB_VID = ONLYKEY_VID;
- YubiKey::KeyMap findValidKeys() override;
+ YubiKey::KeyMap findValidKeys(int& connectedKeys) override;
YubiKey::ChallengeResult
challenge(YubiKeySlot slot, const QByteArray& challenge, Botan::secure_vector& response) override;
diff --git a/src/keys/drivers/YubiKeyStub.cpp b/src/keys/drivers/YubiKeyStub.cpp
index 04e7cb118..64771e77d 100644
--- a/src/keys/drivers/YubiKeyStub.cpp
+++ b/src/keys/drivers/YubiKeyStub.cpp
@@ -50,6 +50,11 @@ YubiKey::KeyMap YubiKey::foundKeys()
return {};
}
+int YubiKey::connectedKeys()
+{
+ return 0;
+}
+
QString YubiKey::errorMessage()
{
return {};
diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp
index c1064518e..02a0b5bea 100644
--- a/tests/TestYkChallengeResponseKey.cpp
+++ b/tests/TestYkChallengeResponseKey.cpp
@@ -24,6 +24,7 @@
#include "keys/ChallengeResponseKey.h"
#include
+#include
#include
#include
@@ -45,9 +46,12 @@ void TestYubiKeyChallengeResponse::testDetectDevices()
// Look at the information retrieved from the key(s)
const auto foundKeys = YubiKey::instance()->foundKeys();
+ QRegularExpression exp{"\\w+\\s+\\[\\d+\\]\\s+-\\s+Slot\\s+\\d"};
+
for (auto i = foundKeys.cbegin(); i != foundKeys.cend(); ++i) {
const auto& displayName = i.value();
- QVERIFY(displayName.contains("Challenge-Response - Slot") || displayName.contains("Configured Slot -"));
+ auto match = exp.match(displayName);
+ QVERIFY(match.hasMatch());
QVERIFY(displayName.contains(QString::number(i.key().first)));
QVERIFY(displayName.contains(QString::number(i.key().second)));
}