Secure settings rework

.gitmodules

@@ -25,3 +25,6 @@
 [submodule "client/3rd/CocoaLumberjack"]
 	path = client/3rd/CocoaLumberjack
 	url = https://github.com/CocoaLumberjack/CocoaLumberjack.git
+[submodule "client/3rd/qtkeychain"]
+	path = client/3rd/qtkeychain
+	url = https://github.com/frankosterfeld/qtkeychain.git

client/3rd/qtkeychain

@@ -0,0 +1 @@
+Subproject commit f197cdb935b0cfd9881fdc6860874cb8379d1238

client/client.pro

@@ -16,6 +16,7 @@ include("3rd/QtSsh/src/botan/botan.pri")
 include ("3rd/SortFilterProxyModel/SortFilterProxyModel.pri")
 include("3rd/qzxing/src/QZXing-components.pri")
 include("3rd/QSimpleCrypto/QSimpleCrypto.pri")
+include("3rd/qtkeychain/qtkeychain.pri")
 
 INCLUDEPATH += $$PWD/3rd/QSimpleCrypto/include
 INCLUDEPATH += $$PWD/3rd/OpenSSL/include

client/platforms/ios/MobileUtils.cpp

@@ -2,7 +2,4 @@
 
 void MobileUtils::shareText(const QStringList&) {}
 
-void MobileUtils::writeToKeychain(const QString&, const QByteArray &) {}
-bool MobileUtils::deleteFromKeychain(const QString& tag) { return false; }
-QByteArray MobileUtils::readFromKeychain(const QString&) { return {}; }
 

client/platforms/ios/MobileUtils.h

@@ -13,9 +13,6 @@ public:
 public slots:
     static void shareText(const QStringList& filesToSend);
 
-    static void writeToKeychain(const QString& tag, const QByteArray& value);
-    static bool deleteFromKeychain(const QString& tag);
-    static QByteArray readFromKeychain(const QString& tag);
 };
 
 #endif // MOBILEUTILS_H

client/platforms/ios/MobileUtils.mm

@@ -35,58 +35,3 @@ void MobileUtils::shareText(const QStringList& filesToSend) {
     }
 }
 
-bool MobileUtils::deleteFromKeychain(const QString& tag) {
-    NSData* nsTag = [tag.toNSString() dataUsingEncoding:NSUTF8StringEncoding];
-
-    NSDictionary *deleteQuery = @{ (id)kSecAttrAccount: nsTag,
-                                   (id)kSecClass: (id)kSecClassGenericPassword,
-                                 };
-
-    OSStatus status = SecItemDelete((__bridge CFDictionaryRef)deleteQuery);
-    if (status != errSecSuccess) {
-        qDebug() << "Error deleteFromKeychain" << status;
-        return false;
-    }
-}
-
-void MobileUtils::writeToKeychain(const QString& tag, const QByteArray& value) {
-    deleteFromKeychain(tag);
-
-    NSData* nsTag = [tag.toNSString() dataUsingEncoding:NSUTF8StringEncoding];
-    NSData* nsValue = value.toNSData();
-
-    NSDictionary* addQuery = @{ (id)kSecAttrAccount: nsTag,
-                                (id)kSecClass: (id)kSecClassGenericPassword,
-                                (id)kSecValueData: nsValue,
-                              };
-
-    OSStatus status = SecItemAdd((__bridge CFDictionaryRef)addQuery, NULL);
-    if (status != errSecSuccess) {
-        qDebug() << "Error writeToKeychain" << status;
-    }
-}
-
-QByteArray MobileUtils::readFromKeychain(const QString& tag) {
-    NSData* nsTag = [tag.toNSString() dataUsingEncoding:NSUTF8StringEncoding];
-    NSData* nsValue = NULL;
-
-    NSDictionary *getQuery = @{ (id)kSecAttrAccount: nsTag,
-                                (id)kSecClass: (id)kSecClassGenericPassword,
-                                (id)kSecMatchLimit: (id)kSecMatchLimitOne,
-                                (id)kSecReturnData: @YES,
-                              };
-    
-    OSStatus status = SecItemCopyMatching((__bridge CFDictionaryRef)getQuery,
-                                          (CFTypeRef *)&nsValue);
-    if (status != errSecSuccess) {
-        qDebug() << "Error readFromKeychain" << status;
-    }
-
-    QByteArray result;
-    if (nsValue) {
-        result = QByteArray::fromNSData(nsValue);
-        CFRelease(nsValue);
-    }
-    
-    return result;
-}

