On 10/31/22 8:56 PM, Michael Paquier wrote:
On Mon, Oct 31, 2022 at 04:27:08PM -0400, Jonathan S. Katz wrote:1. password only -- this defers to the PG defaults for SCRAM 2. password + salt -- this is useful for the password history / dictionary case to allow for a predictable way to check a password.Well, one could pass a salt based on something generated by random() to emulate what we currently do in the default case, as well. The salt length is an extra possibility, letting it be randomly generated by pg_strong_random().
Sure, this is a good point. From a SQL level we can get that from pgcrypto "gen_random_bytes".
Per this and ilmari's feedback, I updated the 2nd argument to be a bytea. See the corresponding tests that then show using decode(..., 'base64') to handle this.
When I write the docs, I'll include that in the examples.
1. Location of the functions. I put them in src/backend/utils/adt/cryptohashfuncs.c as I wasn't sure where it would make sense to have them (and they could easily go into their own file).As of adt/authfuncs.c? cryptohashfuncs.c does not strike me as a good fit.
I went with your suggested name.
Please let me know if you have any questions. I'll add a CF entry for this.+{ oid => '8555', descr => 'Build a SCRAM secret', + proname => 'scram_build_secret_sha256', proleakproof => 't', prorettype => 'text', + proargtypes => 'text', prosrc => 'scram_build_secret_sha256_from_password' }, +{ oid => '8556', descr => 'Build a SCRAM secret', + proname => 'scram_build_secret_sha256', proleakproof => 't', + provolatile => 'i', prorettype => 'text', + proargtypes => 'text text', prosrc => 'scram_build_secret_sha256_from_password_and_salt' }, +{ oid => '8557', descr => 'Build a SCRAM secret', + proname => 'scram_build_secret_sha256', proleakproof => 't', + provolatile => 'i', prorettype => 'text', + proargtypes => 'text text int4', prosrc => 'scram_build_secret_sha256_from_password_and_salt_and_iterations' }, Keeping this approach as-is, I don't think that you should consume 3 OIDs, but 1 (with scram_build_secret_sha256_from_password_and_.. as prosrc) that has two defaults for the second argument (salt string, default as NULL) and third argument (salt, default at 0), with the defaults set up in system_functions.sql via a redefinition.
Thanks for the suggestion. I went with this as well.
Note that you cannot pass down an expression for the password of CREATE/ALTER USER, meaning that this would need a \gset at least if done by a psql client for example, and passing down a password string is not an encouraged practice, either. Another approach is to also provide a role OID in input and store the newly-computed password in pg_authid (as of [1]), so as one can store it easily.
...unless you dynamically generate the CREATE/ALTER ROLE command ;) (and yes, lots of discussion on that).
Did you look at extending \password? Being able to extend PQencryptPasswordConn() with custom parameters has been discussed in the past, but this has gone nowhere. That's rather unrelated to what you are looking for, just mentioning as we are playing with options to have control the iteration number and the salt.
Not yet, but happy to do that as a follow-up patch.Please see version 2. If folks are generally happy with this, I'll address any additional feedback and write up docs.
Thanks, Jonathan
diff --git a/src/backend/catalog/system_functions.sql
b/src/backend/catalog/system_functions.sql
index 30a048f6b0..4aa76b81d9 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -594,6 +594,12 @@ LANGUAGE internal
STRICT IMMUTABLE PARALLEL SAFE
AS 'unicode_is_normalized';
+-- defaults for building a "scram-sha-256" secret
+CREATE OR REPLACE FUNCTION
+ scram_build_secret_sha256(text, bytea DEFAULT NULL, int DEFAULT NULL)
+RETURNS text
+LANGUAGE INTERNAL
+VOLATILE AS 'scram_build_secret_sha256';
--
-- The default permissions for functions mean that anyone can execute them.
-- A number of functions shouldn't be executable by just anyone, but rather
diff --git a/src/backend/utils/adt/Makefile b/src/backend/utils/adt/Makefile
index 0de0bbb1b8..7ddb186f96 100644
--- a/src/backend/utils/adt/Makefile
+++ b/src/backend/utils/adt/Makefile
@@ -22,6 +22,7 @@ OBJS = \
arraysubs.o \
arrayutils.o \
ascii.o \
+ authfuncs.o \
bool.o \
cash.o \
char.o \
diff --git a/src/backend/utils/adt/authfuncs.c
b/src/backend/utils/adt/authfuncs.c
new file mode 100644
index 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;
OpenPGP_signature
Description: OpenPGP digital signature
