Push-to-Talk Support for Jitsi

[anoadragon453]

Continuation from #7709, that was originally on a separate repo's branch.

Implements the riot-web portion of Push-to-Talk support for Jitsi voice and video calls. The matrix-react-sdk portion is matrix-org/matrix-react-sdk#2280.

Adds a new native node addon, iohook, which is used for listening to shortcuts globally, meaning even when Riot is minimized. As such, iohook needs to be compiled, however like most native node addons, compiled versions for your platform are downloaded during npm install for you. The idea is you should never have to compile your own version unless you are using an unsupported platform. The currently supported platforms are Windows, Mac and Linux, however I am unsure what the support is for *BSD.

Of course there are those who wish to compile their own software, including ourselves for our releases. For this usecase, a script has been provided in ./scripts/build-native-modules.sh which supports MacOS and Linux. Usage instructions are at docs/native-node-modules.md and within the script itself.

In terms of security, iohook is set to stop itself if it is no longer in charge of any shortcuts. It will only enable itself during a Jitsi call or while recording a new shortcut in settings. While recording a new shortcut, iohook will report every keyboard shortcut that is pressed on the keyboard. This is necessary for shortcut recording. However, if Riot is minimized or otherwise somehow removed from focus while recording a shortcut, the recording session will end.

All of the security functionality is controlled by the Main process, with all of the Renderer processes acting like unprivileged clients. Thus, assuming a security flaw like an XSS only gives you access to functions in the Renderer process, no malicious keylogging should be possible.

The code is set up to allow for any global shortcut to be registered, not just those regarding Push-To-Talk functionality. However, the amount of shortcuts that can be registered at one time is currently controlled by const keybindingRegistrationLimit, which is set to 1, so this limit will need to be raised if any future functionality need be added.

[jryans]

@anoadragon453, thanks for all your work in this space so far! 😁

I'd like to work out a way to land this soon (so that it's not rotting away in a PR). What do you think about changing to do the following:

• Test for the native node module(s) needed with runtime checks
• Do not add the native node module as an app dependency (instead record what should be added in a doc)

With that, we should hopefully be able to review and merge this at least. It would require manual steps to use it for now, but that's at least forward progress.

[anoadragon453]

Hi @jryans, and thanks! Some clarifying questions though:

• Test for the native node module(s) needed with runtime checks

Test as in checking to see if it exists, and if not, not enabling the functionality?

• Do not add the native node module as an app dependency (instead record what should be added in a doc)

So take it out of package.json and add to/rework docs/native-node-modules.md to reflect that (how to build/run npm install iohook)?

[jryans]

Test as in checking to see if it exists, and if not, not enabling the functionality?

Correct, for example:

let ioHook;
try {
    ioHook = require('iohook');
catch (e) { 
    // Not available
}

The code that makes use of it should be changed to test for ioHook (or maybe create some wrapper function as needed).

So take it out of package.json and add to/rework docs/native-node-modules.md to reflect that (how to build/run npm install iohook)?

Yes, that's right. We can't list it as a dependency for the moment, as that would cause us to accidentally ship the third-party-built version. I suppose the actual command might be closer to yarn add iohook.

[anoadragon453]

@jryans Right, all makes sense. I'm currently working on once again getting the code working with latest develop (and electron 6), then I'll get on cleaning up everything so it can work with or without the module :)

[jryans]

@anoadragon453 When ready, feel free to flag me specifically for review if you like, as I'm mentoring similar work on the seshat PRs.

[t3chguy]

This needs porting to https://github.com/vector-im/riot-desktop when you get a chance :)

[Palid]

@anoadragon453 is this still relevant?

[SimonBrandner]

@anoadragon453 is this still relevant?

I believe it is though it's probably quite bitrotten by now

[anoadragon453]

@Palid @SimonBrandner. Yes and yes. It's relevant but doesn't need active review currently. It probably doesn't need to be open; any new implementation will likely be done in a separate PR anyhow.