From 419eadc67021fd98cc7f0a661e315eefd27f3789 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sat, 9 Mar 2024 00:16:31 +0200 Subject: [PATCH v2 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. To make it harder to guess the key, make it larger. 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. If we wanted to stay compatible, we could teach libpq to reconnect with protocol version 3.0, but I don't think that's necessary. Reviewed-by: Jelte Fennema-Nio Discussion: https://www.postgresql.org/message-id/508d0505-8b7a-4864-a681-e7e5edfe32aa@iki.fi --- doc/src/sgml/protocol.sgml | 120 +++++++++++++++++++++++++--- src/backend/postmaster/postmaster.c | 71 +++++++++++----- src/backend/storage/ipc/pmsignal.c | 42 +++++++--- src/backend/storage/lmgr/proc.c | 10 ++- src/backend/tcop/postgres.c | 12 ++- src/backend/utils/init/globals.c | 1 - src/include/libpq/pqcomm.h | 16 +++- src/include/miscadmin.h | 3 +- src/include/storage/pmsignal.h | 7 +- src/interfaces/libpq/fe-cancel.c | 59 ++++++++++---- src/interfaces/libpq/fe-connect.c | 32 ++++++-- src/interfaces/libpq/fe-protocol3.c | 66 +++++++++++++-- src/interfaces/libpq/libpq-int.h | 8 +- 13 files changed, 366 insertions(+), 81 deletions(-) diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml index a5cb19357f5..357a7cb2fb0 100644 --- a/doc/src/sgml/protocol.sgml +++ b/doc/src/sgml/protocol.sgml @@ -18,9 +18,11 @@ - This document describes version 3.0 of the protocol, implemented in + This document describes version 3.1 of the protocol, introduces in + PostgreSQL 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 + 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 @@ -34,6 +36,17 @@ minor protocol version which it supports. The client may then choose either to continue with the connection using the specified protocol version or to abort the connection. + + TODO: mention that the minor version negotiation was introduced PostgreSQL 9.3 + libpq can no longer connect to PostgreSQL 9.2. Maybe a support matrix? + + + + The difference between protocol version 3.0 and 3.1 is that the secret key + used query cancellation was enlarged from 4 bytes to a variable length + field. The BackendKeyData message was changed to accomodate that, and a new + CancelRequestExtended message with variable length secret was introduced to + replace the old CancelRequest message. @@ -405,8 +418,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. @@ -1381,7 +1400,7 @@ SELCT 1/0; To issue a cancel request, the frontend opens a new connection to - the server and sends a CancelRequest message, rather than the + the server and sends a cancel request message (CancelRequestExtended or CancelRequest), rather StartupMessage message that would ordinarily be sent across a new connection. The server will process this request and then close the connection. For security reasons, no direct reply is made to @@ -1389,7 +1408,7 @@ SELCT 1/0; - A CancelRequest message will be ignored unless it contains the + A cancel request message will be ignored unless it contains the same key data (PID and secret key) passed to the frontend during connection start-up. If the request matches the PID and secret key for a currently executing backend, the processing of the @@ -1531,7 +1550,7 @@ SELCT 1/0; An initial SSLRequest can also be used in a connection that is being - opened to send a CancelRequest message. + opened to send a CancelRequest or CancelRequestExtended message. @@ -1607,7 +1626,7 @@ SELCT 1/0; An initial GSSENCRequest can also be used in a connection that is being - opened to send a CancelRequest message. + opened to send a CancelRequest or CancelRequestExtended message. @@ -3852,13 +3871,13 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" Identifies the message as cancellation key data. The frontend must save these values if it wishes to be - able to issue CancelRequest messages later. + able to issue cancel request messages later. - Int32(12) + Int32 Length of message contents in bytes, including self. @@ -3876,7 +3895,21 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" - Int32 + Int16 + + + 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. @@ -4088,8 +4121,69 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" + + Byte4 + + + The secret key for the target backend. + + + + + + This is for the older protocol version 3.0. Starting with protocol + version 3.1, the secret key is longer and the new CancelRequestExtended + message must be used instead. + + + + + + CancelRequestExtended (F) + + + + Int32(16) + + + Length of message contents in bytes, including self. + + + + + + Int32(80877101) + + + The cancel request code. The value is chosen to contain + 1234 in the most significant 16 bits, + and 5677 in the least significant 16 bits. (To + avoid confusion, this code must not be the same as any protocol + version number.) + + + + Int32 + + + The process ID of the target backend. + + + + + + Int16 + + + Length of the secret key. + + + + + + Byten The secret key for the target backend. @@ -4097,6 +4191,10 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;" + + This message was added in protocol version 3.1. In protocol version 3.0, + use the CancelRequest message instead. + diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 1eefe652e8e..97f4372130e 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -420,7 +420,8 @@ static int ServerLoop(void); static int BackendStartup(Port *port); static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done); static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options); -static void processCancelRequest(Port *port, void *pkt); +static void processExtendedCancelRequestPacket(Port *port, void *pkt, int pktlen); +static void processCancelRequestPacket(Port *port, void *pkt, int pktlen); static void report_fork_failure_to_client(Port *port, int errnum); static CAC_state canAcceptConnections(int backend_type); static void signal_child(pid_t pid, int signal); @@ -2005,16 +2006,15 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done) */ port->proto = proto = pg_ntoh32(*((ProtocolVersion *) buf)); + if (proto == EXTENDED_CANCEL_REQUEST_CODE) + { + processExtendedCancelRequestPacket(port, buf, len); + /* Not really an error, but we don't want to proceed further */ + return STATUS_ERROR; + } if (proto == CANCEL_REQUEST_CODE) { - if (len != sizeof(CancelRequestPacket)) - { - ereport(COMMERROR, - (errcode(ERRCODE_PROTOCOL_VIOLATION), - errmsg("invalid length of startup packet"))); - return STATUS_ERROR; - } - processCancelRequest(port, buf); + processCancelRequestPacket(port, buf, len); /* Not really an error, but we don't want to proceed further */ return STATUS_ERROR; } @@ -2311,21 +2311,54 @@ SendNegotiateProtocolVersion(List *unrecognized_protocol_options) } /* - * 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. + * The client has sent an ExtendedCancelRequest cancel request packet, not a normal + * start-a-new-connection packet. Perform the necessary processing. Nothing + * is sent back to the client. */ static void -processCancelRequest(Port *port, void *pkt) +processExtendedCancelRequestPacket(Port *port, void *pkt, int pktlen) { - CancelRequestPacket *canc = (CancelRequestPacket *) pkt; - int backendPID; - int32 cancelAuthCode; + ExtendedCancelRequestPacket *canc; + int len; - backendPID = (int) pg_ntoh32(canc->backendPID); - cancelAuthCode = (int32) pg_ntoh32(canc->cancelAuthCode); + if (pktlen < offsetof(ExtendedCancelRequestPacket, cancelAuthCode)) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("invalid length of extended query cancel packet"))); + return; + } + canc = (ExtendedCancelRequestPacket *) pkt; + len = pg_ntoh16(canc->cancelAuthCodeLen); + if (pktlen != offsetof(ExtendedCancelRequestPacket, cancelAuthCode) + len) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("invalid length of extended cancel request packet"))); + return; + } + + ProcessCancelRequest(pg_ntoh32(canc->backendPID), canc->cancelAuthCode, len); +} - SendCancelRequest(backendPID, cancelAuthCode); +/* + * like processExtendedCancelRequestPacket, but for the old protocol version + * 3.0 CancelRequest message + */ +static void +processCancelRequestPacket(Port *port, void *pkt, int pktlen) +{ + CancelRequestPacket *canc; + + if (pktlen != sizeof(CancelRequestPacket)) + { + ereport(COMMERROR, + (errcode(ERRCODE_PROTOCOL_VIOLATION), + errmsg("invalid length of query cancel packet"))); + return; + } + canc = (CancelRequestPacket *) pkt; + ProcessCancelRequest(pg_ntoh32(canc->backendPID), canc->cancelAuthCode, 4); } /* diff --git a/src/backend/storage/ipc/pmsignal.c b/src/backend/storage/ipc/pmsignal.c index 9516b869cb2..cac4de5be06 100644 --- a/src/backend/storage/ipc/pmsignal.c +++ b/src/backend/storage/ipc/pmsignal.c @@ -72,7 +72,8 @@ typedef struct ChildSlotData pg_atomic_uint32 state; int pid; - int32 cancel_key; + int cancel_key_len; + char cancel_key[MAX_CANCEL_KEY_LENGTH]; } ChildSlotData; /* "typedef struct PMSignalData PMSignalData" appears in pmsignal.h */ @@ -328,16 +329,18 @@ IsPostmasterChildWalSender(int slot) * actively using shared memory. This is called in the child process. */ void -MarkPostmasterChildActive(int pid, int32 cancelAuthCode) +MarkPostmasterChildActive(int pid, char *cancelKey, int len) { int slot = MyPMChildSlot; + Assert(len <= MAX_CANCEL_KEY_LENGTH); Assert(slot > 0 && slot <= PMSignalState->num_child_slots); slot--; Assert(pg_atomic_read_u32(&PMSignalState->child_slots[slot].state) == PM_CHILD_ASSIGNED); PMSignalState->child_slots[slot].pid = pid; - PMSignalState->child_slots[slot].cancel_key = cancelAuthCode; - pg_memory_barrier(); + memcpy(PMSignalState->child_slots[slot].cancel_key, cancelKey, len); + PMSignalState->child_slots[slot].cancel_key_len = len; + pg_write_barrier(); pg_atomic_write_u32(&PMSignalState->child_slots[slot].state, PM_CHILD_ACTIVE); } @@ -478,8 +481,22 @@ PostmasterDeathSignalInit(void) #endif /* USE_POSTMASTER_DEATH_SIGNAL */ } +static int +pg_const_time_memcmp(const void *a, const void *b, size_t len) +{ + /* + * FIXME: need a constant time implementation. Implement one somewhere in + * src/port. + */ + return memcmp(a, b, len); +} + +/* + * Find the backend with given PID, and send SIGINT to it if the cancel key + * matches. + */ void -SendCancelRequest(int backendPID, int32 cancelAuthCode) +ProcessCancelRequest(int backendPID, char *cancelKey, int len) { /* * See if we have a matching backend. In the EXEC_BACKEND case, we can no @@ -497,18 +514,23 @@ SendCancelRequest(int backendPID, int32 cancelAuthCode) pg_read_barrier(); if (slot->pid == backendPID) { - if (slot->cancel_key == cancelAuthCode) + /* + * Use pg_const_time_memcmp() to prevent an attacker from using + * timing to reveal the cancel key. + */ + if (len == slot->cancel_key_len && pg_const_time_memcmp(slot->cancel_key, cancelKey, len) == 0) { /* Found a match; signal that backend to cancel current op */ ereport(DEBUG2, (errmsg_internal("processing cancel request: sending SIGINT to process %d", backendPID))); - /* - * FIXME: we used to use signal_child. I believe kill() is - * maybe even more correct, but verify that. - */ + /* If we have setsid(), signal the backend's whole process group */ +#ifdef HAVE_SETSID + kill(-backendPID, SIGINT); +#else kill(backendPID, SIGINT); +#endif } else /* Right PID, wrong key: no way, Jose */ diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index a30bba1499c..5ce86417216 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -36,6 +36,7 @@ #include "access/transam.h" #include "access/twophase.h" #include "access/xlogutils.h" +#include "libpq/libpq-be.h" #include "miscadmin.h" #include "pgstat.h" #include "postmaster/autovacuum.h" @@ -65,6 +66,9 @@ bool log_lock_waits = false; /* Pointer to this process's PGPROC struct, if any */ PGPROC *MyProc = NULL; +char MyCancelKey[MAX_CANCEL_KEY_LENGTH]; +int MyCancelKeyLength; + /* * This spinlock protects the freelist of recycled PGPROC structures. * We cannot use an LWLock because the LWLock manager depends on already @@ -314,7 +318,9 @@ InitProcess(void) * better have something random in the field to prevent unfriendly people * from sending cancels to them. */ - if (!pg_strong_random(&MyCancelKey, sizeof(int32))) + MyCancelKeyLength = (MyProcPort == NULL || MyProcPort->proto >= PG_PROTOCOL(3,1)) + ? MAX_CANCEL_KEY_LENGTH : 4; + if (!pg_strong_random(&MyCancelKey, MyCancelKeyLength)) { ereport(ERROR, (errcode(ERRCODE_INTERNAL_ERROR), @@ -384,7 +390,7 @@ InitProcess(void) */ if (IsUnderPostmaster && !AmAutoVacuumLauncherProcess() && !AmLogicalSlotSyncWorkerProcess()) - MarkPostmasterChildActive(MyProcPid, MyCancelKey); + MarkPostmasterChildActive(MyProcPid, MyCancelKey, MyCancelKeyLength); /* * Initialize all fields of MyProc, except for those previously diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index aec1b194424..9c7b63ef4f8 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4274,7 +4274,17 @@ PostgresMain(const char *dbname, const char *username) pq_beginmessage(&buf, PqMsg_BackendKeyData); pq_sendint32(&buf, (int32) MyProcPid); - pq_sendint32(&buf, (int32) MyCancelKey); + + if (MyProcPort->proto >= PG_PROTOCOL(3, 1)) + { + pq_sendint16(&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 5b536ac50d1..fe702af4cfb 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -46,7 +46,6 @@ int MyProcPid; pg_time_t MyStartTime; TimestampTz MyStartTimestamp; struct Port *MyProcPort; -int32 MyCancelKey; int MyPMChildSlot; /* diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h index 9ae469c86c4..d1cf43f12d0 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 */ @@ -129,6 +129,18 @@ 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. */ +#define EXTENDED_CANCEL_REQUEST_CODE PG_PROTOCOL(1234,5677) + +typedef struct ExtendedCancelRequestPacket +{ + /* 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 */ + uint16 cancelAuthCodeLen; /* length of cancelAuthCode */ + char cancelAuthCode[FLEXIBLE_ARRAY_MEMBER]; /* secret key to + * authorize cancel */ +} ExtendedCancelRequestPacket; + #define CANCEL_REQUEST_CODE PG_PROTOCOL(1234,5678) typedef struct CancelRequestPacket @@ -136,7 +148,7 @@ 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[4]; /* secret key to authorize cancel */ } CancelRequestPacket; diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index f900da61573..ae4abc0a43b 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -191,7 +191,8 @@ extern PGDLLIMPORT pg_time_t MyStartTime; extern PGDLLIMPORT TimestampTz MyStartTimestamp; extern PGDLLIMPORT struct Port *MyProcPort; extern PGDLLIMPORT struct Latch *MyLatch; -extern PGDLLIMPORT int32 MyCancelKey; +extern PGDLLIMPORT char MyCancelKey[]; +extern PGDLLIMPORT int MyCancelKeyLength; extern PGDLLIMPORT int MyPMChildSlot; extern PGDLLIMPORT char OutputFileName[]; diff --git a/src/include/storage/pmsignal.h b/src/include/storage/pmsignal.h index 8dff47e86e4..2573c051b0b 100644 --- a/src/include/storage/pmsignal.h +++ b/src/include/storage/pmsignal.h @@ -57,6 +57,9 @@ typedef enum /* PMSignalData is an opaque struct, details known only within pmsignal.c */ typedef struct PMSignalData PMSignalData; +/* Length of query cancel keys generated. */ +#define MAX_CANCEL_KEY_LENGTH 32 + /* * prototypes for functions in pmsignal.c */ @@ -69,12 +72,12 @@ extern QuitSignalReason GetQuitSignalReason(void); extern int AssignPostmasterChildSlot(void); extern bool ReleasePostmasterChildSlot(int slot); extern bool IsPostmasterChildWalSender(int slot); -extern void MarkPostmasterChildActive(int pid, int32 cancelAuthCode); +extern void MarkPostmasterChildActive(int pid, char *cancelKey, int len); extern void MarkPostmasterChildInactive(void); extern void MarkPostmasterChildWalSender(void); extern bool PostmasterIsAliveInternal(void); extern void PostmasterDeathSignalInit(void); -extern void SendCancelRequest(int backendPID, int32 cancelAuthCode); +extern void ProcessCancelRequest(int backendPID, char *cancelKey, int len); /* diff --git a/src/interfaces/libpq/fe-cancel.c b/src/interfaces/libpq/fe-cancel.c index 51f8d8a78c4..608038cf2be 100644 --- a/src/interfaces/libpq/fe-cancel.c +++ b/src/interfaces/libpq/fe-cancel.c @@ -34,6 +34,8 @@ PGcancel * PQgetCancel(PGconn *conn) { PGcancel *cancel; + int cancel_req_len; + bool use_extended; if (!conn) return NULL; @@ -41,13 +43,20 @@ PQgetCancel(PGconn *conn) if (conn->sock == PGINVALID_SOCKET) return NULL; - cancel = malloc(sizeof(PGcancel)); + use_extended = conn->pversion >= PG_PROTOCOL(3, 1); + if (use_extended) + cancel_req_len = offsetof(ExtendedCancelRequestPacket, cancelAuthCode) + conn->be_cancel_key_len; + else + { + Assert(conn->be_cancel_key_len == 4); + cancel_req_len = sizeof(CancelRequestPacket); + } + 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; @@ -90,6 +99,28 @@ PQgetCancel(PGconn *conn) goto fail; } + if (use_extended) + { + ExtendedCancelRequestPacket *req; + + req = (ExtendedCancelRequestPacket *) &cancel->cancel_req; + req->cancelRequestCode = (MsgType) pg_hton32(EXTENDED_CANCEL_REQUEST_CODE); + req->backendPID = pg_hton32(conn->be_pid); + req->cancelAuthCodeLen = pg_hton16(conn->be_cancel_key_len); + memcpy(req->cancelAuthCode, conn->be_cancel_key, conn->be_cancel_key_len); + } + else + { + CancelRequestPacket *req; + + 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, 4); + } + /* include the length field itself in the length */ + cancel->cancel_pkt_len = pg_hton32(cancel_req_len + 4); + return cancel; fail: @@ -150,11 +181,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) { @@ -256,15 +284,14 @@ 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 */ @@ -281,7 +308,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 d4e10a0c4f3..e47b933e27e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -617,7 +617,12 @@ pqDropServerData(PGconn *conn) free(conn->write_err_msg); conn->write_err_msg = NULL; 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; } @@ -2724,7 +2729,7 @@ 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); + conn->pversion = PG_PROTOCOL(3, 1); conn->send_appname = true; #ifdef USE_SSL /* initialize these values based on SSL mode */ @@ -3724,14 +3729,25 @@ keep_going: /* We will come back to here until there is } else if (beresp == PqMsg_NegotiateProtocolVersion) { - if (pqGetNegotiateProtocolVersion3(conn)) + switch (pqGetNegotiateProtocolVersion3(conn)) { - libpq_append_conn_error(conn, "received invalid protocol negotiation message"); - goto error_return; + case 0: + /* OK, we read the message; mark data consumed */ + conn->inStart = conn->inCursor; + /* Stay in the CONNECTION_AWAITING_RESPONSE state */ + goto keep_going; + case 1: + /* + * Negotiation failed. The error message was + * filled in already. + */ + conn->inStart = conn->inCursor; + goto error_return; + case EOF: + /* We'll come back when there is more data */ + libpq_append_conn_error(conn, "received invalid protocol negotiation message"); + goto error_return; } - /* OK, we read the message; mark data consumed */ - conn->inStart = conn->inCursor; - goto error_return; } /* It is an authentication request. */ diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 701d58e1087..877ba69c4a8 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: @@ -1404,7 +1403,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. + * Exit: returns 0 if successfully consumed message and the negotiation succeeded. + * returns 1 if successfully consumed message and the negotiation failed. * returns EOF if not enough data. */ int @@ -1435,10 +1435,13 @@ pqGetNegotiateProtocolVersion3(PGconn *conn) 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", + 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", /* XXX */ 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, @@ -1450,7 +1453,12 @@ 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; @@ -1485,6 +1493,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) +{ + int 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 (pqGetInt(&cancel_key_len, 2, 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 82c18f870d2..3b46a760d23 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -492,7 +492,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; + uint16 be_cancel_key_len; pgParameterStatus *pstatus; /* ParameterStatus data */ int client_encoding; /* encoding id */ bool std_strings; /* standard_conforming_strings */ @@ -629,7 +630,6 @@ struct pg_cancel { SockAddr raddr; /* Remote address */ int be_pid; /* PID of backend --- needed for cancels */ - int be_key; /* key of backend --- needed for cancels */ int pgtcp_user_timeout; /* tcp user timeout */ int keepalives; /* use TCP keepalives? */ int keepalives_idle; /* time between TCP keepalives */ @@ -637,6 +637,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 */ }; -- 2.39.2