User functions for building SCRAM secrets

Started by Jonathan S. Katzabout 3 years ago30 messages
#1Jonathan S. Katz
jkatz@postgresql.org
1 attachment(s)

Hi,

We currently do not provide any SQL functions for generating SCRAM
secrets, whereas we have this support for other passwords types
(plaintext and md5 via `md5(password || username)`). If a user wants to
build a SCRAM secret via SQL, they have to implement our SCRAM hashing
funcs on their own.

Having a set of SCRAM secret building functions would help in a few areas:

1. Ensuring we have a SQL-equivalent of CREATE/ALTER ROLE ... PASSWORD
where we can compute a pre-hashed password.

2. Keeping a history file of user-stored passwords or checking against a
common-password dictionary.

3. Allowing users to build SQL-functions that can precompute SCRAM
secrets on a local server before sending it to a remote server.

Attached is a (draft) patch that adds a function called
"scram_build_secret_sha256" that can take 3 arguments:

* password (text) - a plaintext password
* salt (text) - a base64 encoded salt
* iterations (int) - the number of iterations to hash the plaintext
password.

There are three variations of the function:

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.
3. password + salt + iterations -- this allows the user to modify the
number of iterations to hash a password.

The design of the patch primarily delegates to the existing SCRAM secret
building code and provides a few wrapper functions around it that
evaluate user input.

There are a few open items on this patch, i.e.:

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).

2. I noticed a common set of base64 function calls that could possibly
be refactored into one; I left a TODO comment around that.

3. More tests

4. Docs -- if it seems like we're OK with including these functions,
I'll write these up.

Please let me know if you have any questions. I'll add a CF entry for this.

Thanks,

Jonathan

P.S. I used this as a forcing function to get the meson build system set
up and thus far I quite like it!

Attachments:

scram-funcs-v1.patchtext/plain; charset=UTF-8; name=scram-funcs-v1.patchDownload
diff --git a/src/backend/utils/adt/cryptohashfuncs.c b/src/backend/utils/adt/cryptohashfuncs.c
index 03d84ea217..bdff4d795c 100644
--- a/src/backend/utils/adt/cryptohashfuncs.c
+++ b/src/backend/utils/adt/cryptohashfuncs.c
@@ -13,9 +13,12 @@
  */
 #include "postgres.h"
 
+#include "common/base64.h"
 #include "common/cryptohash.h"
 #include "common/md5.h"
+#include "common/scram-common.h"
 #include "common/sha2.h"
+#include "libpq/scram.h"
 #include "utils/builtins.h"
 
 
@@ -26,6 +29,10 @@
 /* MD5 produces a 16 byte (128 bit) hash; double it for hex */
 #define MD5_HASH_LEN  32
 
+static char *
+scram_build_secret_sha256_internal(const char *password, char *salt_str_enc,
+								int iterations);
+
 /*
  * Create an MD5 hash of a text value and return it as hex string.
  */
@@ -166,3 +173,129 @@ sha512_bytea(PG_FUNCTION_ARGS)
 
 	PG_RETURN_BYTEA_P(result);
 }
