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

feat: Enable end-to-end encryption for WebRTC streams. #14005

Merged
merged 3 commits into from
Jan 17, 2025

Conversation

fancycode
Copy link
Member

πŸ–ŒοΈ UI Checklist

🚧 Tasks

  • Gracefully handle cases where environment doesn't support e2ee.

🏁 Checklist


πŸ› οΈ API Checklist

🚧 Tasks

  • ...

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • πŸ“˜ API documentation in docs/ has been updated or is not required
  • πŸ”– Capability is added or not needed

Comment on lines 22 to 28
locateFile: () => {
return generateFilePath('spreed', 'js', wasmFile.split('/').pop())
},
Copy link
Contributor

Choose a reason for hiding this comment

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

wasmFile is already a link to the file. Why is link generation needed here?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was needed to get the correct URL for the case where the Talk app is not below apps but custom_apps. The import returned something like /apps/spreed/js/olm.wasm?v=aab9861f32e2f862a9ba which then resulted in a 404 error. Generating the path manually fixed it for me.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current solution doesn't work with Talk Desktop and it will break if bundling config slightly change (there is no guarantee that the file will be in the js folder root with the same filename)

Copy link
Member Author

Choose a reason for hiding this comment

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

Something similar was done here:

const filePath = generateFilePath('spreed', 'img', fileName + fileExtension)

I figured it would be ok to load the wasm similarly, but happy to do it differently if you give me a hint how ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

With image generateFilePath generates a path to public file spreed/img.

So it transforms spreed/img/{filename} to {path}/spreed/img/{filename} which is always valid for server and covered by Talk Desktop.

But generateFilePath('spreed', 'js', wasmFile) generates a path to spreed/js/{generatedFilename} for a generated path after import.

So it transforms some generated by bundler URL from spreed/node_modules/ @matrix-org/olm/olm.wasm to {path}/spreed/js/{generatedFilename}.

It only works if this .wasm files are bundler to /js root with the same filename.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should also work without breaking Talk Desktop (but also relies on assets bundling config).

		locateFile: () => {
			return wasmFile.split('/').pop()
		},

Copy link
Member Author

Choose a reason for hiding this comment

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

		locateFile: () => {
			return wasmFile.split('/').pop()
		},

This tries to load the file from "https://myserver/call/olm.wasm?v=aab9861f32e2f862a9ba" (which doesn't work).

Copy link
Member

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Had a rough look and it works (and doesnt with a not reloaded tab/desktop client), so merging for now to be able to test this.

@nickvergessen nickvergessen merged commit 63e8348 into main Jan 17, 2025
76 checks passed
@nickvergessen nickvergessen deleted the feat-webrtc-e2ee branch January 17, 2025 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants