security: authenticate key_base_iv in the WAL key file (M2, WAL keys)

meson.build

@@ -384,6 +384,8 @@ tap_tests = [
   't/checksum_hint_bits.pl',
   't/checksum_startup_warning.pl',
   't/wal_encrypt.pl',
+  't/wal_key_auth.pl',
+  't/wal_key_base_iv_migration.pl',
   't/wal_key_tli.pl',
 ]
 

src/access/open_pg_tde_xlog_keys.c

@@ -33,9 +33,25 @@ typedef struct WalKeyFileHeader
 
 typedef struct WalKeyFileEntry
 {
-	uint32		cipher;			/* Cipher type. We support only AES_128 and
-								 * AES_256 for now. */
-	uint32		range_type;		/* WalEncryptionRangeType */
+	uint32		cipher;			/* Part of AAD. Cipher type. We support only
+								 * AES_128 and AES_256 for now. */
+	uint32		range_type;		/* Part of AAD. WalEncryptionRangeType */
+
+	/*
+	 * key_base_iv is the base IV for the WAL data (AES-CTR). It is placed
+	 * here, ahead of range_start, so that it falls within the AEAD additional
+	 * authenticated data (offsetof(WalKeyFileEntry, range_start)). In the v2
+	 * format it lived after aead_tag and was therefore unauthenticated, which
+	 * let an actor with write access to the WAL key file alter the WAL IV
+	 * undetected. See the v2 migration reader below.
+	 */
+	uint8		key_base_iv[INTERNAL_KEY_IV_LEN];
+
+	/*
+	 * range_start is deliberately outside the AAD: it is zero at key creation
+	 * and updated in place by the first WAL write, so it must not be covered
+	 * by the AEAD tag.
+	 */
 	WalLocation range_start;
 
 	/*
@@ -47,11 +63,9 @@ typedef struct WalKeyFileEntry
 	unsigned char entry_iv[MAP_ENTRY_IV_SIZE];
 	unsigned char aead_tag[MAP_ENTRY_AEAD_TAG_SIZE];
 
-	uint8		key_base_iv[INTERNAL_KEY_IV_LEN];
-
 	/*
 	 * WAL keys are AES-CTR (16 or 32 bytes), never AES-256-XTS, so this stays
-	 * at the pre-AES-256-XTS size to keep the WAL key file format unchanged.
+	 * at the pre-AES-256-XTS size.
 	 */
 	uint8		encrypted_key_data[32];
 } WalKeyFileEntry;
