mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lécharny <elecha...@gmail.com>
Subject Re: CCC support in FTP Server requires changes to Mina's SslFilter?
Date Tue, 11 Jun 2013 14:48:33 GMT
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 


Mime
View raw message