v8-0002-sasl-Allow-backend-mechanisms-to-abandon-exchange.patch
application/octet-stream
Filename: v8-0002-sasl-Allow-backend-mechanisms-to-abandon-exchange.patch
Type: application/octet-stream
Part: 2
Message:
Re: Improve OAuth discovery logging
Patch
Format: format-patch
Series: patch v8-0002
Subject: sasl: Allow backend mechanisms to "abandon" exchanges
| File | + | − |
|---|---|---|
| src/backend/libpq/auth.c | 25 | 7 |
| src/backend/libpq/auth-sasl.c | 22 | 2 |
| src/include/libpq/sasl.h | 9 | 6 |
From f124deb76d95d6ac41cfc8f20f41b10249f9f2d1 Mon Sep 17 00:00:00 2001
From: Jacob Champion <jacob.champion@enterprisedb.com>
Date: Mon, 16 Mar 2026 17:09:08 -0700
Subject: [PATCH v8 2/3] 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 <zsolt.parragi@percona.com>
Co-authored-by: Jacob Champion <jacob.champion@enterprisedb.com>
---
src/include/libpq/sasl.h | 15 +++++++++------
src/backend/libpq/auth-sasl.c | 24 ++++++++++++++++++++++--
src/backend/libpq/auth.c | 32 +++++++++++++++++++++++++-------
3 files changed, 56 insertions(+), 15 deletions(-)
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 */
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);
--
2.34.1