Skip to content

Commit a460251

Browse files
hlinnakahlinnaka
committed
Make cancel request keys longer
Currently, the cancel request key is a 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 longer cancellation keys are generated when using the new protocol version 3.2. For connections using version 3.0, short 4-bytes keys are still used. The new longer key length is not hardcoded in the protocol anymore, the client is expected to deal with variable length keys, up to 256 bytes. This flexibility allows e.g. a connection pooler to add more information to the cancel key, which might be useful for finding the connection. Reviewed-by: Jelte Fennema-Nio <postgres@jeltef.nl> Reviewed-by: Robert Haas <robertmhaas@gmail.com> (earlier versions) Discussion: https://www.postgresql.org/message-id/508d0505-8b7a-4864-a681-e7e5edfe32aa@iki.fi
1 parent 285613c commit a460251

File tree

14 files changed

+252
-84
lines changed

14 files changed

+252
-84
lines changed

‎doc/src/sgml/protocol.sgml

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4062,7 +4062,7 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
40624062
</varlistentry>
40634063

40644064
<varlistentry>
4065-
<term>Int32(12)</term>
4065+
<term>Int32</term>
40664066
<listitem>
40674067
<para>
40684068
Length of message contents in bytes, including self.
@@ -4080,14 +4080,29 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
40804080
</varlistentry>
40814081

40824082
<varlistentry>
4083-
<term>Int32</term>
4083+
<term>Byte<replaceable>n</replaceable></term>
40844084
<listitem>
40854085
<para>
4086-
The secret key of this backend.
4086+
The secret key of this backend. This field extends to the end of the
4087+
message, indicated by the length field.
4088+
</para>
4089+
<para>
4090+
The maximum key length is 256 bytes. The
4091+
<productname>PostgreSQL</productname> server only sends keys up to
4092+
32 bytes, but the larger maximum size allows for future server
4093+
versions, as well as connection poolers and other middleware, to use
4094+
longer keys. One possible use case is augmenting the server's key
4095+
with extra information. Middleware is therefore also encouraged to
4096+
not use up all of the bytes, in case multiple middleware
4097+
applications are layered on top of each other, each of which may
4098+
wrap the key with extra data.
40874099
</para>
40884100
</listitem>
40894101
</varlistentry>
40904102
</variablelist>
4103+
<para>
4104+
Before protocol version 3.2, the secret key was always 4 bytes long.
4105+
</para>
40914106
</listitem>
40924107
</varlistentry>
40934108

@@ -4293,14 +4308,18 @@ psql "dbname=postgres replication=database" -c "IDENTIFY_SYSTEM;"
42934308
</varlistentry>
42944309

42954310
<varlistentry>
4296-
<term>Int32</term>
4311+
<term>Byte<replaceable>n</replaceable></term>
42974312
<listitem>
42984313
<para>
4299-
The secret key for the target backend.
4314+
The secret key for the target backend. This field extends to the end of the
4315+
message, indicated by the length field. The maximum key length is 256 bytes.
43004316
</para>
43014317
</listitem>
43024318
</varlistentry>
43034319
</variablelist>
4320+
<para>
4321+
Before protocol version 3.2, the secret key was always 4 bytes long.
4322+
</para>
43044323
</listitem>
43054324
</varlistentry>
43064325

‎src/backend/storage/ipc/procsignal.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@
6363
typedef struct
6464
{
6565
pg_atomic_uint32 pss_pid;
66-
bool pss_cancel_key_valid;
67-
int32 pss_cancel_key;
66+
int pss_cancel_key_len; /* 0 means no cancellation is possible */
67+
char pss_cancel_key[MAX_CANCEL_KEY_LENGTH];
6868
volatile sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS];
6969
slock_t pss_mutex; /* protects the above fields */
7070

@@ -148,8 +148,7 @@ ProcSignalShmemInit(void)
148148

