From ecf6b855b0fc8cbe16d34b67ae1dca7bd9b69948 Mon Sep 17 00:00:00 2001 From: Tobias Fella Date: Tue, 6 Sep 2022 21:35:27 +0200 Subject: Add autotest for key verification and fix several edge-cases --- autotests/CMakeLists.txt | 1 + autotests/testkeyverification.cpp | 50 +++++++++++++++++++++++++++++++++++++++ autotests/testkeyverification.h | 18 ++++++++++++++ autotests/testolmaccount.cpp | 22 ++--------------- autotests/testutils.h | 32 +++++++++++++++++++++++++ lib/keyverificationsession.cpp | 15 +++++++++--- lib/keyverificationsession.h | 7 ++++++ 7 files changed, 122 insertions(+), 23 deletions(-) create mode 100644 autotests/testkeyverification.cpp create mode 100644 autotests/testkeyverification.h create mode 100644 autotests/testutils.h diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index c11901bf..48edb168 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -19,4 +19,5 @@ if(${PROJECT_NAME}_ENABLE_E2EE) quotient_add_test(NAME testolmsession) quotient_add_test(NAME testolmutility) quotient_add_test(NAME testfilecrypto) + quotient_add_test(NAME testkeyverification) endif() diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp new file mode 100644 index 00000000..fbbb7614 --- /dev/null +++ b/autotests/testkeyverification.cpp @@ -0,0 +1,50 @@ +// SPDX-FileCopyrightText: 2022 Tobias Fella +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#include "testkeyverification.h" +#include "qt_connection_util.h" + +void TestKeyVerificationSession::testVerification() +{ + CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop") + CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone") + + KeyVerificationSession *aSession = nullptr; + connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) { + aSession = session; + QVERIFY(session->remoteDeviceId() == b->deviceId()); + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORREADY); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ + QVERIFY(session->state() == KeyVerificationSession::ACCEPTED); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); + session->sendMac(); + }); + }); + }); + a->startKeyVerificationSession(b->deviceId()); + connect(b.get(), &Connection::newKeyVerificationSession, this, [=](KeyVerificationSession* session) { + QVERIFY(session->remoteDeviceId() == a->deviceId()); + QVERIFY(session->state() == KeyVerificationSession::INCOMING); + session->sendReady(); + // KeyVerificationSession::READY is skipped because we have only one method + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORACCEPT); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORKEY || session->state() == KeyVerificationSession::ACCEPTED); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=]() { + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); + QVERIFY(aSession); + QVERIFY(aSession->sasEmojis() == session->sasEmojis()); + session->sendMac(); + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORMAC); + }); + }); + + }); + b->syncLoop(); + a->syncLoop(); + QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone); + spy.wait(10000); +} +QTEST_GUILESS_MAIN(TestKeyVerificationSession) diff --git a/autotests/testkeyverification.h b/autotests/testkeyverification.h new file mode 100644 index 00000000..28e00e11 --- /dev/null +++ b/autotests/testkeyverification.h @@ -0,0 +1,18 @@ +// SPDX-FileCopyrightText: 2022 Tobias Fella +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#pragma once + +#include +#include "testutils.h" + +class TestKeyVerificationSession : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void testVerification(); + +}; + diff --git a/autotests/testolmaccount.cpp b/autotests/testolmaccount.cpp index 4b32393d..280705d0 100644 --- a/autotests/testolmaccount.cpp +++ b/autotests/testolmaccount.cpp @@ -14,6 +14,8 @@ #include #include +#include "testutils.h" + using namespace Quotient; void TestOlmAccount::pickleUnpickledTest() @@ -168,26 +170,6 @@ void TestOlmAccount::encryptedFile() QCOMPARE(file.key.kty, "oct"); } -#define CREATE_CONNECTION(VAR, USERNAME, SECRET, DEVICE_NAME) \ - NetworkAccessManager::instance()->ignoreSslErrors(true); \ - auto VAR = std::make_shared(); \ - (VAR)->resolveServer("@" USERNAME ":localhost:1234"); \ - connect((VAR).get(), &Connection::loginFlowsChanged, this, [=] { \ - (VAR)->loginWithPassword((USERNAME), SECRET, DEVICE_NAME, ""); \ - }); \ - connect((VAR).get(), &Connection::networkError, [](const QString& error) { \ - QWARN(qUtf8Printable(error)); \ - QFAIL("Network error: make sure synapse is running"); \ - }); \ - connect((VAR).get(), &Connection::loginError, [](const QString& error) { \ - QWARN(qUtf8Printable(error)); \ - QFAIL("Login failed"); \ - }); \ - QSignalSpy spy##VAR((VAR).get(), &Connection::loginFlowsChanged); \ - QSignalSpy spy2##VAR((VAR).get(), &Connection::connected); \ - QVERIFY(spy##VAR.wait(10000)); \ - QVERIFY(spy2##VAR.wait(10000)); - void TestOlmAccount::uploadIdentityKey() { CREATE_CONNECTION(conn, "alice1", "secret", "AlicePhone") diff --git a/autotests/testutils.h b/autotests/testutils.h new file mode 100644 index 00000000..7d016a34 --- /dev/null +++ b/autotests/testutils.h @@ -0,0 +1,32 @@ +// SPDX-FileCopyrightText: 2021 Carl Schwan +// +// SPDX-License-Identifier: LGPL-2.1-or-later + +#pragma once + +#include +#include + +#include + +using namespace Quotient; + +#define CREATE_CONNECTION(VAR, USERNAME, SECRET, DEVICE_NAME) \ + NetworkAccessManager::instance()->ignoreSslErrors(true); \ + auto VAR = std::make_shared(); \ + (VAR)->resolveServer("@" USERNAME ":localhost:1234"); \ + connect((VAR).get(), &Connection::loginFlowsChanged, this, [=] { \ + (VAR)->loginWithPassword((USERNAME), SECRET, DEVICE_NAME, ""); \ + }); \ + connect((VAR).get(), &Connection::networkError, [](const QString& error) { \ + QWARN(qUtf8Printable(error)); \ + QFAIL("Network error: make sure synapse is running"); \ + }); \ + connect((VAR).get(), &Connection::loginError, [](const QString& error) { \ + QWARN(qUtf8Printable(error)); \ + QFAIL("Login failed"); \ + }); \ + QSignalSpy spy##VAR((VAR).get(), &Connection::loginFlowsChanged); \ + QSignalSpy spy2##VAR((VAR).get(), &Connection::connected); \ + QVERIFY(spy##VAR.wait(10000)); \ + QVERIFY(spy2##VAR.wait(10000)); diff --git a/lib/keyverificationsession.cpp b/lib/keyverificationsession.cpp index 44a085fe..24fc08e1 100644 --- a/lib/keyverificationsession.cpp +++ b/lib/keyverificationsession.cpp @@ -154,7 +154,7 @@ EmojiEntry emojiForCode(int code, const QString& language) void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event) { - if (state() != WAITINGFORKEY && state() != WAITINGFORVERIFICATION) { + if (state() != WAITINGFORKEY && state() != ACCEPTED) { cancelVerification(UNEXPECTED_MESSAGE); return; } @@ -176,7 +176,6 @@ void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event) } else { sendKey(); } - setState(WAITINGFORVERIFICATION); std::string key(olm_sas_pubkey_length(m_sas), '\0'); olm_sas_get_pubkey(m_sas, key.data(), key.size()); @@ -214,6 +213,7 @@ void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event) emit sasEmojisChanged(); emit keyReceived(); + setState(WAITINGFORVERIFICATION); } QString KeyVerificationSession::calculateMac(const QString& input, @@ -314,6 +314,10 @@ void KeyVerificationSession::sendStartSas() void KeyVerificationSession::handleReady(const KeyVerificationReadyEvent& event) { + if (state() == ACCEPTED) { + // It's possible to receive ready and start in the same sync, in which case start might be handled before ready. + return; + } if (state() != WAITINGFORREADY) { cancelVerification(UNEXPECTED_MESSAGE); return; @@ -334,7 +338,7 @@ void KeyVerificationSession::handleReady(const KeyVerificationReadyEvent& event) void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event) { - if (state() != READY) { + if (state() != READY && state() != WAITINGFORREADY) { cancelVerification(UNEXPECTED_MESSAGE); return; } @@ -510,3 +514,8 @@ KeyVerificationSession::Error KeyVerificationSession::stringToError(const QStrin } return NONE; } + +QString KeyVerificationSession::remoteDeviceId() const +{ + return m_remoteDeviceId; +} diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h index aa0295cb..31f63453 100644 --- a/lib/keyverificationsession.h +++ b/lib/keyverificationsession.h @@ -19,6 +19,11 @@ struct QUOTIENT_API EmojiEntry { Q_GADGET Q_PROPERTY(QString emoji MEMBER emoji CONSTANT) Q_PROPERTY(QString description MEMBER description CONSTANT) + +public: + bool operator==(const EmojiEntry& rhs) const { + return emoji == rhs.emoji && description == rhs.description; + } }; /** A key verification session. Listen for incoming sessions by connecting to Connection::newKeyVerificationSession. @@ -93,6 +98,8 @@ public: Error error() const; + QString remoteDeviceId() const; + public Q_SLOTS: void sendRequest(); void sendReady(); -- cgit v1.2.3 From 83746abdf677f573287be2c93d2ad12b1aa84b3f Mon Sep 17 00:00:00 2001 From: Tobias Fella <9750016+TobiasFella@users.noreply.github.com> Date: Sat, 10 Sep 2022 13:42:39 +0200 Subject: Update lib/keyverificationsession.h Co-authored-by: Alexey Rusakov --- lib/keyverificationsession.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h index 31f63453..b2e3b36d 100644 --- a/lib/keyverificationsession.h +++ b/lib/keyverificationsession.h @@ -21,9 +21,7 @@ struct QUOTIENT_API EmojiEntry { Q_PROPERTY(QString description MEMBER description CONSTANT) public: - bool operator==(const EmojiEntry& rhs) const { - return emoji == rhs.emoji && description == rhs.description; - } + bool operator==(const EmojiEntry& rhs) const = default; }; /** A key verification session. Listen for incoming sessions by connecting to Connection::newKeyVerificationSession. -- cgit v1.2.3 From fa34778cc377ab98a0b8f3944063e426c73a3941 Mon Sep 17 00:00:00 2001 From: Tobias Fella Date: Sat, 10 Sep 2022 13:50:13 +0200 Subject: Remove header for keyverificationtest --- autotests/testkeyverification.cpp | 83 ++++++++++++++++++++++----------------- autotests/testkeyverification.h | 18 --------- 2 files changed, 46 insertions(+), 55 deletions(-) delete mode 100644 autotests/testkeyverification.h diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp index fbbb7614..59a1c934 100644 --- a/autotests/testkeyverification.cpp +++ b/autotests/testkeyverification.cpp @@ -2,49 +2,58 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later -#include "testkeyverification.h" -#include "qt_connection_util.h" -void TestKeyVerificationSession::testVerification() +#include +#include "testutils.h" +#include + +class TestKeyVerificationSession : public QObject { - CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop") - CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone") + Q_OBJECT + +private Q_SLOTS: + void testVerification() + { + CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop") + CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone") - KeyVerificationSession *aSession = nullptr; - connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) { - aSession = session; - QVERIFY(session->remoteDeviceId() == b->deviceId()); - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORREADY); - connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ - QVERIFY(session->state() == KeyVerificationSession::ACCEPTED); + KeyVerificationSession *aSession = nullptr; + connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) { + aSession = session; + QVERIFY(session->remoteDeviceId() == b->deviceId()); + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORREADY); connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); - session->sendMac(); + QVERIFY(session->state() == KeyVerificationSession::ACCEPTED); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); + session->sendMac(); + }); }); }); - }); - a->startKeyVerificationSession(b->deviceId()); - connect(b.get(), &Connection::newKeyVerificationSession, this, [=](KeyVerificationSession* session) { - QVERIFY(session->remoteDeviceId() == a->deviceId()); - QVERIFY(session->state() == KeyVerificationSession::INCOMING); - session->sendReady(); - // KeyVerificationSession::READY is skipped because we have only one method - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORACCEPT); - connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORKEY || session->state() == KeyVerificationSession::ACCEPTED); - connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=]() { - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); - QVERIFY(aSession); - QVERIFY(aSession->sasEmojis() == session->sasEmojis()); - session->sendMac(); - QVERIFY(session->state() == KeyVerificationSession::WAITINGFORMAC); + a->startKeyVerificationSession(b->deviceId()); + connect(b.get(), &Connection::newKeyVerificationSession, this, [=](KeyVerificationSession* session) { + QVERIFY(session->remoteDeviceId() == a->deviceId()); + QVERIFY(session->state() == KeyVerificationSession::INCOMING); + session->sendReady(); + // KeyVerificationSession::READY is skipped because we have only one method + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORACCEPT); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORKEY || session->state() == KeyVerificationSession::ACCEPTED); + connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=]() { + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); + QVERIFY(aSession); + QVERIFY(aSession->sasEmojis() == session->sasEmojis()); + session->sendMac(); + QVERIFY(session->state() == KeyVerificationSession::WAITINGFORMAC); + }); }); - }); - }); - b->syncLoop(); - a->syncLoop(); - QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone); - spy.wait(10000); -} + }); + b->syncLoop(); + a->syncLoop(); + QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone); + spy.wait(10000); + } +}; QTEST_GUILESS_MAIN(TestKeyVerificationSession) +#include "testkeyverification.moc" diff --git a/autotests/testkeyverification.h b/autotests/testkeyverification.h deleted file mode 100644 index 28e00e11..00000000 --- a/autotests/testkeyverification.h +++ /dev/null @@ -1,18 +0,0 @@ -// SPDX-FileCopyrightText: 2022 Tobias Fella -// -// SPDX-License-Identifier: LGPL-2.1-or-later - -#pragma once - -#include -#include "testutils.h" - -class TestKeyVerificationSession : public QObject -{ - Q_OBJECT - -private Q_SLOTS: - void testVerification(); - -}; - -- cgit v1.2.3 From 6abdd5358c9c5ed89cda5bc5e50a76af423b0634 Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 11 Sep 2022 20:25:46 +0200 Subject: KeyVerificationEvent; KeyVerificationSession::handleEvent() Key verification events gain their own base type and KeyVerificationSession gets a single point of entry for all kinds of incoming events. This allows to drop a pile of `incoming*` signals in Connection and a stack of options inside switchOnType in processIfVerification(). KVS::handleEvent() also makes (some) allowed state transitions a bit clearer. --- autotests/testkeyverification.cpp | 4 +- lib/connection.cpp | 44 +++++-------- lib/connection.h | 11 +--- lib/events/keyverificationevent.h | 74 ++++++++------------- lib/keyverificationsession.cpp | 133 ++++++++++++++------------------------ lib/keyverificationsession.h | 6 +- 6 files changed, 102 insertions(+), 170 deletions(-) diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp index 59a1c934..96aad6c7 100644 --- a/autotests/testkeyverification.cpp +++ b/autotests/testkeyverification.cpp @@ -17,7 +17,7 @@ private Q_SLOTS: CREATE_CONNECTION(a, "alice1", "secret", "AliceDesktop") CREATE_CONNECTION(b, "alice1", "secret", "AlicePhone") - KeyVerificationSession *aSession = nullptr; + QPointer aSession{}; connect(a.get(), &Connection::newKeyVerificationSession, this, [&](KeyVerificationSession* session) { aSession = session; QVERIFY(session->remoteDeviceId() == b->deviceId()); @@ -51,7 +51,7 @@ private Q_SLOTS: }); b->syncLoop(); a->syncLoop(); - QSignalSpy spy(b.get(), &Connection::incomingKeyVerificationDone); + QSignalSpy spy(aSession, &KeyVerificationSession::finished); spy.wait(10000); } }; diff --git a/lib/connection.cpp b/lib/connection.cpp index 53c99969..5003f40c 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -124,6 +124,7 @@ public: // A map from SenderKey to vector of InboundSession UnorderedMap> olmSessions; + QHash verificationSessions; #endif GetCapabilitiesJob* capabilitiesJob = nullptr; @@ -971,6 +972,8 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents) qCDebug(E2EE) << "Consuming" << toDeviceEvents.size() << "to-device events"; for (auto&& tdEvt : toDeviceEvents) { + if (processIfVerificationEvent(*tdEvt, false)) + continue; if (auto&& event = eventCast(std::move(tdEvt))) { if (event->algorithm() != OlmV1Curve25519AesSha2AlgoKey) { qCDebug(E2EE) << "Unsupported algorithm" << event->id() @@ -985,9 +988,7 @@ void Connection::Private::consumeToDeviceEvents(Events&& toDeviceEvents) outdatedUsers += event->senderId(); encryptionUpdateRequired = true; pendingEncryptedEvents.push_back(std::move(event)); - continue; } - processIfVerificationEvent(*tdEvt, false); } } #endif @@ -998,33 +999,22 @@ bool Connection::Private::processIfVerificationEvent(const Event& evt, bool encrypted) { return switchOnType(evt, - [this, encrypted](const KeyVerificationRequestEvent& event) { - auto session = - new KeyVerificationSession(q->userId(), event, q, encrypted); - emit q->newKeyVerificationSession(session); - return true; - }, [this](const KeyVerificationReadyEvent& event) { - emit q->incomingKeyVerificationReady(event); - return true; - }, [this](const KeyVerificationStartEvent& event) { - emit q->incomingKeyVerificationStart(event); - return true; - }, [this](const KeyVerificationAcceptEvent& event) { - emit q->incomingKeyVerificationAccept(event); - return true; - }, [this](const KeyVerificationKeyEvent& event) { - emit q->incomingKeyVerificationKey(event); + [this, encrypted](const KeyVerificationRequestEvent& reqEvt) { + const auto sessionIter = verificationSessions.insert( + reqEvt.transactionId(), + new KeyVerificationSession(q->userId(), reqEvt, q, encrypted)); + emit q->newKeyVerificationSession(*sessionIter); return true; - }, [this](const KeyVerificationMacEvent& event) { - emit q->incomingKeyVerificationMac(event); - return true; - }, [this](const KeyVerificationDoneEvent& event) { - emit q->incomingKeyVerificationDone(event); - return true; - }, [this](const KeyVerificationCancelEvent& event) { - emit q->incomingKeyVerificationCancel(event); + }, + [this](const KeyVerificationEvent& kvEvt) { + if (auto* const session = + verificationSessions.value(kvEvt.transactionId())) { + session->handleEvent(kvEvt); + emit q->keyVerificationStateChanged(session, session->state()); + } return true; - }, false); + }, + false); } void Connection::Private::handleEncryptedToDeviceEvent(const EncryptedEvent& event) diff --git a/lib/connection.h b/lib/connection.h index 79c23c9a..75faf370 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -864,15 +864,10 @@ Q_SIGNALS: void devicesListLoaded(); #ifdef Quotient_E2EE_ENABLED - void incomingKeyVerificationReady(const KeyVerificationReadyEvent& event); - void incomingKeyVerificationStart(const KeyVerificationStartEvent& event); - void incomingKeyVerificationAccept(const KeyVerificationAcceptEvent& event); - void incomingKeyVerificationKey(const KeyVerificationKeyEvent& event); - void incomingKeyVerificationMac(const KeyVerificationMacEvent& event); - void incomingKeyVerificationDone(const KeyVerificationDoneEvent& event); - void incomingKeyVerificationCancel(const KeyVerificationCancelEvent& event); - void newKeyVerificationSession(KeyVerificationSession* session); + void keyVerificationStateChanged( + const KeyVerificationSession* session, + Quotient::KeyVerificationSession::State state); void sessionVerified(const QString& userId, const QString& deviceId); #endif diff --git a/lib/events/keyverificationevent.h b/lib/events/keyverificationevent.h index 0ffd8b2c..0e939508 100644 --- a/lib/events/keyverificationevent.h +++ b/lib/events/keyverificationevent.h @@ -9,13 +9,24 @@ namespace Quotient { static constexpr auto SasV1Method = "m.sas.v1"_ls; +class QUOTIENT_API KeyVerificationEvent : public Event { +public: + QUO_BASE_EVENT(KeyVerificationEvent, "m.key.*"_ls, Event::BaseMetaType) + + using Event::Event; + + /// An opaque identifier for the verification request. Must + /// be unique with respect to the devices involved. + QUO_CONTENT_GETTER(QString, transactionId) +}; + /// Requests a key verification with another user's devices. /// Typically sent as a to-device event. -class QUOTIENT_API KeyVerificationRequestEvent : public Event { +class QUOTIENT_API KeyVerificationRequestEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationRequestEvent, "m.key.verification.request") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationRequestEvent(const QString& transactionId, const QString& fromDevice, const QStringList& methods, @@ -30,10 +41,6 @@ public: /// The device ID which is initiating the request. QUO_CONTENT_GETTER(QString, fromDevice) - /// An opaque identifier for the verification request. Must - /// be unique with respect to the devices involved. - QUO_CONTENT_GETTER(QString, transactionId) - /// The verification methods supported by the sender. QUO_CONTENT_GETTER(QStringList, methods) @@ -44,11 +51,11 @@ public: QUO_CONTENT_GETTER(QDateTime, timestamp) }; -class QUOTIENT_API KeyVerificationReadyEvent : public Event { +class QUOTIENT_API KeyVerificationReadyEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationReadyEvent, "m.key.verification.ready") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationReadyEvent(const QString& transactionId, const QString& fromDevice, const QStringList& methods) @@ -61,19 +68,16 @@ public: /// The device ID which is accepting the request. QUO_CONTENT_GETTER(QString, fromDevice) - /// The transaction id of the verification request - QUO_CONTENT_GETTER(QString, transactionId) - /// The verification methods supported by the sender. QUO_CONTENT_GETTER(QStringList, methods) }; /// Begins a key verification process. -class QUOTIENT_API KeyVerificationStartEvent : public Event { +class QUOTIENT_API KeyVerificationStartEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationStartEvent, "m.key.verification.start") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationStartEvent(const QString& transactionId, const QString& fromDevice) : KeyVerificationStartEvent( @@ -92,10 +96,6 @@ public: /// The device ID which is initiating the process. QUO_CONTENT_GETTER(QString, fromDevice) - /// An opaque identifier for the verification request. Must - /// be unique with respect to the devices involved. - QUO_CONTENT_GETTER(QString, transactionId) - /// The verification method to use. QUO_CONTENT_GETTER(QString, method) @@ -140,11 +140,11 @@ public: /// Accepts a previously sent m.key.verification.start message. /// Typically sent as a to-device event. -class QUOTIENT_API KeyVerificationAcceptEvent : public Event { +class QUOTIENT_API KeyVerificationAcceptEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationAcceptEvent, "m.key.verification.accept") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationAcceptEvent(const QString& transactionId, const QString& commitment) : KeyVerificationAcceptEvent(basicJson( @@ -158,9 +158,6 @@ public: { "commitment"_ls, commitment } })) {} - /// An opaque identifier for the verification process. - QUO_CONTENT_GETTER(QString, transactionId) - /// The verification method to use. Must be 'm.sas.v1'. QUO_CONTENT_GETTER(QString, method) @@ -170,10 +167,7 @@ public: /// The hash method the device is choosing to use, out of the /// options in the m.key.verification.start message. - QString hashData() const - { - return contentPart("hash"_ls); - } + QUO_CONTENT_GETTER_X(QString, hashData, "hash"_ls) /// The message authentication code the device is choosing to use, out /// of the options in the m.key.verification.start message. @@ -188,11 +182,11 @@ public: QUO_CONTENT_GETTER(QString, commitment) }; -class QUOTIENT_API KeyVerificationCancelEvent : public Event { +class QUOTIENT_API KeyVerificationCancelEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationCancelEvent, "m.key.verification.cancel") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationCancelEvent(const QString& transactionId, const QString& reason) : KeyVerificationCancelEvent( @@ -203,9 +197,6 @@ public: })) {} - /// An opaque identifier for the verification process. - QUO_CONTENT_GETTER(QString, transactionId) - /// A human readable description of the code. The client should only /// rely on this string if it does not understand the code. QUO_CONTENT_GETTER(QString, reason) @@ -216,30 +207,27 @@ public: /// Sends the ephemeral public key for a device to the partner device. /// Typically sent as a to-device event. -class QUOTIENT_API KeyVerificationKeyEvent : public Event { +class KeyVerificationKeyEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationKeyEvent, "m.key.verification.key") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationKeyEvent(const QString& transactionId, const QString& key) : KeyVerificationKeyEvent( basicJson(TypeId, { { "transaction_id"_ls, transactionId }, { "key"_ls, key } })) {} - /// An opaque identifier for the verification process. - QUO_CONTENT_GETTER(QString, transactionId) - /// The device's ephemeral public key, encoded as unpadded base64. QUO_CONTENT_GETTER(QString, key) }; /// Sends the MAC of a device's key to the partner device. -class QUOTIENT_API KeyVerificationMacEvent : public Event { +class QUOTIENT_API KeyVerificationMacEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationMacEvent, "m.key.verification.mac") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; KeyVerificationMacEvent(const QString& transactionId, const QString& keys, const QJsonObject& mac) : KeyVerificationMacEvent( @@ -248,9 +236,6 @@ public: { "mac"_ls, mac } })) {} - /// An opaque identifier for the verification process. - QUO_CONTENT_GETTER(QString, transactionId) - /// The device's ephemeral public key, encoded as unpadded base64. QUO_CONTENT_GETTER(QString, keys) @@ -260,17 +245,14 @@ public: } }; -class QUOTIENT_API KeyVerificationDoneEvent : public Event { +class QUOTIENT_API KeyVerificationDoneEvent : public KeyVerificationEvent { public: QUO_EVENT(KeyVerificationDoneEvent, "m.key.verification.done") - using Event::Event; + using KeyVerificationEvent::KeyVerificationEvent; explicit KeyVerificationDoneEvent(const QString& transactionId) : KeyVerificationDoneEvent( basicJson(TypeId, { { "transaction_id"_ls, transactionId } })) {} - - /// The same transactionId as before - QUO_CONTENT_GETTER(QString, transactionId) }; } // namespace Quotient diff --git a/lib/keyverificationsession.cpp b/lib/keyverificationsession.cpp index 24fc08e1..cc4428d7 100644 --- a/lib/keyverificationsession.cpp +++ b/lib/keyverificationsession.cpp @@ -68,42 +68,6 @@ KeyVerificationSession::KeyVerificationSession(QString userId, QString deviceId, void KeyVerificationSession::init(milliseconds timeout) { - connect(m_connection, &Connection::incomingKeyVerificationReady, this, [this](const KeyVerificationReadyEvent& event) { - if (event.transactionId() == m_transactionId && event.fromDevice() == m_remoteDeviceId) { - handleReady(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationStart, this, [this](const KeyVerificationStartEvent& event) { - if (event.transactionId() == m_transactionId && event.fromDevice() == m_remoteDeviceId) { - handleStart(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationAccept, this, [this](const KeyVerificationAcceptEvent& event) { - if (event.transactionId() == m_transactionId) { - handleAccept(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationKey, this, [this](const KeyVerificationKeyEvent& event) { - if (event.transactionId() == m_transactionId) { - handleKey(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationMac, this, [this](const KeyVerificationMacEvent& event) { - if (event.transactionId() == m_transactionId) { - handleMac(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationDone, this, [this](const KeyVerificationDoneEvent& event) { - if (event.transactionId() == m_transactionId) { - handleDone(event); - } - }); - connect(m_connection, &Connection::incomingKeyVerificationCancel, this, [this](const KeyVerificationCancelEvent& event) { - if (event.transactionId() == m_transactionId) { - handleCancel(event); - } - }); - QTimer::singleShot(timeout, this, [this] { cancelVerification(TIMEOUT); }); m_sas = olm_sas(new std::byte[olm_sas_size()]); @@ -118,6 +82,53 @@ KeyVerificationSession::~KeyVerificationSession() delete[] reinterpret_cast(m_sas); } +void KeyVerificationSession::handleEvent(const KeyVerificationEvent& baseEvent) +{ + if (!switchOnType( + baseEvent, + [this](const KeyVerificationCancelEvent& event) { + setError(stringToError(event.code())); + setState(CANCELED); + return true; + }, + [this](const KeyVerificationStartEvent& event) { + if (state() != WAITINGFORREADY && state() != READY) + return false; + handleStart(event); + return true; + }, + [this](const KeyVerificationReadyEvent& event) { + if (state() == WAITINGFORREADY) + handleReady(event); + // ACCEPTED is also fine here because it's possible to receive + // ready and start in the same sync, in which case start might + // be handled before ready. + return state() == WAITINGFORREADY || state() == ACCEPTED; + }, + [this](const KeyVerificationAcceptEvent& event) { + if (state() != WAITINGFORACCEPT) + return false; + m_commitment = event.commitment(); + sendKey(); + setState(WAITINGFORKEY); + return true; + }, + [this](const KeyVerificationKeyEvent& event) { + if (state() != ACCEPTED && state() != WAITINGFORKEY) + return false; + handleKey(event); + return true; + }, + [this](const KeyVerificationMacEvent& event) { + if (state() != WAITINGFORMAC) + return false; + handleMac(event); + return true; + }, + [this](const KeyVerificationDoneEvent&) { return state() == DONE; })) + cancelVerification(UNEXPECTED_MESSAGE); +} + struct EmojiStoreEntry : EmojiEntry { QHash translatedDescriptions; @@ -154,10 +165,6 @@ EmojiEntry emojiForCode(int code, const QString& language) void KeyVerificationSession::handleKey(const KeyVerificationKeyEvent& event) { - if (state() != WAITINGFORKEY && state() != ACCEPTED) { - cancelVerification(UNEXPECTED_MESSAGE); - return; - } auto eventKey = event.key().toLatin1(); olm_sas_set_their_key(m_sas, eventKey.data(), eventKey.size()); @@ -314,34 +321,18 @@ void KeyVerificationSession::sendStartSas() void KeyVerificationSession::handleReady(const KeyVerificationReadyEvent& event) { - if (state() == ACCEPTED) { - // It's possible to receive ready and start in the same sync, in which case start might be handled before ready. - return; - } - if (state() != WAITINGFORREADY) { - cancelVerification(UNEXPECTED_MESSAGE); - return; - } setState(READY); m_remoteSupportedMethods = event.methods(); auto methods = commonSupportedMethods(m_remoteSupportedMethods); - if (methods.isEmpty()) { + if (methods.isEmpty()) cancelVerification(UNKNOWN_METHOD); - return; - } - - if (methods.size() == 1) { - sendStartSas(); - } + else if (methods.size() == 1) + sendStartSas(); // -> WAITINGFORACCEPT } void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event) { - if (state() != READY && state() != WAITINGFORREADY) { - cancelVerification(UNEXPECTED_MESSAGE); - return; - } if (startSentByUs) { if (m_remoteUserId > m_connection->userId() || (m_remoteUserId == m_connection->userId() && m_remoteDeviceId > m_connection->deviceId())) { return; @@ -362,17 +353,6 @@ void KeyVerificationSession::handleStart(const KeyVerificationStartEvent& event) setState(ACCEPTED); } -void KeyVerificationSession::handleAccept(const KeyVerificationAcceptEvent& event) -{ - if(state() != WAITINGFORACCEPT) { - cancelVerification(UNEXPECTED_MESSAGE); - return; - } - m_commitment = event.commitment(); - sendKey(); - setState(WAITINGFORKEY); -} - void KeyVerificationSession::handleMac(const KeyVerificationMacEvent& event) { QStringList keys = event.mac().keys(); @@ -402,19 +382,6 @@ void KeyVerificationSession::handleMac(const KeyVerificationMacEvent& event) } } -void KeyVerificationSession::handleDone(const KeyVerificationDoneEvent&) -{ - if (state() != DONE) { - cancelVerification(UNEXPECTED_MESSAGE); - } -} - -void KeyVerificationSession::handleCancel(const KeyVerificationCancelEvent& event) -{ - setError(stringToError(event.code())); - setState(CANCELED); -} - QVector KeyVerificationSession::sasEmojis() const { return m_sasEmojis; diff --git a/lib/keyverificationsession.h b/lib/keyverificationsession.h index b2e3b36d..9cac1184 100644 --- a/lib/keyverificationsession.h +++ b/lib/keyverificationsession.h @@ -91,6 +91,8 @@ public: ~KeyVerificationSession() override; Q_DISABLE_COPY_MOVE(KeyVerificationSession) + void handleEvent(const KeyVerificationEvent& baseEvent); + QVector sasEmojis() const; State state() const; @@ -108,7 +110,6 @@ public Q_SLOTS: void cancelVerification(Error error); Q_SIGNALS: - void startReceived(); void keyReceived(); void sasEmojisChanged(); void stateChanged(); @@ -133,11 +134,8 @@ private: void handleReady(const KeyVerificationReadyEvent& event); void handleStart(const KeyVerificationStartEvent& event); - void handleAccept(const KeyVerificationAcceptEvent& event); void handleKey(const KeyVerificationKeyEvent& event); void handleMac(const KeyVerificationMacEvent& event); - void handleDone(const KeyVerificationDoneEvent&); - void handleCancel(const KeyVerificationCancelEvent& event); void init(std::chrono::milliseconds timeout); void setState(State state); void setError(Error error); -- cgit v1.2.3 From 79f995da07ca240c768281bf1d040eb94c07583e Mon Sep 17 00:00:00 2001 From: Alexey Rusakov Date: Sun, 11 Sep 2022 21:46:21 +0200 Subject: TestKeyVerification: Add a missing allowed state --- autotests/testkeyverification.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autotests/testkeyverification.cpp b/autotests/testkeyverification.cpp index 96aad6c7..1fa6d8c6 100644 --- a/autotests/testkeyverification.cpp +++ b/autotests/testkeyverification.cpp @@ -23,7 +23,7 @@ private Q_SLOTS: QVERIFY(session->remoteDeviceId() == b->deviceId()); QVERIFY(session->state() == KeyVerificationSession::WAITINGFORREADY); connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ - QVERIFY(session->state() == KeyVerificationSession::ACCEPTED); + QVERIFY(session->state() == KeyVerificationSession::ACCEPTED || session->state() == KeyVerificationSession::READY); connectSingleShot(session, &KeyVerificationSession::stateChanged, this, [=](){ QVERIFY(session->state() == KeyVerificationSession::WAITINGFORVERIFICATION); session->sendMac(); -- cgit v1.2.3