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:

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to