BUG 2.4 regression: a wrong password over PLAIN is reported as a temporary failure, and the invalid_grant branch is now dead code

Summary

With passdb oauth2 doing a password grant, a wrong password makes dovecot
answer the IMAP client

a NO [UNAVAILABLE] Temporary authentication failure.

and log it at Error: level. 2.3 answered AUTHENTICATIONFAILED for the same
exchange. PASSDB_RESULT_PASSWORD_MISMATCH is still in db-oauth2.c, but for
the password grant it can no longer be reached.
Environment

Dovecot *2.4.4 (8b687aa65c)*, the official dovecot/dovecot:2.4.4-root
image, unmodified. Compared against *2.3.21*.
Reproduction

Any RFC 6749 compliant authorization server. Point passdb oauth2 at its
token endpoint with grant_type=password and attempt an IMAP LOGIN with a
wrong password. The server answers, as §5.2 prescribes:

HTTP 400
{"error":"invalid_grant","error_description":"Invalid credentials"}

*2.3.21:* NO [AUTHENTICATIONFAILED], logged at info level. *2.4.4:* NO
[UNAVAILABLE] Temporary authentication failure, logged as

auth(user,ip,sasl:plain): Error: oauth2-grant: oauth2 failed: Password
grant failed: Invalid credentials
imap-login: Login aborted: Logged out (auth service reported temporary
failure, 1 attempts in 3 secs) (temp_fail)

Cause

src/auth/db-oauth2.c is identical in both versions:

passdb_result = PASSDB_RESULT_INTERNAL_FAILURE;
if (result->error != NULL)
        error = result->error;
else {
        error = db_oauth2_field_find(result->fields, "error");
        if (error == NULL)
                error = "OAuth2 server returned failure without error field";
        else if (strcmp("invalid_grant", error) == 0)
                passdb_result = PASSDB_RESULT_PASSWORD_MISMATCH;
}

What changed is what reaches result->error.

In *2.3.21*, src/lib-oauth2/oauth2-request.c:113-129 set error only when
the JSON *parser* failed. A well-formed error body parsed fine, so error
was NULL, res.error stayed NULL, and db-oauth2.c took the else branch,
found invalid_grant among the fields and returned PASSWORD_MISMATCH.

In *2.4.4*, src/lib-oauth2/oauth2-request.c:136-149 added:

/* check if fields contain error now */
const struct oauth2_field *error_field =
        array_lsearch(&req->fields, "error", request_field_cmp);
if (error_field != NULL) {
        error = error_field->value;              /* "invalid_grant" */
        /* check for detailed error */
        error_field = array_lsearch(&req->fields, "error_description",
                                    request_field_cmp);
        if (error_field != NULL)
                error = error_field->value;      /* overwritten:
"Invalid credentials" */
}
req->json_parsed_cb(req, error);

oauth2_request_continue() then does if (error != NULL) res.error = error;,
so result->error is always set on failure and db-oauth2.c never reaches the
branch that recognises invalid_grant. The machine-readable code is replaced
by the human-readable description before anything can act on it.

Note that error_description is *optional* in RFC 6749 §5.2 — so a fully
compliant server triggers this simply by being helpful. Omitting it does
not help either: error would then hold invalid_grant and still be assigned
to res.error, taking the same branch.
Why it matters

   - Mail clients treat [UNAVAILABLE] as a server problem and retry rather
   than prompting for the password, so users are not told their password is
   wrong.
   - Monitoring reads a user error as a service failure — on a farm this is
   the difference between a quiet log and a page.
   - RFC 5530 defines AUTHENTICATIONFAILED for a failed authentication and
   UNAVAILABLE for a temporarily unavailable server. Reporting the former
   as the latter tells the client something untrue about the server.

There is no anti-enumeration benefit in the current behaviour: the same
provider response is returned for a wrong password and for a non-existent
account, so both are indistinguishable to the client under either mapping.

_______________________________________________
dovecot mailing list -- [email protected]
To unsubscribe send an email to [email protected]