Re: Password identifiers, protocol aging and SCRAM protocol

On Mon, Oct 17, 2016 at 6:18 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Mon, Oct 17, 2016 at 5:55 PM, Heikki Linnakangas <hlinnaka@iki.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