fix: Join only the room in the start request

src/main/kotlin/org/jitsi/jibri/CallUrlInfo.kt

@@ -18,6 +18,7 @@
 package org.jitsi.jibri
 
 import com.fasterxml.jackson.annotation.JsonIgnore
+import org.jxmpp.jid.EntityBareJid
 import java.util.Objects
 
 /**
@@ -26,12 +27,19 @@ import java.util.Objects
  * url will be created by joining [baseUrl] and [callName] with a "/".  If
  * set, a list of [urlParams] will be concatenated after the call name with
  * a "#" in between.
+ *
+ * [authorizedRoom] is the room JID which the requester is authorized for. If it is set, the call page must verify
+ * that the conference it joined is this room. It is an [EntityBareJid] so that the comparison uses XMPP string
+ * preparation, like the server does, and not the spelling of the name. It is never read from a request body.
  */
 data class CallUrlInfo(
     val baseUrl: String = "",
     val callName: String = "",
     val tenant: String = "",
-    val urlParams: List<String> = listOf()
+    val urlParams: List<String> = listOf(),
+    @JsonIgnore
+    @get:JsonIgnore
+    val authorizedRoom: EntityBareJid? = null
 ) {
     @get:JsonIgnore
     val callUrl: String

src/main/kotlin/org/jitsi/jibri/Main.kt

@@ -42,6 +42,7 @@ import org.jitsi.metaconfig.config
 import org.jitsi.metaconfig.configSupplier
 import org.jitsi.utils.logging2.Logger
 import org.jitsi.utils.logging2.LoggerImpl
+import org.jitsi.xmpp.Smack
 import java.io.File
 import java.util.concurrent.TimeUnit
 import kotlin.system.exitProcess
@@ -51,6 +52,8 @@ val logger: Logger = LoggerImpl("org.jitsi.jibri.Main")
 fun main(args: Array<String>) {
     setupMetaconfigLogger()
     handleCommandLineArgs(args)
+    // Before anything creates a JID, so that all JIDs use the same stringprep.
+    Smack.initialize(useJitsiXmppStringprep = MainConfig.useJitsiJidValidation)
 
     logger.info("Jibri starting up with id ${MainConfig.jibriId}")
 
@@ -169,6 +172,10 @@ class MainConfig {
             "JibriConfig::jibriId" { Config.legacyConfigSource.jibriId!! }
             "jibri.id".from(Config.configSource)
         }
+
+        val useJitsiJidValidation: Boolean by config {
+            "jibri.api.xmpp.use-jitsi-jid-validation".from(Config.configSource)
+        }
     }
 }
 

src/main/kotlin/org/jitsi/jibri/api/http/HttpApi.kt

