Fix verification with multiple devices in parallel

Fixes #1125
This commit is contained in:
Nicolas Werner 2024-01-15 02:16:04 +01:00
parent 4058a99c6a
commit 51236c3260
No known key found for this signature in database
GPG key ID: C8D75E610773F2D9
3 changed files with 39 additions and 5 deletions

View file

@ -29,6 +29,8 @@ ColumnLayout {
return qsTr("Device verification timed out."); return qsTr("Device verification timed out.");
case DeviceVerificationFlow.User: case DeviceVerificationFlow.User:
return qsTr("Other party canceled the verification."); return qsTr("Other party canceled the verification.");
case DeviceVerificationFlow.AcceptedOnOtherDevice:
return qsTr("The verification was accepted by a different device.");
case DeviceVerificationFlow.OutOfOrder: case DeviceVerificationFlow.OutOfOrder:
return qsTr("Verification messages received out of order!"); return qsTr("Verification messages received out of order!");
default: default:

View file

@ -105,6 +105,9 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
&ChatPage::receivedDeviceVerificationAccept, &ChatPage::receivedDeviceVerificationAccept,
this, this,
[this](const mtx::events::msg::KeyVerificationAccept &msg) { [this](const mtx::events::msg::KeyVerificationAccept &msg) {
if (state_ == Failed || state_ == Success)
return;
nhlog::crypto()->info("verification: received accept with mac methods {}", nhlog::crypto()->info("verification: received accept with mac methods {}",
fmt::join(msg.message_authentication_code, ", ")); fmt::join(msg.message_authentication_code, ", "));
if (msg.transaction_id.has_value()) { if (msg.transaction_id.has_value()) {
@ -114,6 +117,7 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
if (msg.relations.references() != this->relation.event_id) if (msg.relations.references() != this->relation.event_id)
return; return;
} }
if (msg.key_agreement_protocol == "curve25519-hkdf-sha256" && if (msg.key_agreement_protocol == "curve25519-hkdf-sha256" &&
msg.hash == "sha256" && msg.hash == "sha256" &&
(msg.message_authentication_code == mac_method_alg_v1 || (msg.message_authentication_code == mac_method_alg_v1 ||
@ -157,6 +161,9 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
&ChatPage::receivedDeviceVerificationKey, &ChatPage::receivedDeviceVerificationKey,
this, this,
[this](const mtx::events::msg::KeyVerificationKey &msg) { [this](const mtx::events::msg::KeyVerificationKey &msg) {
if (state_ == Failed || state_ == Success)
return;
nhlog::crypto()->info( nhlog::crypto()->info(
"verification: received key, sender {}, state {}", sender, state().toStdString()); "verification: received key, sender {}, state {}", sender, state().toStdString());
if (msg.transaction_id.has_value()) { if (msg.transaction_id.has_value()) {
@ -219,6 +226,9 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
&ChatPage::receivedDeviceVerificationMac, &ChatPage::receivedDeviceVerificationMac,
this, this,
[this](const mtx::events::msg::KeyVerificationMac &msg) { [this](const mtx::events::msg::KeyVerificationMac &msg) {
if (state_ == Failed || state_ == Success)
return;
nhlog::crypto()->info("verification: received mac"); nhlog::crypto()->info("verification: received mac");
if (msg.transaction_id.has_value()) { if (msg.transaction_id.has_value()) {
if (msg.transaction_id.value() != this->transaction_id) if (msg.transaction_id.value() != this->transaction_id)
@ -326,6 +336,7 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
} }
if (!req.signatures.empty()) { if (!req.signatures.empty()) {
nhlog::crypto()->debug("Signatures to send: {}", nlohmann::json(req).dump(2));
http::client()->keys_signatures_upload( http::client()->keys_signatures_upload(
req, req,
[](const mtx::responses::KeySignaturesUpload &res, mtx::http::RequestErr err) { [](const mtx::responses::KeySignaturesUpload &res, mtx::http::RequestErr err) {
@ -374,12 +385,16 @@ DeviceVerificationFlow::DeviceVerificationFlow(QObject *,
&ChatPage::receivedDeviceVerificationReady, &ChatPage::receivedDeviceVerificationReady,
this, this,
[this](const mtx::events::msg::KeyVerificationReady &msg) { [this](const mtx::events::msg::KeyVerificationReady &msg) {
if (state_ == Failed || state_ == Success)
return;
nhlog::crypto()->info("verification: received ready {}", (void *)this); nhlog::crypto()->info("verification: received ready {}", (void *)this);
if (!sender) { if (!sender) {
if (msg.from_device != http::client()->device_id()) { if (msg.from_device != this->deviceId.toStdString()) {
error_ = User; nhlog::crypto()->debug("Accepted by {}, we are communicating with {}",
emit errorChanged(); msg.from_device,
setState(Failed); this->deviceId.toStdString());
cancelVerification(AcceptedOnOtherDevice);
} }
return; return;
@ -558,6 +573,9 @@ void
DeviceVerificationFlow::handleStartMessage(const mtx::events::msg::KeyVerificationStart &msg, DeviceVerificationFlow::handleStartMessage(const mtx::events::msg::KeyVerificationStart &msg,
std::string) std::string)
{ {
if (state_ == Failed || state_ == Success)
return;
if (msg.transaction_id.has_value()) { if (msg.transaction_id.has_value()) {
if (msg.transaction_id.value() != this->transaction_id) if (msg.transaction_id.value() != this->transaction_id)
return; return;
@ -568,6 +586,14 @@ DeviceVerificationFlow::handleStartMessage(const mtx::events::msg::KeyVerificati
return; return;
} }
if (state_ == Failed)
return;
if (msg.from_device != this->deviceId.toStdString()) {
cancelVerification(AcceptedOnOtherDevice);
return;
}
nhlog::crypto()->info("verification: received start with mac methods {}", nhlog::crypto()->info("verification: received start with mac methods {}",
fmt::join(msg.message_authentication_codes, ", ")); fmt::join(msg.message_authentication_codes, ", "));
@ -625,6 +651,9 @@ DeviceVerificationFlow::handleStartMessage(const mtx::events::msg::KeyVerificati
void void
DeviceVerificationFlow::acceptVerificationRequest() DeviceVerificationFlow::acceptVerificationRequest()
{ {
if (state_ == Failed || state_ == Success)
return;
if (acceptSent) if (acceptSent)
return; return;
@ -769,6 +798,8 @@ DeviceVerificationFlow::cancelVerification(DeviceVerificationFlow::Error error_c
this->setState(Failed); this->setState(Failed);
emit errorChanged(); emit errorChanged();
// Don't cancel if the user accepted the request elsewhere, instead just silently stop
if (error_code != AcceptedOnOtherDevice)
send(req); send(req);
} }
//! sends the verification key //! sends the verification key

View file

@ -104,6 +104,7 @@ public:
KeyMismatch, KeyMismatch,
Timeout, Timeout,
User, User,
AcceptedOnOtherDevice,
OutOfOrder, OutOfOrder,
}; };
Q_ENUM(Error) Q_ENUM(Error)