Re: WIP: SCRAM authentication

* Robert Haas (robertmhaas(at)gmail(dot)com) wrote:
> On Fri, Aug 7, 2015 at 6:54 PM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > This filtering machinery definitely looks like a GUC to me, something
> > like password_forbidden_encryption that PASSWORD VERIFIERS looks at
> > and discards the methods listed in there. This definitely needs to be
> > separated from password_encryption.
>
> I don't know what a "password verifier" is and I bet nobody else does
> either. Well, I think I sort of know: I think it's basically an
> encrypted password. Am I right? Even if I am, I bet the average user
> is going to scratch their head and punt.

Password verifier is actually a well understood term when it comes to
these protocols and their implementations. It is not an encrypted
password but rather a value which allows the server to determine if the
client knows the correct password, without having to store the password
directly, or a simple hash of the password, or have the clear password
sent from the client sent to the server.

> I don't see that there's any good reason to allow the same password to
> be stored in the catalog encrypted more than one way, and I don't
> think there's any good reason to introduce the PASSWORD VERIFIER
> terminology. I think we should store (1) your password, either
> encrypted or unencrypted; and (2) the method used to encrypt it. And
> that's it.

Perhaps we don't want to expose what a password verifier is to users,
but we shouldn't be missing the distinction between hashed passwords,
encrypted passwords, and password verifiers in the code and in the
implementation of SCRAM. We really shouldn't use an incorrect term for
what we're storing in pg_authid either though, which is what we do
today.

I can't see us ever storing encrypted passwords as that implies we'd
need a key stored somewhere and further that the server would be able to
get back to the user's original password, neither of which are things we
want to deal with.

You do have a good point that there is some risk associated with having
multiple values in pg_authid related to a user's password and that we
really want to help users move from the old value in pg_authid to the
new one. I don't believe we should force a hard change as it's going to
cause a lot of trouble for users. We have to also consider that clients
also have to be changed for this.

As discussed previously, in an ideal world, we would handle the old
values and the new ones while introducing password ageing, client
support for detecting that a password needs to be changed, protocol
support for changing passwords which avoids having them get logged in
cleartext to the server log, password complexity, and perhaps used
password history to boot. The main issue here is that we really don't
provide any help for large installations to get their userbase moved off
of the old style today. Password ageing (and good support for it in
clients, etc), would help that greatly.

Unfortunately, it's unlikely that we're going to get all of that done in
one release. As such, I'd suggest our next release support the existing
values in pg_authid, add the password verifier when the password is
changed, and then add a GUC in the following release which disables the
old pg_authid mechanism, defaulting to true, and the release after that
remove support for the old value and the field for it completely.

We should also provide documentation about how to check if there are any
old style values, for users who want to be proactive about moving off of
the old style.

I'm travelling and so I haven't looked over the patch yet (or even read
the entire thread in depth), so apologies if I've got something confused
about what's being proposed.

Thanks!

Stephen