client/secure_qsettings.cpp

@@ -3,11 +3,17 @@
 
 #include <QDataStream>
 #include <QDebug>
+#include <QEventLoop>
+#include <QJsonDocument>
+#include <QJsonObject>
+#include <QTimer>
 #include "utils.h"
 #include <QRandomGenerator>
 #include "QAead.h"
 #include "QBlockCipher.h"
 
+using namespace QKeychain;
+
 SecureQSettings::SecureQSettings(const QString &organization, const QString &application, QObject *parent)
     : QObject{parent},
       m_settings(organization, application, parent),
@@ -70,7 +76,6 @@ QVariant SecureQSettings::value(const QString &key, const QVariant &defaultValue
     }
 
     m_cache.insert(key, retVal);
-
     return retVal;
 }
 
@@ -120,35 +125,26 @@ void SecureQSettings::sync()
 
 QByteArray SecureQSettings::backupAppConfig() const
 {
-    QMap<QString, QVariant> cfg;
+    QJsonObject cfg;
+
     for (const QString &key : m_settings.allKeys()) {
-        cfg.insert(key, value(key));
+        cfg.insert(key, QJsonValue::fromVariant(value(key)));
     }
 
-    QByteArray ba;
+    return QJsonDocument(cfg).toJson();
-    {
-        QDataStream ds(&ba, QIODevice::WriteOnly);
-        ds << cfg;
-    }
-
-    return ba.toBase64();
 }
 
-void SecureQSettings::restoreAppConfig(const QByteArray &base64Cfg)
+bool SecureQSettings::restoreAppConfig(const QByteArray &json)
 {
-    QByteArray ba = QByteArray::fromBase64(base64Cfg);
+    QJsonObject cfg = QJsonDocument::fromJson(json).object();
-    QMap<QString, QVariant> cfg;
+    if (cfg.isEmpty()) return false;
-
-    {
-        QDataStream ds(&ba, QIODevice::ReadOnly);
-        ds >> cfg;
-    }
 
     for (const QString &key : cfg.keys()) {
-        setValue(key, cfg.value(key));
+        setValue(key, cfg.value(key).toVariant());
     }
 
     sync();
+    return true;
 }
 
 
