Skip to content

OF-3268 & OF-3273: Refactor SASL EXTERNAL S2S success path #3311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guusdk merged 7 commits into from
May 19, 2026
Merged

OF-3268 & OF-3273: Refactor SASL EXTERNAL S2S success path #3311

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bd00e27
OF-3268: Refactor SASL EXTERNAL S2S success path to remove duplicate …
guusdk May 6, 2026
20e5c87
OF-3273: Enforce session-eligible SASL mechanism validation during au…
guusdk May 8, 2026
20f5293
Set incoming server auth method from negotiated SASL mechanism
guusdk May 8, 2026
2663843
Optimize SASL mechanism validation by avoiding unnecessary XML conver…
guusdk May 8, 2026
d946622
(code review) Gate SASL EXTERNAL advertisement on enabled mechanisms
guusdk May 8, 2026
60a4b67
Minor fixes for code review findings
guusdk May 8, 2026
b909474
Ensure authentication is reported as failed when user is locked out
guusdk May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
266 changes: 176 additions & 90 deletions xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.jivesoftware.openfire.net;

import com.google.common.annotations.VisibleForTesting;
import org.dom4j.DocumentHelper;
import org.dom4j.Element;
import org.dom4j.Namespace;
Expand All @@ -33,23 +34,18 @@
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.session.*;
import org.jivesoftware.openfire.spi.ConnectionType;
import org.jivesoftware.util.CertificateManager;
import org.jivesoftware.util.JiveGlobals;
import org.jivesoftware.util.PropertyEventDispatcher;
import org.jivesoftware.util.PropertyEventListener;
import org.jivesoftware.util.SystemProperty;
import org.jivesoftware.util.channelbinding.ChannelBindingProviderManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nonnull;
import javax.security.sasl.Sasl;
import javax.security.sasl.SaslException;
import javax.security.sasl.SaslServer;
Expand Down Expand Up @@ -165,7 +161,7 @@ public class SASLAuthentication {

initMechanisms();

org.jivesoftware.util.PropertyEventDispatcher.addListener( new PropertyEventListener()
PropertyEventDispatcher.addListener( new PropertyEventListener()
{
@Override
public void propertySet( String property, Map<String, Object> params )
Expand Down Expand Up @@ -264,28 +260,30 @@ else if ( session instanceof LocalIncomingServerSession )
}
}

public static Element getSASLMechanismsElement( ClientSession session )
/**
* Generates an XML element that represents the available SASL mechanisms for a given client session.
*
* Based on the session and server configuration, it optionally returns null if no mechanisms are available and the
* configuration suppresses empty mechanism lists.
*
* @param session the client session for which the available SASL mechanisms are to be retrieved.
* @return an XML element listing the available SASL mechanisms, or null if no mechanisms are available
* and the configuration suppresses empty lists.
*/
public static Element getSASLMechanismsElement(@Nonnull final ClientSession session)
{
final Element result = DocumentHelper.createElement( new QName( "mechanisms", new Namespace( "", SASL_NAMESPACE ) ) );
for (String mech : getSupportedMechanisms()) {
if (mech.equals("EXTERNAL")) {
boolean trustedCert = false;
if (session.isEncrypted()) {
final Connection connection = ( (LocalClientSession) session ).getConnection();
assert connection != null; // While the client is performing a SASL negotiation, the connection can't be null.
if ( SKIP_PEER_CERT_REVALIDATION_CLIENT.getValue() ) {
// Trust that the peer certificate has been validated when TLS got established.
trustedCert = connection.getPeerCertificates() != null && connection.getPeerCertificates().length > 0;
} else {
// Re-evaluate the validity of the peer certificate.
final TrustStore trustStore = connection.getConfiguration().getTrustStore();
trustedCert = trustStore.isTrusted( connection.getPeerCertificates() );
}
}
if ( !trustedCert ) {
continue; // Do not offer EXTERNAL.
}
}
final Set<String> availableMechanisms = getAvailableMechanismsForClientSession(session);

// OF-2072: Return null instead of an empty element, if so configured.
if (JiveGlobals.getBooleanProperty("sasl.client.suppressEmpty", false) && availableMechanisms.isEmpty()) {
return null;
}

final Element result = DocumentHelper.createElement( new QName("mechanisms", new Namespace("", SASL_NAMESPACE)) );
for (final String mech : availableMechanisms) {
final Element mechanism = result.addElement("mechanism");
mechanism.setText(mech);

if (mech.endsWith("-PLUS")) {
// Prevent offering channel binding if the Connection implementation does not support it.
final Connection connection = ( (LocalClientSession) session ).getConnection();
Expand All @@ -299,41 +297,36 @@ public static Element getSASLMechanismsElement( ClientSession session )
continue;
}
}
final Element mechanism = result.addElement("mechanism");
mechanism.setText(mech);
}

// OF-2072: Return null instead of an empty element, if so configured.
if ( JiveGlobals.getBooleanProperty("sasl.client.suppressEmpty", false) && result.elements().isEmpty() ) {
return null;
}

return result;
}

public static Element getSASLMechanismsElement( LocalIncomingServerSession session )
/**
* Generates an XML element that contains the SASL mechanisms available for a given server session.
*
* Depending on the configuration (property "sasl.server.suppressEmpty"), this method can return {@code null}
* instead of an empty mechanisms element if no SASL mechanisms are available.
*
* @param session the local incoming server session for which the available SASL mechanisms are determined.
* @return an XML {@code <mechanisms>} element that lists all available SASL mechanisms for the given session,
* or {@code null} if no mechanisms are available and the configuration suppresses empty elements.
*/
public static Element getSASLMechanismsElement(@Nonnull final LocalIncomingServerSession session)
{
final Element result = DocumentHelper.createElement( new QName( "mechanisms", new Namespace( "", SASL_NAMESPACE ) ) );
if (session.isEncrypted()) {
final Connection connection = session.getConnection();
final TrustStore trustStore = connection.getConfiguration().getTrustStore();
final X509Certificate trusted = trustStore.getEndEntityCertificate( session.getConnection().getPeerCertificates() );

boolean haveTrustedCertificate = trusted != null;
if (trusted != null && session.getDefaultIdentity() != null) {
haveTrustedCertificate = verifyCertificate(trusted, session.getDefaultIdentity());
}
if (haveTrustedCertificate) {
// Offer SASL EXTERNAL only if TLS has already been negotiated and the peer has a trusted cert.
final Element mechanism = result.addElement("mechanism");
mechanism.setText("EXTERNAL");
}
}
final Set<String> availableMechanisms = getAvailableMechanismsForServerSession(session);

// OF-2072: Return null instead of an empty element, if so configured.
if ( JiveGlobals.getBooleanProperty("sasl.server.suppressEmpty", false) && result.elements().isEmpty() ) {
if (JiveGlobals.getBooleanProperty("sasl.server.suppressEmpty", false) && availableMechanisms.isEmpty()) {
return null;
}

final Element result = DocumentHelper.createElement( new QName("mechanisms", new Namespace("", SASL_NAMESPACE)) );
for (final String mech : availableMechanisms) {
final Element mechanism = result.addElement("mechanism");
mechanism.setText(mech);
}

return result;
}

Expand Down Expand Up @@ -376,6 +369,12 @@ public static Status handle(LocalSession session, Element doc)
throw new SaslFailureException( Failure.INVALID_MECHANISM, "The configuration of Openfire does not contain or allow the mechanism." );
}

// Enforce session-specific eligibility (as advertised in stream features) See OF-3273.
if ( !getAvailableMechanismsForSession( session ).contains( mechanismName ) )
{
throw new SaslFailureException( Failure.INVALID_MECHANISM, "The mechanism is not available for this session." );
}

// OF-477: The SASL implementation requires the fully qualified host name (not the domain name!) of this server,
// yet, most of the XMPP implemenations of DIGEST-MD5 will actually use the domain name. To account for that,
// here, we'll use the host name, unless DIGEST-MD5 is being negotiated!
Expand Down Expand Up @@ -453,27 +452,15 @@ else if (encoded.equals("="))
return Status.needResponse;
}

// Success!
if ( session instanceof LocalIncomingServerSession )
{
final LocalIncomingServerSession incomingServerSession = (LocalIncomingServerSession) session;

// Flag that indicates if certificates of the remote server should be validated.
final boolean verify = JiveGlobals.getBooleanProperty( ConnectionSettings.Server.TLS_CERTIFICATE_VERIFY, true );
if ( verify )
{
if ( verifyCertificates( incomingServerSession.getConnection().getPeerCertificates(), saslServer.getAuthorizationID(), true ) )
{
( (LocalIncomingServerSession) session ).setAuthenticationMethod(ServerSession.AuthenticationMethod.SASL_EXTERNAL);
}
else
{
throw new SaslFailureException( Failure.NOT_AUTHORIZED, "Server-to-Server certificate verification failed." );
}
}
if (saslServer.getAuthorizationID() != null && LockOutManager.getInstance().isAccountDisabled(saslServer.getAuthorizationID())) {
// Interception! This person is locked out, fail instead!
LockOutManager.getInstance().recordFailedLogin(saslServer.getAuthorizationID());
throw new SaslFailureException(Failure.ACCOUNT_DISABLED);
}

authenticationSuccessful( session, saslServer.getAuthorizationID(), challenge );
// Success! Any mechanism-specific verification (such as certificate checks for EXTERNAL) is
// performed by the SaslServer implementation.
authenticationSuccessful( session, saslServer.getAuthorizationID(), saslServer.getMechanismName(), challenge );
session.removeSessionData( "SaslServer" );
session.setSessionData("SaslMechanism", saslServer.getMechanismName());
Comment thread
guusdk marked this conversation as resolved.
if (saslServer.getMechanismName().endsWith("-PLUS")) {
Expand Down Expand Up @@ -551,16 +538,21 @@ private static void sendChallenge(Session session, byte[] challenge) {
sendElement(session, "challenge", challenge);
}

private static void authenticationSuccessful(LocalSession session, String username,
byte[] successData) {
if (username != null && LockOutManager.getInstance().isAccountDisabled(username)) {
// Interception! This person is locked out, fail instead!
LockOutManager.getInstance().recordFailedLogin(username);
authenticationFailed(session, Failure.ACCOUNT_DISABLED);
return;
}
/**
* Processes a successful SASL authentication.
*
* For client sessions, generates an authentication token. For inbound server sessions, marks the domain as
* validated and records the authentication method used.
*
* @param session the authenticated session (cannot be null).
* @param username the authorized identity from SASL (can be null for anonymous).
* @param mechanismName the name of the SASL mechanism that was used (cannot be null).
* @param successData mechanism-specific success data (can be null).
*/
@VisibleForTesting
static void authenticationSuccessful(LocalSession session, String username, String mechanismName, byte[] successData)
{
sendElement(session, "success", successData);
// We only support SASL for c2s
if (session instanceof ClientSession) {
final AuthToken authToken;
if (username == null) {
Expand All @@ -571,13 +563,11 @@ private static void authenticationSuccessful(LocalSession session, String userna
}
((LocalClientSession) session).setAuthToken(authToken);
}
else if (session instanceof IncomingServerSession) {
String hostname = username;
// Add the validated domain as a valid domain. The remote server can
// now send packets from this address
((LocalIncomingServerSession) session).addValidatedDomain(hostname);
((LocalIncomingServerSession) session).setAuthenticationMethod(ServerSession.AuthenticationMethod.SASL_EXTERNAL);
Log.info("Inbound Server {} authenticated (via TLS)", username);
else if (session instanceof LocalIncomingServerSession serverSession) {
// Add the validated domain as a valid domain. The remote server can now send packets from this address.
serverSession.addValidatedDomain(username);
serverSession.setAuthenticationMethod(ServerSession.AuthenticationMethod.fromSaslMechanismName(mechanismName));
Log.info("Inbound Server {} authenticated using SASL mechanism {}", username, mechanismName);
}
}

Expand Down Expand Up @@ -745,6 +735,29 @@ public static Set<String> getImplementedMechanisms()
return result;
}

/**
* Returns the set of SASL mechanisms available for the given session.
*
* @param session the session (cannot be null).
* @return a set of available mechanism names for the session (never null, possibly empty).
*/
@VisibleForTesting
static Set<String> getAvailableMechanismsForSession( final LocalSession session )
{
if ( session instanceof ClientSession )
{
return getAvailableMechanismsForClientSession( (ClientSession) session );
}
else if ( session instanceof LocalIncomingServerSession )
{
return getAvailableMechanismsForServerSession( (LocalIncomingServerSession) session );
}
else
{
return Collections.emptySet();
}
}

/**
* Returns a collection of SASL mechanism names that forms the source pool from which the mechanisms that are
* eventually being offered to peers are obtained.
Expand Down Expand Up @@ -791,4 +804,77 @@ private static void initMechanisms()
}
}
}

/**
* Determines and returns the set of SASL mechanisms that are available for a given client session. This includes
* mechanisms from the list of supported mechanisms, applying additional checks to ensure mechanism-specific
* requirements (e.g., encryption and certificate validation) are met.
*
* @param session The client session for which available SASL mechanisms need to be determined.
* Must not be null.
* @return A set of available SASL mechanism names for the specified client session.
* Will never be null but might be empty if no mechanisms are available.
*/
private static Set<String> getAvailableMechanismsForClientSession(@Nonnull final ClientSession session )
{
final Set<String> result = new HashSet<>();
for (String mech : getSupportedMechanisms()) {
if (mech.equals("EXTERNAL")) {
boolean trustedCert = false;
if (session.isEncrypted()) {
final Connection connection = ( (LocalClientSession) session ).getConnection();
assert connection != null; // While the client is performing a SASL negotiation, the connection can't be null.
if ( SKIP_PEER_CERT_REVALIDATION_CLIENT.getValue() ) {
// Trust that the peer certificate has been validated when TLS got established.
trustedCert = connection.getPeerCertificates() != null && connection.getPeerCertificates().length > 0;
} else {
// Re-evaluate the validity of the peer certificate.
final TrustStore trustStore = connection.getConfiguration().getTrustStore();
trustedCert = trustStore.isTrusted( connection.getPeerCertificates() );
}
}
if ( !trustedCert ) {
continue; // Do not offer EXTERNAL.
}
}
result.add(mech);
}
return result;
}

/**
* Determines the set of available SASL mechanisms for the given server session. This method checks the session's
* encryption status and examines the trust relationship to determine if specific mechanisms (such as SASL EXTERNAL)
* can be offered.
*
* @param session the server session for which the available mechanisms are to be determined.
* Must not be null.
* @return a set of SASL mechanism names that can be offered for the specified session.
* If no mechanisms are available, an empty set is returned.
*/
private static Set<String> getAvailableMechanismsForServerSession(@Nonnull final LocalIncomingServerSession session)
{
final Set<String> result = new HashSet<>();

// Check if EXTERNAL is enabled in the supported mechanisms configuration
if (!getSupportedMechanisms().contains("EXTERNAL")) {
return result;
}

if (session.isEncrypted()) {
final Connection connection = session.getConnection();
final TrustStore trustStore = connection.getConfiguration().getTrustStore();
final X509Certificate trusted = trustStore.getEndEntityCertificate( session.getConnection().getPeerCertificates() );

boolean haveTrustedCertificate = trusted != null;
if (trusted != null && session.getDefaultIdentity() != null) {
haveTrustedCertificate = verifyCertificate(trusted, session.getDefaultIdentity());
}
if (haveTrustedCertificate) {
// Offer SASL EXTERNAL only if TLS has already been negotiated and the peer has a trusted cert.
Comment thread
guusdk marked this conversation as resolved.
result.add("EXTERNAL");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return result;
}
}
Loading
Loading