From d20f0c161fe46c7db4cb5c3ac478260a7d3a9ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5var=20Aamb=C3=B8=20Fosstveit?= Date: Fri, 27 Mar 2020 01:36:11 +0100 Subject: [PATCH 1/3] Handle client reconnects better --- app/src/RoomClient.js | 29 +++++++++++ server/lib/Room.js | 115 ++++++++++++++++++++++++++++-------------- server/package.json | 4 +- server/server.js | 4 +- 4 files changed, 111 insertions(+), 41 deletions(-) diff --git a/app/src/RoomClient.js b/app/src/RoomClient.js index e133a30..204643d 100644 --- a/app/src/RoomClient.js +++ b/app/src/RoomClient.js @@ -1520,6 +1520,28 @@ export default class RoomClient }) })); + if (this._screenSharingProducer) + this._screenSharingProducer.close(); + + if (this._webcamProducer) + this._webcamProducer.close(); + + if (this._micProducer) + this._micProducer.close(); + + // Close mediasoup Transports. + if (this._sendTransport) + { + this._sendTransport.close(); + this._sendTransport = null; + } + + if (this._recvTransport) + { + this._recvTransport.close(); + this._recvTransport = null; + } + store.dispatch(roomActions.setRoomState('connecting')); }); @@ -1721,6 +1743,13 @@ export default class RoomClient break; } + + case 'roomBack': + { + await this._joinRoom({ joinVideo }); + + break; + } case 'lockRoom': { diff --git a/server/lib/Room.js b/server/lib/Room.js index f0ceb66..b6ee85b 100644 --- a/server/lib/Room.js +++ b/server/lib/Room.js @@ -2,6 +2,8 @@ const EventEmitter = require('events').EventEmitter; const axios = require('axios'); const Logger = require('./Logger'); const Lobby = require('./Lobby'); +const { v4: uuidv4 } = require('uuid'); +const jwt = require('jsonwebtoken'); const userRoles = require('../userRoles'); const config = require('../config/config'); @@ -46,6 +48,8 @@ class Room extends EventEmitter super(); this.setMaxListeners(Infinity); + this._uuid = uuidv4(); + // Room ID. this._roomId = roomId; @@ -120,9 +124,26 @@ class Room extends EventEmitter this.emit('close'); } - handlePeer(peer) + handlePeer({ peer, token }) { - logger.info('handlePeer() [peer:"%s", roles:"%s"]', peer.id, peer.roles); + logger.info('handlePeer() [peer:"%s", roles:"%s", token:"%s"]', peer.id, peer.roles, token); + + let verifiedPeer = false; + + if (token) + { + try + { + const decoded = jwt.verify(token, this._uuid); + + if (decoded.id === peer.id) + verifiedPeer = true; + } + catch (err) + { + logger.warn('handlePeer() | invalid token'); + } + } // Allow reconnections, remove old peer if (this._peers[peer.id]) @@ -134,8 +155,11 @@ class Room extends EventEmitter this._peers[peer.id].close(); } + // Returning user + if (verifiedPeer) + this._peerJoining(peer, true); // Always let ADMIN in, even if locked - if (peer.roles.includes(userRoles.ADMIN)) + else if (peer.roles.includes(userRoles.ADMIN)) this._peerJoining(peer); else if (this._locked) this._parkPeer(peer); @@ -332,7 +356,7 @@ class Room extends EventEmitter } } - async _peerJoining(peer) + async _peerJoining(peer, returning = false) { peer.socket.join(this._roomId); @@ -343,45 +367,58 @@ class Room extends EventEmitter this._handlePeer(peer); - let turnServers; - - if ('turnAPIURI' in config) + if (returning) { - try - { - const { data } = await axios.get( - config.turnAPIURI, - { - params : { - 'uri_schema' : 'turn', - 'transport' : 'tcp', - 'ip_ver' : 'ipv4', - 'servercount' : '2', - 'api_key' : config.turnAPIKey, - 'ip' : peer.socket.request.connection.remoteAddress - } - }); - - turnServers = [ { - urls : data.uris, - username : data.username, - credential : data.password - } ]; - } - catch (error) - { - if ('backupTurnServers' in config) - turnServers = config.backupTurnServers; - - logger.error('_peerJoining() | error on REST turn [error:"%o"]', error); - } + this._notification(peer.socket, 'roomBack'); } - else if ('backupTurnServers' in config) + else { - turnServers = config.backupTurnServers; - } + const token = jwt.sign({ id: peer.id }, this._uuid, { noTimestamp: true }); - this._notification(peer.socket, 'roomReady', { turnServers }); + peer.socket.handshake.session.token = token; + + peer.socket.handshake.session.save(); + + let turnServers; + + if ('turnAPIURI' in config) + { + try + { + const { data } = await axios.get( + config.turnAPIURI, + { + params : { + 'uri_schema' : 'turn', + 'transport' : 'tcp', + 'ip_ver' : 'ipv4', + 'servercount' : '2', + 'api_key' : config.turnAPIKey, + 'ip' : peer.socket.request.connection.remoteAddress + } + }); + + turnServers = [ { + urls : data.uris, + username : data.username, + credential : data.password + } ]; + } + catch (error) + { + if ('backupTurnServers' in config) + turnServers = config.backupTurnServers; + + logger.error('_peerJoining() | error on REST turn [error:"%o"]', error); + } + } + else if ('backupTurnServers' in config) + { + turnServers = config.backupTurnServers; + } + + this._notification(peer.socket, 'roomReady', { turnServers }); + } } _handlePeer(peer) diff --git a/server/package.json b/server/package.json index b05676d..ae1f3b9 100644 --- a/server/package.json +++ b/server/package.json @@ -25,6 +25,7 @@ "express-socket.io-session": "^1.3.5", "helmet": "^3.21.2", "ims-lti": "^3.0.2", + "jsonwebtoken": "^8.5.1", "mediasoup": "^3.5.5", "openid-client": "^3.7.3", "passport": "^0.4.0", @@ -32,6 +33,7 @@ "pidusage": "^2.0.17", "redis": "^2.8.0", "socket.io": "^2.3.0", - "spdy": "^4.0.1" + "spdy": "^4.0.1", + "uuid": "^7.0.2" } } diff --git a/server/server.js b/server/server.js index ebf951e..2e50a8c 100755 --- a/server/server.js +++ b/server/server.js @@ -467,6 +467,8 @@ async function runWebSocketServer() const room = await getOrCreateRoom({ roomId }); const peer = new Peer({ id: peerId, roomId, socket }); + const { token } = socket.handshake.session; + peers.set(peerId, peer); peer.on('close', () => peers.delete(peerId)); @@ -495,7 +497,7 @@ async function runWebSocketServer() } } - room.handlePeer(peer); + room.handlePeer({ peer, token }); }) .catch((error) => { From 3043098f0c42ad727e7e743b80d9893d265fabda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5var=20Aamb=C3=B8=20Fosstveit?= Date: Fri, 27 Mar 2020 10:38:51 +0100 Subject: [PATCH 2/3] Remove jwt, not needed --- server/lib/Room.js | 27 ++++----------------------- server/package.json | 1 - server/server.js | 25 ++++++++++++++++++++++--- 3 files changed, 26 insertions(+), 27 deletions(-) diff --git a/server/lib/Room.js b/server/lib/Room.js index b6ee85b..4950123 100644 --- a/server/lib/Room.js +++ b/server/lib/Room.js @@ -3,7 +3,6 @@ const axios = require('axios'); const Logger = require('./Logger'); const Lobby = require('./Lobby'); const { v4: uuidv4 } = require('uuid'); -const jwt = require('jsonwebtoken'); const userRoles = require('../userRoles'); const config = require('../config/config'); @@ -128,31 +127,15 @@ class Room extends EventEmitter { logger.info('handlePeer() [peer:"%s", roles:"%s", token:"%s"]', peer.id, peer.roles, token); - let verifiedPeer = false; + // This peer is returning, reconnect + const verifiedPeer = token && token === this._uuid; - if (token) - { - try - { - const decoded = jwt.verify(token, this._uuid); - - if (decoded.id === peer.id) - verifiedPeer = true; - } - catch (err) - { - logger.warn('handlePeer() | invalid token'); - } - } - - // Allow reconnections, remove old peer + // Should not happen if (this._peers[peer.id]) { logger.warn( 'handleConnection() | there is already a peer with same peerId [peer:"%s"]', peer.id); - - this._peers[peer.id].close(); } // Returning user @@ -373,9 +356,7 @@ class Room extends EventEmitter } else { - const token = jwt.sign({ id: peer.id }, this._uuid, { noTimestamp: true }); - - peer.socket.handshake.session.token = token; + peer.socket.handshake.session.token = this._uuid; peer.socket.handshake.session.save(); diff --git a/server/package.json b/server/package.json index ae1f3b9..50a6831 100644 --- a/server/package.json +++ b/server/package.json @@ -25,7 +25,6 @@ "express-socket.io-session": "^1.3.5", "helmet": "^3.21.2", "ims-lti": "^3.0.2", - "jsonwebtoken": "^8.5.1", "mediasoup": "^3.5.5", "openid-client": "^3.7.3", "passport": "^0.4.0", diff --git a/server/server.js b/server/server.js index 2e50a8c..5e19248 100755 --- a/server/server.js +++ b/server/server.js @@ -464,11 +464,30 @@ async function runWebSocketServer() queue.push(async () => { - const room = await getOrCreateRoom({ roomId }); - const peer = new Peer({ id: peerId, roomId, socket }); - const { token } = socket.handshake.session; + const room = await getOrCreateRoom({ roomId }); + + let peer = peers.get(peerId); + + if (peer) + { + if (token) + { + peer.close(); + + peer = null; + } + else + { + socket.disconnect(true); + + return; + } + } + + peer = new Peer({ id: peerId, roomId, socket }); + peers.set(peerId, peer); peer.on('close', () => peers.delete(peerId)); From 87d40375624c1b6559e5734a49aa7909d36ed6da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5var=20Aamb=C3=B8=20Fosstveit?= Date: Sat, 28 Mar 2020 23:20:37 +0100 Subject: [PATCH 3/3] We need jwt to make sure no one can hijack peerId --- app/src/RoomClient.js | 24 +++++++++++++++++++++++- server/lib/Room.js | 30 ++++++++++++++++++++++++------ server/package.json | 1 + server/server.js | 25 ++++++++++++------------- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/app/src/RoomClient.js b/app/src/RoomClient.js index 204643d..d301b86 100644 --- a/app/src/RoomClient.js +++ b/app/src/RoomClient.js @@ -1521,24 +1521,46 @@ export default class RoomClient })); if (this._screenSharingProducer) + { this._screenSharingProducer.close(); + store.dispatch( + producerActions.removeProducer(this._screenSharingProducer.id)); + + this._screenSharingProducer = null; + } + if (this._webcamProducer) + { this._webcamProducer.close(); + store.dispatch( + producerActions.removeProducer(this._webcamProducer.id)); + + this._webcamProducer = null; + } + if (this._micProducer) + { this._micProducer.close(); - // Close mediasoup Transports. + store.dispatch( + producerActions.removeProducer(this._micProducer.id)); + + this._micProducer = null; + } + if (this._sendTransport) { this._sendTransport.close(); + this._sendTransport = null; } if (this._recvTransport) { this._recvTransport.close(); + this._recvTransport = null; } diff --git a/server/lib/Room.js b/server/lib/Room.js index 4950123..521007d 100644 --- a/server/lib/Room.js +++ b/server/lib/Room.js @@ -3,6 +3,7 @@ const axios = require('axios'); const Logger = require('./Logger'); const Lobby = require('./Lobby'); const { v4: uuidv4 } = require('uuid'); +const jwt = require('jsonwebtoken'); const userRoles = require('../userRoles'); const config = require('../config/config'); @@ -123,12 +124,27 @@ class Room extends EventEmitter this.emit('close'); } - handlePeer({ peer, token }) + verifyPeer({ id, token }) { - logger.info('handlePeer() [peer:"%s", roles:"%s", token:"%s"]', peer.id, peer.roles, token); + try + { + const decoded = jwt.verify(token, this._uuid); - // This peer is returning, reconnect - const verifiedPeer = token && token === this._uuid; + logger.info('verifyPeer() [decoded:"%o"]', decoded); + + return decoded.id === id; + } + catch (err) + { + logger.warn('verifyPeer() | invalid token'); + } + + return false; + } + + handlePeer({ peer, returning }) + { + logger.info('handlePeer() [peer:"%s", roles:"%s", returning:"%s"]', peer.id, peer.roles, returning); // Should not happen if (this._peers[peer.id]) @@ -139,7 +155,7 @@ class Room extends EventEmitter } // Returning user - if (verifiedPeer) + if (returning) this._peerJoining(peer, true); // Always let ADMIN in, even if locked else if (peer.roles.includes(userRoles.ADMIN)) @@ -356,7 +372,9 @@ class Room extends EventEmitter } else { - peer.socket.handshake.session.token = this._uuid; + const token = jwt.sign({ id: peer.id }, this._uuid, { noTimestamp: true }); + + peer.socket.handshake.session.token = token; peer.socket.handshake.session.save(); diff --git a/server/package.json b/server/package.json index 50a6831..ae1f3b9 100644 --- a/server/package.json +++ b/server/package.json @@ -25,6 +25,7 @@ "express-socket.io-session": "^1.3.5", "helmet": "^3.21.2", "ims-lti": "^3.0.2", + "jsonwebtoken": "^8.5.1", "mediasoup": "^3.5.5", "openid-client": "^3.7.3", "passport": "^0.4.0", diff --git a/server/server.js b/server/server.js index 5e19248..c1c0f61 100755 --- a/server/server.js +++ b/server/server.js @@ -469,21 +469,20 @@ async function runWebSocketServer() const room = await getOrCreateRoom({ roomId }); let peer = peers.get(peerId); + let returning = false; - if (peer) - { - if (token) - { + if (peer && !token) + { // Don't allow hijacking sessions + socket.disconnect(true); + + return; + } + else if (token && room.verifyPeer({ id: peerId, token })) + { // Returning user, remove if old peer exists + if (peer) peer.close(); - peer = null; - } - else - { - socket.disconnect(true); - - return; - } + returning = true; } peer = new Peer({ id: peerId, roomId, socket }); @@ -516,7 +515,7 @@ async function runWebSocketServer() } } - room.handlePeer({ peer, token }); + room.handlePeer({ peer, returning }); }) .catch((error) => {