OF-946: Should not have unused arguments in ServerTrustManager

Two out of three arguments in the constructor of the Server Trust Manager
are unused. By deprecating them, calling code can be simplified.

src/java/org/jivesoftware/openfire/Connection.java

@@ -354,9 +354,33 @@ public interface Connection extends Closeable {
      *       otherwise a {@link org.jivesoftware.openfire.net.ServerTrustManager} will be used.
      * @param authentication policy to use for authenticating the remote peer.
      * @throws Exception if an error occured while securing the connection.
+     * @deprecated Use {@link #startTLS(boolean, boolean, ClientAuth)} instead, with isClientToServer = (remoteServer == null)
      */
+    @Deprecated
     void startTLS(boolean clientMode, String remoteServer, ClientAuth authentication) throws Exception;
 
+    /**
+     * Secures the plain connection by negotiating TLS with the other peer. In a server-2-server
+     * connection the server requesting the TLS negotiation will be the client and the other server
+     * will be the server during the TLS negotiation. Therefore, the server requesting the TLS
+     * negotiation must pass <code>true</code> in the <tt>clientMode</tt> parameter and the server
+     * receiving the TLS request must pass <code>false</code> in the <tt>clientMode</tt> parameter.
+     * Both servers should specify the xmpp domain of the other server in the <tt>remoteServer</tt>
+     * parameter.<p>
+     *
+     * In the case of client-2-server the XMPP server must pass <code>false</code> in the
+     * <tt>clientMode</tt> parameter since it will behave as the server in the TLS negotiation. The
+     * <tt>remoteServer</tt> parameter will always be <tt>null</tt>.
+     *
+     * @param clientMode boolean indicating if this entity is a client or a server in the TLS negotiation.
+     * @param isClientToServer indicates if the remote party is a server. When false a
+     *       {@link org.jivesoftware.openfire.net.ClientTrustManager} will be used for verifying certificates
+     *       otherwise a {@link org.jivesoftware.openfire.net.ServerTrustManager} will be used.
+     * @param authentication policy to use for authenticating the remote peer.
+     * @throws Exception if an error occured while securing the connection.
+     */
+    void startTLS(boolean clientMode, boolean isClientToServer, ClientAuth authentication) throws Exception;
+
     /**
      * Adds the compression filter to the connection but only filter incoming traffic. Do not filter
      * outgoing traffic since we still need to send an uncompressed stanza to the client indicating

src/java/org/jivesoftware/openfire/net/ClientStanzaHandler.java

@@ -116,6 +116,6 @@ public class ClientStanzaHandler extends StanzaHandler {
         } catch (IllegalArgumentException e) {
             policy = Connection.ClientAuth.disabled;
         }
-        connection.startTLS(false, null, policy);
+        connection.startTLS(false, true, policy);
     }
 }

src/java/org/jivesoftware/openfire/net/ClientTrustManager.java

@@ -75,10 +75,6 @@ public class ClientTrustManager implements X509TrustManager {
      * KeyStore that holds the trusted CA
      */
     private KeyStore trustStore;
-    /**
-     * Holds the domain of the remote server we are trying to connect
-     */
-    private String server;
 
     /**
      * Holds the CRL's to validate certs

src/java/org/jivesoftware/openfire/net/ComponentStanzaHandler.java

@@ -186,8 +186,7 @@ public class ComponentStanzaHandler extends StanzaHandler {
 
     @Override
 	void startTLS() throws Exception {
-        // TODO Finish implementation. We need to get the name of the CM if we want to validate certificates of the CM that requested TLS
-        connection.startTLS(false, "IMPLEMENT_ME", Connection.ClientAuth.disabled);
+        connection.startTLS(false, false, Connection.ClientAuth.disabled);
     }
 
     @Override

src/java/org/jivesoftware/openfire/net/MultiplexerStanzaHandler.java

@@ -151,7 +151,6 @@ public class MultiplexerStanzaHandler extends StanzaHandler {
 
     @Override
 	void startTLS() throws Exception {
-        // TODO Finish implementation. We need to get the name of the CM if we want to validate certificates of the CM that requested TLS
-        connection.startTLS(false, "IMPLEMENT_ME", Connection.ClientAuth.disabled);
+        connection.startTLS(false, false, Connection.ClientAuth.disabled);
     }
 }

src/java/org/jivesoftware/openfire/net/ServerStanzaHandler.java

@@ -107,12 +107,10 @@ public class ServerStanzaHandler extends StanzaHandler {
 
     @Override
 	void startTLS() throws Exception {
-        // TODO Finish implementation. We need to get the name of the remote server if we want to validate certificates of the remote server that requested TLS
-
         boolean needed = JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_CERTIFICATE_VERIFY, true) &&
                 JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_CERTIFICATE_CHAIN_VERIFY, true) &&
                 !JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_ACCEPT_SELFSIGNED_CERTS, false);
-        connection.startTLS(false, "IMPLEMENT_ME", needed ? Connection.ClientAuth.needed : Connection.ClientAuth.wanted);
+        connection.startTLS(false, false, needed ? Connection.ClientAuth.needed : Connection.ClientAuth.wanted);
     }
     @Override
 	protected void processIQ(IQ packet) throws UnauthorizedException {

src/java/org/jivesoftware/openfire/net/ServerTrustManager.java

@@ -54,20 +54,18 @@ public class ServerTrustManager implements X509TrustManager {
      * KeyStore that holds the trusted CA
      */
     private KeyStore trustStore;
+
     /**
-     * Holds the domain of the remote server we are trying to connect
-     */
-    private String server;
-    /**
-     * Holds the LocalIncomingServerSession that is part of the TLS negotiation.
+     * @deprecated Use ServerTrustManager(KeyStore trustStore) instead (there's no functional difference).
      */
-    private Connection connection;
+    @Deprecated
+    public ServerTrustManager(String server, KeyStore trustStore, Connection connection) {
+        this(trustStore);
+    }
 
-    public ServerTrustManager(String server, KeyStore trustTrust, Connection connection) {
+    public ServerTrustManager(KeyStore trustTrust) {
         super();
-        this.server = server;
         this.trustStore = trustTrust;
-        this.connection = connection;
     }
 
     @Override

src/java/org/jivesoftware/openfire/net/SocketConnection.java

@@ -166,8 +166,13 @@ public class SocketConnection implements Connection {
         return tlsStreamHandler;
     }
 
-    @Override
-    public void startTLS(boolean clientMode, String remoteServer, ClientAuth authentication) throws IOException {
+    @Deprecated
+    public void startTLS(boolean clientMode, String remoteServer, ClientAuth authentication) throws Exception {
+        final boolean isClientToServer = ( remoteServer == null );
+        startTLS( clientMode, isClientToServer, authentication );
+    }
+
+    public void startTLS(boolean clientMode, boolean isClientToServer, ClientAuth authentication) throws IOException {
         if (!secure) {
             secure = true;
             // Prepare for TLS
@@ -467,7 +472,7 @@ public class SocketConnection implements Connection {
 
     @Override
     public void close() {
-    	close(false);
+        close( false );
     }
 
     /**

src/java/org/jivesoftware/openfire/net/SocketReadingMode.java

@@ -81,8 +81,8 @@ abstract class SocketReadingMode {
         }
         // Client requested to secure the connection using TLS. Negotiate TLS.
         try {
-            // Temporary workaround to force the usage of ServerTrustManager. This code is only used for s2s
-            socketReader.connection.startTLS(false, "IMPLEMENT_ME", Connection.ClientAuth.disabled);
+            // This code is only used for s2s
+            socketReader.connection.startTLS(false, false, Connection.ClientAuth.disabled);
         }
         catch (IOException e) {
             Log.error("Error while negotiating TLS: " + socketReader.connection, e);

src/java/org/jivesoftware/openfire/net/TLSStreamHandler.java

@@ -87,25 +87,32 @@ public class TLSStreamHandler {
       */
     private static ByteBuffer hsBB = ByteBuffer.allocate(0);
 
+    /**
+     * @deprecated Use the other constructor. There's no functional change.
+     */
+    @Deprecated
+    public TLSStreamHandler(Connection connection, Socket socket, boolean clientMode, String remoteServer,
+                            boolean needClientAuth) throws IOException
+    {
+        this(socket,clientMode,(remoteServer==null),needClientAuth);
+    }
+
     /**
      * Creates a new TLSStreamHandler and secures the plain socket connection. When connecting
      * to a remote server then <tt>clientMode</tt> will be <code>true</code> and
      * <tt>remoteServer</tt> is the server name of the remote server. Otherwise <tt>clientMode</tt>
      * will be <code>false</code> and  <tt>remoteServer</tt> null.
      *
-     * @param connection the connection to secure
      * @param socket the plain socket connection to secure
      * @param clientMode boolean indicating if this entity is a client or a server.
-     * @param remoteServer server name of the remote server we are connecting to or <tt>null</tt>
-     *        when not in client mode.
+     * @param isClientToServer indicates if the remote party is a server.
      * @param needClientAuth boolean that indicates if client should authenticate during the TLS
      *        negotiation. This option is only required when the client is a server since
      *        EXTERNAL SASL is going to be used.
      * @throws java.io.IOException
      */
-    public TLSStreamHandler(Connection connection, Socket socket, boolean clientMode, String remoteServer,
-                            boolean needClientAuth) throws IOException {
-        wrapper = new TLSWrapper(connection, clientMode, needClientAuth, remoteServer);
+    public TLSStreamHandler(Socket socket, boolean clientMode, boolean isClientToServer, boolean needClientAuth) throws IOException {
+        wrapper = new TLSWrapper(clientMode, needClientAuth, isClientToServer);
         tlsEngine = wrapper.getTlsEngine();
         reader = new TLSStreamReader(wrapper, socket);
         writer = new TLSStreamWriter(wrapper, socket);

src/java/org/jivesoftware/openfire/net/TLSWrapper.java

@@ -64,8 +64,16 @@ public class TLSWrapper {
     private int netBuffSize;
     private int appBuffSize;
 
-    public TLSWrapper(Connection connection, boolean clientMode, boolean needClientAuth, String remoteServer) {
-        final boolean isClientToServer = (remoteServer == null);
+    /**
+     * @deprecated Use the other constructor. There's no functional change.
+     */
+    @Deprecated
+    public TLSWrapper(Connection connection, boolean clientMode, boolean needClientAuth, String remoteServer)
+    {
+        this(clientMode,needClientAuth,(remoteServer == null));
+    }
+
+    public TLSWrapper(boolean clientMode, boolean needClientAuth, boolean isClientToServer) {
 
         // Create/initialize the SSLContext with key material
         try {
@@ -88,7 +96,7 @@ public class TLSWrapper {
                 else
                 {
                     // Check if we can trust certificates presented by the server
-                    tm = new TrustManager[]{new ServerTrustManager(remoteServer, ksTrust, connection)};
+                    tm = new TrustManager[]{new ServerTrustManager(ksTrust)};
                 }
             }
             else

src/java/org/jivesoftware/openfire/nio/NIOConnection.java

@@ -53,9 +53,7 @@ import org.jivesoftware.openfire.auth.UnauthorizedException;
 import org.jivesoftware.openfire.keystore.IdentityStoreConfig;
 import org.jivesoftware.openfire.keystore.Purpose;
 import org.jivesoftware.openfire.keystore.TrustStoreConfig;
-import org.jivesoftware.openfire.net.ClientTrustManager;
-import org.jivesoftware.openfire.net.SSLConfig;
-import org.jivesoftware.openfire.net.ServerTrustManager;
+import org.jivesoftware.openfire.net.*;
 import org.jivesoftware.openfire.session.ConnectionSettings;
 import org.jivesoftware.openfire.session.LocalSession;
 import org.jivesoftware.openfire.session.Session;
@@ -76,6 +74,7 @@ public class NIOConnection implements Connection {
 
 	private static final Logger Log = LoggerFactory.getLogger(NIOConnection.class);
 
+
     private LocalSession session;
     private IoSession ioSession;
 
@@ -232,23 +231,21 @@ public class NIOConnection implements Connection {
 
     		if ( session != null ) {
                 session.setStatus( Session.STATUS_CLOSED );
-            }
+                }
 
             try {
-                deliverRawText( flashClient ? "</flash:stream>" : "</stream:stream>" );
+                            deliverRawText( flashClient ? "</flash:stream>" : "</stream:stream>" );
             } catch ( Exception e ) {
                 Log.error("Failed to deliver stream close tag: " + e.getMessage());
-            }
-            
+                }
+
             try {
-            	ioSession.close( true );
+                ioSession.close( true );
             } catch (Exception e) {
                 Log.error("Exception while closing MINA session", e);
             }
-        
             notifyCloseListeners(); // clean up session, etc.
-
-    	}
+        }
     }
 
     @Override
@@ -368,10 +365,14 @@ public class NIOConnection implements Connection {
         }
     }
 
-    @Override
+    @Deprecated
+	@Override
     public void startTLS(boolean clientMode, String remoteServer, ClientAuth authentication) throws Exception {
         final boolean isClientToServer = (remoteServer == null);
+        startTLS( clientMode, isClientToServer, authentication );
+    }
 
+    public void startTLS(boolean clientMode, boolean isClientToServer, ClientAuth authentication) throws Exception {
         Log.debug( "StartTLS: using {}", isClientToServer ? "c2s" : "s2s" );
 
         final SSLConfig sslConfig = SSLConfig.getInstance();
@@ -391,7 +392,7 @@ public class NIOConnection implements Connection {
                 tm = new TrustManager[]{new ClientTrustManager(ksTrust)};
             } else {
                 // Check if we can trust certificates presented by the server
-                tm = new TrustManager[]{new ServerTrustManager(remoteServer, ksTrust, this)};
+                tm = new TrustManager[]{new ServerTrustManager(ksTrust)};
             }
         } else {
             tm = storeConfig.getTrustManagers();

src/java/org/jivesoftware/openfire/session/LocalOutgoingServerSession.java

@@ -382,7 +382,7 @@ public class LocalOutgoingServerSession extends LocalServerSession implements Ou
                 boolean needed = JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_CERTIFICATE_VERIFY, true) &&
                         		 JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_CERTIFICATE_CHAIN_VERIFY, true) &&
                         		 !JiveGlobals.getBooleanProperty(ConnectionSettings.Server.TLS_ACCEPT_SELFSIGNED_CERTS, false);
-                connection.startTLS(true, hostname, needed ? Connection.ClientAuth.needed : Connection.ClientAuth.wanted);
+                connection.startTLS(true, false, needed ? Connection.ClientAuth.needed : Connection.ClientAuth.wanted);
             } catch(Exception e) {
                 log.debug("Got an exception whilst negotiating TLS: " + e.getMessage());
                 throw e;