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