From 7f61fa52c1c26849b301d11e905cf6a815311d2b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 2 Sep 2026 10:40:58 +0200 Subject: [PATCH] OF-3359: Do not let unofferable FAST mechanisms shape the stream features The FAST mechanisms were carried into two calculations that cannot use them: the channel-binding types, which were derived from HT-*-UNIQ/ENDP/EXPR variants even when the FAST inline feature was not being advertised, and the emptiness check that suppresses a SASL1 mechanisms element, which counted mechanisms that a SASL1 element never renders. appendSASLFeatures now passes on only what is actually offered, and the element builder distinguishes the two profiles rather than re-deciding whether FAST is enabled. --- .../openfire/net/SaslStreamFeatures.java | 67 ++++----- .../openfire/net/SaslStreamFeaturesTest.java | 138 +++++++++++++++++- 2 files changed, 171 insertions(+), 34 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java index da97b59d79..4d39e530ae 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SaslStreamFeatures.java @@ -33,7 +33,7 @@ import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; -import java.util.Collections; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Set; @@ -73,20 +73,25 @@ private SaslStreamFeatures() { */ public static void appendSASLFeatures(@Nonnull final LocalSession session, @Nonnull final List features) { - final Set advertisableSASLMechanisms = SaslMechanismEligibility.getAdvertisableSASLMechanisms(session); - final Set fastMechanisms = advertisableSASLMechanisms.stream() - .filter(MechanismName::isFast).collect(Collectors.toUnmodifiableSet()); - final Set standardMechanisms = advertisableSASLMechanisms.stream() - .filter(mechanism -> !MechanismName.isFast(mechanism)).collect(Collectors.toUnmodifiableSet()); + final Set advertisable = SaslMechanismEligibility.getAdvertisableSASLMechanisms(session); + final Set standardMechanisms = advertisable.stream().filter(mechanism -> !MechanismName.isFast(mechanism)).collect(Collectors.toUnmodifiableSet()); + final boolean fastFeatureIsAdvertised = session instanceof ClientSession && SASLAuthentication.checkSASL2Permitted(session).isEmpty() && FastTokenManager.ENABLE_FAST.getValue(); + final Set fastMechanisms = fastFeatureIsAdvertised + ? advertisable.stream().filter(MechanismName::isFast).collect(Collectors.toUnmodifiableSet()) + : Set.of(); + SASLAuthentication.setAdvertisedSASLMechanisms(session, standardMechanisms); - final boolean fastFeatureIsAdvertised = session instanceof ClientSession - && SASLAuthentication.checkSASL2Permitted(session).isEmpty() && FastTokenManager.ENABLE_FAST.getValue(); - FastSessionState.setAdvertisedMechanisms(session, fastFeatureIsAdvertised ? fastMechanisms : Collections.emptySet()); + FastSessionState.setAdvertisedMechanisms(session, fastMechanisms); + + // Everything that is actually offered, and nothing that is not: a FAST mechanism that is filtered out here + // must not go on to contribute a channel-binding type or make a feature element look non-empty. + final Set offered = new HashSet<>(standardMechanisms); + offered.addAll(fastMechanisms); - final Set advertisableChannelBindingTypes = SaslMechanismEligibility.getAdvertisableChannelBindingTypes(session, advertisableSASLMechanisms); - SASLAuthentication.setAdvertisedChannelBindingTypes(session, advertisableChannelBindingTypes); + final Set channelBindingTypes = SaslMechanismEligibility.getAdvertisableChannelBindingTypes(session, offered); + SASLAuthentication.setAdvertisedChannelBindingTypes(session, channelBindingTypes); - features.addAll(asSASLMechanisms(session, advertisableSASLMechanisms, advertisableChannelBindingTypes)); + features.addAll(asSASLMechanisms(session, offered, channelBindingTypes)); } /** @@ -162,36 +167,32 @@ public static List asSASLMechanisms(@Nonnull final LocalSession session @VisibleForTesting static Element asSASLMechanismsElementForClientSessions(@Nonnull final Set advertisableMechanismNames, final boolean usingSASL2) { - final Namespace namespace = new Namespace("", usingSASL2 ? SASLAuthentication.SASL2_NAMESPACE : SASLAuthentication.SASL_NAMESPACE ); - final QName qName = new QName(usingSASL2 ? "authentication" : "mechanisms", namespace); - final Element result = DocumentHelper.createElement( qName ); + final Set fastMechanisms = advertisableMechanismNames.stream().filter(MechanismName::isFast).collect(Collectors.toSet()); + final Set standardMechanisms = advertisableMechanismNames.stream().filter(mechanism -> !MechanismName.isFast(mechanism)).collect(Collectors.toSet()); - for (final String mech : advertisableMechanismNames) { - if (MechanismName.isFast(mech)) continue; // FAST mechanisms live in the inline FAST feature. - final Element mechanism = result.addElement("mechanism"); - mechanism.setText(mech); + // FAST mechanisms live in the inline feature, which only SASL2 carries, so they cannot make a SASL1 element non-empty. + final boolean isEmpty = usingSASL2 ? advertisableMechanismNames.isEmpty() : standardMechanisms.isEmpty(); + if ((usingSASL2 || JiveGlobals.getBooleanProperty("sasl.client.suppressEmpty", false)) && isEmpty) { + return null; } - if ( usingSASL2 ) - { - Element inlineElement = result.addElement("inline"); + + final Namespace namespace = new Namespace("", usingSASL2 ? SASLAuthentication.SASL2_NAMESPACE : SASLAuthentication.SASL_NAMESPACE); + final Element result = DocumentHelper.createElement(new QName(usingSASL2 ? "authentication" : "mechanisms", namespace)); + for (final String mech : standardMechanisms) { + result.addElement("mechanism").setText(mech); + } + + if (usingSASL2) { + final Element inlineElement = result.addElement("inline"); if (StreamManager.isStreamManagementActive()) { inlineElement.add(StreamManager.sasl2InlineFeatureElement()); } inlineElement.add(Bind2Request.featureElement()); - - if (FastTokenManager.ENABLE_FAST.getValue()) { - final Set fastMechanisms = advertisableMechanismNames.stream() - .filter(MechanismName::isFast).collect(Collectors.toSet()); - if (!fastMechanisms.isEmpty()) inlineElement.add(FastTokenManager.featureElement(fastMechanisms)); + if (!fastMechanisms.isEmpty()) { + inlineElement.add(FastTokenManager.featureElement(fastMechanisms)); } } - - // OF-2072: Return null instead of an empty element, if so configured. - if ( (usingSASL2 || JiveGlobals.getBooleanProperty("sasl.client.suppressEmpty", false)) && advertisableMechanismNames.isEmpty() ) { - return null; - } - return result; } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java index 6ebe9be439..60a7a8ce20 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/net/SaslStreamFeaturesTest.java @@ -25,6 +25,7 @@ import org.jivesoftware.openfire.auth.AuthFactory; import org.jivesoftware.openfire.fast.FastSessionState; import org.jivesoftware.openfire.fast.FastTokenManager; +import org.jivesoftware.openfire.sasl.MechanismName; import org.jivesoftware.openfire.sasl.SaslMechanismCatalog; import org.jivesoftware.openfire.sasl.SaslMechanismEligibility; import org.jivesoftware.openfire.session.LocalClientSession; @@ -92,6 +93,7 @@ public void setup() JiveGlobals.setProperty("xmpp.domain", Fixtures.XMPP_DOMAIN); XMPPServer.setInstance(Fixtures.mockXMPPServer()); + FastTokenManager.ENABLE_FAST.setValue(FastTokenManager.ENABLE_FAST.getDefaultValue()); SaslMechanismCatalog.setEnabledMechanisms(Arrays.asList("PLAIN", "EXTERNAL")); } @@ -132,7 +134,6 @@ public void getSASLMechanismsElement_client_sasl1_suppressEmptyFalse_noMechanism @Test public void getSASLMechanismsElement_client_sasl1_suppressEmptyTrue_noMechanisms_returnsNull() { - FastTokenManager.ENABLE_FAST.setValue(false); // Setup test fixture: no mechanisms available (EXTERNAL requires encryption, PLAIN is removed). SaslMechanismCatalog.setEnabledMechanisms(Collections.singletonList("EXTERNAL")); JiveGlobals.setProperty("sasl.client.suppressEmpty", "true"); @@ -203,6 +204,141 @@ public void getSASLMechanismsElement_client_sasl2_suppressEmptyTrue_noMechanisms assertNull(result, "Expected null for SASL2 when no mechanisms are available, even when suppressEmpty is true."); } + /** + * A SASL1 mechanisms element that would carry nothing must be suppressed when configured to be, even when FAST + * mechanisms are eligible for the session. + * + * FAST mechanisms are rendered in the XEP-0484 inline feature, which only the SASL2 element carries, so they can + * never populate a SASL1 element. Counting them when deciding whether that element is empty leaves an empty + * on the wire despite sasl.client.suppressEmpty being set. + */ + @Test + public void getSASLMechanismsElement_client_sasl1_suppressEmptyTrue_onlyFastMechanisms_returnsNull() + { + try (final MockedStatic managers = mockStatic(ChannelBindingProviderManager.class)) + { + // Setup test fixture: no standard mechanism is eligible (EXTERNAL requires encryption), but the FAST + // mechanisms that need no channel binding are. + final ChannelBindingProviderManager manager = mock(ChannelBindingProviderManager.class); + managers.when(ChannelBindingProviderManager::getInstance).thenReturn(manager); + when(manager.getSupportedChannelBindingTypes()).thenReturn(Set.of()); + + FastTokenManager.ENABLE_FAST.setValue(true); + SaslMechanismCatalog.setEnabledMechanisms(Collections.singletonList("EXTERNAL")); + JiveGlobals.setProperty("sasl.client.suppressEmpty", "true"); + + final Connection connection = mock(Connection.class); + when(connection.isEncrypted()).thenReturn(false); + when(connection.getSupportedChannelBindingTypes()).thenReturn(Set.of()); + + final StreamID streamID = new BasicStreamIDFactory().createStreamID(); + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, connection, streamID, Locale.ENGLISH); + + final Set advertisableSASLMechanisms = SaslMechanismEligibility.getAdvertisableSASLMechanisms(session); + assertFalse(advertisableSASLMechanisms.isEmpty(), + "Test setup issue: expected the FAST mechanisms that need no channel binding to be eligible."); + assertTrue(advertisableSASLMechanisms.stream().allMatch(MechanismName::isFast), + "Test setup issue: expected no standard mechanism to be eligible, but found " + advertisableSASLMechanisms); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(advertisableSASLMechanisms, false); + + // Verify result. + assertNull(result, "A SASL1 element that can carry no mechanism must be suppressed, as FAST mechanisms are " + + "rendered in the SASL2 inline feature rather than here."); + } + } + + /** + * No channel-binding types are advertised on the strength of FAST mechanisms that are not themselves being + * offered. + * + * The XEP-0484 inline feature is carried only by the SASL2 element, so a session that is not offered SASL2 is not + * offered any FAST mechanism either. Deriving the XEP-0440 capability from those mechanisms announces a + * channel-binding type that nothing offered can use, and records it as advertised, which the XEP-0474 + * downgrade-protection hash is computed over. + */ + @Test + public void appendSASLFeatures_recordsNoChannelBindingTypes_whenOnlyFastMechanismsNeedThem() + { + try (final MockedStatic managers = mockStatic(ChannelBindingProviderManager.class)) + { + // Setup test fixture: an encrypted session that can supply tls-exporter, offered PLAIN and (were SASL2 + // available) the FAST variants that bind to it. SASL2 is disabled, so none of the latter can be offered. + final ChannelBindingProviderManager manager = mock(ChannelBindingProviderManager.class); + managers.when(ChannelBindingProviderManager::getInstance).thenReturn(manager); + when(manager.getSupportedChannelBindingTypes()).thenReturn(Set.of("tls-exporter")); + when(manager.supportsChannelBinding("tls-exporter")).thenReturn(true); + + FastTokenManager.ENABLE_FAST.setValue(true); + SASLAuthentication.ENABLE_SASL2.setValue(false); + SaslMechanismCatalog.setEnabledMechanisms(Collections.singletonList("PLAIN")); + + final Connection connection = mock(Connection.class); + when(connection.isEncrypted()).thenReturn(true); + when(connection.getSupportedChannelBindingTypes()).thenReturn(Set.of("tls-exporter")); + + final StreamID streamID = new BasicStreamIDFactory().createStreamID(); + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, connection, streamID, Locale.ENGLISH); + + assertTrue(SaslMechanismEligibility.getAdvertisableSASLMechanisms(session).stream() + .anyMatch(mechanism -> MechanismName.isFast(mechanism) && mechanism.endsWith("-EXPR")), + "Test setup issue: expected a channel-binding FAST variant to be eligible, so that suppressing it is " + + "what this test observes."); + + // Execute system under test. + final List features = new ArrayList<>(); + SaslStreamFeatures.appendSASLFeatures(session, features); + + // Verify result. + assertEquals(Set.of("PLAIN"), advertisedMechanismsIn(features), + "Only the standard mechanism can be offered when SASL2, and with it the FAST inline feature, is unavailable."); + assertFalse(features.stream().anyMatch(e -> "sasl-channel-binding".equals(e.getName())), + "No channel-binding capability may be announced when no mechanism that could use one was offered."); + assertEquals(Set.of(), SASLAuthentication.getAdvertisedChannelBindingTypes(session).orElseThrow(), + "No channel-binding types may be recorded as advertised when none were, or the XEP-0474 hash the " + + "server computes will not match the one the peer computes."); + assertEquals(Set.of(), FastSessionState.getAdvertisedMechanisms(session).orElseThrow(), + "No FAST mechanisms may be recorded as advertised when the inline feature carrying them was not."); + } + } + + /** + * A SASL2 element is still offered when only FAST mechanisms are eligible, since the XEP-0484 inline feature it + * carries is how those are advertised. + * + * The element has no children in that case, which is only useful to a client that already holds a + * token — but such a client can complete the negotiation, so suppressing the element would deny it a mechanism it + * can actually use. Contrast the SASL1 element, which cannot carry FAST mechanisms at all. + */ + @Test + public void getSASLMechanismsElement_client_sasl2_onlyFastMechanisms_returnsElementWithInlineFeature() + { + // Setup test fixture: no standard mechanism is eligible, but FAST is enabled. + FastTokenManager.ENABLE_FAST.setValue(true); + SaslMechanismCatalog.setEnabledMechanisms(Collections.singletonList("EXTERNAL")); + JiveGlobals.setProperty("sasl.client.suppressEmpty", "true"); + + final Connection connection = mock(Connection.class); + when(connection.isEncrypted()).thenReturn(false); + + final StreamID streamID = new BasicStreamIDFactory().createStreamID(); + final LocalClientSession session = new LocalClientSession(Fixtures.XMPP_DOMAIN, connection, streamID, Locale.ENGLISH); + + final Set advertisableSASLMechanisms = SaslMechanismEligibility.getAdvertisableSASLMechanisms(session); + assertTrue(advertisableSASLMechanisms.stream().allMatch(MechanismName::isFast), + "Test setup issue: expected only FAST mechanisms to be eligible, but found " + advertisableSASLMechanisms); + + // Execute system under test. + final Element result = SaslStreamFeatures.asSASLMechanismsElementForClientSessions(advertisableSASLMechanisms, true); + + // Verify result. + assertNotNull(result, "A SASL2 element must still be offered when the inline feature can carry a usable mechanism."); + assertTrue(result.elements("mechanism").isEmpty(), "No FAST mechanism may be rendered as a child."); + assertNotNull(result.element("inline").element(new QName("fast", Namespace.get("", FastTokenManager.NAMESPACE))), + "The FAST mechanisms must be advertised in the inline feature instead."); + } + // ------------------------------------------------------------------------- // Mechanism feature elements: inbound server sessions // -------------------------------------------------------------------------