Commit 344a1130 authored by Heikki Linnakangas's avatar Heikki Linnakangas

Don't use SCRAM-specific "e=invalid-proof" on invalid password.

Instead, send the same FATAL message as with other password-based
authentication mechanisms. This gives a more user-friendly message:

psql: FATAL:  password authentication failed for user "test"

instead of:

psql: error received from server in SASL exchange: invalid-proof

Even before this patch, the server sent that FATAL message, after the
SCRAM-specific "e=invalid-proof" message. But libpq would stop at the
SCRAM error message, and not process the ErrorResponse that would come
after that. We could've taught libpq to check for an ErrorResponse after
failed authentication, but it's simpler to modify the server to send only
the ErrorResponse. The SCRAM specification allows for aborting the
authentication at any point, using an application-defined error mechanism,
like PostgreSQL's ErrorResponse. Using the e=invalid-proof message is
optional.

Reported by Jeff Janes.

Discussion: https://www.postgresql.org/message-id/CAMkU%3D1w3jQ53M1OeNfN8Cxd9O%2BA_9VONJivTbYoYRRdRsLT6vA@mail.gmail.com
parent 44c52881
...@@ -343,6 +343,13 @@ pg_be_scram_exchange(void *opaq, char *input, int inputlen, ...@@ -343,6 +343,13 @@ pg_be_scram_exchange(void *opaq, char *input, int inputlen,
* If we performed a "mock" authentication that we knew would fail * If we performed a "mock" authentication that we knew would fail
* from the get go, this is where we fail. * from the get go, this is where we fail.
* *
* The SCRAM specification includes an error code,
* "invalid-proof", for authentication failure, but it also allows
* erroring out in an application-specific way. We choose to do
* the latter, so that the error message for invalid password is
* the same for all authentication methods. The caller will call
* ereport(), when we return SASL_EXCHANGE_FAILURE with no output.
*
* NB: the order of these checks is intentional. We calculate the * NB: the order of these checks is intentional. We calculate the
* client proof even in a mock authentication, even though it's * client proof even in a mock authentication, even though it's
* bound to fail, to thwart timing attacks to determine if a role * bound to fail, to thwart timing attacks to determine if a role
...@@ -350,14 +357,6 @@ pg_be_scram_exchange(void *opaq, char *input, int inputlen, ...@@ -350,14 +357,6 @@ pg_be_scram_exchange(void *opaq, char *input, int inputlen,
*/ */
if (!verify_client_proof(state) || state->doomed) if (!verify_client_proof(state) || state->doomed)
{ {
/*
* Signal invalid-proof, although the real reason might also
* be e.g. that the password has expired, or the user doesn't
* exist. "e=other-error" might be more correct, but
* "e=invalid-proof" is more likely to give a nice error
* message to the user.
*/
*output = psprintf("e=invalid-proof");
result = SASL_EXCHANGE_FAILURE; result = SASL_EXCHANGE_FAILURE;
break; break;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment