Skip to content

Commit

Permalink
feat: Use RocksXmppPrecisStringprep for striter JID validation. (jits…
Browse files Browse the repository at this point in the history
  • Loading branch information
bgrozev authored Jan 18, 2024
1 parent 713f6fb commit ed9df12
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 0 deletions.
4 changes: 4 additions & 0 deletions jicofo/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@
<artifactId>smack-tcp</artifactId>
</dependency>
<!-- https://mvnrepository.com/artifact/org.jetbrains/annotations -->
<dependency>
<groupId>org.jxmpp</groupId>
<artifactId>jxmpp-stringprep-rocksxmppprecis</artifactId>
</dependency>
<dependency>
<groupId>org.jetbrains</groupId>
<artifactId>annotations</artifactId>
Expand Down
1 change: 1 addition & 0 deletions jicofo/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<Bug pattern="SA_LOCAL_SELF_ASSIGNMENT"/>
<!-- False positive with kotlin lazy initialization. -->
<Bug pattern="BC_IMPOSSIBLE_INSTANCEOF"/>
<Bug pattern="MS_EXPOSE_REP"/>
</Or>
</And>
</Match>
Expand Down
6 changes: 6 additions & 0 deletions jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ import org.jivesoftware.smack.SmackConfiguration
import org.jivesoftware.smack.parsing.ExceptionLoggingCallback
import org.jivesoftware.smack.provider.ProviderManager
import org.jivesoftware.smackx.bytestreams.socks5.Socks5Proxy
import org.jxmpp.JxmppContext
import org.jxmpp.stringprep.rocksxmppprecis.RocksXmppPrecisStringprep

fun initializeSmack() {
System.setProperty("jdk.xml.entityExpansionLimit", "0")
Expand All @@ -61,6 +63,10 @@ fun initializeSmack() {
System.setProperty("jdk.xml.totalEntitySizeLimit", "0")
System.setProperty("jdk.xml.maxXMLNameLimit", "524288")
System.setProperty("jdk.xml.entityReplacementLimit", "0")

// The default SimpleXmppStringrep is very permissive and allows some invalid JIDs that we don't want to allow.
JxmppContext.setDefaultXmppStringprep(RocksXmppPrecisStringprep.INSTANCE)

SmackConfiguration.setDefaultReplyTimeout(15000)
// if there is a parsing error, do not break the connection to the server(the default behaviour) as we need it for
// the other conferences.
Expand Down
121 changes: 121 additions & 0 deletions jicofo/src/test/kotlin/org/jitsi/jicofo/xmpp/JidTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Jicofo, the Jitsi Conference Focus.
*
* Copyright @ 2024-Present 8x8, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jitsi.jicofo.xmpp

import io.kotest.assertions.throwables.shouldThrow
import io.kotest.assertions.withClue
import io.kotest.core.spec.IsolationMode
import io.kotest.core.spec.style.ShouldSpec
import io.kotest.core.test.TestCase
import io.kotest.matchers.shouldNotBe
import io.kotest.matchers.types.shouldBeInstanceOf
import org.jxmpp.JxmppContext
import org.jxmpp.jid.impl.JidCreate
import org.jxmpp.stringprep.XmppStringprepException
import org.jxmpp.stringprep.rocksxmppprecis.RocksXmppPrecisStringprep

/**
* Test JID parsing. The lists below are based on the jxmpp corpora here, plus a couple additional ones:
* https://github.com/igniterealtime/jxmpp/tree/master/jxmpp-strings-testframework/src/main/resources/xmpp-strings/jids/valid/main
* https://github.com/igniterealtime/jxmpp/blob/master/jxmpp-strings-testframework/src/main/resources/xmpp-strings/jids/invalid/main
*/
class JidTest : ShouldSpec() {
override fun isolationMode(): IsolationMode {
return IsolationMode.SingleInstance
}
override suspend fun beforeAny(testCase: TestCase) {
super.beforeAny(testCase)
initializeSmack()
}

init {
context("Parsing valid JIDs") {
JxmppContext.getDefaultContext().xmppStringprep.shouldBeInstanceOf<RocksXmppPrecisStringprep>()
validJids.forEach {
withClue(it) {
JidCreate.from(it) shouldNotBe null
}
}
}
context("Parsing invalid JIDs") {
JxmppContext.getDefaultContext().xmppStringprep.shouldBeInstanceOf<RocksXmppPrecisStringprep>()
invalidJids.forEach {
withClue(it) {
shouldThrow<XmppStringprepException> {
JidCreate.from((it))
}
}
}
}
}
}

val validJids = listOf(
"[email protected]",
"[email protected]/foo",
"[email protected]/foo bar",
"[email protected]/foo@bar",
"foo\\[email protected]",
"[email protected]",
"fuß[email protected]",
"π@example.com",
"Σ@example.com",
"ς@example.com",
"[email protected]/♚",
"example.com",
"example.com/foobar",
"a.example.com/[email protected]",
"server/resource@foo",
"server/resource@foo/bar",
"user@CaSe-InSeNsItIvE",
"[email protected]",
// "user@[2001:638:a000:4134::ffff:40]",
// "user@[2001:638:a000:4134::ffff:40%eno1]",
// "user@averylongdomainpartisstillvalideventhoughitexceedsthesixtyfourbytelimitofdnslabels",
"long-conference-name-1245c711a15e466687b6333577d83e0b@" +
"conference.vpaas-magic-cookie-a32a0c3311ee432eab711fa1fdf34793.8x8.vc",
"[email protected]/🍺"
)

val invalidJids = listOf(
"jul\u0001[email protected]",
"\"juliet\"@example.com",
"foo [email protected]",
// This fails due to a corner case in JidCreate when "example.com" is already cached as a DomainpartJid
// "@example.com/",
"henryⅣ@example.com",
"♚@example.com",
"juliet@",
"/foobar",
"node@/server",
"@server",
"@server/resource",
"@/resource",
"@/",
"/",
"@",
"user@",
"user@@",
"user@@host",
"user@@host/resource",
"user@@host/",
"[email protected]/؜x",
"[email protected]@example.org",
"foo\u0000[email protected]",
"foobar@ex\u0000ample.org"
)
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
<assembly.skipAssembly>false</assembly.skipAssembly>
<jetty.version>11.0.14</jetty.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<!-- Make sure this matches the version of the jxmpp artifacts inheriteb from smack. -->
<jxmpp.version>1.0.3</jxmpp.version>
<smack.version>4.4.6</smack.version>
<kotlin.version>1.9.10</kotlin.version>
<kotest.version>5.7.2</kotest.version>
Expand Down Expand Up @@ -137,6 +139,11 @@
<artifactId>smack-tcp</artifactId>
<version>${smack.version}</version>
</dependency>
<dependency>
<groupId>org.jxmpp</groupId>
<artifactId>jxmpp-stringprep-rocksxmppprecis</artifactId>
<version>${jxmpp.version}</version>
</dependency>
<!-- https://mvnrepository.com/artifact/org.jetbrains/annotations -->
<dependency>
<groupId>org.jetbrains</groupId>
Expand Down

0 comments on commit ed9df12

Please sign in to comment.