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

Add support for native e2ee #299

Merged
merged 21 commits into from
Dec 9, 2022
Merged

Add support for native e2ee #299

merged 21 commits into from
Dec 9, 2022

Conversation

Half-Shot
Copy link
Contributor

This PR adds support for native e2ee within the bridge, and drops support for Pantalaimon based bridging.

N.B this doesn't work yet, seeing failures like:

ERROR 11:34:53:742 [Appservice] Decryption error on !ltmLLVxeBLdNUWQISs:beefy $8gHWPs07YA8Ao-Ormsn71WwhHdP37VonGS8gfsYwKsU Error: Decryption failed: decryption failed because the room key is missing
    at OlmMachine.decryptRoomEvent (/home/will/git/matrix-hookshot/node_modules/@turt2live/matrix-sdk-crypto-nodejs/src/ts/OlmMachine.ts:305:37)
    at CryptoClient.<anonymous> (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/e2ee/CryptoClient.js:157:50)
    at Generator.next (<anonymous>)
    at /home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/e2ee/CryptoClient.js:17:71
    at new Promise (<anonymous>)
    at __awaiter (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/e2ee/CryptoClient.js:13:12)
    at CryptoClient.decryptRoomEvent (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/e2ee/CryptoClient.js:156:16)
    at CryptoClient.descriptor.value (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/e2ee/decorators.js:33:35)
    at Appservice.<anonymous> (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/appservice/Appservice.js:540:83)
    at Generator.next (<anonymous>)
    at fulfilled (/home/will/git/matrix-hookshot/node_modules/matrix-bot-sdk/lib/appservice/Appservice.js:5:58)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  code: 'GenericFailure'
}

@Half-Shot
Copy link
Contributor Author

Currently blocked on turt2live/matrix-bot-sdk#208

@B4dM4n
Copy link

B4dM4n commented Jul 28, 2022

The matrix-bot-sdk has recently released v0.6.1 which contains a fix for the mentioned issue.

I tried to rebase this PR onto main and update matrix-bot-sdk version. My current effort can be found here: main...B4dM4n:matrix-hookshot:native-e2ee

The Hookshot Bot can decrypt messages I send him, but Element can't decrypt messages send by the bot (** Unable to decrypt: The sender's device has not sent us the keys for this message. **).

I'm not sure why this happens, but a very simple bot using the matrix-bot-sdk is correctly working.

My current guess is that the RedisStorageProvider is mangling some values, but I didn't look really into it.

For easier testing I created a docker-compose file which can setup Synapse, Element, Redis and Hookshot: https://github.com/B4dM4n/hookkshot-e2ee-test

@Half-Shot Can you please have a look at the problem? I'm not too familiar with the Matrix API's / SDK's.

@Half-Shot Half-Shot self-assigned this Jul 28, 2022
@Half-Shot
Copy link
Contributor Author

Wow, thanks for looking into this. I'll be sure to take a look at your changes and see if I can get a working decryption going.

@B4dM4n
Copy link

B4dM4n commented Aug 1, 2022

I could track down the decryption issue to users not being tracked properly in the matrix-bot-sdk encryption handling: turt2live/matrix-bot-sdk#251

Up next is some general testing of the bot encryption.

@Half-Shot
Copy link
Contributor Author

#417 lands support for bot-sdk 0.6.1 which means we can lean on that for some of this PR.

turt2live/matrix-bot-sdk#251 is gonna be a hard requirement though, given how hookshot is used.

@MatMaul
Copy link

MatMaul commented Sep 29, 2022

It seems like SDK dependencies have landed so this should be mergeable ?

@Half-Shot Half-Shot requested a review from a team as a code owner October 6, 2022 08:30
@AndrewFerr AndrewFerr self-requested a review October 6, 2022 13:41
package.json Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@AndrewFerr
Copy link
Member

This works, and adds some docs to explain the extra configuration steps needed to get this working (namely Synapse experimental configs).

However, bridge startup sometimes fails with M_UNKNOWN_TOKEN, including when successfully setting up the bridge without encryption & updating configs to enable encryption. Either there are race conditions in startup / crypto registration, or I'm doing something wrong.

@AndrewFerr
Copy link
Member

The M_UNKNOWN_TOKEN was a false positive, and was caused by fiddling with the homeserver DB & hookshot's encryption state (i.e. ./data/encryption and the redis instance). I added some brief notes to the docs to point out keeping them in sync.

The only remaining issue is that "greeting" messages sent by the bot may fail to be decryptable (and appear in Element Web with an error of OLM.UNKNOWN_MESSAGE_INDEX). I'll investigate whether this is an issue in the bot SDK, or is due to Hookshot sending messages earlier than it should.

@AndrewFerr
Copy link
Member

Lastest push just rebases onto the tip of main to get the CI fixes.

@AndrewFerr AndrewFerr assigned AndrewFerr and unassigned Half-Shot Oct 20, 2022
@AndrewFerr AndrewFerr requested a review from a team October 20, 2022 20:06
@AndrewFerr AndrewFerr added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements. label Oct 20, 2022
Copy link
Contributor Author

@Half-Shot Half-Shot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✔️ even though I can't review my own stuff!

src/Bridge.ts Show resolved Hide resolved
docs/advanced/encryption.md Show resolved Hide resolved
@AndrewFerr
Copy link
Member

Changes made by the latest force-push:

  • Rebased onto current tip of the main branch.
  • Replaced 2eb91ab with 9853b8c.
  • Added all commits from 6b0e48b onwards.

Note that there remain some issues that can only be fixed in matrix-bot-sdk:

Half-Shot and others added 7 commits December 8, 2022 16:35
This also enables encryption for new admin rooms when appropriate.
- Add comment to clarify Redis (the `queue` section) must be configured
  in order for encryption to work
- Mention that the `encryption` section is optional, and omitting it
  will disable encryption support
@AndrewFerr
Copy link
Member

Changes made by the latest (and hopefully last!) force-push:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants