Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2024-10-01 01:26:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

Prevent byte-by-byte and attachment inference side channel attacks

Browse Source 
Attack - KeeShare attachments can be inferred because of attachment de-duplication.

Solution - Prevent de-duplication of normal database entry attachments with those entry attachments synchronized/associated with a KeeShare database. This is done using the KeeShare database UUID injected into the hash calculation of the attachment prior to de-dupe. The attachments themselves are not modified in any way.

--------

Attack - Side channel byte-by-byte inference due to compression de-duplication of data between a KeeShare database and it's parent.

Solution - Generate a random array between 64 and 512 bytes, convert to hex, and store in the database custom data.

--------

Attack vector assumptions:
1. Compression is enabled
2. The attacker has access to a KeeShare database actively syncing with the victim's database
3. The victim's database is unlocked and syncing
4. The attacker can see the exact size of the victim's database after saving, and syncing, the KeeShare database

Thank you to Andrés Fábrega from Cornell University for theorizing and informing us of this attack vector.
This commit is contained in:
Jonathan White 2024-02-06 07:00:45 -05:00
parent 18cfbf729c
commit fff1b49f73
9 changed files with 293 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/core/Database.cpp
View File

@ -21,6 +21,7 @@
#include "core/AsyncTask.h"
#include "core/FileWatcher.h"
#include "core/Group.h"
#include "crypto/Random.h"
#include "format/KdbxXmlReader.h"
#include "format/KeePass2Reader.h"
#include "format/KeePass2Writer.h"
@ -267,6 +268,10 @@ bool Database::saveAs(const QString& filePath, SaveAction action, const QString&
// Clear read-only flag
m_fileWatcher->stop();
// Add random data to prevent side-channel data deduplication attacks
int length = Random::instance()->randomUIntRange(64, 512);
m_metadata->customData()->set("KPXC_RANDOM_SLUG", Random::instance()->randomArray(length).toHex());
// Prevent destructive operations while saving
QMutexLocker locker(&m_saveMutex);

41
src/format/Kdbx4Writer.cpp
View File

@ -19,10 +19,14 @@
#include <QBuffer>
#include "config-keepassx.h"
#include "crypto/CryptoHash.h"
#include "crypto/Random.h"
#include "format/KdbxXmlWriter.h"
#include "format/KeePass2RandomStream.h"
#ifdef WITH_XC_KEESHARE
#include "keeshare/KeeShare.h"
#include "keeshare/KeeShareSettings.h"
#endif
#include "streams/HmacBlockStream.h"
#include "streams/SymmetricCipherStream.h"
#include "streams/qtiocompressor.h"
@ -156,7 +160,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
writeInnerHeaderField(outputDevice, KeePass2::InnerHeaderFieldID::InnerRandomStreamKey, protectedStreamKey));
// Write attachments to the inner header
writeAttachments(outputDevice, db);
auto idxMap = writeAttachments(outputDevice, db);
CHECK_RETURN_FALSE(writeInnerHeaderField(outputDevice, KeePass2::InnerHeaderFieldID::End, QByteArray()));
@ -166,7 +170,7 @@ bool Kdbx4Writer::writeDatabase(QIODevice* device, Database* db)
return false;
}
KdbxXmlWriter xmlWriter(db->formatVersion());
KdbxXmlWriter xmlWriter(db->formatVersion(), idxMap);
xmlWriter.writeDatabase(outputDevice, db, &randomStream, headerHash);
// Explicitly close/reset streams so they are flushed and we can detect
@ -211,25 +215,42 @@ bool Kdbx4Writer::writeInnerHeaderField(QIODevice* device, KeePass2::InnerHeader
return true;
}
void Kdbx4Writer::writeAttachments(QIODevice* device, Database* db)
KdbxXmlWriter::BinaryIdxMap Kdbx4Writer::writeAttachments(QIODevice* device, Database* db)
{
const QList<Entry*> allEntries = db->rootGroup()->entriesRecursive(true);
QSet<QByteArray> writtenAttachments;
QHash<QByteArray, qint64> writtenAttachments;
KdbxXmlWriter::BinaryIdxMap idxMap;
qint64 nextIdx = 0;
for (Entry* entry : allEntries) {
for (const Entry* entry : allEntries) {
const QList<QString> attachmentKeys = entry->attachments()->keys();
for (const QString& key : attachmentKeys) {
QByteArray data("\x01");
data.append(entry->attachments()->value(key));
if (writtenAttachments.contains(data)) {
continue;
CryptoHash hash(CryptoHash::Sha256);
#ifdef WITH_XC_KEESHARE
// Namespace KeeShare attachments so they don't get deduplicated together with attachments
// from other databases. Prevents potential filesize side channels.
if (auto shared = KeeShare::resolveSharedGroup(entry->group())) {
hash.addData(KeeShare::referenceOf(shared).uuid.toByteArray());
} else {
hash.addData(db->uuid().toByteArray());
}
#endif
hash.addData(data);
writeInnerHeaderField(device, KeePass2::InnerHeaderFieldID::Binary, data);
writtenAttachments.insert(data);
// Deduplicate attachments with the same hash
const auto hashResult = hash.result();
if (!writtenAttachments.contains(hashResult)) {
writeInnerHeaderField(device, KeePass2::InnerHeaderFieldID::Binary, data);
writtenAttachments.insert(hashResult, nextIdx++);
}
idxMap.insert(qMakePair(entry, key), writtenAttachments[hashResult]);
}
}
return idxMap;
}
/**

3
src/format/Kdbx4Writer.h
View File

@ -19,6 +19,7 @@
#define KEEPASSX_KDBX4WRITER_H
#include "KdbxWriter.h"
#include "format/KdbxXmlWriter.h"
/**
* KDBX4 writer implementation.
@ -32,7 +33,7 @@ public:
private:
bool writeInnerHeaderField(QIODevice* device, KeePass2::InnerHeaderFieldID fieldId, const QByteArray& data);
void writeAttachments(QIODevice* device, Database* db);
KdbxXmlWriter::BinaryIdxMap writeAttachments(QIODevice* device, Database* db);
static bool serializeVariantMap(const QVariantMap& map, QByteArray& outputBytes);
};

67
src/format/KdbxXmlWriter.cpp
View File

@ -19,9 +19,13 @@
#include <QBuffer>
#include <QFile>
#include <QMap>
#include "core/Endian.h"
#include "crypto/CryptoHash.h"
#include "format/KeePass2RandomStream.h"
#include "keeshare/KeeShare.h"
#include "keeshare/KeeShareSettings.h"
#include "streams/qtiocompressor.h"
/**
@ -29,6 +33,15 @@
*/
KdbxXmlWriter::KdbxXmlWriter(quint32 version)
: m_kdbxVersion(version)
{
Q_ASSERT_X(m_kdbxVersion < KeePass2::FILE_VERSION_4,
"KDBX version",
"KDBX version >= 4 requires explicit binary index map.");
}
KdbxXmlWriter::KdbxXmlWriter(quint32 version, KdbxXmlWriter::BinaryIdxMap binaryIdxMap)
: m_kdbxVersion(version)
, m_binaryIdxMap(std::move(binaryIdxMap))
{
}
@ -46,7 +59,9 @@ void KdbxXmlWriter::writeDatabase(QIODevice* device,
m_xml.setAutoFormattingIndent(-1); // 1 tab
m_xml.setCodec("UTF-8");
generateIdMap();
if (m_kdbxVersion < KeePass2::FILE_VERSION_4) {
fillBinaryIdxMap();
}
m_xml.setDevice(device);
m_xml.writeStartDocument("1.0", true);
@ -80,18 +95,38 @@ QString KdbxXmlWriter::errorString()
return m_errorStr;
}
void KdbxXmlWriter::generateIdMap()
/**
* Generate a map of entry attachments to deduplicated attachment index IDs.
* This is basically duplicated code from Kdbx4Writer.cpp for KDBX 3 compatibility.
* I don't have a good solution for getting rid of this duplication without getting rid of KDBX 3.
*/
void KdbxXmlWriter::fillBinaryIdxMap()
{
const QList<Entry*> allEntries = m_db->rootGroup()->entriesRecursive(true);
int nextId = 0;
QHash<QByteArray, qint64> writtenAttachments;
qint64 nextIdx = 0;
for (Entry* entry : allEntries) {
const QList<QString> attachmentKeys = entry->attachments()->keys();
for (const QString& key : attachmentKeys) {
QByteArray data = entry->attachments()->value(key);
if (!m_idMap.contains(data)) {
m_idMap.insert(data, nextId++);
CryptoHash hash(CryptoHash::Sha256);
#ifdef WITH_XC_KEESHARE
// Namespace KeeShare attachments so they don't get deduplicated together with attachments
// from other databases. Prevents potential filesize side channels.
if (auto shared = KeeShare::resolveSharedGroup(entry->group())) {
hash.addData(KeeShare::referenceOf(shared).uuid.toByteArray());
} else {
hash.addData(m_db->uuid().toByteArray());
}
#endif
hash.addData(data);
const auto hashResult = hash.result();
if (!writtenAttachments.contains(hashResult)) {
writtenAttachments.insert(hashResult, nextIdx++);
}
m_binaryIdxMap.insert(qMakePair(entry, key), writtenAttachments.value(hashResult));
}
}
}
@ -181,13 +216,19 @@ void KdbxXmlWriter::writeIcon(const QUuid& uuid, const Metadata::CustomIconData&
void KdbxXmlWriter::writeBinaries()
{
// Reverse binary index map
QMap<qint64, QByteArray> binaries;
for (auto i = m_binaryIdxMap.constBegin(); i != m_binaryIdxMap.constEnd(); ++i) {
if (!binaries.contains(i.value())) {
binaries.insert(i.value(), i.key().first->attachments()->value(i.key().second));
}
}
m_xml.writeStartElement("Binaries");
QHash<QByteArray, int>::const_iterator i;
for (i = m_idMap.constBegin(); i != m_idMap.constEnd(); ++i) {
for (auto i = binaries.constBegin(); i != binaries.constEnd(); ++i) {
m_xml.writeStartElement("Binary");
m_xml.writeAttribute("ID", QString::number(i.value()));
m_xml.writeAttribute("ID", QString::number(i.key()));
QByteArray data;
if (m_db->compressionAlgorithm() == Database::CompressionGZip) {
@ -200,15 +241,15 @@ void KdbxXmlWriter::writeBinaries()
compressor.setStreamFormat(QtIOCompressor::GzipFormat);
compressor.open(QIODevice::WriteOnly);
qint64 bytesWritten = compressor.write(i.key());
Q_ASSERT(bytesWritten == i.key().size());
qint64 bytesWritten = compressor.write(i.value());
Q_ASSERT(bytesWritten == i.value().size());
Q_UNUSED(bytesWritten);
compressor.close();
buffer.seek(0);
data = buffer.readAll();
} else {
data = i.key();
data = i.value();
}
if (!data.isEmpty()) {
@ -422,7 +463,7 @@ void KdbxXmlWriter::writeEntry(const Entry* entry)
writeString("Key", key);
m_xml.writeStartElement("Value");
m_xml.writeAttribute("Ref", QString::number(m_idMap[entry->attachments()->value(key)]));
m_xml.writeAttribute("Ref", QString::number(m_binaryIdxMap[qMakePair(entry, key)]));
m_xml.writeEndElement();
m_xml.writeEndElement();

10
src/format/KdbxXmlWriter.h
View File

@ -30,7 +30,13 @@ class KeePass2RandomStream;
class KdbxXmlWriter
{
public:
/**
* Map of entry + attachment key to KDBX 4 inner header binary index.
*/
typedef QHash<QPair<const Entry*, QString>, qint64> BinaryIdxMap;
explicit KdbxXmlWriter(quint32 version);
explicit KdbxXmlWriter(quint32 version, KdbxXmlWriter::BinaryIdxMap binaryIdxMap);
void writeDatabase(QIODevice* device,
const Database* db,
@ -43,7 +49,7 @@ public:
QString errorString();
private:
void generateIdMap();
void fillBinaryIdxMap();
void writeMetadata();
void writeMemoryProtection();
@ -85,7 +91,7 @@ private:
QPointer<const Database> m_db;
QPointer<const Metadata> m_meta;
KeePass2RandomStream* m_randomStream = nullptr;
QHash<QByteArray, int> m_idMap;
BinaryIdxMap m_binaryIdxMap;
QByteArray m_headerHash;
bool m_error = false;

85
tests/TestKdbx3.cpp
View File

@ -169,3 +169,88 @@ void TestKdbx3::testBrokenHeaderHash()
auto db = QSharedPointer<Database>::create();
QVERIFY(!db->open(filename, key, nullptr));
}
void TestKdbx3::testAttachmentIndexStability()
{
QScopedPointer<Database> db(new Database());
db->changeKdf(fastKdf(KeePass2::uuidToKdf(KeePass2::KDF_AES_KDBX3)));
auto compositeKey = QSharedPointer<CompositeKey>::create();
db->setKey(compositeKey);
QVERIFY(!db->uuid().isNull());
auto root = db->rootGroup();
auto group1 = new Group();
group1->setUuid(QUuid::createUuid());
QVERIFY(!group1->uuid().isNull());
group1->setParent(root);
auto attachment1 = QByteArray("qwerty");
auto attachment2 = QByteArray("asdf");
auto attachment3 = QByteArray("zxcv");
auto entry1 = new Entry();
entry1->setUuid(QUuid::createUuid());
QVERIFY(!entry1->uuid().isNull());
auto uuid1 = entry1->uuid();
entry1->attachments()->set("a", attachment1);
QCOMPARE(entry1->attachments()->keys().size(), 1);
QCOMPARE(entry1->attachments()->values().size(), 1);
entry1->setGroup(root);
auto entry2 = new Entry();
entry2->setUuid(QUuid::createUuid());
QVERIFY(!entry2->uuid().isNull());
auto uuid2 = entry2->uuid();
entry2->attachments()->set("a", attachment1);
entry2->attachments()->set("b", attachment2);
QCOMPARE(entry2->attachments()->keys().size(), 2);
QCOMPARE(entry2->attachments()->values().size(), 2);
entry2->setGroup(group1);
auto entry3 = new Entry();
entry3->setUuid(QUuid::createUuid());
QVERIFY(!entry3->uuid().isNull());
auto uuid3 = entry3->uuid();
entry3->attachments()->set("a", attachment1);
entry3->attachments()->set("b", attachment2);
entry3->attachments()->set("x", attachment3);
entry3->attachments()->set("y", attachment3);
QCOMPARE(entry3->attachments()->keys().size(), 4);
QCOMPARE(entry3->attachments()->values().size(), 3);
entry3->setGroup(group1);
QBuffer buffer;
buffer.open(QBuffer::ReadWrite);
KeePass2Writer writer;
QVERIFY(writer.writeDatabase(&buffer, db.data()));
QCOMPARE(writer.version(), KeePass2::FILE_VERSION_3_1);
buffer.seek(0);
KeePass2Reader reader;
// Re-read database and check that all attachments are still correctly assigned
auto db2 = QSharedPointer<Database>::create();
reader.readDatabase(&buffer, QSharedPointer<CompositeKey>::create(), db2.data());
QVERIFY(!reader.hasError());
QVERIFY(!db->uuid().isNull());
auto a1 = db2->rootGroup()->findEntryByUuid(uuid1)->attachments();
QCOMPARE(a1->keys().size(), 1);
QCOMPARE(a1->values().size(), 1);
QCOMPARE(a1->value("a"), attachment1);
auto a2 = db2->rootGroup()->findEntryByUuid(uuid2)->attachments();
QCOMPARE(a2->keys().size(), 2);
QCOMPARE(a2->values().size(), 2);
QCOMPARE(a2->value("a"), attachment1);
QCOMPARE(a2->value("b"), attachment2);
auto a3 = db2->rootGroup()->findEntryByUuid(uuid3)->attachments();
QCOMPARE(a3->keys().size(), 4);
QCOMPARE(a3->values().size(), 3);
QCOMPARE(a3->value("a"), attachment1);
QCOMPARE(a3->value("b"), attachment2);
QCOMPARE(a3->value("x"), attachment3);
QCOMPARE(a3->value("y"), attachment3);
}

1
tests/TestKdbx3.h
View File

@ -30,6 +30,7 @@ private slots:
void testProtectedStrings();
void testBrokenHeaderHash();
void testFormat300();
void testAttachmentIndexStability();
protected:
void initTestCaseImpl() override;

107
tests/TestKdbx4.cpp
View File

@ -24,6 +24,10 @@
#include "format/KeePass2.h"
#include "format/KeePass2Reader.h"
#include "format/KeePass2Writer.h"
#ifdef WITH_XC_KEESHARE
#include "keeshare/KeeShare.h"
#include "keeshare/KeeShareSettings.h"
#endif
#include "keys/FileKey.h"
#include "keys/PasswordKey.h"
#include "mock/MockChallengeResponseKey.h"
@ -71,7 +75,7 @@ QSharedPointer<Database> TestKdbx4Argon2::readXml(QBuffer* buf, bool strictMode,
void TestKdbx4Argon2::writeXml(QBuffer* buf, Database* db, bool& hasError, QString& errorString)
{
KdbxXmlWriter writer(KeePass2::FILE_VERSION_4);
KdbxXmlWriter writer(KeePass2::FILE_VERSION_4, {});
writer.writeDatabase(buf, db);
hasError = writer.hasError();
errorString = writer.errorString();
@ -405,6 +409,107 @@ void TestKdbx4Format::testUpgradeMasterKeyIntegrity_data()
QTest::newRow("Upgrade (implicit): entry-customdata") << QString("entry-customdata") << KeePass2::FILE_VERSION_4;
}
void TestKdbx4Format::testAttachmentIndexStability()
{
QScopedPointer<Database> db(new Database());
db->changeKdf(fastKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2ID)));
auto compositeKey = QSharedPointer<CompositeKey>::create();
db->setKey(compositeKey);
QVERIFY(!db->uuid().isNull());
auto root = db->rootGroup();
auto group1 = new Group();
group1->setUuid(QUuid::createUuid());
QVERIFY(!group1->uuid().isNull());
group1->setParent(root);
// Simulate KeeShare group, which uses its own attachment namespace
auto group2 = new Group();
group2->setUuid(QUuid::createUuid());
QVERIFY(!group2->uuid().isNull());
group2->setParent(group1);
#ifdef WITH_XC_KEESHARE
KeeShareSettings::Reference ref;
ref.type = KeeShareSettings::SynchronizeWith;
ref.path = "123";
KeeShare::setReferenceTo(group2, ref);
QVERIFY(KeeShare::isShared(group2));
#endif
auto attachment1 = QByteArray("qwerty");
auto attachment2 = QByteArray("asdf");
auto attachment3 = QByteArray("zxcv");
auto entry1 = new Entry();
entry1->setUuid(QUuid::createUuid());
QVERIFY(!entry1->uuid().isNull());
auto uuid1 = entry1->uuid();
entry1->attachments()->set("a", attachment1);
QCOMPARE(entry1->attachments()->keys().size(), 1);
QCOMPARE(entry1->attachments()->values().size(), 1);
entry1->setGroup(root);
auto entry2 = new Entry();
entry2->setUuid(QUuid::createUuid());
QVERIFY(!entry2->uuid().isNull());
auto uuid2 = entry2->uuid();
entry2->attachments()->set("a", attachment1);
entry2->attachments()->set("b", attachment2);
QCOMPARE(entry2->attachments()->keys().size(), 2);
QCOMPARE(entry2->attachments()->values().size(), 2);
entry2->setGroup(group1);
auto entry3 = new Entry();
entry3->setUuid(QUuid::createUuid());
QVERIFY(!entry3->uuid().isNull());
auto uuid3 = entry3->uuid();
entry3->attachments()->set("a", attachment1);
entry3->attachments()->set("b", attachment2);
entry3->attachments()->set("x", attachment3);
entry3->attachments()->set("y", attachment3);
QCOMPARE(entry3->attachments()->keys().size(), 4);
QCOMPARE(entry3->attachments()->values().size(), 3);
entry3->setGroup(group2);
QBuffer buffer;
buffer.open(QBuffer::ReadWrite);
KeePass2Writer writer;
QVERIFY(writer.writeDatabase(&buffer, db.data()));
QVERIFY(writer.version() >= KeePass2::FILE_VERSION_4);
buffer.seek(0);
KeePass2Reader reader;
// Re-read database and check that all attachments are still correctly assigned
auto db2 = QSharedPointer<Database>::create();
reader.readDatabase(&buffer, QSharedPointer<CompositeKey>::create(), db2.data());
QVERIFY(!reader.hasError());
QVERIFY(!db->uuid().isNull());
auto a1 = db2->rootGroup()->findEntryByUuid(uuid1)->attachments();
QCOMPARE(a1->keys().size(), 1);
QCOMPARE(a1->values().size(), 1);
QCOMPARE(a1->value("a"), attachment1);
auto a2 = db2->rootGroup()->findEntryByUuid(uuid2)->attachments();
QCOMPARE(a2->keys().size(), 2);
QCOMPARE(a2->values().size(), 2);
QCOMPARE(a2->value("a"), attachment1);
QCOMPARE(a2->value("b"), attachment2);
#ifdef WITH_XC_KEESHARE
QVERIFY(KeeShare::isShared(db2->rootGroup()->findEntryByUuid(uuid3)->group()));
#endif
auto a3 = db2->rootGroup()->findEntryByUuid(uuid3)->attachments();
QCOMPARE(a3->keys().size(), 4);
QCOMPARE(a3->values().size(), 3);
QCOMPARE(a3->value("a"), attachment1);
QCOMPARE(a3->value("b"), attachment2);
QCOMPARE(a3->value("x"), attachment3);
QCOMPARE(a3->value("y"), attachment3);
}
void TestKdbx4Format::testCustomData()
{
Database db;

1
tests/TestKdbx4.h
View File

@ -62,6 +62,7 @@ private slots:
void testFormat410Upgrade();
void testUpgradeMasterKeyIntegrity();
void testUpgradeMasterKeyIntegrity_data();
void testAttachmentIndexStability();
void testCustomData();
};