public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Daniel Gustafsson <[email protected]>
Cc: Jonathan S. Katz <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Raising the SCRAM iteration count
Date: Sat, 17 Dec 2022 12:27:30 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
On Thu, Dec 15, 2022 at 12:09:15PM +0100, Daniel Gustafsson wrote:
>> On 15 Dec 2022, at 00:52, Michael Paquier <[email protected]> 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?
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.
>> @@ -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 :) ?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (18+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Raising the SCRAM iteration count
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox