Add more sanity checks in SASL exchanges

The following checks are added, to make the SASL infrastructure more
aware of defects when implementing new mechanisms:
- Detect that no output is generated by a mechanism if an exchange fails
in the backend, failing if there is a message waiting to be sent.
- Handle zero-length messages in the frontend.  The backend handles that
already, and SCRAM would complain if sending empty messages as this is
not authorized for this mechanism, but other mechanisms may want this
capability (the SASL specification allows that).
- Make sure that a mechanism generates a message in the middle of the
exchange in the frontend.

SCRAM, as implemented, respects all these requirements already, and the
recent refactoring of SASL done in 9fd8557 helps in documenting that in
a cleaner way.

Analyzed-by: Jacob Champion
Author: Michael Paquier
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/3d2a6f5d50e741117d6baf83eb67ebf1a8a35a11.camel@vmware.com

1 files changed, 16 insertions, 1 deletions

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index eaba0ba56d7..3421ed4685b 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -674,7 +674,22 @@ pg_SASL_continue(PGconn *conn, int payloadlen, bool final)
 		return STATUS_ERROR;
 	}
 
-	if (outputlen != 0)
+	/*
+	 * If the exchange is not completed yet, we need to make sure that the
+	 * SASL mechanism has generated a message to send back.
+	 */
+	if (output == NULL && !done)
+	{
+		appendPQExpBufferStr(&conn->errorMessage,
+							 libpq_gettext("no client response found after SASL exchange success\n"));
+		return STATUS_ERROR;
+	}
+
+	/*
+	 * SASL allows zero-length responses, so this check uses "output" and not
+	 * "outputlen" to allow the case of an empty message.
+	 */
+	if (output)
 	{
 		/*
 		 * Send the SASL response to the server.