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?
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.
[1]
https://www.postgresql.org/docs/current/libpq-misc.html#LIBPQ-PQENCRYPTPASSWORDCONN
We went a step further and added an intermediate function that
generates the "ALTER USER ... PASSWORD" SQL.
I don't think we want that in libpq, but in any case separate
patch/discussion IMHO.
3. We only added an "ALTER USER ... PASSWORD" function, not anything to
create a user. There's way too many options for that and keeping this
targeted at just assigning passwords makes it much easier to test.
+1
Also separate patch/discussion, but I don't think the CREATE version is
needed.
4. RE:defaults, the PGJDBC approach is that the encoding-only function
uses the driver's default (SCRAM). The "alterUserPassword(...)" uses the
server's default (again usually SCRAM for modern installs but could be
something else). It's kind of odd that they're different but the use
cases are different as well.
Since PQencryptPasswordConn() already exists, and psql's "\password"
used it with its defaults, I don't think we want to change the behavior.
The patch as written behaves in a backward compatible way.
5. Our SCRAM specific function allows for customizing the algo iteration
and salt parameters. That topic came up on hackers previously[1]. Our
high level "alterUserPassword(...)" function does not have those options
but it is available as part of our PasswordUtil SCRAM API for advanced
users who want to leverage it. The higher level functions have defaults
for iteration counts (4096) and salt size (16-bytes).
Again separate patch/discussion, IMHO.
6. Getting the verbiage right for the PGJDBC version was kind of
annoying as we wanted to match the server's wording, e.g.
"password_encryption", but it's clearly hashing, not encryption. We
settled on "password encoding" for describing the overall task with the
comments referencing the server's usage of the term
"password_encryption". Found a similar topic[2] on changing that
recently as well but looks like that's not going anywhere.
Really this is irrelevant to this discussion, because the new function
is called PQchangePassword().
The function PQencryptPasswordConn() has been around for a while and the
horse is out of the gate on that one.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com