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 <[email protected]>
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
+ }
}
}
