From c4ff16339f07d1e253bdf18e5da5fa25f62a750d Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Tue, 31 Mar 2026 11:47:31 -0700 Subject: [PATCH] sasl: Allow backend mechanisms to "abandon" exchanges Introduce PG_SASL_EXCHANGE_ABANDONED, which allows CheckSASLAuth to suppress the failing log entry for any SASL exchange that isn't actually an authentication attempt. This is desirable for OAUTHBEARER's discovery exchanges (and a subsequent commit will make use of it there). This might have some overlap in the future with in-band aborts for SASL exchanges, but it's intentionally not named _ABORTED to avoid confusion. (We don't currently support clientside aborts in our SASL profile.) Adapted from a patch by Zsolt Parragi. Author: Zsolt Parragi Co-authored-by: Jacob Champion Reviewed-by: Chao Li Discussion: https://postgr.es/m/CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q%40mail.gmail.com --- src/backend/libpq/auth-sasl.c | 24 ++++++++++++++++++++++-- src/backend/libpq/auth.c | 32 +++++++++++++++++++++++++------- src/include/libpq/sasl.h | 15 +++++++++------ 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/backend/libpq/auth-sasl.c b/src/backend/libpq/auth-sasl.c index 36cb748d927..59ac38fca50 100644 --- a/src/backend/libpq/auth-sasl.c +++ b/src/backend/libpq/auth-sasl.c @@ -30,6 +30,12 @@ * be found for the role (or the user does not exist), and the mechanism * should fail the authentication exchange. * + * Some SASL mechanisms (e.g. OAUTHBEARER) define special exchanges for + * parameter discovery. These exchanges will always result in STATUS_ERROR, + * since we can't let the connection continue, but we shouldn't consider them to + * be failed authentication attempts. *abandoned will be set to true in this + * case. + * * Mechanisms must take care not to reveal to the client that a user entry * does not exist; ideally, the external failure mode is identical to that * of an incorrect password. Mechanisms may instead use the logdetail @@ -42,7 +48,7 @@ */ int CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass, - const char **logdetail) + const char **logdetail, bool *abandoned) { StringInfoData sasl_mechs; int mtype; @@ -167,7 +173,7 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass, * PG_SASL_EXCHANGE_FAILURE with some output is forbidden by SASL. * Make sure here that the mechanism used got that right. */ - if (result == PG_SASL_EXCHANGE_FAILURE) + if (result == PG_SASL_EXCHANGE_FAILURE || result == PG_SASL_EXCHANGE_ABANDONED) elog(ERROR, "output message found after SASL exchange failure"); /* @@ -184,6 +190,20 @@ CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, char *shadow_pass, } } while (result == PG_SASL_EXCHANGE_CONTINUE); + if (result == PG_SASL_EXCHANGE_ABANDONED) + { + if (!abandoned) + { + /* + * Programmer error: caller needs to track the abandoned state for + * this mechanism. + */ + elog(ERROR, "SASL exchange was abandoned, but CheckSASLAuth isn't tracking it"); + } + + *abandoned = true; + } + /* Oops, Something bad happened */ if (result != PG_SASL_EXCHANGE_SUCCESS) { diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index e04aa2e68ed..fdacc060381 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -45,7 +45,8 @@ * Global authentication functions *---------------------------------------------------------------- */ -static void auth_failed(Port *port, int status, const char *logdetail); +static void auth_failed(Port *port, int elevel, int status, + const char *logdetail); static char *recv_password_packet(Port *port); @@ -233,15 +234,18 @@ ClientAuthentication_hook_type ClientAuthentication_hook = NULL; * anyway. * Note that many sorts of failure report additional information in the * postmaster log, which we hope is only readable by good guys. In - * particular, if logdetail isn't NULL, we send that string to the log. + * particular, if logdetail isn't NULL, we send that string to the log + * when the elevel allows. */ static void -auth_failed(Port *port, int status, const char *logdetail) +auth_failed(Port *port, int elevel, int status, const char *logdetail) { const char *errstr; char *cdetail; int errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION; + Assert(elevel >= FATAL); /* we must exit here */ + /* * If we failed due to EOF from client, just quit; there's no point in * trying to send a message to the client, and not much point in logging @@ -314,12 +318,13 @@ auth_failed(Port *port, int status, const char *logdetail) else logdetail = cdetail; - ereport(FATAL, + ereport(elevel, (errcode(errcode_return), errmsg(errstr, port->user_name), logdetail ? errdetail_log("%s", logdetail) : 0)); /* doesn't return */ + pg_unreachable(); } @@ -381,6 +386,15 @@ ClientAuthentication(Port *port) int status = STATUS_ERROR; const char *logdetail = NULL; + /* + * "Abandoned" is a SASL-specific state similar to STATUS_EOF, in that we + * don't want to generate any server logs. But it's caused by an in-band + * client action that requires a server response, not an out-of-band + * connection closure, so we can't just proc_exit() like we do with + * STATUS_EOF. + */ + bool abandoned = false; + /* * Get the authentication method to use for this frontend/database * combination. Note: we do not parse the file at this point; this has @@ -625,7 +639,8 @@ ClientAuthentication(Port *port) status = STATUS_OK; break; case uaOAuth: - status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL); + status = CheckSASLAuth(&pg_be_oauth_mech, port, NULL, NULL, + &abandoned); break; } @@ -666,7 +681,10 @@ ClientAuthentication(Port *port) if (status == STATUS_OK) sendAuthRequest(port, AUTH_REQ_OK, NULL, 0); else - auth_failed(port, status, logdetail); + auth_failed(port, + abandoned ? FATAL_CLIENT_ONLY : FATAL, + status, + logdetail); } @@ -860,7 +878,7 @@ CheckPWChallengeAuth(Port *port, const char **logdetail) auth_result = CheckMD5Auth(port, shadow_pass, logdetail); else auth_result = CheckSASLAuth(&pg_be_scram_mech, port, shadow_pass, - logdetail); + logdetail, NULL /* can't abandon SCRAM */ ); if (shadow_pass) pfree(shadow_pass); diff --git a/src/include/libpq/sasl.h b/src/include/libpq/sasl.h index 1e8ec7d6293..bb2af7a7aff 100644 --- a/src/include/libpq/sasl.h +++ b/src/include/libpq/sasl.h @@ -25,6 +25,7 @@ #define PG_SASL_EXCHANGE_CONTINUE 0 #define PG_SASL_EXCHANGE_SUCCESS 1 #define PG_SASL_EXCHANGE_FAILURE 2 +#define PG_SASL_EXCHANGE_ABANDONED 3 /* * Maximum accepted size of SASL messages. @@ -92,8 +93,8 @@ typedef struct pg_be_sasl_mech * * Produces a server challenge to be sent to the client. The callback * must return one of the PG_SASL_EXCHANGE_* values, depending on - * whether the exchange continues, has finished successfully, or has - * failed. + * whether the exchange continues, has finished successfully, has + * failed, or was abandoned by the client. * * Input parameters: * @@ -118,8 +119,9 @@ typedef struct pg_be_sasl_mech * returned and the mechanism requires data to be sent during * a successful outcome). The callback should set this to * NULL if the exchange is over and no output should be sent, - * which should correspond to either PG_SASL_EXCHANGE_FAILURE - * or a PG_SASL_EXCHANGE_SUCCESS with no outcome data. + * which should correspond to either PG_SASL_EXCHANGE_FAILURE, + * PG_SASL_EXCHANGE_ABANDONED, or a PG_SASL_EXCHANGE_SUCCESS + * with no outcome data. * * outputlen: The length of the challenge data. Ignored if *output is * NULL. @@ -128,7 +130,7 @@ typedef struct pg_be_sasl_mech * server log, to disambiguate failure modes. (The client * will only ever see the same generic authentication * failure message.) Ignored if the exchange is completed - * with PG_SASL_EXCHANGE_SUCCESS. + * with PG_SASL_EXCHANGE_SUCCESS or PG_SASL_EXCHANGE_ABANDONED. *--------- */ int (*exchange) (void *state, @@ -142,6 +144,7 @@ typedef struct pg_be_sasl_mech /* Common implementation for auth.c */ extern int CheckSASLAuth(const pg_be_sasl_mech *mech, Port *port, - char *shadow_pass, const char **logdetail); + char *shadow_pass, const char **logdetail, + bool *abandoned); #endif /* PG_SASL_H */