ConfigFile security: Migrate Proxy password to keychain

When specified in the config file, the Proxy password will be migrated
to the keychain, for backward compatibility and to allow admins to
overwrite an existing password by rolling out updated config files.

Once migrated to the keychain, the password will be removed from the
config file.

Signed-off-by: Michael Schuster <michael@schuster.ms>

diff --git a/src/gui/creds/webflowcredentials.cpp b/src/gui/creds/webflowcredentials.cpp
index e6c1f141a3fe460bfb35d6e063c3128f9193a6bf..52eef03413e263faa9ee637814e40eb0747e973a 100644 (file)
--- a/src/gui/creds/webflowcredentials.cpp
+++ b/src/gui/creds/webflowcredentials.cpp
@@ -598,11 +598,7 @@ void WebFlowCredentials::deleteKeychainEntries(bool oldKeychainEntries) {
         job->setKey(keychainKey(_account->url().toString(),
                                 key,
                                 oldKeychainEntries ? QString() : _account->id()));
-
-        connect(job, &Job::finished, this, [](QKeychain::Job *job) {
-            auto *djob = qobject_cast<DeletePasswordJob *>(job);
-            djob->deleteLater();
-        });
+        job->setAutoDelete(true);
         job->start();
     };
 

diff --git a/src/libsync/configfile.cpp b/src/libsync/configfile.cpp
index 39bd1d170d4476b4e68ce3bcefa5db235193ad8c..299ab09d1a6fe58534197ba2d8465be894562d8e 100644 (file)
--- a/src/libsync/configfile.cpp
+++ b/src/libsync/configfile.cpp
@@ -20,6 +20,7 @@
 #include "common/asserts.h"
 
 #include "creds/abstractcredentials.h"
+#include "creds/keychainchunk.h"
 
 #include "csync_exclude.h"
 
@@ -651,7 +652,23 @@ void ConfigFile::setProxyType(int proxyType,
         settings.setValue(QLatin1String(proxyPortC), port);
         settings.setValue(QLatin1String(proxyNeedsAuthC), needsAuth);
         settings.setValue(QLatin1String(proxyUserC), user);
-        settings.setValue(QLatin1String(proxyPassC), pass.toUtf8().toBase64());
+
+        if (pass.isEmpty()) {
+            settings.remove(QLatin1String(proxyPassC));
+
+            auto *job = new QKeychain::DeletePasswordJob(Theme::instance()->appName());
+            job->setInsecureFallback(false);
+            job->setKey(keychainProxyPasswordKey());
+            job->setAutoDelete(true);
+            job->start();
+        } else {
+            // Write password to keychain
+            auto *job = new KeychainChunk::WriteJob(keychainProxyPasswordKey(), pass.toUtf8());
+            if (job->startAwait()) {
+                settings.remove(QLatin1String(proxyPassC));
+            }
+            job->deleteLater();
+        }
     }
     settings.sync();
 }
@@ -726,8 +743,36 @@ QString ConfigFile::proxyUser() const
 
 QString ConfigFile::proxyPassword() const
 {
-    QByteArray pass = getValue(proxyPassC).toByteArray();
-    return QString::fromUtf8(QByteArray::fromBase64(pass));
+    QByteArray passEncoded = getValue(proxyPassC).toByteArray();
+    auto pass = QString::fromUtf8(QByteArray::fromBase64(passEncoded));
+    passEncoded.clear();
+
+    const auto key = keychainProxyPasswordKey();
+
+    if (!pass.isEmpty()) {
+        // Security: Migrate password from config file to keychain
+        auto *job = new KeychainChunk::WriteJob(key, pass.toUtf8());
+        if (job->startAwait()) {
+            QSettings settings(configFile(), QSettings::IniFormat);
+            settings.remove(QLatin1String(proxyPassC));
+            qCInfo(lcConfigFile()) << "Migrated proxy password to keychain for" << key;
+        }
+        job->deleteLater();
+    } else {
+        // Read password from keychain
+        auto *job = new KeychainChunk::ReadJob(key);
+        if (job->startAwait()) {
+            pass = job->textData();
+        }
+        job->deleteLater();
+    }
+
+    return pass;
+}
+
+QString ConfigFile::keychainProxyPasswordKey() const
+{
+    return QString::fromLatin1("proxy-password");
 }
 
 int ConfigFile::useUploadLimit() const

diff --git a/src/libsync/configfile.h b/src/libsync/configfile.h
index 4133123750799bbda4081567ae2cefd1bd818ebd..432720a3a88070822c3cb258400d1c30fe19f9c6 100644 (file)
--- a/src/libsync/configfile.h
+++ b/src/libsync/configfile.h
@@ -198,6 +198,8 @@ private:
         const QVariant &defaultValue = QVariant()) const;
     void setValue(const QString &key, const QVariant &value);
 
+    QString keychainProxyPasswordKey() const;
+
 private:
     typedef QSharedPointer<AbstractCredentials> SharedCreds;