Re: User functions for building SCRAM secrets - Mailing list pgsql-hackers
From | Jonathan S. Katz |
---|---|
Subject | Re: User functions for building SCRAM secrets |
Date | |
Msg-id | 76e2a294-496a-fb8b-2a4b-08e5af9df429@postgresql.org Whole thread Raw |
In response to | Re: User functions for building SCRAM secrets (Michael Paquier <michael@paquier.xyz>) |
Responses |
Re: User functions for building SCRAM secrets
|
List | pgsql-hackers |
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
Attachment
pgsql-hackers by date: