Use QSharedPointer instead of cloning YkChallengeResponseKey and make it a QObject to allow emitting signals

src/core/Database.cpp

@@ -257,18 +257,16 @@ bool Database::verifyKey(const CompositeKey& key) const
 {
     Q_ASSERT(hasKey());
 
-    /* If the database has challenge response keys, then the the verification
-     * key better as well */
     if (!m_data.challengeResponseKey.isEmpty()) {
         QByteArray result;
 
         if (!key.challenge(m_data.masterSeed, result)) {
-            /* Challenge failed, (YubiKey?) removed? */
+            // challenge failed, (YubiKey?) removed?
             return false;
         }
 
         if (m_data.challengeResponseKey != result) {
-            /* Wrong response from challenged device(s) */
+            // wrong response from challenged device(s)
             return false;
         }
     }

src/format/KeePass2Reader.cpp

@@ -115,7 +115,7 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke
 
     if (m_db->challengeMasterSeed(m_masterSeed) == false) {
         raiseError(tr("Unable to issue challenge-response."));
-        return Q_NULLPTR;
+        return nullptr;
     }
 
     CryptoHash hash(CryptoHash::Sha256);

src/gui/ChangeMasterKeyWidget.cpp

@@ -15,8 +15,6 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <QtConcurrentRun>
-
 #include "ChangeMasterKeyWidget.h"
 #include "ui_ChangeMasterKeyWidget.h"
 
@@ -30,6 +28,9 @@
 
 #include "config-keepassx.h"
 
+#include <QtConcurrentRun>
+#include <QSharedPointer>
+
 ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent)
     : DialogyWidget(parent)
     , m_ui(new Ui::ChangeMasterKeyWidget())
@@ -172,9 +173,9 @@ void ChangeMasterKeyWidget::generateKey()
                                              MessageWidget::Error);
             return;
         }
-
-        YkChallengeResponseKey key(i);
-
+        bool blocking = i & true;
+        int slot      = i >> 1;
+        auto key      = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey(slot, blocking));
         m_key.addChallengeResponseKey(key);
     }
 #endif
@@ -210,7 +211,7 @@ void ChangeMasterKeyWidget::pollYubikey()
 void ChangeMasterKeyWidget::yubikeyDetected(int slot, bool blocking)
 {
     YkChallengeResponseKey yk(slot, blocking);
-    m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant(slot));
+    m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1) | blocking));
     m_ui->comboChallengeResponse->setEnabled(m_ui->challengeResponseGroup->isChecked());
     m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked());
     m_ui->yubikeyProgress->setVisible(false);

src/gui/DatabaseOpenWidget.cpp

@@ -15,8 +15,6 @@
  *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include <QtConcurrentRun>
-
 #include "DatabaseOpenWidget.h"
 #include "ui_DatabaseOpenWidget.h"
 
@@ -34,6 +32,9 @@
 
 #include "config-keepassx.h"
 
+#include <QtConcurrentRun>
+#include <QSharedPointer>
+
 
 DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
     : DialogyWidget(parent),
@@ -156,7 +157,7 @@ void DatabaseOpenWidget::openDatabase()
         if (m_ui->messageWidget->isVisible()) {
             m_ui->messageWidget->animatedHide();
         }
