Add CLI tests and improve coding style and i18n

The CLI module was lacking unit test coverage and showed some severe
coding style violations, which this patch addresses.

In addition, all uses of qCritical() with untranslatble raw char*
sequences were removed in favor of proper locale strings. These are
written to STDERR through QTextStreams and support output
redirection for testing purposes. With this change, error messages don't
depend on the global Qt logging settings and targets anymore and go
directly to the terminal or into a file if needed.

This patch also fixes a bug discovered during unit test development,
where the extract command would just dump the raw XML contents without
decrypting embedded Salsa20-protected values first, making the XML
export mostly useless, since passwords are scrambled.

Lastly, all CLI commands received a dedicated -h/--help option.

src/format/Kdbx3Reader.cpp

@@ -110,14 +110,6 @@ Database* Kdbx3Reader::readDatabaseImpl(QIODevice* device,
         return nullptr;
     }
 
-    QBuffer buffer;
-    if (saveXml()) {
-        m_xmlData = xmlDevice->readAll();
-        buffer.setBuffer(&m_xmlData);
-        buffer.open(QIODevice::ReadOnly);
-        xmlDevice = &buffer;
-    }
-
     Q_ASSERT(xmlDevice);
 
     KdbxXmlReader xmlReader(KeePass2::FILE_VERSION_3_1);

src/format/Kdbx4Reader.cpp

@@ -124,14 +124,6 @@ Database* Kdbx4Reader::readDatabaseImpl(QIODevice* device,
         return nullptr;
     }
 
-    QBuffer buffer;
-    if (saveXml()) {
-        m_xmlData = xmlDevice->readAll();
-        buffer.setBuffer(&m_xmlData);
-        buffer.open(QIODevice::ReadOnly);
-        xmlDevice = &buffer;
-    }
-
     Q_ASSERT(xmlDevice);
 
     KdbxXmlReader xmlReader(KeePass2::FILE_VERSION_4, binaryPool());

src/format/KdbxReader.cpp

@@ -1,3 +1,5 @@
+#include <utility>
+
 /*
  * Copyright (C) 2018 KeePassXC Team <team@keepassxc.org>
  *
@@ -18,6 +20,9 @@
 #include "KdbxReader.h"
 #include "core/Database.h"
 #include "core/Endian.h"
+#include "format/KdbxXmlWriter.h"
+
+#include <QBuffer>
 
 #define UUID_LENGTH 16
 
@@ -92,7 +97,14 @@ Database* KdbxReader::readDatabase(QIODevice* device, QSharedPointer<const Compo
     }
 
     // read payload
-    return readDatabaseImpl(device, headerStream.storedData(), key, keepDatabase);
+    auto* db = readDatabaseImpl(device, headerStream.storedData(), std::move(key), keepDatabase);
+
+    if (saveXml()) {
+        m_xmlData.clear();
+        decryptXmlInnerStream(m_xmlData, db);
+    }
+
+    return db;
 }
 
 bool KdbxReader::hasError() const
@@ -258,6 +270,23 @@ void KdbxReader::setInnerRandomStreamID(const QByteArray& data)
     m_irsAlgo = irsAlgo;
 }
 
+/**
+ * Decrypt protected inner stream fields in XML dump on demand.
+ * Without the stream key from the KDBX header, the values become worthless.
+ *
+ * @param xmlOutput XML dump with decrypted fields
+ * @param db the database object for which to generate the decrypted XML dump
+ */
+void KdbxReader::decryptXmlInnerStream(QByteArray& xmlOutput, Database* db) const
+{
+    QBuffer buffer;
+    buffer.setBuffer(&xmlOutput);
+    buffer.open(QIODevice::WriteOnly);
+    KdbxXmlWriter writer(m_kdbxVersion);
+    writer.disableInnerStreamProtection(true);
+    writer.writeDatabase(&buffer, db);
+}
+
 /**
  * Raise an error. Use in case of an unexpected read error.
  *

src/format/KdbxReader.h

@@ -86,6 +86,8 @@ protected:
 
     void raiseError(const QString& errorMessage);
 
+    void decryptXmlInnerStream(QByteArray& xmlOutput, Database* db) const;
+
     QScopedPointer<Database> m_db;
 
     QPair<quint32, quint32> m_kdbxSignature;

src/format/KdbxXmlReader.cpp

@@ -82,7 +82,6 @@ Database* KdbxXmlReader::readDatabase(QIODevice* device)
  * @param db database to read into
  * @param randomStream random stream to use for decryption
  */
