Skip to content

fix: Join only the room in the start request - #648

Merged
damencho merged 11 commits into
masterfrom
fix/room-name-confusion
Oct 27, 2026
Merged

damencho merged 11 commits into
masterfrom
fix/room-name-confusion

Conversation

@bgrozev

@bgrozev bgrozev commented Oct 27, 2026

Copy link
Copy Markdown
Member

Jibri makes the call URL from the room JID in the start request. jitsi-meet and the browser change some room names when they read the URL. For example, a?b, %61 and a* all join the room a. Jibri then joins and records a room that is different from the room in the request.

This PR makes these changes:

  • Refuse a start request when the room name or the tenant is not in the form that jitsi-meet uses. Jibri refuses it as a bad request, before it becomes busy.
    • A room name must be equal to the output of getBackendSafeRoomName in jitsi-meet. It must not contain a character that jitsi-meet removes from the URL. It must not decode to /, . or ...
    • A tenant must contain only letters, digits, _, - and non-ASCII characters. Jicofo and the Prosody modules use the same rule.
  • After the join, compare the JID of the joined room with the room JID in the request. If they are different, leave the room and fail the session before capture starts. The domain must also be equal, so a room with the same name in a different tenant fails.
  • Call Smack.initialize at startup, as Jicofo does.
    • It sets JitsiXmppStringprep to validate JIDs. The new flag jibri.api.xmpp.use-jitsi-jid-validation (default true) controls this.
    • It also makes Smack log and drop a stanza that it cannot parse. Before, Smack closed the connection.

The HTTP API does not change. It uses the room name as it is given.

aaronkvanmeerten and others added 11 commits October 26, 2026 16:21
Refuse a start request when the room localpart or tenant is not in the
form that jitsi-meet's getBackendSafeRoomName produces. Jitsi-meet decodes
and encodes again the name in the URL path, so a name in another form
makes Jibri join a different room. For example, "a?b" becomes "a".

After the join, compare the joined room with the authorized room. On a
mismatch, leave and fail the join before capture starts.
Refuse a tenant that contains a ".". Jitsi-meet maps the "." to "_" in the
MUC domain, so the call joins a different tenant.

Refuse a room name or tenant that decodes to a "/" or is "." or "..". A web
server or the browser can read these as path segments.

Log when the room cannot be read after the join, and when leave fails.
Catch only the expected timeout from leave in AppCallPage.

Log a refused start request in XmppApi. A room name refusal happens
before JibriManager.validate, so nothing logged it before.

Add AppCallPage tests and more room name, tenant and leave tests.
jitsi-meet removes the characters in _ROOM_EXCLUDE_PATTERN from a URL path
segment before it decodes and normalizes the room name. "!", "(", ")", "*"
and "'" are unreserved for encodeURIComponent, so the decode and encode
round trip kept them and did not detect them. A room "a*" made us join "a".
Refuse a name that contains any character from that set.

Use neutral room names in the tests.
A tenant comes from a label of the MUC domain. Nameprep makes it lowercase
and keeps non-ASCII characters raw. nginx decodes the tenant from the URL
path and jitsi-meet maps "." to "_", so a tenant never contains "%" or ".".
Check the tenant with that rule instead of the room name rule, which
accepted a percent-encoded tenant and refused a raw non-ASCII one.
The post-join check compared strings, which only worked because the
request check had already forced the authorized room into lowercase ASCII.
Keep the authorized room as a Localpart and compare it with the prepared
localpart of the room the page joined, so the check holds on its own and
follows the server's string preparation instead of the spelling.

Leave the room when the joined room JID or name does not parse.
The room name checks and the post-join room verification only apply to the
XMPP API, where the request comes from a conference participant. Say so
where the HTTP API start request is defined, so the trust assumption is
explicit.
Call Smack.initialize at startup, before anything creates a JID, as
jicofo does. With the new jibri.api.xmpp.use-jitsi-jid-validation flag
(on by default), JIDs use JitsiXmppStringprep: the Rocks PRECIS profiles,
with "_" and "%" allowed in the domain. With the flag off, Jibri keeps
the default stringprep of Smack.

Smack.initialize also removes the JDK XML parser limits, logs and drops
a stanza that fails to parse instead of closing the connection, and
disables the SOCKS5 proxy.

Run the tests with the same stringprep. A tenant with "?" or "*" now fails
to parse as a JID, so test those cases on their own.
Keep the room JID from the start request as the authorized room, and
compare it with the JID of the room that the client joined. Before, the
check compared only the localpart. Now the domain must also match, so a
join to the same room name in a different tenant fails. Jibri is never
redirected to a visitor node, so the domain of the joined room is the
domain from the request.

AppCallPage reads the room JID from the client. ExternalAPIPage reads it
from getRoomsInfo. Without breakout rooms, getRoomsInfo gives the occupant
JID of the main room, and with breakout rooms it gives the room JID. The
bare form is the room JID in both cases. The recorder page no longer
needs to keep the room name from the videoConferenceJoined event.
The check after the join now compares the full room JID, so it also catches a name that puts us in a different tenant. Say that the check before the join still fails the request before Jibri becomes busy.
…sion

# Conflicts:
#	src/main/resources/reference.conf
@damencho
damencho merged commit e05e116 into master Oct 27, 2026
4 checks passed
@damencho
damencho deleted the fix/room-name-confusion branch October 27, 2026 17:25
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

Successfully merging this pull request may close these issues.

4 participants