From 789902b79a32ba1cb7113bb046044342df37defd Mon Sep 17 00:00:00 2001 From: pokamest Date: Sat, 27 Aug 2022 17:35:43 +0300 Subject: [PATCH] Secure settings rework --- .gitmodules | 3 + client/3rd/qtkeychain | 1 + client/client.pro | 1 + client/platforms/ios/MobileUtils.cpp | 3 - client/platforms/ios/MobileUtils.h | 3 - client/platforms/ios/MobileUtils.mm | 55 -------------- client/secure_qsettings.cpp | 85 ++++++++++++++-------- client/secure_qsettings.h | 8 +- client/settings.h | 2 +- client/ui/pages_logic/AppSettingsLogic.cpp | 13 +++- 10 files changed, 79 insertions(+), 95 deletions(-) create mode 160000 client/3rd/qtkeychain diff --git a/.gitmodules b/.gitmodules index 008ebf89..328bfe76 100644 --- a/.gitmodules +++ b/.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 diff --git a/client/3rd/qtkeychain b/client/3rd/qtkeychain new file mode 160000 index 00000000..f197cdb9 --- /dev/null +++ b/client/3rd/qtkeychain @@ -0,0 +1 @@ +Subproject commit f197cdb935b0cfd9881fdc6860874cb8379d1238 diff --git a/client/client.pro b/client/client.pro index 15702755..3ea95799 100644 --- a/client/client.pro +++ b/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 diff --git a/client/platforms/ios/MobileUtils.cpp b/client/platforms/ios/MobileUtils.cpp index 427cf334..3923d291 100644 --- a/client/platforms/ios/MobileUtils.cpp +++ b/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 {}; } diff --git a/client/platforms/ios/MobileUtils.h b/client/platforms/ios/MobileUtils.h index 045ababb..a7967fdf 100644 --- a/client/platforms/ios/MobileUtils.h +++ b/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 diff --git a/client/platforms/ios/MobileUtils.mm b/client/platforms/ios/MobileUtils.mm index bfe82a6a..a9ad52b5 100644 --- a/client/platforms/ios/MobileUtils.mm +++ b/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; -} diff --git a/client/secure_qsettings.cpp b/client/secure_qsettings.cpp index 82836979..cbb8468d 100644 --- a/client/secure_qsettings.cpp +++ b/client/secure_qsettings.cpp @@ -3,11 +3,17 @@ #include #include +#include +#include +#include +#include #include "utils.h" #include #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 cfg; + QJsonObject cfg; + for (const QString &key : m_settings.allKeys()) { - cfg.insert(key, value(key)); + cfg.insert(key, QJsonValue::fromVariant(value(key))); } - QByteArray ba; - { - QDataStream ds(&ba, QIODevice::WriteOnly); - ds << cfg; - } - - return ba.toBase64(); + return QJsonDocument(cfg).toJson(); } -void SecureQSettings::restoreAppConfig(const QByteArray &base64Cfg) +bool SecureQSettings::restoreAppConfig(const QByteArray &json) { - QByteArray ba = QByteArray::fromBase64(base64Cfg); - QMap cfg; - - { - QDataStream ds(&ba, QIODevice::ReadOnly); - ds >> cfg; - } + QJsonObject cfg = QJsonDocument::fromJson(json).object(); + if (cfg.isEmpty()) return false; 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(); + } +} + diff --git a/client/secure_qsettings.h b/client/secure_qsettings.h index ea6da106..e0be9bfa 100644 --- a/client/secure_qsettings.h +++ b/client/secure_qsettings.h @@ -6,6 +6,8 @@ #include #include +#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; diff --git a/client/settings.h b/client/settings.h index aa32cb0e..c78b9a79 100644 --- a/client/settings.h +++ b/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; diff --git a/client/ui/pages_logic/AppSettingsLogic.cpp b/client/ui/pages_logic/AppSettingsLogic.cpp index e9c614aa..b2907c3d 100644 --- a/client/ui/pages_logic/AppSettingsLogic.cpp +++ b/client/ui/pages_logic/AppSettingsLogic.cpp @@ -7,6 +7,7 @@ #include #include +#include #include 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); }