From af27410ebc0b6dec7751f78e5ddbfe758a37bd63 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Thu, 23 Feb 2023 13:53:36 +0100
Subject: [PATCH v6] Make SCRAM iteration count configurable

Replace the hardcoded value with a GUC such that the iteration count
can be raised in order to increase protection against brute-force
attacks.  The hardcoded value for SCRAM iteration count was defined
to be 4096, which is taken from RFC 7677, so set the default for the
GUC to 4096 to match.  Raising the iteration count will make stored
passwords more resilient to brute-force attacks at the cost of more
computation performed during connection establishment. Lowering the
count will reduce computational overhead during connections at the
tradeoff of reducing strength against brute-force attacks. When the
alternative is to fall back to md5 instead, SCRAM with a very low
iteration count is still preferrable so allow iteration counts all
the way down to one.

The new GUC is intentionally generically named such that it can be
made to support future SCRAM methods should they be standardized.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Jonathan S. Katz <jkatz@postgresql.org>
Discussion: https://postgr.es/m/F72E7BC7-189F-4B17-BF47-9735EB72C364@yesql.se
---
 doc/src/sgml/config.sgml                      | 20 +++++++++++++++++++
 src/backend/libpq/auth-scram.c                |  9 +++++++--
 src/backend/utils/misc/guc_tables.c           | 13 ++++++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/common/scram-common.c                     |  3 +--
 src/include/common/scram-common.h             |  2 +-
 src/include/libpq/scram.h                     |  3 +++
 src/interfaces/libpq/fe-auth-scram.c          |  4 ++--
 src/interfaces/libpq/fe-auth.c                |  4 +++-
 src/interfaces/libpq/fe-auth.h                |  1 +
 src/interfaces/libpq/fe-connect.c             |  2 ++
 src/interfaces/libpq/fe-exec.c                |  4 ++++
 src/interfaces/libpq/libpq-int.h              |  1 +
 src/test/authentication/t/001_password.pl     | 16 +++++++++++++++
 src/test/regress/expected/password.out        |  7 ++++++-
 src/test/regress/sql/password.sql             |  5 +++++
 16 files changed, 86 insertions(+), 9 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6..a51961f197 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1132,6 +1132,26 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>scram_iterations</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>scram_iterations</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        The number of computational iterations to be performed when encrypting
+        a password using SCRAM-SHA-256. The default is <literal>4096</literal>.
+        A higher number of iterations provides additional protection against
+        brute-force attacks on stored passwords, but makes authentication
+        slower. Changing the value has no effect on existing passwords
+        encrypted with SCRAM-SHA-256 as the iteration count is fixed at the
+        time of encryption. In order to make use of a changed value, a new
+        password must be set.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-krb-server-keyfile" xreflabel="krb_server_keyfile">
       <term><varname>krb_server_keyfile</varname> (<type>string</type>)
       <indexterm>
diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 4441e0d774..75583dcbf0 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -191,6 +191,11 @@ static char *scram_mock_salt(const char *username,
 							 pg_cryptohash_type hash_type,
 							 int key_length);
 
+/*
+ * The number of iterations to use when generating new secrets.
+ */
+int			scram_sha_256_iterations;
+
 /*
  * Get a list of SASL mechanisms that this module supports.
  *
@@ -496,7 +501,7 @@ pg_be_scram_build_secret(const char *password)
 
 	result = scram_build_secret(PG_SHA256, SCRAM_SHA_256_KEY_LEN,
 								saltbuf, SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+								scram_sha_256_iterations, password,
 								&errstr);
 
 	if (prep_password)
@@ -717,7 +722,7 @@ mock_scram_secret(const char *username, pg_cryptohash_type *hash_type,
 	encoded_salt[encoded_len] = '\0';
 
 	*salt = encoded_salt;
-	*iterations = SCRAM_DEFAULT_ITERATIONS;
+	*iterations = SCRAM_SHA_256_DEFAULT_ITERATIONS;
 
 	/* StoredKey and ServerKey are not used in a doomed authentication */
 	memset(stored_key, 0, SCRAM_MAX_KEY_LEN);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1c0583fe26..a60bd48499 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -41,9 +41,11 @@
 #include "commands/trigger.h"
 #include "commands/user.h"
 #include "commands/vacuum.h"
+#include "common/scram-common.h"
 #include "jit/jit.h"
 #include "libpq/auth.h"
 #include "libpq/libpq.h"
+#include "libpq/scram.h"
 #include "nodes/queryjumble.h"
 #include "optimizer/cost.h"
 #include "optimizer/geqo.h"
@@ -3468,6 +3470,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"scram_iterations", PGC_USERSET, CONN_AUTH_AUTH,
+			gettext_noop("Sets the iteration count for SCRAM secret generation."),
+			NULL,
+			GUC_REPORT
+		},
+		&scram_sha_256_iterations,
+		SCRAM_SHA_256_DEFAULT_ITERATIONS, 1, INT_MAX,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d06074b86f..fc831565d9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -95,6 +95,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #password_encryption = scram-sha-256	# scram-sha-256 or md5
+#scram_iterations = 4096
 #db_user_namespace = off
 
 # GSSAPI using Kerberos
