Re: Password leakage avoidance

On Sat, Jan 6, 2024 at 11:53 AM Joe Conway <mail(at)joeconway(dot)com> wrote:

> On 1/2/24 07:23, Sehrope Sarkuni wrote:
> > 1. There's two sets of defaults, the client program's default and the
> > server's default. Need to pick one for each implemented function. They
> > don't need to be the same across the board.
>
> Is there a concrete recommendation here?
>

Between writing that and now, we scrapped the "driver picks" variant of
this. Now we only have explicit functions for each of the encoding types
(i.e. one for md5 and one for SCRAM-SHA-256) and an alterUserPassword(...)
method that uses the default for the database via reading the
password_encrypotion GUC. We also have some javadoc comments on the
encoding functions to strongly suggest using the SCRAM functions and only
use the md5 directly for legacy servers.

The "driver picks" one was removed to prevent a situation where an end user
picks the driver default and it's not compatible with their server. The
rationale was if the driver's SCRAM-SHA-256 default is ever replaced with
something else (e.g. SCRAM-SHA-512) we'd end up with an interim state where
an upgraded driver application would try to use that newer encryption
method on an old DB. If a user is going to do that, they would have to be
explicit with their choice of encoding functions (hence removing the
"driver picks" variant).

So the recommendation is to have explicit functions for each variant and
have the end-to-end change password code read from the DB.

My understanding of this patch is that it does exactly that.

> > 2. Password encoding should be split out and made available as its own
> > functions. Not just as part of a wider "create _or_ alter a user's
> > password" function attached to a connection.
>
> It already is split out in libpq[1], unless I don't understand you
> correctly.
>

Sorry for the confusion. My original list wasn't any specific contrasts
with what libpq is doing. Was more of a summary of thoughts having just
concluded implementing the same type of password change stuff in PGJDBC.

From what I've seen in this patch, it either aligns with how we did things
in PGJDBC or it's something that isn't as relevant in this context (e.g.
generating the SQL text as a public function).

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/