@@ -690,10 +704,10 @@ open_pg_tde_initialize_wal_key_file_entry(WalKeyFileEntry *entry,
 				errmsg("could not generate iv for wal key file entry: %s", ERR_error_string(ERR_get_error(), NULL)));
 
 	/*
-	 * TODO: we may want to include `range_start` in AAD. But now it is zero
-	 * on the key (range) creation and later gets updated by the first WAL
-	 * write. The current AAD has a little sense now, so we might want not to
-	 * calculate it at all.
+	 * The AAD runs to range_start, so it authenticates cipher, range_type,
+	 * and key_base_iv. range_start itself is excluded: it is zero at key
+	 * creation and updated in place by the first WAL write, so it cannot be
+	 * under the AEAD tag.
 	 */
 	AesGcmEncrypt(principal_key->keyData, principal_key->keyLength,
 				  entry->entry_iv, MAP_ENTRY_IV_SIZE,
@@ -1007,6 +1021,85 @@ range_from_disk_entry_v1(int fd, off_t *entry_offset, const TDEPrincipalKey *pri
 	return true;
 }
 
+/*
+ * v2 is the layout that shipped before key_base_iv was authenticated: it sits
+ * after aead_tag, so the AAD (offsetof(range_start)) covers only cipher and
+ * range_type. Unlike v1, a v2 entry records its cipher and may hold a 16 or
+ * 32 byte key.
+ */
+typedef struct WalKeyFileEntryV2
+{
+	uint32		cipher;
+	uint32		range_type;
+	WalLocation range_start;
+
+	unsigned char entry_iv[MAP_ENTRY_IV_SIZE];
+	unsigned char aead_tag[MAP_ENTRY_AEAD_TAG_SIZE];
+
+	uint8		key_base_iv[INTERNAL_KEY_IV_LEN];
+
+	uint8		encrypted_key_data[32];
+} WalKeyFileEntryV2;
+
+static bool
+open_pg_tde_read_one_wal_key_file_entry_v2(int fd,
+										   WalKeyFileEntryV2 *entry,
+										   off_t *offset)
+{
+	off_t		bytes_read = 0;
+
+	Assert(entry);
+	Assert(offset);
+
+	bytes_read = pg_pread(fd, entry, sizeof(WalKeyFileEntryV2), *offset);
+
+	/* We've reached the end of the file. */
+	if (bytes_read != sizeof(WalKeyFileEntryV2))
+		return false;
+
+	*offset += bytes_read;
+
+	return true;
+}
+
+static void
+wal_range_from_entry_v2(WalKeyFileEntryV2 *entry, const TDEPrincipalKey *principal_key, WalEncryptionRange *range)
+{
+	Assert(principal_key);
+
+	range->type = entry->range_type;
+	range->start = entry->range_start;
+	range->end.tli = MaxTimeLineID;
+	range->end.lsn = MaxXLogRecPtr;
+	range->key.key_len = open_pg_tde_cipher_key_length(entry->cipher);
+	range->key.cipher = entry->cipher;
+
+	memcpy(range->key.base_iv, entry->key_base_iv, INTERNAL_KEY_IV_LEN);
+	if (!AesGcmDecrypt(principal_key->keyData, principal_key->keyLength,
+					   entry->entry_iv, MAP_ENTRY_IV_SIZE,
+					   (unsigned char *) entry, offsetof(WalKeyFileEntryV2, range_start),
+					   entry->encrypted_key_data, range->key.key_len,
+					   range->key.key,
+					   entry->aead_tag, MAP_ENTRY_AEAD_TAG_SIZE))
+		ereport(ERROR,
+				errmsg("failed to decrypt key, incorrect principal key or corrupted key file"));
+}
+
+static bool
+range_from_disk_entry_v2(int fd, off_t *entry_offset, const TDEPrincipalKey *principal_key, WalKeyFileEntry *out)
+{
+	WalKeyFileEntryV2 disk_entry;
+	WalEncryptionRange range;
+
+	if (!open_pg_tde_read_one_wal_key_file_entry_v2(fd, &disk_entry, entry_offset))
+		return false;
+
+	wal_range_from_entry_v2(&disk_entry, principal_key, &range);
+	open_pg_tde_initialize_wal_key_file_entry(out, principal_key, &range);
+
+	return true;
+}
+
 void
 open_pg_tde_update_wal_keys_file(void)
 {
@@ -1040,6 +1133,8 @@ open_pg_tde_update_wal_keys_file(void)
 	/* The type check later, when extracting the principal key */
 	if (FILEMAGIC_VERSION(fheader.file_version) == 1)
 		read_range = range_from_disk_entry_v1;
+	else if (FILEMAGIC_VERSION(fheader.file_version) == 2)
+		read_range = range_from_disk_entry_v2;
 	else
 		elog(ERROR, "wal_keys migration: unsupported or corrupted old file version %d", FILEMAGIC_VERSION(fheader.file_version));
 

src/include/open_pg_tde.h

@@ -14,8 +14,10 @@
  * Only numeric version (the most left byte) should be changed when updating
  * file format. Otherwise, it will break the migration process.
  */
-#define OPEN_PG_TDE_WAL_KEY_FILE_MAGIC 0x024B4557	/* version ID value = WEK
-													 * 02 */
+#define OPEN_PG_TDE_WAL_KEY_FILE_MAGIC 0x034B4557	/* version ID value = WEK
+													 * 03; v3 authenticates
+													 * key_base_iv in the AEAD
+													 * AAD (was excluded in v2) */
 #define OPEN_PG_TDE_SMGR_FILE_MAGIC		  0x06454454	/* version ID value =
 														 * TDE 06; v6
 														 * authenticates

t/wal_key_auth.pl

@@ -0,0 +1,81 @@
+# Verifies that key_base_iv in the WAL key file is authenticated (M2, WAL).
+#
+# key_base_iv is the base IV for WAL data (AES-CTR). In the version-2 on-disk
+# format it was stored outside the AEAD additional authenticated data, so an
+# actor with write access to the WAL key file could change it undetected,
+# silently shifting the WAL IV. In version 3 it is part of the AAD, so tampering
+# with it fails decryption of the WAL key when it is read.
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf',
+	"shared_preload_libraries = 'open_pg_tde'");
+$node->append_conf('postgresql.conf', "wal_level = 'logical'");
+$node->start;
+
+my $keydir = PostgreSQL::Test::Utils::tempdir;
+$node->safe_psql(
+	'postgres', qq{
+	CREATE EXTENSION open_pg_tde;
+	SELECT open_pg_tde_add_global_key_provider_file('kp', '$keydir/wal.keys');
+	SELECT open_pg_tde_create_key_using_global_key_provider('k', 'kp');
+	SELECT open_pg_tde_set_server_key_using_global_key_provider('k', 'kp');
+});
+
+$node->safe_psql('postgres',
+	'ALTER SYSTEM SET open_pg_tde.wal_encrypt = on;');
+$node->restart;
+
+# Generate WAL so a WAL key/range is written and encrypted.
+$node->safe_psql(
+	'postgres', q{
+	CREATE TABLE wal_probe (id int);
+	INSERT INTO wal_probe VALUES (1);
+	CHECKPOINT;
+});
+is($node->safe_psql('postgres', 'SHOW open_pg_tde.wal_encrypt;'),
+	'on', 'WAL encryption is enabled before tampering');
+
+$node->stop;
+
+# Flip one byte of key_base_iv in the last WAL key entry, the entry that is read
+# when the server needs the current WAL key.
+#
+# v3 WalKeyFileEntry layout: cipher(4) range_type(4) key_base_iv(16)
+# range_start(16) entry_iv(16) aead_tag(16) encrypted_key_data(32) = 104 bytes.
+# key_base_iv sits at entry offset 8. We address the last entry from the end of
+# the file so we do not depend on the header size.
+use constant WAL_KEY_ENTRY_SIZE => 104;
+use constant KEY_BASE_IV_OFFSET => 8;
+
+my $wal_key_file = $node->data_dir . '/open_pg_tde/wal_keys';
+open(my $rfh, '<:raw', $wal_key_file) or die "open $wal_key_file: $!";
+local $/;
+my $data = <$rfh>;
+close $rfh;
+
+my $kbi_off = length($data) - WAL_KEY_ENTRY_SIZE + KEY_BASE_IV_OFFSET;
+die "WAL key file too small" if $kbi_off < 0;
+substr($data, $kbi_off, 1) = chr(ord(substr($data, $kbi_off, 1)) ^ 0xFF);
+
+open(my $wfh, '>:raw', $wal_key_file) or die "open $wal_key_file: $!";
+print $wfh $data;
+close $wfh;
+
+# The tampered key_base_iv is now part of the AAD, so decrypting the WAL key
+# fails. The server reads the WAL key at startup, so it must refuse to start.
+my $started = $node->start(fail_ok => 1);
+ok(!$started, 'server refuses to start after key_base_iv is tampered with');
+
+my $log = slurp_file($node->logfile);
+like(
+	$log,
+	qr/corrupted key file|failed to decrypt key|incorrect principal key/i,
+	'tampering with key_base_iv is detected (key_base_iv is authenticated)');
+
+done_testing();

t/wal_key_base_iv_migration.pl

@@ -0,0 +1,95 @@
+# Migration of the WAL key file format from version 2 to version 3.
+#
+# Version 3 moves key_base_iv into the AEAD additional authenticated data so it
+# is authenticated (in version 2 it was stored after the tag and
+# unauthenticated). This starts a cluster from a preexisting version-2 data
+# directory with WAL encryption enabled and checks that the WAL key file
+# migrates on startup: the version-2 WAL key is decrypted with the old AAD and
+# re-wrapped as version 3. A failure of the version-2 reader would abort startup
+# with a decryption error, so a clean start already exercises the reader; we
+# also confirm the on-disk file is version 3 afterwards and the cluster is
+# usable.
+#
+# The fixture uses a relative keyring path so it is self-contained: the file
+# provider's keyring lives inside the data directory and is resolved against it
+# after extraction.
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# The fixture was generated on PostgreSQL 18.
+my $pg_version = `pg_config --version`;
+if ($pg_version !~ /PostgreSQL 18/)
+{
+	plan skip_all => 'PostgreSQL 18 required for the version-2 fixture';
+}
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+my $host = $node->host;
+my $port = $node->port;
+my $data_dir = $node->data_dir;
+
+mkdir $data_dir or die "could not create $data_dir: $!";
+system_or_bail('tar', 'xf', 't/wal_keys_v2_datadir.tar.gz', '-C', $data_dir);
+chmod(0700, $data_dir) or die "could not chmod $data_dir: $!";
+
+$node->append_conf('postgresql.conf', "unix_socket_directories = '$host'");
+$node->append_conf('postgresql.conf', "listen_addresses = ''");
+$node->append_conf('postgresql.conf', "port = '$port'");
+
+# The WAL key file is version 2 before we start.
+my $wal_key_file = "$data_dir/open_pg_tde/wal_keys";
+is(wal_key_file_version($wal_key_file),
+	2, 'fixture WAL key file is version 2');
+
+# Starting the cluster runs the version-2 -> version-3 migration. If the
+# version-2 reader used the wrong AAD the WAL key would not decrypt and startup
+# would fail here.
+$node->start;
+
+is(wal_key_file_version($wal_key_file),
+	3, 'WAL key file is version 3 after migration');
+
+# WAL encryption is still enabled and the migrated key is usable.
+is( $node->safe_psql(
+		'postgres',
+		'SHOW open_pg_tde.wal_encrypt;',
+		extra_params => [ '-U', 'tde_fixture' ]),
+	'on',
+	'WAL encryption is still enabled after migration');
+
+is( $node->safe_psql(
+		'postgres',
+		'SELECT note FROM wal_probe ORDER BY id',
+		extra_params => [ '-U', 'tde_fixture' ]),
+	"v2-wal-alpha\nv2-wal-bravo",
+	'data written under version 2 is readable after migration');
+
+# A new WAL-generating workload works with the migrated (version-3) key.
+$node->safe_psql(
+	'postgres',
+	'INSERT INTO wal_probe VALUES (3, $$v3-wal-charlie$$); CHECKPOINT;',
+	extra_params => [ '-U', 'tde_fixture' ]);
+is( $node->safe_psql(
+		'postgres',
+		'SELECT count(*) FROM wal_probe',
+		extra_params => [ '-U', 'tde_fixture' ]),
+	'3',
+	'new rows can be written after migration');
+
+$node->stop;
+
+done_testing();
+
+# Read the numeric version from the little-endian file magic in the header.
+sub wal_key_file_version
+{
+	my ($path) = @_;
+	open(my $fh, '<:raw', $path) or die "open $path: $!";
+	read($fh, my $buf, 4) == 4 or die "short read on $path";
+	close $fh;
+	my $magic = unpack('L<', $buf);
+	return ($magic & 0xF000000) >> 24;
+}