diff --git a/src/common/scram-common.c b/src/common/scram-common.c
index bd40d497a9..ef997ef684 100644
--- a/src/common/scram-common.c
+++ b/src/common/scram-common.c
@@ -214,8 +214,7 @@ scram_build_secret(pg_cryptohash_type hash_type, int key_length,
 	/* Only this hash method is supported currently */
 	Assert(hash_type == PG_SHA256);
 
-	if (iterations <= 0)
-		iterations = SCRAM_DEFAULT_ITERATIONS;
+	Assert(iterations > 0);
 
 	/* Calculate StoredKey and ServerKey */
 	if (scram_SaltedPassword(password, hash_type, key_length,
diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h
index 0c75df5559..5ccff96ece 100644
--- a/src/include/common/scram-common.h
+++ b/src/include/common/scram-common.h
@@ -47,7 +47,7 @@
  * Default number of iterations when generating secret.  Should be at least
  * 4096 per RFC 7677.
  */
-#define SCRAM_DEFAULT_ITERATIONS	4096
+#define SCRAM_SHA_256_DEFAULT_ITERATIONS	4096
 
 extern int	scram_SaltedPassword(const char *password,
 								 pg_cryptohash_type hash_type, int key_length,
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index b275e1e87e..310bc36517 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -18,6 +18,9 @@
 #include "libpq/libpq-be.h"
 #include "libpq/sasl.h"
 
+/* Number of iterations when generating new secrets */
+extern PGDLLIMPORT int scram_sha_256_iterations;
+
 /* SASL implementation callbacks */
 extern PGDLLIMPORT const pg_be_sasl_mech pg_be_scram_mech;
 
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 9c42ea4f81..7162fdbb85 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -894,7 +894,7 @@ verify_server_signature(fe_scram_state *state, bool *match,
  * error details.
  */
 char *
-pg_fe_scram_build_secret(const char *password, const char **errstr)
+pg_fe_scram_build_secret(const char *password, int iterations, const char **errstr)
 {
 	char	   *prep_password;
 	pg_saslprep_rc rc;
@@ -926,7 +926,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
 
 	result = scram_build_secret(PG_SHA256, SCRAM_SHA_256_KEY_LEN, saltbuf,
 								SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+								iterations, password,
 								errstr);
 
 	free(prep_password);
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 9afc6f19b9..0935356075 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -1253,7 +1253,9 @@ PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
 	{
 		const char *errstr = NULL;
 
-		crypt_pwd = pg_fe_scram_build_secret(passwd, &errstr);
+		crypt_pwd = pg_fe_scram_build_secret(passwd,
+											 conn->scram_sha_256_iterations,
+											 &errstr);
 		if (!crypt_pwd)
 			libpq_append_conn_error(conn, "could not encrypt password: %s", errstr);
 	}
diff --git a/src/interfaces/libpq/fe-auth.h b/src/interfaces/libpq/fe-auth.h
index 1aa556ea2f..124dd5d031 100644
--- a/src/interfaces/libpq/fe-auth.h
+++ b/src/interfaces/libpq/fe-auth.h
@@ -26,6 +26,7 @@ extern char *pg_fe_getauthname(PQExpBuffer errorMessage);
 /* Mechanisms in fe-auth-scram.c */
 extern const pg_fe_sasl_mech pg_scram_mech;
 extern char *pg_fe_scram_build_secret(const char *password,
+									  int iterations,
 									  const char **errstr);
 
 #endif							/* FE_AUTH_H */
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 8f80c35c89..49501c0a9e 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -586,6 +586,7 @@ pqDropServerData(PGconn *conn)
 	conn->std_strings = false;
 	conn->default_transaction_read_only = PG_BOOL_UNKNOWN;
 	conn->in_hot_standby = PG_BOOL_UNKNOWN;
+	conn->scram_sha_256_iterations = SCRAM_SHA_256_DEFAULT_ITERATIONS;
 	conn->sversion = 0;
 
 	/* Drop large-object lookup data */
@@ -3936,6 +3937,7 @@ makeEmptyPGconn(void)
 	conn->std_strings = false;	/* unless server says differently */
 	conn->default_transaction_read_only = PG_BOOL_UNKNOWN;
 	conn->in_hot_standby = PG_BOOL_UNKNOWN;
+	conn->scram_sha_256_iterations = SCRAM_SHA_256_DEFAULT_ITERATIONS;
 	conn->verbosity = PQERRORS_DEFAULT;
 	conn->show_context = PQSHOW_CONTEXT_ERRORS;
 	conn->sock = PGINVALID_SOCKET;
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index ec62550e38..a16bbf32ef 100644
--- a/src/interfaces/libpq/fe-exec.c
+++ b/src/interfaces/libpq/fe-exec.c
@@ -1181,6 +1181,10 @@ pqSaveParameterStatus(PGconn *conn, const char *name, const char *value)
 		conn->in_hot_standby =
 			(strcmp(value, "on") == 0) ? PG_BOOL_YES : PG_BOOL_NO;
 	}
+	else if (strcmp(name, "scram_iterations") == 0)
+	{
+		conn->scram_sha_256_iterations = atoi(value);
+	}
 }
 
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d7ec5ed429..940d1a35a1 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -515,6 +515,7 @@ struct pg_conn
 	/* Assorted state for SASL, SSL, GSS, etc */
 	const pg_fe_sasl_mech *sasl;
 	void	   *sasl_state;
+	int			scram_sha_256_iterations;
 
 	/* SSL structures */
 	bool		ssl_in_use;
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index a2fde1408b..3302c2e9a9 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -86,6 +86,14 @@ $node->safe_psql('postgres',
 	q{SET password_encryption='md5'; CREATE ROLE "md5,role" LOGIN PASSWORD 'pass';}
 );
 
+# Create a role with a non-default iteration count
+$node->safe_psql(
+	'postgres',
+	"SET password_encryption='scram-sha-256';
+	 SET scram_iterations=10000;
+	 CREATE ROLE scram_role_iter LOGIN PASSWORD 'pass';"
+);
+
 # Create a database to test regular expression.
 $node->safe_psql('postgres', "CREATE database regex_testdb;");
 
@@ -134,6 +142,14 @@ test_conn(
 	log_like => [
 		qr/connection authenticated: identity="scram_role" method=scram-sha-256/
 	]);
+test_conn(
+	$node,
+	'user=scram_role_iter',
+	'scram-sha-256',
+	0,
+	log_like => [
+		qr/connection authenticated: identity="scram_role_iter" method=scram-sha-256/
+	]);
 test_conn($node, 'user=md5_role', 'scram-sha-256', 2,
 	log_unlike => [qr/connection authenticated:/]);
 
diff --git a/src/test/regress/expected/password.out b/src/test/regress/expected/password.out
index 7c84c9da33..ac14d0bc8c 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -72,6 +72,9 @@ CREATE ROLE regress_passwd6 PASSWORD 'SCRAM-SHA-256$1234';
 CREATE ROLE regress_passwd7 PASSWORD 'md5012345678901234567890123456789zz';
 -- invalid length
 CREATE ROLE regress_passwd8 PASSWORD 'md501234567890123456789012345678901zz';
+-- Increasing the SCRAM iteration count
+SET scram_iterations = 9999;
+CREATE ROLE regress_passwd9 PASSWORD 'raisediterationcount';
 SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:<salt>$<storedkey>:<serverkey>') as rolpassword_masked
     FROM pg_authid
     WHERE rolname LIKE 'regress_passwd%'
@@ -86,7 +89,8 @@ SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+
  regress_passwd6 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
  regress_passwd7 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
  regress_passwd8 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
-(8 rows)
+ regress_passwd9 | SCRAM-SHA-256$9999:<salt>$<storedkey>:<serverkey>
+(9 rows)
 
 -- An empty password is not allowed, in any form
 CREATE ROLE regress_passwd_empty PASSWORD '';
@@ -129,6 +133,7 @@ DROP ROLE regress_passwd5;
 DROP ROLE regress_passwd6;
 DROP ROLE regress_passwd7;
 DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
 DROP ROLE regress_passwd_empty;
 DROP ROLE regress_passwd_sha_len0;
 DROP ROLE regress_passwd_sha_len1;
diff --git a/src/test/regress/sql/password.sql b/src/test/regress/sql/password.sql
index 98f49916e5..2679185337 100644
--- a/src/test/regress/sql/password.sql
+++ b/src/test/regress/sql/password.sql
@@ -63,6 +63,10 @@ CREATE ROLE regress_passwd7 PASSWORD 'md5012345678901234567890123456789zz';
 -- invalid length
 CREATE ROLE regress_passwd8 PASSWORD 'md501234567890123456789012345678901zz';
 
+-- Increasing the SCRAM iteration count
+SET scram_iterations = 9999;
+CREATE ROLE regress_passwd9 PASSWORD 'raisediterationcount';
+
 SELECT rolname, regexp_replace(rolpassword, '(SCRAM-SHA-256)\$(\d+):([a-zA-Z0-9+/=]+)\$([a-zA-Z0-9+=/]+):([a-zA-Z0-9+/=]+)', '\1$\2:<salt>$<storedkey>:<serverkey>') as rolpassword_masked
     FROM pg_authid
     WHERE rolname LIKE 'regress_passwd%'
@@ -97,6 +101,7 @@ DROP ROLE regress_passwd5;
 DROP ROLE regress_passwd6;
 DROP ROLE regress_passwd7;
 DROP ROLE regress_passwd8;
+DROP ROLE regress_passwd9;
 DROP ROLE regress_passwd_empty;
 DROP ROLE regress_passwd_sha_len0;
 DROP ROLE regress_passwd_sha_len1;
-- 
2.32.1 (Apple Git-133)

