Subject: Re: Modern SHA2- based password hashes for pgcrypto

From: Alvaro Herrera <[email-masked]>
Date: Thu, 3 Apr 2025 20:39:18 +0200

On 2025-Mar-11, Bernd Helmle wrote:

> Please find attached v4 of this patch. I added the following changes:
> 
> - Check for non-supported characters in the salt like passlib does.
> - Check for reserved tokens when parsing the salt string (i find this
> very strict, but it covers the cases Japin Li pointed out).
> 
> I didn't implement being more strict when it comes to the salt length:
> the salt might be provided from external sources and the password hash
> is just used for validating within the database.
> 
> When hashing the password within postgres, users always should use
> gen_salt(), which generates a reasonable salt string. 

This sounds reasonable to me.

> Maybe, in case of empty salts, we should issue a WARNING instead of
> erroring out and put additional documentation on how to use it right.

I don't know, that doesn't seem ideal to me, because it's very easy to
run stuff and never see the warnings.  If we find that people are
desperate to use empty salts, we can relax that later (turn the error to
a warning), but I'd rather not have it in the first cut.

> Hmm, i didn't understand that passlib does decode them first, i thought
> they use it encoded... at least, in our current form we're pretty much
> compatible with Drepper, passlib and OpenSSL, as far as i tested:

I am ready to believe that I misinterpreted what I read.

> So the hashes we produce are identical, but with being more strict we
> differ in the handling of the provided salt.

Okay.

I can offer a few cosmetic changes.  0001 is a pgindent run, and 0002 is
some manual adjustments after that.  There are only two nontrivial
changes

1. the calculation for rounds was using type long, which is confusing
because the range is different according to the platform.  Since it's
limited by the macro definitions to no more than 999999999, we can make
it an int32 instead.  So we use strtoint() instead of strtoul() to parse
the value, and remove the "l" suffixes from the macros that define the
limits and default, which were bugging me a bit when used in the
gen_list struct.

2. I changed "if (block % 3)" to "if ((block % 3) != 0)".

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"El destino baraja y nosotros jugamos" (A. Schopenhauer)
>From 864395b6468d62f17d2e8f0e593408849564edc1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email-masked]>
Date: Wed, 2 Apr 2025 17:25:20 +0200
Subject: [PATCH 1/2] pgindent

---
 contrib/pgcrypto/crypt-gensalt.c | 14 +++++------
 contrib/pgcrypto/crypt-sha.c     | 42 +++++++++++++++++---------------
 contrib/pgcrypto/px-crypt.c      |  6 ++---
 contrib/pgcrypto/px-crypt.h      |  2 +-
 4 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/contrib/pgcrypto/crypt-gensalt.c b/contrib/pgcrypto/crypt-gensalt.c
index b9aded897d1..922f9eea6fa 100644
--- a/contrib/pgcrypto/crypt-gensalt.c
+++ b/contrib/pgcrypto/crypt-gensalt.c
@@ -190,9 +190,9 @@ static char *
 _crypt_gensalt_sha(unsigned long count,
 				   const char *input, int size, char *output, int output_size)
 {
-	char * s_ptr = output;
+	char	   *s_ptr = output;
 	unsigned int result_bufsize = PX_SHACRYPT_SALT_BUF_LEN;
-	int rc;
+	int			rc;
 
 	/* output buffer must be allocated with PX_MAX_SALT_LEN bytes */
 	if (PX_MAX_SALT_LEN < result_bufsize)
@@ -203,8 +203,8 @@ _crypt_gensalt_sha(unsigned long count,
 	}
 
 	/*
-	 * Care must be taken to not exceed the buffer size allocated for
-	 * the input character buffer.
+	 * Care must be taken to not exceed the buffer size allocated for the
+	 * input character buffer.
 	 */
 	if ((PX_SHACRYPT_SALT_MAX_LEN != size)
 		|| (output_size < size))
@@ -229,8 +229,8 @@ _crypt_gensalt_sha(unsigned long count,
 	/*
 	 * Normalize salt string
 	 *
-	 * size of input buffer was checked above to
-	 * not exceed PX_SHACRYPT_SALT_LEN_MAX.
+	 * size of input buffer was checked above to not exceed
+	 * PX_SHACRYPT_SALT_LEN_MAX.
 	 */
 	for (int i = 0; i < size; i++)
 	{
@@ -268,4 +268,4 @@ _crypt_gensalt_sha256_rn(unsigned long count,
 	output[2] = '$';
 
 	return _crypt_gensalt_sha(count, input, size, output, output_size);
-}
\ No newline at end of file
+}
diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c
index ec51e865a5e..13ee8913052 100644
--- a/contrib/pgcrypto/crypt-sha.c
+++ b/contrib/pgcrypto/crypt-sha.c
@@ -77,24 +77,25 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	int			err;
 
 	const char *dec_salt_binary;	/* pointer into the real salt string */
-	StringInfo decoded_salt = NULL; /* decoded salt string */
+	StringInfo	decoded_salt = NULL;	/* decoded salt string */
 
 	unsigned char sha_buf[PX_SHACRYPT_DIGEST_MAX_LEN];
 	unsigned char sha_buf_tmp[PX_SHACRYPT_DIGEST_MAX_LEN];	/* temporary buffer for
-																 * digests */
+															 * digests */
 
 	char		rounds_custom = 0;
 	char	   *p_bytes = NULL;
 	char	   *s_bytes = NULL;
 	char	   *cp = NULL;
-	const char *fp = NULL;				/* intermediate pointer within salt string */
-	const char *ep = NULL;				/* holds pointer to the end of the salt string */
+	const char *fp = NULL;		/* intermediate pointer within salt string */
+	const char *ep = NULL;		/* holds pointer to the end of the salt string */
 
 	size_t		buf_size = 0;	/* buffer size for sha256crypt/sha512crypt */
 	unsigned int block;			/* number of bytes processed */
 	unsigned long rounds = PX_SHACRYPT_ROUNDS_DEFAULT;
 
-	unsigned	len, salt_len = 0;
+	unsigned	len,
+				salt_len = 0;
 
 	/* Init result buffer */
 	out_buf = makeStringInfoExt(PX_SHACRYPT_BUF_LEN);
@@ -299,16 +300,16 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 
 	/*
 	 * We need the real decoded salt string from salt input, this is every
-	 * character before the last '$' in the preamble. Append every
-	 * compatible character up to PX_SHACRYPT_SALT_MAX_LEN to the result buffer.
-	 * Note that depending on the input, there might be no '$' marker after
-	 * the salt, when there is no password hash attached at the end.
+	 * character before the last '$' in the preamble. Append every compatible
+	 * character up to PX_SHACRYPT_SALT_MAX_LEN to the result buffer. Note
+	 * that depending on the input, there might be no '$' marker after the
+	 * salt, when there is no password hash attached at the end.
 	 *
 	 * We try hard to recognize mistakes, but since we might get an input
 	 * string which might also have the password hash after the salt string
 	 * section we give up as soon we reach the end of the input or if there
-	 * are any bytes consumed for the salt string until we reach the
-	 * first '$' marker thereafter.
+	 * are any bytes consumed for the salt string until we reach the first '$'
+	 * marker thereafter.
 	 */
 	for (ep = dec_salt_binary;
 		 *ep && ep < (dec_salt_binary + PX_SHACRYPT_SALT_MAX_LEN);
@@ -320,9 +321,9 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		 * First check for accidentally embedded magic strings here. We don't
 		 * support '$' in salt strings anyways and seeing a magic byte trying
 		 * to identify shacrypt hashes might indicate that something went
-		 * wrong when generating this salt string. Note that we later check for
-		 * non-supported literals anyways, but any '$' here confuses us
-		 * at this point.
+		 * wrong when generating this salt string. Note that we later check
+		 * for non-supported literals anyways, but any '$' here confuses us at
+		 * this point.
 		 */
 		fp = strstr(dec_salt_binary, magic_bytes[0]);
 
@@ -362,13 +363,14 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		}
 		else
 		{
-			/* We encountered a '$' marker. Check if we already absorbed
-			 * some bytes from input. If true, we are optimistic and
-			 * terminate at this stage. Of not, we try further.
+			/*
+			 * We encountered a '$' marker. Check if we already absorbed some
+			 * bytes from input. If true, we are optimistic and terminate at
+			 * this stage. Of not, we try further.
 			 *
-			 * If we already consumed enough bytes for the salt string, everything
-			 * that is after this marker is considered to be part of an
-			 * optionally specified password hash and ignored.
+			 * If we already consumed enough bytes for the salt string,
+			 * everything that is after this marker is considered to be part
+			 * of an optionally specified password hash and ignored.
 			 */
 			if (decoded_salt->len > 0)
 				break;
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index df9875b9433..a786277aeac 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -71,7 +71,7 @@ static char *
 run_crypt_sha(const char *psw, const char *salt,
 			  char *buf, unsigned len)
 {
-	char *res;
+	char	   *res;
 
 	res = px_crypt_shacrypt(psw, salt, buf, len);
 	return res;
@@ -140,9 +140,9 @@ static struct generator gen_list[] = {
 	{"xdes", _crypt_gensalt_extended_rn, 3, PX_XDES_ROUNDS, 1, 0xFFFFFF},
 	{"bf", _crypt_gensalt_blowfish_rn, 16, PX_BF_ROUNDS, 4, 31},
 	{"sha256crypt", _crypt_gensalt_sha256_rn, PX_SHACRYPT_SALT_MAX_LEN,
-	 PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
+	PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
 	{"sha512crypt", _crypt_gensalt_sha512_rn, PX_SHACRYPT_SALT_MAX_LEN,
-	 PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
+	PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
 	{NULL, NULL, 0, 0, 0, 0}
 };
 
diff --git a/contrib/pgcrypto/px-crypt.h b/contrib/pgcrypto/px-crypt.h
index 9287c33efe1..3829ec6c347 100644
--- a/contrib/pgcrypto/px-crypt.h
+++ b/contrib/pgcrypto/px-crypt.h
@@ -108,6 +108,6 @@ char	   *px_crypt_md5(const char *pw, const char *salt,
 						 char *passwd, unsigned dstlen);
 
 /* crypt-sha.c */
-char       *px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstlen);
+char	   *px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstlen);
 
 #endif							/* _PX_CRYPT_H */
-- 
2.39.5

>From 567d4b3706295a1f2bed53eb0b2f4acb95898899 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email-masked]>
Date: Thu, 3 Apr 2025 20:14:52 +0200
Subject: [PATCH 2/2] review

---
 contrib/pgcrypto/crypt-gensalt.c |  26 ++--
 contrib/pgcrypto/crypt-sha.c     | 248 ++++++++++++-------------------
 contrib/pgcrypto/px-crypt.c      |  14 +-
 contrib/pgcrypto/px-crypt.h      |   6 +-
 4 files changed, 121 insertions(+), 173 deletions(-)

diff --git a/contrib/pgcrypto/crypt-gensalt.c b/contrib/pgcrypto/crypt-gensalt.c
index 922f9eea6fa..ef68ae0e49e 100644
--- a/contrib/pgcrypto/crypt-gensalt.c
+++ b/contrib/pgcrypto/crypt-gensalt.c
@@ -186,6 +186,9 @@ _crypt_gensalt_blowfish_rn(unsigned long count,
 	return output;
 }
 
+/*
+ * Helper for _crypt_gensalt_sha256_rn and _crypt_gensalt_sha512_rn
+ */
 static char *
 _crypt_gensalt_sha(unsigned long count,
 				   const char *input, int size, char *output, int output_size)
@@ -196,32 +199,25 @@ _crypt_gensalt_sha(unsigned long count,
 
 	/* output buffer must be allocated with PX_MAX_SALT_LEN bytes */
 	if (PX_MAX_SALT_LEN < result_bufsize)
-	{
 		ereport(ERROR,
-				(errcode(ERRCODE_SYNTAX_ERROR),
-				 errmsg("invalid size of salt")));
-	}
+				errcode(ERRCODE_SYNTAX_ERROR),
+				errmsg("invalid size of salt"));
 
 	/*
 	 * Care must be taken to not exceed the buffer size allocated for the
 	 * input character buffer.
 	 */
-	if ((PX_SHACRYPT_SALT_MAX_LEN != size)
-		|| (output_size < size))
-	{
+	if ((PX_SHACRYPT_SALT_MAX_LEN != size) || (output_size < size))
 		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("invalid length of salt buffer")));
-	}
+				errcode(ERRCODE_INTERNAL_ERROR),
+				errmsg("invalid length of salt buffer"));
 
 	/* Skip magic bytes, set by callers */
 	s_ptr += 3;
 	if ((rc = pg_snprintf(s_ptr, 18, "rounds=%ld$", count)) <= 0)
