You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Erick Lichtas <EL...@linoma.com> on 2013/06/11 16:29:36 UTC

CCC support in FTP Server requires changes to Mina's SslFilter?

Hi Everyone,

I've been using the mina FTP server for quite some time, and would like to get CCC support added to the server, specifically https://issues.apache.org/jira/browse/FTPSERVER-411 which was opened a couple years ago.

I've take a look at the code attached on the case and have noticed there are some issues in Mina when trying to remove an SslFilter from the chain while it is in use, race conditions exist which frequently result in IllegalStateExceptions.  This appears to be a synchronization issues in the SslFilter. Additionally, it appears the SslFilter is designed to not be removed once it is added to the chain and that I can simply call stopSsl() basically disable the filter. I'm currently working with Mina 2.0.7.

Going with this approach, i updated the CCC.execute(..) method to look like this

public void execute(final FtpIoSession session,
                final FtpServerContext context, final FtpRequest request)
                throws IOException, FtpException {

                // reset state variables
                session.resetState();

                if (session.isSecure()) {
                                synchronized (session) {
                                                session.write(new DefaultFtpReply(200, "Okay")).awaitUninterruptibly();
                                                IoFilterChain filterChain = session.getFilterChain();
                                                SslFilter filter = (SslFilter) filterChain.get(SslFilter.class);
                                                filter.stopSsl(session).awaitUninterruptibly();
                                }
                }
                else {
                                session.write(new DefaultFtpReply(500, "Not a secured session"));
                }
}

This seems to work great with certain FTPS clients that support CCC, such as Commons-NET and SmartFTP, but fails with others, such as CuteFTP and WS_FTP Pro are failing

I believe this ties into this COMMONS-NET case https://issues.apache.org/jira/browse/NET-354, and this ftpserver-users archive http://markmail.org/message/elzz43cqz22d4jap#query:+page:1+mid:bak2icz4n26fadlg+state:results<http://markmail.org/message/elzz43cqz22d4jap%23query:+page:1+mid:bak2icz4n26fadlg+state:results>

They contain some debate on the proper implementation of terminating SSL and it seems there are clearly 2 different client implementations, some that properly terminate SSL by sending the CLOSE_NOTIFY, and some that simply disregard the ssl wrapper and move on.

So while adding the call to filter.stopSsl(session).awaitUninterruptibly() in the CCC.execute method works for some clients, it appears CuteFTP and WS_FTP do not expect the CLOSE_NOTIFY and actually get confused when they receive it.

I've tweaked Mina's SslFilter to add a toggle for whether or not to send a CLOSE_NOTIFY and adjusted the code as follows:

public WriteFuture stopSsl(IoSession session) throws SSLException {
        SslHandler handler = getSslSessionHandler(session);
        NextFilter nextFilter = (NextFilter) session.getAttribute(NEXT_FILTER);
        WriteFuture future;
                                synchronized (handler) {
                                                if (SEND_CLOSE_NOTIFY) {
                                                                future = initiateClosure(nextFilter, session);
                                                }
                                                else {
                                                                synchronized (handler) {
                                                                                // release resources
                                                                                handler.destroy();
                                                                }
                                                }
                                }

                handler.flushScheduledEvents();

                return future;
}

I'm not sure if this is the right approach, but it now seems to be working for the mentioned FTPS clients that were not working with the CCC command before.

My questions are more or less requests for comments and suggestions, whether or not I'm way off track, and if this is something that can be fully integrated into Mina and the FTP server.

Thanks!
Erick Lichtas

Re: CCC support in FTP Server requires changes to Mina's SslFilter?

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 6/11/13 4:29 PM, Erick Lichtas a écrit :
> Hi Everyone,
>
> I've been using the mina FTP server for quite some time, and would like to get CCC support added to the server, specifically https://issues.apache.org/jira/browse/FTPSERVER-411 which was opened a couple years ago.
>
> I've take a look at the code attached on the case and have noticed there are some issues in Mina when trying to remove an SslFilter from the chain while it is in use, race conditions exist which frequently result in IllegalStateExceptions.  This appears to be a synchronization issues in the SslFilter. Additionally, it appears the SslFilter is designed to not be removed once it is added to the chain and that I can simply call stopSsl() basically disable the filter. I'm currently working with Mina 2.0.7.

I confirm : the SSLFilter can't be removed, it would close the
connection. This is quite a problem for StartTLS, for instance...
>
> Going with this approach, i updated the CCC.execute(..) method to look like this
>
> public void execute(final FtpIoSession session,
>                 final FtpServerContext context, final FtpRequest request)
>                 throws IOException, FtpException {
>
>                 // reset state variables
>                 session.resetState();
>
>                 if (session.isSecure()) {
>                                 synchronized (session) {
>                                                 session.write(new DefaultFtpReply(200, "Okay")).awaitUninterruptibly();
>                                                 IoFilterChain filterChain = session.getFilterChain();
>                                                 SslFilter filter = (SslFilter) filterChain.get(SslFilter.class);
>                                                 filter.stopSsl(session).awaitUninterruptibly();
>                                 }
>                 }
>                 else {
>                                 session.write(new DefaultFtpReply(500, "Not a secured session"));
>                 }
> }
>
> This seems to work great with certain FTPS clients that support CCC, such as Commons-NET and SmartFTP, but fails with others, such as CuteFTP and WS_FTP Pro are failing
>
> I believe this ties into this COMMONS-NET case https://issues.apache.org/jira/browse/NET-354, and this ftpserver-users archive http://markmail.org/message/elzz43cqz22d4jap#query:+page:1+mid:bak2icz4n26fadlg+state:results<http://markmail.org/message/elzz43cqz22d4jap%23query:+page:1+mid:bak2icz4n26fadlg+state:results>
>
> They contain some debate on the proper implementation of terminating SSL and it seems there are clearly 2 different client implementations, some that properly terminate SSL by sending the CLOSE_NOTIFY, and some that simply disregard the ssl wrapper and move on.

>From the client POV, whatever they do is their own pb. Regardless, the
server side should be ready to support any kind of client terminaison.
>
> So while adding the call to filter.stopSsl(session).awaitUninterruptibly() in the CCC.execute method works for some clients, it appears CuteFTP and WS_FTP do not expect the CLOSE_NOTIFY and actually get confused when they receive it.
>
> I've tweaked Mina's SslFilter to add a toggle for whether or not to send a CLOSE_NOTIFY and adjusted the code as follows:
>
> public WriteFuture stopSsl(IoSession session) throws SSLException {
>         SslHandler handler = getSslSessionHandler(session);
>         NextFilter nextFilter = (NextFilter) session.getAttribute(NEXT_FILTER);
>         WriteFuture future;
>                                 synchronized (handler) {
>                                                 if (SEND_CLOSE_NOTIFY) {
>                                                                 future = initiateClosure(nextFilter, session);
>                                                 }
>                                                 else {
>                                                                 synchronized (handler) {
>                                                                                 // release resources
>                                                                                 handler.destroy();
>                                                                 }
>                                                 }
>                                 }
>
>                 handler.flushScheduledEvents();
>
>                 return future;
> }
>
> I'm not sure if this is the right approach, but it now seems to be working for the mentioned FTPS clients that were not working with the CCC command before.
>
> My questions are more or less requests for comments and suggestions, whether or not I'm way off track, and if this is something that can be fully integrated into Mina and the FTP server.

Many thanks for thsoe suggestions ! It's funny because we have had a
long discussion last week about how to handle SSL closure, as we are
working on the SSL handling for MINA 3.

I guess we have to spend some time on this aspect for MINA 2 too...


-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com