Re: Password identifiers, protocol aging and SCRAM protocol

----[This is a rather informal user-review]----

Here are some thoughts and experiences on using the new features, I
focused on testing the basic funcionality of setting password_encryption
to scram and then generating some users with passwords. After that, I
took a look at the documentation, specifically all those parts that
mentioned "md5", but not SCRAM, so i took some time to write those down
and add my thoughts on them.

We're quite keen on seeing these features in a future release, so I
suggest that we add these patches to the next commitfest asap in order
to keep the discussion on this topic flowing.

For those of you who like to put the authentication method itself up for
discussion, I'd like to add that it seems fairly simple to insert code
for new authentication mechanisms.
In conclusion I think these patches are very useful.

My remarks follow below.

Kind regards,
Julian Markwort
julian(dot)markwort(at)uni-muenster(dot)de

Things I noticed:
1.
when using either
CREATE ROLE
ALTER ROLE
with the parameter
ENCRYPTED
md5 encryption is always assumed (I've come to realize that
UNENCRYPTED always equals plain and, in the past, ENCRYPTED equaled md5
since there were no other options)

I don't know if this is intended behaviour. Maybe this option
should be omitted (or marked as deprecated in the documentation) from
the CREATE/ALTER functions (since without this Option, the
password_encryption from pg_conf.hba is used)
or maybe it should have it's own parameter like
CREATE ROLE testuser WITH LOGIN ENCRYPTED 'SCRAM' PASSWORD 'test';
so that the desired encryption is used.
From my point of view, this would be the sensible thing to do,
especially if different verifiers should be allowed (as proposed by
these patches).
In either case, a bit of text explaining the (UN)ENCRYPTED option
should be added to the documentation of the CREATE/ALTER ROLE functions.

2.
Documentation
III.
17. Server Setup and Operation
17.2. Creating a Database Cluster: maybe list SCRAM as a
possible method for securing the db-admin

19. Client Authentication
19.1. The pg_hba.conf File: SCRAM is not listed in the list
of available auth_methods to be specified in pg_conf.hba
19.3 Authentication Methods
19.3.2 Password Authentication: SCRAM would belong to
the same category as md5 and password, as they are all password-based.

20. Database Roles
20.2. Role Attributes: password : list SCRAM as
authentication method as well

VI.
ALTER ROLE: is SCRAM also dependent on the role name for
salting? if so, add warning.
(it doesn't seem that way, however I'm curious as
to why the function FlattenPasswordIdentifiers in
src/backend/commands/user.c called by AlterRole passes rolname to
scram_build_verifier(), when that function does absolutely nothing with
this argument?)
CREATE ROLE: can SCRAM also be used in the list of PASSWORD
VERIFIERS?

VII.
49. System Catalogs:
49.9 pg_auth_verifiers: Column names and types are mixed up
in description for column vervalue:
explain some basic stuff about md5
maybe as well?

remark: the statements about the
composition of the string that is md5-hashed are contradictory.
(concatenating "bar" to "foo"
results in foobar, not the other way round, as it is implied in the
explanation of the md5 hashing), this however, is not really linked to
the changes introduced with these patches.

remark: naming inconsistency: md5
vervalues are stored "md5*" why don't we take the same approach and use
it on SCRAM hashes (i.e. "scram*" ).
(if this is a general convention
thing, please ignore this comment, however I couldn't find anything in
the relevant RFC's while skimming through them).

50. Frontend/Backend Protocol
50.2.1 Start-up: add explanation for
"AuthenticationSCRAMPassword" authentication request message. (?)
50.5 message formats see 50.2.1