This is an automated email from the ASF dual-hosted git repository.

johnnyv pushed a commit to branch FTPSERVER-491
in repository https://gitbox.apache.org/repos/asf/mina-ftpserver.git


The following commit(s) were added to refs/heads/FTPSERVER-491 by this push:
     new 36cd856  adds NULL checks for SslConfigurationFactory
36cd856 is described below

commit 36cd8561dd198057773454cbc857a8fa26ef81a5
Author: johnnyv <john...@apache.org>
AuthorDate: Wed Nov 6 20:30:54 2019 -0500

    adds NULL checks for SslConfigurationFactory
---
 .../ftpserver/ssl/SslConfigurationFactory.java     | 259 ++++++++++-----------
 .../apache/ftpserver/ssl/MinaCipherSuitesTest.java |  52 ++---
 2 files changed, 144 insertions(+), 167 deletions(-)

diff --git 
a/core/src/main/java/org/apache/ftpserver/ssl/SslConfigurationFactory.java 
b/core/src/main/java/org/apache/ftpserver/ssl/SslConfigurationFactory.java
index c36ca3b..a590148 100644
--- a/core/src/main/java/org/apache/ftpserver/ssl/SslConfigurationFactory.java
+++ b/core/src/main/java/org/apache/ftpserver/ssl/SslConfigurationFactory.java
@@ -36,15 +36,13 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Used to configure the SSL settings for the control channel or the data
- * channel.
+ * Used to configure the SSL settings for the control channel or the data 
channel.
  *
  * @author <a href="http://mina.apache.org";>Apache MINA Project</a>
  */
 public class SslConfigurationFactory {
 
-    private final Logger LOG = LoggerFactory
-            .getLogger(SslConfigurationFactory.class);
+    private final Logger LOG = 
LoggerFactory.getLogger(SslConfigurationFactory.class);
 
     private File keystoreFile = new File("./res/.keystore");
 
@@ -78,7 +76,7 @@ public class SslConfigurationFactory {
      * @return The key store file
      */
     public File getKeystoreFile() {
-        return keystoreFile;
+       return keystoreFile;
     }
 
     /**
@@ -88,7 +86,9 @@ public class SslConfigurationFactory {
      *            A path to an existing key store file
      */
     public void setKeystoreFile(File keyStoreFile) {
-        this.keystoreFile = keyStoreFile;
+       if (keyStoreFile == null || keyStoreFile.length() == 0)
+           throw new FtpServerConfigurationException("KeystoreFile must not be 
null or zero length");
+       this.keystoreFile = keyStoreFile;
     }
 
     /**
@@ -97,7 +97,7 @@ public class SslConfigurationFactory {
      * @return The password
      */
     public String getKeystorePassword() {
-        return keystorePass;
+       return keystorePass;
     }
 
     /**
@@ -107,7 +107,7 @@ public class SslConfigurationFactory {
      *            The password
      */
     public void setKeystorePassword(String keystorePass) {
-        this.keystorePass = keystorePass;
+       this.keystorePass = keystorePass;
     }
 
     /**
@@ -116,7 +116,7 @@ public class SslConfigurationFactory {
      * @return The key store type
      */
     public String getKeystoreType() {
-        return keystoreType;
+       return keystoreType;
     }
 
     /**
@@ -126,7 +126,9 @@ public class SslConfigurationFactory {
      *            The key store type
      */
     public void setKeystoreType(String keystoreType) {
-        this.keystoreType = keystoreType;
+       if (keystoreType == null || keystoreType.length() == 0)
+           throw new FtpServerConfigurationException("KeystoreType must not be 
null or zero length");
+       this.keystoreType = keystoreType;
     }
 
     /**
@@ -135,7 +137,7 @@ public class SslConfigurationFactory {
      * @return The key store algorithm
      */
     public String getKeystoreAlgorithm() {
-        return keystoreAlgorithm;
+       return keystoreAlgorithm;
     }
 
     /**
@@ -145,49 +147,48 @@ public class SslConfigurationFactory {
      *            The key store algorithm
      */
     public void setKeystoreAlgorithm(String keystoreAlgorithm) {
-        this.keystoreAlgorithm = keystoreAlgorithm;
+       if (keystoreAlgorithm == null || keystoreAlgorithm.length() == 0)
+           throw new FtpServerConfigurationException("KeystoreAlgorithm must 
not be null or zero length");
+       this.keystoreAlgorithm = keystoreAlgorithm;
 
     }
 
     /**
-     * The SSL protocol used for this channel. Supported values are "SSL" and
-     * "TLS". Defaults to "TLS".
+     * The SSL protocol used for this channel. Supported values are "SSL" and 
"TLS". Defaults to "TLS".
      * 
      * @return The SSL protocol
      */
     public String getSslProtocol() {
-        return sslProtocol;
+       return sslProtocol;
     }
 
     /**
-     * Set the SSL protocol used for this channel. Supported values are "SSL"
-     * and "TLS". Defaults to "TLS".
+     * Set the SSL protocol used for this channel. Supported values are "SSL" 
and "TLS". Defaults to "TLS".
      * 
      * @param sslProtocol
      *            The SSL protocol
      */
     public void setSslProtocol(String sslProtocol) {
-        this.sslProtocol = sslProtocol;
+       if (sslProtocol == null || sslProtocol.length() == 0)
+           throw new FtpServerConfigurationException("SslProcotol must not be 
null or zero length");
+       this.sslProtocol = sslProtocol;
     }
 
     /**
-     * Set what client authentication level to use, supported values are "yes"
-     * or "true" for required authentication, "want" for wanted authentication
-     * and "false" or "none" for no authentication. Defaults to "none".
+     * Set what client authentication level to use, supported values are "yes" 
or "true" for required authentication,
+     * "want" for wanted authentication and "false" or "none" for no 
authentication. Defaults to "none".
      * 
      * @param clientAuthReqd
      *            The desired authentication level
      */
     public void setClientAuthentication(String clientAuthReqd) {
-        if ("true".equalsIgnoreCase(clientAuthReqd)
-                || "yes".equalsIgnoreCase(clientAuthReqd)
-                || "need".equalsIgnoreCase(clientAuthReqd)) {
-            this.clientAuth = ClientAuth.NEED;
-        } else if ("want".equalsIgnoreCase(clientAuthReqd)) {
-            this.clientAuth = ClientAuth.WANT;
-        } else {
-            this.clientAuth = ClientAuth.NONE;
-        }
+       if ("true".equalsIgnoreCase(clientAuthReqd) || 
"yes".equalsIgnoreCase(clientAuthReqd) || 
"need".equalsIgnoreCase(clientAuthReqd)) {
+           this.clientAuth = ClientAuth.NEED;
+       } else if ("want".equalsIgnoreCase(clientAuthReqd)) {
+           this.clientAuth = ClientAuth.WANT;
+       } else {
+           this.clientAuth = ClientAuth.NONE;
+       }
     }
 
     /**
@@ -196,7 +197,7 @@ public class SslConfigurationFactory {
      * @return The password
      */
     public String getKeyPassword() {
-        return keyPass;
+       return keyPass;
     }
 
     /**
@@ -206,15 +207,16 @@ public class SslConfigurationFactory {
      *            The password
      */
     public void setKeyPassword(String keyPass) {
-        this.keyPass = keyPass;
+       this.keyPass = keyPass;
     }
 
     /**
      * Get the file used to load the truststore
+     * 
      * @return The {@link File} containing the truststore
      */
     public File getTruststoreFile() {
-        return trustStoreFile;
+       return trustStoreFile;
     }
 
     /**
@@ -224,7 +226,7 @@ public class SslConfigurationFactory {
      *            The password
      */
     public void setTruststoreFile(File trustStoreFile) {
-        this.trustStoreFile = trustStoreFile;
+       this.trustStoreFile = trustStoreFile;
     }
 
     /**
@@ -233,7 +235,7 @@ public class SslConfigurationFactory {
      * @return The password
      */
     public String getTruststorePassword() {
-        return trustStorePass;
+       return trustStorePass;
     }
 
     /**
@@ -243,7 +245,7 @@ public class SslConfigurationFactory {
      *            The password
      */
     public void setTruststorePassword(String trustStorePass) {
-        this.trustStorePass = trustStorePass;
+       this.trustStorePass = trustStorePass;
     }
 
     /**
@@ -252,7 +254,7 @@ public class SslConfigurationFactory {
      * @return The trust store type
      */
     public String getTruststoreType() {
-        return trustStoreType;
+       return trustStoreType;
     }
 
     /**
@@ -262,7 +264,7 @@ public class SslConfigurationFactory {
      *            The trust store type
      */
     public void setTruststoreType(String trustStoreType) {
-        this.trustStoreType = trustStoreType;
+       this.trustStoreType = trustStoreType;
     }
 
     /**
@@ -271,7 +273,7 @@ public class SslConfigurationFactory {
      * @return The trust store algorithm
      */
     public String getTruststoreAlgorithm() {
-        return trustStoreAlgorithm;
+       return trustStoreAlgorithm;
     }
 
     /**
@@ -281,130 +283,111 @@ public class SslConfigurationFactory {
      *            The trust store algorithm
      */
     public void setTruststoreAlgorithm(String trustStoreAlgorithm) {
-        this.trustStoreAlgorithm = trustStoreAlgorithm;
+       this.trustStoreAlgorithm = trustStoreAlgorithm;
 
     }
 
-    private KeyStore loadStore(File storeFile, String storeType,
-            String storePass) throws IOException, GeneralSecurityException {
-        InputStream fin = null;
-        try {
-            if(storeFile.exists()) {
-                LOG.debug("Trying to load store from file");
-                fin = new FileInputStream(storeFile);
-            } else {
-                LOG.debug("Trying to load store from classpath");
-                fin = 
getClass().getClassLoader().getResourceAsStream(storeFile.getPath());
-                
-                if(fin == null) {
-                    throw new FtpServerConfigurationException("Key store could 
not be loaded from " + storeFile.getPath());
-                }
-            }
-            
-            KeyStore store = KeyStore.getInstance(storeType);
-            store.load(fin, storePass.toCharArray());
-
-            return store;
-        } finally {
-            IoUtils.close(fin);
-        }
+    private KeyStore loadStore(File storeFile, String storeType, String 
storePass) throws IOException, GeneralSecurityException {
+       InputStream fin = null;
+       try {
+           if (storeFile.exists()) {
+               LOG.debug("Trying to load store from file");
+               fin = new FileInputStream(storeFile);
+           } else {
+               LOG.debug("Trying to load store from classpath");
+               fin = 
getClass().getClassLoader().getResourceAsStream(storeFile.getPath());
+
+               if (fin == null) {
+                   throw new FtpServerConfigurationException("Key store could 
not be loaded from " + storeFile.getPath());
+               }
+           }
+
+           KeyStore store = KeyStore.getInstance(storeType);
+           store.load(fin, storePass.toCharArray());
+
+           return store;
+       } finally {
+           IoUtils.close(fin);
+       }
     }
 
     /**
-     * Create an instance of {@link SslConfiguration} based on the 
configuration
-     * of this factory.
+     * Create an instance of {@link SslConfiguration} based on the 
configuration of this factory.
+     * 
      * @return The {@link SslConfiguration} instance
      */
     public SslConfiguration createSslConfiguration() {
 
-        try {
-            // initialize keystore
-            LOG
-                    .debug(
-                            "Loading key store from \"{}\", using the key 
store type \"{}\"",
-                            keystoreFile.getAbsolutePath(), keystoreType);
-            KeyStore keyStore = loadStore(keystoreFile, keystoreType,
-                    keystorePass);
-
-            KeyStore trustStore;
-            if (trustStoreFile != null) {
-                LOG
-                        .debug(
-                                "Loading trust store from \"{}\", using the 
key store type \"{}\"",
-                                trustStoreFile.getAbsolutePath(),
-                                trustStoreType);
-                trustStore = loadStore(trustStoreFile, trustStoreType,
-                        trustStorePass);
-            } else {
-                trustStore = keyStore;
-            }
-
-            String keyPassToUse;
-            if (keyPass == null) {
-                keyPassToUse = keystorePass;
-            } else {
-                keyPassToUse = keyPass;
-            }
-            // initialize key manager factory
-            KeyManagerFactory keyManagerFactory = KeyManagerFactory
-                    .getInstance(keystoreAlgorithm);
-            keyManagerFactory.init(keyStore, keyPassToUse.toCharArray());
-
-            // initialize trust manager factory
-            TrustManagerFactory trustManagerFactory = TrustManagerFactory
-                    .getInstance(trustStoreAlgorithm);
-            trustManagerFactory.init(trustStore);
-            
-            return new DefaultSslConfiguration(
-                    keyManagerFactory, trustManagerFactory, 
-                    clientAuth, sslProtocol, 
-                    enabledCipherSuites, keyAlias);
-        } catch (Exception ex) {
-            LOG.error("DefaultSsl.configure()", ex);
-            throw new FtpServerConfigurationException("DefaultSsl.configure()",
-                    ex);
-        }
+       try {
+           // initialize keystore
+           LOG.debug("Loading key store from \"{}\", using the key store type 
\"{}\"", keystoreFile.getAbsolutePath(), keystoreType);
+           KeyStore keyStore = loadStore(keystoreFile, keystoreType, 
keystorePass);
+
+           KeyStore trustStore;
+           if (trustStoreFile != null) {
+               LOG.debug("Loading trust store from \"{}\", using the key store 
type \"{}\"", trustStoreFile.getAbsolutePath(), trustStoreType);
+               trustStore = loadStore(trustStoreFile, trustStoreType, 
trustStorePass);
+           } else {
+               trustStore = keyStore;
+           }
+
+           String keyPassToUse;
+           if (keyPass == null) {
+               keyPassToUse = keystorePass;
+           } else {
+               keyPassToUse = keyPass;
+           }
+           // initialize key manager factory
+           KeyManagerFactory keyManagerFactory = 
KeyManagerFactory.getInstance(keystoreAlgorithm);
+           keyManagerFactory.init(keyStore, keyPassToUse.toCharArray());
+
+           // initialize trust manager factory
+           TrustManagerFactory trustManagerFactory = 
TrustManagerFactory.getInstance(trustStoreAlgorithm);
+           trustManagerFactory.init(trustStore);
+
+           return new DefaultSslConfiguration(keyManagerFactory, 
trustManagerFactory, clientAuth, sslProtocol, enabledCipherSuites, keyAlias);
+       } catch (Exception ex) {
+           LOG.error("DefaultSsl.configure()", ex);
+           throw new FtpServerConfigurationException("DefaultSsl.configure()", 
ex);
+       }
     }
 
     /**
      * Return the required client authentication setting
      * 
-     * @return {@link ClientAuth#NEED} if client authentication is required,
-     *         {@link ClientAuth#WANT} is client authentication is wanted or
-     *         {@link ClientAuth#NONE} if no client authentication is the be
-     *         performed
+     * @return {@link ClientAuth#NEED} if client authentication is required, 
{@link ClientAuth#WANT} is client
+     *         authentication is wanted or {@link ClientAuth#NONE} if no 
client authentication is the be performed
      */
     public ClientAuth getClientAuth() {
-        return clientAuth;
+       return clientAuth;
     }
 
     /**
-     * Returns the cipher suites that should be enabled for this connection.
-     * Must return null if the default (as decided by the JVM) cipher suites
-     * should be used.
+     * Returns the cipher suites that should be enabled for this connection. 
Must return null if the default (as decided
+     * by the JVM) cipher suites should be used.
      * 
      * @return An array of cipher suites, or null.
      */
     public String[] getEnabledCipherSuites() {
-        if (enabledCipherSuites != null) {
-            return enabledCipherSuites.clone();
-        } else {
-            return null;
-        }
+       if (enabledCipherSuites != null) {
+           return enabledCipherSuites.clone();
+       } else {
+           return null;
+       }
     }
 
     /**
-     * Set the allowed cipher suites, note that the exact list of supported
-     * cipher suites differs between JRE implementations.
+     * Set the allowed cipher suites, note that the exact list of supported 
cipher suites differs between JRE
+     * implementations.
      * 
      * @param enabledCipherSuites
      */
     public void setEnabledCipherSuites(String[] enabledCipherSuites) {
-        if (enabledCipherSuites != null) {
-            this.enabledCipherSuites = enabledCipherSuites.clone();
-        } else {
-            this.enabledCipherSuites = null;
-        }
+       if (enabledCipherSuites != null) {
+           this.enabledCipherSuites = enabledCipherSuites.clone();
+       } else {
+           this.enabledCipherSuites = null;
+       }
     }
 
     /**
@@ -413,19 +396,17 @@ public class SslConfigurationFactory {
      * @return The alias, or null if none is set
      */
     public String getKeyAlias() {
-        return keyAlias;
+       return keyAlias;
     }
 
     /**
-     * Set the alias for the key to be used for SSL communication. If the
-     * specified key store contains multiple keys, this alias can be set to
-     * select a specific key.
+     * Set the alias for the key to be used for SSL communication. If the 
specified key store contains multiple keys,
+     * this alias can be set to select a specific key.
      * 
      * @param keyAlias
-     *            The alias to use, or null if JSSE should be allowed to choose
-     *            the key.
+     *            The alias to use, or null if JSSE should be allowed to 
choose the key.
      */
     public void setKeyAlias(String keyAlias) {
-        this.keyAlias = keyAlias;
+       this.keyAlias = keyAlias;
     }
 }
diff --git 
a/core/src/test/java/org/apache/ftpserver/ssl/MinaCipherSuitesTest.java 
b/core/src/test/java/org/apache/ftpserver/ssl/MinaCipherSuitesTest.java
index cd78b4b..aeda3fa 100644
--- a/core/src/test/java/org/apache/ftpserver/ssl/MinaCipherSuitesTest.java
+++ b/core/src/test/java/org/apache/ftpserver/ssl/MinaCipherSuitesTest.java
@@ -24,64 +24,60 @@ import javax.net.ssl.SSLHandshakeException;
 import org.apache.commons.net.ftp.FTPSClient;
 
 /**
-*
-* @author <a href="http://mina.apache.org";>Apache MINA Project</a>
-*
-*/
+ *
+ * @author <a href="http://mina.apache.org";>Apache MINA Project</a>
+ *
+ */
 public class MinaCipherSuitesTest extends SSLTestTemplate {
 
     @Override
     protected String getAuthValue() {
-        return "TLS";
+       return "TLS";
     }
 
     @Override
     protected boolean useImplicit() {
-        return true;
+       return true;
     }
 
     @Override
     protected SslConfigurationFactory createSslConfiguration() {
-        SslConfigurationFactory sslConfigFactory = 
super.createSslConfiguration();
+       SslConfigurationFactory sslConfigFactory = 
super.createSslConfiguration();
 
-        sslConfigFactory
-        .setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA" });
+       sslConfigFactory.setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA" });
 
-        return sslConfigFactory;
+       return sslConfigFactory;
     }
-    
+
     @Override
     protected FTPSClient createFTPClient() throws Exception {
-        return new FTPSClient(true);
+       return new FTPSClient(true);
     }
 
     @Override
     protected boolean isConnectClient() {
-        return false;
+       return false;
     }
 
     /*
-     * Only certain cipher suites will work with the keys and protocol we're
-     * using for this test. Two suites known to work is:
-     * SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA SSL_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA
+     * Only certain cipher suites will work with the keys and protocol we're 
using for this test. Two suites known to
+     * work is: SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA 
SSL_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA
      */
     public void testEnabled() throws Exception {
 
-        ((FTPSClient) client)
-                .setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA" });
+       ((FTPSClient) client).setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA" });
 
-        connectClient();
+       connectClient();
     }
 
     public void testDisabled() throws Exception {
-        ((FTPSClient) client)
-                .setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA" });
-
-        try {
-            doConnect();
-            fail("Must throw SSLHandshakeException");
-        } catch (SSLHandshakeException e) {
-            // OK
-        }
+       ((FTPSClient) client).setEnabledCipherSuites(new String[] { 
"SSL_DHE_DSS_EXPORT_WITH_DES40_CBC_SHA" });
+
+       try {
+           doConnect();
+           fail("Must throw SSLHandshakeException");
+       } catch (SSLHandshakeException e) {
+           // OK
+       }
     }
 }

Reply via email to