From 6bce35e947d23fb4bf8a8e4b3ec5667a1b7d9c44 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Wed, 14 Dec 2022 11:52:36 +0100
Subject: [PATCH v3] 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.

Reviewed-by: Michael Paquier <michael@paquier.xyz>
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             |  4 ++--
 src/include/libpq/scram.h                     |  3 +++
 src/interfaces/libpq/fe-auth-scram.c          |  4 ++--
 src/interfaces/libpq/fe-auth.c                |  2 +-
 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        | 11 +++++++---
 src/test/regress/sql/password.sql             |  5 +++++
 16 files changed, 87 insertions(+), 12 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8e4145979d..c3f28dae86 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1098,6 +1098,26 @@ include_dir 'conf.d'
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><varname>scram_sha_256_iterations</varname> (<type>integer</type>)
+      <indexterm>
+       <primary><varname>scram_sha_256_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, 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 c9bab85e82..d0977ea8c2 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -184,6 +184,11 @@ static char *sanitize_char(char c);
 static char *sanitize_str(const char *s);
 static char *scram_mock_salt(const char *username);
 
+/*
+ * 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.
  *
@@ -483,7 +488,7 @@ pg_be_scram_build_secret(const char *password)
 				 errmsg("could not generate random salt")));
 
 	result = scram_build_secret(saltbuf, SCRAM_DEFAULT_SALT_LEN,
-								SCRAM_DEFAULT_ITERATIONS, password,
+								scram_sha_256_iterations, password,
 								&errstr);
 
 	if (prep_password)
@@ -692,7 +697,7 @@ mock_scram_secret(const char *username, int *iterations, char **salt,
 	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_KEY_LEN);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1bf14eec66..6ba0ece7ec 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -40,9 +40,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 "optimizer/cost.h"
 #include "optimizer/geqo.h"
 #include "optimizer/optimizer.h"
@@ -3423,6 +3425,17 @@ struct config_int ConfigureNamesInt[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"scram_sha_256_iterations", PGC_SUSET, CONN_AUTH_AUTH,
+			gettext_noop("Sets the iteration count for SCRAM secret generation."),
+			NULL,
+			GUC_REPORT | GUC_SUPERUSER_ONLY
+		},
+		&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 043864597f..3ef19f99c0 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -94,6 +94,7 @@
 
 #authentication_timeout = 1min		# 1s-600s
 #password_encryption = scram-sha-256	# scram-sha-256 or md5
+#scram_sha_256_iterations = 4096
 #db_user_namespace = off
 
 # GSSAPI using Kerberos
diff --git a/src/common/scram-common.c b/src/common/scram-common.c
index 1268625929..2dc9679a4d 100644
--- a/src/common/scram-common.c
+++ b/src/common/scram-common.c
@@ -206,8 +206,7 @@ scram_build_secret(const char *salt, int saltlen, int iterations,
 	int			encoded_server_len;
 	int			encoded_result;
 
-	if (iterations <= 0)
-		iterations = SCRAM_DEFAULT_ITERATIONS;
+	Assert(iterations > 0);
 
 	/* Calculate StoredKey and ServerKey */
 	if (scram_SaltedPassword(password, salt, saltlen, iterations,
diff --git a/src/include/common/scram-common.h b/src/include/common/scram-common.h
index 4acf2a78ad..8032e31a89 100644
--- a/src/include/common/scram-common.h
+++ b/src/include/common/scram-common.h
@@ -38,10 +38,10 @@
 #define SCRAM_DEFAULT_SALT_LEN		16
 
 /*
- * Default number of iterations when generating secret.  Should be at least
+ * 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, const char *salt,
 								 int saltlen, int iterations, uint8 *result,
diff --git a/src/include/libpq/scram.h b/src/include/libpq/scram.h
index c51e848c24..10f302aa98 100644
--- a/src/include/libpq/scram.h
+++ b/src/include/libpq/scram.h
@@ -17,6 +17,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 c500bea9e7..cf5e8824d3 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -882,7 +882,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;
@@ -913,7 +913,7 @@ pg_fe_scram_build_secret(const char *password, const char **errstr)
 	}
 
 	result = scram_build_secret(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 4a6c358bb6..52fee8e579 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -1253,7 +1253,7 @@ 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_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 049a8bb1a1..e41423a593 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 f88d672c6c..df9c13138a 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_iterations = SCRAM_SHA_256_DEFAULT_ITERATIONS;
 	conn->sversion = 0;
 
 	/* Drop large-object lookup data */
@@ -3909,6 +3910,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_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 da229d632a..358f6a42eb 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_sha_256_iterations") == 0)
+	{
+		conn->scram_iterations = atoi(value);
+	}
 }
 
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 512762f999..652df5b6ca 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_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 42d3d4c79b..f8add4f376 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_sha_256_iterations=100000;
+	 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..060496b03d 100644
--- a/src/test/regress/expected/password.out
+++ b/src/test/regress/expected/password.out
@@ -72,12 +72,15 @@ 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_sha_256_iterations = 99999;
+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%'
     ORDER BY rolname, rolpassword;
-     rolname     |                rolpassword_masked                 
------------------+---------------------------------------------------
+     rolname     |                 rolpassword_masked                 
+-----------------+----------------------------------------------------
  regress_passwd1 | md5cd3578025fe2c3d7ed1b9a9b26238b70
  regress_passwd2 | md5dfa155cadd5f4ad57860162f3fab9cdb
  regress_passwd3 | SCRAM-SHA-256$4096:<salt>$<storedkey>:<serverkey>
@@ -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$99999:<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..91c2114e8e 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_sha_256_iterations = 99999;
+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)

