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

Query & claim needed keys before encrypting #270

Merged
merged 3 commits into from Dec 19, 2022
Merged

Query & claim needed keys before encrypting #270

merged 3 commits into from Dec 19, 2022

Conversation

AndrewFerr
Copy link
Contributor

Ensure that the bot has all keys needed for sharing a room key with recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti andrewf@element.io

Checklist

  • Tests written for all new code
  • Linter has been satisfied
  • Sign-off given on the changes (see CONTRIBUTING.md)

Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
@turt2live turt2live self-requested a review October 31, 2022 08:31
@@ -106,6 +107,14 @@ export class RustEngine {
settings.rotationPeriod = BigInt(encEv.rotationPeriodMs);
settings.rotationPeriodMessages = BigInt(encEv.rotationPeriodMessages);

await this.run(RequestType.KeysQuery);
Copy link
Owner

Choose a reason for hiding this comment

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

This doesn't appear to cause a query request to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does, because by the time this is hit, the CryptoClient has updated the list of tracked users in the room, which queues outgoing key query requests in the crypto state machine.

Without this line, the first message a bot sends in an encrypted room with a user it hasn't seen before will be unencryptable by that user.

Otherwise, the SDK calls RustEngine.run (and processes all outgoing requests, including key queries) after having already sent its first (encrypted) message, meaning it won't have the device key of new users & can't send them the to-device message containing the room key.

@turt2live turt2live self-requested a review November 29, 2022 21:05
AndrewFerr added a commit to element-hq/matrix-bot-sdk that referenced this pull request Dec 8, 2022
Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
AndrewFerr added a commit to element-hq/matrix-bot-sdk that referenced this pull request Dec 8, 2022
Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
AndrewFerr added a commit to element-hq/matrix-bot-sdk that referenced this pull request Dec 9, 2022
Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
AndrewFerr added a commit to element-hq/matrix-bot-sdk that referenced this pull request Dec 9, 2022
Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
AndrewFerr added a commit to element-hq/matrix-bot-sdk that referenced this pull request Dec 9, 2022
Ensure that the bot has all keys needed for sharing a room key with
recipients before encryping an event in that room.

Signed-off-by: Andrew Ferrazzutti <andrewf@element.io>
Copy link
Owner

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

thanks :)

I've changed this slightly to match some desirables of the bot-sdk's public API, but generally lgtm

@turt2live turt2live merged commit c3bf77c into turt2live:main Dec 19, 2022
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.

None yet

2 participants