I finally got some time to spend on this and here are my thoughts/a
patch for review (not quite finished, but works):
The requirements/assumptions:
1. Client uses a secure connection (either implicit or explicit SSL).
2. Client submits a trusted certificate during SSL handshake
3. Client submits a user name
4. If the server is configured to authenticate users without requiring
passwords, it should let the user IN with the supplied user name and
certificate.
5. Server might have been setup to authenticate users in the following ways -
a) Old fashioned way (user and password)
b) User name and certificate
c) Dual authentication (user name, certificate and password)
5. Individual users may have been configured with any of the
authentication options described above (a, b or c). For example USER1
is setup to use the option a. USER2 is setup to use option b. USER3 is
setup to use option c.
What does this patch do:
1. USER command invokes authenticate method on the registered
UserManager. This might break backward compatibility. If it does and
if we cannot afford it, we should be able to overcome this by creating
UserManager2 extending UserManager. The USER command would then invoke
the authenticate method if and only if the registered user manager is
an instance of UserManager2. Otherwise, it would just do what it
always did. Whether or not we create the new interface is up to us.
2. Created a subclass of AuthenticationFailedException. This exception
is called PasswordRequiredException. If the UserManager cannot
authenticate the user without a password(because of the server/user
settings), it should throw this new exception.
3. If during the USER command execution, if it catches a
PasswordRequiredException, it simply falls back to the old way, by
sending a positive intermediate reply asking for the password. If it
catches the AuthenticationFailedException, then we could either fall
back to the old way, or send a negative completion reply indicating
that the login failed. If the UserManager returns a valid User from
the authenticate method, send a positive completion reply, and do what
ever we normally do in the PASS command (such as setting up file
system for the user etc).
5. A new implementation of Authentication, called
UsernameAuthentication is used by the USER command performing the
authentication. This is essentially same as the current
UsernamePasswordAuthentication, minus the password. If a user manager
does not care about this type of authentication, it can immediately
throw a PasswordRequiredException. Other implementations would check
the user authentication preferences for the user name, and act
accordingly.
In a nutshell, that is about it. I created an AbstractAuthentication
to hold the user metadata, and concrete Authentication classes extend
this abstract class. I also had to add a new flag to the UserMetadata
to indicate if the session is secured or not. Eventually, we might
just use the FtpSession in the UserMetadata so the authenticator can
access any additional information from the session. Lastly, I don't
think we want to affect the anonymous logins in anyway with this
change, so I left it alone.
Your feedback is appreciated.
Thanks.
Sai Pullabhotla
On Wed, Apr 6, 2011 at 4:14 PM, Niklas Gustavsson <[email protected]> wrote:
> On Wed, Apr 6, 2011 at 6:22 PM, Sai Pullabhotla
> <[email protected]> wrote:
>> Thanks, Niklas. Unfortunately we cannot control the clients. We were
>> told that the client's are built to never send PASS command and expect
>> either a 2XX reply on the USER command or 5XX reply. In other words,
>> the server should perform the authentication soon after it receives
>> the USER command (if the client was authenticated with digital
>> certificates), and send a "230 logged in". If the client was not
>> authenticated with digital certificate, then we need to fall back to
>> the regular mode, and send a "331 password required" reply.
>
> Given FTP always requires the PASS command, even for anon users, I
> find this client behavior a bit weird.
>
>> I guess, I will see if I can poke holes into the code and see if I can
>> get it to work. Would you be willing to consider this as an
>> enhancement and like to have the code submitted?
>
> Let's have a look at it when you're done :-)
>
> /niklas
>
