Repository navigation
feat(calls): end-to-end encrypted calls - #6793
AndyScherzinger wants to merge 4 commits into
Conversation
📱 QA build
The QA build installs alongside a released Nextcloud app, so you can keep Downloading the file requires a GitHub account, so open this link on the |
507145d to
dda32f3
Compare
d7d6bdb to
5c4fa8f
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟠 High · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 12.28% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 171 functions across 24 files. (1 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely identifies the primary change: support for end-to-end encrypted calls. Description check The description follows the repository template and provides detailed context, implementation notes, TODOs, checklist status, compatibility details, and testing scope. Some follow-up items remain open… Full details: Description check
-
Explanation
The description follows the repository template and provides detailed context, implementation notes, TODOs, checklist status, compatibility details, and testing scope. Some follow-up items remain open, but the description is complete and relevant.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 12.28% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 171 functions across 24 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Fix all pre-merge checks with AI
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
063971e9-7fa2-48d7-b3da-fdac260b302d
📒 Files selected for processing (25)
app/src/main/java/com/nextcloud/talk/activities/CallActivity.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/CallEncryption.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/CallEncryptionFactory.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/EncryptionMessage.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/FrameCrypto.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/OlmCrypto.ktapp/src/main/java/com/nextcloud/talk/callnotification/CallNotificationActivity.ktapp/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.ktapp/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.ktapp/src/main/java/com/nextcloud/talk/models/json/signaling/NCMessagePayloadDto.ktapp/src/main/java/com/nextcloud/talk/models/json/signaling/OlmMessageDto.ktapp/src/main/java/com/nextcloud/talk/signaling/EncryptionMessageNotifier.ktapp/src/main/java/com/nextcloud/talk/signaling/SignalingMessageReceiver.ktapp/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.ktapp/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.javaapp/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.javaapp/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.ktapp/src/main/res/values/strings.xmlapp/src/test/java/com/nextcloud/talk/call/e2ee/CallEncryptionTest.ktapp/src/test/java/com/nextcloud/talk/models/json/signaling/NCMessagePayloadDtoEncryptionTest.ktapp/src/test/java/com/nextcloud/talk/signaling/SignalingMessageReceiverEncryptionTest.ktapp/src/test/java/com/nextcloud/talk/utils/CapabilitiesUtilCallEncryptionTest.ktapp/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperEncryptionTest.ktapp/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt
💤 Files with no reviewable changes (3)
- app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt
- app/src/main/java/com/nextcloud/talk/callnotification/CallNotificationActivity.kt
- app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Close the key exchange when the server removes the client from a… · WebSocketInstance.kt:330-331
app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt:330-331
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClose the key exchange when the server removes the client from a room.
If the server sends a room response with an empty room ID,
currentRoomTokenbecomes empty. The nonempty-room branch then skips this cleanup, socallEncryptionretains the previous room’s keys until another local lifecycle action closes it. Close the exchange when the room ID is empty as well. The signaling protocol explicitly permits this server-initiated transition. (github.com)
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ad0f6068-c2a9-4d68-9798-438e2d1a3645
📒 Files selected for processing (6)
app/build.gradle.ktsapp/src/main/java/com/nextcloud/talk/call/e2ee/CallEncryptionFactory.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/TalkFrameCrypto.ktapp/src/main/java/com/nextcloud/talk/call/e2ee/VodozemacOlmCrypto.ktapp/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.ktgradle/verification-metadata.xml
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
8863523 to
fd3ad0b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Reused connections can bypass required end-to-end encryption, and codec enforcement and native resource cleanup remain unresolved.
3 open findings
What changed in this PR
Adds end-to-end encrypted calls to Android, integrating pairwise Olm key exchange and native frame encryption for compatibility with web and iOS clients.
Changes:
- Adds key exchange, ratcheting, rotation, and native crypto adapters.
- Integrates encryption into signaling and call setup, replacing unsupported-call restrictions.
- Updates dependencies and adds encryption-focused unit tests.
| File | Description |
|---|---|
| gradle/verification-metadata.xml | Adds dependency verification entries. |
| app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperTest.kt | Updates constructor calls. |
| app/src/test/java/com/nextcloud/talk/webrtc/PeerConnectionWrapperEncryptionTest.kt | Tests cryptor attachment and codec preference. |
| app/src/test/java/com/nextcloud/talk/utils/CapabilitiesUtilCallEncryptionTest.kt | Tests capability checks. |
| app/src/test/java/com/nextcloud/talk/signaling/SignalingMessageReceiverEncryptionTest.kt | Tests encryption message routing. |
| app/src/test/java/com/nextcloud/talk/models/json/signaling/NCMessagePayloadDtoEncryptionTest.kt | Tests payload serialization. |
| app/src/test/java/com/nextcloud/talk/call/e2ee/CallEncryptionTest.kt | Tests key exchange and lifecycle. |
| app/src/main/res/values/strings.xml | Adds encryption failure messages. |
| app/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.kt | Manages room encryption lifecycle. |
| app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java | Advertises encryption support. |
| app/src/main/java/com/nextcloud/talk/webrtc/PeerConnectionWrapper.java | Attaches cryptors and prefers VP8. |
| app/src/main/java/com/nextcloud/talk/utils/CapabilitiesUtil.kt | Checks encryption feature availability. |
| app/src/main/java/com/nextcloud/talk/signaling/SignalingMessageReceiver.kt | Routes key-exchange messages. |
| app/src/main/java/com/nextcloud/talk/signaling/EncryptionMessageNotifier.kt | Notifies encryption listeners. |
| app/src/main/java/com/nextcloud/talk/models/json/signaling/OlmMessageDto.kt | Represents outgoing Olm payloads. |
| app/src/main/java/com/nextcloud/talk/models/json/signaling/NCMessagePayloadDto.kt | Adds encryption payload fields. |
| app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt | Allows encrypted-call notifications. |
| app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt | Removes encrypted-call blocking. |
| app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt | Allows encrypted-call initiation. |
| app/src/main/java/com/nextcloud/talk/callnotification/CallNotificationActivity.kt | Allows answering encrypted calls. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/VodozemacOlmCrypto.kt | Adapts native Olm operations. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/TalkFrameCrypto.kt | Adapts native frame encryption. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/OlmCrypto.kt | Defines Olm interfaces. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/FrameCrypto.kt | Defines frame-crypto interfaces. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/EncryptionMessage.kt | Converts key-exchange messages. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/CallEncryptionFactory.kt | Creates native encryption implementations. |
| app/src/main/java/com/nextcloud/talk/call/e2ee/CallEncryption.kt | Implements key exchange and updates. |
| app/src/main/java/com/nextcloud/talk/activities/CallActivity.kt | Integrates encryption prerequisites and keys. |
| app/build.gradle.kts | Updates WebRTC and adds vodozemac. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
fd3ad0b to
33bb15f
Compare
33bb15f to
8592b46
Compare
8592b46 to
5714191
Compare
5714191 to
ea3cab0
Compare
ea3cab0 to
439d7cf
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
c8a64f9 to
b76ebf5
Compare
Port of the web client's e2ee/encryption.js (and the iOS port of it): every participant generates a random frame key and sends it to each other session through a pairwise Olm session over signaling messages, ratchets it when someone joins and rotates it when someone leaves. The Olm account and the frame key rings are behind two small interfaces, so the protocol runs and is tested on the JVM; the vodozemac and WebRTC backed implementations come with their AARs. Signaling routes "message" payloads of the "encryption.*" types to a new listener, the payload model gains the key exchange fields, and the capability check now requires both the signaling feature and the server config. Assisted-by: Claude Code:claude-fable-5-1 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
b76ebf5 to
bd41f8d
Compare
Calls announce the "encryption" feature to the signaling server, the key exchange lives for one joined room and signaling session, and the peer connections attach an encryptor to every sender of the publisher and a decryptor to every receiver of a subscriber, preferring VP8 as the only codec the frame encryption handles. When the server requires encrypted calls but there is no high-performance backend with MCU, or the key exchange could not be set up, the call ends instead of sending media unencrypted. The old "calls not supported" checks and notification are removed. The factory creating the key exchange still reports encryption as unavailable until the WebRTC build with the frame encryption and the vodozemac bindings are published. Assisted-by: Claude Code:claude-fable-5-1 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The guard clauses mirror the web client's code; detekt's ReturnCount would otherwise push the project over its issue threshold. Assisted-by: Claude Code:claude-fable-5-1 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…dozemac The key exchange now runs on the real implementations: TalkFrameCrypto wraps the TalkKeyRing of the WebRTC build, VodozemacOlmCrypto wraps the vodozemac account and sessions. WebRTC 155.8059.1, already on master, brings the frame encryption; talk-clients-vodozemac 0.0.2 is added, pulling in JNA. Whether a call is encrypted now only depends on the server capability. If the native libraries can not be loaded, the factory returns no key exchange and the call ends instead of sending media unencrypted. Assisted-by: Claude Code:claude-opus-5-5
|
Addressed CodeRabbit's remark from the 9 Oct review that was posted outside the diff (close the key exchange when the server sends a room message with an empty room id): fixed in 8adf0f7, |
bd41f8d to
f294af6
Compare
Android side of end-to-end encrypted calls, compatible with the web client (nextcloud/spreed#14005, nextcloud/spreed#14098) and the iOS client (nextcloud/talk-ios#2735). Port of the web client's
e2ee/encryption.js: every participant sends its own frame key to each other session through a pairwise Olm session over signaling messages, ratchets it when someone joins and rotates it when someone leaves. Like web and iOS this only works through the high-performance backend with MCU, prefers VP8, and ends the call instead of sending media unencrypted when encryption cannot be set up.The native pieces have landed upstream (all four PRs below are merged). WebRTC 155.8059.1 with the frame encryption is already on master (#6898), so this PR only adds
talk-clients-vodozemacand the adapters:CallEncryptionFactorybuildsVodozemacOlmCryptoandTalkFrameCryptodirectly and returnsnullonly when those native libraries cannot be loaded at runtime, in which case an E2EE-enabled server gets "could not be set up" and the call ends; nothing is sent in plain text.Hardening from the review rounds, all checked against the iOS implementation:
PeerConnectionWrapperrefuses to be created without the keys it needs when the call has to be encrypted; the sender of every signaling message is taken from the signaling server's envelope, never from the message itself; the encryption policy is re-read whenever the cached signaling socket is reused and a policy switched on while a room is joined starts a new signaling session; key rings of participants that left are released and superseded key material is wiped.🖼️ Screenshots
No UI changes beyond two new toasts; the old "calling is not supported because end-to-end encryption is enabled" notice and notification are gone.
🚧 TODO
TalkKeyRing→FrameCryptoandVodozemacAccount→OlmCryptoinCallEncryptionFactory, dependency inbuild.gradle.ktsSigned-off-byon the vodozemac/adapter commit (@rapterjet2004)🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)