diff --git a/meson.build b/meson.build index 483e7db..50e2139 100644 --- a/meson.build +++ b/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', ] diff --git a/src/access/open_pg_tde_xlog_keys.c b/src/access/open_pg_tde_xlog_keys.c index 3dd6b5a..f285341 100644 --- a/src/access/open_pg_tde_xlog_keys.c +++ b/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)); diff --git a/src/include/open_pg_tde.h b/src/include/open_pg_tde.h index f252949..f74ec11 100644 --- a/src/include/open_pg_tde.h +++ b/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 diff --git a/t/wal_key_auth.pl b/t/wal_key_auth.pl new file mode 100644 index 0000000..5bbb756 --- /dev/null +++ b/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(); diff --git a/t/wal_key_base_iv_migration.pl b/t/wal_key_base_iv_migration.pl new file mode 100644 index 0000000..c0ce5db --- /dev/null +++ b/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; +} diff --git a/t/wal_keys_v2_datadir.tar.gz b/t/wal_keys_v2_datadir.tar.gz new file mode 100644 index 0000000..1f4ea81 Binary files /dev/null and b/t/wal_keys_v2_datadir.tar.gz differ