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.
percona · Nov 20, 2024 · c22b353 · c22b353
1 parent a713cdc
commit c22b353
Show file tree

Hide file tree

Showing 12 changed files with 28 additions and 50 deletions.
diff --git 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
@@ -881,12 +879,9 @@ pg_tde_move_rel_key(const RelFileLocator *newrlocator, const RelFileLocator *old
 	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.
+	 * 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);
 
 	xlrec.rlocator = *newrlocator;
 	xlrec.relKey = *enc_key;
@@ -974,7 +969,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 +1093,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;