Subject: Re: [REVIEW]: Password identifiers, protocol aging and SCRAM
 protocol
To: Michael Paquier <michael.paquier@gmail.com>
References: 
 <CAB7nPqSMXU35g=W9X74HVeQp0uvgJxvYOuA4A-A3M+0wfEBv-w@mail.gmail.com>
 <56CF2DEA.8070204@postgrespro.ru>
 <CAB7nPqSwDsDZ=CeUcUX+yc5vmdT_N1rjLBJ_uGPfvOi850Sj1A@mail.gmail.com>
 <56D06C67.8050306@postgrespro.ru> <56D42ECF.2060501@postgrespro.ru>
 <CAB7nPqRg0p4VWALZzQiZ_DK8WyvHXYBGrt3_YJvoGx=J2yyXFg@mail.gmail.com>
 <CA+q6zcV7SGHyuspQ1e93q1HFaQpV+=wHgw3PUWSg8Ovs82ZgLQ@mail.gmail.com>
 <CAB7nPqQtCTtNokP==Zfi7HzZK0fVLTR=f1OspnuDX_d--T4fmQ@mail.gmail.com>
 <56D6A7AB.50306@postgrespro.ru>
 <CAB7nPqQxdgQRien=dCJuCPfHvPhsMTkvdU8S3vvNn5+9OAZj8w@mail.gmail.com>
Cc: Dmitry Dolgov <9erthalion6@gmail.com>,
 PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
From: Valery Popov <v.popov@postgrespro.ru>
Message-ID: <56D812C3.80800@postgrespro.ru>
Date: Thu, 3 Mar 2016 13:32:35 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.5.1
MIME-Version: 1.0
In-Reply-To: 
 <CAB7nPqQxdgQRien=dCJuCPfHvPhsMTkvdU8S3vvNn5+9OAZj8w@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

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.popov@postgrespro.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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers