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

@@ -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

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

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()
 }

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 {

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<std::pair<std::string, bool>> 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<std::string>
+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<std::string>
-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)
 {

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<std::string, std::string> secrets;
+    std::mutex mtx;
+};
+
 // this will store the keys of the user with whom a encrypted room is shared with
 struct UserKeyCache
 {

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<QString> &event_ids);
     void roomReadStatus(const std::map<QString, bool> &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<std::pair<std::string, bool>> 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;
 };

src/ChatPage.cpp

@@ -316,21 +316,13 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token)
     try {
         cache::init(userid);
 
-        connect(cache::client(),
-                &Cache::newReadReceipts,
-                view_manager_,
-                &TimelineViewManager::updateReadReceipts);
-
-        connect(cache::client(),
-                &Cache::removeNotification,
-                &notificationsManager,
-                &NotificationsManager::removeNotification);
+        connect(cache::client(), &Cache::databaseReady, this, [this]() {
+            nhlog::db()->info("database ready");
 
             const bool isInitialized = cache::isInitialized();
             const auto cacheVersion  = cache::formatVersion();
 
-        callManager_->refreshTurnServer();
-
+            try {
                 if (!isInitialized) {
                     cache::setCurrentFormat();
                 } else {
@@ -339,7 +331,8 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token)
                         return;
                     } else if (cacheVersion == cache::CacheVersion::Older) {
                         if (!cache::runMigrations()) {
-                    QMessageBox::critical(this,
+                            QMessageBox::critical(
+                              this,
                               tr("Cache migration failed!"),
                               tr("Migrating the cache to the current version failed. "
                                  "This can have different reasons. Please open an "
@@ -355,19 +348,12 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token)
                           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."));
+                             "supports. Please update Nheko 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");
-    }
-
-    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");
@@ -386,6 +372,23 @@ ChatPage::bootstrap(QString userid, QString homeserver, QString token)
             getProfileInfo();
             getBackupVersion();
             tryInitialSync();
+            callManager_->refreshTurnServer();
+        });
+
+        connect(cache::client(),
+                &Cache::newReadReceipts,
+                view_manager_,
+                &TimelineViewManager::updateReadReceipts);
+
+        connect(cache::client(),
+                &Cache::removeNotification,
+                &notificationsManager,
+                &NotificationsManager::removeNotification);
+
+    } catch (const lmdb::error &e) {
+        nhlog::db()->critical("failure during boot: {}", e.what());
+        emit dropToLoginPageCb(tr("Failed to open database, logging out!"));
+    }
 }
 
 void

src/UserSettingsPage.cpp

@@ -1098,7 +1098,7 @@ UserSettingsPage::UserSettingsPage(QSharedPointer<UserSettings> 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);

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()) {
+        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) {});
-        return;
+        });
     }
 
     if (keys->master_keys.keys.empty()) {

src/timeline/TimelineViewManager.cpp

@@ -248,7 +248,12 @@ TimelineViewManager::TimelineViewManager(CallManager *callManager, ChatPage *par
     qmlRegisterSingletonInstance("im.nheko", 1, 0, "VerificationManager", verificationManager_);
     qmlRegisterSingletonType<SelfVerificationStatus>(
       "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<mtx::events::collections::TimelineEvents>();