From ad57a336dc5885ad11965fd72869474bd7f30496 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sun, 8 Aug 2021 18:37:40 +0200 Subject: Breaking: Change secret names and fix bug when storing secrets --- src/Cache.cpp | 56 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 20 deletions(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index ee991dc2..7996c017 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -720,20 +720,34 @@ Cache::storeSecret(const std::string name, const std::string secret) { auto settings = UserSettings::instance(); auto job = new QKeychain::WritePasswordJob(QCoreApplication::applicationName()); + job->setAutoDelete(true); job->setInsecureFallback(true); - job->setKey("matrix." + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), - QCryptographicHash::Sha256)) + - "." + name.c_str()); + + // job->setSettings(new QSettings(job)); + job->setKey( + "matrix." + + QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) + .toBase64()) + + "." + QString::fromStdString(name)); + job->setTextData(QString::fromStdString(secret)); - QObject::connect(job, &QKeychain::Job::finished, job, [name, this](QKeychain::Job *job) { - if (job->error()) { - nhlog::db()->warn( - "Storing secret '{}' failed: {}", name, job->errorString().toStdString()); - } else { - emit secretChanged(name); - } - }); + 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()); + } 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(); } @@ -744,10 +758,11 @@ Cache::deleteSecret(const std::string name) QKeychain::DeletePasswordJob job(QCoreApplication::applicationName()); job.setAutoDelete(false); job.setInsecureFallback(true); - job.setKey("matrix." + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), - QCryptographicHash::Sha256)) + - "." + name.c_str()); + job.setKey( + "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; @@ -765,10 +780,11 @@ Cache::secret(const std::string name) QKeychain::ReadPasswordJob job(QCoreApplication::applicationName()); job.setAutoDelete(false); job.setInsecureFallback(true); - job.setKey("matrix." + - QString(QCryptographicHash::hash(settings->profile().toUtf8(), - QCryptographicHash::Sha256)) + - "." + name.c_str()); + job.setKey( + "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; -- cgit 1.5.1 From 71290e208d2decab6042ce40b372e9aabfba7cef Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sun, 8 Aug 2021 18:57:38 +0200 Subject: Enable insecure fallback for secret storage --- src/Cache.cpp | 9 ++++++++- src/UserSettingsPage.cpp | 3 --- src/UserSettingsPage.h | 5 +++++ 3 files changed, 13 insertions(+), 4 deletions(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index 7996c017..eb9fb028 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -722,8 +722,8 @@ Cache::storeSecret(const std::string name, const std::string secret) auto job = new QKeychain::WritePasswordJob(QCoreApplication::applicationName()); job->setAutoDelete(true); job->setInsecureFallback(true); + job->setSettings(UserSettings::instance()->qsettings()); - // job->setSettings(new QSettings(job)); job->setKey( "matrix." + QString(QCryptographicHash::hash(settings->profile().toUtf8(), QCryptographicHash::Sha256) @@ -731,6 +731,7 @@ Cache::storeSecret(const std::string name, const std::string secret) "." + QString::fromStdString(name)); job->setTextData(QString::fromStdString(secret)); + QObject::connect( job, &QKeychain::WritePasswordJob::finished, @@ -758,11 +759,14 @@ Cache::deleteSecret(const std::string name) QKeychain::DeletePasswordJob job(QCoreApplication::applicationName()); job.setAutoDelete(false); job.setInsecureFallback(true); + job.setSettings(UserSettings::instance()->qsettings()); + job.setKey( "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; @@ -780,11 +784,14 @@ Cache::secret(const std::string name) QKeychain::ReadPasswordJob job(QCoreApplication::applicationName()); job.setAutoDelete(false); job.setInsecureFallback(true); + job.setSettings(UserSettings::instance()->qsettings()); + job.setKey( "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; diff --git a/src/UserSettingsPage.cpp b/src/UserSettingsPage.cpp index ab6ac492..f67c5e2d 100644 --- a/src/UserSettingsPage.cpp +++ b/src/UserSettingsPage.cpp @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -63,7 +62,6 @@ UserSettings::initialize(std::optional profile) void UserSettings::load(std::optional profile) { - QSettings settings; tray_ = settings.value("user/window/tray", false).toBool(); startInTray_ = settings.value("user/window/start_in_tray", false).toBool(); @@ -601,7 +599,6 @@ UserSettings::applyTheme() void UserSettings::save() { - QSettings settings; settings.beginGroup("user"); settings.beginGroup("window"); diff --git a/src/UserSettingsPage.h b/src/UserSettingsPage.h index 096aab81..84940e47 100644 --- a/src/UserSettingsPage.h +++ b/src/UserSettingsPage.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -107,6 +108,8 @@ public: static QSharedPointer instance(); static void initialize(std::optional profile); + QSettings *qsettings() { return &settings; } + enum class Presence { AutomaticPresence, @@ -316,6 +319,8 @@ private: QString homeserver_; QStringList hiddenTags_; + QSettings settings; + static QSharedPointer instance_; }; -- cgit 1.5.1 From dbea031a8601dae57a635f28bceffb3b4134ea53 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Wed, 11 Aug 2021 00:21:02 +0200 Subject: Fix potential crash when trying to read room info too early --- src/Cache.cpp | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index eb9fb028..b79a3b47 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -1586,26 +1586,32 @@ Cache::roomsWithStateUpdates(const mtx::responses::Sync &res) RoomInfo Cache::singleRoomInfo(const std::string &room_id) { - auto txn = ro_txn(env_); - auto statesdb = getStatesDb(txn, room_id); + auto txn = ro_txn(env_); - std::string_view data; + try { + auto statesdb = getStatesDb(txn, room_id); - // Check if the room is joined. - if (roomsDb_.get(txn, room_id, data)) { - try { - RoomInfo tmp = json::parse(data); - tmp.member_count = getMembersDb(txn, room_id).size(txn); - tmp.join_rule = getRoomJoinRule(txn, statesdb); - tmp.guest_access = getRoomGuestAccess(txn, statesdb); + std::string_view data; - return tmp; - } catch (const json::exception &e) { - nhlog::db()->warn("failed to parse room info: room_id ({}), {}: {}", - room_id, - std::string(data.data(), data.size()), - e.what()); + // Check if the room is joined. + if (roomsDb_.get(txn, room_id, data)) { + try { + RoomInfo tmp = json::parse(data); + tmp.member_count = getMembersDb(txn, room_id).size(txn); + tmp.join_rule = getRoomJoinRule(txn, statesdb); + tmp.guest_access = getRoomGuestAccess(txn, statesdb); + + return tmp; + } catch (const json::exception &e) { + nhlog::db()->warn("failed to parse room info: room_id ({}), {}: {}", + room_id, + std::string(data.data(), data.size()), + e.what()); + } } + } catch (const lmdb::error &e) { + nhlog::db()->warn( + "failed to read room info from db: room_id ({}), {}", room_id, e.what()); } return RoomInfo(); -- cgit 1.5.1 From 18ea01e198d112de00ac70e1e1c357424706d10a Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 13 Aug 2021 23:13:09 +0200 Subject: 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 | 226 +++++++++++++++++++++++++++++------------ src/CacheCryptoStructs.h | 8 +- src/Cache_p.h | 8 +- src/timeline/TimelineModel.cpp | 17 ++++ src/timeline/TimelineModel.h | 3 + 8 files changed, 219 insertions(+), 71 deletions(-) (limited to 'src/Cache.cpp') 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); + mtx::requests::QueryKeys req; + std::string last_changed; + { + auto txn = ro_txn(env_); + auto cache_ = userKeys_(user_id, txn); - if (cache_.has_value()) { - if (!cache_->updated_at.empty() && cache_->updated_at == cache_->last_changed) { - cb(cache_.value(), {}); - return; - } - } + 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); - mtx::requests::QueryKeys req; - req.device_keys[user_id] = {}; + req.device_keys[user_id] = {}; - std::string last_changed; - if (cache_) - last_changed = cache_->last_changed; - req.token = last_changed; + 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; + 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 &) { + verified_state.device_verified.insert(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(); -- cgit 1.5.1 From 9bad584931a03717f71e96900202acac43d2a62f Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Fri, 13 Aug 2021 23:58:26 +0200 Subject: Show verification status in memberlist --- resources/qml/RoomMembers.qml | 42 ++++++++++++++++++++++++++++++------ resources/qml/Root.qml | 4 ++-- resources/qml/TopBar.qml | 2 +- src/Cache.cpp | 2 -- src/MemberList.cpp | 12 +++++++++++ src/MemberList.h | 1 + src/timeline/TimelineViewManager.cpp | 8 ++++--- src/timeline/TimelineViewManager.h | 4 ++-- 8 files changed, 59 insertions(+), 16 deletions(-) (limited to 'src/Cache.cpp') diff --git a/resources/qml/RoomMembers.qml b/resources/qml/RoomMembers.qml index 447e6fd1..8e44855c 100644 --- a/resources/qml/RoomMembers.qml +++ b/resources/qml/RoomMembers.qml @@ -13,6 +13,7 @@ ApplicationWindow { id: roomMembersRoot property MemberList members + property Room room title: qsTr("Members of %1").arg(members.roomName) height: 650 @@ -83,9 +84,14 @@ ApplicationWindow { } delegate: RowLayout { + id: del + + width: ListView.view.width spacing: Nheko.paddingMedium Avatar { + id: avatar + width: Nheko.avatarSize height: Nheko.avatarSize userid: model.mxid @@ -97,16 +103,18 @@ ApplicationWindow { ColumnLayout { spacing: Nheko.paddingSmall - Label { - text: model.displayName + ElidedLabel { + fullText: model.displayName color: TimelineManager.userColor(model ? model.mxid : "", Nheko.colors.window) - font.pointSize: fontMetrics.font.pointSize + font.pixelSize: fontMetrics.font.pixelSize + elideWidth: del.width - Nheko.paddingMedium * 2 - avatar.width - encryptInd.width } - Label { - text: model.mxid + ElidedLabel { + fullText: model.mxid color: Nheko.colors.buttonText - font.pointSize: fontMetrics.font.pointSize * 0.9 + font.pixelSize: Math.ceil(fontMetrics.font.pixelSize * 0.9) + elideWidth: del.width - Nheko.paddingMedium * 2 - avatar.width - encryptInd.width } Item { @@ -116,6 +124,28 @@ ApplicationWindow { } + EncryptionIndicator { + id: encryptInd + + Layout.alignment: Qt.AlignRight + visible: room.isEncrypted + encrypted: room.isEncrypted + trust: encrypted ? model.trustlevel : Crypto.Unverified + ToolTip.text: { + if (!encrypted) + return qsTr("This room is not encrypted!"); + + switch (trust) { + case Crypto.Verified: + return qsTr("This user is verified."); + case Crypto.TOFU: + return qsTr("This user isn't verified, but is still using the same master key from the first time you met."); + default: + return qsTr("This user has unverified devices!"); + } + } + } + } footer: Item { diff --git a/resources/qml/Root.qml b/resources/qml/Root.qml index b229acda..79f12bbf 100644 --- a/resources/qml/Root.qml +++ b/resources/qml/Root.qml @@ -153,10 +153,10 @@ Page { packSet.show(); } - function onOpenRoomMembersDialog(members) { + function onOpenRoomMembersDialog(members, room) { var membersDialog = roomMembersComponent.createObject(timelineRoot, { "members": members, - "roomName": Rooms.currentRoom.roomName + "room": room }); membersDialog.show(); } diff --git a/resources/qml/TopBar.qml b/resources/qml/TopBar.qml index 0faaea9c..7f67c028 100644 --- a/resources/qml/TopBar.qml +++ b/resources/qml/TopBar.qml @@ -140,7 +140,7 @@ Rectangle { Platform.MenuItem { text: qsTr("Members") - onTriggered: TimelineManager.openRoomMembers(room.roomId) + onTriggered: TimelineManager.openRoomMembers(room) } Platform.MenuItem { diff --git a/src/Cache.cpp b/src/Cache.cpp index 5edfaf09..ea034dd0 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -3584,8 +3584,6 @@ Cache::roomMembers(const std::string &room_id) crypto::Trust Cache::roomVerificationStatus(const std::string &room_id) { - std::string_view keys; - crypto::Trust trust = crypto::Verified; try { diff --git a/src/MemberList.cpp b/src/MemberList.cpp index 196647fe..0c0f0cdd 100644 --- a/src/MemberList.cpp +++ b/src/MemberList.cpp @@ -53,6 +53,7 @@ MemberList::roleNames() const {Mxid, "mxid"}, {DisplayName, "displayName"}, {AvatarUrl, "avatarUrl"}, + {Trustlevel, "trustlevel"}, }; } @@ -69,6 +70,17 @@ MemberList::data(const QModelIndex &index, int role) const return m_memberList[index.row()].first.display_name; case AvatarUrl: return m_memberList[index.row()].second; + case Trustlevel: { + auto stat = + cache::verificationStatus(m_memberList[index.row()].first.user_id.toStdString()); + + if (!stat) + return crypto::Unverified; + if (stat->unverified_device_count) + return crypto::Unverified; + else + return stat->user_verified; + } default: return {}; } diff --git a/src/MemberList.h b/src/MemberList.h index e6522694..cffcd83d 100644 --- a/src/MemberList.h +++ b/src/MemberList.h @@ -25,6 +25,7 @@ public: Mxid, DisplayName, AvatarUrl, + Trustlevel, }; MemberList(const QString &room_id, QObject *parent = nullptr); diff --git a/src/timeline/TimelineViewManager.cpp b/src/timeline/TimelineViewManager.cpp index b23ed278..906e328f 100644 --- a/src/timeline/TimelineViewManager.cpp +++ b/src/timeline/TimelineViewManager.cpp @@ -375,10 +375,12 @@ TimelineViewManager::TimelineViewManager(CallManager *callManager, ChatPage *par } void -TimelineViewManager::openRoomMembers(QString room_id) +TimelineViewManager::openRoomMembers(TimelineModel *room) { - MemberList *memberList = new MemberList(room_id, this); - emit openRoomMembersDialog(memberList); + if (!room) + return; + MemberList *memberList = new MemberList(room->roomId(), this); + emit openRoomMembersDialog(memberList, room); } void diff --git a/src/timeline/TimelineViewManager.h b/src/timeline/TimelineViewManager.h index 54e3a935..4dd5e996 100644 --- a/src/timeline/TimelineViewManager.h +++ b/src/timeline/TimelineViewManager.h @@ -66,7 +66,7 @@ public: Q_INVOKABLE QString userPresence(QString id) const; Q_INVOKABLE QString userStatus(QString id) const; - Q_INVOKABLE void openRoomMembers(QString room_id); + Q_INVOKABLE void openRoomMembers(TimelineModel *room); Q_INVOKABLE void openRoomSettings(QString room_id); Q_INVOKABLE void openInviteUsers(QString roomId); Q_INVOKABLE void openGlobalUserProfile(QString userId); @@ -92,7 +92,7 @@ signals: void focusChanged(); void focusInput(); void openImageOverlayInternalCb(QString eventId, QImage img); - void openRoomMembersDialog(MemberList *members); + void openRoomMembersDialog(MemberList *members, TimelineModel *room); void openRoomSettingsDialog(RoomSettings *settings); void openInviteUsersDialog(InviteesModel *invitees); void openProfile(UserProfile *profile); -- cgit 1.5.1 From 13633c7644d1a89bc09033ed1df9e4919afb3e25 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sat, 14 Aug 2021 02:06:48 +0200 Subject: Ensure device signatures always get verified on device update --- src/Cache.cpp | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index ea034dd0..00602acf 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -3901,8 +3901,43 @@ Cache::updateUserKeys(const std::string &sync_token, const mtx::responses::Query } } - if (!keyReused && !oldDeviceKeys.count(device_id)) + if (!keyReused && !oldDeviceKeys.count(device_id)) { + // ensure the key has a valid signature from itself + std::string device_signing_key = + "ed25519:" + device_keys.device_id; + if (device_id != device_keys.device_id) { + nhlog::crypto()->warn( + "device {}:{} has a different device id " + "in the body: {}", + user, + device_id, + device_keys.device_id); + continue; + } + if (!device_keys.signatures.count(user) || + !device_keys.signatures.at(user).count( + device_signing_key)) { + nhlog::crypto()->warn( + "device {}:{} has no signature", + user, + device_id); + continue; + } + + if (!mtx::crypto::ed25519_verify_signature( + device_keys.keys.at(device_signing_key), + json(device_keys), + device_keys.signatures.at(user).at( + device_signing_key))) { + nhlog::crypto()->warn( + "device {}:{} has an invalid signature", + user, + device_id); + continue; + } + updateToWrite.device_keys[device_id] = device_keys; + } } for (const auto &[key_id, key] : device_keys.keys) { -- cgit 1.5.1 From 110fef5c68d8a65ecc0faaee3db26c3b7d9c5821 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sat, 14 Aug 2021 02:41:34 +0200 Subject: Request keys when opening a room for the first time --- src/Cache.cpp | 37 ++++++++++++++++++++++++++----------- src/CacheCryptoStructs.h | 2 ++ 2 files changed, 28 insertions(+), 11 deletions(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index 00602acf..fccf1c53 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -3587,24 +3587,33 @@ Cache::roomVerificationStatus(const std::string &room_id) crypto::Trust trust = crypto::Verified; try { - auto txn = ro_txn(env_); + auto txn = lmdb::txn::begin(env_); auto db = getMembersDb(txn, room_id); auto keysDb = getUserKeysDb(txn); + std::vector keysToRequest; 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) + if (verif.unverified_device_count) { + trust = crypto::Unverified; + if (verif.verified_devices.empty() && verif.no_keys) { + // we probably don't have the keys yet, so query them + keysToRequest.push_back(std::string(user_id)); + } + } else if (verif.user_verified == crypto::TOFU && trust == crypto::Verified) trust = crypto::TOFU; } + + if (!keysToRequest.empty()) + markUserKeysOutOfDate(txn, keysDb, keysToRequest, ""); + } catch (std::exception &e) { nhlog::db()->error( "Failed to calculate verification status for {}: {}", room_id, e.what()); - return crypto::Unverified; + trust = crypto::Unverified; } return trust; @@ -4000,14 +4009,16 @@ Cache::markUserKeysOutOfDate(lmdb::txn &txn, nhlog::db()->debug("Marking user keys out of date: {}", user); std::string_view oldKeys; - auto res = db.get(txn, user, oldKeys); - - if (!res) - continue; - auto cacheEntry = - json::parse(std::string_view(oldKeys.data(), oldKeys.size())).get(); + UserKeyCache cacheEntry; + auto res = db.get(txn, user, oldKeys); + if (res) { + auto cacheEntry = + json::parse(std::string_view(oldKeys.data(), oldKeys.size())) + .get(); + } cacheEntry.last_changed = sync_token; + db.put(txn, user, json(cacheEntry).dump()); query.device_keys[user] = {}; @@ -4233,6 +4244,7 @@ Cache::verificationStatus_(const std::string &user_id, lmdb::txn &txn) // 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; + status.no_keys = true; if (auto verifCache = verificationCache(user_id, txn)) { status.verified_devices = verifCache->device_verified; @@ -4284,6 +4296,9 @@ Cache::verificationStatus_(const std::string &user_id, lmdb::txn &txn) // key and their master key auto ourKeys = userKeys_(local_user, txn); auto theirKeys = userKeys_(user_id, txn); + if (theirKeys) + status.no_keys = false; + if (!ourKeys || !theirKeys) { verification_storage.status[user_id] = status; return status; diff --git a/src/CacheCryptoStructs.h b/src/CacheCryptoStructs.h index a992fe79..6c402674 100644 --- a/src/CacheCryptoStructs.h +++ b/src/CacheCryptoStructs.h @@ -117,6 +117,8 @@ struct VerificationStatus std::map verified_device_keys; //! Count of unverified devices int unverified_device_count = 0; + // if the keys are not in cache + bool no_keys = false; }; //! In memory cache of verification status -- cgit 1.5.1 From 69e65cef2f3aff2eed7e333e8962776d3057c475 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sat, 14 Aug 2021 02:52:43 +0200 Subject: Fix shadowing --- src/Cache.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/Cache.cpp') diff --git a/src/Cache.cpp b/src/Cache.cpp index fccf1c53..8b8b2985 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -4013,9 +4013,8 @@ Cache::markUserKeysOutOfDate(lmdb::txn &txn, UserKeyCache cacheEntry; auto res = db.get(txn, user, oldKeys); if (res) { - auto cacheEntry = - json::parse(std::string_view(oldKeys.data(), oldKeys.size())) - .get(); + cacheEntry = json::parse(std::string_view(oldKeys.data(), oldKeys.size())) + .get(); } cacheEntry.last_changed = sync_token; -- cgit 1.5.1