Server-to-Server: IQHandler.process() - UnauthorizedException leads to NPE

Openfire Openfire Dev
1

The IQHandler.handleIQ()-method is allowed to throw a UnauthorizedException. The IQHandler.process() method catches this, creates an error IQ and sends the error IQ.

The problem: sessionManager.getSession(iq.getFrom()).process(response); is used the send the error IQ (line 73 and 87). But getSession() returns null for a client on a remote server of a Server-to-Server connection. Therefore, a NullPointerExeption occurs and fills the error log.

I would make a merge request, but I’m not sure, which of the following solution would be better

  1. deliverer.deliver(response);
  2. XMPPServer.getInstance().getRoutingTable().routePacket(iq.getFrom(), response, true);
  3. XMPPServer.getInstance().getPacketRouter().route(response);

Current method implementation for reference:

@Override
public void process(Packet packet) throws PacketException {
    IQ iq = (IQ) packet;
    try {
        IQ reply = handleIQ(iq);
        if (reply != null) {
            deliverer.deliver(reply);
        }
    }
    catch (org.jivesoftware.openfire.auth.UnauthorizedException e) {
        if (iq != null) {
            try {
                IQ response = IQ.createResultIQ(iq);
                response.setChildElement(iq.getChildElement().createCopy());
                response.setError(PacketError.Condition.not_authorized);
                sessionManager.getSession(iq.getFrom()).process(response);
            }
            catch (Exception de) {
                Log.error(LocaleUtils.getLocalizedString("admin.error"), de);
                sessionManager.getSession(iq.getFrom()).close();
            }
        }
    }
    catch (Exception e) {
        Log.error(LocaleUtils.getLocalizedString("admin.error"), e);
        try {
            IQ response = IQ.createResultIQ(iq);
            response.setChildElement(iq.getChildElement().createCopy());
            response.setError(PacketError.Condition.internal_server_error);
            sessionManager.getSession(iq.getFrom()).process(response);
        }
        catch (Exception e1) {
            // Do nothing
        }
    }
}

Error log:


2021.01.14 19:34:24.806 ERROR [Server SR - 1835632114] (org.jivesoftware.openfire.handler.IQHandler:76) - Internal server error
java.lang.NullPointerException: null
	at org.jivesoftware.openfire.handler.IQHandler.process(IQHandler.java:73) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.IQRouter.handle(IQRouter.java:385) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.IQRouter.route(IQRouter.java:118) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.spi.PacketRouterImpl.route(PacketRouterImpl.java:74) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.processIQ(SocketReader.java:274) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.ServerSocketReader.processIQ(ServerSocketReader.java:73) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.process(SocketReader.java:243) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.BlockingReadingMode.readStream(BlockingReadingMode.java:188) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.BlockingReadingMode.run(BlockingReadingMode.java:81) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.run(SocketReader.java:150) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_275]
2021.01.14 19:34:24.806 ERROR [Server SR - 1835632114] (org.jivesoftware.openfire.IQRouter:426) - Could not route packet
java.lang.NullPointerException: null
	at org.jivesoftware.openfire.handler.IQHandler.process(IQHandler.java:77) ~[xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.IQRouter.handle(IQRouter.java:385) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.IQRouter.route(IQRouter.java:118) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.spi.PacketRouterImpl.route(PacketRouterImpl.java:74) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.processIQ(SocketReader.java:274) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.ServerSocketReader.processIQ(ServerSocketReader.java:73) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.process(SocketReader.java:243) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.BlockingReadingMode.readStream(BlockingReadingMode.java:188) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.BlockingReadingMode.run(BlockingReadingMode.java:81) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at org.jivesoftware.openfire.net.SocketReader.run(SocketReader.java:150) [xmppserver-4.6.0-SNAPSHOT.jar:4.6.0-SNAPSHOT]
	at java.lang.Thread.run(Thread.java:748) [?:1.8.0_275]
2

Thanks for reporting this. I have created this issue in our bug tracker for this problem: https://igniterealtime.atlassian.net/browse/OF-2194

I would not use the third option that you provided, the PacketRouter, as that will treat the stanza as if it was a new stanza that started flowing through the system. Both 1 and 2 are acceptable (and pretty similar). The ‘fromServer’ boolean that distinguishes them only applies to local sessions, and only if the addressed JID is bare, it appears. This smells like some sort of hack that should be looked at. I would keep it simple, and go with the deliverer that’s part of the instance.