diff --git a/src/keys/FileKey.cpp b/src/keys/FileKey.cpp index d3cdfe040..7f2f8a6ea 100644 --- a/src/keys/FileKey.cpp +++ b/src/keys/FileKey.cpp @@ -1,4 +1,5 @@ /* +* Copyright (C) 2017 KeePassXC Team * Copyright (C) 2011 Felix Geyer * * This program is free software: you can redistribute it and/or modify @@ -18,16 +19,30 @@ #include "FileKey.h" #include -#include #include "core/Tools.h" #include "crypto/CryptoHash.h" #include "crypto/Random.h" -FileKey::FileKey() -{ -} - +/** + * Read key file from device while trying to detect its file format. + * + * If no legacy key file format was detected, the SHA-256 hash of the + * key file will be used, allowing usage of arbitrary files as key files. + * In case of a detected legacy key file format, the raw byte contents + * will be extracted from the file. + * + * Supported legacy formats are: + * - KeePass 2 XML key file + * - Fixed 32 byte binary + * - Fixed 32 byte ASCII hex-encoded binary + * + * Usage of legacy formats is discouraged and support for them may be + * removed in a future version. + * + * @param device input device + * @return true if key file was loaded successfully + */ bool FileKey::load(QIODevice* device) { // we may need to read the file multiple times @@ -39,8 +54,7 @@ bool FileKey::load(QIODevice* device) return false; } - // try different key file formats - + // try different legacy key file formats if (!device->reset()) { return false; } @@ -65,13 +79,31 @@ bool FileKey::load(QIODevice* device) if (!device->reset()) { return false; } - if (loadHashed(device)) { - return true; - } - return false; + // if no legacy format was detected, generate SHA-256 hash of key file + return loadHashed(device); } +/** + * Load key file from path while trying to detect its file format. + * + * If no legacy key file format was detected, the SHA-256 hash of the + * key file will be used, allowing usage of arbitrary files as key files. + * In case of a detected legacy key file format, the raw byte contents + * will be extracted from the file. + * + * Supported legacy formats are: + * - KeePass 2 XML key file + * - Fixed 32 byte binary + * - Fixed 32 byte ASCII hex-encoded binary + * + * Usage of legacy formats is discouraged and support for them may be + * removed in a future version. + * + * @param fileName input file name + * @param errorMsg error message if loading failed + * @return true if key file was loaded successfully + */ bool FileKey::load(const QString& fileName, QString* errorMsg) { QFile file(fileName); @@ -95,41 +127,42 @@ bool FileKey::load(const QString& fileName, QString* errorMsg) return result; } +/** + * @return key data as bytes + */ QByteArray FileKey::rawKey() const { return m_key; } +/** + * @return cloned \link FileKey instance + */ FileKey* FileKey::clone() const { return new FileKey(*this); } -void FileKey::create(QIODevice* device) +/** + * Generate a new key file from random bytes. + * + * @param device output device + * @param number of random bytes to generate + */ +void FileKey::create(QIODevice* device, int size) { - QXmlStreamWriter xmlWriter(device); - - xmlWriter.writeStartDocument("1.0"); - - xmlWriter.writeStartElement("KeyFile"); - - xmlWriter.writeStartElement("Meta"); - - xmlWriter.writeTextElement("Version", "1.00"); - - xmlWriter.writeEndElement(); - - xmlWriter.writeStartElement("Key"); - - QByteArray data = randomGen()->randomArray(32); - xmlWriter.writeTextElement("Data", QString::fromLatin1(data.toBase64())); - - xmlWriter.writeEndElement(); - - xmlWriter.writeEndDocument(); + device->write(randomGen()->randomArray(size)); } -bool FileKey::create(const QString& fileName, QString* errorMsg) +/** + * Create a new key file from random bytes. + * + * @param fileName output file name + * @param errorMsg error message if generation failed + * @param number of random bytes to generate + * @return true on successful creation + */ +bool FileKey::create(const QString& fileName, QString* errorMsg, int size) { QFile file(fileName); if (!file.open(QFile::WriteOnly)) { @@ -138,8 +171,7 @@ bool FileKey::create(const QString& fileName, QString* errorMsg) } return false; } - create(&file); - + create(&file, size); file.close(); if (file.error()) { @@ -149,11 +181,17 @@ bool FileKey::create(const QString& fileName, QString* errorMsg) return false; } - else { - return true; - } + + return true; } +/** + * Load key file in legacy KeePass 2 XML format. + * + * @param device input device + * @return true on success + * @deprecated + */ bool FileKey::loadXml(QIODevice* device) { QXmlStreamReader xmlReader(device); @@ -162,8 +200,7 @@ bool FileKey::loadXml(QIODevice* device) if (xmlReader.name() != "KeyFile") { return false; } - } - else { + } else { return false; } @@ -173,8 +210,7 @@ bool FileKey::loadXml(QIODevice* device) while (!xmlReader.error() && xmlReader.readNextStartElement()) { if (xmlReader.name() == "Meta") { correctMeta = loadXmlMeta(xmlReader); - } - else if (xmlReader.name() == "Key") { + } else if (xmlReader.name() == "Key") { data = loadXmlKey(xmlReader); } } @@ -183,18 +219,23 @@ bool FileKey::loadXml(QIODevice* device) m_key = data; return true; } - else { - return false; - } + + return false; } +/** + * Load meta data from legacy KeePass 2 XML key file. + * + * @param xmlReader input XML reader + * @return true on success + * @deprecated + */ bool FileKey::loadXmlMeta(QXmlStreamReader& xmlReader) { bool correctVersion = false; while (!xmlReader.error() && xmlReader.readNextStartElement()) { if (xmlReader.name() == "Version") { - // TODO: error message about incompatible key file version if (xmlReader.readElementText() == "1.00") { correctVersion = true; } @@ -204,13 +245,19 @@ bool FileKey::loadXmlMeta(QXmlStreamReader& xmlReader) return correctVersion; } +/** + * Load base64 key data from legacy KeePass 2 XML key file. + * + * @param xmlReader input XML reader + * @return true on success + * @deprecated + */ QByteArray FileKey::loadXmlKey(QXmlStreamReader& xmlReader) { QByteArray data; while (!xmlReader.error() && xmlReader.readNextStartElement()) { if (xmlReader.name() == "Data") { - // TODO: do we need to enforce a specific data.size()? QByteArray rawData = xmlReader.readElementText().toLatin1(); if (Tools::isBase64(rawData)) { data = QByteArray::fromBase64(rawData); @@ -221,6 +268,13 @@ QByteArray FileKey::loadXmlKey(QXmlStreamReader& xmlReader) return data; } +/** + * Load fixed 32-bit binary key file. + * + * @param device input device + * @return true on success + * @deprecated + */ bool FileKey::loadBinary(QIODevice* device) { if (device->size() != 32) { @@ -230,13 +284,19 @@ bool FileKey::loadBinary(QIODevice* device) QByteArray data; if (!Tools::readAllFromDevice(device, data) || data.size() != 32) { return false; - } - else { + } else { m_key = data; return true; } } +/** + * Load hex-encoded representation of fixed 32-bit binary key file. + * + * @param device input device + * @return true on success + * @deprecated + */ bool FileKey::loadHex(QIODevice* device) { if (device->size() != 64) { @@ -262,6 +322,12 @@ bool FileKey::loadHex(QIODevice* device) return true; } +/** + * Generate SHA-256 hash of arbitrary text or binary key file. + * + * @param device input device + * @return true on success + */ bool FileKey::loadHashed(QIODevice* device) { CryptoHash cryptoHash(CryptoHash::Sha256); @@ -272,7 +338,8 @@ bool FileKey::loadHashed(QIODevice* device) return false; } cryptoHash.addData(buffer); - } while (!buffer.isEmpty()); + } + while (!buffer.isEmpty()); m_key = cryptoHash.result(); diff --git a/src/keys/FileKey.h b/src/keys/FileKey.h index 7a9b1caf0..af324e530 100644 --- a/src/keys/FileKey.h +++ b/src/keys/FileKey.h @@ -1,4 +1,5 @@ /* +* Copyright (C) 2017 KeePassXC Team * Copyright (C) 2011 Felix Geyer * * This program is free software: you can redistribute it and/or modify @@ -24,16 +25,15 @@ class QIODevice; -class FileKey : public Key +class FileKey: public Key { public: - FileKey(); bool load(QIODevice* device); bool load(const QString& fileName, QString* errorMsg = nullptr); - QByteArray rawKey() const; - FileKey* clone() const; - static void create(QIODevice* device); - static bool create(const QString& fileName, QString* errorMsg = nullptr); + QByteArray rawKey() const override; + FileKey* clone() const override; + static void create(QIODevice* device, int size = 128); + static bool create(const QString& fileName, QString* errorMsg = nullptr, int size = 128); private: bool loadXml(QIODevice* device); diff --git a/tests/TestKeys.cpp b/tests/TestKeys.cpp index 738b39a26..95a72a49f 100644 --- a/tests/TestKeys.cpp +++ b/tests/TestKeys.cpp @@ -1,6 +1,6 @@ /* - * Copyright (C) 2011 Felix Geyer * Copyright (C) 2017 KeePassXC Team + * 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 @@ -22,12 +22,14 @@ #include #include "config-keepassx-tests.h" + #include "core/Database.h" #include "core/Metadata.h" +#include "core/Tools.h" #include "crypto/Crypto.h" +#include "crypto/CryptoHash.h" #include "format/KeePass2Reader.h" #include "format/KeePass2Writer.h" -#include "keys/CompositeKey.h" #include "keys/FileKey.h" #include "keys/PasswordKey.h" @@ -40,31 +42,27 @@ void TestKeys::initTestCase() void TestKeys::testComposite() { - CompositeKey* compositeKey1 = new CompositeKey(); - PasswordKey* passwordKey1 = new PasswordKey(); - PasswordKey* passwordKey2 = new PasswordKey("test"); + QScopedPointer compositeKey1(new CompositeKey()); + QScopedPointer passwordKey1(new PasswordKey()); + QScopedPointer passwordKey2(new PasswordKey("test")); bool ok; QString errorString; // make sure that addKey() creates a copy of the keys compositeKey1->addKey(*passwordKey1); compositeKey1->addKey(*passwordKey2); - delete passwordKey1; - delete passwordKey2; QByteArray transformed = compositeKey1->transform(QByteArray(32, '\0'), 1, &ok, &errorString); QVERIFY(ok); QCOMPARE(transformed.size(), 32); // make sure the subkeys are copied - CompositeKey* compositeKey2 = compositeKey1->clone(); - delete compositeKey1; + QScopedPointer compositeKey2(compositeKey1->clone()); QCOMPARE(compositeKey2->transform(QByteArray(32, '\0'), 1, &ok, &errorString), transformed); QVERIFY(ok); - delete compositeKey2; - CompositeKey* compositeKey3 = new CompositeKey(); - CompositeKey* compositeKey4 = new CompositeKey(); + QScopedPointer compositeKey3(new CompositeKey()); + QScopedPointer compositeKey4(new CompositeKey()); // test clear() compositeKey3->addKey(PasswordKey("test")); @@ -79,9 +77,6 @@ void TestKeys::testComposite() // test self-assignment *compositeKey3 = *compositeKey3; QCOMPARE(compositeKey3->rawKey(), compositeKey4->rawKey()); - - delete compositeKey3; - delete compositeKey4; } void TestKeys::testFileKey() @@ -101,12 +96,10 @@ void TestKeys::testFileKey() QCOMPARE(fileKey.rawKey().size(), 32); compositeKey.addKey(fileKey); - Database* db = reader.readDatabase(dbFilename, compositeKey); + QScopedPointer db(reader.readDatabase(dbFilename, compositeKey)); QVERIFY(db); QVERIFY(!reader.hasError()); QCOMPARE(db->metadata()->name(), QString("%1 Database").arg(name)); - - delete db; } void TestKeys::testFileKey_data() @@ -120,6 +113,20 @@ void TestKeys::testFileKey_data() } void TestKeys::testCreateFileKey() +{ + QBuffer keyBuffer1; + keyBuffer1.open(QBuffer::ReadWrite); + + FileKey::create(&keyBuffer1, 128); + QCOMPARE(keyBuffer1.size(), 128); + + QBuffer keyBuffer2; + keyBuffer2.open(QBuffer::ReadWrite); + FileKey::create(&keyBuffer2, 64); + QCOMPARE(keyBuffer2.size(), 64); +} + +void TestKeys::testCreateAndOpenFileKey() { const QString dbName("testCreateFileKey database"); @@ -134,7 +141,7 @@ void TestKeys::testCreateFileKey() CompositeKey compositeKey; compositeKey.addKey(fileKey); - Database* dbOrg = new Database(); + QScopedPointer dbOrg(new Database()); QVERIFY(dbOrg->setKey(compositeKey)); dbOrg->metadata()->setName(dbName); @@ -142,16 +149,30 @@ void TestKeys::testCreateFileKey() dbBuffer.open(QBuffer::ReadWrite); KeePass2Writer writer; - writer.writeDatabase(&dbBuffer, dbOrg); + writer.writeDatabase(&dbBuffer, dbOrg.data()); dbBuffer.reset(); - delete dbOrg; KeePass2Reader reader; - Database* dbRead = reader.readDatabase(&dbBuffer, compositeKey); + QScopedPointer dbRead(reader.readDatabase(&dbBuffer, compositeKey)); QVERIFY(dbRead); QVERIFY(!reader.hasError()); QCOMPARE(dbRead->metadata()->name(), dbName); - delete dbRead; +} + +void TestKeys::testFileKeyHash() +{ + QBuffer keyBuffer; + keyBuffer.open(QBuffer::ReadWrite); + + FileKey::create(&keyBuffer); + + CryptoHash cryptoHash(CryptoHash::Sha256); + cryptoHash.addData(keyBuffer.data()); + + FileKey fileKey; + fileKey.load(&keyBuffer); + + QCOMPARE(fileKey.rawKey(), cryptoHash.result()); } void TestKeys::testFileKeyError() diff --git a/tests/TestKeys.h b/tests/TestKeys.h index 1cbe7bf96..237225f8f 100644 --- a/tests/TestKeys.h +++ b/tests/TestKeys.h @@ -31,6 +31,8 @@ private slots: void testFileKey(); void testFileKey_data(); void testCreateFileKey(); + void testCreateAndOpenFileKey(); + void testFileKeyHash(); void testFileKeyError(); void benchmarkTransformKey(); };