Support party_id

Send party_id on events and check the party_id of incoming events matches

Includes a basic test to assert that it actually does: we should
build out a decent test suite for calls as there's a lot of edge-case
functionality that can break and slip through the cracks (eg. glare).
This is a start.

Fixes https://github.com/matrix-org/matrix-js-sdk/issues/1511

spec/TestClient.js

@@ -69,6 +69,9 @@ export function TestClient(
 
     this.deviceKeys = null;
     this.oneTimeKeys = {};
+    this._callEventHandler = {
+        calls: new Map(),
+    };
 }
 
 TestClient.prototype.toString = function() {
@@ -230,3 +233,7 @@ TestClient.prototype.flushSync = function() {
         logger.log(`${this}: flushSync completed`);
     });
 };
+
+TestClient.prototype.isFallbackICEServerAllowed = function() {
+    return true;
+};

spec/unit/webrtc/call.spec.js

@@ -0,0 +1,169 @@
+/*
+Copyright 2020 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import {TestClient} from '../../TestClient';
+import {MatrixCall} from '../../../src/webrtc/call';
+
+const DUMMY_SDP = (
+    "v=0\r\n" +
+    "o=- 5022425983810148698 2 IN IP4 127.0.0.1\r\n" +
+    "s=-\r\nt=0 0\r\na=group:BUNDLE 0\r\n" + 
+    "a=msid-semantic: WMS h3wAi7s8QpiQMH14WG3BnDbmlOqo9I5ezGZA\r\n" +
+    "m=audio 9 UDP/TLS/RTP/SAVPF 111 103 104 9 0 8 106 105 13 110 112 113 126\r\n" +
+    "c=IN IP4 0.0.0.0\r\na=rtcp:9 IN IP4 0.0.0.0\r\na=ice-ufrag:hLDR\r\n" +
+    "a=ice-pwd:bMGD9aOldHWiI+6nAq/IIlRw\r\n" +
+    "a=ice-options:trickle\r\n" +
+    "a=fingerprint:sha-256 E4:94:84:F9:4A:98:8A:56:F5:5F:FD:AF:72:B9:32:89:49:5C:4B:9A:4A:15:8E:41:8A:F3:69:E4:39:52:DC:D6\r\n" +
+    "a=setup:active\r\n" +
+    "a=mid:0\r\n" +
+    "a=extmap:1 urn:ietf:params:rtp-hdrext:ssrc-audio-level\r\n" +
+    "a=extmap:2 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\r\n" +
+    "a=extmap:3 http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n" +
+    "a=extmap:4 urn:ietf:params:rtp-hdrext:sdes:mid\r\n" +
+    "a=extmap:5 urn:ietf:params:rtp-hdrext:sdes:rtp-stream-id\r\n" +
+    "a=extmap:6 urn:ietf:params:rtp-hdrext:sdes:repaired-rtp-stream-id\r\n" +
+    "a=sendrecv\r\n" +
+    "a=msid:h3wAi7s8QpiQMH14WG3BnDbmlOqo9I5ezGZA 4357098f-3795-4131-bff4-9ba9c0348c49\r\n" +
+    "a=rtcp-mux\r\n" +
+    "a=rtpmap:111 opus/48000/2\r\n" +
+    "a=rtcp-fb:111 transport-cc\r\n" +
+    "a=fmtp:111 minptime=10;useinbandfec=1\r\n" +
+    "a=rtpmap:103 ISAC/16000\r\n" +
+    "a=rtpmap:104 ISAC/32000\r\n" +
+    "a=rtpmap:9 G722/8000\r\n" +
+    "a=rtpmap:0 PCMU/8000\r\n" +
+    "a=rtpmap:8 PCMA/8000\r\n" +
+    "a=rtpmap:106 CN/32000\r\n" +
+    "a=rtpmap:105 CN/16000\r\n" +
+    "a=rtpmap:13 CN/8000\r\n" +
+    "a=rtpmap:110 telephone-event/48000\r\n" +
+    "a=rtpmap:112 telephone-event/32000\r\n" +
+    "a=rtpmap:113 telephone-event/16000\r\n" +
+    "a=rtpmap:126 telephone-event/8000\r\n" +
+    "a=ssrc:3619738545 cname:2RWtmqhXLdoF4sOi\r\n"
+);
+
+class MockRTCPeerConnection {
+    constructor() {
+        this.localDescription = {
+            sdp: DUMMY_SDP,
+            type: '',
+        }
+    }
+
+    addEventListener() {}
+    createOffer() {
+        return Promise.resolve({});
+    }
+    setRemoteDescription() {
+        return Promise.resolve();
+    }
+    setLocalDescription() {
+        return Promise.resolve();
+    }
+}
+
+describe('Call', function() {
+    let client;
+    let call;
+    let prevNavigator;
+    let prevDocument;
+    let prevWindow;
+
+    beforeEach(function() {
+        prevNavigator = global.navigator;
+        prevDocument = global.document;
+        prevWindow = global.window;
+
+        global.navigator = {
+            mediaDevices: {
+                getUserMedia: () => {
+                    return {
+                        getTracks: () => [],
+                        getAudioTracks: () => [],
+                        getVideoTracks: () => [],
+                    };
+                }
+            }
+        };
+
+        global.window = {
+            RTCPeerConnection: MockRTCPeerConnection,
+            RTCSessionDescription: {},
+            RTCIceCandidate: {},
+            getUserMedia: {},
+        };
+        global.document = {};
+
+        client = new TestClient();
+        call = new MatrixCall({
+            client: client.client,
+            roomId: '!foo:bar',
+        });
+        // call checks one of these is wired up
+        call.on('error', () => {});
+    });
+
+    afterEach(function() {
+        client.stop();
+        global.navigator = prevNavigator;
+        global.window = prevWindow;
+        global.document = prevDocument;
+    });
+
+    it('should ignore candidate events from non-matching party ID', async function() {
+        await call.placeVoiceCall();
+        await call.receivedAnswer({
+            version: 0,
+            call_id: call.callId,
+            party_id: 'the_correct_party_id',
+            answer: {
+                sdp: DUMMY_SDP,
+            }
+        });
+
+        call.peerConn.addIceCandidate = jest.fn();
+        call.onRemoteIceCandidatesReceived({
+            getContent: () => { return {
+                version: 0,
+                call_id: call.callId,
+                party_id: 'the_correct_party_id',
+                candidates: [
+                    {
+                        candidate: '',
+                        sdpMid: '',
+                    }
+                ]
+            }},
+        });
+        expect(call.peerConn.addIceCandidate.mock.calls.length).toBe(1);
+
+        call.onRemoteIceCandidatesReceived({
+            getContent: () => { return {
+                version: 0,
+                call_id: call.callId,
+                party_id: 'some_other_party_id',
+                candidates: [
+                    {
+                        candidate: '',
+                        sdpMid: '',
+                    }
+                ]
+            }},
+        });
+        expect(call.peerConn.addIceCandidate.mock.calls.length).toBe(1);
+    });
+});

src/webrtc/call.ts

@@ -231,6 +231,10 @@ export class MatrixCall extends EventEmitter {
     private config: MediaStreamConstraints;
     private successor: MatrixCall;
     private opponentVersion: number;
+    private ourPartyId: string;
+    // The party ID of the other side: undefined if we haven't chosen a partner
+    // yet, null if we have but they didn't send a party ID.
+    private opponentPartyId: string;
 
     constructor(opts: CallOpts) {
         super();
@@ -238,6 +242,7 @@ export class MatrixCall extends EventEmitter {
         this.client = opts.client;
         this.type = null;
         this.forceTURN = opts.forceTURN;
+        this.ourPartyId = this.client.deviceId;
         // Array of Objects with urls, username, credential keys
         this.turnServers = opts.turnServers || [];
         if (this.turnServers.length === 0 && this.client.isFallbackICEServerAllowed()) {
@@ -449,6 +454,7 @@ export class MatrixCall extends EventEmitter {
         this.setState(CallState.Ringing);
         this.direction = CallDirection.Inbound;
         this.opponentVersion = this.msg.version;
+        this.opponentPartyId = this.msg.party_id || null;
 
         if (event.getLocalAge()) {
             setTimeout(() => {
@@ -545,14 +551,11 @@ export class MatrixCall extends EventEmitter {
 
         logger.debug("Ending call " + this.callId);
         this.terminate(CallParty.Local, reason, !suppressEvent);
-        const content = {
+        const content = {};
-            version: VOIP_PROTO_VERSION,
-            call_id: this.callId,
-        };
         // Continue to send no reason for user hangups temporarily, until
         // clients understand the user_hangup reason (voip v1)
         if (reason !== CallErrorCode.UserHangup) content['reason'] = reason;
-        this.sendEvent('m.call.hangup', content);
+        this.sendVoipEvent('m.call.hangup', {});
     }
 
     /**
@@ -575,11 +578,7 @@ export class MatrixCall extends EventEmitter {
 
         logger.debug("Rejecting call: " + this.callId);
         this.terminate(CallParty.Local, CallErrorCode.UserHangup, true);
-        const content = {
+        this.sendVoipEvent('m.call.reject', {});
-            version: VOIP_PROTO_VERSION,
-            call_id: this.callId,
-        };
-        this.sendEvent('m.call.reject', content);
     }
 
     /**
@@ -692,7 +691,7 @@ export class MatrixCall extends EventEmitter {
 
     private sendAnswer() {
         this.setState(CallState.Connecting);
-        this.sendEvent('m.call.answer', this.answerContent).then(() => {
+        this.sendVoipEvent('m.call.answer', this.answerContent).then(() => {
             // If this isn't the first time we've tried to send the answer,
             // we may have candidates queued up, so send them now.
             this.sendCandidateQueue();
@@ -750,8 +749,6 @@ export class MatrixCall extends EventEmitter {
             await this.peerConn.setLocalDescription(myAnswer);
 
             this.answerContent = {
-                version: VOIP_PROTO_VERSION,
-                call_id: this.callId,
                 answer: {
                     sdp: this.peerConn.localDescription.sdp,
                     // type is now deprecated as of Matrix VoIP v1, but
@@ -808,24 +805,37 @@ export class MatrixCall extends EventEmitter {
         }
     };
 
-    /**
+    onRemoteIceCandidatesReceived(ev: MatrixEvent) {
-     * Used by MatrixClient.
-     * @param {Object} cand
-     */
-    gotRemoteIceCandidate(cand: RTCIceCandidate) {
         if (this.state == CallState.Ended) {
             //debuglog("Ignoring remote ICE candidate because call has ended");
             return;
         }
-        if (
+
-            (cand.sdpMid === null || cand.sdpMid === undefined) &&
+        if (!this.partyIdMatches(ev.getContent())) {
-            (cand.sdpMLineIndex === null || cand.sdpMLineIndex === undefined)
+            logger.info(
-        ) {
+                `Ignoring candidates from party ID ${ev.getContent().party_id}: ` +
-            logger.debug("Ignoring remote ICE candidate with no sdpMid or sdpMLineIndex");
+                `we have chosen party ID ${this.opponentPartyId}`,
+            );
             return;
         }
-        logger.debug("Got remote ICE " + cand.sdpMid + " candidate: " + cand.candidate);
+
-        this.peerConn.addIceCandidate(cand);
+        const cands = ev.getContent().candidates;
+        if (!cands) {
+            logger.info("Ignoring candidates event with no candidates!");
+            return;
+        }
+
+        for (const cand of cands) {
+            if (
+                (cand.sdpMid === null || cand.sdpMid === undefined) &&
+                (cand.sdpMLineIndex === null || cand.sdpMLineIndex === undefined)
+            ) {
+                logger.debug("Ignoring remote ICE candidate with no sdpMid or sdpMLineIndex");
+                return;
+            }
+            logger.debug("Got remote ICE " + cand.sdpMid + " candidate: " + cand.candidate);
+            this.peerConn.addIceCandidate(cand);
+        }
     }
 
     /**
@@ -837,10 +847,19 @@ export class MatrixCall extends EventEmitter {
             return;
         }
 
+        if (this.opponentPartyId !== undefined) {
+            logger.info(
+                `Ignoring answer from party ID ${msg.party_id}: ` +
+                `we already have an answer/reject from ${this.opponentPartyId}`,
+            );
+            return;
+        }
+
         this.opponentVersion = msg.version;
+        this.opponentPartyId = msg.party_id || null;
 
         try {
-            this.peerConn.setRemoteDescription(msg.answer);
+            await this.peerConn.setRemoteDescription(msg.answer);
         } catch (e) {
             logger.debug("Failed to set remote description", e);
             this.terminate(CallParty.Local, CallErrorCode.SetRemoteDescription, false);
@@ -868,8 +887,6 @@ export class MatrixCall extends EventEmitter {
         }
 
         const content = {
-            version: VOIP_PROTO_VERSION,
-            call_id: this.callId,
             // OpenWebRTC appears to add extra stuff (like the DTLS fingerprint)
             // to the description when setting it on the peerconnection.
             // According to the spec it should only add ICE
@@ -888,7 +905,7 @@ export class MatrixCall extends EventEmitter {
             lifetime: CALL_TIMEOUT_MS,
         };
         try {
-            await this.sendEvent('m.call.invite', content);
+            await this.sendVoipEvent('m.call.invite', content);
             this.setState(CallState.InviteSent);
             setTimeout(() => {
                 if (this.state === CallState.InviteSent) {
@@ -1043,12 +1060,22 @@ export class MatrixCall extends EventEmitter {
 
     onHangupReceived = (msg) => {
         logger.debug("Hangup received");
+
+        if (!this.partyIdMatches(msg)) {
+            logger.info(`Ignoring message from party ID ${msg.party_id}: our partner is ${this.opponentPartyId}`);
+            return;
+        }
+
         // default reason is user_hangup
         this.terminate(CallParty.Remote, msg.reason || CallErrorCode.UserHangup, true);
     };
 
     onRejectReceived = (msg) => {
         logger.debug("Reject received");
+
+        // No need to check party_id for reject because if we'd received either
+        // an answer or reject, we wouldn't be in state InviteSent
+
         if (this.state === CallState.InviteSent) {
             this.terminate(CallParty.Remote, CallErrorCode.UserHangup, true);
         } else {
@@ -1073,8 +1100,12 @@ export class MatrixCall extends EventEmitter {
      * @param {Object} content
      * @return {Promise}
      */
-    private sendEvent(eventType: string, content: object) {
+    private sendVoipEvent(eventType: string, content: object) {
-        return this.client.sendEvent(this.roomId, eventType, content);
+        return this.client.sendEvent(this.roomId, eventType, Object.assign({}, content, {
+            version: VOIP_PROTO_VERSION,
+            call_id: this.callId,
+            party_id: this.ourPartyId,
+        }));
     }
 
     sendCandidate(content: RTCIceCandidate) {
@@ -1176,12 +1207,10 @@ export class MatrixCall extends EventEmitter {
         this.candidateSendQueue = [];
         ++this.candidateSendTries;
         const content = {
-            version: VOIP_PROTO_VERSION,
-            call_id: this.callId,
             candidates: cands,
         };
         logger.debug("Attempting to send " + cands.length + " candidates");
-        this.sendEvent('m.call.candidates', content).then(() => {
+        this.sendVoipEvent('m.call.candidates', content).then(() => {
             this.candidateSendTries = 0;
             this.sendCandidateQueue();
         }, (error) => {
@@ -1224,7 +1253,7 @@ export class MatrixCall extends EventEmitter {
     }
 
     private createPeerConnection(): RTCPeerConnection {
-        const pc = new RTCPeerConnection({
+        const pc = new window.RTCPeerConnection({
             iceTransportPolicy: this.forceTURN ? 'relay' : undefined,
             iceServers: this.turnServers,
         });
@@ -1238,6 +1267,12 @@ export class MatrixCall extends EventEmitter {
 
         return pc;
     }
+
+    private partyIdMatches(msg): boolean {
+        // They must either match or both be absent (in which case opponentPartyId will be null)
+        const msgPartyId = msg.party_id || null;
+        return msgPartyId === this.opponentPartyId;
+    }
 }
 
 function setTracksEnabled(tracks: Array<MediaStreamTrack>, enabled: boolean) {
@@ -1256,7 +1291,7 @@ function isTracksEnabled(tracks: Array<MediaStreamTrack>) {
 }
 
 function getUserMediaVideoContraints(callType: CallType) {
-    const isWebkit = !!window.navigator.webkitGetUserMedia;
+    const isWebkit = !!navigator.webkitGetUserMedia;
 
     switch (callType) {
         case CallType.Voice:
@@ -1332,7 +1367,7 @@ export function createNewMatrixCall(client: any, roomId: string, options?: CallO
     try {
         const supported = Boolean(
             window.RTCPeerConnection || window.RTCSessionDescription ||
-            window.RTCIceCandidate || navigator.getUserMedia,
+            window.RTCIceCandidate || navigator.mediaDevices,
         );
         if (!supported) {
             logger.error("WebRTC is not supported in this browser / environment");

src/webrtc/callEventHandler.ts

@@ -28,7 +28,7 @@ export class CallEventHandler {
     client: MatrixClient;
     calls: Map<string, MatrixCall>;
     callEventBuffer: MatrixEvent[];
-    candidatesByCall: Map<string, Array<RTCIceCandidate>>;
+    candidateEventsByCall: Map<string, Array<MatrixEvent>>;
 
     constructor(client: MatrixClient) {
         this.client = client;
@@ -42,7 +42,7 @@ export class CallEventHandler {
         // This happens quite often, eg. replaying sync from storage, catchup sync
         // after loading and after we've been offline for a bit.
         this.callEventBuffer = [];
-        this.candidatesByCall = new Map<string, Array<RTCIceCandidate>>();
+        this.candidateEventsByCall = new Map<string, Array<MatrixEvent>>();
         this.client.on("sync", this.evaluateEventBuffer);
         this.client.on("event", this.onEvent);
     }
@@ -157,9 +157,9 @@ export class CallEventHandler {
             this.calls.set(call.callId, call);
 
             // if we stashed candidate events for that call ID, play them back now
-            if (this.candidatesByCall.get(call.callId)) {
+            if (this.candidateEventsByCall.get(call.callId)) {
-                for (const cand of this.candidatesByCall.get(call.callId)) {
+                for (const ev of this.candidateEventsByCall.get(call.callId)) {
-                    call.gotRemoteIceCandidate(cand);
+                    call.onRemoteIceCandidatesReceived(ev);
                 }
             }
 
@@ -221,16 +221,12 @@ export class CallEventHandler {
             }
             if (!call) {
                 // store the candidates; we may get a call eventually.
-                if (!this.candidatesByCall.has(content.call_id)) {
+                if (!this.candidateEventsByCall.has(content.call_id)) {
-                    this.candidatesByCall.set(content.call_id, []);
+                    this.candidateEventsByCall.set(content.call_id, []);
                 }
-                this.candidatesByCall.set(content.call_id, this.candidatesByCall.get(
+                this.candidateEventsByCall.get(content.call_id).push(event);
-                    content.call_id,
-                ).concat(content.candidates));
             } else {
-                for (const cand of content.candidates) {
+                call.onRemoteIceCandidatesReceived(event);
-                    call.gotRemoteIceCandidate(cand);
-                }
             }
         } else if ([EventType.CallHangup, EventType.CallReject].includes(event.getType())) {
             // Note that we also observe our own hangups here so we can see