Re: pg_authid.rolpassword format (was Re: Password identifiers, protocol aging and SCRAM protocol)

On 12/15/2016 03:00 AM, Michael Paquier wrote:
> On Wed, Dec 14, 2016 at 8:33 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> But, a password stored in plaintext works with either MD5 or SCRAM, or any
>> future authentication mechanism. So as soon as we have SCRAM authentication,
>> it becomes somewhat useful again.
>>
>> In a nutshell:
>>
>> auth / stored MD5 SCRAM plaintext
>> -----------------------------------------
>> password Y Y Y
>> md5 Y N Y
>> scram N Y Y
>>
>> If a password is stored in plaintext, it can be used with any authentication
>> mechanism. And the plaintext 'password' authentication mechanism works with
>> any kind of a stored password. But an MD5 hash cannot be used with SCRAM
>> authentication, or vice versa.
>
> So.. I have been thinking about this portion of the thread. And what I
> find the most scary is not the fact that we use plain passwords for
> SCRAM authentication, it is the fact that we would need to do a
> catalog lookup earlier in the connection workflow to decide what is
> the connection protocol to use depending on the username provided in
> the startup packet if the pg_hba.conf entry matching the user and
> database names uses "password".

I don't see why we would need to do a catalog lookup any earlier. With
"password" authentication, the server can simply request the client to
send its password. When it receives it, it performs the catalog lookup
to get pg_authid.rolpassword. If it's in plaintext, just compare it, if
it's an MD5 hash, hash the client's password and compare, and if it's a
SCRAM verifier, build a verifier with the same salt and iteration count
and compare.

> And, honestly, why do we actually need to have a support table that
> spread? SCRAM is designed to be secure, so it seems to me that it
> would on the contrary a bad idea to encourage the use of plain
> passwords if we actually think that they should never be used (they
> are actually useful for located, development instances, not production
> ones).

I agree we should not encourage bad password practices. But as long as
we support passwords to be stored in plaintext at all, it makes no sense
to not allow them to be used with SCRAM. The fact that you can use a
password stored in plaintext with both MD5 and SCRAM is literally the
only reason you would store a password in plaintext, so if we don't want
to allow that, we should disallow storing passwords in plaintext altogether.

> So what I would suggest would be to have a support table like
> that:
> auth / stored MD5 SCRAM plaintext
> -----------------------------------------
> password Y Y N
> md5 Y N Y
> scram N N Y

I was using 'Y' to indicate that the combination works, and 'N' to
indicate that it does not. Assuming you're using the same notation, the
above doesn't make any sense.

> So here is an idea for things to do now:
> 1) do not change the format of the existing passwords
> 2) do not change pg_authid
> 3) block access to instances if "password" or "md5" are used in
> pg_hba.conf if the user have a SCRAM verifier.
> 4) block access if "scram" is used and if user has a plain or md5 verifier.
> 5) Allow access if "scram" is used and if user has a SCRAM verifier.
> We had a similar discussion regarding verifier/password formats last
> year but that did not end well. It would be sad to fall back again
> into this discussion and get no result. If somebody wants to support
> access to SCRAM with plain password entries, why not. But that would
> gain a -1 from me regarding the earlier lookup of pg_authid needed to
> do the decision making on the protocol to use. And I think that we
> want SCRAM to be designed to be a maximum stable and secure.

The bottom line is that at the moment, when plaintext passwords are
stored as is, without any indicator that it's a plaintext password, it's
ambiguous whether a password is a SCRAM verifier, or if it's a plaintext
password that just happens to begin with the word "scram:". That is
completely unrelated to which combinations of stored passwords and
authentication mechanisms we actually support or allow to work.

The only way to distinguish, is to know about every verifier kind there
is, and check whether rolpassword looks valid as anything else than a
plaintext password. And we already got tripped by a bug-of-omission on
that once. If we add more verifier formats in the future, it's bound to
happen again. Let's nip that source of bugs in the bud. Attached is a
patch to implement what I have in mind.

Alternatively, you could argue that we should forbid storing passwords
in plaintext altogether. I'm OK with that, too, if that's what people
prefer. Then you cannot have a user that can log in with both MD5 and
SCRAM authentication, but it's certainly more secure, and it's easier to
document.

- Heikki