From 94e4f459fdc4e481e69db9d0742f0f44d671173f Mon Sep 17 00:00:00 2001 From: Guus der Kinderen Date: Sat, 5 Sep 2026 22:18:26 +0200 Subject: [PATCH] OF-2534: Prevent null-session crash in SASL2 handling; log session-creation failures A session-creation failure that isn't a StreamErrorException can leave a StanzaHandler's session null while the connection is still processing pipelined stanzas, causing SASLAuthentication.handle() to NPE and escape uncaught (abortSasl2 dereferenced a null session with no guard). - abortSasl2: accept null session, log and return instead of throwing. - NettyConnectionHandler: skip dispatch if the connection is already closed. - StanzaHandler: log when a null session reaches SASL handling or session creation fails to assign one. - All StanzaHandler subclasses: catch and log RuntimeExceptions from their createSession() calls before rethrowing. Root trigger (Hazelcast stuck in SHUT_DOWN after a cluster-leave?) is not addressed here and needs separate investigation. --- .../openfire/net/ClientStanzaHandler.java | 10 +++++++++- .../openfire/net/ComponentStanzaHandler.java | 10 +++++++++- .../net/MultiplexerStanzaHandler.java | 14 +++++++++++--- .../net/RespondingServerStanzaHandler.java | 7 ++++++- .../openfire/net/SASLAuthentication.java | 7 ++++++- .../openfire/net/ServerStanzaHandler.java | 6 ++++++ .../openfire/net/StanzaHandler.java | 19 ++++++++++++++++++- .../openfire/nio/NettyConnectionHandler.java | 6 +++++- .../WebSocketClientStanzaHandler.java | 9 +++++++-- 9 files changed, 77 insertions(+), 11 deletions(-) diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java index 33654eb593..8cb4625b18 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java @@ -80,7 +80,15 @@ protected boolean validateJIDs() { protected void createSession(String serverName, XmlPullParser xpp, Connection connection) throws XmlPullParserException { // The connected client is a regular client so create a ClientSession - session = LocalClientSession.createSession(serverName, xpp, connection); + try { + session = LocalClientSession.createSession(serverName, xpp, connection); + } catch (final RuntimeException e) { + Log.warn("LocalClientSession.createSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } + if (session == null) { + Log.trace("LocalClientSession.createSession() returned null (no exception) for connection {}.", connection); + } } @Override diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java index 37b9bd2440..5a0e81000c 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java @@ -222,6 +222,14 @@ boolean validateJIDs() { void createSession(String serverName, XmlPullParser xpp, Connection connection) throws XmlPullParserException { // The connected client is a connection manager so create a ConnectionMultiplexerSession - session = LocalComponentSession.createSession(serverName, xpp, connection); + try { + session = LocalComponentSession.createSession(serverName, xpp, connection); + } catch (final RuntimeException e) { + Log.warn("LocalComponentSession.createSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } + if (session == null) { + Log.trace("LocalComponentSession.createSession() returned null (no exception) for connection {}.", connection); + } } } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/MultiplexerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/MultiplexerStanzaHandler.java index 2c5c737a6e..069041d6ac 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/MultiplexerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/MultiplexerStanzaHandler.java @@ -144,9 +144,17 @@ boolean validateJIDs() { void createSession(String serverName, XmlPullParser xpp, Connection connection) throws XmlPullParserException { // The connected client is a connection manager so create a ConnectionMultiplexerSession - session = LocalConnectionMultiplexerSession.createSession(serverName, xpp, connection); - if (session != null) { - packetHandler = new MultiplexerPacketHandler(session.getAddress().getDomain()); + try { + session = LocalConnectionMultiplexerSession.createSession(serverName, xpp, connection); + if (session != null) { + packetHandler = new MultiplexerPacketHandler(session.getAddress().getDomain()); + } + } catch (final RuntimeException e) { + Log.warn("LocalConnectionMultiplexerSession.createSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } + if (session == null) { + Log.trace("LocalConnectionMultiplexerSession.createSession() returned null (no exception) for connection {}.", connection); } } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java index 2783dfb390..58ea38fb4b 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java @@ -384,7 +384,12 @@ public CompletableFuture isSessionAuthenticated() { @Override void createSession(String serverName, XmlPullParser xpp, Connection connection) throws XmlPullParserException { String currentStreamId = xpp.getAttributeValue("", "id"); - session = createLocalOutgoingServerSession(currentStreamId, connection); + try { + session = createLocalOutgoingServerSession(currentStreamId, connection); + } catch (final RuntimeException e) { + LOG.warn("createLocalOutgoingServerSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } } /** 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 5de8850a03..232e2c3cf7 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/SASLAuthentication.java @@ -470,6 +470,7 @@ else if ( encoded.equals("=") ) */ public static Status handle(LocalSession session, Element doc, boolean usingSASL2) { + Log.trace("handle() invoked: usingSASL2={}, element={}, session={}", usingSASL2, doc.getName(), session); try { if (usingSASL2) @@ -989,8 +990,12 @@ private static FastToken issueFastToken(final String username, final String clie * @param session The LocalSession object representing the session. Must not be null. * @param failure The Failure object representing the reason for the authentication failure. Must not be null. */ - private static void abortSasl2(@Nonnull final LocalSession session, @Nonnull final Failure failure) + private static void abortSasl2(@Nullable final LocalSession session, @Nonnull final Failure failure) { + if (session == null) { + Log.warn("Unable to report SASL2 failure ({}): session is unexpectedly null.", failure); + return; + } if (session instanceof LocalClientSession clientSession) { clientSession.setAuthToken(null); } diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java index ee91d22a1a..3509c93347 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java @@ -129,6 +129,12 @@ void createSession(String serverName, XmlPullParser xpp, Connection connection) session = LocalIncomingServerSession.createSession(serverName, xpp, connection, this.directTLS, this.startedTLS); } catch (IOException e) { Log.error(e.getMessage(), e); + } catch (final RuntimeException e) { + Log.warn("LocalIncomingServerSession.createSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } + if (session == null) { + Log.trace("LocalIncomingServerSession.createSession() returned null (no exception) for connection {}.", connection); } } 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 32f03a616a..340a57ce46 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/net/StanzaHandler.java @@ -198,6 +198,7 @@ protected void processStanza(String stanza, XMPPPacketReader reader) throws Exce } return; } + // Ignore stanzas sent by clients if (stanza.startsWith(" for session {}.", authenticatingSession); saslStatus = SASLAuthentication.handle(authenticatingSession, doc, usingSASL2); if (saslStatus == SASLAuthentication.Status.authenticated && usingSASL2) { // No Bind2: send features synchronously now. @@ -274,6 +284,11 @@ else if ("auth".equals(tag)) { // User is responding to SASL challenge. Process response // See the 'authenticate' branch: an inline XEP-0198 resumption can replace this handler's session. final LocalSession authenticatingSession = session; + if (authenticatingSession == null) { + Log.warn("Dispatching SASL2 <{}> with a null session. sessionCreated={}, connection={}, stanza={}", tag, sessionCreated, connection, doc.asXML()); + } else { + Log.trace("Dispatching SASL2 <{}> for session {}.", tag, authenticatingSession); + } saslStatus = SASLAuthentication.handle(authenticatingSession, doc, usingSASL2); if (saslStatus == SASLAuthentication.Status.failed) { startedSASL = false; @@ -828,11 +843,13 @@ protected void createSession(XmlPullParser xpp) throws XmlPullParserException, I createSession(serverName, xpp, connection); if (session == null) { + Log.warn("createSession(serverName, xpp, connection) returned without assigning a session for connection: {}. Converting to a stream error.", connection); throw new StreamErrorException(StreamError.Condition.internal_server_error, "Unable to create a session."); } + Log.trace("Session created for connection {}: {}", connection, session); } catch (final StreamErrorException ex) { - Log.warn("Failed to create a session, as the stream opened by the peer has a problem: {} - '{}' (a full stack trace is logged on debug level). Closing connection: {}", ex.getStreamError().getCondition(), ex.getStreamError().getText(), connection); + Log.warn("Failed to create a session, as the stream opened by the peer has a problem: {} - '{}' (a full stack trace is logged on debug level). Closing connection: {}. session is now: {}", ex.getStreamError().getCondition(), ex.getStreamError().getText(), connection, session); Log.debug("Failed to create a session.", ex); final Element stream = DocumentHelper.createElement(QName.get("stream", "stream", "http://etherx.jabber.org/streams")); final Document document = DocumentHelper.createDocument(stream); diff --git a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnectionHandler.java b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnectionHandler.java index 7da7a95386..ef6ca05c69 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnectionHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/nio/NettyConnectionHandler.java @@ -159,6 +159,11 @@ public void handlerRemoved(ChannelHandlerContext ctx) { @Override public void channelRead0(ChannelHandlerContext ctx, String message) { + final Connection connection = ctx.channel().attr(CONNECTION).get(); + if (connection != null && connection.isClosed()) { + Log.warn("Processing message on {} whose connection is already closed. This can cascade into a null-session error downstream: {}", + ctx.channel().remoteAddress() == null ? ctx.channel().localAddress() : ctx.channel().localAddress() + "--" + ctx.channel().remoteAddress(), message); + } // Get the parser to use to process stanza. For optimization there is going // to be a parser for each running thread. Each Filter will be executed // by the Executor placed as the first Filter. So we can have a parser associated @@ -174,7 +179,6 @@ public void channelRead0(ChannelHandlerContext ctx, String message) { ctx.channel().attr(HANDLER).get().process(message, parser); } catch (Throwable e) { // Make sure to catch Throwable, not (only) Exception! See OF-2367 Log.error("Closing connection on {} due to error while processing message: {}", ctx.channel().remoteAddress() == null ? ctx.channel().localAddress() : ctx.channel().localAddress() + "--" + ctx.channel().remoteAddress(), message, e); - final Connection connection = ctx.channel().attr(CONNECTION).get(); if ( connection != null ) { connection.close(new StreamError(StreamError.Condition.internal_server_error, "An error occurred while processing data raw inbound data.")); } 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 a2feca77fc..767a90c36b 100644 --- a/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java +++ b/xmppserver/src/main/java/org/jivesoftware/openfire/websocket/WebSocketClientStanzaHandler.java @@ -148,8 +148,13 @@ protected void createSession(String serverName, XmlPullParser xpp, Connection co connection.setXMPPVersion(1, 0); // Create a ClientSession for this user. - session = SessionManager.getInstance().createClientSession(connection, language); - session.setSessionData("ws", Boolean.TRUE); + try { + session = SessionManager.getInstance().createClientSession(connection, language); + session.setSessionData("ws", Boolean.TRUE); + } catch (final RuntimeException e) { + Log.warn("SessionManager.getInstance().createClientSession() threw for connection {}. session remains: {}.", connection, session, e); + throw e; + } // RFC 7395 gives the same attributes as a stream header, including the (unverified) identity that the // peer claims. Record it before generating features: the advertised SASL mechanisms are derived from it.