On Thu, Dec 8, 2016 at 10:05 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Thu, Dec 8, 2016 at 5:55 PM, Heikki Linnakangas <hlinn...@iki.fi> wrote: >> On 12/08/2016 10:18 AM, Michael Paquier wrote: >>> Hmmm. How do we handle the case where the user name does not match >>> then? The spec gives an error message e= specifically for this case. >> >> Hmm, interesting. I wonder how/when they imagine that error message to be >> used. I suppose you could send a dummy server-first message, with a made-up >> salt and iteration count, if the user is not found, so that you can report >> that in the server-final message. But that seems unnecessarily complicated, >> compared to just sending the error immediately. I could imagine using a >> dummy server-first message to hide whether the user exists, but that >> argument doesn't hold water if you're going to report an "unknown-user" >> error, anyway. > > Using directly an error message would map with MD5 and plain, but > that's definitely a new protocol piece so I'd rather think that using > e= once the client has sent its first message in the exchange should > be answered with an appropriate SASL error... > >> Actually, we don't give away that information currently. If you try to log >> in with password or MD5 authentication, and the user doesn't exist, you get >> the same error as with an incorrect password. So, I think we do need to give >> the client a made-up salt and iteration count in that case, to hide the fact >> that the user doesn't exist. Furthermore, you can't just generate random >> salt and iteration count, because then you could simply try connecting >> twice, and see if you get the same salt and iteration count. We need to >> deterministically derive the salt from the username, so that you get the >> same salt/iteration count every time you try connecting with that username. >> But it needs indistinguishable from a random salt, to the client. Perhaps a >> SHA hash of the username and some per-cluster secret value, created by >> initdb. There must be research papers out there on how to do this.. > > A simple idea would be to use the system ID when generating this fake > salt? That's generated by initdb, once per cluster. I am wondering if > it would be risky to use it for the salt. For the number of iterations > the default number could be used.
I have been thinking more about this part quite a bit, and here is the most simple thing that we could do while respecting the protocol. That's more or less what I think you have in mind by re-reading upthread, but it does not hurt to rewrite the whole flow to be clear: 1) Server gets the startup packet, maps pg_hba.conf and moves on to the scram authentication code path. 2) Server sends back sendAuthRequest() to request user to provide a password. This maps to the plain/md5 behavior as no errors would be issued to user until he has provided a password. 3) Client sends back the password, and the first message with the user name. 4) Server receives it, and checks the data. If a failure happens at this stage, just ERROR on PG-side without sending back a e= message. This includes the username-mismatch, empty password and end of password validity. So we would never use e=unknown-user. This sticks with what you quoted upthread that the server may end the exchange before sending the final message. 5) Server sends back the challenge, and client answers back with its reply to it. Then enters the final stage of the exchange, at which point the server would issue its final message that would be e= in case of errors. If something like an OOM happens, no message would be sent so failing on an OOM ERROR on PG side would be fine as well. 6) Read final message from client and validate. 7) issue final message of server. On failure at steps 6) or 7), an e= message is returned instead of the final message. Does that look right? One thing is: when do we look up at pg_authid? After receiving the first message from client or before beginning the exchange? As the first message from client has the user name, it would make sense to do the lookup after receiving it, but from PG prospective it would just make sense to use the data already present in the startup packet. The current patch does the latter. What do you think? By the way, I have pushed the extra patches you sent into this branch: https://github.com/michaelpq/postgres/tree/scram -- Michael -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers