PBServerFactory should support plain password credentials

[victorlin]

Due to the authentication method build-in in PBServerFactory, it is impossible to login with a plain password like this: factory.login(credentials.UsernamePassword("victor", "abc")). Therefore, I can't use a hashed password file at server side. Like this one: checkers.FilePasswordDB(htpasswordFile, hash=hashFunction). The server will said that there is no checker for the credentials.

Surely, it is not secure to send a plain password over internet, but how about through SSL? I think the PBServerFactory should support both plain and hashed authentication methods. Otherwise, it is also insecure to store passwords as plain text on server side.

[exarkun]

Due to the authentication method build-in in PBServerFactory, it is impossible to login with a plain password like this: factory.login(credentials.UsernamePassword("victor", "abc"))

This is not accurate. You can log in using a UsernamePassword.

Therefore, I can't use a hashed password file at server side. The server will said that there is no checker for the credentials.

This doesn't follow. When you log in with UsernamePassword (which is allowed), PB actually uses a challenge-response negotiation which does not transmit the cleartext password to the server. This is why you can't use a hashing function in the checker: because the password has already been hashed by the client.

I think the PBServerFactory should support both plain and hashed authentication methods.

You can support any additional credentials types you want via a PBServerFactory subclass. I suggest implementing the scheme you want and then, if it still seems to make sense as a general extension to PB, re-proposing.

[glyph]

While this ticket is invalid, I'm slightly curious why the submitter didn't immediately figure out the right way to do this with PB. Maybe there's a doc bug lurking here?

[Grindaizer]

allow to defined custom hash metod to PB authentication.

[Grindaizer]

example wich custom hash method and FilePasswordBD

[Grindaizer]

Hi to all.

Replying to exarkun:

You can support any additional credentials types you want via a PBServerFactory subclass. I suggest implementing the scheme you want and then, if it still seems to make sense as a general extension to PB, re-proposing.

We've faced almost the same problem recently, and i wanted to submit a patch which could also resolve the problem described here.

Actually the solution suggested here is to allow plain text to be transmitted across the network, but it's not what we've found the most interesting. In deed, most of the application that deal with authentication have two main requirements: * The password should never be plain text when it cross the network. * The password should never be stored as plain text

And most of the time password hash algorithm and storage format is specific to application. When we look at pb code we see that password hashing behaviour is defined by two function respond and challenge which are always the same.

So the idea here is to allow user to define their own "hashing" behaviour. (a kind of custom respond and challenge functions).

The password will still be transmitted hashed, but as pb hash the password is the same way the application does, identification procedure will be possible without having the plain text password on server side.

ex: (complete example attached below).

server side: define hashMetod and challenge on checker.

from common import MD5doubleHash


class PBFilePasswordChecker(checkers.FilePasswordDB):
    implements(pb.IChallenger)
    def challengeFor(self, username):
        try:
            u, p = self.getUser(username)
        except KeyError:
            return str(len(username))
        _, salt = p.split('$$')
        return salt

    def hashMethod(self, salt, secret):
        return MD5doubleHash(salt, secret)
    

client side: define the same hashMethod on credentials class

!#python
from common import MD5doubleHash

class PBUsernamePassword(credentials.UsernamePassword):
    implements(pb.IHashMethod)
    def hashMethod(self, salt, secret):
        return MD5doubleHash(salt, secret)

[spalax]

Patch allowing to use custom hash method for PB authentication. Improves IUsernamePassword_over_pb.patch

[spalax]

Patch

I improved Grindaizer's patch in several ways.

1. It is backward compatible, as class spread.pb._PortalAuthChallenger still implements spread.pb.IUsernameMD5Password.
2. It separates two hash methods, which have no obligation to be the same: the method used to hash the password (crypt(3) for Unix shadow password, for example), which we will call password hash, and the method used during the challenge/response step of authentication, which we will call challenge hash. With this implementation, a default challenge hash is given, but both password and challenge hashes may be redefined, and they can be different.
3. It separates the two salts used with the two hash methods. The first one (challenge salt) is a randomly generated each time a client tries to authenticate; the second one (password salt) is always the same for a given hashed password. With my implementation, the two salts are different: the challenge salt is still randomly generated, while a method must be defined, which generate the password salt from the username and the hashed password (depending on the password hash used, the salt may depends on the username, the hashed password, or both).
4. More tests are provided.

