Avoid keeping Account alive via a shared ptr in ClientSideEncryption

This account object was really only used during the initialization phase
or for forgetting the sensitive data. So let's receive it as parameter
there and pass it on from job to job as needed.

Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>

diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp
index 5bb87b320dc618e0a3db7019a95852f6d73fa8b4..7ee5ed3acc57a510df83f8fd6e2604ddda582f58 100644 (file)
--- a/src/gui/accountmanager.cpp
+++ b/src/gui/accountmanager.cpp
@@ -378,7 +378,7 @@ void AccountManager::deleteAccount(AccountState *account)
     settings->remove(account->account()->id());
 
     // Forget E2E keys
-    account->account()->e2e()->forgetSensitiveData();
+    account->account()->e2e()->forgetSensitiveData(account->account());
 
     emit accountRemoved(account);
 }

diff --git a/src/gui/connectionvalidator.cpp b/src/gui/connectionvalidator.cpp
index 70617195997664a037512bbd737c347b0dec7c1e..f93b9862c049734e5fead2361004c8101e9dd656 100644 (file)
--- a/src/gui/connectionvalidator.cpp
+++ b/src/gui/connectionvalidator.cpp
@@ -316,7 +316,7 @@ void ConnectionValidator::slotUserFetched(UserInfo *userInfo)
 
 #ifndef TOKEN_AUTH_ONLY
     connect(_account->e2e(), &ClientSideEncryption::initializationFinished, this, &ConnectionValidator::reportConnected);
-    _account->e2e()->initialize();
+    _account->e2e()->initialize(_account);
 #else
     reportResult(Connected);
 #endif

diff --git a/src/libsync/account.cpp b/src/libsync/account.cpp
index df8e13ec0fd9c4c48ddffb268b06cf39205d27ea..3aa7e7d8d0864e01acf11179dc76d06b53b6c05f 100644 (file)
--- a/src/libsync/account.cpp
+++ b/src/libsync/account.cpp
@@ -64,11 +64,6 @@ AccountPtr Account::create()
 {
     AccountPtr acc = AccountPtr(new Account);
     acc->setSharedThis(acc);
-
-        //TODO: This probably needs to have a better
-        // coupling, but it should work for now.
-        acc->e2e()->setAccount(acc);
-
     return acc;
 }
 

diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp
index 92bd1a103c2106058318784ec4f2b26ea9d5934f..5029c960d070136bc9030fc6daa9653644bc0d13 100644 (file)
--- a/src/libsync/clientsideencryption.cpp
+++ b/src/libsync/clientsideencryption.cpp
@@ -58,6 +58,8 @@ QString baseUrl(){
 }
 
 namespace {
+    constexpr char accountProperty[] = "account";
+
     const char e2e_cert[] = "_e2e-certificate";
     const char e2e_private[] = "_e2e-private";
     const char e2e_mnemonic[] = "_e2e-mnemonic";
@@ -768,50 +770,52 @@ QByteArray encryptStringAsymmetric(EVP_PKEY *publicKey, const QByteArray& data)
 
 ClientSideEncryption::ClientSideEncryption() = default;
 
-void ClientSideEncryption::setAccount(AccountPtr account)
+void ClientSideEncryption::initialize(const AccountPtr &account)
 {
-    _account = account;
-}
+    Q_ASSERT(account);
 
-void ClientSideEncryption::initialize()
-{
     qCInfo(lcCse()) << "Initializing";
-    if (!_account->capabilities().clientSideEncryptionAvailable()) {
+    if (!account->capabilities().clientSideEncryptionAvailable()) {
         qCInfo(lcCse()) << "No Client side encryption available on server.";
         emit initializationFinished();
         return;
     }
 
-    fetchFromKeyChain();
+    fetchFromKeyChain(account);
 }
 
-void ClientSideEncryption::fetchFromKeyChain() {
+void ClientSideEncryption::fetchFromKeyChain(const AccountPtr &account)
+{
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_cert,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_cert,
+                account->id()
     );
 
     auto *job = new ReadPasswordJob(Theme::instance()->appName());
+    job->setProperty(accountProperty, QVariant::fromValue(account));
     job->setInsecureFallback(false);
     job->setKey(kck);
     connect(job, &ReadPasswordJob::finished, this, &ClientSideEncryption::publicKeyFetched);
     job->start();
 }
 
-void ClientSideEncryption::publicKeyFetched(Job *incoming) {
+void ClientSideEncryption::publicKeyFetched(Job *incoming)
+{
     auto *readJob = static_cast<ReadPasswordJob *>(incoming);
+    auto account = readJob->property(accountProperty).value<AccountPtr>();
+    Q_ASSERT(account);
 
     // Error or no valid public key error out
     if (readJob->error() != NoError || readJob->binaryData().length() == 0) {
-        getPublicKeyFromServer();
+        getPublicKeyFromServer(account);
         return;
     }
 
     _certificate = QSslCertificate(readJob->binaryData(), QSsl::Pem);
 
     if (_certificate.isNull()) {
-        getPublicKeyFromServer();
+        getPublicKeyFromServer(account);
         return;
     }
 
@@ -820,26 +824,30 @@ void ClientSideEncryption::publicKeyFetched(Job *incoming) {
     qCInfo(lcCse()) << "Public key fetched from keychain";
 
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_private,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_private,
+                account->id()
     );
 
     auto *job = new ReadPasswordJob(Theme::instance()->appName());
+    job->setProperty(accountProperty, QVariant::fromValue(account));
     job->setInsecureFallback(false);
     job->setKey(kck);
     connect(job, &ReadPasswordJob::finished, this, &ClientSideEncryption::privateKeyFetched);
     job->start();
 }
 
-void ClientSideEncryption::privateKeyFetched(Job *incoming) {
+void ClientSideEncryption::privateKeyFetched(Job *incoming)
+{
     auto *readJob = static_cast<ReadPasswordJob *>(incoming);
+    auto account = readJob->property(accountProperty).value<AccountPtr>();
+    Q_ASSERT(account);
 
     // Error or no valid public key error out
     if (readJob->error() != NoError || readJob->binaryData().length() == 0) {
         _certificate = QSslCertificate();
         _publicKey = QSslKey();
-        getPublicKeyFromServer();
+        getPublicKeyFromServer(account);
         return;
     }
 
@@ -847,34 +855,38 @@ void ClientSideEncryption::privateKeyFetched(Job *incoming) {
     _privateKey = readJob->binaryData();
 
     if (_privateKey.isNull()) {
-        getPrivateKeyFromServer();
+        getPrivateKeyFromServer(account);
         return;
     }
 
     qCInfo(lcCse()) << "Private key fetched from keychain";
 
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_mnemonic,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_mnemonic,
+                account->id()
     );
 
     auto *job = new ReadPasswordJob(Theme::instance()->appName());
+    job->setProperty(accountProperty, QVariant::fromValue(account));
     job->setInsecureFallback(false);
     job->setKey(kck);
     connect(job, &ReadPasswordJob::finished, this, &ClientSideEncryption::mnemonicKeyFetched);
     job->start();
 }
 
-void ClientSideEncryption::mnemonicKeyFetched(QKeychain::Job *incoming) {
+void ClientSideEncryption::mnemonicKeyFetched(QKeychain::Job *incoming)
+{
     auto *readJob = static_cast<ReadPasswordJob *>(incoming);
+    auto account = readJob->property(accountProperty).value<AccountPtr>();
+    Q_ASSERT(account);
 
     // Error or no valid public key error out
     if (readJob->error() != NoError || readJob->textData().length() == 0) {
         _certificate = QSslCertificate();
         _publicKey = QSslKey();
         _privateKey = QByteArray();
-        getPublicKeyFromServer();
+        getPublicKeyFromServer(account);
         return;
     }
 
@@ -885,11 +897,12 @@ void ClientSideEncryption::mnemonicKeyFetched(QKeychain::Job *incoming) {
     emit initializationFinished();
 }
 
-void ClientSideEncryption::writePrivateKey() {
+void ClientSideEncryption::writePrivateKey(const AccountPtr &account)
+{
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_private,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_private,
+                account->id()
     );
 
     auto *job = new WritePasswordJob(Theme::instance()->appName());
@@ -903,11 +916,12 @@ void ClientSideEncryption::writePrivateKey() {
     job->start();
 }
 
-void ClientSideEncryption::writeCertificate() {
+void ClientSideEncryption::writeCertificate(const AccountPtr &account)
+{
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_cert,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_cert,
+                account->id()
     );
 
     auto *job = new WritePasswordJob(Theme::instance()->appName());
@@ -921,45 +935,47 @@ void ClientSideEncryption::writeCertificate() {
     job->start();
 }
 
-void ClientSideEncryption::writeMnemonic() {
+void ClientSideEncryption::writeMnemonic(const AccountPtr &account)
+{
     const QString kck = AbstractCredentials::keychainKey(
-                _account->url().toString(),
-                _account->credentials()->user() + e2e_mnemonic,
-                _account->id()
+                account->url().toString(),
+                account->credentials()->user() + e2e_mnemonic,
+                account->id()
     );
 
     auto *job = new WritePasswordJob(Theme::instance()->appName());
     job->setInsecureFallback(false);
     job->setKey(kck);
     job->setTextData(_mnemonic);
-    connect(job, &WritePasswordJob::finished, [this](Job *incoming) {
+    connect(job, &WritePasswordJob::finished, [](Job *incoming) {
         Q_UNUSED(incoming);
         qCInfo(lcCse()) << "Mnemonic stored in keychain";
     });
     job->start();
 }
 
-void ClientSideEncryption::forgetSensitiveData()
+void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account)
 {
     _privateKey = QByteArray();
     _certificate = QSslCertificate();
     _publicKey = QSslKey();
     _mnemonic = QString();
 
-    auto startDeleteJob = [this](QString user) {
+    auto startDeleteJob = [account](QString user) {
         auto *job = new DeletePasswordJob(Theme::instance()->appName());
         job->setInsecureFallback(false);
-        job->setKey(AbstractCredentials::keychainKey(_account->url().toString(), user, _account->id()));
+        job->setKey(AbstractCredentials::keychainKey(account->url().toString(), user, account->id()));
         job->start();
     };
 
-    auto user = _account->credentials()->user();
+    auto user = account->credentials()->user();
     startDeleteJob(user + e2e_private);
     startDeleteJob(user + e2e_cert);
     startDeleteJob(user + e2e_mnemonic);
 }
 
-void ClientSideEncryption::slotRequestMnemonic() {
+void ClientSideEncryption::slotRequestMnemonic()
+{
     emit showMnemonic(_mnemonic);
 }
 
@@ -973,7 +989,7 @@ bool ClientSideEncryption::hasPublicKey() const
     return !_publicKey.isNull();
 }
 
-void ClientSideEncryption::generateKeyPair()
+void ClientSideEncryption::generateKeyPair(const AccountPtr &account)
 {
     // AES/GCM/NoPadding,
     // metadataKeys with RSA/ECB/OAEPWithSHA-256AndMGF1Padding
@@ -1013,13 +1029,13 @@ void ClientSideEncryption::generateKeyPair()
     _privateKey = key;
 
     qCInfo(lcCse()) << "Keys generated correctly, sending to server.";
-    generateCSR(localKeyPair);
+    generateCSR(account, localKeyPair);
 }
 
-void ClientSideEncryption::generateCSR(EVP_PKEY *keyPair)
+void ClientSideEncryption::generateCSR(const AccountPtr &account, EVP_PKEY *keyPair)
 {
     // OpenSSL expects const char.
-    auto cnArray = _account->davUser().toLocal8Bit();
+    auto cnArray = account->davUser().toLocal8Bit();
     qCInfo(lcCse()) << "Getting the following array for the account Id" << cnArray;
 
     auto certParams = std::map<const char *, const char*>{
@@ -1071,23 +1087,23 @@ void ClientSideEncryption::generateCSR(EVP_PKEY *keyPair)
     qCInfo(lcCse()) << "Returning the certificate";
     qCInfo(lcCse()) << output;
 
-    auto job = new SignPublicKeyApiJob(_account, baseUrl() + "public-key", this);
+    auto job = new SignPublicKeyApiJob(account, baseUrl() + "public-key", this);
     job->setCsr(output);
 
-    connect(job, &SignPublicKeyApiJob::jsonReceived, [this](const QJsonDocument& json, int retCode) {
+    connect(job, &SignPublicKeyApiJob::jsonReceived, [this, account](const QJsonDocument& json, int retCode) {
         if (retCode == 200) {
             QString cert = json.object().value("ocs").toObject().value("data").toObject().value("public-key").toString();
             _certificate = QSslCertificate(cert.toLocal8Bit(), QSsl::Pem);
             _publicKey = _certificate.publicKey();
             qCInfo(lcCse()) << "Certificate saved, Encrypting Private Key.";
-            encryptPrivateKey();
+            encryptPrivateKey(account);
         }
         qCInfo(lcCse()) << retCode;
     });
     job->start();
 }
 
-void ClientSideEncryption::encryptPrivateKey()
+void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account)
 {
     QStringList list = WordList::getRandomWords(12);
     _mnemonic = list.join(' ');
@@ -1104,16 +1120,16 @@ void ClientSideEncryption::encryptPrivateKey()
     auto cryptedText = EncryptionHelper::encryptPrivateKey(secretKey, EncryptionHelper::privateKeyToPem(_privateKey), salt);
 
     // Send private key to the server
-    auto job = new StorePrivateKeyApiJob(_account, baseUrl() + "private-key", this);
+    auto job = new StorePrivateKeyApiJob(account, baseUrl() + "private-key", this);
     job->setPrivateKey(cryptedText);
-    connect(job, &StorePrivateKeyApiJob::jsonReceived, [this](const QJsonDocument& doc, int retCode) {
+    connect(job, &StorePrivateKeyApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) {
         Q_UNUSED(doc);
         switch(retCode) {
             case 200:
                 qCInfo(lcCse()) << "Private key stored encrypted on server.";
-                writePrivateKey();
-                writeCertificate();
-                writeMnemonic();
+                writePrivateKey(account);
+                writeCertificate(account);
+                writeMnemonic(account);
                 emit initializationFinished();
                 break;
             default:
@@ -1128,13 +1144,13 @@ bool ClientSideEncryption::newMnemonicGenerated() const
     return _newMnemonicGenerated;
 }
 
-void ClientSideEncryption::decryptPrivateKey(const QByteArray &key) {
+void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QByteArray &key) {
     QString msg = tr("Please enter your end to end encryption passphrase:<br>"
                      "<br>"
                      "User: %2<br>"
                      "Account: %3<br>")
-                      .arg(Utility::escape(_account->credentials()->user()),
-                           Utility::escape(_account->displayName()));
+                      .arg(Utility::escape(account->credentials()->user()),
+                           Utility::escape(account->displayName()));
 
     QInputDialog dialog;
     dialog.setWindowTitle(tr("Enter E2E passphrase"));
@@ -1169,9 +1185,9 @@ void ClientSideEncryption::decryptPrivateKey(const QByteArray &key) {
             qCInfo(lcCse()) << "Private key: " << _privateKey;
 
             if (!_privateKey.isNull()) {
-                writePrivateKey();
-                writeCertificate();
-                writeMnemonic();
+                writePrivateKey(account);
+                writeCertificate(account);
+                writeMnemonic(account);
                 break;
             }
         } else {
@@ -1185,16 +1201,16 @@ void ClientSideEncryption::decryptPrivateKey(const QByteArray &key) {
     emit initializationFinished();
 }
 
-void ClientSideEncryption::getPrivateKeyFromServer()
+void ClientSideEncryption::getPrivateKeyFromServer(const AccountPtr &account)
 {
     qCInfo(lcCse()) << "Retrieving private key from server";
-    auto job = new JsonApiJob(_account, baseUrl() + "private-key", this);
-    connect(job, &JsonApiJob::jsonReceived, [this](const QJsonDocument& doc, int retCode) {
+    auto job = new JsonApiJob(account, baseUrl() + "private-key", this);
+    connect(job, &JsonApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) {
             if (retCode == 200) {
                 QString key = doc.object()["ocs"].toObject()["data"].toObject()["private-key"].toString();
                 qCInfo(lcCse()) << key;
                 qCInfo(lcCse()) << "Found private key, lets decrypt it!";
-                decryptPrivateKey(key.toLocal8Bit());
+                decryptPrivateKey(account, key.toLocal8Bit());
             } else if (retCode == 404) {
                 qCInfo(lcCse()) << "No private key on the server: setup is incomplete.";
             } else {
@@ -1204,21 +1220,21 @@ void ClientSideEncryption::getPrivateKeyFromServer()
     job->start();
 }
 
-void ClientSideEncryption::getPublicKeyFromServer()
+void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account)
 {
     qCInfo(lcCse()) << "Retrieving public key from server";
-    auto job = new JsonApiJob(_account, baseUrl() + "public-key", this);
-    connect(job, &JsonApiJob::jsonReceived, [this](const QJsonDocument& doc, int retCode) {
+    auto job = new JsonApiJob(account, baseUrl() + "public-key", this);
+    connect(job, &JsonApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) {
             if (retCode == 200) {
-                QString publicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-keys"].toObject()[_account->davUser()].toString();
+                QString publicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-keys"].toObject()[account->davUser()].toString();
                 _certificate = QSslCertificate(publicKey.toLocal8Bit(), QSsl::Pem);
                 _publicKey = _certificate.publicKey();
                 qCInfo(lcCse()) << publicKey;
                 qCInfo(lcCse()) << "Found Public key, requesting Private Key.";
-                getPrivateKeyFromServer();
+                getPrivateKeyFromServer(account);
             } else if (retCode == 404) {
                 qCInfo(lcCse()) << "No public key on the server";
-                generateKeyPair();
+                generateKeyPair(account);
             } else {
                 qCInfo(lcCse()) << "Error while requesting public key: " << retCode;
             }

diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h
index c32e5470701b64a0e56c66586cfa568df428eeb9..6dd8d0954528bbb91c83827b569599796997a08e 100644 (file)
--- a/src/libsync/clientsideencryption.h
+++ b/src/libsync/clientsideencryption.h
@@ -71,15 +71,17 @@ class OWNCLOUDSYNC_EXPORT ClientSideEncryption : public QObject {
     Q_OBJECT
 public:
     ClientSideEncryption();
-    void initialize();
-    void setAccount(AccountPtr account);
+    void initialize(const AccountPtr &account);
+
+private:
     bool hasPrivateKey() const;
     bool hasPublicKey() const;
-    void generateKeyPair();
-    void generateCSR(EVP_PKEY *keyPair);
-    void encryptPrivateKey();
+    void generateKeyPair(const AccountPtr &account);
+    void generateCSR(const AccountPtr &account, EVP_PKEY *keyPair);
+    void encryptPrivateKey(const AccountPtr &account);
 
-    void forgetSensitiveData();
+public:
+    void forgetSensitiveData(const AccountPtr &account);
 
     bool newMnemonicGenerated() const;
 
@@ -97,17 +99,16 @@ signals:
     void showMnemonic(const QString& mnemonic);
 
 private:
-    void getPrivateKeyFromServer();
-    void getPublicKeyFromServer();
-    void decryptPrivateKey(const QByteArray &key);
+    void getPrivateKeyFromServer(const AccountPtr &account);
+    void getPublicKeyFromServer(const AccountPtr &account);
+    void decryptPrivateKey(const AccountPtr &account, const QByteArray &key);
 
-    void fetchFromKeyChain();
+    void fetchFromKeyChain(const AccountPtr &account);
 
-    void writePrivateKey();
-    void writeCertificate();
-    void writeMnemonic();
+    void writePrivateKey(const AccountPtr &account);
+    void writeCertificate(const AccountPtr &account);
+    void writeMnemonic(const AccountPtr &account);
 
-    AccountPtr _account;
     bool isInitialized = false;
 
 public: