diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f7616dce6..9e2f44a6d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -152,6 +152,7 @@ set(keepassx_SOURCES gui/group/EditGroupWidget.cpp gui/group/GroupModel.cpp gui/group/GroupView.cpp + keys/ChallengeResponseKey.h keys/CompositeKey.cpp keys/drivers/YubiKey.h keys/FileKey.cpp diff --git a/src/core/Database.cpp b/src/core/Database.cpp index 00cb76f48..8fe8d04c7 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -320,7 +320,12 @@ bool Database::verifyKey(const CompositeKey& key) const return (m_data.key.rawKey() == key.rawKey()); } -QVariantMap Database::publicCustomData() const +QVariantMap& Database::publicCustomData() +{ + return m_data.publicCustomData; +} + +const QVariantMap& Database::publicCustomData() const { return m_data.publicCustomData; } diff --git a/src/core/Database.h b/src/core/Database.h index 394e31475..583ed3cac 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -105,7 +105,8 @@ public: bool updateTransformSalt = false); bool hasKey() const; bool verifyKey(const CompositeKey& key) const; - QVariantMap publicCustomData() const; + QVariantMap& publicCustomData(); + const QVariantMap& publicCustomData() const; void setPublicCustomData(const QVariantMap& customData); void recycleEntry(Entry* entry); void recycleGroup(Group* group); diff --git a/src/core/MacPasteboard.h b/src/core/MacPasteboard.h index 8461cbc5d..d471a096a 100644 --- a/src/core/MacPasteboard.h +++ b/src/core/MacPasteboard.h @@ -20,8 +20,9 @@ #include #include +#include -class MacPasteboard : public QMacPasteboardMime +class MacPasteboard : public QObject, public QMacPasteboardMime { public: explicit MacPasteboard() : QMacPasteboardMime(MIME_ALL) {} diff --git a/src/gui/Clipboard.cpp b/src/gui/Clipboard.cpp index 78bad2731..2c0d3d6ed 100644 --- a/src/gui/Clipboard.cpp +++ b/src/gui/Clipboard.cpp @@ -24,14 +24,19 @@ #include "core/Config.h" Clipboard* Clipboard::m_instance(nullptr); +#ifdef Q_OS_MAC +QPointer Clipboard::m_pasteboard(nullptr); +#endif Clipboard::Clipboard(QObject* parent) : QObject(parent) , m_timer(new QTimer(this)) -#ifdef Q_OS_MAC - , m_pasteboard(new MacPasteboard) -#endif { +#ifdef Q_OS_MAC + if (!m_pasteboard) { + m_pasteboard = new MacPasteboard(); + } +#endif m_timer->setSingleShot(true); connect(m_timer, SIGNAL(timeout()), SLOT(clearClipboard())); connect(qApp, SIGNAL(aboutToQuit()), SLOT(clearCopiedText())); diff --git a/src/gui/Clipboard.h b/src/gui/Clipboard.h index 6f8ff9ace..279ae7f03 100644 --- a/src/gui/Clipboard.h +++ b/src/gui/Clipboard.h @@ -21,6 +21,7 @@ #include #ifdef Q_OS_MAC #include "core/MacPasteboard.h" +#include #endif class QTimer; @@ -47,7 +48,9 @@ private: QTimer* m_timer; #ifdef Q_OS_MAC - QScopedPointer m_pasteboard; + // This object lives for the whole program lifetime and we cannot delete it on exit, + // so ignore leak warnings. See https://bugreports.qt.io/browse/QTBUG-54832 + static QPointer m_pasteboard; #endif QString m_lastCopied; }; diff --git a/src/keys/YkChallengeResponseKey.cpp b/src/keys/YkChallengeResponseKey.cpp index 4c77e7670..ac50a5bba 100644 --- a/src/keys/YkChallengeResponseKey.cpp +++ b/src/keys/YkChallengeResponseKey.cpp @@ -83,7 +83,7 @@ bool YkChallengeResponseKey::challenge(const QByteArray& challenge, unsigned ret } // 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) { + if (retries > 0 && !YubiKey::instance()->init()) { continue; } diff --git a/src/keys/YkChallengeResponseKey.h b/src/keys/YkChallengeResponseKey.h index 66d821a69..2816602a4 100644 --- a/src/keys/YkChallengeResponseKey.h +++ b/src/keys/YkChallengeResponseKey.h @@ -1,18 +1,18 @@ /* -* Copyright (C) 2017 KeePassXC Team -* -* 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 . + * Copyright (C) 2017 KeePassXC Team + * + * 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 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3b8ada32d..df5c37f83 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -112,10 +112,10 @@ add_unit_test(NAME testkdbx2 SOURCES TestKdbx2.cpp add_unit_test(NAME testkdbx3 SOURCES TestKeePass2Format.cpp FailDevice.cpp TestKdbx3.cpp LIBS ${TEST_LIBRARIES}) -add_unit_test(NAME testkdbx4 SOURCES TestKeePass2Format.cpp FailDevice.cpp TestKdbx4.cpp +add_unit_test(NAME testkdbx4 SOURCES TestKeePass2Format.cpp FailDevice.cpp mock/MockChallengeResponseKey.cpp TestKdbx4.cpp LIBS ${TEST_LIBRARIES}) -add_unit_test(NAME testkeys SOURCES TestKeys.cpp +add_unit_test(NAME testkeys SOURCES TestKeys.cpp mock/MockChallengeResponseKey.cpp LIBS ${TEST_LIBRARIES}) add_unit_test(NAME testgroupmodel SOURCES TestGroupModel.cpp diff --git a/tests/TestKdbx4.cpp b/tests/TestKdbx4.cpp index 08b24c47f..4bbff1fec 100644 --- a/tests/TestKdbx4.cpp +++ b/tests/TestKdbx4.cpp @@ -20,6 +20,8 @@ #include "core/Metadata.h" #include "keys/PasswordKey.h" +#include "keys/FileKey.h" +#include "mock/MockChallengeResponseKey.h" #include "format/KeePass2.h" #include "format/KeePass2Reader.h" #include "format/KeePass2Writer.h" @@ -211,6 +213,106 @@ void TestKdbx4::testFormat400Upgrade_data() QTest::newRow("AES-KDF (legacy) + Twofish + CustomData") << KeePass2::KDF_AES_KDBX3 << KeePass2::CIPHER_TWOFISH << true << kdbx4; } +void TestKdbx4::testUpgradeMasterKeyIntegrity() +{ + QFETCH(QString, upgradeAction); + QFETCH(quint32, expectedVersion); + + // prepare composite key + PasswordKey passwordKey("turXpGMQiUE6CkPvWacydAKsnp4cxz"); + + QByteArray fileKeyBytes("Ma6hHov98FbPeyAL22XhcgmpJk8xjQ"); + QBuffer fileKeyBuffer(&fileKeyBytes); + fileKeyBuffer.open(QBuffer::ReadOnly); + FileKey fileKey; + fileKey.load(&fileKeyBuffer); + + auto crKey = QSharedPointer::create(QByteArray("azdJnbVCFE76vV6t9RJ2DS6xvSS93k")); + + CompositeKey compositeKey; + compositeKey.addKey(passwordKey); + compositeKey.addKey(fileKey); + compositeKey.addChallengeResponseKey(crKey); + + QScopedPointer db(new Database()); + db->setKey(compositeKey); + + // upgrade the database by a specific method + if (upgradeAction == "none") { + // do nothing + } else if (upgradeAction == "meta-customdata") { + db->metadata()->customData()->set("abc", "def"); + } else if (upgradeAction == "kdf-aes-kdbx3") { + db->changeKdf(KeePass2::uuidToKdf(KeePass2::KDF_AES_KDBX3)); + } else if (upgradeAction == "kdf-argon2") { + db->changeKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2)); + } else if (upgradeAction == "kdf-aes-kdbx4") { + db->changeKdf(KeePass2::uuidToKdf(KeePass2::KDF_AES_KDBX4)); + } else if (upgradeAction == "public-customdata") { + db->publicCustomData().insert("abc", "def"); + } else if (upgradeAction == "rootgroup-customdata") { + db->rootGroup()->customData()->set("abc", "def"); + } else if (upgradeAction == "group-customdata") { + auto group = new Group(); + group->setParent(db->rootGroup()); + group->setUuid(Uuid::random()); + group->customData()->set("abc", "def"); + } else if (upgradeAction == "rootentry-customdata") { + auto entry = new Entry(); + entry->setGroup(db->rootGroup()); + entry->setUuid(Uuid::random()); + entry->customData()->set("abc", "def"); + } else if (upgradeAction == "entry-customdata") { + auto group = new Group(); + group->setParent(db->rootGroup()); + group->setUuid(Uuid::random()); + auto entry = new Entry(); + entry->setGroup(group); + entry->setUuid(Uuid::random()); + entry->customData()->set("abc", "def"); + } else { + QFAIL(qPrintable(QString("Unknown action: %s").arg(upgradeAction))); + } + + QBuffer buffer; + buffer.open(QBuffer::ReadWrite); + KeePass2Writer writer; + QVERIFY(writer.writeDatabase(&buffer, db.data())); + + // paranoid check that we cannot decrypt the database without a key + buffer.seek(0); + KeePass2Reader reader; + QScopedPointer db2; + db2.reset(reader.readDatabase(&buffer, CompositeKey())); + QVERIFY(reader.hasError()); + + // check that we can read back the database with the original composite key, + // i.e., no components have been lost on the way + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKey)); + if (reader.hasError()) { + QFAIL(qPrintable(reader.errorString())); + } + QCOMPARE(reader.version(), expectedVersion & KeePass2::FILE_VERSION_CRITICAL_MASK); +} + +void TestKdbx4::testUpgradeMasterKeyIntegrity_data() +{ + QTest::addColumn("upgradeAction"); + QTest::addColumn("expectedVersion"); + + QTest::newRow("Upgrade: none") << QString("none") << KeePass2::FILE_VERSION_3; + QTest::newRow("Upgrade: none (meta-customdata)") << QString("meta-customdata") << KeePass2::FILE_VERSION_3; + QTest::newRow("Upgrade: none (explicit kdf-aes-kdbx3)") << QString("kdf-aes-kdbx3") << KeePass2::FILE_VERSION_3; + QTest::newRow("Upgrade (explicit): kdf-argon2") << QString("kdf-argon2") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (explicit): kdf-aes-kdbx4") << QString("kdf-aes-kdbx4") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (implicit): public-customdata") << QString("public-customdata") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (implicit): rootgroup-customdata") << QString("rootgroup-customdata") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (implicit): group-customdata") << QString("group-customdata") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (implicit): rootentry-customdata") << QString("rootentry-customdata") << KeePass2::FILE_VERSION_4; + QTest::newRow("Upgrade (implicit): entry-customdata") << QString("entry-customdata") << KeePass2::FILE_VERSION_4; +} + void TestKdbx4::testCustomData() { Database db; @@ -220,8 +322,9 @@ void TestKdbx4::testCustomData() publicCustomData.insert("CD1", 123); publicCustomData.insert("CD2", true); publicCustomData.insert("CD3", "abcäöü"); - publicCustomData.insert("CD4", QByteArray::fromHex("ababa123ff")); db.setPublicCustomData(publicCustomData); + publicCustomData.insert("CD4", QByteArray::fromHex("ababa123ff")); + db.publicCustomData().insert("CD4", publicCustomData.value("CD4")); QCOMPARE(db.publicCustomData(), publicCustomData); const QString customDataKey1 = "CD1"; @@ -229,7 +332,7 @@ void TestKdbx4::testCustomData() const QString customData1 = "abcäöü"; const QString customData2 = "Hello World"; const int dataSize = customDataKey1.toUtf8().size() + customDataKey1.toUtf8().size() + - customData1.toUtf8().size() + customData2.toUtf8().size(); + customData1.toUtf8().size() + customData2.toUtf8().size(); // test custom database data db.metadata()->customData()->set(customDataKey1, customData1); diff --git a/tests/TestKdbx4.h b/tests/TestKdbx4.h index 5c398d196..efb3449f3 100644 --- a/tests/TestKdbx4.h +++ b/tests/TestKdbx4.h @@ -28,6 +28,8 @@ private slots: void testFormat400(); void testFormat400Upgrade(); void testFormat400Upgrade_data(); + void testUpgradeMasterKeyIntegrity(); + void testUpgradeMasterKeyIntegrity_data(); void testCustomData(); protected: diff --git a/tests/TestKeys.cpp b/tests/TestKeys.cpp index 094a397d7..f39d3aa74 100644 --- a/tests/TestKeys.cpp +++ b/tests/TestKeys.cpp @@ -32,6 +32,7 @@ #include "format/KeePass2Writer.h" #include "keys/FileKey.h" #include "keys/PasswordKey.h" +#include "mock/MockChallengeResponseKey.h" QTEST_GUILESS_MAIN(TestKeys) Q_DECLARE_METATYPE(FileKey::Type); @@ -232,3 +233,85 @@ void TestKeys::benchmarkTransformKey() Q_UNUSED(compositeKey.transform(kdf, result)); }; } + +void TestKeys::testCompositeKeyComponents() +{ + PasswordKey passwordKeyEnc("password"); + FileKey fileKeyEnc; + QString error; + fileKeyEnc.load(QString("%1/%2").arg(QString(KEEPASSX_TEST_DATA_DIR), "FileKeyHashed.key"), &error); + if (!error.isNull()) { + QFAIL(qPrintable(error)); + } + auto challengeResponseKeyEnc = QSharedPointer::create(QByteArray(16, 0x10)); + + CompositeKey compositeKeyEnc; + compositeKeyEnc.addKey(passwordKeyEnc); + compositeKeyEnc.addKey(fileKeyEnc); + compositeKeyEnc.addChallengeResponseKey(challengeResponseKeyEnc); + + QScopedPointer db1(new Database()); + db1->setKey(compositeKeyEnc); + + KeePass2Writer writer; + QBuffer buffer; + buffer.open(QBuffer::ReadWrite); + QVERIFY(writer.writeDatabase(&buffer, db1.data())); + + buffer.seek(0); + QScopedPointer db2; + KeePass2Reader reader; + CompositeKey compositeKeyDec1; + + // try decryption and subsequently add key components until decryption is successful + db2.reset(reader.readDatabase(&buffer, compositeKeyDec1)); + QVERIFY(reader.hasError()); + + compositeKeyDec1.addKey(passwordKeyEnc); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec1)); + QVERIFY(reader.hasError()); + + compositeKeyDec1.addKey(fileKeyEnc); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec1)); + QVERIFY(reader.hasError()); + + compositeKeyDec1.addChallengeResponseKey(challengeResponseKeyEnc); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec1)); + // now we should be able to open the database + if (reader.hasError()) { + QFAIL(qPrintable(reader.errorString())); + } + + // try the same again, but this time with one wrong key component each time + CompositeKey compositeKeyDec2; + compositeKeyDec2.addKey(PasswordKey("wrong password")); + compositeKeyDec2.addKey(fileKeyEnc); + compositeKeyDec2.addChallengeResponseKey(challengeResponseKeyEnc); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec2)); + QVERIFY(reader.hasError()); + + CompositeKey compositeKeyDec3; + compositeKeyDec3.addKey(passwordKeyEnc); + FileKey fileKeyWrong; + fileKeyWrong.load(QString("%1/%2").arg(QString(KEEPASSX_TEST_DATA_DIR), "FileKeyHashed2.key"), &error); + if (!error.isNull()) { + QFAIL(qPrintable(error)); + } + compositeKeyDec3.addKey(fileKeyWrong); + compositeKeyDec3.addChallengeResponseKey(challengeResponseKeyEnc); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec3)); + QVERIFY(reader.hasError()); + + CompositeKey compositeKeyDec4; + compositeKeyDec4.addKey(passwordKeyEnc); + compositeKeyDec4.addKey(fileKeyEnc); + compositeKeyDec4.addChallengeResponseKey(QSharedPointer::create(QByteArray(16, 0x20))); + buffer.seek(0); + db2.reset(reader.readDatabase(&buffer, compositeKeyDec4)); + QVERIFY(reader.hasError()); +} diff --git a/tests/TestKeys.h b/tests/TestKeys.h index 237225f8f..1ea53b090 100644 --- a/tests/TestKeys.h +++ b/tests/TestKeys.h @@ -34,6 +34,7 @@ private slots: void testCreateAndOpenFileKey(); void testFileKeyHash(); void testFileKeyError(); + void testCompositeKeyComponents(); void benchmarkTransformKey(); }; diff --git a/tests/data/FileKeyHashed2.key b/tests/data/FileKeyHashed2.key new file mode 100644 index 000000000..46e2ea632 Binary files /dev/null and b/tests/data/FileKeyHashed2.key differ diff --git a/tests/mock/MockChallengeResponseKey.cpp b/tests/mock/MockChallengeResponseKey.cpp new file mode 100644 index 000000000..9cfdc0501 --- /dev/null +++ b/tests/mock/MockChallengeResponseKey.cpp @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2018 KeePassXC Team + * + * 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 "MockChallengeResponseKey.h" + +MockChallengeResponseKey::MockChallengeResponseKey(const QByteArray& secret) + : m_secret(secret) +{ +} + +MockChallengeResponseKey::~MockChallengeResponseKey() +{ +} + +QByteArray MockChallengeResponseKey::rawKey() const +{ + return m_challenge + m_secret; +} + +bool MockChallengeResponseKey::challenge(const QByteArray& challenge) +{ + m_challenge = challenge; + return true; +} diff --git a/tests/mock/MockChallengeResponseKey.h b/tests/mock/MockChallengeResponseKey.h new file mode 100644 index 000000000..af795d935 --- /dev/null +++ b/tests/mock/MockChallengeResponseKey.h @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2018 KeePassXC Team + * + * 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 KEEPASSXC_MOCKCHALLENGERESPONSEKEY_H +#define KEEPASSXC_MOCKCHALLENGERESPONSEKEY_H + +#include "keys/ChallengeResponseKey.h" + +/** + * Mock challenge-response key implementation that simply + * returns the challenge concatenated with a fixed secret. + */ +class MockChallengeResponseKey : public ChallengeResponseKey +{ +public: + explicit MockChallengeResponseKey(const QByteArray& secret); + ~MockChallengeResponseKey() override; + QByteArray rawKey() const override; + bool challenge(const QByteArray& challenge) override; + +private: + QByteArray m_challenge; + QByteArray m_secret; +}; + +#endif //KEEPASSXC_MOCKCHALLENGERESPONSEKEY_H