From: Michael Schuster <michael@schuster.ms>
Date: Thu, 25 Jun 2020 14:59:57 +0000 (+0200)
Subject: ConfigFile security: Migrate Proxy password to keychain
X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~222^2^2~111^2~5
X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=81c644e7025829cfd973db618724e0751e1fa162;p=nextcloud-desktop.git

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 e6c1f141a..52eef0341 100644
--- 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 39bd1d170..299ab09d1 100644
--- 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 413312375..432720a3a 100644
--- 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;