WebRTC revisions and updates including the addition of secure connections. by ctrlaltdavid · Pull Request #1429 · vircadia/vircadia-native-core

ctrlaltdavid · 2021-10-26T07:10:50Z

Fix STUN servers used by WebRTC.
Fix handling of ICE candidates received from client.
Add server setting to enable/disable WebRTC client connections.
Add secure WebSocket support to the WebRTC signaling channel.
In order to use the microphone a web app needs to be served as HTTPS. And an HTTPS page requires WebSocket connections be WSS.
Use avatar position for "no skeleton" orb if the head position isn't available.
Fix domain server and assignment client crashes when app doesn't cleanly disconnect.

Works in conjunction with SDK PR: vircadia/vircadia-web-sdk#50

Cert names are now: vircadia-cert.key, etc.

vircadia-build-notifier · 2021-10-26T09:13:07Z

The following links are available:
build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-9bb2698-_2971dd61.dmg

build (ubuntu-18.04, full)

build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-9bb2698-_2971dd61.dmg

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-9bb2698-_2971dd61.exe

build (self-hosted_debian-11_aarch64, full)

vircadia-build-notifier · 2021-10-27T10:45:26Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-675dd90-_bb3d0fa5.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-675dd90-_bb3d0fa5.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-675dd90-_bb3d0fa5.exe

build (self-hosted_debian-11_aarch64, full)

daleglass · 2021-10-28T22:02:42Z

+            this));
+
+        auto dsDirPath = PathUtils::getAppLocalDataPath();
+        const QString KEY_FILENAME = "cert.key";


Constant should probably be on top.

Also it would be nice to have a more unique name than "cert.key". How about "vircadia-webrtc-cert.key"?

Same comment re constant locations.

Regarding the file name...
The cert isn't necessarily just for WebRTC: it could also be used to secure the server settings page as HTTPS.
The cert is for the DNS domain name being used as the address to connect to the domain server at sop could, in theory, be used for other things also. This might change if we switch to using IP address certs or some other SSL method. Perhaps vircadia-cert.key might be suitable.
See also: #1432

My thought re: the cert name: I'd like something unique because I found myself trying to figure out where's a given cert for a given program a few times, and there are cases where you may end up with multiple certs in the same directory. It's just nice to clearly know what a cert is for at a glance.

But yeah, vircadia-cert.key works for me.

I just realized one issue... It's entirely possible that multiple domains on the same server may use a different cert. We should probably keep that in mind now as that will most certainly become a thing.

vircadia-build-notifier · 2021-10-29T12:06:51Z

The following links are available:
build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-49a1abd-_8e3cfe98.dmg

build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-49a1abd-_8e3cfe98.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-49a1abd-_8e3cfe98.exe

build (self-hosted_debian-11_aarch64, full)

vircadia-build-notifier · 2021-10-29T21:33:51Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-f8cff4b-_ccd525a8.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-f8cff4b-_ccd525a8.dmg

build (ubuntu-18.04, full)

