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

Valery Popov <v.popov@postgrespro.ru>

From: Valery Popov <v.popov@postgrespro.ru>
To: pgsql-hackers@postgresql.org, Michael Paquier <michael.paquier@gmail.com>
Date: 2016-02-29T11:43:11Z
Lists: pgsql-hackers
Hi, Michael
>>>
>>>
>>> 23.02.2016 10:17, Michael Paquier пишет:
>>>> Attached is a set of patches implementing a couple of things that have
>>>> been discussed, so let's roll in.
>>>>
>>>> Those 4 patches are aimed at putting in-core basics for the concept I
>>>> call password protocol aging, which is a way to allow multiple
>>>> password protocols to be defined in Postgres, and aimed at easing
>>>> administration as well as retirement of outdated protocols, which is
>>>> something that is not doable now in Postgres.
>>>>
>>>> The second set of patch 0005~0008 introduces a new protocol, SCRAM.
>>>> 9) 0009 is the SCRAM authentication itself....
>>> The theme with password checking is interesting for me, and I can give
>>> review for CF for some features.
>>> I think that review of all suggested features will require a lot of 
>>> time.
>>> Is it possible to make subset of patches concerning only password 
>>> strength
>>> and its aging?
>>> The patches you have applied are non-independent. They should be apply
>>> consequentially one by one.
>>> Thus the patch 0009 can't be applied without git error  before 0001.
>>> In this conditions all patches were successfully applied and compiled.
>>> All tests successfully passed.
>> If you want to focus on the password protocol aging, you could just
>> have a look at 0001~0004.
> OK, I will review patches 0001-0004, for starting.
>
Below are the results of compiling and testing.
============================
I've got the last version of sources from 
git://git.postgresql.org/git/postgresql.git.

vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git branch
* master

Then I've applied patches 0001-0004 with two warnings:
vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 
0001-Add-facility-to-store-multiple-password-verifiers.patch
0001-Add-facility-to-store-multiple-password-verifiers.patch:2547: 
trailing whitespace.
warning: 1 line adds whitespace errors.
vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 
0002-Introduce-password_protocols.patch
vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 
0003-Add-pg_auth_verifiers_sanitize.patch
0003-Add-pg_auth_verifiers_sanitize.patch:87: indent with spaces.
     if (!superuser())
warning: 1 line adds whitespace errors.
vpopov@vpopov-Ubuntu:~/Projects/pwdtest/postgresql$ git apply 
0004-Remove-password-verifiers-for-unsupported-protocols-.patch
The compilation with option ./configure --enable-debug --enable-nls 
--enable-cassert  --enable-tap-tests --with-perl
was successful.
Regression tests and all TAP-tests also passed successfully.

Also I've applied patches 0005-0008 into clean sources directory with no 
warnings.
vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 
0005-Move-sha1.c-to-src-common.patch
vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 
0006-Refactor-sendAuthRequest.patch
vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 
0007-Refactor-RandomSalt-to-handle-salts-of-different-len.patch
vpopov@vpopov-Ubuntu:~/Projects/pwdtest2/postgresql$ git apply 
0008-Move-encoding-routines-to-src-common.patch
The compilation with option ./configure --enable-debug --enable-nls 
--enable-cassert  --enable-tap-tests --with-perl
was successful.
Regression and the TAP-tests also passed successfully.

The patch 0009 depends on all previous patches 0001-0008: first we need 
to apply patches 0001-0008, then 0009.
Then, all patches were successfully compiled.
All test passed.

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



Commits

  1. Support SCRAM-SHA-256 authentication (RFC 5802 and 7677).

  2. Refactor SHA2 functions and move them to src/common/.

  3. Replace isMD5() with a more future-proof way to check if pw is encrypted.

  4. Remove bogus notice that older clients might not work with MD5 passwords.

  5. Refactor the code for verifying user's password.

  6. Replace PostmasterRandom() with a stronger source, second attempt.

  7. Remove support for (insecure) crypt authentication.