Re: SCRAM authentication, take three

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Aleksander Alekseev <a(dot)alekseev(at)postgrespro(dot)ru>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SCRAM authentication, take three
Date: 2017-03-02 06:50:34
Message-ID: CAB7nPqRbR3GmFYdedCAhzukfKrgBLTLtMvENOmPrVWREsZkF8g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Feb 21, 2017 at 9:53 AM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Mon, Feb 20, 2017 at 9:41 PM, Aleksander Alekseev
> <a(dot)alekseev(at)postgrespro(dot)ru> wrote:
>>> Speaking about flaws, it looks like there is a memory leak in
>>> array_to_utf procedure - result is allocated twice.
>
> Pushed a fix for this one on my branch.
>
>> And a few more things I've noticed after a closer look:
>>
>> * build_client_first_message does not free `state->client_nonce` if
>> second malloc (for `buf`) fails
>> * same for `state->client_first_message_bare`
>> * ... and most other procedures declared in fe-auth-scram.c file
>> (see malloc and strdup calls)
>
> You are visibly missing pg_fe_scram_free().
>
>> * scram_Normalize doesn't check malloc return value
>
> Yes, I am aware of this one. This makes the interface utterly ugly
> though because an error log message needs to be handled across many
> API layers. We could just assume anything returning NULL is equivalent
> to an OOM and nothing else though.

Attached is a new patch set. I have combined SASLprep with the rest
and fixed some conflicts. At the same time when going through NFKC
this morning I have noticed that the implementation was doing the
canonical decomposition and reordered the characters using the
combining classes, but the string recomposition was still missing.
This is addressed in this patch set, and well as on my dev tree:
https://github.com/michaelpq/postgres/tree/scram
--
Michael

Attachment Content-Type Size
0001-Refactor-SHA2-functions-and-move-them-to-src-common.patch application/octet-stream 35.4 KB
0002-Add-encoding-routines-for-base64-without-whitespace-.patch application/octet-stream 7.0 KB
0003-Add-clause-PASSWORD-val-USING-protocol-to-CREATE-ALT.patch application/octet-stream 10.0 KB
0004-Support-for-SCRAM-SHA-256-authentication-RFC-5802-an.patch.gz application/x-gzip 28.4 KB
0005-Add-regression-tests-for-passwords.patch application/octet-stream 10.4 KB
0006-Add-TAP-tests-for-authentication-methods.patch application/octet-stream 3.4 KB
0007-Make-hba-configuration-for-SASL-more-extensible.patch application/octet-stream 12.3 KB
0008-Implement-SASLprep-aka-NFKC-for-SCRAM-authentication.patch.gz application/x-gzip 60.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Corey Huinker 2017-03-02 06:54:27 Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless)
Previous Message Kyotaro HORIGUCHI 2017-03-02 06:42:37 Re: multivariate statistics (v24)