PG-1867 Make pg_tde_restore_encrypt re-use old keys

jeltz · jeltz · commit 80e0bb0b56a2 · 2025-08-19T12:58:08.000+02:00
Unfortunately the logic for generating a new key to protect the stream
cipher used to encrypt the WAL stream in our restore command was based
on totally incorrect assumptions due to how the recovery is implemented.

Recovery is a state machine which can go back and forward between one
mode where it streams from a primary and another where it first tries to
fetch WAL from the archive and if that fails from the pg_wal directory,
and in the pg_wal directory we may have files which are encrypted with
whatever keys were there originally.

To handle all the possible scenarios we remove the ability of
pg_tde_restore_encrypt to generate new keys and just has it use whatever
keys there are in the key file. This unfortunately means we open
ourselves to some attacks on the stream cipher if the system is tricked
into encrypting a different WAL stream at the same TLI and LSN as we
already have encrypted. As far as I know this should be rare under
normal operations since normally e.g. the WAL should be the same in the
archive as the one in pg_wal or which we receive through streaming.

Ideally we would want to fix this but for now it is better to have WAL
encryption with this weakness than to not have it at all.

This also incidentally fixes a bug we discovered caused by generating a
new key only invalidating one key rather than all keys which should have
become invalid, since we no longer generate a new key.
diff --git a/contrib/pg_tde/src/access/pg_tde_xlog_smgr.c b/contrib/pg_tde/src/access/pg_tde_xlog_smgr.c
@@ -266,16 +266,33 @@ TDEXLogSmgrInitWrite(bool encrypt_xlog)
 		pfree(key);
 }
 
+/*
+ * Used by pg_tde_restore_encrypt to simulate being constantly in recovery
+ * since the command does not have access to any information about if we are in
+ * recovery or not.
+ *
+ * Creates a dummy key which points at the very end of the WAL stream.
+ */
 void
-TDEXLogSmgrInitWriteReuseKey()
+TDEXLogSmgrInitWriteOldKeys()
 {
-	WalEncryptionKey *key = pg_tde_read_last_wal_key();
+	WALKeyCacheRec *keys;
+	WalEncryptionKey dummy = {
+		.type = WAL_KEY_TYPE_UNENCRYPTED,
+		.wal_start = {.tli = -1,.lsn = -1}
+	};
 
-	if (key)
+	EncryptionKey = dummy;
+	TDEXLogSetEncKeyLocation(dummy.wal_start);
+
+	keys = pg_tde_get_wal_cache_keys();
+
+	if (keys == NULL)
 	{
-		EncryptionKey = *key;
-		TDEXLogSetEncKeyLocation(EncryptionKey.wal_start);
-		pfree(key);
+		WalLocation start = {.tli = 1,.lsn = 0};
+
+		/* cache is empty, prefetch keys from disk */
+		pg_tde_fetch_wal_keys(start);
 	}
 }
 
diff --git a/contrib/pg_tde/src/bin/pg_tde_restore_encrypt.c b/contrib/pg_tde/src/bin/pg_tde_restore_encrypt.c
@@ -68,7 +68,7 @@ write_encrypted_segment(const char *segpath, const char *segname, const char *tm
 
 	XLogFromFileName(segname, &tli, &segno, walsegsz);
 
-	TDEXLogSmgrInitWriteReuseKey();
+	TDEXLogSmgrInitWriteOldKeys();
 
 	w = xlog_smgr->seg_write(segfd, buf.data, r, pos, tli, segno, walsegsz);
 
diff --git a/contrib/pg_tde/src/include/access/pg_tde_xlog_smgr.h b/contrib/pg_tde/src/include/access/pg_tde_xlog_smgr.h
@@ -11,7 +11,7 @@ extern Size TDEXLogEncryptStateSize(void);
 extern void TDEXLogShmemInit(void);
 extern void TDEXLogSmgrInit(void);
 extern void TDEXLogSmgrInitWrite(bool encrypt_xlog);
-extern void TDEXLogSmgrInitWriteReuseKey(void);
+extern void TDEXLogSmgrInitWriteOldKeys(void);
 
 extern void TDEXLogCryptBuffer(const void *buf, void *out_buf, size_t count, off_t offset,
 							   TimeLineID tli, XLogSegNo segno, int segSize);
diff --git a/contrib/pg_tde/t/wal_archiving.pl b/contrib/pg_tde/t/wal_archiving.pl
@@ -58,7 +58,7 @@
 unlike(
 	`strings $data_dir/pg_wal/0000000100000000000000??`,
 	qr/foobar_enc/,
-	'should not find foobar_enc in WAL');
+	'should not find foobar_enc in WAL since it is encrypted');
 
 $primary->stop;
 
@@ -84,10 +84,10 @@
 
 $data_dir = $replica->data_dir;
 
-unlike(
+like(
 	`strings $data_dir/pg_wal/0000000100000000000000??`,
 	qr/foobar_plain/,
-	'should not find foobar_plain in WAL since it is encrypted');
+	'should find foobar_plain in WAL since we use the same key file');
 unlike(
 	`strings $data_dir/pg_wal/0000000100000000000000??`,
 	qr/foobar_enc/,
