On 11/29/22 8:12 PM, Michael Paquier wrote:
> On Tue, Nov 29, 2022 at 09:32:34PM +0100, Daniel Gustafsson wrote:
>> On the whole I tend to agree with Jacob upthread, while this does provide
>> consistency it doesn't seem to move the needle for best practices. Allowing
>> scram_build_secret_sha256('password', 'a', 1); with the password potentially
>> going in cleartext over the wire and into the logs doesn't seem like a great
>> tradeoff for the (IMHO) niche usecases it would satisfy.
>
> Should we try to make \password and libpq more flexible instead? Two
> things got discussed in this area since v10:
> - The length of the random salt.
> - The iteration number.
>
> Or we could bump up the defaults, and come back to that in a few
> years, again.. ;p
Here is another attempt at this patch that takes into account the SCRAM
code refactor. I addressed some of Daniel's previous feedback, but will
need to make another pass on the docs and the assert trace as the main
focus of this revision was bringing the code inline with the recent changes.
This patch changes the function name to "scram_build_secret" and now
accepts a new parameter of hash type. This sets it up to handle
additional hashes in the future.
I do agree we should make libpq more flexible, but as mentioned in the
original thread, this does not solve the *server side* cases where a
user needs to build a SCRAM secret. For example, being able to
precompute hashes on one server before sending them to another server,
which can require no plaintext passwords if the server is randomly
generating the data.
Another use case comes from the "pg_tle" project, specifically with the
ability to write a "check_password_hook" from an available PL[1]. If a
user does follow our best practices and sends a pre-built SCRAM secret
over the wire, a hook can then verify that the secret is not contained
within a common password dictionary.
Thanks,
Jonathan
[1] https://github.com/aws/pg_tle/blob/main/docs/04_hooks.md