+
+/*
+ * Create a SCRAM secret from a password
+ */
+Datum
+scram_build_secret_sha256_from_password(PG_FUNCTION_ARGS)
+{
+	const char	  *password = PG_GETARG_CSTRING(0);
+	char	   *secret;
+
+	secret = scram_build_secret_sha256_internal(password, NULL, 0);
+
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+/*
+ * Create a SCRAM secret from a password and salt.
+ * The salt should be passed in as a base64 encoded string
+ */
+Datum
+scram_build_secret_sha256_from_password_and_salt(PG_FUNCTION_ARGS)
+{
+	const char	  *password = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	char 		*salt_str_enc = text_to_cstring(PG_GETARG_TEXT_PP(1));
+	char	   *secret;
+	/*
+	 * Generate the SCRAM secret, using the default number of iterations for the
+	 * hash.
+	 */
+	secret = scram_build_secret_sha256_internal(password, salt_str_enc, 0);
+
+	Assert(secret != NULL);
+
+	/* convert to text and return it */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+/*
+ * Create a SCRAM secret from a password and salt.
+ * The salt should be passed in as a base64 encoded string
+ */
+Datum
+scram_build_secret_sha256_from_password_and_salt_and_iterations(PG_FUNCTION_ARGS)
+{
+	const char	  *password = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	char 		*salt_str_enc = text_to_cstring(PG_GETARG_TEXT_PP(1));
+	int		iterations = PG_GETARG_INT32(2);
+	char	   *secret;
+	/*
+	 * Generate the SCRAM secret, using the default number of iterations for the
+	 * hash.
+	 */
+	secret = scram_build_secret_sha256_internal(password, salt_str_enc, iterations);
+
+	Assert(secret != NULL);
+
+	/* convert to text and return it */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+/*
+ * Workhorse function to that creates SCRAM secrets from user provided info.
+ * Returns the SCRAM secret in "text" form.
+ *
+ * This function can take three parameters:
+ *
+ * - password: a plaintext password. This argument is required. If none of the
+ *             other arguments is set, the function short circuits to use a
+ *             SCRAM secret generation function that relies on defaults.
+ * - salt_str_enc: a base64 encoded salt. If this is not provided, a salt using
+ *                 the  defaults is generated.
+ * - iterations: the number of iterations to hash the password. If set to 0
+ *               or less, the default number of iterations is used.
+ */
+static char *
+scram_build_secret_sha256_internal(const char *password, char *salt_str_enc,
+								int iterations)
+{
+	char 		*salt_str;
+	char 		*salt_str_dec;
+	char	   *secret;
+	int 		salt_str_dec_len;
+	const char *errstr = NULL;
+
+	Assert(password != NULL);
+	
+	if (salt_str_enc == NULL && iterations <= 0)
+	{
+		return pg_be_scram_build_secret(password);
+	}
+
+	Assert(salt_str_enc != NULL);
+
+	/*
+	 * 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.")));
+	}
+	salt_str = pnstrdup(salt_str_dec, salt_str_dec_len);
+
+	/* if iterations is <= 0, set to the default */
+	if (iterations <= 0)
+		iterations = SCRAM_DEFAULT_ITERATIONS;
+
+	/*
+	 * As this is a backend function, the "errstr" will not be set.
+	 * The current behavior is to elog an ERROR. We will at least assert that we
+	 * don't return a NULL secret.
+	 */
+	secret = scram_build_secret(salt_str, strlen(salt_str), iterations, password,
+							&errstr);
+
+	Assert(secret != NULL);
+	
+	return secret;
+}
\ No newline at end of file
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 20f5aa56ea..9ad0492e6f 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7531,6 +7531,17 @@
 { oid => '3422', descr => 'SHA-512 hash',
   proname => 'sha512', proleakproof => 't', prorettype => 'bytea',
   proargtypes => 'bytea', prosrc => 'sha512_bytea' },
+{ 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' },
 
 # crosstype operations for date vs. timestamp and timestamptz
 { oid => '2338',
diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out
index 330eb0f765..0411c3202c 100644
--- a/src/test/regress/expected/opr_sanity.out
+++ b/src/test/regress/expected/opr_sanity.out
@@ -841,6 +841,9 @@ xid8ge(xid8,xid8)
 xid8eq(xid8,xid8)
 xid8ne(xid8,xid8)
 xid8cmp(xid8,xid8)
+scram_build_secret_sha256(text)
+scram_build_secret_sha256(text,text)
+scram_build_secret_sha256(text,text,integer)
 -- restore normal output mode
 \a\t
 -- List of functions used by libpq's fe-lobj.c
diff --git a/src/test/regress/expected/scram.out b/src/test/regress/expected/scram.out
new file mode 100644
index 0000000000..2f318b9620
--- /dev/null
+++ b/src/test/regress/expected/scram.out
@@ -0,0 +1,40 @@
+-- Test building SCRAM functions
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password', 'MTIzNDU2Nzg5MGFiY2RlZg==');
+                                                       scram_build_secret_sha256                                                       
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password', 'abc');
+ERROR:  invalid base64 encoded string
+HINT:  Use the "encode" function to convert to valid base64 string.
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password', 'MTIzNDU2Nzg5MGFiY2RlZg==', 10000);
+                                                       scram_build_secret_sha256                                                        
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value
+SELECT scram_build_secret_sha256('secret password', NULL::text, 10000);
+ scram_build_secret_sha256 
+---------------------------
+ 
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 9a139f1e24..a02c9c0322 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -33,7 +33,7 @@ test: strings md5 numerology point lseg line box path polygon circle date time t
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc scram
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/sql/scram.sql b/src/test/regress/sql/scram.sql
new file mode 100644
index 0000000000..0d140239d8
--- /dev/null
+++ b/src/test/regress/sql/scram.sql
@@ -0,0 +1,23 @@
+-- Test building SCRAM functions
+
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password', 'MTIzNDU2Nzg5MGFiY2RlZg==');
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password', 'abc');
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password', 'MTIzNDU2Nzg5MGFiY2RlZg==', 10000);
+
+-- test what happens when the salt is a NULL value
+SELECT scram_build_secret_sha256('secret password', NULL::text, 10000);
In reply to: Jonathan S. Katz (#1)
Re: User functions for building SCRAM secrets

"Jonathan S. Katz" <jkatz@postgresql.org> 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

#3Jonathan S. Katz
jkatz@postgresql.org
In reply to: Dagfinn Ilmari Mannsåker (#2)
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

#4Michael Paquier
michael@paquier.xyz
In reply to: Jonathan S. Katz (#1)
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]https://github.com/michaelpq/pg_plugins/blob/main/scram_utils/scram_utils.c -- Michael), 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
--
Michael

#5Jacob Champion
jchampion@timescale.com
In reply to: Jonathan S. Katz (#1)
Re: User functions for building SCRAM secrets

On Mon, Oct 31, 2022 at 1:27 PM Jonathan S. Katz <jkatz@postgresql.org> 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

#6Jacob Champion
jchampion@timescale.com
In reply to: Jacob Champion (#5)
Re: User functions for building SCRAM secrets

On Tue, Nov 1, 2022 at 4:02 PM Jacob Champion <jchampion@timescale.com> 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]/messages/by-id/89157929-c2b6-817b-6025-8e4b2d89d88f@enterprisedb.com. 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]: /messages/by-id/89157929-c2b6-817b-6025-8e4b2d89d88f@enterprisedb.com

#7Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Jacob Champion (#6)
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()).

#8Jacob Champion
jchampion@timescale.com
In reply to: Peter Eisentraut (#7)
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

#9Michael Paquier
michael@paquier.xyz
In reply to: Jacob Champion (#8)
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

#10Jacob Champion
jchampion@timescale.com
In reply to: Michael Paquier (#9)
Re: User functions for building SCRAM secrets

On Tue, Nov 8, 2022 at 9:28 PM Michael Paquier <michael@paquier.xyz> 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

#11Jonathan S. Katz
jkatz@postgresql.org
In reply to: Michael Paquier (#4)
1 attachment(s)
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

Attachments:

scram-funcs-v2.patchtext/plain; charset=UTF-8; name=scram-funcs-v2.patchDownload
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 0000000000..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
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "common/scram-common.h"
+#include "libpq/scram.h"
+#include "utils/builtins.h"
+
+static char *
+scram_build_secret_sha256_internal(const char *password, char *salt_str_enc,
+								int iterations);
+
+
+/*
+ * Build a SCRAM secret that can be used for SCRAM-SHA-256 authentication.
+ */
+Datum
+scram_build_secret_sha256(PG_FUNCTION_ARGS)
+{
+	const char	  *password;
+  char 		*salt_str = NULL;
+	int		iterations = 0;
+	char	   *secret;
+
+	if (PG_ARGISNULL(0))
+	{
+		ereport(ERROR,
+			(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+			 errmsg("password must not be null")));
+  }
+
+  password = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	if (!PG_ARGISNULL(1))
+		salt_str = text_to_cstring((text *) PG_GETARG_BYTEA_PP(1));
+
+	if (!PG_ARGISNULL(2))
+		iterations = PG_GETARG_INT32(2);
+
+	secret = scram_build_secret_sha256_internal(password, salt_str, iterations);
+
+	Assert(secret != NULL);
+
+	/*
+   * convert the SCRAM secret to text which matches the type for
+   * pg_authid.rolpassword
+   */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+
+/*
+ * Workhorse function to that creates SCRAM secrets from user provided info.
+ * Returns the SCRAM secret in "text" form.
+ *
+ * This function can take three parameters:
+ *
+ * - password: a plaintext password. This argument is required. If none of the
+ *             other arguments is set, the function short circuits to use a
+ *             SCRAM secret generation function that relies on defaults.
+ * - salt_str_enc: a base64 encoded salt. If this is not provided, a salt using
+ *                 the  defaults is generated.
+ * - iterations: the number of iterations to hash the password. If set to 0
+ *               or less, the default number of iterations is used.
+ */
+static char *
+scram_build_secret_sha256_internal(const char *password, char *salt_str,
+								int iterations)
+{
+	char		salt_default[SCRAM_DEFAULT_SALT_LEN];
+	char	   *secret;
+	const char *errstr = NULL;
+
+	Assert(password != NULL);
+
+	if (salt_str == NULL && iterations <= 0)
+	{
+		return pg_be_scram_build_secret(password);
+	}
+
+  /*
+   * If the salt is still null, generate a random salt based on the defaults.
+   * Otherwise, work to base64 decode the salt.
+   */
+	if (salt_str == NULL)
+	{
+		if (!pg_strong_random(salt_default, SCRAM_DEFAULT_SALT_LEN))
+			ereport(ERROR,
+					(errcode(ERRCODE_INTERNAL_ERROR),
+					 errmsg("could not generate random salt")));  
+		salt_str = pnstrdup(salt_default, SCRAM_DEFAULT_SALT_LEN);
+	}
+
+	/* if iterations is <= 0, set to the default */
+	if (iterations <= 0)
+		iterations = SCRAM_DEFAULT_ITERATIONS;
+
+	/*
+	 * As this is a backend function, the "errstr" will not be set.
+	 * The current behavior is to elog an ERROR. We will at least assert that we
+	 * don't return a NULL secret.
+	 */
+	secret = scram_build_secret(salt_str, strlen(salt_str), iterations, password,
+							&errstr);
+
+	Assert(secret != NULL);
+
+	return secret;
+}
diff --git a/src/backend/utils/adt/meson.build b/src/backend/utils/adt/meson.build
index ed9ceadfef..910f141e83 100644
--- a/src/backend/utils/adt/meson.build
+++ b/src/backend/utils/adt/meson.build
@@ -9,6 +9,7 @@ backend_sources += files(
   'arraysubs.c',
   'arrayutils.c',
   'ascii.c',
+  'authfuncs.c',
   'bool.c',
   'cash.c',
   'char.c',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 20f5aa56ea..04b662c348 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7531,6 +7531,10 @@
 { oid => '3422', descr => 'SHA-512 hash',
   proname => 'sha512', proleakproof => 't', prorettype => 'bytea',
   proargtypes => 'bytea', prosrc => 'sha512_bytea' },
+{ oid => '8557', descr => 'Build a SCRAM secret',
+  proname => 'scram_build_secret_sha256', prorettype => 'text',
+  proisstrict => 'f', proargtypes => 'text bytea int4',
+  prosrc => 'scram_build_secret_sha256' },
 
 # crosstype operations for date vs. timestamp and timestamptz
 { oid => '2338',
diff --git a/src/test/regress/expected/scram.out b/src/test/regress/expected/scram.out
new file mode 100644
index 0000000000..4783310ca9
--- /dev/null
+++ b/src/test/regress/expected/scram.out
@@ -0,0 +1,81 @@
+-- Test building SCRAM functions
+-- test all nulls
+-- fail
+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
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+                                                       scram_build_secret_sha256                                                       
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password',
+  decode('abc', 'base64'));
+ERROR:  invalid base64 end sequence
+HINT:  Input data is missing padding, is truncated, or is otherwise corrupted.
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret_sha256('secret password', 'abc');
+                                             scram_build_secret_sha256                                             
+-------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:YWJj$L27WlKwqjMDY5ZNsyaxGSMii2mhmoUB7xONbxjykmw4=:u1ofGUXUqTbMwfiH+ANWDCpwEjk3j1Xrocy3va/jaCU=
+(1 row)
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret_sha256('secret password', '\xabba5432');
+                                               scram_build_secret_sha256                                               
+-----------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+(1 row)
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+                                                       scram_build_secret_sha256                                                        
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                    scram_secret                    
+----------------------------------------------------
+ SCRAM-SHA-256$10000:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+ has_default_iterations 
+------------------------
+ t
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 9a139f1e24..a02c9c0322 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -33,7 +33,7 @@ test: strings md5 numerology point lseg line box path polygon circle date time t
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc scram
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/sql/scram.sql b/src/test/regress/sql/scram.sql
new file mode 100644
index 0000000000..53f87530f8
--- /dev/null
+++ b/src/test/regress/sql/scram.sql
@@ -0,0 +1,50 @@
+-- Test building SCRAM functions
+
+-- test all nulls
+-- fail
+SELECT scram_build_secret_sha256(NULL);
+SELECT scram_build_secret_sha256(NULL, NULL);
+SELECT scram_build_secret_sha256(NULL, NULL, NULL);
+
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password',
+  decode('abc', 'base64'));
+
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret_sha256('secret password', 'abc');
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret_sha256('secret password', '\xabba5432');
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
#12Michael Paquier
michael@paquier.xyz
In reply to: Jonathan S. Katz (#11)
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

#13Jonathan S. Katz
jkatz@postgresql.org
In reply to: Michael Paquier (#12)
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]https://www.postgresql.org/docs/current/functions-uuid.html. 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

#14Jonathan S. Katz
jkatz@postgresql.org
In reply to: Jonathan S. Katz (#13)
1 attachment(s)
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

Attachments:

scram-funcs-v3.patchtext/plain; charset=UTF-8; name=scram-funcs-v3.patchDownload
From 2b04b1aca0415b726fdacc7c0cc4903ee864257c Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
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) <returnvalue>PgPgPgPg</returnvalue>
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>scram_build_secret_sha256</primary>
+        </indexterm>
+        <function>scram_build_secret_sha256</function> ( <parameter>password</parameter> <type>text</type>
+        [, <parameter>salt</parameter> <type>bytea</type>
+        [, <parameter>iterations</parameter> <type>integer</type> ] ])
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Using the value provided in <parameter>password</parameter>, builds a
+        SCRAM secret equilvaent to what is stored in
+        <link linkend="catalog-pg-authid"><structname>pg_authid</structname></link>.<structfield>rolpassword</structfield>
+        and used with <link linkend="sasl-scram-sha-256"><literal>scram-sha-256</literal></link>
+        authentication. If not provided or set to <literal>NULL</literal>,
+        <parameter>salt</parameter> is randomly generated and
+        <parameter>iterations</parameter> defaults to <literal>4096</literal>.
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret_sha256('secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret_sha256('secret password', '\xabba5432');</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret_sha256('secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+</programlisting>
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
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/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index ee7f52218a..8d778f4346 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -456,10 +456,15 @@ scram_exchange(void *opaq, const char *input, int inputlen,
 /*
  * Construct a SCRAM secret, for storing in pg_authid.rolpassword.
  *
+ * "salt_str" can be NULL. If it is, this function will generate a random salt.
+ *
+ * If "iterations" is 0 or less, this function will set it to the default value.
+ *
  * The result is palloc'd, so caller is responsible for freeing it.
  */
 char *
-pg_be_scram_build_secret(const char *password)
+pg_be_scram_build_secret(const char *password, char *salt_str, int salt_str_len,
+				   int iterations)
 {
 	char	   *prep_password;
 	pg_saslprep_rc rc;
@@ -476,14 +481,22 @@ pg_be_scram_build_secret(const char *password)
 	if (rc == SASLPREP_SUCCESS)
 		password = (const char *) prep_password;
 
-	/* Generate random salt */
-	if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not generate random salt")));
+	/* If salt_str is NULL, generate random salt */
+	if (salt_str == NULL)
+	{
+		if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
+			ereport(ERROR,
+					(errcode(ERRCODE_INTERNAL_ERROR),
+					 errmsg("could not generate random salt")));
+
+		salt_str = saltbuf;
+		salt_str_len = SCRAM_DEFAULT_SALT_LEN;
+	}
+
+	if (iterations <= 0)
+		iterations = SCRAM_DEFAULT_ITERATIONS;
 
-	result = scram_build_secret(saltbuf, SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+	result = scram_build_secret(salt_str, salt_str_len, iterations, password,
 								&errstr);
 
 	if (prep_password)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index 1ff8b0507d..649c51f990 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -138,7 +138,7 @@ encrypt_password(PasswordType target_type, const char *role,
 			return encrypted_password;
 
 		case PASSWORD_TYPE_SCRAM_SHA_256:
-			return pg_be_scram_build_secret(password);
+			return pg_be_scram_build_secret(password, NULL, -1, 0);
 
 		case PASSWORD_TYPE_PLAINTEXT:
 			elog(ERROR, "cannot encrypt password with 'plaintext'");
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 0000000000..a08489248e
--- /dev/null
+++ b/src/backend/utils/adt/authfuncs.c
@@ -0,0 +1,69 @@
+/*-------------------------------------------------------------------------
+ *
+ * authfuncs.c
+ *	  Functions that assist with authentication management
+ *
+ * Portions Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/adt/authfuncs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "libpq/scram.h"
+#include "utils/builtins.h"
+
+/*
+ * Build a SCRAM secret that can be used for SCRAM-SHA-256 authentication.
+ *
+ * This function can take three arguments:
+ *
+ * - password: a plaintext password. This argument is required. If none of the
+ *             other arguments is set, the function short circuits to use a
+ *             SCRAM secret generation function that relies on defaults.
+ * - salt_str_enc: a base64 encoded salt. If this is not provided, a salt using
+ *                 the defaults is generated.
+ * - iterations: the number of iterations to hash the password. If set to 0
+ *               or less, the default number of iterations is used.
+ */
+Datum
+scram_build_secret_sha256(PG_FUNCTION_ARGS)
+{
+	const char	  *password;
+  char 		*salt_str = NULL;
+  int	     salt_str_len = -1;
+	int		iterations = 0;
+	char	   *secret;
+
+	if (PG_ARGISNULL(0))
+	{
+		ereport(ERROR,
+			(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+			 errmsg("password must not be null")));
+  }
+
+  password = text_to_cstring(PG_GETARG_TEXT_PP(0));
+
+	if (!PG_ARGISNULL(1))
+	{
+		salt_str = text_to_cstring((text *) PG_GETARG_BYTEA_PP(1));
+		salt_str_len = strlen(salt_str);
+	}
+
+	if (!PG_ARGISNULL(2))
+		iterations = PG_GETARG_INT32(2);
+
+	secret = pg_be_scram_build_secret(password, salt_str, salt_str_len,
+		iterations);
+
+	Assert(secret != NULL);
+
+	/*
+   * convert the SCRAM secret to text which matches the type for
+   * pg_authid.rolpassword
+   */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
diff --git a/src/backend/utils/adt/meson.build b/src/backend/utils/adt/meson.build
index ed9ceadfef..910f141e83 100644
--- a/src/backend/utils/adt/meson.build
+++ b/src/backend/utils/adt/meson.build
@@ -9,6 +9,7 @@ backend_sources += files(
   'arraysubs.c',
   'arrayutils.c',
   'ascii.c',
+  'authfuncs.c',
   'bool.c',
   'cash.c',
   'char.c',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 20f5aa56ea..04b662c348 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7531,6 +7531,10 @@
 { oid => '3422', descr => 'SHA-512 hash',
   proname => 'sha512', proleakproof => 't', prorettype => 'bytea',
   proargtypes => 'bytea', prosrc => 'sha512_bytea' },
+{ oid => '8557', descr => 'Build a SCRAM secret',
+  proname => 'scram_build_secret_sha256', prorettype => 'text',
+  proisstrict => 'f', proargtypes => 'text bytea int4',
+  prosrc => 'scram_build_secret_sha256' },
 
 # crosstype operations for date vs. timestamp and timestamptz
 { oid => '2338',
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index c51e848c24..ba0e5f624f 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -21,7 +21,8 @@
 extern PGDLLIMPORT const pg_be_sasl_mech pg_be_scram_mech;
 
 /* Routines to handle and check SCRAM-SHA-256 secret */
-extern char *pg_be_scram_build_secret(const char *password);
+extern char *pg_be_scram_build_secret(const char *password, char *salt_str,
+										int salt_str_len, int iterations);
 extern bool parse_scram_secret(const char *secret, int *iterations, char **salt,
 							   uint8 *stored_key, uint8 *server_key);
 extern bool scram_verify_plain_password(const char *username,
diff --git a/src/test/regress/expected/scram.out b/src/test/regress/expected/scram.out
new file mode 100644
index 0000000000..6ff14d9979
--- /dev/null
+++ b/src/test/regress/expected/scram.out
@@ -0,0 +1,91 @@
+-- Test building SCRAM functions
+-- test all nulls
+-- fail
+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
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+                                                       scram_build_secret_sha256                                                       
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password',
+  decode('abc', 'base64'));
+ERROR:  invalid base64 end sequence
+HINT:  Input data is missing padding, is truncated, or is otherwise corrupted.
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret_sha256('secret password', 'abc');
+                                             scram_build_secret_sha256                                             
+-------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:YWJj$L27WlKwqjMDY5ZNsyaxGSMii2mhmoUB7xONbxjykmw4=:u1ofGUXUqTbMwfiH+ANWDCpwEjk3j1Xrocy3va/jaCU=
+(1 row)
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret_sha256('secret password', '\xabba5432');
+                                               scram_build_secret_sha256                                               
+-----------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+(1 row)
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+                                                       scram_build_secret_sha256                                                        
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                    scram_secret                    
+----------------------------------------------------
+ SCRAM-SHA-256$10000:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+ has_default_iterations 
+------------------------
+ t
+(1 row)
+
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret_sha256(U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
+ ?column? 
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 9a139f1e24..a02c9c0322 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -33,7 +33,7 @@ test: strings md5 numerology point lseg line box path polygon circle date time t
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc scram
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/sql/scram.sql b/src/test/regress/sql/scram.sql
new file mode 100644
index 0000000000..fc89d782a5
--- /dev/null
+++ b/src/test/regress/sql/scram.sql
@@ -0,0 +1,56 @@
+-- Test building SCRAM functions
+
+-- test all nulls
+-- fail
+SELECT scram_build_secret_sha256(NULL);
+SELECT scram_build_secret_sha256(NULL, NULL);
+SELECT scram_build_secret_sha256(NULL, NULL, NULL);
+
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret_sha256('secret password',
+  decode('abc', 'base64'));
+
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret_sha256('secret password', 'abc');
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret_sha256('secret password', '\xabba5432');
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret_sha256('secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret_sha256('secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret_sha256('secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret_sha256(U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
-- 
2.32.1 (Apple Git-133)

#15Daniel Gustafsson
daniel@yesql.se
In reply to: Jonathan S. Katz (#14)
Re: User functions for building SCRAM secrets

On 27 Nov 2022, at 06:21, Jonathan S. Katz <jkatz@postgresql.org> 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/

+        <literal>SELECT scram_build_secret_sha256('secret password', '\xabba5432');</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+</programlisting>
Shouldn't the function output be inside <returnvalue></returnvalue>?  IIRC the
use if <programlisting> with an empty <returnvalue> 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/

#16Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#15)
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

#17Jonathan S. Katz
jkatz@postgresql.org
In reply to: Michael Paquier (#16)
1 attachment(s)
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]https://github.com/aws/pg_tle/blob/main/docs/04_hooks.md. 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

Attachments:

scram-funcs-v4.patchtext/plain; charset=UTF-8; name=scram-funcs-v4.patchDownload
From 756c93f83869b7f8cbb87a7e4ccd967cbd8e8553 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
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) <returnvalue>PgPgPgPg</returnvalue>
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>scram_build_secret</primary>
+        </indexterm>
+        <function>scram_build_secret</function> ( <parameter>hash_type</parameter> <type>text</type>
+        , <parameter>password</parameter> <type>text</type>
+        [, <parameter>salt</parameter> <type>bytea</type>
+        [, <parameter>iterations</parameter> <type>integer</type> ] ])
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Using the values provided in <parameter>hash type</parameter> and
+        <parameter>password</parameter>, builds a SCRAM secret equilvaent to
+        what is stored in
+        <link linkend="catalog-pg-authid"><structname>pg_authid</structname></link>.<structfield>rolpassword</structfield>
+        and used with <link linkend="sasl-scram-sha-256"><literal>scram-sha-256</literal></link>
+        authentication. If not provided or set to <literal>NULL</literal>,
+        <parameter>salt</parameter> is randomly generated and
+        <parameter>iterations</parameter> defaults to <literal>4096</literal>.
+        Currently <parameter>hash type</parameter> only supports
+        <literal>sha256</literal>.
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+</programlisting>
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 83ca893444..5a7444df84 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -627,6 +627,13 @@ CREATE OR REPLACE FUNCTION
  STABLE PARALLEL SAFE
  AS 'sql_localtimestamp';
 
+-- defaults for building a SCRAM secret
+CREATE OR REPLACE FUNCTION
+  scram_build_secret(text, text, bytea DEFAULT NULL, int DEFAULT NULL)
+RETURNS text
+LANGUAGE INTERNAL
+VOLATILE AS 'scram_build_secret_str';
+
 --
 -- 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/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 4441e0d774..44efffd044 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -468,10 +468,15 @@ scram_exchange(void *opaq, const char *input, int inputlen,
 /*
  * Construct a SCRAM secret, for storing in pg_authid.rolpassword.
  *
+ * "salt_str" can be NULL. If it is, this function will generate a random salt.
+ *
+ * If "iterations" is 0 or less, this function will set it to the default value.
+ *
  * The result is palloc'd, so caller is responsible for freeing it.
  */
 char *
-pg_be_scram_build_secret(const char *password)
+pg_be_scram_build_secret(const char *password, char *salt_str, int salt_str_len,
+						 int iterations, pg_cryptohash_type hash_type)
 {
 	char	   *prep_password;
 	pg_saslprep_rc rc;
@@ -488,15 +493,23 @@ pg_be_scram_build_secret(const char *password)
 	if (rc == SASLPREP_SUCCESS)
 		password = (const char *) prep_password;
 
-	/* Generate random salt */
-	if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not generate random salt")));
+	/* If salt_str is NULL, generate random salt */
+	if (salt_str == NULL)
+	{
+		if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
+			ereport(ERROR,
+					(errcode(ERRCODE_INTERNAL_ERROR),
+					 errmsg("could not generate random salt")));
+		salt_str = saltbuf;
+		salt_str_len = SCRAM_DEFAULT_SALT_LEN;
+	}
+
+	if (iterations <= 0)
+		iterations = SCRAM_DEFAULT_ITERATIONS;
 
-	result = scram_build_secret(PG_SHA256, SCRAM_SHA_256_KEY_LEN,
-								saltbuf, SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+	result = scram_build_secret(hash_type, SCRAM_SHA_256_KEY_LEN,
+								salt_str, salt_str_len,
+								iterations, password,
 								&errstr);
 
 	if (prep_password)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index ef496a0bea..edf94444b6 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -140,7 +140,7 @@ encrypt_password(PasswordType target_type, const char *role,
 			return encrypted_password;
 
 		case PASSWORD_TYPE_SCRAM_SHA_256:
-			return pg_be_scram_build_secret(password);
+			return pg_be_scram_build_secret(password, NULL, -1, 0, PG_SHA256);
 
 		case PASSWORD_TYPE_PLAINTEXT:
 			elog(ERROR, "cannot encrypt password with 'plaintext'");
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 0000000000..43631abbeb
--- /dev/null
+++ b/src/backend/utils/adt/authfuncs.c
@@ -0,0 +1,109 @@
+/*-------------------------------------------------------------------------
+ *
+ * authfuncs.c
+ *	  Functions that assist with authentication management
+ *
+ * Portions Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/adt/authfuncs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "libpq/scram.h"
+#include "utils/builtins.h"
+
+#define SCRAM_BUILD_SECRET_HASH_STR_LEN	6
+
+void		parse_cryptohash_type(const char *hash_type_str,
+								  pg_cryptohash_type *hash_type);
+
+/*
+ * Build a SCRAM secret that can be used for SCRAM-SHA-256 authentication.
+ *
+ * This function can take four arguments:
+ *
+ * - hash_type_str: the type of hash to use when building the SCRAM secret.
+ *              Currently only "sha256" is supported.
+ * - password: a plaintext password. This argument is required. If none of the
+ *             other arguments is set, the function short circuits to use a
+ *             SCRAM secret generation function that relies on defaults.
+ * - salt_str_enc: a base64 encoded salt. If this is not provided, a salt using
+ *                 the defaults is generated.
+ * - iterations: the number of iterations to hash the password. If set to 0
+ *               or less, the default number of iterations is used.
+ */
+Datum
+scram_build_secret_str(PG_FUNCTION_ARGS)
+{
+	const char *hash_type_str;
+	pg_cryptohash_type hash_type;
+	const char *password;
+	char	   *salt_str = NULL;
+	int			salt_str_len = -1;
+	int			iterations = 0;
+	char	   *secret;
+
+	if (PG_ARGISNULL(0))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				 errmsg("hash type must not be null")));
+	}
+
+	hash_type_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	parse_cryptohash_type(hash_type_str, &hash_type);
+
+	if (PG_ARGISNULL(1))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				 errmsg("password must not be null")));
+	}
+
+	password = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	if (!PG_ARGISNULL(2))
+	{
+		salt_str = text_to_cstring((text *) PG_GETARG_BYTEA_PP(2));
+		salt_str_len = strlen(salt_str);
+	}
+
+	if (!PG_ARGISNULL(3))
+		iterations = PG_GETARG_INT32(3);
+
+	secret = pg_be_scram_build_secret(password, salt_str, salt_str_len,
+									  iterations, hash_type);
+
+	Assert(secret != NULL);
+
+	/*
+	 * convert the SCRAM secret to text which matches the type for
+	 * pg_authid.rolpassword
+	 */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+/*
+ * If "hash_type_str" is a valid cryptohash type that can be used for SCRAM
+ * authetnication, set the value to "hash_type". Otherwise, return an
+ * unsupported error.
+ */
+void
+parse_cryptohash_type(const char *hash_type_str, pg_cryptohash_type *hash_type)
+{
+	pg_cryptohash_type result_type;
+
+	if (pg_strncasecmp(hash_type_str, "sha256", SCRAM_BUILD_SECRET_HASH_STR_LEN) == 0)
+		result_type = PG_SHA256;
+	else
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("hash not supported: \"%s\"", hash_type_str),
+				 errmsg("supported hashes are \"sha256\"")));
+
+	*hash_type = result_type;
+}
diff --git a/src/backend/utils/adt/meson.build b/src/backend/utils/adt/meson.build
index 8515cd9365..52363a2f26 100644
--- a/src/backend/utils/adt/meson.build
+++ b/src/backend/utils/adt/meson.build
@@ -11,6 +11,7 @@ backend_sources += files(
   'arraysubs.c',
   'arrayutils.c',
   'ascii.c',
+  'authfuncs.c',
   'bool.c',
   'cash.c',
   'char.c',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 86eb8e8c58..b55eac911c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7583,6 +7583,10 @@
 { oid => '3422', descr => 'SHA-512 hash',
   proname => 'sha512', proleakproof => 't', prorettype => 'bytea',
   proargtypes => 'bytea', prosrc => 'sha512_bytea' },
+{ oid => '8557', descr => 'Build a SCRAM secret',
+  proname => 'scram_build_secret', prorettype => 'text',
+  proisstrict => 'f', proargtypes => 'text text bytea int4',
+  prosrc => 'scram_build_secret_str' },
 
 # crosstype operations for date vs. timestamp and timestamptz
 { oid => '2338',
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index b275e1e87e..400f551778 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -22,7 +22,9 @@
 extern PGDLLIMPORT const pg_be_sasl_mech pg_be_scram_mech;
 
 /* Routines to handle and check SCRAM-SHA-256 secret */
-extern char *pg_be_scram_build_secret(const char *password);
+extern char *pg_be_scram_build_secret(const char *password, char *salt_str,
+									  int salt_str_len, int iterations,
+									  pg_cryptohash_type hash_type);
 extern bool parse_scram_secret(const char *secret,
 							   int *iterations,
 							   pg_cryptohash_type *hash_type,
diff --git a/src/test/regress/expected/scram.out b/src/test/regress/expected/scram.out
new file mode 100644
index 0000000000..0f3c5467b1
--- /dev/null
+++ b/src/test/regress/expected/scram.out
@@ -0,0 +1,99 @@
+-- Test building SCRAM functions
+-- test all nulls
+-- fail
+SELECT scram_build_secret(NULL, NULL);
+ERROR:  hash type must not be null
+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
+-- test unsupported hashes
+-- fail
+SELECT scram_build_secret('sha384', 'password');
+ERROR:  supported hashes are "sha256"
+SELECT scram_build_secret('sha512', 'password');
+ERROR:  supported hashes are "sha256"
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+                                                          scram_build_secret                                                           
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('abc', 'base64'));
+ERROR:  invalid base64 end sequence
+HINT:  Input data is missing padding, is truncated, or is otherwise corrupted.
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret('sha256', 'secret password', 'abc');
+                                                scram_build_secret                                                 
+-------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:YWJj$L27WlKwqjMDY5ZNsyaxGSMii2mhmoUB7xONbxjykmw4=:u1ofGUXUqTbMwfiH+ANWDCpwEjk3j1Xrocy3va/jaCU=
+(1 row)
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');
+                                                  scram_build_secret                                                   
+-----------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+(1 row)
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+                                                           scram_build_secret                                                           
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                    scram_secret                    
+----------------------------------------------------
+ SCRAM-SHA-256$10000:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+ has_default_iterations 
+------------------------
+ t
+(1 row)
+
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret('sha256', U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
+ ?column? 
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index a930dfe48c..b526d610e2 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -33,7 +33,7 @@ test: strings md5 numerology point lseg line box path polygon circle date time t
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc scram
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/sql/scram.sql b/src/test/regress/sql/scram.sql
new file mode 100644
index 0000000000..0ca3291969
--- /dev/null
+++ b/src/test/regress/sql/scram.sql
@@ -0,0 +1,62 @@
+-- Test building SCRAM functions
+
+-- test all nulls
+-- fail
+SELECT scram_build_secret(NULL, NULL);
+SELECT scram_build_secret('sha256', NULL);
+SELECT scram_build_secret('sha256', NULL, NULL);
+SELECT scram_build_secret('sha256', NULL, NULL, NULL);
+
+-- test unsupported hashes
+-- fail
+SELECT scram_build_secret('sha384', 'password');
+SELECT scram_build_secret('sha512', 'password');
+
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('abc', 'base64'));
+
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret('sha256', 'secret password', 'abc');
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret('sha256', U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
-- 
2.37.1 (Apple Git-137.1)

#18Andres Freund
andres@anarazel.de
In reply to: Jonathan S. Katz (#17)
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

#19Jonathan S. Katz
jkatz@postgresql.org
In reply to: Andres Freund (#18)
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

#20Jonathan S. Katz
jkatz@postgresql.org
In reply to: Jonathan S. Katz (#19)
1 attachment(s)
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]https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/strings.sql
and ICU collations[2]https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/collate.icu.utf8.sql.

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]https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/collate.icu.utf8.sql. 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
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
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/test/regress/sql/collate.icu.utf8.sql

Attachments:

scram-funcs-v5.patchtext/plain; charset=UTF-8; name=scram-funcs-v5.patchDownload
From 789fb67e5dec1627a1d20427403cee70184a01f6 Mon Sep 17 00:00:00 2001
From: "Jonathan S. Katz" <jonathan.katz@excoventures.com>
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) <returnvalue>PgPgPgPg</returnvalue>
        </para></entry>
       </row>
 
+      <row>
+       <entry role="func_table_entry"><para role="func_signature">
+        <indexterm>
+         <primary>scram_build_secret</primary>
+        </indexterm>
+        <function>scram_build_secret</function> ( <parameter>hash_type</parameter> <type>text</type>
+        , <parameter>password</parameter> <type>text</type>
+        [, <parameter>salt</parameter> <type>bytea</type>
+        [, <parameter>iterations</parameter> <type>integer</type> ] ])
+        <returnvalue>text</returnvalue>
+       </para>
+       <para>
+        Using the values provided in <parameter>hash type</parameter> and
+        <parameter>password</parameter>, builds a SCRAM secret equilvaent to
+        what is stored in
+        <link linkend="catalog-pg-authid"><structname>pg_authid</structname></link>.<structfield>rolpassword</structfield>
+        and used with <link linkend="sasl-scram-sha-256"><literal>scram-sha-256</literal></link>
+        authentication. If not provided or set to <literal>NULL</literal>,
+        <parameter>salt</parameter> is randomly generated and
+        <parameter>iterations</parameter> defaults to <literal>4096</literal>.
+        Currently <parameter>hash type</parameter> only supports
+        <literal>sha256</literal>.
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+</programlisting>
+       </para>
+       <para>
+        <literal>SELECT scram_build_secret('sha256', 'secret password', decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);</literal>
+        <returnvalue></returnvalue>
+<programlisting>
+  SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+</programlisting>
+       </para></entry>
+      </row>
+
       <row>
        <entry role="func_table_entry"><para role="func_signature">
         <indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index 83ca893444..5a7444df84 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -627,6 +627,13 @@ CREATE OR REPLACE FUNCTION
  STABLE PARALLEL SAFE
  AS 'sql_localtimestamp';
 
+-- defaults for building a SCRAM secret
+CREATE OR REPLACE FUNCTION
+  scram_build_secret(text, text, bytea DEFAULT NULL, int DEFAULT NULL)
+RETURNS text
+LANGUAGE INTERNAL
+VOLATILE AS 'scram_build_secret_str';
+
 --
 -- 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/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 4441e0d774..44efffd044 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -468,10 +468,15 @@ scram_exchange(void *opaq, const char *input, int inputlen,
 /*
  * Construct a SCRAM secret, for storing in pg_authid.rolpassword.
  *
+ * "salt_str" can be NULL. If it is, this function will generate a random salt.
+ *
+ * If "iterations" is 0 or less, this function will set it to the default value.
+ *
  * The result is palloc'd, so caller is responsible for freeing it.
  */
 char *
-pg_be_scram_build_secret(const char *password)
+pg_be_scram_build_secret(const char *password, char *salt_str, int salt_str_len,
+						 int iterations, pg_cryptohash_type hash_type)
 {
 	char	   *prep_password;
 	pg_saslprep_rc rc;
@@ -488,15 +493,23 @@ pg_be_scram_build_secret(const char *password)
 	if (rc == SASLPREP_SUCCESS)
 		password = (const char *) prep_password;
 
-	/* Generate random salt */
-	if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
-		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("could not generate random salt")));
+	/* If salt_str is NULL, generate random salt */
+	if (salt_str == NULL)
+	{
+		if (!pg_strong_random(saltbuf, SCRAM_DEFAULT_SALT_LEN))
+			ereport(ERROR,
+					(errcode(ERRCODE_INTERNAL_ERROR),
+					 errmsg("could not generate random salt")));
+		salt_str = saltbuf;
+		salt_str_len = SCRAM_DEFAULT_SALT_LEN;
+	}
+
+	if (iterations <= 0)
+		iterations = SCRAM_DEFAULT_ITERATIONS;
 
-	result = scram_build_secret(PG_SHA256, SCRAM_SHA_256_KEY_LEN,
-								saltbuf, SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+	result = scram_build_secret(hash_type, SCRAM_SHA_256_KEY_LEN,
+								salt_str, salt_str_len,
+								iterations, password,
 								&errstr);
 
 	if (prep_password)
diff --git a/src/backend/libpq/crypt.c b/src/backend/libpq/crypt.c
index ef496a0bea..edf94444b6 100644
--- a/src/backend/libpq/crypt.c
+++ b/src/backend/libpq/crypt.c
@@ -140,7 +140,7 @@ encrypt_password(PasswordType target_type, const char *role,
 			return encrypted_password;
 
 		case PASSWORD_TYPE_SCRAM_SHA_256:
-			return pg_be_scram_build_secret(password);
+			return pg_be_scram_build_secret(password, NULL, -1, 0, PG_SHA256);
 
 		case PASSWORD_TYPE_PLAINTEXT:
 			elog(ERROR, "cannot encrypt password with 'plaintext'");
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 0000000000..43631abbeb
--- /dev/null
+++ b/src/backend/utils/adt/authfuncs.c
@@ -0,0 +1,109 @@
+/*-------------------------------------------------------------------------
+ *
+ * authfuncs.c
+ *	  Functions that assist with authentication management
+ *
+ * Portions Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/adt/authfuncs.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+#include "libpq/scram.h"
+#include "utils/builtins.h"
+
+#define SCRAM_BUILD_SECRET_HASH_STR_LEN	6
+
+void		parse_cryptohash_type(const char *hash_type_str,
+								  pg_cryptohash_type *hash_type);
+
+/*
+ * Build a SCRAM secret that can be used for SCRAM-SHA-256 authentication.
+ *
+ * This function can take four arguments:
+ *
+ * - hash_type_str: the type of hash to use when building the SCRAM secret.
+ *              Currently only "sha256" is supported.
+ * - password: a plaintext password. This argument is required. If none of the
+ *             other arguments is set, the function short circuits to use a
+ *             SCRAM secret generation function that relies on defaults.
+ * - salt_str_enc: a base64 encoded salt. If this is not provided, a salt using
+ *                 the defaults is generated.
+ * - iterations: the number of iterations to hash the password. If set to 0
+ *               or less, the default number of iterations is used.
+ */
+Datum
+scram_build_secret_str(PG_FUNCTION_ARGS)
+{
+	const char *hash_type_str;
+	pg_cryptohash_type hash_type;
+	const char *password;
+	char	   *salt_str = NULL;
+	int			salt_str_len = -1;
+	int			iterations = 0;
+	char	   *secret;
+
+	if (PG_ARGISNULL(0))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				 errmsg("hash type must not be null")));
+	}
+
+	hash_type_str = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	parse_cryptohash_type(hash_type_str, &hash_type);
+
+	if (PG_ARGISNULL(1))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+				 errmsg("password must not be null")));
+	}
+
+	password = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	if (!PG_ARGISNULL(2))
+	{
+		salt_str = text_to_cstring((text *) PG_GETARG_BYTEA_PP(2));
+		salt_str_len = strlen(salt_str);
+	}
+
+	if (!PG_ARGISNULL(3))
+		iterations = PG_GETARG_INT32(3);
+
+	secret = pg_be_scram_build_secret(password, salt_str, salt_str_len,
+									  iterations, hash_type);
+
+	Assert(secret != NULL);
+
+	/*
+	 * convert the SCRAM secret to text which matches the type for
+	 * pg_authid.rolpassword
+	 */
+	PG_RETURN_TEXT_P(cstring_to_text(secret));
+}
+
+/*
+ * If "hash_type_str" is a valid cryptohash type that can be used for SCRAM
+ * authetnication, set the value to "hash_type". Otherwise, return an
+ * unsupported error.
+ */
+void
+parse_cryptohash_type(const char *hash_type_str, pg_cryptohash_type *hash_type)
+{
+	pg_cryptohash_type result_type;
+
+	if (pg_strncasecmp(hash_type_str, "sha256", SCRAM_BUILD_SECRET_HASH_STR_LEN) == 0)
+		result_type = PG_SHA256;
+	else
+		ereport(ERROR,
+				(errcode(ERRCODE_SYNTAX_ERROR),
+				 errmsg("hash not supported: \"%s\"", hash_type_str),
+				 errmsg("supported hashes are \"sha256\"")));
+
+	*hash_type = result_type;
+}
diff --git a/src/backend/utils/adt/meson.build b/src/backend/utils/adt/meson.build
index 8515cd9365..52363a2f26 100644
--- a/src/backend/utils/adt/meson.build
+++ b/src/backend/utils/adt/meson.build
@@ -11,6 +11,7 @@ backend_sources += files(
   'arraysubs.c',
   'arrayutils.c',
   'ascii.c',
+  'authfuncs.c',
   'bool.c',
   'cash.c',
   'char.c',
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 66b73c3900..0e5484c8d9 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -7619,6 +7619,10 @@
 { oid => '3422', descr => 'SHA-512 hash',
   proname => 'sha512', proleakproof => 't', prorettype => 'bytea',
   proargtypes => 'bytea', prosrc => 'sha512_bytea' },
+{ oid => '8557', descr => 'Build a SCRAM secret',
+  proname => 'scram_build_secret', prorettype => 'text',
+  proisstrict => 'f', proargtypes => 'text text bytea int4',
+  prosrc => 'scram_build_secret_str' },
 
 # crosstype operations for date vs. timestamp and timestamptz
 { oid => '2338',
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index b275e1e87e..400f551778 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -22,7 +22,9 @@
 extern PGDLLIMPORT const pg_be_sasl_mech pg_be_scram_mech;
 
 /* Routines to handle and check SCRAM-SHA-256 secret */
-extern char *pg_be_scram_build_secret(const char *password);
+extern char *pg_be_scram_build_secret(const char *password, char *salt_str,
+									  int salt_str_len, int iterations,
+									  pg_cryptohash_type hash_type);
 extern bool parse_scram_secret(const char *secret,
 							   int *iterations,
 							   pg_cryptohash_type *hash_type,
diff --git a/src/test/regress/expected/scram.out b/src/test/regress/expected/scram.out
new file mode 100644
index 0000000000..77313320b5
--- /dev/null
+++ b/src/test/regress/expected/scram.out
@@ -0,0 +1,104 @@
+-- Test building SCRAM functions
+-- test all nulls
+-- fail
+SELECT scram_build_secret(NULL, NULL);
+ERROR:  hash type must not be null
+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
+-- test unsupported hashes
+-- fail
+SELECT scram_build_secret('sha384', 'password');
+ERROR:  supported hashes are "sha256"
+SELECT scram_build_secret('sha512', 'password');
+ERROR:  supported hashes are "sha256"
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+                                                          scram_build_secret                                                           
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('abc', 'base64'));
+ERROR:  invalid base64 end sequence
+HINT:  Input data is missing padding, is truncated, or is otherwise corrupted.
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret('sha256', 'secret password', 'abc');
+                                                scram_build_secret                                                 
+-------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:YWJj$L27WlKwqjMDY5ZNsyaxGSMii2mhmoUB7xONbxjykmw4=:u1ofGUXUqTbMwfiH+ANWDCpwEjk3j1Xrocy3va/jaCU=
+(1 row)
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');
+                                                  scram_build_secret                                                   
+-----------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+(1 row)
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+                                                           scram_build_secret                                                           
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                    scram_secret                    
+----------------------------------------------------
+ SCRAM-SHA-256$10000:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+ has_default_iterations 
+------------------------
+ t
+(1 row)
+
+-- skip the remaining tests if this not a UTF8 server
+SELECT getdatabaseencoding() <> 'UTF8' AS skip_test \gset
+\if :skip_test
+\quit
+\endif
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret('sha256', U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
+ ?column? 
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/expected/scram_1.out b/src/test/regress/expected/scram_1.out
new file mode 100644
index 0000000000..27cd91c01d
--- /dev/null
+++ b/src/test/regress/expected/scram_1.out
@@ -0,0 +1,93 @@
+-- Test building SCRAM functions
+-- test all nulls
+-- fail
+SELECT scram_build_secret(NULL, NULL);
+ERROR:  hash type must not be null
+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
+-- test unsupported hashes
+-- fail
+SELECT scram_build_secret('sha384', 'password');
+ERROR:  supported hashes are "sha256"
+SELECT scram_build_secret('sha512', 'password');
+ERROR:  supported hashes are "sha256"
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                   scram_secret                    
+---------------------------------------------------
+ SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+                                                          scram_build_secret                                                           
+---------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:MTIzNDU2Nzg5MGFiY2RlZg==$D5BmucT796UQKargx2k3fdqjDYR7cH/L0viKKhGo6kA=:M33+iHFOESP8C3DKLDb2k5QAhkNVWEbp/YUIFd2CxN4=
+(1 row)
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('abc', 'base64'));
+ERROR:  invalid base64 end sequence
+HINT:  Input data is missing padding, is truncated, or is otherwise corrupted.
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret('sha256', 'secret password', 'abc');
+                                                scram_build_secret                                                 
+-------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:YWJj$L27WlKwqjMDY5ZNsyaxGSMii2mhmoUB7xONbxjykmw4=:u1ofGUXUqTbMwfiH+ANWDCpwEjk3j1Xrocy3va/jaCU=
+(1 row)
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');
+                                                  scram_build_secret                                                   
+-----------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$4096:q7pUMg==$05Nb9QHwHkMA0CRcYaEfwtgZ+3kStIefz8fLMjTEtio=:P126h1ycyP938E69yxktEfhoAILbiwL/UMsMk3Efb6o=
+(1 row)
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+                                                           scram_build_secret                                                           
+----------------------------------------------------------------------------------------------------------------------------------------
+ SCRAM-SHA-256$10000:MTIzNDU2Nzg5MGFiY2RlZg==$9NkDu1TFpx3L30zMgHUqjRNSq3GRZRrdWU4TuGOnT3Q=:svuIH9L6HH8loyKWguT64XXoOLCrr4FkVViPd2JVR4M=
+(1 row)
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+                    scram_secret                    
+----------------------------------------------------
+ SCRAM-SHA-256$10000:<salt>$<storedkey>:<serverkey>
+(1 row)
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+ has_default_iterations 
+------------------------
+ t
+(1 row)
+
+-- skip the remaining tests if this not a UTF8 server
+SELECT getdatabaseencoding() <> 'UTF8' AS skip_test \gset
+\if :skip_test
+\quit
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index 15e015b3d6..b360ed07f2 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -28,7 +28,7 @@ test: strings md5 numerology point lseg line box path polygon circle date time t
 # geometry depends on point, lseg, line, box, path, polygon, circle
 # horology depends on date, time, timetz, timestamp, timestamptz, interval
 # ----------
-test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc
+test: geometry horology tstypes regex type_sanity opr_sanity misc_sanity comments expressions unicode xid mvcc scram
 
 # ----------
 # Load huge amounts of data
diff --git a/src/test/regress/sql/scram.sql b/src/test/regress/sql/scram.sql
new file mode 100644
index 0000000000..be6216f6d3
--- /dev/null
+++ b/src/test/regress/sql/scram.sql
@@ -0,0 +1,68 @@
+-- Test building SCRAM functions
+
+-- test all nulls
+-- fail
+SELECT scram_build_secret(NULL, NULL);
+SELECT scram_build_secret('sha256', NULL);
+SELECT scram_build_secret('sha256', NULL, NULL);
+SELECT scram_build_secret('sha256', NULL, NULL, NULL);
+
+-- test unsupported hashes
+-- fail
+SELECT scram_build_secret('sha384', 'password');
+SELECT scram_build_secret('sha512', 'password');
+
+-- generated a SCRAM secret from a plaintext password
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password'),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test building a SCRAM secret with a predefined salt with a valid base64
+-- encoded string
+SELECT scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'));
+
+-- test building a SCRAM secret with a predefined salt that is not a valid
+-- base64 string
+-- fail
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('abc', 'base64'));
+
+-- test building a SCRAM secret with a salt that looks like a string but is
+-- cast to a bytea
+SELECT scram_build_secret('sha256', 'secret password', 'abc');
+
+-- test building a SCRAM secret with a bytea salt using the hex format
+SELECT scram_build_secret('sha256', 'secret password', '\xabba5432');
+
+-- test building a SCRAM secret with a valid salt and a different set of
+-- iterations
+SELECT scram_build_secret('sha256', 'secret password',
+  decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), 10000);
+
+-- test what happens when the salt is a NULL value.
+-- this should build a SCRAM secret using a random salt.
+SELECT regexp_replace(
+  scram_build_secret('sha256', 'secret password', NULL, 10000),
+    '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]{24})\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)',
+    '\1$\2:<salt>$<storedkey>:<serverkey>') AS scram_secret;
+
+-- test what happens when iterations is a null value. this should set iterations
+-- to be the default, currently 4096.
+SELECT
+  scram_build_secret('sha256', 'secret password',
+    decode('MTIzNDU2Nzg5MGFiY2RlZg==', 'base64'), NULL) ~
+  '^SCRAM-SHA-256\$4096' AS has_default_iterations;
+
+-- skip the remaining tests if this not a UTF8 server
+SELECT getdatabaseencoding() <> 'UTF8' AS skip_test \gset
+\if :skip_test
+\quit
+\endif
+
+-- test SASLprep. This tests the case where a user supplies a non-ASCII space
+-- character.
+SELECT
+  scram_build_secret('sha256', U&'one\1680space', decode('h2y81+nUwWp5uIJc4PgyXA==', 'base64')) =
+  'SCRAM-SHA-256$4096:h2y81+nUwWp5uIJc4PgyXA==$EiywEpO6rM3z3DGehubeoRpp8Orq0XuDUbdT9fQWwz8=:Wh7fq4C+bageihh3vTrkCr7YrlcDTG+JhfcFAuHn/6E=';
-- 
2.37.1 (Apple Git-137.1)

#21Michael Paquier
michael@paquier.xyz
In reply to: Jonathan S. Katz (#20)
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:
/messages/by-id/CAAWbhmhjcFc4oaGA_7YLUhtj6J+rxEY+BoDryGzNdaFLGfZZMg@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

#22Jonathan S. Katz
jkatz@postgresql.org
In reply to: Michael Paquier (#21)
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:
/messages/by-id/CAAWbhmhjcFc4oaGA_7YLUhtj6J+rxEY+BoDryGzNdaFLGfZZMg@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]https://github.com/aws/pg_tle/blob/main/docs/06_plpgsql_examples.md#example-password-check-hook-against-bad-password-dictionary (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
https://github.com/aws/pg_tle/blob/main/docs/06_plpgsql_examples.md#example-password-check-hook-against-bad-password-dictionary

#23Daniel Gustafsson
daniel@yesql.se
In reply to: Jonathan S. Katz (#22)
Re: User functions for building SCRAM secrets

On 22 Mar 2023, at 15:06, Jonathan S. Katz <jkatz@postgresql.org> wrote:
On 3/22/23 2:48 AM, Michael Paquier wrote:

I was reading this thread again, and pondered on this particular
point:
/messages/by-id/CAAWbhmhjcFc4oaGA_7YLUhtj6J+rxEY+BoDryGzNdaFLGfZZMg@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

#24Magnus Hagander
magnus@hagander.net
In reply to: Daniel Gustafsson (#23)
Re: User functions for building SCRAM secrets

On Fri, Mar 24, 2023 at 4:48 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 22 Mar 2023, at 15:06, Jonathan S. Katz <jkatz@postgresql.org> wrote:
On 3/22/23 2:48 AM, Michael Paquier wrote:

I was reading this thread again, and pondered on this particular
point:
/messages/by-id/CAAWbhmhjcFc4oaGA_7YLUhtj6J+rxEY+BoDryGzNdaFLGfZZMg@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 the control of an extension or
whatever you want. If one had the client under control one could for
example do the policy validation on the client and pass it to the
backend with some internal hash as well -- this would be entirely
under the control of the application though, as a generic libpq
connection could and should not be considered "client under control"
in this case.

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/

#25Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#24)
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

#26Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#25)
Re: User functions for building SCRAM secrets

On 14 Apr 2023, at 01:14, Michael Paquier <michael@paquier.xyz> 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

#27Michael Paquier
michael@paquier.xyz
In reply to: Daniel Gustafsson (#26)
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

#28Daniel Gustafsson
daniel@yesql.se
In reply to: Michael Paquier (#27)
Re: User functions for building SCRAM secrets

On 14 Apr 2023, at 05:50, Michael Paquier <michael@paquier.xyz> 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

#29John Naylor
johncnaylorls@gmail.com
In reply to: Jonathan S. Katz (#22)
Re: User functions for building SCRAM secrets

On Wed, Mar 22, 2023 at 9:06 PM Jonathan S. Katz <jkatz@postgresql.org> 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?

#30vignesh C
vignesh21@gmail.com
In reply to: John Naylor (#29)
Re: User functions for building SCRAM secrets

On Sat, 2 Dec 2023 at 12:22, John Naylor <johncnaylorls@gmail.com> wrote:

On Wed, Mar 22, 2023 at 9:06 PM Jonathan S. Katz <jkatz@postgresql.org> 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