On 12/08/2016 10:18 AM, Michael Paquier wrote:
On Thu, Dec 8, 2016 at 5:54 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
Attached those here, as add-on patches to your latest patch set.

Thanks for looking at it!

I'll continue reviewing, but a couple of things caught my eye that you may want
to jump on, in the meanwhile:

On error messages, the spec says:

o  e: This attribute specifies an error that occurred during
      authentication exchange.  It is sent by the server in its final
      message and can help diagnose the reason for the authentication
      exchange failure.  On failed authentication, the entire server-
      final-message is OPTIONAL; specifically, a server implementation
      MAY conclude the SASL exchange with a failure without sending the
      server-final-message.  This results in an application-level error
      response without an extra round-trip.  If the server-final-message
      is sent on authentication failure, then the "e" attribute MUST be

Note that it says that the server can send the error message with the e=
attribute, in the *final message*. It's not a valid response in the earlier
state, before sending server-first-message. I think we need to change the
INIT state handling in pg_be_scram_exchange() to not send e= messages to the
client. On an error at that state, it needs to just bail out without a
message. The spec allows that. We can always log the detailed reason in the
server log, anyway.

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 messaage to hide whether the user exists, but that argument doesn't hold water if you're going to report an "unknown-user" error, anyway.

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..

To be really pedantic about that, we should also ward off timing attacks, by making sure that the dummy authentication is no faster/slower than a real one..

If this is taken into account we need to perform sanity checks at
initialization phase I am afraid as the number of iterations and the
salt are part of the verifier. So you mean that just sending out a
normal ERROR message is fine at an earlier step (with *logdetails
filled for the backend)? I just want to be sure I understand what you
mean here.

That's right, we can send a normal ERROR message. (But not for the "user-not-found" case, as discussed above.)

As Peter E pointed out earlier, the documentation is lacking, on how to
configure MD5 and/or SCRAM. If you put "scram" as the authentication method
in pg_hba.conf, what does it mean? If you have a line for both "scram" and
"md5" in pg_hba.conf, with the same database/user/hostname combo, what does
that mean? Answer: The first one takes effect, the second one has no effect.
Yet the example in the docs now has that, which is nonsense :-). Hopefully
we'll have some kind of a "both" option, before the release, but in the
meanwhile, we need describe how this works now in the docs.

OK, it would be better to add a paragraph in client-auth.sgml
regarding the mapping of the two settings. For the example of file in
postgresql.conf, I would have really thought that adding directly a
line with "scram" listed was enough though. Perhaps a comment to say
that if md5 and scram are specified the first one wins where a user
and database name map?

So, I think this makes no sense:

 # Allow any user from host to connect to database
-# "postgres" if the user's password is correctly supplied.
+# "postgres" if the user's password is correctly supplied and is
+# using the correct password method.
 # TYPE  DATABASE        USER            ADDRESS                 METHOD
 host    postgres        all           md5
+host    postgres        all           scram

But this is OK:

+# Same as previous entry, except that the supplied password must be
+# encrypted with SCRAM-SHA-256.
+host    all             all             .example.com            scram

Although, currently, the whole pg_hba.conf file in that example is a valid file that someone might have on a real server. With the above addition, it would not be. You would never have the two lines with the same host/database/user combination in pg_hba.conf.

Overall, I think something like this would make sense in the example:

# Allow any user from hosts in the example.com domain to connect to
# any database, if the user's password is correctly supplied.
# Most users use SCRAM authentication, but some users use older clients
# that don't support SCRAM authentication, and need to be able to log
# in using MD5 authentication. Such users are put in the @md5users
# group, everyone else must use SCRAM.
# TYPE  DATABASE        USER            ADDRESS                 METHOD
host    all             @md5users       .example.com            md5
host    all             all             .example.com            scram

- Heikki

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

Reply via email to