-        Q_EMIT editFinished(true);
+        emit editFinished(true);
     }
     else {
         m_ui->messageWidget->showMessage(tr("Unable to open the database.")
@@ -209,8 +210,10 @@ CompositeKey DatabaseOpenWidget::databaseKey()
     if (m_ui->checkChallengeResponse->isChecked()) {
         int i = m_ui->comboChallengeResponse->currentIndex();
         i = m_ui->comboChallengeResponse->itemData(i).toInt();
-        YkChallengeResponseKey key(i);
 
+        bool blocking = i & true;
+        int slot      = i >> 1;
+        auto key      = QSharedPointer<YkChallengeResponseKey>(new YkChallengeResponseKey(slot, blocking));
         masterKey.addChallengeResponseKey(key);
     }
 #endif
@@ -250,20 +253,21 @@ void DatabaseOpenWidget::browseKeyFile()
 
 void DatabaseOpenWidget::pollYubikey()
 {
-    // YubiKey init is slow, detect asynchronously to not block the UI
     m_ui->buttonRedetectYubikey->setEnabled(false);
     m_ui->checkChallengeResponse->setEnabled(false);
     m_ui->checkChallengeResponse->setChecked(false);
     m_ui->comboChallengeResponse->setEnabled(false);
     m_ui->comboChallengeResponse->clear();
     m_ui->yubikeyProgress->setVisible(true);
+
+    // YubiKey init is slow, detect asynchronously to not block the UI
     QtConcurrent::run(YubiKey::instance(), &YubiKey::detect);
 }
 
 void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking)
 {
     YkChallengeResponseKey yk(slot, blocking);
-    m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant(slot));
+    m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1) | blocking));
     m_ui->comboChallengeResponse->setEnabled(true);
     m_ui->checkChallengeResponse->setEnabled(true);
     m_ui->buttonRedetectYubikey->setEnabled(true);

src/keys/ChallengeResponseKey.h

@@ -25,7 +25,6 @@ class ChallengeResponseKey
 public:
     virtual ~ChallengeResponseKey() {}
     virtual QByteArray rawKey() const = 0;
-    virtual ChallengeResponseKey* clone() const = 0;
     virtual bool challenge(const QByteArray& challenge) = 0;
 };
 

src/keys/CompositeKey.cpp

@@ -46,7 +46,6 @@ CompositeKey::~CompositeKey()
 void CompositeKey::clear()
 {
     qDeleteAll(m_keys);
-    qDeleteAll(m_challengeResponseKeys);
     m_keys.clear();
     m_challengeResponseKeys.clear();
 }
@@ -73,8 +72,8 @@ CompositeKey& CompositeKey::operator=(const CompositeKey& key)
     for (const Key* subKey : asConst(key.m_keys)) {
         addKey(*subKey);
     }
