From ca3d7c2ed5922a2ae4d9d7dd2e50c7fc40980d87 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 21 Apr 2026 21:00:00 +0200 Subject: [PATCH 01/13] OF-2694: Introduce pluggable channel binding providers with manager Add a channel binding abstraction consisting of: - ChannelBindingProvider interface for extracting channel binding data from an SSLEngine - ChannelBindingProviderManager to register, manage, and resolve providers per channel binding type - ChannelBindingType enum defining RFC-aligned binding identifiers - Comprehensive unit tests covering provider registration, ordering, resolution, and failure handling This introduces a best-effort mechanism to obtain channel binding data (as defined in RFC 5705, RFC 5929, and RFC 9266) without introducing hard dependencies on a specific TLS implementation or JDK version. The manager maintains an ordered list of providers per channel binding type and iterates through them until one successfully produces a value. Failures are isolated per provider, ensuring graceful degradation. Why this is needed Channel binding support is highly dependent on the capabilities of the underlying TLS stack and JDK version. Some mechanisms (notably newer exporter-based bindings) are difficult or effectively impossible to implement on older Java versions (e.g., Java 17), even when using reflection. At the same time, increasing the minimum required Java version for Openfire is undesirable, as it would negatively impact existing deployments. This change introduces a "manager/provider" pattern that: - Aligns with existing extensibility mechanisms used elsewhere in the codebase - Enables runtime discovery and prioritization of multiple implementations - Allows third parties to contribute additional providers via plugins Crucially, this design decouples channel binding support from the core runtime: - Core Openfire can remain compatible with Java 17 - Optional plugins can provide advanced implementations that depend on newer Java versions (e.g., Java 25+) These plugins can register their providers with the manager at runtime, seamlessly extending functionality without impacting baseline compatibility --- .../ChannelBindingProvider.java | 57 ++++ .../ChannelBindingProviderManager.java | 200 +++++++++++++ .../channelbinding/ChannelBindingType.java | 55 ++++ .../ChannelBindingProviderManagerTest.java | 267 ++++++++++++++++++ 4 files changed, 579 insertions(+) create mode 100644 xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProvider.java create mode 100644 xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java create mode 100644 xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingType.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProvider.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProvider.java new file mode 100644 index 0000000000..67f511f841 --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProvider.java @@ -0,0 +1,57 @@ +/* + * 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.util.channelbinding; + +import javax.annotation.Nonnull; +import javax.net.ssl.SSLEngine; +import java.util.Optional; + +/** + * Provides a mechanism to extract channel binding data of a specific type from an SSL engine. + * + * Implementations of this interface attempt to obtain the channel binding value as defined in relevant RFCs + * from a given SSL session, for the requested channel binding type (label). The availability and method of extraction + * may depend on the underlying TLS provider, JDK version, or presence of third-party libraries. + * + * @see RFC 5929: Channel Bindings for TLS + * @see RFC 5705: Keying Material Exporters for Transport Layer Security (TLS) + * @see RFC 9266: Channel Bindings for TLS 1.3 + */ +public interface ChannelBindingProvider +{ + /** + * Returns the RFC-defined unique prefix for the channel binding type this provider supports (e.g., "tls-exporter", + * "tls-server-end-point"). + * + * Note that these values are case-sensitive and must match exactly as defined in the respective RFCs. + * + * @return the channel binding type unique prefix (never null or empty) + */ + String getType(); + + /** + * Attempts to extract the channel binding data from the provided SSL session. + * + * The returned value, if present, is the channel binding data as specified by the RFC for this provider's type. + * If the session or provider does not support this operation, an empty Optional is returned. + * + * Callers should treat returned arrays as immutable. + * + * @param engine the SSL engine from which to extract channel binding data (must not be null) + * @return an Optional containing the channel binding data, or empty if unavailable or unsupported + */ + Optional getChannelBinding(@Nonnull final SSLEngine engine); +} diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java new file mode 100644 index 0000000000..8e0722e1f1 --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java @@ -0,0 +1,200 @@ +/* + * 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.util.channelbinding; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nonnull; +import javax.net.ssl.SSLEngine; +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; + +/** + * Manages a set of providers that can extract channel binding data of various types from SSL engines. + * + * This class offers a best-effort mechanism to obtain channel binding values as defined in RFC 5705, RFC 5929, RFC 9266, etc. + * It dynamically detects, at runtime, whether the underlying TLS implementation supports exporting keying material for the + * requested channel binding type, without requiring a hard dependency on any particular provider or JDK version. Providers + * are tried in order of registration until one succeeds or all fail. + * + * @author Guus der Kinderen, guus@goodbytes.nl + * @see RFC 5929: Channel Bindings for TLS + * @see RFC 5705: Keying Material Exporters for Transport Layer Security (TLS) + * @see RFC 9266: Channel Bindings for TLS 1.3 + */ +public class ChannelBindingProviderManager +{ + private static final Logger Log = LoggerFactory.getLogger(ChannelBindingProviderManager.class); + + private static final ChannelBindingProviderManager INSTANCE = new ChannelBindingProviderManager(); + + // Map from RFC-defined unique prefix to ordered list of providers for that type. + private final Map> providers = new ConcurrentHashMap<>(); + + /** + * Returns the singleton instance of the manager. + * + * @return the singleton ChannelBindingProviderManager instance + */ + public static ChannelBindingProviderManager getInstance() + { + return INSTANCE; + } + + /** + * Constructs a new manager instance. Intended primarily for testing; typical usage should prefer the singleton + * returned by getInstance(). + */ + @VisibleForTesting + ChannelBindingProviderManager() + { + } + + /** + * Registers a provider for its declared channel binding type (RFC-defined unique prefix) at the tail (end) of the + * list. Multiple providers can be registered for the same type; they are tried in registration order (head to tail). + * + * @param provider the provider to register + */ + public void addProvider(@Nonnull final ChannelBindingProvider provider) + { + Objects.requireNonNull(provider, "provider must not be null"); + final String prefix = provider.getType(); + Objects.requireNonNull(prefix, "provider's type must not be null"); + if (prefix.isEmpty()) { + throw new IllegalArgumentException("provider's type must not be empty"); + } + + Log.trace("Registering channel binding provider at tail: {} for prefix '{}'", provider.getClass().getName(), prefix); + providers.computeIfAbsent(prefix, k -> new CopyOnWriteArrayList<>()).add(provider); + } + + /** + * Registers a provider for its declared channel binding type (RFC-defined unique prefix) at the head (start) of the + * list. Multiple providers can be registered for the same type; they are tried in registration order (head to tail). + * + * @param provider the provider to register + */ + public void addProviderToHead(@Nonnull final ChannelBindingProvider provider) + { + Objects.requireNonNull(provider, "provider must not be null"); + final String prefix = provider.getType(); + Objects.requireNonNull(prefix, "provider's type must not be null"); + if (prefix.isEmpty()) { + throw new IllegalArgumentException("provider's type must not be empty"); + } + + Log.trace("Registering channel binding provider at head: {} for prefix '{}'", provider.getClass().getName(), prefix); + providers.computeIfAbsent(prefix, k -> new CopyOnWriteArrayList<>()).add(0, provider); + } + + /** + * Removes a specific provider instance for the given channel binding type prefix, if present. + * + * When multiple instances are registered, only the first instance is removed. + * + * @param provider the provider instance to remove + * @return if this manager contained the specified provider + */ + public boolean removeProvider(@Nonnull final ChannelBindingProvider provider) + { + Objects.requireNonNull(provider, "provider must not be null"); + final String prefix = provider.getType(); + Objects.requireNonNull(prefix, "provider's type must not be null"); + if (prefix.isEmpty()) { + throw new IllegalArgumentException("provider's type must not be empty"); + } + + Log.trace("Removing channel binding provider {} for prefix '{}'", provider.getClass().getName(), prefix); + final List list = providers.get(prefix); + boolean removed = false; + if (list != null) { + removed = list.remove(provider); + if (list.isEmpty()) { + providers.remove(prefix); + } + } + return removed; + } + + /** + * Attempts to obtain the channel binding data for the given type prefix and SSL engine by delegating to registered + * providers in order. Returns the first successful result, or an empty Optional if none succeed. + * + * @param cbPrefix the RFC-defined unique prefix for the channel binding type (must not be null or empty) + * @param engine the SSL engine from which to extract channel binding data + * @return an Optional containing the channel binding data, or empty if unavailable + */ + public Optional getChannelBinding(@Nonnull final String cbPrefix, @Nonnull final SSLEngine engine) + { + Objects.requireNonNull(cbPrefix, "cbPrefix must not be null"); + if (cbPrefix.isEmpty()) { + throw new IllegalArgumentException("type must not be empty"); + } + Objects.requireNonNull(engine, "engine must not be null"); + + Log.trace("Getting channel binding '{}' for engine: {}", cbPrefix, engine); + final List list = providers.get(cbPrefix); + if (list == null || list.isEmpty()) + { + Log.debug("No channel binding provider registered for prefix '{}'", cbPrefix); + return Optional.empty(); + } + for (final ChannelBindingProvider provider : list) { + try { + Log.trace("Trying provider: {}", provider.getClass().getName()); + final Optional channelBindingData = provider.getChannelBinding(engine); + if (channelBindingData.isPresent()) { + Log.debug("Channel binding '{}' found for engine: {} by provider {}", cbPrefix, engine, provider.getClass().getName()); + return channelBindingData; + } + } catch (Exception t) { + Log.warn("Provider '{}' failed to obtain channel binding '{}' for engine: {}", provider.getClass().getName(), cbPrefix, engine, t); + } + } + Log.debug("No channel binding '{}' found for engine: {}", cbPrefix, engine); + return Optional.empty(); + } + + /** + * Checks if there is at least one provider registered for the given channel binding type prefix. + * + * @param cbPrefix the RFC-defined unique prefix for the channel binding type (must not be null or empty) + * @return true if at least one provider is registered for the prefix, false otherwise + */ + public boolean supportsChannelBinding(@Nonnull final String cbPrefix) + { + Objects.requireNonNull(cbPrefix, "cbPrefix must not be null"); + if (cbPrefix.isEmpty()) { + throw new IllegalArgumentException("cbPrefix must not be empty"); + } + + return !providers.getOrDefault(cbPrefix, List.of()).isEmpty(); + } + + /** + * Returns an unmodifiable set of all supported channel binding type prefixes for which at least one provider is registered. + * + * @return a set of RFC-defined unique prefixes for supported channel binding types + */ + public Set getSupportedChannelBindingTypes() + { + return Collections.unmodifiableSet(providers.keySet()); + } +} diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingType.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingType.java new file mode 100644 index 0000000000..3c3498aa05 --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingType.java @@ -0,0 +1,55 @@ +/* + * 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.util.channelbinding; + +/** + * Enumerates supported channel binding types. + * + * @see RFC 5705: Keying Material Exporters for Transport Layer Security (TLS) + * @see RFC 5929: Channel Bindings for TLS + * @see RFC 9266: Channel Bindings for TLS 1.3 + */ +public enum ChannelBindingType +{ + /** + * tls-exporter: TLS exporter-based channel binding (RFC 5705, RFC 9266). + */ + TLS_EXPORTER("tls-exporter"), + + /** + * tls-server-end-point: server certificate hash channel binding (RFC 5929). + */ + TLS_SERVER_END_POINT("tls-server-end-point"), + + /** + * tls-unique: TLS Finished message channel binding (RFC 5929, deprecated). + */ + TLS_UNIQUE("tls-unique"); + + /** + * RFC-defined Channel-binding unique prefix + */ + private final String prefix; + + ChannelBindingType(String prefix) { + this.prefix = prefix; + } + + public String getPrefix() + { + return prefix; + } +} diff --git a/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java new file mode 100644 index 0000000000..8f34027b25 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java @@ -0,0 +1,267 @@ +/* + * 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.util.channelbinding; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import javax.net.ssl.SSLEngine; +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +/** + * Unit tests for {@link ChannelBindingProviderManager}. + */ +class ChannelBindingProviderManagerTest +{ + private ChannelBindingProviderManager manager; + + @BeforeEach + void setUp() + { + manager = new ChannelBindingProviderManager(); + } + + /** + * Should return the binding from the first provider that supplies it. + */ + @Test + void returnsBindingFromRegisteredProvider() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + final byte[] expected = new byte[] {1, 2, 3}; + when(provider.getType()).thenReturn("tls-exporter"); + when(provider.getChannelBinding(engine)).thenReturn(Optional.of(expected)); + manager.addProvider(provider); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertTrue(result.isPresent(), "Expected binding to be present when provider returns a value"); + assertArrayEquals(expected, result.get(), "Returned binding does not match expected value"); + } + + /** + * Should return empty when there are no providers. + */ + @Test + void returnsEmptyWhenNoProviders() + { + // Setup test fixture + final SSLEngine engine = mock(SSLEngine.class); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertFalse(result.isPresent(), "Expected empty result when no providers are registered"); + } + + /** + * Should return empty when the provider returns empty. + */ + @Test + void returnsEmptyWhenProviderReturnsEmpty() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(provider.getType()).thenReturn("tls-exporter"); + when(provider.getChannelBinding(engine)).thenReturn(Optional.empty()); + manager.addProvider(provider); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertFalse(result.isPresent(), "Expected empty result when provider returns empty"); + } + + /** + * Should return empty when the provider throws an exception. + */ + @Test + void returnsEmptyWhenProviderThrows() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(provider.getType()).thenReturn("tls-exporter"); + when(provider.getChannelBinding(engine)).thenThrow(new RuntimeException("fail")); + manager.addProvider(provider); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertFalse(result.isPresent(), "Expected empty result when provider throws"); + } + + /** + * Should remove a provider and no longer return its binding. + */ + @Test + void removeProviderRemovesIt() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(provider.getType()).thenReturn("tls-exporter"); + when(provider.getChannelBinding(engine)).thenReturn(Optional.of(new byte[] {1})); + manager.addProvider(provider); + assertTrue(manager.removeProvider(provider), "Provider should be removed successfully"); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertFalse(result.isPresent(), "Expected empty result after provider is removed"); + } + + /** + * Should return false when removing a provider that was never registered. + */ + @Test + void removeProviderReturnsFalseIfNotPresent() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + when(provider.getType()).thenReturn("tls-exporter"); + + // Execute system under test & verify result + assertFalse(manager.removeProvider(provider), "Should return false when provider was not registered"); + } + + /** + * Should use the provider added to the head before those added to the tail. + */ + @Test + void addProviderToHeadOverridesOrder() + { + // Setup test fixture + final ChannelBindingProvider tail = mock(ChannelBindingProvider.class); + final ChannelBindingProvider head = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(tail.getType()).thenReturn("tls-exporter"); + when(head.getType()).thenReturn("tls-exporter"); + when(tail.getChannelBinding(engine)).thenReturn(Optional.empty()); + final byte[] expected = new byte[] {9, 9, 9}; + when(head.getChannelBinding(engine)).thenReturn(Optional.of(expected)); + manager.addProvider(tail); + manager.addProviderToHead(head); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertTrue(result.isPresent(), "Expected binding to be present from head provider"); + assertArrayEquals(expected, result.get(), "Returned binding does not match expected value from head provider"); + } + + /** + * Should try multiple providers for the same prefix in order and return the first non-empty result. + */ + @Test + void multipleProvidersForSamePrefixAreTriedInOrder_emptyFirst() + { + // Setup test fixture + final ChannelBindingProvider first = mock(ChannelBindingProvider.class); + final ChannelBindingProvider second = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(first.getType()).thenReturn("tls-exporter"); + when(second.getType()).thenReturn("tls-exporter"); + when(first.getChannelBinding(engine)).thenReturn(Optional.empty()); + final byte[] expected = new byte[] {7, 7, 7}; + when(second.getChannelBinding(engine)).thenReturn(Optional.of(expected)); + manager.addProvider(first); + manager.addProvider(second); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertTrue(result.isPresent(), "Expected binding to be present from second provider"); + assertArrayEquals(expected, result.get(), "Returned binding does not match expected value from second provider"); + } + + /** + * Should try multiple providers for the same prefix in order and return the first non-empty result, skipping exceptions. + */ + @Test + void multipleProvidersForSamePrefixAreTriedInOrder_exceptionFirst() + { + // Setup test fixture + final ChannelBindingProvider first = mock(ChannelBindingProvider.class); + final ChannelBindingProvider second = mock(ChannelBindingProvider.class); + final SSLEngine engine = mock(SSLEngine.class); + when(first.getType()).thenReturn("tls-exporter"); + when(second.getType()).thenReturn("tls-exporter"); + when(first.getChannelBinding(engine)).thenThrow(new RuntimeException("fail")); + final byte[] expected = new byte[] {8, 8, 8}; + when(second.getChannelBinding(engine)).thenReturn(Optional.of(expected)); + manager.addProvider(first); + manager.addProvider(second); + + // Execute system under test + final Optional result = manager.getChannelBinding("tls-exporter", engine); + + // Verify result + assertTrue(result.isPresent(), "Expected binding to be present from second provider after exception in first"); + assertArrayEquals(expected, result.get(), "Returned binding does not match expected value from second provider"); + } + + /** + * Should reflect registered providers in supportsChannelBinding. + */ + @Test + void supportsChannelBindingReflectsRegisteredProviders() + { + // Setup test fixture + final ChannelBindingProvider provider = mock(ChannelBindingProvider.class); + when(provider.getType()).thenReturn("tls-exporter"); + assertFalse(manager.supportsChannelBinding("tls-exporter"), "Should not support prefix before registration"); + manager.addProvider(provider); + assertTrue(manager.supportsChannelBinding("tls-exporter"), "Should support prefix after registration"); + manager.removeProvider(provider); + assertFalse(manager.supportsChannelBinding("tls-exporter"), "Should not support prefix after removal"); + } + + /** + * Should reflect registered providers in getSupportedChannelBindingTypes. + */ + @Test + void getSupportedChannelBindingTypesReflectsRegisteredProviders() + { + // Setup test fixture + final ChannelBindingProvider exporter = mock(ChannelBindingProvider.class); + final ChannelBindingProvider serverEndPoint = mock(ChannelBindingProvider.class); + when(exporter.getType()).thenReturn("tls-exporter"); + when(serverEndPoint.getType()).thenReturn("tls-server-end-point"); + assertTrue(manager.getSupportedChannelBindingTypes().isEmpty(), "Should be empty before registration"); + manager.addProvider(exporter); + assertTrue(manager.getSupportedChannelBindingTypes().contains("tls-exporter"), "Should contain tls-exporter after registration"); + manager.addProvider(serverEndPoint); + assertTrue(manager.getSupportedChannelBindingTypes().contains("tls-server-end-point"), "Should contain tls-server-end-point after registration"); + manager.removeProvider(exporter); + assertFalse(manager.getSupportedChannelBindingTypes().contains("tls-exporter"), "Should not contain tls-exporter after removal"); + assertTrue(manager.getSupportedChannelBindingTypes().contains("tls-server-end-point"), "Should still contain tls-server-end-point"); + } +} From d4bb4a9c31d1fc94f6f5a753fa48e16ec9cc5704 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Fri, 17 Apr 2026 19:57:00 +0200 Subject: [PATCH 02/13] OF-2694: Add tls-server-end-point channel binding provider implementation Introduce TlsServerEndPointChannelBindingProvider, an implementation of ChannelBindingProvider for the tls-server-end-point channel binding type as defined in RFC 5929. Implements logic to select the appropriate hash algorithm based on certificate signature algorithm, with special handling for RSASSA-PSS and weak hashes. --- ...sServerEndPointChannelBindingProvider.java | 216 ++++++++ ...verEndPointChannelBindingProviderTest.java | 481 ++++++++++++++++++ 2 files changed, 697 insertions(+) create mode 100644 xmppserver/src/main/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProvider.java create mode 100644 xmppserver/src/test/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProviderTest.java diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProvider.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProvider.java new file mode 100644 index 0000000000..ee110e8fbc --- /dev/null +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProvider.java @@ -0,0 +1,216 @@ +/* + * 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.util.channelbinding; + +import com.google.common.annotations.VisibleForTesting; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.Nonnull; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSession; +import java.security.AlgorithmParameters; +import java.security.MessageDigest; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.security.spec.PSSParameterSpec; +import java.util.*; +import java.util.regex.Pattern; + +/** + * Implementation of {@link ChannelBindingProvider} for the tls-server-end-point channel binding type (RFC 5929). + * + * This provider extracts channel binding data from a {@link SSLEngine}, using the hash of the server's certificate + * as specified by RFC 5929. The hash algorithm is chosen based on the certificate's signature algorithm. + * + * The channel binding data is always derived from the server certificate, regardless of which side computes it. + * + * @see RFC 5929: Channel Bindings for TLS + */ +public class TlsServerEndPointChannelBindingProvider implements ChannelBindingProvider +{ + private static final Logger Log = LoggerFactory.getLogger(TlsServerEndPointChannelBindingProvider.class); + private static final Set SERVER_END_POINT_WEAK_HASH_ALGORITHMS = Set.of("MD5", "SHA1", "SHA-1"); + private static final String SERVER_END_POINT_FALLBACK_HASH_ALGORITHM = "SHA-256"; + private static final Pattern ALGORITHM_NAME_PATTERN = Pattern.compile("^SHA(?!\\d+-)(\\d)"); + + @Override + public String getType() + { + return ChannelBindingType.TLS_SERVER_END_POINT.getPrefix(); + } + + /** + * Attempts to extract the channel binding data from the provided SSLEngine. This is typically the hash of the + * server's certificate. The hash algorithm is chosen based on the certificate's signature algorithm per + * RFC 5929 §4.1. + * + * The tls-server-end-point binding is always derived from the server certificate, regardless of which side computes + * it. To determine if the local entity is acting in server or client mode, the engine's #getUseClientMode() method + * is evaluated. + * + * @param engine the SSLEngine from which to extract channel binding data (must not be null) + * @return an Optional containing the channel binding data, or empty if unavailable or unsupported + */ + @Override + public Optional getChannelBinding(@Nonnull final SSLEngine engine) + { + Objects.requireNonNull(engine, "engine must not be null"); + final SSLSession session = engine.getSession(); + try + { + // This binding requires the server certificate to be used, which is the local certificate when we're the + // server, but the peer's certificate when we're establishing a connection to a remote server (e.g. s2s). + final Certificate[] certs; + try { + certs = engine.getUseClientMode() ? session.getPeerCertificates() : session.getLocalCertificates(); + } catch (SSLPeerUnverifiedException e) { + return Optional.empty(); + } + + // RFC 5929 specifies the end-entity certificate (first in chain) + if (certs == null || certs.length == 0 || !(certs[0] instanceof X509Certificate cert)) { + return Optional.empty(); + } + + final String hashAlg; + if ("1.2.840.113549.1.1.10".equals(cert.getSigAlgOID())) { // Use OID instead of name for PSS detection (more robust) + // RSASSA-PSS is effectively the only commonly used signature algorithm where the hash function is not + // encoded in the algorithm name; all other mainstream algorithms either include the hash in the name or + // (like Ed25519) use no hash at all, in which case returning "undefined" is correct per RFC 5929. + hashAlg = resolveServerEndPointHashAlgorithmPSS(cert); + } else { + hashAlg = resolveServerEndPointHashAlgorithm(cert.getSigAlgName()); + } + if (hashAlg == null) { + Log.debug("TLS server end point channel binding is undefined for signature algorithm '{}' for session: {}", cert.getSigAlgName(), session); + return Optional.empty(); + } + final MessageDigest md = MessageDigest.getInstance(hashAlg); + return Optional.of(md.digest(cert.getEncoded())); + } catch (final Exception e) { + Log.trace("Failed to compute TLS server end point channel binding for session: {}", session, e); + } + return Optional.empty(); + } + + /** + * Resolves the hash algorithm for a certificate using RSASSA-PSS signature algorithm. + * + * Extracts the hash algorithm from the PSS parameters of the certificate and normalizes it for use in TLS server + * end point channel binding. Returns null if the hash algorithm cannot be determined. + * + * @param cert the X509Certificate with RSASSA-PSS signature algorithm + * @return the normalized hash algorithm name, or null if unavailable + */ + @VisibleForTesting + String resolveServerEndPointHashAlgorithmPSS(final X509Certificate cert) + { + final String pssHash = extractPssHashAlgorithm(cert); + if (pssHash == null) { + return null; + } + final String normalized = normalizeServerEndPointHashAlgorithmName(pssHash); + return substituteWeakServerEndPointHashAlgorithm(normalized); + } + + /** + * Extracts the hash algorithm name from the RSASSA-PSS parameters of the given X509 certificate. + * + * @param cert the X509Certificate with RSASSA-PSS signature algorithm + * @return the hash algorithm name, or null if unavailable or parsing fails + */ + @VisibleForTesting + String extractPssHashAlgorithm(X509Certificate cert) + { + try { + final byte[] params = cert.getSigAlgParams(); + if (params == null) { + return null; + } + final AlgorithmParameters ap = AlgorithmParameters.getInstance("RSASSA-PSS"); + ap.init(params); + final PSSParameterSpec spec = ap.getParameterSpec(PSSParameterSpec.class); + return spec.getDigestAlgorithm(); + } catch (Exception e) { + Log.trace("Failed to parse RSASSA-PSS parameters", e); + return null; + } + } + + /** + * Resolves the hash algorithm to use for TLS server end point channel binding per RFC 5929 §4.1: + * + *
    + *
  • MD5 or SHA-1 -> SHA-256
    • + *
  • Any other single hash function -> that hash function
    • + *
  • No hash function or multiple hash functions -> undefined (returns null)
    • + *
+ * + * @param sigAlgName the certificate's signature algorithm name (e.g. {@code "SHA256withRSA"}) + * @return the JCA hash algorithm name to use, or {@code null} if channel binding is undefined + * @see RFC 5929: Channel Bindings for TLS + */ + @VisibleForTesting + String resolveServerEndPointHashAlgorithm(final String sigAlgName) + { + if (sigAlgName == null) { + return null; + } + final String[] parts = sigAlgName.split("(?i)with", 2); + if (parts.length != 2 || parts[0].isBlank() || parts[1].isBlank()) { + Log.trace("No hash function for signature algorithm '{}'", sigAlgName); + return null; + } + final String normalizedHash = normalizeServerEndPointHashAlgorithmName(parts[0].trim()); + final String result = substituteWeakServerEndPointHashAlgorithm(normalizedHash); + Log.trace("Hash function for signature algorithm '{}': '{}'", sigAlgName, result); + return result; + } + + /** + * Normalizes a hash algorithm name to its canonical JCA form for TLS server end point channel binding by inserting the dash that + * {@link java.security.cert.X509Certificate#getSigAlgName()} omits in SHA-2 family names. For example: + * {@code "SHA256"} becomes {@code "SHA-256"}, {@code "SHA512"} becomes {@code "SHA-512"}. Already-correct names + * ({@code "SHA-256"}, {@code "SHA3-256"}, {@code "MD5"}) are unchanged. + */ + @VisibleForTesting + String normalizeServerEndPointHashAlgorithmName(final String name) + { + return ALGORITHM_NAME_PATTERN + .matcher(name.toUpperCase(Locale.ROOT)) + .replaceAll("SHA-$1"); + } + + /** + * Substitutes weak hash algorithms (MD5, SHA-1) with SHA-256 for TLS server end point channel binding, per RFC 5929 §4.1. + * + * @param hashAlg the hash algorithm name + * @return SHA-256 if the input is a weak hash, otherwise the input hash algorithm + */ + @VisibleForTesting + String substituteWeakServerEndPointHashAlgorithm(final String hashAlg) + { + if (SERVER_END_POINT_WEAK_HASH_ALGORITHMS.contains(hashAlg)) { + return SERVER_END_POINT_FALLBACK_HASH_ALGORITHM; + } + return hashAlg; + } +} + + diff --git a/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProviderTest.java b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProviderTest.java new file mode 100644 index 0000000000..26092c9d35 --- /dev/null +++ b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/TlsServerEndPointChannelBindingProviderTest.java @@ -0,0 +1,481 @@ +/* + * 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.util.channelbinding; + +import org.junit.jupiter.api.Test; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSession; +import java.security.MessageDigest; +import java.security.cert.Certificate; +import java.security.cert.X509Certificate; +import java.util.Optional; +import java.security.spec.PSSParameterSpec; +import java.security.AlgorithmParameters; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +/** + * Unit tests for TLS Server End Point channel binding extraction. + * + * These tests verify the correct extraction, normalization, and resolution of channel binding data + * for the server end point type as defined in RFC 5929. + */ +public class TlsServerEndPointChannelBindingProviderTest +{ + // The provider under test + private final TlsServerEndPointChannelBindingProvider provider = new TlsServerEndPointChannelBindingProvider(); + + // ----------- Server-side (local certificates) tests ----------- + + /** + * Should return empty when no local certificates are present (server-side). + */ + @Test + void testServerSide_noLocalCertificates_returnsEmpty() + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + when(session.getLocalCertificates()).thenReturn(null); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(false); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertFalse(result.isPresent(), "Should be empty when no local certificates are present (server-side)"); + } + + /** + * Should return the expected digest when a valid X509 local certificate is present (server-side). + */ + @Test + void testServerSide_withLocalX509Certificate_returnsDigest() throws Exception + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + final X509Certificate cert = mock(X509Certificate.class); + when(cert.getSigAlgName()).thenReturn("SHA256withRSA"); + when(cert.getEncoded()).thenReturn(new byte[] {10, 20, 30}); + when(session.getLocalCertificates()).thenReturn(new Certificate[] { cert }); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(false); + final byte[] expected = MessageDigest.getInstance("SHA-256").digest(new byte[] {10, 20, 30}); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertTrue(result.isPresent(), "Should be present when X509 certificate is present (server-side)"); + assertArrayEquals(expected, result.get(), "Digest should match expected SHA-256 digest (server-side)"); + } + + /** + * Should return empty when the local certificate is not X509 (server-side). + */ + @Test + void testServerSide_localCertificateNotX509_returnsEmpty() + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + final Certificate cert = mock(Certificate.class); // Not X509 + when(session.getLocalCertificates()).thenReturn(new Certificate[] { cert }); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(false); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertFalse(result.isPresent(), "Should be empty when local certificate is not X509 (server-side)"); + } + + // ----------- Client-side (peer certificates) tests ----------- + + /** + * Should return empty when no peer certificates are present (client-side). + */ + @Test + void testClientSide_noPeerCertificates_returnsEmpty() throws SSLPeerUnverifiedException + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + when(session.getPeerCertificates()).thenReturn(null); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(true); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertFalse(result.isPresent(), "Should be empty when no peer certificates are present (client-side)"); + } + + /** + * Should return the expected digest when a valid X509 peer certificate is present (client-side). + */ + @Test + void testClientSide_withPeerX509Certificate_returnsDigest() throws Exception + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + final X509Certificate cert = mock(X509Certificate.class); + when(cert.getSigAlgName()).thenReturn("SHA256withRSA"); + when(cert.getEncoded()).thenReturn(new byte[] {40, 50, 60}); + when(session.getPeerCertificates()).thenReturn(new Certificate[] { cert }); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(true); + final byte[] expected = MessageDigest.getInstance("SHA-256").digest(new byte[] {40, 50, 60}); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertTrue(result.isPresent(), "Should be present when X509 peer certificate is present (client-side)"); + assertArrayEquals(expected, result.get(), "Digest should match expected SHA-256 digest (client-side)"); + } + + /** + * Should return empty when the peer certificate is not X509 (client-side). + */ + @Test + void testClientSide_peerCertificateNotX509_returnsEmpty() throws SSLPeerUnverifiedException + { + // Setup test fixture + final SSLSession session = mock(SSLSession.class); + final Certificate cert = mock(Certificate.class); // Not X509 + when(session.getPeerCertificates()).thenReturn(new Certificate[] { cert }); + final SSLEngine engine = mock(SSLEngine.class); + when(engine.getSession()).thenReturn(session); + when(engine.getUseClientMode()).thenReturn(true); + + // Execute system under test + final Optional result = provider.getChannelBinding(engine); + + // Verify results + assertFalse(result.isPresent(), "Should be empty when peer certificate is not X509 (client-side)"); + } + + /** + * Should return null if extractPssHashAlgorithm returns null. + */ + @Test + void testResolveServerEndPointHashAlgorithmPSS_null() + { + // Setup test fixture + final X509Certificate cert = mock(X509Certificate.class); + + // Execute system under test + final String result = provider.resolveServerEndPointHashAlgorithmPSS(cert); + + // Verify results + assertNull(result, "Should return null when extractPssHashAlgorithm returns null (e.g., null sigAlgParams)"); + } + + /** + * Verifies that extractPssHashAlgorithm returns null if sigAlgParams is null or if an exception is thrown. + */ + @Test + void testExtractPssHashAlgorithm_nullOrException() + { + // Setup test fixture + final X509Certificate certNull = mock(X509Certificate.class); + when(certNull.getSigAlgParams()).thenReturn(null); + final X509Certificate certException = mock(X509Certificate.class); + when(certException.getSigAlgParams()).thenThrow(new RuntimeException("fail")); + + // Execute system under test & Verify results + assertNull(provider.extractPssHashAlgorithm(certNull), "Should return null when sigAlgParams is null"); + assertNull(provider.extractPssHashAlgorithm(certException), "Should return null when sigAlgParams throws an exception"); + } + + /** + * Verifies that extractPssHashAlgorithm returns the correct hash algorithm when valid PSS parameters are present. + */ + @Test + void testExtractPssHashAlgorithm_happyFlow() throws Exception + { + // Setup test fixture + final X509Certificate cert = mock(X509Certificate.class); + final PSSParameterSpec pssSpec = new PSSParameterSpec("SHA-512", "MGF1", PSSParameterSpec.DEFAULT.getMGFParameters(), 32, 1); + final AlgorithmParameters ap = AlgorithmParameters.getInstance("RSASSA-PSS"); + ap.init(pssSpec); + final byte[] params = ap.getEncoded(); + when(cert.getSigAlgParams()).thenReturn(params); + + // Execute system under test + final String result = provider.extractPssHashAlgorithm(cert); + + // Verify results + assertEquals("SHA-512", result, "Should return the correct digest algorithm from PSS parameters"); + } + + /** + * Verifies normalization: "SHA256" becomes "SHA-256". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA256() + { + // Execute & Verify results + assertEquals("SHA-256", provider.normalizeServerEndPointHashAlgorithmName("SHA256"), "Should normalize 'SHA256' to 'SHA-256'"); + } + + /** + * Verifies normalization: "SHA-256" becomes "SHA-256". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA_256() + { + // Execute & Verify results + assertEquals("SHA-256", provider.normalizeServerEndPointHashAlgorithmName("SHA-256"), "Should normalize 'SHA-256' to 'SHA-256'"); + } + + /** + * Verifies normalization: "SHA512" becomes "SHA-512". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA512() + { + // Execute & Verify results + assertEquals("SHA-512", provider.normalizeServerEndPointHashAlgorithmName("SHA512"), "Should normalize 'SHA512' to 'SHA-512'"); + } + + /** + * Verifies normalization: "SHA3-256" becomes "SHA3-256". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA3_256() + { + // Execute & Verify results + assertEquals("SHA3-256", provider.normalizeServerEndPointHashAlgorithmName("SHA3-256"), "Should normalize 'SHA3-256' to 'SHA3-256'"); + } + + /** + * Verifies normalization: "MD5" becomes "MD5". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_MD5() + { + // Execute & Verify results + assertEquals("MD5", provider.normalizeServerEndPointHashAlgorithmName("MD5"), "Should normalize 'MD5' to 'MD5'"); + } + + /** + * Verifies normalization: "sha1" becomes "SHA-1". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_sha1() + { + // Execute & Verify results + assertEquals("SHA-1", provider.normalizeServerEndPointHashAlgorithmName("sha1"), "Should normalize 'sha1' to 'SHA-1'"); + } + + /** + * Verifies normalization: "SHA384" becomes "SHA-384". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA384() + { + // Execute & Verify results + assertEquals("SHA-384", provider.normalizeServerEndPointHashAlgorithmName("SHA384"), "Should normalize 'SHA384' to 'SHA-384'"); + } + + /** + * Verifies normalization: "SHA3-512" becomes "SHA3-512". + */ + @Test + void testNormalizeServerEndPointHashAlgorithmName_SHA3_512() + { + // Execute & Verify results + assertEquals("SHA3-512", provider.normalizeServerEndPointHashAlgorithmName("SHA3-512"), "Should normalize 'SHA3-512' to 'SHA3-512'"); + } + + // ----------- Hash algorithm resolution tests ----------- + + /** + * Verifies resolution: "SHA256withRSA" becomes "SHA-256". + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA256withRSA() + { + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("SHA256withRSA"), "Resolution of 'SHA256withRSA' did not yield 'SHA-256'."); + } + + /** + * Verifies resolution: "SHA512withECDSA" becomes "SHA-512". + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA512withECDSA() + { + assertEquals("SHA-512", provider.resolveServerEndPointHashAlgorithm("SHA512withECDSA"), "Resolution of 'SHA512withECDSA' did not yield 'SHA-512'."); + } + + /** + * Verifies resolution: "SHA1withRSA" becomes fallback "SHA-256". + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA1withRSA_fallback() + { + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("SHA1withRSA"), "Resolution of weak hash 'SHA1withRSA' did not yield fallback 'SHA-256'."); + } + + /** + * Verifies resolution: "MD5withRSA" becomes fallback "SHA-256". + */ + @Test + void testResolveServerEndPointHashAlgorithm_MD5withRSA_fallback() + { + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("MD5withRSA"), "Resolution of weak hash 'MD5withRSA' did not yield fallback 'SHA-256'."); + } + + /** + * Verifies resolution: "SHA3-256withECDSA" becomes "SHA3-256". + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA3_256withECDSA() + { + assertEquals("SHA3-256", provider.resolveServerEndPointHashAlgorithm("SHA3-256withECDSA"), "Resolution of 'SHA3-256withECDSA' did not yield 'SHA3-256'."); + } + + /** + * Verifies resolution: "RSASSA-PSS" becomes null (no 'with'). + */ + @Test + void testResolveServerEndPointHashAlgorithm_RSASSA_PSS_null() + { + assertNull(provider.resolveServerEndPointHashAlgorithm("RSASSA-PSS"), "Resolution of 'RSASSA-PSS' (no 'with') did not yield null as expected."); + } + + /** + * Verifies resolution: null becomes null. + */ + @Test + void testResolveServerEndPointHashAlgorithm_null_null() + { + assertNull(provider.resolveServerEndPointHashAlgorithm(null), "Resolution of null algorithm name did not yield null as expected."); + } + + /** + * Verifies resolution: "SHA256" becomes null (no 'with'). + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA256_null() + { + assertNull(provider.resolveServerEndPointHashAlgorithm("SHA256"), "Resolution of 'SHA256' (no 'with') did not yield null as expected."); + } + + /** + * Verifies resolution: "SHA256with" becomes null (missing key algorithm). + */ + @Test + void testResolveServerEndPointHashAlgorithm_SHA256with_null() + { + assertNull(provider.resolveServerEndPointHashAlgorithm("SHA256with"), "Resolution of 'SHA256with' (missing key algorithm) did not yield null as expected."); + } + + /** + * Verifies resolution: "withRSA" becomes null (missing hash algorithm). + */ + @Test + void testResolveServerEndPointHashAlgorithm_withRSA_null() + { + assertNull(provider.resolveServerEndPointHashAlgorithm("withRSA"), "Resolution of 'withRSA' (missing hash algorithm) did not yield null as expected."); + } + + /** + * Verifies that substituteWeakServerEndPointHashAlgorithm substitutes MD5 with SHA-256. + */ + @Test + void testSubstituteWeakServerEndPointHashAlgorithm_MD5() + { + assertEquals("SHA-256", provider.substituteWeakServerEndPointHashAlgorithm("MD5"), "MD5 should be substituted with SHA-256"); + } + + /** + * Verifies that substituteWeakServerEndPointHashAlgorithm substitutes SHA1 with SHA-256. + */ + @Test + void testSubstituteWeakServerEndPointHashAlgorithm_SHA1() + { + assertEquals("SHA-256", provider.substituteWeakServerEndPointHashAlgorithm("SHA1"), "SHA1 should be substituted with SHA-256"); + } + + /** + * Verifies that substituteWeakServerEndPointHashAlgorithm substitutes SHA-1 with SHA-256. + */ + @Test + void testSubstituteWeakServerEndPointHashAlgorithm_SHA_1() + { + assertEquals("SHA-256", provider.substituteWeakServerEndPointHashAlgorithm("SHA-1"), "SHA-1 should be substituted with SHA-256"); + } + + /** + * Verifies that substituteWeakServerEndPointHashAlgorithm does not substitute SHA-512. + */ + @Test + void testSubstituteWeakServerEndPointHashAlgorithm_SHA_512() + { + assertEquals("SHA-512", provider.substituteWeakServerEndPointHashAlgorithm("SHA-512"), "SHA-512 should not be substituted"); + } + + /** + * Verifies that substituteWeakServerEndPointHashAlgorithm does not substitute SHA3-256. + */ + @Test + void testSubstituteWeakServerEndPointHashAlgorithm_SHA3_256() + { + assertEquals("SHA3-256", provider.substituteWeakServerEndPointHashAlgorithm("SHA3-256"), "SHA3-256 should not be substituted"); + } + + /** + * Verifies that Ed25519 (no hash) returns null for channel binding hash algorithm. + */ + @Test + void testResolveServerEndPointHashAlgorithm_Ed25519_returnsNull() + { + assertNull(provider.resolveServerEndPointHashAlgorithm("Ed25519"), "Ed25519 should yield null (undefined) for channel binding hash algorithm"); + } + + /** + * Verifies that extractPssHashAlgorithm returns null if PSS parameters are garbage (parsing fails). + */ + @Test + void testExtractPssHashAlgorithm_garbageParams_returnsNull() + { + final X509Certificate cert = mock(X509Certificate.class); + // Provide invalid/garbage params + when(cert.getSigAlgParams()).thenReturn(new byte[] {1,2,3,4,5,6,7,8,9}); + assertNull(provider.extractPssHashAlgorithm(cert), "Should return null when PSS parameters are garbage"); + } + + /** + * Verifies that weird signature names/casing are handled correctly. + */ + @Test + void testResolveServerEndPointHashAlgorithm_weirdCasing() + { + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("SHA256WITHRSA"), "Should normalize and resolve weird casing 'SHA256WITHRSA'"); + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("sha256withrsa"), "Should normalize and resolve lower-case 'sha256withrsa'"); + assertEquals("SHA-256", provider.resolveServerEndPointHashAlgorithm("Sha256WithRsa"), "Should normalize and resolve mixed-case 'Sha256WithRsa'"); + } +} From daa7632d8d6ee1bc951a5714b5d874e3f7a6c846 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Thu, 16 Apr 2026 15:42:27 +0200 Subject: [PATCH 03/13] OF-2694: Add TLS channel binding primitive support Expose per-connection channel binding data retrieval for SASL. --- .../org/jivesoftware/openfire/Connection.java | 27 +++++++++++++++++++ .../openfire/nio/NettyConnection.java | 15 +++++++++++ 2 files changed, 42 insertions(+) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java b/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java index 20103f04eb..92325c81ed 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java @@ -136,6 +136,33 @@ public interface Connection extends Closeable { */ Certificate[] getPeerCertificates(); + /** + * Returns channel binding data for this connection, as defined by the provided type. + * + * Channel binding data is used to bind higher-level authentication to the underlying transport layer, improving + * security against man-in-the-middle attacks. + * + * The type, identified by a unique prefix that's typically defined in an RFC, determines which channel binding + * mechanism is used, such as: + *
    + *
  • tls-exporter: TLS exporter-based channel binding.
    • + *
  • tls-server-end-point: Uses the hash of the server certificate (RFC 5929).
    • + *
+ * + * Note that channel binding type prefixes are case-sensitive. + * + * If the connection is not encrypted, or the requested channel binding type is not available, returns {@link Optional#empty()}. + * + * @param cbPrefix the RFC-defined unique prefix for the channel binding type (must not be null) + * @return An Optional containing the channel binding data, or empty if not available. + * @see RFC 5705: Keying Material Exporters for Transport Layer Security (TLS) + * @see RFC 5929: Channel Bindings for TLS + * @see RFC 9266: Channel Bindings for TLS 1.3 + */ + default Optional getChannelBindingData(@Nonnull final String cbPrefix) { + return Optional.empty(); + } + /** * Keeps track if the other peer of this session presented a self-signed certificate. When * using self-signed certificate for server-2-server sessions then SASL EXTERNAL will not be diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java index d17504411a..cf2fa49f63 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java @@ -34,12 +34,15 @@ import org.jivesoftware.openfire.session.Session; import org.jivesoftware.openfire.spi.ConnectionConfiguration; import org.jivesoftware.openfire.spi.EncryptionArtifactFactory; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmpp.packet.Packet; import org.xmpp.packet.StreamError; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLPeerUnverifiedException; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -405,6 +408,18 @@ public boolean isCompressed() { return channelHandlerContext.channel().pipeline().get(JZlibDecoder.class) != null; } + @Override + public Optional getChannelBindingData(@Nonnull final String cbPrefix) + { + final SslHandler sslhandler = (SslHandler) channelHandlerContext.channel().pipeline().get(SSL_HANDLER_NAME); + if (sslhandler == null) { + return Optional.empty(); + } + + final SSLEngine engine = sslhandler.engine(); + return ChannelBindingProviderManager.getInstance().getChannelBinding(cbPrefix, engine); + } + @Override public String toString() { final SocketAddress peer = channelHandlerContext.channel().remoteAddress(); From c54238ece1b41a1172f8edb1b90bd97baaf7c6db Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 21 Apr 2026 21:30:06 +0200 Subject: [PATCH 04/13] OF-2694: Make available `tls-server-end-point` channel binding by default This registers the corresponding provider without any ceremony. Future modifications could introduce configurability, but as support for this channel binding is mandatory per XEP, hard-coding seems acceptable. --- .../util/channelbinding/ChannelBindingProviderManager.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java index 8e0722e1f1..ed556c1a4c 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java @@ -44,6 +44,10 @@ public class ChannelBindingProviderManager private static final ChannelBindingProviderManager INSTANCE = new ChannelBindingProviderManager(); + static { + INSTANCE.addProvider(new TlsServerEndPointChannelBindingProvider()); + } + // Map from RFC-defined unique prefix to ordered list of providers for that type. private final Map> providers = new ConcurrentHashMap<>(); From 7eb37c2ed078a58989cb34caad059bf6561ac3d7 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 22 Apr 2026 17:39:24 +0200 Subject: [PATCH 05/13] OF-2694: Add SCRAM-SHA-1-PLUS mechanism with channel binding support Introduces SCRAM-SHA-1-PLUS SASL mechanism, implementing RFC 5802 channel binding for TLS. Advertise SCRAM-SHA-1-PLUS by default, but only when channel binding is available and session is encrypted. --- .../main/resources/openfire_i18n.properties | 1 + .../resources/openfire_i18n_nl.properties | 5 +- .../openfire/net/SASLAuthentication.java | 18 +- .../openfire/sasl/SaslProvider.java | 4 +- .../openfire/sasl/SaslServerFactoryImpl.java | 8 +- .../openfire/sasl/ScramSha1SaslServer.java | 155 ++++++++++++++++-- .../sasl/ScramSha1SaslServerTest.java | 117 ++++++++++++- 7 files changed, 286 insertions(+), 22 deletions(-) diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index 0f1d18a94a..0d7cf825f3 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -1155,6 +1155,7 @@ reg.settings.description.SKEY=An S/KEY mechanism. reg.settings.description.CRAM-MD5=Simple challenge-response scheme based on HMAC-MD5. reg.settings.description.DIGEST-MD5=Challenge-response scheme based upon MD5. DIGEST-MD5 offered a data security layer. reg.settings.description.SCRAM-SHA-1=Salted challenge-response scheme based on SHA-1. +reg.settings.description.SCRAM-SHA-1-PLUS=Salted challenge-response scheme based on SHA-1 with channel binding. reg.settings.description.SCRAM=Challenge-response scheme based mechanism with channel binding support. reg.settings.description.NTLM=NT LAN Manager authentication mechanism. reg.settings.description.GSSAPI=Kerberos V5 authentication via the GSSAPI. GSSAPI offers a data-security layer. diff --git a/i18n/src/main/resources/openfire_i18n_nl.properties b/i18n/src/main/resources/openfire_i18n_nl.properties index 6691842312..9c318fc513 100644 --- a/i18n/src/main/resources/openfire_i18n_nl.properties +++ b/i18n/src/main/resources/openfire_i18n_nl.properties @@ -1071,8 +1071,9 @@ reg.settings.description.OTP=Eenmalig wachtwoord mechanisme. reg.settings.description.SKEY=Een S/KEY mechanisme. reg.settings.description.CRAM-MD5=Eenvoudig Challenge-response mechanisme gebaseed op HMAC-MD5. reg.settings.description.DIGEST-MD5=Challenge-response mechanisme gebaseed op MD5. DIGEST-MD5 biedt data-laag beveiliging. -reg.settings.description.SCRAM-SHA-1=Salted challenge-response mechanisme gebaseed op SHA-1. -reg.settings.description.SCRAM=Chellenge-response-gebaseerd mechanisme met channel-binding ondersteuning. +reg.settings.description.SCRAM-SHA-1=Salted challenge-response mechanisme gebaseerd op SHA-1. +reg.settings.description.SCRAM-SHA-1-PLUS=Salted challenge-response mechanisme (met channel binding) gebaseerd op SHA-1. +reg.settings.description.SCRAM=Challenge-response-gebaseerd mechanisme met channel-binding ondersteuning. reg.settings.description.NTLM=NT LAN Manager authenticatie mechanisme. reg.settings.description.GSSAPI=Kerberos V5 authenticatie via GSSAPI. GSSAPI biedt een data-laag beveiliging. reg.settings.description.EAP-AES128=GSS EAP authenticatie. diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index 88edb54fa6..b25f03164d 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -38,6 +38,7 @@ import org.jivesoftware.util.JiveGlobals; import org.jivesoftware.util.PropertyEventListener; import org.jivesoftware.util.SystemProperty; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -277,6 +278,12 @@ public static Element getSASLMechanismsElement( ClientSession session ) continue; // Do not offer EXTERNAL. } } + if (mech.endsWith("-PLUS")) { + // Channel binding would be a binding to TLS. + if (!session.isEncrypted()) { + continue; // Cannot bind to TLS when there's no TLS. + } + } final Element mechanism = result.addElement("mechanism"); mechanism.setText(mech); } @@ -639,6 +646,12 @@ public static Set getSupportedMechanisms() continue; } + if (mechanism.endsWith("-PLUS") && ChannelBindingProviderManager.getInstance().getSupportedChannelBindingTypes().isEmpty()) { + Log.trace( "Cannot support '{}' as there's no implementation available for channel binding.", mechanism ); + it.remove(); + continue; + } + switch ( mechanism ) { case "CRAM-MD5": // intended fall-through @@ -651,7 +664,8 @@ public static Set getSupportedMechanisms() } break; - case "SCRAM-SHA-1": + case "SCRAM-SHA-1": // intended fall-through + case "SCRAM-SHA-1-PLUS": if ( !AuthFactory.supportsScram() ) { Log.trace( "Cannot support '{}' as the AuthFactory that's in use does not support SCRAM.", mechanism ); @@ -728,7 +742,7 @@ public static Set getImplementedMechanisms() */ public static List getEnabledMechanisms() { - return JiveGlobals.getListProperty("sasl.mechs", Arrays.asList( "ANONYMOUS","PLAIN","DIGEST-MD5","CRAM-MD5","SCRAM-SHA-1","JIVE-SHAREDSECRET","GSSAPI","EXTERNAL" ) ); + return JiveGlobals.getListProperty("sasl.mechs", Arrays.asList( "ANONYMOUS","PLAIN","DIGEST-MD5","CRAM-MD5","SCRAM-SHA-1","SCRAM-SHA-1-PLUS","JIVE-SHAREDSECRET","GSSAPI","EXTERNAL" ) ); } /** diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslProvider.java b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslProvider.java index f2aad87ec5..f1300a7978 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslProvider.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2004-2008 Jive Software, 2017-2018 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2004-2008 Jive Software, 2017-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. @@ -31,7 +31,7 @@ public class SaslProvider extends Provider { */ public SaslProvider() { - super("JiveSoftware", 1.1, "JiveSoftware Openfire SASL provider v1.1" ); + super("JiveSoftware", 1.2, "JiveSoftware Openfire SASL provider v1.2" ); final SaslServerFactoryImpl serverFactory = new SaslServerFactoryImpl(); for ( final String name : serverFactory.getMechanismNames( null ) ) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslServerFactoryImpl.java b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslServerFactoryImpl.java index e149060700..25ce160bcd 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslServerFactoryImpl.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/SaslServerFactoryImpl.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2004-2008 Jive Software, 2017-2018 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2004-2008 Jive Software, 2017-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. @@ -54,6 +54,7 @@ public SaslServerFactoryImpl() allMechanisms.add( new Mechanism( "ANONYMOUS", true, true ) ); allMechanisms.add( new Mechanism( "PLAIN", false, true ) ); allMechanisms.add( new Mechanism( "SCRAM-SHA-1", false, false ) ); + allMechanisms.add( new Mechanism( "SCRAM-SHA-1-PLUS", false, false ) ); allMechanisms.add( new Mechanism( "JIVE-SHAREDSECRET", true, false ) ); allMechanisms.add( new Mechanism( "EXTERNAL", false, false ) ); } @@ -78,7 +79,10 @@ public SaslServer createSaslServer(String mechanism, String protocol, String ser return new SaslServerPlainImpl( protocol, serverName, props, cbh ); case "SCRAM-SHA-1": - return new ScramSha1SaslServer(); + return new ScramSha1SaslServer(false, props); + + case "SCRAM-SHA-1-PLUS": + return new ScramSha1SaslServer(true, props); case "ANONYMOUS": if ( !props.containsKey( LocalSession.class.getCanonicalName() ) ) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java index 4b38c57190..ac8ab7f999 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java @@ -20,6 +20,9 @@ import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; +import java.util.Arrays; +import java.util.Map; +import java.util.Optional; import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -36,9 +39,12 @@ import org.jivesoftware.openfire.auth.DefaultAuthProvider; import org.jivesoftware.openfire.auth.InternalUnauthenticatedException; import org.jivesoftware.openfire.auth.ScramUtils; +import org.jivesoftware.openfire.net.SASLAuthentication; +import org.jivesoftware.openfire.session.LocalSession; import org.jivesoftware.openfire.user.UserNotFoundException; import org.jivesoftware.util.StringUtils; import org.jivesoftware.util.SystemProperty; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -93,6 +99,8 @@ public synchronized static String getServerSecretForNonExistentUsers() return SERVER_SECRET_NONEXISTENT_USERS.getValue(); } + public static final String PROPNAME_CHANNELBINDINGTYPE = "channelbindingtype"; + public static final SystemProperty ITERATION_COUNT = SystemProperty.Builder.ofType(Integer.class) .setKey("sasl.scram-sha-1.iteration-count") .setDefaultValue(ScramUtils.DEFAULT_ITERATION_COUNT) @@ -103,20 +111,27 @@ public synchronized static String getServerSecretForNonExistentUsers() CLIENT_FIRST_MESSAGE = Pattern.compile("^(([pny])=?([^,]*),([^,]*),)(m?=?[^,]*,?n=([^,]*),r=([^,]*),?.*)$"), CLIENT_FINAL_MESSAGE = Pattern.compile("(c=([^,]*),r=([^,]*)),p=(.*)$"); + private final boolean isPlusMechanism; + private final Map props; private String username; private State state = State.INITIAL; private String nonce; private String serverFirstMessage; private String clientFirstMessageBare; - private SecureRandom random = new SecureRandom(); + private final SecureRandom random = new SecureRandom(); + private byte[] expectedChannelBindingPayloadInFinalClientMessage; + private String gs2CbindName; private enum State { INITIAL, IN_PROGRESS, COMPLETE; } - - public ScramSha1SaslServer() { + + public ScramSha1SaslServer(final boolean isPlusMechanism, final Map props) + { + this.isPlusMechanism = isPlusMechanism; + this.props = props; } /** @@ -126,7 +141,7 @@ public ScramSha1SaslServer() { */ @Override public String getMechanismName() { - return "SCRAM-SHA-1"; + return isPlusMechanism ? "SCRAM-SHA-1-PLUS" : "SCRAM-SHA-1"; } /** @@ -194,13 +209,68 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce if (!m.matches()) { throw new SaslException("Invalid first client message"); } -// String gs2Header = m.group(1); -// String gs2CbindFlag = m.group(2); -// String gs2CbindName = m.group(3); + String gs2Header = m.group(1); + String gs2CbindFlag = m.group(2); + gs2CbindName = m.group(3); // String authzId = m.group(4); clientFirstMessageBare = m.group(5); username = m.group(6); String clientNonce = m.group(7); + + if (username == null || username.isEmpty()) { + throw new SaslException("Invalid first client message: Username cannot be empty"); + } + if (clientNonce == null || clientNonce.isEmpty()) { + throw new SaslException("Invalid first client message: Client nonce cannot be empty"); + } + + if (isPlusMechanism) + { + // https://www.rfc-editor.org/rfc/rfc5802.html#section-6: If the flag is set to "y" and the server supports + // channel binding, the server MUST fail authentication. This is because if the client sets the channel binding + // flag to "y", then the client must have believed that the server did not support channel binding -- if the + // server did in fact support channel binding, then this is an indication that there has been a downgrade attack + // (e.g., an attacker changed the server's mechanism list to exclude the -PLUS suffixed SCRAM mechanism name(s)). + final boolean clientSupportsChannelBindingButThinksServerDoesNot = "y".equals(gs2CbindFlag); + final boolean serverSupportsChannelBinding = SASLAuthentication.getSupportedMechanisms().stream().anyMatch(mechanism -> mechanism.endsWith("-PLUS")); + if (clientSupportsChannelBindingButThinksServerDoesNot && serverSupportsChannelBinding) { + throw new SaslException("Client supports channel binding, but thinks the server does not (while it does). Rejecting authentication to prevent downgrade attack."); + } + + final boolean clientRequiresChannelBinding = "p".equals(gs2CbindFlag); + if (!clientRequiresChannelBinding) { + throw new SaslException("Channel binding required for -PLUS. Rejecting authentication."); + } + + if (!serverSupportsChannelBinding) { + throw new SaslException("Client requires channel binding, but server does not support channel binding. Rejecting authentication."); + } + + // https://www.rfc-editor.org/rfc/rfc5802.html#section-6: If the channel binding flag was "p" and the server + // does not support the indicated channel binding type, then the server MUST fail authentication. + if (gs2CbindName == null || gs2CbindName.isEmpty() || !ChannelBindingProviderManager.getInstance().supportsChannelBinding(gs2CbindName)) { + throw new SaslException("Client requires channel binding, but server does not support the indicated channel binding type '" + gs2CbindName + "'. Rejecting authentication."); + } + + // Prepare channel binding data. + final LocalSession session = (LocalSession) props.get(LocalSession.class.getCanonicalName()); + if (session == null || session.getConnection() == null) { + throw new SaslException("Local session not found in properties. Rejecting authentication."); + } + final Optional channelBindingData = session.getConnection().getChannelBindingData(gs2CbindName); + if (channelBindingData.isEmpty()) { + Log.debug("Unable to retrieve channel binding data for '{}'. Rejecting authentication.", gs2CbindName); + throw new SaslException("Unable to retrieve channel binding data for '" + gs2CbindName + "'. Rejecting authentication."); + } + + // In the final client message, we expect to find a combination of the gs2 header and channel binding data. + final byte[] gs2_header = extractRawGS2Header(response); // Using raw header to prevent any normalization issues that might pop up when using something like: gs2Header.getBytes(StandardCharsets.UTF_8); + final byte[] cb_data = channelBindingData.get(); + expectedChannelBindingPayloadInFinalClientMessage = new byte[gs2_header.length + cb_data.length]; + System.arraycopy(gs2_header, 0, expectedChannelBindingPayloadInFinalClientMessage, 0 , gs2_header.length); + System.arraycopy(cb_data, 0, expectedChannelBindingPayloadInFinalClientMessage, gs2_header.length, cb_data.length); + } + nonce = clientNonce + UUID.randomUUID().toString(); serverFirstMessage = String.format("r=%s,s=%s,i=%d", nonce, DatatypeConverter.printBase64Binary(getOrCreateSalt(username)), @@ -208,6 +278,41 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce return serverFirstMessage.getBytes(StandardCharsets.UTF_8); } + /** + * Extracts the raw GS2 header from a SCRAM client-first-message byte array. + * + * The GS2 header is defined in RFC 5802 as: + * 
+     * gs2-header = gs2-cbind-flag "," [authzid] ","
+     *
+ * and always terminates with a trailing comma. + * + * This method performs a byte-level scan of the input and returns a copy of the original byte array from index + * {@code 0} up to and including the second comma (i.e., the full GS2 header including its trailing comma). + * + * No character decoding or normalization is performed. This ensures that the returned GS2 header is byte-for- + * byte identical to the original input, which is required for correct SCRAM-SHA-1-PLUS channel binding validation. + * + * @param data the raw SCRAM client-first-message bytes + * @return a byte array containing the complete GS2 header including the trailing comma + * @throws SaslException if the input does not contain a valid GS2 header + */ + @VisibleForTesting + static byte[] extractRawGS2Header(final byte[] data) throws SaslException + { + // The GS2 header ends at the second comma. + int commaCount = 0; + for (int i = 0; i < data.length; i++) { + if (data[i] == ',') { + commaCount++; + if (commaCount == 2) { + return Arrays.copyOfRange(data, 0, i+1); // +1 to include the comma itself. + } + } + } + throw new SaslException("Invalid GS2 header format"); + } + /** * Final response returns the server signature. */ @@ -218,13 +323,37 @@ private byte[] generateServerFinalMessage(final byte[] response) throws SaslExce throw new SaslException("Invalid client final message"); } - String clientFinalMessageWithoutProof = m.group(1); -// String channelBinding = m.group(2); - String clientNonce = m.group(3); - String proof = m.group(4); + // client-final-message regex: (c=([^,]*),r=([^,]*)),p=(.*)$") + final String clientFinalMessageWithoutProof = m.group(1); // (c=([^,]*),r=([^,]*)) + final String channelBinding = m.group(2); // c=([^,]*) + final String clientNonce = m.group(3); // r=([^,]*) + final String proof = m.group(4); // p=(.*) + + if (proof == null || proof.isEmpty()) { + throw new SaslException("Invalid client final message: missing proof attribute"); + } + + if (isPlusMechanism && (channelBinding == null || channelBinding.isEmpty())) { + throw new SaslException("Invalid client final message: missing channel binding attribute"); + } + if (clientNonce == null || clientNonce.isEmpty()) { + throw new SaslException("Invalid client final message: missing nonce attribute"); + } + + // Verify nonce: RFC 5802 §5: must equal client_nonce (from initial client response) + server_nonce (from initial server response) if (!nonce.equals(clientNonce)) { // Constant-time operation is important for keys, not for public protocol values like nonces. - throw new SaslException("Client final message has incorrect nonce value"); + // Possible replay or tampering + throw new SaslException("Invalid client final message: incorrect nonce attribute value"); + } + + // Verify channel binding payload. + if (isPlusMechanism) + { + final byte[] decodedChannelBinding = DatatypeConverter.parseBase64Binary(channelBinding); + if (!Arrays.equals(expectedChannelBindingPayloadInFinalClientMessage, decodedChannelBinding)) { + throw new SaslException("Invalid client final message: channel binding payload does not match expected payload"); + } } try { @@ -325,6 +454,8 @@ public Object getNegotiatedProperty(String propName) { if (isComplete()) { if (propName.equals(Sasl.QOP)) { return "auth"; + } else if (isPlusMechanism && propName.equals(PROPNAME_CHANNELBINDINGTYPE)) { + return gs2CbindName; } else { return null; } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java index 083816e748..11cc504351 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2023-2024 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2023-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. @@ -32,7 +32,9 @@ import javax.xml.bind.DatatypeConverter; import java.nio.charset.StandardCharsets; import java.security.spec.KeySpec; +import java.util.Arrays; import java.util.Base64; +import java.util.HashMap; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -83,7 +85,7 @@ public void testSuccess() throws Exception authFactory.when(() -> AuthFactory.getServerKey(any())).thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("0fe09258b3ac852ba502cc62ba903eaacdbf7d31"))); // Setup test fixture: prepare initial client message. - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>()); final byte[] initialMessage = ("n,,n=user,r=" + hardCodedClientNonce).getBytes(StandardCharsets.UTF_8); // Execute system under test: getting the first server message. @@ -143,4 +145,115 @@ public void testSuccess() throws Exception fail("Authentication should not fail (but it did)"); } } + + /** + * Verifies GS2 header extraction when an authzid is present. + */ + @Test + void extractsGs2Header_withAuthzId() throws Exception + { + // Setup test fixture + final byte[] input = "p=tls,,n=user,r=abc123,rest".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + assertEquals("p=tls,,", new String(result, StandardCharsets.UTF_8)); + } + + /** + * Verifies GS2 header extraction when no authzid is present. + */ + @Test + void extractsGs2Header_withoutAuthzId() throws Exception + { + // Setup test fixture + final byte[] input = "n,,n=user,r=abc123,rest".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + assertEquals("n,,", new String(result, StandardCharsets.UTF_8)); + } + + /** + * Ensures the GS2 header includes a trailing comma as specified. + */ + @Test + void includesTrailingComma_exactlyAsSpecified() throws Exception + { + // Setup test fixture + final byte[] input = "p=tls,,n=user,r=abc123".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + assertEquals(',', result[result.length - 1], "GS2 header must end with a comma"); + } + + /** + * Ensures GS2 header extraction preserves the exact bytes, with no re-encoding. + */ + @Test + void preservesExactBytes_noReEncoding() throws Exception + { + // Setup test fixture + final byte[] input = "p=tls,,n=user,r=abc123".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + byte[] expected = Arrays.copyOfRange(input, 0, result.length); + assertArrayEquals(expected, result, "Must be exact prefix of original bytes"); + } + + /** + * Verifies that an exception is thrown when the GS2 header does not contain a second comma. + */ + @Test + void throwsException_whenNoSecondComma() + { + // Setup test fixture + final byte[] input = "p=tls,n=user".getBytes(StandardCharsets.UTF_8); + + // Execute System under test & Verify result + assertThrows(SaslException.class, () -> + ScramSha1SaslServer.extractRawGS2Header(input)); + } + + /** + * Verifies that the minimal valid GS2 header is handled correctly. + */ + @Test + void handlesMinimalValidGs2Header() throws Exception + { + // Setup test fixture + final byte[] input = "n,,rest".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + assertEquals("n,,", new String(result, StandardCharsets.UTF_8)); + } + + /** + * Ensures GS2 header extraction stops at the second comma only. + */ + @Test + void stopsAtSecondComma_only() throws Exception + { + // Setup test fixture + final byte[] input = "p=tls,,n=user,r=abc,extra,stuff".getBytes(StandardCharsets.UTF_8); + + // Execute system under test + final byte[] result = ScramSha1SaslServer.extractRawGS2Header(input); + + // Verify result + assertEquals("p=tls,,", new String(result, StandardCharsets.UTF_8)); + } } From e71c12200edb796c1a7400d5fa4d22bce588e4ca Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 22 Apr 2026 18:10:31 +0200 Subject: [PATCH 06/13] OF-2879: Implement XEP-0440: SASL Channel-Binding Type Capability With Channel Binding now available (as this commit follows commits that implements these under OF-2694), Openfire should announce what type of channel binding types it supports. The mechanism for this is define in XEP-0440. This commit implements this mechanism. Additionally, the channel binding type that is used for a session (if any) is now shown on its 'session details' page on the admin console. --- documentation/openfire.doap | 7 ++ documentation/protocol-support.html | 1 + .../main/resources/openfire_i18n.properties | 1 + .../openfire/net/SASLAuthentication.java | 3 + .../openfire/net/SocketReadingMode.java | 5 +- .../openfire/net/StanzaHandler.java | 5 +- .../openfire/session/LocalClientSession.java | 2 + .../session/LocalIncomingServerSession.java | 4 +- .../WebSocketClientStanzaHandler.java | 4 +- .../ChannelBindingProviderManager.java | 33 +++++++++ .../src/main/webapp/session-details.jsp | 12 +++- .../ChannelBindingProviderManagerTest.java | 69 +++++++++++++++++++ 12 files changed, 141 insertions(+), 5 deletions(-) diff --git a/documentation/openfire.doap b/documentation/openfire.doap index c912769ca8..09363e735b 100644 --- a/documentation/openfire.doap +++ b/documentation/openfire.doap @@ -441,6 +441,13 @@ As Openfire does not support MIX, the 'service types' search form field is not supported by the implementation. All searches cover MUC only, as per the specification. + + + + full + 1.0.0 + + diff --git a/documentation/protocol-support.html b/documentation/protocol-support.html index b69f868e5a..1b60263f56 100644 --- a/documentation/protocol-support.html +++ b/documentation/protocol-support.html @@ -399,6 +399,7 @@

List of other XEPs Supported

XEP-0359: Unique and Stable Stanza IDs XEP-0398: User Avatar to vCard-Based Avatars Conversion XEP-0433: Extended Channel Search + XEP-0440: SASL Channel-Binding Type Capability XEP-0478: Stream Limits Advertisement XEP-0483: HTTP Online Meetings [19] XEP-0485: PubSub Server Information [18] diff --git a/i18n/src/main/resources/openfire_i18n.properties b/i18n/src/main/resources/openfire_i18n.properties index 0d7cf825f3..08074fe39f 100644 --- a/i18n/src/main/resources/openfire_i18n.properties +++ b/i18n/src/main/resources/openfire_i18n.properties @@ -1856,6 +1856,7 @@ session.details.anon-status=Using Anonymous Authentication session.details.anon-true=Yes session.details.anon-false=No session.details.sasl-mechanism=SASL Mechanism +session.details.channel-binding-type=Channel Binding Type session.details.flomr-status=Flexible Offline Message Retrieval session.details.flomr-enabled=Enabled session.details.flomr-disabled=Disabled diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index b25f03164d..84d1197c6c 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -461,6 +461,9 @@ else if (encoded.equals("=")) authenticationSuccessful( session, saslServer.getAuthorizationID(), challenge ); session.removeSessionData( "SaslServer" ); session.setSessionData("SaslMechanism", saslServer.getMechanismName()); + if (saslServer.getMechanismName().endsWith("-PLUS")) { + session.setSessionData("ChannelBindingType", saslServer.getNegotiatedProperty(ScramSha1SaslServer.PROPNAME_CHANNELBINDINGTYPE)); + } return Status.authenticated; default: diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SocketReadingMode.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SocketReadingMode.java index 2ac63210a9..8014673f3a 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SocketReadingMode.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SocketReadingMode.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2008 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2005-2008 Jive Software, 2017-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. @@ -20,6 +20,7 @@ import org.jivesoftware.openfire.Connection; import org.jivesoftware.openfire.session.Session; import org.jivesoftware.util.StringUtils; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParserException; @@ -108,6 +109,7 @@ protected void tlsNegotiated() throws XmlPullParserException, IOException final Element features = DocumentHelper.createElement(QName.get("features", "stream", "http://etherx.jabber.org/streams")); final Element mechanisms = SASLAuthentication.getSASLMechanisms(socketReader.session); if (mechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(mechanisms).ifPresent(features::add); features.add(mechanisms); } final List specificFeatures = socketReader.session.getAvailableStreamFeatures(); @@ -253,6 +255,7 @@ protected void compressionSuccessful() throws XmlPullParserException, IOExceptio // Include available SASL Mechanisms final Element saslMechanisms = SASLAuthentication.getSASLMechanisms(socketReader.session); if (saslMechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(saslMechanisms).ifPresent(features::add); features.add(saslMechanisms); } } 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 51d51162ec..e7e73e4743 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -29,6 +29,7 @@ import org.jivesoftware.openfire.spi.BasicStreamIDFactory; import org.jivesoftware.openfire.streammanagement.StreamManager; import org.jivesoftware.util.*; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParser; @@ -494,7 +495,8 @@ protected void tlsNegotiated(XmlPullParser xpp) throws XmlPullParserException, I // Include available SASL Mechanisms final Element mechanismsElement=SASLAuthentication.getSASLMechanisms(session); if (mechanismsElement!=null) { - features.add(mechanismsElement); + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(mechanismsElement).ifPresent(features::add); + features.add(mechanismsElement); } // Include specific features such as auth and register for client sessions @@ -597,6 +599,7 @@ protected void compressionSuccessful() { if (!session.isAuthenticated()) { final Element saslMechanisms = SASLAuthentication.getSASLMechanisms(session); if (saslMechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(saslMechanisms).ifPresent(features::add); features.add(saslMechanisms); } } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java index fb97cdbe5a..9c854d4e62 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalClientSession.java @@ -40,6 +40,7 @@ import org.jivesoftware.util.LocaleUtils; import org.jivesoftware.util.StringUtils; import org.jivesoftware.util.cache.Cache; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParser; @@ -294,6 +295,7 @@ public static LocalClientSession createSession(String serverName, XmlPullParser // Include available SASL Mechanisms final Element saslMechanisms = SASLAuthentication.getSASLMechanisms(session); if (saslMechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(saslMechanisms).ifPresent(features::add); features.add(saslMechanisms); } // Include Stream features 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 b80ab56466..bf24bbaa7d 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/session/LocalIncomingServerSession.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2005-2008 Jive Software, 2016-2025 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2005-2008 Jive Software, 2016-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. @@ -29,6 +29,7 @@ import org.jivesoftware.util.CertificateManager; import org.jivesoftware.util.StreamErrorException; import org.jivesoftware.util.StringUtils; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParser; @@ -189,6 +190,7 @@ public static LocalIncomingServerSession createSession(String serverName, XmlPul // Include available SASL Mechanisms final Element saslMechanisms = SASLAuthentication.getSASLMechanisms(session); if (saslMechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(saslMechanisms).ifPresent(features::add); features.add(saslMechanisms); } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java index 68ee0039ba..f2c8fc7c56 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2023 Ignite Realtime Foundation. All rights reserved. + * Copyright (C) 2023-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. @@ -29,6 +29,7 @@ import org.jivesoftware.openfire.session.LocalClientSession; import org.jivesoftware.openfire.session.Session; import org.jivesoftware.util.StreamErrorException; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xmlpull.v1.XmlPullParser; @@ -166,6 +167,7 @@ private void sendStreamFeatures() { // Include available SASL Mechanisms final Element saslMechanisms = SASLAuthentication.getSASLMechanisms(session); if (saslMechanisms != null) { + ChannelBindingProviderManager.getInstance().getSASLChannelBindingTypeCapabilityElement(saslMechanisms).ifPresent(features::add); features.add(saslMechanisms); } } diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java index ed556c1a4c..145abd9c30 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java @@ -16,6 +16,10 @@ package org.jivesoftware.util.channelbinding; import com.google.common.annotations.VisibleForTesting; +import org.dom4j.DocumentHelper; +import org.dom4j.Element; +import org.dom4j.Namespace; +import org.dom4j.QName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -201,4 +205,33 @@ public Set getSupportedChannelBindingTypes() { return Collections.unmodifiableSet(providers.keySet()); } + + /** + * Returns an XML element that describes the supported SASL channel binding types, if applicable. + * + * This method inspects the provided SASL mechanisms element. If at least one mechanism ends with "-PLUS" + * and the server supports one or more channel binding types, it returns an element that advertises these types. + * Otherwise, it returns an empty Optional. + * + * @param saslMechanisms The XML element containing SASL mechanisms to inspect. + * @return An Optional containing the capability element if channel binding types should be advertised, or empty otherwise. + * @see XEP-0440: SASL Channel-Binding Type Capability + */ + public Optional getSASLChannelBindingTypeCapabilityElement(@Nonnull final Element saslMechanisms) + { + if (saslMechanisms.elements("mechanism").stream().noneMatch(mech -> mech.getText().endsWith("-PLUS"))) { + return Optional.empty(); + } + + final Set supportedChannelBindingTypes = this.getSupportedChannelBindingTypes(); + if (supportedChannelBindingTypes.isEmpty()) { + return Optional.empty(); + } + + final Element result = DocumentHelper.createElement(new QName("sasl-channel-binding", new Namespace("", "urn:xmpp:sasl-cb:0"))); + for (final String channelBindingType : supportedChannelBindingTypes) { + result.addElement("channel-binding").addAttribute("type", channelBindingType); + } + return Optional.of(result); + } } diff --git a/xmppserver/src/main/webapp/session-details.jsp b/xmppserver/src/main/webapp/session-details.jsp index e24ba039e4..e452d0f772 100644 --- a/xmppserver/src/main/webapp/session-details.jsp +++ b/xmppserver/src/main/webapp/session-details.jsp @@ -1,7 +1,7 @@ <%@ page contentType="text/html; charset=UTF-8" %> <%-- - - - Copyright (C) 2004-2008 Jive Software, 2017-2025 Ignite Realtime Foundation. All rights reserved. + - Copyright (C) 2004-2008 Jive Software, 2017-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. @@ -460,6 +460,16 @@ <% } %> + <% if (currentSess instanceof LocalSession && ((LocalSession) currentSess).getSessionData("ChannelBindingType") != null) { %> + + + : + + + <%=StringUtils.escapeHTMLTags(((LocalSession) currentSess).getSessionData("ChannelBindingType").toString())%> + + + <% } %> diff --git a/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java index 8f34027b25..97c8e80118 100644 --- a/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManagerTest.java @@ -15,6 +15,8 @@ */ package org.jivesoftware.util.channelbinding; +import org.dom4j.DocumentHelper; +import org.dom4j.Element; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -264,4 +266,71 @@ void getSupportedChannelBindingTypesReflectsRegisteredProviders() assertFalse(manager.getSupportedChannelBindingTypes().contains("tls-exporter"), "Should not contain tls-exporter after removal"); assertTrue(manager.getSupportedChannelBindingTypes().contains("tls-server-end-point"), "Should still contain tls-server-end-point"); } + + /** + * Test when no mechanism ends with -PLUS, the result of #getSASLChannelBindingTypeCapabilityElement should be empty. + */ + @Test + void testNoPlusMechanismReturnsEmpty() + { + // Setup test fixture + final ChannelBindingProviderManager mgr = new ChannelBindingProviderManager(); + final Element mechanisms = DocumentHelper.createElement("mechanisms"); + mechanisms.addElement("mechanism").setText("PLAIN"); + + // Execute system under test + final Optional result = mgr.getSASLChannelBindingTypeCapabilityElement(mechanisms); + + // Verify result + assertTrue(result.isEmpty(), "Should return empty when no -PLUS mechanism is present"); + } + + /** + * Test when a -PLUS mechanism is present but no supported channel binding types, the result of + * #getSASLChannelBindingTypeCapabilityElement should be empty. + */ + @Test + void testPlusMechanismNoChannelBindingTypesReturnsEmpty() + { + // Setup test fixture + final ChannelBindingProviderManager mgr = new ChannelBindingProviderManager(); + final Element mechanisms = DocumentHelper.createElement("mechanisms"); + mechanisms.addElement("mechanism").setText("SCRAM-SHA-1-PLUS"); + + // Execute system under test + Optional result = mgr.getSASLChannelBindingTypeCapabilityElement(mechanisms); + + // Verify result + assertTrue(result.isEmpty(), "Should return empty when no channel binding types are supported"); + } + + /** + * Test when a -PLUS mechanism and supported channel binding types are present, the result of + * #getSASLChannelBindingTypeCapabilityElement should contain the expected element. + */ + @Test + void testPlusMechanismWithChannelBindingTypesReturnsElement() + { + // Setup test fixture + final ChannelBindingProviderManager mgr = new ChannelBindingProviderManager(); + final Element mechanisms = DocumentHelper.createElement("mechanisms"); + mechanisms.addElement("mechanism").setText("SCRAM-SHA-1-PLUS"); + // Register mock providers for two channel binding types + final ChannelBindingProvider cbp1 = mock(ChannelBindingProvider.class); + when(cbp1.getType()).thenReturn("tls-server-end-point"); + final ChannelBindingProvider cbp2 = mock(ChannelBindingProvider.class); + when(cbp2.getType()).thenReturn("tls-exporter"); + mgr.addProvider(cbp1); + mgr.addProvider(cbp2); + + // Execute system under test + final Optional result = mgr.getSASLChannelBindingTypeCapabilityElement(mechanisms); + + // Verify result + assertTrue(result.isPresent(), "Should return element when -PLUS mechanism and channel binding types are present"); + final Element cbElement = result.get(); + assertEquals("sasl-channel-binding", cbElement.getName(), "Element name should be sasl-channel-binding"); + assertEquals("urn:xmpp:sasl-cb:0", cbElement.getNamespaceURI(), "Namespace should be urn:xmpp:sasl-cb:0"); + assertEquals(2, cbElement.elements("channel-binding").size(), "Should have two channel-binding elements"); + } } From 0ba53838ad63c6cd415b95f8c705a8740b7ef1b2 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 22 Apr 2026 19:47:41 +0200 Subject: [PATCH 07/13] OF-2694 (code review feedback): Additional hardening + unit test coverage This adds more validation of the exchanged data in the SASL handshake, and adds more unit test coverage. Small refactorings have been applied to make ScramSha1SaslServer easier to unit-test. It now no longer unconditionally uses static calls to other classes. Such classes can be provided via a `@VisibleForTesting` constructor. --- .../openfire/sasl/ScramSha1SaslServer.java | 80 ++- .../ChannelBindingProviderManager.java | 2 +- .../sasl/ScramSha1SaslServerFakeKeyTest.java | 13 +- .../sasl/ScramSha1SaslServerSaltTest.java | 11 +- .../sasl/ScramSha1SaslServerTest.java | 616 +++++++++++++++++- 5 files changed, 685 insertions(+), 37 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java index ac8ab7f999..3e6594ad71 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -49,9 +50,9 @@ import org.slf4j.LoggerFactory; /** - * Implements the SCRAM-SHA-1 server-side mechanism. + * Implements the SCRAM-SHA-1 (and it's channel binding -PLUS variant) server-side mechanism. * - * @author Richard Midwinter + * @author Richard Midwinter, Guus der Kinderen */ public class ScramSha1SaslServer implements SaslServer { @@ -111,6 +112,9 @@ public synchronized static String getServerSecretForNonExistentUsers() CLIENT_FIRST_MESSAGE = Pattern.compile("^(([pny])=?([^,]*),([^,]*),)(m?=?[^,]*,?n=([^,]*),r=([^,]*),?.*)$"), CLIENT_FINAL_MESSAGE = Pattern.compile("(c=([^,]*),r=([^,]*)),p=(.*)$"); + private final ChannelBindingProviderManager channelBindingProviderManager; + private final Set serverSupportedSaslMechanismNames; + private final boolean isPlusMechanism; private final Map props; private String username; @@ -132,6 +136,20 @@ public ScramSha1SaslServer(final boolean isPlusMechanism, final Map p { this.isPlusMechanism = isPlusMechanism; this.props = props; + this.channelBindingProviderManager = ChannelBindingProviderManager.getInstance(); + this.serverSupportedSaslMechanismNames = SASLAuthentication.getSupportedMechanisms(); + } + + /** + * Constructor for testing purposes. + */ + @VisibleForTesting + ScramSha1SaslServer(final boolean isPlusMechanism, final Map props, final ChannelBindingProviderManager channelBindingProviderManager, final Set serverSupportedSaslMechanismNames) + { + this.isPlusMechanism = isPlusMechanism; + this.props = props; + this.channelBindingProviderManager = channelBindingProviderManager; + this.serverSupportedSaslMechanismNames = serverSupportedSaslMechanismNames; } /** @@ -187,6 +205,7 @@ public byte[] evaluateResponse(final byte[] response) throws SaslException { challenge = new byte[0]; break; } + throw new SaslException("Unexpected response after authentication completed"); default: throw new SaslException("No response expected in state " + state); @@ -209,7 +228,8 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce if (!m.matches()) { throw new SaslException("Invalid first client message"); } - String gs2Header = m.group(1); + final byte[] gs2_header = extractRawGS2Header(response); // Using raw header to prevent any normalization issues that might pop up when using something like: gs2Header.getBytes(StandardCharsets.UTF_8); +// String gs2Header = m.group(1); String gs2CbindFlag = m.group(2); gs2CbindName = m.group(3); // String authzId = m.group(4); @@ -224,20 +244,24 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce throw new SaslException("Invalid first client message: Client nonce cannot be empty"); } + // https://www.rfc-editor.org/rfc/rfc5802.html#section-6: If the flag is set to "y" and the server supports + // channel binding, the server MUST fail authentication. This is because if the client sets the channel binding + // flag to "y", then the client must have believed that the server did not support channel binding -- if the + // server did in fact support channel binding, then this is an indication that there has been a downgrade attack + // (e.g., an attacker changed the server's mechanism list to exclude the -PLUS suffixed SCRAM mechanism name(s)). + final boolean clientSupportsChannelBindingButThinksServerDoesNot = "y".equals(gs2CbindFlag); + final boolean serverSupportsChannelBinding = serverSupportedSaslMechanismNames.stream().anyMatch(mechanism -> mechanism.endsWith("-PLUS")); + if (clientSupportsChannelBindingButThinksServerDoesNot && serverSupportsChannelBinding) { + throw new SaslException("Client supports channel binding, but thinks the server does not (while it does). Rejecting authentication to prevent downgrade attack."); + } + + final boolean clientRequiresChannelBinding = "p".equals(gs2CbindFlag); + if (clientRequiresChannelBinding && !isPlusMechanism) { + throw new SaslException("Client requires channel binding, but is not using a -PLUS mechanism. Rejecting authentication."); + } + if (isPlusMechanism) { - // https://www.rfc-editor.org/rfc/rfc5802.html#section-6: If the flag is set to "y" and the server supports - // channel binding, the server MUST fail authentication. This is because if the client sets the channel binding - // flag to "y", then the client must have believed that the server did not support channel binding -- if the - // server did in fact support channel binding, then this is an indication that there has been a downgrade attack - // (e.g., an attacker changed the server's mechanism list to exclude the -PLUS suffixed SCRAM mechanism name(s)). - final boolean clientSupportsChannelBindingButThinksServerDoesNot = "y".equals(gs2CbindFlag); - final boolean serverSupportsChannelBinding = SASLAuthentication.getSupportedMechanisms().stream().anyMatch(mechanism -> mechanism.endsWith("-PLUS")); - if (clientSupportsChannelBindingButThinksServerDoesNot && serverSupportsChannelBinding) { - throw new SaslException("Client supports channel binding, but thinks the server does not (while it does). Rejecting authentication to prevent downgrade attack."); - } - - final boolean clientRequiresChannelBinding = "p".equals(gs2CbindFlag); if (!clientRequiresChannelBinding) { throw new SaslException("Channel binding required for -PLUS. Rejecting authentication."); } @@ -248,7 +272,7 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce // https://www.rfc-editor.org/rfc/rfc5802.html#section-6: If the channel binding flag was "p" and the server // does not support the indicated channel binding type, then the server MUST fail authentication. - if (gs2CbindName == null || gs2CbindName.isEmpty() || !ChannelBindingProviderManager.getInstance().supportsChannelBinding(gs2CbindName)) { + if (gs2CbindName == null || gs2CbindName.isEmpty() || !channelBindingProviderManager.supportsChannelBinding(gs2CbindName)) { throw new SaslException("Client requires channel binding, but server does not support the indicated channel binding type '" + gs2CbindName + "'. Rejecting authentication."); } @@ -264,11 +288,14 @@ private byte[] generateServerFirstMessage(final byte[] response) throws SaslExce } // In the final client message, we expect to find a combination of the gs2 header and channel binding data. - final byte[] gs2_header = extractRawGS2Header(response); // Using raw header to prevent any normalization issues that might pop up when using something like: gs2Header.getBytes(StandardCharsets.UTF_8); final byte[] cb_data = channelBindingData.get(); expectedChannelBindingPayloadInFinalClientMessage = new byte[gs2_header.length + cb_data.length]; System.arraycopy(gs2_header, 0, expectedChannelBindingPayloadInFinalClientMessage, 0 , gs2_header.length); System.arraycopy(cb_data, 0, expectedChannelBindingPayloadInFinalClientMessage, gs2_header.length, cb_data.length); + } else { + // If this is _not_ a -PLUS mechanism, we still need to verify the channel binding payload in the final client message. + // In that case, it should not have trailing channel binding data. + expectedChannelBindingPayloadInFinalClientMessage = gs2_header; } nonce = clientNonce + UUID.randomUUID().toString(); @@ -333,7 +360,7 @@ private byte[] generateServerFinalMessage(final byte[] response) throws SaslExce throw new SaslException("Invalid client final message: missing proof attribute"); } - if (isPlusMechanism && (channelBinding == null || channelBinding.isEmpty())) { + if (channelBinding == null || channelBinding.isEmpty()) { throw new SaslException("Invalid client final message: missing channel binding attribute"); } @@ -348,12 +375,9 @@ private byte[] generateServerFinalMessage(final byte[] response) throws SaslExce } // Verify channel binding payload. - if (isPlusMechanism) - { - final byte[] decodedChannelBinding = DatatypeConverter.parseBase64Binary(channelBinding); - if (!Arrays.equals(expectedChannelBindingPayloadInFinalClientMessage, decodedChannelBinding)) { - throw new SaslException("Invalid client final message: channel binding payload does not match expected payload"); - } + final byte[] decodedChannelBinding = DatatypeConverter.parseBase64Binary(channelBinding); + if (!Arrays.equals(expectedChannelBindingPayloadInFinalClientMessage, decodedChannelBinding)) { + throw new SaslException("Invalid client final message: channel binding payload does not match expected payload"); } try { @@ -366,6 +390,9 @@ private byte[] generateServerFinalMessage(final byte[] response) throws SaslExce byte[] clientKey = clientSignature.clone(); byte[] decodedProof = DatatypeConverter.parseBase64Binary(proof); + if (decodedProof.length != clientKey.length) { + throw new SaslException("Invalid proof length: expected " + clientKey.length + " bytes, got " + decodedProof.length); + } for (int i = 0; i < clientKey.length; i++) { clientKey[i] ^= decodedProof[i]; } @@ -474,6 +501,11 @@ public Object getNegotiatedProperty(String propName) { @Override public void dispose() throws SaslException { username = null; + nonce = null; + serverFirstMessage = null; + clientFirstMessageBare = null; + expectedChannelBindingPayloadInFinalClientMessage = null; + gs2CbindName = null; state = State.INITIAL; } diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java index 145abd9c30..35a666f4a2 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java @@ -70,7 +70,7 @@ public static ChannelBindingProviderManager getInstance() * returned by getInstance(). */ @VisibleForTesting - ChannelBindingProviderManager() + public ChannelBindingProviderManager() // TODO: It is not ideal to have this test-only constructor be 'public', but that's currently required for ScramSha1SaslServerTest. Can this be refactored to avoid such wide access? { } diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerFakeKeyTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerFakeKeyTest.java index 9cf7a7f84e..7b34c22b74 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerFakeKeyTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerFakeKeyTest.java @@ -16,8 +16,11 @@ package org.jivesoftware.openfire.sasl; import org.jivesoftware.Fixtures; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.junit.jupiter.api.*; import java.util.Arrays; +import java.util.HashMap; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -56,7 +59,7 @@ void fakeKeyIsDeterministicForSameInput() { // Setup test fixture ScramSha1SaslServer.SERVER_SECRET_NONEXISTENT_USERS.setValue(SERVER_SECRET_1); - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] key1 = server.getOrFakeStoredKey(USERNAME1); @@ -74,7 +77,7 @@ void fakeKeysDifferForDifferentUsernames() { // Setup test fixture ScramSha1SaslServer.SERVER_SECRET_NONEXISTENT_USERS.setValue(SERVER_SECRET_1); - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] key1 = server.getOrFakeStoredKey(USERNAME1); @@ -92,7 +95,7 @@ void fakeKeyChangesWhenServerSecretChanges() { // Setup test fixture ScramSha1SaslServer.SERVER_SECRET_NONEXISTENT_USERS.setValue(SERVER_SECRET_1); - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] key1 = server.getOrFakeStoredKey(USERNAME1); @@ -111,7 +114,7 @@ void fakeKeyHasExpectedLength() { // Setup test fixture ScramSha1SaslServer.SERVER_SECRET_NONEXISTENT_USERS.setValue(SERVER_SECRET_1); - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] key = server.getOrFakeStoredKey(USERNAME1); @@ -128,7 +131,7 @@ void storedAndServerKeysDifferForSameUsername() { // Setup test fixture ScramSha1SaslServer.SERVER_SECRET_NONEXISTENT_USERS.setValue(SERVER_SECRET_1); - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] stored = server.getOrFakeStoredKey(USERNAME1); diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerSaltTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerSaltTest.java index 2d05c49dbc..6d58c9e319 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerSaltTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerSaltTest.java @@ -18,11 +18,14 @@ import org.jivesoftware.Fixtures; import org.jivesoftware.openfire.auth.AuthFactory; import org.jivesoftware.openfire.user.UserNotFoundException; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.junit.jupiter.api.*; import org.mockito.MockedStatic; import org.mockito.Mockito; import java.util.Arrays; +import java.util.HashMap; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -87,7 +90,7 @@ void tearDown() void fakeSaltIsDeterministicForNonExistentUser() { // Setup test fixture - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] salt1 = server.getOrCreateSalt(NON_EXISTENT_USER); @@ -104,7 +107,7 @@ void fakeSaltIsDeterministicForNonExistentUser() void fakeSaltIsDifferentFromRealUserSalt() { // Setup test fixture - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] fakeSalt = server.getOrCreateSalt(NON_EXISTENT_USER); @@ -121,7 +124,7 @@ void fakeSaltIsDifferentFromRealUserSalt() void fakeSaltChangesWhenServerSecretChanges() { // Setup test fixture - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] salt1 = server.getOrCreateSalt(NON_EXISTENT_USER); @@ -139,7 +142,7 @@ void fakeSaltChangesWhenServerSecretChanges() void realUserSaltIsConsistent() { // Setup test fixture - final ScramSha1SaslServer server = new ScramSha1SaslServer(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); // Execute system under test final byte[] salt1 = server.getOrCreateSalt(EXISTENT_USER); diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java index 11cc504351..be40c56e7e 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java @@ -17,17 +17,21 @@ import org.apache.commons.codec.digest.HmacAlgorithms; import org.apache.commons.codec.digest.HmacUtils; +import org.jivesoftware.openfire.Connection; import org.jivesoftware.openfire.auth.AuthFactory; +import org.jivesoftware.openfire.session.LocalSession; import org.jivesoftware.util.StringUtils; +import org.jivesoftware.util.channelbinding.ChannelBindingProvider; +import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; - import org.mockito.MockedStatic; import org.mockito.Mockito; import javax.crypto.SecretKeyFactory; import javax.crypto.spec.PBEKeySpec; +import javax.security.sasl.Sasl; import javax.security.sasl.SaslException; import javax.xml.bind.DatatypeConverter; import java.nio.charset.StandardCharsets; @@ -35,11 +39,23 @@ import java.util.Arrays; import java.util.Base64; import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Unit tests that verify the implementation of {@link ScramSha1SaslServer} @@ -85,7 +101,7 @@ public void testSuccess() throws Exception authFactory.when(() -> AuthFactory.getServerKey(any())).thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("0fe09258b3ac852ba502cc62ba903eaacdbf7d31"))); // Setup test fixture: prepare initial client message. - final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>()); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); final byte[] initialMessage = ("n,,n=user,r=" + hardCodedClientNonce).getBytes(StandardCharsets.UTF_8); // Execute system under test: getting the first server message. @@ -146,6 +162,107 @@ public void testSuccess() throws Exception } } + /** + * Implements a successful SCRAM-SHA-1-PLUS exchange with channel binding. + */ + @Test + public void testSuccessPlus() throws Exception + { + // Setup test fixture + final SecretKeyFactory HmacSHA1Factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + final String hardCodedClientNonce = "fyko+d2lbbFgONRv9qkxdawL"; + final String hardCodedSalt = "QSXCR+Q6sek8bf92"; + final int hardCodedIterations = 4096; + final String hardCodedPassword = "pencil"; + final String hardCodedClientKey = "Client Key"; + final String hardCodedServerKey = "Server Key"; + final String channelBindingType = "tls-server-end-point"; + final byte[] channelBindingData = "mocked-channel-binding-data".getBytes(StandardCharsets.UTF_8); + + final ChannelBindingProviderManager channelBindingProviderManager = new ChannelBindingProviderManager(); + final ChannelBindingProvider serverEndPointProvider = mock(ChannelBindingProvider.class); + when(serverEndPointProvider.getType()).thenReturn("tls-server-end-point"); + when(serverEndPointProvider.getChannelBinding(any())).thenReturn(Optional.of(channelBindingData)); + + final LocalSession mockSession = mock(LocalSession.class); + final Connection mockConnection = mock(Connection.class); + when(mockConnection.getChannelBindingData(channelBindingType)).thenReturn(Optional.of(channelBindingData)); + when(mockSession.getConnection()).thenReturn(mockConnection); + + channelBindingProviderManager.addProvider(serverEndPointProvider); + + authFactory.when(() -> AuthFactory.getSalt(any())).thenReturn(hardCodedSalt); + authFactory.when(() -> AuthFactory.getIterations(any())).thenReturn(hardCodedIterations); + authFactory.when(() -> AuthFactory.getPassword(any())).thenReturn(hardCodedPassword); + authFactory.when(() -> AuthFactory.getStoredKey(any())).thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("e9d94660c39d65c38fbad91c358f14da0eef2bd6"))); + authFactory.when(() -> AuthFactory.getServerKey(any())).thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("0fe09258b3ac852ba502cc62ba903eaacdbf7d31"))); + + // Setup test fixture: prepare initial client message with channel binding. + final Map props = new HashMap<>(); + props.put(LocalSession.class.getCanonicalName(), mockSession); + final ScramSha1SaslServer server = new ScramSha1SaslServer(true, props, channelBindingProviderManager, Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String gs2Header = "p=" + channelBindingType + ",,"; + final byte[] initialMessage = (gs2Header + "n=user,r=" + hardCodedClientNonce).getBytes(StandardCharsets.UTF_8); + + // Execute system under test: getting the first server message. + final String firstServerResponse = new String(server.evaluateResponse(initialMessage), StandardCharsets.UTF_8); + + // Verify result (first server message should match a pattern, and contain a number of properties) + final Matcher firstServerResponseMatcher = Pattern.compile("r=([^,]*),s=([^,]*),i=(.*)$").matcher(firstServerResponse); + assertTrue(firstServerResponseMatcher.matches(), "First server message does not match expected pattern."); + final String serverNonce = firstServerResponseMatcher.group(1); + assertTrue(serverNonce != null && !serverNonce.isBlank(), "First server message should contain a non-empty server nonce (but did not)"); + assertTrue(serverNonce.startsWith(hardCodedClientNonce), "First server message should contain a server nonce that starts with the client nonce, but did not."); + + byte[] salt = null; + try { + salt = DatatypeConverter.parseBase64Binary(firstServerResponseMatcher.group(2)); + assertEquals(hardCodedSalt, firstServerResponseMatcher.group(2), "First server message should include the 'salt' value configured for this unit test (but did not)"); + } catch (IllegalArgumentException e) { + fail("First server message should contain a valid 'salt' value (but did not)."); + } + + int iterations = -1; + try { + iterations = Integer.parseInt(firstServerResponseMatcher.group(3)); + assertEquals(hardCodedIterations, iterations, "First server message should include the 'iterations' value configured for this unit test (but did not)"); + } catch (NumberFormatException e) { + fail("First server message should contain a valid 'iterations' value (but did not)."); + } + + // Setup test fixture: prepare second client message with channel binding. + final String gs2HeaderBase64 = Base64.getEncoder().encodeToString(gs2Header.getBytes(StandardCharsets.UTF_8)); + final String channelBindingBase64 = Base64.getEncoder().encodeToString(channelBindingData); + final String clientFinalMessageBare = "c=" + gs2HeaderBase64 + channelBindingBase64 + ",r=" + serverNonce; + + final KeySpec saltedPasswordSpec = new PBEKeySpec(hardCodedPassword.toCharArray(), salt, iterations, 20*8); + final byte[] saltedPassword = HmacSHA1Factory.generateSecret(saltedPasswordSpec).getEncoded(); + + final byte[] clientKey = new HmacUtils(HmacAlgorithms.HMAC_SHA_1, saltedPassword).hmac(hardCodedClientKey); + final byte[] storedKey = StringUtils.decodeHex(StringUtils.hash(clientKey, "SHA-1")); + final String authMessage = new String(initialMessage, StandardCharsets.UTF_8).substring(gs2Header.length()) + "," + firstServerResponse + "," + clientFinalMessageBare; + + final byte[] clientSignature = new HmacUtils(HmacAlgorithms.HMAC_SHA_1, storedKey).hmac(authMessage); + final byte[] clientProof = new byte[clientKey.length]; + for (int i=0; i(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test + final String mechanismName = server.getMechanismName(); + + // Verify result + assertEquals("SCRAM-SHA-1", mechanismName, "Non-PLUS mechanism should return 'SCRAM-SHA-1' as its name"); + } + + /** + * Verifies the mechanism name for PLUS instances. + */ + @Test + void getMechanismName_returnsScramSha1Plus_forPlusMechanism() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(true, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test + final String mechanismName = server.getMechanismName(); + + // Verify result + assertEquals("SCRAM-SHA-1-PLUS", mechanismName, "PLUS mechanism should return 'SCRAM-SHA-1-PLUS' as its name"); + } + + /** + * Verifies that a completely malformed first client message is rejected. + */ + @Test + void rejectsFirstMessage_invalidFormat() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(SaslException.class, + () -> server.evaluateResponse("not-a-valid-scram-message".getBytes(StandardCharsets.UTF_8)), + "Malformed first client message should be rejected with SaslException"); + } + + /** + * Verifies that a first client message containing an empty username is rejected. + */ + @Test + void rejectsFirstMessage_emptyUsername() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(SaslException.class, + () -> server.evaluateResponse("n,,n=,r=clientnonce".getBytes(StandardCharsets.UTF_8)), + "First client message with empty username should be rejected"); + } + + /** + * Verifies that a first client message containing an empty client nonce is rejected. + */ + @Test + void rejectsFirstMessage_emptyClientNonce() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(SaslException.class, + () -> server.evaluateResponse("n,,n=user,r=".getBytes(StandardCharsets.UTF_8)), + "First client message with empty client nonce should be rejected"); + } + + /** + * Verifies that a 'p' GS2 channel-binding flag is rejected when using the non-PLUS mechanism. + */ + @Test + void rejectsFirstMessage_channelBindingRequestedOnNonPlusMechanism() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(SaslException.class, + () -> server.evaluateResponse("p=tls-unique,,n=user,r=clientnonce".getBytes(StandardCharsets.UTF_8)), + "Channel binding requested on non-PLUS mechanism should be rejected"); + } + + /** + * Verifies RFC 5802 §6: a 'y' GS2 flag MUST be rejected when the server advertises a -PLUS mechanism, + * because this is a signal that a downgrade attack may be in progress. + */ + @Test + void rejectsFirstMessage_downgradeAttackDetected() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse("y,,n=user,r=clientnonce".getBytes(StandardCharsets.UTF_8)), + "Downgrade attack (y-flag) should be rejected when -PLUS is advertised"); + } + + /** + * Verifies that a completely malformed final client message is rejected. + */ + @Test + void rejectsFinalMessage_invalidFormat() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + server.evaluateResponse("n,,n=user,r=clientnonce".getBytes(StandardCharsets.UTF_8)); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse("not-a-valid-final-message".getBytes(StandardCharsets.UTF_8)), + "Malformed final client message should be rejected with SaslException"); + } + + /** + * Verifies that a final client message with an empty proof attribute is rejected. + */ + @Test + void rejectsFinalMessage_emptyProof() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String serverNonce = doFirstExchange(server); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse(("c=biws,r=" + serverNonce + ",p=").getBytes(StandardCharsets.UTF_8)), + "Final client message with empty proof should be rejected"); + } + + /** + * Verifies that a final client message with an empty channel binding attribute is rejected. + */ + @Test + void rejectsFinalMessage_emptyChannelBinding() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String serverNonce = doFirstExchange(server); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse(("c=,r=" + serverNonce + ",p=dGVzdA==").getBytes(StandardCharsets.UTF_8)), + "Final client message with empty channel binding should be rejected"); + } + + /** + * Verifies that a final client message containing an incorrect nonce is rejected. + */ + @Test + void rejectsFinalMessage_incorrectNonce() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + doFirstExchange(server); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse("c=biws,r=completely-wrong-nonce,p=dGVzdA==".getBytes(StandardCharsets.UTF_8)), + "Final client message with incorrect nonce should be rejected"); + } + + /** + * Verifies that a final client message carrying an incorrect channel binding value is rejected + * for a non-PLUS exchange. For non-PLUS, c= must decode to exactly the GS2 header ("n,,"), + * whose base64 encoding is "biws". + */ + @Test + void rejectsFinalMessage_incorrectChannelBindingValue_nonPlusMechanism() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String serverNonce = doFirstExchange(server); + final String wrongBinding = Base64.getEncoder().encodeToString("p=tls-unique,,".getBytes(StandardCharsets.UTF_8)); + + // Execute system under test & Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse(("c=" + wrongBinding + ",r=" + serverNonce + ",p=dGVzdA==").getBytes(StandardCharsets.UTF_8)), + "Final client message with incorrect channel binding value should be rejected"); + } + + /** + * Verifies that a proof whose decoded length differs from the HMAC-SHA-1 output length (20 bytes) + * is rejected with a clean SaslException rather than an ArrayIndexOutOfBoundsException. + */ + @Test + void rejectsFinalMessage_proofWithWrongLength() throws Exception + { + // Setup test fixture + setupSaltAndIterationsAndKeys(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String serverNonce = doFirstExchange(server); + final String shortProof = Base64.getEncoder().encodeToString(new byte[10]); // 10 bytes, not 20 + + // Execute system under test + final SaslException ex = assertThrows(SaslException.class, () -> server.evaluateResponse(("c=biws,r=" + serverNonce + ",p=" + shortProof).getBytes(StandardCharsets.UTF_8)), + "Final client message with proof of wrong length should be rejected"); + + // Verify result + assertTrue(ex.getMessage().contains("proof"), "Exception should mention the proof"); + } + + /** + * Verifies that a correctly structured final message carrying a wrong (but correctly sized) proof + * results in an authentication failure rather than a successful login. + */ + @Test + void rejectsFinalMessage_incorrectProof() throws Exception + { + // Setup test fixture + setupSaltAndIterationsAndKeys(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final String serverNonce = doFirstExchange(server); + final String wrongProof = Base64.getEncoder().encodeToString(new byte[20]); // 20 zero bytes + + // Execute system under test & Verify result + assertThrows(SaslException.class,() -> server.evaluateResponse(("c=biws,r=" + serverNonce + ",p=" + wrongProof).getBytes(StandardCharsets.UTF_8)), + "Final client message with incorrect proof should be rejected"); + } + + /** + * Verifies that isComplete() returns false before any exchange has taken place. + */ + @Test + void isComplete_returnsFalse_initially() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test + final boolean complete = server.isComplete(); + + // Verify result + assertFalse(complete, "isComplete() should return false before any exchange has taken place"); + } + + /** + * Verifies that isComplete() returns false after only the first exchange round. + */ + @Test + void isComplete_returnsFalse_afterFirstExchangeOnly() throws Exception + { + // Setup test fixture + setupSaltAndIterations(); + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + server.evaluateResponse("n,,n=user,r=clientnonce".getBytes(StandardCharsets.UTF_8)); + + // Execute system under test + final boolean complete = server.isComplete(); + + // Verify result + assertFalse(complete, "isComplete() should return false after only the first exchange"); + } + + /** + * Verifies that a non-empty response submitted after a completed exchange is rejected. + */ + @Test + void rejectsNonEmptyResponse_afterExchangeComplete() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + + // Execute system under test + assertTrue(server.isComplete(), "Server should be complete after successful exchange"); + + // Verify result + assertThrows(SaslException.class, () -> server.evaluateResponse("unexpected".getBytes(StandardCharsets.UTF_8)), + "Non-empty response after exchange complete should be rejected"); + } + + /** + * Verifies that an empty response submitted after a completed exchange is tolerated + * (some SASL frameworks send an empty final acknowledgement). + */ + @Test + void acceptsEmptyResponse_afterExchangeComplete() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + + // Execute system under test & Verify result + assertDoesNotThrow(() -> server.evaluateResponse(new byte[0]), + "Empty response after exchange complete should be tolerated"); + } + + /** + * Verifies that getAuthorizationID() throws before the exchange completes. + */ + @Test + void getAuthorizationID_throwsIllegalStateException_beforeCompletion() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(IllegalStateException.class, server::getAuthorizationID, + "getAuthorizationID() before completion should throw IllegalStateException"); + } + + /** + * Verifies that getAuthorizationID() returns the authenticated username after a successful exchange. + */ + @Test + void getAuthorizationID_returnsUsername_afterCompletion() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + + // Execute system under test + final String authzId = server.getAuthorizationID(); + + // Verify result + assertEquals("user", authzId, "getAuthorizationID() should return the authenticated username after completion"); + } + + /** + * Verifies that getNegotiatedProperty() throws before the exchange completes. + */ + @Test + void getNegotiatedProperty_throwsIllegalStateException_beforeCompletion() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(IllegalStateException.class, () -> server.getNegotiatedProperty(Sasl.QOP), + "getNegotiatedProperty() before completion should throw IllegalStateException"); + } + + /** + * Verifies that getNegotiatedProperty() reports "auth" for QOP after a successful exchange, + * as SCRAM-SHA-1 provides authentication only (no integrity or confidentiality layer). + */ + @Test + void getNegotiatedProperty_returnsAuth_forQOP_afterCompletion() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + + // Execute system under test + final Object qop = server.getNegotiatedProperty(Sasl.QOP); + + // Verify result + assertEquals("auth", qop, "getNegotiatedProperty(Sasl.QOP) should return 'auth' after completion"); + } + + /** + * Verifies that getNegotiatedProperty() returns null for unknown properties after completion. + */ + @Test + void getNegotiatedProperty_returnsNull_forUnknownProperty_afterCompletion() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + + // Execute system under test + final Object unknown = server.getNegotiatedProperty("unknown.property"); + + // Verify result + assertNull(unknown, "getNegotiatedProperty() should return null for unknown properties after completion"); + } + + /** + * Verifies that unwrap() always throws, as SCRAM-SHA-1 has no security layer. + */ + @Test + void unwrap_throwsIllegalStateException_always() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(IllegalStateException.class, () -> server.unwrap(new byte[]{1, 2, 3}, 0, 3), + "unwrap() should always throw IllegalStateException as SCRAM-SHA-1 has no security layer"); + } + + /** + * Verifies that wrap() always throws, as SCRAM-SHA-1 has no security layer. + */ + @Test + void wrap_throwsIllegalStateException_always() + { + // Setup test fixture + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + + // Execute system under test & Verify result + assertThrows(IllegalStateException.class, () -> server.wrap(new byte[]{1, 2, 3}, 0, 3), + "wrap() should always throw IllegalStateException as SCRAM-SHA-1 has no security layer"); + } + + /** + * Verifies that dispose() resets the server to its initial state, making isComplete() return false + * and preventing getAuthorizationID() from returning stale data. + */ + @Test + void dispose_resetsStateAndClearsSensitiveFields() throws Exception + { + // Setup test fixture + final ScramSha1SaslServer server = completeSuccessfulExchange(); + assertTrue(server.isComplete(), "Server should be complete after successful exchange"); + + // Execute system under test + server.dispose(); + + // Verify result + assertFalse(server.isComplete(), "Server should not be complete after dispose()"); + assertThrows(IllegalStateException.class, server::getAuthorizationID, + "getAuthorizationID() should throw after dispose()"); + } + + // ------------------------------------------------------------------------- + // Private helpers + // ------------------------------------------------------------------------- + + private void setupSaltAndIterations() + { + authFactory.when(() -> AuthFactory.getSalt(any())).thenReturn("QSXCR+Q6sek8bf92"); + authFactory.when(() -> AuthFactory.getIterations(any())).thenReturn(4096); + } + + private void setupSaltAndIterationsAndKeys() + { + setupSaltAndIterations(); + authFactory.when(() -> AuthFactory.getStoredKey(any())) + .thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("e9d94660c39d65c38fbad91c358f14da0eef2bd6"))); + authFactory.when(() -> AuthFactory.getServerKey(any())) + .thenReturn(DatatypeConverter.printBase64Binary(StringUtils.decodeHex("0fe09258b3ac852ba502cc62ba903eaacdbf7d31"))); + } + + /** + * Performs the first exchange round and returns the composite server nonce. + */ + private String doFirstExchange(final ScramSha1SaslServer server) throws SaslException + { + final String firstServerResponse = new String( + server.evaluateResponse("n,,n=user,r=clientnonce".getBytes(StandardCharsets.UTF_8)), + StandardCharsets.UTF_8); + final Matcher m = Pattern.compile("r=([^,]*),.+").matcher(firstServerResponse); + assertTrue(m.matches(), "First server response did not match expected pattern"); + return m.group(1); + } + + /** + * Drives a complete successful SCRAM-SHA-1 exchange and returns the completed server instance. + */ + private ScramSha1SaslServer completeSuccessfulExchange() throws Exception + { + final String hardCodedClientNonce = "fyko+d2lbbFgONRv9qkxdawL"; + final String hardCodedPassword = "pencil"; + + setupSaltAndIterationsAndKeys(); + authFactory.when(() -> AuthFactory.getPassword(any())).thenReturn(hardCodedPassword); + + final ScramSha1SaslServer server = new ScramSha1SaslServer(false, new HashMap<>(), new ChannelBindingProviderManager(), Set.of("SCRAM-SHA-1", "SCRAM-SHA-1-PLUS")); + final byte[] initialMessage = ("n,,n=user,r=" + hardCodedClientNonce).getBytes(StandardCharsets.UTF_8); + final String firstServerResponse = new String(server.evaluateResponse(initialMessage), StandardCharsets.UTF_8); + + final Matcher m = Pattern.compile("r=([^,]*),s=([^,]*),i=(.*)$").matcher(firstServerResponse); + assertTrue(m.matches()); + final String serverNonce = m.group(1); + final byte[] salt = DatatypeConverter.parseBase64Binary(m.group(2)); + final int iterations = Integer.parseInt(m.group(3)); + + final SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + final KeySpec spec = new PBEKeySpec(hardCodedPassword.toCharArray(), salt, iterations, 160); + final byte[] saltedPassword = factory.generateSecret(spec).getEncoded(); + + final byte[] clientKey = new HmacUtils(HmacAlgorithms.HMAC_SHA_1, saltedPassword).hmac("Client Key"); + final byte[] storedKey = StringUtils.decodeHex(StringUtils.hash(clientKey, "SHA-1")); + final String clientFinalBare = "c=biws,r=" + serverNonce; + final String authMessage = "n=user,r=" + hardCodedClientNonce + "," + firstServerResponse + "," + clientFinalBare; + final byte[] clientSignature = new HmacUtils(HmacAlgorithms.HMAC_SHA_1, storedKey).hmac(authMessage); + + final byte[] clientProof = new byte[clientKey.length]; + for (int i = 0; i < clientKey.length; i++) { + clientProof[i] = (byte) (clientKey[i] ^ clientSignature[i]); + } + + final String clientFinalMessage = clientFinalBare + ",p=" + Base64.getEncoder().encodeToString(clientProof); + server.evaluateResponse(clientFinalMessage.getBytes(StandardCharsets.UTF_8)); + return server; + } } From 8518d6e52284061bd4790161c0e567aa629d6512 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Wed, 22 Apr 2026 21:45:29 +0200 Subject: [PATCH 08/13] OF-2694 (code review feedback): Only advertise SASL -PLUS mechanisms when channel binding is supported Refactored SASL mechanism advertisement logic to ensure that -PLUS mechanisms (e.g., SCRAM-SHA-1-PLUS) are only offered when the current connection supports channel binding. The check now relies on Connection#getSupportedChannelBindingTypes(), which must return a non-empty set for -PLUS mechanisms to be advertised. Updated all relevant connection implementations (only NettyConnection, and the default method in the Connection interface) to ensure correct reporting of supported channel binding types. This change prevents authentication failures caused by advertising -PLUS mechanisms on connections that cannot provide channel binding data (e.g., WebSocket connections). --- .../org/jivesoftware/openfire/Connection.java | 17 +++++++++++++++++ .../openfire/net/SASLAuthentication.java | 18 +++++++++++------- .../openfire/nio/NettyConnection.java | 17 ++++++++++++++++- 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java b/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java index 92325c81ed..0be277bd32 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/Connection.java @@ -28,6 +28,7 @@ import java.io.Closeable; import java.net.UnknownHostException; import java.security.cert.Certificate; +import java.util.Collections; import java.util.Optional; import java.util.Set; import java.util.concurrent.CompletionStage; @@ -163,6 +164,22 @@ default Optional getChannelBindingData(@Nonnull final String cbPrefix) { return Optional.empty(); } + /** + * Returns the unique prefixes of the channel binding types that are supported by this connection in its current + * state. Notably, this may change if the connection is encrypted or if the underlying TLS implementation changes. + * When no channel binding types are supported, an empty set is returned. + * + * Implementation note: This method is used to determine if SASL -PLUS mechanisms (such as SCRAM-SHA-1-PLUS) + * should be offered to the client. If channel binding is not supported in the current state (e.g., not encrypted, + * or the connection type does not support channel binding), this method must return an empty set. + * + * @return supported channel binding types. + */ + default Set getSupportedChannelBindingTypes() + { + return Collections.emptySet(); + } + /** * Keeps track if the other peer of this session presented a self-signed certificate. When * using self-signed certificate for server-2-server sessions then SASL EXTERNAL will not be diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index 84d1197c6c..f9beb6f2dd 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -28,10 +28,7 @@ import org.jivesoftware.openfire.keystore.CertificateStoreManager; import org.jivesoftware.openfire.keystore.TrustStore; import org.jivesoftware.openfire.lockout.LockOutManager; -import org.jivesoftware.openfire.sasl.AnonymousSaslServer; -import org.jivesoftware.openfire.sasl.Failure; -import org.jivesoftware.openfire.sasl.JiveSharedSecretSaslServer; -import org.jivesoftware.openfire.sasl.SaslFailureException; +import org.jivesoftware.openfire.sasl.*; import org.jivesoftware.openfire.session.*; import org.jivesoftware.openfire.spi.ConnectionType; import org.jivesoftware.util.CertificateManager; @@ -279,9 +276,16 @@ public static Element getSASLMechanismsElement( ClientSession session ) } } if (mech.endsWith("-PLUS")) { - // Channel binding would be a binding to TLS. - if (!session.isEncrypted()) { - continue; // Cannot bind to TLS when there's no TLS. + // Prevent offering channel binding if the Connection implementation does not support it. + final Connection connection = ( (LocalClientSession) session ).getConnection(); + assert connection != null; // While the client is performing a SASL negotiation, the connection can't be null. + if (connection.getSupportedChannelBindingTypes().isEmpty()) { + continue; + } + + // Channel binding would be a binding to TLS, thus encryption is required for channel binding. + if (!session.isEncrypted()) { // This aught to be redundant, as getSupportedChannelBindingTypes() will return an empty set if not encrypted. + continue; } } final Element mechanism = result.addElement("mechanism"); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java index cf2fa49f63..bfafc49856 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnection.java @@ -49,7 +49,9 @@ import java.net.SocketAddress; import java.net.UnknownHostException; import java.security.cert.Certificate; +import java.util.Collections; import java.util.Optional; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static com.jcraft.jzlib.JZlib.Z_BEST_COMPRESSION; @@ -86,10 +88,13 @@ public class NettyConnection extends AbstractConnection private final AtomicReference state = new AtomicReference<>(State.OPEN); private boolean isEncrypted = false; + private ChannelBindingProviderManager channelBindingProviderManager; // TODO allow this to be set for unit testing. + public NettyConnection(ChannelHandlerContext channelHandlerContext, @Nullable PacketDeliverer packetDeliverer, ConnectionConfiguration configuration ) { this.channelHandlerContext = channelHandlerContext; this.backupDeliverer = packetDeliverer; this.configuration = configuration; + this.channelBindingProviderManager = ChannelBindingProviderManager.getInstance(); } @Override @@ -417,7 +422,17 @@ public Optional getChannelBindingData(@Nonnull final String cbPrefix) } final SSLEngine engine = sslhandler.engine(); - return ChannelBindingProviderManager.getInstance().getChannelBinding(cbPrefix, engine); + return channelBindingProviderManager.getChannelBinding(cbPrefix, engine); + } + + @Override + public Set getSupportedChannelBindingTypes() + { + final SslHandler sslhandler = (SslHandler) channelHandlerContext.channel().pipeline().get(SSL_HANDLER_NAME); + if (sslhandler == null) { + return Collections.emptySet(); + } + return channelBindingProviderManager.getSupportedChannelBindingTypes(); } @Override From adb11466eebda0b6d178c9c4bb20f5e10b93480b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Mon, 27 Apr 2026 08:54:35 +0200 Subject: [PATCH 09/13] OF-2694 (code review feedback): Various text fixes This addresses various typos and spelling issues. None of these are functionally changing anything. --- .../resources/openfire_i18n_nl.properties | 1 + .../container/AdminConsolePlugin.java | 2 +- .../openfire/group/AbstractGroupProvider.java | 2 +- .../openfire/net/SASLAuthentication.java | 21 ++++++++++++++----- .../openfire/sasl/ScramSha1SaslServer.java | 2 +- .../jivesoftware/util/CertificateManager.java | 2 +- .../util/cache/DefaultLocalCacheStrategy.java | 2 +- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/i18n/src/main/resources/openfire_i18n_nl.properties b/i18n/src/main/resources/openfire_i18n_nl.properties index 9c318fc513..16addd06f7 100644 --- a/i18n/src/main/resources/openfire_i18n_nl.properties +++ b/i18n/src/main/resources/openfire_i18n_nl.properties @@ -1733,6 +1733,7 @@ session.details.anon-status=Gebruik Anonieme Authenticatie session.details.anon-true=Ja session.details.anon-false=Nee session.details.sasl-mechanism=SASL Mechanisme +session.details.channel-binding-type=Channel Binding Type session.details.flomr-status=Offline Berichten Flexibel Ophalen session.details.flomr-enabled=Ingeschakeld session.details.flomr-disabled=Uitgeschakeld diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java b/xmppserver/src/main/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java index b14891850f..b222a5751a 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/container/AdminConsolePlugin.java @@ -500,7 +500,7 @@ public ContextHandlerCollection getContexts() { } /** - * Restart the admin console (and it's HTTP server) without restarting the plugin. + * Restart the admin console (and its HTTP server) without restarting the plugin. */ public void restart() { try { diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/group/AbstractGroupProvider.java b/xmppserver/src/main/java/org/jivesoftware/openfire/group/AbstractGroupProvider.java index 3271f7a284..188479b17f 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/group/AbstractGroupProvider.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/group/AbstractGroupProvider.java @@ -148,7 +148,7 @@ public Group createGroup(String name) throws GroupAlreadyExistsException, GroupN sharedGroupMetaCache.clear(); } - return null; // aught to be overridden. + return null; // ought to be overridden. } /** diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java index f9beb6f2dd..dba8971f6e 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -28,8 +28,19 @@ import org.jivesoftware.openfire.keystore.CertificateStoreManager; import org.jivesoftware.openfire.keystore.TrustStore; import org.jivesoftware.openfire.lockout.LockOutManager; -import org.jivesoftware.openfire.sasl.*; -import org.jivesoftware.openfire.session.*; +import org.jivesoftware.openfire.sasl.AnonymousSaslServer; +import org.jivesoftware.openfire.sasl.Failure; +import org.jivesoftware.openfire.sasl.JiveSharedSecretSaslServer; +import org.jivesoftware.openfire.sasl.SaslFailureException; +import org.jivesoftware.openfire.sasl.ScramSha1SaslServer; +import org.jivesoftware.openfire.session.ClientSession; +import org.jivesoftware.openfire.session.ConnectionSettings; +import org.jivesoftware.openfire.session.IncomingServerSession; +import org.jivesoftware.openfire.session.LocalClientSession; +import org.jivesoftware.openfire.session.LocalIncomingServerSession; +import org.jivesoftware.openfire.session.LocalSession; +import org.jivesoftware.openfire.session.ServerSession; +import org.jivesoftware.openfire.session.Session; import org.jivesoftware.openfire.spi.ConnectionType; import org.jivesoftware.util.CertificateManager; import org.jivesoftware.util.JiveGlobals; @@ -284,7 +295,7 @@ public static Element getSASLMechanismsElement( ClientSession session ) } // Channel binding would be a binding to TLS, thus encryption is required for channel binding. - if (!session.isEncrypted()) { // This aught to be redundant, as getSupportedChannelBindingTypes() will return an empty set if not encrypted. + if (!session.isEncrypted()) { // This ought to be redundant, as getSupportedChannelBindingTypes() will return an empty set if not encrypted. continue; } } @@ -666,7 +677,7 @@ public static Set getSupportedMechanisms() // Check if the user provider in use supports passwords retrieval. Access to the users passwords will be required by the CallbackHandler. if ( !AuthFactory.supportsPasswordRetrieval() ) { - Log.trace( "Cannot support '{}' as the AuthFactory that's in use does not support password retrieval.", mechanism ); + Log.trace( "Cannot support '{}' as the AuthProvider that's in use does not support password retrieval.", mechanism ); it.remove(); } break; @@ -675,7 +686,7 @@ public static Set getSupportedMechanisms() case "SCRAM-SHA-1-PLUS": if ( !AuthFactory.supportsScram() ) { - Log.trace( "Cannot support '{}' as the AuthFactory that's in use does not support SCRAM.", mechanism ); + Log.trace( "Cannot support '{}' as the AuthProvider that's in use does not support SCRAM.", mechanism ); it.remove(); } break; diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java index 3e6594ad71..51374718a7 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServer.java @@ -50,7 +50,7 @@ import org.slf4j.LoggerFactory; /** - * Implements the SCRAM-SHA-1 (and it's channel binding -PLUS variant) server-side mechanism. + * Implements the SCRAM-SHA-1 (and its channel binding -PLUS variant) server-side mechanism. * * @author Richard Midwinter, Guus der Kinderen */ diff --git a/xmppserver/src/main/java/org/jivesoftware/util/CertificateManager.java b/xmppserver/src/main/java/org/jivesoftware/util/CertificateManager.java index 2e163221df..03fb5bbadf 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/CertificateManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/CertificateManager.java @@ -628,7 +628,7 @@ protected static GeneralNames getSubjectAlternativeNames( Set sanDnsName } /** - * Finds all values that aught to be added as a Subject Alternate Name of the dnsName type to a certificate that + * Finds all values that ought to be added as a Subject Alternate Name of the dnsName type to a certificate that * identifies this XMPP server. * * @return A set of names, possibly empty, never null. diff --git a/xmppserver/src/main/java/org/jivesoftware/util/cache/DefaultLocalCacheStrategy.java b/xmppserver/src/main/java/org/jivesoftware/util/cache/DefaultLocalCacheStrategy.java index 059b4f1954..a91faf8006 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/cache/DefaultLocalCacheStrategy.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/cache/DefaultLocalCacheStrategy.java @@ -24,7 +24,7 @@ import java.util.Map; /** - * CacheFactoryStrategy for use in Openfire. It creates and manages local caches, and it's cluster + * CacheFactoryStrategy for use in Openfire. It creates and manages local caches, and its cluster * related method implementations do nothing. * * @see Cache From 8179c7bb9140e23fe119a0b7d5cb44f855f3af9b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 12 May 2026 16:17:20 +0200 Subject: [PATCH 10/13] OF-2694 (code review feedback): Make removal of ChannelBindingProvider thread safe This mimics the concurrency control of the implementation that adds these providers. --- .../ChannelBindingProviderManager.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java index 35a666f4a2..b114ba1ca4 100644 --- a/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java +++ b/xmppserver/src/main/java/org/jivesoftware/util/channelbinding/ChannelBindingProviderManager.java @@ -28,6 +28,7 @@ import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; /** * Manages a set of providers that can extract channel binding data of various types from SSL engines. @@ -130,15 +131,12 @@ public boolean removeProvider(@Nonnull final ChannelBindingProvider provider) } Log.trace("Removing channel binding provider {} for prefix '{}'", provider.getClass().getName(), prefix); - final List list = providers.get(prefix); - boolean removed = false; - if (list != null) { - removed = list.remove(provider); - if (list.isEmpty()) { - providers.remove(prefix); - } - } - return removed; + final AtomicBoolean removed = new AtomicBoolean(false); + providers.computeIfPresent(prefix, (k, list) -> { + removed.set(list.remove(provider)); + return list.isEmpty() ? null : list; + }); + return removed.get(); } /** From b834c8f2bb4d74d80ccab7f4a3758d9131586d28 Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 12 May 2026 16:30:20 +0200 Subject: [PATCH 11/13] OF-2694 (code review feedback): (more) correctly encode SCRAM-SHA-1 test vector Instead of concatenating two base64 strings, concat the byte-parts of the channel-binding data, and base64 encode the result. This better conforms to https://datatracker.ietf.org/doc/html/rfc5802#section-7 --- .../openfire/sasl/ScramSha1SaslServerTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java index be40c56e7e..595b669b77 100644 --- a/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java +++ b/xmppserver/src/test/java/org/jivesoftware/openfire/sasl/ScramSha1SaslServerTest.java @@ -231,9 +231,11 @@ public void testSuccessPlus() throws Exception } // Setup test fixture: prepare second client message with channel binding. - final String gs2HeaderBase64 = Base64.getEncoder().encodeToString(gs2Header.getBytes(StandardCharsets.UTF_8)); - final String channelBindingBase64 = Base64.getEncoder().encodeToString(channelBindingData); - final String clientFinalMessageBare = "c=" + gs2HeaderBase64 + channelBindingBase64 + ",r=" + serverNonce; + final byte[] gs2HeaderBytes = gs2Header.getBytes(StandardCharsets.UTF_8); + final byte[] cbindInput = new byte[gs2HeaderBytes.length + channelBindingData.length]; + System.arraycopy(gs2HeaderBytes, 0, cbindInput, 0, gs2HeaderBytes.length); + System.arraycopy(channelBindingData, 0, cbindInput, gs2HeaderBytes.length, channelBindingData.length); + final String clientFinalMessageBare = "c=" + Base64.getEncoder().encodeToString(cbindInput) + ",r=" + serverNonce; final KeySpec saltedPasswordSpec = new PBEKeySpec(hardCodedPassword.toCharArray(), salt, iterations, 20*8); final byte[] saltedPassword = HmacSHA1Factory.generateSecret(saltedPasswordSpec).getEncoded(); From c4a8340d0bfe65125f9bdc25560ff2953808c2de Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 12 May 2026 19:51:28 +0200 Subject: [PATCH 12/13] OF-2694: Add Maestro test to CI for Channel Binding This new test launches Conversations against a standard Openfire server (the 'demoboot' configuration) and attempts to authenticate. It is asserted that: - the authentication attempt was successful - a SASL mechanism ending with `-PLUS` was used When this test passes, it's verified that Conversations successfully used channel binding. --- .../continuous-integration-workflow.yml | 3 ++ build/ci/conversations/flows/cb.yaml | 47 +++++++++++++++++++ 2 files changed, 50 insertions(+) create mode 100644 build/ci/conversations/flows/cb.yaml diff --git a/.github/workflows/continuous-integration-workflow.yml b/.github/workflows/continuous-integration-workflow.yml index cf2c8b2c09..1f3f8043ca 100644 --- a/.github/workflows/continuous-integration-workflow.yml +++ b/.github/workflows/continuous-integration-workflow.yml @@ -325,6 +325,9 @@ jobs: - name: demoboot maestro-tags: demoboot config-file: '' # Use default demoboot config + - name: Channel Binding + maestro-tags: cb + config-file: '' # Use default demoboot config #- name: sasl2 # maestro-tags: sasl2 # config-file: build/ci/conversations/configs/sasl2.xml diff --git a/build/ci/conversations/flows/cb.yaml b/build/ci/conversations/flows/cb.yaml new file mode 100644 index 0000000000..05b149659f --- /dev/null +++ b/build/ci/conversations/flows/cb.yaml @@ -0,0 +1,47 @@ +appId: eu.siacs.conversations +tags: + - cb +onFlowStart: + - runScript: scripts/checkHealth.js +--- + +- runScript: scripts/startSession.js + +- launchApp: + clearState: true + +- assertVisible: "I already have an account" + +- runScript: + file: scripts/checkForLogs.js + env: + PATTERN: initializing database.* + +- tapOn: "I already have an account" +- tapOn: "More options" +- tapOn: "Settings" +- tapOn: "Connection" +- tapOn: "Show extended connection settings when setting up an account" +- tapOn: "Navigate up" +- tapOn: "Navigate up" + +- tapOn: "XMPP address" +- inputText: "jane@example.org" +- tapOn: "Password" +- inputText: "secret" +- tapOn: "Hostname" +- inputText: "10.0.2.2" +- tapOn: "Next" + +- assertVisible: "Accept Unknown Certificate?" +- tapOn: "Always" + +- runScript: + file: scripts/checkForLogs.js + env: + PATTERN: 'jane@example\.org.*Authenticating with SASL\/[A-Z0-9-]+-PLUS' + +- runScript: + file: scripts/checkForLogs.js + env: + PATTERN: 'jane@example\.org.*logged in \(using SASL\)' From d9851578614b4f9c81f2708de63289f90897cf3b Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Tue, 19 May 2026 11:58:13 +0200 Subject: [PATCH 13/13] CI: merge 'cb' and 'demoboot' Maestro tags The Conversations tests for channel binding added a second matrix for the `cb` tag, but it uses the same default demoboot config. Test hygiene would say: merge them. Basically, Dan told me to do this. --- .github/workflows/continuous-integration-workflow.yml | 3 --- build/ci/conversations/flows/cb.yaml | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/continuous-integration-workflow.yml b/.github/workflows/continuous-integration-workflow.yml index 1f3f8043ca..cf2c8b2c09 100644 --- a/.github/workflows/continuous-integration-workflow.yml +++ b/.github/workflows/continuous-integration-workflow.yml @@ -325,9 +325,6 @@ jobs: - name: demoboot maestro-tags: demoboot config-file: '' # Use default demoboot config - - name: Channel Binding - maestro-tags: cb - config-file: '' # Use default demoboot config #- name: sasl2 # maestro-tags: sasl2 # config-file: build/ci/conversations/configs/sasl2.xml diff --git a/build/ci/conversations/flows/cb.yaml b/build/ci/conversations/flows/cb.yaml index 05b149659f..079b2cb869 100644 --- a/build/ci/conversations/flows/cb.yaml +++ b/build/ci/conversations/flows/cb.yaml @@ -1,6 +1,6 @@ appId: eu.siacs.conversations tags: - - cb + - demoboot onFlowStart: - runScript: scripts/checkHealth.js ---