From 11e6ded229bfc684f511591477db96f6fba48006 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 14 Aug 2024 18:25:16 +0200 Subject: [PATCH v17 3/8] 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 | 6 ++- src/interfaces/libpq/fe-protocol3.c | 65 +++++++++++++++++------------ 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 85d1ca2864f..f07c30d8862 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -4067,9 +4067,13 @@ keep_going: /* We will come back to here until there is libpq_append_conn_error(conn, "received invalid protocol negotiation message"); goto error_return; } + + if (conn->error_result) + 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..2ed1b21c046 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -1407,49 +1407,60 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding) int pqGetNegotiateProtocolVersion3(PGconn *conn) { - int tmp; - ProtocolVersion their_version; + int their_version; int num; - PQExpBufferData buf; - if (pqGetInt(&tmp, 4, conn) != 0) + if (pqGetInt(&their_version, 4, conn) != 0) return EOF; - their_version = tmp; if (pqGetInt(&num, 4, conn) != 0) return EOF; - initPQExpBuffer(&buf); + if (their_version > conn->pversion) + { + libpq_append_conn_error(conn, "received invalid protocol negotiation message: server requests downgrade to higher-numbered version"); + goto failure; + } + + 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) + { + libpq_append_conn_error(conn, "received invalid protocol negotiation message: server reported negative number of unsupported parameters"); + goto failure; + } + + 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)) { - termPQExpBuffer(&buf); return EOF; } - if (buf.len > 0) - appendPQExpBufferChar(&buf, ' '); - appendPQExpBufferStr(&buf, conn->workBuffer.data); - } - - 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) - { - 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'); + 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; } - /* neither -- server shouldn't have sent it */ - if (!(their_version < conn->pversion) && !(num > 0)) - libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion"); + return 0; - termPQExpBuffer(&buf); +failure: + conn->asyncStatus = PGASYNC_READY; + pqSaveErrorResult(conn); return 0; } -- 2.43.0