Re: SCRAM authentication, take three

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