From 34dd50b4bc91d7283fff5d73fc66e15f65b5ed08 Mon Sep 17 00:00:00 2001 From: Andrew Pogrebnoy Date: Wed, 20 Nov 2024 15:24:48 +0200 Subject: [PATCH] Don't use tablespaceId with the principal key We use tablespaceId as a part of IV for the internal keys encryption which doesn't add any security because dbId (used as well) is unique anyway. But having tablespaceId really complicates things as a principal created for the entire database but then different relations in this db can be located in different tablespaces... So it is better not to use tablespace with the principal key (database level) as it belongs to the relation level. --- src/access/pg_tde_tdemap.c | 31 +++++++++++-------------- src/access/pg_tde_xlog.c | 3 +-- src/access/pg_tde_xlog_encrypt.c | 1 - src/catalog/tde_global_space.c | 12 ++++------ src/catalog/tde_principal_key.c | 9 ++++--- src/common/pg_tde_utils.c | 1 - src/encryption/enc_tde.c | 10 ++++---- src/include/access/pg_tde_tdemap.h | 5 ++-- src/include/catalog/tde_global_space.h | 1 + src/include/catalog/tde_principal_key.h | 1 - src/include/encryption/enc_tde.h | 4 ++-- src/pg_tde_event_capture.c | 6 ----- 12 files changed, 32 insertions(+), 52 deletions(-) diff --git a/src/access/pg_tde_tdemap.c b/src/access/pg_tde_tdemap.c index cb1d5c21..6809d23b 100644 --- a/src/access/pg_tde_tdemap.c +++ b/src/access/pg_tde_tdemap.c @@ -187,7 +187,7 @@ pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, uint32 entry_type /* Encrypt the key */ rel_key_data = tde_create_rel_key(newrlocator->relNumber, &int_key, &principal_key->keyInfo); - enc_rel_key_data = tde_encrypt_rel_key(principal_key, rel_key_data, newrlocator); + enc_rel_key_data = tde_encrypt_rel_key(principal_key, rel_key_data, newrlocator->dbOid); /* * XLOG internal key @@ -241,12 +241,12 @@ tde_create_rel_key(RelFileNumber rel_num, InternalKey *key, TDEPrincipalKeyInfo * Encrypts a given key and returns the encrypted one. */ RelKeyData * -tde_encrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *rel_key_data, const RelFileLocator *rlocator) +tde_encrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *rel_key_data, Oid dbOid) { RelKeyData *enc_rel_key_data; size_t enc_key_bytes; - AesEncryptKey(principal_key, rlocator, rel_key_data, &enc_rel_key_data, &enc_key_bytes); + AesEncryptKey(principal_key, dbOid, rel_key_data, &enc_rel_key_data, &enc_key_bytes); return enc_rel_key_data; } @@ -713,14 +713,13 @@ pg_tde_perform_rotate_key(TDEPrincipalKey *principal_key, TDEPrincipalKey *new_p rloc.relNumber = read_map_entry.relNumber; rloc.dbOid = principal_key->keyInfo.databaseId; - rloc.spcOid = principal_key->keyInfo.tablespaceId; /* Let's get the decrypted key and re-encrypt it with the new key. */ enc_rel_key_data[OLD_PRINCIPAL_KEY] = pg_tde_read_one_keydata(k_fd[OLD_PRINCIPAL_KEY], key_index[OLD_PRINCIPAL_KEY], principal_key); /* Decrypt and re-encrypt keys */ - rel_key_data[OLD_PRINCIPAL_KEY] = tde_decrypt_rel_key(principal_key, enc_rel_key_data[OLD_PRINCIPAL_KEY], &rloc); - enc_rel_key_data[NEW_PRINCIPAL_KEY] = tde_encrypt_rel_key(new_principal_key, rel_key_data[OLD_PRINCIPAL_KEY], &rloc); + rel_key_data[OLD_PRINCIPAL_KEY] = tde_decrypt_rel_key(principal_key, enc_rel_key_data[OLD_PRINCIPAL_KEY], principal_key->keyInfo.databaseId); + enc_rel_key_data[NEW_PRINCIPAL_KEY] = tde_encrypt_rel_key(new_principal_key, rel_key_data[OLD_PRINCIPAL_KEY], principal_key->keyInfo.databaseId); /* Write the given entry at the location pointed by prev_pos */ prev_pos[NEW_PRINCIPAL_KEY] = curr_pos[NEW_PRINCIPAL_KEY]; @@ -853,8 +852,7 @@ pg_tde_write_map_keydata_files(off_t map_size, char *m_file_data, off_t keydata_ } /* - * Move relation's key - re-encrypts and saves the relation key with the new - * relfilenode. + * Saves the relation key with the new relfilenode. * Needed by ALTER TABLE SET TABLESPACE for example. */ bool @@ -878,15 +876,14 @@ pg_tde_move_rel_key(const RelFileLocator *newrlocator, const RelFileLocator *old principal_key = GetPrincipalKey(oldrlocator->dbOid, LW_EXCLUSIVE); Assert(principal_key); + /* + * We don't use internal_key cache to avoid locking complications. + */ key_index = pg_tde_process_map_entry(oldrlocator, MAP_ENTRY_VALID, db_map_path, &offset, false); Assert(key_index != -1); - /* - * Re-encrypt relation key. We don't use internal_key cache to avoid locking - * complications. - */ + enc_key = pg_tde_read_keydata(db_keydata_path, key_index, principal_key); - rel_key = tde_decrypt_rel_key(principal_key, enc_key, oldrlocator); - enc_key = tde_encrypt_rel_key(principal_key, rel_key, newrlocator); + rel_key = tde_decrypt_rel_key(principal_key, enc_key, oldrlocator->dbOid); xlrec.rlocator = *newrlocator; xlrec.relKey = *enc_key; @@ -974,7 +971,7 @@ pg_tde_get_key_from_file(const RelFileLocator *rlocator, uint32 key_type, bool n enc_rel_key_data = pg_tde_read_keydata(db_keydata_path, key_index, principal_key); LWLockRelease(lock_pk); - rel_key_data = tde_decrypt_rel_key(principal_key, enc_rel_key_data, rlocator); + rel_key_data = tde_decrypt_rel_key(principal_key, enc_rel_key_data, rlocator->dbOid); return rel_key_data; } @@ -1098,12 +1095,12 @@ pg_tde_read_keydata(char *db_keydata_path, int32 key_index, TDEPrincipalKey *pri * Decrypts a given key and returns the decrypted one. */ RelKeyData * -tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data, const RelFileLocator *rlocator) +tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data, Oid dbOid) { RelKeyData *rel_key_data = NULL; size_t key_bytes; - AesDecryptKey(principal_key, rlocator, &rel_key_data, enc_rel_key_data, &key_bytes); + AesDecryptKey(principal_key, dbOid, &rel_key_data, enc_rel_key_data, &key_bytes); return rel_key_data; } diff --git a/src/access/pg_tde_xlog.c b/src/access/pg_tde_xlog.c index bd065445..f7583da0 100644 --- a/src/access/pg_tde_xlog.c +++ b/src/access/pg_tde_xlog.c @@ -17,7 +17,6 @@ #include "access/xlog.h" #include "access/xlog_internal.h" #include "access/xloginsert.h" -#include "catalog/pg_tablespace_d.h" #include "catalog/tde_keyring.h" #include "storage/bufmgr.h" #include "storage/shmem.h" @@ -108,7 +107,7 @@ tdeheap_rmgr_desc(StringInfo buf, XLogReaderState *record) { TDEPrincipalKeyInfo *xlrec = (TDEPrincipalKeyInfo *) XLogRecGetData(record); - appendStringInfo(buf, "add tde principal key for db %u/%u", xlrec->databaseId, xlrec->tablespaceId); + appendStringInfo(buf, "add tde principal key for db %u", xlrec->databaseId); } if (info == XLOG_TDE_EXTENSION_INSTALL_KEY) { diff --git a/src/access/pg_tde_xlog_encrypt.c b/src/access/pg_tde_xlog_encrypt.c index 804e5917..5c40f73e 100644 --- a/src/access/pg_tde_xlog_encrypt.c +++ b/src/access/pg_tde_xlog_encrypt.c @@ -18,7 +18,6 @@ #include "access/xlog.h" #include "access/xlog_internal.h" #include "access/xloginsert.h" -#include "catalog/pg_tablespace_d.h" #include "storage/bufmgr.h" #include "storage/shmem.h" #include "utils/guc.h" diff --git a/src/catalog/tde_global_space.c b/src/catalog/tde_global_space.c index be1294e2..936f6d3b 100644 --- a/src/catalog/tde_global_space.c +++ b/src/catalog/tde_global_space.c @@ -14,7 +14,6 @@ #ifdef PERCONA_EXT -#include "catalog/pg_tablespace_d.h" #include "utils/memutils.h" #include "access/pg_tde_tdemap.h" @@ -42,8 +41,7 @@ static void init_keys(void); static void init_default_keyring(void); static TDEPrincipalKey *create_principal_key(const char *key_name, - GenericKeyring *keyring, Oid dbOid, - Oid spcOid); + GenericKeyring *keyring, Oid dbOid); #endif /* !FRONTEND */ @@ -144,7 +142,7 @@ init_keys(void) mkey = create_principal_key(PRINCIPAL_KEY_DEFAULT_NAME, DefaultKeyProvider, - GLOBAL_DATA_TDE_OID, GLOBALTABLESPACE_OID); + GLOBAL_DATA_TDE_OID); memset(&int_key, 0, sizeof(InternalKey)); @@ -161,7 +159,7 @@ init_keys(void) rlocator = &GLOBAL_SPACE_RLOCATOR(XLOG_TDE_OID); rel_key_data = tde_create_rel_key(rlocator->relNumber, &int_key, &mkey->keyInfo); - enc_rel_key_data = tde_encrypt_rel_key(mkey, rel_key_data, rlocator); + enc_rel_key_data = tde_encrypt_rel_key(mkey, rel_key_data, rlocator->dbOid); pg_tde_write_key_map_entry(rlocator, enc_rel_key_data, &mkey->keyInfo); pfree(enc_rel_key_data); pfree(mkey); @@ -177,15 +175,13 @@ init_keys(void) * first. */ static TDEPrincipalKey * -create_principal_key(const char *key_name, GenericKeyring *keyring, - Oid dbOid, Oid spcOid) +create_principal_key(const char *key_name, GenericKeyring *keyring, Oid dbOid) { TDEPrincipalKey *principalKey; keyInfo *keyInfo = NULL; principalKey = palloc(sizeof(TDEPrincipalKey)); principalKey->keyInfo.databaseId = dbOid; - principalKey->keyInfo.tablespaceId = spcOid; principalKey->keyInfo.keyId.version = DEFAULT_PRINCIPAL_KEY_VERSION; principalKey->keyInfo.keyringId = keyring->key_id; strncpy(principalKey->keyInfo.keyId.name, key_name, TDE_KEY_NAME_LEN); diff --git a/src/catalog/tde_principal_key.c b/src/catalog/tde_principal_key.c index 5095fdab..8d805382 100644 --- a/src/catalog/tde_principal_key.c +++ b/src/catalog/tde_principal_key.c @@ -84,7 +84,7 @@ static keyInfo *load_latest_versioned_key_name(TDEPrincipalKeyInfo *principal_ke bool ensure_new_key); static TDEPrincipalKey *set_principal_key_with_keyring(const char *key_name, GenericKeyring *keyring, - Oid dbOid, Oid spcOid, + Oid dbOid, bool ensure_new_key); static const TDEShmemSetupRoutine principal_key_info_shmem_routine = { @@ -222,7 +222,7 @@ save_principal_key_info(TDEPrincipalKeyInfo *principal_key_info) */ TDEPrincipalKey * set_principal_key_with_keyring(const char *key_name, GenericKeyring *keyring, - Oid dbOid, Oid spcOid, bool ensure_new_key) + Oid dbOid, bool ensure_new_key) { TDEPrincipalKey *principalKey = NULL; LWLock *lock_files = tde_lwlock_enc_keys(); @@ -246,7 +246,6 @@ set_principal_key_with_keyring(const char *key_name, GenericKeyring *keyring, principalKey = palloc(sizeof(TDEPrincipalKey)); principalKey->keyInfo.databaseId = dbOid; - principalKey->keyInfo.tablespaceId = spcOid; principalKey->keyInfo.keyId.version = DEFAULT_PRINCIPAL_KEY_VERSION; principalKey->keyInfo.keyringId = keyring->key_id; strncpy(principalKey->keyInfo.keyId.name, key_name, TDE_KEY_NAME_LEN); @@ -302,7 +301,7 @@ SetPrincipalKey(const char *key_name, const char *provider_name, bool ensure_new { TDEPrincipalKey *principal_key = set_principal_key_with_keyring(key_name, GetKeyProviderByName(provider_name, MyDatabaseId), - MyDatabaseId, MyDatabaseTableSpace, + MyDatabaseId, ensure_new_key); return (principal_key != NULL); @@ -366,7 +365,7 @@ RotatePrincipalKey(TDEPrincipalKey *current_key, const char *new_key_name, const memcpy(new_principal_key.keyData, keyInfo->data.data, keyInfo->data.len); is_rotated = pg_tde_perform_rotate_key(current_key, &new_principal_key); - if (is_rotated && current_key->keyInfo.tablespaceId != GLOBALTABLESPACE_OID) + if (is_rotated && !TDEisInGlobalSpace(current_key->keyInfo.databaseId)) { clear_principal_key_cache(current_key->keyInfo.databaseId); push_principal_key_to_cache(&new_principal_key); diff --git a/src/common/pg_tde_utils.c b/src/common/pg_tde_utils.c index d12f6040..d8fdbe1d 100644 --- a/src/common/pg_tde_utils.c +++ b/src/common/pg_tde_utils.c @@ -11,7 +11,6 @@ #include "postgres.h" -#include "catalog/pg_tablespace_d.h" #include "utils/snapmgr.h" #include "commands/defrem.h" #include "common/pg_tde_utils.h" diff --git a/src/encryption/enc_tde.c b/src/encryption/enc_tde.c index c4e3abbe..a24c7d16 100644 --- a/src/encryption/enc_tde.c +++ b/src/encryption/enc_tde.c @@ -240,15 +240,14 @@ PGTdePageAddItemExtended(RelFileLocator rel, * short lifespan until it is written to disk. */ void -AesEncryptKey(const TDEPrincipalKey *principal_key, const RelFileLocator *rlocator, RelKeyData *rel_key_data, RelKeyData **p_enc_rel_key_data, size_t *enc_key_bytes) +AesEncryptKey(const TDEPrincipalKey *principal_key, Oid dbOid, RelKeyData *rel_key_data, RelKeyData **p_enc_rel_key_data, size_t *enc_key_bytes) { unsigned char iv[16] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; /* Ensure we are getting a valid pointer here */ Assert(principal_key); - memcpy(iv, &rlocator->spcOid, sizeof(Oid)); - memcpy(iv + sizeof(Oid), &rlocator->dbOid, sizeof(Oid)); + memcpy(iv, &dbOid, sizeof(Oid)); *p_enc_rel_key_data = (RelKeyData *) palloc(sizeof(RelKeyData)); memcpy(*p_enc_rel_key_data, rel_key_data, sizeof(RelKeyData)); @@ -266,15 +265,14 @@ AesEncryptKey(const TDEPrincipalKey *principal_key, const RelFileLocator *rlocat * to our key cache. */ void -AesDecryptKey(const TDEPrincipalKey *principal_key, const RelFileLocator *rlocator, RelKeyData **p_rel_key_data, RelKeyData *enc_rel_key_data, size_t *key_bytes) +AesDecryptKey(const TDEPrincipalKey *principal_key, Oid dbOid, RelKeyData **p_rel_key_data, RelKeyData *enc_rel_key_data, size_t *key_bytes) { unsigned char iv[16] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}; /* Ensure we are getting a valid pointer here */ Assert(principal_key); - memcpy(iv, &rlocator->spcOid, sizeof(Oid)); - memcpy(iv + sizeof(Oid), &rlocator->dbOid, sizeof(Oid)); + memcpy(iv, &dbOid, sizeof(Oid)); #ifndef FRONTEND MemoryContext oldcontext; diff --git a/src/include/access/pg_tde_tdemap.h b/src/include/access/pg_tde_tdemap.h index 2c7ec6dd..9ab39578 100644 --- a/src/include/access/pg_tde_tdemap.h +++ b/src/include/access/pg_tde_tdemap.h @@ -11,7 +11,6 @@ #include "pg_tde.h" #include "utils/rel.h" #include "access/xlog_internal.h" -#include "catalog/pg_tablespace_d.h" #include "catalog/tde_principal_key.h" #include "storage/relfilelocator.h" @@ -70,8 +69,8 @@ extern bool pg_tde_save_principal_key(TDEPrincipalKeyInfo *principal_key_info); extern bool pg_tde_perform_rotate_key(TDEPrincipalKey *principal_key, TDEPrincipalKey *new_principal_key); extern bool pg_tde_write_map_keydata_files(off_t map_size, char *m_file_data, off_t keydata_size, char *k_file_data); extern RelKeyData *tde_create_rel_key(RelFileNumber rel_num, InternalKey *key, TDEPrincipalKeyInfo *principal_key_info); -extern RelKeyData *tde_encrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *rel_key_data, const RelFileLocator *rlocator); -extern RelKeyData *tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data, const RelFileLocator *rlocator); +extern RelKeyData *tde_encrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *rel_key_data, Oid dbOid); +extern RelKeyData *tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data, Oid dbOid); extern RelKeyData *pg_tde_get_key_from_file(const RelFileLocator *rlocator, uint32 key_type, bool no_map_ok); extern bool pg_tde_move_rel_key(const RelFileLocator *newrlocator, const RelFileLocator *oldrlocator); diff --git a/src/include/catalog/tde_global_space.h b/src/include/catalog/tde_global_space.h index 88099eb4..0656ef4d 100644 --- a/src/include/catalog/tde_global_space.h +++ b/src/include/catalog/tde_global_space.h @@ -12,6 +12,7 @@ #define TDE_GLOBAL_CATALOG_H #include "postgres.h" +#include "catalog/pg_tablespace_d.h" #include "access/pg_tde_tdemap.h" #include "catalog/tde_principal_key.h" diff --git a/src/include/catalog/tde_principal_key.h b/src/include/catalog/tde_principal_key.h index 2ac1ddc6..5ba90646 100644 --- a/src/include/catalog/tde_principal_key.h +++ b/src/include/catalog/tde_principal_key.h @@ -33,7 +33,6 @@ typedef struct TDEPrincipalKeyId typedef struct TDEPrincipalKeyInfo { Oid databaseId; - Oid tablespaceId; Oid userId; Oid keyringId; struct timeval creationTime; diff --git a/src/include/encryption/enc_tde.h b/src/include/encryption/enc_tde.h index e1654c30..552888b3 100644 --- a/src/include/encryption/enc_tde.h +++ b/src/include/encryption/enc_tde.h @@ -52,7 +52,7 @@ extern OffsetNumber pg_tde_crypt(_iv_prefix, _start_offset, _data, _data_len, _out, _key, "ENCRYPT-PAGE-ITEM"); \ } while(0) -extern void AesEncryptKey(const TDEPrincipalKey *principal_key, const RelFileLocator *rlocator, RelKeyData *rel_key_data, RelKeyData **p_enc_rel_key_data, size_t *enc_key_bytes); -extern void AesDecryptKey(const TDEPrincipalKey *principal_key, const RelFileLocator *rlocator, RelKeyData **p_rel_key_data, RelKeyData *enc_rel_key_data, size_t *key_bytes); +extern void AesEncryptKey(const TDEPrincipalKey *principal_key, Oid dbOid, RelKeyData *rel_key_data, RelKeyData **p_enc_rel_key_data, size_t *enc_key_bytes); +extern void AesDecryptKey(const TDEPrincipalKey *principal_key, Oid dbOid, RelKeyData **p_rel_key_data, RelKeyData *enc_rel_key_data, size_t *key_bytes); #endif /* ENC_TDE_H */ diff --git a/src/pg_tde_event_capture.c b/src/pg_tde_event_capture.c index beba9d0b..8d7794ae 100644 --- a/src/pg_tde_event_capture.c +++ b/src/pg_tde_event_capture.c @@ -21,7 +21,6 @@ #include "commands/event_trigger.h" #include "common/pg_tde_utils.h" #include "pg_tde_event_capture.h" -#include "commands/tablespace.h" #include "catalog/tde_principal_key.h" #include "miscadmin.h" #include "access/tableam.h" @@ -149,11 +148,6 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS) } } - /* - * TODO: also check for tablespace change, if current or new AM is - * tde_heap! - */ - if (tdeCurrentCreateEvent.encryptMode) { TDEPrincipalKey * principal_key;