Minor cleanup of backend SCRAM code.

Free each SASL message after sending it. It's not a lot of wasted memory,
and it's short-lived, but the authentication code in general tries to
pfree() stuff, so let's follow the example.

Adding the pfree() revealed a little bug in build_server_first_message().
It attempts to keeps a copy of the sent message, but it was missing a
pstrdup(), so the pointer started to dangle, after adding the pfree()
into CheckSCRAMAuth().

Reword comments and debug messages slightly, while we're at it.

Reviewed by Michael Paquier.

Discussion: https://www.postgresql.org/message-id/6490b975-5ee1-6280-ac1d-af975b19fb9a@iki.fi

diff --git a/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 5077ff33b16acb0a65f2470f4d1854bd66c0b82c..a47c48d980501acdc4ef7d6277859ae43bce16a3 100644 (file)
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -161,10 +161,10 @@ static char *scram_MockSalt(const char *username);
  * needs to be called before doing any exchange.  It will be filled later
  * after the beginning of the exchange with verifier data.
  *
- * 'username' is the provided by the client.  'shadow_pass' is the role's
- * password verifier, from pg_authid.rolpassword.  If 'shadow_pass' is NULL, we
- * still perform an authentication exchange, but it will fail, as if an
- * incorrect password was given.
+ * 'username' is the username provided by the client in the startup message.
+ * 'shadow_pass' is the role's password verifier, from pg_authid.rolpassword.
+ * If 'shadow_pass' is NULL, we still perform an authentication exchange, but
+ * it will fail, as if an incorrect password was given.
  */
 void *
 pg_be_scram_init(const char *username, const char *shadow_pass)
@@ -984,7 +984,7 @@ build_server_first_message(scram_state *state)
                                 state->client_nonce, state->server_nonce,
                                 state->salt, state->iterations);
 
-       return state->server_first_message;
+       return pstrdup(state->server_first_message);
 }
 
 

diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 66ead9381d3cc025661379eda362aa95173786b1..b4c98c45c9f5f8c07988a8bf4ba9eb5040ae245b 100644 (file)
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -872,6 +872,8 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
                                        strlen(SCRAM_SHA256_NAME) + 1);
 
        /*
+        * Initialize the status tracker for message exchanges.
+        *
         * If the user doesn't exist, or doesn't have a valid password, or it's
         * expired, we still go through the motions of SASL authentication, but
         * tell the authentication method that the authentication is "doomed".
@@ -880,8 +882,6 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
         * This is because we don't want to reveal to an attacker what usernames
         * are valid, nor which users have a valid password.
         */
-
-       /* Initialize the status tracker for message exchanges */
        scram_opaq = pg_be_scram_init(port->user_name, shadow_pass);
 
        /*
@@ -918,7 +918,7 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
                        return STATUS_ERROR;
                }
 
-               elog(DEBUG4, "Processing received SASL token of length %d", buf.len);
+               elog(DEBUG4, "Processing received SASL response of length %d", buf.len);
 
                /*
                 * we pass 'logdetail' as NULL when doing a mock authentication,
@@ -931,14 +931,16 @@ CheckSCRAMAuth(Port *port, char *shadow_pass, char **logdetail)
                /* input buffer no longer used */
                pfree(buf.data);
 
-               if (outputlen > 0)
+               if (output)
                {
                        /*
                         * Negotiation generated data to be sent to the client.
                         */
-                       elog(DEBUG4, "sending SASL response token of length %u", outputlen);
+                       elog(DEBUG4, "sending SASL challenge of length %u", outputlen);
 
                        sendAuthRequest(port, AUTH_REQ_SASL_CONT, output, outputlen);
+
+                       pfree(output);
                }
        } while (result == SASL_EXCHANGE_CONTINUE);