Re: WIP: SCRAM authentication - Mailing list pgsql-hackers

On Mon, Mar 30, 2015 at 7:52 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

There have been numerous threads on replacing our MD5 authentication method, so I started hacking on that to see what it might look like. Just to be clear, this is 9.6 material. Attached is a WIP patch series that adds support for SCRAM. There's no need to look at the details yet, but it demonstrates what the protocol changes and the code structure would be like.

I'm not wedded to SCRAM - SRP or JPAKE or something else might be better. But replacing the algorithm, or adding more of them, should be straightforward with this.

There is no negotiation of the authentication mechanism. SCRAM is just added as a new one, alongside all the existing ones. If the server requests SCRAM authentication, but the client doesn't support it, the attempt will fail. We might want to do something about that, to make the transition easier, but it's an orthogonal feature and not absolutely required.

There are four patches in the series. The first two are just refactoring: moving the SHA-1 implementation from pgcrypto to src/common, and some refactoring in src/backend/auth.c that IMHO would make sense anyway.

Patches three and four are the interesting ones:

3. Allow storing multiple verifiers in pg_authid
------------------------------------------------

Replace the pg_authid.rolpassword text field with an array, and rename it to 'rolverifiers'. This allows storing multiple password hashes: an MD5 hash for MD5 authentication, and a SCRAM salt and stored key for SCRAM authentication, etc. Each element in the array is a string that begins with the method's name. For example "md5:<MD5 hash>", or "password:<plaintext>".

For dump/reload, and for clients that wish to create the hashes in the client-side, there is a new option to CREATE/ALTER USER commands: PASSWORD VERIFIERS '{ ... }', that allows replacing the array.

The old "ENCRYPTED/UNENCRYPTED PASSWORD 'foo'" options are still supported for backwards-compatibility, but it's not clear what it should mean now.

TODO:

* Password-checking hook needs to be redesigned, to allow for more kinds of hashes.

* With "CREATE USER PASSWORD 'foo'", which hashes/verifiers should be generated by default? We currently have a boolean password_encryption setting for that. Needs to be a list.

- New password-checking hook (contrib/passwordcheck will need a refresh). As of now, we have that:
void (*check_password_hook_type)
    (const char *username,
     const char *password,
     int password_type,
     Datum validuntil_time,
     bool validuntil_null);

We need to switch to something that checks a list of hashes:
void (*check_password_hook_type)
    (const char *username,
     list *passwd,
     Datum validuntil_time,
     bool validuntil_null);

passwd is a structure containing the password type and the hash value. Password type can then be "plain" (or password to match pg_hba.conf) or "md5" for now.
- When password_encryption is switched to a list, true means md5, and false means plain. At the addition of SCRAM, we could think harder the default value, "true" may be worth meaning "md5,scram".

- For CREATE ROLE/ALTER ROLE, it is necessary to be able to define the list of hashes that need to be generated, with something like that for example:
[ ENCRYPTED [(md5[, scram])] | UNENCRYPTED ] PASSWORD 'password'
When UNENCRYPTED is used, we could simply store the password as plain. When only ENCRYPTED is used, we store it for all the methods available, except "plain". ENCRYPTED and plain are not allowed combinations.
- Also, do we really want an option at SQL level to allow storing custom hashes generated on client side as a first step? We could have something like WITH (md5 = 'blah', scram = 'blah2') appended after PASSWORD for example.
- rolpassword is removed from pg_authid.

I am willing to write a patch for the next CF following more or less those lines, depending of course on the outcome of the discussion we can have here, so feel free to comment.

I'll have a look more in-depth at the scram patch as well.

Regards,