From 27d05ddb6e8e08df3a2dd31d065d89e18887e6e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85strand?= Date: Mon, 9 Jun 2025 12:20:02 +0200 Subject: [PATCH] Delete relation keys before create There are crash scenarios where keys are left behind in the key file even though the OID for the table goes unused. This meant that we could have keys laying around for newly created plaintext relations after OID wraparound. Simply removing any existing keys when relations are created seems appriopriate. Also move creation of pg_tde data dir to library init. This directory is used by the SMgr which is loaded regardless of whether any database are yet to create extension or not. --- contrib/pg_tde/src/access/pg_tde_xlog.c | 6 ++++ .../pg_tde/src/include/access/pg_tde_xlog.h | 1 + contrib/pg_tde/src/include/smgr/pg_tde_smgr.h | 1 + contrib/pg_tde/src/pg_tde.c | 3 +- contrib/pg_tde/src/smgr/pg_tde_smgr.c | 33 +++++++++++++++++++ 5 files changed, 42 insertions(+), 2 deletions(-) diff --git a/contrib/pg_tde/src/access/pg_tde_xlog.c b/contrib/pg_tde/src/access/pg_tde_xlog.c index 0d0581d0604..43407ee3698 100644 --- a/contrib/pg_tde/src/access/pg_tde_xlog.c +++ b/contrib/pg_tde/src/access/pg_tde_xlog.c @@ -61,6 +61,12 @@ tdeheap_rmgr_redo(XLogReaderState *record) pg_tde_save_principal_key_redo(mkey); } + else if (info == XLOG_TDE_REMOVE_RELATION_KEY) + { + XLogRelKey *xlrec = (XLogRelKey *) XLogRecGetData(record); + + tde_smgr_delete_key_redo(&xlrec->rlocator); + } else if (info == XLOG_TDE_ROTATE_PRINCIPAL_KEY) { XLogPrincipalKeyRotate *xlrec = (XLogPrincipalKeyRotate *) XLogRecGetData(record); diff --git a/contrib/pg_tde/src/include/access/pg_tde_xlog.h b/contrib/pg_tde/src/include/access/pg_tde_xlog.h index 3938ae77637..2f08fecb371 100644 --- a/contrib/pg_tde/src/include/access/pg_tde_xlog.h +++ b/contrib/pg_tde/src/include/access/pg_tde_xlog.h @@ -17,6 +17,7 @@ #define XLOG_TDE_ROTATE_PRINCIPAL_KEY 0x20 #define XLOG_TDE_WRITE_KEY_PROVIDER 0x30 #define XLOG_TDE_INSTALL_EXTENSION 0x40 +#define XLOG_TDE_REMOVE_RELATION_KEY 0x50 /* ID 140 is registered for Percona TDE extension: https://wiki.postgresql.org/wiki/CustomWALResourceManagers */ #define RM_TDERMGR_ID 140 diff --git a/contrib/pg_tde/src/include/smgr/pg_tde_smgr.h b/contrib/pg_tde/src/include/smgr/pg_tde_smgr.h index e0f09efc4c3..31a0d525f4f 100644 --- a/contrib/pg_tde/src/include/smgr/pg_tde_smgr.h +++ b/contrib/pg_tde/src/include/smgr/pg_tde_smgr.h @@ -14,6 +14,7 @@ extern void RegisterStorageMgr(void); extern void tde_smgr_create_key_redo(const RelFileLocator *rlocator); +extern void tde_smgr_delete_key_redo(const RelFileLocator *rlocator); extern bool tde_smgr_rel_is_encrypted(SMgrRelation reln); #endif /* PG_TDE_SMGR_H */ diff --git a/contrib/pg_tde/src/pg_tde.c b/contrib/pg_tde/src/pg_tde.c index 3127c2c4cff..410c7ce7cb6 100644 --- a/contrib/pg_tde/src/pg_tde.c +++ b/contrib/pg_tde/src/pg_tde.c @@ -93,6 +93,7 @@ _PG_init(void) check_percona_api_version(); + pg_tde_init_data_dir(); AesInit(); TdeGucInit(); TdeEventCaptureInit(); @@ -113,8 +114,6 @@ _PG_init(void) static void extension_install(Oid databaseId) { - /* Initialize the TDE dir */ - pg_tde_init_data_dir(); key_provider_startup_cleanup(databaseId); principal_key_startup_cleanup(databaseId); } diff --git a/contrib/pg_tde/src/smgr/pg_tde_smgr.c b/contrib/pg_tde/src/smgr/pg_tde_smgr.c index 67e44e9d367..476a30369e1 100644 --- a/contrib/pg_tde/src/smgr/pg_tde_smgr.c +++ b/contrib/pg_tde/src/smgr/pg_tde_smgr.c @@ -109,6 +109,26 @@ tde_smgr_create_key_redo(const RelFileLocator *rlocator) pg_tde_save_smgr_key(*rlocator, &key); } +static void +tde_smgr_delete_key(const RelFileLocatorBackend *smgr_rlocator) +{ + XLogRelKey xlrec = { + .rlocator = smgr_rlocator->locator, + }; + + pg_tde_free_key_map_entry(smgr_rlocator->locator); + + XLogBeginInsert(); + XLogRegisterData((char *) &xlrec, sizeof(xlrec)); + XLogInsert(RM_TDERMGR_ID, XLOG_TDE_REMOVE_RELATION_KEY); +} + +void +tde_smgr_delete_key_redo(const RelFileLocator *rlocator) +{ + pg_tde_free_key_map_entry(*rlocator); +} + static bool tde_smgr_is_encrypted(const RelFileLocatorBackend *smgr_rlocator) { @@ -363,6 +383,19 @@ tde_mdcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool return; } + if (!isRedo) + { + /* + * If we have a key for this relation already, we need to remove it. + * This can happen if OID is re-used after a crash left a key for a + * non-existing relation in the key file. + * + * If we're in redo, a separate WAL record will make sure the key is + * removed. + */ + tde_smgr_delete_key(&reln->smgr_rlocator); + } + if (!tde_smgr_should_encrypt(&reln->smgr_rlocator, &relold)) { tdereln->encryption_status = RELATION_NOT_ENCRYPTED;