-	{
 		ereport(ERROR,
-				(errcode(ERRCODE_INTERNAL_ERROR),
-				 errmsg("cannot format salt string")));
-	}
+				errcode(ERRCODE_INTERNAL_ERROR),
+				errmsg("cannot format salt string"));
 
 	/* s_ptr should now be positioned at the start of the salt string */
 	s_ptr += rc;
@@ -242,6 +238,7 @@ _crypt_gensalt_sha(unsigned long count,
 	return output;
 }
 
+/* gen_list->gen function for sha512 */
 char *
 _crypt_gensalt_sha512_rn(unsigned long count,
 						 char const *input, int size,
@@ -256,6 +253,7 @@ _crypt_gensalt_sha512_rn(unsigned long count,
 	return _crypt_gensalt_sha(count, input, size, output, output_size);
 }
 
+/* gen_list->gen function for sha256 */
 char *
 _crypt_gensalt_sha256_rn(unsigned long count,
 						 const char *input, int size,
diff --git a/contrib/pgcrypto/crypt-sha.c b/contrib/pgcrypto/crypt-sha.c
index 13ee8913052..862f3fb9a68 100644
--- a/contrib/pgcrypto/crypt-sha.c
+++ b/contrib/pgcrypto/crypt-sha.c
@@ -44,6 +44,8 @@
  *
  */
 #include "postgres.h"
+
+#include "common/string.h"
 #include "miscadmin.h"
 
 #include "px-crypt.h"
@@ -78,23 +80,20 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 
 	const char *dec_salt_binary;	/* pointer into the real salt string */
 	StringInfo	decoded_salt = NULL;	/* decoded salt string */
-
 	unsigned char sha_buf[PX_SHACRYPT_DIGEST_MAX_LEN];
-	unsigned char sha_buf_tmp[PX_SHACRYPT_DIGEST_MAX_LEN];	/* temporary buffer for
-															 * digests */
 
+	/* temporary buffer for digests */
+	unsigned char sha_buf_tmp[PX_SHACRYPT_DIGEST_MAX_LEN];
 	char		rounds_custom = 0;
 	char	   *p_bytes = NULL;
 	char	   *s_bytes = NULL;
 	char	   *cp = NULL;
 	const char *fp = NULL;		/* intermediate pointer within salt string */
 	const char *ep = NULL;		/* holds pointer to the end of the salt string */
-
 	size_t		buf_size = 0;	/* buffer size for sha256crypt/sha512crypt */
 	unsigned int block;			/* number of bytes processed */
-	unsigned long rounds = PX_SHACRYPT_ROUNDS_DEFAULT;
-
-	unsigned	len,
+	uint32		rounds = PX_SHACRYPT_ROUNDS_DEFAULT;
+	unsigned int len,
 				salt_len = 0;
 
 	/* Init result buffer */
@@ -106,22 +105,16 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		return NULL;
 
 	if (pw == NULL)
-	{
 		elog(ERROR, "null value for password rejected");
-	}
 
 	if (salt == NULL)
-	{
 		elog(ERROR, "null value for salt rejected");
-	}
 
 	/*
 	 * Make sure result buffers are large enough.
 	 */
 	if (dstlen < PX_SHACRYPT_BUF_LEN)
-	{
 		elog(ERROR, "insufficient result buffer size to encrypt password");
-	}
 
 	/* Init contents of buffers properly */
 	memset(&sha_buf, '\0', sizeof(sha_buf));
@@ -138,27 +131,22 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	 * Analyze and prepare the salt string
 	 *
 	 * The magic string should be specified in the first three bytes of the
-	 * salt string. But do some sanity checks before.
+	 * salt string.  Do some sanity checks first.
 	 */
 	if (strlen(dec_salt_binary) < 3)
-	{
 		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid salt")));
-	}
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("invalid salt"));
 
 	/*
 	 * Check format of magic bytes. These should define either 5=sha256crypt
 	 * or 6=sha512crypt in the second byte, enclosed by ascii dollar signs.
 	 */
-	if ((dec_salt_binary[0] != '$')
-		&& (dec_salt_binary[2] != '$'))
-	{
+	if ((dec_salt_binary[0] != '$') || (dec_salt_binary[2] != '$'))
 		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("invalid format of salt")),
+				errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				errmsg("invalid format of salt"),
 				errhint("magic byte format for shacrypt is either \"$5$\" or \"$6$\""));
