OF-1004: Reduce encryption configuration complexity

My original implementation had options to both include as well as exclude specific
encryption protocols and cipher suites. This proved to be to much - Configuration
becomes a nightmare and setups ended up with having no enabled suites at all. This
commit simplifies things: a connection listener now gets a list of protocols and
suites that are enabled - that's it. When no setting is provided, the JVM default
is used (which itself is tweakable through generic Java configuration revolving
around the file JRE_HOME/lib/security/java.security )

src/java/org/jivesoftware/openfire/spi/ConnectionConfiguration.java

@@ -34,10 +34,8 @@ public class ConnectionConfiguration
     private final CertificateStoreConfiguration trustStoreConfiguration;
     private final boolean acceptSelfSignedCertificates;
     private final boolean verifyCertificateValidity;
-    private final Set<String> encryptionProtocolsEnabled;
-    private final Set<String> encryptionProtocolsDisabled;
-    private final Set<String> cipherSuitesEnabled;
-    private final Set<String> cipherSuitesDisabled;
+    private final Set<String> encryptionProtocols;
+    private final Set<String> encryptionCipherSuites;
     private final Connection.CompressionPolicy compressionPolicy;
 
     // derived
@@ -55,7 +53,7 @@ public class ConnectionConfiguration
      * @param tlsPolicy The TLS policy that is applied to connections (cannot be null).
      */
     // TODO input validation
-    public ConnectionConfiguration( ConnectionType type, boolean enabled, int maxThreadPoolSize, int maxBufferSize, Connection.ClientAuth clientAuth, InetAddress bindAddress, int port, Connection.TLSPolicy tlsPolicy, CertificateStoreConfiguration identityStoreConfiguration, CertificateStoreConfiguration trustStoreConfiguration, boolean acceptSelfSignedCertificates, boolean verifyCertificateValidity, Set<String> encryptionProtocolsEnabled, Set<String> encryptionProtocolsDisabled, Set<String> cipherSuitesEnabled, Set<String> cipherSuitesDisabled, Connection.CompressionPolicy compressionPolicy )
+    public ConnectionConfiguration( ConnectionType type, boolean enabled, int maxThreadPoolSize, int maxBufferSize, Connection.ClientAuth clientAuth, InetAddress bindAddress, int port, Connection.TLSPolicy tlsPolicy, CertificateStoreConfiguration identityStoreConfiguration, CertificateStoreConfiguration trustStoreConfiguration, boolean acceptSelfSignedCertificates, boolean verifyCertificateValidity, Set<String> encryptionProtocols, Set<String> encryptionCipherSuites, Connection.CompressionPolicy compressionPolicy )
     {
         if ( maxThreadPoolSize <= 0 ) {
             throw new IllegalArgumentException( "Argument 'maxThreadPoolSize' must be equal to or greater than one." );
@@ -76,21 +74,8 @@ public class ConnectionConfiguration
         this.trustStoreConfiguration = trustStoreConfiguration;
         this.acceptSelfSignedCertificates = acceptSelfSignedCertificates;
         this.verifyCertificateValidity = verifyCertificateValidity;
-
-        // Remove all disabled protocols from the enabled ones.
-        final Set<String> protocolsEnabled = new HashSet<>();
-        protocolsEnabled.addAll( encryptionProtocolsEnabled );
-        protocolsEnabled.removeAll( encryptionProtocolsDisabled );
-        this.encryptionProtocolsEnabled = Collections.unmodifiableSet( protocolsEnabled );
-        this.encryptionProtocolsDisabled = Collections.unmodifiableSet( encryptionProtocolsDisabled );
-
-        // Remove all disabled suites from the enabled ones.
-        final Set<String> suitesEnabled = new HashSet<>();
-        suitesEnabled.addAll( cipherSuitesEnabled );
-        suitesEnabled.removeAll( cipherSuitesDisabled );
-        this.cipherSuitesEnabled = Collections.unmodifiableSet( suitesEnabled );
-        this.cipherSuitesDisabled = Collections.unmodifiableSet( cipherSuitesDisabled );
-
+        this.encryptionProtocols = Collections.unmodifiableSet( encryptionProtocols );
+        this.encryptionCipherSuites = Collections.unmodifiableSet( encryptionCipherSuites );
         this.compressionPolicy = compressionPolicy;
 
         final CertificateStoreManager certificateStoreManager = XMPPServer.getInstance().getCertificateStoreManager();
@@ -175,66 +160,30 @@ public class ConnectionConfiguration
      * When non-empty, the list is intended to specify those protocols (from a larger collection of implementation-
      * supported protocols) that can be used to establish encryption.
      *
-     * Values returned by {@link #getEncryptionProtocolsDisabled()} are not included in the result of this method.
-     *
      * The order over which values are iterated in the result is equal to the order of values in the comma-separated
      * configuration string. This can, but is not guaranteed to, indicate preference.
      *
      * @return An (ordered) set of protocols, never null but possibly empty.
      */
-    public Set<String> getEncryptionProtocolsEnabled()
+    public Set<String> getEncryptionProtocols()
     {
-        return encryptionProtocolsEnabled;
-    }
-
-    /**
-     * A collection of protocols that must not be used for encryption of connections.
-     *
-     * When non-empty, the list is intended to specify those protocols (from a larger collection of implementation-
-     * supported protocols) that must not be used to establish encryption.
-     *
-     * The order over which values are iterated in the result is equal to the order of values in the comma-separated
-     * configuration string.
-     *
-     * @return An (ordered) set of protocols, never null but possibly empty.
-     */
-    public Set<String> getEncryptionProtocolsDisabled()
-    {
-        return encryptionProtocolsDisabled;
+        return encryptionProtocols;
     }
 
     /**
      * A collection of cipher suite names that can be used for encryption of connections.
      *
      * When non-empty, the list is intended to specify those cipher suites (from a larger collection of implementation-
-     * supported cipher suties) that can be used to establish encryption.
-     *
-     * Values returned by {@link #getCipherSuitesDisabled()} are not included in the result of this method.
+     * supported cipher suites) that can be used to establish encryption.
      *
      * The order over which values are iterated in the result is equal to the order of values in the comma-separated
      * configuration string. This can, but is not guaranteed to, indicate preference.
      *
      * @return An (ordered) set of cipher suites, never null but possibly empty.
      */
-    public Set<String> getCipherSuitesEnabled()
-    {
-        return cipherSuitesEnabled;
-    }
-
-    /**
-     * A collection of cipher suites that must not be used for encryption of connections.
-     *
-     * When non-empty, the list is intended to specify those cipher suites (from a larger collection of implementation-
-     * supported cipher suites) that must not be used to establish encryption.
-     *
-     * The order over which values are iterated in the result is equal to the order of values in the comma-separated
-     * configuration string.
-     *
-     * @return An (ordered) set of cipher suites, never null but possibly empty.
-     */
-    public Set<String> getCipherSuitesDisabled()
+    public Set<String> getEncryptionCipherSuites()
     {
-        return cipherSuitesDisabled;
+        return encryptionCipherSuites;
     }
 
     public IdentityStore getIdentityStore()

src/java/org/jivesoftware/openfire/spi/EncryptionArtifactFactory.java

@@ -9,6 +9,8 @@ import org.slf4j.LoggerFactory;
 import javax.net.ssl.*;
 import java.security.*;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Set;
 
 /**
@@ -109,49 +111,19 @@ public class EncryptionArtifactFactory
         final SSLEngine sslEngine = sslContext.createSSLEngine();
 
         // Configure protocol support.
-        final Set<String> protocolsEnabled = configuration.getEncryptionProtocolsEnabled();
-        if ( !protocolsEnabled.isEmpty() )
+        final Set<String> protocols = configuration.getEncryptionProtocols();
+        if ( !protocols.isEmpty() )
         {
-            // When an explicit list of enabled protocols is defined, use only those.
-            sslEngine.setEnabledProtocols( protocolsEnabled.toArray( new String[ protocolsEnabled.size() ] ) );
-        }
-        else
-        {
-            // Otherwise, use all supported protocols (except for the ones that are explicitly disabled).
-            final Set<String> disabled = configuration.getEncryptionProtocolsDisabled();
-            final ArrayList<String> supported = new ArrayList<>();
-            for ( final String candidate : sslEngine.getSupportedProtocols() )
-            {
-                if ( !disabled.contains( candidate ) )
-                {
-                    supported.add( candidate );
-                }
-            }
-
-            sslEngine.setEnabledProtocols( supported.toArray( new String[ supported.size()] ) );
+            // When an explicit list of enabled protocols is defined, use only those (otherwise, an implementation-specific default will be used).
+            sslEngine.setEnabledProtocols( protocols.toArray( new String[ protocols.size() ] ) );
         }
 
         // Configure cipher suite support.
-        final Set<String> cipherSuitesEnabled = configuration.getCipherSuitesEnabled();
-        if ( !cipherSuitesEnabled.isEmpty() )
-        {
-            // When an explicit list of enabled protocols is defined, use only those.
-            sslEngine.setEnabledCipherSuites( cipherSuitesEnabled.toArray( new String[ cipherSuitesEnabled.size() ] ) );
-        }
-        else
+        final Set<String> cipherSuites = configuration.getEncryptionCipherSuites();
+        if ( !cipherSuites.isEmpty() )
         {
-            // Otherwise, use all supported cipher suites (except for the ones that are explicitly disabled).
-            final Set<String> disabled = configuration.getCipherSuitesDisabled();
-            final ArrayList<String> supported = new ArrayList<>();
-            for ( final String candidate : sslEngine.getSupportedCipherSuites() )
-            {
-                if ( !disabled.contains( candidate ) )
-                {
-                    supported.add( candidate );
-                }
-            }
-
-            sslEngine.setEnabledCipherSuites( supported.toArray( new String[ supported.size() ] ) );
+            // When an explicit list of enabled protocols is defined, use only those (otherwise, an implementation-specific default will be used)..
+            sslEngine.setEnabledCipherSuites( cipherSuites.toArray( new String[ cipherSuites.size() ] ) );
         }
 
         return sslEngine;
@@ -221,20 +193,20 @@ public class EncryptionArtifactFactory
             sslContextFactory.setKeyStorePassword( new String( configuration.getIdentityStore().getConfiguration().getPassword() ) );
 
             // Configure protocol support
-            if ( configuration.getEncryptionProtocolsEnabled() != null && !configuration.getEncryptionProtocolsEnabled().isEmpty() )
+            final Set<String> protocols = configuration.getEncryptionProtocols();
+            if ( !protocols.isEmpty() )
             {
-                sslContextFactory.setIncludeProtocols( configuration.getEncryptionProtocolsEnabled().toArray( new String[ configuration.getEncryptionProtocolsEnabled().size() ] ) );
+                sslContextFactory.setIncludeProtocols( protocols.toArray( new String[ protocols.size() ] ) );
             }
-            sslContextFactory.setExcludeProtocols( configuration.getEncryptionProtocolsDisabled().toArray( new String[ configuration.getEncryptionProtocolsDisabled().size() ] ) );
 
             // Configure cipher suite support.
-            if ( configuration.getCipherSuitesEnabled() != null && !configuration.getCipherSuitesEnabled().isEmpty() )
+            final Set<String> cipherSuites = configuration.getEncryptionCipherSuites();
+            if ( !cipherSuites.isEmpty() )
             {
-                sslContextFactory.setIncludeCipherSuites( configuration.getCipherSuitesEnabled().toArray( new String[ configuration.getCipherSuitesEnabled().size() ] ) );
+                sslContextFactory.setIncludeCipherSuites( cipherSuites.toArray( new String[ cipherSuites.size() ] ) );
             }
-            sslContextFactory.setExcludeCipherSuites( configuration.getCipherSuitesDisabled().toArray( new String[ configuration.getCipherSuitesDisabled().size() ] ) );
 
-            //Set policy for checking client certificates
+            // Set policy for checking client certificates.
             switch ( configuration.getClientAuth() )
             {
                 case disabled:
@@ -331,12 +303,25 @@ public class EncryptionArtifactFactory
      *
      * @return An array of protocol names. Not expected to be empty.
      */
-    public static String[] getSupportedProtocols() throws NoSuchAlgorithmException, KeyManagementException
+    public static List<String> getSupportedProtocols() throws NoSuchAlgorithmException, KeyManagementException
     {
         // TODO Might want to cache the result. It's unlikely to change at runtime.
         final SSLContext context = SSLContext.getInstance( "TLSv1" );
         context.init( null, null, null );
-        return context.createSSLEngine().getSupportedProtocols();
+        return Arrays.asList( context.createSSLEngine().getSupportedProtocols() );
+    }
+
+    /**
+     * Returns the names of all encryption protocols that are enabled by default.
+     *
+     * @return An array of protocol names. Not expected to be empty.
+     */
+    public static List<String> getDefaultProtocols() throws NoSuchAlgorithmException, KeyManagementException
+    {
+        // TODO Might want to cache the result. It's unlikely to change at runtime.
+        final SSLContext context = SSLContext.getInstance( "TLSv1" );
+        context.init( null, null, null );
+        return Arrays.asList( context.createSSLEngine().getEnabledProtocols() );
     }
 
     /**
@@ -344,11 +329,25 @@ public class EncryptionArtifactFactory
      *
      * @return An array of cipher suite names. Not expected to be empty.
      */
-    public static String[] getSupportedCipherSuites() throws NoSuchAlgorithmException, KeyManagementException
+    public static List<String> getSupportedCipherSuites() throws NoSuchAlgorithmException, KeyManagementException
     {
         // TODO Might want to cache the result. It's unlikely to change at runtime.
         final SSLContext context = SSLContext.getInstance( "TLSv1" );
         context.init( null, null, null );
-        return context.createSSLEngine().getSupportedCipherSuites();
+        return Arrays.asList( context.createSSLEngine().getSupportedCipherSuites() );
     }
+
+    /**
+     * Returns the names of all encryption cipher suites that are enabled by default.
+     *
+     * @return An array of cipher suite names. Not expected to be empty.
+     */
+    public static List<String> getDefaultCipherSuites() throws NoSuchAlgorithmException, KeyManagementException
+    {
+        // TODO Might want to cache the result. It's unlikely to change at runtime.
+        final SSLContext context = SSLContext.getInstance( "TLSv1" );
+        context.init( null, null, null );
+        return Arrays.asList( context.createSSLEngine().getEnabledCipherSuites() );
+    }
+
 }

src/test/java/org/jivesoftware/openfire/spi/EncryptionArtifactFactoryTest.java

@@ -3,6 +3,8 @@ package org.jivesoftware.openfire.spi;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.Collection;
+
 /**
  * Unit tests that verify the functionality of {@link EncryptionArtifactFactory}.
  *
@@ -20,10 +22,26 @@ public class EncryptionArtifactFactoryTest
         // (not needed)
 
         // Execute system under test.
-        final String[] result = EncryptionArtifactFactory.getSupportedProtocols();
+        final Collection<String> result = EncryptionArtifactFactory.getSupportedProtocols();
+
+        // Verify results.
+        Assert.assertFalse( result.isEmpty() );
+    }
+
+    /**
+     * Verifies that the collection of default encryption protocols is not empty.
+     */
+    @Test
+    public void testHasDefaultProtocols() throws Exception
+    {
+        // Setup fixture.
+        // (not needed)
+
+        // Execute system under test.
+        final Collection<String> result = EncryptionArtifactFactory.getDefaultProtocols();
 
         // Verify results.
-        Assert.assertTrue( result.length > 0 );
+        Assert.assertFalse( result.isEmpty() );
     }
 
     /**
@@ -36,9 +54,25 @@ public class EncryptionArtifactFactoryTest
         // (not needed)
 
         // Execute system under test.
-        final String[] result = EncryptionArtifactFactory.getSupportedCipherSuites();
+        final Collection<String> result = EncryptionArtifactFactory.getSupportedCipherSuites();
+
+        // Verify results.
+        Assert.assertFalse( result.isEmpty() );
+    }
+
+    /**
+     * Verifies that the collection of default cipher suites is not empty.
+     */
+    @Test
+    public void testHasDefaultCipherSuites() throws Exception
+    {
+        // Setup fixture.
+        // (not needed)
+
+        // Execute system under test.
+        final Collection<String> result = EncryptionArtifactFactory.getDefaultCipherSuites();
 
         // Verify results.
-        Assert.assertTrue( result.length > 0 );
+        Assert.assertFalse( result.isEmpty() );
     }
 }