-    for (const ChallengeResponseKey* subKey : asConst(key.m_challengeResponseKeys)) {
-        addChallengeResponseKey(*subKey);
+    for (const auto subKey : asConst(key.m_challengeResponseKeys)) {
+        addChallengeResponseKey(subKey);
     }
 
     return *this;
@@ -176,9 +175,8 @@ QByteArray CompositeKey::transformKeyRaw(const QByteArray& key, const QByteArray
 
 bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
 {
-    /* If no challenge response was requested, return nothing to
-     * maintain backwards compatability with regular databases.
-     */
+    // if no challenge response was requested, return nothing to
+    // maintain backwards compatibility with regular databases.
     if (m_challengeResponseKeys.length() == 0) {
         result.clear();
         return true;
@@ -186,9 +184,9 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const
 
     CryptoHash cryptoHash(CryptoHash::Sha256);
 
-    for (ChallengeResponseKey* key : m_challengeResponseKeys) {
-        /* If the device isn't present or fails, return an error */
-        if (key->challenge(seed) == false) {
+    for (const auto key : m_challengeResponseKeys) {
+        // if the device isn't present or fails, return an error
+        if (!key->challenge(seed)) {
             return false;
         }
         cryptoHash.addData(key->rawKey());
@@ -203,11 +201,12 @@ void CompositeKey::addKey(const Key& key)
     m_keys.append(key.clone());
 }
 
-void CompositeKey::addChallengeResponseKey(const ChallengeResponseKey& key)
+void CompositeKey::addChallengeResponseKey(QSharedPointer<ChallengeResponseKey> key)
 {
-    m_challengeResponseKeys.append(key.clone());
+    m_challengeResponseKeys.append(key);
 }
 
+
 int CompositeKey::transformKeyBenchmark(int msec)
 {
     TransformKeyBenchmarkThread thread1(msec);

src/keys/CompositeKey.h

@@ -20,6 +20,7 @@
 
 #include <QList>
 #include <QString>
+#include <QSharedPointer>
 
 #include "keys/Key.h"
 #include "keys/ChallengeResponseKey.h"
@@ -41,7 +42,7 @@ public:
     bool challenge(const QByteArray& seed, QByteArray &result) const;
 
     void addKey(const Key& key);
-    void addChallengeResponseKey(const ChallengeResponseKey& key);
+    void addChallengeResponseKey(QSharedPointer<ChallengeResponseKey> key);
 
     static int transformKeyBenchmark(int msec);
     static CompositeKey readFromLine(QString line);
@@ -51,7 +52,7 @@ private:
                                       quint64 rounds, bool* ok, QString* errorString);
 
     QList<Key*> m_keys;
-    QList<ChallengeResponseKey*> m_challengeResponseKeys;
+    QList<QSharedPointer<ChallengeResponseKey>> m_challengeResponseKeys;
 };
 
 #endif // KEEPASSX_COMPOSITEKEY_H

src/keys/YkChallengeResponseKey.cpp

@@ -15,7 +15,6 @@
 *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-
 #include <QFile>
 #include <QXmlStreamReader>
 
@@ -26,11 +25,7 @@
 #include "keys/YkChallengeResponseKey.h"
 #include "keys/drivers/YubiKey.h"
 
-#include <QDebug>
-#include <QObject>
-
-YkChallengeResponseKey::YkChallengeResponseKey(int slot,
-                                               bool blocking)
+YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking)
     : m_slot(slot),
       m_blocking(blocking)
 {
@@ -41,40 +36,41 @@ QByteArray YkChallengeResponseKey::rawKey() const
     return m_key;
 }
 
-YkChallengeResponseKey* YkChallengeResponseKey::clone() const
-{
-    return new YkChallengeResponseKey(*this);
-}
-
-
-/** Assumes yubikey()->init() was called */
+/**
+ * Assumes yubikey()->init() was called
+ */
 bool YkChallengeResponseKey::challenge(const QByteArray& chal)
 {
     return challenge(chal, 1);
 }
-
-bool YkChallengeResponseKey::challenge(const QByteArray& chal, int retries)
+#include <QDebug>
+bool YkChallengeResponseKey::challenge(const QByteArray& chal, unsigned retries)
 {
-    if (YubiKey::instance()->challenge(m_slot, true, chal, m_key) != YubiKey::ERROR) {
-        return true;
-    }
+    Q_ASSERT(retries > 0);
 
-    /* If challenge failed, retry to detect YubiKeys in the event the YubiKey
-     *  was un-plugged and re-plugged */
-    while (retries > 0) {
-#ifdef QT_DEBUG
-        qDebug() << "Attempt" << retries << "to re-detect YubiKey(s)";
-#endif
-        retries--;
+    do {
+        --retries;
 
-        if (YubiKey::instance()->init() != true) {
+        if (m_blocking) {
+            emit userInteractionRequired();
+        }
+
+        auto result = YubiKey::instance()->challenge(m_slot, true, chal, m_key);
+
+        if (m_blocking) {
+            emit userConfirmed();
+        }
+
+        if (result != YubiKey::ERROR) {
+            return true;
+        }
+
+        // if challenge failed, retry to detect YubiKeys in the event the YubiKey was un-plugged and re-plugged
+        if (retries > 0 && YubiKey::instance()->init() != true) {
             continue;
         }
 
-        if (YubiKey::instance()->challenge(m_slot, true, chal, m_key) != YubiKey::ERROR) {
-            return true;
-        }
-    }
+    } while (retries > 0);
 
     return false;
 }

src/keys/YkChallengeResponseKey.h

@@ -22,20 +22,34 @@
 #include "keys/ChallengeResponseKey.h"
 #include "keys/drivers/YubiKey.h"
 
-class YkChallengeResponseKey : public ChallengeResponseKey
+#include <QObject>
+
+class YkChallengeResponseKey : public QObject, public ChallengeResponseKey
 {
+    Q_OBJECT
+
 public:
 
-    YkChallengeResponseKey(int slot = -1,
-                           bool blocking = false);
+    YkChallengeResponseKey(int slot = -1, bool blocking = false);
 
     QByteArray rawKey() const;
-    YkChallengeResponseKey* clone() const;
     bool challenge(const QByteArray& chal);
-    bool challenge(const QByteArray& chal, int retries);
+    bool challenge(const QByteArray& chal, unsigned 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:
     QByteArray m_key;
     int m_slot;