From 18ea01e198d112de00ac70e1e1c357424706d10a Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 13 Aug 2021 23:13:09 +0200 Subject: [PATCH] Show if there are unverified devices in a room Also fixes some issues where nested transactions will poison the verification cache. --- resources/qml/RoomList.qml | 1 + resources/qml/TimelineView.qml | 1 + resources/qml/TopBar.qml | 26 +++- src/Cache.cpp | 230 +++++++++++++++++++++++---------- src/CacheCryptoStructs.h | 8 +- src/Cache_p.h | 8 +- src/timeline/TimelineModel.cpp | 17 +++ src/timeline/TimelineModel.h | 3 + 8 files changed, 221 insertions(+), 73 deletions(-) diff --git a/resources/qml/RoomList.qml b/resources/qml/RoomList.qml index 576383e2..8fbfce91 100644 --- a/resources/qml/RoomList.qml +++ b/resources/qml/RoomList.qml @@ -35,6 +35,7 @@ Page { function onCurrentRoomChanged() { if (Rooms.currentRoom) roomlist.positionViewAtIndex(Rooms.roomidToIndex(Rooms.currentRoom.roomId), ListView.Contain); + } target: Rooms diff --git a/resources/qml/TimelineView.qml b/resources/qml/TimelineView.qml index 5e99ee5c..104da160 100644 --- a/resources/qml/TimelineView.qml +++ b/resources/qml/TimelineView.qml @@ -88,6 +88,7 @@ Item { Loader { active: room || roomPreview Layout.fillWidth: true + sourceComponent: MessageView { implicitHeight: msgView.height - typingIndicator.height } diff --git a/resources/qml/TopBar.qml b/resources/qml/TopBar.qml index 8543d02a..0faaea9c 100644 --- a/resources/qml/TopBar.qml +++ b/resources/qml/TopBar.qml @@ -15,6 +15,8 @@ Rectangle { property string roomName: room ? room.roomName : qsTr("No room selected") property string avatarUrl: room ? room.roomAvatarUrl : "" property string roomTopic: room ? room.roomTopic : "" + property bool isEncrypted: room ? room.isEncrypted : false + property int trustlevel: room ? room.trustlevel : Crypto.Unverified Layout.fillWidth: true implicitHeight: topLayout.height + Nheko.paddingMedium * 2 @@ -92,11 +94,33 @@ Rectangle { text: roomTopic } + EncryptionIndicator { + Layout.column: 3 + Layout.row: 0 + Layout.rowSpan: 2 + visible: isEncrypted + encrypted: isEncrypted + trust: trustlevel + ToolTip.text: { + if (!encrypted) + return qsTr("This room is not encrypted!"); + + switch (trust) { + case Crypto.Verified: + return qsTr("This room contains only verified devices."); + case Crypto.TOFU: + return qsTr("This rooms contain verified devices and devices which have never changed their master key."); + default: + return qsTr("This room contains unverified devices!"); + } + } + } + ImageButton { id: roomOptionsButton visible: !!room - Layout.column: 3 + Layout.column: 4 Layout.row: 0 Layout.rowSpan: 2 Layout.alignment: Qt.AlignVCenter diff --git a/src/Cache.cpp b/src/Cache.cpp index b79a3b47..5edfaf09 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -114,7 +114,13 @@ ro_txn(lmdb::env &env) txn = lmdb::txn::begin(env, nullptr, MDB_RDONLY); reuse_counter = 0; } else if (reuse_counter > 0) { - txn.renew(); + try { + txn.renew(); + } catch (...) { + txn.abort(); + txn = lmdb::txn::begin(env, nullptr, MDB_RDONLY); + reuse_counter = 0; + } } reuse_counter++; @@ -289,7 +295,9 @@ Cache::setup() megolmSessionDataDb_ = lmdb::dbi::open(txn, MEGOLM_SESSIONS_DATA_DB, MDB_CREATE); // What rooms are encrypted - encryptedRooms_ = lmdb::dbi::open(txn, ENCRYPTED_ROOMS_DB, MDB_CREATE); + encryptedRooms_ = lmdb::dbi::open(txn, ENCRYPTED_ROOMS_DB, MDB_CREATE); + [[maybe_unused]] auto verificationDb = getVerificationDb(txn); + [[maybe_unused]] auto userKeysDb = getUserKeysDb(txn); txn.commit(); @@ -861,6 +869,9 @@ Cache::setNextBatchToken(lmdb::txn &txn, const QString &token) bool Cache::isInitialized() { + if (!env_.handle()) + return false; + auto txn = ro_txn(env_); std::string_view token; @@ -3570,6 +3581,37 @@ Cache::roomMembers(const std::string &room_id) return members; } +crypto::Trust +Cache::roomVerificationStatus(const std::string &room_id) +{ + std::string_view keys; + + crypto::Trust trust = crypto::Verified; + + try { + auto txn = ro_txn(env_); + + auto db = getMembersDb(txn, room_id); + auto keysDb = getUserKeysDb(txn); + + std::string_view user_id, unused; + auto cursor = lmdb::cursor::open(txn, db); + while (cursor.get(user_id, unused, MDB_NEXT)) { + auto verif = verificationStatus_(std::string(user_id), txn); + if (verif.unverified_device_count) + return crypto::Unverified; + else if (verif.user_verified == crypto::TOFU) + trust = crypto::TOFU; + } + } catch (std::exception &e) { + nhlog::db()->error( + "Failed to calculate verification status for {}: {}", room_id, e.what()); + return crypto::Unverified; + } + + return trust; +} + std::map> Cache::getMembersWithKeys(const std::string &room_id, bool verified_only) { @@ -3751,11 +3793,17 @@ from_json(const json &j, UserKeyCache &info) std::optional Cache::userKeys(const std::string &user_id) +{ + auto txn = ro_txn(env_); + return userKeys_(user_id, txn); +} + +std::optional +Cache::userKeys_(const std::string &user_id, lmdb::txn &txn) { std::string_view keys; try { - auto txn = ro_txn(env_); auto db = getUserKeysDb(txn); auto res = db.get(txn, user_id, keys); @@ -3764,7 +3812,8 @@ Cache::userKeys(const std::string &user_id) } else { return {}; } - } catch (std::exception &) { + } catch (std::exception &e) { + nhlog::db()->error("Failed to retrieve user keys for {}: {}", user_id, e.what()); return {}; } } @@ -3799,8 +3848,14 @@ Cache::updateUserKeys(const std::string &sync_token, const mtx::responses::Query auto last_changed = updateToWrite.last_changed; // skip if we are tracking this and expect it to be up to date with the last // sync token - if (!last_changed.empty() && last_changed != sync_token) + if (!last_changed.empty() && last_changed != sync_token) { + nhlog::db()->debug("Not storing update for user {}, because " + "last_changed {}, but we fetched update for {}", + user, + last_changed, + sync_token); continue; + } if (!updateToWrite.master_keys.keys.empty() && update.master_keys.keys != updateToWrite.master_keys.keys) { @@ -3859,6 +3914,7 @@ Cache::updateUserKeys(const std::string &sync_token, const mtx::responses::Query updateToWrite.seen_device_ids.insert(device_id); } } + updateToWrite.updated_at = sync_token; db.put(txn, user, json(updateToWrite).dump()); } @@ -3944,35 +4000,46 @@ void Cache::query_keys(const std::string &user_id, std::function cb) { - auto cache_ = cache::userKeys(user_id); - - if (cache_.has_value()) { - if (!cache_->updated_at.empty() && cache_->updated_at == cache_->last_changed) { - cb(cache_.value(), {}); - return; - } - } - mtx::requests::QueryKeys req; - req.device_keys[user_id] = {}; - std::string last_changed; - if (cache_) - last_changed = cache_->last_changed; - req.token = last_changed; + { + auto txn = ro_txn(env_); + auto cache_ = userKeys_(user_id, txn); + + if (cache_.has_value()) { + if (cache_->updated_at == cache_->last_changed) { + cb(cache_.value(), {}); + return; + } else + nhlog::db()->info("Keys outdated for {}: {} vs {}", + user_id, + cache_->updated_at, + cache_->last_changed); + } else + nhlog::db()->info("No keys found for {}", user_id); + + req.device_keys[user_id] = {}; + + if (cache_) + last_changed = cache_->last_changed; + req.token = last_changed; + } // use context object so that we can disconnect again QObject *context{new QObject(this)}; - QObject::connect(this, - &Cache::verificationStatusChanged, - context, - [cb, user_id, context_ = context](std::string updated_user) mutable { - if (user_id == updated_user) { - context_->deleteLater(); - auto keys = cache::userKeys(user_id); - cb(keys.value_or(UserKeyCache{}), {}); - } - }); + QObject::connect( + this, + &Cache::verificationStatusChanged, + context, + [cb, user_id, context_ = context, this](std::string updated_user) mutable { + if (user_id == updated_user) { + context_->deleteLater(); + auto txn = ro_txn(env_); + auto keys = this->userKeys_(user_id, txn); + cb(keys.value_or(UserKeyCache{}), {}); + } + }, + Qt::QueuedConnection); http::client()->query_keys( req, @@ -4000,17 +4067,16 @@ to_json(json &j, const VerificationCache &info) void from_json(const json &j, VerificationCache &info) { - info.device_verified = j.at("device_verified").get>(); - info.device_blocked = j.at("device_blocked").get>(); + info.device_verified = j.at("device_verified").get>(); + info.device_blocked = j.at("device_blocked").get>(); } std::optional -Cache::verificationCache(const std::string &user_id) +Cache::verificationCache(const std::string &user_id, lmdb::txn &txn) { std::string_view verifiedVal; - auto txn = lmdb::txn::begin(env_); - auto db = getVerificationDb(txn); + auto db = getVerificationDb(txn); try { VerificationCache verified_state; @@ -4029,26 +4095,28 @@ Cache::verificationCache(const std::string &user_id) void Cache::markDeviceVerified(const std::string &user_id, const std::string &key) { - std::string_view val; + { + std::string_view val; - auto txn = lmdb::txn::begin(env_); - auto db = getVerificationDb(txn); + auto txn = lmdb::txn::begin(env_); + auto db = getVerificationDb(txn); - try { - VerificationCache verified_state; - auto res = db.get(txn, user_id, val); - if (res) { - verified_state = json::parse(val); + try { + VerificationCache verified_state; + auto res = db.get(txn, user_id, val); + if (res) { + verified_state = json::parse(val); + } + + for (const auto &device : verified_state.device_verified) + if (device == key) + return; + + verified_state.device_verified.insert(key); + db.put(txn, user_id, json(verified_state).dump()); + txn.commit(); + } catch (std::exception &) { } - - for (const auto &device : verified_state.device_verified) - if (device == key) - return; - - verified_state.device_verified.push_back(key); - db.put(txn, user_id, json(verified_state).dump()); - txn.commit(); - } catch (std::exception &) { } const auto local_user = utils::localUser().toStdString(); @@ -4086,11 +4154,7 @@ Cache::markDeviceUnverified(const std::string &user_id, const std::string &key) verified_state = json::parse(val); } - verified_state.device_verified.erase( - std::remove(verified_state.device_verified.begin(), - verified_state.device_verified.end(), - key), - verified_state.device_verified.end()); + verified_state.device_verified.erase(key); db.put(txn, user_id, json(verified_state).dump()); txn.commit(); @@ -4119,6 +4183,13 @@ Cache::markDeviceUnverified(const std::string &user_id, const std::string &key) VerificationStatus Cache::verificationStatus(const std::string &user_id) +{ + auto txn = ro_txn(env_); + return verificationStatus_(user_id, txn); +} + +VerificationStatus +Cache::verificationStatus_(const std::string &user_id, lmdb::txn &txn) { std::unique_lock lock(verification_storage.verification_storage_mtx); if (verification_storage.status.count(user_id)) @@ -4126,7 +4197,11 @@ Cache::verificationStatus(const std::string &user_id) VerificationStatus status; - if (auto verifCache = verificationCache(user_id)) { + // assume there is at least one unverified device until we have checked we have the device + // list for that user. + status.unverified_device_count = 1; + + if (auto verifCache = verificationCache(user_id, txn)) { status.verified_devices = verifCache->device_verified; } @@ -4134,12 +4209,10 @@ Cache::verificationStatus(const std::string &user_id) crypto::Trust trustlevel = crypto::Trust::Unverified; if (user_id == local_user) { - status.verified_devices.push_back(http::client()->device_id()); + status.verified_devices.insert(http::client()->device_id()); trustlevel = crypto::Trust::Verified; } - verification_storage.status[user_id] = status; - auto verifyAtLeastOneSig = [](const auto &toVerif, const std::map &keys, const std::string &keyOwner) { @@ -4157,6 +4230,16 @@ Cache::verificationStatus(const std::string &user_id) return false; }; + auto updateUnverifiedDevices = [&status](auto &theirDeviceKeys) { + int currentVerifiedDevices = 0; + for (auto device_id : status.verified_devices) { + if (theirDeviceKeys.count(device_id)) + currentVerifiedDevices++; + } + status.unverified_device_count = + static_cast(theirDeviceKeys.size()) - currentVerifiedDevices; + }; + try { // for local user verify this device_key -> our master_key -> our self_signing_key // -> our device_keys @@ -4166,17 +4249,24 @@ Cache::verificationStatus(const std::string &user_id) // // This means verifying the other user adds 2 extra steps,verifying our user_signing // key and their master key - auto ourKeys = userKeys(local_user); - auto theirKeys = userKeys(user_id); - if (!ourKeys || !theirKeys) + auto ourKeys = userKeys_(local_user, txn); + auto theirKeys = userKeys_(user_id, txn); + if (!ourKeys || !theirKeys) { + verification_storage.status[user_id] = status; return status; + } + + // Update verified devices count to count without cross-signing + updateUnverifiedDevices(theirKeys->device_keys); if (!mtx::crypto::ed25519_verify_signature( olm::client()->identity_keys().ed25519, json(ourKeys->master_keys), ourKeys->master_keys.signatures.at(local_user) - .at("ed25519:" + http::client()->device_id()))) + .at("ed25519:" + http::client()->device_id()))) { + verification_storage.status[user_id] = status; return status; + } auto master_keys = ourKeys->master_keys.keys; @@ -4191,14 +4281,17 @@ Cache::verificationStatus(const std::string &user_id) trustlevel = crypto::Trust::Verified; else if (!theirKeys->master_key_changed) trustlevel = crypto::Trust::TOFU; - else + else { + verification_storage.status[user_id] = status; return status; + } master_keys = theirKeys->master_keys.keys; } status.user_verified = trustlevel; + verification_storage.status[user_id] = status; if (!verifyAtLeastOneSig(theirKeys->self_signing_keys, master_keys, user_id)) return status; @@ -4209,16 +4302,19 @@ Cache::verificationStatus(const std::string &user_id) device_key.keys.at("curve25519:" + device_key.device_id); if (verifyAtLeastOneSig( device_key, theirKeys->self_signing_keys.keys, user_id)) { - status.verified_devices.push_back(device_key.device_id); + status.verified_devices.insert(device_key.device_id); status.verified_device_keys[identkey] = trustlevel; } } catch (...) { } } + updateUnverifiedDevices(theirKeys->device_keys); verification_storage.status[user_id] = status; return status; - } catch (std::exception &) { + } catch (std::exception &e) { + nhlog::db()->error( + "Failed to calculate verification status of {}: {}", user_id, e.what()); return status; } } diff --git a/src/CacheCryptoStructs.h b/src/CacheCryptoStructs.h index 69d64885..a992fe79 100644 --- a/src/CacheCryptoStructs.h +++ b/src/CacheCryptoStructs.h @@ -112,9 +112,11 @@ struct VerificationStatus //! True, if the users master key is verified crypto::Trust user_verified = crypto::Trust::Unverified; //! List of all devices marked as verified - std::vector verified_devices; + std::set verified_devices; //! Map from sender key/curve25519 to trust status std::map verified_device_keys; + //! Count of unverified devices + int unverified_device_count = 0; }; //! In memory cache of verification status @@ -154,9 +156,9 @@ from_json(const nlohmann::json &j, UserKeyCache &info); struct VerificationCache { //! list of verified device_ids with device-verification - std::vector device_verified; + std::set device_verified; //! list of devices the user blocks - std::vector device_blocked; + std::set device_blocked; }; void diff --git a/src/Cache_p.h b/src/Cache_p.h index 51fe9978..748404d1 100644 --- a/src/Cache_p.h +++ b/src/Cache_p.h @@ -46,7 +46,6 @@ public: std::string statusMessage(const std::string &user_id); // user cache stores user keys - std::optional userKeys(const std::string &user_id); std::map> getMembersWithKeys( const std::string &room_id, bool verified_only); @@ -63,9 +62,11 @@ public: std::function cb); // device & user verification cache + std::optional userKeys(const std::string &user_id); VerificationStatus verificationStatus(const std::string &user_id); void markDeviceVerified(const std::string &user_id, const std::string &device); void markDeviceUnverified(const std::string &user_id, const std::string &device); + crypto::Trust roomVerificationStatus(const std::string &room_id); std::vector joinedRooms(); @@ -681,7 +682,10 @@ private: return QString::fromStdString(event.state_key); } - std::optional verificationCache(const std::string &user_id); + std::optional verificationCache(const std::string &user_id, + lmdb::txn &txn); + VerificationStatus verificationStatus_(const std::string &user_id, lmdb::txn &txn); + std::optional userKeys_(const std::string &user_id, lmdb::txn &txn); void setNextBatchToken(lmdb::txn &txn, const std::string &token); void setNextBatchToken(lmdb::txn &txn, const QString &token); diff --git a/src/timeline/TimelineModel.cpp b/src/timeline/TimelineModel.cpp index 513f08bc..79c28edf 100644 --- a/src/timeline/TimelineModel.cpp +++ b/src/timeline/TimelineModel.cpp @@ -418,6 +418,14 @@ TimelineModel::TimelineModel(TimelineViewManager *manager, QString room_id, QObj &events, &EventStore::enableKeyRequests); + connect(this, &TimelineModel::encryptionChanged, this, &TimelineModel::trustlevelChanged); + connect( + this, &TimelineModel::roomMemberCountChanged, this, &TimelineModel::trustlevelChanged); + connect(cache::client(), + &Cache::verificationStatusChanged, + this, + &TimelineModel::trustlevelChanged); + showEventTimer.callOnTimeout(this, &TimelineModel::scrollTimerEvent); } @@ -1993,6 +2001,15 @@ TimelineModel::roomTopic() const QString::fromStdString(info[room_id_].topic).toHtmlEscaped())); } +crypto::Trust +TimelineModel::trustlevel() const +{ + if (!isEncrypted_) + return crypto::Trust::Unverified; + + return cache::client()->roomVerificationStatus(room_id_.toStdString()); +} + int TimelineModel::roomMemberCount() const { diff --git a/src/timeline/TimelineModel.h b/src/timeline/TimelineModel.h index ad7cfbbb..aa07fe01 100644 --- a/src/timeline/TimelineModel.h +++ b/src/timeline/TimelineModel.h @@ -175,6 +175,7 @@ class TimelineModel : public QAbstractListModel Q_PROPERTY(int roomMemberCount READ roomMemberCount NOTIFY roomMemberCountChanged) Q_PROPERTY(bool isEncrypted READ isEncrypted NOTIFY encryptionChanged) Q_PROPERTY(bool isSpace READ isSpace CONSTANT) + Q_PROPERTY(int trustlevel READ trustlevel NOTIFY trustlevelChanged) Q_PROPERTY(InputBar *input READ input CONSTANT) Q_PROPERTY(Permissions *permissions READ permissions NOTIFY permissionsChanged) @@ -287,6 +288,7 @@ public: DescInfo lastMessage() const { return lastMessage_; } bool isSpace() const { return isSpace_; } bool isEncrypted() const { return isEncrypted_; } + crypto::Trust trustlevel() const; int roomMemberCount() const; public slots: @@ -372,6 +374,7 @@ signals: void updateFlowEventId(std::string event_id); void encryptionChanged(); + void trustlevelChanged(); void roomNameChanged(); void plainRoomNameChanged(); void roomTopicChanged();