meetecho / simple-whip-server

Simple WHIP Server (based on the Janus WebRTC Server)
GNU General Public License v3.0
93 stars 24 forks source link

Randomize resource address when contacting WHIP endpoint #10

Closed lminiero closed 1 year ago

lminiero commented 1 year ago

Last week, at CommCon, Sandro Gauci made a very interesting presentation on difference security considerations related to VoIP and WebRTC, and briefly touched on WHIP as well. You can refer to the presentation for the details, but one of the main issues was related to the potential predictability of the resource address, which could easily lead to abuses.

This implementation does indeed have predictable resource addresses, since it just turns /endpoint/<id> to /resource/<id>: as such, I thought I'd randomize the ID part of the resource URI instead, meaning that publishing to /endpoint/<id> will now result in a random /resource/<something else> url instead, a bit like we already do for WHEP (where random resources are needed since in that case multiple subscribers can consume the same endpoint). I took advantage of this fix to fix a couple of other things (mostly eslint related), but nothing meaningful.

Planning to merge soon, so feedback welcome!

mondain commented 1 year ago

Given enough time, resources, and interest; pretty much anything can be hacked. I'm not reflecting on your change here, but more of the use-case for it; to me it seems there is an overwhelming need to over-complicate some of the flows we deal with on the back-end. In terms of this particular item, what is the actual harm in predicting the id's from one url to the location follow up response? If tokens and / or auth are in-play who really cares, there would be no loss or exposure of the content.

lminiero commented 1 year ago

@mondain I'm not sure if the slides are publicly available, but in a nutshell the predictability of the resource url could be used to quite simply send a DELETE on WHIP sessions that aren't your own, or possibly flood it with PATCH requests with broken candidates, or forced ICE restarts, etc. While the token partly protects you on that, if the token is known (e.g., it's shared) it's still a problem, because it allows someone else to highjack your session if they want.

lminiero commented 1 year ago

Notice that this impacts the resource url, not the endpoint one: that one remains the same, and multiple POSTs would still be rejected once a session is up (and until it goes away).