See the patch.

How does this work now?

One can define a credendial and a checker that implements a custom hash method to authenticate users.

Here is a sequence diagram that explains what happens.

  Client knows a username,              Server knows the hashed password,
  a password, and the challenge         a method getSalt that, given
  and password hash methods             username and hashed password,
                                        returns the salt, and the challenge
                                        hash method.

  Client                                                    Server
  ======                                                    ======
    |           "I want to authenticate as" username           |
    |--------------------------------------------------------->|
    |                                                          |
    |  challenge, salt = getSalt(username, hashedPassword)     |
    |<---------------------------------------------------------|
    |                                                          |
    | challengeHash(challenge, passwordHash(password, salt))   |
    |--------------------------------------------------------->|

                                               Now, server only has to compare
                                               that what it got from the client
                                               is equal to
                                               challengeHash(challenge, hashedPassword)

Example

Here is an example of credential, which uses the default challenge hash, and defines a very easy hash method. We note that on this side, we do not need to specify how the salt is defined, since the salt is given by the server.

class DummyCredential(pb.UsernameHashPassword):
    """A dummy credential."""
    def pwHashMethod(self, secret, salt):
        return secret + salt

On the other side, here is the corresponding checker. It is a subclass of InMemoryUsernamePasswordDatabaseDontUse, in which passwords are stored, hashed used the dummy hash method we gave above. Here, the salt is the length of the username.

class DummyChallengerChecker(checkers.InMemoryUsernamePasswordDatabaseDontUse):
    implements(checkers.IChallenger)

    def __init__(self, **users):
        hashed = dict([(user, users[user] + str(len(user))) for user in users])
        return checkers.InMemoryUsernamePasswordDatabaseDontUse(**hashed)
    def challengeHashMethod(self, challenge, data):
        return pb.challengeHash(challenge, data)
    def getSalt(self, username):
        return str(len(username))

Default

I also added two default credentials, which I think might be the most used ones: spread.pb.UsernameHashlibPassword and spread.pb.UsernameCryptPassword, which use respectively hash method from the  Python hashlib library, and  crypt.

Documentation?

If this patch is accepted, I offer to write some documentation about it. That is, some page which would fit in the  Perspective Broker documentation, and would include my beautiful sequence diagram and explain how to build and use these credentials and checkers.

[tom.prince]

It appears that the algorithm described doesn't require the client to actually have the original password, merely the hashed password. Which means that the storing the hashed passwords on the server is no more secure than storing the plain-text passwords. Given that, it seems that using this is just going to give users a false sense of security.

[spalax]

Replying to tom.prince:

It appears that the algorithm described doesn't require the client to actually have the original password, merely the hashed password. Which means that the storing the hashed passwords on the server is no more secure than storing the plain-text passwords. Given that, it seems that using this is just going to give users a false sense of security.

I agree, this is a flaw. Although I would still say that this is an improvement compared to the original version, given that the plain password is not needed on the server side (as a system administrator, I do not like to be able to read my users' passwords).

An improved solution is given  here. Using this algorithm, the hash stored on the server side is not sufficient to log in.

If you consider it safe, I can implement it (although I do not have that much available time these days, so it might take a few weeks to be done).

[tom.prince]

I'm not a security expert, so I don't have enough knowledge, or confidence to analyze the safety of the proposed solution. The linked algorithms don't appear to be standard, and don't have any references to external analysis of the security of the protocol.

Any proposed patch implementing a security protocol like this should contain sufficient documentation of the security implications, that somebody can comfortably asses both the implementation and the algorithms. This is particularly true for custom protocols like PB, that aren't standardized, and don't have alternative implementations.