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

[PB-3099] feat: remove deferred class usage from OlmAdapter #8

Open
wants to merge 1 commit into
base: feature/PB-2667-add-pq-layer
Choose a base branch
from

Conversation

apsantiso
Copy link
Collaborator

@apsantiso apsantiso commented Nov 19, 2024

Removed usage of Deferred class from OlmAdapter.

  • SessionInitializationInProgress variable added to prevent multiple calls to initSessions.
  • this._init is used just to check if the OLM adapter is initialized, it should be initialized when users access to the meeting as it loads even if e2ee is not enabled.
  • Deferred was deprecated. We just started using raw Promises, the pattern of resolving promises after a while is not avoidable as this flow is pretty async and we rely on users sending back ACK messages.

To consider:

  • There are still some problems like the audio sometimes not working as expected if you enable e2ee, however, you can check that the encryption keys are being shared successfully from the keys manager.
  • Like on the original implementation, when a moderator enables e2ee, it enables that feature for ALL users in the meeting, this triggers an _onParticipantPropertyChanged event, which calls sendKeyInfoToAll event, which leads to sometimes seeing messages such as "session not initialized with user xxxxxx". This is not related to any race issues and keys should be shared with users despite this.
    Check:
    await this.sendKeyInfoToAll();
  • I had performance problems while debugging the project due to trophy logging too many times things to the console while on DEBUG mode, you can avoid this by commenting out one line, it makes debugging much faster.

@apsantiso apsantiso changed the title feat: remove deferred class usage from OlmAdapter [PB-3099] feat: remove deferred class usage from OlmAdapter Nov 19, 2024
@apsantiso apsantiso self-assigned this Nov 19, 2024
Copy link
Collaborator

@TamaraFinogina TamaraFinogina left a comment

Choose a reason for hiding this comment

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

It's not working for me. I'm getting uncaught runtime errors that kill the meeting the moment e2e is turned on. I've tried to start a meeting many times and the best I could get is two users out of 3 have a session maybe once every several tries.
Screenshot 2024-11-20 at 09 49 14
Screenshot 2024-11-20 at 10 00 42

UPDATE The problem was solved after updating jitsi-meet

@TamaraFinogina TamaraFinogina self-requested a review December 2, 2024 17:12
@TamaraFinogina TamaraFinogina self-requested a review December 9, 2024 12:57
@@ -807,8 +836,8 @@ export class OlmAdapter extends Listenable {
const { ciphertext, sharedSecret } =
await this._encapsulateKey(participantPublicKey64);

olmData.pqSessionKey = await this.deriveKey(
decapsilatedSecret,
olmData.pqSessionKey = this.deriveKey(
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be still await, no?

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.

2 participants