Implement loading old megolm keys from SSSS #687
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Kudos, SonarCloud Quality Gate passed! |
KitsuneRal
requested changes
Jul 30, 2023
TobiasFella
force-pushed
the
tobias/ssss
branch
from
2b6250b
to
87b4810
Compare
KitsuneRal
reviewed
Aug 6, 2023
KitsuneRal
reviewed
Aug 6, 2023
KitsuneRal
reviewed
Aug 6, 2023
TobiasFella
changed the title
|
I think this is ready from my side. What's "missing" for a complete SSSS implementation is the "other side" of the whole thing - uploading keys to the backup, etc. - I'll do that in a different merge request. One thing I'm not sure about is the verified status of the megolm sessions - thing like the |
Merged
TobiasFella
force-pushed
the
tobias/ssss
branch
from
b9b1dee
to
4585628
Compare
TobiasFella
force-pushed
the
tobias/ssss
branch
from
4585628
to
1f125e8
Compare
Using an iterator instead of repeated hashmap lookups is not only slightly more efficient but also allows to make QOlmInboundSession constructor private again.
cryptoutils: more type safety with compile-time-sized spans
KitsuneRal
reviewed
Dec 26, 2023
aesCtr256En/Decrypt used FixedBuffer for the target (encrypted and decrypted, respectively) buffer. FixedBuffer uses secure memory, and it is purposefully limited to 64k, because it's meant to be used for secrets, not for payloads (for now at least). This commit reverts to using normal QByteArray to store the result, using asWritableCBytes() to get a span adapter that is more convenient to work with in OpenSSL.
...to work with QUOTIENT_FORCE_NAMESPACED_INCLUDES
1. Instead of templating decryptKey (which effectively emits the same function 4 times), just pass the event type as a parameter. 2. Similarly, just use event type strings instead of full-blown Event derivatives. 3. requestKeyFromDevices() is pretty much all about doing stuff in the Connection context, so move it there.
1. The rename is because it does so much more than just default key calculation. 2. Speaking of the default key, it's now only retrieved once, not 5 times. 3. The default key "event" type definition can be removed now that usage of it becomes one-time access of a single key.
KitsuneRal
reviewed
Mar 13, 2024
KitsuneRal
reviewed
Mar 14, 2024
The method became rather large and with additional error checking would get even larger - so now the initial preparations concerned with default key and key description retrieval are put out to UnlockData::prepare() - a static factory method returning the loaded information that is used further in the process. That allows to put the piece concerned with making the key out of the passphrase to unlockSSSSWithPassphrase() (formerly not quite correctly named as unlockSSSSFromPassword). More byte_views are introduced, to ensure correct payload sizes as early (preferably at compile time) as possible.
Fixes building on macOS.
Also: fix failing to jump over the result-using code in case of failing to decrypt the session.
|
This now looks ready from my side, too. Sorry for taking plenty of time to polish the code, I trust it was worth it though. Let me know if you're good to go with this. |
KitsuneRal
previously approved these changes
Mar 14, 2024
Thanks to @tobias for spotting that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some security checks are still missing, also a bunch of refactoring and cleanup
Best enjoyed with https://invent.kde.org/network/neochat/-/merge_requests/1111