149149
SpinLockInit(&slot->pss_mutex);
150150
pg_atomic_init_u32(&slot->pss_pid, 0);
151-
slot->pss_cancel_key_valid = false;
152-
slot->pss_cancel_key = 0;
151+
slot->pss_cancel_key_len = 0;
153152
MemSet(slot->pss_signalFlags, 0, sizeof(slot->pss_signalFlags));
154153
pg_atomic_init_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX);
155154
pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0);
@@ -163,12 +162,13 @@ ProcSignalShmemInit(void)
163162
* Register the current process in the ProcSignal array
164163
*/
165164
void
166-
ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
165+
ProcSignalInit(char *cancel_key, int cancel_key_len)
167166
{
168167
ProcSignalSlot *slot;
169168
uint64 barrier_generation;
170169
uint32 old_pss_pid;
171170

171+
Assert(cancel_key_len >= 0 && cancel_key_len <= MAX_CANCEL_KEY_LENGTH);
172172
if (MyProcNumber < 0)
173173
elog(ERROR, "MyProcNumber not set");
174174
if (MyProcNumber >= NumProcSignalSlots)
@@ -199,8 +199,9 @@ ProcSignalInit(bool cancel_key_valid, int32 cancel_key)
199199
pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
200200
pg_atomic_write_u64(&slot->pss_barrierGeneration, barrier_generation);
201201

202-
slot->pss_cancel_key_valid = cancel_key_valid;
203-
slot->pss_cancel_key = cancel_key;
202+
if (cancel_key_len > 0)
203+
memcpy(slot->pss_cancel_key, cancel_key, cancel_key_len);
204+
slot->pss_cancel_key_len = cancel_key_len;
204205
pg_atomic_write_u32(&slot->pss_pid, MyProcPid);
205206

206207
SpinLockRelease(&slot->pss_mutex);
@@ -254,8 +255,7 @@ CleanupProcSignalState(int status, Datum arg)
254255

255256
/* Mark the slot as unused */
256257
pg_atomic_write_u32(&slot->pss_pid, 0);
257-
slot->pss_cancel_key_valid = false;
258-
slot->pss_cancel_key = 0;
258+
slot->pss_cancel_key_len = 0;
259259

260260
/*
261261
* Make this slot look like it's absorbed all possible barriers, so that
@@ -725,7 +725,7 @@ procsignal_sigusr1_handler(SIGNAL_ARGS)
725725
* fields in the ProcSignal slots.
726726
*/
727727
void
728-
SendCancelRequest(int backendPID, int32 cancelAuthCode)
728+
SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len)
729729
{
730730
Assert(backendPID != 0);
731731

@@ -754,7 +754,8 @@ SendCancelRequest(int backendPID, int32 cancelAuthCode)
754754
}
755755
else
756756
{
757-
match = slot->pss_cancel_key_valid && slot->pss_cancel_key == cancelAuthCode;
757+
match = slot->pss_cancel_key_len == cancel_key_len &&
758+
timingsafe_bcmp(slot->pss_cancel_key, cancel_key, cancel_key_len) == 0;
758759

759760
SpinLockRelease(&slot->pss_mutex);
760761

‎src/backend/tcop/backend_startup.c

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ ConnectionTiming conn_timing = {.ready_for_use = TIMESTAMP_MINUS_INFINITY};
6060
static void BackendInitialize(ClientSocket *client_sock, CAC_state cac);
6161
static int ProcessSSLStartup(Port *port);
6262
static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
63+
static void ProcessCancelRequestPacket(Port *port, void *pkt, int pktlen);
6364
static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
6465
static void process_startup_packet_die(SIGNAL_ARGS);
6566
static void StartupPacketTimeoutHandler(void);
@@ -565,28 +566,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
565566

566567
if (proto == CANCEL_REQUEST_CODE)
567568
{
568-
/*
569-
* The client has sent a cancel request packet, not a normal
570-
* start-a-new-connection packet. Perform the necessary processing.
571-
* Nothing is sent back to the client.
572-
*/
573-
CancelRequestPacket *canc;
574-
int backendPID;
575-
int32 cancelAuthCode;
576-
577-
if (len != sizeof(CancelRequestPacket))
578-
{
579-
ereport(COMMERROR,
580-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
581-
errmsg("invalid length of startup packet")));
582-
return STATUS_ERROR;
583-
}
584-
canc = (CancelRequestPacket *) buf;
585-
backendPID = (int) pg_ntoh32(canc->backendPID);
586-
cancelAuthCode = (int32) pg_ntoh32(canc->cancelAuthCode);
587-
588-
if (backendPID != 0)
589-
SendCancelRequest(backendPID, cancelAuthCode);
569+
ProcessCancelRequestPacket(port, buf, len);
590570
/* Not really an error, but we don't want to proceed further */
591571
return STATUS_ERROR;
592572
}
@@ -886,6 +866,37 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
886866
return STATUS_OK;
887867
}
888868

