Re: Password identifiers, protocol aging and SCRAM protocol

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: David Steele <david(at)pgmasters(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Fetter <david(at)fetter(dot)org>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>, Stephen Frost <sfrost(at)snowman(dot)net>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Valery Popov <v(dot)popov(at)postgrespro(dot)ru>
Subject: Re: Password identifiers, protocol aging and SCRAM protocol
Date: 2016-07-25 08:04:17
Message-ID: CAB7nPqQvO4sxLFeS9D+NM3wpy08ieZdAj_6e117MQHZAfxBFsg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 22, 2016 at 3:43 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Jul 22, 2016 at 9:06 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> On Fri, Jul 22, 2016 at 9:02 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Michael Paquier <michael(dot)paquier(at)gmail(dot)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

Attachment Content-Type Size
0004-Move-encoding-routines-to-src-common.patch application/x-patch 23.8 KB
0005-Switch-password_encryption-to-a-enum.patch application/x-patch 9.1 KB
0006-Refactor-decision-making-of-password-encryption-into.patch application/x-patch 4.6 KB
0007-Create-generic-routine-to-fetch-password-and-valid-u.patch application/x-patch 4.5 KB
0008-Support-for-SCRAM-SHA-256-authentication-RFC-5802-an.patch application/x-patch 73.8 KB
0009-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch application/x-patch 8.7 KB
0010-Add-regression-tests-for-passwords.patch application/x-patch 9.1 KB
0001-Refactor-SHA-functions-and-move-them-to-src-common.patch application/x-patch 73.2 KB
0002-Refactor-sendAuthRequest.patch application/x-patch 5.6 KB
0003-Refactor-RandomSalt-to-handle-salts-of-different-len.patch application/x-patch 2.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2016-07-25 08:18:33 Fix comment in ATExecValidateConstraint
Previous Message Amit Langote 2016-07-25 06:57:00 Re: Constraint merge and not valid status