From: David Steele <david@pgmasters.net>
To: Michael Paquier <michael.paquier@gmail.com>
Cc: PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Cc: v.popov@postgrespro.ru
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
Date: Tue, 15 Mar 2016 13:38:49 -0400
Message-ID: <56E848A9.1070203@pgmasters.net> (raw)
In-Reply-To: <CAB7nPqTssJpBUZ8g9oUTPJ6RPLCZYFtPypzdYgHL1gtuskxwkw@mail.gmail.com>
References: <CAB7nPqSMXU35g=W9X74HVeQp0uvgJxvYOuA4A-A3M+0wfEBv-w@mail.gmail.com>
<56E6D98F.1070701@pgmasters.net>
<CAB7nPqQYim-=PBHa19p04wSBbgGT=+ts1qWeQj2FG9HSmBX78Q@mail.gmail.com>
<CAB7nPqTssJpBUZ8g9oUTPJ6RPLCZYFtPypzdYgHL1gtuskxwkw@mail.gmail.com>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-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 produce a 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: david@pgmasters.net, michael.paquier@gmail.com, v.popov@postgrespro.ru
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
In-Reply-To: <56E848A9.1070203@pgmasters.net>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox