From df728083cc147f9669d11b6306999b36c4a79fae Mon Sep 17 00:00:00 2001 From: Janek Bevendorff Date: Sun, 7 Jan 2018 18:46:24 +0100 Subject: [PATCH] Add challenge response to key before transformation, resolves #1060 * Re-implement KDBX4 challenge-response key assembly with transform seed instead of master seed --- src/core/Database.cpp | 9 +++++++ src/format/Kdbx4Reader.cpp | 9 +------ src/format/Kdbx4Writer.cpp | 6 ----- src/keys/CompositeKey.cpp | 54 +++++++++++++++++++++++++++++++++++++- src/keys/CompositeKey.h | 7 ++--- 5 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 98a4fc817..a579ed02d 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -255,10 +255,19 @@ void Database::setCompressionAlgo(Database::CompressionAlgorithm algo) m_data.compressionAlgo = algo; } +/** + * Set and transform a new encryption key. + * + * @param key key to set and transform + * @param updateChangedTime true to update database change time + * @param updateTransformSalt true to update the transform salt + * @return true on success + */ bool Database::setKey(const CompositeKey& key, bool updateChangedTime, bool updateTransformSalt) { if (updateTransformSalt) { m_data.kdf->randomizeSeed(); + Q_ASSERT(!m_data.kdf->seed().isEmpty()); } QByteArray oldTransformedMasterKey = m_data.transformedMasterKey; diff --git a/src/format/Kdbx4Reader.cpp b/src/format/Kdbx4Reader.cpp index 05ef71160..1cde6d18c 100644 --- a/src/format/Kdbx4Reader.cpp +++ b/src/format/Kdbx4Reader.cpp @@ -20,7 +20,6 @@ #include #include "core/Group.h" -#include "core/Database.h" #include "core/Endian.h" #include "crypto/CryptoHash.h" #include "format/KeePass2RandomStream.h" @@ -48,19 +47,13 @@ Database* Kdbx4Reader::readDatabaseImpl(QIODevice* device, const QByteArray& hea return nullptr; } - if (!m_db->setKey(key, false)) { + if (!m_db->setKey(key, false, false)) { raiseError(tr("Unable to calculate master key")); return nullptr; } - if (!m_db->challengeMasterSeed(m_masterSeed)) { - raiseError(tr("Unable to issue challenge-response.")); - return nullptr; - } - CryptoHash hash(CryptoHash::Sha256); hash.addData(m_masterSeed); - hash.addData(m_db->challengeResponseKey()); hash.addData(m_db->transformedMasterKey()); QByteArray finalKey = hash.result(); diff --git a/src/format/Kdbx4Writer.cpp b/src/format/Kdbx4Writer.cpp index e151ab965..70bfa2d5b 100644 --- a/src/format/Kdbx4Writer.cpp +++ b/src/format/Kdbx4Writer.cpp @@ -51,11 +51,6 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) QByteArray startBytes; QByteArray endOfHeader = "\r\n\r\n"; - if (!db->challengeMasterSeed(masterSeed)) { - raiseError(tr("Unable to issue challenge-response.")); - return false; - } - if (!db->setKey(db->key(), false, true)) { raiseError(tr("Unable to calculate master key")); return false; @@ -64,7 +59,6 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db) // generate transformed master key CryptoHash hash(CryptoHash::Sha256); hash.addData(masterSeed); - hash.addData(db->challengeResponseKey()); 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 8f6917afc..7a9b2cbe0 100644 --- a/src/keys/CompositeKey.cpp +++ b/src/keys/CompositeKey.cpp @@ -19,6 +19,7 @@ #include "CompositeKey.h" #include #include +#include #include "core/Global.h" #include "crypto/CryptoHash.h" @@ -73,7 +74,31 @@ CompositeKey& CompositeKey::operator=(const CompositeKey& key) return *this; } +/** + * Get raw key hash as bytes. + * + * The key hash does not contain contributions by challenge-response components for + * backwards compatibility with KeePassXC's pre-KDBX4 challenge-response + * implementation. To include challenge-response in the raw key, + * use \link CompositeKey::rawKey(const QByteArray*) instead. + * + * @return key hash + */ QByteArray CompositeKey::rawKey() const +{ + return rawKey(nullptr); +} + +/** + * Get raw key hash as bytes. + * + * Challenge-response key components will use the provided transformSeed + * as a challenge to acquire their key contribution. + * + * @param transformSeed transform seed to challenge or nullptr to exclude challenge-response components + * @return key hash + */ +QByteArray CompositeKey::rawKey(const QByteArray* transformSeed) const { CryptoHash cryptoHash(CryptoHash::Sha256); @@ -81,12 +106,38 @@ QByteArray CompositeKey::rawKey() const cryptoHash.addData(key->rawKey()); } + if (transformSeed) { + QByteArray challengeResult; + challenge(*transformSeed, challengeResult); + cryptoHash.addData(challengeResult); + } + return cryptoHash.result(); } +/** + * Transform this composite key. + * + * If using AES-KDF as transform function, the transformed key will not include + * any challenge-response components. Only static key components will be hashed + * for backwards-compatibility with KeePassXC's KDBX3 implementation, which added + * challenge response key components after key transformation. + * KDBX4+ KDFs transform the whole key including challenge-response components. + * + * @param kdf key derivation function + * @param result transformed key hash + * @return true on success + */ bool CompositeKey::transform(const Kdf& kdf, QByteArray& result) const { - return kdf.transform(rawKey(), result); + if (kdf.uuid() == KeePass2::KDF_AES) { + // legacy KDBX3 AES-KDF, challenge response is added later to the hash + return kdf.transform(rawKey(), result); + } + + QByteArray seed = kdf.seed(); + Q_ASSERT(!seed.isEmpty()); + return kdf.transform(rawKey(&seed), result); } bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const @@ -103,6 +154,7 @@ bool CompositeKey::challenge(const QByteArray& seed, QByteArray& result) const for (const auto key : m_challengeResponseKeys) { // if the device isn't present or fails, return an error if (!key->challenge(seed)) { + qWarning("Failed to issue challenge"); return false; } cryptoHash.addData(key->rawKey()); diff --git a/src/keys/CompositeKey.h b/src/keys/CompositeKey.h index 35bc309fa..9018276c3 100644 --- a/src/keys/CompositeKey.h +++ b/src/keys/CompositeKey.h @@ -32,13 +32,14 @@ class CompositeKey : public Key public: CompositeKey(); CompositeKey(const CompositeKey& key); - ~CompositeKey(); + ~CompositeKey() override; void clear(); bool isEmpty() const; - CompositeKey* clone() const; + CompositeKey* clone() const override; CompositeKey& operator=(const CompositeKey& key); - QByteArray rawKey() const; + QByteArray rawKey() const override; + QByteArray rawKey(const QByteArray* transformSeed) const; bool transform(const Kdf& kdf, QByteArray& result) const Q_REQUIRED_RESULT; bool challenge(const QByteArray& seed, QByteArray &result) const;