Updated jitsi-xmpp-extensions version & send jibri base url in xmppcl… #430
Conversation
|
Xmpp-extension upgrade PR. |
|
Hi, thanks for your contribution! |
| @@ -286,7 +286,7 @@ class XmppApi( | |||
| startIq.room, | |||
| xmppEnvironment.stripFromRoomDomain, | |||
| xmppEnvironment.xmppDomain, | |||
| xmppEnvironment.baseUrl | |||
| startIq.baseUrl | |||
There was a problem hiding this comment.
My concern is that we're not doing any checking of this baseUrl. Without addition checks, this could be used by a malicious user to send your jibris to a poisoned site which records the credentials jibri uses when logging in. In addition, this entirely overrides the Xmpp API without keeping the previous behavior as default. In my opinion this should be a separate section of code which first checks for a startIq baseUrl and confirms it's allowed, and then uses the environment base URL as a backup the start IQ doesn't include it.
There was a problem hiding this comment.
We do not do any validation for baseUrl in case of Http,
https://github.com/jitsi/jibri/blob/master/doc/http_api.md
There was a problem hiding this comment.
This is because the HTTP api is entirely a different use case. It's meant to be for jibris who expose a command and control interface. It's up to whomever is building that to control access to it. In the case of XMPP it's a much more exposed interface and therefore deserves this level of checks. Without this check, every default jibri and jitsi-meet install will be vulnerable to this type of attack.
|
I'm -1 on this approach. Jibri is already able to connect to several XMPP environments and having one extra connection should not be problematic since it uses very little traffic. |
@aaronkvanmeerten Thanks for reviewing. Are you asking to keep an allowed list of baseUrl in xmpp jibri config & validate the same here. |
|
You can have a jibri connect to the same host under 2 different names, if you have DNS records for it. |
…ient