Re: User functions for building SCRAM secrets

On Fri, Mar 24, 2023 at 4:48 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
> > On 22 Mar 2023, at 15:06, Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
> > On 3/22/23 2:48 AM, Michael Paquier wrote:
>
> >> I was reading this thread again, and pondered on this particular
> >> point:
> >> https://www.postgresql.org/message-id/CAAWbhmhjcFc4oaGA_7YLUhtj6J+rxEY+BoDryGzNdaFLGfZZMg@mail.gmail.com
> >> We've had our share of complains over the years that Postgres logs
> >> password data in the logs with various DDLs, so I'd tend to agree that
> >> this is not a practice we should try to encourage more. The
> >> parameterization of the SCRAM verifiers through GUCs (like Daniel's
> >> https://commitfest.postgresql.org/42/4201/ for the iteration number)
> >> is more promising because it is possible to not have to send the
> >> password over the wire with once we let libpq take care of the
> >> computation, and the server would not know about that
> >
> > I generally agree with not allowing password data to be in logs, but in practice, there are a variety of tools and extensions that obfuscate or remove passwords from PostgreSQL logs. Additionally, this function is not targeted for SQL statements directly, but stored procedures.
> >
> > For example, an extension like "pg_tle" exposes the ability for someone to write a "check_password_hook" directly from PL/pgSQL[1] (and other languages). As we've made it a best practice to pre-hash the password on the client-side, a user who wants to write a check password hook against a SCRAM verifier needs to be able to compare the verifier against some existing set of plaintext criteria, and has to write their own function to do it. I have heard several users who have asked to do this, and the only feedback I can give them is "implement your own SCRAM build secret function."
>
> I'm not sure I follow; doesn't this - coupled with this patch - imply passing
> the plaintext password from client to the server, hashing it with a known salt
> and comparing this with something in plaintext hashed with the same known salt?
> If so, that's admittedly not a usecase I am terribly excited about. My
> preference is to bring password checks to the plaintext password, not bring the
> plaintext password to the password check.

Given how much we marketed, for good reasons, SCRAM as a way to
finally make postgres *not* do this, it seems like a really strange
path to take to go back to doing it again.

Having the function always generate a random salt seems more
reasonable though, and would perhaps be something that helps in some
of the cases? It won't help with the password policy one, as it's too
secure for that, but it would help with the postgres-is-the-client
one?

> > And, if my PostgreSQL server _is_ the client, e.g. it's making a dblink call to another PostgreSQL server, the only way it can modify a password is by sending the plaintext credential over the wire.
>
> My experience with dblink setups is too small to have much insight here, but I
> (perhaps naively) assumed that dblink setups generally involved two servers
> under the same control making such changes be possible out of band.

I have seen a few, and certainly more FDW based ones these days. But
I'm not sure I've come across one yet where one wants to *change the
password* through dblink. Since it's server-to-server, most people
would just change the password on the target server and then update
the FDW/dblink configuration with the new password. (Of course, the
storage of that password on the FDW/dblink layer is a terrible thing
in the first place from a security perspective, but it's what we
have).

> > Maybe I'm not conveying the problem this is solving -- I'm happy to go one more round trying to make it clearer -- but if this is not clear, it'd be good to at develop an alternative approach to this before withdrawing the patch.
>
> If this is mainly targeting extension use, is there a way in which an extension
> could have all this functionality with no: a) not exposing any of it from the
> server; b) not having to copy/paste lots into the extension and; c) have a
> reasonable way to keep up with any changes made in the backend?

I realize I forgot to write this reply back when the thread was alive,
so here's a zombie-awakener!

One way to accomplish this would be to create a new predefined role,
say pg_change_own_password, by default granted to public. When a user
is a member of this role they can, well, change their own password.
And it will be done using the full security of scram, without cutting
anything. For those that want to enforce a password policy or anything
else that requires the server to see the cleartext or
cleartext-equivalent of the password can then revoke this role from
public, and force password changes to go through a security definer
funciton, like SELECT pg_change_password_with_policy('joe',
'mysupersecretpasswrod').

This function can then be under the control of an extension or
whatever you want. If one had the client under control one could for
example do the policy validation on the client and pass it to the
backend with some internal hash as well -- this would be entirely
under the control of the application though, as a generic libpq
connection could and should not be considered "client under control"
in this case.

--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/