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: 2022-12-15 11:09:15
Message-ID: 1D275A6B-EAE5-4C5E-B223-32C28AB79E36@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> 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_DEFAULT_ITERATIONS/SCRAM_SHA_256_DEFAULT_ITERATIONS/ and
> 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?

> +#ifndef FRONTEND
> +/*
> + * Number of iterations when generating new secrets.
> + */
> +extern PGDLLIMPORT int scram_sha256_iterations;
> +#endif
>
> It looks like libpq/scram.h, which is backend-only, would be a better
> location.

Fixed.

> @@ -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.

--
Daniel Gustafsson https://vmware.com/

Attachment Content-Type Size
v3-0001-Make-SCRAM-iteration-count-configurable.patch application/octet-stream 15.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-12-15 11:10:44 Re: Add proper planner support for ORDER BY / DISTINCT aggregates
Previous Message Daniel Gustafsson 2022-12-15 11:09:01 Re: Raising the SCRAM iteration count