v4-0003-libpq-Handle-NegotiateProtocolVersion-message-dif.patch

From d4264c2487cb7c84efeaee09cbe52685487c0b88 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <postgres@jeltef.nl>
Date: Wed, 14 Aug 2024 18:25:16 +0200
Subject: [PATCH v4 3/7] libpq: Handle NegotiateProtocolVersion message
 differently

Previously libpq would always error when the server returns a
NegotiateProtocolVersion message. This was fine because libpq only
supported a single protocol version and did not support any protocol
parameters. But we now that we're discussing protocol changes we need to
change this behaviour, and introduce a fallback mechanism when
connecting to an older server.

This patch modifies the client side checks to allow a range of supported
protocol versions, instead of only allowing the exact version that was
requested. Currently this "range" only contains the 3.0 version, but in
a future commit we'll change this. In addition it changes the errors for
protocol to say that they error because we did not request any
parameters, not because the server did not support some of them. In a
future commit more infrastructure for protocol parameters will be added,
so that these checks can also succeed when receiving unsupported
parameters back.

Note that at the moment this change does not have any behavioural
effect, because libpq will only request version 3.0 and will never send
protocol parameters. Which means that the client never receives a
NegotiateProtocolVersion message from the server.
---
 src/interfaces/libpq/fe-connect.c   |  3 +-
 src/interfaces/libpq/fe-protocol3.c | 80 +++++++++++++++++------------
 2 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index d5051f5e820..281710b0616 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4091,9 +4091,10 @@ keep_going:						/* We will come back to here until there is
 						libpq_append_conn_error(conn, "received invalid protocol negotiation message");
 						goto error_return;
 					}
+
 					/* OK, we read the message; mark data consumed */
 					pqParseDone(conn, conn->inCursor);
-					goto error_return;
+					goto keep_going;
 				}
 
 				/* It is an authentication request. */
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index 95dd456f076..68a94281eb4 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1399,58 +1399,72 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding)
 
 
 /*
- * Attempt to read a NegotiateProtocolVersion message.
+ * Attempt to read a NegotiateProtocolVersion message. Sets conn->pversion
+ * to the version that's negotiated by the server.
  * Entry: 'v' message type and length have already been consumed.
  * Exit: returns 0 if successfully consumed message.
- *		 returns EOF if not enough data.
+ *		 returns 1 on failure. The error message is filled in.
  */
 int
 pqGetNegotiateProtocolVersion3(PGconn *conn)
 {
-	int			tmp;
-	ProtocolVersion their_version;
+	int			their_version;
 	int			num;
-	PQExpBufferData buf;
 
-	if (pqGetInt(&tmp, 4, conn) != 0)
-		return EOF;
-	their_version = tmp;
+	if (pqGetInt(&their_version, 4, conn) != 0)
+		goto eof;
 
 	if (pqGetInt(&num, 4, conn) != 0)
-		return EOF;
+		goto eof;
 
-	initPQExpBuffer(&buf);
-	for (int i = 0; i < num; i++)
+	if (their_version > conn->pversion)
 	{
-		if (pqGets(&conn->workBuffer, conn))
-		{
-			termPQExpBuffer(&buf);
-			return EOF;
-		}
-		if (buf.len > 0)
-			appendPQExpBufferChar(&buf, ' ');
-		appendPQExpBufferStr(&buf, conn->workBuffer.data);
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to higher-numbered version");
+		goto failure;
 	}
 
-	if (their_version < conn->pversion)
-		libpq_append_conn_error(conn, "protocol version not supported by server: client uses %u.%u, server supports up to %u.%u",
-								PG_PROTOCOL_MAJOR(conn->pversion), PG_PROTOCOL_MINOR(conn->pversion),
-								PG_PROTOCOL_MAJOR(their_version), PG_PROTOCOL_MINOR(their_version));
-	if (num > 0)
+	if (their_version < PG_PROTOCOL(3, 0))
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to pre-3.0 protocol version");
+		goto failure;
+	}
+	
+	if (num < 0)
 	{
-		appendPQExpBuffer(&conn->errorMessage,
-						  libpq_ngettext("protocol extension not supported by server: %s",
-										 "protocol extensions not supported by server: %s", num),
-						  buf.data);
-		appendPQExpBufferChar(&conn->errorMessage, '\n');
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported negative number of unsupported parameters");
+		goto failure;
 	}
 
-	/* neither -- server shouldn't have sent it */
-	if (!(their_version < conn->pversion) && !(num > 0))
-		libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion");
+	if (their_version == conn->pversion && num == 0)
+	{
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server negotiated but asks for no changes");
+		goto failure;
+	}
+
+	conn->pversion = their_version;
+	for (int i = 0; i < num; i++)
+	{
+		if (pqGets(&conn->workBuffer, conn))
+		{
+			goto eof;
+		}
+		if (strncmp(conn->workBuffer.data, "_pq_.", 5) != 0)
+		{
+			libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported unsupported parameter name without a _pq_. prefix (\"%s\")", conn->workBuffer.data);
+			goto failure;
+		}
+		libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported an unsupported parameter that was not requested (\"%s\")", conn->workBuffer.data);
+		goto failure;
+	}
 
-	termPQExpBuffer(&buf);
 	return 0;
+
+eof:
+	libpq_append_conn_error(conn, "received invalid protocol negotation message: message too short");
+failure:
+	conn->asyncStatus = PGASYNC_READY;
+	pqSaveErrorResult(conn);
+	return 1;
 }
 
 
-- 
2.43.0