On 12/20/22 2:25 AM, Michael Paquier wrote:
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.
Great! Thanks for doing a quick "stress test" on this.
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.
This does seem like a challenge, particularly if we have to support multiple different SCRAM hashes.
Perhaps this can be done with an interface change in HBA. For example, we could rename the auth method from "scram-sha-256" to "scram" and support an option list of hashes (e.g. "hash=sha-512,sha-256"). We can then advertise the user-selected hashes as part of the handshake.
For backwards compatibility, we can take an auth method of "scram-sha-256" to mean "scram" + using a sha-256 hash. Similarly, if no hash map is defined, we can default to "scram-sha-256".
Anyway, I understand this point would require more discussion, but perhaps it is a way to simplify the amount of code we would need to write to support more hashes.
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.
*nods*
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?
Is this the case where we specify "md5" as the auth method but the user-password is stored in SCRAM?
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.
Yeah, I think a user may want to select which ones they want to use (e.g. they may not want to advertise SHA-256).
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.
Thanks for starting this. Jonathan
OpenPGP_signature
Description: OpenPGP digital signature