-#include "QDebug"
 void KdbxXmlReader::readDatabase(QIODevice* device, Database* db, KeePass2RandomStream* randomStream)
 {
     m_error = false;

src/format/KdbxXmlWriter.cpp

@@ -358,10 +358,10 @@ void KdbxXmlWriter::writeEntry(const Entry* entry)
 
         bool protect =
             (((key == "Title") && m_meta->protectTitle()) || ((key == "UserName") && m_meta->protectUsername())
-             || ((key == "Password") && m_meta->protectPassword())
-             || ((key == "URL") && m_meta->protectUrl())
-             || ((key == "Notes") && m_meta->protectNotes())
-             || entry->attributes()->isProtected(key));
+            || ((key == "Password") && m_meta->protectPassword())
+            || ((key == "URL") && m_meta->protectUrl())
+            || ((key == "Notes") && m_meta->protectNotes())
+            || entry->attributes()->isProtected(key));
 
         writeString("Key", key);
 
@@ -369,7 +369,7 @@ void KdbxXmlWriter::writeEntry(const Entry* entry)
         QString value;
 
         if (protect) {
-            if (m_randomStream) {
+            if (!m_innerStreamProtectionDisabled && m_randomStream) {
                 m_xml.writeAttribute("Protected", "True");
                 bool ok;
                 QByteArray rawData = m_randomStream->process(entry->attributes()->value(key).toUtf8(), &ok);
@@ -596,3 +596,24 @@ void KdbxXmlWriter::raiseError(const QString& errorMessage)
     m_error = true;
     m_errorStr = errorMessage;
 }
+
+/**
+ * Disable inner stream protection and write protected fields
+ * in plaintext instead. This is useful for plaintext XML exports
+ * where the inner stream key is not available.
+ *
+ * @param disable true to disable protection
+ */
+void KdbxXmlWriter::disableInnerStreamProtection(bool disable)
+{
+    m_innerStreamProtectionDisabled = disable;
+}
+
+/**
+ * @return true if inner stream protection is disabled and protected
+ *         fields will be saved in plaintext
+ */
+bool KdbxXmlWriter::innerStreamProtectionDisabled() const
+{
+    return m_innerStreamProtectionDisabled;
+}

src/format/KdbxXmlWriter.h

@@ -41,6 +41,8 @@ public:
                        KeePass2RandomStream* randomStream = nullptr,
                        const QByteArray& headerHash = QByteArray());
     void writeDatabase(const QString& filename, Database* db);
+    void disableInnerStreamProtection(bool disable);
+    bool innerStreamProtectionDisabled() const;
     bool hasError();
     QString errorString();
 
@@ -81,6 +83,8 @@ private:
 
     const quint32 m_kdbxVersion;
 
+    bool m_innerStreamProtectionDisabled = false;
+
     QXmlStreamWriter m_xml;
     QPointer<Database> m_db;
     QPointer<Metadata> m_meta;

src/format/KeePass2.cpp

@@ -23,7 +23,8 @@
 
 #define UUID_LENGTH 16
 
-const QUuid KeePass2::CIPHER_AES      = QUuid("31c1f2e6-bf71-4350-be58-05216afc5aff");
+const QUuid KeePass2::CIPHER_AES128   = QUuid("61ab05a1-9464-41c3-8d74-3a563df8dd35");
+const QUuid KeePass2::CIPHER_AES256   = QUuid("31c1f2e6-bf71-4350-be58-05216afc5aff");
 const QUuid KeePass2::CIPHER_TWOFISH  = QUuid("ad68f29f-576f-4bb9-a36a-d47af965346c");
 const QUuid KeePass2::CIPHER_CHACHA20 = QUuid("d6038a2b-8b6f-4cb5-a524-339a31dbb59a");
 
@@ -47,7 +48,7 @@ const QString KeePass2::KDFPARAM_ARGON2_SECRET("K");
 const QString KeePass2::KDFPARAM_ARGON2_ASSOCDATA("A");
 
 const QList<QPair<QUuid, QString>> KeePass2::CIPHERS{
-    qMakePair(KeePass2::CIPHER_AES, QObject::tr("AES: 256-bit")),
+    qMakePair(KeePass2::CIPHER_AES256, QObject::tr("AES: 256-bit")),
     qMakePair(KeePass2::CIPHER_TWOFISH, QObject::tr("Twofish: 256-bit")),
     qMakePair(KeePass2::CIPHER_CHACHA20, QObject::tr("ChaCha20: 256-bit"))
 };

src/format/KeePass2.h

@@ -46,7 +46,8 @@ namespace KeePass2
 
     const QSysInfo::Endian BYTEORDER = QSysInfo::LittleEndian;
 
-extern const QUuid CIPHER_AES;
+extern const QUuid CIPHER_AES128;
+extern const QUuid CIPHER_AES256;
 extern const QUuid CIPHER_TWOFISH;
 extern const QUuid CIPHER_CHACHA20;
 

src/format/KeePass2Reader.cpp

@@ -93,7 +93,7 @@ Database* KeePass2Reader::readDatabase(QIODevice* device, QSharedPointer<const C
     }
 
     m_reader->setSaveXml(m_saveXml);
-    return m_reader->readDatabase(device, key, keepDatabase);
+    return m_reader->readDatabase(device, std::move(key), keepDatabase);
 }
 
 bool KeePass2Reader::hasError() const