Re: Black/White List in FTP Server
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 nik...@protocol7.com wrote: On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla sai.pullabho...@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
On Thu, Mar 18, 2010 at 7:20 PM, Sai Pullabhotla sai.pullabho...@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
Done. Regards, Sai Pullabhotla On Thu, Mar 18, 2010 at 1:32 PM, Niklas Gustavsson nik...@protocol7.com wrote: On Thu, Mar 18, 2010 at 7:20 PM, Sai Pullabhotla sai.pullabho...@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
On Tue, Mar 16, 2010 at 5:19 AM, Ashish paliwalash...@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
On Mon, Mar 15, 2010 at 7:27 PM, Sai Pullabhotla sai.pullabho...@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
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 nik...@protocol7.com wrote: On Mon, Mar 15, 2010 at 7:27 PM, Sai Pullabhotla sai.pullabho...@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
On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla sai.pullabho...@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
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 nik...@protocol7.com wrote: On Tue, Mar 16, 2010 at 1:05 PM, Sai Pullabhotla sai.pullabho...@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
On Tue, Mar 16, 2010 at 1:29 PM, Sai Pullabhotla sai.pullabho...@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
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 nik...@protocol7.com wrote: On Tue, Mar 16, 2010 at 1:29 PM, Sai Pullabhotla sai.pullabho...@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
On Tue, Mar 16, 2010 at 2:24 PM, Sai Pullabhotla sai.pullabho...@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
On Tue, Mar 16, 2010 at 2:52 PM, Sai Pullabhotla sai.pullabho...@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
2010/3/16 Niklas Gustavsson nik...@protocol7.com: On Tue, Mar 16, 2010 at 2:52 PM, Sai Pullabhotla sai.pullabho...@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
On Mon, Mar 15, 2010 at 11:57 PM, Sai Pullabhotla sai.pullabho...@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
