From 893512584461027c7922123dbfb0f94bdeaa9a92 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Tue, 11 Jun 2024 20:02:51 +0100 Subject: [PATCH] Minimal working prototype for SMGR + Event trigger + keyring encryption (#199) * Introduces `pg_tde2` access method * New access method uses the event trigger changes from #196 * Keys are now loaded from the keyring * This required changes to the map file / master key infrastructure * This commit only modifies/fixes those as little as required for simplicity. More refactoring/changes coming in separate commits / PRs * Removes reliance from MyDatabaseId, as things now have to work with multiple databases in the checkpointer * Removes some error reports, where functions should work even without a configured keyring * Fixes some bugs in the map file functions * Map file functions now work with multiple databases in a single process, but this is a hackish solution, global state needs a proper refactoring * Contains anti-recursion hack in the new SMGR code, which is needed until we store the metadata in the catalog --- pg_tde--1.0.sql | 9 +++ src/access/pg_tde_tdemap.c | 40 ++++++---- src/access/pg_tdeam_handler.c | 14 ++-- src/catalog/tde_master_key.c | 14 +--- src/common/pg_tde_utils.c | 8 +- src/include/access/pg_tde_tdemap.h | 3 +- src/include/catalog/tde_master_key.h | 2 +- src/include/common/pg_tde_utils.h | 3 +- src/include/pg_tde_defines.h | 2 - src/pg_tde_event_capture.c | 2 +- src/smgr/pg_tde_smgr.c | 106 +++++++++++++++++++++------ t/results/001_basic.out | 13 ---- 12 files changed, 142 insertions(+), 74 deletions(-) delete mode 100644 t/results/001_basic.out diff --git a/pg_tde--1.0.sql b/pg_tde--1.0.sql index 4474e7496cb..9d43e98a939 100644 --- a/pg_tde--1.0.sql +++ b/pg_tde--1.0.sql @@ -87,6 +87,12 @@ RETURNS table_am_handler AS 'MODULE_PATHNAME' LANGUAGE C; +-- Table access method +CREATE FUNCTION pg_tde2am_handler(internal) +RETURNS table_am_handler +AS 'MODULE_PATHNAME' +LANGUAGE C; + CREATE FUNCTION pgtde_is_encrypted(table_name VARCHAR) RETURNS boolean AS $$ @@ -129,6 +135,9 @@ CREATE FUNCTION pg_tde_version() RETURNS TEXT AS 'MODULE_PATHNAME' LANGUAGE C; CREATE ACCESS METHOD pg_tde TYPE TABLE HANDLER pg_tdeam_handler; COMMENT ON ACCESS METHOD pg_tde IS 'pg_tde table access method'; +CREATE ACCESS METHOD pg_tde2 TYPE TABLE HANDLER pg_tde2am_handler; +COMMENT ON ACCESS METHOD pg_tde2 IS 'pg_tde2 table access method'; + -- Per database extension initialization SELECT pg_tde_extension_initialize(); diff --git a/src/access/pg_tde_tdemap.c b/src/access/pg_tde_tdemap.c index 0f3b1307b0a..ced8f6a26d1 100644 --- a/src/access/pg_tde_tdemap.c +++ b/src/access/pg_tde_tdemap.c @@ -108,8 +108,8 @@ static void finalize_key_rotation(char *m_path_old, char *k_path_old, char *m_pa /* * Generate an encrypted key for the relation and store it in the keymap file. */ -void -pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, Relation rel) +RelKeyData* +pg_tde_create_key_map_entry(const RelFileLocator *newrlocator) { InternalKey int_key; RelKeyData *rel_key_data; @@ -117,11 +117,14 @@ pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, Relation rel) TDEMasterKey *master_key; XLogRelKey xlrec; - master_key = GetMasterKey(); + pg_tde_set_db_file_paths(newrlocator->dbOid); + master_key = GetMasterKey(newrlocator->dbOid); if (master_key == NULL) { ereport(ERROR, (errmsg("failed to retrieve master key"))); + + return NULL; } memset(&int_key, 0, sizeof(InternalKey)); @@ -131,7 +134,9 @@ pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, Relation rel) ereport(FATAL, (errcode(ERRCODE_INTERNAL_ERROR), errmsg("could not generate internal key for relation \"%s\": %s", - RelationGetRelationName(rel), ERR_error_string(ERR_get_error(), NULL)))); + "TODO", ERR_error_string(ERR_get_error(), NULL)))); + + return NULL; } /* Encrypt the key */ @@ -152,6 +157,8 @@ pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, Relation rel) * Add the encyrpted key to the key map data file structure. */ pg_tde_write_key_map_entry(newrlocator, enc_rel_key_data, &master_key->keyInfo); + + return rel_key_data; } /* Head of the key cache (linked list) */ @@ -179,7 +186,10 @@ GetRelationKey(RelFileLocator rel) key = pg_tde_get_key_from_file(&rel); - put_key_into_map(rel.relNumber, key); + if (key != NULL) + { + put_key_into_map(rel.relNumber, key); + } return key; } @@ -269,8 +279,9 @@ static void pg_tde_set_db_file_paths(Oid dbOid) { /* Return if the values are already set */ - if (*db_path && *db_map_path && *db_keydata_path) - return; + // TODO: remove this entire global state thing + //if (*db_path && *db_map_path && *db_keydata_path) + // return; /* Fill in the values */ snprintf(db_path, MAXPGPATH, "%s", GetDatabasePath(dbOid, DEFAULTTABLESPACE_OID)); @@ -953,7 +964,8 @@ pg_tde_get_key_from_file(const RelFileLocator *rlocator) LWLockAcquire(lock_files, LW_SHARED); /* Get/generate a master, create the key for relation and get the encrypted key with bytes to write */ - master_key = GetMasterKey(); + pg_tde_set_db_file_paths(rlocator->dbOid); + master_key = GetMasterKey(rlocator->dbOid); if (master_key == NULL) { LWLockRelease(lock_files); @@ -961,13 +973,15 @@ pg_tde_get_key_from_file(const RelFileLocator *rlocator) (errmsg("failed to retrieve master key"))); } - /* Get the file paths */ - pg_tde_set_db_file_paths(rlocator->dbOid); - /* Read the map entry and get the index of the relation key */ key_index = pg_tde_process_map_entry(rlocator, db_map_path, &offset, false); - /* Add the encrypted key to the data file. */ + if (key_index == -1) + { + LWLockRelease(lock_files); + return NULL; + } + enc_rel_key_data = pg_tde_read_keydata(db_keydata_path, key_index, master_key); LWLockRelease(lock_files); @@ -1198,4 +1212,4 @@ pg_tde_write_map_keydata_files(off_t map_size, char *m_file_data, off_t keydata_ LWLockRelease(lock_files); return true; -} \ No newline at end of file +} diff --git a/src/access/pg_tdeam_handler.c b/src/access/pg_tdeam_handler.c index a944c4e21e3..0c697b06621 100644 --- a/src/access/pg_tdeam_handler.c +++ b/src/access/pg_tdeam_handler.c @@ -53,6 +53,7 @@ #include "utils/rel.h" PG_FUNCTION_INFO_V1(pg_tdeam_handler); +PG_FUNCTION_INFO_V1(pg_tde2am_handler); static void reform_and_rewrite_tuple(HeapTuple tuple, @@ -643,7 +644,7 @@ pg_tdeam_relation_set_new_filelocator(Relation rel, ereport(DEBUG1, (errmsg("creating key file for relation %s", RelationGetRelationName(rel)))); - pg_tde_create_key_map_entry(newrlocator, rel); + pg_tde_create_key_map_entry(newrlocator); } } @@ -2622,17 +2623,16 @@ static const TableAmRoutine pg_tdeam_methods = { .scan_sample_next_tuple = pg_tdeam_scan_sample_next_tuple }; - -const TableAmRoutine * -GetPGTdeamTableAmRoutine(void) +Datum +pg_tdeam_handler(PG_FUNCTION_ARGS) { - return &pg_tdeam_methods; + PG_RETURN_POINTER(&pg_tdeam_methods); } Datum -pg_tdeam_handler(PG_FUNCTION_ARGS) +pg_tde2am_handler(PG_FUNCTION_ARGS) { - PG_RETURN_POINTER(&pg_tdeam_methods); + PG_RETURN_POINTER(GetHeapamTableAmRoutine()); } bool diff --git a/src/catalog/tde_master_key.c b/src/catalog/tde_master_key.c index 2d2cd9cda35..610f9a6c5e5 100644 --- a/src/catalog/tde_master_key.c +++ b/src/catalog/tde_master_key.c @@ -214,14 +214,13 @@ save_master_key_info(TDEMasterKeyInfo *master_key_info) * throws an error. */ TDEMasterKey * -GetMasterKey(void) +GetMasterKey(Oid dbOid) { TDEMasterKey *masterKey = NULL; TDEMasterKeyInfo *masterKeyInfo = NULL; GenericKeyring *keyring = NULL; const keyInfo *keyInfo = NULL; KeyringReturnCodes keyring_ret; - Oid dbOid = MyDatabaseId; LWLock *lock_files = tde_lwlock_mk_files(); LWLock *lock_cache = tde_lwlock_mk_cache(); @@ -255,9 +254,6 @@ GetMasterKey(void) LWLockRelease(lock_cache); LWLockRelease(lock_files); - ereport(ERROR, - (errmsg("Master key does not exists for the database"), - errhint("Use set_master_key interface to set the master key"))); return NULL; } @@ -268,8 +264,6 @@ GetMasterKey(void) LWLockRelease(lock_cache); LWLockRelease(lock_files); - ereport(ERROR, - (errmsg("Key provider with ID:\"%d\" does not exists", masterKeyInfo->keyringId))); return NULL; } @@ -279,8 +273,6 @@ GetMasterKey(void) LWLockRelease(lock_cache); LWLockRelease(lock_files); - ereport(ERROR, - (errmsg("failed to retrieve master key \"%s\" from keyring.", masterKeyInfo->keyId.versioned_name))); return NULL; } @@ -404,7 +396,7 @@ SetMasterKey(const char *key_name, const char *provider_name, bool ensure_new_ke bool RotateMasterKey(const char *new_key_name, const char *new_provider_name, bool ensure_new_key) { - TDEMasterKey *master_key = GetMasterKey(); + TDEMasterKey *master_key = GetMasterKey(MyDatabaseId); TDEMasterKey new_master_key; const keyInfo *keyInfo = NULL; GenericKeyring *keyring; @@ -737,7 +729,7 @@ Datum pg_tde_master_key_info(PG_FUNCTION_ARGS) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("function returning record called in context that cannot accept type record"))); - master_key = GetMasterKey(); + master_key = GetMasterKey(MyDatabaseId); if (master_key == NULL) PG_RETURN_NULL(); diff --git a/src/common/pg_tde_utils.c b/src/common/pg_tde_utils.c index 5f5c9e938f4..af359693ac4 100644 --- a/src/common/pg_tde_utils.c +++ b/src/common/pg_tde_utils.c @@ -33,6 +33,12 @@ get_tde_table_am_oid(void) return get_table_am_oid("pg_tde", false); } +Oid +get_tde2_table_am_oid(void) +{ + return get_table_am_oid("pg_tde2", false); +} + /* * Returns the list of OIDs for all TDE tables in a database */ @@ -208,4 +214,4 @@ extract_json_option_value(Datum top_json, const char* field_name) elog(ERROR, "Unknown type for object %s: %s", field_name, type_cstr); return NULL; } -} \ No newline at end of file +} diff --git a/src/include/access/pg_tde_tdemap.h b/src/include/access/pg_tde_tdemap.h index c6ff9083f1c..a0b4a899e2a 100644 --- a/src/include/access/pg_tde_tdemap.h +++ b/src/include/access/pg_tde_tdemap.h @@ -46,12 +46,11 @@ typedef struct XLogRelKey RelKeyData relKey; } XLogRelKey; -extern void pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, Relation rel); +extern RelKeyData* pg_tde_create_key_map_entry(const RelFileLocator *newrlocator); extern void pg_tde_write_key_map_entry(const RelFileLocator *rlocator, RelKeyData *enc_rel_key_data, TDEMasterKeyInfo *master_key_info); extern void pg_tde_delete_key_map_entry(const RelFileLocator *rlocator); extern void pg_tde_free_key_map_entry(const RelFileLocator *rlocator, off_t offset); -extern RelKeyData *pg_tde_get_key_from_fork(const RelFileLocator *rlocator); extern RelKeyData *GetRelationKey(RelFileLocator rel); extern void pg_tde_cleanup_path_vars(void); diff --git a/src/include/catalog/tde_master_key.h b/src/include/catalog/tde_master_key.h index b12e2b1f5f8..77bad07f11c 100644 --- a/src/include/catalog/tde_master_key.h +++ b/src/include/catalog/tde_master_key.h @@ -68,7 +68,7 @@ extern LWLock *tde_lwlock_mk_cache(void); extern bool save_master_key_info(TDEMasterKeyInfo *masterKeyInfo); extern Oid GetMasterKeyProviderId(void); -extern TDEMasterKey* GetMasterKey(void); +extern TDEMasterKey* GetMasterKey(Oid); extern bool SetMasterKey(const char *key_name, const char *provider_name, bool ensure_new_key); extern bool RotateMasterKey(const char *new_key_name, const char *new_provider_name, bool ensure_new_key); extern bool xl_tde_perform_rotate_key(XLogMasterKeyRotate *xlrec); diff --git a/src/include/common/pg_tde_utils.h b/src/include/common/pg_tde_utils.h index c33c14b6235..9e2d856e993 100644 --- a/src/include/common/pg_tde_utils.h +++ b/src/include/common/pg_tde_utils.h @@ -12,10 +12,11 @@ #include "nodes/pg_list.h" extern Oid get_tde_table_am_oid(void); +extern Oid get_tde2_table_am_oid(void); extern List *get_all_tde_tables(void); extern int get_tde_tables_count(void); extern const char *extract_json_cstr(Datum json, const char* field_name); const char *extract_json_option_value(Datum top_json, const char* field_name); -#endif /*PG_TDE_UTILS_H*/ \ No newline at end of file +#endif /*PG_TDE_UTILS_H*/ diff --git a/src/include/pg_tde_defines.h b/src/include/pg_tde_defines.h index 0c9847bb7df..74f47f9db16 100644 --- a/src/include/pg_tde_defines.h +++ b/src/include/pg_tde_defines.h @@ -38,8 +38,6 @@ #define pgstat_count_pg_tde_insert pgstat_count_heap_insert #define pg_tde_getattr heap_getattr -#define GetPGTdeamTableAmRoutine GetHeapamTableAmRoutine - #define TDE_PageAddItem(rel, oid, blkno, page, item, size, offsetNumber, overwrite, is_heap) \ PGTdePageAddItemExtended(rel, oid, blkno, page, item, size, offsetNumber, \ ((overwrite) ? PAI_OVERWRITE : 0) | \ diff --git a/src/pg_tde_event_capture.c b/src/pg_tde_event_capture.c index 1797ec5773c..3645b45e1ea 100644 --- a/src/pg_tde_event_capture.c +++ b/src/pg_tde_event_capture.c @@ -105,7 +105,7 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS) tdeCurrentCreateEvent.relation = stmt->relation; elog(DEBUG1, "CREATING TABLE %s Using Access Method %s", stmt->relation->relname, stmt->accessMethod); - if (stmt->accessMethod && !strcmp(stmt->accessMethod, "pg_tde")) + if (stmt->accessMethod && !strcmp(stmt->accessMethod, "pg_tde2")) { tdeCurrentCreateEvent.encryptMode = true; } diff --git a/src/smgr/pg_tde_smgr.c b/src/smgr/pg_tde_smgr.c index 9e679ec5138..ceb83cfa186 100644 --- a/src/smgr/pg_tde_smgr.c +++ b/src/smgr/pg_tde_smgr.c @@ -5,14 +5,72 @@ #include "storage/md.h" #include "catalog/catalog.h" #include "encryption/enc_aes.h" +#include "access/pg_tde_tdemap.h" +#include "pg_tde_event_capture.h" #if PG_VERSION_NUM >= 170000 -// TODO: implement proper key/IV -static char key[16] = {0,}; -// iv should be based on blocknum, available in the API +// TODO: implement proper IV +// iv should be based on blocknum + relfile, available in the API static char iv[16] = {0,}; +static RelKeyData* +tde_smgr_get_key(SMgrRelation reln) +{ + // TODO: This recursion counter is a dirty hack until the metadata is in the catalog + // As otherwise we would call GetMasterKey recursively and deadlock + static int recursion = 0; + + ereport(NOTICE, + (errmsg("Trying to decide if table is encrypted: %u", reln->smgr_rlocator.locator.relNumber))); + + + if(IsCatalogRelationOid(reln->smgr_rlocator.locator.relNumber)) + { + // do not try to encrypt/decrypt catalog tables + return NULL; + } + + if(recursion != 0) + { + return NULL; + } + + recursion++; + + + if(GetMasterKey(reln->smgr_rlocator.locator.relNumber)==NULL) + { + recursion--; + return NULL; + } + + TdeCreateEvent* event = GetCurrentTdeCreateEvent(); + + // if this is a CREATE TABLE, we have to generate the key + if(event->encryptMode == true && event->eventType == TDE_TABLE_CREATE_EVENT) + { + recursion--; + return pg_tde_create_key_map_entry(&reln->smgr_rlocator.locator); + } + + // if this is a CREATE INDEX, we have to load the key based on the table + if(event->encryptMode == true && event->eventType == TDE_INDEX_CREATE_EVENT) + { + // For now keep it simple and create separate key for indexes + // Later we might modify the map infrastructure to support the same keys + recursion--; + return pg_tde_create_key_map_entry(&reln->smgr_rlocator.locator); + } + + // otherwise, see if we have a key for the relation, and return if yes + RelKeyData* rkd = GetRelationKey(reln->smgr_rlocator.locator); + + recursion--; + + return rkd; +} + void tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, const void **buffers, BlockNumber nblocks, bool skipFsync) @@ -23,13 +81,10 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char* local_blocks_aligned = (char*)TYPEALIGN(PG_IO_ALIGN_SIZE, local_blocks); const void** local_buffers = malloc ( sizeof(void*) * nblocks ); - // TODO: add check to only encrypt/decrypt tables with specific AM/DB? + RelKeyData* rkd = tde_smgr_get_key(reln); - if(IsCatalogRelationOid(reln->smgr_rlocator.locator.spcOid)) + if(rkd == NULL) { - // Don't try to encrypt catalog tables: - // Issues with bootstrap and encryption metadata - mdwritev(reln, forknum, blocknum, buffers, nblocks, skipFsync); return; @@ -39,7 +94,7 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, { local_buffers[i] = &local_blocks_aligned[i*BLCKSZ]; int out_len = BLCKSZ; - AesEncrypt(key, iv, ((char**)buffers)[i], BLCKSZ, local_buffers[i], &out_len); + AesEncrypt(rkd->internal_key.key, iv, ((char**)buffers)[i], BLCKSZ, local_buffers[i], &out_len); } mdwritev(reln, forknum, blocknum, @@ -58,19 +113,17 @@ tde_mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, char* local_blocks = malloc( BLCKSZ * (1+1) ); char* local_blocks_aligned = (char*)TYPEALIGN(PG_IO_ALIGN_SIZE, local_blocks); - // TODO: add check to only encrypt/decrypt tables with specific AM/DB? + RelKeyData* rkd = tde_smgr_get_key(reln); - if(IsCatalogRelationOid(reln->smgr_rlocator.locator.spcOid)) + if(rkd == NULL) { - // Don't try to encrypt catalog tables: - // Issues with bootstrap and encryption metadata mdextend(reln, forknum, blocknum, buffer, skipFsync); return; } int out_len = BLCKSZ; - AesEncrypt(key, iv, ((char*)buffer), BLCKSZ, local_blocks_aligned, &out_len); + AesEncrypt(rkd->internal_key.key, iv, ((char*)buffer), BLCKSZ, local_blocks_aligned, &out_len); mdextend(reln, forknum, blocknum, local_blocks_aligned, skipFsync); @@ -86,10 +139,9 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, mdreadv(reln, forknum, blocknum, buffers, nblocks); - // TODO: add check to only encrypt/decrypt tables with specific AM/DB? + RelKeyData* rkd = tde_smgr_get_key(reln); - // Don't try to decrypt catalog tables, those are not encrypted - if(IsCatalogRelationOid(reln->smgr_rlocator.locator.spcOid)) + if(rkd == NULL) { return; } @@ -113,12 +165,22 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, if(allZero) continue; int out_len = BLCKSZ; - AesDecrypt(key, iv, ((char**)buffers)[i], BLCKSZ, ((char**)buffers)[i], &out_len); + AesDecrypt(rkd->internal_key.key, iv, ((char**)buffers)[i], BLCKSZ, ((char**)buffers)[i], &out_len); } +} + +void +tde_mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) +{ + // This is the only function that gets called during actual CREATE TABLE/INDEX (EVENT TRIGGER) + // so we create the key here by loading it + // Later calls then decide to encrypt or not based on the existence of the key + tde_smgr_get_key(reln); - // And now decrypt buffers in place - // We check the first few bytes of the page: if all zero, we assume it is zero and keep it as is + return mdcreate(reln, forknum, isRedo); } + + static SMgrId tde_smgr_id; static const struct f_smgr tde_smgr = { .name = "tde", @@ -126,7 +188,7 @@ static const struct f_smgr tde_smgr = { .smgr_shutdown = NULL, .smgr_open = mdopen, .smgr_close = mdclose, - .smgr_create = mdcreate, + .smgr_create = tde_mdcreate, .smgr_exists = mdexists, .smgr_unlink = mdunlink, .smgr_extend = tde_mdextend, @@ -152,4 +214,4 @@ void RegisterStorageMgr() void RegisterStorageMgr() { } -#endif \ No newline at end of file +#endif diff --git a/t/results/001_basic.out b/t/results/001_basic.out deleted file mode 100644 index d6f838c98e8..00000000000 --- a/t/results/001_basic.out +++ /dev/null @@ -1,13 +0,0 @@ -CREATE EXTENSION pg_tde; --- server restart -CREATE TABLE test_enc(id SERIAL,k INTEGER,PRIMARY KEY (id)) USING pg_tde; -INSERT INTO test_enc (k) VALUES (5),(6); -SELECT * FROM test_enc ORDER BY id ASC; -1|5 -2|6 --- server restart -SELECT * FROM test_enc ORDER BY id ASC; -1|5 -2|6 -DROP TABLE test_enc; -DROP EXTENSION pg_tde;