From ff82452816449eb9ccea872bf192cf153d596858 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Sun, 19 Nov 2023 20:09:38 +0100 Subject: [PATCH] Upgrade trust of megolm sessions when receiving RoomKey Before we only did that, when we basically didn't have the key yet. But since we usually get sent a RoomKey when a new message is sent after we sign in, we were discarding, that those messages should usually now be trusted. --- resources/qml/EncryptionIndicator.qml | 5 ++- src/Cache.cpp | 26 +++++++++++++-- src/CacheCryptoStructs.h | 17 +++++----- src/encryption/Olm.cpp | 46 +++++++++++++++++++-------- src/encryption/Olm.h | 4 ++- src/timeline/TimelineModel.cpp | 3 +- 6 files changed, 72 insertions(+), 29 deletions(-) diff --git a/resources/qml/EncryptionIndicator.qml b/resources/qml/EncryptionIndicator.qml index b606a531..99ccebd4 100644 --- a/resources/qml/EncryptionIndicator.qml +++ b/resources/qml/EncryptionIndicator.qml @@ -21,6 +21,7 @@ Image { case Crypto.TOFU: return "image://colorimage/:/icons/icons/ui/shield-filled.svg?"; case Crypto.Unverified: + case Crypto.MessageUnverified: return "image://colorimage/:/icons/icons/ui/shield-filled-exclamation-mark.svg?"; default: return "image://colorimage/:/icons/icons/ui/shield-filled-cross.svg?"; @@ -39,8 +40,10 @@ Image { return qsTr("Encrypted by a verified device"); case Crypto.TOFU: return qsTr("Encrypted by an unverified device, but you have trusted that user so far."); + case Crypto.MessageUnverified: + return qsTr("Key is from an untrusted source like forwarded from another user or the online key backup. For this reason we can't verify who sent the message."); default: - return qsTr("Encrypted by an unverified device or the key is from an untrusted source like the key backup."); + return qsTr("Encrypted by an unverified device."); } } ToolTip.visible: stateImg.hovered diff --git a/src/Cache.cpp b/src/Cache.cpp index 58dc689b..35bfe9dd 100644 --- a/src/Cache.cpp +++ b/src/Cache.cpp @@ -924,9 +924,29 @@ Cache::saveInboundMegolmSession(const MegolmSessionIndex &index, std::string_view value; if (inboundMegolmSessionDb_.get(txn, key, value)) { auto oldSession = unpickle(std::string(value), pickle_secret_); - if (olm_inbound_group_session_first_known_index(session.get()) > - olm_inbound_group_session_first_known_index(oldSession.get())) { - nhlog::crypto()->warn("Not storing inbound session with newer first known index"); + + auto newIndex = olm_inbound_group_session_first_known_index(session.get()); + auto oldIndex = olm_inbound_group_session_first_known_index(oldSession.get()); + + // merge trusted > untrusted + // first known index minimum + if (megolmSessionDataDb_.get(txn, key, value)) { + auto oldData = nlohmann::json::parse(value).get(); + if (oldData.trusted && newIndex >= oldIndex) { + nhlog::crypto()->warn( + "Not storing inbound session of lesser trust or bigger index."); + return; + } + + oldData.trusted = data.trusted || oldData.trusted; + + if (newIndex < oldIndex) { + inboundMegolmSessionDb_.put(txn, key, pickled); + oldData.message_index = newIndex; + } + + megolmSessionDataDb_.put(txn, key, nlohmann::json(oldData).dump()); + txn.commit(); return; } } diff --git a/src/CacheCryptoStructs.h b/src/CacheCryptoStructs.h index 2a5b895f..22c7bcf0 100644 --- a/src/CacheCryptoStructs.h +++ b/src/CacheCryptoStructs.h @@ -22,10 +22,12 @@ QML_NAMED_ELEMENT(Crypto) //! How much a participant is trusted. enum Trust { - Unverified, //! Device unverified or master key changed. - TOFU, //! Device is signed by the sender, but the user is not verified, but they never - //! changed the master key. - Verified, //! User was verified and has crosssigned this device or device is verified. + Unverified, //! Device unverified or master key changed. + MessageUnverified, //! Only for messages. The sender might be trusted, but we don't know, who + //! was the sender for the message. + TOFU, //! Device is signed by the sender, but the user is not verified, but they never + //! changed the master key. + Verified, //! User was verified and has crosssigned this device or device is verified. }; Q_ENUM_NS(Trust) } @@ -50,10 +52,9 @@ struct GroupSessionData uint64_t timestamp = 0; uint32_t message_index = 0; - // If we got the session via key sharing or forwarding, we can usually trust it. - // If it came from asymmetric key backup, it is not trusted. - // TODO(Nico): What about forwards? They might come from key backup? - bool trusted = true; + // We generally don't trust keys unless they were sent to us by the original sender and include + // that senders signature. + bool trusted = false; // the original 25519 key std::string sender_key; diff --git a/src/encryption/Olm.cpp b/src/encryption/Olm.cpp index 0352d8d3..f336c2ba 100644 --- a/src/encryption/Olm.cpp +++ b/src/encryption/Olm.cpp @@ -629,6 +629,7 @@ encrypt_group_message(const std::string &room_id, const std::string &device_id, // Saving the new megolm session. GroupSessionData session_data{}; session_data.message_index = 0; + session_data.trusted = true; session_data.timestamp = QDateTime::currentMSecsSinceEpoch(); session_data.sender_claimed_ed25519_key = olm::client()->identity_keys().ed25519; session_data.sender_key = olm::client()->identity_keys().curve25519; @@ -753,13 +754,16 @@ create_inbound_megolm_session(const mtx::events::DeviceEventinit_inbound_group_session(roomKey.content.session_key); + GroupSessionData data{}; data.forwarding_curve25519_key_chain = {sender_key}; data.sender_claimed_ed25519_key = sender_ed25519; data.sender_key = sender_key; - auto megolm_session = - olm::client()->init_inbound_group_session(roomKey.content.session_key); + data.trusted = olm_inbound_group_session_is_verified(megolm_session.get()); + backup_session_key(index, data, megolm_session); cache::saveInboundMegolmSession(index, std::move(megolm_session), data); } catch (const lmdb::error &e) { @@ -792,14 +796,9 @@ import_inbound_megolm_session( data.forwarding_curve25519_key_chain = roomKey.content.forwarding_curve25519_key_chain; data.sender_claimed_ed25519_key = roomKey.content.sender_claimed_ed25519_key; data.sender_key = roomKey.content.sender_key; - // may have come from online key backup, so we can't trust it... - data.trusted = false; - // if we got it forwarded from the sender, assume it is trusted. They may still have - // used key backup, but it is unlikely. - if (roomKey.content.forwarding_curve25519_key_chain.size() == 1 && - roomKey.content.forwarding_curve25519_key_chain.back() == roomKey.content.sender_key) { - data.trusted = true; - } + // Keys from online key backup won't have a signature, so they will be untrusted. But the + // original sender might send us a signed session. + data.trusted = olm_inbound_group_session_is_verified(megolm_session.get()); backup_session_key(index, data, megolm_session); cache::saveInboundMegolmSession(index, std::move(megolm_session), data); @@ -1023,6 +1022,7 @@ lookup_keybackup(const std::string &room, const std::string &session_id) data.forwarding_curve25519_key_chain = session.forwarding_curve25519_key_chain; data.sender_claimed_ed25519_key = session.sender_claimed_keys["ed25519"]; data.sender_key = session.sender_key; + // online key backup can't be trusted, because anyone can upload to it. data.trusted = false; @@ -1285,15 +1285,33 @@ decryptEvent(const MegolmSessionIndex &index, } crypto::Trust -calculate_trust(const std::string &user_id, const MegolmSessionIndex &index) +calculate_trust(const std::string &user_id, + const std::string &room_id, + const mtx::events::msg::Encrypted &event) { - auto status = cache::client()->verificationStatus(user_id); + auto index = MegolmSessionIndex(room_id, event); auto megolmData = cache::client()->getMegolmSessionData(index); - crypto::Trust trustlevel = crypto::Trust::Unverified; + crypto::Trust trustlevel = crypto::Trust::MessageUnverified; + + try { + auto session = cache::client()->getInboundMegolmSession(index); + if (!session) { + return trustlevel; + } + + olm::client()->decrypt_group_message(session.get(), event.ciphertext); + } catch (const lmdb::error &e) { + return trustlevel; + } catch (const mtx::crypto::olm_exception &e) { + return trustlevel; + } + + auto status = cache::client()->verificationStatus(user_id); if (megolmData && megolmData->trusted && - status.verified_device_keys.count(megolmData->sender_key)) + status.verified_device_keys.count(megolmData->sender_key)) { trustlevel = status.verified_device_keys.at(megolmData->sender_key); + } return trustlevel; } diff --git a/src/encryption/Olm.h b/src/encryption/Olm.h index 252d08b4..58760005 100644 --- a/src/encryption/Olm.h +++ b/src/encryption/Olm.h @@ -96,7 +96,9 @@ decryptEvent(const MegolmSessionIndex &index, const mtx::events::EncryptedEvent &event, bool dont_write_db = false); crypto::Trust -calculate_trust(const std::string &user_id, const MegolmSessionIndex &index); +calculate_trust(const std::string &user_id, + const std::string &room_id, + const mtx::events::msg::Encrypted &event); void mark_keys_as_published(); diff --git a/src/timeline/TimelineModel.cpp b/src/timeline/TimelineModel.cpp index f4f2361c..ab0e3aef 100644 --- a/src/timeline/TimelineModel.cpp +++ b/src/timeline/TimelineModel.cpp @@ -854,8 +854,7 @@ TimelineModel::data(const mtx::events::collections::TimelineEvents &event, int r std::get_if>( &*encrypted_event)) { return olm::calculate_trust( - encrypted->sender, - MegolmSessionIndex(room_id_.toStdString(), encrypted->content)); + encrypted->sender, room_id_.toStdString(), encrypted->content); } } return crypto::Trust::Unverified;