Re: [REVIEW]: Password identifiers, protocol aging and SCRAM protocol

From: Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [REVIEW]: Password identifiers, protocol aging and SCRAM protocol
Date: 2016-03-03 10:32:35
Message-ID: 56D812C3.80800@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

This is a review of "Password identifiers, protocol aging and SCRAM
protocol" patches
http://www.postgresql.org/message-id/CAB7nPqSMXU35g=W9X74HVeQp0uvgJxvYOuA4A-A3M+0wfEBv-w@mail.gmail.com

Contents & Purpose
--------------------------
There was a discussion dedicated to SCRAM:
http://www.postgresql.org/message-id/55192AFE.6080106@iki.fi

This set of patches implements the following:
- Introduce in Postgres an extensible password aging facility, by having
a new concept of 1 user/multiple password verifier, one password
verifier per protocol.
- Give to system administrators tools to decide unsupported protocols,
and have pg_upgrade use that
- Introduce new password protocols for Postgres, aimed at replacing
existing, say limited ones.

This set of patches consists of 9 separate patches.
Description of each patch is well described in initial thread email and
in comments.
The first set of patches 0001-0008 adds facility to store multiple
password verifiers,
CREATE ROLE and ALTER ROLE are extended with PASSWORD VERIFIERS, new
superuser GUC parameters which specifies a list of supported password
protocols in Postgres backend, added pg_auth_verifiers_sanitize
function, removed password verifiers for unsupported protocols in
pg_upgrade, and more features.
The second set of patch 0005~0008 introduces a new protocol, SCRAM, and
0009 is SCRAM itself.

Initial Run
-------------
Included in the patches are:
- source code
- regression tests
- documentation
The source code is well commented.
The patches are in context diff format and were applied correctly to
HEAD (there were 2 warnings, and it was fixed by author).
There were several markup warnings, should be fixed by author.
Regression tests pass successfully, without errors. It seems that the
patches work as expected.
The patch 0009 depends on all previous patches 0001-0008: first we need
to apply patches 0001-0008, then 0009.

Performance
-----------
I have not tested possible performance issues yet.

Conclusion
--------------
I think introduced features are useful and I vote for commit +1.

On 03/02/2016 02:55 PM, Michael Paquier wrote:
> On Wed, Mar 2, 2016 at 5:43 PM, Valery Popov <v(dot)popov(at)postgrespro(dot)ru> wrote:
> So the <literal> markup is missing. Thanks. I am taking note of it.

--
Regards,
Valery Popov
Postgres Professional http://www.postgrespro.com
The Russian Postgres Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Emre Hasegeli 2016-03-03 10:45:19 Re: SP-GiST support for inet datatypes
Previous Message Dmitry Dolgov 2016-03-03 10:31:04 Re: jsonb array-style subscription