From d4264c2487cb7c84efeaee09cbe52685487c0b88 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio 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