Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2025-01-12 07:49:55 -05:00
Code Issues Packages Projects Releases Wiki Activity

Show a clear error if no slots on hardware key(s) are configured (#11609)

Browse Source 
Fixes #11543

Also fix delayed polling on window activation

---------

Co-authored-by: w15dev <w15developer@proton.me>
Co-authored-by: Jonathan White <support@dmapps.us>
This commit is contained in:
Kuznetsov Oleg 2025-01-03 07:05:22 +03:00 committed by GitHub
parent 2afec91e87
commit 9e29b5c7b6
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 89 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
share/translations/keepassxc_en.ts
View File

@ -1664,6 +1664,10 @@ Are you sure you want to continue with this file?.</source>
<source>&lt;a href=&quot;#&quot; style=&quot;text-decoration: underline&quot;&gt;I have a key file&lt;/a&gt;</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Hardware keys found, but no slots are configured.</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>DatabaseSettingWidgetMetaData</name>
@ -10230,6 +10234,10 @@ Example: JBSWY3DPEHPK3PXP</source>
<source>&lt;p&gt;If you own a &lt;a href=&quot;https://www.yubico.com/&quot;&gt;YubiKey&lt;/a&gt; or &lt;a href=&quot;https://onlykey.io&quot;&gt;OnlyKey&lt;/a&gt;, you can use it for additional security.&lt;/p&gt;&lt;p&gt;The key requires one of its slots to be programmed with &lt;a href=&quot;https://keepassxc.org/docs/#faq-yubikey-howto&quot;&gt;Challenge-Response&lt;/a&gt;.&lt;/p&gt;</source>
<translation type="unfinished"></translation>
</message>
<message>
<source>Hardware keys found, but no slots are configured</source>
<translation type="unfinished"></translation>
</message>
</context>
<context>
<name>YubiKeyInterfacePCSC</name>

12
src/gui/DatabaseOpenWidget.cpp
View File

@ -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(); });
}

2
src/gui/DatabaseOpenWidget.h
View File

@ -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:

4
src/gui/databasekey/YubiKeyEditWidget.cpp
View File

@ -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;
}

21
src/keys/drivers/YubiKey.cpp
View File

@ -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);

6
src/keys/drivers/YubiKey.h
View File

@ -38,7 +38,8 @@ class YubiKey : public QObject
Q_OBJECT
public:
typedef QMap<YubiKeySlot, QString> KeyMap;
using KeyMap = QMap<YubiKeySlot, QString>;
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<char>& 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)
};

2
src/keys/drivers/YubiKeyInterface.h
View File

@ -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<char>& response) = 0;
virtual bool testChallenge(YubiKeySlot slot, bool* wouldBlock) = 0;

9
src/keys/drivers/YubiKeyInterfacePCSC.cpp
View File

@ -20,6 +20,8 @@
#include "core/Tools.h"
#include "crypto/Random.h"
#include <QScopeGuard>
// 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).

2
src/keys/drivers/YubiKeyInterfacePCSC.h
View File

@ -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<char>& response) override;

68
src/keys/drivers/YubiKeyInterfaceUSB.cpp
View File

@ -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<YK_KEY, decltype(&closeKey)>;
using YubikeyStatus = std::unique_ptr<YK_STATUS, decltype(&ykds_free)>;
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;

2
src/keys/drivers/YubiKeyInterfaceUSB.h
View File

@ -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<char>& response) override;

5
src/keys/drivers/YubiKeyStub.cpp
View File

@ -50,6 +50,11 @@ YubiKey::KeyMap YubiKey::foundKeys()
return {};
}
int YubiKey::connectedKeys()
{
return 0;
}
QString YubiKey::errorMessage()
{
return {};

6
tests/TestYkChallengeResponseKey.cpp
View File

@ -24,6 +24,7 @@
#include "keys/ChallengeResponseKey.h"
#include <QCryptographicHash>
#include <QRegularExpression>
#include <QSignalSpy>
#include <QTest>
@ -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)));
}