From e313fe11d0b3253edbdc79212c4e055473ae0d39 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Thu, 15 Aug 2024 19:49:29 +0300 Subject: [PATCH v5 2/2] Make cancel request keys longer, bump minor protocol version Currently, cancel request key is 32-bit token, which isn't very much entropy. If you want to cancel another session's query, you can brute-force it. In most environments, an unauthorized cancellation of a query isn't very serious, but it nevertheless would be nice to have more protection from it. Hence make the key longer, to make it harder to guess. The new longer key length is not hardcoded in the protocol anymore, the client is expected to deal with variable length keys, up to some reasonable upper limit (TODO: document the maximum). This flexibility allows e.g. a connection pooler to add more information to the cancel key, which might be useful for finding the connection. This bumps the protocol version from 3.0 to 3.1. The server and libpq still both understand version 3.0, for interoperability with old client and server versions. If an old client connects to the server, the server falls back to generate a short 32-bit token like before. Note that the minor protocol version negotiation was added to the server in version 9.3. Connecting to PostgreSQL 9.2 with new libpq will fail with "unsupported frontend protocol" error, unless you force the old protocol to be used with "protocol_version=3.0". If we wanted to stay compatible, we could teach libpq to reconnect with protocol version 3.0 automatically, but I don't think that's necessary. Reviewed-by: Jelte Fennema-Nio Discussion: /message-id/508d0505-8b7a-4864-a681-e7e5edfe32aa@iki.fi --- doc/src/sgml/libpq.sgml | 12 +++ doc/src/sgml/protocol.sgml | 62 ++++++++++-- src/backend/storage/ipc/procsignal.c | 23 ++--- src/backend/tcop/backend_startup.c | 55 ++++++----- src/backend/tcop/postgres.c | 24 ++++- src/backend/utils/init/globals.c | 5 +- src/backend/utils/init/postinit.c | 2 +- src/include/libpq/pqcomm.h | 9 +- src/include/miscadmin.h | 4 +- src/include/storage/procsignal.h | 14 ++- src/interfaces/libpq/fe-cancel.c | 95 +++++++++++++++---- src/interfaces/libpq/fe-connect.c | 56 ++++++++--- src/interfaces/libpq/fe-protocol3.c | 91 ++++++++++++++++-- src/interfaces/libpq/libpq-int.h | 10 +- .../libpq_pipeline/t/001_libpq_pipeline.pl | 8 ++ src/tools/pgindent/typedefs.list | 1 + 16 files changed, 375 insertions(+), 96 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 068ee60771..9b0a858282 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2335,6 +2335,18 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname + + protocol_version + + + FE/BE protocol version to use. The default is "auto", to use the + highest version supported by both the client and the server. To + connect to old PostgreSQL servers versions + 9.2 and below, use "3.0". + + + + diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index 79cd599692..aa8ddd0638 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -18,10 +18,16 @@ - This document describes version 3.0 of the protocol, implemented in + This document describes version 3.1 of the protocol, introduced in + PostgreSQL version 17. The server is compatible with + protocol version 3.0, implemented in PostgreSQL 7.4 and later. For descriptions - of the earlier protocol versions, see previous releases of the - PostgreSQL documentation. A single server + of earlier protocol versions, see previous releases of the + PostgreSQL documentation. + + + + A single server can support multiple protocol versions. The initial startup-request message tells the server which protocol version the client is attempting to use. If the major version requested by the client is not supported by @@ -36,6 +42,20 @@ to abort the connection. + + The protocol negotiation was introduced in + PostgreSQL version 9.3.21. Earlier versions would + reject the connection if the client requested a minor version that was not + supported by the server. + + + + The difference between protocol version 3.0 and 3.1 is that the secret key + used in query cancellation was enlarged from 4 bytes to a variable length + field. The BackendKeyData message was changed to accomodate that, and the + CancelRequest message was redefined to have a variable length payload. + + In order to serve multiple clients efficiently, the server launches a new backend process for each client. @@ -405,8 +425,14 @@ this message indicates the highest supported minor version. This message will also be sent if the client requested unsupported protocol options (i.e., beginning with _pq_.) in the - startup packet. This message will be followed by an ErrorResponse or - a message indicating the success or failure of authentication. + startup packet. + + + After this message, the authentication will continue using the version + indicated by the server. If the client does not support the older + version, it should immediately close the connection. If the server + does not send this message, it supports the client's requested + protocol version and all the protocol options. @@ -3917,7 +3943,7 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" - Int32(12) + Int32 Length of message contents in bytes, including self. @@ -3935,7 +3961,21 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" - Int32 + Int8 + + + Length of the secret key. + + + This field was added in minor protocol version 3.1. In protocol + version 3.0, it is omitted, and the secret key is always 4 bytes + long. + + + + + + Byten The secret key of this backend. @@ -4148,14 +4188,18 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" - Int32 + Byte4 - The secret key for the target backend. + The secret key for the target backend. This field extends to the end of the + message, indicated by the length field. The maximum key length is 256 bytes. + + Before protocol version 3.1, the secret key was always 4 bytes long. + diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index 87027f27eb..1b0e5e13b3 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -63,8 +63,8 @@ typedef struct { pg_atomic_uint32 pss_pid; - bool pss_cancel_key_valid; - int32 pss_cancel_key; + int pss_cancel_key_len; /* 0 means no cancellation is possible */ + char pss_cancel_key[MAX_CANCEL_KEY_LENGTH]; volatile sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS]; slock_t pss_mutex; /* protects the above fields */ @@ -148,8 +148,7 @@ ProcSignalShmemInit(void) SpinLockInit(&slot->pss_mutex); pg_atomic_init_u32(&slot->pss_pid, 0); - slot->pss_cancel_key_valid = false; - slot->pss_cancel_key = 0; + slot->pss_cancel_key_len = 0; MemSet(slot->pss_signalFlags, 0, sizeof(slot->pss_signalFlags)); pg_atomic_init_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX); pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0); @@ -163,11 +162,12 @@ ProcSignalShmemInit(void) * Register the current process in the ProcSignal array */ void -ProcSignalInit(bool cancel_key_valid, int32 cancel_key) +ProcSignalInit(char *cancel_key, int cancel_key_len) { ProcSignalSlot *slot; uint64 barrier_generation; + Assert(cancel_key_len >= 0 && cancel_key_len <= MAX_CANCEL_KEY_LENGTH); if (MyProcNumber < 0) elog(ERROR, "MyProcNumber not set"); if (MyProcNumber >= NumProcSignalSlots) @@ -202,8 +202,9 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key) pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration); pg_atomic_write_u64(&slot->pss_barrierGeneration, barrier_generation); - slot->pss_cancel_key_valid = cancel_key_valid; - slot->pss_cancel_key = cancel_key; + if (cancel_key_len > 0) + memcpy(slot->pss_cancel_key, cancel_key, cancel_key_len); + slot->pss_cancel_key_len = cancel_key_len; pg_atomic_write_u32(&slot->pss_pid, MyProcPid); SpinLockRelease(&slot->pss_mutex); @@ -252,8 +253,7 @@ CleanupProcSignalState(int status, Datum arg) /* Mark the slot as unused */ pg_atomic_write_u32(&slot->pss_pid, 0); - slot->pss_cancel_key_valid = false; - slot->pss_cancel_key = 0; + slot->pss_cancel_key_len = 0; /* * Make this slot look like it's absorbed all possible barriers, so that @@ -723,7 +723,7 @@ procsignal_sigusr1_handler(SIGNAL_ARGS) * fields in the ProcSignal slots. */ void -SendCancelRequest(int backendPID, int32 cancelAuthCode) +SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len) { Assert(backendPID != 0); @@ -752,7 +752,8 @@ SendCancelRequest(int backendPID, int32 cancelAuthCode) } else { - match = slot->pss_cancel_key_valid && slot->pss_cancel_key == cancelAuthCode; + match = slot->pss_cancel_key_len == cancel_key_len && + timingsafe_bcmp(slot->pss_cancel_key, cancel_key, cancel_key_len) == 0; SpinLockRelease(&slot->pss_mutex); diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c index 2a96c81f92..20fff425c0 100644 --- a/src/backend/tcop/backend_startup.c +++ b/src/backend/tcop/backend_startup.c @@ -45,6 +45,7 @@ bool Trace_connection_negotiation = false; static void BackendInitialize(ClientSocket *client_sock, CAC_state cac); static int ProcessSSLStartup(Port *port); static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done); +static void ProcessCancelRequestPacket(Port *port, void *pkt, int pktlen); static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options); static void process_startup_packet_die(SIGNAL_ARGS); static void StartupPacketTimeoutHandler(void); @@ -542,28 +543,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) if (proto == CANCEL_REQUEST_CODE) { - /* - * The client has sent a cancel request packet, not a normal - * start-a-new-connection packet. Perform the necessary processing. - * Nothing is sent back to the client. - */ - CancelRequestPacket *canc; - int backendPID; - int32 cancelAuthCode; - - if (len != sizeof(CancelRequestPacket)) - { - ereport(COMMERROR, - (errcode(ERRCODE_PROTOCOL_VIOLATION), - errmsg("invalid length of startup packet"))); - return STATUS_ERROR; - } - canc = (CancelRequestPacket *) buf; - backendPID = (int) pg_ntoh32(canc->backendPID); - cancelAuthCode = (int32) pg_ntoh32(canc->cancelAuthCode); - - if (backendPID != 0) - SendCancelRequest(backendPID, cancelAuthCode); + ProcessCancelRequestPacket(port, buf, len); /* Not really an error, but we don't want to proceed further */ return STATUS_ERROR; } @@ -854,6 +834,37 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) return STATUS_OK; } +/* + * The client has sent a cancel request packet, not a normal + * start-a-new-connection packet. Perform the necessary processing. Nothing + * is sent back to the client. + */ +static void +ProcessCancelRequestPacket(Port *port, void *pkt, int pktlen) +{ + CancelRequestPacket *canc; + int len; + + if (pktlen < offsetof(CancelRequestPacket, cancelAuthCode)) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("invalid length of query cancel packet"))); + return; + } + len = pktlen - offsetof(CancelRequestPacket, cancelAuthCode); + if (len == 0 || len > 256) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("invalid length of query cancel key"))); + return; + } + + canc = (CancelRequestPacket *) pkt; + SendCancelRequest(pg_ntoh32(canc->backendPID), canc->cancelAuthCode, len); +} + /* * Send a NegotiateProtocolVersion to the client. This lets the client know * that they have either requested a newer minor protocol version than we are diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 8bc6bea113..c148167a62 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4298,16 +4298,20 @@ PostgresMain(const char *dbname, const char *username) * Generate a random cancel key, if this is a backend serving a * connection. InitPostgres() will advertise it in shared memory. */ - Assert(!MyCancelKeyValid); + Assert(MyCancelKeyLength == 0); if (whereToSendOutput == DestRemote) { - if (!pg_strong_random(&MyCancelKey, sizeof(int32))) + int len; + + len = (MyProcPort == NULL || MyProcPort->proto >= PG_PROTOCOL(3, 1)) + ? MAX_CANCEL_KEY_LENGTH : 4; + if (!pg_strong_random(&MyCancelKey, len)) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("could not generate random cancel key"))); } - MyCancelKeyValid = true; + MyCancelKeyLength = len; } /* @@ -4362,10 +4366,20 @@ PostgresMain(const char *dbname, const char *username) { StringInfoData buf; - Assert(MyCancelKeyValid); + Assert(MyCancelKeyLength > 0); pq_beginmessage(&buf, PqMsg_BackendKeyData); pq_sendint32(&buf, (int32) MyProcPid); - pq_sendint32(&buf, (int32) MyCancelKey); + + if (MyProcPort->proto >= PG_PROTOCOL(3, 1)) + { + pq_sendint8(&buf, MyCancelKeyLength); + pq_sendbytes(&buf, MyCancelKey, MyCancelKeyLength); + } + else + { + Assert(MyCancelKeyLength == 4); + pq_sendbytes(&buf, MyCancelKey, 4); + } pq_endmessage(&buf); /* Need not flush since ReadyForQuery will do it. */ } diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 03a54451ac..7f3c838f83 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -24,6 +24,7 @@ #include "miscadmin.h" #include "postmaster/postmaster.h" #include "storage/procnumber.h" +#include "storage/procsignal.h" ProtocolVersion FrontendProtocol; @@ -48,8 +49,8 @@ pg_time_t MyStartTime; TimestampTz MyStartTimestamp; struct ClientSocket *MyClientSocket; struct Port *MyProcPort; -bool MyCancelKeyValid = false; -int32 MyCancelKey = 0; +char MyCancelKey[MAX_CANCEL_KEY_LENGTH]; +uint8 MyCancelKeyLength = 0; int MyPMChildSlot; /* diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 13524ea488..c5d82f5ce4 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -683,7 +683,7 @@ InitPostgres(const char *in_dbname, Oid dboid, */ SharedInvalBackendInit(false); - ProcSignalInit(MyCancelKeyValid, MyCancelKey); + ProcSignalInit(MyCancelKey, MyCancelKeyLength); /* * Also set up timeout handlers needed for backend operation. We need diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index 527735e3db..20f6bad320 100644 --- a/src/include/libpq/pqcomm.h +++ b/src/include/libpq/pqcomm.h @@ -94,7 +94,7 @@ is_unixsock_path(const char *path) */ #define PG_PROTOCOL_EARLIEST PG_PROTOCOL(3,0) -#define PG_PROTOCOL_LATEST PG_PROTOCOL(3,0) +#define PG_PROTOCOL_LATEST PG_PROTOCOL(3,1) typedef uint32 ProtocolVersion; /* FE/BE protocol version number */ @@ -128,7 +128,11 @@ typedef uint32 AuthRequest; * * The cancel request code must not match any protocol version number * we're ever likely to use. This random choice should do. + * + * Before PostgreSQL v17 and the protocol version bump from 3.0 to 3.1, the + * cancel key was always 4 bytes. Starting with v17, it's variable length. */ + #define CANCEL_REQUEST_CODE PG_PROTOCOL(1234,5678) typedef struct CancelRequestPacket @@ -136,7 +140,8 @@ typedef struct CancelRequestPacket /* Note that each field is stored in network byte order! */ MsgType cancelRequestCode; /* code to identify a cancel request */ uint32 backendPID; /* PID of client's backend */ - uint32 cancelAuthCode; /* secret key to authorize cancel */ + char cancelAuthCode[FLEXIBLE_ARRAY_MEMBER]; /* secret key to + * authorize cancel */ } CancelRequestPacket; /* Application-Layer Protocol Negotiation is required for direct connections diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 25348e71eb..53b6f63aaa 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -191,8 +191,8 @@ extern PGDLLIMPORT pg_time_t MyStartTime; extern PGDLLIMPORT TimestampTz MyStartTimestamp; extern PGDLLIMPORT struct Port *MyProcPort; extern PGDLLIMPORT struct Latch *MyLatch; -extern PGDLLIMPORT bool MyCancelKeyValid; -extern PGDLLIMPORT int32 MyCancelKey; +extern PGDLLIMPORT char MyCancelKey[]; +extern PGDLLIMPORT uint8 MyCancelKeyLength; extern PGDLLIMPORT int MyPMChildSlot; extern PGDLLIMPORT char OutputFileName[]; diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index f94c11a9a8..071d302d63 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -56,16 +56,26 @@ typedef enum PROCSIGNAL_BARRIER_SMGRRELEASE, /* ask smgr to close files */ } ProcSignalBarrierType; +/* + * Length of query cancel keys generated. + * + * Note that the protocol allows for longer keys, or shorter, but this is the + * length we actually generate. Client code, and the server code that handles + * incoming cancellation packets from clients, cannot use this hardcoded + * length. + */ +#define MAX_CANCEL_KEY_LENGTH 32 + /* * prototypes for functions in procsignal.c */ extern Size ProcSignalShmemSize(void); extern void ProcSignalShmemInit(void); -extern void ProcSignalInit(bool cancel_key_valid, int32 cancel_key); +extern void ProcSignalInit(char *cancel_key, int cancel_key_len); extern int SendProcSignal(pid_t pid, ProcSignalReason reason, ProcNumber procNumber); -extern void SendCancelRequest(int backendPID, int32 cancelAuthCode); +extern void SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len); extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type); extern void WaitForProcSignalBarrier(uint64 generation); diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c index 213a6f43c2..0a59c8904a 100644 --- a/src/interfaces/libpq/fe-cancel.c +++ b/src/interfaces/libpq/fe-cancel.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * fe-cancel.c - * functions related to setting up a connection to the backend + * functions related to query cancellation * * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California @@ -41,7 +41,6 @@ struct pg_cancel { SockAddr raddr; /* Remote address */ int be_pid; /* PID of to-be-canceled backend */ - int be_key; /* cancel key of to-be-canceled backend */ int pgtcp_user_timeout; /* tcp user timeout */ int keepalives; /* use TCP keepalives? */ int keepalives_idle; /* time between TCP keepalives */ @@ -49,6 +48,10 @@ struct pg_cancel * retransmits */ int keepalives_count; /* maximum number of TCP keepalive * retransmits */ + + /* Pre-constructed cancel request packet starts here */ + int32 cancel_pkt_len; /* in network-byte-order */ + char cancel_req[FLEXIBLE_ARRAY_MEMBER]; /* CancelRequestPacket */ }; @@ -83,6 +86,13 @@ PQcancelCreate(PGconn *conn) return (PGcancelConn *) cancelConn; } + /* Check that we have received a cancellation key */ + if (conn->be_cancel_key_len == 0) + { + libpq_append_conn_error(cancelConn, "no cancellation key received"); + return (PGcancelConn *) cancelConn; + } + /* * Indicate that this connection is used to send a cancellation */ @@ -101,7 +111,15 @@ PQcancelCreate(PGconn *conn) * Copy cancellation token data from the original connection */ cancelConn->be_pid = conn->be_pid; - cancelConn->be_key = conn->be_key; + if (conn->be_cancel_key != NULL) + { + cancelConn->be_cancel_key = malloc(conn->be_cancel_key_len); + if (!conn->be_cancel_key) + goto oom_error; + memcpy(cancelConn->be_cancel_key, conn->be_cancel_key, conn->be_cancel_key_len); + } + cancelConn->be_cancel_key_len = conn->be_cancel_key_len; + cancelConn->pversion = conn->pversion; /* * Cancel requests should not iterate over all possible hosts. The request @@ -349,6 +367,8 @@ PGcancel * PQgetCancel(PGconn *conn) { PGcancel *cancel; + int cancel_req_len; + CancelRequestPacket *req; if (!conn) return NULL; @@ -356,13 +376,17 @@ PQgetCancel(PGconn *conn) if (conn->sock == PGINVALID_SOCKET) return NULL; - cancel = malloc(sizeof(PGcancel)); + /* Check that we have received a cancellation key */ + if (conn->be_cancel_key_len == 0) + return NULL; + + cancel_req_len = offsetof(CancelRequestPacket, cancelAuthCode) + conn->be_cancel_key_len; + cancel = malloc(offsetof(PGcancel, cancel_req) + cancel_req_len); if (cancel == NULL) return NULL; memcpy(&cancel->raddr, &conn->raddr, sizeof(SockAddr)); - cancel->be_pid = conn->be_pid; - cancel->be_key = conn->be_key; + /* We use -1 to indicate an unset connection option */ cancel->pgtcp_user_timeout = -1; cancel->keepalives = -1; @@ -405,6 +429,13 @@ PQgetCancel(PGconn *conn) goto fail; } + req = (CancelRequestPacket *) &cancel->cancel_req; + req->cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); + req->backendPID = pg_hton32(conn->be_pid); + memcpy(req->cancelAuthCode, conn->be_cancel_key, conn->be_cancel_key_len); + /* include the length field itself in the length */ + cancel->cancel_pkt_len = pg_hton32(cancel_req_len + 4); + return cancel; fail: @@ -412,6 +443,35 @@ fail: return NULL; } +int +pqSendCancelRequest(PGconn *cancelConn) +{ + CancelRequestPacket req; + + /* Start the message. */ + if (pqPutMsgStart(0, cancelConn)) + return STATUS_ERROR; + + /* Send the message body. */ + memset(&req, 0, offsetof(CancelRequestPacket, cancelAuthCode)); + req.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); + req.backendPID = pg_hton32(cancelConn->be_pid); + if (pqPutnchar((char *) &req, offsetof(CancelRequestPacket, cancelAuthCode), cancelConn)) + return STATUS_ERROR; + if (pqPutnchar(cancelConn->be_cancel_key, cancelConn->be_cancel_key_len, cancelConn)) + return STATUS_ERROR; + + /* Finish the message. */ + if (pqPutMsgEnd(cancelConn)) + return STATUS_ERROR; + + /* Flush to ensure backend gets it. */ + if (pqFlush(cancelConn)) + return STATUS_ERROR; + + return STATUS_OK; +} + /* PQfreeCancel: free a cancel structure */ void PQfreeCancel(PGcancel *cancel) @@ -465,11 +525,8 @@ PQcancel(PGcancel *cancel, char *errbuf, int errbufsize) int save_errno = SOCK_ERRNO; pgsocket tmpsock = PGINVALID_SOCKET; int maxlen; - struct - { - uint32 packetlen; - CancelRequestPacket cp; - } crp; + char recvbuf; + int cancel_pkt_len; if (!cancel) { @@ -571,15 +628,15 @@ retry3: goto cancel_errReturn; } - /* Create and send the cancel request packet. */ - - crp.packetlen = pg_hton32((uint32) sizeof(crp)); - crp.cp.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); - crp.cp.backendPID = pg_hton32(cancel->be_pid); - crp.cp.cancelAuthCode = pg_hton32(cancel->be_key); + cancel_pkt_len = pg_ntoh32(cancel->cancel_pkt_len); retry4: - if (send(tmpsock, (char *) &crp, sizeof(crp), 0) != (int) sizeof(crp)) + + /* + * Send the cancel request packet. It starts with the message length at + * cancel_pkt_len, followed by the actual packet. + */ + if (send(tmpsock, (char *) &cancel->cancel_pkt_len, cancel_pkt_len, 0) != cancel_pkt_len) { if (SOCK_ERRNO == EINTR) /* Interrupted system call - we'll just try again */ @@ -596,7 +653,7 @@ retry4: * read to obtain any data, we are just waiting for EOF to be signaled. */ retry5: - if (recv(tmpsock, (char *) &crp, 1, 0) < 0) + if (recv(tmpsock, &recvbuf, 1, 0) < 0) { if (SOCK_ERRNO == EINTR) /* Interrupted system call - we'll just try again */ diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index a5055271ae..d8aca4d973 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -243,6 +243,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Fallback-Application-Name", "", 64, offsetof(struct pg_conn, fbappname)}, + {"protocol_version", NULL, NULL, NULL, + "Protocol-Version", "", 5, + offsetof(struct pg_conn, protocol_version_str)}, + {"keepalives", NULL, NULL, NULL, "TCP-Keepalives", "", 1, /* should be just '0' or '1' */ offsetof(struct pg_conn, keepalives)}, @@ -636,7 +640,12 @@ pqDropServerData(PGconn *conn) if (!conn->cancelRequest) { conn->be_pid = 0; - conn->be_key = 0; + if (conn->be_cancel_key != NULL) + { + free(conn->be_cancel_key); + conn->be_cancel_key = NULL; + } + conn->be_cancel_key_len = 0; } } @@ -1762,6 +1771,29 @@ pqConnectOptions2(PGconn *conn) goto oom_error; } + /* + * Validate protocol_version + */ + if (conn->protocol_version_str) + { + if (strcmp(conn->protocol_version_str, "auto") == 0) + conn->requested_pversion = 0; + else if (strcmp(conn->protocol_version_str, "3.1") == 0) + conn->requested_pversion = PG_PROTOCOL(3, 1); + else if (strcmp(conn->protocol_version_str, "3.0") == 0) + conn->requested_pversion = PG_PROTOCOL(3, 0); + else + { + conn->status = CONNECTION_BAD; + libpq_append_conn_error(conn, "invalid %s value: \"%s\"", + "protocol_version", + conn->protocol_version_str); + return false; + } + } + else + conn->requested_pversion = 0; + /* * validate target_session_attrs option, and set target_server_type */ @@ -2836,7 +2868,10 @@ keep_going: /* We will come back to here until there is * must persist across individual connection attempts, but we must * reset them when we start to consider a new server. */ - conn->pversion = PG_PROTOCOL(3, 0); + if (conn->requested_pversion == 0) + conn->pversion = PG_PROTOCOL(3, 1); + else + conn->pversion = conn->requested_pversion; conn->send_appname = true; conn->failed_enc_methods = 0; conn->current_enc_method = 0; @@ -3401,13 +3436,7 @@ keep_going: /* We will come back to here until there is */ if (conn->cancelRequest) { - CancelRequestPacket cancelpacket; - - packetlen = sizeof(cancelpacket); - cancelpacket.cancelRequestCode = (MsgType) pg_hton32(CANCEL_REQUEST_CODE); - cancelpacket.backendPID = pg_hton32(conn->be_pid); - cancelpacket.cancelAuthCode = pg_hton32(conn->be_key); - if (pqPacketSend(conn, 0, &cancelpacket, packetlen) != STATUS_OK) + if (pqSendCancelRequest(conn) != STATUS_OK) { libpq_append_conn_error(conn, "could not send cancel packet: %s", SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf))); @@ -3872,12 +3901,17 @@ keep_going: /* We will come back to here until there is { if (pqGetNegotiateProtocolVersion3(conn)) { - libpq_append_conn_error(conn, "received invalid protocol negotiation message"); + /* + * Negotiation failed. The error message was filled + * in already. + */ goto error_return; } /* OK, we read the message; mark data consumed */ + conn->inStart = conn->inCursor; - goto error_return; + /* Stay in the CONNECTION_AWAITING_RESPONSE state */ + 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 3170d484f0..50bec54068 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -48,6 +48,7 @@ static int getRowDescriptions(PGconn *conn, int msgLength); static int getParamDescriptions(PGconn *conn, int msgLength); static int getAnotherTuple(PGconn *conn, int msgLength); static int getParameterStatus(PGconn *conn); +static int getBackendKeyData(PGconn *conn); static int getNotify(PGconn *conn); static int getCopyStart(PGconn *conn, ExecStatusType copytype); static int getReadyForQuery(PGconn *conn); @@ -308,9 +309,7 @@ pqParseInput3(PGconn *conn) * just as easy to handle it as part of the main loop. * Save the data and continue processing. */ - if (pqGetInt(&(conn->be_pid), 4, conn)) - return; - if (pqGetInt(&(conn->be_key), 4, conn)) + if (getBackendKeyData(conn)) return; break; case PqMsg_RowDescription: @@ -1405,8 +1404,8 @@ reportErrorPosition(PQExpBuffer msg, const char *query, int loc, int encoding) /* * Attempt to read a NegotiateProtocolVersion message. * Entry: 'v' message type and length have already been consumed. - * Exit: returns 0 if successfully consumed message. - * returns EOF if not enough data. + * Exit: returns 0 if successfully consumed message and the negotiation succeeded. + * returns 1 on failure. The error message is filled in. */ int pqGetNegotiateProtocolVersion3(PGconn *conn) @@ -1417,11 +1416,11 @@ pqGetNegotiateProtocolVersion3(PGconn *conn) PQExpBufferData buf; if (pqGetInt(&tmp, 4, conn) != 0) - return EOF; + goto eof; their_version = tmp; if (pqGetInt(&num, 4, conn) != 0) - return EOF; + goto eof; initPQExpBuffer(&buf); for (int i = 0; i < num; i++) @@ -1429,17 +1428,36 @@ pqGetNegotiateProtocolVersion3(PGconn *conn) if (pqGets(&conn->workBuffer, conn)) { termPQExpBuffer(&buf); - return EOF; + goto eof; } if (buf.len > 0) appendPQExpBufferChar(&buf, ' '); appendPQExpBufferStr(&buf, conn->workBuffer.data); } - if (their_version < conn->pversion) + /* + * If a specific protocol version was requested by the user and the server + * does not support that exact version -> fail. + */ + if (conn->requested_pversion != 0 && their_version != conn->pversion) + { + libpq_append_conn_error(conn, "requested protocol version %u.%u not supported by server: 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)); + return 1; + } + + /* + * If we don't support any of the server's supported versions -> fail + */ + if (their_version < PG_PROTOCOL(3, 0)) + { 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)); + return 1; + } + if (num > 0) { appendPQExpBuffer(&conn->errorMessage, @@ -1451,10 +1469,19 @@ pqGetNegotiateProtocolVersion3(PGconn *conn) /* neither -- server shouldn't have sent it */ if (!(their_version < conn->pversion) && !(num > 0)) + { libpq_append_conn_error(conn, "invalid %s message", "NegotiateProtocolVersion"); + return 1; + } + + conn->pversion = their_version; termPQExpBuffer(&buf); return 0; + +eof: + libpq_append_conn_error(conn, "insufficient data in \"v\" (NegotiateProtocolVersion) message"); + return 1; } @@ -1486,6 +1513,52 @@ getParameterStatus(PGconn *conn) return 0; } +/* + * parseInput subroutine to read a BackendKeyData message. + * Entry: 'v' message type and length have already been consumed. + * Exit: returns 0 if successfully consumed message. + * returns EOF if not enough data. + */ +static int +getBackendKeyData(PGconn *conn) +{ + uint8 cancel_key_len; + + if (conn->be_cancel_key) + { + free(conn->be_cancel_key); + conn->be_cancel_key = NULL; + conn->be_cancel_key_len = 0; + } + + if (pqGetInt(&(conn->be_pid), 4, conn)) + return EOF; + + if (conn->pversion >= PG_PROTOCOL(3, 1)) + { + if (pqGetc((char *) &cancel_key_len, conn)) + return EOF; + } + else + cancel_key_len = 4; + + conn->be_cancel_key = malloc(cancel_key_len); + if (conn->be_cancel_key == NULL) + { + libpq_append_conn_error(conn, "out of memory"); + /* discard the message */ + return EOF; + } + if (pqGetnchar(conn->be_cancel_key, cancel_key_len, conn)) + { + free(conn->be_cancel_key); + conn->be_cancel_key = NULL; + return EOF; + } + conn->be_cancel_key_len = cancel_key_len; + return 0; +} + /* * Attempt to read a Notify response message. diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index d97b595c97..727110fb0f 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -397,6 +397,7 @@ struct pg_conn char *pguser; /* Postgres username and password, if any */ char *pgpass; char *pgpassfile; /* path to a file containing password(s) */ + char *protocol_version_str; /* protocol version to use */ char *channel_binding; /* channel binding mode * (require,prefer,disable) */ char *keepalives; /* use TCP keepalives? */ @@ -507,6 +508,8 @@ struct pg_conn * sending */ /* Transient state needed while establishing connection */ + ProtocolVersion requested_pversion; /* FE/BE protocol version requested, + * or 0 for "auto" */ PGTargetServerType target_server_type; /* desired session properties */ PGLoadBalanceType load_balance_type; /* desired load balancing * algorithm */ @@ -520,7 +523,8 @@ struct pg_conn /* Miscellaneous stuff */ int be_pid; /* PID of backend --- needed for cancels */ - int be_key; /* key of backend --- needed for cancels */ + char *be_cancel_key; /* query cancellation key and its length */ + uint16 be_cancel_key_len; pgParameterStatus *pstatus; /* ParameterStatus data */ int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ @@ -744,6 +748,10 @@ extern PGresult *pqFunctionCall3(PGconn *conn, Oid fnid, int result_is_int, const PQArgBlock *args, int nargs); +/* === in fe-cancel.c === */ + +extern int pqSendCancelRequest(PGconn *cancelConn); + /* === in fe-misc.c === */ /* diff --git a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl index f9e6d07fc0..c543e366bb 100644 --- a/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl +++ b/src/test/modules/libpq_pipeline/t/001_libpq_pipeline.pl @@ -72,6 +72,14 @@ for my $testname (@tests) } } +# There were changes to query cancellation in protocol version 3.1, so +# test separately that it still works the old protocol version too. +$node->command_ok( + [ + 'libpq_pipeline', 'cancel', $node->connstr('postgres') . " protocol_version=3.0" + ], + "libpq_pipeline cancel with protocol 3.0"); + $node->stop('fast'); done_testing(); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 547d14b3e7..c52e772442 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -375,6 +375,7 @@ CachedPlanSource CallContext CallStmt CancelRequestPacket +CancelRequestExtendedPacket Cardinality CaseExpr CaseKind -- 2.39.2