Re: Password identifiers, protocol aging and SCRAM protocol

On Fri, Jul 22, 2016 at 3:43 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Fri, Jul 22, 2016 at 9:06 AM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Fri, Jul 22, 2016 at 9:02 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Michael Paquier <michael.paquier@gmail.com> writes:
>>>> One thing about my current set of patches is that I have begun adding
>>>> files from src/common/ to libpq's list of files. As that would be new
>>>> I am wondering if I should avoid doing so.
>>>
>>> Well, it could link source files from there just as easily as from the
>>> backend.  Not object files, though.
>>
>> OK. I'll just keep things the current way then :)
>
> Note: I have put more energy into that and I think that I will be able
> to publish a new patch set pretty soon, like at the beginning of next
> week.

Ok, here is the real deal. As discussed at PGcon, I have shaved off
from the set of patches the following things:
- No separate catalog pg_auth_verifier
- No additional column in pg_authid to determine the password type.
All the logic used check if the password string has a wanted format.
We do that for MD5 now, this set does it for SCRAM.
- Removal of the pg_upgrade stuff.
- Removal of password_protocols, so we don't care anymore about protocol aging.
In short, the SCRAM verifiers get stored in rolpassword.

And here is what this set of patches does:
- Implementation of SCRAM-SHA-256, and not SHA1. I have moved to the
one that makes the most sense considering the current situation based
on RFC 5802 and 7677.
- No channel binding support. I guess that this could be added later on.
- password_encryption is now an enum, and gains three values: md5,
plain and scram. true => md5, false => plain for backward
compatibility
- Grammar of CREATE/ALTER ROLE is extended with PASSWORD val USING
protocol, that's a separate patch applying on top of the core patch
for SASL.

I have noticed as well a couple of bugs in the previous set(s) of patches:
- valid_until was not checked for SCRAM
- When using ENCRYPTED or UNENCRYPTED, already encrypted password
should be used as-is. The same is applied to PASSWORD USING protocol
to ease dump and reload. That's actually what is used for MD5.

And here is a detail of the patches:
- 0001, refactoring of SHA functions into src/common.
- 0002, refactoring for sendAuthRequest
- 0003, Refactoring for RandomSalt to accomodate with the salt used by
scram (length of 10 bytes, md5 is 4).
- 0004, move encoding routines to src/common/
- 0005, make password_encryption an enum
- 0006, refactor some code in CREATE/ALTER role code paths related the
use of password_encryption
- 0007, refactor some code to have a single routine to fetch password
and valid_until from pg_authid
- 0008, The core implementation of SCRAM-SHA-256, with the SASL
communication protocol. if you want to use SCRAM with that, things go
with password_encryption = 'scram'.
- 0009, addition of PASSWORD val USING protocol
- 0010. regression tests for passwords. Not sure how useful they would
be. But they helped me a bit.

I am adding an entry in the next CF. Comments are welcome.
-- 
Michael