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

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

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

Reply via email to