| Age | Commit message (Collapse) | Author |
|---|
|
|
|
Facility macros to report Olm errors: QOLM_INTERNAL_ERROR[_X],
QOLM_FAIL_OR_LOG[_X]
|
|
A convenient abstraction swallowing all the type casts and, more
importantly, cleanup on destruction (previous code only cleaned up
the buffer upon a successful call to Olm API but not upon an error).
|
|
Notably:
- simplified unnecessarily verbose constructs;
- formally aligned (no re-numeration was necessary)
QOlmMessage::Type with corresponding OLM_ constants;
- dropped QOlmSession::encryptMessageType() because it's very
sensitive to the order of calling with QOlmSession::encrypt()
(and encrypt() itself already calls it and returns the message
type);
- simplify the return type of pickle() calls that can only fail due to
an internal error;
- replace const QString& with QStringView or const QByteArray&
where appropriate;
- use '\0' where it was meant to be instead of '0'.
|
|
QOlmError represents a subset of OlmErrorCode, and the associated
fromString() function uses undocumented strings produced inside Olm;
meanwhile OlmErrorCode is documented in its own header file. Each QOlm*
class now has lastErrorCode() next to lastError() (that, from now,
returns a textual representation straight from Olm, not QOlmError enum).
Also: including olm/error.h in e2ee/e2ee.h required some rearrangement
of the code to make sure non-E2EE configuration still builds.
|
|
|
|
|
|
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.
|
|
Ironically, this slicing would not break anything as all the necessary
data are saved in the Event parent class; but the code is very fragile
and scary.
|
|
Constrain types to derive from Event (or the chosen class), where
applicable.
|
|
Direct construction (using makeEvent() or explicitly constructing
an event) from JSON may create an event that has a type conflicting
with that stored in JSON. There's no such problem with loadEvent(),
even though it's considerably slower. Driven by the fact that almost
nowhere in the code direct construction is used on checked JSON
(one test is the only valid case), this commit moves all JSON-loading
constructors to the protected section, thereby disabling usage of
makeEvent() in JSON-loading capacity, and switches such cases across
the library to loadEvent().
|
|
This commit introduces a few things to further reduce the boilerplate
across event type definitions:
- Event type is no more separately stored in Event and therefore no more
passed to base event constructors. Until the previous commit, it was
used by is() to quickly match the event type; with the new event
metatype class, the same is achieved even quicker by comparing
metatype pointers.
- EventTemplate is a generalisation of StateEvent for all event types
providing common constructor signatures and content() for (most) leaf
event types. StateEvent therefore has become a partial specialisation
of EventTemplate for types derived from StateEventBase; as the known
client code base does not use it directly, a compatibility alias is
not provided. Also, DEFINE_SIMPLE_EVENT now expands into a class
deriving from EventTemplate.
- On top of StateEvent->EventTemplate specialisation,
KeyedStateEventBase and KeylessStateEventBase types are introduced
with appropriate constructor signatures (with or without state_key,
respectively) to allow `using` of them from derived event types.
To facilitate writing of constraints, concepts for keyed and keyless
state event types are also introduced; RoomStateView, e.g., makes use
of those to provide appropriate method signatures.
- typeId(), unknownEventTypeId(), UnknownEventTypeId are no more
provided - they weren't used throughout the known code base
(Quaternion, NeoChat), and the concept of "unknown event types" is
hereby eliminated entirely.
- RoomKeyEvent no more accepts senderId as a parameter; it has never
been a good practice as the sender is assigned by Connection anyway.
|
|
|
|
|
|
|
|
- Common switchOnType() piece for key verification events is factored
out into processIfVerificationEvent()
- Bare event JSON removed from KeyVerificationSession into constructors
of respective events
- Connection::sendToDevice() uses assembleEncryptedContent() introduced
in the previous commit
- commonSupportedMethods() moved out to .cpp; error/string converters
made static
|
|
What was partially factored out before into encryptSessionKeyEvent()
is now the complete algorithm converting any event json into encrypted
content.
|
|
- Use std::chrono for the timeout (it's more readable and
less ambiguous) and make it a local variable
- Only pass a Connection object once to constructors
- Ensure buildability even without E2EE (key verification is disabled
in that case)
- Reorder #includes
- Other cleanup following clang-tidy warnings
|
|
# Conflicts:
# autotests/testfilecrypto.cpp
# lib/connection.cpp
# lib/connection.h
# lib/database.cpp
# lib/database.h
# lib/e2ee/qolmoutboundsession.cpp
# lib/e2ee/qolmoutboundsession.h
# lib/eventitem.h
# lib/events/encryptedevent.cpp
# lib/events/encryptedevent.h
# lib/events/encryptedfile.cpp
# lib/events/encryptedfile.h
# lib/events/keyverificationevent.cpp
# lib/events/keyverificationevent.h
# lib/events/roomkeyevent.h
# lib/room.cpp
# lib/room.h
|
|
The template version has never worked, to the point where instantiating
it would immediately lead to FTBFS. The new version returns an event
pointer as a simpler fix that would make it usable - in particular,
there's no more need to have separate
Connection::Private::unpackAccountData(). To simplify the fix,
eventCast() has been made more tolerating - passing nullptr to it is
processed in an expected (no-op) way now.
|
|
In a situation where you have an EventPtr that you want to place
somewhere as an `event_ptr_tt<SomeMoreSpecificEventType>` you have to
carefully check that the stored event is actually of
SomeMoreSpecificType and if it is, release() that event pointer,
downcast, and re-wrap it into that new event_ptr_tt - or, as can be seen
from the diff here, re-loadEvent() from JSON, which is simpler but
inefficient. To help clients, and the library, eventCast() can now
accept an rvalue smart pointer and do all the necessary things with it.
|
|
If userMap only holds valid ids, there's no reason to spend time
validating the sought id: if it's invalid, it won't be found. And
lookups over a hash map are cheap.
|
|
|
|
Since this object has to be verified against a signature it also carries
there's a rather specific procedure described in The Spec for that.
That procedure basically assumes handling the signed one-time key
object as a JSON object, not as a C++ object. And originally Quotient
E2EE code was exactly like that (obtaining the right QJsonObject from
the job result and handling it as specced) but then one enthusiastic
developer (me) decided it's better to use a proper C++ structure -
breaking the verification logic along the way. After a couple attempts
to fix it, here we are again: SignedOneTimeKey is a proper QJsonObject,
and even provides a method returning its JSON in the form prepared for
verification (according to the spec).
|
|
|
|
|
|
Also: build with Qt 6 first, so that it fails sooner.
|
|
Although Qt 5 didn't complain about that, you could never really use
sendToDevices() in its slot (or even invocable) capacity because
Qt's meta-type system could not handle move-only UsersToDevicesToEvents.
Qt 6 is more stringent; the build fails at trying to instantiate
QMetaType for that type (with a rather unhelpful error message thrown
by Clang, and more helpful but very verbose diagnostic from MSVC)
because it does not provide a copy constructor.
However, sendToDevice doesn't really need to have full-blown events
in that parameter; just the content of the event is equally fine.
This commit does exactly that: replaces UsersToDevicesToEvents with
UsersToDevicesToContent that contains QJsonObject's instead of
EventPtr's. The code around is updated accordingly.
Also: factor out the key event JSON creation from
makeMessageEventForSessionKey() because it's the same JSON for each
target device; the function therefore is called encryptSessionKeyEvent()
now.
|
|
This reimplements #558 in a more reliable way. Deconstruction of
AccountRegistry may (or may not, yay for static initialisation) occur
after deconstruction of QCoreApplication, in which case an attempt
to determine the directory for the state fails because it depends on
the application object existence.
|
|
|
|
|
|
...not before.
|
|
Load and store accounts in the keychain
|
|
Functions (Room::Private::)createOlmSession, payloadForUserDevice
and sendRoomKeyToDevices don't have a lot to do with the given Room
object but deal with quite a few things stored in Connection. This
commit moves them to Connection::Private, exposing
sendSessionKeyToDevices (the new name for sendRoomKeyToDevices) in
Connection so that Room could call it from Room::P::sendMegolmSession().
While moving these over, a few additional things were adjusted:
- more functions marked as const
- a few functions could be moved now from Connection
to Connection::Private
- false slots in Connection (such as picklingMode) are moved out of
the slots block
- keys.yml in Matrix CS API definitions has been adjusted to match
the real structure of `/claim` response (see quotient-im/matrix-spec
repo); csapi/keys.h has been regenerated accordingly.
|
|
Notably, replace a multi-level hash map with QMultiHash and factor out
Room::P::createOlmSession().
|
|
|
|
Besides having a misleading name (and it goes back to the spec),
EncryptedFile under `file` key preempts the `url` (or `thumbnail_url`)
string value so only one of the two should exist. This is a case for
using std::variant<> - despite its clumsy syntax, it can actually
simplify and streamline code when all the necessary bits are in place
(such as conversion to JSON and getting the common piece - the URL -
out of it). This commit replaces `FileInfo::url` and `FileInfo::file`
with a common field `source` of type `FileSourceInfo` that is an alias
for a variant type covering both underlying types; and `url()` is
reintroduced as a function instead, to allow simplified access
to whichever URL is available inside the variant.
Oh, and EncryptedFile is EncryptedFileMetadata now, to clarify that it
does not represent the file payload itself but rather the data necessary
to obtain that payload.
|
|
|
|
|
|
Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net>
|
|
|
|
|
|
|
|
|
|
|
|
As mentioned in the commit introducing `Expected`, `QOlmExpected` is
simply an alias for `Expected<T, QOlmError>`. This simplifies quite
a few function signatures in `QOlm*` classes and collapses unwieldy
`std::holds_alternative<>`/`std::get<>` constructs into a neat
contextual bool cast and an invocation of `operator*` or
`value()`/`error()` accessors that don't need to specify the type.
While refactoring the code, I found a couple of cases of mismatching
`uint32_t` and `qint32_t` in return values; a couple of cases where
`decrypt()` returns `QString` which is in fact `QByteArray` (e.g., in
`QOlmSession::decrypt()`); there's a repetitive algorithm in
`Connection::Private::sessionDecryptPrekey()` and
`sessionDecryptGeneral()`
|
|
There's no particular use in letting `QOlmError` out, only to confirm
that, well, `QOlmError` is just another form of no-match.
|
|
|
|
Mainly driven by clang-tidy and SonarCloud warnings (sadly, SonarCloud
doesn't store historical reports so no link can be provided here).
|
|
QCoreApplication::processEvents() is well-known to be a _wrong_ solution
to the unresponsive UI problem; despite that, connection.cpp has long
had that call to let UI update itself while processing bulky room
updates (mainly from the initial sync). This commit finally fixes this,
after an (admittedly rare) race condition has been hit, as follows:
0. Pre-requisite: quotest runs all the tests and is about to leave
the room; there's an ongoing sync request.
1. Quotest calls /leave
2. Sync returns, with the batch of _several_ rooms (that's important)
3. The above code handles the first room in the batch
4. processEvents() is called, just in time for the /leave response.
5. The /leave response handler in quotest ends up calling
Connection::logout() (processEvents() still hasn't returned).
6. Connection::logout() calls abandon() on the ongoing SyncJob,
pulling the rug from under onSyncSuccess()/consumeRoomData().
7. processEvents() returns and the above code proceeds to the next
room - only to find that the roomDataList (that is a ref to
a structure owned by SyncJob), is now pointing to garbage.
Morals of the story:
1. processEvents() effectively makes code multi-threaded: one flow is
suspended and another one may run _on the same data_. After the first
flow is resumed, it cannot make any assumptions regarding which data
the second flow touched and/or changed.
2. The library had quite a few cases of using &&-refs, avoiding even
move operations but also leaving ownership of the data with the
original producer (SyncJob). If the lifetime of that producer ends
too soon, those refs become dangling.
The fix makes two important things, respectively:
2. Ownership of room data is now transfered to the processing side,
the moment it is scheduled (see below), in the form of moving
into a lambda capture.
1. Instead of processEvents(), processing of room data is scheduled
via QMetaObject::invokeMethod(), uncoupling the moment when the
data was received in SyncJob from the moment they are processed
in Room::updateData() (and all the numerous signal-slots it calls).
Also: Room::baseStateLoaded now causes Connection::loadedRoomState, not
the other way round - this is more natural and doesn't need Connection
to keep firstTimeRooms map around.
|
