Re: Password identifiers, protocol aging and SCRAM protocol

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: David Steele <david(at)pgmasters(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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>, 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-10-18 07:35:27
Message-ID: CAB7nPqTxH8Nnvo0ii4w6881cT3OODgJcE7RFk-WaS+mYD-7pyg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 17, 2016 at 6:18 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Mon, Oct 17, 2016 at 5:55 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>> Ok, committed to HEAD.

Attached is a rebased patch set for SCRAM, with the following things:
- 0001, moving all the SHA2 functions to src/common/ and introducing a
PG-like interface. No actual changes here.
- 0002, creating a set of base64 routines without whitespace handling.
Previous version sent had a bug: I missed the point that the backend
version of base64 was adding a newline every 76 characters. So this is
removed to make the encoding not using any whitespace. Also the
routines are reworked so as they return -1 in the event of an error
instead of generating an elog by themselves. That will be useful for
SCRAM that needs to do its own error handling with the e= messages
from the server. I think that's cleaner this way. Encoding does not
have any error code paths, but decoding has, so one possible
improvement would be to add in arguments a string to store an error
message to make things easier for callers to debug.
- 0003 does some refactoring regarding encrypted passwords in user.c.
I am pretty happy with this one as well.
- 0004 adds the extension for CREATE ROLE .. PASSWORD foo USING
protocol. I found a bug in this one when using CREATE|ALTER ROLE ..
PASSWORD missing to update the given password correctly using
password_encryption. This one I am happy with it. Even if it depends
on 0005 in this patch set it is possible to make it independent of it
to introduce the grammar just for 'plain' and 'md5' first. In previous
sets it was located after SCRAM, but it looks cleaner to get that
first. I don't think I am going to change that much more now.
- 0005 adds support for SCRAM-SHA-256. There is still some work to do
here, particularly the error handling that requires to be extended
with the e= messages sent back to the client before moving to a
PG-like error code path. Those need to be set in the context of the
SASL message exchange. I noticed as well that this is missing a hell
lot of error checks when building the exchange messages, and when
doing encoding and decoding of base64 strings. I'll address that in
the next couple of days.
- 0006 is the basic set of regression tests for passwords. Nothing new
here, they are useful as basic tests when checking the patch. I don't
think that they are worth having committed at the end.
--
Michael

Attachment Content-Type Size
0001-Refactor-SHA2-functions-and-move-them-to-src-common.patch application/x-download 47.3 KB
0002-Add-encoding-routines-for-base64-without-whitespace-.patch application/x-download 6.9 KB
0003-Refactor-decision-making-of-password-encryption-into.patch application/x-download 4.7 KB
0004-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch application/x-download 9.3 KB
0005-Support-for-SCRAM-SHA-256-authentication-RFC-5802-an.patch application/x-download 83.2 KB
0006-Add-regression-tests-for-passwords.patch application/x-download 9.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Erik Rijkers 2016-10-18 07:41:14 simplehash.h typo fix
Previous Message Tatsuo Ishii 2016-10-18 05:34:33 Re: Illegal SJIS mapping