build (self-hosted_debian-11_aarch64, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-f8cff4b-_ccd525a8.exe

vircadia-build-notifier · 2021-10-31T03:17:04Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-894f6e8-_c76dc535.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-894f6e8-_c76dc535.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-894f6e8-_c76dc535.exe

build (self-hosted_debian-11_aarch64, full)

ksuprynowicz · 2021-11-13T17:58:35Z

For some reason it looks like 1429 solves high CPU usage bug. Can someone else build interface from this PR so we can connect to the server and see if audio and avatar mixers work?

ksuprynowicz · 2021-11-13T18:19:17Z

I tested it with Kalila. It indeed solves CPU usage bug.

ctrlaltdavid · 2021-11-13T18:32:28Z

Note that this PR is against the webapp branch which is currently based on master as at 8 Oct.

vircadia-build-notifier · 2021-11-14T14:45:38Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-5c9cab0-_861b8398.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-5c9cab0-_861b8398.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-5c9cab0-_861b8398.exe

build (self-hosted_debian-11_aarch64, full)

ksuprynowicz · 2021-11-14T17:15:20Z

The connection works now, but it's possible to crash server using web-sdk. To cause crash, you need at least two users logged in. Then one user opens multiple tabs and connects, mutes and unmutes each one a few times and closes them without disconnecting. It causes server to crash.
Server logs:
http://oaktown.pl/tmp/assignment_crash.txt
http://oaktown.pl/tmp/domain_crash.txt

ctrlaltdavid · 2021-11-15T08:36:11Z

The domain-server.exe crash happens 15 - 30s after closing or reloading the browser tab with the SDK example app connected to the domain server - if the WebRTC connection isn't explicitly disconnected. The following SDK example app PR prevents the crash but doesn't fix the underlying problem: vircadia/vircadia-web-sdk#61

vircadia-build-notifier · 2021-11-20T10:26:50Z

The following links are available:
build (macOS-10.15, client)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-Interface-PR1429-f037b52-_56d33b55.dmg

build (macOS-10.15, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-f037b52-_56d33b55.dmg

build (ubuntu-18.04, full)

build (windows-latest, full)

https://athena-public.s3.eu-west-3.amazonaws.com/builds/PR/vircadia_vircadia/PR1429/Vircadia-PR1429-f037b52-_56d33b55.exe

build (self-hosted_debian-11_aarch64, full)

ksuprynowicz · 2021-11-20T15:32:49Z

I just built and tested it on Linux and I can confirm. Crashes are gone now.

digisomni

Using a Windows 10 Interface + Chrome and testing against both the dev-1 (Windows 10) and dev-2 (Ubuntu 18.04) test servers, I couldn't get crashing to happen. Maybe there was one or two oddities with the example not connecting on the first try but otherwise it seems stable.

ctrlaltdavid added 2 commits October 26, 2021 11:21

Fix ICE servers used for STUN

0fcb942

Fix handling of ICE candidate received from client

2971dd6

ctrlaltdavid added the do not merge do not merge due to issues or pending updates label Oct 26, 2021

This was referenced Oct 26, 2021

WebRTC revisions. vircadia/vircadia-web-sdk#50

Merged

WebRTC tests #1417

Closed

Add WebRTC on/off switch to domain server settings

eb21eb3

ctrlaltdavid added 2 commits October 27, 2021 21:41

Reduce the number of ICE servers used

92e3d14

Add secure WebSocket support

bb3d0fa

ctrlaltdavid added needs CR (code review) needs testing (QA) The PR is ready for testing web sdk Related to the Vircadia Web SDK and removed do not merge do not merge due to issues or pending updates labels Oct 27, 2021

ctrlaltdavid changed the title ~~WebRTC Revisions~~ WebRTC revisions. Oct 27, 2021

digisomni requested a review from daleglass October 28, 2021 21:56

daleglass suggested changes Oct 28, 2021

View reviewed changes

Fix non-const lvalue reference

8e3cfe9

CR

ccd525a

digisomni added the unmerged-dev A project being developed in parallel to master, to be merged at a later date. label Oct 30, 2021

digisomni linked an issue Oct 30, 2021 that may be closed by this pull request

Security: Domain-Server WebUI is unsecure by default #867

Closed

digisomni mentioned this pull request Oct 30, 2021

Security: Domain-Server WebUI is unsecure by default #867

Closed

digisomni removed a link to an issue Oct 30, 2021

Security: Domain-Server WebUI is unsecure by default #867

Closed

Rename domain server cert files

c76dc53

daleglass approved these changes Oct 31, 2021

View reviewed changes

digisomni added CR Approved At least one code reviewer has approved the PR. and removed needs CR (code review) labels Oct 31, 2021

digisomni added the CR Approved At least one code reviewer has approved the PR. label Nov 13, 2021

daleglass approved these changes Nov 13, 2021

View reviewed changes

digisomni removed the QA Approved The PR has been tested successfully. label Nov 13, 2021

Merge branch 'webapp' into dev/webrtc-revisions

861b839

ctrlaltdavid added 3 commits November 20, 2021 11:06

Improve WebRTC debug

fb79d57

Fix domain server crash

cf57b50

Fix assignment client crash

56d33b5

ctrlaltdavid added needs CR (code review) needs testing (QA) The PR is ready for testing and removed CR Approved At least one code reviewer has approved the PR. labels Nov 20, 2021

digisomni requested review from daleglass and digisomni November 20, 2021 17:12

digisomni approved these changes Nov 20, 2021

View reviewed changes

digisomni added QA Approved The PR has been tested successfully. and removed needs testing (QA) The PR is ready for testing labels Nov 20, 2021

daleglass approved these changes Nov 20, 2021

View reviewed changes

digisomni changed the title ~~WebRTC revisions.~~ WebRTC revisions and updates including the addition of secure connections. Nov 20, 2021

digisomni added CR Approved At least one code reviewer has approved the PR. and removed needs CR (code review) labels Nov 20, 2021

digisomni merged commit 66fe21e into vircadia:webapp Nov 20, 2021

ctrlaltdavid mentioned this pull request Nov 20, 2021

Explicitly disconnect example app from domain upon window closing. vircadia/vircadia-web-sdk#61

Open

ctrlaltdavid deleted the dev/webrtc-revisions branch November 20, 2021 22:10

Uh oh!

Conversation

ctrlaltdavid commented Oct 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vircadia-build-notifier commented Oct 26, 2021

Uh oh!

vircadia-build-notifier commented Oct 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daleglass Oct 28, 2021

Choose a reason for hiding this comment

Uh oh!

ctrlaltdavid Oct 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daleglass Oct 30, 2021

Choose a reason for hiding this comment

Uh oh!

digisomni Oct 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vircadia-build-notifier commented Oct 29, 2021

Uh oh!

vircadia-build-notifier commented Oct 29, 2021

Uh oh!

vircadia-build-notifier commented Oct 31, 2021

Uh oh!

ksuprynowicz commented Nov 13, 2021

Uh oh!

ksuprynowicz commented Nov 13, 2021

Uh oh!

ctrlaltdavid commented Nov 13, 2021

Uh oh!

vircadia-build-notifier commented Nov 14, 2021

Uh oh!

ksuprynowicz commented Nov 14, 2021

Uh oh!

ctrlaltdavid commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vircadia-build-notifier commented Nov 20, 2021

Uh oh!

ksuprynowicz commented Nov 20, 2021

Uh oh!

digisomni left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ctrlaltdavid commented Oct 26, 2021 •

edited

Loading

ctrlaltdavid Oct 29, 2021 •

edited

Loading

ctrlaltdavid commented Nov 15, 2021 •

edited

Loading