Re: User functions for building SCRAM secrets
On Sat, 2 Dec 2023 at 12:22, John Naylor wrote: > > On Wed, Mar 22, 2023 at 9:06 PM Jonathan S. Katz wrote: > > > > Maybe I'm not conveying the problem this is solving -- I'm happy to go > > one more round trying to make it clearer -- but if this is not clear, > > it'd be good to at develop an alternative approach to this before > > withdrawing the patch. > > This thread had some lively discussion, but it doesn't seem to have > converged towards consensus, and hasn't had activity since April. That > being the case, maybe it's time to withdraw and reconsider the > approach later? I have changed the status of this commitfest entry to "Returned with Feedback" as currently nobody pursued the discussion to get a conclusion. Feel free to discuss more on this and once it reaches a better shape, add a new entry for this to take it forward. Regards, Vignesh
Re: User functions for building SCRAM secrets
On 11/29/22 8:12 PM, Michael Paquier wrote: On Tue, Nov 29, 2022 at 09:32:34PM +0100, Daniel Gustafsson wrote: On the whole I tend to agree with Jacob upthread, while this does provide consistency it doesn't seem to move the needle for best practices. Allowing scram_build_secret_sha256('password', 'a', 1); with the password potentially going in cleartext over the wire and into the logs doesn't seem like a great tradeoff for the (IMHO) niche usecases it would satisfy. Should we try to make \password and libpq more flexible instead? Two things got discussed in this area since v10: - The length of the random salt. - The iteration number. Or we could bump up the defaults, and come back to that in a few years, again.. ;p Here is another attempt at this patch that takes into account the SCRAM code refactor. I addressed some of Daniel's previous feedback, but will need to make another pass on the docs and the assert trace as the main focus of this revision was bringing the code inline with the recent changes. This patch changes the function name to "scram_build_secret" and now accepts a new parameter of hash type. This sets it up to handle additional hashes in the future. I do agree we should make libpq more flexible, but as mentioned in the original thread, this does not solve the *server side* cases where a user needs to build a SCRAM secret. For example, being able to precompute hashes on one server before sending them to another server, which can require no plaintext passwords if the server is randomly generating the data. Another use case comes from the "pg_tle" project, specifically with the ability to write a "check_password_hook" from an available PL[1]. If a user does follow our best practices and sends a pre-built SCRAM secret over the wire, a hook can then verify that the secret is not contained within a common password dictionary. Thanks, Jonathan [1] https://github.com/aws/pg_tle/blob/main/docs/04_hooks.md From 756c93f83869b7f8cbb87a7e4ccd967cbd8e8553 Mon Sep 17 00:00:00 2001 From: "Jonathan S. Katz" Date: Mon, 31 Oct 2022 16:13:08 -0400 Subject: [PATCH] Add "scram_build_secret" SQL function This function lets users build SCRAM secrets from SQL functions and provides the ability for the user to select the password, salt, and number of iterations for the password hashing algorithm. Currently this only supports the "sha256" hash, but can be modified to support additional hashes in the future. --- doc/src/sgml/func.sgml | 46 ++ src/backend/catalog/system_functions.sql | 7 ++ src/backend/libpq/auth-scram.c | 31 +-- src/backend/libpq/crypt.c| 2 +- src/backend/utils/adt/Makefile | 1 + src/backend/utils/adt/authfuncs.c| 109 +++ src/backend/utils/adt/meson.build| 1 + src/include/catalog/pg_proc.dat | 4 + src/include/libpq/scram.h| 4 +- src/test/regress/expected/scram.out | 99 src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/scram.sql | 62 + 12 files changed, 356 insertions(+), 12 deletions(-) create mode 100644 src/backend/utils/adt/authfuncs.c create mode 100644 src/test/regress/expected/scram.out create mode 100644 src/test/regress/sql/scram.sql diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index b8dac9ef46..68f11e953a 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -3513,6 +3513,52 @@ repeat('Pg', 4) PgPgPgPg + + + + scram_build_secret + +scram_build_secret ( hash_type text +, password text +[, salt bytea +[, iterations integer ] ]) +text + + +Using the values provided in hash type and +password, builds a SCRAM secret equilvaent to +what is stored in +pg_authid.rolpassword +and used with scram-sha-256 +authentication. If not provided or set to NULL, +salt is randomly generated and +iterations defaults to 4096. +Currently hash type only supports +sha256. + + +SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64')); + + + SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4= + + + +SELECT scram_build_secret('sha256', 'secret password', '\xabba5432'); + + + SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o= + + + +SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 1); + + + SCRAM-SHA-256$1:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXo
Re: User functions for building SCRAM secrets
On Wed, Mar 22, 2023 at 9:06 PM Jonathan S. Katz wrote: > > Maybe I'm not conveying the problem this is solving -- I'm happy to go > one more round trying to make it clearer -- but if this is not clear, > it'd be good to at develop an alternative approach to this before > withdrawing the patch. This thread had some lively discussion, but it doesn't seem to have converged towards consensus, and hasn't had activity since April. That being the case, maybe it's time to withdraw and reconsider the approach later?
Re: User functions for building SCRAM secrets
"Jonathan S. Katz" writes: > Attached is a (draft) patch that adds a function called > "scram_build_secret_sha256" that can take 3 arguments: This seems like a reasonable piece of functionality, I just have one comment on the implementation. > * password (text) - a plaintext password > * salt (text) - a base64 encoded salt […] > + /* > + * determine if this a valid base64 encoded string > + * TODO: look into refactoring the SCRAM decode code in > libpq/auth-scram.c > + */ > + salt_str_dec_len = pg_b64_dec_len(strlen(salt_str_enc)); > + salt_str_dec = palloc(salt_str_dec_len); > + salt_str_dec_len = pg_b64_decode(salt_str_enc, strlen(salt_str_enc), > + salt_str_dec, > salt_str_dec_len); > + if (salt_str_dec_len < 0) > + { > + ereport(ERROR, > + (errcode(ERRCODE_DATA_EXCEPTION), > + errmsg("invalid base64 encoded string"), > + errhint("Use the \"encode\" function to > convert to valid base64 string."))); > + } Instead of going through all these machinations to base64-decode the salt and tell the user off if they encoded it wrong, why not accept the binary salt directly as a bytea? - ilmari
Re: User functions for building SCRAM secrets
On 10/31/22 6:05 PM, Dagfinn Ilmari Mannsåker wrote: * password (text) - a plaintext password * salt (text) - a base64 encoded salt […] + /* +* determine if this a valid base64 encoded string +* TODO: look into refactoring the SCRAM decode code in libpq/auth-scram.c +*/ + salt_str_dec_len = pg_b64_dec_len(strlen(salt_str_enc)); + salt_str_dec = palloc(salt_str_dec_len); + salt_str_dec_len = pg_b64_decode(salt_str_enc, strlen(salt_str_enc), + salt_str_dec, salt_str_dec_len); + if (salt_str_dec_len < 0) + { + ereport(ERROR, + (errcode(ERRCODE_DATA_EXCEPTION), +errmsg("invalid base64 encoded string"), +errhint("Use the \"encode\" function to convert to valid base64 string."))); + } Instead of going through all these machinations to base64-decode the salt and tell the user off if they encoded it wrong, why not accept the binary salt directly as a bytea? If we did that, I think we'd have to offer both. Most users are going to be manipulating the salt as a base64 string, both because of 1/ how we store it within the SCRAM secret and 2/ it's convenient in many languages to work with base64 encoded binary data. So in that case, we'd still have to go through the "base64 machinations". However, I'd be OK with allowing for users to specify a "bytea" salt in addition to a base64 one if that seems reasonable. I would be -1 for swapping the base64 salt for just a "bytea" one as I think that would present usability challenges. Thanks, Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: User functions for building SCRAM secrets
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(). > 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. > 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. 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. 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. [1]: https://github.com/michaelpq/pg_plugins/blob/main/scram_utils/scram_utils.c -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
On Mon, Oct 31, 2022 at 1:27 PM Jonathan S. Katz wrote: > Having a set of SCRAM secret building functions would help in a few areas: I have mixed-to-negative feelings about this. Orthogonality with other methods seems reasonable, except we don't really recommend that people use those other methods today. SCRAM is supposed to be one of the solutions where the server does not know your password at any point and cannot impersonate you to others. If we don't provide an easy client-side equivalent for the new functionality, via \password or some such, then the path of least resistance for some of these intermediate use cases (i.e. higher iteration count) will be "just get used to sending your password in plaintext," and that doesn't really sound all that great. Similar to pgcrypto's current state. > 2. Keeping a history file of user-stored passwords Could you expand on this? How does being able to generate SCRAM secrets help you keep a password history? > or checking against a common-password dictionary. People really want to do this using SQL? Maybe my idea of the use case is way off, but I'm skeptical that this scales (safely and/or performantly) to a production system, *especially* if you have your iteration counts high enough. > 3. Allowing users to build SQL-functions that can precompute SCRAM > secrets on a local server before sending it to a remote server. I guess I have fewer problems with this use case in theory, but I'm wondering if better client-side support might also solve this one as well, without the additional complication. Is there a reason it would not? Thanks, --Jacob
Re: User functions for building SCRAM secrets
On Tue, Nov 1, 2022 at 4:02 PM Jacob Champion wrote: > I guess I have fewer problems with this use case in theory, but I'm > wondering if better client-side support might also solve this one as > well, without the additional complication. Is there a reason it would > not? To expand on this question, after giving it some more thought: It seems to me that the use case here is extremely similar to the one being tackled by Peter E's client-side encryption [1]. People want to write SQL to perform a cryptographic operation using a secret, and then send the resulting ciphertext (or in this case, a one-way hash) to the server, but ideally the server should not actually have the secret. I don't think it's helpful for me to try to block progress on this patchset behind the other one. But is there a way for me to help this proposal skate in the same general direction? Could Peter's encryption framework expand to fit this case in the future? Thanks, --Jacob [1] https://www.postgresql.org/message-id/flat/89157929-c2b6-817b-6025-8e4b2d89d88f%40enterprisedb.com
Re: User functions for building SCRAM secrets
On 04.11.22 21:39, Jacob Champion wrote: It seems to me that the use case here is extremely similar to the one being tackled by Peter E's client-side encryption [1]. People want to write SQL to perform a cryptographic operation using a secret, and then send the resulting ciphertext (or in this case, a one-way hash) to the server, but ideally the server should not actually have the secret. It might be possible, but it's a bit of a reach. For instance, there are no keys and no decryption associated with this kind of operation. I don't think it's helpful for me to try to block progress on this patchset behind the other one. But is there a way for me to help this proposal skate in the same general direction? Could Peter's encryption framework expand to fit this case in the future? We already have support in libpq for doing this (PQencryptPasswordConn()).
Re: User functions for building SCRAM secrets
On 11/8/22 12:26, Peter Eisentraut wrote: > On 04.11.22 21:39, Jacob Champion wrote: >> I don't think it's helpful for me to try to block progress on this >> patchset behind the other one. But is there a way for me to help this >> proposal skate in the same general direction? Could Peter's encryption >> framework expand to fit this case in the future? > > We already have support in libpq for doing this (PQencryptPasswordConn()). Sure, but you can't access that in SQL, right? The hand-wavy part is to combine that existing function with your transparent encryption proposal, as a special-case encryptor whose output could be bound to the query. But I guess that wouldn't really help with ALTER ROLE ... PASSWORD, because you can't parameterize it. Hm... --Jacob
Re: User functions for building SCRAM secrets
On Tue, Nov 08, 2022 at 04:57:09PM -0800, Jacob Champion wrote: > But I guess that wouldn't really help with ALTER ROLE ... PASSWORD, > because you can't parameterize it. Hm... Yeah, and I'd like to think that this is never something we should allow, either, as that could be easily a footgun for users (?). -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
On Tue, Nov 8, 2022 at 9:28 PM Michael Paquier wrote: > On Tue, Nov 08, 2022 at 04:57:09PM -0800, Jacob Champion wrote: > > But I guess that wouldn't really help with ALTER ROLE ... PASSWORD, > > because you can't parameterize it. Hm... > > Yeah, and I'd like to think that this is never something we should > allow, either, as that could be easily a footgun for users (?). What would make it unsafe? I don't know a lot about the tradeoffs for parameterizing queries. --Jacob
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 diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql index 30a048f6b0..4aa76b81d9 100644 --- a/src/backend/catalog/system_functions.sql +++ b/src/backend/catalog/system_functions.sql @@ -594,6 +594,12 @@ LANGUAGE internal STRICT IMMUTABLE PARALLEL SAFE AS 'unicode_is_normalized'; +-- defaults for building a "scram-sha-256" secret +CREATE OR REPLACE FUNCTION + scram_build_secret_sha256(text, bytea DEFAULT NULL, int DEFAULT NULL) +RETURNS text +LANGUAGE INTERNAL +VOLATILE AS 'scram_build_secret_sha256'; -- -- The default permissions for functions mean that anyone can execute them. -- A number of functions shouldn't be executable by just anyone, but rather diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile index 0de0bbb1b8..7ddb186f96 100644 --- a/src/backend/utils/adt/Makefile +++ b/src/backend/utils/adt/Makefile @@ -22,6 +22,7 @@ OBJS = \ arraysubs.o \ arrayutils.o \ ascii.o \ + authfuncs.o \ bool.o \ cash.o \ char.o \ diff --git a/src/backend/utils/adt/authfuncs.c b/src/backend/utils/adt/authfuncs.c new file mode 100644 index 00..4b4bbd7b7f --- /dev/null +++ b/src/backend/utils/adt/authfuncs.c @@ -0,0 +1,120 @@ +/*- + * + * authfuncs.c + * Functions that assist with authentication management + * + * Portions Copyright (c) 2022, PostgreSQL Global Development Group + * + * + * IDENTIFICATION + * src/backend/utils/adt/authfuncs.c + * + *---
Re: User functions for building SCRAM secrets
Hi, On 2023-01-23 00:58:40 -0500, Jonathan S. Katz wrote: > Here is another attempt at this patch that takes into account the SCRAM code > refactor. I addressed some of Daniel's previous feedback, but will need to > make another pass on the docs and the assert trace as the main focus of this > revision was bringing the code inline with the recent changes. This reliably fails on CI: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3988 I think this is related to encoding issues. The 32bit debian task intentionally uses LANG=C. Resulting in failures like: https://api.cirrus-ci.com/v1/artifact/task/6696410851049472/testrun/build-32/testrun/regress/regress/regression.diffs Windows fails with a similar issue: https://api.cirrus-ci.com/v1/artifact/task/5676064060473344/testrun/build/testrun/regress/regress/regression.diffs I've set the patch as waiting on author for now. - Andres
Re: User functions for building SCRAM secrets
On 2/14/23 3:17 PM, Andres Freund wrote: Hi, On 2023-01-23 00:58:40 -0500, Jonathan S. Katz wrote: Here is another attempt at this patch that takes into account the SCRAM code refactor. I addressed some of Daniel's previous feedback, but will need to make another pass on the docs and the assert trace as the main focus of this revision was bringing the code inline with the recent changes. This reliably fails on CI: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3988 I think this is related to encoding issues. The 32bit debian task intentionally uses LANG=C. Resulting in failures like: https://api.cirrus-ci.com/v1/artifact/task/6696410851049472/testrun/build-32/testrun/regress/regress/regression.diffs Windows fails with a similar issue: https://api.cirrus-ci.com/v1/artifact/task/5676064060473344/testrun/build/testrun/regress/regress/regression.diffs I've set the patch as waiting on author for now. Thanks for the explanation. I'll work on fixing that in the next go round. Jonathan OpenPGP_signature Description: OpenPGP digital signature
Re: User functions for building SCRAM secrets
On 2/14/23 3:19 PM, Jonathan S. Katz wrote: On 2/14/23 3:17 PM, Andres Freund wrote: This reliably fails on CI: https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest%2F42%2F3988 I think this is related to encoding issues. The 32bit debian task intentionally uses LANG=C. Resulting in failures like: https://api.cirrus-ci.com/v1/artifact/task/6696410851049472/testrun/build-32/testrun/regress/regress/regression.diffs Windows fails with a similar issue: https://api.cirrus-ci.com/v1/artifact/task/5676064060473344/testrun/build/testrun/regress/regress/regression.diffs Thanks for the explanation. I'll work on fixing that in the next go round. (First -- I really like the current status of running the tests with Meson. I'm finding it very easy to use -- doing the locale testing was pretty easy too!) I stared at this for a bit to see what we do in other regression tests using unicode strings. I looked at the regression tests for strings[1] and ICU collations[2]. In "strings", all the escaped Unicode strings are in the low bits so they're convertible to ASCII. In the ICU test, it does a check to see if we're using UTF-8: if we're not, it bails. For this patch, the value of the failing test is to ensure that the SCRAM function honors SASLprep when building the secret. It makes more sense to use the current character (U+1680), which will be converted to a space by the algorithm, vs. moving to U+0020 or something that does not exercise the SASLprep code. I opted for the approach in [2]. v5 contains the branching logic for the UTF8 only tests, and the corresponding output files. I tested locally on macOS against both UTF8 + C locales. Thanks, Jonathan [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/strings.sql [2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/collate.icu.utf8.sql From 789fb67e5dec1627a1d20427403cee70184a01f6 Mon Sep 17 00:00:00 2001 From: "Jonathan S. Katz" Date: Mon, 31 Oct 2022 16:13:08 -0400 Subject: [PATCH] Add "scram_build_secret" SQL function This function lets users build SCRAM secrets from SQL functions and provides the ability for the user to select the password, salt, and number of iterations for the password hashing algorithm. Currently this only supports the "sha256" hash, but can be modified to support additional hashes in the future. --- doc/src/sgml/func.sgml | 46 ++ src/backend/catalog/system_functions.sql | 7 ++ src/backend/libpq/auth-scram.c | 31 +-- src/backend/libpq/crypt.c| 2 +- src/backend/utils/adt/Makefile | 1 + src/backend/utils/adt/authfuncs.c| 109 +++ src/backend/utils/adt/meson.build| 1 + src/include/catalog/pg_proc.dat | 4 + src/include/libpq/scram.h| 4 +- src/test/regress/expected/scram.out | 104 + src/test/regress/expected/scram_1.out| 93 +++ src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/scram.sql | 68 ++ 13 files changed, 460 insertions(+), 12 deletions(-) create mode 100644 src/backend/utils/adt/authfuncs.c create mode 100644 src/test/regress/expected/scram.out create mode 100644 src/test/regress/expected/scram_1.out create mode 100644 src/test/regress/sql/scram.sql diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index e09e289a43..a305767e30 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -3513,6 +3513,52 @@ repeat('Pg', 4) PgPgPgPg + + + + scram_build_secret + +scram_build_secret ( hash_type text +, password text +[, salt bytea +[, iterations integer ] ]) +text + + +Using the values provided in hash type and +password, builds a SCRAM secret equilvaent to +what is stored in +pg_authid.rolpassword +and used with scram-sha-256 +authentication. If not provided or set to NULL, +salt is randomly generated and +iterations defaults to 4096. +Currently hash type only supports +sha256. + + +SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64')); + + + SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4= + + + +SELECT scram_build_secret('sha256', 'secret password', '\xabba5432'); + + + SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o= + + + +SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 1); + + + SCRAM-SHA-256$1:MTIzNDU2Nzg5MGFiY2RlZg
Re: User functions for building SCRAM secrets
On Thu, Nov 10, 2022 at 11:14:34PM -0500, Jonathan S. Katz wrote: > On 10/31/22 8:56 PM, Michael Paquier wrote: >> 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". Could it be something we could just push into core? FWIW, I've used that quite a bit in the last to cheaply build long random strings of data for other things. Without pgcrypto, random() with generate_series() has always been kind of.. fun. +SELECT scram_build_secret_sha256(NULL); +ERROR: password must not be null +SELECT scram_build_secret_sha256(NULL, NULL); +ERROR: password must not be null +SELECT scram_build_secret_sha256(NULL, NULL, NULL); +ERROR: password must not be null This is just testing three times the same thing as per the defaults. I would cut the second and third cases. git diff --check reports some whitespaces. scram_build_secret_sha256_internal() is missing SASLprep on the password string. Perhaps the best thing to do here is just to extend pg_be_scram_build_secret() with more arguments so as callers can optionally pass down a custom salt with its length, leaving the responsibility to pg_be_scram_build_secret() to create a random salt if nothing has been given? -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
On 11/16/22 10:09 PM, Michael Paquier wrote: On Thu, Nov 10, 2022 at 11:14:34PM -0500, Jonathan S. Katz wrote: On 10/31/22 8:56 PM, Michael Paquier wrote: 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". Could it be something we could just push into core? FWIW, I've used that quite a bit in the last to cheaply build long random strings of data for other things. Without pgcrypto, random() with generate_series() has always been kind of.. fun. I would be a +1 for moving that into core, given we did something similar with gen_random_uuid[1]. Separate patch, of course :) +SELECT scram_build_secret_sha256(NULL); +ERROR: password must not be null +SELECT scram_build_secret_sha256(NULL, NULL); +ERROR: password must not be null +SELECT scram_build_secret_sha256(NULL, NULL, NULL); +ERROR: password must not be null This is just testing three times the same thing as per the defaults. I would cut the second and third cases. AFAICT it's not returning the defaults. Quick other example: CREATE FUNCTION ab (a int DEFAULT 0) RETURNS int AS $$ SELECT a; $$ LANGUAGE SQL; SELECT ab(); ab 0 (1 row) SELECT ab(NULL::int); ab (1 row) Given scram_build_secret_sha256 is not a strict function, I'd prefer to test all of the NULL cases in case anything in the underlying code changes in the future. It's a cheap cost to be a bit more careful. git diff --check reports some whitespaces. Ack. Will fix on the next pass. (I've been transitioning editors, which could have resulted in that), scram_build_secret_sha256_internal() is missing SASLprep on the password string. Perhaps the best thing to do here is just to extend pg_be_scram_build_secret() with more arguments so as callers can optionally pass down a custom salt with its length, leaving the responsibility to pg_be_scram_build_secret() to create a random salt if nothing has been given? Ah, good catch! I think if we go with passing down the salt, we'd also have to allow for the passing down of the iterations, too, and we're close to rebuilding "scram_build_secret". I'll stare a bit at this on the next pass and either 1/ just SASLprep the string in the new "scram_build_secret_sha256_internal" func, or 2/ change the definition of "pg_be_scram_build_secret" to accommodate more overrides. Thanks, Jonathan [1] https://www.postgresql.org/docs/current/functions-uuid.html OpenPGP_signature Description: OpenPGP digital signature
Re: User functions for building SCRAM secrets
On 11/26/22 2:53 PM, Jonathan S. Katz wrote: On 11/16/22 10:09 PM, Michael Paquier wrote: git diff --check reports some whitespaces. Ack. Will fix on the next pass. (I've been transitioning editors, which could have resulted in that), Fixed (and have run that check subsequently). scram_build_secret_sha256_internal() is missing SASLprep on the password string. Perhaps the best thing to do here is just to extend pg_be_scram_build_secret() with more arguments so as callers can optionally pass down a custom salt with its length, leaving the responsibility to pg_be_scram_build_secret() to create a random salt if nothing has been given? Ah, good catch! I think if we go with passing down the salt, we'd also have to allow for the passing down of the iterations, too, and we're close to rebuilding "scram_build_secret". I'll stare a bit at this on the next pass and either 1/ just SASLprep the string in the new "scram_build_secret_sha256_internal" func, or 2/ change the definition of "pg_be_scram_build_secret" to accommodate more overrides. In the end I went with your suggested approach as it limited the amount of code duplication. I did keep in all the permutations of the tests as it did help me catch an error in my code that let to a panic. As this seems to be closer to completion, I did include docs in this patch. I added this function as part of the "string functions" section of the docs as "md5" was already there. If we continue to add more authentication helper functions, perhaps we should consider breaking those out into their own documentation section. Thanks, Jonathan From 2b04b1aca0415b726fdacc7c0cc4903ee864257c Mon Sep 17 00:00:00 2001 From: "Jonathan S. Katz" Date: Mon, 31 Oct 2022 16:13:08 -0400 Subject: [PATCH] Add "scram_build_secret_sha_256" SQL function This function lets users build SCRAM secrets from SQL functions and provides the ability for the user to select the password, salt, and number of iterations for the password hashing algorithm. --- doc/src/sgml/func.sgml | 42 +++ src/backend/catalog/system_functions.sql | 6 ++ src/backend/libpq/auth-scram.c | 29 +--- src/backend/libpq/crypt.c| 2 +- src/backend/utils/adt/Makefile | 1 + src/backend/utils/adt/authfuncs.c| 69 ++ src/backend/utils/adt/meson.build| 1 + src/include/catalog/pg_proc.dat | 4 ++ src/include/libpq/scram.h| 3 +- src/test/regress/expected/scram.out | 91 src/test/regress/parallel_schedule | 2 +- src/test/regress/sql/scram.sql | 56 +++ 12 files changed, 295 insertions(+), 11 deletions(-) create mode 100644 src/backend/utils/adt/authfuncs.c create mode 100644 src/test/regress/expected/scram.out create mode 100644 src/test/regress/sql/scram.sql diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 6e0425cb3d..f582da138f 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -3485,6 +3485,48 @@ repeat('Pg', 4) PgPgPgPg + + + + scram_build_secret_sha256 + +scram_build_secret_sha256 ( password text +[, salt bytea +[, iterations integer ] ]) +text + + +Using the value provided in password, builds a +SCRAM secret equilvaent to what is stored in +pg_authid.rolpassword +and used with scram-sha-256 +authentication. If not provided or set to NULL, +salt is randomly generated and +iterations defaults to 4096. + + +SELECT scram_build_secret_sha256('secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64')); + + + SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4= + + + +SELECT scram_build_secret_sha256('secret password', '\xabba5432'); + + + SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o= + + + +SELECT scram_build_secret_sha256('secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 1); + + + SCRAM-SHA-256$1:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M= + + + + diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql index 30a048f6b0..4aa76b81d9 100644 --- a/src/backend/catalog/system_functions.sql +++ b/src/backend/catalog/system_functions.sql @@ -594,6 +594,12 @@ LANGUAGE internal STRICT IMMUTABLE PARALLEL SAFE AS 'unicode_is_normalized'; +-- defaults for building a "scram-sha-256" secret +CREATE OR REPLACE FUNCTION + scram_build_secret_sha256(text, bytea DEFAULT NULL, int DEFAULT NULL) +RETURNS text +LANGUA
Re: User functions for building SCRAM secrets
> On 27 Nov 2022, at 06:21, Jonathan S. Katz wrote: > On 11/26/22 2:53 PM, Jonathan S. Katz wrote: >> On 11/16/22 10:09 PM, Michael Paquier wrote: > >>> git diff --check reports some whitespaces. >> Ack. Will fix on the next pass. (I've been transitioning editors, which >> could have resulted in that), > > Fixed (and have run that check subsequently). The spaces-before-tabs that git is complaining about are gone but there are still whitespace issues like scram_build_secret_sha256() which has a mix of two-space and tab indentation. I recommend taking it for a spin with pgindent. Sorry for not noticing the thread earlier, below are some review comments and +SCRAM secret equilvaent to what is stored in s/equilvaent/equivalent/ +SELECT scram_build_secret_sha256('secret password', '\xabba5432'); + + + SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o= + Shouldn't the function output be inside ? IIRC the use if with an empty is for multiline output, but I'm not 100% sure there. + if (iterations <= 0) + iterations = SCRAM_DEFAULT_ITERATIONS; According to the RFC, the iteration-count "SHOULD be at least 4096", so we can reduce it, but do we gain anything by allowing users to set it lower? If so, scram_build_secret() is already converting (iterations <= 0) to the default so there is no need to duplicate that logic. Personally I'd prefer if we made 4096 the minimum and only allowed higher regardless of the fate of this patch, but thats for another thread. + Assert(secret != NULL); I don't think there are any paths where this is possible to trigger, did you see any? On the whole I tend to agree with Jacob upthread, while this does provide consistency it doesn't seem to move the needle for best practices. Allowing scram_build_secret_sha256('password', 'a', 1); with the password potentially going in cleartext over the wire and into the logs doesn't seem like a great tradeoff for the (IMHO) niche usecases it would satisfy. -- Daniel Gustafsson https://vmware.com/
Re: User functions for building SCRAM secrets
On Tue, Nov 29, 2022 at 09:32:34PM +0100, Daniel Gustafsson wrote: > On the whole I tend to agree with Jacob upthread, while this does provide > consistency it doesn't seem to move the needle for best practices. Allowing > scram_build_secret_sha256('password', 'a', 1); with the password potentially > going in cleartext over the wire and into the logs doesn't seem like a great > tradeoff for the (IMHO) niche usecases it would satisfy. Should we try to make \password and libpq more flexible instead? Two things got discussed in this area since v10: - The length of the random salt. - The iteration number. Or we could bump up the defaults, and come back to that in a few years, again.. ;p -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
> On 14 Apr 2023, at 05:50, Michael Paquier wrote: > > On Fri, Apr 14, 2023 at 01:27:46AM +0200, Daniel Gustafsson wrote: >> What would be the intended usecase? I don’t have the RFC handy, does >> it say anything about salt length? > > Hmm. I thought it did, but RFC 5802 has only these two paragraphs: > > If the authentication information is stolen from the authentication > database, then an offline dictionary or brute-force attack can be > used to recover the user's password. The use of salt mitigates this > attack somewhat by requiring a separate attack on each password. > Authentication mechanisms that protect against this attack are > available (e.g., the EKE class of mechanisms). RFC 2945 [RFC2945] is > an example of such technology. The WG elected not to use EKE like > mechanisms as a basis for SCRAM. > > If an attacker obtains the authentication information from the > authentication repository and either eavesdrops on one authentication > exchange or impersonates a server, the attacker gains the ability to > impersonate that user to all servers providing SCRAM access using the > same hash function, password, iteration count, and salt. For this > reason, it is important to use randomly generated salt values. The salt needs to be unique, unpredictable and shall not repeat across password generation. The current 16 byte salted with pg_strong_random should provide that and I'm not sure I see a usecase for allowing users to configure that. The iteration count has a direct effect with the security/speed tradeoff but changing the salt can basically only lead to lowering the security while not gaining efficiency, or am I missing something? -- Daniel Gustafsson
Re: User functions for building SCRAM secrets
On Tue, Feb 14, 2023 at 06:16:18PM -0500, Jonathan S. Katz wrote: > I opted for the approach in [2]. v5 contains the branching logic for the > UTF8 only tests, and the corresponding output files. I tested locally on > macOS against both UTF8 + C locales. I was reading this thread again, and pondered on this particular point: https://www.postgresql.org/message-id/caawbhmhjcfc4oaga_7yluhtj6j+rxey+bodrygzndaflgfz...@mail.gmail.com We've had our share of complains over the years that Postgres logs password data in the logs with various DDLs, so I'd tend to agree that this is not a practice we should try to encourage more. The parameterization of the SCRAM verifiers through GUCs (like Daniel's https://commitfest.postgresql.org/42/4201/ for the iteration number) is more promising because it is possible to not have to send the password over the wire with once we let libpq take care of the computation, and the server would not know about that. -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
On 3/22/23 2:48 AM, Michael Paquier wrote: On Tue, Feb 14, 2023 at 06:16:18PM -0500, Jonathan S. Katz wrote: I opted for the approach in [2]. v5 contains the branching logic for the UTF8 only tests, and the corresponding output files. I tested locally on macOS against both UTF8 + C locales. I was reading this thread again, and pondered on this particular point: https://www.postgresql.org/message-id/caawbhmhjcfc4oaga_7yluhtj6j+rxey+bodrygzndaflgfz...@mail.gmail.com We've had our share of complains over the years that Postgres logs password data in the logs with various DDLs, so I'd tend to agree that this is not a practice we should try to encourage more. The parameterization of the SCRAM verifiers through GUCs (like Daniel's https://commitfest.postgresql.org/42/4201/ for the iteration number) is more promising because it is possible to not have to send the password over the wire with once we let libpq take care of the computation, and the server would not know about that I generally agree with not allowing password data to be in logs, but in practice, there are a variety of tools and extensions that obfuscate or remove passwords from PostgreSQL logs. Additionally, this function is not targeted for SQL statements directly, but stored procedures. For example, an extension like "pg_tle" exposes the ability for someone to write a "check_password_hook" directly from PL/pgSQL[1] (and other languages). As we've made it a best practice to pre-hash the password on the client-side, a user who wants to write a check password hook against a SCRAM verifier needs to be able to compare the verifier against some existing set of plaintext criteria, and has to write their own function to do it. I have heard several users who have asked to do this, and the only feedback I can give them is "implement your own SCRAM build secret function." And, if my PostgreSQL server _is_ the client, e.g. it's making a dblink call to another PostgreSQL server, the only way it can modify a password is by sending the plaintext credential over the wire. I don't see how the parameterization work applies here -- would we allow salts to be parameterized? -- and it still would not allow the server to build out a SCRAM secret for these cases. Maybe I'm not conveying the problem this is solving -- I'm happy to go one more round trying to make it clearer -- but if this is not clear, it'd be good to at develop an alternative approach to this before withdrawing the patch. Thanks, Jonathan [1] https://github.com/aws/pg_tle/blob/main/docs/06_plpgsql_examples.md#example-password-check-hook-against-bad-password-dictionary OpenPGP_signature Description: OpenPGP digital signature
Re: User functions for building SCRAM secrets
> On 22 Mar 2023, at 15:06, Jonathan S. Katz wrote: > On 3/22/23 2:48 AM, Michael Paquier wrote: >> I was reading this thread again, and pondered on this particular >> point: >> https://www.postgresql.org/message-id/caawbhmhjcfc4oaga_7yluhtj6j+rxey+bodrygzndaflgfz...@mail.gmail.com >> We've had our share of complains over the years that Postgres logs >> password data in the logs with various DDLs, so I'd tend to agree that >> this is not a practice we should try to encourage more. The >> parameterization of the SCRAM verifiers through GUCs (like Daniel's >> https://commitfest.postgresql.org/42/4201/ for the iteration number) >> is more promising because it is possible to not have to send the >> password over the wire with once we let libpq take care of the >> computation, and the server would not know about that > > I generally agree with not allowing password data to be in logs, but in > practice, there are a variety of tools and extensions that obfuscate or > remove passwords from PostgreSQL logs. Additionally, this function is not > targeted for SQL statements directly, but stored procedures. > > For example, an extension like "pg_tle" exposes the ability for someone to > write a "check_password_hook" directly from PL/pgSQL[1] (and other > languages). As we've made it a best practice to pre-hash the password on the > client-side, a user who wants to write a check password hook against a SCRAM > verifier needs to be able to compare the verifier against some existing set > of plaintext criteria, and has to write their own function to do it. I have > heard several users who have asked to do this, and the only feedback I can > give them is "implement your own SCRAM build secret function." I'm not sure I follow; doesn't this - coupled with this patch - imply passing the plaintext password from client to the server, hashing it with a known salt and comparing this with something in plaintext hashed with the same known salt? If so, that's admittedly not a usecase I am terribly excited about. My preference is to bring password checks to the plaintext password, not bring the plaintext password to the password check. > And, if my PostgreSQL server _is_ the client, e.g. it's making a dblink call > to another PostgreSQL server, the only way it can modify a password is by > sending the plaintext credential over the wire. My experience with dblink setups is too small to have much insight here, but I (perhaps naively) assumed that dblink setups generally involved two servers under the same control making such changes be possible out of band. > Maybe I'm not conveying the problem this is solving -- I'm happy to go one > more round trying to make it clearer -- but if this is not clear, it'd be > good to at develop an alternative approach to this before withdrawing the > patch. If this is mainly targeting extension use, is there a way in which an extension could have all this functionality with no: a) not exposing any of it from the server; b) not having to copy/paste lots into the extension and; c) have a reasonable way to keep up with any changes made in the backend? -- Daniel Gustafsson
Re: User functions for building SCRAM secrets
On Fri, Mar 24, 2023 at 4:48 PM Daniel Gustafsson wrote: > > > On 22 Mar 2023, at 15:06, Jonathan S. Katz wrote: > > On 3/22/23 2:48 AM, Michael Paquier wrote: > > >> I was reading this thread again, and pondered on this particular > >> point: > >> https://www.postgresql.org/message-id/caawbhmhjcfc4oaga_7yluhtj6j+rxey+bodrygzndaflgfz...@mail.gmail.com > >> We've had our share of complains over the years that Postgres logs > >> password data in the logs with various DDLs, so I'd tend to agree that > >> this is not a practice we should try to encourage more. The > >> parameterization of the SCRAM verifiers through GUCs (like Daniel's > >> https://commitfest.postgresql.org/42/4201/ for the iteration number) > >> is more promising because it is possible to not have to send the > >> password over the wire with once we let libpq take care of the > >> computation, and the server would not know about that > > > > I generally agree with not allowing password data to be in logs, but in > > practice, there are a variety of tools and extensions that obfuscate or > > remove passwords from PostgreSQL logs. Additionally, this function is not > > targeted for SQL statements directly, but stored procedures. > > > > For example, an extension like "pg_tle" exposes the ability for someone to > > write a "check_password_hook" directly from PL/pgSQL[1] (and other > > languages). As we've made it a best practice to pre-hash the password on > > the client-side, a user who wants to write a check password hook against a > > SCRAM verifier needs to be able to compare the verifier against some > > existing set of plaintext criteria, and has to write their own function to > > do it. I have heard several users who have asked to do this, and the only > > feedback I can give them is "implement your own SCRAM build secret > > function." > > I'm not sure I follow; doesn't this - coupled with this patch - imply passing > the plaintext password from client to the server, hashing it with a known salt > and comparing this with something in plaintext hashed with the same known > salt? > If so, that's admittedly not a usecase I am terribly excited about. My > preference is to bring password checks to the plaintext password, not bring > the > plaintext password to the password check. Given how much we marketed, for good reasons, SCRAM as a way to finally make postgres *not* do this, it seems like a really strange path to take to go back to doing it again. Having the function always generate a random salt seems more reasonable though, and would perhaps be something that helps in some of the cases? It won't help with the password policy one, as it's too secure for that, but it would help with the postgres-is-the-client one? > > And, if my PostgreSQL server _is_ the client, e.g. it's making a dblink > > call to another PostgreSQL server, the only way it can modify a password is > > by sending the plaintext credential over the wire. > > My experience with dblink setups is too small to have much insight here, but I > (perhaps naively) assumed that dblink setups generally involved two servers > under the same control making such changes be possible out of band. I have seen a few, and certainly more FDW based ones these days. But I'm not sure I've come across one yet where one wants to *change the password* through dblink. Since it's server-to-server, most people would just change the password on the target server and then update the FDW/dblink configuration with the new password. (Of course, the storage of that password on the FDW/dblink layer is a terrible thing in the first place from a security perspective, but it's what we have). > > Maybe I'm not conveying the problem this is solving -- I'm happy to go one > > more round trying to make it clearer -- but if this is not clear, it'd be > > good to at develop an alternative approach to this before withdrawing the > > patch. > > If this is mainly targeting extension use, is there a way in which an > extension > could have all this functionality with no: a) not exposing any of it from the > server; b) not having to copy/paste lots into the extension and; c) have a > reasonable way to keep up with any changes made in the backend? I realize I forgot to write this reply back when the thread was alive, so here's a zombie-awakener! One way to accomplish this would be to create a new predefined role, say pg_change_own_password, by default granted to public. When a user is a member of this role they can, well, change their own password. And it will be done using the full security of scram, without cutting anything. For those that want to enforce a password policy or anything else that requires the server to see the cleartext or cleartext-equivalent of the password can then revoke this role from public, and force password changes to go through a security definer funciton, like SELECT pg_change_password_with_policy('joe', 'mysupersecretpasswrod'). This function can then be under
Re: User functions for building SCRAM secrets
On Tue, Apr 11, 2023 at 11:27:17AM +0200, Magnus Hagander wrote: > Having the function always generate a random salt seems more > reasonable though, and would perhaps be something that helps in some > of the cases? It won't help with the password policy one, as it's too > secure for that, but it would help with the postgres-is-the-client > one? While this is still hot.. Would it make sense to have a scram_salt_length GUC to control the length of the salt used when generating the SCRAM secret? -- Michael signature.asc Description: PGP signature
Re: User functions for building SCRAM secrets
> On 14 Apr 2023, at 01:14, Michael Paquier wrote: > > On Tue, Apr 11, 2023 at 11:27:17AM +0200, Magnus Hagander wrote: >> Having the function always generate a random salt seems more >> reasonable though, and would perhaps be something that helps in some >> of the cases? It won't help with the password policy one, as it's too >> secure for that, but it would help with the postgres-is-the-client >> one? > > While this is still hot.. Would it make sense to have a > scram_salt_length GUC to control the length of the salt used when > generating the SCRAM secret? What would be the intended usecase? I don’t have the RFC handy, does it say anything about salt length? ./daniel
Re: User functions for building SCRAM secrets
On Fri, Apr 14, 2023 at 01:27:46AM +0200, Daniel Gustafsson wrote: > What would be the intended usecase? I don’t have the RFC handy, does > it say anything about salt length? Hmm. I thought it did, but RFC 5802 has only these two paragraphs: If the authentication information is stolen from the authentication database, then an offline dictionary or brute-force attack can be used to recover the user's password. The use of salt mitigates this attack somewhat by requiring a separate attack on each password. Authentication mechanisms that protect against this attack are available (e.g., the EKE class of mechanisms). RFC 2945 [RFC2945] is an example of such technology. The WG elected not to use EKE like mechanisms as a basis for SCRAM. If an attacker obtains the authentication information from the authentication repository and either eavesdrops on one authentication exchange or impersonates a server, the attacker gains the ability to impersonate that user to all servers providing SCRAM access using the same hash function, password, iteration count, and salt. For this reason, it is important to use randomly generated salt values. -- Michael signature.asc Description: PGP signature