From 15f3e2acaeb047e0b23782c32c89f9419bcab92d Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 7 Apr 2023 16:24:57 +0200 Subject: [PATCH 01/16] backup private key early Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 23 +++++++++++++++++++++++ src/libsync/clientsideencryption.h | 2 ++ 2 files changed, 25 insertions(+) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 5889ef94d3..f8c4243b0c 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1292,6 +1292,9 @@ void ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair) qCInfo(lcCse()) << "Returning the certificate"; qCInfo(lcCse()) << output; + writeMnemonic(account); + writeKeyPair(account); + sendSignRequestCSR(account, std::move(keyPair), output); } @@ -1326,6 +1329,26 @@ void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey ke job->start(); } +bool ClientSideEncryption::writeKeyPair(const AccountPtr &account, + const PKey &keyPair) +{ + const QString kck = AbstractCredentials::keychainKey( + account->url().toString(), + account->credentials()->user() + e2e_private, + account->id() + ); + + auto *job = new WritePasswordJob(Theme::instance()->appName()); + job->setInsecureFallback(false); + job->setKey(kck); + job->setBinaryData(_privateKey); + connect(job, &WritePasswordJob::finished, [](Job *incoming) { + Q_UNUSED(incoming); + qCInfo(lcCse()) << "Private key stored in keychain"; + }); + job->start(); +} + void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) { QStringList list = WordList::getRandomWords(12); diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 4d074c965d..a9138b13d8 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,6 +166,8 @@ private slots: private: void generateCSR(const AccountPtr &account, PKey keyPair); void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent); + [[nodiscard]] bool writeKeyPair(const AccountPtr &account, + const PKey &keyPair); [[nodiscard]] bool checkPublicKeyValidity(const AccountPtr &account) const; [[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const; From 1cb632234c8b532dc29dd3a7f85ee440ac1e775e Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Wed, 12 Apr 2023 17:28:14 +0200 Subject: [PATCH 02/16] e2ee init rework Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 55 ++++++++++++++++++++++------ src/libsync/clientsideencryption.h | 5 ++- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index f8c4243b0c..8057ba05be 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1287,15 +1287,19 @@ void ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair) Bio out; ret = PEM_write_bio_X509_REQ(out, x509_req); - QByteArray output = BIO2ByteArray(out); + if (ret <= 0){ + qCInfo(lcCse()) << "Error exporting the csr to the BIO"; + return; + } + + const auto output = BIO2ByteArray(out); qCInfo(lcCse()) << "Returning the certificate"; qCInfo(lcCse()) << output; - writeMnemonic(account); - writeKeyPair(account); - sendSignRequestCSR(account, std::move(keyPair), output); + writeMnemonic(account); + writeKeyPair(account, std::move(keyPair), output); } void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent) @@ -1329,8 +1333,9 @@ void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey ke job->start(); } -bool ClientSideEncryption::writeKeyPair(const AccountPtr &account, - const PKey &keyPair) +void ClientSideEncryption::writeKeyPair(const AccountPtr &account, + PKey keyPair, + const QByteArray &output) { const QString kck = AbstractCredentials::keychainKey( account->url().toString(), @@ -1338,15 +1343,41 @@ bool ClientSideEncryption::writeKeyPair(const AccountPtr &account, account->id() ); - auto *job = new WritePasswordJob(Theme::instance()->appName()); - job->setInsecureFallback(false); - job->setKey(kck); - job->setBinaryData(_privateKey); - connect(job, &WritePasswordJob::finished, [](Job *incoming) { + Bio privateKey; + if (PEM_write_bio_PrivateKey(privateKey, keyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { + qCInfo(lcCse()) << "Could not read private key from bio."; + return; + } + const auto bytearrayPrivateKey = BIO2ByteArray(privateKey); + + auto *privateKeyJob = new WritePasswordJob(Theme::instance()->appName()); + privateKeyJob->setInsecureFallback(false); + privateKeyJob->setKey(kck); + privateKeyJob->setBinaryData(bytearrayPrivateKey); + connect(privateKeyJob, &WritePasswordJob::finished, [&keyPair, kck, &account, &output, this](Job *incoming) { Q_UNUSED(incoming); qCInfo(lcCse()) << "Private key stored in keychain"; + + Bio publicKey; + if (PEM_write_bio_PUBKEY(publicKey, keyPair) <= 0) { + qCInfo(lcCse()) << "Could not read public key from bio."; + return; + } + const auto bytearrayPublicKey = BIO2ByteArray(publicKey); + + auto *publicKeyJob = new WritePasswordJob(Theme::instance()->appName()); + publicKeyJob->setInsecureFallback(false); + publicKeyJob->setKey(kck); + publicKeyJob->setBinaryData(bytearrayPublicKey); + connect(publicKeyJob, &WritePasswordJob::finished, [&account, &keyPair, &output, this](Job *incoming) { + Q_UNUSED(incoming); + qCInfo(lcCse()) << "Public key stored in keychain"; + + sendSignRequestCSR(account, std::move(keyPair), output); + }); + publicKeyJob->start(); }); - job->start(); + privateKeyJob->start(); } void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index a9138b13d8..ed1b560a70 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,8 +166,9 @@ private slots: private: void generateCSR(const AccountPtr &account, PKey keyPair); void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent); - [[nodiscard]] bool writeKeyPair(const AccountPtr &account, - const PKey &keyPair); + void writeKeyPair(const AccountPtr &account, + PKey keyPair, + const QByteArray &output); [[nodiscard]] bool checkPublicKeyValidity(const AccountPtr &account) const; [[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const; From 1b7c16e8ff030238913e83895f75f64183e79e69 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 13 Apr 2023 11:44:57 +0200 Subject: [PATCH 03/16] solve memory mismanagement of object life time Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 27 +++++++++++++++++---------- src/libsync/clientsideencryption.h | 4 ++-- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 8057ba05be..416d1c4114 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -63,9 +63,10 @@ QString e2eeBaseUrl() namespace { constexpr char accountProperty[] = "account"; -const char e2e_cert[] = "_e2e-certificate"; -const char e2e_private[] = "_e2e-private"; -const char e2e_mnemonic[] = "_e2e-mnemonic"; +constexpr char e2e_cert[] = "_e2e-certificate"; +constexpr char e2e_private[] = "_e2e-private"; +constexpr char e2e_public[] = "_e2e-public"; +constexpr char e2e_mnemonic[] = "_e2e-mnemonic"; constexpr auto metadataKeyJsonKey = "metadataKey"; @@ -1333,16 +1334,22 @@ void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey ke job->start(); } -void ClientSideEncryption::writeKeyPair(const AccountPtr &account, +void ClientSideEncryption::writeKeyPair(AccountPtr account, PKey keyPair, - const QByteArray &output) + QByteArray output) { - const QString kck = AbstractCredentials::keychainKey( + const auto privateKeyKeychainId = AbstractCredentials::keychainKey( account->url().toString(), account->credentials()->user() + e2e_private, account->id() ); + const auto publicKeyKeychainId = AbstractCredentials::keychainKey( + account->url().toString(), + account->credentials()->user() + e2e_public, + account->id() + ); + Bio privateKey; if (PEM_write_bio_PrivateKey(privateKey, keyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { qCInfo(lcCse()) << "Could not read private key from bio."; @@ -1352,9 +1359,9 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, auto *privateKeyJob = new WritePasswordJob(Theme::instance()->appName()); privateKeyJob->setInsecureFallback(false); - privateKeyJob->setKey(kck); + privateKeyJob->setKey(privateKeyKeychainId); privateKeyJob->setBinaryData(bytearrayPrivateKey); - connect(privateKeyJob, &WritePasswordJob::finished, [&keyPair, kck, &account, &output, this](Job *incoming) { + connect(privateKeyJob, &WritePasswordJob::finished, [keyPair = std::move(keyPair), publicKeyKeychainId, account, output, this] (Job *incoming) mutable { Q_UNUSED(incoming); qCInfo(lcCse()) << "Private key stored in keychain"; @@ -1367,9 +1374,9 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, auto *publicKeyJob = new WritePasswordJob(Theme::instance()->appName()); publicKeyJob->setInsecureFallback(false); - publicKeyJob->setKey(kck); + publicKeyJob->setKey(publicKeyKeychainId); publicKeyJob->setBinaryData(bytearrayPublicKey); - connect(publicKeyJob, &WritePasswordJob::finished, [&account, &keyPair, &output, this](Job *incoming) { + connect(publicKeyJob, &WritePasswordJob::finished, [account, keyPair = std::move(keyPair), output, this](Job *incoming) mutable { Q_UNUSED(incoming); qCInfo(lcCse()) << "Public key stored in keychain"; diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index ed1b560a70..03720697fc 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,9 +166,9 @@ private slots: private: void generateCSR(const AccountPtr &account, PKey keyPair); void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent); - void writeKeyPair(const AccountPtr &account, + void writeKeyPair(AccountPtr account, PKey keyPair, - const QByteArray &output); + QByteArray output); [[nodiscard]] bool checkPublicKeyValidity(const AccountPtr &account) const; [[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const; From 17484cd69f543e6ce35701693a1f76c9670ae17a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 13 Apr 2023 16:39:08 +0200 Subject: [PATCH 04/16] finish local save of keys and upload them to server Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 159 +++++++++++++++++++++++---- src/libsync/clientsideencryption.h | 35 +++++- 2 files changed, 171 insertions(+), 23 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 416d1c4114..9264cc46e9 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -882,10 +882,10 @@ void ClientSideEncryption::initialize(const AccountPtr &account) return; } - fetchFromKeyChain(account); + fetchCertificateFromKeyChain(account); } -void ClientSideEncryption::fetchFromKeyChain(const AccountPtr &account) +void ClientSideEncryption::fetchCertificateFromKeyChain(const AccountPtr &account) { const QString kck = AbstractCredentials::keychainKey( account->url().toString(), @@ -893,6 +893,22 @@ void ClientSideEncryption::fetchFromKeyChain(const AccountPtr &account) 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::publicCertificateFetched); + job->start(); +} + +void ClientSideEncryption::fetchPublicKeyFromKeyChain(const AccountPtr &account) +{ + const QString kck = AbstractCredentials::keychainKey( + account->url().toString(), + account->credentials()->user() + e2e_public, + account->id() + ); + auto *job = new ReadPasswordJob(Theme::instance()->appName()); job->setProperty(accountProperty, QVariant::fromValue(account)); job->setInsecureFallback(false); @@ -951,7 +967,7 @@ bool ClientSideEncryption::checkServerPublicKeyValidity(const QByteArray &server return true; } -void ClientSideEncryption::publicKeyFetched(Job *incoming) +void ClientSideEncryption::publicCertificateFetched(Job *incoming) { auto *readJob = dynamic_cast(incoming); auto account = readJob->property(accountProperty).value(); @@ -959,14 +975,14 @@ void ClientSideEncryption::publicKeyFetched(Job *incoming) // Error or no valid public key error out if (readJob->error() != NoError || readJob->binaryData().length() == 0) { - getPublicKeyFromServer(account); + fetchPublicKeyFromKeyChain(account); return; } _certificate = QSslCertificate(readJob->binaryData(), QSsl::Pem); if (_certificate.isNull()) { - getPublicKeyFromServer(account); + fetchPublicKeyFromKeyChain(account); return; } @@ -988,6 +1004,43 @@ void ClientSideEncryption::publicKeyFetched(Job *incoming) job->start(); } +void ClientSideEncryption::publicKeyFetched(QKeychain::Job *incoming) +{ + auto *readJob = dynamic_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(account); + return; + } + + const auto publicKey = QSslKey(readJob->binaryData(), QSsl::Rsa, QSsl::Pem, QSsl::PublicKey); + + if (publicKey.isNull()) { + getPublicKeyFromServer(account); + return; + } + + _publicKey = publicKey; + + qCInfo(lcCse()) << "Public key fetched from keychain"; + + const QString kck = AbstractCredentials::keychainKey( + 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) { auto *readJob = dynamic_cast(incoming); @@ -1045,7 +1098,7 @@ void ClientSideEncryption::mnemonicKeyFetched(QKeychain::Job *incoming) qCInfo(lcCse()) << "Mnemonic key fetched from keychain: " << _mnemonic; - emit initializationFinished(); + checkServerHasSavedKeys(account); } void ClientSideEncryption::writePrivateKey(const AccountPtr &account) @@ -1086,7 +1139,16 @@ void ClientSideEncryption::writeCertificate(const AccountPtr &account) job->start(); } -void ClientSideEncryption::writeMnemonic(const AccountPtr &account) +void ClientSideEncryption::generateMnemonic() +{ + QStringList list = WordList::getRandomWords(12); + _mnemonic = list.join(' '); + qCInfo(lcCse()) << "mnemonic Generated:" << _mnemonic; +} + +template +void ClientSideEncryption::writeMnemonic(OCC::AccountPtr account, + L nextCall) { const QString kck = AbstractCredentials::keychainKey( account->url().toString(), @@ -1098,9 +1160,11 @@ void ClientSideEncryption::writeMnemonic(const AccountPtr &account) job->setInsecureFallback(false); job->setKey(kck); job->setTextData(_mnemonic); - connect(job, &WritePasswordJob::finished, [](Job *incoming) { + connect(job, &WritePasswordJob::finished, [account, nextCall = std::move(nextCall)](Job *incoming) mutable { Q_UNUSED(incoming); qCInfo(lcCse()) << "Mnemonic stored in keychain"; + + nextCall(); }); job->start(); } @@ -1238,7 +1302,7 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) generateCSR(account, std::move(localKeyPair)); } -void ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair) +void ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) { // OpenSSL expects const char. auto cnArray = account->davUser().toLocal8Bit(); @@ -1298,12 +1362,16 @@ void ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair) qCInfo(lcCse()) << "Returning the certificate"; qCInfo(lcCse()) << output; + if (_mnemonic.isEmpty()) { + generateMnemonic(); + } - writeMnemonic(account); - writeKeyPair(account, std::move(keyPair), output); + writeMnemonic(account, [account, keyPair = std::move(keyPair), output, this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); } -void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent) +void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, + PKey keyPair, + QByteArray csrContent) { auto job = new SignPublicKeyApiJob(account, e2eeBaseUrl() + "public-key", this); job->setCsr(csrContent); @@ -1387,13 +1455,67 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, privateKeyJob->start(); } +void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) +{ + const auto keyIsNotOnServer = [account, this] () { + qCInfo(lcCse) << "server is missing keys. upload is necessary"; + + generateCSR(account, keyPair) + sendSignRequestCSR(account, {}, {}); + }; + + const auto privateKeyOnServerIsValid = [this] () { + Q_EMIT initializationFinished(); + }; + + const auto publicKeyOnServerIsValid = [this, account, privateKeyOnServerIsValid, keyIsNotOnServer] () { + checkUserPrivateKeyOnServer(account, privateKeyOnServerIsValid, keyIsNotOnServer); + }; + + checkUserPublicKeyOnServer(account, publicKeyOnServerIsValid, keyIsNotOnServer); +} + +template +void ClientSideEncryption::checkUserKeyOnServer(const QString &keyType, + OCC::AccountPtr account, + SUCCESS_CALLBACK nextCheck, + ERROR_CALLBACK onError) +{ + qCInfo(lcCse()) << "Retrieving" << keyType << "from server"; + auto job = new JsonApiJob(account, e2eeBaseUrl() + keyType, this); + connect(job, &JsonApiJob::jsonReceived, [nextCheck, onError](const QJsonDocument& doc, int retCode) { + Q_UNUSED(doc) + + if (retCode == 200) { + nextCheck(); + } else { + onError(); + } + }); + job->start(); +} + +template +void ClientSideEncryption::checkUserPublicKeyOnServer(AccountPtr account, + SUCCESS_CALLBACK nextCheck, + ERROR_CALLBACK onError) +{ + checkUserKeyOnServer("public-key", account, nextCheck, onError); +} + +template +void ClientSideEncryption::checkUserPrivateKeyOnServer(AccountPtr account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) +{ + checkUserKeyOnServer("private-key", account, nextCheck, onError); +} + void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) { - QStringList list = WordList::getRandomWords(12); - _mnemonic = list.join(' '); - qCInfo(lcCse()) << "mnemonic Generated:" << _mnemonic; + if (_mnemonic.isEmpty()) { + generateMnemonic(); + } - QString passPhrase = list.join(QString()).toLower(); + const auto passPhrase = _mnemonic.remove(' ').toLower(); qCInfo(lcCse()) << "Passphrase Generated:" << passPhrase; auto salt = EncryptionHelper::generateRandom(40); @@ -1410,8 +1532,7 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) qCInfo(lcCse()) << "Private key stored encrypted on server."; writePrivateKey(account); writeCertificate(account); - writeMnemonic(account); - emit initializationFinished(true); + writeMnemonic(account, [this] () {emit initializationFinished(true);}); break; default: qCInfo(lcCse()) << "Store private key failed, return code:" << retCode; @@ -1469,7 +1590,7 @@ void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QB if (!_privateKey.isNull() && checkPublicKeyValidity(account)) { writePrivateKey(account); writeCertificate(account); - writeMnemonic(account); + writeMnemonic(account, [] () {}); break; } } else { diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 03720697fc..fe95d89121 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -144,6 +144,7 @@ private slots: void generateKeyPair(const OCC::AccountPtr &account); void encryptPrivateKey(const OCC::AccountPtr &account); + void publicCertificateFetched(QKeychain::Job *incoming); void publicKeyFetched(QKeychain::Job *incoming); void privateKeyFetched(QKeychain::Job *incoming); void mnemonicKeyFetched(QKeychain::Job *incoming); @@ -158,18 +159,44 @@ private slots: void fetchAndValidatePublicKeyFromServer(const OCC::AccountPtr &account); void decryptPrivateKey(const OCC::AccountPtr &account, const QByteArray &key); - void fetchFromKeyChain(const OCC::AccountPtr &account); + void fetchCertificateFromKeyChain(const OCC::AccountPtr &account); + void fetchPublicKeyFromKeyChain(const OCC::AccountPtr &account); void writePrivateKey(const OCC::AccountPtr &account); void writeCertificate(const OCC::AccountPtr &account); - void writeMnemonic(const OCC::AccountPtr &account); private: - void generateCSR(const AccountPtr &account, PKey keyPair); - void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, const QByteArray &csrContent); + void generateMnemonic(); + void generateCSR(AccountPtr account, + PKey keyPair); + void sendSignRequestCSR(AccountPtr account, + PKey keyPair, + QByteArray csrContent); void writeKeyPair(AccountPtr account, PKey keyPair, QByteArray output); + template + void writeMnemonic(OCC::AccountPtr account, + L nextCall); + + void checkServerHasSavedKeys(AccountPtr account); + + template + void checkUserPublicKeyOnServer(OCC::AccountPtr account, + SUCCESS_CALLBACK nextCheck, + ERROR_CALLBACK onError); + + template + void checkUserPrivateKeyOnServer(OCC::AccountPtr account, + SUCCESS_CALLBACK nextCheck, + ERROR_CALLBACK onError); + + template + void checkUserKeyOnServer(const QString &keyType, + OCC::AccountPtr account, + SUCCESS_CALLBACK nextCheck, + ERROR_CALLBACK onError); + [[nodiscard]] bool checkPublicKeyValidity(const AccountPtr &account) const; [[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const; [[nodiscard]] bool sensitiveDataRemaining() const; From d3b583d967d2127ece48bbc3d4b158525f827ddd Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Mon, 17 Apr 2023 18:04:31 +0200 Subject: [PATCH 05/16] improvement to be able to retry the e2ee init steps Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 26 ++++++++++++++++---------- src/libsync/clientsideencryption.h | 6 ++++-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 9264cc46e9..d680e320fd 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1300,10 +1300,13 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) qCInfo(lcCse()) << "Keys generated correctly, sending to server."; generateCSR(account, std::move(localKeyPair)); + writeMnemonic(account, [account, keyPair = std::move(keyPair), output, this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); } -void ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) +QByteArray ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) { + auto result = QByteArray{}; + // OpenSSL expects const char. auto cnArray = account->davUser().toLocal8Bit(); qCInfo(lcCse()) << "Getting the following array for the account Id" << cnArray; @@ -1334,39 +1337,39 @@ void ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) ret = X509_NAME_add_entry_by_txt(x509_name, v.first, MBSTRING_ASC, (const unsigned char*) v.second, -1, -1, 0); if (ret != 1) { qCInfo(lcCse()) << "Error Generating the Certificate while adding" << v.first << v.second; - return; + return result; } } ret = X509_REQ_set_pubkey(x509_req, keyPair); if (ret != 1){ qCInfo(lcCse()) << "Error setting the public key on the csr"; - return; + return result; } ret = X509_REQ_sign(x509_req, keyPair, EVP_sha1()); // return x509_req->signature->length if (ret <= 0){ qCInfo(lcCse()) << "Error setting the public key on the csr"; - return; + return result; } Bio out; ret = PEM_write_bio_X509_REQ(out, x509_req); if (ret <= 0){ qCInfo(lcCse()) << "Error exporting the csr to the BIO"; - return; + return result; } - const auto output = BIO2ByteArray(out); + result = BIO2ByteArray(out); qCInfo(lcCse()) << "Returning the certificate"; - qCInfo(lcCse()) << output; + qCInfo(lcCse()) << result; if (_mnemonic.isEmpty()) { generateMnemonic(); } - writeMnemonic(account, [account, keyPair = std::move(keyPair), output, this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); + return result; } void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, @@ -1460,8 +1463,11 @@ void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) const auto keyIsNotOnServer = [account, this] () { qCInfo(lcCse) << "server is missing keys. upload is necessary"; - generateCSR(account, keyPair) - sendSignRequestCSR(account, {}, {}); + Bio privateKeyBio; + auto keyPair = PKey::readPublicKey(privateKeyBio); + auto csrData = generateCSR(account, std::move(keyPair)); + auto keyPair2 = PKey::readPublicKey(privateKeyBio); + sendSignRequestCSR(account, std::move(keyPair2), csrData); }; const auto privateKeyOnServerIsValid = [this] () { diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index fe95d89121..1a4d75e1f2 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,11 +166,13 @@ private slots: private: void generateMnemonic(); - void generateCSR(AccountPtr account, - PKey keyPair); + [[nodiscard]] QByteArray generateCSR(AccountPtr account, + PKey keyPair); + void sendSignRequestCSR(AccountPtr account, PKey keyPair, QByteArray csrContent); + void writeKeyPair(AccountPtr account, PKey keyPair, QByteArray output); From 50234c5859038b5342386e260b8fc0b8ec68c3b2 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 20 Apr 2023 18:54:36 +0200 Subject: [PATCH 06/16] try to resend the public key if it is not on server but on local storage Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 20 ++++++++++---------- src/libsync/clientsideencryption.h | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index d680e320fd..3b7211e9bd 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1299,11 +1299,12 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) _privateKey = key; qCInfo(lcCse()) << "Keys generated correctly, sending to server."; - generateCSR(account, std::move(localKeyPair)); - writeMnemonic(account, [account, keyPair = std::move(keyPair), output, this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); + auto csrOutput = generateCSR(account, std::move(localKeyPair)); + writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); } -QByteArray ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) +std::pair ClientSideEncryption::generateCSR(AccountPtr account, + PKey keyPair) { auto result = QByteArray{}; @@ -1337,27 +1338,27 @@ QByteArray ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) ret = X509_NAME_add_entry_by_txt(x509_name, v.first, MBSTRING_ASC, (const unsigned char*) v.second, -1, -1, 0); if (ret != 1) { qCInfo(lcCse()) << "Error Generating the Certificate while adding" << v.first << v.second; - return result; + return {result, std::move(keyPair)}; } } ret = X509_REQ_set_pubkey(x509_req, keyPair); if (ret != 1){ qCInfo(lcCse()) << "Error setting the public key on the csr"; - return result; + return {result, std::move(keyPair)}; } ret = X509_REQ_sign(x509_req, keyPair, EVP_sha1()); // return x509_req->signature->length if (ret <= 0){ qCInfo(lcCse()) << "Error setting the public key on the csr"; - return result; + return {result, std::move(keyPair)}; } Bio out; ret = PEM_write_bio_X509_REQ(out, x509_req); if (ret <= 0){ qCInfo(lcCse()) << "Error exporting the csr to the BIO"; - return result; + return {result, std::move(keyPair)}; } result = BIO2ByteArray(out); @@ -1369,7 +1370,7 @@ QByteArray ClientSideEncryption::generateCSR(AccountPtr account, PKey keyPair) generateMnemonic(); } - return result; + return {result, std::move(keyPair)}; } void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, @@ -1466,8 +1467,7 @@ void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) Bio privateKeyBio; auto keyPair = PKey::readPublicKey(privateKeyBio); auto csrData = generateCSR(account, std::move(keyPair)); - auto keyPair2 = PKey::readPublicKey(privateKeyBio); - sendSignRequestCSR(account, std::move(keyPair2), csrData); + sendSignRequestCSR(account, std::move(csrData.second), std::move(csrData.first)); }; const auto privateKeyOnServerIsValid = [this] () { diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 1a4d75e1f2..5b61a9035f 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,8 +166,8 @@ private slots: private: void generateMnemonic(); - [[nodiscard]] QByteArray generateCSR(AccountPtr account, - PKey keyPair); + [[nodiscard]] std::pair generateCSR(AccountPtr account, + PKey keyPair); void sendSignRequestCSR(AccountPtr account, PKey keyPair, From 3a0e0f2097f2a9e91a1ab7746c25e272568d7f17 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 20 Apr 2023 21:28:11 +0200 Subject: [PATCH 07/16] can now generate the CSR again if the first try to upload failed Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 23 ++++++++++++++++------- src/libsync/clientsideencryption.h | 4 +++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 3b7211e9bd..e1d782a27e 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1294,17 +1294,19 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) qCInfo(lcCse()) << "Could not read private key from bio."; return; } - QByteArray key = BIO2ByteArray(privKey); + auto privateKey = PKey::readPrivateKey(privKey); + const auto key = BIO2ByteArray(privKey); //_privateKey = QSslKey(key, QSsl::Rsa, QSsl::Pem, QSsl::PrivateKey); _privateKey = key; qCInfo(lcCse()) << "Keys generated correctly, sending to server."; - auto csrOutput = generateCSR(account, std::move(localKeyPair)); + auto csrOutput = generateCSR(account, std::move(localKeyPair), std::move(privateKey)); writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); } std::pair ClientSideEncryption::generateCSR(AccountPtr account, - PKey keyPair) + PKey keyPair, + PKey privateKey) { auto result = QByteArray{}; @@ -1348,9 +1350,9 @@ std::pair ClientSideEncryption::generate return {result, std::move(keyPair)}; } - ret = X509_REQ_sign(x509_req, keyPair, EVP_sha1()); // return x509_req->signature->length + ret = X509_REQ_sign(x509_req, privateKey, EVP_sha1()); // return x509_req->signature->length if (ret <= 0){ - qCInfo(lcCse()) << "Error setting the public key on the csr"; + qCInfo(lcCse()) << "Error signing the csr with the private key"; return {result, std::move(keyPair)}; } @@ -1464,9 +1466,16 @@ void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) const auto keyIsNotOnServer = [account, this] () { qCInfo(lcCse) << "server is missing keys. upload is necessary"; + Bio publicKeyBio; + const auto publicKeyData = _publicKey.toPem(); + BIO_write(publicKeyBio, publicKeyData.constData(), publicKeyData.size()); + auto publicKey = PKey::readPublicKey(publicKeyBio); + Bio privateKeyBio; - auto keyPair = PKey::readPublicKey(privateKeyBio); - auto csrData = generateCSR(account, std::move(keyPair)); + BIO_write(privateKeyBio, _privateKey.constData(), _privateKey.size()); + auto privateKey = PKey::readPrivateKey(privateKeyBio); + + auto csrData = generateCSR(account, std::move(publicKey), std::move(privateKey)); sendSignRequestCSR(account, std::move(csrData.second), std::move(csrData.first)); }; diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 5b61a9035f..dffeee3ddd 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -166,8 +166,10 @@ private slots: private: void generateMnemonic(); + [[nodiscard]] std::pair generateCSR(AccountPtr account, - PKey keyPair); + PKey keyPair, + PKey privateKey); void sendSignRequestCSR(AccountPtr account, PKey keyPair, From c8e5ac7b5fc8231b9e9e685a0b4b52dc83cf721e Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 25 Apr 2023 17:19:06 +0200 Subject: [PATCH 08/16] fix upload of private key when initializing the end-to-end encryption Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index e1d782a27e..2dcce06ce5 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -146,6 +146,11 @@ public: BIO_free_all(_bio); } + operator const BIO*() const + { + return _bio; + } + operator BIO*() { return _bio; @@ -1289,15 +1294,24 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) qCInfo(lcCse()) << "Key correctly generated"; qCInfo(lcCse()) << "Storing keys locally"; + { + Bio privKey; + if (PEM_write_bio_PrivateKey(privKey, localKeyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { + qCInfo(lcCse()) << "Could not read private key from bio."; + return; + } + + const auto key = BIO2ByteArray(privKey); + _privateKey = key; qCDebug(lcCse) << _privateKey; + } + Bio privKey; if (PEM_write_bio_PrivateKey(privKey, localKeyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { qCInfo(lcCse()) << "Could not read private key from bio."; return; } + auto privateKey = PKey::readPrivateKey(privKey); - const auto key = BIO2ByteArray(privKey); - //_privateKey = QSslKey(key, QSsl::Rsa, QSsl::Pem, QSsl::PrivateKey); - _privateKey = key; qCInfo(lcCse()) << "Keys generated correctly, sending to server."; auto csrOutput = generateCSR(account, std::move(localKeyPair), std::move(privateKey)); @@ -1530,7 +1544,8 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) generateMnemonic(); } - const auto passPhrase = _mnemonic.remove(' ').toLower(); + auto passPhrase = _mnemonic; + passPhrase = passPhrase.remove(' ').toLower(); qCInfo(lcCse()) << "Passphrase Generated:" << passPhrase; auto salt = EncryptionHelper::generateRandom(40); From bef5d1a893b92724e3f724190dfe98c3089eff73 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 9 May 2023 16:10:04 +0200 Subject: [PATCH 09/16] if keys are not on server, also delete local keys Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 30 +++++++++++++++++----------- src/libsync/clientsideencryption.h | 2 ++ 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 2dcce06ce5..1367a99841 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1194,10 +1194,12 @@ void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account) const auto deletePrivateKeyJob = createDeleteJob(user + e2e_private); const auto deleteCertJob = createDeleteJob(user + e2e_cert); const auto deleteMnemonicJob = createDeleteJob(user + e2e_mnemonic); + const auto deletePublicKeyJob = createDeleteJob(user + e2e_public); connect(deletePrivateKeyJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handlePrivateKeyDeleted); connect(deleteCertJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handleCertificateDeleted); connect(deleteMnemonicJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handleMnemonicDeleted); + connect(deletePublicKeyJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handlePublicKeyDeleted); deletePrivateKeyJob->start(); deleteCertJob->start(); deleteMnemonicJob->start(); @@ -1245,6 +1247,20 @@ void ClientSideEncryption::handleMnemonicDeleted(const QKeychain::Job* const inc checkAllSensitiveDataDeleted(); } +void ClientSideEncryption::handlePublicKeyDeleted(const QKeychain::Job * const incoming) +{ + const auto error = incoming->error(); + if (error != QKeychain::NoError && error != QKeychain::EntryNotFound) { + qCWarning(lcCse) << "Public key could not be deleted:" << incoming->errorString(); + return; + } + + qCDebug(lcCse) << "Public key successfully deleted from keychain. Clearing."; + _publicKey = QByteArray(); + Q_EMIT publicKeyDeleted(); + checkAllSensitiveDataDeleted(); +} + bool ClientSideEncryption::sensitiveDataRemaining() const { return !_privateKey.isEmpty() || !_certificate.isNull() || !_mnemonic.isEmpty(); @@ -1478,19 +1494,9 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) { const auto keyIsNotOnServer = [account, this] () { - qCInfo(lcCse) << "server is missing keys. upload is necessary"; + qCInfo(lcCse) << "server is missing keys. deleting local keys"; - Bio publicKeyBio; - const auto publicKeyData = _publicKey.toPem(); - BIO_write(publicKeyBio, publicKeyData.constData(), publicKeyData.size()); - auto publicKey = PKey::readPublicKey(publicKeyBio); - - Bio privateKeyBio; - BIO_write(privateKeyBio, _privateKey.constData(), _privateKey.size()); - auto privateKey = PKey::readPrivateKey(privateKeyBio); - - auto csrData = generateCSR(account, std::move(publicKey), std::move(privateKey)); - sendSignRequestCSR(account, std::move(csrData.second), std::move(csrData.first)); + forgetSensitiveData(account); }; const auto privateKeyOnServerIsValid = [this] () { diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index dffeee3ddd..1c3ba01417 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -135,6 +135,7 @@ signals: void privateKeyDeleted(); void certificateDeleted(); void mnemonicDeleted(); + void publicKeyDeleted(); public slots: void initialize(const OCC::AccountPtr &account); @@ -152,6 +153,7 @@ private slots: void handlePrivateKeyDeleted(const QKeychain::Job* const incoming); void handleCertificateDeleted(const QKeychain::Job* const incoming); void handleMnemonicDeleted(const QKeychain::Job* const incoming); + void handlePublicKeyDeleted(const QKeychain::Job* const incoming); void checkAllSensitiveDataDeleted(); void getPrivateKeyFromServer(const OCC::AccountPtr &account); From ad34de16226e1f4bf6a9d30b628964f11789465a Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 9 May 2023 17:20:03 +0200 Subject: [PATCH 10/16] make sure e2ee init is either fully done or not at all make sure that we have only two cases: 1) keys are stored on the server and the client 2) keys are stored on the server and not yet on the client Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 69 +++++++++++++++++++++------- src/libsync/clientsideencryption.h | 2 + 2 files changed, 55 insertions(+), 16 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 1367a99841..db398191e8 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1165,8 +1165,12 @@ void ClientSideEncryption::writeMnemonic(OCC::AccountPtr account, job->setInsecureFallback(false); job->setKey(kck); job->setTextData(_mnemonic); - connect(job, &WritePasswordJob::finished, [account, nextCall = std::move(nextCall)](Job *incoming) mutable { - Q_UNUSED(incoming); + connect(job, &WritePasswordJob::finished, [this, account, nextCall = std::move(nextCall)](Job *incoming) mutable { + if (incoming->error() != Error::NoError) { + failedToInitialize(account); + return; + } + qCInfo(lcCse()) << "Mnemonic stored in keychain"; nextCall(); @@ -1256,7 +1260,7 @@ void ClientSideEncryption::handlePublicKeyDeleted(const QKeychain::Job * const i } qCDebug(lcCse) << "Public key successfully deleted from keychain. Clearing."; - _publicKey = QByteArray(); + _publicKey.clear(); Q_EMIT publicKeyDeleted(); checkAllSensitiveDataDeleted(); } @@ -1266,6 +1270,12 @@ bool ClientSideEncryption::sensitiveDataRemaining() const return !_privateKey.isEmpty() || !_certificate.isNull() || !_mnemonic.isEmpty(); } +void ClientSideEncryption::failedToInitialize(const AccountPtr &account) +{ + forgetSensitiveData(account); + Q_EMIT initializationFinished(); +} + void ClientSideEncryption::checkAllSensitiveDataDeleted() { if (sensitiveDataRemaining()) { @@ -1293,17 +1303,20 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) if(EVP_PKEY_keygen_init(ctx) <= 0) { qCInfo(lcCse()) << "Couldn't initialize the key generator"; + failedToInitialize(account); return; } if(EVP_PKEY_CTX_set_rsa_keygen_bits(ctx, rsaKeyLen) <= 0) { qCInfo(lcCse()) << "Couldn't initialize the key generator bits"; + failedToInitialize(account); return; } auto localKeyPair = PKey::generate(ctx); if(!localKeyPair) { qCInfo(lcCse()) << "Could not generate the key"; + failedToInitialize(account); return; } @@ -1314,16 +1327,19 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) Bio privKey; if (PEM_write_bio_PrivateKey(privKey, localKeyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { qCInfo(lcCse()) << "Could not read private key from bio."; + failedToInitialize(account); return; } const auto key = BIO2ByteArray(privKey); - _privateKey = key; qCDebug(lcCse) << _privateKey; + _privateKey = key; + qCDebug(lcCse) << _privateKey; } Bio privKey; if (PEM_write_bio_PrivateKey(privKey, localKeyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { qCInfo(lcCse()) << "Could not read private key from bio."; + failedToInitialize(account); return; } @@ -1331,7 +1347,9 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) qCInfo(lcCse()) << "Keys generated correctly, sending to server."; auto csrOutput = generateCSR(account, std::move(localKeyPair), std::move(privateKey)); - writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void {writeKeyPair(account, std::move(keyPair), output);}); + writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void { + writeKeyPair(account, std::move(keyPair), output); + }); } std::pair ClientSideEncryption::generateCSR(AccountPtr account, @@ -1427,13 +1445,16 @@ void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, qCInfo(lcCse()) << ERR_lib_error_string(lastError); lastError = ERR_get_error(); } - forgetSensitiveData(account); + failedToInitialize(account); return; } qCInfo(lcCse()) << "received a valid certificate"; fetchAndValidatePublicKeyFromServer(account); + } else { + qCInfo(lcCse()) << retCode; + failedToInitialize(account); + return; } - qCInfo(lcCse()) << retCode; }); job->start(); } @@ -1457,6 +1478,7 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, Bio privateKey; if (PEM_write_bio_PrivateKey(privateKey, keyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { qCInfo(lcCse()) << "Could not read private key from bio."; + failedToInitialize(account); return; } const auto bytearrayPrivateKey = BIO2ByteArray(privateKey); @@ -1466,14 +1488,20 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, privateKeyJob->setKey(privateKeyKeychainId); privateKeyJob->setBinaryData(bytearrayPrivateKey); connect(privateKeyJob, &WritePasswordJob::finished, [keyPair = std::move(keyPair), publicKeyKeychainId, account, output, this] (Job *incoming) mutable { - Q_UNUSED(incoming); + if (incoming->error() != Error::NoError) { + failedToInitialize(account); + return; + } + qCInfo(lcCse()) << "Private key stored in keychain"; Bio publicKey; if (PEM_write_bio_PUBKEY(publicKey, keyPair) <= 0) { qCInfo(lcCse()) << "Could not read public key from bio."; + failedToInitialize(account); return; } + const auto bytearrayPublicKey = BIO2ByteArray(publicKey); auto *publicKeyJob = new WritePasswordJob(Theme::instance()->appName()); @@ -1481,7 +1509,11 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, publicKeyJob->setKey(publicKeyKeychainId); publicKeyJob->setBinaryData(bytearrayPublicKey); connect(publicKeyJob, &WritePasswordJob::finished, [account, keyPair = std::move(keyPair), output, this](Job *incoming) mutable { - Q_UNUSED(incoming); + if (incoming->error() != Error::NoError) { + failedToInitialize(account); + return; + } + qCInfo(lcCse()) << "Public key stored in keychain"; sendSignRequestCSR(account, std::move(keyPair), output); @@ -1496,7 +1528,7 @@ void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) const auto keyIsNotOnServer = [account, this] () { qCInfo(lcCse) << "server is missing keys. deleting local keys"; - forgetSensitiveData(account); + failedToInitialize(account); }; const auto privateKeyOnServerIsValid = [this] () { @@ -1568,10 +1600,13 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) qCInfo(lcCse()) << "Private key stored encrypted on server."; writePrivateKey(account); writeCertificate(account); - writeMnemonic(account, [this] () {emit initializationFinished(true);}); + writeMnemonic(account, [this] () { + emit initializationFinished(true); + }); break; default: qCInfo(lcCse()) << "Store private key failed, return code:" << retCode; + failedToInitialize(account); } }); job->start(); @@ -1580,7 +1615,7 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QByteArray &key) { if (!account->askUserForMnemonic()) { qCDebug(lcCse) << "Not allowed to ask user for mnemonic"; - emit initializationFinished(); + failedToInitialize(account); return; } @@ -1630,10 +1665,9 @@ void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QB break; } } else { - _mnemonic = QString(); - _privateKey = QByteArray(); qCInfo(lcCse()) << "Cancelled"; - break; + failedToInitialize(account); + return; } } @@ -1678,12 +1712,13 @@ void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account) qCInfo(lcCse()) << "No public key on the server"; if (!account->e2eEncryptionKeysGenerationAllowed()) { qCInfo(lcCse()) << "User did not allow E2E keys generation."; - emit initializationFinished(); + failedToInitialize(account); return; } generateKeyPair(account); } else { qCInfo(lcCse()) << "Error while requesting public key: " << retCode; + failedToInitialize(account); } }); job->start(); @@ -1715,6 +1750,8 @@ void ClientSideEncryption::fetchAndValidatePublicKeyFromServer(const AccountPtr } } else { qCInfo(lcCse()) << "Error while requesting server public key: " << retCode; + failedToInitialize(account); + return; } }); job->start(); diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index 1c3ba01417..dda48275c6 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -207,6 +207,8 @@ private: [[nodiscard]] bool checkServerPublicKeyValidity(const QByteArray &serverPublicKeyString) const; [[nodiscard]] bool sensitiveDataRemaining() const; + void failedToInitialize(const AccountPtr &account); + bool isInitialized = false; }; From fad55470d59d3aa80ff577dbf6bfd4434be6538d Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 23 Jun 2023 18:20:47 +0200 Subject: [PATCH 11/16] ensure e2ee can be disabled from client Signed-off-by: Matthieu Gallien --- src/gui/accountsettings.cpp | 21 +++++++++++++-------- src/gui/accountsettings.h | 2 ++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index 36ed056ab8..21df46b085 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -1561,13 +1561,7 @@ void AccountSettings::initializeE2eEncryption() if (!_accountState->account()->e2e()->_mnemonic.isEmpty()) { slotE2eEncryptionMnemonicReady(); } else { - _ui->encryptionMessage->setMessageType(KMessageWidget::Information); - _ui->encryptionMessage->setText(tr("This account supports end-to-end encryption")); - _ui->encryptionMessage->setIcon(Theme::createColorAwareIcon(QStringLiteral(":/client/theme/black/state-info.svg"))); - _ui->encryptionMessage->hide(); - - auto *const actionEnableE2e = addActionToEncryptionMessage(tr("Set up encryption"), e2EeUiActionEnableEncryptionId); - connect(actionEnableE2e, &QAction::triggered, this, &AccountSettings::slotE2eEncryptionGenerateKeys); + initializeE2eEncryptionSettingsMessage(); connect(_accountState->account()->e2e(), &ClientSideEncryption::initializationFinished, this, [this] { if (!_accountState->account()->e2e()->_publicKey.isNull()) { @@ -1590,7 +1584,7 @@ void AccountSettings::resetE2eEncryption() } _ui->encryptionMessage->setText({}); _ui->encryptionMessage->setIcon({}); - initializeE2eEncryption(); + initializeE2eEncryptionSettingsMessage(); checkClientSideEncryptionState(); const auto account = _accountState->account(); @@ -1627,6 +1621,17 @@ QAction *AccountSettings::addActionToEncryptionMessage(const QString &actionTitl return action; } +void AccountSettings::initializeE2eEncryptionSettingsMessage() +{ + _ui->encryptionMessage->setMessageType(KMessageWidget::Information); + _ui->encryptionMessage->setText(tr("This account supports end-to-end encryption")); + _ui->encryptionMessage->setIcon(Theme::createColorAwareIcon(QStringLiteral(":/client/theme/black/state-info.svg"))); + _ui->encryptionMessage->hide(); + + auto *const actionEnableE2e = addActionToEncryptionMessage(tr("Set up encryption"), e2EeUiActionEnableEncryptionId); + connect(actionEnableE2e, &QAction::triggered, this, &AccountSettings::slotE2eEncryptionGenerateKeys); +} + } // namespace OCC #include "accountsettings.moc" diff --git a/src/gui/accountsettings.h b/src/gui/accountsettings.h index 0be744d723..1ddbc097d4 100644 --- a/src/gui/accountsettings.h +++ b/src/gui/accountsettings.h @@ -132,6 +132,8 @@ private: bool event(QEvent *) override; QAction *addActionToEncryptionMessage(const QString &actionTitle, const QString &actionId); + void initializeE2eEncryptionSettingsMessage(); + /// Returns the alias of the selected folder, empty string if none [[nodiscard]] QString selectedFolderAlias() const; From b9761a23fe4e36835127cb62dbd39789dc9c5852 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 23 Jun 2023 19:00:49 +0200 Subject: [PATCH 12/16] tidy code Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index db398191e8..ba74be6d6d 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -898,7 +898,7 @@ void ClientSideEncryption::fetchCertificateFromKeyChain(const AccountPtr &accoun account->id() ); - auto *job = new ReadPasswordJob(Theme::instance()->appName()); + const auto job = new ReadPasswordJob(Theme::instance()->appName()); job->setProperty(accountProperty, QVariant::fromValue(account)); job->setInsecureFallback(false); job->setKey(kck); @@ -1011,8 +1011,8 @@ void ClientSideEncryption::publicCertificateFetched(Job *incoming) void ClientSideEncryption::publicKeyFetched(QKeychain::Job *incoming) { - auto *readJob = dynamic_cast(incoming); - auto account = readJob->property(accountProperty).value(); + const auto readJob = dynamic_cast(incoming); + const auto account = readJob->property(accountProperty).value(); Q_ASSERT(account); // Error or no valid public key error out @@ -1038,7 +1038,7 @@ void ClientSideEncryption::publicKeyFetched(QKeychain::Job *incoming) account->id() ); - auto *job = new ReadPasswordJob(Theme::instance()->appName()); + const auto job = new ReadPasswordJob(Theme::instance()->appName()); job->setProperty(accountProperty, QVariant::fromValue(account)); job->setInsecureFallback(false); job->setKey(kck); @@ -1146,7 +1146,7 @@ void ClientSideEncryption::writeCertificate(const AccountPtr &account) void ClientSideEncryption::generateMnemonic() { - QStringList list = WordList::getRandomWords(12); + const auto list = WordList::getRandomWords(12); _mnemonic = list.join(' '); qCInfo(lcCse()) << "mnemonic Generated:" << _mnemonic; } @@ -1483,7 +1483,7 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, } const auto bytearrayPrivateKey = BIO2ByteArray(privateKey); - auto *privateKeyJob = new WritePasswordJob(Theme::instance()->appName()); + const auto privateKeyJob = new WritePasswordJob(Theme::instance()->appName()); privateKeyJob->setInsecureFallback(false); privateKeyJob->setKey(privateKeyKeychainId); privateKeyJob->setBinaryData(bytearrayPrivateKey); @@ -1504,7 +1504,7 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, const auto bytearrayPublicKey = BIO2ByteArray(publicKey); - auto *publicKeyJob = new WritePasswordJob(Theme::instance()->appName()); + const auto publicKeyJob = new WritePasswordJob(Theme::instance()->appName()); publicKeyJob->setInsecureFallback(false); publicKeyJob->setKey(publicKeyKeychainId); publicKeyJob->setBinaryData(bytearrayPublicKey); From af1162804a8c9b91dc1b5479b72c2e0d282e7367 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Tue, 27 Jun 2023 23:29:25 +0200 Subject: [PATCH 13/16] make sure we clean only what is needed if e2ee is disabled Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index ba74be6d6d..220fbd0003 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1054,8 +1054,7 @@ void ClientSideEncryption::privateKeyFetched(Job *incoming) // Error or no valid public key error out if (readJob->error() != NoError || readJob->binaryData().length() == 0) { - _certificate = QSslCertificate(); - _publicKey = QSslKey(); + forgetSensitiveData(account); getPublicKeyFromServer(account); return; } @@ -1092,9 +1091,7 @@ void ClientSideEncryption::mnemonicKeyFetched(QKeychain::Job *incoming) // Error or no valid public key error out if (readJob->error() != NoError || readJob->textData().length() == 0) { - _certificate = QSslCertificate(); - _publicKey = QSslKey(); - _privateKey = QByteArray(); + forgetSensitiveData(account); getPublicKeyFromServer(account); return; } @@ -1180,8 +1177,6 @@ void ClientSideEncryption::writeMnemonic(OCC::AccountPtr account, void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account) { - _publicKey = QSslKey(); - if (!sensitiveDataRemaining()) { checkAllSensitiveDataDeleted(); return; @@ -1198,12 +1193,10 @@ void ClientSideEncryption::forgetSensitiveData(const AccountPtr &account) const auto deletePrivateKeyJob = createDeleteJob(user + e2e_private); const auto deleteCertJob = createDeleteJob(user + e2e_cert); const auto deleteMnemonicJob = createDeleteJob(user + e2e_mnemonic); - const auto deletePublicKeyJob = createDeleteJob(user + e2e_public); connect(deletePrivateKeyJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handlePrivateKeyDeleted); connect(deleteCertJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handleCertificateDeleted); connect(deleteMnemonicJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handleMnemonicDeleted); - connect(deletePublicKeyJob, &DeletePasswordJob::finished, this, &ClientSideEncryption::handlePublicKeyDeleted); deletePrivateKeyJob->start(); deleteCertJob->start(); deleteMnemonicJob->start(); @@ -1742,9 +1735,7 @@ void ClientSideEncryption::fetchAndValidatePublicKeyFromServer(const AccountPtr } } else { qCInfo(lcCse()) << "Error invalid server public key"; - _certificate = QSslCertificate(); - _publicKey = QSslKey(); - _privateKey = QByteArray(); + forgetSensitiveData(account); getPublicKeyFromServer(account); return; } From 1712f98b3ccf15ad853ce45ebf7cdd5575a1d6d0 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 29 Jun 2023 11:02:33 +0200 Subject: [PATCH 14/16] make sure to pass shared pointer by const ref when possible avoid unnecessary copies of shared pointers Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 16 ++++++++-------- src/libsync/clientsideencryption.h | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 220fbd0003..2b69b10f3d 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1345,7 +1345,7 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) }); } -std::pair ClientSideEncryption::generateCSR(AccountPtr account, +std::pair ClientSideEncryption::generateCSR(const AccountPtr &account, PKey keyPair, PKey privateKey) { @@ -1416,7 +1416,7 @@ std::pair ClientSideEncryption::generate return {result, std::move(keyPair)}; } -void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, +void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey keyPair, QByteArray csrContent) { @@ -1452,7 +1452,7 @@ void ClientSideEncryption::sendSignRequestCSR(AccountPtr account, job->start(); } -void ClientSideEncryption::writeKeyPair(AccountPtr account, +void ClientSideEncryption::writeKeyPair(const AccountPtr &account, PKey keyPair, QByteArray output) { @@ -1516,7 +1516,7 @@ void ClientSideEncryption::writeKeyPair(AccountPtr account, privateKeyJob->start(); } -void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) +void ClientSideEncryption::checkServerHasSavedKeys(const AccountPtr &account) { const auto keyIsNotOnServer = [account, this] () { qCInfo(lcCse) << "server is missing keys. deleting local keys"; @@ -1537,7 +1537,7 @@ void ClientSideEncryption::checkServerHasSavedKeys(AccountPtr account) template void ClientSideEncryption::checkUserKeyOnServer(const QString &keyType, - OCC::AccountPtr account, + const AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) { @@ -1556,7 +1556,7 @@ void ClientSideEncryption::checkUserKeyOnServer(const QString &keyType, } template -void ClientSideEncryption::checkUserPublicKeyOnServer(AccountPtr account, +void ClientSideEncryption::checkUserPublicKeyOnServer(const AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) { @@ -1564,7 +1564,7 @@ void ClientSideEncryption::checkUserPublicKeyOnServer(AccountPtr account, } template -void ClientSideEncryption::checkUserPrivateKeyOnServer(AccountPtr account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) +void ClientSideEncryption::checkUserPrivateKeyOnServer(const AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) { checkUserKeyOnServer("private-key", account, nextCheck, onError); } @@ -1759,7 +1759,7 @@ FolderMetadata::FolderMetadata(AccountPtr account, RequiredMetadataVersion requiredMetadataVersion, const QByteArray& metadata, int statusCode) - : _account(account) + : _account(std::move(account)) , _requiredMetadataVersion(requiredMetadataVersion) { if (metadata.isEmpty() || statusCode == 404) { diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index dda48275c6..e95fc21bb4 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -169,15 +169,15 @@ private slots: private: void generateMnemonic(); - [[nodiscard]] std::pair generateCSR(AccountPtr account, + [[nodiscard]] std::pair generateCSR(const AccountPtr &account, PKey keyPair, PKey privateKey); - void sendSignRequestCSR(AccountPtr account, + void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, QByteArray csrContent); - void writeKeyPair(AccountPtr account, + void writeKeyPair(const AccountPtr &account, PKey keyPair, QByteArray output); @@ -185,21 +185,21 @@ private: void writeMnemonic(OCC::AccountPtr account, L nextCall); - void checkServerHasSavedKeys(AccountPtr account); + void checkServerHasSavedKeys(const AccountPtr &account); template - void checkUserPublicKeyOnServer(OCC::AccountPtr account, + void checkUserPublicKeyOnServer(const OCC::AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError); template - void checkUserPrivateKeyOnServer(OCC::AccountPtr account, + void checkUserPrivateKeyOnServer(const OCC::AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError); template void checkUserKeyOnServer(const QString &keyType, - OCC::AccountPtr account, + const OCC::AccountPtr &account, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError); From 0b3d67437afc1f50f2dda5d3510787b10ae7a5a6 Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Thu, 29 Jun 2023 11:19:52 +0200 Subject: [PATCH 15/16] improve logs of e2ee such that errors are easy to see removed some internal debug logs that should not be needed Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 85 +++++++++------------------- 1 file changed, 26 insertions(+), 59 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 2b69b10f3d..8c6f7ba748 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1030,8 +1030,6 @@ void ClientSideEncryption::publicKeyFetched(QKeychain::Job *incoming) _publicKey = publicKey; - qCInfo(lcCse()) << "Public key fetched from keychain"; - const QString kck = AbstractCredentials::keychainKey( account->url().toString(), account->credentials()->user() + e2e_private, @@ -1145,7 +1143,6 @@ void ClientSideEncryption::generateMnemonic() { const auto list = WordList::getRandomWords(12); _mnemonic = list.join(' '); - qCInfo(lcCse()) << "mnemonic Generated:" << _mnemonic; } template @@ -1168,8 +1165,6 @@ void ClientSideEncryption::writeMnemonic(OCC::AccountPtr account, return; } - qCInfo(lcCse()) << "Mnemonic stored in keychain"; - nextCall(); }); job->start(); @@ -1252,7 +1247,6 @@ void ClientSideEncryption::handlePublicKeyDeleted(const QKeychain::Job * const i return; } - qCDebug(lcCse) << "Public key successfully deleted from keychain. Clearing."; _publicKey.clear(); Q_EMIT publicKeyDeleted(); checkAllSensitiveDataDeleted(); @@ -1272,14 +1266,13 @@ void ClientSideEncryption::failedToInitialize(const AccountPtr &account) void ClientSideEncryption::checkAllSensitiveDataDeleted() { if (sensitiveDataRemaining()) { - qCDebug(lcCse) << "Some sensitive data emaining:" - << "Private key:" << _privateKey - << "Certificate is null:" << _certificate.isNull() - << "Mnemonic:" << _mnemonic; + qCWarning(lcCse) << "Some sensitive data emaining:" + << "Private key:" << (_privateKey.isEmpty() ? "is empty" : "is not empty") + << "Certificate is null:" << (_certificate.isNull() ? "true" : "false") + << "Mnemonic:" << (_mnemonic.isEmpty() ? "is empty" : "is not empty"); return; } - qCDebug(lcCse) << "All sensitive encryption data has been deleted."; Q_EMIT sensitiveDataForgotten(); } @@ -1313,20 +1306,16 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) return; } - qCInfo(lcCse()) << "Key correctly generated"; - qCInfo(lcCse()) << "Storing keys locally"; - { Bio privKey; if (PEM_write_bio_PrivateKey(privKey, localKeyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { - qCInfo(lcCse()) << "Could not read private key from bio."; + qCWarning(lcCse()) << "Could not read private key from bio."; failedToInitialize(account); return; } const auto key = BIO2ByteArray(privKey); _privateKey = key; - qCDebug(lcCse) << _privateKey; } Bio privKey; @@ -1338,7 +1327,8 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) auto privateKey = PKey::readPrivateKey(privKey); - qCInfo(lcCse()) << "Keys generated correctly, sending to server."; + qCDebug(lcCse()) << "Key correctly generated"; + auto csrOutput = generateCSR(account, std::move(localKeyPair), std::move(privateKey)); writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void { writeKeyPair(account, std::move(keyPair), output); @@ -1353,7 +1343,6 @@ std::pair ClientSideEncryption::generate // OpenSSL expects const char. auto cnArray = account->davUser().toLocal8Bit(); - qCInfo(lcCse()) << "Getting the following array for the account Id" << cnArray; auto certParams = std::map{ {"C", "DE"}, @@ -1380,34 +1369,33 @@ std::pair ClientSideEncryption::generate for(const auto& v : certParams) { ret = X509_NAME_add_entry_by_txt(x509_name, v.first, MBSTRING_ASC, (const unsigned char*) v.second, -1, -1, 0); if (ret != 1) { - qCInfo(lcCse()) << "Error Generating the Certificate while adding" << v.first << v.second; + qCWarning(lcCse()) << "Error Generating the Certificate while adding" << v.first << v.second; return {result, std::move(keyPair)}; } } ret = X509_REQ_set_pubkey(x509_req, keyPair); if (ret != 1){ - qCInfo(lcCse()) << "Error setting the public key on the csr"; + qCWarning(lcCse()) << "Error setting the public key on the csr"; return {result, std::move(keyPair)}; } ret = X509_REQ_sign(x509_req, privateKey, EVP_sha1()); // return x509_req->signature->length if (ret <= 0){ - qCInfo(lcCse()) << "Error signing the csr with the private key"; + qCWarning(lcCse()) << "Error signing the csr with the private key"; return {result, std::move(keyPair)}; } Bio out; ret = PEM_write_bio_X509_REQ(out, x509_req); if (ret <= 0){ - qCInfo(lcCse()) << "Error exporting the csr to the BIO"; + qCWarning(lcCse()) << "Error exporting the csr to the BIO"; return {result, std::move(keyPair)}; } result = BIO2ByteArray(out); - qCInfo(lcCse()) << "Returning the certificate"; - qCInfo(lcCse()) << result; + qCDebug(lcCse()) << "CSR generated"; if (_mnemonic.isEmpty()) { generateMnemonic(); @@ -1435,16 +1423,15 @@ void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, if (!X509_check_private_key(x509Certificate, keyPair)) { auto lastError = ERR_get_error(); while (lastError) { - qCInfo(lcCse()) << ERR_lib_error_string(lastError); + qCWarning(lcCse()) << ERR_lib_error_string(lastError); lastError = ERR_get_error(); } failedToInitialize(account); return; } - qCInfo(lcCse()) << "received a valid certificate"; fetchAndValidatePublicKeyFromServer(account); } else { - qCInfo(lcCse()) << retCode; + qCWarning(lcCse()) << retCode; failedToInitialize(account); return; } @@ -1470,7 +1457,7 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, Bio privateKey; if (PEM_write_bio_PrivateKey(privateKey, keyPair, nullptr, nullptr, 0, nullptr, nullptr) <= 0) { - qCInfo(lcCse()) << "Could not read private key from bio."; + qCWarning(lcCse()) << "Could not read private key from bio."; failedToInitialize(account); return; } @@ -1486,11 +1473,9 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, return; } - qCInfo(lcCse()) << "Private key stored in keychain"; - Bio publicKey; if (PEM_write_bio_PUBKEY(publicKey, keyPair) <= 0) { - qCInfo(lcCse()) << "Could not read public key from bio."; + qCWarning(lcCse()) << "Could not read public key from bio."; failedToInitialize(account); return; } @@ -1507,8 +1492,6 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, return; } - qCInfo(lcCse()) << "Public key stored in keychain"; - sendSignRequestCSR(account, std::move(keyPair), output); }); publicKeyJob->start(); @@ -1541,7 +1524,6 @@ void ClientSideEncryption::checkUserKeyOnServer(const QString &keyType, SUCCESS_CALLBACK nextCheck, ERROR_CALLBACK onError) { - qCInfo(lcCse()) << "Retrieving" << keyType << "from server"; auto job = new JsonApiJob(account, e2eeBaseUrl() + keyType, this); connect(job, &JsonApiJob::jsonReceived, [nextCheck, onError](const QJsonDocument& doc, int retCode) { Q_UNUSED(doc) @@ -1577,7 +1559,7 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) auto passPhrase = _mnemonic; passPhrase = passPhrase.remove(' ').toLower(); - qCInfo(lcCse()) << "Passphrase Generated:" << passPhrase; + qCDebug(lcCse) << "Passphrase Generated"; auto salt = EncryptionHelper::generateRandom(40); auto secretKey = EncryptionHelper::generatePassword(passPhrase, salt); @@ -1590,7 +1572,6 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) Q_UNUSED(doc); switch(retCode) { case 200: - qCInfo(lcCse()) << "Private key stored encrypted on server."; writePrivateKey(account); writeCertificate(account); writeMnemonic(account, [this] () { @@ -1598,7 +1579,7 @@ void ClientSideEncryption::encryptPrivateKey(const AccountPtr &account) }); break; default: - qCInfo(lcCse()) << "Store private key failed, return code:" << retCode; + qCWarning(lcCse) << "Store private key failed, return code:" << retCode; failedToInitialize(account); } }); @@ -1632,25 +1613,20 @@ void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QB } bool ok = dialog.exec(); if (ok) { - qCInfo(lcCse()) << "Got mnemonic:" << dialog.textValue(); prev = dialog.textValue(); _mnemonic = prev; QString mnemonic = prev.split(" ").join(QString()).toLower(); - qCInfo(lcCse()) << "mnemonic:" << mnemonic; // split off salt const auto salt = EncryptionHelper::extractPrivateKeySalt(key); auto pass = EncryptionHelper::generatePassword(mnemonic, salt); - qCInfo(lcCse()) << "Generated key:" << pass; QByteArray privateKey = EncryptionHelper::decryptPrivateKey(pass, key); //_privateKey = QSslKey(privateKey, QSsl::Rsa, QSsl::Pem, QSsl::PrivateKey); _privateKey = privateKey; - qCInfo(lcCse()) << "Private key: " << _privateKey; - if (!_privateKey.isNull() && checkPublicKeyValidity(account)) { writePrivateKey(account); writeCertificate(account); @@ -1658,7 +1634,7 @@ void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QB break; } } else { - qCInfo(lcCse()) << "Cancelled"; + qCDebug(lcCse()) << "Cancelled"; failedToInitialize(account); return; } @@ -1669,20 +1645,17 @@ void ClientSideEncryption::decryptPrivateKey(const AccountPtr &account, const QB void ClientSideEncryption::getPrivateKeyFromServer(const AccountPtr &account) { - qCInfo(lcCse()) << "Retrieving private key from server"; auto job = new JsonApiJob(account, e2eeBaseUrl() + "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(account, key.toLocal8Bit()); } else if (retCode == 404) { - qCInfo(lcCse()) << "No private key on the server: setup is incomplete."; + qCWarning(lcCse) << "No private key on the server: setup is incomplete."; emit initializationFinished(); return; } else { - qCInfo(lcCse()) << "Error while requesting public key: " << retCode; + qCWarning(lcCse) << "Error while requesting public key: " << retCode; emit initializationFinished(); return; } @@ -1692,25 +1665,23 @@ void ClientSideEncryption::getPrivateKeyFromServer(const AccountPtr &account) void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account) { - qCInfo(lcCse()) << "Retrieving public key from server"; auto job = new JsonApiJob(account, e2eeBaseUrl() + "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(); _certificate = QSslCertificate(publicKey.toLocal8Bit(), QSsl::Pem); _publicKey = _certificate.publicKey(); - qCInfo(lcCse()) << "Found Public key, requesting Server Public Key. Public key:" << publicKey; fetchAndValidatePublicKeyFromServer(account); } else if (retCode == 404) { - qCInfo(lcCse()) << "No public key on the server"; + qCDebug(lcCse()) << "No public key on the server"; if (!account->e2eEncryptionKeysGenerationAllowed()) { - qCInfo(lcCse()) << "User did not allow E2E keys generation."; + qCDebug(lcCse()) << "User did not allow E2E keys generation."; failedToInitialize(account); return; } generateKeyPair(account); } else { - qCInfo(lcCse()) << "Error while requesting public key: " << retCode; + qCWarning(lcCse) << "Error while requesting public key: " << retCode; failedToInitialize(account); } }); @@ -1719,28 +1690,24 @@ void ClientSideEncryption::getPublicKeyFromServer(const AccountPtr &account) void ClientSideEncryption::fetchAndValidatePublicKeyFromServer(const AccountPtr &account) { - qCInfo(lcCse()) << "Retrieving public key from server"; auto job = new JsonApiJob(account, e2eeBaseUrl() + "server-key", this); connect(job, &JsonApiJob::jsonReceived, [this, account](const QJsonDocument& doc, int retCode) { if (retCode == 200) { const auto serverPublicKey = doc.object()["ocs"].toObject()["data"].toObject()["public-key"].toString().toLatin1(); - qCInfo(lcCse()) << "Found Server Public key, checking it. Server public key:" << serverPublicKey; if (checkServerPublicKeyValidity(serverPublicKey)) { if (_privateKey.isEmpty()) { - qCInfo(lcCse()) << "Valid Server Public key, requesting Private Key."; getPrivateKeyFromServer(account); } else { - qCInfo(lcCse()) << "Certificate saved, Encrypting Private Key."; encryptPrivateKey(account); } } else { - qCInfo(lcCse()) << "Error invalid server public key"; + qCWarning(lcCse) << "Error invalid server public key"; forgetSensitiveData(account); getPublicKeyFromServer(account); return; } } else { - qCInfo(lcCse()) << "Error while requesting server public key: " << retCode; + qCWarning(lcCse) << "Error while requesting server public key: " << retCode; failedToInitialize(account); return; } From 10cac0f46f9e9b7b4a150357341e5202c94ed3aa Mon Sep 17 00:00:00 2001 From: Matthieu Gallien Date: Fri, 30 Jun 2023 10:36:57 +0200 Subject: [PATCH 16/16] fix review comments Signed-off-by: Matthieu Gallien --- src/libsync/clientsideencryption.cpp | 23 ++++++++++------------- src/libsync/clientsideencryption.h | 4 ++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/libsync/clientsideencryption.cpp b/src/libsync/clientsideencryption.cpp index 8c6f7ba748..c1732e13fb 100644 --- a/src/libsync/clientsideencryption.cpp +++ b/src/libsync/clientsideencryption.cpp @@ -1314,8 +1314,7 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) return; } - const auto key = BIO2ByteArray(privKey); - _privateKey = key; + _privateKey = BIO2ByteArray(privKey); } Bio privKey; @@ -1325,13 +1324,11 @@ void ClientSideEncryption::generateKeyPair(const AccountPtr &account) return; } - auto privateKey = PKey::readPrivateKey(privKey); - qCDebug(lcCse()) << "Key correctly generated"; - auto csrOutput = generateCSR(account, std::move(localKeyPair), std::move(privateKey)); - writeMnemonic(account, [account, keyPair = std::move(csrOutput.second), output = std::move(csrOutput.first), this]() mutable -> void { - writeKeyPair(account, std::move(keyPair), output); + auto csrContent = generateCSR(account, std::move(localKeyPair), PKey::readPrivateKey(privKey)); + writeMnemonic(account, [account, keyPair = std::move(csrContent.second), csrContent = std::move(csrContent.first), this]() mutable -> void { + writeKeyPair(account, std::move(keyPair), csrContent); }); } @@ -1356,7 +1353,7 @@ std::pair ClientSideEncryption::generate int nVersion = 1; // 2. set version of x509 req - X509_REQ *x509_req = X509_REQ_new(); + auto x509_req = X509_REQ_new(); auto release_on_exit_x509_req = qScopeGuard([&] { X509_REQ_free(x509_req); }); @@ -1406,7 +1403,7 @@ std::pair ClientSideEncryption::generate void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, PKey keyPair, - QByteArray csrContent) + const QByteArray &csrContent) { auto job = new SignPublicKeyApiJob(account, e2eeBaseUrl() + "public-key", this); job->setCsr(csrContent); @@ -1441,7 +1438,7 @@ void ClientSideEncryption::sendSignRequestCSR(const AccountPtr &account, void ClientSideEncryption::writeKeyPair(const AccountPtr &account, PKey keyPair, - QByteArray output) + const QByteArray &csrContent) { const auto privateKeyKeychainId = AbstractCredentials::keychainKey( account->url().toString(), @@ -1467,7 +1464,7 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, privateKeyJob->setInsecureFallback(false); privateKeyJob->setKey(privateKeyKeychainId); privateKeyJob->setBinaryData(bytearrayPrivateKey); - connect(privateKeyJob, &WritePasswordJob::finished, [keyPair = std::move(keyPair), publicKeyKeychainId, account, output, this] (Job *incoming) mutable { + connect(privateKeyJob, &WritePasswordJob::finished, [keyPair = std::move(keyPair), publicKeyKeychainId, account, csrContent, this] (Job *incoming) mutable { if (incoming->error() != Error::NoError) { failedToInitialize(account); return; @@ -1486,13 +1483,13 @@ void ClientSideEncryption::writeKeyPair(const AccountPtr &account, publicKeyJob->setInsecureFallback(false); publicKeyJob->setKey(publicKeyKeychainId); publicKeyJob->setBinaryData(bytearrayPublicKey); - connect(publicKeyJob, &WritePasswordJob::finished, [account, keyPair = std::move(keyPair), output, this](Job *incoming) mutable { + connect(publicKeyJob, &WritePasswordJob::finished, [account, keyPair = std::move(keyPair), csrContent, this](Job *incoming) mutable { if (incoming->error() != Error::NoError) { failedToInitialize(account); return; } - sendSignRequestCSR(account, std::move(keyPair), output); + sendSignRequestCSR(account, std::move(keyPair), csrContent); }); publicKeyJob->start(); }); diff --git a/src/libsync/clientsideencryption.h b/src/libsync/clientsideencryption.h index e95fc21bb4..a4a4c8be4f 100644 --- a/src/libsync/clientsideencryption.h +++ b/src/libsync/clientsideencryption.h @@ -175,11 +175,11 @@ private: void sendSignRequestCSR(const AccountPtr &account, PKey keyPair, - QByteArray csrContent); + const QByteArray &csrContent); void writeKeyPair(const AccountPtr &account, PKey keyPair, - QByteArray output); + const QByteArray &csrContent); template void writeMnemonic(OCC::AccountPtr account,