Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: e2ee #17

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

wip: e2ee #17

wants to merge 4 commits into from

Conversation

YousefED
Copy link
Owner

@YousefED YousefED commented Sep 9, 2022

This PR adds support for private rooms and end to end encryption (e2ee).

  • added private rooms
  • added support for startClient / olm in tests
  • cleaned up unit tests + added unit tests for encrypted / private rooms

In this PR, we move from a per-room polling based approach, to using the sync API. We are now calling startClient which causes matrix-js-sdk to call the Matrix sync API and keeps all rooms in sync. This is necessary because matrix-js-sdk takes care of crypto events and key sharing, so we can easily send / receive encrypted messages by reusing the SDK.

(Before, we pretty much only used the matrix-js-sdk as a wrapper to the Matrix HS APIs, not to keep track of any state like rooms, keys, etc.)

This move has a few downsides (which is why we initially went for the per-room polling approach):

  • matrix-js-sdk keeps events in memory when we don't need them anymore
  • matrix-js-sdk syncs ALL rooms a user has joined. With the previous poll-based approach, Matrix-CRDT would only sync rooms we're currently interested in. This should be improved once Matrix releases Sync v3 / Sliding Sync

Migrating to startClient brings a few extra todos:

  • Actually disable polling as we now get messages received by its sync calls in matrixRoomListener instead (we now use both methods at the same time)
  • Prevent Matrix-js-sdk from keeping all updates in memory. After applying the updates to the Y.Doc, we don't need the event anymore.
  • Sync only rooms we're interested in, when Sliding Sync is released
  • (minor) Migrate away from MatrixMemberReader as we can now use maySendEvent of matrix-js-sdk

Other open todos

  • Private rooms are now readable + writable by anyone who is in the room. Add support for private read-only rooms (using power levels)
  • (minor) implement getMatrixRoomSecurity and updateMatrixRoomAccess for private rooms
  • "late-joiner"; how can we make it possible for new users to decrypt an existing document? They won't have the keys to decrypt older events ("Unable to decrypt: The sender's device has not sent us the keys for this message." (The UISI bug) element-hq/element-web#2996 might be relevant)
  • Furthermore, having support for e2ee is nice, but creating a client on top of matrix-js-sdk that supports all UI flows required (cross signing / backups / key management) is quite cumbersome. This should be made easier before we can expect people to actually start using this

@coveralls
Copy link

coveralls commented Sep 9, 2022

Pull Request Test Coverage Report for Build 3047582861

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 75 of 96 (78.13%) changed or added relevant lines in 7 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.4%) to 71.82%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/matrix-crdt/src/matrixRoomManagement.ts 25 46 54.35%
Totals Coverage Status
Change from base Build 2980699428: 1.4%
Covered Lines: 2288
Relevant Lines: 3195

💛 - Coveralls

@jacobcoro
Copy link

@YousefED Have you looked into how this the matrix-files-sdk handles crypto? Very easy to follow code. It might help to take a look at it. https://github.com/vector-im/files-sdk-demo/blob/7b47ef65c5e9bb60f976c9b96a60b0f666f1d4ba/src/MatrixCrypto.ts

@jacobcoro
Copy link

Furthermore, having support for e2ee is nice, but creating a client on top of matrix-js-sdk that supports all UI flows required (cross signing / backups / key management) is quite cumbersome. This should be made easier before we can expect people to actually start using this

I think that you can just mention like in a jsdoc for an enableE2ee option that if you want to enable e2ee you need to handle these things yourself, and maybe provide a few links to documentation and examples

@jacobcoro
Copy link

"late-joiner"; how can we make it possible for new users to decrypt an existing document? They won't have the keys to decrypt older events (element-hq/element-web#2996 might be relevant)

Could we use 'snapshots' to solve this? When a latecomer joins, generate a new snapshot of the current state of the ydoc, which the newcomer can use to load theirs?

@jacobcoro jacobcoro mentioned this pull request May 3, 2023
@jacobcoro
Copy link

I would also suggest upgrading the version you are using of the matrix-js-sdk as it is very out of date. The issue you had with needing to explicitly require and set the request method seems to have been fixed in recent versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants