From: Kevin Ottens Date: Tue, 19 Jan 2021 14:09:38 +0000 (+0100) Subject: Avoid keeping Account alive via a shared ptr in ClientSideEncryption X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~21^2~399^2~1 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=4168c0d082de4ce7ea50bb75a086072ea87e2880;p=nextcloud-desktop.git 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 --- diff --git a/src/gui/accountmanager.cpp b/src/gui/accountmanager.cpp index 5bb87b320..7ee5ed3ac 100644 --- 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 706171959..f93b9862c 100644 --- 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 df8e13ec0..3aa7e7d8d 100644 --- 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 92bd1a103..5029c960d 100644 --- 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(incoming); + auto account = readJob->property(accountProperty).value(); + 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(incoming); + auto account = readJob->property(accountProperty).value(); + 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(incoming); + auto account = readJob->property(accountProperty).value(); + 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{ @@ -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:
" "
" "User: %2
" "Account: %3
") - .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 c32e54707..6dd8d0954 100644 --- 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: