You are viewing a plain text version of this content. The canonical link for it is here.
Posted to ftpserver-dev@incubator.apache.org by Anton Goldberg <a....@gmail.com> on 2006/05/05 18:14:09 UTC
UserManager & Credentials
Hi,
I agree with the Rana's viewpoint from the previous letter. If any
component depends upon specific implementation/functionality of User
implementation, it's a configuration problem. FtpServer implementation
in such a case must be configured to use an appropriate UserManager
implementation and the component in question (FileSystemManager)
should check the class of User and take an action if the User is not
what it expected (fail fast).
Dividing the credentials (as a term, not a class) into User and
Credentials where they are used separately, depending on
implementation and the state the request is at doesn't sound as a good
idea to me.
In general, if the general opinion is that security implementation
needs to be more standard/powerful/complex we should consider
implementing JAAS or more modern frameworks.
P.S. Chances are this email will not show up in the list bcs for some
reason all emails from me to the list are going to the big hard drive
in the sky. I'm working on a solution for this problem (sending emails
to apache@ and infrastructure@) but that's the state things are right
now.
--
Anton
Re: UserManager & Credentials
Posted by Rana Bhattacharyya <ra...@yahoo.com>.
Hi,
I understand the issue of not passing the actual
User object to the FileSystemManager. My main question
was about SecurityManager and UserManager.
The patch looks good. I have only one small
observation. Instead of tmpUserName, getTmpUserName,
setTmpUserName parameter and methods, we can use
userArgument, getUserArgument, setUserArgument. The
tmpUserName is somewhat confusing and does not clearly
portray the meaning behind the parameter.
We are already using few temporary variables in
FtpRequestImpl like renameFrom and fileOffset. So it
is not an issue.
Please feel free to apply the patch and commit your
changes.
Thanks,
Rana Bhattacharyya
--- Sergey Vladimirov <vl...@gmail.com> wrote:
> Hi, ftpserver-dev,
>
> New patch variant is in attachment. No credentials
> class. Only replacing
> BaseUser with User from UserManager and replcaing
> fake BaseUser between USER
> and PASS with tmpUserName in FTPRequest.
>
> Sergey
>
> 2006/5/5, Sergey Vladimirov <vl...@gmail.com>:
> >
> > Anton, Rana,
> >
> > It is possible that I miss this point from Rana
> letter, so I don't quite
> > understand what is his viewpoint about.
> >
> > But anyway, with current implementation
> FileSystemManager will must expect
> > User to be instance of BaseUser. So, I agree, that
> such checks should have
> > place, but I want to get FileSystemManager chanses
> to have
> > application-specific User classes.
> >
> > *"Dividing the credentials (as a term, not a
> class) into User and
> > Credentials where they are used separately,
> depending on implementation and
> > the state the request is at doesn't sound as a
> good idea to me." *
> > I don't fully understand the idea of using
> Credentials for anything,
> > except storing username between USER and PASS
> command. Ok, ok, I can just
> > replace Credentials class with tmpUserName field,
> if it is more
> > understandable. I never said that Credentials
> should be used instead of User
> > anywere, except USER & PASS commands.
> >
> > Implementing JAAS is intresting, but it is more
> complex for simple
> > FTPServer. And I don't remember what is license
> terms of jaas.jar from
> > Sun(r).
> >
> > Sergey
> >
> > 2006/5/5, Anton Goldberg <a....@gmail.com>:
> >
> > > Hi,
> > > I agree with the Rana's viewpoint from the
> previous letter. If any
> > > component depends upon specific
> implementation/functionality of User
> > > implementation, it's a configuration problem.
> FtpServer implementation
> > > in such a case must be configured to use an
> appropriate UserManager
> > > implementation and the component in question
> (FileSystemManager)
> > > should check the class of User and take an
> action if the User is not
> > > what it expected (fail fast).
> > > Dividing the credentials (as a term, not a
> class) into User and
> > > Credentials where they are used separately,
> depending on
> > > implementation and the state the request is at
> doesn't sound as a good
> > > idea to me.
> > >
> > > In general, if the general opinion is that
> security implementation
> > > needs to be more standard/powerful/complex we
> should consider
> > > implementing JAAS or more modern frameworks.
> > >
> > > P.S. Chances are this email will not show up in
> the list bcs for some
> > > reason all emails from me to the list are going
> to the big hard drive
> > > in the sky. I'm working on a solution for this
> problem (sending emails
> > > to apache@ and infrastructure@) but that's the
> state things are right
> > > now.
> > > --
> > > Anton
> > >
> >
> >
> >
> > --
> >
> > Sergey Vladimirov
> >
>
>
>
> --
> Sergey Vladimirov
> > Index:
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
>
===================================================================
> ---
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
> (revision 399733)
> +++
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/FtpRequestImpl.java
> (working copy)
> @@ -26,12 +26,8 @@
> import java.util.zip.DeflaterOutputStream;
> import java.util.zip.InflaterInputStream;
>
> -import org.apache.ftpserver.ftplet.FileObject;
> -import org.apache.ftpserver.ftplet.FileSystemView;
> -import org.apache.ftpserver.ftplet.FtpRequest;
> -import org.apache.ftpserver.ftplet.User;
> +import org.apache.ftpserver.ftplet.*;
> import
> org.apache.ftpserver.interfaces.ConnectionObserver;
> -import org.apache.ftpserver.usermanager.BaseUser;
>
> /**
> * FTP request object.
> @@ -45,6 +41,10 @@
> private String command;
> private String argument;
>
> + /**
> + * Contains user name between USER and PASS
> commands
> + */
> + private String tmpUserName;
> private User user;
> private HashMap attributeMap;
> private InetAddress remoteAddr;
> @@ -66,7 +66,8 @@
> */
> public FtpRequestImpl() {
> attributeMap = new HashMap();
> - user = new BaseUser();
> + tmpUserName = null;
> + user = null;
> connectionTime =
> System.currentTimeMillis();
> }
>
> @@ -103,7 +104,8 @@
> * Reinitialize request.
> */
> public void reinitialize() {
> - user = new BaseUser();
> + tmpUserName = null;
> + user = null;
> loginTime = 0L;
> fileSystemView = null;
> renameFrom = null;
> @@ -240,8 +242,24 @@
> public void setRenameFrom(FileObject file) {
> renameFrom = file;
> }
> -
> +
> /**
> + * Returns user name entered in USER command
> + *
> + * @return user name entered in USER command
> + */
> + public String getTmpUserName() {
> + return tmpUserName;
> + }
> +
> + /**
> + * Set user name entered from USER command
> + */
> + public void setTmpUserName(String tmpUserName)
> {
> + this.tmpUserName = tmpUserName;
> + }
> +
> + /**
> * Get the ftp command.
> */
> public String getCommand() {
> @@ -290,6 +308,10 @@
> return user;
> }
>
> + public void setUser(User user) {
> + this.user = user;
> + }
> +
> /**
> * Get remote address
> */
> Index:
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
>
===================================================================
> ---
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
> (revision 399733)
> +++
>
C:/Vladimirov/workspace/ftpserver/src/java/org/apache/ftpserver/command/PASS.java
> (working copy)
> @@ -23,18 +23,11 @@
> import org.apache.ftpserver.FtpRequestImpl;
> import org.apache.ftpserver.FtpWriter;
> import org.apache.ftpserver.RequestHandler;
> -import
> org.apache.ftpserver.ftplet.FileSystemManager;
> -import org.apache.ftpserver.ftplet.FileSystemView;
> -import org.apache.ftpserver.ftplet.FtpException;
> -import org.apache.ftpserver.ftplet.Ftplet;
> -import org.apache.ftpserver.ftplet.FtpletEnum;
> -import org.apache.ftpserver.ftplet.User;
> -import org.apache.ftpserver.ftplet.UserManager;
> +import org.apache.ftpserver.ftplet.*;
> import org.apache.ftpserver.interfaces.ICommand;
> import
> org.apache.ftpserver.interfaces.IConnectionManager;
> import org.apache.ftpserver.interfaces.IFtpConfig;
> import
> org.apache.ftpserver.interfaces.IFtpStatistics;
> -import org.apache.ftpserver.usermanager.BaseUser;
>
> /**
> * <code>PASS <SP> <password>
> <CRLF></code><br>
> @@ -73,8 +66,7 @@
> }
>
> // check user name
> - User user = request.getUser();
> - String userName = user.getName();
> + String userName =
> request.getTmpUserName();
> if(userName == null) {
> out.send(503, "PASS", null);
> return;
> @@ -106,36 +98,32 @@
>
> // authenticate user
> UserManager userManager =
> fconfig.getUserManager();
> + User user = null;
> try {
> if(bAnonymous) {
> bSuccess =
> userManager.doesExist("anonymous");
> + if (bSuccess)
> + user =
> userManager.getUserByName("anonymous");
> }
> else {
> bSuccess =
> userManager.authenticate(userName, password);
> + if (bSuccess)
> + user =
> userManager.getUserByName(userName);
> }
> }
> catch(Exception ex) {
> bSuccess = false;
> log.warn("PASS.execute()", ex);
> }
> - if(!bSuccess) {
> + if(bSuccess) {
> + request.setUser(user);
> + request.setTmpUserName(null);
> + } else {
> log.warn("Login failure - " +
> userName);
> out.send(530, "PASS", userName);
> return;
> }
>
> - // populate user information
> - try {
> - populateUser(handler, userName);
> - }
> - catch(FtpException ex) {
> - bSuccess = false;
> - }
> - if(!bSuccess) {
> - out.send(530, "PASS", userName);
> - return;
> - }
> -
> // everything is fine - send login ok
> message
> out.send(230, "PASS", userName);
> if(bAnonymous) {
> @@ -168,25 +156,4 @@
> }
> }
> }
> -
> - /**
> - * Populate user.
> - */
> - private void populateUser(RequestHandler
> handler,
>
=== message truncated ===
__________________________________________________
Do You Yahoo!?
Tired of spam? Yahoo! Mail has the best spam protection around
http://mail.yahoo.com
Re: UserManager & Credentials
Posted by Sergey Vladimirov <vl...@gmail.com>.
Hi, ftpserver-dev,
New patch variant is in attachment. No credentials class. Only replacing
BaseUser with User from UserManager and replcaing fake BaseUser between USER
and PASS with tmpUserName in FTPRequest.
Sergey
2006/5/5, Sergey Vladimirov <vl...@gmail.com>:
>
> Anton, Rana,
>
> It is possible that I miss this point from Rana letter, so I don't quite
> understand what is his viewpoint about.
>
> But anyway, with current implementation FileSystemManager will must expect
> User to be instance of BaseUser. So, I agree, that such checks should have
> place, but I want to get FileSystemManager chanses to have
> application-specific User classes.
>
> *"Dividing the credentials (as a term, not a class) into User and
> Credentials where they are used separately, depending on implementation and
> the state the request is at doesn't sound as a good idea to me." *
> I don't fully understand the idea of using Credentials for anything,
> except storing username between USER and PASS command. Ok, ok, I can just
> replace Credentials class with tmpUserName field, if it is more
> understandable. I never said that Credentials should be used instead of User
> anywere, except USER & PASS commands.
>
> Implementing JAAS is intresting, but it is more complex for simple
> FTPServer. And I don't remember what is license terms of jaas.jar from
> Sun(r).
>
> Sergey
>
> 2006/5/5, Anton Goldberg <a....@gmail.com>:
>
> > Hi,
> > I agree with the Rana's viewpoint from the previous letter. If any
> > component depends upon specific implementation/functionality of User
> > implementation, it's a configuration problem. FtpServer implementation
> > in such a case must be configured to use an appropriate UserManager
> > implementation and the component in question (FileSystemManager)
> > should check the class of User and take an action if the User is not
> > what it expected (fail fast).
> > Dividing the credentials (as a term, not a class) into User and
> > Credentials where they are used separately, depending on
> > implementation and the state the request is at doesn't sound as a good
> > idea to me.
> >
> > In general, if the general opinion is that security implementation
> > needs to be more standard/powerful/complex we should consider
> > implementing JAAS or more modern frameworks.
> >
> > P.S. Chances are this email will not show up in the list bcs for some
> > reason all emails from me to the list are going to the big hard drive
> > in the sky. I'm working on a solution for this problem (sending emails
> > to apache@ and infrastructure@) but that's the state things are right
> > now.
> > --
> > Anton
> >
>
>
>
> --
>
> Sergey Vladimirov
>
--
Sergey Vladimirov
Re: UserManager & Credentials
Posted by Sergey Vladimirov <vl...@gmail.com>.
Anton, Rana,
It is possible that I miss this point from Rana letter, so I don't quite
understand what is his viewpoint about.
But anyway, with current implementation FileSystemManager will must expect
User to be instance of BaseUser. So, I agree, that such checks should have
place, but I want to get FileSystemManager chanses to have
application-specific User classes.
*"Dividing the credentials (as a term, not a class) into User and
Credentials where they are used separately, depending on implementation and
the state the request is at doesn't sound as a good idea to me."*
I don't fully understand the idea of using Credentials for anything, except
storing username between USER and PASS command. Ok, ok, I can just replace
Credentials class with tmpUserName field, if it is more understandable. I
never said that Credentials should be used instead of User anywere, except
USER & PASS commands.
Implementing JAAS is intresting, but it is more complex for simple
FTPServer. And I don't remember what is license terms of jaas.jar from
Sun(r).
Sergey
2006/5/5, Anton Goldberg <a....@gmail.com>:
>
> Hi,
> I agree with the Rana's viewpoint from the previous letter. If any
> component depends upon specific implementation/functionality of User
> implementation, it's a configuration problem. FtpServer implementation
> in such a case must be configured to use an appropriate UserManager
> implementation and the component in question (FileSystemManager)
> should check the class of User and take an action if the User is not
> what it expected (fail fast).
> Dividing the credentials (as a term, not a class) into User and
> Credentials where they are used separately, depending on
> implementation and the state the request is at doesn't sound as a good
> idea to me.
>
> In general, if the general opinion is that security implementation
> needs to be more standard/powerful/complex we should consider
> implementing JAAS or more modern frameworks.
>
> P.S. Chances are this email will not show up in the list bcs for some
> reason all emails from me to the list are going to the big hard drive
> in the sky. I'm working on a solution for this problem (sending emails
> to apache@ and infrastructure@) but that's the state things are right
> now.
> --
> Anton
>
--
Sergey Vladimirov