Re: Refactor SCRAM code to dynamically handle hash type and key length

On Tue, Dec 20, 2022 at 08:58:38AM +0900, Michael Paquier wrote:
> Thanks! I have applied for I have here.. There are other pieces to
> think about in this area.

FYI, I have spent a few hours looking at the remaining parts of the
SCRAM code that could be simplified if a new hash method is added, and
this b3bb7d1 has really made things easier. There are a few things
that will need more thoughts. Here are my notes, assuming that
SHA-512 is done:
1) HBA entries had better use a new keyword for scram-sha-512, implying
a new uaSCRAM512 to combine with the existing uaSCRAM. One reason
behind that it to advertise the mechanisms supported back to the
client depending on the matching HBA entry.
2) If a role has a SCRAM-SHA-256 password and the HBA entry matches
scram-sha-512, the SASL exchange needs to go through the mock process
with SHA-512 and fail.
3) If a role has a SCRAM-SHA-512 password and the HBA entry matches
scram-sha-256, the SASL exchange needs to go through the mock process
with SHA-256 and fail.
4) The case of MD5 is something that looks a bit tricky at quick
glance. We know that if the role has a MD5 password stored, we will
fail anyway. So we could just advertise the SHA-256 mechanisms in
this case and map the mock to that?
5) The mechanism choice in libpq needs to be reworked a bit based on
what the backend sends. There may be no point in advertising all the
SHA-256 and SHA-512 mechanisms at the same time, I guess.

Attached is a WIP patch that I have played with. This shows the parts
of the code that would need more thoughts if implementing such
things. This works for the cases 1~3 (see the TAP tests). I've given
up on the MD5 case 4 for now, but perhaps I just missed a simple trick.
5 in libpq uses dirty tricks. I have marked this CF entry as
committed, and I'll come back to each relevant part on new separate
threads.
--
Michael