@@ -60,6 +60,12 @@ import org.jitsi.xmpp.extensions.jibri.JibriIq
 // TODO: this needs to include usageTimeout
 data class StartServiceParams(
     val sessionId: String,
+    /**
+     * The call to join. The HTTP API is for the operator of this Jibri, so the room name and the tenant in
+     * [CallParams.callUrlInfo] are taken as given: they are not checked for the form jitsi-meet uses, and
+     * [org.jitsi.jibri.CallUrlInfo.authorizedRoom] stays unset, so the call page does not verify the room it
+     * joined. The XMPP API does both, because there the request comes from a conference participant.
+     */
     val callParams: CallParams,
     /**
      * XMPP login information to be used if [RecordingSinkType] is

src/main/kotlin/org/jitsi/jibri/api/xmpp/XmppApi.kt

@@ -362,6 +362,7 @@ class XmppApi(
                 shouldRetry = true
             }
         } catch (e: BadRequestException) {
+            logger.info("Refusing the start request for room ${startJibriIq.room}: ${e.detail}")
             rejectBadRequest(startJibriIq, serviceStatusHandler, e)
         } catch (iq: UnsupportedIqMode) {
             logger.error("Unsupported IQ mode: ${iq.iqMode}")

src/main/kotlin/org/jitsi/jibri/selenium/pageobjects/AppCallPage.kt

@@ -3,6 +3,9 @@ package org.jitsi.jibri.selenium.pageobjects
 import org.jitsi.jibri.CallUrlInfo
 import org.jitsi.jibri.util.Resolution
 import org.jitsi.utils.logging2.createLogger
+import org.jxmpp.jid.EntityBareJid
+import org.jxmpp.jid.impl.JidCreate
+import org.jxmpp.stringprep.XmppStringprepException
 import org.openqa.selenium.TimeoutException
 import org.openqa.selenium.remote.RemoteWebDriver
 import org.openqa.selenium.support.PageFactory
@@ -54,11 +57,52 @@ class AppCallPage(driver: RemoteWebDriver) : AbstractPageObject(driver), CallPag
             }
         }
 
-        if (result) {
-            logger.info("Waited $totalTime to join the conference")
+        if (!result) {
+            return false
         }
+        logger.info("Waited $totalTime to join the conference")
+
+        return url.authorizedRoom == null || isInAuthorizedRoom(url.authorizedRoom)
+    }
 
-        return result
+    private fun isInAuthorizedRoom(authorizedRoom: EntityBareJid): Boolean {
+        val result = driver.executeScript(
+            """
+            try {
+                return { roomJid: APP.conference._room.room.roomjid };
+            } catch (e) {
+                return { error: e.message };
+            }
+            """.trimMargin()
+        ) as? Map<*, *>
+        val joinedRoomJid = result?.get("roomJid") as? String
+        // Compare prepared JIDs, not strings: the room the page joined went through the server's string preparation,
+        // which for example makes the name lowercase. The domain must match too, so a room with the same name in
+        // another tenant fails.
+        val joinedRoom = joinedRoomJid?.let {
+            try {
+                JidCreate.entityBareFrom(it)
+            } catch (e: XmppStringprepException) {
+                logger.error("The joined room JID '$it' is not a valid JID: ${e.message}")
+                null
+            }
+        }
+        if (joinedRoom == null) {
+            logger.error("Cannot read the joined room (${result?.get("error")}), so cannot verify it. Leaving.")
+        } else if (joinedRoom == authorizedRoom) {
+            return true
+        } else {
+            logger.error("Joined room $joinedRoomJid, but the request is for room $authorizedRoom. Leaving.")
+        }
+        try {
+            if (!leave()) {
+                logger.error("Failed to leave the room")
+            }
+        } catch (e: TimeoutException) {
+            // leave() times out when other participants stay in the room. We refuse the room either way.
+            logger.warn("Timed out waiting to be alone in the room after leaving")
+        }
+        return false
     }
 
     /**

src/main/kotlin/org/jitsi/jibri/selenium/pageobjects/ExternalAPIPage.kt

@@ -3,6 +3,9 @@ package org.jitsi.jibri.selenium.pageobjects
 import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
 import org.jitsi.jibri.CallUrlInfo
 import org.jitsi.utils.logging2.createLogger
+import org.jxmpp.jid.EntityBareJid
+import org.jxmpp.jid.impl.JidCreate
+import org.jxmpp.stringprep.XmppStringprepException
 import org.openqa.selenium.TimeoutException
 import org.openqa.selenium.remote.RemoteWebDriver
 import org.openqa.selenium.support.PageFactory
@@ -70,7 +73,7 @@ class ExternalAPIPage(driver: RemoteWebDriver) : AbstractPageObject(driver), Cal
                 result
             }
             logger.info("Recorder page initialized successfully")
-            true
+            url.authorizedRoom == null || isInAuthorizedRoom(url.authorizedRoom)
         } catch (e: TimeoutException) {
             logger.error("Failed to join conference: timeout waiting for conferenceJoined event")
             false
@@ -80,6 +83,44 @@ class ExternalAPIPage(driver: RemoteWebDriver) : AbstractPageObject(driver), Cal
         }
     }
 
+    /**
+     * The main room's "jid" from getRoomsInfo is our occupant JID when there are no breakout rooms, and the room JID
+     * when there are. Its bare form is the room JID in both cases.
+     */
+    private fun isInAuthorizedRoom(authorizedRoom: EntityBareJid): Boolean {
+        val joinedRoomJid = callRecorderApiAsync(
+            "getRoomsInfo",
+            """
+            api.getRoomsInfo().then(roomsData => {
+                const mainRoom = (roomsData.rooms || []).find(r => r?.isMainRoom);
+                cb(mainRoom?.jid || null);
+            }).catch(() => cb(null));
+            """
+        ) as? String
+        // Compare prepared JIDs, not strings: the room the page joined went through the server's string preparation,
+        // which for example makes the name lowercase. The domain must match too, so a room with the same name in
+        // another tenant fails.
+        val joinedRoom = joinedRoomJid?.let {
+            try {
+                JidCreate.from(it).asEntityBareJidIfPossible()
+            } catch (e: XmppStringprepException) {
+                logger.error("The joined room JID '$it' is not a valid JID: ${e.message}")
+                null
+            }
+        }
+        if (joinedRoom == null) {
+            logger.error("Cannot read the joined room, so cannot verify it. Leaving.")
+        } else if (joinedRoom == authorizedRoom) {
+            return true
+        } else {
+            logger.error("Joined room $joinedRoomJid, but the request is for room $authorizedRoom. Leaving.")
+        }
+        if (!leave()) {
+            logger.error("Failed to leave the room")
+        }
+        return false
+    }
+
     private fun extractRecorderHtml(): File {
         val tmpFile = File.createTempFile("jibri-recorder", ".html").apply { deleteOnExit() }
         val resource = javaClass.getResourceAsStream("/recorder.html")

src/main/kotlin/org/jitsi/jibri/util/XmppUtils.kt

@@ -18,8 +18,12 @@
 package org.jitsi.jibri.util
 
 import org.jitsi.jibri.CallUrlInfo
+import org.jitsi.jibri.error.BadRequestException
 import org.jitsi.xmpp.extensions.jibri.JibriIq
 import org.jxmpp.jid.EntityBareJid
+import java.net.URLDecoder
+import java.text.Normalizer
+import java.util.Locale
 
 /**
  * When we get a start [JibriIq] message, the room is given to us an [EntityBareJid] that we need to translate
@@ -31,6 +35,10 @@ fun getCallUrlInfoFromJid(
     xmppDomain: String,
     baseUrl: String?
 ): CallUrlInfo {
+    val callName = roomJid.localpart.toString()
+    if (!isBackendSafeRoomName(callName)) {
+        throw BadRequestException("Room name '$callName' is not in the form jitsi-meet uses")
+    }
     try {
         // The url domain is pulled from the xmpp domain of the connection sending the request
         var domain = roomJid.domain.toString()
@@ -40,8 +48,6 @@ fun getCallUrlInfoFromJid(
         // Now we need to extract a potential call subdomain, which will be anything that's left in the domain
         //  at this point before the configured xmpp domain.
         val subdomain = domain.subSequence(0, domain.indexOf(xmppDomain, ignoreCase = true)).trim('.')
-        // Now just grab the call name
-        val callName = roomJid.localpart.toString()
 
         // The call url is constructed from the baseCallUrl (base-url if any, or the xmpp domain), an optional subdomain, and a callname like so:
         // https://baseCallUrl/subdomain/callName
@@ -51,9 +57,17 @@ fun getCallUrlInfoFromJid(
         }
 
         return when {
-            subdomain.isEmpty() -> CallUrlInfo(baseCallUrl, callName, "")
-            else -> CallUrlInfo("$baseCallUrl/$subdomain", callName, subdomain.toString())
+            subdomain.isEmpty() -> CallUrlInfo(baseCallUrl, callName, "", authorizedRoom = roomJid)
+
+            else -> {
+                if (!isBackendSafeTenant(subdomain.toString())) {
+                    throw BadRequestException("Tenant '$subdomain' is not in the form jitsi-meet uses")
+                }
+                CallUrlInfo("$baseCallUrl/$subdomain", callName, subdomain.toString(), authorizedRoom = roomJid)
+            }
         }
+    } catch (e: BadRequestException) {
+        throw e
     } catch (e: Exception) {
         throw CallUrlInfoFromJidException(
             "Unable to extract call url info from Jid $roomJid (stripFromRoomDomain = $stripFromRoomDomain, " +
@@ -63,3 +77,58 @@ fun getCallUrlInfoFromJid(
 }
 
 class CallUrlInfoFromJidException(message: String) : Exception(message)
+
+private const val URI_COMPONENT_UNRESERVED = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.!~*'()"
+private val ENCODED_URI_COMPONENT = Regex("(?:[A-Za-z0-9\\-_.!~*'()]|%[0-9A-Fa-f]{2})+")
+
+// jitsi-meet removes these characters from a URL path segment (_ROOM_EXCLUDE_PATTERN in
+// react/features/base/util/uri.ts) before it decodes and normalizes the name. "!", "(", ")", "*" and "'" are
+// unreserved for encodeURIComponent, so a round trip through decode and encode alone does not detect them.
+private val REMOVED_BY_JITSI_MEET = Regex("[:?#\\[\\]@!$&'()*+,;=></\"]")
+
+/**
+ * Whether [name] is a room name (or tenant) equal, ignoring case, to what jitsi-meet's getBackendSafeRoomName
+ * produces. A name in any other form makes us join a room other than the one we were asked to join. For example, the
+ * browser cuts "a?b" at the "?" and resolves ".." as a path, jitsi-meet decodes "%61" to "a", and jitsi-meet removes
+ * the "*" from "a*".
+ */
+fun isBackendSafeRoomName(name: String): Boolean {
+    if (name == "." || name == ".." || !ENCODED_URI_COMPONENT.matches(name)) {
+        return false
+    }
+    if (REMOVED_BY_JITSI_MEET.containsMatchIn(name)) {
+        return false
+    }
+    val decoded = URLDecoder.decode(name, Charsets.UTF_8)
+    val normalized = Normalizer.normalize(decoded, Normalizer.Form.NFKC).lowercase(Locale.ROOT)
+    // A web server can decode "%2f" before it splits the tenant from the room, so "a%2fb" can put us in tenant "a".
+    // The check after the join also catches this, because the domain of the joined room is different. We refuse the
+    // name here too, so that the request fails before Jibri becomes busy.
+    if (normalized.contains('/')) {
+        return false
+    }
+    // XMPP nodeprep makes the localpart lowercase, so a name that differs only in case is the same room.
+    return encodeUriComponent(normalized).equals(name, ignoreCase = true)
+}
+
+// A tenant comes from a label of the MUC domain, which the XMPP server prepares: nameprep makes it lowercase and keeps
+// non-ASCII characters raw. nginx decodes the tenant from the URL path and jitsi-meet maps "." to "_", so jitsi-meet
+// never joins a tenant with a "%" or a "." in the MUC. Jicofo and the Prosody modules use the same rule.
+private val TENANT = Regex("(?:[A-Za-z0-9_-]|[^\\x00-\\x7f])+")
+
+/**
+ * Whether [tenant] is a tenant in the form jitsi-meet uses. A tenant in another form can put us in a different tenant
+ * once it is in a URL: a web server decodes "%2f" before it splits the path, and jitsi-meet maps "." to "_".
+ */
+fun isBackendSafeTenant(tenant: String): Boolean = TENANT.matches(tenant)
+
+private fun encodeUriComponent(str: String): String = buildString {
+    for (byte in str.toByteArray(Charsets.UTF_8)) {
+        val b = byte.toInt() and 0xff
+        if (b < 0x80 && b.toChar() in URI_COMPONENT_UNRESERVED) {
+            append(b.toChar())
+        } else {
+            append("%%%02x".format(b))
+        }
+    }
+}

src/main/resources/reference.conf

@@ -17,6 +17,8 @@ jibri {
     xmpp {
       // See example_xmpp_envs.conf for an example of what is expected here
       environments = []
+      // Whether to use JitsiXmppStringprep to validate JIDs. If set to false uses the default validation in Smack.
+      use-jitsi-jid-validation = true
       // When the XMPP connection a service was started on drops, how long to keep the service running while the
       // connection tries to resume its session (XEP-0198 stream management). Jicofo does not notice a drop that is
       // resumed. If the session is not resumed, or the connection was closed cleanly, the service stops right away.

src/test/kotlin/org/jitsi/jibri/CallUrlInfoTest.kt

@@ -17,6 +17,8 @@
 
 package org.jitsi.jibri
 
+import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
+import com.fasterxml.jackson.module.kotlin.readValue
 import io.kotest.core.spec.IsolationMode
 import io.kotest.core.spec.style.ShouldSpec
 import io.kotest.data.forAll
@@ -48,6 +50,15 @@ class CallUrlInfoTest : ShouldSpec() {
                 }
             }
         }
+        context("parsing a CallUrlInfo from JSON") {
+            should("ignore an authorizedRoom in the JSON") {
+                val info = jacksonObjectMapper().readValue<CallUrlInfo>(
+                    """{"baseUrl":"baseUrl","callName":"callName","authorizedRoom":"other"}"""
+                )
+                info.callName shouldBe "callName"
+                info.authorizedRoom shouldBe null
+            }
+        }
         context("a nullable CallUrlInfo instance") {
             should("not equal null") {
                 val nullableInfo: CallUrlInfo? = CallUrlInfo("baseUrl", "callName")

src/test/kotlin/org/jitsi/jibri/KotestProjectConfig.kt

@@ -19,8 +19,14 @@ package org.jitsi.jibri
 import io.kotest.core.config.AbstractProjectConfig
 import io.kotest.core.extensions.Extension
 import io.kotest.extensions.junitxml.JunitXmlReporter
+import org.jitsi.xmpp.Smack
 
 class KotestProjectConfig : AbstractProjectConfig() {
+    override suspend fun beforeProject() {
+        // Use the same JID stringprep as Main does with the default config.
+        Smack.initialize(useJitsiXmppStringprep = true)
+    }
+
     override fun extensions(): List<Extension> = listOf(
         /*
          * The JunitXmlReporter writes a junit5 compatible unit test output