869+
/*
870+
* The client has sent a cancel request packet, not a normal
871+
* start-a-new-connection packet. Perform the necessary processing. Nothing
872+
* is sent back to the client.
873+
*/
874+
static void
875+
ProcessCancelRequestPacket(Port *port, void *pkt, int pktlen)
876+
{
877+
CancelRequestPacket *canc;
878+
int len;
879+
880+
if (pktlen < offsetof(CancelRequestPacket, cancelAuthCode))
881+
{
882+
ereport(COMMERROR,
883+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
884+
errmsg("invalid length of query cancel packet")));
885+
return;
886+
}
887+
len = pktlen - offsetof(CancelRequestPacket, cancelAuthCode);
888+
if (len == 0 || len > 256)
889+
{
890+
ereport(COMMERROR,
891+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
892+
errmsg("invalid length of query cancel key")));
893+
return;
894+
}
895+
896+
canc = (CancelRequestPacket *) pkt;
897+
SendCancelRequest(pg_ntoh32(canc->backendPID), canc->cancelAuthCode, len);
898+
}
899+
889900
/*
890901
* Send a NegotiateProtocolVersion to the client. This lets the client know
891902
* that they have either requested a newer minor protocol version than we are

‎src/backend/tcop/postgres.c

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4259,16 +4259,20 @@ PostgresMain(const char *dbname, const char *username)
42594259
* Generate a random cancel key, if this is a backend serving a
42604260
* connection. InitPostgres() will advertise it in shared memory.
42614261
*/
4262-
Assert(!MyCancelKeyValid);
4262+
Assert(MyCancelKeyLength == 0);
42634263
if (whereToSendOutput == DestRemote)
42644264
{
4265-
if (!pg_strong_random(&MyCancelKey, sizeof(int32)))
4265+
int len;
4266+
4267+
len = (MyProcPort == NULL || MyProcPort->proto >= PG_PROTOCOL(3, 2))
4268+
? MAX_CANCEL_KEY_LENGTH : 4;
4269+
if (!pg_strong_random(&MyCancelKey, len))
42664270
{
42674271
ereport(ERROR,
42684272
(errcode(ERRCODE_INTERNAL_ERROR),
42694273
errmsg("could not generate random cancel key")));
42704274
}
4271-
MyCancelKeyValid = true;
4275+
MyCancelKeyLength = len;
42724276
}
42734277

42744278
/*
@@ -4323,10 +4327,11 @@ PostgresMain(const char *dbname, const char *username)
43234327
{
43244328
StringInfoData buf;
43254329

4326-
Assert(MyCancelKeyValid);
4330+
Assert(MyCancelKeyLength > 0);
43274331
pq_beginmessage(&buf, PqMsg_BackendKeyData);
43284332
pq_sendint32(&buf, (int32) MyProcPid);
4329-
pq_sendint32(&buf, (int32) MyCancelKey);
4333+
4334+
pq_sendbytes(&buf, MyCancelKey, MyCancelKeyLength);
43304335
pq_endmessage(&buf);
43314336
/* Need not flush since ReadyForQuery will do it. */
43324337
}

‎src/backend/utils/init/globals.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "miscadmin.h"
2525
#include "postmaster/postmaster.h"
2626
#include "storage/procnumber.h"
27+
#include "storage/procsignal.h"
2728

2829

2930
ProtocolVersion FrontendProtocol;
@@ -48,8 +49,8 @@ pg_time_t MyStartTime;
4849
TimestampTz MyStartTimestamp;
4950
struct ClientSocket *MyClientSocket;
5051
struct Port *MyProcPort;
51-
bool MyCancelKeyValid = false;
52-
int32 MyCancelKey = 0;
52+
char MyCancelKey[MAX_CANCEL_KEY_LENGTH];
53+
uint8 MyCancelKeyLength = 0;
5354
int MyPMChildSlot;
5455

5556
/*

‎src/backend/utils/init/postinit.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ InitPostgres(const char *in_dbname, Oid dboid,
753753
*/
754754
SharedInvalBackendInit(false);
755755

756-
ProcSignalInit(MyCancelKeyValid, MyCancelKey);
756+
ProcSignalInit(MyCancelKey, MyCancelKeyLength);
757757

758758
/*
759759
* Also set up timeout handlers needed for backend operation. We need

‎src/include/libpq/pqcomm.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,21 @@ typedef uint32 AuthRequest;
128128
*
129129
* The cancel request code must not match any protocol version number
130130
* we're ever likely to use. This random choice should do.
131+
*
132+
* Before PostgreSQL v18 and the protocol version bump from 3.0 to 3.2, the
133+
* cancel key was always 4 bytes. With protocol version 3.2, it's variable
134+
* length.
131135
*/
136+
132137
#define CANCEL_REQUEST_CODE PG_PROTOCOL(1234,5678)
133138

134139
typedef struct CancelRequestPacket
135140
{
136141
/* Note that each field is stored in network byte order! */
137142
MsgType cancelRequestCode; /* code to identify a cancel request */
138143
uint32 backendPID; /* PID of client's backend */
139-
uint32 cancelAuthCode; /* secret key to authorize cancel */
144+
char cancelAuthCode[FLEXIBLE_ARRAY_MEMBER]; /* secret key to
145+
* authorize cancel */
140146
} CancelRequestPacket;
141147

142148
/* Application-Layer Protocol Negotiation is required for direct connections

‎src/include/miscadmin.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,8 @@ extern PGDLLIMPORT pg_time_t MyStartTime;
191191
extern PGDLLIMPORT TimestampTz MyStartTimestamp;
192192
extern PGDLLIMPORT struct Port *MyProcPort;
193193
extern PGDLLIMPORT struct Latch *MyLatch;
194-
extern PGDLLIMPORT bool MyCancelKeyValid;
195-
extern PGDLLIMPORT int32 MyCancelKey;
194+
extern PGDLLIMPORT char MyCancelKey[];
195+
extern PGDLLIMPORT uint8 MyCancelKeyLength;
196196
extern PGDLLIMPORT int MyPMChildSlot;
197197

198198
extern PGDLLIMPORT char OutputFileName[];

‎src/include/storage/procsignal.h

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,26 @@ typedef enum
5656
PROCSIGNAL_BARRIER_SMGRRELEASE, /* ask smgr to close files */
5757
} ProcSignalBarrierType;
5858

59+
/*
60+
* Length of query cancel keys generated.
61+
*
62+
* Note that the protocol allows for longer keys, or shorter, but this is the
63+
* length we actually generate. Client code, and the server code that handles
64+
* incoming cancellation packets from clients, mustn't use this hardcoded
65+
* length.
66+
*/
67+
#define MAX_CANCEL_KEY_LENGTH 32
68+
5969
/*
6070
* prototypes for functions in procsignal.c
6171
*/
6272
extern Size ProcSignalShmemSize(void);
6373
extern void ProcSignalShmemInit(void);
6474

65-
extern void ProcSignalInit(bool cancel_key_valid, int32 cancel_key);
75+
extern void ProcSignalInit(char *cancel_key, int cancel_key_len);
6676
extern int SendProcSignal(pid_t pid, ProcSignalReason reason,
6777
ProcNumber procNumber);
68-
extern void SendCancelRequest(int backendPID, int32 cancelAuthCode);
78+
extern void SendCancelRequest(int backendPID, char *cancel_key, int cancel_key_len);
6979

7080
extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type);
7181
extern void WaitForProcSignalBarrier(uint64 generation);

0 commit comments

Comments
 (0)