Re: Raising the SCRAM iteration count

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Raising the SCRAM iteration count
Date: 2023-02-22 13:39:23
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 17 Dec 2022, at 04:27, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> On Thu, Dec 15, 2022 at 12:09:15PM +0100, Daniel Gustafsson wrote:
>>> On 15 Dec 2022, at 00:52, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>>> conn->in_hot_standby = PG_BOOL_UNKNOWN;
>>> + conn->scram_iterations = SCRAM_DEFAULT_ITERATIONS;
>>> s/scram_iterations/scram_sha_256_interations/ perhaps?
>> Distinct members in the conn object is only of interest if there is a way for
>> the user to select a different password method in \password right? I can
>> rename it now but I think doing too much here is premature, awaiting work on
>> \password (should that materialize) seems reasonable no?
> You could do that already, somewhat indirectly, with
> password_encryption, assuming that it supports more than one mode
> whose password build is influenced by it. If you wish to keep it
> named this way, this is no big deal for me either way, so feel free to
> use what you think is best based on the state of HEAD. I think that
> I'd value more the consistency with the backend in terms of naming,
> though.

ok, renamed.

>>> @@ -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_sha256_iterations;
>>> This looks incorrect to me? The mock authentication is here to
>>> produce a realistic verifier, still it will fail. It seems to me that
>>> we'd better stick to the default in all the cases.
>> For avoiding revealing anything, I think a case can be argued for both. I've
>> reverted back to the default though.
>> I also renamed the GUC sha_256 to match terminology we use.
> + "SET password_encryption='scram-sha-256';
> + SET scram_sha_256_iterations=100000;
> Maybe use a lower value to keep the test cheap?


> + time of encryption. In order to make use of a changed value, new
> + password must be set.
> "A new password must be set".


> Superuser-only GUCs should be documented as such, or do you intend to
> make it user-settable like I suggested upthread :) ?

I don't really have strong feelings, so I reverted to being user-settable since
I can't really present a strong argument for superuser-only.

The attached is a rebase on top of master with no other additional hacking done
on top of the above review comments.

Daniel Gustafsson

Attachment Content-Type Size
v4-0001-Make-SCRAM-iteration-count-configurable.patch application/octet-stream 14.8 KB
unknown_filename text/plain 1 byte

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2023-02-22 13:40:41 Re: meson and in src/test/ssl/
Previous Message Peter Eisentraut 2023-02-22 13:35:34 Re: Seek for helper documents to implement WAL with an FDW