Opened 5 years ago

Last modified 7 days ago

#4398 enhancement new

PBServerFactory should support plain password credentials

Reported by: victorlin Owned by: spalax
Priority: normal Milestone:
Component: pb Keywords:
Cc: spalax@…, grindizer@… Branch:
Author: Launchpad Bug:

Description

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.

Attachments (3)

IUsernamePassword_over_pb.patch (7.9 KB) - added by Grindaizer 3 years ago.
allow to defined custom hash metod to PB authentication.
pbHashMethod.tar.gz (2.2 KB) - added by Grindaizer 3 years ago.
example wich custom hash method and FilePasswordBD
IUsernamePassword_over_pb2.patch (18.2 KB) - added by spalax 2 years ago.
Patch allowing to use custom hash method for PB authentication. Improves IUsernamePassword_over_pb.patch

Download all attachments as: .zip

Change History (17)

comment:1 Changed 5 years ago by exarkun

  • Resolution set to invalid
  • Status changed from new to closed

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.

comment:2 Changed 5 years ago by 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?

comment:3 Changed 4 years ago by <automation>

  • Owner glyph deleted

Changed 3 years ago by Grindaizer

allow to defined custom hash metod to PB authentication.

Changed 3 years ago by Grindaizer

example wich custom hash method and FilePasswordBD

comment:4 Changed 3 years ago by Grindaizer

  • Resolution invalid deleted
  • Status changed from closed to reopened

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)

Changed 2 years ago by spalax

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

comment:5 Changed 2 years ago by spalax

  • Component changed from core to pb
  • Keywords review added

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.

comment:6 Changed 2 years ago by spalax

  • Cc spalax@… added

comment:7 follow-up: Changed 22 months ago by 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.

comment:8 in reply to: ↑ 7 Changed 22 months ago by 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).

comment:9 Changed 21 months ago by tom.prince

  • Keywords review removed
  • Owner set to spalax
  • Status changed from reopened to new

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.

comment:10 Changed 12 months ago by Grindaizer

  • Cc grindizer@… added

comment:11 follow-up: Changed 6 months ago by spalax

Hello,
I worked on this issue almost two years ago, and this problem has been in my mind since then. I have some time right now (and the next few days), and I will have some more next summer (starting mid-June). Here is a proposal to address this issue. It would be great if it was reviewed in the next couple of months, so that I can implement it next summer.

The Salted Challenge Response Authentication Mechanism (SCRAM) seems to be a reliable authentication mechanism that solves this issue: it is a challenge authentication scheme, allowing one to authenticate to a server, given that the server only stores a hash of the password that is not sufficient to authenticate. I offer to (try to) implement it in Twisted, which means:

  • implement SCRAM in Twisted so that one can authenticate using PB with a challenge-response authentication scheme, where only hashed passwords are stored by the server;
  • one of my goals is to be able to authenticate against the Unix /etc/passwd (or /etc/shadow);
  • write some tests;
  • ensure that it is retro-compatible;
  • write some documentation (if it is relevant, and if I am able to do so as a non-native English speaker).

SCRAM is implemented by Python library Passlib (compatible with Python 2.6, 2.7 and 3), so it migth be easier to use this.

If you agree, I should start working on it next summer.

comment:12 Changed 6 months ago by spalax

  • Keywords review added

comment:13 Changed 5 months ago by spalax

  • Owner spalax deleted

comment:14 in reply to: ↑ 11 Changed 7 days ago by habnabit

  • Keywords review removed
  • Owner set to spalax

Replying to spalax:

Sorry, did you mean to post new code? I'm not clear on why you put the review keyword back on this ticket, unless you forgot to post a new patch or wanted a review of your plan.

Regarding your plan, I'm not intimately familiar with PB, but it seems like implementing SCRAM would be changing fundamentals of PB's implementation. How exactly do you plan to fit SCRAM's client/server authentication messages into PB?

Note: See TracTickets for help on using tickets.