Draft: feat: Add encrypted push #1242
Conversation
This adds helper methods to set pushers, making sure that the pushers are still up-to-date with what you want to set. Additionally, this detects if the server supports encrypted push and the user does not opt-out. Furthermore, there are helper methods to process / decrypt a push payload.
|
|
|
In GitLab by @nico-famedly on Nov 1, 2021, 16:38 Commented on pubspec.yaml line 21 @nico-famedly adds a note, that this needs to be fixed before getting merged of course. |
|
In GitLab by @Sorunome on Nov 1, 2021, 16:43 Commented on pubspec.yaml line 21 well yeah, the CI also fails because of this, so it'd be hard to miss xP |
|
In GitLab by @nico-famedly on Nov 1, 2021, 16:44 Commented on pubspec.yaml line 21 Oh, I guess it should. I didn't think thaaaat far :D |
|
In GitLab by @krille-chan on Nov 2, 2021, 08:27 Commented on lib/encryption/push_helper.dart line 28 What about we make this an extension on Encryption instead of an own class? Then we just name privateKey and publicKey pushPrivateKey and pushPublicKey. |
|
In GitLab by @Sorunome on Nov 5, 2021, 12:04 added 1 commit
|
|
In GitLab by @Sorunome on Dec 7, 2021, 17:07 added 1 commit
|
|
In GitLab by @Sorunome on Dec 8, 2021, 14:50 Commented on pubspec.yaml line 21 changed this line in version 4 of the diff |
|
In GitLab by @Sorunome on Dec 8, 2021, 14:50 added 120 commits
|
|
In GitLab by @Sorunome on Dec 9, 2021, 13:45 added 3 commits
|
|
In GitLab by @Sorunome on Dec 9, 2021, 13:45 Commented on lib/encryption/push_helper.dart line 28 extensions can't add new variables to the class it extends |
|
In GitLab by @Sorunome on Dec 9, 2021, 13:45 resolved all threads |
|
In GitLab by @Sorunome on Dec 9, 2021, 13:46 marked the checklist item I read the [Contributor Guide] and followed the process outlined there for submitting PRs. as completed |
|
In GitLab by @Sorunome on Dec 9, 2021, 13:46 marked the checklist item I did not add or update methods, functions or widgets without tests. as completed |
|
In GitLab by @Sorunome on Dec 9, 2021, 13:57 marked the checklist item I updated/added relevant documentation (doc comments with ///). as completed |
|
In GitLab by @Sorunome on Dec 9, 2021, 13:57 marked this merge request as ready |
|
In GitLab by @Sorunome on Dec 17, 2021, 13:49 resolved all threads |
|
In GitLab by @Sorunome on Dec 17, 2021, 13:49 added 1 commit
|
|
In GitLab by @nico-famedly on Dec 22, 2021, 21:46 Commented on lib/encryption/push_helper.dart line 40 Might make sense to link to dart-lang/sdk#39510, since this is documented almost nowhere... |
|
In GitLab by @nico-famedly on Dec 22, 2021, 22:07 Commented on lib/encryption/push_helper.dart line 27 Can you add a comment to this class, what it does or rename it to EncryptedPushHelper? PushHelper is very generic and it only does crypto, afaik. |
|
In GitLab by @nico-famedly on Dec 22, 2021, 22:07 Commented on test/push_helper_test.dart line 1 Did you duplicate this? |
|
In GitLab by @nico-famedly on Dec 22, 2021, 22:07 Looks good to me mostly. Although I haven't looked into the crypto thaaaat deply. I can do that, if I have to, but maybe adding some tests comparing it to olm could work too? |
|
In GitLab by @Sorunome on Dec 27, 2021, 12:08 added 1 commit
|
|
In GitLab by @Sorunome on Dec 27, 2021, 12:09 Commented on lib/encryption/push_helper.dart line 27 Soru thought the location inside |
|
In GitLab by @Sorunome on Dec 27, 2021, 12:11 the issue is that we can't easily compare it to libolm due to the mac issue and, when encryption, generating ephemeral keys |
|
In GitLab by @Sorunome on Dec 27, 2021, 12:13 Commented on test/push_helper_test.dart line 1 yup, oops |
|
In GitLab by @Sorunome on Dec 27, 2021, 12:17 added 1 commit
|
|
In GitLab by @Sorunome on Dec 27, 2021, 12:18 added 5 commits
|
|
In GitLab by @nico-famedly on Dec 27, 2021, 15:12 Apart from the mac, the code should be identical though? |
|
In GitLab by @nico-famedly on Dec 27, 2021, 15:13 Commented on lib/encryption/push_helper.dart line 27 Hm, I guess. It wasn't as obvious to me, when I saw the class usage in the other class. Because there you don't see, where it is originally declared, but yea, probably fine. |
|
In GitLab by @Sorunome on Dec 29, 2021, 10:09 yeah, but in both cases (libolm and this method) the functions throw errors prior decrypting if the mac does not match, meaning we'd need to fork libolm or this method to be able to test against that |
|
In GitLab by @Sorunome on Dec 29, 2021, 10:25 added 1 commit
|
|
In GitLab by @nico-famedly on Dec 29, 2021, 18:03 Ah, right, that would cause issues. |
|
In GitLab by @nico-famedly on Dec 29, 2021, 18:03 resolved all threads |
|
In GitLab by @Sorunome on Dec 29, 2021, 18:15 resolved all threads |
|
In GitLab by @Sorunome on Dec 29, 2021, 18:15 added 1 commit
|
|
In GitLab by @Sorunome on Dec 31, 2021, 10:49 resolved all threads |
|
In GitLab by @Sorunome on Jan 3, 2022, 09:47 requested review from @krille-chan, @lukaslihotzki, @nico-famedly, @Sorunome, @techno-disaster, @cloudwebrtc, and @TheOneWithTheBraid |
|
In GitLab by @nico-famedly on Jan 10, 2022, 07:24 Commented on test/push_helper_test.dart line 1 Looks still duplicated to me :3 |
|
In GitLab by @nico-famedly on Jan 10, 2022, 07:25 Commented on test/encryption/push_helper_test.dart line 31 No test/implementation for the count stuff for iOS? |
|
In GitLab by @nico-famedly on May 4, 2022, 11:51 marked this merge request as draft |
|
In GitLab by @nico-famedly on May 4, 2022, 11:52 Marking as draft as the SCT wants us to change a few things like the encryption algorithm used. |
|
This depends on us finally fixing the iOS push and then updating the MSC and synapse implementation to match the review comments. |
In GitLab by @Sorunome on Nov 1, 2021, 16:34
Description
This PR adds push helper methods to setting up pushers and processing push payloads. Additionally, this PR also adds support to encrypted push, dynamically making pushers encrypted, if the server supports encrypted push.
Checklist