-	}
 
 	/*
 	 * Check magic byte for supported shacrypt digest.
@@ -190,62 +178,54 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	 * string, we don't treat this special: This is just absorbed as part of
 	 * the salt with up to PX_SHACRYPT_SALT_LEN_MAX.
 	 *
-	 * Unknown magic byte is handled later below
+	 * Unknown magic byte is handled further below.
 	 */
 	if (strncmp(dec_salt_binary,
 				rounds_prefix, sizeof(rounds_prefix) - 1) == 0)
 	{
-
 		const char *num = dec_salt_binary + sizeof(rounds_prefix) - 1;
 		char	   *endp;
-		long		srounds = strtoul(num, &endp, 10);
+		int			srounds = strtoint(num, &endp, 10);
 
-		if (*endp == '$')
-		{
-			dec_salt_binary = endp + 1;
-
-			/*
-			 * We violate supported lower or upper bound of rounds, but in
-			 * this case we change this value to the supported lower or upper
-			 * value. We don't do this silently and print a NOTICE in such a
-			 * case.
-			 *
-			 * Note that a salt string generated with gen_salt() would never
-			 * generated such a salt string, since it would error out.
-			 *
-			 * But Drepper's upstream reference implementation supports this
-			 * when passing the salt string directly, so we maintain
-			 * compatibility here.
-			 */
-			if (srounds > PX_SHACRYPT_ROUNDS_MAX)
-			{
-				ereport(NOTICE,
-						errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-						errmsg("rounds=%ld exceeds maximum supported value (%ld), using %ld instead",
-							   srounds, PX_SHACRYPT_ROUNDS_MAX,
-							   PX_SHACRYPT_ROUNDS_MAX));
-				srounds = PX_SHACRYPT_ROUNDS_MAX;
-			}
-			else if (srounds < PX_SHACRYPT_ROUNDS_MIN)
-			{
-				ereport(NOTICE,
-						errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-						errmsg("rounds=%ld is below supported value (%ld), using %ld instead",
-							   srounds, PX_SHACRYPT_ROUNDS_MIN,
-							   PX_SHACRYPT_ROUNDS_MIN));
-				srounds = PX_SHACRYPT_ROUNDS_MIN;
-			}
-
-			rounds = (unsigned long) srounds;
-			rounds_custom = 1;
-		}
-		else
-		{
+		if (*endp != '$')
 			ereport(ERROR,
 					errcode(ERRCODE_SYNTAX_ERROR),
 					errmsg("could not parse salt options"));
+
+		dec_salt_binary = endp + 1;
+
+		/*
+		 * We violate supported lower or upper bound of rounds, but in this
+		 * case we change this value to the supported lower or upper value. We
+		 * don't do this silently and print a NOTICE in such a case.
+		 *
+		 * Note that a salt string generated with gen_salt() would never
+		 * generated such a salt string, since it would error out.
+		 *
+		 * But Drepper's upstream reference implementation supports this when
+		 * passing the salt string directly, so we maintain compatibility.
+		 */
+		if (srounds > PX_SHACRYPT_ROUNDS_MAX)
+		{
+			ereport(NOTICE,
+					errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+					errmsg("rounds=%d exceeds maximum supported value (%d), using %d instead",
+						   srounds, PX_SHACRYPT_ROUNDS_MAX,
+						   PX_SHACRYPT_ROUNDS_MAX));
+			srounds = PX_SHACRYPT_ROUNDS_MAX;
+		}
+		else if (srounds < PX_SHACRYPT_ROUNDS_MIN)
+		{
+			ereport(NOTICE,
+					errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+					errmsg("rounds=%d is below supported value (%d), using %d instead",
+						   srounds, PX_SHACRYPT_ROUNDS_MIN,
+						   PX_SHACRYPT_ROUNDS_MIN));
+			srounds = PX_SHACRYPT_ROUNDS_MIN;
 		}
 
+		rounds = (uint32) srounds;
+		rounds_custom = 1;
 	}
 
 	/*
@@ -294,9 +274,7 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	}
 
 	if (rounds_custom > 0)
-	{
-		appendStringInfo(out_buf, "rounds=%lu$", rounds);
-	}
+		appendStringInfo(out_buf, "rounds=%u$", rounds);
 
 	/*
 	 * We need the real decoded salt string from salt input, this is every
@@ -326,47 +304,34 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		 * this point.
 		 */
 		fp = strstr(dec_salt_binary, magic_bytes[0]);
-
 		if (fp != NULL)
-		{
 			elog(ERROR, "bogus magic byte found in salt string");
-		}
 
 		fp = strstr(dec_salt_binary, magic_bytes[1]);
-
 		if (fp != NULL)
-		{
 			elog(ERROR, "bogus magic byte found in salt string");
-		}
 
 		/*
 		 * This looks very strict, but we assume the caller did something
 		 * wrong when we see a "rounds=" option here.
 		 */
 		fp = strstr(dec_salt_binary, rounds_prefix);
-
 		if (fp != NULL)
-		{
 			elog(ERROR, "invalid rounds option specified in salt string");
-		}
 
 		if (*ep != '$')
 		{
 			if (isalpha(*ep) || isdigit(*ep) || (*ep == '.') || (*ep == '/'))
-			{
 				appendStringInfoCharMacro(decoded_salt, *ep);
-			}
 			else
-			{
 				elog(ERROR, "invalid character in salt string: \"%c\"", *ep);
-			}
 		}
 		else
 		{
 			/*
 			 * We encountered a '$' marker. Check if we already absorbed some
 			 * bytes from input. If true, we are optimistic and terminate at
-			 * this stage. Of not, we try further.
+			 * this stage. If not, we try further.
 			 *
 			 * If we already consumed enough bytes for the salt string,
 			 * everything that is after this marker is considered to be part
@@ -375,23 +340,19 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 			if (decoded_salt->len > 0)
 				break;
 		}
-
 	}
 
 	salt_len = decoded_salt->len;
 	appendStringInfoString(out_buf, decoded_salt->data);
-	elog(DEBUG1, "using salt \"%s\", salt len = %d, rounds = %lu",
+	elog(DEBUG1, "using salt \"%s\", salt len = %d, rounds = %u",
 		 decoded_salt->data, decoded_salt->len, rounds);
 
 	/*
-	 * Sanity check:
-	 *
-	 * At this point the salt string buffer must not exceed expected size
+	 * Sanity check: at this point the salt string buffer must not exceed
+	 * expected size.
 	 */
 	if (out_buf->len > (3 + 17 * rounds_custom + salt_len))
-	{
 		elog(ERROR, "unexpected length of salt string");
-	}
 
 	/*-
 	 * 1. Start digest A
@@ -413,29 +374,26 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	px_md_update(digestB, (const unsigned char *) pw, len);
 	px_md_finish(digestB, sha_buf);
 
-	/*-
-	 * 9. For each block of (excluding the NULL byte), add
-	 *    digest B to digest A.
+	/*
+	 * 9. For each block (excluding the NULL byte), add digest B to digest A.
 	 */
 	for (block = len; block > buf_size; block -= buf_size)
-	{
 		px_md_update(digestA, sha_buf, buf_size);
-	}
 
 	/*-
-	 * 10 For the remaining N bytes of the password string, add
-	 * the first N bytes of digest B to A */
+	 * 10. For the remaining N bytes of the password string, add the first N
+	 * bytes of digest B to A.
+	 */
 	px_md_update(digestA, sha_buf, block);
 
 	/*-
-	 * 11 For each bit of the binary representation of the length of the
-	 * password string up to and including the highest 1-digit, starting
-	 * from to lowest bit position (numeric value 1)
+	 * 11. For each bit of the binary representation of the length of the
+	 * password string up to and including the highest 1-digit, starting from
+	 * to lowest bit position (numeric value 1)
 	 *
 	 * a) for a 1-digit add digest B (sha_buf) to digest A
 	 * b) for a 0-digit add the password string
 	 */
-
 	block = len;
 	while (block)
 	{
@@ -447,10 +405,10 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		block >>= 1;
 	}
 
-	/* 12 Finalize digest A */
+	/* 12. Finalize digest A */
 	px_md_finish(digestA, sha_buf);
 
-	/* 13 Start digest DP */
+	/* 13. Start digest DP */
 	px_md_reset(digestB);
 
 	/*-
@@ -458,20 +416,17 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	 * to the digest DP
 	 */
 	for (block = len; block > 0; block--)
-	{
 		px_md_update(digestB, (const unsigned char *) pw, len);
-	}
 
-	/* 15 Finalize digest DP */
+	/* 15. Finalize digest DP */
 	px_md_finish(digestB, sha_buf_tmp);
 
 	/*-
-	 * 16 produce byte sequence P with same length as password.
-	 *
+	 * 16. produce byte sequence P with same length as password.
 	 *     a) for each block of 32 or 64 bytes of length of the password
 	 *        string the entire digest DP is used
 	 *     b) for the remaining N (up to  31 or 63) bytes use the
-	 *         first N bytes of digest DP
+	 *        first N bytes of digest DP
 	 */
 	if ((p_bytes = palloc0(len)) == NULL)
 	{
@@ -484,30 +439,28 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	memcpy(cp, sha_buf_tmp, block);
 
 	/*
-	 * 17 Start digest DS
+	 * 17. Start digest DS
 	 */
 	px_md_reset(digestB);
 
 	/*-
-	 * 18 Repeat the following 16+A[0] times, where A[0] represents the first
+	 * 18. Repeat the following 16+A[0] times, where A[0] represents the first
 	 *    byte in digest A interpreted as an 8-bit unsigned value
 	 *    add the salt to digest DS
 	 */
 	for (block = 16 + sha_buf[0]; block > 0; block--)
-	{
 		px_md_update(digestB, (const unsigned char *) dec_salt_binary, salt_len);
-	}
 
 	/*
-	 * 19 Finalize digest DS
+	 * 19. Finalize digest DS
 	 */
 	px_md_finish(digestB, sha_buf_tmp);
 
 	/*-
-	 * 20 Produce byte sequence S of the same length as the salt string where
+	 * 20. Produce byte sequence S of the same length as the salt string where
 	 *
 	 * a) for each block of 32 or 64 bytes of length of the salt string the
-	 *     entire digest DS is used
+	 *    entire digest DS is used
 	 *
 	 * b) for the remaining N (up to  31 or 63) bytes use the first N
 	 *    bytes of digest DS
@@ -516,27 +469,24 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		goto error;
 
 	for (cp = s_bytes, block = salt_len; block > buf_size; block -= buf_size, cp += buf_size)
-	{
 		memcpy(cp, sha_buf_tmp, buf_size);
-	}
 	memcpy(cp, sha_buf_tmp, block);
 
 	/* Make sure we don't leave something important behind */
 	px_memset(&sha_buf_tmp, 0, sizeof sha_buf);
 
 	/*-
-	 * 21 Repeat a loop according to the number specified in the rounds=<N>
-	 *    specification in the salt (or the default value if none is
-	 *    present).  Each round is numbered, starting with 0 and up to N-1.
+	 * 21. Repeat a loop according to the number specified in the rounds=<N>
+	 *     specification in the salt (or the default value if none is
+	 *     present).  Each round is numbered, starting with 0 and up to N-1.
 	 *
-	 *    The loop uses a digest as input.  In the first round it is the
-	 *    digest produced in step 12.  In the latter steps it is the digest
-	 *    produced in step 21.h of the previous round.  The following text
-	 *    uses the notation "digest A/B" to describe this behavior.
+	 *     The loop uses a digest as input.  In the first round it is the
+	 *     digest produced in step 12.  In the latter steps it is the digest
+	 *     produced in step 21.h of the previous round.  The following text
+	 *     uses the notation "digest A/B" to describe this behavior.
 	 */
 	for (block = 0; block < rounds; block++)
 	{
-
 		/*
 		 * Make it possible to abort in case large values for "rounds" are
 		 * specified.
@@ -546,29 +496,25 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		/* a) start digest B */
 		px_md_reset(digestB);
 
-		/*
-		 * b) for odd round numbers add the byte sequense P to digest B c) for
-		 * even round numbers add digest A/B
+		/*-
+		 * b) for odd round numbers add the byte sequence P to digest B
+		 * c) for even round numbers add digest A/B
 		 */
 		px_md_update(digestB,
 					 (block & 1) ? (const unsigned char *) p_bytes : sha_buf,
 					 (block & 1) ? len : buf_size);
 
 		/* d) for all round numbers not divisible by 3 add the byte sequence S */
-		if (block % 3)
-		{
+		if ((block % 3) != 0)
 			px_md_update(digestB, (const unsigned char *) s_bytes, salt_len);
-		}
 
 		/* e) for all round numbers not divisible by 7 add the byte sequence P */
-		if (block % 7)
-		{
+		if ((block % 7) != 0)
 			px_md_update(digestB, (const unsigned char *) p_bytes, len);
-		}
 
-		/*
-		 * f) for odd round numbers add digest A/C g) for even round numbers
-		 * add the byte sequence P
+		/*-
+		 * f) for odd round numbers add digest A/C
+		 * g) for even round numbers add the byte sequence P
 		 */
 		px_md_update(digestB,
 					 (block & 1) ? sha_buf : (const unsigned char *) p_bytes,
@@ -576,7 +522,6 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 
 		/* h) finish digest C. */
 		px_md_finish(digestB, sha_buf);
-
 	}
 
 	px_md_free(digestA);
@@ -594,15 +539,15 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 	/* prepare final result buffer */
 	appendStringInfoCharMacro(out_buf, '$');
 
-#define b64_from_24bit(B2, B1, B0, N)                      \
-	do {                                                    \
-		unsigned int w = ((B2) << 16) | ((B1) << 8) | (B0); \
-		int i = (N);                                        \
-		while (i-- > 0)                                     \
-		{                                                   \
+#define b64_from_24bit(B2, B1, B0, N)                                    \
+	do {                                                                 \
+		unsigned int w = ((B2) << 16) | ((B1) << 8) | (B0);              \
+		int		i = (N);                                                 \
+		while (i-- > 0)                                                  \
+		{                                                                \
 			appendStringInfoCharMacro(out_buf, _crypt_itoa64[w & 0x3f]); \
-			w >>= 6;                                        \
-		}                                                   \
+			w >>= 6;                                                     \
+		}                                                                \
 	} while (0)
 
 	switch (type)
@@ -655,7 +600,6 @@ px_crypt_shacrypt(const char *pw, const char *salt, char *passwd, unsigned dstle
 		case PGCRYPTO_SHA_UNKOWN:
 			/* we shouldn't land here ... */
 			elog(ERROR, "unsupported digest length");
-
 	}
 
 	*cp = '\0';
@@ -690,7 +634,7 @@ error:
 		destroyStringInfo(out_buf);
 
 	ereport(ERROR,
-			(errcode(ERRCODE_INTERNAL_ERROR),
-			 errmsg("cannot create encrypted password")));
+			errcode(ERRCODE_INTERNAL_ERROR),
+			errmsg("cannot create encrypted password"));
 	return NULL;				/* keep compiler quiet */
 }
diff --git a/contrib/pgcrypto/px-crypt.c b/contrib/pgcrypto/px-crypt.c
index a786277aeac..d7729eec9bc 100644
--- a/contrib/pgcrypto/px-crypt.c
+++ b/contrib/pgcrypto/px-crypt.c
@@ -139,10 +139,16 @@ static struct generator gen_list[] = {
 	{"md5", _crypt_gensalt_md5_rn, 6, 0, 0, 0},
 	{"xdes", _crypt_gensalt_extended_rn, 3, PX_XDES_ROUNDS, 1, 0xFFFFFF},
 	{"bf", _crypt_gensalt_blowfish_rn, 16, PX_BF_ROUNDS, 4, 31},
-	{"sha256crypt", _crypt_gensalt_sha256_rn, PX_SHACRYPT_SALT_MAX_LEN,
-	PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
-	{"sha512crypt", _crypt_gensalt_sha512_rn, PX_SHACRYPT_SALT_MAX_LEN,
-	PX_SHACRYPT_ROUNDS_DEFAULT, PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX},
+	{
+		"sha256crypt", _crypt_gensalt_sha256_rn,
+		PX_SHACRYPT_SALT_MAX_LEN, PX_SHACRYPT_ROUNDS_DEFAULT,
+		PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX
+	},
+	{
+		"sha512crypt", _crypt_gensalt_sha512_rn,
+		PX_SHACRYPT_SALT_MAX_LEN, PX_SHACRYPT_ROUNDS_DEFAULT,
+		PX_SHACRYPT_ROUNDS_MIN, PX_SHACRYPT_ROUNDS_MAX
+	},
 	{NULL, NULL, 0, 0, 0, 0}
 };
 
diff --git a/contrib/pgcrypto/px-crypt.h b/contrib/pgcrypto/px-crypt.h
index 3829ec6c347..bb03cf5592a 100644
--- a/contrib/pgcrypto/px-crypt.h
+++ b/contrib/pgcrypto/px-crypt.h
@@ -61,13 +61,13 @@
 #define PX_SHACRYPT_BUF_LEN (PX_SHACRYPT_SALT_BUF_LEN + 86 + 1)
 
 /* Default number of rounds of shacrypt if not explicitly specified.  */
-#define PX_SHACRYPT_ROUNDS_DEFAULT 5000l
+#define PX_SHACRYPT_ROUNDS_DEFAULT 5000
 
 /* Minimum number of rounds of shacrypt.  */
-#define PX_SHACRYPT_ROUNDS_MIN 1000l
+#define PX_SHACRYPT_ROUNDS_MIN 1000
 
 /* Maximum number of rounds of shacrypt.  */
-#define PX_SHACRYPT_ROUNDS_MAX 999999999l
+#define PX_SHACRYPT_ROUNDS_MAX 999999999
 
 /*
  * main interface
-- 
2.39.5

---

From: Bernd Helmle <[email-masked]>
Date: Sat, 05 Apr 2025 12:45:02 +0200

> a warning), but I'd rather not have it in the first cut.

That's a good idea, Let's go with that. Thanks again for working on
this.

> I am ready to believe that I misinterpreted what I read.

I hope i didn't parse it wrong either. But i didn't see forcing
something like this according in either passlib and Drepper's code.
Maybe we need have to look closer again ...

[...]

> limits and default, which were bugging me a bit when used in the

+1

-- 
Bernd Helmle

Blücherstrasse 17
41061 Mönchengladbach

Tel.: +49 172 726 99 66