New Feature: KDBX4 by phoerious · Pull Request #1230 · keepassxreboot/keepassxc

phoerious · 2017-11-26T21:12:09Z

Working PR for KDBX4. Follows up on #1179.

Closes #148 and closes #1060

Types of changes

✅ New feature (non-breaking change which adds functionality)
✅ Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

✅ I have read the CONTRIBUTING document. [REQUIRED]
✅ My code follows the code style of this project. [REQUIRED]
✅ All new and existing tests passed. [REQUIRED]
✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]
✅ I have added tests to cover my changes.

angelsl · 2017-11-29T12:32:19Z

Be sure to pick up the rest of the KDBX 4 changes as well.

droidmonkey · 2017-12-22T04:57:42Z

Reminder to implement #1060

phoerious · 2017-12-22T12:44:46Z

Aye.

adolfogc · 2017-12-28T02:19:05Z

src/format/Kdbx3Reader.cpp

+
+    if (xmlReader.hasError()) {
+        raiseError(xmlReader.errorString());
+        if (keepDatabase) {


I think this reads better as:

return keepDatabase ? db.take() : nullptr;

adolfogc · 2017-12-30T01:16:58Z

src/format/Kdbx3Reader.cpp

+    }
+
+    QIODevice* xmlDevice;
+    QScopedPointer<QtIOCompressor> ioCompressor;


What do you think of replacing QtIOCompressor with KCompressionDevice from KF5Archive?

What would be the purpose?

Remove the embedded version.

I'm against adding new dependencies here if not really needed.

Ok; makes sense. I've also noticed there is no package available for Ubuntu Trusty. Please disregard.

phoerious · 2018-01-06T16:22:14Z

src/crypto/kdf/Argon2Kdf.cpp

+Argon2Kdf::Argon2Kdf()
+        : Kdf::Kdf(KeePass2::KDF_ARGON2)
+        , m_version(0x13)
+        , m_memory(1<<16)


phoerious · 2018-01-06T16:22:48Z

src/crypto/kdf/Argon2Kdf.cpp

+bool Argon2Kdf::setMemory(quint64 kibibytes)
+{
+    // MIN=8KB; MAX=2,147,483,648KB
+    if (kibibytes >= 8 && kibibytes < (1ULL<<32)) {


phoerious · 2018-01-06T16:22:54Z

src/crypto/kdf/Argon2Kdf.cpp

+bool Argon2Kdf::setParallelism(quint32 threads)
+{
+    // MIN=1; MAX=16,777,215
+    if (threads >= 1 && threads < (1<<24)) {


phoerious · 2018-01-06T16:27:49Z

src/format/Kdbx3Reader.cpp

+{
+    if (data.size() != 32) {
+        raiseError("Invalid master seed size");
+    } else {


phoerious · 2018-01-06T16:28:07Z

src/format/Kdbx3Reader.cpp

+{
+    if (data.size() != 4) {
+        raiseError("Invalid compression flags length");
+    } else {


return here, drop else.

phoerious · 2018-01-06T19:15:29Z

src/streams/HmacBlockStream.cpp

+    }
+
+    m_bufferPos = 0;
+    m_blockIndex++;


++m_blockIndex

phoerious · 2018-01-06T19:16:40Z

src/streams/SymmetricCipherStream.cpp

+                    m_buffer.resize(blockSize() - padLength);
+                    return true;
+                }
+            } else {


phoerious · 2018-01-06T19:16:57Z

src/streams/SymmetricCipherStream.cpp

+int SymmetricCipherStream::blockSize() const {
+    if (m_streamCipher) {
+        return 1024;
+    } else {


phoerious · 2018-01-06T19:18:12Z

tests/TestKeePass2Reader.cpp

+
+void TestKeePass2Reader::testFormat400()
+{
+    QString filename = QString(KEEPASSX_TEST_DATA_DIR).append("/Format400.kdbx");


Was this file generated by KeePass2? We should use that as reference.

phoerious · 2018-01-06T19:19:08Z

tests/TestKeePass2XmlReader.cpp

    QVERIFY(!reader.hasError());
 }

+void TestKdbx3XmlReader::readDatabase(QString path, bool strictMode, Database*& db, bool& hasError, QString& errorString)


hasError should be the return value.

TheZ3ro · 2018-01-07T22:15:30Z

src/format/KeePass2.cpp

+    }
+
+    Q_ASSERT_X(false, "uuidToKdf", "Invalid UUID");
+    return nullptr;


return {} instead

Note: This implementation is not yet connected to the database itself and will corrupt existing kdbx3 db's. * Implemented memory and parallelism parameters for Argon2Kdf * Using libargon2; libsodium does not support Argon2d algorithm * Moved basic rounds parameter into Kdf class * Reimplemented benchmark algorithm; previous was utterly broken

* Adds KDBX4 reader/writer interfaces * Adds KDBX4 XML reader/write interfaces * Implements test cases for KDBX4 * Fully compatible with KeePass2 * Corrects minor issues with Argon2 KDF

Increase default AES-KDF rounds to 100k

* Refactor Kdbx*Reader * Refactor KdbxWriter * Refactor KdbxXmlReader * Refactor KdbxXmlWriter

* Re-implement KDBX4 challenge-response key assembly with transform seed instead of master seed

…o number of available processors

…h AES-KDF back to KDBX3 when saving

* Use legacy AES-KDF mode for KeePass1Reader

droidmonkey · 2018-01-13T19:49:00Z

src/format/KeePass2.cpp

+};
+
+const QList<QPair<Uuid, QString>> KeePass2::KDFS{
+    qMakePair(KeePass2::KDF_ARGON2, QObject::tr("Argon2 (recommended)")),


I think this should be "Argon2 (KDBX 4 - recommended)"

droidmonkey · 2018-01-13T19:53:34Z

src/gui/DatabaseSettingsWidget.cpp

+        m_uiEncryption->parallelismSpinBox->setValue(argon2Kdf->parallelism());
+    }
+
+    m_uiGeneral->dbNameEdit->setFocus();


Need to add m_ui->categoryList->setCurrentCategory(0); after this line to highlight the first category by default.

Good catch. Fixed.

…base settings by default

- Add support for KDBX 4.0, Argon2 and ChaCha20 [#148, #1179, #1230, #1494] - Add SSH Agent feature [#1098, #1450, #1463] - Add preview panel with details of the selected entry [#879, #1338] - Add more and configurable columns to entry table and allow copying of values by double click [#1305] - Add KeePassXC-Browser API as a replacement for KeePassHTTP [#608] - Deprecate KeePassHTTP [#1392] - Add support for Steam one-time passwords [#1206] - Add support for multiple Auto-Type sequences for a single entry [#1390] - Adjust YubiKey HMAC-SHA1 challenge-response key generation for KDBX 4.0 [#1060] - Replace qHttp with cURL for website icon downloads [#1460] - Remove lock file [#1231] - Add option to create backup file before saving [#1385] - Ask to save a generated password before closing the entry password generator [#1499] - Resolve placeholders recursively [#1078] - Add Auto-Type button to the toolbar [#1056] - Improve window focus handling for Auto-Type dialogs [#1204, #1490] - Auto-Type dialog and password generator can now be exited with ESC [#1252, #1412] - Add optional dark tray icon [#1154] - Add new "Unsafe saving" option to work around saving problems with file sync services [#1385] - Add IBus support to AppImage and additional image formats to Windows builds [#1534, #1537] - Add diceware password generator to CLI [#1406] - Add --key-file option to CLI [#816, #824] - Add DBus interface for opening and closing KeePassXC databases [#283] - Add KDBX compression options to database settings [#1419] - Discourage use of old fixed-length key files in favor of arbitrary files [#1326, #1327] - Correct reference resolution in entry fields [#1486] - Fix window state and recent databases not being remembered on exit [#1453] - Correct history item generation when configuring TOTP for an entry [#1446] - Correct multiple TOTP bugs [#1414] - Automatic saving after every change is now a default [#279] - Allow creation of new entries during search [#1398] - Correct menu issues on macOS [#1335] - Allow compilation on OpenBSD [#1328] - Improve entry attachments view [#1139, #1298] - Fix auto lock for Gnome and Xfce [#910, #1249] - Don't remember key files in file dialogs when the setting is disabled [#1188] - Improve database merging and conflict resolution [#807, #1165] - Fix macOS pasteboard issues [#1202] - Improve startup times on some platforms [#1205] - Hide the notes field by default [#1124] - Toggle main window by clicking tray icon with the middle mouse button [#992] - Fix custom icons not copied over when databases are merged [#1008] - Allow use of DEL key to delete entries [#914] - Correct intermittent crash due to stale history items [#1527] - Sanitize newline characters in title, username and URL fields [#1502] - Reopen previously opened databases in correct order [#774] - Use system's zxcvbn library if available [#701] - Implement various i18n improvements [#690, #875, #1436]

phoerious added the security label Nov 26, 2017

phoerious added this to the v2.3.0 milestone Nov 26, 2017

phoerious assigned phoerious and droidmonkey Nov 26, 2017

phoerious added the file format label Nov 26, 2017

droidmonkey requested review from TheZ3ro, droidmonkey, louib and weslly November 26, 2017 22:18

phoerious force-pushed the feature/kdbx4 branch 2 times, most recently from a5ddf93 to 06179bc Compare December 17, 2017 16:26

phoerious mentioned this pull request Dec 24, 2017

Adding support for KDBX4 file format #148

Closed

phoerious force-pushed the feature/kdbx4 branch 2 times, most recently from 356c6df to c57a878 Compare December 26, 2017 22:59

phoerious changed the title ~~New Feature: KDBX4~~ New Feature: KDBX4 (Prep) Dec 26, 2017

adolfogc reviewed Dec 28, 2017

View reviewed changes

adolfogc reviewed Dec 30, 2017

View reviewed changes

droidmonkey force-pushed the feature/kdbx4 branch from 90e7479 to da617c0 Compare January 5, 2018 15:46

phoerious commented Jan 6, 2018

View reviewed changes

phoerious changed the title ~~New Feature: KDBX4 (Prep)~~ New Feature: KDBX4 Jan 6, 2018

phoerious force-pushed the feature/kdbx4 branch from 95fb856 to b94567e Compare January 7, 2018 03:14

TheZ3ro reviewed Jan 7, 2018

View reviewed changes

phoerious force-pushed the feature/kdbx4 branch 2 times, most recently from e58326d to bafaf3c Compare January 7, 2018 22:44

phoerious added the core label Jan 7, 2018

phoerious force-pushed the feature/kdbx4 branch 2 times, most recently from 3950750 to b4e42fc Compare January 10, 2018 20:51

droidmonkey and others added 23 commits January 13, 2018 14:23

Formatting and consistency fixes

61c4fe8

Removed dead code

2866bc6

Correct Endian function use in Random tests

9140893

Correct regression in database modification signal

738f870

Correct failure in GUI tests due to widget name

7dba788

Implements KDBX4 format with Argon2 KDF

bef7ba2

* Adds KDBX4 reader/writer interfaces * Adds KDBX4 XML reader/write interfaces * Implements test cases for KDBX4 * Fully compatible with KeePass2 * Corrects minor issues with Argon2 KDF

Report database read failure reason in tests

564188b

Fix compilation error

f7d3c90

Redesign DatabaseSettingsWidget

995d664

Show warning when using inappropriate transform round number

54fb0d9

Increase default AES-KDF rounds to 100k

Fix coding style and GUI test

ccfd7a0

Fix memory leaks in tests

72a1c65

Refactor database readers/writers and XML handling

a6ddc22

* Refactor Kdbx*Reader * Refactor KdbxWriter * Refactor KdbxXmlReader * Refactor KdbxXmlWriter

Add challenge response to key before transformation, resolves #1060

df72808

* Re-implement KDBX4 challenge-response key assembly with transform seed instead of master seed

Fix type conversion error in older Qt versions

c51585c

Increase Argon2 warning threshold and set parallelism default value t…

d327c16

…o number of available processors

Explicitly support AES-KDF in KDBX4 and don't convert KDBX4 files wit…

b04c8c2

…h AES-KDF back to KDBX3 when saving

Do not assert KDF UUID and improve error message

6df54cf

Save AES-KDF round parameter as quint64

337a21f

Install needed PPA packages for deployment

8246070

Fix test compilation errors

90380ad

* Use legacy AES-KDF mode for KeePass1Reader

Force Docker rebuild to fetch fixed version of libgpg-error-127

96e271e

droidmonkey force-pushed the feature/kdbx4 branch from 193de9c to 96e271e Compare January 13, 2018 19:32

droidmonkey requested changes Jan 13, 2018

View reviewed changes

Add 'KDBX 4' to Argon2 KDF name and select first category in the data…

1575088

…base settings by default

droidmonkey approved these changes Jan 13, 2018

View reviewed changes

phoerious merged commit 7a55ab6 into develop Jan 13, 2018

phoerious deleted the feature/kdbx4 branch January 13, 2018 22:45

Uh oh!

Conversation

phoerious commented Nov 26, 2017 • edited by droidmonkey Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

Checklist:

Uh oh!

angelsl commented Nov 29, 2017

Uh oh!

droidmonkey commented Dec 22, 2017

Uh oh!

phoerious commented Dec 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phoerious Jan 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

phoerious commented Nov 26, 2017 •

edited by droidmonkey

Loading

phoerious Jan 13, 2018 •

edited

Loading