Horton

Re: Password identifiers, protocol aging and SCRAM protocol

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Michael Paquier <michael.paquier@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, David Steele <david@pgmasters.net>, Tom Lane <tgl@sss.pgh.pa.us>, David Fetter <david@fetter.org>, Alvaro Herrera <alvherre@2ndquadrant.com>, Magnus Hagander <magnus@hagander.net>, Julian Markwort <julian.markwort@uni-muenster.de>, Stephen Frost <sfrost@snowman.net>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>, Valery Popov <v.popov@postgrespro.ru>
Date: 2016-11-15T18:40:34Z
Lists: pgsql-hackers 
On Fri, Nov 4, 2016 at 11:58 AM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:
> The organization of these patches makes sense to me.
>
> On 10/20/16 1:14 AM, Michael Paquier wrote:
>> - 0001, moving all the SHA2 functions to src/common/ and introducing a
>> PG-like interface. No actual changes here.
>
> That's probably alright, although the patch contains a lot more changes
> than I would imagine for a simple file move.  I'll still have to review
> that in detail.

Even with git diff -M, reviewing 0001 is very difficult.  It does
things that are considerably in excess of what is needed to move these
files from point A to point B, such as:

- Renaming static functions to have a "pg" prefix.
- Changing the order of the functions in the file.
- Renaming an argument called "context" to "cxt".

I think that is a bad plan.  I think we should insist that 0001
content itself with a minimal move of the files changing no more than
is absolutely necessary.  If refactoring is needed, those changes can
be submitted separately, which will be much easier to review.  My
preliminary judgement is that most of this change is pointless and
should be reverted.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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.