@@ -166,17 +162,14 @@ QByteArray SecureQSettings::decryptText(const QByteArray& ba) const
 
 bool SecureQSettings::encryptionRequired() const
 {
-#if defined Q_OS_IOS // || defined Q_OS_ANDROID
+    // TODO: review on linux
     return true;
-#endif
-
-    return false;
 }
 
 QByteArray SecureQSettings::getEncKey() const
 {
     // load keys from system key storage
-    m_key = MobileUtils::readFromKeychain(settingsKeyTag);
+    m_key = getSecTag(settingsKeyTag);
 
     if (m_key.isEmpty()) {
         // Create new key
@@ -186,10 +179,10 @@ QByteArray SecureQSettings::getEncKey() const
             qCritical() << "SecureQSettings::getEncKey Unable to generate new enc key";
         }
 
-        MobileUtils::writeToKeychain(settingsKeyTag, key);
+        setSecTag(settingsKeyTag, key);
 
         // check
-        m_key = MobileUtils::readFromKeychain(settingsKeyTag);
+        m_key = getSecTag(settingsKeyTag);
         if (key != m_key) {
             qCritical() << "SecureQSettings::getEncKey Unable to store key in keychain" << key.size() << m_key.size();
             return {};
@@ -202,7 +195,7 @@ QByteArray SecureQSettings::getEncKey() const
 QByteArray SecureQSettings::getEncIv() const
 {
     // load keys from system key storage
-    m_iv = MobileUtils::readFromKeychain(settingsIvTag);
+    m_iv = getSecTag(settingsIvTag);
 
     if (m_iv.isEmpty()) {
         // Create new IV
@@ -211,10 +204,10 @@ QByteArray SecureQSettings::getEncIv() const
         if (iv.isEmpty()) {
             qCritical() << "SecureQSettings::getEncIv Unable to generate new enc IV";
         }
-        MobileUtils::writeToKeychain(settingsIvTag, iv);
+        setSecTag(settingsIvTag, iv);
 
         // check
-        m_iv = MobileUtils::readFromKeychain(settingsIvTag);
+        m_iv = getSecTag(settingsIvTag);
         if (iv != m_iv) {
             qCritical() << "SecureQSettings::getEncIv Unable to store IV in keychain" << iv.size() << m_iv.size();
             return {};
@@ -224,4 +217,38 @@ QByteArray SecureQSettings::getEncIv() const
     return m_iv;
 }
 
+QByteArray SecureQSettings::getSecTag(const QString &tag)
+{
+    ReadPasswordJob job("get-" + tag);
+    job.setAutoDelete(false);
+    job.setKey(tag);
+    QEventLoop loop;
+    job.connect(&job, SIGNAL(finished(QKeychain::Job*)), &loop, SLOT(quit()));
+    job.start();
+    loop.exec();
+
+    if ( job.error() ) {
+        qCritical() << "SecureQSettings::getSecTag Error:" << job.errorString();
+    }
+
+    return job.binaryData();
+}
+
+void SecureQSettings::setSecTag(const QString &tag, const QByteArray &data)
+{
+    WritePasswordJob job("set-" + tag);
+    job.setAutoDelete(false);
+    job.setKey(tag);
+    job.setBinaryData(data);
+    QEventLoop loop;
+    QTimer::singleShot(1000, &loop, SLOT(quit()));
+    job.connect(&job, SIGNAL(finished(QKeychain::Job*)), &loop, SLOT(quit()));
+    job.start();
+    loop.exec();
+
+    if (job.error()) {
+        qCritical() << "SecureQSettings::setSecTag Error:" << job.errorString();
+    }
+}
+
 

client/secure_qsettings.h

@@ -6,6 +6,8 @@
 #include <QMutex>
 #include <QMutexLocker>
 
+#include "keychain.h"
+
 
 constexpr const char* settingsKeyTag = "settingsKeyTag";
 constexpr const char* settingsIvTag = "settingsIvTag";
@@ -22,7 +24,7 @@ public:
     void sync();
 
     QByteArray backupAppConfig() const;
-    void restoreAppConfig(const QByteArray &base64Cfg);
+    bool restoreAppConfig(const QByteArray &json);
 
     QByteArray encryptText(const QByteArray &value) const;
     QByteArray decryptText(const QByteArray& ba) const;
@@ -31,6 +33,10 @@ public:
 
     QByteArray getEncKey() const;
     QByteArray getEncIv() const;
+
+    static QByteArray getSecTag(const QString &tag);
+    static void setSecTag(const QString &tag, const QByteArray &data);
+
 private:
     QSettings m_settings;
 

client/settings.h

@@ -111,7 +111,7 @@ public:
 //    static constexpr char openNicNs13[] = "144.76.103.143";
 
     QByteArray backupAppConfig() const { return m_settings.backupAppConfig(); }
-    void restoreAppConfig(const QByteArray &cfg) { m_settings.restoreAppConfig(cfg); }
+    bool restoreAppConfig(const QByteArray &cfg) { return m_settings.restoreAppConfig(cfg); }
 
 private:
     SecureQSettings m_settings;

client/ui/pages_logic/AppSettingsLogic.cpp

@@ -7,6 +7,7 @@
 
 #include <QDesktopServices>
 #include <QFileDialog>
+#include <QMessageBox>
 #include <QStandardPaths>
 
 using namespace amnezia;
@@ -91,9 +92,15 @@ void AppSettingsLogic::onPushButtonRestoreAppConfigClicked()
     file.open(QIODevice::ReadOnly);
     QByteArray data = file.readAll();
 
-    m_settings->restoreAppConfig(data);
+    bool ok = m_settings->restoreAppConfig(data);
+    if (ok) {
+        emit uiLogic()->goToPage(Page::Vpn);
+        emit uiLogic()->setStartPage(Page::Vpn);
+    }
+    else {
+        QMessageBox::warning(nullptr, APPLICATION_NAME,
+            tr("Can't import config, file is corrupted."));
+    }
 
-    emit uiLogic()->goToPage(Page::Vpn);
-    emit uiLogic()->setStartPage(Page::Vpn);
 }