From a017643254adaeb9687e80d2b3f1f4f0b748f651 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Mon, 26 Oct 2026 10:56:33 +0000 Subject: [PATCH 01/22] OF-2643: Share one rule for the domains that S2S peers may address Server Dialback decided whether it services the domain in a db:result 'to' with its own check. That check matched subdomains with contains(), so any domain that included the XMPP domain's text could pass if it had a component route, and it threw a NullPointerException when 'to' was missing. Stream header validation needs the same decision, so the rule moves to ServerStanzaHandler.isLocallyServiced: the XMPP domain, a proper subdomain with a component route, or a domain for which trunking is enabled (exact match, as before). A db:result without 'to' is now an unknown host, as XEP-0220 requires the attribute. --- .../openfire/net/ServerStanzaHandler.java | 29 +++ .../openfire/server/ServerDialback.java | 27 ++- ...erverStanzaHandlerLocallyServicedTest.java | 182 ++++++++++++++++++ .../server/ServerDialbackHostUnknownTest.java | 140 ++++++++++++++ 4 files changed, 364 insertions(+), 14 deletions(-) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index 3509c93347..9cf877cd48 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -35,6 +35,7 @@ import org.xmlpull.v1.XmlPullParserException; import org.xmpp.packet.*; +import javax.annotation.Nonnull; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -71,6 +72,34 @@ public class ServerStanzaHandler extends StanzaHandler { .setDynamic(true) .build(); + /** + * Checks if a domain that a remote server addresses is serviced by this server. That is the case for the XMPP + * domain of this server, for subdomains of it that have a component route, and for domains for which trunking is + * enabled. + * + * @param domain the domain to check. + * @return true if the domain is serviced by this server, otherwise false (including when the value is not a valid domain name). + */ + public static boolean isLocallyServiced(@Nonnull final String domain) { + final String requested; + try { + requested = JID.domainprep(domain); + } catch (final IllegalArgumentException e) { + Log.debug("Value '{}' is not a valid domain name.", domain, e); + return false; + } + + final XMPPServer server = XMPPServer.getInstance(); + final String xmppDomain = server.getServerInfo().getXMPPDomain(); + if (requested.equals(xmppDomain)) { + return true; + } + if (requested.endsWith("." + xmppDomain) && server.getRoutingTable().hasComponentRoute(new JID(requested))) { + return true; + } + return Trunking.isTrunkingEnabledFor(requested); + } + private final boolean directTLS; public ServerStanzaHandler(PacketRouter router, Connection connection, boolean directTLS) { diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java index f53a92a8b3..51069452b1 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java @@ -105,7 +105,6 @@ private enum VerifyResult { private Connection connection; private DomainPair domainPair; private final SessionManager sessionManager = SessionManager.getInstance(); - private final RoutingTable routingTable = XMPPServer.getInstance().getRoutingTable(); /** * Returns true if server dialback is enabled. When enabled remote servers may connect to this @@ -643,19 +642,19 @@ else if (isHostUnknown(recipient)) { } } - private boolean isHostUnknown(String recipient) { - boolean host_unknown = !domainPair.getLocal().equals(recipient); - // If the recipient does not match the local domain then check if it matches a subdomain. This - // trick is useful when subdomains of this server are registered in the DNS so remote - // servers may establish connections directly to a subdomain of this server - if (host_unknown && recipient.contains(domainPair.getLocal())) { - host_unknown = !routingTable.hasComponentRoute(new JID(recipient)); - } - - if (host_unknown) { - host_unknown = !Trunking.isTrunkingEnabledFor(recipient); - } - return host_unknown; + /** + * Checks if the domain that the remote server addresses is not serviced by this server. Subdomains of this server + * are serviced when they have a component route, which is useful when they are registered in the DNS so that remote + * servers can establish connections directly to them. + * + * XEP-0220 requires the 'to' attribute on dialback requests, so a request that has none is for an unknown host. + * + * @param recipient the domain addressed by the remote server (can be null). + * @return true if the domain is not serviced by this server. + * @see ServerStanzaHandler#isLocallyServiced(String) + */ + boolean isHostUnknown(String recipient) { + return recipient == null || !ServerStanzaHandler.isLocallyServiced(recipient); } private VerifyResult sendVerifyKey(String key, StreamID streamID, String recipient, String remoteDomain, Writer writer, XMPPPacketReader reader, Socket socket, boolean skipTLS, boolean directTLS) throws IOException, XmlPullParserException, RemoteConnectionFailedException { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java new file mode 100644 index 0000000000..ae595b31e9 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -0,0 +1,182 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.net; + +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.RoutingTable; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.xmpp.packet.JID; + +import java.util.List; +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doReturn; + +/** + * Verifies {@link ServerStanzaHandler#isLocallyServiced(String)}, which decides whether a domain that a remote server + * addresses is serviced by this server (OF-2643). Both the validation of stream headers and Server Dialback use it. + */ +public class ServerStanzaHandlerLocallyServicedTest +{ + private static final String COMPONENT_DOMAIN = "conference." + Fixtures.XMPP_DOMAIN; + + private static final String TRUNKED_DOMAIN = "trunked.example.net"; + + private static final String UNSERVICED_DOMAIN = "unserviced.example.com"; + + private RoutingTable routingTable; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @BeforeEach + public void setUp() + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + routingTable = Fixtures.mockRoutingTable(); + doReturn(true).when(routingTable).hasComponentRoute(new JID(COMPONENT_DOMAIN)); + doReturn(routingTable).when(xmppServer).getRoutingTable(); + XMPPServer.setInstance(xmppServer); + } + + /** + * Asserts that the XMPP domain of this server is serviced. + */ + @Test + public void testXmppDomainIsServiced() + { + assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN)); + } + + /** + * Asserts that the XMPP domain of this server is serviced when a peer names it in a different case. Domain names + * are compared after normalisation, as RFC 6120 requires. + */ + @Test + public void testXmppDomainInDifferentCaseIsServiced() + { + assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT))); + } + + /** + * Asserts that a subdomain of the XMPP domain that has a component route (such as a multi-user chat service) is + * serviced. + */ + @Test + public void testComponentSubdomainIsServiced() + { + assertTrue(ServerStanzaHandler.isLocallyServiced(COMPONENT_DOMAIN)); + } + + /** + * Asserts that a subdomain of the XMPP domain that has no component route is not serviced. + */ + @Test + public void testSubdomainWithoutComponentIsNotServiced() + { + assertFalse(ServerStanzaHandler.isLocallyServiced("unknown." + Fixtures.XMPP_DOMAIN)); + } + + /** + * Asserts that a domain that merely ends with the text of the XMPP domain, without being a subdomain of it, is not + * serviced, even when a component route exists for it. + */ + @Test + public void testDomainThatContainsXmppDomainIsNotServiced() + { + // Setup test fixture. + final String lookalike = "not" + Fixtures.XMPP_DOMAIN; + doReturn(true).when(routingTable).hasComponentRoute(new JID(lookalike)); + + // Execute system under test & verify result. + assertFalse(ServerStanzaHandler.isLocallyServiced(lookalike), "A domain that is not a subdomain of the XMPP domain must not be treated as one."); + } + + /** + * Asserts that a domain for which trunking is enabled is serviced. + */ + @Test + public void testTrunkedDomainIsServiced() + { + // Setup test fixture. + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + + // Execute system under test & verify result. + assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + } + + /** + * Asserts that a domain that is configured for trunking is not serviced when trunking itself is disabled. + */ + @Test + public void testTrunkedDomainIsNotServicedWhenTrunkingIsDisabled() + { + // Setup test fixture. + Trunking.ENABLED.setValue(false); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + + // Execute system under test & verify result. + assertFalse(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + } + + /** + * Asserts that a subdomain of a domain for which trunking is enabled is not serviced: trunking applies to the + * domains that are configured, and nothing else. + */ + @Test + public void testSubdomainOfTrunkedDomainIsNotServiced() + { + // Setup test fixture. + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + + // Execute system under test & verify result. + assertFalse(ServerStanzaHandler.isLocallyServiced("conference." + TRUNKED_DOMAIN)); + } + + /** + * Asserts that a domain that is unrelated to this server is not serviced. + */ + @Test + public void testUnrelatedDomainIsNotServiced() + { + assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN)); + } + + /** + * Asserts that a value that cannot be a domain name (here, because a label exceeds 63 characters) is not serviced, + * rather than causing an exception. + */ + @Test + public void testInvalidDomainIsNotServiced() + { + assertFalse(ServerStanzaHandler.isLocallyServiced("a".repeat(64) + ".example.org")); + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java new file mode 100644 index 0000000000..ef9a6e8f49 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.server; + +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.RoutingTable; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.net.Trunking; +import org.jivesoftware.openfire.session.DomainPair; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.xmpp.packet.JID; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; + +/** + * Verifies how Server Dialback, acting as the Receiving Server, decides whether the domain that a remote server asks + * to be authenticated for is serviced by this server (OF-2643). + * + * This decision is shared with the validation of stream headers, so that both paths accept the same domains. + */ +public class ServerDialbackHostUnknownTest +{ + private static final String COMPONENT_DOMAIN = "conference." + Fixtures.XMPP_DOMAIN; + + private static final String TRUNKED_DOMAIN = "trunked.example.net"; + + private RoutingTable routingTable; + + private ServerDialback dialback; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @BeforeEach + public void setUp() + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + routingTable = Fixtures.mockRoutingTable(); + doReturn(true).when(routingTable).hasComponentRoute(new JID(COMPONENT_DOMAIN)); + doReturn(routingTable).when(xmppServer).getRoutingTable(); + XMPPServer.setInstance(xmppServer); + + dialback = new ServerDialback(mock(Connection.class), new DomainPair(Fixtures.XMPP_DOMAIN, "remote.example.org")); + } + + /** + * Asserts that the XMPP domain of this server is not unknown. + */ + @Test + public void testXmppDomainIsKnown() + { + assertFalse(dialback.isHostUnknown(Fixtures.XMPP_DOMAIN)); + } + + /** + * Asserts that a subdomain of the XMPP domain that has a component route is not unknown. + */ + @Test + public void testComponentSubdomainIsKnown() + { + assertFalse(dialback.isHostUnknown(COMPONENT_DOMAIN)); + } + + /** + * Asserts that a domain for which trunking is enabled is not unknown. + */ + @Test + public void testTrunkedDomainIsKnown() + { + // Setup test fixture. + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + + // Execute system under test & verify result. + assertFalse(dialback.isHostUnknown(TRUNKED_DOMAIN)); + } + + /** + * Asserts that a domain that is unrelated to this server is unknown. + */ + @Test + public void testUnrelatedDomainIsUnknown() + { + assertTrue(dialback.isHostUnknown("unserviced.example.com")); + } + + /** + * Asserts that a domain that merely ends with the text of the XMPP domain, without being a subdomain of it, is + * unknown, even when a component route exists for it. + */ + @Test + public void testDomainThatContainsXmppDomainIsUnknown() + { + // Setup test fixture. + final String lookalike = "not" + Fixtures.XMPP_DOMAIN; + doReturn(true).when(routingTable).hasComponentRoute(new JID(lookalike)); + + // Execute system under test & verify result. + assertTrue(dialback.isHostUnknown(lookalike), "A domain that is not a subdomain of the XMPP domain must not be treated as one."); + } + + /** + * Asserts that a dialback request that does not name the domain it is for is treated as being for an unknown + * domain, rather than causing an exception. XEP-0220 requires the 'to' attribute on these requests. + */ + @Test + public void testMissingDomainIsUnknown() + { + assertTrue(dialback.isHostUnknown(null)); + } +} From a8adb0d52c3b38e8d5364140df694349f7378a00 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Mon, 26 Oct 2026 11:06:33 +0000 Subject: [PATCH 02/22] OF-2643: Extract the refusal of a stream into StanzaHandler.refuseStream The error path of createSession opens a stream that carries the stream error, delivers it and closes the connection. Validating the stream header that follows TLS negotiation needs the same response, so that code moves to a method of its own. Behaviour does not change; the new tests pin the existing response, which had no coverage, and pass both before and after the extraction. --- .../openfire/net/StanzaHandler.java | 44 +++++---- .../net/StanzaHandlerRefuseStreamTest.java | 91 ++++++++----------- 2 files changed, 68 insertions(+), 67 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index 4a5c7eaf10..0cd490728a 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -933,24 +933,36 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I catch (final StreamErrorException ex) { Log.warn("Failed to create a session, as the stream opened by the peer has a problem: {} - '{}' (a full stack trace is logged on debug level). Closing connection: {}. session is now: {}", ex.getStreamError().getCondition(), ex.getStreamError().getText(), connection, session); Log.debug("Failed to create a session.", ex); - final Element stream = DocumentHelper.createElement(QName.get("stream", "stream", "http://etherx.jabber.org/streams")); - final Document document = DocumentHelper.createDocument(stream); - document.setXMLEncoding(StandardCharsets.UTF_8.toString()); - // The refused header need not declare a content namespace. When it does not, the error is answered in the - // namespace of this handler, rather than copying the absent namespace (which would fail to build the error). - final String contentNamespace = xpp.getNamespace(null); - stream.add(Namespace.get(contentNamespace != null ? contentNamespace : getNamespace().getURI())); - stream.addAttribute("from", host); - stream.addAttribute("id", STREAM_ID_FACTORY.createStreamID().getID()); - stream.addAttribute("version", "1.0"); - stream.add(ex.getStreamError().getElement()); + refuseStream(xpp, host, ex.getStreamError()); + } + } - // Deliver stanza - connection.deliverRawText(StringUtils.asUnclosedStream(document)); + /** + * Responds to a stream header that cannot be accepted by opening a stream that carries the provided stream error, + * and then closes the underlying connection. + * + * @param xpp a parser positioned on the stream header that is refused. + * @param host the value of the 'to' attribute of the refused stream header (can be null). + * @param error the stream error to send to the peer. + */ + protected void refuseStream(XmlPullParser xpp, String host, StreamError error) { + final Element stream = DocumentHelper.createElement(QName.get("stream", "stream", "http://etherx.jabber.org/streams")); + final Document document = DocumentHelper.createDocument(stream); + document.setXMLEncoding(StandardCharsets.UTF_8.toString()); + // The refused header need not declare a content namespace. When it does not, the error is answered in the + // namespace of this handler, rather than copying the absent namespace (which would fail to build the error). + final String contentNamespace = xpp.getNamespace(null); + stream.add(Namespace.get(contentNamespace != null ? contentNamespace : getNamespace().getURI())); + stream.addAttribute("from", host); + stream.addAttribute("id", STREAM_ID_FACTORY.createStreamID().getID()); + stream.addAttribute("version", "1.0"); + stream.add(error.getElement()); + + // Deliver stanza + connection.deliverRawText(StringUtils.asUnclosedStream(document)); - // Close the underlying connection - connection.close(); - } + // Close the underlying connection + connection.close(); } /** diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 8ad656af65..0a7fb76ef7 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -21,46 +21,30 @@ import org.jivesoftware.openfire.PacketRouter; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.util.JiveGlobals; -import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.xmlpull.v1.XmlPullParserFactory; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.withSettings; /** - * Verifies that {@link StanzaHandler} delivers a stream error to the peer when it refuses a stream header that does - * not declare a content namespace. - * - * A peer can open a stream with a header that names a stream namespace but no default (content) namespace, such as - * {@code }. When such a - * header is refused, building the stream that carries the error used to copy the content namespace of the refused - * header, which is absent here, so the peer never received the stream error. - * - * These tests trigger the refusal with a stream namespace that is not the one RFC 6120 section 4.8.1 requires. That is - * a reason to refuse which is independent of the missing content namespace itself, so the tests exercise the - * error-building defect without asserting that a header which omits the content namespace is refused in its own right. - * - * @author Matthew Vivian + * Verifies how {@link StanzaHandler} refuses a stream header that it cannot accept: it opens a stream that carries + * the stream error, and closes the connection without creating a session. */ public class StanzaHandlerRefuseStreamTest { - /** - * A stream namespace that is not the one that RFC 6120 section 4.8.1 requires, used to have a stream header - * refused for a reason that does not depend on the content namespace that the header declares. - */ - private static final String WRONG_STREAM_NAMESPACE = "http://example.org/not-the-stream-namespace"; - private Connection connection; + private ClientStanzaHandler handler; + private XMPPPacketReader reader; @BeforeAll @@ -70,12 +54,6 @@ public static void setUpClass() throws Exception Fixtures.disableDatabasePersistence(); } - @AfterAll - public static void tearDownClass() - { - Fixtures.clearExistingProperties(); - } - @BeforeEach public void setUp() throws Exception { @@ -84,6 +62,7 @@ public void setUp() throws Exception XMPPServer.setInstance(Fixtures.mockXMPPServer()); connection = mock(Connection.class, withSettings().lenient()); + handler = new ClientStanzaHandler(mock(PacketRouter.class), connection); final XmlPullParserFactory factory = XmlPullParserFactory.newInstance(MXParser.class.getName(), null); factory.setNamespaceAware(true); @@ -92,60 +71,70 @@ public void setUp() throws Exception } /** - * Asserts that a client stream header that declares no content namespace, and that is refused, is answered with the - * stream error, opened in the namespace of the handler, and that the connection is then closed. + * Asserts that a stream header with a content namespace that does not match the handler is refused with an + * invalid-namespace stream error. */ @Test - public void testClientStreamWithoutNamespaceIsRefusedWithError() throws Exception + public void testStreamWithInvalidNamespaceIsRefused() throws Exception { - // Setup test fixture. - final ClientStanzaHandler handler = new ClientStanzaHandler(mock(PacketRouter.class), connection); - // Execute system under test. - handler.initiateSession("", reader); + handler.initiateSession("", reader); // Verify result. - final String delivered = deliveredText(connection); + final String delivered = deliveredText(); assertTrue(delivered.contains("", reader); + handler.initiateSession("", reader); // Verify result. - final String delivered = deliveredText(connection); - assertTrue(delivered.contains(" captor = ArgumentCaptor.forClass(String.class); - verify(connection, atLeastOnce().description("Text must have been delivered to the peer.")).deliverRawText(captor.capture()); + verify(connection, atLeastOnce()).deliverRawText(captor.capture()); return String.join("", captor.getAllValues()); } + + /** + * Counts the occurrences of a fragment in a text. + * + * @param text the text to search. + * @param fragment the fragment to count. + * @return the number of occurrences. + */ + private static int countOccurrences(final String text, final String fragment) + { + int count = 0; + for (int index = text.indexOf(fragment); index >= 0; index = text.indexOf(fragment, index + 1)) { + count++; + } + return count; + } } From a78fd2421b7e8c3de2fd2f5f8d1d32c80aa81151 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Mon, 26 Oct 2026 11:08:06 +0000 Subject: [PATCH 03/22] OF-2643: Optionally refuse S2S streams that address an unserviced domain ServerStanzaHandler never validated the 'to' of inbound stream headers, so a peer that authenticated with SASL EXTERNAL was accepted whatever domain it addressed. The new property xmpp.server.validate.host enables the host-unknown check that client connections already have. It is off by default so that upgrades do not change behaviour; OF-3421 tracks enabling it by default. A stream without 'to' stays acceptable, as legacy dialback peers omit it. --- .../main/resources/openfire_i18n.properties | 1 + .../openfire/net/ServerStanzaHandler.java | 21 +- ...ServerStanzaHandlerHostValidationTest.java | 240 ++++++++++++++++++ 3 files changed, 260 insertions(+), 2 deletions(-) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index 71a05aacb9..bef002035c 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -1346,6 +1346,7 @@ system_property.xmpp.gateway.enabled=Controls if Openfire's 'trunking' or 'gatew system_property.xmpp.gateway.domains=A collection of XMPP domain names for which Openfire will provide trunking functionality. For each domain in this collection, Openfire will accept data from other domains, and forward that data to that domain. system_property.xmpp.server.rewrite.replace-missing-to=If the server receives a message or IQ stanza with no 'to' attribute, set the 'to' attribute to the bare JID representation of the 'from' attribute value. system_property.xmpp.server.incoming.skip-jid-validation=Controls if JIDs that are in the addresses of stanzas supplied by remote domains are validated. +system_property.xmpp.server.validate.host=When enabled, an inbound server-to-server stream that addresses a domain this server does not service is refused with a host-unknown stream error. Serviced domains are the XMPP domain, its subdomains that have a component, and domains configured for trunking. A stream that does not address a domain is accepted. system_property.xmpp.server.outgoing.max.threads=Maximum amount of threads in the thread pool that is used to establish outbound server-to-server connections. Surplus connections will be created on the calling thread, possibly / gracefully slowing down other operations considerably. system_property.xmpp.server.outgoing.min.threads=Minimum amount of threads in the thread pool that is used to establish outbound server-to-server connections system_property.xmpp.server.outgoing.threads-timeout=Amount of time after which idle, surplus threads are removed from the thread pool that is used to establish outbound server-to-server connections. diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index 9cf877cd48..692433ad5b 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -72,6 +72,18 @@ public class ServerStanzaHandler extends StanzaHandler { .setDynamic(true) .build(); + /** + * Controls if the 'to' attribute of stream headers sent by remote servers is validated. When enabled, a stream that + * names a domain that is not serviced by this server is refused with a host-unknown stream error. + * + * @see #isLocallyServiced(String) + */ + public static final SystemProperty VALIDATE_HOST = SystemProperty.Builder.ofType(Boolean.class) + .setKey("xmpp.server.validate.host") + .setDefaultValue(false) + .setDynamic(true) + .build(); + /** * Checks if a domain that a remote server addresses is serviced by this server. That is the case for the XMPP * domain of this server, for subdomains of it that have a component route, and for domains for which trunking is @@ -132,8 +144,13 @@ Namespace getNamespace() { @Override boolean validateHost() { - // Hosts are not currently validated for S2S - return false; + return VALIDATE_HOST.getValue(); + } + + @Override + protected boolean isHostUnknown(String host) { + // A missing 'to' attribute is accepted, as servers that use legacy Server Dialback omit it. + return host != null && !isLocallyServiced(host); } @Override diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java new file mode 100644 index 0000000000..f1f7d487d8 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -0,0 +1,240 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.net; + +import org.dom4j.io.XMPPPacketReader; +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.PacketRouter; +import org.jivesoftware.openfire.RoutingTable; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.session.LocalIncomingServerSession; +import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.ArgumentCaptor; +import org.xmlpull.v1.XmlPullParser; +import org.xmlpull.v1.XmlPullParserFactory; +import org.xmpp.packet.JID; + +import java.util.List; +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.withSettings; + +/** + * Verifies the validation of the 'to' attribute of stream headers that are sent by remote servers (OF-2643). + * + * When validation is enabled, a stream that names a domain that is not serviced by this server is refused with a + * host-unknown stream error. Serviced domains are the XMPP domain of this server, subdomains of it that have a + * component route, and domains for which trunking is enabled. A stream header that has no 'to' attribute is accepted, + * as servers that use legacy Server Dialback omit it. + */ +public class ServerStanzaHandlerHostValidationTest +{ + private static final String COMPONENT_DOMAIN = "conference." + Fixtures.XMPP_DOMAIN; + + private static final String TRUNKED_DOMAIN = "trunked.example.net"; + + private static final String UNSERVICED_DOMAIN = "unserviced.example.com"; + + private Connection connection; + + private RoutingTable routingTable; + + private RecordingServerStanzaHandler handler; + + private XMPPPacketReader reader; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @BeforeEach + public void setUp() throws Exception + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + routingTable = Fixtures.mockRoutingTable(); + doReturn(true).when(routingTable).hasComponentRoute(new JID(COMPONENT_DOMAIN)); + doReturn(routingTable).when(xmppServer).getRoutingTable(); + XMPPServer.setInstance(xmppServer); + + connection = mock(Connection.class, withSettings().lenient()); + handler = new RecordingServerStanzaHandler(mock(PacketRouter.class), connection); + reader = newPacketReader(); + } + + /** + * Asserts that validation is disabled by default, so that existing installations keep accepting the connections + * that they accepted before validation was introduced. + */ + @Test + public void testValidationIsDisabledByDefault() + { + assertFalse(handler.validateHost()); + } + + /** + * Asserts that, with validation enabled, a stream that names a domain that is not serviced is refused with a + * host-unknown stream error before a session is created. + */ + @Test + public void testInitialStreamToUnservicedDomainIsRefused() throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + + // Execute system under test. + handler.initiateSession(streamHeader(UNSERVICED_DOMAIN), reader); + + // Verify result. + assertEquals(0, handler.sessionsCreated, "No session must be created for a stream that names a domain that is not serviced."); + assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); + verify(connection).close(); + } + + /** + * Asserts that, with validation disabled, a stream that names a domain that is not serviced is accepted, as it was + * before validation was introduced. + */ + @Test + public void testInitialStreamToUnservicedDomainIsAcceptedWhenValidationIsDisabled() throws Exception + { + // Execute system under test. + handler.initiateSession(streamHeader(UNSERVICED_DOMAIN), reader); + + // Verify result. + assertEquals(1, handler.sessionsCreated, "A session must be created when validation is disabled."); + } + + /** + * Asserts that, with validation enabled, streams that name a serviced domain are accepted. + */ + @ParameterizedTest + @ValueSource(strings = {Fixtures.XMPP_DOMAIN, COMPONENT_DOMAIN, TRUNKED_DOMAIN}) + public void testInitialStreamToServicedDomainIsAccepted(final String domain) throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + + // Execute system under test. + handler.initiateSession(streamHeader(domain), reader); + + // Verify result. + assertEquals(1, handler.sessionsCreated, "A session must be created for a stream that names '" + domain + "'."); + } + + /** + * Asserts that, with validation enabled, a stream that does not name a domain at all is accepted, as servers that + * use legacy Server Dialback omit the 'to' attribute. + */ + @Test + public void testInitialStreamWithoutToIsAccepted() throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + + // Execute system under test. + handler.initiateSession(streamHeader(null), reader); + + // Verify result. + assertEquals(1, handler.sessionsCreated, "A session must be created for a stream that has no 'to' attribute."); + } + + /** + * Returns an opening stream header as sent by a remote server. + * + * @param to the value of the 'to' attribute, or null to omit the attribute. + * @return a stream header. + */ + private static String streamHeader(final String to) + { + return ""; + } + + /** + * Returns all text that was delivered to the peer, concatenated. + * + * @return delivered text. + */ + private String deliveredText() + { + final ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(connection, atLeastOnce()).deliverRawText(captor.capture()); + return String.join("", captor.getAllValues()); + } + + /** + * Returns a namespace-aware packet reader, configured as the production code configures its own. + * + * @return a packet reader. + */ + private static XMPPPacketReader newPacketReader() throws Exception + { + final XmlPullParserFactory factory = XmlPullParserFactory.newInstance(MXParser.class.getName(), null); + factory.setNamespaceAware(true); + final XMPPPacketReader reader = new XMPPPacketReader(); + reader.setXPPFactory(factory); + return reader; + } + + /** + * A {@link ServerStanzaHandler} that replaces the creation of a session with a stub, and counts how often a session + * is created. A real session requires collaborators (an identity store, a session manager) that are not what these + * tests are about. + */ + private static class RecordingServerStanzaHandler extends ServerStanzaHandler + { + private int sessionsCreated; + + RecordingServerStanzaHandler(final PacketRouter router, final Connection connection) + { + super(router, connection, false); + } + + @Override + void createSession(final String serverName, final XmlPullParser xpp, final Connection connection) + { + sessionsCreated++; + final LocalIncomingServerSession stub = mock(LocalIncomingServerSession.class, withSettings().lenient()); + doReturn(new BasicStreamIDFactory().createStreamID()).when(stub).getStreamID(); + doReturn(Locale.ENGLISH).when(stub).getLanguage(); + doReturn(List.of()).when(stub).getAvailableStreamFeatures(); + session = stub; + } + } +} From b3a99b09227b010e86cf7bf88dddbd16901bdaa0 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Mon, 26 Oct 2026 11:08:14 +0000 Subject: [PATCH 04/22] OF-2643: Validate the S2S stream header that follows STARTTLS Only the first stream header reached the host check: after STARTTLS, ServerStanzaHandler.tlsNegotiated re-creates the session directly. RFC 6120 section 5.4.3.3 requires values from before TLS to be discarded, which makes the header sent after STARTTLS the one that counts, and the only one that an active attacker cannot have altered. When xmpp.server.validate.host is enabled, that header is now checked in the same way, before the session is re-created. --- .../openfire/net/ServerStanzaHandler.java | 10 +++ ...ServerStanzaHandlerHostValidationTest.java | 67 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index 692433ad5b..8b754adee9 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -221,6 +221,16 @@ protected void tlsNegotiated(XmlPullParser xpp) throws XmlPullParserException, I for (int eventType = xpp.getEventType(); eventType != XmlPullParser.START_TAG;) { eventType = xpp.next(); } + + // The stream header that was sent before TLS was negotiated has been validated already, but its values are to + // be discarded. Validate the 'to' attribute of the stream header that was sent after TLS was negotiated. + final String host = xpp.getAttributeValue("", "to"); + if (validateHost() && isHostUnknown(host)) { + Log.warn("Refusing stream that was opened after TLS was negotiated, as it addresses a host that is not serviced: '{}'. Closing connection: {}", host, connection); + refuseStream(xpp, host, new StreamError(StreamError.Condition.host_unknown, "Incorrect hostname in stream header: " + host)); + return; + } + this.domainPair = new DomainPair(XMPPServer.getInstance().getServerInfo().getXMPPDomain(), xpp.getAttributeValue("", "from")); // Discard session data obtained from unencrypted initiating stream diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index f1f7d487d8..792e45bead 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -53,6 +53,10 @@ * host-unknown stream error. Serviced domains are the XMPP domain of this server, subdomains of it that have a * component route, and domains for which trunking is enabled. A stream header that has no 'to' attribute is accepted, * as servers that use legacy Server Dialback omit it. + * + * Stream headers are validated both when the stream is first opened, and when it is restarted after TLS has been + * negotiated: RFC 6120 section 5.4.3.3 requires values obtained before TLS took effect to be discarded, which makes the + * header that follows TLS negotiation the one that counts. */ public class ServerStanzaHandlerHostValidationTest { @@ -173,6 +177,69 @@ public void testInitialStreamWithoutToIsAccepted() throws Exception assertEquals(1, handler.sessionsCreated, "A session must be created for a stream that has no 'to' attribute."); } + /** + * Asserts that, with validation enabled, a stream that names a serviced domain before TLS is negotiated, but a + * domain that is not serviced after, is refused with a host-unknown stream error. + */ + @Test + public void testStreamAfterTlsToUnservicedDomainIsRefused() throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + handler.initiateSession(streamHeader(Fixtures.XMPP_DOMAIN), reader); + handler.startedTLS = true; + + // Execute system under test. + handler.initiateSession(streamHeader(UNSERVICED_DOMAIN), reader); + + // Verify result. + assertEquals(1, handler.sessionsCreated, "The session must not be re-created for a stream that names a domain that is not serviced."); + assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); + verify(connection).close(); + } + + /** + * Asserts that, with validation enabled, a stream that names a serviced domain after TLS is negotiated is + * accepted. Peers that use trunking typically negotiate TLS before authenticating with Server Dialback, which + * makes this the path by which trunked domains are normally validated. + */ + @ParameterizedTest + @ValueSource(strings = {Fixtures.XMPP_DOMAIN, COMPONENT_DOMAIN, TRUNKED_DOMAIN}) + public void testStreamAfterTlsToServicedDomainIsAccepted(final String domain) throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + handler.initiateSession(streamHeader(domain), reader); + handler.startedTLS = true; + + // Execute system under test. + handler.initiateSession(streamHeader(domain), reader); + + // Verify result. + assertEquals(2, handler.sessionsCreated, "The session must be re-created for the stream that follows TLS negotiation, when it names '" + domain + "'."); + assertFalse(deliveredText().contains("host-unknown"), "The stream must not be refused."); + } + + /** + * Asserts that, with validation disabled, a stream that names a domain that is not serviced after TLS is + * negotiated is accepted, as it was before validation was introduced. + */ + @Test + public void testStreamAfterTlsToUnservicedDomainIsAcceptedWhenValidationIsDisabled() throws Exception + { + // Setup test fixture. + handler.initiateSession(streamHeader(Fixtures.XMPP_DOMAIN), reader); + handler.startedTLS = true; + + // Execute system under test. + handler.initiateSession(streamHeader(UNSERVICED_DOMAIN), reader); + + // Verify result. + assertEquals(2, handler.sessionsCreated, "The session must be re-created when validation is disabled."); + } + /** * Returns an opening stream header as sent by a remote server. * From 268c4c5bcd715edf19c1032e65da634136e8a3c4 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Mon, 26 Oct 2026 16:03:19 +0000 Subject: [PATCH 05/22] OF-2643: Refuse a stream for an unknown host from our own domain When a stream header is refused, the stream that carries the error was opened from the host that the peer addressed. For a host-unknown error that names a domain this server has just said it does not service, and when the header had no to attribute the from attribute was omitted. RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity, or the empty string, in both cases. The refusal is now opened from the XMPP domain in those cases. Other refusals still answer from the addressed host, which is serviced. This applies to client streams refused under xmpp.client.validate.host too. --- .../openfire/net/StanzaHandler.java | 9 ++++- ...ServerStanzaHandlerHostValidationTest.java | 2 + .../net/StanzaHandlerRefuseStreamTest.java | 37 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index 0cd490728a..e4ab8da274 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -941,11 +941,18 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I * Responds to a stream header that cannot be accepted by opening a stream that carries the provided stream error, * and then closes the underlying connection. * + * The stream is opened from the host that the peer addressed, unless that host is missing or unknown: then it is + * opened from the XMPP domain of this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the + * receiving entity. + * * @param xpp a parser positioned on the stream header that is refused. * @param host the value of the 'to' attribute of the refused stream header (can be null). * @param error the stream error to send to the peer. */ protected void refuseStream(XmlPullParser xpp, String host, StreamError error) { + final String from = host == null || error.getCondition() == StreamError.Condition.host_unknown + ? XMPPServer.getInstance().getServerInfo().getXMPPDomain() + : host; final Element stream = DocumentHelper.createElement(QName.get("stream", "stream", "http://etherx.jabber.org/streams")); final Document document = DocumentHelper.createDocument(stream); document.setXMLEncoding(StandardCharsets.UTF_8.toString()); @@ -953,7 +960,7 @@ protected void refuseStream(XmlPullParser xpp, String host, StreamError error) { // namespace of this handler, rather than copying the absent namespace (which would fail to build the error). final String contentNamespace = xpp.getNamespace(null); stream.add(Namespace.get(contentNamespace != null ? contentNamespace : getNamespace().getURI())); - stream.addAttribute("from", host); + stream.addAttribute("from", from); stream.addAttribute("id", STREAM_ID_FACTORY.createStreamID().getID()); stream.addAttribute("version", "1.0"); stream.add(error.getElement()); diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index 792e45bead..c115d4480c 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -124,6 +124,7 @@ public void testInitialStreamToUnservicedDomainIsRefused() throws Exception // Verify result. assertEquals(0, handler.sessionsCreated, "No session must be created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); + assertTrue(deliveredText().contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of this server."); verify(connection).close(); } @@ -195,6 +196,7 @@ public void testStreamAfterTlsToUnservicedDomainIsRefused() throws Exception // Verify result. assertEquals(1, handler.sessionsCreated, "The session must not be re-created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); + assertFalse(deliveredText().contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); verify(connection).close(); } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 0a7fb76ef7..db1268da5d 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -28,6 +28,7 @@ import org.xmlpull.v1.XmlPullParserFactory; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.atLeastOnce; @@ -110,6 +111,42 @@ public void testClientStreamToUnknownHostIsRefused() throws Exception assertNull(handler.session, "No session must be created for a refused stream."); } + /** + * Asserts that a stream that is refused because it addresses an unknown host is opened from the XMPP domain of + * this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity, rather than + * from the unknown host. + */ + @Test + public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception + { + // Setup test fixture. + JiveGlobals.setProperty("xmpp.client.validate.host", "true"); + + // Execute system under test. + handler.initiateSession("", reader); + + // Verify result. + final String delivered = deliveredText(); + assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The stream must be opened from the XMPP domain of this server."); + assertFalse(delivered.contains("from=\"unserviced.example.com\""), "The stream must not be opened from the unknown host."); + } + + /** + * Asserts that a stream header that does not address a host, and that is refused, is answered with a stream that + * is opened from the XMPP domain of this server, as RFC 6120 section 4.9.1.3 requires. + */ + @Test + public void testStreamWithoutToIsRefusedFromOwnDomain() throws Exception + { + // Execute system under test. + handler.initiateSession("", reader); + + // Verify result. + final String delivered = deliveredText(); + assertTrue(delivered.contains(" Date: Tue, 27 Oct 2026 09:25:34 +0000 Subject: [PATCH 06/22] OF-2643: Refuse from our own domain whenever the host is not ours A refused stream was opened from our own domain only for a host-unknown error. A stream that names a domain this server does not service, and that is refused for another reason (such as an invalid namespace while host validation is disabled), was still opened from that domain. The refusal is now opened from the XMPP domain whenever the addressed host is missing or is unknown according to isHostUnknown, whatever the error, as RFC 6120 section 4.9.1.3 requires. A serviced host that is addressed is still used. Component streams, whose handler accepts only the XMPP domain as known, are refused from the XMPP domain too. --- .../openfire/net/StanzaHandler.java | 8 ++-- ...ServerStanzaHandlerHostValidationTest.java | 47 ++++++++++++++++++- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index e4ab8da274..3018011f79 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -941,16 +941,16 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I * Responds to a stream header that cannot be accepted by opening a stream that carries the provided stream error, * and then closes the underlying connection. * - * The stream is opened from the host that the peer addressed, unless that host is missing or unknown: then it is - * opened from the XMPP domain of this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the - * receiving entity. + * The stream is opened from the host that the peer addressed, unless that host is missing or is not serviced (as + * determined by {@link #isHostUnknown(String)}), whatever the reason for the refusal: then it is opened from the XMPP + * domain of this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity. * * @param xpp a parser positioned on the stream header that is refused. * @param host the value of the 'to' attribute of the refused stream header (can be null). * @param error the stream error to send to the peer. */ protected void refuseStream(XmlPullParser xpp, String host, StreamError error) { - final String from = host == null || error.getCondition() == StreamError.Condition.host_unknown + final String from = host == null || isHostUnknown(host) ? XMPPServer.getInstance().getServerInfo().getXMPPDomain() : host; final Element stream = DocumentHelper.createElement(QName.get("stream", "stream", "http://etherx.jabber.org/streams")); diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index c115d4480c..1175628003 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -178,6 +178,39 @@ public void testInitialStreamWithoutToIsAccepted() throws Exception assertEquals(1, handler.sessionsCreated, "A session must be created for a stream that has no 'to' attribute."); } + /** + * Asserts that a stream that is refused for a reason other than its host, while validation is disabled, is + * answered from the XMPP domain when it names a domain that is not serviced, rather than from that domain. + */ + @Test + public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDomain() throws Exception + { + // Execute system under test. + handler.initiateSession(streamHeaderWithNamespace(UNSERVICED_DOMAIN, "jabber:client"), reader); + + // Verify result. + final String delivered = deliveredText(); + assertTrue(delivered.contains("invalid-namespace"), "The stream must be refused for its namespace."); + assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of this server."); + assertFalse(delivered.contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); + } + + /** + * Asserts that a stream that is refused for a reason other than its host is answered from the domain that it names, + * when that domain is serviced. + */ + @Test + public void testStreamToServicedDomainRefusedForOtherReasonIsRefusedFromThatDomain() throws Exception + { + // Execute system under test. + handler.initiateSession(streamHeaderWithNamespace(COMPONENT_DOMAIN, "jabber:client"), reader); + + // Verify result. + final String delivered = deliveredText(); + assertTrue(delivered.contains("invalid-namespace"), "The stream must be refused for its namespace."); + assertTrue(delivered.contains("from=\"" + COMPONENT_DOMAIN + "\""), "The refusal must be sent from the serviced domain that the peer addressed."); + } + /** * Asserts that, with validation enabled, a stream that names a serviced domain before TLS is negotiated, but a * domain that is not serviced after, is refused with a host-unknown stream error. @@ -250,7 +283,19 @@ public void testStreamAfterTlsToUnservicedDomainIsAcceptedWhenValidationIsDisabl */ private static String streamHeader(final String to) { - return ""; } From fbe6fef851ab0a130a821f45fc50f0b435f0497a Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Tue, 27 Oct 2026 09:37:15 +0000 Subject: [PATCH 07/22] OF-2643: Ignore an invalid trunked domain instead of failing Trunking.isTrunkingEnabledFor normalised every value of xmpp.gateway.domains with JID.domainprep, which throws for a value that is not a valid domain name. With trunking enabled and one such value in the list, any check that reached it threw. Server Dialback already used this method; with xmpp.server.validate.host enabled, the validation of each inbound stream header now does too, so one bad value would break incoming server connections instead of refusing them with host-unknown. An invalid configured value is now logged and ignored, and the other configured domains are still recognised. --- .../jivesoftware/openfire/net/Trunking.java | 29 ++++++++++++++- ...erverStanzaHandlerLocallyServicedTest.java | 37 ++++++++++++++++++- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java index a3615fc7ca..50e151f7da 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2022 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2022-2026 Ignite Realtime Foundation. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,8 @@ package org.jivesoftware.openfire.net; import org.jivesoftware.util.SystemProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.xmpp.packet.JID; import javax.annotation.Nonnull; @@ -32,6 +34,8 @@ */ public class Trunking { + private static final Logger Log = LoggerFactory.getLogger(Trunking.class); + /** * Enables or disables the trunking functionality that allows Openfire to act as a gateway to transfer data between * other XMPP domains. @@ -61,7 +65,28 @@ public class Trunking */ public static boolean isTrunkingEnabledFor(@Nonnull final String domain) { - return ENABLED.getValue() && ALLOWABLE_DOMAINS.getValue().stream().anyMatch(v -> JID.domainprep(v).equals(JID.domainprep(domain))); + if (!ENABLED.getValue()) { + return false; + } + final String requested = JID.domainprep(domain); + return ALLOWABLE_DOMAINS.getValue().stream().anyMatch(v -> requested.equals(domainprepOrNull(v))); + } + + /** + * Normalises a domain that is configured for trunking. A configured value that is not a valid domain name is + * ignored, so that it does not prevent the other configured domains from being recognised. + * + * @param configured a value of the trunking domains property. + * @return the normalised domain, or null when the value is not a valid domain name. + */ + private static String domainprepOrNull(@Nonnull final String configured) + { + try { + return JID.domainprep(configured); + } catch (final IllegalArgumentException e) { + Log.warn("Ignoring a value of {} that is not a valid domain name: {}", ALLOWABLE_DOMAINS.getKey(), configured, e); + return null; + } } } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index ae595b31e9..612bcebf6b 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -43,6 +43,11 @@ public class ServerStanzaHandlerLocallyServicedTest private static final String UNSERVICED_DOMAIN = "unserviced.example.com"; + /** + * A value that cannot be a domain name, as its first label exceeds 63 characters. + */ + private static final String INVALID_DOMAIN = "a".repeat(64) + ".example.org"; + private RoutingTable routingTable; @BeforeAll @@ -161,6 +166,36 @@ public void testSubdomainOfTrunkedDomainIsNotServiced() assertFalse(ServerStanzaHandler.isLocallyServiced("conference." + TRUNKED_DOMAIN)); } + /** + * Asserts that a domain for which trunking is enabled is serviced when the configuration also lists a value that + * is not a valid domain name. + */ + @Test + public void testTrunkedDomainIsServicedDespiteInvalidConfiguredDomain() + { + // Setup test fixture. + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(INVALID_DOMAIN, TRUNKED_DOMAIN)); + + // Execute system under test & verify result. + assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + } + + /** + * Asserts that a domain that is not serviced is reported as such, rather than causing an exception, when the + * configuration for trunking lists a value that is not a valid domain name. + */ + @Test + public void testUnrelatedDomainIsNotServicedDespiteInvalidConfiguredDomain() + { + // Setup test fixture. + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(INVALID_DOMAIN)); + + // Execute system under test & verify result. + assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN)); + } + /** * Asserts that a domain that is unrelated to this server is not serviced. */ @@ -177,6 +212,6 @@ public void testUnrelatedDomainIsNotServiced() @Test public void testInvalidDomainIsNotServiced() { - assertFalse(ServerStanzaHandler.isLocallyServiced("a".repeat(64) + ".example.org")); + assertFalse(ServerStanzaHandler.isLocallyServiced(INVALID_DOMAIN)); } } From db92e3a1c936e8b298b3d7619102ec292f12124b Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Tue, 27 Oct 2026 09:50:04 +0000 Subject: [PATCH 08/22] OF-2643: Clear test properties after the new test classes The new test classes set properties such as xmpp.server.validate.host and the trunking configuration, and clear them before each test, but left the values of their last test in place for whichever test class runs next in the same JVM. They now clear them after the class too, as SaslOutcomeTest and StanzaHandlerTest do. Also rewords the description of one test that read awkwardly. --- .../net/ServerStanzaHandlerHostValidationTest.java | 11 +++++++++-- .../net/ServerStanzaHandlerLocallyServicedTest.java | 7 +++++++ .../openfire/net/StanzaHandlerRefuseStreamTest.java | 7 +++++++ .../server/ServerDialbackHostUnknownTest.java | 7 +++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index 1175628003..a69fd2a023 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -24,6 +24,7 @@ import org.jivesoftware.openfire.session.LocalIncomingServerSession; import org.jivesoftware.openfire.spi.BasicStreamIDFactory; import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -81,6 +82,12 @@ public static void setUpClass() throws Exception Fixtures.disableDatabasePersistence(); } + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + @BeforeEach public void setUp() throws Exception { @@ -179,8 +186,8 @@ public void testInitialStreamWithoutToIsAccepted() throws Exception } /** - * Asserts that a stream that is refused for a reason other than its host, while validation is disabled, is - * answered from the XMPP domain when it names a domain that is not serviced, rather than from that domain. + * Asserts that, with validation disabled, a stream that names a domain that is not serviced, and that is refused + * for another reason, is answered from the XMPP domain. */ @Test public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDomain() throws Exception diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index 612bcebf6b..09c9250866 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -19,6 +19,7 @@ import org.jivesoftware.openfire.RoutingTable; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -57,6 +58,12 @@ public static void setUpClass() throws Exception Fixtures.disableDatabasePersistence(); } + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + @BeforeEach public void setUp() { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index db1268da5d..f29fdc06e7 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -21,6 +21,7 @@ import org.jivesoftware.openfire.PacketRouter; import org.jivesoftware.openfire.XMPPServer; import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -55,6 +56,12 @@ public static void setUpClass() throws Exception Fixtures.disableDatabasePersistence(); } + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + @BeforeEach public void setUp() throws Exception { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java index ef9a6e8f49..2b5cb6b459 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java @@ -22,6 +22,7 @@ import org.jivesoftware.openfire.net.Trunking; import org.jivesoftware.openfire.session.DomainPair; import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -57,6 +58,12 @@ public static void setUpClass() throws Exception Fixtures.disableDatabasePersistence(); } + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + @BeforeEach public void setUp() { From 69692392d6b4a322ae536336d29169941178c6ca Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Tue, 27 Oct 2026 10:49:54 +0000 Subject: [PATCH 09/22] OF-2643: Normalise the trunked domains once per configuration change Every check of a trunked domain normalised each configured value of xmpp.gateway.domains again. With an invalid value in the list, that logged a warning, with a stack trace, for every inbound stream header and dialback request that reached it (reported by CodeRabbit on #3525). The normalised domains are now kept until the configuration changes, so an invalid value is reported once per change, without a stack trace, and each check is a set lookup. A change to the configuration still takes effect on the next check, which a new test verifies. --- .../jivesoftware/openfire/net/Trunking.java | 42 +++++++++++++------ ...erverStanzaHandlerLocallyServicedTest.java | 21 ++++++++++ 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java index 50e151f7da..276c48f4cf 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/Trunking.java @@ -23,7 +23,9 @@ import javax.annotation.Nonnull; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * Configuration of Openfire's 'gateway' or 'trunking' functionality, which allows Openfire to act as a gateway to @@ -68,25 +70,41 @@ public static boolean isTrunkingEnabledFor(@Nonnull final String domain) if (!ENABLED.getValue()) { return false; } - final String requested = JID.domainprep(domain); - return ALLOWABLE_DOMAINS.getValue().stream().anyMatch(v -> requested.equals(domainprepOrNull(v))); + return normalisedDomains().contains(JID.domainprep(domain)); } /** - * Normalises a domain that is configured for trunking. A configured value that is not a valid domain name is - * ignored, so that it does not prevent the other configured domains from being recognised. + * The configured trunking domains, and their normalised form. Recomputed only when the configuration changes. + */ + private record NormalisedDomains(List configured, Set normalised) {} + + private static volatile NormalisedDomains normalisedDomains = new NormalisedDomains(List.of(), Set.of()); + + /** + * Returns the normalised form of the domains that are configured for trunking. A configured value that is not a + * valid domain name is ignored, so that it does not prevent the other configured domains from being recognised. + * The configuration is normalised again only when it changes, so that an invalid value is reported once per change + * rather than on every check. * - * @param configured a value of the trunking domains property. - * @return the normalised domain, or null when the value is not a valid domain name. + * @return the normalised domains. */ - private static String domainprepOrNull(@Nonnull final String configured) + private static Set normalisedDomains() { - try { - return JID.domainprep(configured); - } catch (final IllegalArgumentException e) { - Log.warn("Ignoring a value of {} that is not a valid domain name: {}", ALLOWABLE_DOMAINS.getKey(), configured, e); - return null; + final List configured = ALLOWABLE_DOMAINS.getValue(); + NormalisedDomains current = normalisedDomains; + if (!current.configured().equals(configured)) { + final Set normalised = new HashSet<>(); + for (final String value : configured) { + try { + normalised.add(JID.domainprep(value)); + } catch (final IllegalArgumentException e) { + Log.warn("Ignoring a value of {} that is not a valid domain name: '{}'", ALLOWABLE_DOMAINS.getKey(), value); + } + } + current = new NormalisedDomains(List.copyOf(configured), Set.copyOf(normalised)); + normalisedDomains = current; } + return current.normalised(); } } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index 09c9250866..ca18ee96b1 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -173,6 +173,27 @@ public void testSubdomainOfTrunkedDomainIsNotServiced() assertFalse(ServerStanzaHandler.isLocallyServiced("conference." + TRUNKED_DOMAIN)); } + /** + * Asserts that a change to the domains that are configured for trunking takes effect for the checks that follow + * it: a domain that is added becomes serviced, and a domain that is removed stops being serviced. + */ + @Test + public void testChangeOfTrunkedDomainsTakesEffect() + { + // Setup test fixture. + final String otherTrunkedDomain = "other-trunked.example.net"; + Trunking.ENABLED.setValue(true); + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); + assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN), "The initially configured domain must be serviced."); + + // Execute system under test. + Trunking.ALLOWABLE_DOMAINS.setValue(List.of(otherTrunkedDomain)); + + // Verify result. + assertTrue(ServerStanzaHandler.isLocallyServiced(otherTrunkedDomain), "A domain that is added must be serviced."); + assertFalse(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN), "A domain that is removed must no longer be serviced."); + } + /** * Asserts that a domain for which trunking is enabled is serviced when the configuration also lists a value that * is not a valid domain name. From 5b1618241496c1c3edea538cab0b33ad8ca6b139 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Tue, 27 Oct 2026 10:57:33 +0000 Subject: [PATCH 10/22] OF-2643: Continue dialback with the normalised form of the domain The shared rule normalises a domain before deciding whether it is serviced, so Server Dialback now accepts a db:result whose 'to' names the XMPP domain in another case, which its old, case-sensitive check refused. The value was then used as received: the check for an existing session for the same domain compared it case-sensitively, so a second session was not recognised as a duplicate, and the result was reported from the domain as the peer wrote it (reported by CodeRabbit on #3526). Once the domain is accepted, validateRemoteDomain continues with its normalised form and records it in the request, which LocalIncomingServerSession.validateSubsequentDomain reads to report the result. The permission and certificate checks that precede and follow it are stubbed in the new tests, as they read the database and the peer's certificates. --- .../openfire/server/ServerDialback.java | 7 +- ...erverDialbackValidateRemoteDomainTest.java | 160 ++++++++++++++++++ 2 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java index 51069452b1..5af99100df 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java @@ -435,7 +435,7 @@ public LocalIncomingServerSession createIncomingSession(XMPPPacketReader reader) session.setAuthenticationMethod(ServerSession.AuthenticationMethod.DIALBACK); // Set the domain or subdomain of the local server used when // validating the session - session.setLocalDomain(recipient); + session.setLocalDomain(doc.attributeValue("to")); // After the session has been created, inform all listeners as well. ServerSessionEventDispatcher.dispatchEvent(session, ServerSessionEventDispatcher.EventType.session_created); return session; @@ -538,6 +538,11 @@ else if (isHostUnknown(recipient)) { throw new ServerDialbackErrorException(recipient, remoteDomain, new PacketError(PacketError.Condition.item_not_found, PacketError.Type.cancel, "Service not hosted here")); } else { + // The domain is serviced. Continue with its normalised form, and record that in the request, so that the + // session is created for, and the result is reported from, the domain as this server knows it. + recipient = JID.domainprep(recipient); + doc.addAttribute("to", recipient); + log.debug( "Check if the remote domain already has a connection to the target domain/subdomain" ); boolean alreadyExists = false; for (IncomingServerSession session : sessionManager.getIncomingServerSessions(remoteDomain)) { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java new file mode 100644 index 0000000000..2a8cbad16f --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.server; + +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.SessionManager; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.net.SASLAuthentication; +import org.jivesoftware.openfire.session.DomainPair; +import org.jivesoftware.openfire.session.IncomingServerSession; +import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.openfire.spi.ConnectionConfiguration; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import java.security.cert.Certificate; +import java.util.List; +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.withSettings; + +/** + * Verifies how {@link ServerDialback#validateRemoteDomain(Element, org.jivesoftware.openfire.StreamID)} treats the + * domain that a remote server asks to be authenticated for, once that domain is accepted as serviced (OF-2643). + * + * The remote-server permission check and the certificate check are replaced with stubs, as they read the database and + * the peer's certificates, which are not what these tests are about. + * + * @author Matthew Vivian + */ +public class ServerDialbackValidateRemoteDomainTest +{ + private static final String REMOTE_DOMAIN = "remote.example.org"; + + private SessionManager sessionManager; + + private ServerDialback dialback; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + + @BeforeEach + public void setUp() + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + doReturn(Fixtures.mockRoutingTable()).when(xmppServer).getRoutingTable(); + sessionManager = xmppServer.getSessionManager(); + XMPPServer.setInstance(xmppServer); + + final ConnectionConfiguration configuration = mock(ConnectionConfiguration.class, withSettings().lenient()); + doReturn(Connection.TLSPolicy.optional).when(configuration).getTlsPolicy(); + final Connection connection = mock(Connection.class, withSettings().lenient()); + doReturn(configuration).when(connection).getConfiguration(); + doReturn(new Certificate[0]).when(connection).getPeerCertificates(); + + dialback = new ServerDialback(connection, new DomainPair(Fixtures.XMPP_DOMAIN, REMOTE_DOMAIN)); + } + + /** + * Asserts that a request to authenticate for the XMPP domain written in a different case is recognised as a + * duplicate of an existing session for the XMPP domain, as domain names are compared after normalisation. + */ + @Test + public void testExistingSessionIsDetectedForDomainInDifferentCase() + { + // Setup test fixture. + final IncomingServerSession existing = mock(IncomingServerSession.class, withSettings().lenient()); + doReturn(Fixtures.XMPP_DOMAIN).when(existing).getLocalDomain(); + doReturn(List.of(existing)).when(sessionManager).getIncomingServerSessions(REMOTE_DOMAIN); + doReturn(false).when(sessionManager).isMultipleServerConnectionsAllowed(); + + final Element dbResult = DocumentHelper.createElement("result"); + dbResult.addAttribute("from", REMOTE_DOMAIN); + dbResult.addAttribute("to", Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)); + dbResult.setText("key"); + + try (final MockedStatic remoteServers = mockStatic(RemoteServerManager.class); + final MockedStatic sasl = mockStatic(SASLAuthentication.class)) + { + remoteServers.when(() -> RemoteServerManager.canAccess(anyString())).thenReturn(true); + sasl.when(() -> SASLAuthentication.verifyCertificates(any(), anyString(), anyBoolean())).thenReturn(true); + + // Execute system under test & verify result. + final ServerDialbackErrorException exception = assertThrows(ServerDialbackErrorException.class, + () -> dialback.validateRemoteDomain(dbResult, new BasicStreamIDFactory().createStreamID()), + "A second session for the same domain, written in a different case, must be refused."); + assertEquals("resource-constraint", exception.getError().getCondition().toXMPP()); + } + } + + /** + * Asserts that, once accepted, the domain that a remote server asks to be authenticated for is normalised in the + * request, so that the session is created for, and the result is reported from, the normalised domain. + */ + @Test + public void testAcceptedDomainIsNormalised() throws Exception + { + // Setup test fixture. + doReturn(List.of()).when(sessionManager).getIncomingServerSessions(REMOTE_DOMAIN); + + final Element dbResult = DocumentHelper.createElement("result"); + dbResult.addAttribute("from", REMOTE_DOMAIN); + dbResult.addAttribute("to", Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)); + dbResult.setText("key"); + + try (final MockedStatic remoteServers = mockStatic(RemoteServerManager.class); + final MockedStatic sasl = mockStatic(SASLAuthentication.class)) + { + remoteServers.when(() -> RemoteServerManager.canAccess(anyString())).thenReturn(true); + sasl.when(() -> SASLAuthentication.verifyCertificates(any(), anyString(), anyBoolean())).thenReturn(true); + + // Execute system under test. + dialback.validateRemoteDomain(dbResult, new BasicStreamIDFactory().createStreamID()); + } + + // Verify result. + assertEquals(Fixtures.XMPP_DOMAIN, dbResult.attributeValue("to"), "The accepted domain must be normalised."); + } +} From 434c99145ca4823486f9e476382d97d3d0361384 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Tue, 27 Oct 2026 10:57:48 +0000 Subject: [PATCH 11/22] OF-2643: Describe host validation in the trunking guide With xmpp.server.validate.host enabled, a trunk refuses inbound connections for domains it does not service. An administrator who enables it on a trunk needs to know that the trunked domains are still accepted, and that their subdomains are not. --- documentation/trunking-guide.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/documentation/trunking-guide.html b/documentation/trunking-guide.html index 36da8b0ccc..aed39002e6 100644 --- a/documentation/trunking-guide.html +++ b/documentation/trunking-guide.html @@ -128,6 +128,10 @@

Setup

To enable the trunking functionality in Openfire, the property xmpp.gateway.enabled needs to be set to true, and the property xmpp.gateway.domains needs to be populated with a (comma-separated) list of XMPP domain names of remote XMPP domains that the Openfire server will forward data to.

+

+ When the property xmpp.server.validate.host is enabled, Openfire refuses inbound server-to-server connections that address a domain it does not service. The domains listed in xmpp.gateway.domains count as serviced, so connections to them are still accepted while trunking is enabled. Subdomains of those domains do not count: list each domain that is to be trunked. +

+

Full example

From 573c7263cfa0b97731f604ed0e4e77dbc68dbc34 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:48:04 +0100 Subject: [PATCH 12/22] OF-2643: Refer to the local XMPP service rather than to this server The documentation and messages that were added for host validation referred to the domains that are serviced by "this server". In a cluster, several Openfire servers together provide one XMPP service, which makes that phrase ambiguous. They now refer to the local XMPP service instead. The description of xmpp.server.validate.host explains that term once. No functional change. --- i18n/src/main/resources/openfire_i18n.properties | 2 +- .../openfire/net/ServerStanzaHandler.java | 11 ++++++----- .../org/jivesoftware/openfire/net/StanzaHandler.java | 3 ++- .../jivesoftware/openfire/server/ServerDialback.java | 10 +++++----- .../net/ServerStanzaHandlerHostValidationTest.java | 8 ++++---- .../net/ServerStanzaHandlerLocallyServicedTest.java | 11 ++++++----- .../openfire/net/StanzaHandlerRefuseStreamTest.java | 10 +++++----- .../server/ServerDialbackHostUnknownTest.java | 6 +++--- 8 files changed, 32 insertions(+), 29 deletions(-) diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index bef002035c..f1a7abff3d 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -1346,7 +1346,7 @@ system_property.xmpp.gateway.enabled=Controls if Openfire's 'trunking' or 'gatew system_property.xmpp.gateway.domains=A collection of XMPP domain names for which Openfire will provide trunking functionality. For each domain in this collection, Openfire will accept data from other domains, and forward that data to that domain. system_property.xmpp.server.rewrite.replace-missing-to=If the server receives a message or IQ stanza with no 'to' attribute, set the 'to' attribute to the bare JID representation of the 'from' attribute value. system_property.xmpp.server.incoming.skip-jid-validation=Controls if JIDs that are in the addresses of stanzas supplied by remote domains are validated. -system_property.xmpp.server.validate.host=When enabled, an inbound server-to-server stream that addresses a domain this server does not service is refused with a host-unknown stream error. Serviced domains are the XMPP domain, its subdomains that have a component, and domains configured for trunking. A stream that does not address a domain is accepted. +system_property.xmpp.server.validate.host=When enabled, an inbound server-to-server stream that addresses a domain that is not serviced by the local XMPP service is refused with a host-unknown stream error. Serviced domains are the XMPP domain, its subdomains that have a component, and domains configured for trunking. A stream that does not address a domain is accepted. system_property.xmpp.server.outgoing.max.threads=Maximum amount of threads in the thread pool that is used to establish outbound server-to-server connections. Surplus connections will be created on the calling thread, possibly / gracefully slowing down other operations considerably. system_property.xmpp.server.outgoing.min.threads=Minimum amount of threads in the thread pool that is used to establish outbound server-to-server connections system_property.xmpp.server.outgoing.threads-timeout=Amount of time after which idle, surplus threads are removed from the thread pool that is used to establish outbound server-to-server connections. diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index 8b754adee9..3932113337 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -74,7 +74,8 @@ public class ServerStanzaHandler extends StanzaHandler { /** * Controls if the 'to' attribute of stream headers sent by remote servers is validated. When enabled, a stream that - * names a domain that is not serviced by this server is refused with a host-unknown stream error. + * names a domain that is not serviced by the local XMPP service (the service that this Openfire instance provides, + * alone or together with the other nodes of its cluster) is refused with a host-unknown stream error. * * @see #isLocallyServiced(String) */ @@ -85,12 +86,12 @@ public class ServerStanzaHandler extends StanzaHandler { .build(); /** - * Checks if a domain that a remote server addresses is serviced by this server. That is the case for the XMPP - * domain of this server, for subdomains of it that have a component route, and for domains for which trunking is - * enabled. + * Checks if a domain that a remote server addresses is serviced by the local XMPP service. That is the case for the + * XMPP domain of the service, for subdomains of it that have a component route, and for domains for which trunking + * is enabled. * * @param domain the domain to check. - * @return true if the domain is serviced by this server, otherwise false (including when the value is not a valid domain name). + * @return true if the domain is serviced by the local XMPP service, otherwise false (including when the value is not a valid domain name). */ public static boolean isLocallyServiced(@Nonnull final String domain) { final String requested; diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index 3018011f79..a7a6e37e40 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -943,7 +943,8 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I * * The stream is opened from the host that the peer addressed, unless that host is missing or is not serviced (as * determined by {@link #isHostUnknown(String)}), whatever the reason for the refusal: then it is opened from the XMPP - * domain of this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity. + * domain of the local XMPP service, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving + * entity. * * @param xpp a parser positioned on the stream header that is refused. * @param host the value of the 'to' attribute of the refused stream header (can be null). diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java index 5af99100df..bfcce85c84 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java @@ -539,7 +539,7 @@ else if (isHostUnknown(recipient)) { } else { // The domain is serviced. Continue with its normalised form, and record that in the request, so that the - // session is created for, and the result is reported from, the domain as this server knows it. + // session is created for, and the result is reported from, the domain as the local XMPP service knows it. recipient = JID.domainprep(recipient); doc.addAttribute("to", recipient); @@ -648,14 +648,14 @@ else if (isHostUnknown(recipient)) { } /** - * Checks if the domain that the remote server addresses is not serviced by this server. Subdomains of this server - * are serviced when they have a component route, which is useful when they are registered in the DNS so that remote - * servers can establish connections directly to them. + * Checks if the domain that the remote server addresses is not serviced by the local XMPP service. Subdomains of its + * XMPP domain are serviced when they have a component route, which is useful when they are registered in the DNS so + * that remote servers can establish connections directly to them. * * XEP-0220 requires the 'to' attribute on dialback requests, so a request that has none is for an unknown host. * * @param recipient the domain addressed by the remote server (can be null). - * @return true if the domain is not serviced by this server. + * @return true if the domain is not serviced by the local XMPP service. * @see ServerStanzaHandler#isLocallyServiced(String) */ boolean isHostUnknown(String recipient) { diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index a69fd2a023..e08caa4e2a 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -50,8 +50,8 @@ /** * Verifies the validation of the 'to' attribute of stream headers that are sent by remote servers (OF-2643). * - * When validation is enabled, a stream that names a domain that is not serviced by this server is refused with a - * host-unknown stream error. Serviced domains are the XMPP domain of this server, subdomains of it that have a + * When validation is enabled, a stream that names a domain that is not serviced by the local XMPP service is refused + * with a host-unknown stream error. Serviced domains are the XMPP domain of the service, subdomains of it that have a * component route, and domains for which trunking is enabled. A stream header that has no 'to' attribute is accepted, * as servers that use legacy Server Dialback omit it. * @@ -131,7 +131,7 @@ public void testInitialStreamToUnservicedDomainIsRefused() throws Exception // Verify result. assertEquals(0, handler.sessionsCreated, "No session must be created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); - assertTrue(deliveredText().contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of this server."); + assertTrue(deliveredText().contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of the local XMPP service."); verify(connection).close(); } @@ -198,7 +198,7 @@ public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDom // Verify result. final String delivered = deliveredText(); assertTrue(delivered.contains("invalid-namespace"), "The stream must be refused for its namespace."); - assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of this server."); + assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of the local XMPP service."); assertFalse(delivered.contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index ca18ee96b1..ddf6cb53be 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -34,7 +34,8 @@ /** * Verifies {@link ServerStanzaHandler#isLocallyServiced(String)}, which decides whether a domain that a remote server - * addresses is serviced by this server (OF-2643). Both the validation of stream headers and Server Dialback use it. + * addresses is serviced by the local XMPP service (OF-2643). Both the validation of stream headers and Server Dialback + * use it. */ public class ServerStanzaHandlerLocallyServicedTest { @@ -78,7 +79,7 @@ public void setUp() } /** - * Asserts that the XMPP domain of this server is serviced. + * Asserts that the XMPP domain of the local XMPP service is serviced. */ @Test public void testXmppDomainIsServiced() @@ -87,8 +88,8 @@ public void testXmppDomainIsServiced() } /** - * Asserts that the XMPP domain of this server is serviced when a peer names it in a different case. Domain names - * are compared after normalisation, as RFC 6120 requires. + * Asserts that the XMPP domain of the local XMPP service is serviced when a peer names it in a different case. + * Domain names are compared after normalisation, as RFC 6120 requires. */ @Test public void testXmppDomainInDifferentCaseIsServiced() @@ -225,7 +226,7 @@ public void testUnrelatedDomainIsNotServicedDespiteInvalidConfiguredDomain() } /** - * Asserts that a domain that is unrelated to this server is not serviced. + * Asserts that a domain that is unrelated to the local XMPP service is not serviced. */ @Test public void testUnrelatedDomainIsNotServiced() diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index f29fdc06e7..ede47ea72a 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -120,8 +120,8 @@ public void testClientStreamToUnknownHostIsRefused() throws Exception /** * Asserts that a stream that is refused because it addresses an unknown host is opened from the XMPP domain of - * this server, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity, rather than - * from the unknown host. + * the local XMPP service, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity, + * rather than from the unknown host. */ @Test public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception @@ -134,13 +134,13 @@ public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception // Verify result. final String delivered = deliveredText(); - assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The stream must be opened from the XMPP domain of this server."); + assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The stream must be opened from the XMPP domain of the local XMPP service."); assertFalse(delivered.contains("from=\"unserviced.example.com\""), "The stream must not be opened from the unknown host."); } /** * Asserts that a stream header that does not address a host, and that is refused, is answered with a stream that - * is opened from the XMPP domain of this server, as RFC 6120 section 4.9.1.3 requires. + * is opened from the XMPP domain of the local XMPP service, as RFC 6120 section 4.9.1.3 requires. */ @Test public void testStreamWithoutToIsRefusedFromOwnDomain() throws Exception @@ -151,7 +151,7 @@ public void testStreamWithoutToIsRefusedFromOwnDomain() throws Exception // Verify result. final String delivered = deliveredText(); assertTrue(delivered.contains(" Date: Wed, 28 Oct 2026 19:48:39 +0100 Subject: [PATCH 13/22] OF-2643: Describe the reason for every assertion in the new tests Several assertions and Mockito verifications in the tests for host validation carried no message, so a failure did not say what was expected. Each one now has a message, using a verification mode description for the Mockito verifications. No functional change. --- ...ServerStanzaHandlerHostValidationTest.java | 29 ++++++++++++++++--- ...erverStanzaHandlerLocallyServicedTest.java | 22 +++++++------- .../net/StanzaHandlerRefuseStreamTest.java | 23 +++++++++++++-- .../server/ServerDialbackHostUnknownTest.java | 10 +++---- ...erverDialbackValidateRemoteDomainTest.java | 2 +- 5 files changed, 62 insertions(+), 24 deletions(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index e08caa4e2a..57f5caf315 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -44,6 +44,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.withSettings; @@ -112,7 +113,7 @@ public void setUp() throws Exception @Test public void testValidationIsDisabledByDefault() { - assertFalse(handler.validateHost()); + assertFalse(handler.validateHost(), "Host validation must be disabled when it has not been configured."); } /** @@ -132,7 +133,7 @@ public void testInitialStreamToUnservicedDomainIsRefused() throws Exception assertEquals(0, handler.sessionsCreated, "No session must be created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); assertTrue(deliveredText().contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of the local XMPP service."); - verify(connection).close(); + verify(connection, times(1).description("The connection must be closed after the refusal.")).close(); } /** @@ -202,6 +203,26 @@ public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDom assertFalse(delivered.contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); } + /** + * Asserts that, with validation enabled, a stream that is opened after TLS is negotiated without declaring a + * content namespace, and that names a domain that is not serviced, is refused with a host-unknown stream error. + */ + @Test + public void testStreamAfterTlsWithoutNamespaceToUnservicedDomainIsRefused() throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + handler.initiateSession(streamHeader(Fixtures.XMPP_DOMAIN), reader); + handler.startedTLS = true; + + // Execute system under test. + handler.initiateSession("", reader); + + // Verify result. + assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); + verify(connection, times(1).description("The connection must be closed after the refusal.")).close(); + } + /** * Asserts that a stream that is refused for a reason other than its host is answered from the domain that it names, * when that domain is serviced. @@ -237,7 +258,7 @@ public void testStreamAfterTlsToUnservicedDomainIsRefused() throws Exception assertEquals(1, handler.sessionsCreated, "The session must not be re-created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); assertFalse(deliveredText().contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); - verify(connection).close(); + verify(connection, times(1).description("The connection must be closed after the refusal.")).close(); } /** @@ -315,7 +336,7 @@ private static String streamHeaderWithNamespace(final String to, final String na private String deliveredText() { final ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - verify(connection, atLeastOnce()).deliverRawText(captor.capture()); + verify(connection, atLeastOnce().description("Text must have been delivered to the peer.")).deliverRawText(captor.capture()); return String.join("", captor.getAllValues()); } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index ddf6cb53be..611b3c02c5 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -84,7 +84,7 @@ public void setUp() @Test public void testXmppDomainIsServiced() { - assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN)); + assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN), "The XMPP domain must be serviced."); } /** @@ -94,7 +94,7 @@ public void testXmppDomainIsServiced() @Test public void testXmppDomainInDifferentCaseIsServiced() { - assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT))); + assertTrue(ServerStanzaHandler.isLocallyServiced(Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)), "The XMPP domain must be serviced when it is written in a different case."); } /** @@ -104,7 +104,7 @@ public void testXmppDomainInDifferentCaseIsServiced() @Test public void testComponentSubdomainIsServiced() { - assertTrue(ServerStanzaHandler.isLocallyServiced(COMPONENT_DOMAIN)); + assertTrue(ServerStanzaHandler.isLocallyServiced(COMPONENT_DOMAIN), "A subdomain that has a component route must be serviced."); } /** @@ -113,7 +113,7 @@ public void testComponentSubdomainIsServiced() @Test public void testSubdomainWithoutComponentIsNotServiced() { - assertFalse(ServerStanzaHandler.isLocallyServiced("unknown." + Fixtures.XMPP_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced("unknown." + Fixtures.XMPP_DOMAIN), "A subdomain that has no component route must not be serviced."); } /** @@ -142,7 +142,7 @@ public void testTrunkedDomainIsServiced() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); // Execute system under test & verify result. - assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN), "A domain for which trunking is enabled must be serviced."); } /** @@ -156,7 +156,7 @@ public void testTrunkedDomainIsNotServicedWhenTrunkingIsDisabled() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); // Execute system under test & verify result. - assertFalse(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN), "A domain that is configured for trunking must not be serviced while trunking is disabled."); } /** @@ -171,7 +171,7 @@ public void testSubdomainOfTrunkedDomainIsNotServiced() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); // Execute system under test & verify result. - assertFalse(ServerStanzaHandler.isLocallyServiced("conference." + TRUNKED_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced("conference." + TRUNKED_DOMAIN), "A subdomain of a domain for which trunking is enabled must not be serviced."); } /** @@ -207,7 +207,7 @@ public void testTrunkedDomainIsServicedDespiteInvalidConfiguredDomain() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(INVALID_DOMAIN, TRUNKED_DOMAIN)); // Execute system under test & verify result. - assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN)); + assertTrue(ServerStanzaHandler.isLocallyServiced(TRUNKED_DOMAIN), "A domain for which trunking is enabled must be serviced when another configured value is invalid."); } /** @@ -222,7 +222,7 @@ public void testUnrelatedDomainIsNotServicedDespiteInvalidConfiguredDomain() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(INVALID_DOMAIN)); // Execute system under test & verify result. - assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN), "An unrelated domain must not be serviced when a configured trunking value is invalid."); } /** @@ -231,7 +231,7 @@ public void testUnrelatedDomainIsNotServicedDespiteInvalidConfiguredDomain() @Test public void testUnrelatedDomainIsNotServiced() { - assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced(UNSERVICED_DOMAIN), "An unrelated domain must not be serviced."); } /** @@ -241,6 +241,6 @@ public void testUnrelatedDomainIsNotServiced() @Test public void testInvalidDomainIsNotServiced() { - assertFalse(ServerStanzaHandler.isLocallyServiced(INVALID_DOMAIN)); + assertFalse(ServerStanzaHandler.isLocallyServiced(INVALID_DOMAIN), "A value that is not a valid domain name must not be serviced."); } } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index ede47ea72a..91cec40516 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.withSettings; @@ -93,7 +94,23 @@ public void testStreamWithInvalidNamespaceIsRefused() throws Exception assertTrue(delivered.contains("", reader); + + // Verify result. + assertTrue(deliveredText().contains(" captor = ArgumentCaptor.forClass(String.class); - verify(connection, atLeastOnce()).deliverRawText(captor.capture()); + verify(connection, atLeastOnce().description("Text must have been delivered to the peer.")).deliverRawText(captor.capture()); return String.join("", captor.getAllValues()); } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java index c405688791..a2fad3e307 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackHostUnknownTest.java @@ -85,7 +85,7 @@ public void setUp() @Test public void testXmppDomainIsKnown() { - assertFalse(dialback.isHostUnknown(Fixtures.XMPP_DOMAIN)); + assertFalse(dialback.isHostUnknown(Fixtures.XMPP_DOMAIN), "The XMPP domain must not be unknown."); } /** @@ -94,7 +94,7 @@ public void testXmppDomainIsKnown() @Test public void testComponentSubdomainIsKnown() { - assertFalse(dialback.isHostUnknown(COMPONENT_DOMAIN)); + assertFalse(dialback.isHostUnknown(COMPONENT_DOMAIN), "A subdomain that has a component route must not be unknown."); } /** @@ -108,7 +108,7 @@ public void testTrunkedDomainIsKnown() Trunking.ALLOWABLE_DOMAINS.setValue(List.of(TRUNKED_DOMAIN)); // Execute system under test & verify result. - assertFalse(dialback.isHostUnknown(TRUNKED_DOMAIN)); + assertFalse(dialback.isHostUnknown(TRUNKED_DOMAIN), "A domain for which trunking is enabled must not be unknown."); } /** @@ -117,7 +117,7 @@ public void testTrunkedDomainIsKnown() @Test public void testUnrelatedDomainIsUnknown() { - assertTrue(dialback.isHostUnknown("unserviced.example.com")); + assertTrue(dialback.isHostUnknown("unserviced.example.com"), "An unrelated domain must be unknown."); } /** @@ -142,6 +142,6 @@ public void testDomainThatContainsXmppDomainIsUnknown() @Test public void testMissingDomainIsUnknown() { - assertTrue(dialback.isHostUnknown(null)); + assertTrue(dialback.isHostUnknown(null), "A dialback request without a 'to' attribute must be treated as being for an unknown host."); } } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java index 2a8cbad16f..235e54748b 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java @@ -125,7 +125,7 @@ public void testExistingSessionIsDetectedForDomainInDifferentCase() final ServerDialbackErrorException exception = assertThrows(ServerDialbackErrorException.class, () -> dialback.validateRemoteDomain(dbResult, new BasicStreamIDFactory().createStreamID()), "A second session for the same domain, written in a different case, must be refused."); - assertEquals("resource-constraint", exception.getError().getCondition().toXMPP()); + assertEquals("resource-constraint", exception.getError().getCondition().toXMPP(), "The refusal must report that a session for the domain already exists."); } } From 410c251dd7034ae3e15017cd31a979e394a5b6fb Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:48:47 +0100 Subject: [PATCH 14/22] OF-2643: Add the Dutch description of xmpp.server.validate.host --- i18n/src/main/resources/openfire_i18n_nl.properties | 1 + 1 file changed, 1 insertion(+) diff --git a/i18n/src/main/resources/openfire_i18n_nl.properties b/i18n/src/main/resources/openfire_i18n_nl.properties index 919ed0de98..7f76ab2497 100644 --- a/i18n/src/main/resources/openfire_i18n_nl.properties +++ b/i18n/src/main/resources/openfire_i18n_nl.properties @@ -1244,6 +1244,7 @@ system_property.xmpp.gateway.enabled=Bepaald of Openfire's 'trunking' of 'gatewa system_property.xmpp.gateway.domains=Een collectie XMPP domeinnamen waar Openfire trunkingfunctionaliteit voor zal bieden. Openfire zal voor elk domein in deze collectie data accepteren van andere domeinen, om die data vervolgens door te sturen naar dat domein. system_property.xmpp.server.rewrite.replace-missing-to=Als de server een IQ of message stanza ontvangt zonder 'to' attribuut, zet dan de waarde van het 'to' attribuut op de bare JID representatie van het 'from' attribuut. system_property.xmpp.server.incoming.skip-jid-validation=Definieert of JIDs die de addressen zijn van stanzas die door externe domeinen worden aangeleverd, worden gevalideerd. +system_property.xmpp.server.validate.host=Indien ingeschakeld, wordt een inkomende server-naar-server stream die een domein adresseert dat niet door de lokale XMPP-dienst wordt bediend, geweigerd met een host-unknown stream error. Bediende domeinen zijn het XMPP-domein, de subdomeinen daarvan die een component hebben, en domeinen die voor trunking zijn geconfigureerd. Een stream die geen domein adresseert, wordt geaccepteerd. system_property.xmpp.server.outgoing.max.threads=Maximum aantal threads in de threadpool die gebruikt wordt om server-naar-server verbindingen op te zetten. Als meer connecties tegelijkertijd worden aangevraagd, dan zal de aanvragende thread de connectie opzetten. Dit kan een vertragend effect hebben. system_property.xmpp.server.outgoing.min.threads=Minimum aantal threads in de threadpool die gebruikt wordt om server-naar-server verbindingen op te zetten. system_property.xmpp.server.outgoing.threads-timeout=Tijd waarna inactieve en overbodige threads worden verwijderd van de threadpool die gebruikt wordt om server-naar-server verbindingen op te zetten. From a3f15af407c1d2a4cab227c6ff6ca327dfd17937 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:49:08 +0100 Subject: [PATCH 15/22] OF-2643: Define xmpp.client.validate.host as a SystemProperty xmpp.client.validate.host was read with JiveGlobals, while its server-to-server counterpart xmpp.server.validate.host is a SystemProperty. The client property was therefore not listed with a description on the system properties page of the admin console, and the two could not be managed in the same way. It is now ClientStanzaHandler.VALIDATE_HOST, with the same key, default value and dynamic behaviour as before, and with a description in English and Dutch. No functional change. --- i18n/src/main/resources/openfire_i18n.properties | 1 + .../main/resources/openfire_i18n_nl.properties | 1 + .../openfire/net/ClientStanzaHandler.java | 16 ++++++++++++++-- .../net/StanzaHandlerRefuseStreamTest.java | 4 ++-- 4 files changed, 18 insertions(+), 4 deletions(-) diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index f1a7abff3d..c98a6c0fa0 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -1342,6 +1342,7 @@ system_property.xmpp.client.idle.ping=Set to true to ping idle clients, otherwis system_property.xmpp.client.limits.advertisement.disabled=Disables the advertisement of server limits in the XMPP stream features (XEP-0478) on client connections. system_property.xmpp.client.version-query.enabled=Send a version request query to clients when they connect. system_property.xmpp.client.version-query.delay=After this amount of time has passed since a new client connection has been accepted, a version request is being sent to the peer. +system_property.xmpp.client.validate.host=When enabled, a client stream that addresses a domain other than the XMPP domain is refused with a host-unknown stream error. A stream that does not address a domain is accepted. system_property.xmpp.gateway.enabled=Controls if Openfire's 'trunking' or 'gateway' functionality is enabled. This feature allows Openfire to act as a gateway to transfer data between other XMPP domains. system_property.xmpp.gateway.domains=A collection of XMPP domain names for which Openfire will provide trunking functionality. For each domain in this collection, Openfire will accept data from other domains, and forward that data to that domain. system_property.xmpp.server.rewrite.replace-missing-to=If the server receives a message or IQ stanza with no 'to' attribute, set the 'to' attribute to the bare JID representation of the 'from' attribute value. diff --git a/i18n/src/main/resources/openfire_i18n_nl.properties b/i18n/src/main/resources/openfire_i18n_nl.properties index 7f76ab2497..51fe1e9c04 100644 --- a/i18n/src/main/resources/openfire_i18n_nl.properties +++ b/i18n/src/main/resources/openfire_i18n_nl.properties @@ -1240,6 +1240,7 @@ system_property.xmpp.client.idle.ping=Zet op 'true' om inactieve sessies te ping system_property.xmpp.client.limits.advertisement.disabled=Schakelt het adverteren van server-limieten in XMPP stream features (XEP-0478) uit voor client-connecties. system_property.xmpp.client.version-query.enabled=Probeer de versie van een client op te vragen, zodra er een nieuwe verbinding wordt gemaakt. system_property.xmpp.client.version-query.delay=De hoeveelheid tijd die verstreken moet zijn nadat een nieuwe client-connectie is opgezet, voordat de client wordt gevraagd naar haar software versie. +system_property.xmpp.client.validate.host=Indien ingeschakeld, wordt een client-stream die een ander domein dan het XMPP-domein adresseert, geweigerd met een host-unknown stream error. Een stream die geen domein adresseert, wordt geaccepteerd. system_property.xmpp.gateway.enabled=Bepaald of Openfire's 'trunking' of 'gateway' functionaliteit beschikbaar is. Deze functionaliteit stelt Openfire in staat om als gateway te dienen die data tussen andere XMPP domeinen kan verplaatsen. system_property.xmpp.gateway.domains=Een collectie XMPP domeinnamen waar Openfire trunkingfunctionaliteit voor zal bieden. Openfire zal voor elk domein in deze collectie data accepteren van andere domeinen, om die data vervolgens door te sturen naar dat domein. system_property.xmpp.server.rewrite.replace-missing-to=Als de server een IQ of message stanza ontvangt zonder 'to' attribuut, zet dan de waarde van het 'to' attribuut op de bare JID representatie van het 'from' attribuut. diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java index 8cb4625b18..4439c8524e 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java @@ -23,7 +23,7 @@ import org.jivesoftware.openfire.auth.UnauthorizedException; import org.jivesoftware.openfire.csi.CsiManager; import org.jivesoftware.openfire.session.LocalClientSession; -import org.jivesoftware.util.JiveGlobals; +import org.jivesoftware.util.SystemProperty; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParser; @@ -47,6 +47,18 @@ public class ClientStanzaHandler extends StanzaHandler { private static final Logger Log = LoggerFactory.getLogger(ClientStanzaHandler.class); + /** + * Controls if the 'to' attribute of stream headers sent by clients is validated. When enabled, a stream that names + * a domain other than the XMPP domain of the local XMPP service is refused with a host-unknown stream error. + * + * @see #isHostUnknown(String) + */ + public static final SystemProperty VALIDATE_HOST = SystemProperty.Builder.ofType(Boolean.class) + .setKey("xmpp.client.validate.host") + .setDefaultValue(false) + .setDynamic(true) + .build(); + public ClientStanzaHandler(PacketRouter router, Connection connection) { super(router, connection); } @@ -68,7 +80,7 @@ protected Namespace getNamespace() { @Override protected boolean validateHost() { - return JiveGlobals.getBooleanProperty("xmpp.client.validate.host",false); + return VALIDATE_HOST.getValue(); } @Override diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 91cec40516..939838bd6b 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -122,7 +122,7 @@ public void testStreamWithoutNamespaceIsRefused() throws Exception public void testClientStreamToUnknownHostIsRefused() throws Exception { // Setup test fixture. - JiveGlobals.setProperty("xmpp.client.validate.host", "true"); + ClientStanzaHandler.VALIDATE_HOST.setValue(true); // Execute system under test. handler.initiateSession("", reader); @@ -144,7 +144,7 @@ public void testClientStreamToUnknownHostIsRefused() throws Exception public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception { // Setup test fixture. - JiveGlobals.setProperty("xmpp.client.validate.host", "true"); + ClientStanzaHandler.VALIDATE_HOST.setValue(true); // Execute system under test. handler.initiateSession("", reader); From d8196c30862e315d47d351405941d1485e69cd8f Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:49:50 +0100 Subject: [PATCH 16/22] OF-2643: Allow the empty string when refusing a stream for an unknown host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 6120 §4.9.1.3 allows the stream that refuses a header with a missing or unknown 'to' to be opened from either an authoritative name of the receiving entity or the empty string. The documentation of StanzaHandler#refuseStream() and the tests described the authoritative name as the only option, and the tests asserted the XMPP domain specifically. The documentation now names both options, and states that the XMPP domain is the authoritative name that is used. The tests accept either value, so that they verify the requirement of the RFC rather than the choice that is made. No functional change. --- .../openfire/net/StanzaHandler.java | 4 +-- ...ServerStanzaHandlerHostValidationTest.java | 22 +++++++++++--- .../net/StanzaHandlerRefuseStreamTest.java | 30 ++++++++++++++----- 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java index a7a6e37e40..f1bb1cabb2 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -943,8 +943,8 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I * * The stream is opened from the host that the peer addressed, unless that host is missing or is not serviced (as * determined by {@link #isHostUnknown(String)}), whatever the reason for the refusal: then it is opened from the XMPP - * domain of the local XMPP service, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving - * entity. + * domain of the local XMPP service. In that case, RFC 6120 section 4.9.1.3 allows either an authoritative name for + * the receiving entity or the empty string; the XMPP domain is such an authoritative name. * * @param xpp a parser positioned on the stream header that is refused. * @param host the value of the 'to' attribute of the refused stream header (can be null). diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index 57f5caf315..8dc47c3a65 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -132,7 +132,7 @@ public void testInitialStreamToUnservicedDomainIsRefused() throws Exception // Verify result. assertEquals(0, handler.sessionsCreated, "No session must be created for a stream that names a domain that is not serviced."); assertTrue(deliveredText().contains("host-unknown"), "The peer must be told that the host is unknown."); - assertTrue(deliveredText().contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of the local XMPP service."); + assertTrue(isOpenedFromAuthoritativeNameOrEmptyString(deliveredText()), "The refusal must be sent from an authoritative name of the local XMPP service, or from the empty string."); verify(connection, times(1).description("The connection must be closed after the refusal.")).close(); } @@ -188,10 +188,11 @@ public void testInitialStreamWithoutToIsAccepted() throws Exception /** * Asserts that, with validation disabled, a stream that names a domain that is not serviced, and that is refused - * for another reason, is answered from the XMPP domain. + * for another reason, is answered from an authoritative name of the local XMPP service or from the empty string, + * as RFC 6120 section 4.9.1.3 allows. */ @Test - public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDomain() throws Exception + public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromAuthoritativeNameOrEmptyString() throws Exception { // Execute system under test. handler.initiateSession(streamHeaderWithNamespace(UNSERVICED_DOMAIN, "jabber:client"), reader); @@ -199,7 +200,7 @@ public void testStreamToUnservicedDomainRefusedForOtherReasonIsRefusedFromOwnDom // Verify result. final String delivered = deliveredText(); assertTrue(delivered.contains("invalid-namespace"), "The stream must be refused for its namespace."); - assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The refusal must be sent from the XMPP domain of the local XMPP service."); + assertTrue(isOpenedFromAuthoritativeNameOrEmptyString(delivered), "The refusal must be sent from an authoritative name of the local XMPP service, or from the empty string."); assertFalse(delivered.contains("from=\"" + UNSERVICED_DOMAIN + "\""), "The refusal must not be sent from the domain that is not serviced."); } @@ -340,6 +341,19 @@ private String deliveredText() return String.join("", captor.getAllValues()); } + /** + * Checks if text that was delivered to the peer opens a stream from a value that RFC 6120 section 4.9.1.3 allows + * when the peer addressed no host or an unknown host: an authoritative name for the receiving entity (in these + * tests, the XMPP domain), or the empty string. + * + * @param delivered text that was delivered to the peer. + * @return true if the stream is opened from an allowed value. + */ + private static boolean isOpenedFromAuthoritativeNameOrEmptyString(final String delivered) + { + return delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\"") || delivered.contains("from=\"\""); + } + /** * Returns a namespace-aware packet reader, configured as the production code configures its own. * diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 939838bd6b..42d6af7679 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -136,12 +136,12 @@ public void testClientStreamToUnknownHostIsRefused() throws Exception } /** - * Asserts that a stream that is refused because it addresses an unknown host is opened from the XMPP domain of - * the local XMPP service, as RFC 6120 section 4.9.1.3 requires an authoritative name for the receiving entity, - * rather than from the unknown host. + * Asserts that a stream that is refused because it addresses an unknown host is opened from an authoritative name + * of the local XMPP service or from the empty string, as RFC 6120 section 4.9.1.3 requires, rather than from the + * unknown host. */ @Test - public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception + public void testStreamToUnknownHostIsRefusedFromAuthoritativeNameOrEmptyString() throws Exception { // Setup test fixture. ClientStanzaHandler.VALIDATE_HOST.setValue(true); @@ -151,16 +151,17 @@ public void testStreamToUnknownHostIsRefusedFromOwnDomain() throws Exception // Verify result. final String delivered = deliveredText(); - assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The stream must be opened from the XMPP domain of the local XMPP service."); + assertTrue(isOpenedFromAuthoritativeNameOrEmptyString(delivered), "The stream must be opened from an authoritative name of the local XMPP service, or from the empty string."); assertFalse(delivered.contains("from=\"unserviced.example.com\""), "The stream must not be opened from the unknown host."); } /** * Asserts that a stream header that does not address a host, and that is refused, is answered with a stream that - * is opened from the XMPP domain of the local XMPP service, as RFC 6120 section 4.9.1.3 requires. + * is opened from an authoritative name of the local XMPP service or from the empty string, as RFC 6120 section + * 4.9.1.3 requires. */ @Test - public void testStreamWithoutToIsRefusedFromOwnDomain() throws Exception + public void testStreamWithoutToIsRefusedFromAuthoritativeNameOrEmptyString() throws Exception { // Execute system under test. handler.initiateSession("", reader); @@ -168,7 +169,7 @@ public void testStreamWithoutToIsRefusedFromOwnDomain() throws Exception // Verify result. final String delivered = deliveredText(); assertTrue(delivered.contains(" Date: Wed, 28 Oct 2026 19:49:57 +0100 Subject: [PATCH 17/22] OF-2643: Reference the label length limit that makes the test domain invalid --- .../openfire/net/ServerStanzaHandlerLocallyServicedTest.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java index 611b3c02c5..d14a4fcfb6 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerLocallyServicedTest.java @@ -46,7 +46,10 @@ public class ServerStanzaHandlerLocallyServicedTest private static final String UNSERVICED_DOMAIN = "unserviced.example.com"; /** - * A value that cannot be a domain name, as its first label exceeds 63 characters. + * A value that cannot be a domain name, as its first label exceeds 63 characters, which is the maximum length of a + * label that RFC 1035 section 2.3.4 defines. + * + * @see RFC 1035 section 2.3.4: Size limits */ private static final String INVALID_DOMAIN = "a".repeat(64) + ".example.org"; From e1ace8efd6362ba644c1a4ae85b8aaebdac60400 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:50:12 +0100 Subject: [PATCH 18/22] OF-2643: Give the current reason for accepting a stream header without 'to' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reason given for accepting an S2S stream header without a 'to' attribute was that servers using legacy Server Dialback omit it. That describes pre-XMPP 1.0 peers only, and does not say why accepting such a header does not undermine host validation. The comment now refers to RFC 6120 §4.7.2, which requires 'to' but asks receiving entities to be liberal towards peers that omit it, and notes that such a stream is treated as addressing the XMPP domain, which is serviced. No functional change. --- .../org/jivesoftware/openfire/net/ServerStanzaHandler.java | 3 ++- .../openfire/net/ServerStanzaHandlerHostValidationTest.java | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index 3932113337..57c2fbfed0 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -150,7 +150,8 @@ boolean validateHost() { @Override protected boolean isHostUnknown(String host) { - // A missing 'to' attribute is accepted, as servers that use legacy Server Dialback omit it. + // OF-2643: Accept a missing 'to', as RFC 6120 section 4.7.2 asks to be liberal towards peers that omit it. The + // stream is then treated as addressing the XMPP domain (see LocalIncomingServerSession#createSession). return host != null && !isLocallyServiced(host); } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index 8dc47c3a65..fd28788507 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -54,7 +54,7 @@ * When validation is enabled, a stream that names a domain that is not serviced by the local XMPP service is refused * with a host-unknown stream error. Serviced domains are the XMPP domain of the service, subdomains of it that have a * component route, and domains for which trunking is enabled. A stream header that has no 'to' attribute is accepted, - * as servers that use legacy Server Dialback omit it. + * as RFC 6120 section 4.7.2 asks to be liberal towards peers that omit it. * * Stream headers are validated both when the stream is first opened, and when it is restarted after TLS has been * negotiated: RFC 6120 section 5.4.3.3 requires values obtained before TLS took effect to be discarded, which makes the @@ -170,8 +170,8 @@ public void testInitialStreamToServicedDomainIsAccepted(final String domain) thr } /** - * Asserts that, with validation enabled, a stream that does not name a domain at all is accepted, as servers that - * use legacy Server Dialback omit the 'to' attribute. + * Asserts that, with validation enabled, a stream that does not name a domain at all is accepted, as RFC 6120 + * section 4.7.2 asks to be liberal towards peers that omit the 'to' attribute. */ @Test public void testInitialStreamWithoutToIsAccepted() throws Exception From 9ddb5491b657b6940d55fc42a9cd0f2b7227c2d1 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 19:51:01 +0100 Subject: [PATCH 19/22] OF-2643: Verify that a stream header without 'to' after STARTTLS is accepted A stream header without a 'to' attribute is accepted when host validation is enabled. A test verified that for the first stream header, but not for the header that follows TLS negotiation, which is validated on a separate path. A test now verifies that the latter is accepted too. --- ...ServerStanzaHandlerHostValidationTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java index fd28788507..a9a3b2f280 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/ServerStanzaHandlerHostValidationTest.java @@ -286,6 +286,26 @@ public void testStreamAfterTlsToServicedDomainIsAccepted(final String domain) th assertFalse(deliveredText().contains("host-unknown"), "The stream must not be refused."); } + /** + * Asserts that, with validation enabled, a stream that does not name a domain after TLS is negotiated is accepted, + * in the same way as a stream that does not name a domain before TLS is negotiated. + */ + @Test + public void testStreamAfterTlsWithoutToIsAccepted() throws Exception + { + // Setup test fixture. + ServerStanzaHandler.VALIDATE_HOST.setValue(true); + handler.initiateSession(streamHeader(Fixtures.XMPP_DOMAIN), reader); + handler.startedTLS = true; + + // Execute system under test. + handler.initiateSession(streamHeader(null), reader); + + // Verify result. + assertEquals(2, handler.sessionsCreated, "The session must be re-created for the stream that follows TLS negotiation, when it has no 'to' attribute."); + assertFalse(deliveredText().contains("host-unknown"), "The stream must not be refused."); + } + /** * Asserts that, with validation disabled, a stream that names a domain that is not serviced after TLS is * negotiated is accepted, as it was before validation was introduced. From ebf3073df288205877f5a2dead890d441ef85d6c Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 28 Oct 2026 20:56:16 +0100 Subject: [PATCH 20/22] OF-2643: Return the normalised domain from ServerDialback#validateRemoteDomain() ServerDialback#validateRemoteDomain() passed the normalised form of the validated domain to its callers by rewriting the 'to' attribute of the db:result element that it was given. createIncomingSession() and LocalIncomingServerSession#validateSubsequentDomain() read that attribute after the call. Nothing in the signature showed this dependency, so a caller that read the attribute before the call would get the value as the peer wrote it. The method now returns the normalised domain, and both callers use the returned value. The element is no longer modified. No functional change. --- .../openfire/server/ServerDialback.java | 20 +++++++++---------- .../session/LocalIncomingServerSession.java | 4 +--- ...erverDialbackValidateRemoteDomainTest.java | 9 +++++---- 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java index bfcce85c84..523745e56a 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/server/ServerDialback.java @@ -425,7 +425,7 @@ public LocalIncomingServerSession createIncomingSession(XMPPPacketReader reader) String recipient = doc.attributeValue("to"); Log.debug("ServerDialback: RS - Validating remote domain for incoming session from {} to {}", hostname, recipient); try { - validateRemoteDomain(doc, streamID); + final String localDomain = validateRemoteDomain(doc, streamID); Log.debug("ServerDialback: RS - Validation of remote domain for incoming session from {} to {} was successful.", hostname, recipient); // Create a server Session for the remote server LocalIncomingServerSession session = sessionManager. @@ -435,7 +435,7 @@ public LocalIncomingServerSession createIncomingSession(XMPPPacketReader reader) session.setAuthenticationMethod(ServerSession.AuthenticationMethod.DIALBACK); // Set the domain or subdomain of the local server used when // validating the session - session.setLocalDomain(doc.attributeValue("to")); + session.setLocalDomain(localDomain); // After the session has been created, inform all listeners as well. ServerSessionEventDispatcher.dispatchEvent(session, ServerSessionEventDispatcher.EventType.session_created); return session; @@ -501,8 +501,8 @@ protected void dialbackError(String from, String to, PacketError err) { } /** - * Returns true if the domain requested by the remote server was validated by the Authoritative - * Server. To validate the domain a new TCP connection will be established to the + * Validates the domain requested by the remote server with the Authoritative Server, and returns that domain in + * normalised form. To validate the domain a new TCP connection will be established to the * Authoritative Server. The Authoritative Server may be the same Originating Server or * some other machine in the Originating Server's network.

* @@ -511,9 +511,10 @@ protected void dialbackError(String from, String to, PacketError err) { * * @param doc the request for validating the new domain. * @param streamID the stream id generated by this server for the Originating Server. + * @return the validated domain, normalised to the form in which the local XMPP service knows it. * @throws StreamErrorException when validation did not succeed. */ - public void validateRemoteDomain(Element doc, StreamID streamID) throws StreamErrorException, ServerDialbackErrorException, ServerDialbackKeyInvalidException + public String validateRemoteDomain(Element doc, StreamID streamID) throws StreamErrorException, ServerDialbackErrorException, ServerDialbackKeyInvalidException { StringBuilder sb; String recipient = doc.attributeValue("to"); @@ -538,10 +539,9 @@ else if (isHostUnknown(recipient)) { throw new ServerDialbackErrorException(recipient, remoteDomain, new PacketError(PacketError.Condition.item_not_found, PacketError.Type.cancel, "Service not hosted here")); } else { - // The domain is serviced. Continue with its normalised form, and record that in the request, so that the - // session is created for, and the result is reported from, the domain as the local XMPP service knows it. + // The domain is serviced. Continue with, and return, its normalised form, so that the session is created + // for, and the result is reported from, the domain as the local XMPP service knows it. recipient = JID.domainprep(recipient); - doc.addAttribute("to", recipient); log.debug( "Check if the remote domain already has a connection to the target domain/subdomain" ); boolean alreadyExists = false; @@ -558,7 +558,7 @@ else if (isHostUnknown(recipient)) { if (SASLAuthentication.verifyCertificates(connection.getPeerCertificates(), remoteDomain, true)) { log.debug( "Host authenticated based on SASL. Weaker dialback-based authentication is skipped." ); log.debug( "Domain validated successfully!" ); - return; + return recipient; } log.debug( "Unable to authenticate host based on stronger SASL. Proceeding with dialback..." ); @@ -627,7 +627,7 @@ else if (isHostUnknown(recipient)) { switch(result) { case valid: log.debug( "Successfully validated domain!" ); - return; + return recipient; case invalid: throw new ServerDialbackKeyInvalidException(recipient, remoteDomain); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java index b25a217d03..ee6ec1b2e6 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java @@ -248,9 +248,7 @@ public boolean validateSubsequentDomain(Element dbResult) { final DomainPair domainPair = new DomainPair(getServerName(), fromDomain); ServerDialback method = new ServerDialback(getConnection(), domainPair); try { - method.validateRemoteDomain(dbResult, getStreamID()); - - final String recipient = dbResult.attributeValue("to"); + final String recipient = method.validateRemoteDomain(dbResult, getStreamID()); final String remoteDomain = dbResult.attributeValue("from"); // Add the validated domain as a valid domain. Do this before notifying the remote domain of success! (OF-2626) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java index 235e54748b..9769ee2eab 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackValidateRemoteDomainTest.java @@ -130,8 +130,8 @@ public void testExistingSessionIsDetectedForDomainInDifferentCase() } /** - * Asserts that, once accepted, the domain that a remote server asks to be authenticated for is normalised in the - * request, so that the session is created for, and the result is reported from, the normalised domain. + * Asserts that, once accepted, the domain that a remote server asks to be authenticated for is returned in + * normalised form, so that the session is created for, and the result is reported from, the normalised domain. */ @Test public void testAcceptedDomainIsNormalised() throws Exception @@ -144,6 +144,7 @@ public void testAcceptedDomainIsNormalised() throws Exception dbResult.addAttribute("to", Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)); dbResult.setText("key"); + final String result; try (final MockedStatic remoteServers = mockStatic(RemoteServerManager.class); final MockedStatic sasl = mockStatic(SASLAuthentication.class)) { @@ -151,10 +152,10 @@ public void testAcceptedDomainIsNormalised() throws Exception sasl.when(() -> SASLAuthentication.verifyCertificates(any(), anyString(), anyBoolean())).thenReturn(true); // Execute system under test. - dialback.validateRemoteDomain(dbResult, new BasicStreamIDFactory().createStreamID()); + result = dialback.validateRemoteDomain(dbResult, new BasicStreamIDFactory().createStreamID()); } // Verify result. - assertEquals(Fixtures.XMPP_DOMAIN, dbResult.attributeValue("to"), "The accepted domain must be normalised."); + assertEquals(Fixtures.XMPP_DOMAIN, result, "The accepted domain must be returned in normalised form."); } } From 510a3009c506cb873f691ed93a6353f58d382d78 Mon Sep 17 00:00:00 2001 From: Matthew Vivian Date: Thu, 29 Oct 2026 11:14:58 +0000 Subject: [PATCH 21/22] OF-2643: Restore the namespace-refusal tests from OF-3425 Rebasing this branch onto OF-3425 (#3527) re-created StanzaHandlerRefuseStreamTest from this branch, which overwrote the version added by OF-3425 and dropped its testClientStreamWithoutNamespaceIsRefusedWithError and testServerStreamWithoutNamespaceIsRefusedWithError. Those two are restored here, covering both client and server streams and checking the namespace in which the refusal is answered. The narrower testStreamWithoutNamespaceIsRefused, which covered only client streams and did not check that namespace, is dropped as they supersede it. --- .../net/StanzaHandlerRefuseStreamTest.java | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 42d6af7679..19330e520e 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -44,6 +44,12 @@ */ public class StanzaHandlerRefuseStreamTest { + /** + * A stream namespace that is not the one that RFC 6120 section 4.8.1 requires, used to have a stream header + * refused for a reason that does not depend on the content namespace that the header declares. + */ + private static final String WRONG_STREAM_NAMESPACE = "http://example.org/not-the-stream-namespace"; + private Connection connection; private ClientStanzaHandler handler; @@ -99,21 +105,50 @@ public void testStreamWithInvalidNamespaceIsRefused() throws Exception } /** - * Asserts that a stream header that does not declare a content namespace is refused with an invalid-namespace - * stream error. + * Asserts that a client stream header that declares no content namespace, and that is refused, is answered with the + * stream error, opened in the namespace of the handler, and that the connection is then closed. Building that error + * must not depend on the refused header declaring a content namespace (OF-3425). */ @Test - public void testStreamWithoutNamespaceIsRefused() throws Exception + public void testClientStreamWithoutNamespaceIsRefusedWithError() throws Exception { - // Execute system under test. - handler.initiateSession("", reader); + // Execute system under test. The stream namespace is not the one RFC 6120 section 4.8.1 requires, so the header + // is refused; it also declares no content namespace, which is what exercises the error-building defect. + handler.initiateSession("", reader); // Verify result. - assertTrue(deliveredText().contains("", reader); + + // Verify result. + final String delivered = deliveredText(); + assertTrue(delivered.contains(" Date: Fri, 30 Oct 2026 12:47:14 +0000 Subject: [PATCH 22/22] OF-2643: Test the normalised-domain callers and the client host-validation default Mutation testing of this PR found three behaviours that no test caught, each a missing test rather than a bug: - ServerDialback.createIncomingSession() records the normalised domain returned by validateRemoteDomain() as the session local domain. Only the return value was tested, not that a caller uses it. - LocalIncomingServerSession.validateSubsequentDomain() reports success from that normalised domain; this second caller was untested. - ClientStanzaHandler.VALIDATE_HOST defaults to off, so upgrading a deployment that never set xmpp.client.validate.host does not start refusing client streams. Every test set the property explicitly, so none pinned the default. Each test fails against the corresponding mutation and passes against the current code. --- .../net/StanzaHandlerRefuseStreamTest.java | 10 ++ ...rverDialbackCreateIncomingSessionTest.java | 154 ++++++++++++++++++ ...mingServerSessionSubsequentDomainTest.java | 137 ++++++++++++++++ 3 files changed, 301 insertions(+) create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackCreateIncomingSessionTest.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/openfire/session/LocalIncomingServerSessionSubsequentDomainTest.java diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java index 19330e520e..cead7af8c2 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/StanzaHandlerRefuseStreamTest.java @@ -207,6 +207,16 @@ public void testStreamWithoutToIsRefusedFromAuthoritativeNameOrEmptyString() thr assertTrue(isOpenedFromAuthoritativeNameOrEmptyString(delivered), "The stream must be opened from an authoritative name of the local XMPP service, or from the empty string."); } + /** + * Asserts that client host validation is disabled unless it is configured, so that upgrading a deployment that has + * not set xmpp.client.validate.host does not start refusing client streams that address another domain. + */ + @Test + public void testClientHostValidationIsDisabledByDefault() + { + assertFalse(handler.validateHost(), "Client host validation must be disabled when it has not been configured."); + } + /** * Returns all text that was delivered to the peer, concatenated. * diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackCreateIncomingSessionTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackCreateIncomingSessionTest.java new file mode 100644 index 0000000000..9212a6486e --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/server/ServerDialbackCreateIncomingSessionTest.java @@ -0,0 +1,154 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.server; + +import org.dom4j.Document; +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.dom4j.Namespace; +import org.dom4j.QName; +import org.dom4j.io.XMPPPacketReader; +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.SessionManager; +import org.jivesoftware.openfire.StreamID; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.net.MXParser; +import org.jivesoftware.openfire.net.SASLAuthentication; +import org.jivesoftware.openfire.session.DomainPair; +import org.jivesoftware.openfire.session.LocalIncomingServerSession; +import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.openfire.spi.ConnectionConfiguration; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import java.security.cert.Certificate; +import java.util.List; +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.withSettings; + +/** + * Verifies that {@link ServerDialback#createIncomingSession(XMPPPacketReader)} records the normalised form of the + * validated domain as the local domain of the session it creates (OF-2643). + * + * Server Dialback validates the domain through {@link ServerDialback#validateRemoteDomain(Element, StreamID)}, which + * returns the domain as the local XMPP service knows it. This test checks that the created session's local domain is + * that returned value, rather than the form the peer wrote. The reader, the session factory, the remote-server + * permission check and the certificate check are replaced with stubs, as they parse a live stream, create sessions, + * read the database and read the peer's certificates, which are not what this test is about. + * + * @author Matthew Vivian + */ +public class ServerDialbackCreateIncomingSessionTest +{ + private static final String REMOTE_DOMAIN = "remote.example.org"; + + private SessionManager sessionManager; + + private Connection connection; + + private ServerDialback dialback; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + + @BeforeEach + public void setUp() + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + doReturn(Fixtures.mockRoutingTable()).when(xmppServer).getRoutingTable(); + sessionManager = xmppServer.getSessionManager(); + doReturn(List.of()).when(sessionManager).getIncomingServerSessions(REMOTE_DOMAIN); + XMPPServer.setInstance(xmppServer); + + final ConnectionConfiguration configuration = mock(ConnectionConfiguration.class, withSettings().lenient()); + doReturn(Connection.TLSPolicy.optional).when(configuration).getTlsPolicy(); + connection = mock(Connection.class, withSettings().lenient()); + doReturn(configuration).when(connection).getConfiguration(); + doReturn(new Certificate[0]).when(connection).getPeerCertificates(); + + dialback = new ServerDialback(connection, new DomainPair(Fixtures.XMPP_DOMAIN, REMOTE_DOMAIN)); + } + + /** + * Asserts that, when a peer opens a dialback session addressing the XMPP domain in a different case, the session + * that is created records the normalised domain as its local domain, rather than the form the peer wrote. + */ + @Test + public void testCreatedSessionRecordsNormalisedLocalDomain() throws Exception + { + // Setup test fixture: a reader that yields a db:result addressed to the XMPP domain in a different case. + final Document document = DocumentHelper.createDocument(); + final Element dbResult = document.addElement(QName.get("result", Namespace.get("db", "jabber:server:dialback"))); + dbResult.addAttribute("from", REMOTE_DOMAIN); + dbResult.addAttribute("to", Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)); + dbResult.setText("key"); + + final MXParser parser = mock(MXParser.class); + doReturn("jabber:server:dialback").when(parser).getNamespace("db"); + final XMPPPacketReader reader = mock(XMPPPacketReader.class); + doReturn(parser).when(reader).getXPPParser(); + doReturn(document).when(reader).parseDocument(); + + final StreamID streamID = new BasicStreamIDFactory().createStreamID(); + doReturn(streamID).when(sessionManager).nextStreamID(); + + final LocalIncomingServerSession createdSession = mock(LocalIncomingServerSession.class); + doReturn(createdSession).when(sessionManager).createIncomingServerSession(connection, streamID, REMOTE_DOMAIN); + + // Execute system under test. + final LocalIncomingServerSession result; + try (final MockedStatic remoteServers = mockStatic(RemoteServerManager.class); + final MockedStatic sasl = mockStatic(SASLAuthentication.class)) + { + remoteServers.when(() -> RemoteServerManager.canAccess(anyString())).thenReturn(true); + sasl.when(() -> SASLAuthentication.verifyCertificates(any(), anyString(), anyBoolean())).thenReturn(true); + + result = dialback.createIncomingSession(reader); + } + + // Verify result. + assertSame(createdSession, result, "The session that was created must be returned."); + verify(createdSession, times(1).description("The new session must record the normalised domain as its local domain.")).setLocalDomain(Fixtures.XMPP_DOMAIN); + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/session/LocalIncomingServerSessionSubsequentDomainTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/session/LocalIncomingServerSessionSubsequentDomainTest.java new file mode 100644 index 0000000000..c3eceede6d --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/session/LocalIncomingServerSessionSubsequentDomainTest.java @@ -0,0 +1,137 @@ +/* + * Copyright (C) 2026 Ignite Realtime Foundation. All rights reserved. + * + * 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.jivesoftware.openfire.session; + +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.jivesoftware.Fixtures; +import org.jivesoftware.openfire.Connection; +import org.jivesoftware.openfire.SessionManager; +import org.jivesoftware.openfire.XMPPServer; +import org.jivesoftware.openfire.net.SASLAuthentication; +import org.jivesoftware.openfire.server.RemoteServerManager; +import org.jivesoftware.openfire.spi.BasicStreamIDFactory; +import org.jivesoftware.openfire.spi.ConnectionConfiguration; +import org.jivesoftware.util.JiveGlobals; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.MockedStatic; + +import java.security.cert.Certificate; +import java.util.List; +import java.util.Locale; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.withSettings; + +/** + * Verifies that {@link LocalIncomingServerSession#validateSubsequentDomain(Element)} reports success to the peer from + * the normalised form of the validated domain (OF-2643). + * + * Server Dialback validates the domain through {@link org.jivesoftware.openfire.server.ServerDialback}, which returns + * the domain as the local XMPP service knows it. This test checks that the {@code } reply uses + * that returned value, rather than the form the peer wrote. The remote-server permission check and the certificate + * check are replaced with stubs, as they read the database and the peer's certificates, which are not what this test + * is about. + * + * @author Matthew Vivian + */ +public class LocalIncomingServerSessionSubsequentDomainTest +{ + private static final String REMOTE_DOMAIN = "remote.example.org"; + + private Connection connection; + + private LocalIncomingServerSession session; + + @BeforeAll + public static void setUpClass() throws Exception + { + Fixtures.reconfigureOpenfireHome(); + Fixtures.disableDatabasePersistence(); + } + + @AfterAll + public static void tearDownClass() + { + Fixtures.clearExistingProperties(); + } + + @BeforeEach + public void setUp() + { + Fixtures.clearExistingProperties(); + JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); + + final XMPPServer xmppServer = Fixtures.mockXMPPServer(); + doReturn(Fixtures.mockRoutingTable()).when(xmppServer).getRoutingTable(); + final SessionManager sessionManager = xmppServer.getSessionManager(); + doReturn(List.of()).when(sessionManager).getIncomingServerSessions(REMOTE_DOMAIN); + XMPPServer.setInstance(xmppServer); + + final ConnectionConfiguration configuration = mock(ConnectionConfiguration.class, withSettings().lenient()); + doReturn(Connection.TLSPolicy.optional).when(configuration).getTlsPolicy(); + connection = mock(Connection.class, withSettings().lenient()); + doReturn(configuration).when(connection).getConfiguration(); + doReturn(new Certificate[0]).when(connection).getPeerCertificates(); + + session = new LocalIncomingServerSession(Fixtures.XMPP_DOMAIN, connection, new BasicStreamIDFactory().createStreamID(), REMOTE_DOMAIN); + } + + /** + * Asserts that, when a peer addresses a subsequent domain in a different case from the XMPP domain, the reply that + * reports success carries the normalised domain in its 'from' attribute, rather than the form the peer wrote. + */ + @Test + public void testSubsequentDomainReplyReportsNormalisedDomain() + { + // Setup test fixture. + final Element dbResult = DocumentHelper.createElement("result"); + dbResult.addAttribute("from", REMOTE_DOMAIN); + dbResult.addAttribute("to", Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT)); + dbResult.setText("key"); + + // Execute system under test. + try (final MockedStatic remoteServers = mockStatic(RemoteServerManager.class); + final MockedStatic sasl = mockStatic(SASLAuthentication.class)) + { + remoteServers.when(() -> RemoteServerManager.canAccess(anyString())).thenReturn(true); + sasl.when(() -> SASLAuthentication.verifyCertificates(any(), anyString(), anyBoolean())).thenReturn(true); + + final boolean result = session.validateSubsequentDomain(dbResult); + assertTrue(result, "The subsequent domain must be validated successfully."); + } + + // Verify result. + final ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + verify(connection, atLeastOnce().description("A reply must have been delivered to the peer.")).deliverRawText(captor.capture()); + final String delivered = String.join("", captor.getAllValues()); + assertTrue(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN + "\""), "The reply must report success from the normalised domain."); + assertFalse(delivered.contains("from=\"" + Fixtures.XMPP_DOMAIN.toUpperCase(Locale.ROOT) + "\""), "The reply must not report success from the domain in the case the peer wrote."); + } +}