From 9bdb41a72716edfe0a3113227a3b9b63b43b9e56 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 00:14:39 -0700 Subject: [PATCH 01/48] keys: Add ChallengeResponseKey header * Add initial header file for forthcoming challenge response support. * A ChallengeResponseKey operates by submitting some challenge data and getting a deterministic result. * In the case of the forthcoming YubiKey integration, the master seed is submitted as the challenge to the YubiKey hardware and the YubiKey returns a HMAC-SHA1 response. Signed-off-by: Kyle Manna --- src/keys/ChallengeResponseKey.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 src/keys/ChallengeResponseKey.h diff --git a/src/keys/ChallengeResponseKey.h b/src/keys/ChallengeResponseKey.h new file mode 100644 index 000000000..e03a2f9f9 --- /dev/null +++ b/src/keys/ChallengeResponseKey.h @@ -0,0 +1,32 @@ +/* +* Copyright (C) 2014 Kyle Manna +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + +#ifndef KEEPASSX_CHALLENGE_RESPONSE_KEY_H +#define KEEPASSX_CHALLENGE_RESPONSE_KEY_H + +#include + +class ChallengeResponseKey +{ +public: + virtual ~ChallengeResponseKey() {} + virtual QByteArray rawKey() const = 0; + virtual ChallengeResponseKey* clone() const = 0; + virtual bool challenge(const QByteArray& challenge) = 0; +}; + +#endif // KEEPASSX_CHALLENGE_RESPONSE_KEY_H From ccd6704b8f2b0fac4101f15baeb18c01cdf09192 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 01:38:07 -0700 Subject: [PATCH 02/48] keys: CompositeKey: Add ChallengeResponseKey support * Each Challenge Response Key consists of a list of regular keys and now challenge response keys. * Copy ChallengeResponseKeys when copying the object. * Challenge consists of challenging each driver in the list and hashing the concatenated data result using SHA256. Signed-off-by: Kyle Manna --- src/keys/CompositeKey.cpp | 30 +++++++++++++++++++++++++++++- src/keys/CompositeKey.h | 5 +++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index 16b48592e..2270d96eb 100644 --- a/src/keys/CompositeKey.cpp +++ b/src/keys/CompositeKey.cpp @@ -17,6 +17,7 @@ #include "CompositeKey.h" #include "CompositeKey_p.h" +#include "ChallengeResponseKey.h" #include #include @@ -47,7 +48,7 @@ void CompositeKey::clear() bool CompositeKey::isEmpty() const { - return m_keys.isEmpty(); + return m_keys.isEmpty() && m_challengeResponseKeys.isEmpty(); } CompositeKey* CompositeKey::clone() const @@ -67,6 +68,9 @@ CompositeKey& CompositeKey::operator=(const CompositeKey& key) for (const Key* subKey : asConst(key.m_keys)) { addKey(*subKey); } + Q_FOREACH (const ChallengeResponseKey* subKey, key.m_challengeResponseKeys) { + addChallengeResponseKey(*subKey); + } return *this; } @@ -142,11 +146,35 @@ QByteArray CompositeKey::transformKeyRaw(const QByteArray& key, const QByteArray return result; } +QByteArray CompositeKey::challenge(const QByteArray& seed) const +{ + /* If no challenge response was requested, return nothing to + * maintain backwards compatability with regular databases. + */ + if (m_challengeResponseKeys.length() == 0) { + return QByteArray(); + } + + CryptoHash cryptoHash(CryptoHash::Sha256); + + Q_FOREACH (ChallengeResponseKey* key, m_challengeResponseKeys) { + key->challenge(seed); + cryptoHash.addData(key->rawKey()); + } + + return cryptoHash.result(); +} + void CompositeKey::addKey(const Key& key) { m_keys.append(key.clone()); } +void CompositeKey::addChallengeResponseKey(const ChallengeResponseKey& key) +{ + m_challengeResponseKeys.append(key.clone()); +} + int CompositeKey::transformKeyBenchmark(int msec) { TransformKeyBenchmarkThread thread1(msec); diff --git a/src/keys/CompositeKey.h b/src/keys/CompositeKey.h index 3290d3671..66ad91ad9 100644 --- a/src/keys/CompositeKey.h +++ b/src/keys/CompositeKey.h @@ -21,6 +21,7 @@ #include #include "keys/Key.h" +#include "keys/ChallengeResponseKey.h" class CompositeKey : public Key { @@ -36,7 +37,10 @@ public: QByteArray rawKey() const; QByteArray transform(const QByteArray& seed, quint64 rounds, bool* ok, QString* errorString) const; + QByteArray challenge(const QByteArray& seed) const; + void addKey(const Key& key); + void addChallengeResponseKey(const ChallengeResponseKey& key); static int transformKeyBenchmark(int msec); @@ -45,6 +49,7 @@ private: quint64 rounds, bool* ok, QString* errorString); QList m_keys; + QList m_challengeResponseKeys; }; #endif // KEEPASSX_COMPOSITEKEY_H From e354a0ee0eb01bedf18931fb0f65bdfb50f0d66f Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 01:40:38 -0700 Subject: [PATCH 03/48] database: Pass master seed to challenge response keys * Pass the master seed from the database to CompositeKey::challenge() function which will in turn issue challenges to all selected drivers. Signed-off-by: Kyle Manna --- src/core/Database.cpp | 5 +++++ src/core/Database.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 336820381..db13e2499 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -176,6 +176,11 @@ QByteArray Database::transformedMasterKey() const return m_data.transformedMasterKey; } +QByteArray Database::challengeMasterSeed(const QByteArray& masterSeed) const +{ + return m_data.key.challenge(masterSeed); +} + void Database::setCipher(const Uuid& cipher) { Q_ASSERT(!cipher.isNull()); diff --git a/src/core/Database.h b/src/core/Database.h index 3cd5ed1b1..607792332 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -89,6 +89,7 @@ public: quint64 transformRounds() const; QByteArray transformedMasterKey() const; const CompositeKey & key() const; + QByteArray challengeMasterSeed(const QByteArray& masterSeed) const; void setCipher(const Uuid& cipher); void setCompressionAlgo(Database::CompressionAlgorithm algo); From add4846d799315d4149b96ca09c65db0dd7675eb Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 00:29:41 -0700 Subject: [PATCH 04/48] format: Add challenge response result to final key hash * The challengeMasterSeed() function return empty if not present maintaining backwards compatability. * This commit is where the challenge response result is computed into the final key used to encrypt or decrypt the database. Signed-off-by: Kyle Manna --- src/format/KeePass2Reader.cpp | 1 + src/format/KeePass2Writer.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index b45cefa6c..17e007d76 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -115,6 +115,7 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke CryptoHash hash(CryptoHash::Sha256); hash.addData(m_masterSeed); + hash.addData(m_db->challengeMasterSeed(m_masterSeed)); hash.addData(m_db->transformedMasterKey()); QByteArray finalKey = hash.result(); diff --git a/src/format/KeePass2Writer.cpp b/src/format/KeePass2Writer.cpp index dfbbf3532..3a3195a0b 100644 --- a/src/format/KeePass2Writer.cpp +++ b/src/format/KeePass2Writer.cpp @@ -53,6 +53,7 @@ void KeePass2Writer::writeDatabase(QIODevice* device, Database* db) CryptoHash hash(CryptoHash::Sha256); hash.addData(masterSeed); + hash.addData(db->challengeMasterSeed(masterSeed)); Q_ASSERT(!db->transformedMasterKey().isEmpty()); hash.addData(db->transformedMasterKey()); QByteArray finalKey = hash.result(); From 82aed2caabf94836a13dfb476601b5640e60d4c2 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 00:41:54 -0700 Subject: [PATCH 05/48] keys: yk: Add YubiKey hardware driver support * Use compile time detection of the YubiKey libraries and link against the libraries if present. Can be disabled with: $ cmake -DCMAKE_DISABLE_FIND_PACKAGE_YubiKey=FALSE * A stub file provides empty calls for all the function calls integrated in to the UI to support this. In the future a more modular approach maybe better, but opting for simplicity initially. Signed-off-by: Kyle Manna --- CMakeLists.txt | 8 + cmake/FindYubiKey.cmake | 29 ++++ src/CMakeLists.txt | 11 ++ src/keys/drivers/YubiKey.cpp | 245 +++++++++++++++++++++++++++++++ src/keys/drivers/YubiKey.h | 70 +++++++++ src/keys/drivers/YubiKeyStub.cpp | 71 +++++++++ 6 files changed, 434 insertions(+) create mode 100644 cmake/FindYubiKey.cmake create mode 100644 src/keys/drivers/YubiKey.cpp create mode 100644 src/keys/drivers/YubiKey.h create mode 100644 src/keys/drivers/YubiKeyStub.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 883f462ef..ff7b05e5a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -191,6 +191,14 @@ if(NOT ZLIB_SUPPORTS_GZIP) message(FATAL_ERROR "zlib 1.2.x or higher is required to use the gzip format") endif() +# Optional +find_package(YubiKey) + +if(YUBIKEY_FOUND) + include_directories(SYSTEM ${YUBIKEY_INCLUDE_DIRS}) +endif() + + if(UNIX) check_cxx_source_compiles("#include int main() { prctl(PR_SET_DUMPABLE, 0); return 0; }" diff --git a/cmake/FindYubiKey.cmake b/cmake/FindYubiKey.cmake new file mode 100644 index 000000000..297b68387 --- /dev/null +++ b/cmake/FindYubiKey.cmake @@ -0,0 +1,29 @@ +# Copyright (C) 2014 Kyle Manna +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 2 or (at your option) +# version 3 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +find_path(YUBIKEY_CORE_INCLUDE_DIR yubikey.h) +find_path(YUBIKEY_PERS_INCLUDE_DIR ykcore.h PATH_SUFFIXES ykpers-1) +set(YUBIKEY_INCLUDE_DIRS ${YUBIKEY_CORE_INCLUDE_DIR} ${YUBIKEY_PERS_INCLUDE_DIR}) + +find_library(YUBIKEY_CORE_LIBRARY yubikey) +find_library(YUBIKEY_PERS_LIBRARY ykpers-1) +set(YUBIKEY_LIBRARIES ${YUBIKEY_CORE_LIBRARY} ${YUBIKEY_PERS_LIBRARY}) + +include(FindPackageHandleStandardArgs) +find_package_handle_standard_args(YubiKey DEFAULT_MSG YUBIKEY_LIBRARIES YUBIKEY_INCLUDE_DIRS) + +# TODO: Is mark_as_advanced() necessary? It's used in many examples with +# little explanation. Disable for now in favor of simplicity. +#mark_as_advanced(YUBIKEY_LIBRARIES YUBIKEY_INCLUDE_DIRS) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 30332c71e..5e5e7d58a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -111,6 +111,7 @@ set(keepassx_SOURCES gui/group/GroupView.cpp keys/CompositeKey.cpp keys/CompositeKey_p.h + keys/drivers/YubiKey.h keys/FileKey.cpp keys/Key.h keys/PasswordKey.cpp @@ -190,6 +191,12 @@ if(MINGW) ${CMAKE_SOURCE_DIR}/share/windows/icon.rc) endif() +if(YUBIKEY_FOUND) + set(keepassx_SOURCES ${keepassx_SOURCES} keys/drivers/YubiKey.cpp) +else() + set(keepassx_SOURCES ${keepassx_SOURCES} keys/drivers/YubiKeyStub.cpp) +endif() + qt5_wrap_ui(keepassx_SOURCES ${keepassx_FORMS}) add_library(zxcvbn STATIC zxcvbn/zxcvbn.cpp) @@ -220,6 +227,10 @@ target_link_libraries(${PROGNAME} ${GCRYPT_LIBRARIES} ${ZLIB_LIBRARIES}) +if(YUBIKEY_FOUND) + target_link_libraries(keepassx_core ${YUBIKEY_LIBRARIES}) +endif() + set_target_properties(${PROGNAME} PROPERTIES ENABLE_EXPORTS ON) if(APPLE) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp new file mode 100644 index 000000000..1d69bb5b9 --- /dev/null +++ b/src/keys/drivers/YubiKey.cpp @@ -0,0 +1,245 @@ +/* +* Copyright (C) 2014 Kyle Manna +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + +#include + +#include + +#include +#include +#include +#include + +#include "core/Global.h" +#include "crypto/Random.h" + +#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(m_yk_void)) +#define m_ykds (static_cast(m_ykds_void)) + +YubiKey::YubiKey() : m_yk_void(NULL), m_ykds_void(NULL) +{ +} + +YubiKey* YubiKey::m_instance(Q_NULLPTR); + +/** + * @brief YubiKey::instance - get instance of singleton + * @return + */ +YubiKey* YubiKey::instance() +{ + if (!m_instance) { + m_instance = new YubiKey(); + } + + return m_instance; +} + +/** + * @brief YubiKey::init - initialize yubikey library and hardware + * @return + */ +bool YubiKey::init() +{ + /* Previously initalized */ + if (m_yk != NULL && m_ykds != NULL) { + + if (yk_get_status(m_yk, m_ykds)) { + /* Still connected */ + return true; + } else { + /* Initialized but not connected anymore, re-init */ + deinit(); + } + } + + if (!yk_init()) { + return false; + } + + /* TODO: handle multiple attached hardware devices, currently own one */ + m_yk_void = static_cast(yk_open_first_key()); + if (m_yk == NULL) { + return false; + } + + m_ykds_void = static_cast(ykds_alloc()); + if (m_ykds == NULL) { + yk_close_key(m_yk); + m_yk_void = NULL; + return false; + } + + return true; +} + +/** + * @brief YubiKey::deinit - cleanup after init + * @return true on success + */ +bool YubiKey::deinit() +{ + if (m_yk) { + yk_close_key(m_yk); + m_yk_void = NULL; + } + + if (m_ykds) { + ykds_free(m_ykds); + m_ykds_void = NULL; + } + + return true; +} + +/** + * @brief YubiKey::detect - probe for attached YubiKeys + */ +void YubiKey::detect() +{ + if (init()) { + + for (int i = 1; i < 3; i++) { + YubiKey::ChallengeResult result; + QByteArray rand = randomGen()->randomArray(1); + QByteArray resp; + + result = challenge(i, false, rand, resp); + + if (result != YubiKey::ERROR) { + Q_EMIT detected(i, result == YubiKey::WOULDBLOCK ? true : false); + } + } + } +} + +/** + * @brief YubiKey::getSerial - serial number of yubikey + * @param serial + * @return + */ +bool YubiKey::getSerial(unsigned int& serial) const +{ + if (!yk_get_serial(m_yk, 1, 0, &serial)) { + return false; + } + + return true; +} + +#ifdef QT_DEBUG +/** + * @brief printByteArray - debug raw data + * @param a array input + * @return string representation of array + */ +static inline QString printByteArray(const QByteArray& a) +{ + QString s; + for (int i = 0; i < a.size(); i++) + s.append(QString::number(a[i] & 0xff, 16).rightJustified(2, '0')); + return s; +} +#endif + +/** + * @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 chal challenge input to YubiKey + * @param resp response output from YubiKey + * @return SUCCESS when successful + */ +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, + const QByteArray& chal, + QByteArray& resp) const +{ + int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2; + QByteArray paddedChal = chal; + + /* yk_challenge_response() insists on 64 byte response buffer */ + resp.resize(64); + + /* The challenge sent to the yubikey should always be 64 bytes for + * compatibility with all configurations. Follow PKCS7 padding. + * + * There is some question whether or not 64 byte fixed length + * configurations even work, some docs say avoid it. + */ + const int padLen = 64 - paddedChal.size(); + if (padLen > 0) { + paddedChal.append(QByteArray(padLen, padLen)); + } + + const unsigned char *c; + unsigned char *r; + c = reinterpret_cast(paddedChal.constData()); + r = reinterpret_cast(resp.data()); + +#ifdef QT_DEBUG + qDebug().nospace() << __func__ << "(" << slot << ") c = " + << printByteArray(paddedChal); +#endif + + int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, + paddedChal.size(), c, + resp.size(), r); + + 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:" << yk_usb_strerror(); + } else { + qWarning() << "YubiKey core error:" << yk_strerror(yk_errno); + } + + return ERROR; + } + } + + /* Actual HMAC-SHA1 response is only 20 bytes */ + resp.resize(20); + +#ifdef QT_DEBUG + qDebug().nospace() << __func__ << "(" << slot << ") r = " + << printByteArray(resp) << ", ret = " << ret; +#endif + + return SUCCESS; +} diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h new file mode 100644 index 000000000..492fba01d --- /dev/null +++ b/src/keys/drivers/YubiKey.h @@ -0,0 +1,70 @@ +/* +* Copyright (C) 2014 Kyle Manna +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + +#ifndef KEEPASSX_YUBIKEY_H +#define KEEPASSX_YUBIKEY_H + +#include + +/** + * Singleton class to manage the interface to the hardware + */ +class YubiKey : public QObject +{ + Q_OBJECT + +public: + enum ChallengeResult { ERROR = -1, SUCCESS = 0, WOULDBLOCK }; + + static YubiKey* instance(); + + /** Initialize the underlying yubico libraries */ + bool init(); + bool deinit(); + + /** Issue a challenge to the hardware */ + ChallengeResult challenge(int slot, bool mayBlock, + const QByteArray& chal, + QByteArray& resp) const; + + /** Read the serial number from the hardware */ + bool getSerial(unsigned int& serial) const; + + /** Start looking for attached hardware devices */ + void detect(); + +Q_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 + */ + void detected(int slot, bool blocking); + +private: + explicit YubiKey(); + static YubiKey* m_instance; + + /* Create void ptr here to avoid ifdef header include mess */ + void *m_yk_void; + void *m_ykds_void; + + Q_DISABLE_COPY(YubiKey) +}; + +#endif // KEEPASSX_YUBIKEY_H diff --git a/src/keys/drivers/YubiKeyStub.cpp b/src/keys/drivers/YubiKeyStub.cpp new file mode 100644 index 000000000..c00790f38 --- /dev/null +++ b/src/keys/drivers/YubiKeyStub.cpp @@ -0,0 +1,71 @@ +/* +* Copyright (C) 2014 Kyle Manna +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + +#include + +#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::instance() +{ + if (!m_instance) { + m_instance = new YubiKey(); + } + + return m_instance; +} + +bool YubiKey::init() +{ + return false; +} + +bool YubiKey::deinit() +{ + return false; +} + +void YubiKey::detect() +{ +} + +bool YubiKey::getSerial(unsigned int& serial) const +{ + Q_UNUSED(serial); + + return false; +} + +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, + const QByteArray& chal, + QByteArray& resp) const +{ + Q_UNUSED(slot); + Q_UNUSED(mayBlock); + Q_UNUSED(chal); + Q_UNUSED(resp); + + return ERROR; +} From 5b8b4c8c7b877227cb7b6602450aaaa3e859a14e Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 02:06:26 -0700 Subject: [PATCH 06/48] keys: yk: Implement ChallengeResponseKey for YubiKey * Implement a YubiKey challenge response class. One object will be created for each challenge response key available. Signed-off-by: Kyle Manna --- src/CMakeLists.txt | 1 + src/keys/YkChallengeResponseKey.cpp | 72 +++++++++++++++++++++++++++++ src/keys/YkChallengeResponseKey.h | 44 ++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 src/keys/YkChallengeResponseKey.cpp create mode 100644 src/keys/YkChallengeResponseKey.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5e5e7d58a..cf4d593ab 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -115,6 +115,7 @@ set(keepassx_SOURCES keys/FileKey.cpp keys/Key.h keys/PasswordKey.cpp + keys/YkChallengeResponseKey.cpp streams/HashedBlockStream.cpp streams/LayeredStream.cpp streams/qtiocompressor.cpp diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp new file mode 100644 index 000000000..5f495a340 --- /dev/null +++ b/src/keys/YkChallengeResponseKey.cpp @@ -0,0 +1,72 @@ +/* +* Copyright (C) 2014 Kyle Manna +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + + +#include +#include + +#include "core/Tools.h" +#include "crypto/CryptoHash.h" +#include "crypto/Random.h" + +#include "keys/YkChallengeResponseKey.h" +#include "keys/drivers/YubiKey.h" + +YkChallengeResponseKey::YkChallengeResponseKey(int slot, + bool blocking) + : m_slot(slot), + m_blocking(blocking) +{ +} + +QByteArray YkChallengeResponseKey::rawKey() const +{ + return m_key; +} + +YkChallengeResponseKey* YkChallengeResponseKey::clone() const +{ + return new YkChallengeResponseKey(*this); +} + + +/** Assumes yubikey()->init() was called */ +bool YkChallengeResponseKey::challenge(const QByteArray& chal) +{ + if (YubiKey::instance()->challenge(m_slot, true, chal, m_key) != YubiKey::ERROR) { + return true; + } + + return false; +} + +QString YkChallengeResponseKey::getName() const +{ + unsigned int serial; + QString fmt("YubiKey[%1] Challenge Response - Slot %2 - %3"); + + YubiKey::instance()->getSerial(serial); + + return fmt.arg(QString::number(serial), + QString::number(m_slot), + (m_blocking) ? "Press" : "Passive"); +} + +bool YkChallengeResponseKey::isBlocking() const +{ + return m_blocking; +} diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h new file mode 100644 index 000000000..a52367486 --- /dev/null +++ b/src/keys/YkChallengeResponseKey.h @@ -0,0 +1,44 @@ +/* +* Copyright (C) 2011 Felix Geyer +* +* This program is free software: you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation, either version 2 or (at your option) +* version 3 of the License. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program. If not, see . +*/ + +#ifndef KEEPASSX_YK_CHALLENGERESPONSEKEY_H +#define KEEPASSX_YK_CHALLENGERESPONSEKEY_H + +#include "core/Global.h" +#include "keys/ChallengeResponseKey.h" +#include "keys/drivers/YubiKey.h" + +class YkChallengeResponseKey : public ChallengeResponseKey +{ +public: + + YkChallengeResponseKey(int slot = -1, + bool blocking = false); + + QByteArray rawKey() const; + YkChallengeResponseKey* clone() const; + bool challenge(const QByteArray& challenge); + QString getName() const; + bool isBlocking() const; + +private: + QByteArray m_key; + int m_slot; + bool m_blocking; +}; + +#endif // KEEPASSX_YK_CHALLENGERESPONSEKEY_H From 9556d8e6dad2143a8af0bbd9620ae06f8329c953 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 00:46:41 -0700 Subject: [PATCH 07/48] tests: Add YubiKey Tests * Basic testing for YubiKey code. Signed-off-by: Kyle Manna --- tests/CMakeLists.txt | 8 ++ tests/TestYkChallengeResponseKey.cpp | 108 +++++++++++++++++++++++++++ tests/TestYkChallengeResponseKey.h | 54 ++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 tests/TestYkChallengeResponseKey.cpp create mode 100644 tests/TestYkChallengeResponseKey.h diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 0ea73b2fe..7edf89395 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -99,6 +99,10 @@ set(testsupport_SOURCES modeltest.cpp FailDevice.cpp) add_library(testsupport STATIC ${testsupport_SOURCES}) target_link_libraries(testsupport ${MHD_LIBRARIES} Qt5::Core Qt5::Concurrent Qt5::Widgets Qt5::Test) +if(YUBIKEY_FOUND) + set(TEST_LIBRARIES ${TEST_LIBRARIES} ${YUBIKEY_LIBRARIES}) +endif() + add_unit_test(NAME testgroup SOURCES TestGroup.cpp LIBS ${TEST_LIBRARIES}) @@ -165,6 +169,10 @@ add_unit_test(NAME testexporter SOURCES TestExporter.cpp add_unit_test(NAME testcsvexporter SOURCES TestCsvExporter.cpp LIBS ${TEST_LIBRARIES}) +add_unit_test(NAME testykchallengeresponsekey + SOURCES TestYkChallengeResponseKey.cpp TestYkChallengeResponseKey.h + LIBS ${TEST_LIBRARIES}) + if(WITH_GUI_TESTS) add_subdirectory(gui) endif(WITH_GUI_TESTS) diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp new file mode 100644 index 000000000..91fba83cb --- /dev/null +++ b/tests/TestYkChallengeResponseKey.cpp @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2014 Kyle Manna + * + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "TestYkChallengeResponseKey.h" + +#include +#include + +#include "keys/YkChallengeResponseKey.h" + +QTEST_GUILESS_MAIN(TestYubiKeyChalResp) + +void TestYubiKeyChalResp::initTestCase() +{ + m_detected = 0; + m_key = NULL; +} + +void TestYubiKeyChalResp::cleanupTestCase() +{ + if (m_key) + delete m_key; +} + +void TestYubiKeyChalResp::init() +{ + bool result = YubiKey::instance()->init(); + + if (!result) { + QSKIP("Unable to connect to YubiKey", SkipAll); + } +} + +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() +{ + QVERIFY(m_key); + if (m_key->isBlocking()) { + /* Testing active mode in unit tests is unreasonable */ + QSKIP("YubiKey not in passive mode", SkipSingle); + } + + 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 past. + * + * Might be worth it for integrity verification though. + */ +} + +void TestYubiKeyChalResp::ykDetected(int slot, bool blocking) +{ + Q_UNUSED(blocking); + + if (slot > 0) + m_detected++; + + /* Key used for later testing */ + if (!m_key) + m_key = new YkChallengeResponseKey(slot, blocking); +} + +void TestYubiKeyChalResp::deinit() +{ + QVERIFY(YubiKey::instance()->deinit()); +} diff --git a/tests/TestYkChallengeResponseKey.h b/tests/TestYkChallengeResponseKey.h new file mode 100644 index 000000000..4699b9101 --- /dev/null +++ b/tests/TestYkChallengeResponseKey.h @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2014 Kyle Manna + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSX_TESTYUBIKEYCHALRESP_H +#define KEEPASSX_TESTYUBIKEYCHALRESP_H + +#include + +#include "keys/YkChallengeResponseKey.h" + +class TestYubiKeyChalResp: public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + void cleanupTestCase(); + + 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; + YkChallengeResponseKey *m_key; +}; + +#endif // KEEPASSX_TESTYUBIKEYCHALRESP_H From ba8fd256045ae1ec83b8aa00b1dce2fd44771102 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Mon, 26 May 2014 00:49:28 -0700 Subject: [PATCH 08/48] gui: Add YubiKey support to widgets * Add YubiKey support to the GUI widgets. Signed-off-by: Kyle Manna --- src/gui/ChangeMasterKeyWidget.cpp | 29 ++++++++++++++++++ src/gui/ChangeMasterKeyWidget.h | 1 + src/gui/ChangeMasterKeyWidget.ui | 25 ++++++++++++++++ src/gui/DatabaseOpenWidget.cpp | 49 +++++++++++++++++++++++++++++++ src/gui/DatabaseOpenWidget.h | 3 ++ src/gui/DatabaseOpenWidget.ui | 17 +++++++++++ 6 files changed, 124 insertions(+) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 3e346bc10..c69cf2dcc 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -15,14 +15,18 @@ * along with this program. If not, see . */ +#include + #include "ChangeMasterKeyWidget.h" #include "ui_ChangeMasterKeyWidget.h" #include "core/FilePath.h" #include "keys/FileKey.h" #include "keys/PasswordKey.h" +#include "keys/YkChallengeResponseKey.h" #include "gui/FileDialog.h" #include "gui/MessageBox.h" +#include "crypto/Random.h" ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) : DialogyWidget(parent) @@ -81,6 +85,15 @@ void ChangeMasterKeyWidget::clearForms() m_ui->togglePasswordButton->setChecked(false); // TODO: clear m_ui->keyFileCombo + m_ui->challengeResponseGroup->setChecked(false); + m_ui->challengeResponseCombo->clear(); + + /* YubiKey init is slow */ + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), + SLOT(ykDetected(int,bool)), + Qt::QueuedConnection); + QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + m_ui->enterPasswordEdit->setFocus(); } @@ -128,6 +141,14 @@ void ChangeMasterKeyWidget::generateKey() m_key.addKey(fileKey); } + if (m_ui->challengeResponseGroup->isChecked()) { + int i = m_ui->challengeResponseCombo->currentIndex(); + i = m_ui->challengeResponseCombo->itemData(i).toInt(); + YkChallengeResponseKey key(i); + + m_key.addChallengeResponseKey(key); + } + Q_EMIT editFinished(true); } @@ -136,3 +157,11 @@ void ChangeMasterKeyWidget::reject() { Q_EMIT editFinished(false); } + + +void ChangeMasterKeyWidget::ykDetected(int slot, bool blocking) +{ + YkChallengeResponseKey yk(slot, blocking); + m_ui->challengeResponseCombo->addItem(yk.getName(), QVariant(slot)); + m_ui->challengeResponseGroup->setEnabled(true); +} diff --git a/src/gui/ChangeMasterKeyWidget.h b/src/gui/ChangeMasterKeyWidget.h index 8985ff7a8..9b765c205 100644 --- a/src/gui/ChangeMasterKeyWidget.h +++ b/src/gui/ChangeMasterKeyWidget.h @@ -47,6 +47,7 @@ private Q_SLOTS: void reject(); void createKeyFile(); void browseKeyFile(); + void ykDetected(int slot, bool blocking); private: const QScopedPointer m_ui; diff --git a/src/gui/ChangeMasterKeyWidget.ui b/src/gui/ChangeMasterKeyWidget.ui index d14941ccc..335a67c92 100644 --- a/src/gui/ChangeMasterKeyWidget.ui +++ b/src/gui/ChangeMasterKeyWidget.ui @@ -123,6 +123,31 @@ + + + + false + + + Challenge Response + + + true + + + + + + + + + Challenge Response: + + + + + + diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 781b836fd..5d3815963 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -15,6 +15,8 @@ * along with this program. If not, see . */ +#include + #include "DatabaseOpenWidget.h" #include "ui_DatabaseOpenWidget.h" @@ -27,6 +29,9 @@ #include "format/KeePass2Reader.h" #include "keys/FileKey.h" #include "keys/PasswordKey.h" +#include "keys/YkChallengeResponseKey.h" +#include "crypto/Random.h" + DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) : DialogyWidget(parent) @@ -49,6 +54,13 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->editPassword, SIGNAL(textChanged(QString)), SLOT(activatePassword())); connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(activateKeyFile())); + connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(activateChallengeResponse())); + + connect(m_ui->checkPassword, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); + connect(m_ui->checkKeyFile, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); + connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(setOkButtonEnabled())); + connect(m_ui->checkChallengeResponse, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); + connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(setOkButtonEnabled())); connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); @@ -79,6 +91,13 @@ void DatabaseOpenWidget::load(const QString& filename) } m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); + + /* YubiKey init is slow */ + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), + SLOT(ykDetected(int,bool)), + Qt::QueuedConnection); + QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + m_ui->editPassword->setFocus(); } @@ -156,6 +175,15 @@ CompositeKey DatabaseOpenWidget::databaseKey() config()->set("LastKeyFiles", lastKeyFiles); } + + if (m_ui->checkChallengeResponse->isChecked()) { + int i = m_ui->comboChallengeResponse->currentIndex(); + i = m_ui->comboChallengeResponse->itemData(i).toInt(); + YkChallengeResponseKey key(i); + + masterKey.addChallengeResponseKey(key); + } + return masterKey; } @@ -174,6 +202,19 @@ void DatabaseOpenWidget::activateKeyFile() m_ui->checkKeyFile->setChecked(true); } +void DatabaseOpenWidget::activateChallengeResponse() +{ + m_ui->checkChallengeResponse->setChecked(true); +} + +void DatabaseOpenWidget::setOkButtonEnabled() +{ + bool enable = m_ui->checkPassword->isChecked() || m_ui->checkChallengeResponse->isChecked() + || (m_ui->checkKeyFile->isChecked() && !m_ui->comboKeyFile->currentText().isEmpty()); + + m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(enable); +} + void DatabaseOpenWidget::browseKeyFile() { QString filters = QString("%1 (*);;%2 (*.key)").arg(tr("All files"), tr("Key files")); @@ -183,3 +224,11 @@ void DatabaseOpenWidget::browseKeyFile() m_ui->comboKeyFile->lineEdit()->setText(filename); } } + +void DatabaseOpenWidget::ykDetected(int slot, bool blocking) +{ + YkChallengeResponseKey yk(slot, blocking); + m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant(slot)); + m_ui->comboChallengeResponse->setEnabled(true); + m_ui->checkChallengeResponse->setEnabled(true); +} diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index 34f401a09..fadb5ee70 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -55,7 +55,10 @@ protected Q_SLOTS: private Q_SLOTS: void activatePassword(); void activateKeyFile(); + void activateChallengeResponse(); + void setOkButtonEnabled(); void browseKeyFile(); + void ykDetected(int slot, bool blocking); protected: const QScopedPointer m_ui; diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index 4aae5faf2..3d651ee8d 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -111,6 +111,23 @@ + + + + false + + + Challenge Response: + + + + + + + false + + + From faa055010f247d37c73154f1ecbad00e6b632e9e Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 6 Sep 2014 17:49:39 -0700 Subject: [PATCH 09/48] challenge: Propagate failed challenge to caller * If a removed Yubikey is to blame, re-inserting the Yubikey won't resolve the issue. Hot plug isn't supported at this point. * The caller should detect the error and cancel the database write. Signed-off-by: Kyle Manna --- src/core/Database.cpp | 4 ++-- src/core/Database.h | 2 +- src/format/KeePass2Reader.cpp | 8 +++++++- src/format/KeePass2Writer.cpp | 8 +++++++- src/keys/CompositeKey.cpp | 12 ++++++++---- src/keys/CompositeKey.h | 2 +- 6 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index db13e2499..fd9e02dd1 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -176,9 +176,9 @@ QByteArray Database::transformedMasterKey() const return m_data.transformedMasterKey; } -QByteArray Database::challengeMasterSeed(const QByteArray& masterSeed) const +bool Database::challengeMasterSeed(const QByteArray& masterSeed, QByteArray& result) const { - return m_data.key.challenge(masterSeed); + return m_data.key.challenge(masterSeed, result); } void Database::setCipher(const Uuid& cipher) diff --git a/src/core/Database.h b/src/core/Database.h index 607792332..e2eb0fb14 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -89,7 +89,7 @@ public: quint64 transformRounds() const; QByteArray transformedMasterKey() const; const CompositeKey & key() const; - QByteArray challengeMasterSeed(const QByteArray& masterSeed) const; + bool challengeMasterSeed(const QByteArray& masterSeed, QByteArray& result) const; void setCipher(const Uuid& cipher); void setCompressionAlgo(Database::CompressionAlgorithm algo); diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index 17e007d76..8ac983276 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -113,9 +113,15 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke return nullptr; } + QByteArray challengeResult; + if (m_db->challengeMasterSeed(m_masterSeed, challengeResult) == false) { + raiseError(tr("Unable to issue challenge-response.")); + return Q_NULLPTR; + } + CryptoHash hash(CryptoHash::Sha256); hash.addData(m_masterSeed); - hash.addData(m_db->challengeMasterSeed(m_masterSeed)); + hash.addData(challengeResult); hash.addData(m_db->transformedMasterKey()); QByteArray finalKey = hash.result(); diff --git a/src/format/KeePass2Writer.cpp b/src/format/KeePass2Writer.cpp index 3a3195a0b..6c0bf2e1b 100644 --- a/src/format/KeePass2Writer.cpp +++ b/src/format/KeePass2Writer.cpp @@ -51,9 +51,15 @@ void KeePass2Writer::writeDatabase(QIODevice* device, Database* db) QByteArray startBytes = randomGen()->randomArray(32); QByteArray endOfHeader = "\r\n\r\n"; + QByteArray challengeResult; + if (db->challengeMasterSeed(masterSeed, challengeResult) == false) { + raiseError("Unable to issue challenge-response."); + return; + } + CryptoHash hash(CryptoHash::Sha256); hash.addData(masterSeed); - hash.addData(db->challengeMasterSeed(masterSeed)); + hash.addData(challengeResult); Q_ASSERT(!db->transformedMasterKey().isEmpty()); hash.addData(db->transformedMasterKey()); QByteArray finalKey = hash.result(); diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index 2270d96eb..ed64b8ef4 100644 --- a/src/keys/CompositeKey.cpp +++ b/src/keys/CompositeKey.cpp @@ -146,23 +146,27 @@ QByteArray CompositeKey::transformKeyRaw(const QByteArray& key, const QByteArray return result; } -QByteArray CompositeKey::challenge(const QByteArray& seed) const +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 (m_challengeResponseKeys.length() == 0) { - return QByteArray(); + return true; } CryptoHash cryptoHash(CryptoHash::Sha256); Q_FOREACH (ChallengeResponseKey* key, m_challengeResponseKeys) { - key->challenge(seed); + /* If the device isn't present or fails, return an error */ + if (key->challenge(seed) == false) { + return false; + } cryptoHash.addData(key->rawKey()); } - return cryptoHash.result(); + result = cryptoHash.result(); + return true; } void CompositeKey::addKey(const Key& key) diff --git a/src/keys/CompositeKey.h b/src/keys/CompositeKey.h index 66ad91ad9..b5f973d20 100644 --- a/src/keys/CompositeKey.h +++ b/src/keys/CompositeKey.h @@ -37,7 +37,7 @@ public: QByteArray rawKey() const; QByteArray transform(const QByteArray& seed, quint64 rounds, bool* ok, QString* errorString) const; - QByteArray challenge(const QByteArray& seed) const; + bool challenge(const QByteArray& seed, QByteArray &result) const; void addKey(const Key& key); void addChallengeResponseKey(const ChallengeResponseKey& key); From f7ee528d41325d335b5cada4b4dfe514d76cc9e8 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 6 Sep 2014 22:09:06 -0700 Subject: [PATCH 10/48] YubiKey: Retry to recover hotplugging * Attempt one retry in the event the event the device was removed and re-inserted. Signed-off-by: Kyle Manna --- src/keys/YkChallengeResponseKey.cpp | 22 ++++++++++++++++++++++ src/keys/YkChallengeResponseKey.h | 3 ++- src/keys/drivers/YubiKey.cpp | 5 +++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 5f495a340..17982ab62 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -26,6 +26,8 @@ #include "keys/YkChallengeResponseKey.h" #include "keys/drivers/YubiKey.h" +#include + YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) : m_slot(slot), @@ -46,11 +48,31 @@ YkChallengeResponseKey* YkChallengeResponseKey::clone() const /** Assumes yubikey()->init() was called */ bool YkChallengeResponseKey::challenge(const QByteArray& chal) +{ + return challenge(chal, 1); +} + +bool YkChallengeResponseKey::challenge(const QByteArray& chal, int retries) { if (YubiKey::instance()->challenge(m_slot, true, chal, m_key) != YubiKey::ERROR) { return true; } + /* If challenge failed, retry to detect YubiKeys int the event the YubiKey + * was un-plugged and re-plugged */ + while (retries > 0) { + qDebug() << "Attempt" << retries << "to re-detect YubiKey(s)"; + retries--; + + if (YubiKey::instance()->init() != true) { + continue; + } + + if (YubiKey::instance()->challenge(m_slot, true, chal, m_key) != YubiKey::ERROR) { + return true; + } + } + return false; } diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index a52367486..8acb0f9e9 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/src/keys/YkChallengeResponseKey.h @@ -31,7 +31,8 @@ public: QByteArray rawKey() const; YkChallengeResponseKey* clone() const; - bool challenge(const QByteArray& challenge); + bool challenge(const QByteArray& chal); + bool challenge(const QByteArray& chal, int retries); QString getName() const; bool isBlocking() const; diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 1d69bb5b9..9c87ec154 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -182,6 +182,11 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2; QByteArray paddedChal = chal; + /* Ensure that YubiKey::init() succeeded */ + if (m_yk == NULL) { + return ERROR; + } + /* yk_challenge_response() insists on 64 byte response buffer */ resp.resize(64); From 62190d79be49771d003ca1a507a1a371c7f27334 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 6 Sep 2014 22:19:05 -0700 Subject: [PATCH 11/48] YubiKey: Whitespace clean-up * This was bugging me. Oops. * No functional changes. Signed-off-by: Kyle Manna --- src/keys/drivers/YubiKey.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 9c87ec154..5ca175836 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -104,8 +104,8 @@ bool YubiKey::deinit() } if (m_ykds) { - ykds_free(m_ykds); - m_ykds_void = NULL; + ykds_free(m_ykds); + m_ykds_void = NULL; } return true; @@ -215,7 +215,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, paddedChal.size(), c, resp.size(), r); - if(!ret) { + if (!ret) { if (yk_errno == YK_EWOULDBLOCK) { return WOULDBLOCK; } else if (yk_errno == YK_ETIMEOUT) { From 77cc99acd3de0ff1e3df24403eeb136e5faaeb0a Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sun, 7 Sep 2014 16:37:46 -0700 Subject: [PATCH 12/48] YubiKey: Clean-up master seed challenge * Tweak the logic so it more closely resembles other code (i.e. trasnformKey()). Matches existing style better. * Save the challengeResponseKey in the database structure so that it can be referred to later (i.e. database unlocking). Signed-off-by: Kyle Manna --- src/core/Database.cpp | 9 +++++++-- src/core/Database.h | 4 +++- src/format/KeePass2Reader.cpp | 5 ++--- src/format/KeePass2Writer.cpp | 5 ++--- 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index fd9e02dd1..5297c2ad3 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -176,9 +176,14 @@ QByteArray Database::transformedMasterKey() const return m_data.transformedMasterKey; } -bool Database::challengeMasterSeed(const QByteArray& masterSeed, QByteArray& result) const +QByteArray Database::challengeResponseKey() const { - return m_data.key.challenge(masterSeed, result); + return m_data.challengeResponseKey; +} + +bool Database::challengeMasterSeed(const QByteArray& masterSeed) +{ + return m_data.key.challenge(masterSeed, m_data.challengeResponseKey); } void Database::setCipher(const Uuid& cipher) diff --git a/src/core/Database.h b/src/core/Database.h index e2eb0fb14..be022ae39 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -59,6 +59,7 @@ public: QByteArray transformedMasterKey; CompositeKey key; bool hasKey; + QByteArray challengeResponseKey; }; Database(); @@ -89,7 +90,8 @@ public: quint64 transformRounds() const; QByteArray transformedMasterKey() const; const CompositeKey & key() const; - bool challengeMasterSeed(const QByteArray& masterSeed, QByteArray& result) const; + QByteArray challengeResponseKey() const; + bool challengeMasterSeed(const QByteArray& masterSeed); void setCipher(const Uuid& cipher); void setCompressionAlgo(Database::CompressionAlgorithm algo); diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index 8ac983276..73960a7a1 100644 --- a/src/format/KeePass2Reader.cpp +++ b/src/format/KeePass2Reader.cpp @@ -113,15 +113,14 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, const CompositeKey& ke return nullptr; } - QByteArray challengeResult; - if (m_db->challengeMasterSeed(m_masterSeed, challengeResult) == false) { + if (m_db->challengeMasterSeed(m_masterSeed) == false) { raiseError(tr("Unable to issue challenge-response.")); return Q_NULLPTR; } CryptoHash hash(CryptoHash::Sha256); hash.addData(m_masterSeed); - hash.addData(challengeResult); + hash.addData(m_db->challengeResponseKey()); hash.addData(m_db->transformedMasterKey()); QByteArray finalKey = hash.result(); diff --git a/src/format/KeePass2Writer.cpp b/src/format/KeePass2Writer.cpp index 6c0bf2e1b..6bb316bac 100644 --- a/src/format/KeePass2Writer.cpp +++ b/src/format/KeePass2Writer.cpp @@ -51,15 +51,14 @@ void KeePass2Writer::writeDatabase(QIODevice* device, Database* db) QByteArray startBytes = randomGen()->randomArray(32); QByteArray endOfHeader = "\r\n\r\n"; - QByteArray challengeResult; - if (db->challengeMasterSeed(masterSeed, challengeResult) == false) { + if (db->challengeMasterSeed(masterSeed) == false) { raiseError("Unable to issue challenge-response."); return; } CryptoHash hash(CryptoHash::Sha256); hash.addData(masterSeed); - hash.addData(challengeResult); + hash.addData(db->challengeResponseKey()); Q_ASSERT(!db->transformedMasterKey().isEmpty()); hash.addData(db->transformedMasterKey()); QByteArray finalKey = hash.result(); From 951fa968480a861718371e25d7ca3f8106821100 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sun, 7 Sep 2014 16:43:04 -0700 Subject: [PATCH 13/48] YubiKey: Fix database locking * Save the master seed upon first challenge so it can be used as a challenge at a later point. * When verifyKey() is called, verify that the challenge is successful. * Uncheck YubiKey box to not leak information about how the database is protected. Signed-off-by: Kyle Manna --- src/core/Database.cpp | 17 +++++++++++++++++ src/core/Database.h | 1 + src/gui/UnlockDatabaseWidget.cpp | 1 + 3 files changed, 19 insertions(+) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 5297c2ad3..22fc07230 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -183,6 +183,7 @@ QByteArray Database::challengeResponseKey() const bool Database::challengeMasterSeed(const QByteArray& masterSeed) { + m_data.masterSeed = masterSeed; return m_data.key.challenge(masterSeed, m_data.challengeResponseKey); } @@ -256,6 +257,22 @@ 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? */ + return false; + } + + if (m_data.challengeResponseKey != result) { + /* Wrong response from challenged device(s) */ + return false; + } + } + return (m_data.key.rawKey() == key.rawKey()); } diff --git a/src/core/Database.h b/src/core/Database.h index be022ae39..3f946a1cf 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -59,6 +59,7 @@ public: QByteArray transformedMasterKey; CompositeKey key; bool hasKey; + QByteArray masterSeed; QByteArray challengeResponseKey; }; diff --git a/src/gui/UnlockDatabaseWidget.cpp b/src/gui/UnlockDatabaseWidget.cpp index a005d0e60..d6beb1339 100644 --- a/src/gui/UnlockDatabaseWidget.cpp +++ b/src/gui/UnlockDatabaseWidget.cpp @@ -33,6 +33,7 @@ void UnlockDatabaseWidget::clearForms() m_ui->comboKeyFile->clear(); m_ui->checkPassword->setChecked(false); m_ui->checkKeyFile->setChecked(false); + m_ui->checkChallengeResponse->setChecked(false); m_ui->buttonTogglePassword->setChecked(false); m_db = nullptr; } From d398d367c1e2b6fe9534f42769cc6e05cd4ceafe Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Tue, 27 Jan 2015 17:38:26 +0000 Subject: [PATCH 14/48] Allow a previously yubikey protected database to be saved without the yubikey challenge-response code. --- src/keys/CompositeKey.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index ed64b8ef4..d2334e3c2 100644 --- a/src/keys/CompositeKey.cpp +++ b/src/keys/CompositeKey.cpp @@ -43,7 +43,9 @@ CompositeKey::~CompositeKey() void CompositeKey::clear() { qDeleteAll(m_keys); + qDeleteAll(m_challengeResponseKeys); m_keys.clear(); + m_challengeResponseKeys.clear(); } bool CompositeKey::isEmpty() const @@ -152,6 +154,7 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const * maintain backwards compatability with regular databases. */ if (m_challengeResponseKeys.length() == 0) { + result.clear(); return true; } From ef06165ea2f4998e8c04c6c62b04a2fe504e5656 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sun, 8 Jan 2017 18:45:02 -0800 Subject: [PATCH 15/48] keys: CompositeKey: Change Q_FOREACH to C++11 for() * Use the C++11 range based loop as recommended from https://github.com/keepassxreboot/keepassxc/pull/119 Signed-off-by: Kyle Manna --- src/keys/CompositeKey.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index d2334e3c2..ae654eb74 100644 --- a/src/keys/CompositeKey.cpp +++ b/src/keys/CompositeKey.cpp @@ -70,7 +70,7 @@ CompositeKey& CompositeKey::operator=(const CompositeKey& key) for (const Key* subKey : asConst(key.m_keys)) { addKey(*subKey); } - Q_FOREACH (const ChallengeResponseKey* subKey, key.m_challengeResponseKeys) { + for (const ChallengeResponseKey* subKey : asConst(key.m_challengeResponseKeys)) { addChallengeResponseKey(*subKey); } @@ -160,7 +160,7 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const CryptoHash cryptoHash(CryptoHash::Sha256); - Q_FOREACH (ChallengeResponseKey* key, m_challengeResponseKeys) { + for (ChallengeResponseKey* key : m_challengeResponseKeys) { /* If the device isn't present or fails, return an error */ if (key->challenge(seed) == false) { return false; From 05774854efb4e32280aac7d7bcd420779c4ec375 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 14 Jan 2017 16:07:02 -0800 Subject: [PATCH 16/48] travis-ci: Add YubiKey development libraries * With these packages cmake will detect the YubiKey headers and libraries and build in YubiKey support. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index a7807b612..adb8afdd7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,7 @@ git: before_install: - if [ "$TRAVIS_OS_NAME" = "linux" ]; then sudo apt-get -qq update; fi - - if [ "$TRAVIS_OS_NAME" = "linux" ]; then sudo apt-get -qq install cmake libmicrohttpd10 libmicrohttpd-dev libxi-dev qtbase5-dev libqt5x11extras5-dev qttools5-dev qttools5-dev-tools libgcrypt20-dev zlib1g-dev libxtst-dev xvfb; fi + - if [ "$TRAVIS_OS_NAME" = "linux" ]; then sudo apt-get -qq install cmake libmicrohttpd10 libmicrohttpd-dev libxi-dev qtbase5-dev libqt5x11extras5-dev qttools5-dev qttools5-dev-tools libgcrypt20-dev zlib1g-dev libxtst-dev xvfb libyubikey-dev libykpers-1-dev; fi - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew update; fi - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew ls | grep -wq cmake || brew install cmake; fi - if [ "$TRAVIS_OS_NAME" = "osx" ]; then brew ls | grep -wq qt5 || brew install qt5; fi From f33cd1541941a088b90f463d457f4a9f86aa712a Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 14 Jan 2017 17:08:48 -0800 Subject: [PATCH 17/48] gui: Clear YubiKeys detected on each load * Clear the YubiKey detected list on each load. * In the event the YubiKey was removed, it will no longer be displayed. * If it's still present it won't be duplicated. --- src/gui/DatabaseOpenWidget.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 5d3815963..0b63bc0fc 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -64,6 +64,10 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); + + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), + SLOT(ykDetected(int,bool)), + Qt::QueuedConnection); } DatabaseOpenWidget::~DatabaseOpenWidget() @@ -92,10 +96,8 @@ void DatabaseOpenWidget::load(const QString& filename) m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); - /* YubiKey init is slow */ - connect(YubiKey::instance(), SIGNAL(detected(int,bool)), - SLOT(ykDetected(int,bool)), - Qt::QueuedConnection); + /* YubiKey init is slow, detect asynchronously to not block the UI */ + m_ui->comboChallengeResponse->clear(); QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); m_ui->editPassword->setFocus(); From a7cf39c7cd07fd27eb3d521403ad92d69ef4cf74 Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 14 Jan 2017 17:36:14 -0800 Subject: [PATCH 18/48] gui: ChangeMasterKeyWidget: Clear YubiKeys detected * Clear the YubiKey detected list on each load. * In the event the YubiKey was removed, it will no longer be displayed. * If it's still present it won't be duplicated. --- src/gui/ChangeMasterKeyWidget.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index c69cf2dcc..f2cf3b8d9 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -41,6 +41,10 @@ ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) m_ui->repeatPasswordEdit->enableVerifyMode(m_ui->enterPasswordEdit); connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); + + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), + SLOT(ykDetected(int,bool)), + Qt::QueuedConnection); } ChangeMasterKeyWidget::~ChangeMasterKeyWidget() @@ -88,10 +92,8 @@ void ChangeMasterKeyWidget::clearForms() m_ui->challengeResponseGroup->setChecked(false); m_ui->challengeResponseCombo->clear(); - /* YubiKey init is slow */ - connect(YubiKey::instance(), SIGNAL(detected(int,bool)), - SLOT(ykDetected(int,bool)), - Qt::QueuedConnection); + /* YubiKey init is slow, detect asynchronously to not block the UI */ + m_ui->challengeResponseCombo->clear(); QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); m_ui->enterPasswordEdit->setFocus(); From 5a3ed27fedd5668bab0b9ffab7a815d1c112884e Mon Sep 17 00:00:00 2001 From: Kyle Manna Date: Sat, 14 Jan 2017 17:58:05 -0800 Subject: [PATCH 19/48] tests: yk: Initialize Crypto class Initialize the Crypto class before running the tests as YubiKey detection code depends on it for random data. --- tests/TestYkChallengeResponseKey.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp index 91fba83cb..bd58ac018 100644 --- a/tests/TestYkChallengeResponseKey.cpp +++ b/tests/TestYkChallengeResponseKey.cpp @@ -21,6 +21,7 @@ #include #include +#include "crypto/Crypto.h" #include "keys/YkChallengeResponseKey.h" QTEST_GUILESS_MAIN(TestYubiKeyChalResp) @@ -44,6 +45,9 @@ void TestYubiKeyChalResp::init() if (!result) { QSKIP("Unable to connect to YubiKey", SkipAll); } + + /* Crypto subsystem needs to be initalized for YubiKey testing */ + QVERIFY(Crypto::init()); } void TestYubiKeyChalResp::detectDevices() From 7174549441d79f6f7de17caf9253d1fac5eafbb0 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 20:35:28 +0100 Subject: [PATCH 20/48] Align YubiKey combobox with rest of interface --- src/gui/DatabaseOpenWidget.ui | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index b3abbb287..01b82d407 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -7,7 +7,7 @@ 0 0 596 - 250 + 262 @@ -85,7 +85,7 @@ - + 5 @@ -118,7 +118,7 @@ - + 5 @@ -142,7 +142,7 @@ - + false @@ -152,12 +152,22 @@ - - - - false + + + + 5 - + + 5 + + + + + false + + + + From eb23dda99b187871745b274c34f1d1cd15d4ded9 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 22:07:01 +0100 Subject: [PATCH 21/48] Remember if challenge-response was used for each database and allow to re-detect Yubikeys without closing the database first --- src/gui/DatabaseOpenWidget.cpp | 47 ++++++++++++++++++++++++++++------ src/gui/DatabaseOpenWidget.h | 5 +++- src/gui/DatabaseOpenWidget.ui | 21 ++++++++++++++- src/keys/drivers/YubiKey.cpp | 5 ++-- src/keys/drivers/YubiKey.h | 5 ++++ 5 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index f5d582c60..83488317f 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -67,9 +67,10 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); - connect(YubiKey::instance(), SIGNAL(detected(int,bool)), - SLOT(ykDetected(int,bool)), - Qt::QueuedConnection); + connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); + + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); + connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection); #ifdef Q_OS_MACOS // add random padding to layouts to align widgets properly @@ -105,9 +106,8 @@ void DatabaseOpenWidget::load(const QString& filename) m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); - /* YubiKey init is slow, detect asynchronously to not block the UI */ m_ui->comboChallengeResponse->clear(); - QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + pollYubikey(); m_ui->editPassword->setFocus(); } @@ -169,6 +169,7 @@ CompositeKey DatabaseOpenWidget::databaseKey() } QHash lastKeyFiles = config()->get("LastKeyFiles").toHash(); + QHash lastChallengeResponse = config()->get("LastChallengeResponse").toHash(); if (m_ui->checkKeyFile->isChecked()) { FileKey key; @@ -181,13 +182,19 @@ CompositeKey DatabaseOpenWidget::databaseKey() } masterKey.addKey(key); lastKeyFiles[m_filename] = keyFilename; - } - else { + } else { lastKeyFiles.remove(m_filename); } + if (m_ui->checkChallengeResponse->isChecked()) { + lastChallengeResponse[m_filename] = true; + } else { + lastChallengeResponse.remove(m_filename); + } + if (config()->get("RememberLastKeyFiles").toBool()) { config()->set("LastKeyFiles", lastKeyFiles); + config()->set("LastChallengeResponse", lastChallengeResponse); } @@ -240,10 +247,34 @@ void DatabaseOpenWidget::browseKeyFile() } } -void DatabaseOpenWidget::ykDetected(int slot, bool blocking) +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(); + 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->setEnabled(true); m_ui->checkChallengeResponse->setEnabled(true); + m_ui->buttonRedetectYubikey->setEnabled(true); + + if (config()->get("RememberLastKeyFiles").toBool()) { + QHash lastChallengeResponse = config()->get("LastChallengeResponse").toHash(); + if (lastChallengeResponse.contains(m_filename)) { + m_ui->checkChallengeResponse->setChecked(true); + } + } +} + +void DatabaseOpenWidget::noYubikeyFound() +{ + m_ui->buttonRedetectYubikey->setEnabled(true); } diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index fadb5ee70..d02346055 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -58,9 +58,12 @@ private Q_SLOTS: void activateChallengeResponse(); void setOkButtonEnabled(); void browseKeyFile(); - void ykDetected(int slot, bool blocking); + void yubikeyDetected(int slot, bool blocking); + void noYubikeyFound(); + void pollYubikey(); protected: + const QScopedPointer m_ui; Database* m_db; QString m_filename; diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index 01b82d407..f835fc2f6 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -7,7 +7,7 @@ 0 0 596 - 262 + 264 @@ -165,6 +165,25 @@ false + + + 0 + 0 + + + + false + + + + + + + false + + + Refresh + diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 5ca175836..20e91237d 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -117,7 +117,6 @@ bool YubiKey::deinit() void YubiKey::detect() { if (init()) { - for (int i = 1; i < 3; i++) { YubiKey::ChallengeResult result; QByteArray rand = randomGen()->randomArray(1); @@ -126,10 +125,12 @@ void YubiKey::detect() result = challenge(i, false, rand, resp); if (result != YubiKey::ERROR) { - Q_EMIT detected(i, result == YubiKey::WOULDBLOCK ? true : false); + emit detected(i, result == YubiKey::WOULDBLOCK ? true : false); + return; } } } + emit notFound(); } /** diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 492fba01d..0441e69a7 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -56,6 +56,11 @@ Q_SIGNALS: */ void detected(int slot, bool blocking); + /** + * Emitted when no Yubikey could be found. + */ + void notFound(); + private: explicit YubiKey(); static YubiKey* m_instance; From c7defdc06f698118569ad687c0c0ae34fa607542 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 22:41:39 +0100 Subject: [PATCH 22/48] Add redetect button to ChangeMasterKeyWidget and only poll for Yubikeys when the challenge response group is enabled --- src/gui/ChangeMasterKeyWidget.cpp | 50 +++++++++++++++++++++---------- src/gui/ChangeMasterKeyWidget.h | 5 +++- src/gui/ChangeMasterKeyWidget.ui | 30 ++++++++++++------- 3 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 7f0fb0663..bb2b201dc 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -44,9 +44,11 @@ ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); - connect(YubiKey::instance(), SIGNAL(detected(int,bool)), - SLOT(ykDetected(int,bool)), - Qt::QueuedConnection); + connect(m_ui->challengeResponseGroup, SIGNAL(clicked(bool)), SLOT(challengeResponseGroupToggled(bool))); + connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); + + connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); + connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection); } ChangeMasterKeyWidget::~ChangeMasterKeyWidget() @@ -89,14 +91,9 @@ void ChangeMasterKeyWidget::clearForms() m_ui->repeatPasswordEdit->setText(""); m_ui->keyFileGroup->setChecked(false); m_ui->togglePasswordButton->setChecked(false); - // TODO: clear m_ui->keyFileCombo m_ui->challengeResponseGroup->setChecked(false); - m_ui->challengeResponseCombo->clear(); - - /* YubiKey init is slow, detect asynchronously to not block the UI */ - m_ui->challengeResponseCombo->clear(); - QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); + m_ui->comboChallengeResponse->clear(); m_ui->enterPasswordEdit->setFocus(); } @@ -146,29 +143,50 @@ void ChangeMasterKeyWidget::generateKey() } if (m_ui->challengeResponseGroup->isChecked()) { - int i = m_ui->challengeResponseCombo->currentIndex(); - i = m_ui->challengeResponseCombo->itemData(i).toInt(); + int i = m_ui->comboChallengeResponse->currentIndex(); + i = m_ui->comboChallengeResponse->itemData(i).toInt(); YkChallengeResponseKey key(i); m_key.addChallengeResponseKey(key); } m_ui->messageWidget->hideMessage(); - Q_EMIT editFinished(true); + emit editFinished(true); } void ChangeMasterKeyWidget::reject() { - Q_EMIT editFinished(false); + emit editFinished(false); } +void ChangeMasterKeyWidget::challengeResponseGroupToggled(bool checked) +{ + if (checked) + pollYubikey(); +} -void ChangeMasterKeyWidget::ykDetected(int slot, bool blocking) +void ChangeMasterKeyWidget::pollYubikey() +{ + m_ui->buttonRedetectYubikey->setEnabled(false); + m_ui->comboChallengeResponse->setEnabled(false); + m_ui->comboChallengeResponse->clear(); + + // YubiKey init is slow, detect asynchronously to not block the UI + QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); +} + +void ChangeMasterKeyWidget::yubikeyDetected(int slot, bool blocking) { YkChallengeResponseKey yk(slot, blocking); - m_ui->challengeResponseCombo->addItem(yk.getName(), QVariant(slot)); - m_ui->challengeResponseGroup->setEnabled(true); + m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant(slot)); + m_ui->comboChallengeResponse->setEnabled(m_ui->challengeResponseGroup->isChecked()); + m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); +} + +void ChangeMasterKeyWidget::noYubikeyFound() +{ + m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); } void ChangeMasterKeyWidget::setCancelEnabled(bool enabled) diff --git a/src/gui/ChangeMasterKeyWidget.h b/src/gui/ChangeMasterKeyWidget.h index cbf67c1f6..0a99037f0 100644 --- a/src/gui/ChangeMasterKeyWidget.h +++ b/src/gui/ChangeMasterKeyWidget.h @@ -48,7 +48,10 @@ private Q_SLOTS: void reject(); void createKeyFile(); void browseKeyFile(); - void ykDetected(int slot, bool blocking); + void yubikeyDetected(int slot, bool blocking); + void noYubikeyFound(); + void challengeResponseGroupToggled(bool checked); + void pollYubikey(); private: const QScopedPointer m_ui; diff --git a/src/gui/ChangeMasterKeyWidget.ui b/src/gui/ChangeMasterKeyWidget.ui index c867e1971..5640364ed 100644 --- a/src/gui/ChangeMasterKeyWidget.ui +++ b/src/gui/ChangeMasterKeyWidget.ui @@ -7,7 +7,7 @@ 0 0 818 - 397 + 424 @@ -90,7 +90,7 @@ - Key file + &Key file true @@ -129,22 +129,32 @@ - false + true - Challenge Response + Cha&llenge Response true - - - - + + true + + - + + + + 0 + 0 + + + + + + - Challenge Response: + Refresh From c49aa6beef446d04966d9b6bddf975722e732065 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 22:50:12 +0100 Subject: [PATCH 23/48] Show error message when trying to use challenge response without YubiKey --- src/gui/ChangeMasterKeyWidget.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index bb2b201dc..b5a465af5 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -145,6 +145,13 @@ void ChangeMasterKeyWidget::generateKey() if (m_ui->challengeResponseGroup->isChecked()) { int i = m_ui->comboChallengeResponse->currentIndex(); i = m_ui->comboChallengeResponse->itemData(i).toInt(); + + if (0 == i) { + m_ui->messageWidget->showMessage(tr("Changing master key failed: no YubiKey inserted."), + MessageWidget::Error); + return; + } + YkChallengeResponseKey key(i); m_key.addChallengeResponseKey(key); From 5d068dfb2359fa8cecc750aaf792cd65e366e039 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 23:20:32 +0100 Subject: [PATCH 24/48] Show busy indicator while scanning for YubiKeys --- src/gui/ChangeMasterKeyWidget.cpp | 13 +++++- src/gui/ChangeMasterKeyWidget.ui | 56 +++++++++++++++++++------- src/gui/DatabaseOpenWidget.cpp | 8 ++++ src/gui/DatabaseOpenWidget.ui | 67 +++++++++++++++++++++++-------- 4 files changed, 111 insertions(+), 33 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index b5a465af5..8f9839c97 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -36,11 +36,17 @@ ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) m_ui->messageWidget->setHidden(true); + m_ui->togglePasswordButton->setIcon(filePath()->onOffIcon("actions", "password-show")); + m_ui->repeatPasswordEdit->enableVerifyMode(m_ui->enterPasswordEdit); + + m_ui->yubikeyProgress->setVisible(false); + QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); + sp.setRetainSizeWhenHidden(true); + m_ui->yubikeyProgress->setSizePolicy(sp); + connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(generateKey())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); - m_ui->togglePasswordButton->setIcon(filePath()->onOffIcon("actions", "password-show")); connect(m_ui->togglePasswordButton, SIGNAL(toggled(bool)), m_ui->enterPasswordEdit, SLOT(setShowPassword(bool))); - m_ui->repeatPasswordEdit->enableVerifyMode(m_ui->enterPasswordEdit); connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); @@ -178,6 +184,7 @@ void ChangeMasterKeyWidget::pollYubikey() m_ui->buttonRedetectYubikey->setEnabled(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); @@ -189,11 +196,13 @@ void ChangeMasterKeyWidget::yubikeyDetected(int slot, bool blocking) m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant(slot)); m_ui->comboChallengeResponse->setEnabled(m_ui->challengeResponseGroup->isChecked()); m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); + m_ui->yubikeyProgress->setVisible(false); } void ChangeMasterKeyWidget::noYubikeyFound() { m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); + m_ui->yubikeyProgress->setVisible(false); } void ChangeMasterKeyWidget::setCancelEnabled(bool enabled) diff --git a/src/gui/ChangeMasterKeyWidget.ui b/src/gui/ChangeMasterKeyWidget.ui index 5640364ed..693d8ac1d 100644 --- a/src/gui/ChangeMasterKeyWidget.ui +++ b/src/gui/ChangeMasterKeyWidget.ui @@ -7,7 +7,7 @@ 0 0 818 - 424 + 471 @@ -142,21 +142,47 @@ - - - - 0 - 0 - + + + 0 - - - - - - Refresh - - + + + + Refresh + + + + + + + + 0 + 0 + + + + + + + + + 16777215 + 2 + + + + 0 + + + -1 + + + false + + + + diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 83488317f..d9dd78f0a 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -49,6 +49,11 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false); + m_ui->yubikeyProgress->setVisible(false); + QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); + sp.setRetainSizeWhenHidden(true); + m_ui->yubikeyProgress->setSizePolicy(sp); + m_ui->buttonTogglePassword->setIcon(filePath()->onOffIcon("actions", "password-show")); connect(m_ui->buttonTogglePassword, SIGNAL(toggled(bool)), m_ui->editPassword, SLOT(setShowPassword(bool))); @@ -255,6 +260,7 @@ void DatabaseOpenWidget::pollYubikey() m_ui->checkChallengeResponse->setChecked(false); m_ui->comboChallengeResponse->setEnabled(false); m_ui->comboChallengeResponse->clear(); + m_ui->yubikeyProgress->setVisible(true); QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); } @@ -265,6 +271,7 @@ void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) m_ui->comboChallengeResponse->setEnabled(true); m_ui->checkChallengeResponse->setEnabled(true); m_ui->buttonRedetectYubikey->setEnabled(true); + m_ui->yubikeyProgress->setVisible(false); if (config()->get("RememberLastKeyFiles").toBool()) { QHash lastChallengeResponse = config()->get("LastChallengeResponse").toHash(); @@ -277,4 +284,5 @@ void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) void DatabaseOpenWidget::noYubikeyFound() { m_ui->buttonRedetectYubikey->setEnabled(true); + m_ui->yubikeyProgress->setVisible(false); } diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index f835fc2f6..7a97d0083 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -7,7 +7,7 @@ 0 0 596 - 264 + 302 @@ -142,25 +142,28 @@ - - - - false - - - Challenge Response: - - - - - + + 5 5 - + + 0 + + + + + false + + + Refresh + + + + false @@ -176,13 +179,45 @@ + + + + + 16777215 + 2 + + + + 0 + + + 0 + + + -1 + + + false + + + + + + + + + 0 + + + 2 + - + false - Refresh + Challenge Response: From 2d4e08e060ce18d0040ffcc0c9a2af17e28a81ff Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 20 Feb 2017 23:35:03 +0100 Subject: [PATCH 25/48] Warn user when no authentication factor was chosen --- src/gui/ChangeMasterKeyWidget.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 8f9839c97..051ff76ae 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -118,12 +118,24 @@ void ChangeMasterKeyWidget::generateKey() { m_key.clear(); + bool anyChecked = m_ui->passwordGroup->isChecked(); + anyChecked |= m_ui->keyFileGroup->isChecked(); + anyChecked |= m_ui->challengeResponseGroup->isChecked(); + + if (!anyChecked) { + if (MessageBox::warning(this, tr("No authentication factor chosen"), + tr("Your database will be completely unprotected!
Do you really want to continue?"), + QMessageBox::Yes | QMessageBox::No) != QMessageBox::Yes) { + return; + } + } + if (m_ui->passwordGroup->isChecked()) { if (m_ui->enterPasswordEdit->text() == m_ui->repeatPasswordEdit->text()) { if (m_ui->enterPasswordEdit->text().isEmpty()) { - if (MessageBox::question(this, tr("Question"), - tr("Do you really want to use an empty string as password?"), - QMessageBox::Yes | QMessageBox::No) != QMessageBox::Yes) { + if (MessageBox::warning(this, tr("Empty password"), + tr("Do you really want to use an empty string as password?"), + QMessageBox::Yes | QMessageBox::No) != QMessageBox::Yes) { return; } } From 8d3e0687a0e54a5fab5866929ae38005277280c2 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 21 Feb 2017 00:28:01 +0100 Subject: [PATCH 26/48] Restructure doc comments and make hard-coded strings translatable --- src/keys/YkChallengeResponseKey.cpp | 9 +++-- src/keys/drivers/YubiKey.cpp | 55 +++++------------------------ src/keys/drivers/YubiKey.h | 45 +++++++++++++++++++---- 3 files changed, 53 insertions(+), 56 deletions(-) diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 17982ab62..51520e3d4 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -27,6 +27,7 @@ #include "keys/drivers/YubiKey.h" #include +#include YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) @@ -58,10 +59,12 @@ bool YkChallengeResponseKey::challenge(const QByteArray& chal, int retries) return true; } - /* If challenge failed, retry to detect YubiKeys int the event the YubiKey + /* 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--; if (YubiKey::instance()->init() != true) { @@ -79,13 +82,13 @@ bool YkChallengeResponseKey::challenge(const QByteArray& chal, int retries) QString YkChallengeResponseKey::getName() const { unsigned int serial; - QString fmt("YubiKey[%1] Challenge Response - Slot %2 - %3"); + QString fmt(QObject::tr("YubiKey[%1] Challenge Response - Slot %2 - %3")); YubiKey::instance()->getSerial(serial); return fmt.arg(QString::number(serial), QString::number(m_slot), - (m_blocking) ? "Press" : "Passive"); + (m_blocking) ? QObject::tr("Press") : QObject::tr("Passive")); } bool YkChallengeResponseKey::isBlocking() const diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 20e91237d..b287c13b2 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -29,9 +29,8 @@ #include "YubiKey.h" -/* Cast the void pointer from the generalized class definition - * to the proper pointer type from the now included system headers - */ +// 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(m_yk_void)) #define m_ykds (static_cast(m_ykds_void)) @@ -41,10 +40,6 @@ YubiKey::YubiKey() : m_yk_void(NULL), m_ykds_void(NULL) YubiKey* YubiKey::m_instance(Q_NULLPTR); -/** - * @brief YubiKey::instance - get instance of singleton - * @return - */ YubiKey* YubiKey::instance() { if (!m_instance) { @@ -54,20 +49,16 @@ YubiKey* YubiKey::instance() return m_instance; } -/** - * @brief YubiKey::init - initialize yubikey library and hardware - * @return - */ bool YubiKey::init() { - /* Previously initalized */ + // previously initialized if (m_yk != NULL && m_ykds != NULL) { if (yk_get_status(m_yk, m_ykds)) { - /* Still connected */ + // Still connected return true; } else { - /* Initialized but not connected anymore, re-init */ + // Initialized but not connected anymore, re-init deinit(); } } @@ -76,7 +67,7 @@ bool YubiKey::init() return false; } - /* TODO: handle multiple attached hardware devices, currently own one */ + // TODO: handle multiple attached hardware devices m_yk_void = static_cast(yk_open_first_key()); if (m_yk == NULL) { return false; @@ -92,10 +83,6 @@ bool YubiKey::init() return true; } -/** - * @brief YubiKey::deinit - cleanup after init - * @return true on success - */ bool YubiKey::deinit() { if (m_yk) { @@ -111,9 +98,6 @@ bool YubiKey::deinit() return true; } -/** - * @brief YubiKey::detect - probe for attached YubiKeys - */ void YubiKey::detect() { if (init()) { @@ -133,11 +117,6 @@ void YubiKey::detect() emit notFound(); } -/** - * @brief YubiKey::getSerial - serial number of yubikey - * @param serial - * @return - */ bool YubiKey::getSerial(unsigned int& serial) const { if (!yk_get_serial(m_yk, 1, 0, &serial)) { @@ -162,23 +141,7 @@ static inline QString printByteArray(const QByteArray& a) } #endif -/** - * @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 chal challenge input to YubiKey - * @param resp response output from YubiKey - * @return SUCCESS when successful - */ -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, - const QByteArray& chal, - QByteArray& resp) const +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) const { int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2; QByteArray paddedChal = chal; @@ -188,7 +151,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, return ERROR; } - /* yk_challenge_response() insists on 64 byte response buffer */ + // yk_challenge_response() insists on 64 byte response buffer */ resp.resize(64); /* The challenge sent to the yubikey should always be 64 bytes for @@ -239,7 +202,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, } } - /* Actual HMAC-SHA1 response is only 20 bytes */ + // actual HMAC-SHA1 response is only 20 bytes resp.resize(20); #ifdef QT_DEBUG diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 0441e69a7..acf4feb72 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -30,21 +30,52 @@ class YubiKey : public QObject public: enum ChallengeResult { ERROR = -1, SUCCESS = 0, WOULDBLOCK }; + /** + * @brief YubiKey::instance - get instance of singleton + * @return instance + */ static YubiKey* instance(); - /** Initialize the underlying yubico libraries */ + /** + * @brief YubiKey::init - initialize yubikey library and hardware + * @return true on success + */ bool init(); + + /** + * @brief YubiKey::deinit - cleanup after init + * @return true on success + */ bool deinit(); - /** Issue a challenge to the hardware */ + /** + * @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 chal challenge input to YubiKey + * @param resp response output from YubiKey + * @return true on success + */ ChallengeResult challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) const; - /** Read the serial number from the hardware */ + /** + * @brief YubiKey::getSerial - serial number of YubiKey + * @param serial serial number + * @return true on success + */ bool getSerial(unsigned int& serial) const; - /** Start looking for attached hardware devices */ + /** + * @brief YubiKey::detect - probe for attached YubiKeys + */ void detect(); Q_SIGNALS: @@ -65,9 +96,9 @@ private: explicit YubiKey(); static YubiKey* m_instance; - /* Create void ptr here to avoid ifdef header include mess */ - void *m_yk_void; - void *m_ykds_void; + // Create void ptr here to avoid ifdef header include mess + void* m_yk_void; + void* m_ykds_void; Q_DISABLE_COPY(YubiKey) }; From b2650c5a965a9da902dd294a0133f21ca8707ce5 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 21 Feb 2017 01:06:32 +0100 Subject: [PATCH 27/48] Hide UI elements when KeePassXC was compiled without -DWITH_XC_YUBIKEY --- CMakeLists.txt | 5 ++-- src/CMakeLists.txt | 9 +++---- src/gui/ChangeMasterKeyWidget.cpp | 17 +++++++++--- src/gui/DatabaseOpenWidget.cpp | 45 ++++++++++++++++++++----------- src/keys/drivers/YubiKeyStub.cpp | 4 +-- 5 files changed, 50 insertions(+), 30 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c3bf58cc..ecce4f0ad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -195,13 +195,12 @@ if(NOT ZLIB_SUPPORTS_GZIP) endif() # Optional -find_package(YubiKey) +if(WITH_XC_YUBIKEY) + find_package(YubiKey REQUIRED) -if(YUBIKEY_FOUND) include_directories(SYSTEM ${YUBIKEY_INCLUDE_DIRS}) endif() - if(UNIX) check_cxx_source_compiles("#include int main() { prctl(PR_SET_DUMPABLE, 0); return 0; }" diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 45c711b73..e27c24928 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -179,7 +179,7 @@ if(MINGW) ${CMAKE_SOURCE_DIR}/share/windows/icon.rc) endif() -if(YUBIKEY_FOUND) +if(WITH_XC_YUBIKEY) set(keepassx_SOURCES ${keepassx_SOURCES} keys/drivers/YubiKey.cpp) else() set(keepassx_SOURCES ${keepassx_SOURCES} keys/drivers/YubiKeyStub.cpp) @@ -209,7 +209,8 @@ target_link_libraries(keepassx_core Qt5::Network ${GCRYPT_LIBRARIES} ${GPGERROR_LIBRARIES} - ${ZLIB_LIBRARIES}) + ${ZLIB_LIBRARIES} + ${YUBIKEY_LIBRARIES}) if (UNIX AND NOT APPLE) target_link_libraries(keepassx_core Qt5::DBus) endif() @@ -234,10 +235,6 @@ endif() add_executable(${PROGNAME} WIN32 MACOSX_BUNDLE ${keepassx_SOURCES_MAINEXE} ${WIN32_ProductVersionFiles}) target_link_libraries(${PROGNAME} keepassx_core) -if(YUBIKEY_FOUND) - target_link_libraries(keepassx_core ${YUBIKEY_LIBRARIES}) -endif() - set_target_properties(${PROGNAME} PROPERTIES ENABLE_EXPORTS ON) if(APPLE) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 051ff76ae..32048e342 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -28,6 +28,8 @@ #include "gui/MessageBox.h" #include "crypto/Random.h" +#include "config-keepassx.h" + ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) : DialogyWidget(parent) , m_ui(new Ui::ChangeMasterKeyWidget()) @@ -50,11 +52,15 @@ ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); +#ifdef WITH_XC_YUBIKEY connect(m_ui->challengeResponseGroup, SIGNAL(clicked(bool)), SLOT(challengeResponseGroupToggled(bool))); connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection); +#else + m_ui->challengeResponseGroup->setVisible(false); +#endif } ChangeMasterKeyWidget::~ChangeMasterKeyWidget() @@ -98,8 +104,10 @@ void ChangeMasterKeyWidget::clearForms() m_ui->keyFileGroup->setChecked(false); m_ui->togglePasswordButton->setChecked(false); +#ifdef WITH_XC_YUBIKEY m_ui->challengeResponseGroup->setChecked(false); m_ui->comboChallengeResponse->clear(); +#endif m_ui->enterPasswordEdit->setFocus(); } @@ -118,9 +126,10 @@ void ChangeMasterKeyWidget::generateKey() { m_key.clear(); - bool anyChecked = m_ui->passwordGroup->isChecked(); - anyChecked |= m_ui->keyFileGroup->isChecked(); - anyChecked |= m_ui->challengeResponseGroup->isChecked(); + bool anyChecked = m_ui->passwordGroup->isChecked() | m_ui->keyFileGroup->isChecked(); +#ifdef WITH_XC_YUBIKEY + anyChecked |= m_ui->challengeResponseGroup->isChecked(); +#endif if (!anyChecked) { if (MessageBox::warning(this, tr("No authentication factor chosen"), @@ -160,6 +169,7 @@ void ChangeMasterKeyWidget::generateKey() m_key.addKey(fileKey); } +#ifdef WITH_XC_YUBIKEY if (m_ui->challengeResponseGroup->isChecked()) { int i = m_ui->comboChallengeResponse->currentIndex(); i = m_ui->comboChallengeResponse->itemData(i).toInt(); @@ -174,6 +184,7 @@ void ChangeMasterKeyWidget::generateKey() m_key.addChallengeResponseKey(key); } +#endif m_ui->messageWidget->hideMessage(); emit editFinished(true); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index d9dd78f0a..bac987c0e 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -29,14 +29,16 @@ #include "format/KeePass2Reader.h" #include "keys/FileKey.h" #include "keys/PasswordKey.h" -#include "keys/YkChallengeResponseKey.h" #include "crypto/Random.h" +#include "keys/YkChallengeResponseKey.h" + +#include "config-keepassx.h" DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) - : DialogyWidget(parent) - , m_ui(new Ui::DatabaseOpenWidget()) - , m_db(nullptr) + : DialogyWidget(parent), + m_ui(new Ui::DatabaseOpenWidget()), + m_db(nullptr) { m_ui->setupUi(this); @@ -49,11 +51,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false); - m_ui->yubikeyProgress->setVisible(false); - QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); - sp.setRetainSizeWhenHidden(true); - m_ui->yubikeyProgress->setSizePolicy(sp); - m_ui->buttonTogglePassword->setIcon(filePath()->onOffIcon("actions", "password-show")); connect(m_ui->buttonTogglePassword, SIGNAL(toggled(bool)), m_ui->editPassword, SLOT(setShowPassword(bool))); @@ -61,21 +58,33 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->editPassword, SIGNAL(textChanged(QString)), SLOT(activatePassword())); connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(activateKeyFile())); - connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(activateChallengeResponse())); connect(m_ui->checkPassword, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); connect(m_ui->checkKeyFile, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(setOkButtonEnabled())); - connect(m_ui->checkChallengeResponse, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); - connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(setOkButtonEnabled())); connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); +#ifdef WITH_XC_YUBIKEY + m_ui->yubikeyProgress->setVisible(false); + QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); + sp.setRetainSizeWhenHidden(true); + m_ui->yubikeyProgress->setSizePolicy(sp); + connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); + connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(activateChallengeResponse())); + connect(m_ui->checkChallengeResponse, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); + connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(setOkButtonEnabled())); connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection); +#else + m_ui->checkChallengeResponse->setVisible(false); + m_ui->buttonRedetectYubikey->setVisible(false); + m_ui->comboChallengeResponse->setVisible(false); + m_ui->yubikeyProgress->setVisible(false); +#endif #ifdef Q_OS_MACOS // add random padding to layouts to align widgets properly @@ -109,10 +118,12 @@ void DatabaseOpenWidget::load(const QString& filename) } } - m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); - +#ifdef WITH_XC_YUBIKEY m_ui->comboChallengeResponse->clear(); pollYubikey(); +#endif + + m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); m_ui->editPassword->setFocus(); } @@ -199,9 +210,12 @@ CompositeKey DatabaseOpenWidget::databaseKey() if (config()->get("RememberLastKeyFiles").toBool()) { config()->set("LastKeyFiles", lastKeyFiles); - config()->set("LastChallengeResponse", lastChallengeResponse); } +#ifdef WITH_XC_YUBIKEY + if (config()->get("RememberLastKeyFiles").toBool()) { + config()->set("LastChallengeResponse", lastChallengeResponse); + } if (m_ui->checkChallengeResponse->isChecked()) { int i = m_ui->comboChallengeResponse->currentIndex(); @@ -210,6 +224,7 @@ CompositeKey DatabaseOpenWidget::databaseKey() masterKey.addChallengeResponseKey(key); } +#endif return masterKey; } diff --git a/src/keys/drivers/YubiKeyStub.cpp b/src/keys/drivers/YubiKeyStub.cpp index c00790f38..e93099bf4 100644 --- a/src/keys/drivers/YubiKeyStub.cpp +++ b/src/keys/drivers/YubiKeyStub.cpp @@ -58,9 +58,7 @@ bool YubiKey::getSerial(unsigned int& serial) const return false; } -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, - const QByteArray& chal, - QByteArray& resp) const +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) const { Q_UNUSED(slot); Q_UNUSED(mayBlock); From 8e91a89a3735b3e14597a2537542bacb9f05508d Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 21 Feb 2017 01:28:50 +0100 Subject: [PATCH 28/48] Force at least one encryption key (no more unencrypted databases) --- src/gui/ChangeMasterKeyWidget.cpp | 45 +++++++++++++++++-------------- src/gui/ChangeMasterKeyWidget.h | 7 +++-- src/gui/DatabaseOpenWidget.cpp | 2 -- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 32048e342..8c17ff575 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -41,19 +41,25 @@ ChangeMasterKeyWidget::ChangeMasterKeyWidget(QWidget* parent) m_ui->togglePasswordButton->setIcon(filePath()->onOffIcon("actions", "password-show")); m_ui->repeatPasswordEdit->enableVerifyMode(m_ui->enterPasswordEdit); + connect(m_ui->passwordGroup, SIGNAL(clicked(bool)), SLOT(setOkEnabled())); + connect(m_ui->togglePasswordButton, SIGNAL(toggled(bool)), m_ui->enterPasswordEdit, SLOT(setShowPassword(bool))); + + connect(m_ui->keyFileGroup, SIGNAL(clicked(bool)), SLOT(setOkEnabled())); + connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); + connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); + connect(m_ui->keyFileCombo, SIGNAL(editTextChanged(QString)), SLOT(setOkEnabled())); + + connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(generateKey())); + connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); + +#ifdef WITH_XC_YUBIKEY m_ui->yubikeyProgress->setVisible(false); QSizePolicy sp = m_ui->yubikeyProgress->sizePolicy(); sp.setRetainSizeWhenHidden(true); m_ui->yubikeyProgress->setSizePolicy(sp); - connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(generateKey())); - connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); - connect(m_ui->togglePasswordButton, SIGNAL(toggled(bool)), m_ui->enterPasswordEdit, SLOT(setShowPassword(bool))); - connect(m_ui->createKeyFileButton, SIGNAL(clicked()), SLOT(createKeyFile())); - connect(m_ui->browseKeyFileButton, SIGNAL(clicked()), SLOT(browseKeyFile())); - -#ifdef WITH_XC_YUBIKEY connect(m_ui->challengeResponseGroup, SIGNAL(clicked(bool)), SLOT(challengeResponseGroupToggled(bool))); + connect(m_ui->challengeResponseGroup, SIGNAL(clicked(bool)), SLOT(setOkEnabled())); connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); @@ -126,19 +132,6 @@ void ChangeMasterKeyWidget::generateKey() { m_key.clear(); - bool anyChecked = m_ui->passwordGroup->isChecked() | m_ui->keyFileGroup->isChecked(); -#ifdef WITH_XC_YUBIKEY - anyChecked |= m_ui->challengeResponseGroup->isChecked(); -#endif - - if (!anyChecked) { - if (MessageBox::warning(this, tr("No authentication factor chosen"), - tr("Your database will be completely unprotected!
Do you really want to continue?"), - QMessageBox::Yes | QMessageBox::No) != QMessageBox::Yes) { - return; - } - } - if (m_ui->passwordGroup->isChecked()) { if (m_ui->enterPasswordEdit->text() == m_ui->repeatPasswordEdit->text()) { if (m_ui->enterPasswordEdit->text().isEmpty()) { @@ -208,6 +201,7 @@ void ChangeMasterKeyWidget::pollYubikey() m_ui->comboChallengeResponse->setEnabled(false); m_ui->comboChallengeResponse->clear(); m_ui->yubikeyProgress->setVisible(true); + setOkEnabled(); // YubiKey init is slow, detect asynchronously to not block the UI QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); @@ -220,12 +214,23 @@ void ChangeMasterKeyWidget::yubikeyDetected(int slot, bool blocking) m_ui->comboChallengeResponse->setEnabled(m_ui->challengeResponseGroup->isChecked()); m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); m_ui->yubikeyProgress->setVisible(false); + setOkEnabled(); } void ChangeMasterKeyWidget::noYubikeyFound() { m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); m_ui->yubikeyProgress->setVisible(false); + setOkEnabled(); +} + +void ChangeMasterKeyWidget::setOkEnabled() +{ + bool ok = m_ui->passwordGroup->isChecked() || + (m_ui->challengeResponseGroup->isChecked() && !m_ui->comboChallengeResponse->currentText().isEmpty()) || + (m_ui->keyFileGroup->isChecked() && !m_ui->keyFileCombo->currentText().isEmpty()); + + m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(ok); } void ChangeMasterKeyWidget::setCancelEnabled(bool enabled) diff --git a/src/gui/ChangeMasterKeyWidget.h b/src/gui/ChangeMasterKeyWidget.h index 0a99037f0..b3e097276 100644 --- a/src/gui/ChangeMasterKeyWidget.h +++ b/src/gui/ChangeMasterKeyWidget.h @@ -38,12 +38,15 @@ public: void clearForms(); CompositeKey newMasterKey(); QLabel* headlineLabel(); + +public slots: + void setOkEnabled(); void setCancelEnabled(bool enabled); -Q_SIGNALS: +signals: void editFinished(bool accepted); -private Q_SLOTS: +private slots: void generateKey(); void reject(); void createKeyFile(); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index bac987c0e..107304266 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -123,8 +123,6 @@ void DatabaseOpenWidget::load(const QString& filename) pollYubikey(); #endif - m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(true); - m_ui->editPassword->setFocus(); } From 91761a2beab80b408e43b92cecf8e1e167950d27 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 21 Feb 2017 02:19:11 +0100 Subject: [PATCH 29/48] Only poll YubiKey for currently visible tab --- src/gui/DatabaseOpenWidget.cpp | 11 +++++------ src/gui/DatabaseOpenWidget.h | 11 ++++++----- src/gui/DatabaseOpenWidget.ui | 2 +- src/gui/DatabaseTabWidget.cpp | 4 ++-- src/gui/DatabaseTabWidget.h | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 107304266..475d00c72 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -102,6 +102,10 @@ void DatabaseOpenWidget::showEvent(QShowEvent* event) { DialogyWidget::showEvent(event); m_ui->editPassword->setFocus(); + +#ifdef WITH_XC_YUBIKEY + pollYubikey(); +#endif } void DatabaseOpenWidget::load(const QString& filename) @@ -118,11 +122,6 @@ void DatabaseOpenWidget::load(const QString& filename) } } -#ifdef WITH_XC_YUBIKEY - m_ui->comboChallengeResponse->clear(); - pollYubikey(); -#endif - m_ui->editPassword->setFocus(); } @@ -229,7 +228,7 @@ CompositeKey DatabaseOpenWidget::databaseKey() void DatabaseOpenWidget::reject() { - Q_EMIT editFinished(false); + emit editFinished(false); } void DatabaseOpenWidget::activatePassword() diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index d02346055..975a530c4 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -41,18 +41,21 @@ public: void enterKey(const QString& pw, const QString& keyFile); Database* database(); -Q_SIGNALS: +public slots: + void pollYubikey(); + +signals: void editFinished(bool accepted); protected: void showEvent(QShowEvent* event) override; CompositeKey databaseKey(); -protected Q_SLOTS: +protected slots: virtual void openDatabase(); void reject(); -private Q_SLOTS: +private slots: void activatePassword(); void activateKeyFile(); void activateChallengeResponse(); @@ -60,10 +63,8 @@ private Q_SLOTS: void browseKeyFile(); void yubikeyDetected(int slot, bool blocking); void noYubikeyFound(); - void pollYubikey(); protected: - const QScopedPointer m_ui; Database* m_db; QString m_filename; diff --git a/src/gui/DatabaseOpenWidget.ui b/src/gui/DatabaseOpenWidget.ui index 7a97d0083..dba71d0fa 100644 --- a/src/gui/DatabaseOpenWidget.ui +++ b/src/gui/DatabaseOpenWidget.ui @@ -156,7 +156,7 @@ - false + true Refresh diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 3a48098dd..734dc3f2e 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -53,7 +53,7 @@ const int DatabaseTabWidget::LastDatabasesCount = 5; DatabaseTabWidget::DatabaseTabWidget(QWidget* parent) : QTabWidget(parent) - , m_dbWidgetSateSync(new DatabaseWidgetStateSync(this)) + , m_dbWidgetStateSync(new DatabaseWidgetStateSync(this)) { DragTabBar* tabBar = new DragTabBar(this); setTabBar(tabBar); @@ -61,7 +61,7 @@ DatabaseTabWidget::DatabaseTabWidget(QWidget* parent) connect(this, SIGNAL(tabCloseRequested(int)), SLOT(closeDatabase(int))); connect(this, SIGNAL(currentChanged(int)), SLOT(emitActivateDatabaseChanged())); - connect(this, SIGNAL(activateDatabaseChanged(DatabaseWidget*)), m_dbWidgetSateSync, SLOT(setActive(DatabaseWidget*))); + connect(this, SIGNAL(activateDatabaseChanged(DatabaseWidget*)), m_dbWidgetStateSync, SLOT(setActive(DatabaseWidget*))); connect(autoType(), SIGNAL(globalShortcutTriggered()), SLOT(performGlobalAutoType())); } diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index 8f01a987d..6eabcd49c 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -116,7 +116,7 @@ private: KeePass2Writer m_writer; QHash m_dbList; - DatabaseWidgetStateSync* m_dbWidgetSateSync; + DatabaseWidgetStateSync* m_dbWidgetStateSync; }; #endif // KEEPASSX_DATABASETABWIDGET_H From e93e4a99315cb011bd8794a94ce2d79928c42a42 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Tue, 21 Feb 2017 02:40:23 +0100 Subject: [PATCH 30/48] Allow opening of unprotected databases (but don't allow creating them) --- src/gui/DatabaseOpenWidget.cpp | 16 ---------------- src/gui/DatabaseOpenWidget.h | 1 - 2 files changed, 17 deletions(-) diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 475d00c72..1543dc43c 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -49,8 +49,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) font.setPointSize(font.pointSize() + 2); m_ui->labelHeadline->setFont(font); - m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(false); - m_ui->buttonTogglePassword->setIcon(filePath()->onOffIcon("actions", "password-show")); connect(m_ui->buttonTogglePassword, SIGNAL(toggled(bool)), m_ui->editPassword, SLOT(setShowPassword(bool))); @@ -59,10 +57,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->editPassword, SIGNAL(textChanged(QString)), SLOT(activatePassword())); connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(activateKeyFile())); - connect(m_ui->checkPassword, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); - connect(m_ui->checkKeyFile, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); - connect(m_ui->comboKeyFile, SIGNAL(editTextChanged(QString)), SLOT(setOkButtonEnabled())); - connect(m_ui->buttonBox, SIGNAL(accepted()), SLOT(openDatabase())); connect(m_ui->buttonBox, SIGNAL(rejected()), SLOT(reject())); @@ -74,8 +68,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey())); connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(activateChallengeResponse())); - connect(m_ui->checkChallengeResponse, SIGNAL(toggled(bool)), SLOT(setOkButtonEnabled())); - connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(setOkButtonEnabled())); connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection); connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection); @@ -246,14 +238,6 @@ void DatabaseOpenWidget::activateChallengeResponse() m_ui->checkChallengeResponse->setChecked(true); } -void DatabaseOpenWidget::setOkButtonEnabled() -{ - bool enable = m_ui->checkPassword->isChecked() || m_ui->checkChallengeResponse->isChecked() - || (m_ui->checkKeyFile->isChecked() && !m_ui->comboKeyFile->currentText().isEmpty()); - - m_ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(enable); -} - void DatabaseOpenWidget::browseKeyFile() { QString filters = QString("%1 (*);;%2 (*.key)").arg(tr("All files"), tr("Key files")); diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index 975a530c4..caba70a61 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -59,7 +59,6 @@ private slots: void activatePassword(); void activateKeyFile(); void activateChallengeResponse(); - void setOkButtonEnabled(); void browseKeyFile(); void yubikeyDetected(int slot, bool blocking); void noYubikeyFound(); From 093fe5c7ef9a513f18b7f258ffcf6711e7346ba8 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Thu, 23 Feb 2017 23:52:36 +0100 Subject: [PATCH 31/48] Use QSharedPointer instead of cloning YkChallengeResponseKey and make it a QObject to allow emitting signals --- src/core/Database.cpp | 6 ++-- src/format/KeePass2Reader.cpp | 2 +- src/gui/ChangeMasterKeyWidget.cpp | 13 +++---- src/gui/DatabaseOpenWidget.cpp | 16 +++++---- src/keys/ChallengeResponseKey.h | 1 - src/keys/CompositeKey.cpp | 21 ++++++----- src/keys/CompositeKey.h | 5 +-- src/keys/YkChallengeResponseKey.cpp | 56 ++++++++++++++--------------- src/keys/YkChallengeResponseKey.h | 24 ++++++++++--- 9 files changed, 78 insertions(+), 66 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 22fc07230..a441910d1 100644 --- a/src/core/Database.cpp +++ b/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; } } diff --git a/src/format/KeePass2Reader.cpp b/src/format/KeePass2Reader.cpp index 33bea620c..ffe4e94fc 100644 --- a/src/format/KeePass2Reader.cpp +++ b/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); diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 8c17ff575..453498a17 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -15,8 +15,6 @@ * along with this program. If not, see . */ -#include - #include "ChangeMasterKeyWidget.h" #include "ui_ChangeMasterKeyWidget.h" @@ -30,6 +28,9 @@ #include "config-keepassx.h" +#include +#include + 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(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); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 1543dc43c..2eebcae3c 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -15,8 +15,6 @@ * along with this program. If not, see . */ -#include - #include "DatabaseOpenWidget.h" #include "ui_DatabaseOpenWidget.h" @@ -34,6 +32,9 @@ #include "config-keepassx.h" +#include +#include + 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(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); diff --git a/src/keys/ChallengeResponseKey.h b/src/keys/ChallengeResponseKey.h index e03a2f9f9..ac8c81650 100644 --- a/src/keys/ChallengeResponseKey.h +++ b/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; }; diff --git a/src/keys/CompositeKey.cpp b/src/keys/CompositeKey.cpp index 4e79cd05c..6114fd366 100644 --- a/src/keys/CompositeKey.cpp +++ b/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 key) { - m_challengeResponseKeys.append(key.clone()); + m_challengeResponseKeys.append(key); } + int CompositeKey::transformKeyBenchmark(int msec) { TransformKeyBenchmarkThread thread1(msec); diff --git a/src/keys/CompositeKey.h b/src/keys/CompositeKey.h index 531c2d9b2..50b2f699a 100644 --- a/src/keys/CompositeKey.h +++ b/src/keys/CompositeKey.h @@ -20,6 +20,7 @@ #include #include +#include #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 key); static int transformKeyBenchmark(int msec); static CompositeKey readFromLine(QString line); @@ -51,7 +52,7 @@ private: quint64 rounds, bool* ok, QString* errorString); QList m_keys; - QList m_challengeResponseKeys; + QList> m_challengeResponseKeys; }; #endif // KEEPASSX_COMPOSITEKEY_H diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 51520e3d4..a6e5cdbcf 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -15,7 +15,6 @@ * along with this program. If not, see . */ - #include #include @@ -26,11 +25,7 @@ #include "keys/YkChallengeResponseKey.h" #include "keys/drivers/YubiKey.h" -#include -#include - -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 +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; } diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index 8acb0f9e9..96e44220d 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/src/keys/YkChallengeResponseKey.h @@ -22,20 +22,34 @@ #include "keys/ChallengeResponseKey.h" #include "keys/drivers/YubiKey.h" -class YkChallengeResponseKey : public ChallengeResponseKey +#include + +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; From 70816f90b2c4488c9636ca60124eb897c7da6371 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 00:15:57 +0100 Subject: [PATCH 32/48] Make challenge() member thread-safe --- src/keys/drivers/YubiKey.cpp | 27 ++++++++++++++++----------- src/keys/drivers/YubiKey.h | 12 ++++++++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index b287c13b2..1d4445cd6 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -107,8 +107,10 @@ void YubiKey::detect() QByteArray resp; result = challenge(i, false, rand, resp); - - if (result != YubiKey::ERROR) { + if (result == YubiKey::ALREADY_RUNNING) { + emit alreadyRunning(); + return; + } else if (result != YubiKey::ERROR) { emit detected(i, result == YubiKey::WOULDBLOCK ? true : false); return; } @@ -141,13 +143,18 @@ static inline QString printByteArray(const QByteArray& a) } #endif -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) const +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) { + if (!m_mutex.tryLock()) { + return ALREADY_RUNNING; + } + int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2; QByteArray paddedChal = chal; - /* Ensure that YubiKey::init() succeeded */ + // ensure that YubiKey::init() succeeded if (m_yk == NULL) { + m_mutex.unlock(); return ERROR; } @@ -171,13 +178,12 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte r = reinterpret_cast(resp.data()); #ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") c = " - << printByteArray(paddedChal); + qDebug().nospace() << __func__ << "(" << slot << ") c = " << printByteArray(paddedChal); #endif - int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, - paddedChal.size(), c, - resp.size(), r); + int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChal.size(), c, resp.size(), r); + + m_mutex.unlock(); if (!ret) { if (yk_errno == YK_EWOULDBLOCK) { @@ -206,8 +212,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte resp.resize(20); #ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") r = " - << printByteArray(resp) << ", ret = " << ret; + qDebug().nospace() << __func__ << "(" << slot << ") r = " << printByteArray(resp) << ", ret = " << ret; #endif return SUCCESS; diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index acf4feb72..6c3504ed1 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -19,6 +19,7 @@ #define KEEPASSX_YUBIKEY_H #include +#include /** * Singleton class to manage the interface to the hardware @@ -28,7 +29,7 @@ class YubiKey : public QObject Q_OBJECT public: - enum ChallengeResult { ERROR = -1, SUCCESS = 0, WOULDBLOCK }; + enum ChallengeResult { ERROR = -1, SUCCESS = 0, WOULDBLOCK, ALREADY_RUNNING }; /** * @brief YubiKey::instance - get instance of singleton @@ -64,7 +65,7 @@ public: */ ChallengeResult challenge(int slot, bool mayBlock, const QByteArray& chal, - QByteArray& resp) const; + QByteArray& resp); /** * @brief YubiKey::getSerial - serial number of YubiKey @@ -92,6 +93,11 @@ Q_SIGNALS: */ void notFound(); + /** + * Emitted when detection is already running. + */ + void alreadyRunning(); + private: explicit YubiKey(); static YubiKey* m_instance; @@ -100,6 +106,8 @@ private: void* m_yk_void; void* m_ykds_void; + QMutex m_mutex; + Q_DISABLE_COPY(YubiKey) }; From 44ac7d152b4024c2355cb83c09ce3bf41491f0bf Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 01:06:09 +0100 Subject: [PATCH 33/48] Use better variable names --- src/keys/drivers/YubiKey.cpp | 24 ++++++++++++------------ src/keys/drivers/YubiKey.h | 8 +++----- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index 1d4445cd6..e68949394 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -111,7 +111,7 @@ void YubiKey::detect() emit alreadyRunning(); return; } else if (result != YubiKey::ERROR) { - emit detected(i, result == YubiKey::WOULDBLOCK ? true : false); + emit detected(i, result == YubiKey::WOULDBLOCK); return; } } @@ -143,14 +143,14 @@ static inline QString printByteArray(const QByteArray& a) } #endif -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response) { if (!m_mutex.tryLock()) { return ALREADY_RUNNING; } int yk_cmd = (slot == 1) ? SLOT_CHAL_HMAC1 : SLOT_CHAL_HMAC2; - QByteArray paddedChal = chal; + QByteArray paddedChallenge = challenge; // ensure that YubiKey::init() succeeded if (m_yk == NULL) { @@ -159,7 +159,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte } // yk_challenge_response() insists on 64 byte response buffer */ - resp.resize(64); + response.resize(64); /* The challenge sent to the yubikey should always be 64 bytes for * compatibility with all configurations. Follow PKCS7 padding. @@ -167,21 +167,21 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte * There is some question whether or not 64 byte fixed length * configurations even work, some docs say avoid it. */ - const int padLen = 64 - paddedChal.size(); + const int padLen = 64 - paddedChallenge.size(); if (padLen > 0) { - paddedChal.append(QByteArray(padLen, padLen)); + paddedChallenge.append(QByteArray(padLen, padLen)); } const unsigned char *c; unsigned char *r; - c = reinterpret_cast(paddedChal.constData()); - r = reinterpret_cast(resp.data()); + c = reinterpret_cast(paddedChallenge.constData()); + r = reinterpret_cast(response.data()); #ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") c = " << printByteArray(paddedChal); + qDebug().nospace() << __func__ << "(" << slot << ") c = " << printByteArray(paddedChallenge); #endif - int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChal.size(), c, resp.size(), r); + int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r); m_mutex.unlock(); @@ -209,10 +209,10 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte } // actual HMAC-SHA1 response is only 20 bytes - resp.resize(20); + response.resize(20); #ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") r = " << printByteArray(resp) << ", ret = " << ret; + qDebug().nospace() << __func__ << "(" << slot << ") r = " << printByteArray(response) << ", ret = " << ret; #endif return SUCCESS; diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 6c3504ed1..2382b69cf 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -59,13 +59,11 @@ public: * * @param slot YubiKey configuration slot * @param mayBlock operation is allowed to block - * @param chal challenge input to YubiKey - * @param resp response output from YubiKey + * @param challenge challenge input to YubiKey + * @param response response output from YubiKey * @return true on success */ - ChallengeResult challenge(int slot, bool mayBlock, - const QByteArray& chal, - QByteArray& resp); + ChallengeResult challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response); /** * @brief YubiKey::getSerial - serial number of YubiKey From d6c48a5cf1144e6d67a76fdf95708d2443451a36 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 03:25:08 +0100 Subject: [PATCH 34/48] Show message when user needs to touch their YubiKey (still buggy when using multiple databases) --- src/gui/Application.cpp | 5 +++++ src/gui/Application.h | 1 + src/gui/ChangeMasterKeyWidget.cpp | 15 +++++++++++++ src/gui/ChangeMasterKeyWidget.h | 2 ++ src/gui/DatabaseOpenWidget.cpp | 14 ++++++++++++ src/gui/DatabaseOpenWidget.h | 2 ++ src/gui/MainWindow.h | 9 +++++--- src/keys/YkChallengeResponseKey.cpp | 33 +++++++++++++++++++---------- src/keys/YkChallengeResponseKey.h | 4 ++-- src/keys/drivers/YubiKey.cpp | 1 + src/keys/drivers/YubiKey.h | 5 +++++ 11 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/gui/Application.cpp b/src/gui/Application.cpp index 26d9d2283..4c84fda86 100644 --- a/src/gui/Application.cpp +++ b/src/gui/Application.cpp @@ -87,6 +87,11 @@ Application::Application(int& argc, char** argv) #endif } +QWidget* Application::mainWindow() const +{ + return m_mainWindow; +} + void Application::setMainWindow(QWidget* mainWindow) { m_mainWindow = mainWindow; diff --git a/src/gui/Application.h b/src/gui/Application.h index 9bfe4d549..c6f4aae66 100644 --- a/src/gui/Application.h +++ b/src/gui/Application.h @@ -29,6 +29,7 @@ class Application : public QApplication public: Application(int& argc, char** argv); + QWidget* mainWindow() const; void setMainWindow(QWidget* mainWindow); bool event(QEvent* event) override; diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index 453498a17..b223d3922 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -25,6 +25,7 @@ #include "gui/FileDialog.h" #include "gui/MessageBox.h" #include "crypto/Random.h" +#include "MainWindow.h" #include "config-keepassx.h" @@ -176,6 +177,8 @@ void ChangeMasterKeyWidget::generateKey() bool blocking = i & true; int slot = i >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); + connect(key.data(), SIGNAL(userInteractionRequired()), SLOT(showYubiKeyPopup())); + connect(key.data(), SIGNAL(userConfirmed()), SLOT(hideYubiKeyPopup())); m_key.addChallengeResponseKey(key); } #endif @@ -238,3 +241,15 @@ void ChangeMasterKeyWidget::setCancelEnabled(bool enabled) { m_ui->buttonBox->button(QDialogButtonBox::Cancel)->setEnabled(enabled); } + +void ChangeMasterKeyWidget::showYubiKeyPopup() +{ + KEEPASSXC_MAIN_WINDOW->displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); + KEEPASSXC_MAIN_WINDOW->setEnabled(false); +} + +void ChangeMasterKeyWidget::hideYubiKeyPopup() +{ + KEEPASSXC_MAIN_WINDOW->hideGlobalMessage(); + KEEPASSXC_MAIN_WINDOW->setEnabled(true); +} diff --git a/src/gui/ChangeMasterKeyWidget.h b/src/gui/ChangeMasterKeyWidget.h index b3e097276..fdc7f9529 100644 --- a/src/gui/ChangeMasterKeyWidget.h +++ b/src/gui/ChangeMasterKeyWidget.h @@ -55,6 +55,8 @@ private slots: void noYubikeyFound(); void challengeResponseGroupToggled(bool checked); void pollYubikey(); + void showYubiKeyPopup(); + void hideYubiKeyPopup(); private: const QScopedPointer m_ui; diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 2eebcae3c..03a560c2f 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -214,6 +214,8 @@ CompositeKey DatabaseOpenWidget::databaseKey() bool blocking = i & true; int slot = i >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); + connect(key.data(), SIGNAL(userInteractionRequired()), SLOT(showYubiKeyPopup())); + connect(key.data(), SIGNAL(userConfirmed()), SLOT(hideYubiKeyPopup())); masterKey.addChallengeResponseKey(key); } #endif @@ -264,6 +266,18 @@ void DatabaseOpenWidget::pollYubikey() QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); } +void DatabaseOpenWidget::showYubiKeyPopup() +{ + m_ui->messageWidget->showMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); + KEEPASSXC_MAIN_WINDOW->setEnabled(false); +} + +void DatabaseOpenWidget::hideYubiKeyPopup() +{ + m_ui->messageWidget->hideMessage(); + KEEPASSXC_MAIN_WINDOW->setEnabled(true); +} + void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) { YkChallengeResponseKey yk(slot, blocking); diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index caba70a61..eee705a79 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -53,6 +53,8 @@ protected: protected slots: virtual void openDatabase(); + void showYubiKeyPopup(); + void hideYubiKeyPopup(); void reject(); private slots: diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index 694b38e7a..c3262a3cc 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -24,6 +24,7 @@ #include "core/SignalMultiplexer.h" #include "gui/DatabaseWidget.h" +#include "gui/Application.h" namespace Ui { class MainWindow; @@ -43,6 +44,9 @@ public Q_SLOTS: void openDatabase(const QString& fileName, const QString& pw = QString(), const QString& keyFile = QString()); void appExit(); + void displayGlobalMessage(const QString& text, MessageWidget::MessageType type); + void displayTabMessage(const QString& text, MessageWidget::MessageType type); + void hideGlobalMessage(); protected: void closeEvent(QCloseEvent* event) override; @@ -75,9 +79,6 @@ private Q_SLOTS: void toggleWindow(); void lockDatabasesAfterInactivity(); void repairDatabase(); - void displayGlobalMessage(const QString& text, MessageWidget::MessageType type); - void displayTabMessage(const QString& text, MessageWidget::MessageType type); - void hideGlobalMessage(); void hideTabMessage(); private: @@ -106,4 +107,6 @@ private: bool appExitCalled; }; +#define KEEPASSXC_MAIN_WINDOW qobject_cast(qobject_cast(qApp)->mainWindow()) + #endif // KEEPASSX_MAINWINDOW_H diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index a6e5cdbcf..039d7a1e6 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -14,16 +14,19 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ - -#include -#include +#include "keys/YkChallengeResponseKey.h" +#include "keys/drivers/YubiKey.h" #include "core/Tools.h" #include "crypto/CryptoHash.h" #include "crypto/Random.h" -#include "keys/YkChallengeResponseKey.h" -#include "keys/drivers/YubiKey.h" +#include +#include +#include +#include +#include +#include YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) : m_slot(slot), @@ -39,12 +42,12 @@ QByteArray YkChallengeResponseKey::rawKey() const /** * Assumes yubikey()->init() was called */ -bool YkChallengeResponseKey::challenge(const QByteArray& chal) +bool YkChallengeResponseKey::challenge(const QByteArray& challenge) { - return challenge(chal, 1); + return this->challenge(challenge, 1); } -#include -bool YkChallengeResponseKey::challenge(const QByteArray& chal, unsigned retries) + +bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned retries) { Q_ASSERT(retries > 0); @@ -55,13 +58,21 @@ bool YkChallengeResponseKey::challenge(const QByteArray& chal, unsigned retries) emit userInteractionRequired(); } - auto result = YubiKey::instance()->challenge(m_slot, true, chal, m_key); + QFuture future = QtConcurrent::run([this, challenge]() { + return YubiKey::instance()->challenge(m_slot, true, challenge, m_key); + }); + + QEventLoop loop; + QFutureWatcher watcher; + watcher.setFuture(future); + connect(&watcher, SIGNAL(finished()), &loop, SLOT(quit())); + loop.exec(); if (m_blocking) { emit userConfirmed(); } - if (result != YubiKey::ERROR) { + if (future.result() != YubiKey::ERROR) { return true; } diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index 96e44220d..8c566ca41 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/src/keys/YkChallengeResponseKey.h @@ -33,8 +33,8 @@ public: YkChallengeResponseKey(int slot = -1, bool blocking = false); QByteArray rawKey() const; - bool challenge(const QByteArray& chal); - bool challenge(const QByteArray& chal, unsigned retries); + bool challenge(const QByteArray& challenge); + bool challenge(const QByteArray& challenge, unsigned retries); QString getName() const; bool isBlocking() const; diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index e68949394..bab0d7215 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -182,6 +182,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte #endif int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r); + emit challenged(); m_mutex.unlock(); diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index 2382b69cf..b938ff86b 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -86,6 +86,11 @@ Q_SIGNALS: */ void detected(int slot, bool blocking); + /** + * Emitted when the YubiKey was challenged and has returned a response. + */ + void challenged(); + /** * Emitted when no Yubikey could be found. */ From b10cb1c83c1b8ac1b94becf0c8af94ca68af04f6 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 17:27:27 +0100 Subject: [PATCH 35/48] Show YubiKey message from MainWindow to ensure it's always shown when a challenge is generated --- src/gui/ChangeMasterKeyWidget.cpp | 14 -------------- src/gui/ChangeMasterKeyWidget.h | 2 -- src/gui/DatabaseOpenWidget.cpp | 14 -------------- src/gui/DatabaseOpenWidget.h | 2 -- src/gui/MainWindow.cpp | 11 +++++++++++ src/gui/MainWindow.h | 2 ++ src/keys/YkChallengeResponseKey.cpp | 4 ++++ 7 files changed, 17 insertions(+), 32 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index b223d3922..bb963d3cd 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -177,8 +177,6 @@ void ChangeMasterKeyWidget::generateKey() bool blocking = i & true; int slot = i >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); - connect(key.data(), SIGNAL(userInteractionRequired()), SLOT(showYubiKeyPopup())); - connect(key.data(), SIGNAL(userConfirmed()), SLOT(hideYubiKeyPopup())); m_key.addChallengeResponseKey(key); } #endif @@ -241,15 +239,3 @@ void ChangeMasterKeyWidget::setCancelEnabled(bool enabled) { m_ui->buttonBox->button(QDialogButtonBox::Cancel)->setEnabled(enabled); } - -void ChangeMasterKeyWidget::showYubiKeyPopup() -{ - KEEPASSXC_MAIN_WINDOW->displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); - KEEPASSXC_MAIN_WINDOW->setEnabled(false); -} - -void ChangeMasterKeyWidget::hideYubiKeyPopup() -{ - KEEPASSXC_MAIN_WINDOW->hideGlobalMessage(); - KEEPASSXC_MAIN_WINDOW->setEnabled(true); -} diff --git a/src/gui/ChangeMasterKeyWidget.h b/src/gui/ChangeMasterKeyWidget.h index fdc7f9529..b3e097276 100644 --- a/src/gui/ChangeMasterKeyWidget.h +++ b/src/gui/ChangeMasterKeyWidget.h @@ -55,8 +55,6 @@ private slots: void noYubikeyFound(); void challengeResponseGroupToggled(bool checked); void pollYubikey(); - void showYubiKeyPopup(); - void hideYubiKeyPopup(); private: const QScopedPointer m_ui; diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 03a560c2f..2eebcae3c 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -214,8 +214,6 @@ CompositeKey DatabaseOpenWidget::databaseKey() bool blocking = i & true; int slot = i >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); - connect(key.data(), SIGNAL(userInteractionRequired()), SLOT(showYubiKeyPopup())); - connect(key.data(), SIGNAL(userConfirmed()), SLOT(hideYubiKeyPopup())); masterKey.addChallengeResponseKey(key); } #endif @@ -266,18 +264,6 @@ void DatabaseOpenWidget::pollYubikey() QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); } -void DatabaseOpenWidget::showYubiKeyPopup() -{ - m_ui->messageWidget->showMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); - KEEPASSXC_MAIN_WINDOW->setEnabled(false); -} - -void DatabaseOpenWidget::hideYubiKeyPopup() -{ - m_ui->messageWidget->hideMessage(); - KEEPASSXC_MAIN_WINDOW->setEnabled(true); -} - void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) { YkChallengeResponseKey yk(slot, blocking); diff --git a/src/gui/DatabaseOpenWidget.h b/src/gui/DatabaseOpenWidget.h index eee705a79..caba70a61 100644 --- a/src/gui/DatabaseOpenWidget.h +++ b/src/gui/DatabaseOpenWidget.h @@ -53,8 +53,6 @@ protected: protected slots: virtual void openDatabase(); - void showYubiKeyPopup(); - void hideYubiKeyPopup(); void reject(); private slots: diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index e9a05e5d1..9a9ef3dcf 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -871,3 +871,14 @@ void MainWindow::hideTabMessage() } } +void MainWindow::showYubiKeyPopup() +{ + displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); + setEnabled(false); +} + +void MainWindow::hideYubiKeyPopup() +{ + hideGlobalMessage(); + setEnabled(true); +} diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index c3262a3cc..eb677fbbb 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -47,6 +47,8 @@ public Q_SLOTS: void displayGlobalMessage(const QString& text, MessageWidget::MessageType type); void displayTabMessage(const QString& text, MessageWidget::MessageType type); void hideGlobalMessage(); + void showYubiKeyPopup(); + void hideYubiKeyPopup(); protected: void closeEvent(QCloseEvent* event) override; diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 039d7a1e6..31ad1d860 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -20,6 +20,7 @@ #include "core/Tools.h" #include "crypto/CryptoHash.h" #include "crypto/Random.h" +#include "gui/MainWindow.h" #include #include @@ -32,6 +33,9 @@ YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) : m_slot(slot), m_blocking(blocking) { + + connect(this, SIGNAL(userInteractionRequired()), KEEPASSXC_MAIN_WINDOW, SLOT(showYubiKeyPopup())); + connect(this, SIGNAL(userConfirmed()), KEEPASSXC_MAIN_WINDOW, SLOT(hideYubiKeyPopup())); } QByteArray YkChallengeResponseKey::rawKey() const From 18844d096a4e0348120040368d53542fad5cca6e Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 17:50:19 +0100 Subject: [PATCH 36/48] Make other YubiKey driver methods thread-safe --- src/keys/YkChallengeResponseKey.cpp | 1 - src/keys/drivers/YubiKey.cpp | 21 ++++++++++++++++++--- src/keys/drivers/YubiKey.h | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 31ad1d860..60db42823 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -33,7 +33,6 @@ YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) : m_slot(slot), m_blocking(blocking) { - connect(this, SIGNAL(userInteractionRequired()), KEEPASSXC_MAIN_WINDOW, SLOT(showYubiKeyPopup())); connect(this, SIGNAL(userConfirmed()), KEEPASSXC_MAIN_WINDOW, SLOT(hideYubiKeyPopup())); } diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index bab0d7215..a490f0699 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -34,7 +34,7 @@ #define m_yk (static_cast(m_yk_void)) #define m_ykds (static_cast(m_ykds_void)) -YubiKey::YubiKey() : m_yk_void(NULL), m_ykds_void(NULL) +YubiKey::YubiKey() : m_yk_void(NULL), m_ykds_void(NULL), m_mutex(QMutex::Recursive) { } @@ -51,11 +51,14 @@ YubiKey* YubiKey::instance() bool YubiKey::init() { + m_mutex.lock(); + // previously initialized if (m_yk != NULL && m_ykds != NULL) { if (yk_get_status(m_yk, m_ykds)) { // Still connected + m_mutex.unlock(); return true; } else { // Initialized but not connected anymore, re-init @@ -64,12 +67,14 @@ bool YubiKey::init() } if (!yk_init()) { + m_mutex.unlock(); return false; } // TODO: handle multiple attached hardware devices m_yk_void = static_cast(yk_open_first_key()); if (m_yk == NULL) { + m_mutex.unlock(); return false; } @@ -77,14 +82,18 @@ bool YubiKey::init() if (m_ykds == NULL) { yk_close_key(m_yk); m_yk_void = NULL; + m_mutex.unlock(); return false; } + m_mutex.unlock(); return true; } bool YubiKey::deinit() { + m_mutex.lock(); + if (m_yk) { yk_close_key(m_yk); m_yk_void = NULL; @@ -95,6 +104,8 @@ bool YubiKey::deinit() m_ykds_void = NULL; } + m_mutex.unlock(); + return true; } @@ -119,9 +130,13 @@ void YubiKey::detect() emit notFound(); } -bool YubiKey::getSerial(unsigned int& serial) const +bool YubiKey::getSerial(unsigned int& serial) { - if (!yk_get_serial(m_yk, 1, 0, &serial)) { + m_mutex.lock(); + int result = yk_get_serial(m_yk, 1, 0, &serial); + m_mutex.unlock(); + + if (!result) { return false; } diff --git a/src/keys/drivers/YubiKey.h b/src/keys/drivers/YubiKey.h index b938ff86b..47341f9a2 100644 --- a/src/keys/drivers/YubiKey.h +++ b/src/keys/drivers/YubiKey.h @@ -70,7 +70,7 @@ public: * @param serial serial number * @return true on success */ - bool getSerial(unsigned int& serial) const; + bool getSerial(unsigned int& serial); /** * @brief YubiKey::detect - probe for attached YubiKeys From 2721317fc3187f66f2007f11ab8439deaeffeb64 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 18:43:15 +0100 Subject: [PATCH 37/48] Block and unblock autoreload in timed mutex style to prevent a double challenge when saving the database and the YubiKey requires user interaction --- src/gui/DatabaseTabWidget.cpp | 28 ++++++++++++------------- src/gui/DatabaseWidget.cpp | 39 ++++++++++++++++++++--------------- src/gui/DatabaseWidget.h | 13 ++++++------ 3 files changed, 42 insertions(+), 38 deletions(-) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 734dc3f2e..0503531ab 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -322,17 +322,18 @@ bool DatabaseTabWidget::closeAllDatabases() bool DatabaseTabWidget::saveDatabase(Database* db) { DatabaseManagerStruct& dbStruct = m_dbList[db]; - // temporarily disable autoreload - dbStruct.dbWidget->ignoreNextAutoreload(); if (dbStruct.saveToFilename) { QSaveFile saveFile(dbStruct.canonicalFilePath); if (saveFile.open(QIODevice::WriteOnly)) { // write the database to the file + dbStruct.dbWidget->blockAutoReload(true); m_writer.writeDatabase(&saveFile, db); + dbStruct.dbWidget->blockAutoReload(false); + if (m_writer.hasError()) { - Q_EMIT messageTab(tr("Writing the database failed.").append("\n") - .append(m_writer.errorString()), MessageWidget::Error); + emit messageTab(tr("Writing the database failed.").append("\n") + .append(m_writer.errorString()), MessageWidget::Error); return false; } @@ -341,22 +342,19 @@ bool DatabaseTabWidget::saveDatabase(Database* db) dbStruct.modified = false; dbStruct.dbWidget->databaseSaved(); updateTabName(db); - Q_EMIT messageDismissTab(); + emit messageDismissTab(); return true; - } - else { - Q_EMIT messageTab(tr("Writing the database failed.").append("\n") - .append(saveFile.errorString()), MessageWidget::Error); + } else { + emit messageTab(tr("Writing the database failed.").append("\n") + .append(saveFile.errorString()), MessageWidget::Error); return false; } - } - else { - Q_EMIT messageTab(tr("Writing the database failed.").append("\n") - .append(saveFile.errorString()), MessageWidget::Error); + } else { + emit messageTab(tr("Writing the database failed.").append("\n") + .append(saveFile.errorString()), MessageWidget::Error); return false; } - } - else { + } else { return saveDatabaseAs(db); } } diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index a67646ef1..9af79f777 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -168,14 +168,14 @@ DatabaseWidget::DatabaseWidget(Database* db, QWidget* parent) connect(m_unlockDatabaseDialog, SIGNAL(unlockDone(bool)), SLOT(unlockDatabase(bool))); connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), this, SLOT(onWatchedFileChanged())); connect(&m_fileWatchTimer, SIGNAL(timeout()), this, SLOT(reloadDatabaseFile())); - connect(&m_ignoreWatchTimer, SIGNAL(timeout()), this, SLOT(onWatchedFileChanged())); + connect(&m_fileWatchUnblockTimer, SIGNAL(timeout()), this, SLOT(unblockAutoReload())); connect(this, SIGNAL(currentChanged(int)), this, SLOT(emitCurrentModeChanged())); m_databaseModified = false; m_fileWatchTimer.setSingleShot(true); - m_ignoreWatchTimer.setSingleShot(true); - m_ignoreNextAutoreload = false; + m_fileWatchUnblockTimer.setSingleShot(true); + m_ignoreAutoReload = false; m_searchCaseSensitive = false; @@ -1001,7 +1001,7 @@ void DatabaseWidget::lock() void DatabaseWidget::updateFilename(const QString& fileName) { - if (! m_filename.isEmpty()) { + if (!m_filename.isEmpty()) { m_fileWatcher.removePath(m_filename); } @@ -1009,26 +1009,31 @@ void DatabaseWidget::updateFilename(const QString& fileName) m_filename = fileName; } -void DatabaseWidget::ignoreNextAutoreload() +void DatabaseWidget::blockAutoReload(bool block) { - m_ignoreNextAutoreload = true; - m_ignoreWatchTimer.start(100); + if (block) { + m_ignoreAutoReload = true; + m_fileWatchTimer.stop(); + } else { + m_fileWatchUnblockTimer.start(500); + } +} + +void DatabaseWidget::unblockAutoReload() +{ + m_ignoreAutoReload = false; + updateFilename(m_filename); } void DatabaseWidget::onWatchedFileChanged() { - if (m_ignoreNextAutoreload) { - // Reset the watch - m_ignoreNextAutoreload = false; - m_ignoreWatchTimer.stop(); - m_fileWatcher.addPath(m_filename); + if (m_ignoreAutoReload) { + return; } - else { - if (m_fileWatchTimer.isActive()) - return; + if (m_fileWatchTimer.isActive()) + return; - m_fileWatchTimer.start(500); - } + m_fileWatchTimer.start(500); } void DatabaseWidget::reloadDatabaseFile() diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 78b6131de..6e9462508 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -98,9 +98,9 @@ public: EntryView* entryView(); void showUnlockDialog(); void closeUnlockDialog(); - void ignoreNextAutoreload(); + void blockAutoReload(bool block = true); -Q_SIGNALS: +signals: void closeRequest(); void currentModeChanged(DatabaseWidget::Mode mode); void groupChanged(); @@ -118,7 +118,7 @@ Q_SIGNALS: void entryColumnSizesChanged(); void updateSearch(QString text); -public Q_SLOTS: +public slots: void createEntry(); void cloneEntry(); void deleteEntries(); @@ -154,7 +154,7 @@ public Q_SLOTS: void showMessage(const QString& text, MessageWidget::MessageType type); void hideMessage(); -private Q_SLOTS: +private slots: void entryActivationSignalReceived(Entry* entry, EntryModel::ModelColumn column); void switchBackToEntryEdit(); void switchToHistoryView(Entry* entry); @@ -172,6 +172,7 @@ private Q_SLOTS: void onWatchedFileChanged(); void reloadDatabaseFile(); void restoreGroupEntryFocus(Uuid groupUuid, Uuid EntryUuid); + void unblockAutoReload(); private: void setClipboardTextAndMinimize(const QString& text); @@ -209,8 +210,8 @@ private: // Autoreload QFileSystemWatcher m_fileWatcher; QTimer m_fileWatchTimer; - bool m_ignoreNextAutoreload; - QTimer m_ignoreWatchTimer; + QTimer m_fileWatchUnblockTimer; + bool m_ignoreAutoReload; bool m_databaseModified; }; From 46942413db9a9b41e86180e19b9f13fd01c5be3e Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 19:47:03 +0100 Subject: [PATCH 38/48] Fix unit test crash --- src/gui/MainWindow.h | 3 ++- src/keys/YkChallengeResponseKey.cpp | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index eb677fbbb..4435c13ba 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -109,6 +109,7 @@ private: bool appExitCalled; }; -#define KEEPASSXC_MAIN_WINDOW qobject_cast(qobject_cast(qApp)->mainWindow()) +#define KEEPASSXC_MAIN_WINDOW (qobject_cast(qApp) ? \ + qobject_cast(qobject_cast(qApp)->mainWindow()) : nullptr) #endif // KEEPASSX_MAINWINDOW_H diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 60db42823..dcd583358 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -33,8 +33,10 @@ YkChallengeResponseKey::YkChallengeResponseKey(int slot, bool blocking) : m_slot(slot), m_blocking(blocking) { - connect(this, SIGNAL(userInteractionRequired()), KEEPASSXC_MAIN_WINDOW, SLOT(showYubiKeyPopup())); - connect(this, SIGNAL(userConfirmed()), KEEPASSXC_MAIN_WINDOW, SLOT(hideYubiKeyPopup())); + if (KEEPASSXC_MAIN_WINDOW) { + connect(this, SIGNAL(userInteractionRequired()), KEEPASSXC_MAIN_WINDOW, SLOT(showYubiKeyPopup())); + connect(this, SIGNAL(userConfirmed()), KEEPASSXC_MAIN_WINDOW, SLOT(hideYubiKeyPopup())); + } } QByteArray YkChallengeResponseKey::rawKey() const From 9a94c6d85e9d3c3dc9b3ef0acb7715c2b6a6129d Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 20:44:06 +0100 Subject: [PATCH 39/48] Remove debug output to reduce console spam when running in debug mode --- src/keys/drivers/YubiKey.cpp | 8 -------- tests/TestYkChallengeResponseKey.cpp | 4 ++-- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index a490f0699..ffb48fc74 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -192,10 +192,6 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte c = reinterpret_cast(paddedChallenge.constData()); r = reinterpret_cast(response.data()); -#ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") c = " << printByteArray(paddedChallenge); -#endif - int ret = yk_challenge_response(m_yk, yk_cmd, mayBlock, paddedChallenge.size(), c, response.size(), r); emit challenged(); @@ -227,9 +223,5 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte // actual HMAC-SHA1 response is only 20 bytes response.resize(20); -#ifdef QT_DEBUG - qDebug().nospace() << __func__ << "(" << slot << ") r = " << printByteArray(response) << ", ret = " << ret; -#endif - return SUCCESS; } diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp index bd58ac018..046c06660 100644 --- a/tests/TestYkChallengeResponseKey.cpp +++ b/tests/TestYkChallengeResponseKey.cpp @@ -46,7 +46,7 @@ void TestYubiKeyChalResp::init() QSKIP("Unable to connect to YubiKey", SkipAll); } - /* Crypto subsystem needs to be initalized for YubiKey testing */ + // crypto subsystem needs to be initialized for YubiKey testing QVERIFY(Crypto::init()); } @@ -57,7 +57,7 @@ void TestYubiKeyChalResp::detectDevices() Qt::QueuedConnection); QtConcurrent::run(YubiKey::instance(), &YubiKey::detect); - /* Need to wait for the hardware (that's hopefully plugged in)... */ + // need to wait for the hardware (that's hopefully plugged in)... QTest::qWait(2000); QVERIFY2(m_detected > 0, "Is a YubiKey attached?"); } From a001553c5ecdd7540109c02ce7b0c4b7bddb1314 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 24 Feb 2017 21:00:48 +0100 Subject: [PATCH 40/48] Fix warnings about Crypto already having been initialized --- tests/TestYkChallengeResponseKey.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/TestYkChallengeResponseKey.cpp b/tests/TestYkChallengeResponseKey.cpp index 046c06660..40eda3bf9 100644 --- a/tests/TestYkChallengeResponseKey.cpp +++ b/tests/TestYkChallengeResponseKey.cpp @@ -30,6 +30,9 @@ void TestYubiKeyChalResp::initTestCase() { m_detected = 0; m_key = NULL; + + // crypto subsystem needs to be initialized for YubiKey testing + QVERIFY(Crypto::init()); } void TestYubiKeyChalResp::cleanupTestCase() @@ -45,9 +48,6 @@ void TestYubiKeyChalResp::init() if (!result) { QSKIP("Unable to connect to YubiKey", SkipAll); } - - // crypto subsystem needs to be initialized for YubiKey testing - QVERIFY(Crypto::init()); } void TestYubiKeyChalResp::detectDevices() From 44206cf088fb0b7d600a8a819e6d384a785e8741 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 25 Feb 2017 17:04:00 +0100 Subject: [PATCH 41/48] Fix stub compilation --- src/keys/drivers/YubiKeyStub.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/keys/drivers/YubiKeyStub.cpp b/src/keys/drivers/YubiKeyStub.cpp index e93099bf4..15eef27ad 100644 --- a/src/keys/drivers/YubiKeyStub.cpp +++ b/src/keys/drivers/YubiKeyStub.cpp @@ -51,14 +51,14 @@ void YubiKey::detect() { } -bool YubiKey::getSerial(unsigned int& serial) const +bool YubiKey::getSerial(unsigned int& serial) { Q_UNUSED(serial); return false; } -YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) const +YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& chal, QByteArray& resp) { Q_UNUSED(slot); Q_UNUSED(mayBlock); From 48366d245cd0972fa0ae3c7726afaaa1c5fd0fce Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 25 Feb 2017 17:11:02 +0100 Subject: [PATCH 42/48] Add CMake feature description --- CMakeLists.txt | 6 +++--- src/CMakeLists.txt | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ecce4f0ad..0c2fa267b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,9 +34,9 @@ option(WITH_GUI_TESTS "Enable building of GUI tests" OFF) option(WITH_DEV_BUILD "Use only for development. Disables/warns about deprecated methods." OFF) option(WITH_COVERAGE "Use to build with coverage tests. (GCC ONLY)." OFF) -option(WITH_XC_AUTOTYPE "Include Autotype." OFF) -option(WITH_XC_HTTP "Include KeePassHTTP." OFF) -option(WITH_XC_YUBIKEY "Include Yubikey support." OFF) +option(WITH_XC_AUTOTYPE "Include Auto-Type." OFF) +option(WITH_XC_HTTP "Include KeePassHTTP support." OFF) +option(WITH_XC_YUBIKEY "Include YubiKey support." OFF) set(KEEPASSXC_VERSION "2.1.2") set(KEEPASSXC_VERSION_NUM "2.1.2") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 380a6c2dd..418f1e798 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -152,8 +152,9 @@ set(keepassx_FORMS gui/group/EditGroupWidgetMain.ui ) -add_feature_info(KeePassHTTP WITH_XC_HTTP "KeePassHTTP support for ChromeIPass and PassIFox") -add_feature_info(Autotype WITH_XC_AUTOTYPE "Auto-type passwords in Input fields") +add_feature_info(AutoType WITH_XC_AUTOTYPE "Automatic password typing") +add_feature_info(KeePassHTTP WITH_XC_HTTP "Browser integration compatible with ChromeIPass and PassIFox") +add_feature_info(YubiKey WITH_XC_YUBIKEY "YubiKey HMAC-SHA1 challenge-response") add_subdirectory(http) if(WITH_XC_HTTP) From 3715286eba54970802b85eb4f759b6e1992a7f7c Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sat, 25 Feb 2017 22:09:55 +0100 Subject: [PATCH 43/48] Hide close button on YubiKey user interaction message --- src/gui/MainWindow.cpp | 8 +++++--- src/gui/MainWindow.h | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 1258d9f6e..ff63d7b3f 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -862,13 +862,15 @@ bool MainWindow::isTrayIconEnabled() const #endif } -void MainWindow::displayGlobalMessage(const QString& text, MessageWidget::MessageType type) +void MainWindow::displayGlobalMessage(const QString& text, MessageWidget::MessageType type, bool showClosebutton) { + m_ui->globalMessageWidget->setCloseButtonVisible(showClosebutton); m_ui->globalMessageWidget->showMessage(text, type); } -void MainWindow::displayTabMessage(const QString& text, MessageWidget::MessageType type) +void MainWindow::displayTabMessage(const QString& text, MessageWidget::MessageType type, bool showClosebutton) { + m_ui->globalMessageWidget->setCloseButtonVisible(showClosebutton); m_ui->tabWidget->currentDatabaseWidget()->showMessage(text, type); } @@ -886,7 +888,7 @@ void MainWindow::hideTabMessage() void MainWindow::showYubiKeyPopup() { - displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information); + displayGlobalMessage(tr("Please touch the button on your YubiKey!"), MessageWidget::Information, false); setEnabled(false); } diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index 4435c13ba..131ccc225 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -44,8 +44,8 @@ public Q_SLOTS: void openDatabase(const QString& fileName, const QString& pw = QString(), const QString& keyFile = QString()); void appExit(); - void displayGlobalMessage(const QString& text, MessageWidget::MessageType type); - void displayTabMessage(const QString& text, MessageWidget::MessageType type); + void displayGlobalMessage(const QString& text, MessageWidget::MessageType type, bool showClosebutton = true); + void displayTabMessage(const QString& text, MessageWidget::MessageType type, bool showClosebutton = true); void hideGlobalMessage(); void showYubiKeyPopup(); void hideYubiKeyPopup(); From 0a85279bcb2e3d4beff9cdca210e3d12ee1f6885 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sun, 26 Feb 2017 00:08:48 +0100 Subject: [PATCH 44/48] Enable Yubikey in release-tool by default --- release-tool | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-tool b/release-tool index a508d79f7..e5a49b05e 100755 --- a/release-tool +++ b/release-tool @@ -37,7 +37,7 @@ DOCKER_CONTAINER_NAME="keepassxc-build-container" CMAKE_OPTIONS="" COMPILER="g++" MAKE_OPTIONS="-j8" -BUILD_PLUGINS="autotype http" +BUILD_PLUGINS="autotype http yubikey" INSTALL_PREFIX="/usr/local" BUILD_SOURCE_TARBALL=true ORIG_BRANCH="" From 6125988f35032bdfb88cc5eaf30a3d76371aa879 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sun, 26 Feb 2017 18:39:03 +0100 Subject: [PATCH 45/48] Mark CMake library variables as advanced --- cmake/FindYubiKey.cmake | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cmake/FindYubiKey.cmake b/cmake/FindYubiKey.cmake index 297b68387..e5e0bb681 100644 --- a/cmake/FindYubiKey.cmake +++ b/cmake/FindYubiKey.cmake @@ -24,6 +24,4 @@ set(YUBIKEY_LIBRARIES ${YUBIKEY_CORE_LIBRARY} ${YUBIKEY_PERS_LIBRARY}) include(FindPackageHandleStandardArgs) find_package_handle_standard_args(YubiKey DEFAULT_MSG YUBIKEY_LIBRARIES YUBIKEY_INCLUDE_DIRS) -# TODO: Is mark_as_advanced() necessary? It's used in many examples with -# little explanation. Disable for now in favor of simplicity. -#mark_as_advanced(YUBIKEY_LIBRARIES YUBIKEY_INCLUDE_DIRS) +mark_as_advanced(YUBIKEY_LIBRARIES YUBIKEY_INCLUDE_DIRS) From 2ec500f926246531d9b3d80a4cf70058462b2761 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Mon, 6 Mar 2017 13:51:52 +0100 Subject: [PATCH 46/48] Reorder link dependencies --- src/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 677d2380c..5e221b916 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -206,14 +206,14 @@ set_target_properties(keepassx_core PROPERTIES COMPILE_DEFINITIONS KEEPASSX_BUIL target_link_libraries(keepassx_core ${keepasshttp_LIB} ${autotype_LIB} + ${YUBIKEY_LIBRARIES} zxcvbn Qt5::Core Qt5::Concurrent Qt5::Widgets ${GCRYPT_LIBRARIES} ${GPGERROR_LIBRARIES} - ${ZLIB_LIBRARIES} - ${YUBIKEY_LIBRARIES}) + ${ZLIB_LIBRARIES}) if (UNIX AND NOT APPLE) target_link_libraries(keepassx_core Qt5::DBus) endif() From 429bef6830ef1d3695e7edbed1fca4a29b6a7de6 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 10 Mar 2017 18:06:22 +0100 Subject: [PATCH 47/48] Remove unused debug function --- src/keys/drivers/YubiKey.cpp | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/keys/drivers/YubiKey.cpp b/src/keys/drivers/YubiKey.cpp index ffb48fc74..dfbc57c69 100644 --- a/src/keys/drivers/YubiKey.cpp +++ b/src/keys/drivers/YubiKey.cpp @@ -143,21 +143,6 @@ bool YubiKey::getSerial(unsigned int& serial) return true; } -#ifdef QT_DEBUG -/** - * @brief printByteArray - debug raw data - * @param a array input - * @return string representation of array - */ -static inline QString printByteArray(const QByteArray& a) -{ - QString s; - for (int i = 0; i < a.size(); i++) - s.append(QString::number(a[i] & 0xff, 16).rightJustified(2, '0')); - return s; -} -#endif - YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByteArray& challenge, QByteArray& response) { if (!m_mutex.tryLock()) { From 2ff57c2eb75f64db39111c6a5a02cf7c1cad87e5 Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Fri, 10 Mar 2017 20:42:59 +0100 Subject: [PATCH 48/48] Coding style fixes --- src/gui/ChangeMasterKeyWidget.cpp | 13 ++++++++----- src/gui/DatabaseOpenWidget.cpp | 16 +++++++++------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/gui/ChangeMasterKeyWidget.cpp b/src/gui/ChangeMasterKeyWidget.cpp index bb963d3cd..616b0ee01 100644 --- a/src/gui/ChangeMasterKeyWidget.cpp +++ b/src/gui/ChangeMasterKeyWidget.cpp @@ -166,16 +166,18 @@ void ChangeMasterKeyWidget::generateKey() #ifdef WITH_XC_YUBIKEY if (m_ui->challengeResponseGroup->isChecked()) { - int i = m_ui->comboChallengeResponse->currentIndex(); - i = m_ui->comboChallengeResponse->itemData(i).toInt(); + int selectionIndex = m_ui->comboChallengeResponse->currentIndex(); + int comboPayload = m_ui->comboChallengeResponse->itemData(selectionIndex).toInt(); - if (0 == i) { + if (0 == comboPayload) { m_ui->messageWidget->showMessage(tr("Changing master key failed: no YubiKey inserted."), MessageWidget::Error); return; } - bool blocking = i & true; - int slot = i >> 1; + + // read blocking mode from LSB and slot index number from second LSB + bool blocking = comboPayload & 1; + int slot = comboPayload >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); m_key.addChallengeResponseKey(key); } @@ -212,6 +214,7 @@ void ChangeMasterKeyWidget::pollYubikey() void ChangeMasterKeyWidget::yubikeyDetected(int slot, bool blocking) { 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->comboChallengeResponse->setEnabled(m_ui->challengeResponseGroup->isChecked()); m_ui->buttonRedetectYubikey->setEnabled(m_ui->challengeResponseGroup->isChecked()); diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp index 2eebcae3c..f7d432479 100644 --- a/src/gui/DatabaseOpenWidget.cpp +++ b/src/gui/DatabaseOpenWidget.cpp @@ -37,9 +37,9 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent) - : DialogyWidget(parent), - m_ui(new Ui::DatabaseOpenWidget()), - m_db(nullptr) + : DialogyWidget(parent) + , m_ui(new Ui::DatabaseOpenWidget()) + , m_db(nullptr) { m_ui->setupUi(this); @@ -208,11 +208,12 @@ CompositeKey DatabaseOpenWidget::databaseKey() } if (m_ui->checkChallengeResponse->isChecked()) { - int i = m_ui->comboChallengeResponse->currentIndex(); - i = m_ui->comboChallengeResponse->itemData(i).toInt(); + int selectionIndex = m_ui->comboChallengeResponse->currentIndex(); + int comboPayload = m_ui->comboChallengeResponse->itemData(selectionIndex).toInt(); - bool blocking = i & true; - int slot = i >> 1; + // read blocking mode from LSB and slot index number from second LSB + bool blocking = comboPayload & 1; + int slot = comboPayload >> 1; auto key = QSharedPointer(new YkChallengeResponseKey(slot, blocking)); masterKey.addChallengeResponseKey(key); } @@ -267,6 +268,7 @@ void DatabaseOpenWidget::pollYubikey() void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking) { 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->comboChallengeResponse->setEnabled(true); m_ui->checkChallengeResponse->setEnabled(true);