Re: User functions for building SCRAM secrets

On 10/31/22 8:56 PM, Michael Paquier wrote:
> On Mon, Oct 31, 2022 at 04:27:08PM -0400, Jonathan S. Katz wrote:
>> 1. password only -- this defers to the PG defaults for SCRAM
>> 2. password + salt -- this is useful for the password history / dictionary
>> case to allow for a predictable way to check a password.
>
> Well, one could pass a salt based on something generated by random()
> to emulate what we currently do in the default case, as well. The
> salt length is an extra possibility, letting it be randomly generated
> by pg_strong_random().

Sure, this is a good point. From a SQL level we can get that from
pgcrypto "gen_random_bytes".

Per this and ilmari's feedback, I updated the 2nd argument to be a
bytea. See the corresponding tests that then show using decode(...,
'base64') to handle this.

When I write the docs, I'll include that in the examples.

>> 1. Location of the functions. I put them in
>> src/backend/utils/adt/cryptohashfuncs.c as I wasn't sure where it would make
>> sense to have them (and they could easily go into their own file).
>
> As of adt/authfuncs.c? cryptohashfuncs.c does not strike me as a good
> fit.

I went with your suggested name.

>> Please let me know if you have any questions. I'll add a CF entry for this.
>
> +{ oid => '8555', descr => 'Build a SCRAM secret',
> + proname => 'scram_build_secret_sha256', proleakproof => 't', prorettype => 'text',
> + proargtypes => 'text', prosrc => 'scram_build_secret_sha256_from_password' },
> +{ oid => '8556', descr => 'Build a SCRAM secret',
> + proname => 'scram_build_secret_sha256', proleakproof => 't',
> + provolatile => 'i', prorettype => 'text',
> + proargtypes => 'text text', prosrc => 'scram_build_secret_sha256_from_password_and_salt' },
> +{ oid => '8557', descr => 'Build a SCRAM secret',
> + proname => 'scram_build_secret_sha256', proleakproof => 't',
> + provolatile => 'i', prorettype => 'text',
> + proargtypes => 'text text int4', prosrc => 'scram_build_secret_sha256_from_password_and_salt_and_iterations' },
>
> Keeping this approach as-is, I don't think that you should consume 3
> OIDs, but 1 (with scram_build_secret_sha256_from_password_and_..
> as prosrc) that has two defaults for the second argument (salt string,
> default as NULL) and third argument (salt, default at 0), with the
> defaults set up in system_functions.sql via a redefinition.

Thanks for the suggestion. I went with this as well.

> Note that you cannot pass down an expression for the password of
> CREATE/ALTER USER, meaning that this would need a \gset at least if
> done by a psql client for example, and passing down a password string
> is not an encouraged practice, either. Another approach is to also
> provide a role OID in input and store the newly-computed password in
> pg_authid (as of [1]), so as one can store it easily.

...unless you dynamically generate the CREATE/ALTER ROLE command ;) (and
yes, lots of discussion on that).

> Did you look at extending \password? Being able to extend
> PQencryptPasswordConn() with custom parameters has been discussed in
> the past, but this has gone nowhere. That's rather unrelated to what
> you are looking for, just mentioning as we are playing with options to
> have control the iteration number and the salt.

Not yet, but happy to do that as a follow-up patch.

Please see version 2. If folks are generally happy with this, I'll
address any additional feedback and write up docs.

Thanks,

Jonathan