Re: scram and \password

On 05/02/2017 07:47 PM, Robert Haas wrote:
> On Tue, May 2, 2017 at 3:42 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> There's going to be a default, one way or another. The default is going to
>> come from password_encryption, or it's going to be a hard-coded value or
>> logic based on server-version in PQencryptPasswordConn(). Or it's going to
>> be a hard-coded value or logic implemented in every application that uses
>> PQencryptPasswordConn(). I think looking at password_encryption makes the
>> most sense. The application is not in a good position to make the decision,
>> and forcing the end-user to choose every time they change a password is too
>> onerous.
>
> I think there should be no default, and the caller should have to pass
> the algorithm explicitly. If they want to determine what default to
> pass by running 'SHOW password_encryption', that's their choice.

Ok, gotcha. I disagree, I think we should provide a default. Libpq is in
a better position to make a good choice than most applications.

I've committed the new PQencryptPasswordConn function, with the default
behavior of doing "show password_encryption", and the changes to use it
in psql and createuser. This closes the open issue with \password.

On 04/27/2017 07:03 AM, Michael Paquier wrote:
> I think that it is a mistake to move SASLprep out of
> scram_build_verifier, because pre-processing the password is not
> necessary, it is normally mandatory. The BE/FE versions that you are
> adding also duplicate the calls to pg_saslprep().

I played with that a little bit, but decided to keep pg_saslprep() out
of scram_build_verifier() after all. It would seem asymmetric to have
scram_build_verifier() call pg_saslprep(), but require callers of
scram_SaltedPassword() to call it. So for consistency, I think
scram_SaltedPassword() should also call pg_saslprep(). That would
complicated the scram_SaltedPassword() function, however. It would need
to report an OOM error somehow, for starters. Not an insurmountable
issue, of course, but it felt cleaner this way, after all, despite the
duplication.

> Using "encrypt" instead of "hash" in the function name :(

Yeah. For better or worse, I've kept the "encrypt" nomenclature
everywhere, for consistency.

Thanks!

- Heikki