Re: Password identifiers, protocol aging and SCRAM protocol
Michael Paquier <michael.paquier@gmail.com>
From: Michael Paquier <michael.paquier@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
David Steele <david@pgmasters.net>, Tom Lane <tgl@sss.pgh.pa.us>, David Fetter <david@fetter.org>, Alvaro Herrera <alvherre@2ndquadrant.com>,
Magnus Hagander <magnus@hagander.net>, Julian Markwort <julian.markwort@uni-muenster.de>,
Stephen Frost <sfrost@snowman.net>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>,
Valery Popov <v.popov@postgrespro.ru>
Date: 2016-12-08T08:18:16Z
Lists: pgsql-hackers
On Thu, Dec 8, 2016 at 5:54 AM, Heikki Linnakangas <hlinnaka@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? -- Michael
Commits
-
Support SCRAM-SHA-256 authentication (RFC 5802 and 7677).
- 818fd4a67d61 10.0 landed
-
Refactor SHA2 functions and move them to src/common/.
- 273c458a2b3a 10.0 landed
-
Replace isMD5() with a more future-proof way to check if pw is encrypted.
- dbd69118c05d 10.0 landed
-
Remove bogus notice that older clients might not work with MD5 passwords.
- 7e3ae5455948 9.2.20 landed
- 470af1f41c8b 9.3.16 landed
- ada2cdb61015 9.4.11 landed
- 65a7f190b253 9.5.6 landed
- 7546c135dc30 9.6.2 landed
- 31c54096a18f 10.0 landed
-
Refactor the code for verifying user's password.
- e7f051b8f9a6 10.0 landed
-
Replace PostmasterRandom() with a stronger source, second attempt.
- fe0a0b5993df 10.0 landed
-
Remove support for (insecure) crypt authentication.
- 53a5026b5cb3 8.4.0 cited