Re: Password identifiers, protocol aging and SCRAM protocol - Mailing list pgsql-hackers
From | David Steele |
---|---|
Subject | Re: Password identifiers, protocol aging and SCRAM protocol |
Date | |
Msg-id | 56E848A9.1070203@pgmasters.net Whole thread Raw |
In response to | Re: Password identifiers, protocol aging and SCRAM protocol (Michael Paquier <michael.paquier@gmail.com>) |
Responses |
Re: Password identifiers, protocol aging and SCRAM protocol
|
List | pgsql-hackers |
Hi Michael, On 3/14/16 7:07 PM, Michael Paquier wrote: > On Mon, Mar 14, 2016 at 5:06 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > >> On Mon, Mar 14, 2016 at 4:32 PM, David Steele <david@pgmasters.net> wrote: >> >>> Could you provide an updated set of patches for review? Meanwhile I am >>> marking this as "waiting for author". >> >> Sure. I'll provide them shortly with all the comments addressed. Up to >> now I just had a couple of comments about docs and whitespaces, so I >> didn't really bother sending a new set, but this meritates a rebase. > > And here they are. I have addressed the documentation and the > whitespaces reported up to now at the same time. For this first review I would like to focus on the user visible changes introduced in 0001-0002. First I created two new users with each type of supported verifier: postgres=# create user test with password 'test'; CREATE ROLE postgres=# create user testu with unencrypted password 'testu' valid until '2017-01-01'; CREATE ROLE 1) I see that rolvaliduntil is still in pg_authid: postgres=# select oid, rolname, rolvaliduntil from pg_authid; oid | rolname | rolvaliduntil -------+---------+------------------------ 10 | vagrant |16387 | test |16388 | testu | 2017-01-01 00:00:00+00 I think that's OK if we now define it to be "role validity" (it's still password validity in the patched docs). I would also like to see a validuntil column in pg_auth_verifiers so we can track password expiration for each verifier separately. For now I think it's enough to copy the same validity both places since there can only be one verifier. 2) I don't think the column naming in pg_auth_verifiers is consistent with other catalogs: postgres=# select * from pg_auth_verifiers; roleid | verimet | verival --------+---------+------------------------------------- 16387 | m | md505a671c66aefea124cc08b76ea6d30bb 16388 | p | testu System catalogs generally use a 3 character prefix so I would expect the columns to be (if we pick avr as a prefix): avrrole avrmethod avrverifier avrvaliduntil I'm not a big fan in abbreviating too much so you can see I've expanded the names a bit. 3) rolpassword is still in pg_shadow even though it is not useful anymore: postgres=# select usename, passwd, valuntil from pg_shadow; usename | passwd | valuntil ---------+----------+------------------------vagrant | ******** |test | ******** |testu | ******** | 2017-01-01 00:00:00+00 If anyone is actually using this column in a meaningful way they are in for a nasty surprise when trying use the value in passwd as a verifier.I would prefer to drop the column entirely and producea clear error. Perhaps a better option would be to drop pg_shadow entirely since it seems to have no further purpose in life. Thanks, -- -David david@pgmasters.net
pgsql-hackers by date: