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 files changed, 12 insertions, 11 deletions

diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 7d201965503..b7c39a4c5f0 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,12 +162,13 @@ 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;
 	uint32		old_pss_pid;
 
+	Assert(cancel_key_len >= 0 && cancel_key_len <= MAX_CANCEL_KEY_LENGTH);
 	if (MyProcNumber < 0)
 		elog(ERROR, "MyProcNumber not set");
 	if (MyProcNumber >= NumProcSignalSlots)
@@ -199,8 +199,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);
@@ -254,8 +255,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
@@ -725,7 +725,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);
 
@@ -754,7 +754,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);