You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by ng...@apache.org on 2008/08/13 21:29:54 UTC
svn commit: r685647 - in /mina/ftpserver/trunk/core/src:
main/java/org/apache/ftpserver/command/
main/java/org/apache/ftpserver/interfaces/
main/resources/org/apache/ftpserver/message/
test/java/org/apache/ftpserver/ssl/
Author: ngn
Date: Wed Aug 13 12:29:47 2008
New Revision: 685647
URL: http://svn.apache.org/viewvc?rev=685647&view=rev
Log:
Crappy implementation of isSecure on the control socket as it only checked for the session base SSL filter (FTPSERVER-149). Same (but opposite) problem for getClientCertificates() (FTPSERVER-151). Also, we should not allow AUTH to be issued on an already secure session or trouble will occur (multiple SSL filters) (FTPSERVER-154) All three fixed and tests added.
Modified:
mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/command/AUTH.java
mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/interfaces/FtpIoSession.java
mina/ftpserver/trunk/core/src/main/resources/org/apache/ftpserver/message/FtpStatus.properties
mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/ExplicitSecurityTestTemplate.java
mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/MinaClientAuthTest.java
Modified: mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/command/AUTH.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/command/AUTH.java?rev=685647&r1=685646&r2=685647&view=diff
==============================================================================
--- mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/command/AUTH.java (original)
+++ mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/command/AUTH.java Wed Aug 13 12:29:47 2008
@@ -15,7 +15,7 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
- */
+ */
package org.apache.ftpserver.command;
@@ -37,79 +37,103 @@
/**
* This server supports explicit SSL support.
*/
-public
-class AUTH extends AbstractCommand {
+public class AUTH extends AbstractCommand {
+ private static final String SSL_SESSION_FILTER_NAME = "sslSessionFilter";
private final Logger LOG = LoggerFactory.getLogger(AUTH.class);
/**
* Execute command
*/
- public void execute(final FtpIoSession session,
- final FtpServerContext context,
- final FtpRequest request) throws IOException, FtpException {
-
+ public void execute(final FtpIoSession session, final FtpServerContext context, final FtpRequest request)
+ throws IOException, FtpException {
+
// reset state variables
session.resetState();
-
+
// argument check
- if(!request.hasArgument()) {
- session.write(FtpReplyUtil.translate(session, request, context, FtpReply.REPLY_501_SYNTAX_ERROR_IN_PARAMETERS_OR_ARGUMENTS, "AUTH", null));
- return;
+ if (!request.hasArgument()) {
+ session.write(FtpReplyUtil.translate(session, request, context,
+ FtpReply.REPLY_501_SYNTAX_ERROR_IN_PARAMETERS_OR_ARGUMENTS, "AUTH", null));
+ return;
}
-
+
// check SSL configuration
- if(session.getListener().getSslConfiguration() == null) {
+ if (session.getListener().getSslConfiguration() == null) {
session.write(FtpReplyUtil.translate(session, request, context, 431, "AUTH", null));
return;
}
-
+
+ // check that we don't already have a SSL filter in place due to running
+ // in implicit mode
+ // or because the AUTH command has already been issued. This is what the
+ // RFC says:
+
+ // "Some servers will allow the AUTH command to be reissued in order
+ // to establish new authentication. The AUTH command, if accepted,
+ // removes any state associated with prior FTP Security commands.
+ // The server must also require that the user reauthorize (that is,
+ // reissue some or all of the USER, PASS, and ACCT commands) in this
+ // case (see section 4 for an explanation of "authorize" in this
+ // context)."
+
+ // Here we choose not to support reissued AUTH
+ if (session.getFilterChain().contains(SslFilter.class)) {
+ session.write(FtpReplyUtil.translate(session, request, context, 534, "AUTH", null));
+ return;
+ }
+
// check parameter
String authType = request.getArgument().toUpperCase();
- if(authType.equals("SSL")) {
+ if (authType.equals("SSL")) {
try {
secureSession(session, "SSL");
session.write(FtpReplyUtil.translate(session, request, context, 234, "AUTH.SSL", null));
- } catch(FtpException ex) {
+ } catch (FtpException ex) {
throw ex;
- } catch(Exception ex) {
+ } catch (Exception ex) {
LOG.warn("AUTH.execute()", ex);
throw new FtpException("AUTH.execute()", ex);
}
- }
- else if(authType.equals("TLS")) {
+ } else if (authType.equals("TLS")) {
try {
secureSession(session, "TLS");
session.write(FtpReplyUtil.translate(session, request, context, 234, "AUTH.TLS", null));
- } catch(FtpException ex) {
+ } catch (FtpException ex) {
throw ex;
- } catch(Exception ex) {
+ } catch (Exception ex) {
LOG.warn("AUTH.execute()", ex);
throw new FtpException("AUTH.execute()", ex);
}
- }
- else {
- session.write(FtpReplyUtil.translate(session, request, context, FtpReply.REPLY_502_COMMAND_NOT_IMPLEMENTED, "AUTH", null));
+ } else {
+ session.write(FtpReplyUtil.translate(session, request, context, FtpReply.REPLY_502_COMMAND_NOT_IMPLEMENTED,
+ "AUTH", null));
}
}
-
- private void secureSession(final FtpIoSession session, final String type) throws GeneralSecurityException, FtpException {
+
+ private void secureSession(final FtpIoSession session, final String type) throws GeneralSecurityException,
+ FtpException {
SslConfiguration ssl = session.getListener().getSslConfiguration();
-
- if(ssl != null) {
+
+ if (ssl != null) {
session.setAttribute(SslFilter.DISABLE_ENCRYPTION_ONCE);
-
- SslFilter sslFilter = new SslFilter( ssl.getSSLContext() );
- if(ssl.getClientAuth() == ClientAuth.NEED) {
+
+ SslFilter sslFilter = new SslFilter(ssl.getSSLContext());
+ if (ssl.getClientAuth() == ClientAuth.NEED) {
sslFilter.setNeedClientAuth(true);
- } else if(ssl.getClientAuth() == ClientAuth.WANT) {
+ } else if (ssl.getClientAuth() == ClientAuth.WANT) {
sslFilter.setWantClientAuth(true);
}
-
- if(ssl.getEnabledCipherSuites() != null) {
+
+ // note that we do not care about the protocol, we allow both types
+ // and leave it to the SSL handshake to determine the protocol to
+ // use. Thus the type argument is ignored.
+
+ if (ssl.getEnabledCipherSuites() != null) {
sslFilter.setEnabledCipherSuites(ssl.getEnabledCipherSuites());
}
- session.getFilterChain().addFirst("sslSessionFilter", sslFilter);
+
+ session.getFilterChain().addFirst(SSL_SESSION_FILTER_NAME, sslFilter);
} else {
throw new FtpException("Socket factory SSL not configured");
Modified: mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/interfaces/FtpIoSession.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/interfaces/FtpIoSession.java?rev=685647&r1=685646&r2=685647&view=diff
==============================================================================
--- mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/interfaces/FtpIoSession.java (original)
+++ mina/ftpserver/trunk/core/src/main/java/org/apache/ftpserver/interfaces/FtpIoSession.java Wed Aug 13 12:29:47 2008
@@ -661,8 +661,8 @@
}
public Certificate[] getClientCertificates() {
- if(getFilterChain().contains("sslFilter")) {
- SslFilter sslFilter = (SslFilter) getFilterChain().get("sslFilter");
+ if(getFilterChain().contains(SslFilter.class)) {
+ SslFilter sslFilter = (SslFilter) getFilterChain().get(SslFilter.class);
SSLSession sslSession = sslFilter.getSslSession(this);
@@ -727,7 +727,7 @@
* @return true if the control socket is secured
*/
public boolean isSecure() {
- return getFilterChain().contains("sslSessionFilter");
+ return getFilterChain().contains(SslFilter.class);
}
}
Modified: mina/ftpserver/trunk/core/src/main/resources/org/apache/ftpserver/message/FtpStatus.properties
URL: http://svn.apache.org/viewvc/mina/ftpserver/trunk/core/src/main/resources/org/apache/ftpserver/message/FtpStatus.properties?rev=685647&r1=685646&r2=685647&view=diff
==============================================================================
--- mina/ftpserver/trunk/core/src/main/resources/org/apache/ftpserver/message/FtpStatus.properties (original)
+++ mina/ftpserver/trunk/core/src/main/resources/org/apache/ftpserver/message/FtpStatus.properties Wed Aug 13 12:29:47 2008
@@ -42,6 +42,7 @@
234.AUTH.TLS=Command AUTH okay; starting TLS connection.
502.AUTH=Command not implemented.
431.AUTH=Service is unavailable.
+534.AUTH=Session already secured
250.CDUP=Directory changed to {output.msg}.
550.CDUP=No such directory.
Modified: mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/ExplicitSecurityTestTemplate.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/ExplicitSecurityTestTemplate.java?rev=685647&r1=685646&r2=685647&view=diff
==============================================================================
--- mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/ExplicitSecurityTestTemplate.java (original)
+++ mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/ExplicitSecurityTestTemplate.java Wed Aug 13 12:29:47 2008
@@ -25,6 +25,7 @@
import org.apache.commons.net.ftp.FTPReply;
import org.apache.commons.net.ftp.FTPSClient;
+import org.apache.ftpserver.interfaces.FtpIoSession;
import org.apache.ftpserver.util.IoUtils;
@@ -40,20 +41,41 @@
client.login(ADMIN_USERNAME, ADMIN_PASSWORD);
}
+ private FtpIoSession getActiveSession() {
+ return server.getListener("default").getActiveSessions().iterator().next();
+ }
+
/**
* Tests that we can send command over the command channel.
* This is, in fact already tested by login in setup but
* an explicit test is good anyways.
*/
public void testCommandChannel() throws Exception {
+ assertTrue(getActiveSession().isSecure());
assertTrue(FTPReply.isPositiveCompletion(client.noop()));
}
+
+ public void testReissueAuth() throws Exception {
+ assertTrue(getActiveSession().isSecure());
+ assertTrue(FTPReply.isPositiveCompletion(client.noop()));
+
+ // we do not accept reissued AUTH or AUTH on implicitly secured socket
+ assertEquals(534, client.sendCommand("AUTH SSL"));
+ }
+
+
+ public void testIsSecure() {
+ assertTrue(getActiveSession().isSecure());
+ }
+
public void testStoreWithProtPInPassiveMode() throws Exception {
client.setRemoteVerificationEnabled(false);
client.enterLocalPassiveMode();
((FTPSClient)client).execPROT("P");
+
+ assertTrue(getActiveSession().getDataConnection().isSecure());
client.storeFile(TEST_FILE1.getName(), new ByteArrayInputStream(TEST_DATA));
@@ -66,6 +88,8 @@
client.enterLocalPassiveMode();
((FTPSClient)client).execPROT("P");
+
+ assertTrue(getActiveSession().getDataConnection().isSecure());
client.storeFile(TEST_FILE1.getName(), new ByteArrayInputStream(TEST_DATA));
@@ -74,6 +98,8 @@
((FTPSClient)client).execPROT("C");
+ assertFalse(getActiveSession().getDataConnection().isSecure());
+
client.storeFile(TEST_FILE2.getName(), new ByteArrayInputStream(TEST_DATA));
assertTrue(TEST_FILE2.exists());
@@ -82,6 +108,7 @@
public void testStoreWithProtPInActiveMode() throws Exception {
((FTPSClient)client).execPROT("P");
+ assertTrue(getActiveSession().getDataConnection().isSecure());
client.storeFile(TEST_FILE1.getName(), new ByteArrayInputStream(TEST_DATA));
@@ -91,6 +118,7 @@
public void testStoreWithProtPAndReturnToProtCInActiveMode() throws Exception {
((FTPSClient)client).execPROT("P");
+ assertTrue(getActiveSession().getDataConnection().isSecure());
client.storeFile(TEST_FILE1.getName(), new ByteArrayInputStream(TEST_DATA));
@@ -113,6 +141,7 @@
client.enterLocalPassiveMode();
((FTPSClient)client).execPROT("P");
+ assertTrue(getActiveSession().getDataConnection().isSecure());
File dir = new File(ROOT_DIR, "dir");
dir.mkdir();
@@ -125,6 +154,7 @@
client.enterLocalPassiveMode();
((FTPSClient)client).execPROT("P");
+ assertTrue(getActiveSession().getDataConnection().isSecure());
File file = new File(ROOT_DIR, "foo");
file.createNewFile();
Modified: mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/MinaClientAuthTest.java
URL: http://svn.apache.org/viewvc/mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/MinaClientAuthTest.java?rev=685647&r1=685646&r2=685647&view=diff
==============================================================================
--- mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/MinaClientAuthTest.java (original)
+++ mina/ftpserver/trunk/core/src/test/java/org/apache/ftpserver/ssl/MinaClientAuthTest.java Wed Aug 13 12:29:47 2008
@@ -21,11 +21,13 @@
import java.io.FileInputStream;
import java.security.KeyStore;
+import java.security.cert.X509Certificate;
import javax.net.ssl.KeyManagerFactory;
import org.apache.commons.net.ftp.FTPReply;
import org.apache.commons.net.ftp.FTPSClient;
+import org.apache.ftpserver.interfaces.FtpIoSession;
public class MinaClientAuthTest extends SSLTestTemplate {
@@ -35,7 +37,9 @@
client.setNeedClientAuth(true);
KeyStore ks = KeyStore.getInstance("JKS");
- ks.load(new FileInputStream(FTPCLIENT_KEYSTORE), KEYSTORE_PASSWORD.toCharArray());
+ FileInputStream fis = new FileInputStream(FTPCLIENT_KEYSTORE);
+ ks.load(fis, KEYSTORE_PASSWORD.toCharArray());
+ fis.close();
KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm());
kmf.init(ks, KEYSTORE_PASSWORD.toCharArray());
@@ -59,4 +63,13 @@
}
+ public void testClientCertificates() throws Exception {
+ FtpIoSession session = server.getListener("default").getActiveSessions().iterator().next();
+ assertEquals(1, session.getClientCertificates().length);
+
+ X509Certificate cert = (X509Certificate) session.getClientCertificates()[0];
+
+ assertTrue(cert.getSubjectDN().toString().contains("FtpClient"));
+ }
+
}