Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow multiple jibri baseurl for single Xmpp host #429

Open
Tinkesh-Kumar opened this issue Aug 8, 2021 · 9 comments
Open

Allow multiple jibri baseurl for single Xmpp host #429

Tinkesh-Kumar opened this issue Aug 8, 2021 · 9 comments

Comments

@Tinkesh-Kumar
Copy link

If multiple meeting host is using single prosody, Jibri will not allow to record the session as base url will be same for both company1 & company2.
Ex:-
meet.company1.com is using XMPP host1
meet.company2.com is also using XMPP host1

In this case, since jibri allow only one base url for XMPP host, Recording will not work for both company1 & company2. Is my understanding correct?

Is there any reason for single baseurl for xmpp hosts?

@oanaianc
Copy link
Contributor

oanaianc commented Aug 9, 2021

If you want to setup both company1 & company2, you will have to set two xmpp environments, one for each domain. 
Also the baseUrl is not mandatory; if is set the jibri will join it, otherwise it will join the https://xmpp-domain
eg.
xmpp {
environments = [
{
name = "company1",
xmpp-server-hosts = [""],
xmpp-domain = "meet.company1.com",
base-url = "",
control-login {
......
},
{
name = "company2",
xmpp-server-hosts = [""],
xmpp-domain = "meet.company2.com",
base-url = "",
control-login {
......
},
]
}

@Tinkesh-Kumar
Copy link
Author

We wanted to optimise this. We do not want to create multiple xmpp host just to support jibri. I see httpAPI already support this. Can same thing being done for XMPP API.

@aaronkvanmeerten
Copy link
Member

The main reasoning for not doing this in the XMPP message was to avoid jibri being launched against unsupported domains, via a maliciously built XMPP message. So, one way to handle this will be to have a list of allowed domains in jibri, and check the incoming requested domain against that. I believe the team would be open to pull requests that implemented this functionality, if anyone is interested. Changes will likely be needed in Jicofo and Jibri to fully enable this functionality.

@aaronkvanmeerten
Copy link
Member

Another thought I had: if XMPP host1 is being re-used, then either URL will work, no? Unless you have special prosody configuration, two webservers with meet.company1.com and meet.company2.com pointed to XMPP host1 will simply re-use the same prosody backend. This means that meet.company1.com/abc and meet.company2.com/abc are equivalent, unless you've done some special configuration of both config.js for meet, and prosody.cfg.lua to provide separate MUCs for both. Once you've already done this much customization, one additional jibri configuration doesn't seem particularly onerous to support this use-case.

@Tinkesh-Kumar
Copy link
Author

The main reasoning for not doing this in the XMPP message was to avoid jibri being launched against unsupported domains, via a maliciously built XMPP message. So, one way to handle this will be to have a list of allowed domains in jibri, and check the incoming requested domain against that. I believe the team would be open to pull requests that implemented this functionality, if anyone is interested. Changes will likely be needed in Jicofo and Jibri to fully enable this functionality.

Thanks for understanding the requirement. Can you please suggest the change in jibri & jicofo. I see that jibri picks up the configuration (public-url/ baseUrl) from XMPP config. Xmpp config is being picked up based on the host of of the jibri start iq. In our use case, baseUrl need to be set at runtime the way it is done in httpAPI.

Another thought I had: if XMPP host1 is being re-used, then either URL will work, no? Unless you have special prosody configuration, two webservers with meet.company1.com and meet.company2.com pointed to XMPP host1 will simply re-use the same prosody backend. This means that meet.company1.com/abc and meet.company2.com/abc are equivalent, unless you've done some special configuration of both config.js for meet, and prosody.cfg.lua to provide separate MUCs for both. Once you've already done this much customization, one additional jibri configuration doesn't seem particularly onerous to support this use-case

If XMPP host1 is being re-used, then either URL will work, no? :-> No we allow multiple meet host to connect to same xmpp server. So in order to allow recording for both meet.company1.com and meet.company2.com, jibri has to launched with different base url.

This means that meet.company1.com/abc and meet.company2.com/abc are equivalent
Yes, This is equivalent for prosody.
For both it will have same muc id.
Yes.

I think we can just take as input in start JibriIQ and set the baseurl, as we are doing in the httpAPI. This should solve the problem.

@aaronkvanmeerten
Copy link
Member

The main reasoning for not doing this in the XMPP message was to avoid jibri being launched against unsupported domains, via a maliciously built XMPP message. So, one way to handle this will be to have a list of allowed domains in jibri, and check the incoming requested domain against that. I believe the team would be open to pull requests that implemented this functionality, if anyone is interested. Changes will likely be needed in Jicofo and Jibri to fully enable this functionality.

Thanks for understanding the requirement. Can you please suggest the change in jibri & jicofo. I see that jibri picks up the configuration (public-url/ baseUrl) from XMPP config. Xmpp config is being picked up based on the host of of the jibri start iq. In our use case, baseUrl need to be set at runtime the way it is done in httpAPI.

The issue here I believe would be linking the public URL to something jicofo knows about, and then allowing jicofo to set this when creating the start JibriIQ. At the moment in my understanding, this is not available. Jicofo knows the XMPP domain of the room as well as the room name, but not explicitly the URL where the jibri needs to join.

Another thought I had: if XMPP host1 is being re-used, then either URL will work, no? Unless you have special prosody configuration, two webservers with meet.company1.com and meet.company2.com pointed to XMPP host1 will simply re-use the same prosody backend. This means that meet.company1.com/abc and meet.company2.com/abc are equivalent, unless you've done some special configuration of both config.js for meet, and prosody.cfg.lua to provide separate MUCs for both. Once you've already done this much customization, one additional jibri configuration doesn't seem particularly onerous to support this use-case

If XMPP host1 is being re-used, then either URL will work, no? :-> No we allow multiple meet host to connect to same xmpp server. So in order to allow recording for both meet.company1.com and meet.company2.com, jibri has to launched with different base url.

It makes sense that you'd want this, but in order to make it work, jicofo will need to know which URL goes with which domain. Depending on how you've already configured this, the clients would also need to be in a different muc with a different XMPP hostname in prosody. If you haven't already configured it this way explicitly, you may want to test and confirm that meet.company1.com/abc and meet.company2.com/abc are actually distinct conferences.

This means that meet.company1.com/abc and meet.company2.com/abc are equivalent
Yes, This is equivalent for prosody.
For both it will have same muc id.
Yes.

I think we can just take as input in start JibriIQ and set the baseurl, as we are doing in the httpAPI. This should solve the problem.

Correct, as long as this information can be sourced reliably from jicofo, who initiates the start message.

@Tinkesh-Kumar
Copy link
Author

Tinkesh-Kumar commented Aug 9, 2021

https://github.com/jitsi/jitsi-xmpp-extensions/pull/44/files
https://github.com/jitsi/jibri/pull/430/files

This two changes would suffice our requirement. Because JibriStartIQ message is being sent by the frontend. Frontend will be able to detect the baseurl from where it is being sent. I know this is not backward compatible change.But if this looks good this can be made backward compatible.
Can you please review.

@Tinkesh-Kumar
Copy link
Author

It makes sense that you'd want this, but in order to make it work, jicofo will need to know which URL goes with which domain. Depending on how you've already configured this, the clients would also need to be in a different muc with a different XMPP hostname in prosody. If you haven't already configured it this way explicitly, you may want to test and confirm that meet.company1.com/abc and meet.company2.com/abc are actually distinct conferences.

Valid point, Will verify this.

@damencho
Copy link
Member

damencho commented Aug 9, 2021

We wanted to optimise this. We do not want to create multiple xmpp host just to support jibri. I see httpAPI already support this. Can same thing being done for XMPP API.

Will the Oana proposal work for you? I don't see the point of adding code complexity for a corner case, and the change for it adds possible security exploit when this can be achieved already with a configuration...
Even your approach with the client passing the base-url, cannot be merged as it is without a configuration which you will need to update with the whitelisted domains, so there is a step where you need to whitelist all possible domains in jibri ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants