From 64d5a193f1059cac8225df7b7c3c9eaedfcfa41a Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Tue, 6 Oct 2020 17:02:41 +0200 Subject: [PATCH] Fix in room verification --- .../DeviceVerification.qml | 2 + src/DeviceVerificationFlow.cpp | 75 +++++++---- src/DeviceVerificationFlow.h | 7 +- src/timeline/EventStore.cpp | 118 ++++++++---------- src/timeline/EventStore.h | 8 +- 5 files changed, 103 insertions(+), 107 deletions(-) diff --git a/resources/qml/device-verification/DeviceVerification.qml b/resources/qml/device-verification/DeviceVerification.qml index 64f10b3..d6185a0 100644 --- a/resources/qml/device-verification/DeviceVerification.qml +++ b/resources/qml/device-verification/DeviceVerification.qml @@ -7,6 +7,8 @@ import im.nheko 1.0 ApplicationWindow { property var flow + onClosing: TimelineManager.removeVerificationFlow(flow) + title: stack.currentItem.title id: dialog diff --git a/src/DeviceVerificationFlow.cpp b/src/DeviceVerificationFlow.cpp index 79f1de8..549569f 100644 --- a/src/DeviceVerificationFlow.cpp +++ b/src/DeviceVerificationFlow.cpp @@ -13,6 +13,15 @@ static constexpr int TIMEOUT = 2 * 60 * 1000; // 2 minutes namespace msgs = mtx::events::msg; +static mtx::events::msg::KeyVerificationMac +key_verification_mac(mtx::crypto::SAS *sas, + mtx::identifiers::User sender, + const std::string &senderDevice, + mtx::identifiers::User receiver, + const std::string &receiverDevice, + const std::string &transactionId, + std::map keys); + DeviceVerificationFlow::DeviceVerificationFlow(QObject *, DeviceVerificationFlow::Type flow_type, TimelineModel *model, @@ -45,11 +54,7 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *, return; } - for (const auto &[algorithm, key] : - res.device_keys.at(deviceId.toStdString()).keys) { - // TODO: Verify Signatures - this->device_keys[algorithm] = key; - } + this->their_keys = res; }); if (model) { @@ -115,7 +120,7 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *, return; } error_ = User; - Emit errorChanged(); + emit errorChanged(); setState(Failed); }); @@ -160,6 +165,8 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *, "|" + this->transaction_id; } + nhlog::ui()->info("Info is: '{}'", info); + if (this->sender == false) { this->sendVerificationKey(); } else { @@ -193,28 +200,40 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *, if (msg.relates_to.value().event_id != this->relation.event_id) return; } - std::string info = "MATRIX_KEY_VERIFICATION_MAC" + this->toClient.to_string() + - this->deviceId.toStdString() + - http::client()->user_id().to_string() + - http::client()->device_id() + this->transaction_id; - std::vector key_list; + std::map key_list; std::string key_string; - for (auto mac : msg.mac) { - key_string += mac.first + ","; - if (device_keys[mac.first] != "") { - if (mac.second == - this->sas->calculate_mac(this->device_keys[mac.first], - info + mac.first)) { - } else { - this->cancelVerification( - DeviceVerificationFlow::Error::KeyMismatch); - return; - } + for (const auto &mac : msg.mac) { + for (const auto &[deviceid, key] : their_keys.device_keys) + if (key.keys.count(mac.first)) + key_list[mac.first] = key.keys.at(mac.first); + + if (their_keys.master_keys.keys.count(mac.first)) + key_list[mac.first] = their_keys.master_keys.keys[mac.first]; + if (their_keys.user_signing_keys.keys.count(mac.first)) + key_list[mac.first] = + their_keys.user_signing_keys.keys[mac.first]; + if (their_keys.self_signing_keys.keys.count(mac.first)) + key_list[mac.first] = + their_keys.self_signing_keys.keys[mac.first]; + } + auto macs = key_verification_mac(sas.get(), + toClient, + this->deviceId.toStdString(), + http::client()->user_id(), + http::client()->device_id(), + this->transaction_id, + key_list); + + for (const auto &[key, mac] : macs.mac) { + if (mac != msg.mac.at(key)) { + this->cancelVerification( + DeviceVerificationFlow::Error::KeyMismatch); + return; } } - key_string = key_string.substr(0, key_string.length() - 1); - if (msg.keys == this->sas->calculate_mac(key_string, info + "KEY_IDS")) { + + if (msg.keys == macs.keys) { this->isMacVerified = true; this->acceptDevice(); } else { @@ -630,9 +649,13 @@ DeviceVerificationFlow::NewInRoomVerification(QObject *parent_, QString event_id_) { QSharedPointer flow( - new DeviceVerificationFlow(parent_, Type::RoomMsg, timelineModel_, other_user_, "")); + new DeviceVerificationFlow(parent_, + Type::RoomMsg, + timelineModel_, + other_user_, + QString::fromStdString(msg.from_device))); - flow->event_id = event_id_.toStdString(); + flow->setEventId(event_id_.toStdString()); if (std::find(msg.methods.begin(), msg.methods.end(), diff --git a/src/DeviceVerificationFlow.h b/src/DeviceVerificationFlow.h index a1ceaf8..9ca45a9 100644 --- a/src/DeviceVerificationFlow.h +++ b/src/DeviceVerificationFlow.h @@ -56,7 +56,6 @@ using sas_ptr = std::unique_ptr; class DeviceVerificationFlow : public QObject { Q_OBJECT - // Q_CLASSINFO("RegisterEnumClassesUnscoped", "false") Q_PROPERTY(QString state READ state NOTIFY stateChanged) Q_PROPERTY(Error error READ error NOTIFY errorChanged) Q_PROPERTY(QString userId READ getUserId CONSTANT) @@ -179,11 +178,7 @@ private: //! Completes the verification flow void acceptDevice(); - // for to_device messages std::string transaction_id; - // for room messages - std::optional room_id; - std::optional event_id; bool sender; Type type; @@ -198,7 +193,7 @@ private: nlohmann::json canonical_json; std::vector sasList; - std::map device_keys; + UserKeyCache their_keys; TimelineModel *model_; mtx::common::RelatesTo relation; diff --git a/src/timeline/EventStore.cpp b/src/timeline/EventStore.cpp index dc92a37..141d2ec 100644 --- a/src/timeline/EventStore.cpp +++ b/src/timeline/EventStore.cpp @@ -280,8 +280,6 @@ EventStore::handleSync(const mtx::responses::Timeline &events) } } - handle_room_verification(event); - // decrypting and checking some encrypted messages if (auto encrypted = std::get_if>( @@ -292,81 +290,65 @@ EventStore::handleSync(const mtx::responses::Timeline &events) [](auto e) { return (e.sender != utils::localUser().toStdString()); }, *d_event)) { handle_room_verification(*d_event); - } else { - // only the key.verification.ready sent by localuser's other device - // is of significance as it is used for detecting accepted request - if (std::get_if>(d_event)) { - auto msg = std::get_if>(d_event); - ChatPage::instance()->receivedDeviceVerificationReady( - msg->content); - } } + // else { + // // only the key.verification.ready sent by localuser's other + // device + // // is of significance as it is used for detecting accepted request + // if (std::get_if>(d_event)) { + // auto msg = std::get_if>(d_event); + // ChatPage::instance()->receivedDeviceVerificationReady( + // msg->content); + // } + //} } } +} - if (last_verification_request_event.has_value()) { - if (last_verification_request_event.value().origin_server_ts > - last_verification_cancel_event.origin_server_ts) { - emit startDMVerification(last_verification_request_event.value()); - last_verification_request_event = {}; - } - } +namespace { +template +struct overloaded : Ts... +{ + using Ts::operator()...; +}; +template +overloaded(Ts...) -> overloaded; } void EventStore::handle_room_verification(mtx::events::collections::TimelineEvents event) { - if (std::get_if>(&event)) { - auto msg = - std::get>(event); - last_verification_request_event = msg; - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - last_verification_cancel_event = msg; - ChatPage::instance()->receivedDeviceVerificationCancel(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationAccept(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationKey(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationMac(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationReady(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationDone(msg.content); - return; - } else if (std::get_if>( - &event)) { - auto msg = - std::get>(event); - ChatPage::instance()->receivedDeviceVerificationStart(msg.content, msg.sender); - return; - } + std::visit( + overloaded{ + [this](const mtx::events::RoomEvent &msg) { + emit startDMVerification(msg); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationCancel(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationAccept(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationKey(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationMac(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationReady(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationDone(msg.content); + }, + [](const mtx::events::RoomEvent &msg) { + ChatPage::instance()->receivedDeviceVerificationStart(msg.content, msg.sender); + }, + [](const auto &) {}, + }, + event); } QVariantList diff --git a/src/timeline/EventStore.h b/src/timeline/EventStore.h index 89a5147..954e271 100644 --- a/src/timeline/EventStore.h +++ b/src/timeline/EventStore.h @@ -99,7 +99,7 @@ signals: void messageSent(std::string txn_id, std::string event_id); void messageFailed(std::string txn_id); void startDMVerification( - mtx::events::RoomEvent &msg); + const mtx::events::RoomEvent &msg); void updateFlowEventId(std::string event_id); public slots: @@ -123,10 +123,4 @@ private: std::string current_txn; int current_txn_error_count = 0; - - // probably not the best way to do - std::optional> - last_verification_request_event; - mtx::events::RoomEvent - last_verification_cancel_event; };