OF-2534: Prevent null-session crash in SASL2 handling; log session-creation failures

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

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);
+        }
     }
 }

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);
         }
     }
 

xmppserver/src/main/java/org/jivesoftware/openfire/net/RespondingServerStanzaHandler.java

@@ -384,7 +384,12 @@ public CompletableFuture<Void> 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;
+        }
     }
 
     /**

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);
         }

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);
         }
     }
 

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 <?xml version="1.0"?> stanzas sent by clients
         if (stanza.startsWith("<?xml")) {
             return;
@@ -230,6 +231,13 @@ protected void processStanza(String stanza, XMPPPacketReader reader) throws Exce
             // No document found.
             return;
         }
+
+        if (session == null) {
+            Log.warn("Processing data received on a connection that is associated to a null session. sessionCreated={}, connection={}, data={}", sessionCreated, connection, doc.asXML());
+        } else {
+            Log.trace("Processing data received on session {}, element name={}.", session, doc.getName());
+        }
+
         String tag = doc.getName();
         if ("starttls".equals(tag)) {
             // Negotiate TLS
@@ -238,6 +246,7 @@ protected void processStanza(String stanza, XMPPPacketReader reader) throws Exce
             }
             else {
                 connection.close();
+                Log.debug("TLS negotiation failed; session for connection {} is being cleared.", connection);
                 session = null;
             }
         }
@@ -254,6 +263,7 @@ else if ("auth".equals(tag)) {
             // Connection#reinit) replaces this handler's 'session' field before handle() returns. Retain the session
             // that is negotiating the authentication, as that is where the outcome of the negotiation is recorded.
             final LocalSession authenticatingSession = session;
+            Log.trace("Dispatching SASL2 <authenticate> 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);

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."));
             }

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 <open/> 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.