From 569ea5b5f475a09d6257ba5529ffa36d4074dd4d Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 15 Mar 2021 16:24:01 +0100 Subject: [PATCH] Rotate session keys properly --- CMakeLists.txt | 2 +- io.github.NhekoReborn.Nheko.json | 2 +- src/Cache.cpp | 32 +++++++ src/CacheCryptoStructs.h | 1 + src/Cache_p.h | 2 + src/Olm.cpp | 152 +++++++++++++++++-------------- 6 files changed, 121 insertions(+), 70 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d64b033a..905e6159 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -358,7 +358,7 @@ if(USE_BUNDLED_MTXCLIENT) FetchContent_Declare( MatrixClient GIT_REPOSITORY https://github.com/Nheko-Reborn/mtxclient.git - GIT_TAG b1b1ef9ad088f9666582f46d81b75a80c6b2b1c0 + GIT_TAG 7194b4f058406b1c10d3741d83abcf2d8963d849 ) set(BUILD_LIB_EXAMPLES OFF CACHE INTERNAL "") set(BUILD_LIB_TESTS OFF CACHE INTERNAL "") diff --git a/io.github.NhekoReborn.Nheko.json b/io.github.NhekoReborn.Nheko.json index 4b40aaf0..34f68fb7 100644 --- a/io.github.NhekoReborn.Nheko.json +++ b/io.github.NhekoReborn.Nheko.json @@ -220,7 +220,7 @@ "name": "mtxclient", "sources": [ { - "commit": "b1b1ef9ad088f9666582f46d81b75a80c6b2b1c0", + "commit": "7194b4f058406b1c10d3741d83abcf2d8963d849", "type": "git", "url": "https://github.com/Nheko-Reborn/mtxclient.git" } diff --git a/src/Cache.cpp b/src/Cache.cpp index 0817a2d1..cfc6a727 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -261,6 +261,36 @@ Cache::isRoomEncrypted(const std::string &room_id) return res; } +std::optional +Cache::roomEncryptionSettings(const std::string &room_id) +{ + using namespace mtx::events; + using namespace mtx::events::state; + + try { + auto txn = lmdb::txn::begin(env_, nullptr, MDB_RDONLY); + auto statesdb = getStatesDb(txn, room_id); + std::string_view event; + bool res = + statesdb.get(txn, to_string(mtx::events::EventType::RoomEncryption), event); + + if (res) { + try { + StateEvent msg = json::parse(event); + + return msg.content; + } catch (const json::exception &e) { + nhlog::db()->warn("failed to parse m.room.encryption event: {}", + e.what()); + return Encryption{}; + } + } + } catch (lmdb::error &) { + } + + return std::nullopt; +} + mtx::crypto::ExportedSessionKeys Cache::exportSessionKeys() { @@ -3893,6 +3923,7 @@ to_json(nlohmann::json &obj, const OutboundGroupSessionData &msg) obj["session_id"] = msg.session_id; obj["session_key"] = msg.session_key; obj["message_index"] = msg.message_index; + obj["ts"] = msg.timestamp; obj["initially"] = msg.initially; obj["currently"] = msg.currently; @@ -3904,6 +3935,7 @@ from_json(const nlohmann::json &obj, OutboundGroupSessionData &msg) msg.session_id = obj.at("session_id"); msg.session_key = obj.at("session_key"); msg.message_index = obj.at("message_index"); + msg.timestamp = obj.value("ts", 0ULL); msg.initially = obj.value("initially", SharedWithUsers{}); msg.currently = obj.value("currently", SharedWithUsers{}); diff --git a/src/CacheCryptoStructs.h b/src/CacheCryptoStructs.h index 383d7b05..c884107e 100644 --- a/src/CacheCryptoStructs.h +++ b/src/CacheCryptoStructs.h @@ -28,6 +28,7 @@ struct OutboundGroupSessionData std::string session_id; std::string session_key; uint64_t message_index = 0; + uint64_t timestamp = 0; // who has access to this session. // Rotate, when a user leaves the room and share, when a user gets added. diff --git a/src/Cache_p.h b/src/Cache_p.h index 09fc277d..b6c555dc 100644 --- a/src/Cache_p.h +++ b/src/Cache_p.h @@ -221,6 +221,8 @@ public: //! Mark a room that uses e2e encryption. void setEncryptedRoom(lmdb::txn &txn, const std::string &room_id); bool isRoomEncrypted(const std::string &room_id); + std::optional roomEncryptionSettings( + const std::string &room_id); //! Check if a user is a member of the room. bool isRoomMember(const std::string &user_id, const std::string &room_id); diff --git a/src/Olm.cpp b/src/Olm.cpp index 311aeb7f..d2f78b76 100644 --- a/src/Olm.cpp +++ b/src/Olm.cpp @@ -431,92 +431,107 @@ encrypt_group_message(const std::string &room_id, const std::string &device_id, if (cache::outboundMegolmSessionExists(room_id)) { auto res = cache::getOutboundMegolmSession(room_id); + auto encryptionSettings = cache::client()->roomEncryptionSettings(room_id); + mtx::events::state::Encryption defaultSettings; - auto member_it = members.begin(); - auto session_member_it = res.data.currently.keys.begin(); - auto session_member_it_end = res.data.currently.keys.end(); + // rotate if we crossed the limits for this key + if (res.data.message_index < + encryptionSettings.value_or(defaultSettings).rotation_period_msgs && + (QDateTime::currentMSecsSinceEpoch() - res.data.timestamp) < + encryptionSettings.value_or(defaultSettings).rotation_period_ms) { + auto member_it = members.begin(); + auto session_member_it = res.data.currently.keys.begin(); + auto session_member_it_end = res.data.currently.keys.end(); - while (member_it != members.end() || session_member_it != session_member_it_end) { - if (member_it == members.end()) { - // a member left, purge session! - nhlog::crypto()->debug( - "Rotating megolm session because of left member"); - break; - } + while (member_it != members.end() || + session_member_it != session_member_it_end) { + if (member_it == members.end()) { + // a member left, purge session! + nhlog::crypto()->debug( + "Rotating megolm session because of left member"); + break; + } - if (session_member_it == session_member_it_end) { - // share with all remaining members - while (member_it != members.end()) { + if (session_member_it == session_member_it_end) { + // share with all remaining members + while (member_it != members.end()) { + sendSessionTo[member_it->first] = {}; + + if (member_it->second) + for (const auto &dev : + member_it->second->device_keys) + if (member_it->first != + own_user_id || + dev.first != device_id) + sendSessionTo[member_it + ->first] + .push_back(dev.first); + + ++member_it; + } + + session = std::move(res.session); + break; + } + + if (member_it->first > session_member_it->first) { + // a member left, purge session + nhlog::crypto()->debug( + "Rotating megolm session because of left member"); + break; + } else if (member_it->first < session_member_it->first) { + // new member, send them the session at this index sendSessionTo[member_it->first] = {}; - if (member_it->second) + if (member_it->second) { for (const auto &dev : member_it->second->device_keys) if (member_it->first != own_user_id || dev.first != device_id) sendSessionTo[member_it->first] .push_back(dev.first); + } ++member_it; - } + } else { + // compare devices + bool device_removed = false; + for (const auto &dev : session_member_it->second.devices) { + if (!member_it->second || + !member_it->second->device_keys.count( + dev.first)) { + device_removed = true; + break; + } + } - session = std::move(res.session); - break; - } - - if (member_it->first > session_member_it->first) { - // a member left, purge session - nhlog::crypto()->debug( - "Rotating megolm session because of left member"); - break; - } else if (member_it->first < session_member_it->first) { - // new member, send them the session at this index - sendSessionTo[member_it->first] = {}; - - if (member_it->second) { - for (const auto &dev : member_it->second->device_keys) - if (member_it->first != own_user_id || - dev.first != device_id) - sendSessionTo[member_it->first].push_back( - dev.first); - } - - ++member_it; - } else { - // compare devices - bool device_removed = false; - for (const auto &dev : session_member_it->second.devices) { - if (!member_it->second || - !member_it->second->device_keys.count(dev.first)) { - device_removed = true; + if (device_removed) { + // device removed, rotate session! + nhlog::crypto()->debug( + "Rotating megolm session because of removed " + "device of {}", + member_it->first); break; } - } - if (device_removed) { - // device removed, rotate session! - nhlog::crypto()->debug( - "Rotating megolm session because of removed device of {}", - member_it->first); - break; - } + // check for new devices to share with + if (member_it->second) + for (const auto &dev : + member_it->second->device_keys) + if (!session_member_it->second.devices + .count(dev.first) && + (member_it->first != own_user_id || + dev.first != device_id)) + sendSessionTo[member_it->first] + .push_back(dev.first); - // check for new devices to share with - if (member_it->second) - for (const auto &dev : member_it->second->device_keys) - if (!session_member_it->second.devices.count( - dev.first) && - (member_it->first != own_user_id || - dev.first != device_id)) - sendSessionTo[member_it->first].push_back( - dev.first); - - ++member_it; - ++session_member_it; - if (member_it == members.end() && - session_member_it == session_member_it_end) { - // all devices match or are newly added - session = std::move(res.session); + ++member_it; + ++session_member_it; + if (member_it == members.end() && + session_member_it == session_member_it_end) { + // all devices match or are newly added + session = std::move(res.session); + } } } } @@ -537,6 +552,7 @@ encrypt_group_message(const std::string &room_id, const std::string &device_id, session_data.session_id = mtx::crypto::session_id(session.get()); session_data.session_key = mtx::crypto::session_key(session.get()); session_data.message_index = 0; + session_data.timestamp = QDateTime::currentMSecsSinceEpoch(); sendSessionTo.clear();