From ed9df126de882a34809c967fb1e06728f8ac31da Mon Sep 17 00:00:00 2001 From: bgrozev Date: Thu, 18 Jan 2024 09:34:07 -0600 Subject: [PATCH] feat: Use RocksXmppPrecisStringprep for striter JID validation. (#1125) --- jicofo/pom.xml | 4 + jicofo/spotbugs-exclude.xml | 1 + .../kotlin/org/jitsi/jicofo/xmpp/Smack.kt | 6 + .../kotlin/org/jitsi/jicofo/xmpp/JidTest.kt | 121 ++++++++++++++++++ pom.xml | 7 + 5 files changed, 139 insertions(+) create mode 100644 jicofo/src/test/kotlin/org/jitsi/jicofo/xmpp/JidTest.kt diff --git a/jicofo/pom.xml b/jicofo/pom.xml index a7e8603c9b..1e95d079b8 100644 --- a/jicofo/pom.xml +++ b/jicofo/pom.xml @@ -55,6 +55,10 @@ smack-tcp + + org.jxmpp + jxmpp-stringprep-rocksxmppprecis + org.jetbrains annotations diff --git a/jicofo/spotbugs-exclude.xml b/jicofo/spotbugs-exclude.xml index 017a93fa65..765714637d 100644 --- a/jicofo/spotbugs-exclude.xml +++ b/jicofo/spotbugs-exclude.xml @@ -34,6 +34,7 @@ + diff --git a/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt b/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt index f3577eb0a5..b04a88fde9 100644 --- a/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt +++ b/jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/Smack.kt @@ -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") @@ -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. diff --git a/jicofo/src/test/kotlin/org/jitsi/jicofo/xmpp/JidTest.kt b/jicofo/src/test/kotlin/org/jitsi/jicofo/xmpp/JidTest.kt new file mode 100644 index 0000000000..bea00ea5d6 --- /dev/null +++ b/jicofo/src/test/kotlin/org/jitsi/jicofo/xmpp/JidTest.kt @@ -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() + validJids.forEach { + withClue(it) { + JidCreate.from(it) shouldNotBe null + } + } + } + context("Parsing invalid JIDs") { + JxmppContext.getDefaultContext().xmppStringprep.shouldBeInstanceOf() + invalidJids.forEach { + withClue(it) { + shouldThrow { + JidCreate.from((it)) + } + } + } + } + } +} + +val validJids = listOf( + "juliet@example.com", + "juliet@example.com/foo", + "juliet@example.com/foo bar", + "juliet@example.com/foo@bar", + "foo\\20bar@example.com", + "fussball@example.com", + "fußball@example.com", + "π@example.com", + "Σ@example.com", + "ς@example.com", + "king@example.com/♚", + "example.com", + "example.com/foobar", + "a.example.com/b@example.net", + "server/resource@foo", + "server/resource@foo/bar", + "user@CaSe-InSeNsItIvE", + "user@192.168.1.1", + // "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", + "user@example.org/🍺" +) + +val invalidJids = listOf( + "jul\u0001iet@example.c", + "\"juliet\"@example.com", + "foo bar@example.com", + // 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/", + "xsf@muc.xmpp.org/؜x", + "username@example.org@example.org", + "foo\u0000bar@example.org", + "foobar@ex\u0000ample.org" +) diff --git a/pom.xml b/pom.xml index ec8ba20b64..137b5f3267 100644 --- a/pom.xml +++ b/pom.xml @@ -36,6 +36,8 @@ false 11.0.14 UTF-8 + + 1.0.3 4.4.6 1.9.10 5.7.2 @@ -137,6 +139,11 @@ smack-tcp ${smack.version} + + org.jxmpp + jxmpp-stringprep-rocksxmppprecis + ${jxmpp.version} + org.jetbrains