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>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Discussion: https://postgr.es/m/CAN4CZFPim7hUiyb7daNKQPSZ8CvQRBGkVhbvED7yZi8VktSn4Q%40mail.gmail.com

diff --git a/src/backend/libpq/auth-sasl.c b/src/backend/libpq/auth-sasl.c
index 36cb748d927ca1387620476056ffe515a714b25c..59ac38fca50fa3a511bc22d7654ddab8086e25ed 100644 (file)
--- 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 e04aa2e68ed3aa997032505731e88301c15790c7..fdacc060381466358660cfe13a9d45eeaf09e07f 100644 (file)
--- 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 1e8ec7d6293f22e81c6b322d51f543aaeeb25e06..bb2af7a7aff28bab9cc85be709f7354e469c9d33 100644 (file)
--- 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 */