Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

I've been trying to implement white/black lists in FTP server and
thought of running my findings/ideas with you guys.

Currently, each listener can have a black list. There is NO white
listing capability.

I've been thinking, instead of having the black list IPs/Subnets,
simply have an interface called IPFilter. Each listener can have at
most one IPFilter. The IPFilter requires an implementation for a
method named accept(), which tells if the client's connection should
be accepted or rejected based on the IP address. This gives us the
flexibility of having a black or white list which ever is preferred by
the server administrator. By default, we can ship default
implementation for IPFilter which can be a black or white filter. For
example, in the spring configuration, instead of having a blacklist
element, we would have a <ipFilter> element as shown below:

<ipFilter type="whitelist|blacklist">
     192.168.1.200/32, 192.168.1.201/32
</ipFilter>

The type attribute in the ipFilter element tells us if it should be a
white or black list. The value for this attribute could be "whitelist"
or "blacklist" or something similar such as BLOCK/PASS.

I could not think of any good usage scenarios where one might want to
have both white and black lists for a given listener. So, one IP
Filter per listener should be good enough, unless you guys think
otherwise.

The above should work for users who want to run the FTP server
out-of-the-box. For people who want to override the default IP filter
implementaton, could create a new class that implements the IPFilter
interface and specify the class name(?) in the spring config or
programmatically call ListenerFactory.setIPFilter(IPFilter) method.

Let me know what do you guys think and we can decide on how best it
should be implemented. I do have sometime this week to work on this if
we finalize on something.

Thanks & Regards,
Sai Pullabhotla

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

Done.

Regards,
Sai Pullabhotla





On Thu, Mar 18, 2010 at 1:32 PM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Thu, Mar 18, 2010 at 7:20 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Attached is the alpha release ;) for the black/white lists for your
>> review.
>
> Create a JIRA issue and attach it there. Mailing lists strip attachments :-)
>
> /niklas
>

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Thu, Mar 18, 2010 at 7:20 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Attached is the alpha release ;) for the black/white lists for your
> review.

Create a JIRA issue and attach it there. Mailing lists strip attachments :-)

/niklas

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

Attached is the alpha release ;) for the black/white lists for your
review. This patch is created from the trunk. I briefly tested this as
a stand-alone server using spring config as well as programmatically
configuring the server with a custom IP Filter. I updated the spring
config/parser to support old style blacklist element or the new
ip-filter element. If both elements are used, it raises an error. Both
elements use the new IP Filter. Keep in mind that I still may have to
do some tweaking and cleanup, but thought I should get this out to you
guys for review so we can tweak it as needed. I appreciate any
feedback.

Regards,
Sai Pullabhotla





On Tue, Mar 16, 2010 at 7:18 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Does this mean you want to wait until Mina 3.0, or should we start
>> working on the FTP Server right away and share relevant code with
>> MINA?
>
> I think we can start right away and copy the code upstream until we
> move FtpServer to MINA 3.0.
>
> /niklas
>

Re: Black/White List in FTP Server

[David Latorre <dv...@gmail.com>]

2010/3/16 Niklas Gustavsson <ni...@protocol7.com>:
> On Tue, Mar 16, 2010 at 2:52 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> If a filter can make use of this event, why not an
>> Ftplet?
>
> To me, because writing your own Ftplet should require minimal
> knowledge of the internals of FtpServer. Compare this to writing a
> Servlet and your own Valve for Tomcat.
>
> /niklas

niklas opinion makes sense but, of course, some users might need this
sessionCreated event. My suggestion is that we wait until some of the
inner workings of MINA 3.0 are agreed upson so we don't have to make a
decision based on a threading/event model that is very likely to
change.


The IPFilter that can act as a blacklist or a whitelist is a pretty
clever idea. What about programmatically changing the IPFilter? as if
you want to update the blacklist but the whitelist 'version' is in
use...

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Tue, Mar 16, 2010 at 2:52 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> If a filter can make use of this event, why not an
> Ftplet?

To me, because writing your own Ftplet should require minimal
knowledge of the internals of FtpServer. Compare this to writing a
Servlet and your own Valve for Tomcat.

/niklas

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

Yes, you are correct, and I did read that note in the MINA
documentation. Essentially, an IP filter should make use of Session
Created to filter out unwanted clients. Filtering may take a while
depending on how big the list is or how efficient the filtering
algorithm is. If a filter can make use of this event, why not an
Ftplet? I agree that Ftplet authors should know what they are doing
when using this method, but do not see any harm in making this event
available. Just throwing in my ideas for discussion/evaluation, not
that I really want it to be included. :)

Regards,
Sai Pullabhotla


On Tue, Mar 16, 2010 at 8:31 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Tue, Mar 16, 2010 at 2:24 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Currently, we call the Ftplet.onConnect from the sessionOpened method.
>> May be we should add yet another method to the Ftplets to indicate a
>> sessionCreated event, in case if some one wants to use it?
>
> If I recall correctly, sessionCreated is called from the IO processor
> thread and we would therefore shortcut our threading model if we
> expose it to Ftplets (letting an Ftplet block the entire listener). My
> understanding is that sessionCreated is useful if you really know what
> you're doing and I'm not sure that should be required by Ftplet users
> :-)
>
> /niklas
>

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Tue, Mar 16, 2010 at 2:24 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Currently, we call the Ftplet.onConnect from the sessionOpened method.
> May be we should add yet another method to the Ftplets to indicate a
> sessionCreated event, in case if some one wants to use it?

If I recall correctly, sessionCreated is called from the IO processor
thread and we would therefore shortcut our threading model if we
expose it to Ftplets (letting an Ftplet block the entire listener). My
understanding is that sessionCreated is useful if you really know what
you're doing and I'm not sure that should be required by Ftplet users
:-)

/niklas

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

I would not disagree that the IP restrictions should be handled by the
filters, but at the same time. I was just looking for other ways that
can be used when the filters do not provide the functionality the user
wants. I do See that MINA propagates two different events  -

1. session opened
2. session created

Currently, we call the Ftplet.onConnect from the sessionOpened method.
May be we should add yet another method to the Ftplets to indicate a
sessionCreated event, in case if some one wants to use it?


Regards,
Sai Pullabhotla

On Tue, Mar 16, 2010 at 7:38 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Tue, Mar 16, 2010 at 1:29 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> However, with FTPS (Implicit), the SSL negotiation is initiated prior
>> to sending the onConnect event to the Ftplets. To be precise, the
>> client does get the server's certificate before onConnect is called. I
>> was wondering if this should be done differently so no data is
>> exchanged (read/written) unless onConnect of all Ftplets are executed.
>
> With "data" in this case, we're only talking about the SSL handshake,
> right? I think onConnect should indicate that the socket (session) is
> established. With SSL, the socket might be ended (e.g due to
> certificate validation failing) during the handshake. So, I think the
> current behavior is correct. Besides, I think IP restriction is better
> handled in the filter chain, rather than in Ftplets which I think
> should contain things more like "business logic" (if you excuse the
> very broad use of that term :-).
>
> /niklas
>

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Tue, Mar 16, 2010 at 1:29 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> However, with FTPS (Implicit), the SSL negotiation is initiated prior
> to sending the onConnect event to the Ftplets. To be precise, the
> client does get the server's certificate before onConnect is called. I
> was wondering if this should be done differently so no data is
> exchanged (read/written) unless onConnect of all Ftplets are executed.

With "data" in this case, we're only talking about the SSL handshake,
right? I think onConnect should indicate that the socket (session) is
established. With SSL, the socket might be ended (e.g due to
certificate validation failing) during the handshake. So, I think the
current behavior is correct. Besides, I think IP restriction is better
handled in the filter chain, rather than in Ftplets which I think
should contain things more like "business logic" (if you excuse the
very broad use of that term :-).

/niklas

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

Cool, I will see what I can do. I was also looking at other ways to
implement this feature and looks like one should be able to make use
of an Ftplet and capture the onConnect event to determine if the
connection should be allowed or not. I did some quick tests and found
the following:

Plain FTP/Explicit SSL does not seem to have any visible issues. Check
the client's IP in the Ftplet's onConnect and return DISCONNECT if the
client connection should not be accepted.

However, with FTPS (Implicit), the SSL negotiation is initiated prior
to sending the onConnect event to the Ftplets. To be precise, the
client does get the server's certificate before onConnect is called. I
was wondering if this should be done differently so no data is
exchanged (read/written) unless onConnect of all Ftplets are executed.
What do you think?

Regards,
Sai Pullabhotla





On Tue, Mar 16, 2010 at 7:18 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> Does this mean you want to wait until Mina 3.0, or should we start
>> working on the FTP Server right away and share relevant code with
>> MINA?
>
> I think we can start right away and copy the code upstream until we
> move FtpServer to MINA 3.0.
>
> /niklas
>

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> Does this mean you want to wait until Mina 3.0, or should we start
> working on the FTP Server right away and share relevant code with
> MINA?

I think we can start right away and copy the code upstream until we
move FtpServer to MINA 3.0.

/niklas

Re: Black/White List in FTP Server

[Sai Pullabhotla <sa...@jmethods.com>]

Does this mean you want to wait until Mina 3.0, or should we start
working on the FTP Server right away and share relevant code with
MINA?

Regards,
Sai Pullabhotla





On Tue, Mar 16, 2010 at 1:13 AM, Niklas Gustavsson <ni...@protocol7.com> wrote:
> On Mon, Mar 15, 2010 at 7:27 PM, Sai Pullabhotla
> <sa...@jmethods.com> wrote:
>> I've been trying to implement white/black lists in FTP server and
>> thought of running my findings/ideas with you guys.
>
> +1 to all you say. I think we should aim to land this work in MINA 3.0.
>
> /niklas
>

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Mon, Mar 15, 2010 at 7:27 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> I've been trying to implement white/black lists in FTP server and
> thought of running my findings/ideas with you guys.

+1 to all you say. I think we should aim to land this work in MINA 3.0.

/niklas

Re: Black/White List in FTP Server

[Niklas Gustavsson <ni...@protocol7.com>]

On Tue, Mar 16, 2010 at 5:19 AM, Ashish <pa...@gmail.com> wrote:
> Is this IoListener or something specific to FtpServer? AFAIK, our
> filters are part of chain and get applied at session level.

Yes, listeners are part of the FtpServer API and basically maps 1:1 to
an IoAcceptor.

> One suggestion is make the implementation more efficient. The current
> MINA balklist filter uses List for storing IP's
> and iteration through the list for each call is not efficient. So may
> be ConcurrentHashMap would be good idea for storing.
> However, this works fine for IP's. Need to think about Subnet and other stuff ;)

Given that these lists are usually pretty short, at least for subnets,
I would see some benchmarking before making the implementation more
complex. For IPs, I agree that a HashSet/HashMap or similar makes
sense.

/niklas

Re: Black/White List in FTP Server

[Ashish <pa...@gmail.com>]

On Mon, Mar 15, 2010 at 11:57 PM, Sai Pullabhotla
<sa...@jmethods.com> wrote:
> I've been trying to implement white/black lists in FTP server and
> thought of running my findings/ideas with you guys.
>
> Currently, each listener can have a black list. There is NO white
> listing capability.

Is this IoListener or something specific to FtpServer? AFAIK, our
filters are part of chain and get applied at session level.

>
> I've been thinking, instead of having the black list IPs/Subnets,
> simply have an interface called IPFilter. Each listener can have at
> most one IPFilter. The IPFilter requires an implementation for a
> method named accept(), which tells if the client's connection should
> be accepted or rejected based on the IP address. This gives us the
> flexibility of having a black or white list which ever is preferred by
> the server administrator. By default, we can ship default
> implementation for IPFilter which can be a black or white filter. For
> example, in the spring configuration, instead of having a blacklist
> element, we would have a <ipFilter> element as shown below:
>
> <ipFilter type="whitelist|blacklist">
>     192.168.1.200/32, 192.168.1.201/32
> </ipFilter>
>
> The type attribute in the ipFilter element tells us if it should be a
> white or black list. The value for this attribute could be "whitelist"
> or "blacklist" or something similar such as BLOCK/PASS.
>
> I could not think of any good usage scenarios where one might want to
> have both white and black lists for a given listener. So, one IP
> Filter per listener should be good enough, unless you guys think
> otherwise.

me neither :)

>
> The above should work for users who want to run the FTP server
> out-of-the-box. For people who want to override the default IP filter
> implementaton, could create a new class that implements the IPFilter
> interface and specify the class name(?) in the spring config or
> programmatically call ListenerFactory.setIPFilter(IPFilter) method.
>
> Let me know what do you guys think and we can decide on how best it
> should be implemented. I do have sometime this week to work on this if
> we finalize on something.

One suggestion is make the implementation more efficient. The current
MINA balklist filter uses List for storing IP's
and iteration through the list for each call is not efficient. So may
be ConcurrentHashMap would be good idea for storing.
However, this works fine for IP's. Need to think about Subnet and other stuff ;)

-- 
thanks
ashish