Feedback-ID: i0fe9450f:Fastmail
Date: Sat, 17 Dec 2022 12:27:30 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: "Jonathan S. Katz" <jkatz@postgresql.org>,
	Andres Freund <andres@anarazel.de>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Raising the SCRAM iteration count
Message-ID: <Y503IsOdgSrUP6Se@paquier.xyz>
References: <F72E7BC7-189F-4B17-BF47-9735EB72C364@yesql.se>
 <20221210001538.czikyvk3xtkphxfu@awork3.anarazel.de>
 <4880738a-cbf7-c9a1-4faf-ba861c731c63@postgresql.org>
 <75619A47-4CD0-4E0F-8A30-32F83FF593DD@yesql.se>
 <Y5kgHT43XqDEc3Ev@paquier.xyz>
 <FDBA5DA0-1015-4162-A8F5-E7BC1F620837@yesql.se>
 <869f3aee-5c46-998d-be42-b3b232415bef@postgresql.org>
 <Y5phwMQOMCZAAZYK@paquier.xyz>
 <1D275A6B-EAE5-4C5E-B223-32C28AB79E36@yesql.se>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="zuSA3iHrWub3mXD+"
Content-Disposition: inline
In-Reply-To: <1D275A6B-EAE5-4C5E-B223-32C28AB79E36@yesql.se>
Archived-At: 
 <https://www.postgresql.org/message-id/Y503IsOdgSrUP6Se%40paquier.xyz>
Precedence: bulk


--zuSA3iHrWub3mXD+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Dec 15, 2022 at 12:09:15PM +0100, Daniel Gustafsson wrote:
>> On 15 Dec 2022, at 00:52, Michael Paquier <michael@paquier.xyz> wrote:
>>    conn->in_hot_standby =3D PG_BOOL_UNKNOWN;
>> +   conn->scram_iterations =3D SCRAM_DEFAULT_ITERATIONS;
>>=20
>> s/SCRAM_DEFAULT_ITERATIONS/SCRAM_SHA_256_DEFAULT_ITERATIONS/ and
>> s/scram_iterations/scram_sha_256_interations/ perhaps? =20
>=20
> 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 *iterati=
ons, char **salt,
>>    encoded_salt[encoded_len] =3D '\0';
>>=20
>>    *salt =3D encoded_salt;
>> -   *iterations =3D SCRAM_DEFAULT_ITERATIONS;
>> +   *iterations =3D scram_sha256_iterations;
>>=20
>> 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.
>=20
> For avoiding revealing anything, I think a case can be argued for both.  =
I've
> reverted back to the default though.
>=20
> I also renamed the GUC sha_256 to match terminology we use.

+   "SET password_encryption=3D'scram-sha-256';
+    SET scram_sha_256_iterations=3D100000;
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

--zuSA3iHrWub3mXD+
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmOdNyIACgkQnvQgOdby
QH0vpw/9EjbqKLHK9jzLBEP8Hb2R5sn+nRBGWnMywxfTgTFwWMl8sYNqaWeKq+br
WSg8OPJTEmHwaKO9RREdBSHjEk61havnrsZo3dksX+DxEk9lNBPev5UbscXp3aWa
25NQp+vac+RC3bRNGxJI8V6AKz8NKohT6DF0dIprb7AaLRrCj0tHX0DQW9Z1p/N5
hlJ608Fp5rY+sgmTVzkz4eeRl7ygFanfnHmAQssm4BDO+SpF/5K1ToLtM75DVRBp
864h9gXe6fPWjnl47eP2jUud5gjIS3zzweY2rXx5Yqa/svRkHUE60FNe6PNeeXgz
nEVn/7OHB4qjaKHl27L9dzmgcQvU+5D+nk/7/phQTKoV2WMNvH0k6VQc6I21g0T7
zVvcGK+Er8wDiVLMKkX7RDzH/Bqp7oVwz03+yWCKNbfZbFsXcCHZG+9rrBhOkMZC
UZ1/Qa/Oe140RCMwOKdDsDb5KIafPG43dUDW2hT4O21PlJs3rWlTmSA+ugzUsexb
VbddWzoOvtP1H+UZeNA7I0Rc0diSPnUoSMBK2mIM35CVQ8uyh4XT6ykUXFHgCGzs
4nPLEb9TZW31DsxkGmTSQw7dlOCW9Oif1UB4KDD8I/k8GulevCL+OYzmEWeLdnbO
aeKgUjezvg3G3rVjq7x6f18jk9srJ/H+mmuINGsupZWHvKBlVtg=
=Sb3g
-----END PGP SIGNATURE-----

--zuSA3iHrWub3mXD+--