Re: [HACKERS] SCRAM authentication, take three - Mailing list pgsql-hackers

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)
* scram_Normalize doesn't check malloc return value

Sorry for lots of emails.

On Mon, Feb 20, 2017 at 03:15:14PM +0300, Aleksander Alekseev wrote:
> Speaking about flaws, it looks like there is a memory leak in
> array_to_utf procedure - result is allocated twice.
>
> On Mon, Feb 20, 2017 at 02:51:13PM +0300, Aleksander Alekseev wrote:
> > Hi!
> >
> > Currently I don't see any significant flaws in these patches. However I
> > would like to verify that implemented algorithms are compatible with
> > algorithms implemented by third party.
> >
> > For instance, for user 'eax' and password 'pass' I got the following
> > record in pg_authid:
> >
> > ```
> > scram-sha-256:
> > xtznkRN/nc/1DQ==:
> > 4096:
> > 2387c124a3139a276b848c910f43ece05dd670d0977ace4f20d724af522312e4:
> > 5e3bdd6584880198b0375acabd8754c460af2389499f71a756660a10a8aaa843
> > ```
> >
> > Let's say I would like to implement SCRAM in pure Python, for instance
> > add it to pg8000 driver. Firstly I need to know how to generate server
> > key and client key having only user name and password. Somehow like
> > this:
> >
> > ```
> >  >>> import base64
> >  >>> import hashlib
> >  >>> base64.b16encode(hashlib.pbkdf2_hmac('sha256', b'pass',
> >  ...    base64.b64decode(b'xtznkRN/nc/1DQ=='), 4096))
> > b'14B90CFCF690120399A0E6D30C60DD9D9D221CD9C2E31EA0A00514C41823E6C3'
> > ```
> >
> > Naturally it doesn't work because both SCRAM_SERVER_KEY_NAME and
> > SCRAM_CLIENT_KEY_NAME should also be involved. I see PostgreSQL
> > implementation just in front of me but unfortunately I'm still having
> > problems calculating exactly the same server key and client key. It makes
> > me worry whether PostgreSQL implementation is OK.
> >
> > Could you please give an example of how to do it?
> >
> > On Mon, Feb 20, 2017 at 03:29:19PM +0900, Michael Paquier wrote:
> > > On Sun, Feb 19, 2017 at 10:07 PM, Michael Paquier
> > > <michael.paquier@gmail.com> wrote:
> > > > There is something that I think is still unwelcome in this patch: the
> > > > interface in pg_hba.conf. I mentioned that in the previous thread but
> > > > now if you want to match a user and a database with a scram password
> > > > you need to do that with the current set of patches:
> > > > local $dbname $user scram
> > > > That's not really portable as SCRAM is one protocol in the SASL
> > > > family, and even worse in our case we use SCRAM-SHA-256. I'd like to
> > > > change pg_hba.conf to be as follows:
> > > > local $dbname $user sasl protocol=scram_sha_256
> > > > This is extensible for the future, and protocol is a mandatory option
> > > > that would have now just a single value: scram_sha_256. Heikki,
> > > > others, are you fine with that?
> > >
> > > I have implemented that as 0009 which is attached, and need to be
> > > applied on the rest of upthread. I am not sure if we want to make the
> > > case where no protocol is specified map to everything. This would be a
> > > tricky support for users in the future if new authentication
> > > mechanisms for SASL are added in the future.
> > >
> > > Another issue that I have is: do we really want to have
> > > password_encryption being set to "scram" for verifiers of
> > > SCRAM-SHA-256? I would think that scram_sha_256 makes the most sense.
> > > Who knows, perhaps there could be in a couple of years a SHA-SHA-512..
> > >
> > > At the same time, attached is a new version of 0008 that implements
> > > SASLprep, I have stabilized the beast after fixing some allocation
> > > calculations when converting the decomposed pg_wchar array back to a
> > > utf8 string.
> > > --
> > > Michael
> >
> > --
> > Best regards,
> > Aleksander Alekseev
>
>
>
> --
> Best regards,
> Aleksander Alekseev



--
Best regards,
Aleksander Alekseev