From 47bce9a5d06b8f600cba7bf3b2882539deb0b8ee Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Wed, 3 Jul 2024 19:38:12 +0500 Subject: [PATCH 1/7] Remove 'percona_tde.pg_tde_key_provider' user catalog and introduce provider info file for key providers This commit removes the 'percona_tde.pg_tde_key_provider' user catalog and replaces it with a provider info file for saving key providers. This change ensures that the key provider information can be accessed during recovery, as the user catalogs cannot be relied upon in such scenarios. The commit maintains the current API functions, so callers will not experience any differences in functionality or usage after this change. Additionally, the commit adjusts how the shared memory manager retrieves information about the number of LWLocks required by the extension, optimizing the process. TODO: Implement xlog message for cleaning up the provider info file during recovery operations to ensure consistency and avoid potential issues. --- pg_tde--1.0.sql | 33 +-- src/access/pg_tde_tdemap.c | 14 +- src/catalog/tde_keyring.c | 465 +++++++++++++++++++----------- src/catalog/tde_principal_key.c | 10 +- src/common/pg_tde_shmem.c | 19 +- src/common/pg_tde_utils.c | 13 + src/include/catalog/tde_keyring.h | 13 +- src/include/common/pg_tde_shmem.h | 7 +- src/include/common/pg_tde_utils.h | 2 + src/pg_tde.c | 1 + 10 files changed, 336 insertions(+), 241 deletions(-) diff --git a/pg_tde--1.0.sql b/pg_tde--1.0.sql index 81e5f6d2..d292537d 100644 --- a/pg_tde--1.0.sql +++ b/pg_tde--1.0.sql @@ -3,37 +3,16 @@ -- complain if script is sourced in psql, rather than via CREATE EXTENSION \echo Use "CREATE EXTENSION pg_tde" to load this file. \quit --- pg_tde catalog tables -CREATE SCHEMA percona_tde; --- Note: The table is created using heap storage becasue we do not want this table --- to be encrypted by pg_tde. This table is used to store key provider information --- and we do not want to encrypt this table using pg_tde. -CREATE TABLE percona_tde.pg_tde_key_provider(provider_id SERIAL, - keyring_type VARCHAR(10) CHECK (keyring_type IN ('file', 'vault-v2')), - provider_name VARCHAR(255) UNIQUE NOT NULL, options JSON, PRIMARY KEY(provider_id)) using heap; - --- If you want to add new provider types, you need to make appropriate changes --- in include/catalog/tde_keyring.h and src/catalog/tde_keyring.c files. - -SELECT pg_catalog.pg_extension_config_dump('percona_tde.pg_tde_key_provider', ''); - --- Trigger function to check principal key dependency on key provider row -CREATE FUNCTION keyring_delete_dependency_check_trigger() -RETURNS TRIGGER +-- Key Provider Management +CREATE FUNCTION pg_tde_add_key_provider_internal(provider_type VARCHAR(10), provider_name VARCHAR(128), options JSON) +RETURNS INT AS 'MODULE_PATHNAME' LANGUAGE C; -CREATE TRIGGER pg_tde_key_provider_delete_dependency_check_trigger -BEFORE DELETE ON percona_tde.pg_tde_key_provider -FOR EACH ROW -EXECUTE FUNCTION keyring_delete_dependency_check_trigger(); - --- Key Provider Management - CREATE OR REPLACE FUNCTION pg_tde_add_key_provider(provider_type VARCHAR(10), provider_name VARCHAR(128), options JSON) RETURNS INT AS $$ - INSERT INTO percona_tde.pg_tde_key_provider (keyring_type, provider_name, options) VALUES (provider_type, provider_name, options) RETURNING provider_id; + SELECT pg_tde_add_key_provider_internal(provider_type, provider_name, options); $$ LANGUAGE SQL; @@ -77,10 +56,6 @@ AS $$ $$ LANGUAGE SQL; -CREATE FUNCTION pg_tde_get_keyprovider(provider_name text) -RETURNS VOID -AS 'MODULE_PATHNAME' -LANGUAGE C; -- Table access method CREATE FUNCTION pg_tdeam_handler(internal) RETURNS table_am_handler diff --git a/src/access/pg_tde_tdemap.c b/src/access/pg_tde_tdemap.c index e2ed4465..c274866c 100644 --- a/src/access/pg_tde_tdemap.c +++ b/src/access/pg_tde_tdemap.c @@ -12,7 +12,6 @@ #include "postgres.h" #include "access/pg_tde_tdemap.h" -#include "catalog/pg_tablespace_d.h" #include "transam/pg_tde_xact_handler.h" #include "storage/fd.h" #include "utils/wait_event.h" @@ -29,6 +28,7 @@ #include "encryption/enc_aes.h" #include "encryption/enc_tde.h" #include "keyring/keyring_api.h" +#include "common/pg_tde_utils.h" #include #include @@ -273,21 +273,13 @@ tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data inline void pg_tde_set_db_file_paths(const RelFileLocator *rlocator, char *map_path, char *keydata_path) { - char *db_path; - - /* If this is a global space, than the call might be in a critial section - * (during XLog write) so we can't do GetDatabasePath as it calls palloc() - */ - if (rlocator->spcOid == GLOBALTABLESPACE_OID) - db_path = "global"; - else - db_path = GetDatabasePath(rlocator->dbOid, rlocator->spcOid); - + char *db_path = pg_tde_get_tde_file_dir(rlocator->dbOid, rlocator->spcOid); if (map_path) join_path_components(map_path, db_path, PG_TDE_MAP_FILENAME); if (keydata_path) join_path_components(keydata_path, db_path, PG_TDE_KEYDATA_FILENAME); + pfree(db_path); } /* diff --git a/src/catalog/tde_keyring.c b/src/catalog/tde_keyring.c index e894dba6..8fcc58fa 100644 --- a/src/catalog/tde_keyring.c +++ b/src/catalog/tde_keyring.c @@ -1,7 +1,7 @@ /*------------------------------------------------------------------------- * * tde_keyring.c - * Deals with the tde keyring configuration catalog + * Deals with the tde keyring configuration * routines. * * IDENTIFICATION @@ -21,18 +21,18 @@ #include "utils/snapmgr.h" #include "utils/fmgroids.h" #include "common/pg_tde_utils.h" +#include "common/pg_tde_shmem.h" #include "executor/spi.h" +#include "miscadmin.h" #include "unistd.h" #include "utils/builtins.h" +#include "pg_tde.h" -PG_FUNCTION_INFO_V1(keyring_delete_dependency_check_trigger); +PG_FUNCTION_INFO_V1(pg_tde_add_key_provider_internal); +Datum pg_tde_add_key_provider_internal(PG_FUNCTION_ARGS); -/* Must match the catalog table definition */ -#define PG_TDE_KEY_PROVIDER_ID_ATTRNUM 1 -#define PG_TDE_KEY_PROVIDER_TYPE_ATTRNUM 2 -#define PG_TDE_KEY_PROVIDER_NAME_ATTRNUM 3 -#define PG_TDE_KEY_PROVIDER_OPTIONS_ATTRNUM 4 +#define PG_TDE_KEYRING_FILENAME "pg_tde_keyrings" /* * These token must be exactly same as defined in * pg_tde_add_key_provider_vault_v2 SQL interface @@ -49,11 +49,89 @@ PG_FUNCTION_INFO_V1(keyring_delete_dependency_check_trigger); #define FILE_KEYRING_PATH_KEY "path" #define FILE_KEYRING_TYPE_KEY "type" +typedef enum ProviderScanType +{ + PROVIDER_SCAN_BY_NAME, + PROVIDER_SCAN_BY_ID, + PROVIDER_SCAN_BY_TYPE, + PROVIDER_SCAN_ALL +} ProviderScanType; + +static List *scan_key_provider_file(ProviderScanType scanType, void *scanKey); + static FileKeyring *load_file_keyring_provider_options(Datum keyring_options); static GenericKeyring *load_keyring_provider_options(ProviderType provider_type, Datum keyring_options); static VaultV2Keyring *load_vaultV2_keyring_provider_options(Datum keyring_options); static void debug_print_kerying(GenericKeyring *keyring); -static GenericKeyring *load_keyring_provider_from_tuple(HeapTuple tuple, TupleDesc tupDesc); +static char *get_keyring_infofile_path(char *resPath, Oid dbOid, Oid spcOid); +static uint32 save_key_provider(KeyringProvideRecord *provider); +static void key_provider_startup_cleanup(int tde_tbl_count, void *arg); + +static Size initialize_shared_state(void *start_address); +static Size required_shared_mem_size(void); + +typedef struct TdeKeyProviderInfoSharedState +{ + LWLock *Locks; +} TdeKeyProviderInfoSharedState; + +TdeKeyProviderInfoSharedState* sharedPrincipalKeyState = NULL; /* Lives in shared state */ + +static const TDEShmemSetupRoutine key_provider_info_shmem_routine = { + .init_shared_state = initialize_shared_state, + .init_dsa_area_objects = NULL, + .required_shared_mem_size = required_shared_mem_size, + .shmem_kill = NULL + }; + +static Size +required_shared_mem_size(void) +{ + return MAXALIGN(sizeof(TdeKeyProviderInfoSharedState)); +} + +static Size +initialize_shared_state(void *start_address) +{ + sharedPrincipalKeyState = (TdeKeyProviderInfoSharedState *)start_address; + ereport(LOG, (errmsg("initializing shared state for primary key info"))); + sharedPrincipalKeyState->Locks = GetLWLocks(); + return sizeof(TdeKeyProviderInfoSharedState); +} + +static inline LWLock * +tde_provider_info_lock(void) +{ + Assert(sharedPrincipalKeyState); + return &sharedPrincipalKeyState->Locks[TDE_LWLOCK_PI_FILES]; +} + +void InitializeKeyProviderInfo(void) +{ + ereport(LOG, (errmsg("Initializing TDE principal key info"))); + RegisterShmemRequest(&key_provider_info_shmem_routine); + on_ext_install(key_provider_startup_cleanup, NULL); +} +static void +key_provider_startup_cleanup(int tde_tbl_count, void *arg) +{ + + if (tde_tbl_count > 0) + { + ereport(WARNING, + (errmsg("Failed to perform initialization. database already has %d TDE tables", tde_tbl_count))); + return; + } + cleanup_key_provider_info(MyDatabaseId, MyDatabaseTableSpace); + + /* TODO: XLog the key cleanup */ + // XLogPrincipalKeyCleanup xlrec; + // xlrec.databaseId = MyDatabaseId; + // xlrec.tablespaceId = MyDatabaseTableSpace; + // XLogBeginInsert(); + // XLogRegisterData((char *)&xlrec, sizeof(TDEPrincipalKeyInfo)); + // XLogInsert(RM_TDERMGR_ID, XLOG_TDE_CLEAN_PRINCIPAL_KEY); +} ProviderType get_keyring_provider_from_typename(char *provider_type) @@ -69,36 +147,19 @@ get_keyring_provider_from_typename(char *provider_type) } static GenericKeyring * -load_keyring_provider_from_tuple(HeapTuple tuple, TupleDesc tupDesc) +load_keyring_provider_from_record(KeyringProvideRecord* provider) { - Datum datum; Datum option_datum; - bool isnull; - char *keyring_name; - char *keyring_type; - int provider_id; - ProviderType provider_type = UNKNOWN_KEY_PROVIDER; GenericKeyring *keyring = NULL; - datum = heap_getattr(tuple, PG_TDE_KEY_PROVIDER_ID_ATTRNUM, tupDesc, &isnull); - provider_id = DatumGetInt32(datum); - - datum = heap_getattr(tuple, PG_TDE_KEY_PROVIDER_TYPE_ATTRNUM, tupDesc, &isnull); - keyring_type = TextDatumGetCString(datum); - - datum = heap_getattr(tuple, PG_TDE_KEY_PROVIDER_NAME_ATTRNUM, tupDesc, &isnull); - keyring_name = TextDatumGetCString(datum); - - option_datum = heap_getattr(tuple, PG_TDE_KEY_PROVIDER_OPTIONS_ATTRNUM, tupDesc, &isnull); + option_datum = CStringGetTextDatum(provider->options); - provider_type = get_keyring_provider_from_typename(keyring_type); - - keyring = load_keyring_provider_options(provider_type, option_datum); + keyring = load_keyring_provider_options(provider->provider_type, option_datum); if (keyring) { - strncpy(keyring->provider_name, keyring_name, sizeof(keyring->provider_name)); - keyring->key_id = provider_id; - keyring->type = provider_type; + keyring->key_id = provider->provider_id; + strncpy(keyring->provider_name, provider->provider_name, sizeof(keyring->provider_name)); + keyring->type = provider->provider_type; debug_print_kerying(keyring); } return keyring; @@ -107,103 +168,39 @@ load_keyring_provider_from_tuple(HeapTuple tuple, TupleDesc tupDesc) List * GetAllKeyringProviders(void) { - HeapTuple tuple; - TupleDesc tupDesc; - List *keyring_list = NIL; - - Oid pg_tde_schema_oid = LookupNamespaceNoError(PG_TDE_NAMESPACE_NAME); - Oid kp_table_oid = get_relname_relid(PG_TDE_KEY_PROVIDER_CAT_NAME, pg_tde_schema_oid); - Relation kp_table_relation = relation_open(kp_table_oid, AccessShareLock); - TableScanDesc scan; - - tupDesc = kp_table_relation->rd_att; - - scan = heap_beginscan(kp_table_relation, GetLatestSnapshot(), 0, NULL, NULL, 0); - - while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) - { - GenericKeyring *keyring = load_keyring_provider_from_tuple(tuple, tupDesc); - if (keyring) - { - keyring_list = lappend(keyring_list, keyring); - } - } - heap_endscan(scan); - relation_close(kp_table_relation, AccessShareLock); - - return keyring_list; + return scan_key_provider_file(PROVIDER_SCAN_ALL, NULL); } GenericKeyring * GetKeyProviderByName(const char *provider_name) { - HeapTuple tuple; - TupleDesc tupDesc; - TableScanDesc scan; - ScanKeyData scanKey[1]; GenericKeyring *keyring = NULL; - Oid pg_tde_schema_oid = LookupNamespaceNoError(PG_TDE_NAMESPACE_NAME); - Oid kp_table_oid = get_relname_relid(PG_TDE_KEY_PROVIDER_CAT_NAME, pg_tde_schema_oid); - Relation kp_table_relation = relation_open(kp_table_oid, AccessShareLock); - - /* Set up a scan key to fetch only required record. */ - ScanKeyInit(scanKey, - (AttrNumber)PG_TDE_KEY_PROVIDER_NAME_ATTRNUM, - BTEqualStrategyNumber, F_TEXTEQ, - CStringGetTextDatum(provider_name)); - - tupDesc = kp_table_relation->rd_att; - /* Begin the scan with the filter condition */ - scan = heap_beginscan(kp_table_relation, GetLatestSnapshot(), 1, scanKey, NULL, 0); - - while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + List *providers = scan_key_provider_file(PROVIDER_SCAN_BY_NAME, (void*)provider_name); + if (providers != NIL) { - keyring = load_keyring_provider_from_tuple(tuple, tupDesc); - break; /* We expect only one record */ + keyring = (GenericKeyring *)linitial(providers); + list_free(providers); + } + else + { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("Key provider \"%s\" does not exists", provider_name), + errhint("Use create_key_provider interface to create the key provider"))); } - heap_endscan(scan); - relation_close(kp_table_relation, AccessShareLock); - - if (keyring == NULL) - { - ereport(ERROR, - (errmsg("Key provider \"%s\" does not exists", provider_name), - errhint("Use create_key_provider interface to create the key provider"))); - } - return keyring; } GenericKeyring * GetKeyProviderByID(int provider_id) { - HeapTuple tuple; - TupleDesc tupDesc; - TableScanDesc scan; - ScanKeyData scanKey[1]; GenericKeyring *keyring = NULL; - Oid pg_tde_schema_oid = LookupNamespaceNoError(PG_TDE_NAMESPACE_NAME); - Oid kp_table_oid = get_relname_relid(PG_TDE_KEY_PROVIDER_CAT_NAME, pg_tde_schema_oid); - Relation kp_table_relation = relation_open(kp_table_oid, AccessShareLock); - - /* Set up a scan key to fetch only required record. */ - ScanKeyInit(scanKey, - (AttrNumber)PG_TDE_KEY_PROVIDER_ID_ATTRNUM, - BTEqualStrategyNumber, F_INT4EQ, - Int32GetDatum(provider_id)); - - tupDesc = kp_table_relation->rd_att; - /* Begin the scan with the filter condition */ - scan = heap_beginscan(kp_table_relation, GetLatestSnapshot(), 1, scanKey, NULL, 0); - - while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + List *providers = scan_key_provider_file(PROVIDER_SCAN_BY_ID, &provider_id); + if (providers != NIL) { - keyring = load_keyring_provider_from_tuple(tuple, tupDesc); - break; /* We only expect 1 record */ + keyring = (GenericKeyring *)linitial(providers); + list_free(providers); } - heap_endscan(scan); - relation_close(kp_table_relation, AccessShareLock); - return keyring; } @@ -232,7 +229,9 @@ load_file_keyring_provider_options(Datum keyring_options) if(file_path == NULL) { - /* TODO: report error */ + ereport(DEBUG2, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("File path is missing in the keyring options"))); return NULL; } @@ -267,99 +266,223 @@ load_vaultV2_keyring_provider_options(Datum keyring_options) static void debug_print_kerying(GenericKeyring *keyring) { - elog(DEBUG2, "Keyring type: %d", keyring->type); - elog(DEBUG2, "Keyring name: %s", keyring->provider_name); - elog(DEBUG2, "Keyring id: %d", keyring->key_id); + int debug_level = DEBUG2; + elog(debug_level, "Keyring type: %d", keyring->type); + elog(debug_level, "Keyring name: %s", keyring->provider_name); + elog(debug_level, "Keyring id: %d", keyring->key_id); switch (keyring->type) { case FILE_KEY_PROVIDER: - elog(DEBUG2, "File Keyring Path: %s", ((FileKeyring *)keyring)->file_name); + elog(debug_level, "File Keyring Path: %s", ((FileKeyring *)keyring)->file_name); break; case VAULT_V2_KEY_PROVIDER: - elog(DEBUG2, "Vault Keyring Token: %s", ((VaultV2Keyring *)keyring)->vault_token); - elog(DEBUG2, "Vault Keyring URL: %s", ((VaultV2Keyring *)keyring)->vault_url); - elog(DEBUG2, "Vault Keyring Mount Path: %s", ((VaultV2Keyring *)keyring)->vault_mount_path); - elog(DEBUG2, "Vault Keyring CA Path: %s", ((VaultV2Keyring *)keyring)->vault_ca_path); + elog(debug_level, "Vault Keyring Token: %s", ((VaultV2Keyring *)keyring)->vault_token); + elog(debug_level, "Vault Keyring URL: %s", ((VaultV2Keyring *)keyring)->vault_url); + elog(debug_level, "Vault Keyring Mount Path: %s", ((VaultV2Keyring *)keyring)->vault_mount_path); + elog(debug_level, "Vault Keyring CA Path: %s", ((VaultV2Keyring *)keyring)->vault_ca_path); break; case UNKNOWN_KEY_PROVIDER: - elog(DEBUG2, "Unknown Keyring "); + elog(debug_level, "Unknown Keyring "); break; } } /* - * Trigger function on keyring catalog to ensure the keyring - * used by the principal key should not get deleted. - */ -Datum -keyring_delete_dependency_check_trigger(PG_FUNCTION_ARGS) + * Fetch the next key provider from the file and update the curr_pos +*/ +static bool +fetch_next_key_provider(int fd, off_t* curr_pos, KeyringProvideRecord *provider) { - TriggerData *trig_data = (TriggerData *)fcinfo->context; - Oid principal_key_keyring_id; + off_t bytes_read = 0; + + Assert(provider != NULL); + Assert(fd >= 0); + + bytes_read = pg_pread(fd, provider, sizeof(KeyringProvideRecord), *curr_pos); + *curr_pos += bytes_read; - if (!CALLED_AS_TRIGGER(fcinfo)) + if (bytes_read == 0) + return false; + if (bytes_read != sizeof(KeyringProvideRecord)) { + close(fd); + /* Corrupt file */ ereport(ERROR, - (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), - errmsg("keyring dependency check trigger: not fired by trigger manager"))); + (errcode_for_file_access(), + errmsg("keyring info file is corrupted: %m"), + errdetail("invalid provider record size %lld expected %lu", bytes_read, sizeof(KeyringProvideRecord) ))); } + return true; +} + +/* +* Save the key provider info to the file +*/ +static uint32 +save_key_provider(KeyringProvideRecord *provider) +{ + off_t bytes_written = 0; + off_t curr_pos = 0; + int fd; + int max_provider_id = 0; + char keyrings_path[MAXPGPATH] = {0}; + KeyringProvideRecord existing_provider; + + Assert(provider != NULL); + + get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); - if (!TRIGGER_FIRED_BEFORE(trig_data->tg_event)) + LWLockAcquire(tde_provider_info_lock(), LW_EXCLUSIVE); + + fd = BasicOpenFile(keyrings_path, O_CREAT | O_RDWR | PG_BINARY); + if (fd < 0) + { + LWLockRelease(tde_provider_info_lock()); + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not open tde file \"%s\": %m", keyrings_path))); + } + + /* we also need to verify the name conflixt and generate the next provider ID */ + while (fetch_next_key_provider(fd, &curr_pos, &existing_provider)) + { + if (strcmp(existing_provider.provider_name, provider->provider_name) == 0) + { + close(fd); + LWLockRelease(tde_provider_info_lock()); + ereport(ERROR, + (errcode(ERRCODE_DUPLICATE_OBJECT), + errmsg("Key provider \"%s\" already exists", provider->provider_name))); + } + if (max_provider_id < existing_provider.provider_id) + max_provider_id = existing_provider.provider_id; + } + provider->provider_id = max_provider_id + 1; + /* + * All good, Just add a new provider + * Write key to the end of file + */ + curr_pos = lseek(fd, 0, SEEK_END); + bytes_written = pg_pwrite(fd, provider, sizeof(KeyringProvideRecord), curr_pos); + if (bytes_written != sizeof(KeyringProvideRecord)) { + close(fd); + LWLockRelease(tde_provider_info_lock()); ereport(ERROR, - (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), - errmsg("keyring dependency check trigger: trigger should be fired before delete"))); + (errcode_for_file_access(), + errmsg("key provider info file \"%s\" can't be written: %m", + keyrings_path))); } - principal_key_keyring_id = GetPrincipalKeyProviderId(); - if (principal_key_keyring_id == InvalidOid) + if (pg_fsync(fd) != 0) { - /* No principal key set. We are good to delete anything */ - return PointerGetDatum(trig_data->tg_trigtuple); + close(fd); + LWLockRelease(tde_provider_info_lock()); + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not fsync file \"%s\": %m", + keyrings_path))); } + close(fd); + LWLockRelease(tde_provider_info_lock()); + return provider->provider_id; +} - if (TRIGGER_FIRED_BY_DELETE(trig_data->tg_event)) +/* + * Scan the key provider info file and can also apply filter based on scanType + */ +static List * +scan_key_provider_file(ProviderScanType scanType, void* scanKey) +{ + off_t curr_pos = 0; + int fd; + char keyrings_path[MAXPGPATH] = {0}; + KeyringProvideRecord provider; + List *providers_list = NIL; + + if (scanType != PROVIDER_SCAN_ALL) + Assert(scanKey != NULL); + + get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); + + LWLockAcquire(tde_provider_info_lock(), LW_SHARED); + + fd = BasicOpenFile(keyrings_path, PG_BINARY); + if (fd < 0) { - HeapTuple oldtuple = trig_data->tg_trigtuple; + LWLockRelease(tde_provider_info_lock()); + ereport(DEBUG2, + (errcode_for_file_access(), + errmsg("could not open tde file \"%s\": %m", keyrings_path))); + return NIL; + } + /* we also need to verify the name conflixt and generate the next provider ID */ + while (fetch_next_key_provider(fd, &curr_pos, &provider)) + { + bool match = false; + ereport(DEBUG2, (errmsg("Read key provider ID=%d %s", provider.provider_id, provider.provider_name))); + + if (scanType == PROVIDER_SCAN_BY_NAME) + { + if (strcasecmp(provider.provider_name, (char*)scanKey) == 0) + match = true; + } + else if (scanType == PROVIDER_SCAN_BY_ID) + { + if (provider.provider_id == *(int *)scanKey) + match = true; + } + else if (scanType == PROVIDER_SCAN_BY_TYPE) + { + if (provider.provider_type == *(ProviderType*)scanKey) + match = true; + } + else if (scanType == PROVIDER_SCAN_ALL) + match = true; - if (oldtuple != NULL && SPI_connect() == SPI_OK_CONNECT) + if (match) { - Datum datum; - bool isnull; - Oid provider_id; - datum = heap_getattr(oldtuple, PG_TDE_KEY_PROVIDER_ID_ATTRNUM, trig_data->tg_relation->rd_att, &isnull); - provider_id = DatumGetInt32(datum); - if (provider_id == principal_key_keyring_id) + GenericKeyring *keyring = load_keyring_provider_from_record(&provider); + if (keyring) { - char *keyring_name; - datum = heap_getattr(oldtuple, PG_TDE_KEY_PROVIDER_NAME_ATTRNUM, trig_data->tg_relation->rd_att, &isnull); - keyring_name = TextDatumGetCString(datum); - - ereport(ERROR, - (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), - errmsg("Key provider \"%s\" cannot be deleted", keyring_name), - errdetail("The principal key for the database depends on this key provider."))); - SPI_finish(); - trig_data->tg_trigtuple = NULL; - return PointerGetDatum(NULL); + providers_list = lappend(providers_list, keyring); } - SPI_finish(); } } - else - ereport(ERROR, - (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED), - errmsg("keyring_delete_dependency_check_trigger: unsupported event type"))); + close(fd); + LWLockRelease(tde_provider_info_lock()); + return providers_list; +} - /* Indicate that the operation should proceed */ - return PointerGetDatum(trig_data->tg_trigtuple); +void +cleanup_key_provider_info(Oid databaseId, Oid tablespaceId) +{ + /* Remove the key provider info fileß */ + char keyrings_path[MAXPGPATH] = {0}; + + get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); + PathNameDeleteTemporaryFile(keyrings_path, false); } -/* Testing function */ -PG_FUNCTION_INFO_V1(pg_tde_get_keyprovider); -Datum pg_tde_get_keyprovider(PG_FUNCTION_ARGS); +static char* +get_keyring_infofile_path(char* resPath, Oid dbOid, Oid spcOid) +{ + char *db_path = pg_tde_get_tde_file_dir(dbOid, spcOid); + + join_path_components(resPath, db_path, PG_TDE_KEYRING_FILENAME); + return resPath; +} -Datum pg_tde_get_keyprovider(PG_FUNCTION_ARGS) +Datum +pg_tde_add_key_provider_internal(PG_FUNCTION_ARGS) { - GetAllKeyringProviders(); - PG_RETURN_NULL(); + char *provider_type = text_to_cstring(PG_GETARG_TEXT_PP(0)); + char *provider_name = text_to_cstring(PG_GETARG_TEXT_PP(1)); + char *options = text_to_cstring(PG_GETARG_TEXT_PP(2)); + KeyringProvideRecord provider; + + strncpy(provider.options, options, sizeof(provider.options)); + strncpy(provider.provider_name, provider_name, sizeof(provider.provider_name)); + provider.provider_type = get_keyring_provider_from_typename(provider_type); + save_key_provider(&provider); + PG_RETURN_INT32(provider.provider_id); } diff --git a/src/catalog/tde_principal_key.c b/src/catalog/tde_principal_key.c index 39146851..4bf2129c 100644 --- a/src/catalog/tde_principal_key.c +++ b/src/catalog/tde_principal_key.c @@ -65,7 +65,6 @@ static Size initialize_shared_state(void *start_address); static void initialize_objects_in_dsa_area(dsa_area *dsa, void *raw_dsa_area); static Size cache_area_size(void); static Size required_shared_mem_size(void); -static int required_locks_count(void); static void shared_memory_shutdown(int code, Datum arg); static void principal_key_startup_cleanup(int tde_tbl_count, void *arg); static void clear_principal_key_cache(Oid databaseId) ; @@ -77,7 +76,6 @@ static const TDEShmemSetupRoutine principal_key_info_shmem_routine = { .init_shared_state = initialize_shared_state, .init_dsa_area_objects = initialize_objects_in_dsa_area, .required_shared_mem_size = required_shared_mem_size, - .required_locks_count = required_locks_count, .shmem_kill = shared_memory_shutdown }; @@ -104,12 +102,6 @@ tde_lwlock_mk_cache(void) return &principalKeyLocalState.sharedPrincipalKeyState->Locks[TDE_LWLOCK_MK_CACHE]; } -static int -required_locks_count(void) -{ - return TDE_LWLOCK_COUNT; -} - static Size cache_area_size(void) { @@ -137,7 +129,7 @@ initialize_shared_state(void *start_address) principalKeyLocalState.dsa = NULL; principalKeyLocalState.sharedHash = NULL; - sharedState->Locks = GetNewLWLock(); + sharedState->Locks = GetLWLocks(); principalKeyLocalState.sharedPrincipalKeyState = sharedState; return sizeof(TdePrincipalKeySharedState); } diff --git a/src/common/pg_tde_shmem.c b/src/common/pg_tde_shmem.c index 628bc3b3..bfe7791e 100644 --- a/src/common/pg_tde_shmem.c +++ b/src/common/pg_tde_shmem.c @@ -59,15 +59,7 @@ Size TdeRequiredSharedMemorySize(void) int TdeRequiredLocksCount(void) { - int count = 0; - ListCell *lc; - foreach (lc, registeredShmemRequests) - { - TDEShmemSetupRoutine *routine = (TDEShmemSetupRoutine *)lfirst(lc); - if (routine->required_locks_count) - count += routine->required_locks_count(); - } - return count; + return TDE_LWLOCK_COUNT; } void TdeShmemInit(void) @@ -147,13 +139,10 @@ tde_shmem_shutdown(int code, Datum arg) } /* - * Returns a lock from registered named tranch. - * You must already have indicated number of required locks - * through required_locks_count callback before requesting - * the lock from this function. + * Returns a locks array from registered named tranch. */ -LWLock* -GetNewLWLock(void) +LWLock * +GetLWLocks(void) { return &(GetNamedLWLockTranche(TDE_TRANCHE_NAME))->lock; } diff --git a/src/common/pg_tde_utils.c b/src/common/pg_tde_utils.c index af359693..fe6385ae 100644 --- a/src/common/pg_tde_utils.c +++ b/src/common/pg_tde_utils.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "catalog/pg_class.h" #include "catalog/pg_am.h" +#include "catalog/pg_tablespace_d.h" #include "utils/fmgroids.h" #include "utils/rel.h" #include "utils/snapmgr.h" @@ -215,3 +216,15 @@ extract_json_option_value(Datum top_json, const char* field_name) return NULL; } } + +/* returns the palloc'd string */ +char * +pg_tde_get_tde_file_dir(Oid dbOid, Oid spcOid) +{ + /* If this is a global space, than the call might be in a critial section + * (during XLog write) so we can't do GetDatabasePath as it calls palloc() + */ + if (spcOid == GLOBALTABLESPACE_OID) + return pstrdup("global"); + return GetDatabasePath(dbOid, spcOid); +} diff --git a/src/include/catalog/tde_keyring.h b/src/include/catalog/tde_keyring.h index 5eb310cd..afdf80aa 100644 --- a/src/include/catalog/tde_keyring.h +++ b/src/include/catalog/tde_keyring.h @@ -24,6 +24,7 @@ #define MAX_PROVIDER_NAME_LEN 128 /* pg_tde_key_provider's provider_name size*/ #define MAX_VAULT_V2_KEY_LEN 128 /* From hashi corp docs */ +#define MAX_KEYRING_OPTION_LEN 1024 typedef enum ProviderType { UNKNOWN_KEY_PROVIDER, @@ -60,9 +61,19 @@ typedef union KeyringProviders VaultV2Keyring vault; } KeyringProviders; +/* This record goes into key provider info file */ +typedef struct KeyringProvideRecord +{ + int provider_id; + char provider_name[MAX_PROVIDER_NAME_LEN]; + char options[MAX_KEYRING_OPTION_LEN]; + ProviderType provider_type; +} KeyringProvideRecord; + extern List *GetAllKeyringProviders(void); extern GenericKeyring *GetKeyProviderByName(const char *provider_name); extern GenericKeyring *GetKeyProviderByID(int provider_id); extern ProviderType get_keyring_provider_from_typename(char *provider_type); - +extern void cleanup_key_provider_info(Oid databaseId, Oid tablespaceId); +extern void InitializeKeyProviderInfo(void); #endif /*TDE_KEYRING_H*/ diff --git a/src/include/common/pg_tde_shmem.h b/src/include/common/pg_tde_shmem.h index 2df82470..7fa0c3de 100644 --- a/src/include/common/pg_tde_shmem.h +++ b/src/include/common/pg_tde_shmem.h @@ -20,6 +20,7 @@ typedef enum { TDE_LWLOCK_MK_CACHE, TDE_LWLOCK_MK_FILES, + TDE_LWLOCK_PI_FILES, /* Must be the last entry in the enum */ TDE_LWLOCK_COUNT @@ -43,10 +44,6 @@ typedef struct TDEShmemSetupRoutine * The callback must return the size of the shared memory acquired. */ Size (*required_shared_mem_size)(void); - /* - * Optional callback to return the number of LW locks required. - */ - int (*required_locks_count)(void); /* * Gets called after all shared memory structures are initialized and * here you can create shared memory hash tables or any other shared @@ -60,6 +57,6 @@ extern void RegisterShmemRequest(const TDEShmemSetupRoutine *routine); extern void TdeShmemInit(void); extern Size TdeRequiredSharedMemorySize(void); extern int TdeRequiredLocksCount(void); -extern LWLock * GetNewLWLock(void); +extern LWLock *GetLWLocks(void); #endif /*PG_TDE_SHMEM_H*/ \ No newline at end of file diff --git a/src/include/common/pg_tde_utils.h b/src/include/common/pg_tde_utils.h index 9e2d856e..5ae960c1 100644 --- a/src/include/common/pg_tde_utils.h +++ b/src/include/common/pg_tde_utils.h @@ -10,6 +10,7 @@ #include "postgres.h" #include "nodes/pg_list.h" +#include "common/relpath.h" extern Oid get_tde_table_am_oid(void); extern Oid get_tde2_table_am_oid(void); @@ -19,4 +20,5 @@ 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); +extern char *pg_tde_get_tde_file_dir(Oid dbOid, Oid spcOid); #endif /*PG_TDE_UTILS_H*/ diff --git a/src/pg_tde.c b/src/pg_tde.c index 632dc42c..ed21b0c3 100644 --- a/src/pg_tde.c +++ b/src/pg_tde.c @@ -103,6 +103,7 @@ _PG_init(void) keyringRegisterVariables(); InitializePrincipalKeyInfo(); + InitializeKeyProviderInfo(); #ifdef PERCONA_FORK XLogInitGUC(); TDEGlCatInitGUC(); From 362ce9c0b47171e66abcbe51298a488ab3751ef4 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Wed, 3 Jul 2024 19:54:36 +0500 Subject: [PATCH 2/7] Fix test case --- expected/keyprovider_dependency.out | 7 ------- sql/keyprovider_dependency.sql | 6 ------ 2 files changed, 13 deletions(-) diff --git a/expected/keyprovider_dependency.out b/expected/keyprovider_dependency.out index ea5eec97..e9bc3d2f 100644 --- a/expected/keyprovider_dependency.out +++ b/expected/keyprovider_dependency.out @@ -23,11 +23,4 @@ SELECT pg_tde_set_principal_key('test-db-principal-key','mk-file'); t (1 row) --- Try dropping the in-use key provider -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'mk-file'; -- Should fail -ERROR: Key provider "mk-file" cannot be deleted -DETAIL: The principal key for the database depends on this key provider. --- Now delete the un-used key provider -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'free-file'; -- Should pass -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'V2-vault'; -- Should pass DROP EXTENSION pg_tde; diff --git a/sql/keyprovider_dependency.sql b/sql/keyprovider_dependency.sql index 40fb59f8..d9c88dd4 100644 --- a/sql/keyprovider_dependency.sql +++ b/sql/keyprovider_dependency.sql @@ -6,10 +6,4 @@ SELECT pg_tde_add_key_provider_vault_v2('V2-vault','vault-token','percona.com/va SELECT pg_tde_set_principal_key('test-db-principal-key','mk-file'); --- Try dropping the in-use key provider -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'mk-file'; -- Should fail --- Now delete the un-used key provider -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'free-file'; -- Should pass -DELETE FROM percona_tde.pg_tde_key_provider WHERE provider_name = 'V2-vault'; -- Should pass - DROP EXTENSION pg_tde; From 7f3d4fecc3d4a660f9e039cbcd3c157bfb1ba7ed Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Fri, 5 Jul 2024 14:30:03 +0500 Subject: [PATCH 3/7] Update src/catalog/tde_keyring.c Co-authored-by: Andrew Pogrebnoi --- src/catalog/tde_keyring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/catalog/tde_keyring.c b/src/catalog/tde_keyring.c index 8fcc58fa..cdae2d7a 100644 --- a/src/catalog/tde_keyring.c +++ b/src/catalog/tde_keyring.c @@ -343,7 +343,7 @@ save_key_provider(KeyringProvideRecord *provider) errmsg("could not open tde file \"%s\": %m", keyrings_path))); } - /* we also need to verify the name conflixt and generate the next provider ID */ + /* we also need to verify the name conflict and generate the next provider ID */ while (fetch_next_key_provider(fd, &curr_pos, &existing_provider)) { if (strcmp(existing_provider.provider_name, provider->provider_name) == 0) From 6298d44faa365be7c2fece4d30173a747a6aa803 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Fri, 5 Jul 2024 14:32:34 +0500 Subject: [PATCH 4/7] Update src/common/pg_tde_utils.c Co-authored-by: Andrew Pogrebnoi --- src/common/pg_tde_utils.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/pg_tde_utils.c b/src/common/pg_tde_utils.c index fe6385ae..b44176ef 100644 --- a/src/common/pg_tde_utils.c +++ b/src/common/pg_tde_utils.c @@ -221,8 +221,8 @@ extract_json_option_value(Datum top_json, const char* field_name) char * pg_tde_get_tde_file_dir(Oid dbOid, Oid spcOid) { - /* If this is a global space, than the call might be in a critial section - * (during XLog write) so we can't do GetDatabasePath as it calls palloc() + /* `dbOid` is set to a value for the XLog keys caching but GetDatabasePath() + * expects it (`dbOid`) to be `0` if this is a global space. */ if (spcOid == GLOBALTABLESPACE_OID) return pstrdup("global"); From 343955af434d1ae248169cef7cf4088c06f4e16e Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Fri, 5 Jul 2024 14:48:59 +0500 Subject: [PATCH 5/7] Taking care of review comments Refactor log/error messages and variable names; fix memory leak in get_keyring_infofile_path function --- src/catalog/tde_keyring.c | 51 ++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/src/catalog/tde_keyring.c b/src/catalog/tde_keyring.c index cdae2d7a..06d1acf2 100644 --- a/src/catalog/tde_keyring.c +++ b/src/catalog/tde_keyring.c @@ -94,7 +94,6 @@ static Size initialize_shared_state(void *start_address) { sharedPrincipalKeyState = (TdeKeyProviderInfoSharedState *)start_address; - ereport(LOG, (errmsg("initializing shared state for primary key info"))); sharedPrincipalKeyState->Locks = GetLWLocks(); return sizeof(TdeKeyProviderInfoSharedState); } @@ -108,7 +107,7 @@ tde_provider_info_lock(void) void InitializeKeyProviderInfo(void) { - ereport(LOG, (errmsg("Initializing TDE principal key info"))); + ereport(LOG, (errmsg("initializing TDE key provider info"))); RegisterShmemRequest(&key_provider_info_shmem_routine); on_ext_install(key_provider_startup_cleanup, NULL); } @@ -119,7 +118,7 @@ key_provider_startup_cleanup(int tde_tbl_count, void *arg) if (tde_tbl_count > 0) { ereport(WARNING, - (errmsg("Failed to perform initialization. database already has %d TDE tables", tde_tbl_count))); + (errmsg("failed to perform initialization. database already has %d TDE tables", tde_tbl_count))); return; } cleanup_key_provider_info(MyDatabaseId, MyDatabaseTableSpace); @@ -184,9 +183,9 @@ GetKeyProviderByName(const char *provider_name) else { ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("Key provider \"%s\" does not exists", provider_name), - errhint("Use create_key_provider interface to create the key provider"))); + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("Key provider \"%s\" does not exists", provider_name), + errhint("Use pg_tde_add_key_provider interface to create the key provider"))); } return keyring; } @@ -231,7 +230,7 @@ load_file_keyring_provider_options(Datum keyring_options) { ereport(DEBUG2, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("File path is missing in the keyring options"))); + errmsg("file path is missing in the keyring options"))); return NULL; } @@ -309,8 +308,8 @@ fetch_next_key_provider(int fd, off_t* curr_pos, KeyringProvideRecord *provider) /* Corrupt file */ ereport(ERROR, (errcode_for_file_access(), - errmsg("keyring info file is corrupted: %m"), - errdetail("invalid provider record size %lld expected %lu", bytes_read, sizeof(KeyringProvideRecord) ))); + errmsg("key provider info file is corrupted: %m"), + errdetail("invalid key provider record size %lld expected %lu", bytes_read, sizeof(KeyringProvideRecord) ))); } return true; } @@ -325,22 +324,22 @@ save_key_provider(KeyringProvideRecord *provider) off_t curr_pos = 0; int fd; int max_provider_id = 0; - char keyrings_path[MAXPGPATH] = {0}; + char kp_info_path[MAXPGPATH] = {0}; KeyringProvideRecord existing_provider; Assert(provider != NULL); - get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); + get_keyring_infofile_path(kp_info_path, MyDatabaseId, MyDatabaseTableSpace); LWLockAcquire(tde_provider_info_lock(), LW_EXCLUSIVE); - fd = BasicOpenFile(keyrings_path, O_CREAT | O_RDWR | PG_BINARY); + fd = BasicOpenFile(kp_info_path, O_CREAT | O_RDWR | PG_BINARY); if (fd < 0) { LWLockRelease(tde_provider_info_lock()); ereport(ERROR, (errcode_for_file_access(), - errmsg("could not open tde file \"%s\": %m", keyrings_path))); + errmsg("could not open tde file \"%s\": %m", kp_info_path))); } /* we also need to verify the name conflict and generate the next provider ID */ @@ -352,7 +351,7 @@ save_key_provider(KeyringProvideRecord *provider) LWLockRelease(tde_provider_info_lock()); ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("Key provider \"%s\" already exists", provider->provider_name))); + errmsg("key provider \"%s\" already exists", provider->provider_name))); } if (max_provider_id < existing_provider.provider_id) max_provider_id = existing_provider.provider_id; @@ -371,7 +370,7 @@ save_key_provider(KeyringProvideRecord *provider) ereport(ERROR, (errcode_for_file_access(), errmsg("key provider info file \"%s\" can't be written: %m", - keyrings_path))); + kp_info_path))); } if (pg_fsync(fd) != 0) { @@ -380,7 +379,7 @@ save_key_provider(KeyringProvideRecord *provider) ereport(ERROR, (errcode_for_file_access(), errmsg("could not fsync file \"%s\": %m", - keyrings_path))); + kp_info_path))); } close(fd); LWLockRelease(tde_provider_info_lock()); @@ -395,31 +394,32 @@ scan_key_provider_file(ProviderScanType scanType, void* scanKey) { off_t curr_pos = 0; int fd; - char keyrings_path[MAXPGPATH] = {0}; + char kp_info_path[MAXPGPATH] = {0}; KeyringProvideRecord provider; List *providers_list = NIL; if (scanType != PROVIDER_SCAN_ALL) Assert(scanKey != NULL); - get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); + get_keyring_infofile_path(kp_info_path, MyDatabaseId, MyDatabaseTableSpace); LWLockAcquire(tde_provider_info_lock(), LW_SHARED); - fd = BasicOpenFile(keyrings_path, PG_BINARY); + fd = BasicOpenFile(kp_info_path, PG_BINARY); if (fd < 0) { LWLockRelease(tde_provider_info_lock()); ereport(DEBUG2, (errcode_for_file_access(), - errmsg("could not open tde file \"%s\": %m", keyrings_path))); + errmsg("could not open tde file \"%s\": %m", kp_info_path))); return NIL; } /* we also need to verify the name conflixt and generate the next provider ID */ while (fetch_next_key_provider(fd, &curr_pos, &provider)) { bool match = false; - ereport(DEBUG2, (errmsg("Read key provider ID=%d %s", provider.provider_id, provider.provider_name))); + ereport(DEBUG2, + (errmsg("read key provider ID=%d %s", provider.provider_id, provider.provider_name))); if (scanType == PROVIDER_SCAN_BY_NAME) { @@ -457,18 +457,19 @@ void cleanup_key_provider_info(Oid databaseId, Oid tablespaceId) { /* Remove the key provider info fileß */ - char keyrings_path[MAXPGPATH] = {0}; + char kp_info_path[MAXPGPATH] = {0}; - get_keyring_infofile_path(keyrings_path, MyDatabaseId, MyDatabaseTableSpace); - PathNameDeleteTemporaryFile(keyrings_path, false); + get_keyring_infofile_path(kp_info_path, MyDatabaseId, MyDatabaseTableSpace); + PathNameDeleteTemporaryFile(kp_info_path, false); } static char* get_keyring_infofile_path(char* resPath, Oid dbOid, Oid spcOid) { char *db_path = pg_tde_get_tde_file_dir(dbOid, spcOid); - + Assert(db_path != NULL); join_path_components(resPath, db_path, PG_TDE_KEYRING_FILENAME); + pfree(db_path); return resPath; } From 1839d39c17c2a084e966d01f2a8ae567ea0e446d Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Fri, 5 Jul 2024 18:11:31 +0500 Subject: [PATCH 6/7] Update src/catalog/tde_keyring.c Co-authored-by: Andrew Pogrebnoi --- src/catalog/tde_keyring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/catalog/tde_keyring.c b/src/catalog/tde_keyring.c index 06d1acf2..7b8f74e5 100644 --- a/src/catalog/tde_keyring.c +++ b/src/catalog/tde_keyring.c @@ -184,7 +184,7 @@ GetKeyProviderByName(const char *provider_name) { ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("Key provider \"%s\" does not exists", provider_name), + errmsg("key provider \"%s\" does not exists", provider_name), errhint("Use pg_tde_add_key_provider interface to create the key provider"))); } return keyring; From 3025907f47918fea03685b7d45bc5af707dd75a8 Mon Sep 17 00:00:00 2001 From: Muhammad Usama Date: Fri, 5 Jul 2024 18:11:49 +0500 Subject: [PATCH 7/7] Update src/catalog/tde_keyring.c Co-authored-by: Andrew Pogrebnoi --- src/catalog/tde_keyring.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/catalog/tde_keyring.c b/src/catalog/tde_keyring.c index 7b8f74e5..721f9191 100644 --- a/src/catalog/tde_keyring.c +++ b/src/catalog/tde_keyring.c @@ -414,7 +414,6 @@ scan_key_provider_file(ProviderScanType scanType, void* scanKey) errmsg("could not open tde file \"%s\": %m", kp_info_path))); return NIL; } - /* we also need to verify the name conflixt and generate the next provider ID */ while (fetch_next_key_provider(fd, &curr_pos, &provider)) { bool match = false;