From 5ca1fb18bbf9789a65eaf3113ee1ff65449ff086 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sun, 7 Nov 2021 03:38:48 +0100 Subject: [PATCH] Move away from using an event loop to access secrets Fixes messages in room flickering and being stuck fixes #760 relates to #770 relates to #789 --- resources/qml/MessageView.qml | 4 +- resources/qml/delegates/MessageDelegate.qml | 2 +- resources/qml/dialogs/LogoutDialog.qml | 2 +- resources/qml/dialogs/RoomSettings.qml | 2 +- src/Cache.cpp | 313 +++++++++++--------- src/CacheCryptoStructs.h | 8 + src/Cache_p.h | 5 + src/ChatPage.cpp | 117 ++++---- src/UserSettingsPage.cpp | 2 +- src/encryption/SelfVerificationStatus.cpp | 13 +- src/timeline/TimelineViewManager.cpp | 7 +- 11 files changed, 267 insertions(+), 208 deletions(-) diff --git a/resources/qml/MessageView.qml b/resources/qml/MessageView.qml index 7ed30112..b70335bb 100644 --- a/resources/qml/MessageView.qml +++ b/resources/qml/MessageView.qml @@ -363,7 +363,7 @@ ScrollView { anchors.horizontalCenter: parent ? parent.horizontalCenter : undefined width: chat.delegateMaxWidth - height: Math.max(section.active ? section.height + timelinerow.height : timelinerow.height, 10) + height: section.active ? section.height + timelinerow.height : timelinerow.height Rectangle { id: scrollHighlight @@ -420,7 +420,7 @@ ScrollView { property string day: wrapper.day property string previousMessageDay: wrapper.previousMessageDay property string userName: wrapper.userName - property var timestamp: wrapper.timestamp + property date timestamp: wrapper.timestamp z: 4 active: previousMessageUserId !== undefined && previousMessageUserId !== userId || previousMessageDay !== day diff --git a/resources/qml/delegates/MessageDelegate.qml b/resources/qml/delegates/MessageDelegate.qml index 9f889106..5f560505 100644 --- a/resources/qml/delegates/MessageDelegate.qml +++ b/resources/qml/delegates/MessageDelegate.qml @@ -34,7 +34,7 @@ Item { required property int encryptionError required property int relatedEventCacheBuster - height: Math.max(chooser.child.height, 20) + height: chooser.child ? chooser.child.height : Nheko.paddingLarge DelegateChooser { id: chooser diff --git a/resources/qml/dialogs/LogoutDialog.qml b/resources/qml/dialogs/LogoutDialog.qml index eb82dd15..c571a89e 100644 --- a/resources/qml/dialogs/LogoutDialog.qml +++ b/resources/qml/dialogs/LogoutDialog.qml @@ -14,6 +14,6 @@ MessageDialog { text: CallManager.isOnCall ? qsTr("A call is in progress. Log out?") : qsTr("Are you sure you want to log out?") modality: Qt.WindowModal flags: Qt.Tool | Qt.WindowStaysOnTopHint | Qt.WindowCloseButtonHint | Qt.WindowTitleHint - buttons: Dialog.Ok | Dialog.Cancel + buttons: MessageDialog.Ok | MessageDialog.Cancel onAccepted: Nheko.logout() } diff --git a/resources/qml/dialogs/RoomSettings.qml b/resources/qml/dialogs/RoomSettings.qml index 0e7749ce..b0f7730b 100644 --- a/resources/qml/dialogs/RoomSettings.qml +++ b/resources/qml/dialogs/RoomSettings.qml @@ -239,7 +239,7 @@ ApplicationWindow { onRejected: { encryptionToggle.checked = false; } - buttons: Dialog.Ok | Dialog.Cancel + buttons: Platform.MessageDialog.Ok | Platform.MessageDialog.Cancel } MatrixText { diff --git a/src/Cache.cpp b/src/Cache.cpp index b2db9edd..280e00ed 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -199,7 +199,6 @@ Cache::Cache(const QString &userId, QObject *parent) , env_{nullptr} , localUserId_{userId} { - setup(); connect(this, &Cache::userKeysUpdate, this, &Cache::updateUserKeys, Qt::QueuedConnection); connect( this, @@ -212,6 +211,7 @@ Cache::Cache(const QString &userId, QObject *parent) } }, Qt::QueuedConnection); + setup(); } void @@ -308,7 +308,178 @@ Cache::setup() txn.commit(); - databaseReady_ = true; + loadSecrets({ + {mtx::secret_storage::secrets::cross_signing_master, false}, + {mtx::secret_storage::secrets::cross_signing_self_signing, false}, + {mtx::secret_storage::secrets::cross_signing_user_signing, false}, + {mtx::secret_storage::secrets::megolm_backup_v1, false}, + {"pickle_secret", true}, + }); +} + +static void +fatalSecretError() +{ + QMessageBox::critical( + ChatPage::instance(), + QCoreApplication::translate("SecretStorage", "Failed to connect to secret storage"), + QCoreApplication::translate( + "SecretStorage", + "Nheko could not connect to the secure storage to save encryption secrets to. This can " + "have multiple reasons. Check if your D-Bus service is running and you have configured a " + "service like KWallet, Gnome Keyring, KeePassXC or the equivalent for your platform. If " + "you are having trouble, feel free to open an issue here: " + "https://github.com/Nheko-Reborn/nheko/issues")); + + QCoreApplication::exit(1); + exit(1); +} + +static QString +secretName(std::string name, bool internal) +{ + auto settings = UserSettings::instance(); + return (internal ? "nheko." : "matrix.") + + QString( + QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) + .toBase64()) + + "." + QString::fromStdString(name); +} + +void +Cache::loadSecrets(std::vector> toLoad) +{ + if (toLoad.empty()) { + this->databaseReady_ = true; + emit databaseReady(); + return; + } + + auto [name_, internal] = toLoad.front(); + + auto job = new QKeychain::ReadPasswordJob(QCoreApplication::applicationName()); + job->setAutoDelete(true); + job->setInsecureFallback(true); + job->setSettings(UserSettings::instance()->qsettings()); + auto name = secretName(name_, internal); + job->setKey(name); + + connect(job, + &QKeychain::ReadPasswordJob::finished, + this, + [this, name, toLoad, job](QKeychain::Job *) mutable { + const QString secret = job->textData(); + if (job->error() && job->error() != QKeychain::Error::EntryNotFound) { + nhlog::db()->error("Restoring secret '{}' failed ({}): {}", + name.toStdString(), + job->error(), + job->errorString().toStdString()); + + fatalSecretError(); + } + if (secret.isEmpty()) { + nhlog::db()->debug("Restored empty secret '{}'.", name.toStdString()); + } else { + std::unique_lock lock(secret_storage.mtx); + secret_storage.secrets[name.toStdString()] = secret.toStdString(); + } + + // load next secret + toLoad.erase(toLoad.begin()); + + // You can't start a job from the finish signal of a job. + QTimer::singleShot(0, [this, toLoad] { loadSecrets(toLoad); }); + }); + job->start(); +} + +std::optional +Cache::secret(const std::string name_, bool internal) +{ + auto name = secretName(name_, internal); + std::unique_lock lock(secret_storage.mtx); + if (auto secret = secret_storage.secrets.find(name.toStdString()); + secret != secret_storage.secrets.end()) + return secret->second; + else + return std::nullopt; +} + +void +Cache::storeSecret(const std::string name_, const std::string secret, bool internal) +{ + auto name = secretName(name_, internal); + { + std::unique_lock lock(secret_storage.mtx); + secret_storage.secrets[name.toStdString()] = secret; + } + + auto settings = UserSettings::instance(); + auto job = new QKeychain::WritePasswordJob(QCoreApplication::applicationName()); + job->setAutoDelete(true); + job->setInsecureFallback(true); + job->setSettings(UserSettings::instance()->qsettings()); + + job->setKey(name); + + job->setTextData(QString::fromStdString(secret)); + + QObject::connect( + job, + &QKeychain::WritePasswordJob::finished, + this, + [name_, this](QKeychain::Job *job) { + if (job->error()) { + nhlog::db()->warn( + "Storing secret '{}' failed: {}", name_, job->errorString().toStdString()); + fatalSecretError(); + } else { + // if we emit the signal directly, qtkeychain breaks and won't execute new + // jobs. You can't start a job from the finish signal of a job. + QTimer::singleShot(0, [this, name_] { emit secretChanged(name_); }); + nhlog::db()->info("Storing secret '{}' successful", name_); + } + }, + Qt::ConnectionType::DirectConnection); + job->start(); +} + +void +Cache::deleteSecret(const std::string name, bool internal) +{ + auto name_ = secretName(name, internal); + { + std::unique_lock lock(secret_storage.mtx); + secret_storage.secrets.erase(name_.toStdString()); + } + + auto settings = UserSettings::instance(); + auto job = new QKeychain::DeletePasswordJob(QCoreApplication::applicationName()); + job->setAutoDelete(true); + job->setInsecureFallback(true); + job->setSettings(UserSettings::instance()->qsettings()); + + job->setKey(name_); + + job->connect( + job, &QKeychain::Job::finished, this, [this, name]() { emit secretChanged(name); }); + job->start(); +} + +std::string +Cache::pickleSecret() +{ + if (pickle_secret_.empty()) { + auto s = secret("pickle_secret", true); + if (!s) { + this->pickle_secret_ = mtx::client::utils::random_token(64, true); + storeSecret("pickle_secret", pickle_secret_, true); + } else { + this->pickle_secret_ = *s; + } + } + + return pickle_secret_; } void @@ -758,144 +929,6 @@ Cache::backupVersion() } } -static void -fatalSecretError() -{ - QMessageBox::critical( - ChatPage::instance(), - QCoreApplication::translate("SecretStorage", "Failed to connect to secret storage"), - QCoreApplication::translate( - "SecretStorage", - "Nheko could not connect to the secure storage to save encryption secrets to. This can " - "have multiple reasons. Check if your D-Bus service is running and you have configured a " - "service like KWallet, Gnome Keyring, KeePassXC or the equivalent for your platform. If " - "you are having trouble, feel free to open an issue here: " - "https://github.com/Nheko-Reborn/nheko/issues")); - - QCoreApplication::exit(1); - exit(1); -} - -void -Cache::storeSecret(const std::string name, const std::string secret, bool internal) -{ - auto settings = UserSettings::instance(); - auto job = new QKeychain::WritePasswordJob(QCoreApplication::applicationName()); - job->setAutoDelete(true); - job->setInsecureFallback(true); - job->setSettings(UserSettings::instance()->qsettings()); - - job->setKey( - (internal ? "nheko." : "matrix.") + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) - .toBase64()) + - "." + QString::fromStdString(name)); - - job->setTextData(QString::fromStdString(secret)); - - QObject::connect( - job, - &QKeychain::WritePasswordJob::finished, - this, - [name, this](QKeychain::Job *job) { - if (job->error()) { - nhlog::db()->warn( - "Storing secret '{}' failed: {}", name, job->errorString().toStdString()); - fatalSecretError(); - } else { - // if we emit the signal directly, qtkeychain breaks and won't execute new - // jobs. You can't start a job from the finish signal of a job. - QTimer::singleShot(100, [this, name] { emit secretChanged(name); }); - nhlog::db()->info("Storing secret '{}' successful", name); - } - }, - Qt::ConnectionType::DirectConnection); - job->start(); -} - -void -Cache::deleteSecret(const std::string name, bool internal) -{ - auto settings = UserSettings::instance(); - QKeychain::DeletePasswordJob job(QCoreApplication::applicationName()); - job.setAutoDelete(false); - job.setInsecureFallback(true); - job.setSettings(UserSettings::instance()->qsettings()); - - job.setKey( - (internal ? "nheko." : "matrix.") + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) - .toBase64()) + - "." + QString::fromStdString(name)); - - // FIXME(Nico): Nested event loops are dangerous. Some other slots may resume in the mean - // time! - QEventLoop loop; - job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit); - job.start(); - loop.exec(); - - emit secretChanged(name); -} - -std::optional -Cache::secret(const std::string name, bool internal) -{ - auto settings = UserSettings::instance(); - QKeychain::ReadPasswordJob job(QCoreApplication::applicationName()); - job.setAutoDelete(false); - job.setInsecureFallback(true); - job.setSettings(UserSettings::instance()->qsettings()); - - job.setKey( - (internal ? "nheko." : "matrix.") + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) - .toBase64()) + - "." + QString::fromStdString(name)); - - // FIXME(Nico): Nested event loops are dangerous. Some other slots may resume in the mean - // time! - QEventLoop loop; - job.connect(&job, &QKeychain::Job::finished, &loop, &QEventLoop::quit); - job.start(); - loop.exec(); - - const QString secret = job.textData(); - if (job.error()) { - if (job.error() == QKeychain::Error::EntryNotFound) - return std::nullopt; - nhlog::db()->error("Restoring secret '{}' failed ({}): {}", - name, - job.error(), - job.errorString().toStdString()); - - fatalSecretError(); - return std::nullopt; - } - if (secret.isEmpty()) { - nhlog::db()->debug("Restored empty secret '{}'.", name); - return std::nullopt; - } - - return secret.toStdString(); -} - -std::string -Cache::pickleSecret() -{ - if (pickle_secret_.empty()) { - auto s = secret("pickle_secret", true); - if (!s) { - this->pickle_secret_ = mtx::client::utils::random_token(64, true); - storeSecret("pickle_secret", pickle_secret_, true); - } else { - this->pickle_secret_ = *s; - } - } - - return pickle_secret_; -} - void Cache::removeInvite(lmdb::txn &txn, const std::string &room_id) { diff --git a/src/CacheCryptoStructs.h b/src/CacheCryptoStructs.h index b7461848..f56c8685 100644 --- a/src/CacheCryptoStructs.h +++ b/src/CacheCryptoStructs.h @@ -141,6 +141,14 @@ struct VerificationStorage std::mutex verification_storage_mtx; }; +//! In memory cache of verification status +struct SecretsStorage +{ + //! secret name -> secret + std::map secrets; + std::mutex mtx; +}; + // this will store the keys of the user with whom a encrypted room is shared with struct UserKeyCache { diff --git a/src/Cache_p.h b/src/Cache_p.h index 651d73d7..a529bc37 100644 --- a/src/Cache_p.h +++ b/src/Cache_p.h @@ -304,6 +304,7 @@ public: return get_skey(a).compare(get_skey(b)); } + signals: void newReadReceipts(const QString &room_id, const std::vector &event_ids); void roomReadStatus(const std::map &status); @@ -312,8 +313,11 @@ signals: void verificationStatusChanged(const std::string &userid); void selfVerificationStatusChanged(); void secretChanged(const std::string name); + void databaseReady(); private: + void loadSecrets(std::vector> toLoad); + //! Save an invited room. void saveInvite(lmdb::txn &txn, lmdb::dbi &statesdb, @@ -684,6 +688,7 @@ private: std::string pickle_secret_; VerificationStorage verification_storage; + SecretsStorage secret_storage; bool databaseReady_ = false; }; diff --git a/src/ChatPage.cpp b/src/ChatPage.cpp index d262387c..eec8a4d1 100644 --- a/src/ChatPage.cpp +++ b/src/ChatPage.cpp @@ -316,6 +316,65 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token) try { cache::init(userid); + connect(cache::client(), &Cache::databaseReady, this, [this]() { + nhlog::db()->info("database ready"); + + const bool isInitialized = cache::isInitialized(); + const auto cacheVersion = cache::formatVersion(); + + try { + if (!isInitialized) { + cache::setCurrentFormat(); + } else { + if (cacheVersion == cache::CacheVersion::Current) { + loadStateFromCache(); + return; + } else if (cacheVersion == cache::CacheVersion::Older) { + if (!cache::runMigrations()) { + QMessageBox::critical( + this, + tr("Cache migration failed!"), + tr("Migrating the cache to the current version failed. " + "This can have different reasons. Please open an " + "issue and try to use an older version in the mean " + "time. Alternatively you can try deleting the cache " + "manually.")); + QCoreApplication::quit(); + } + loadStateFromCache(); + return; + } else if (cacheVersion == cache::CacheVersion::Newer) { + QMessageBox::critical( + this, + tr("Incompatible cache version"), + tr("The cache on your disk is newer than this version of Nheko " + "supports. Please update Nheko or clear your cache.")); + QCoreApplication::quit(); + return; + } + } + + // It's the first time syncing with this device + // There isn't a saved olm account to restore. + nhlog::crypto()->info("creating new olm account"); + olm::client()->create_new_account(); + cache::saveOlmAccount(olm::client()->save(cache::client()->pickleSecret())); + } catch (const lmdb::error &e) { + nhlog::crypto()->critical("failed to save olm account {}", e.what()); + emit dropToLoginPageCb(QString::fromStdString(e.what())); + return; + } catch (const mtx::crypto::olm_exception &e) { + nhlog::crypto()->critical("failed to create new olm account {}", e.what()); + emit dropToLoginPageCb(QString::fromStdString(e.what())); + return; + } + + getProfileInfo(); + getBackupVersion(); + tryInitialSync(); + callManager_->refreshTurnServer(); + }); + connect(cache::client(), &Cache::newReadReceipts, view_manager_, @@ -326,66 +385,10 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token) ¬ificationsManager, &NotificationsManager::removeNotification); - const bool isInitialized = cache::isInitialized(); - const auto cacheVersion = cache::formatVersion(); - - callManager_->refreshTurnServer(); - - if (!isInitialized) { - cache::setCurrentFormat(); - } else { - if (cacheVersion == cache::CacheVersion::Current) { - loadStateFromCache(); - return; - } else if (cacheVersion == cache::CacheVersion::Older) { - if (!cache::runMigrations()) { - QMessageBox::critical(this, - tr("Cache migration failed!"), - tr("Migrating the cache to the current version failed. " - "This can have different reasons. Please open an " - "issue and try to use an older version in the mean " - "time. Alternatively you can try deleting the cache " - "manually.")); - QCoreApplication::quit(); - } - loadStateFromCache(); - return; - } else if (cacheVersion == cache::CacheVersion::Newer) { - QMessageBox::critical( - this, - tr("Incompatible cache version"), - tr("The cache on your disk is newer than this version of Nheko " - "supports. Please update or clear your cache.")); - QCoreApplication::quit(); - return; - } - } - } catch (const lmdb::error &e) { nhlog::db()->critical("failure during boot: {}", e.what()); - cache::deleteData(); - nhlog::net()->info("falling back to initial sync"); + emit dropToLoginPageCb(tr("Failed to open database, logging out!")); } - - try { - // It's the first time syncing with this device - // There isn't a saved olm account to restore. - nhlog::crypto()->info("creating new olm account"); - olm::client()->create_new_account(); - cache::saveOlmAccount(olm::client()->save(cache::client()->pickleSecret())); - } catch (const lmdb::error &e) { - nhlog::crypto()->critical("failed to save olm account {}", e.what()); - emit dropToLoginPageCb(QString::fromStdString(e.what())); - return; - } catch (const mtx::crypto::olm_exception &e) { - nhlog::crypto()->critical("failed to create new olm account {}", e.what()); - emit dropToLoginPageCb(QString::fromStdString(e.what())); - return; - } - - getProfileInfo(); - getBackupVersion(); - tryInitialSync(); } void diff --git a/src/UserSettingsPage.cpp b/src/UserSettingsPage.cpp index b3edf722..52567289 100644 --- a/src/UserSettingsPage.cpp +++ b/src/UserSettingsPage.cpp @@ -1098,7 +1098,7 @@ UserSettingsPage::UserSettingsPage(QSharedPointer settings, QWidge backupSecretCached, tr("The key to decrypt online key backups. If it is cached, you can enable online " "key backup to store encryption keys securely encrypted on the server.")); - updateSecretStatus(); + // updateSecretStatus(); auto scrollArea_ = new QScrollArea{this}; scrollArea_->setFrameShape(QFrame::NoFrame); diff --git a/src/encryption/SelfVerificationStatus.cpp b/src/encryption/SelfVerificationStatus.cpp index bbfb6f9c..32386fdb 100644 --- a/src/encryption/SelfVerificationStatus.cpp +++ b/src/encryption/SelfVerificationStatus.cpp @@ -259,15 +259,20 @@ SelfVerificationStatus::invalidate() using namespace mtx::secret_storage; nhlog::db()->info("Invalidating self verification status"); + if (cache::isInitialized()) { + return; + } + this->hasSSSS_ = false; emit hasSSSSChanged(); auto keys = cache::client()->userKeys(http::client()->user_id().to_string()); if (!keys || keys->device_keys.find(http::client()->device_id()) == keys->device_keys.end()) { - cache::client()->markUserKeysOutOfDate({http::client()->user_id().to_string()}); - cache::client()->query_keys(http::client()->user_id().to_string(), - [](const UserKeyCache &, mtx::http::RequestErr) {}); - return; + QTimer::singleShot(500, [] { + cache::client()->markUserKeysOutOfDate({http::client()->user_id().to_string()}); + cache::client()->query_keys(http::client()->user_id().to_string(), + [](const UserKeyCache &, mtx::http::RequestErr) {}); + }); } if (keys->master_keys.keys.empty()) { diff --git a/src/timeline/TimelineViewManager.cpp b/src/timeline/TimelineViewManager.cpp index 94e6a0d7..84aa2c90 100644 --- a/src/timeline/TimelineViewManager.cpp +++ b/src/timeline/TimelineViewManager.cpp @@ -248,7 +248,12 @@ TimelineViewManager::TimelineViewManager(CallManager *callManager, ChatPage *par qmlRegisterSingletonInstance("im.nheko", 1, 0, "VerificationManager", verificationManager_); qmlRegisterSingletonType( "im.nheko", 1, 0, "SelfVerificationStatus", [](QQmlEngine *, QJSEngine *) -> QObject * { - return new SelfVerificationStatus(); + auto ptr = new SelfVerificationStatus(); + QObject::connect(ChatPage::instance(), + &ChatPage::initializeEmptyViews, + ptr, + &SelfVerificationStatus::invalidate); + return ptr; }); qRegisterMetaType();