From 3572393ca44036df64c49089680e280969d83233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85strand?= Date: Thu, 19 Jun 2025 16:36:55 +0200 Subject: [PATCH 1/2] DO NOT MERGE Protect decrypted relation keys Using openssl's secure allocators for these ensures they are never swapped to disk or present in a core dump. Using ResourceOwner for them ensures we don't leak memory when transactions are aborted. This commit is mostly a poc for only keeping encrypted keys in the smgr cache, as that memory is not protected. --- contrib/pg_tde/Makefile | 1 + contrib/pg_tde/meson.build | 1 + contrib/pg_tde/src/access/pg_tde_tdemap.c | 109 ++++++++---- contrib/pg_tde/src/encryption/tde_keys.c | 168 ++++++++++++++++++ .../pg_tde/src/include/access/pg_tde_tdemap.h | 5 +- .../pg_tde/src/include/encryption/tde_keys.h | 39 ++++ contrib/pg_tde/src/smgr/pg_tde_smgr.c | 151 ++++++++++------ 7 files changed, 385 insertions(+), 89 deletions(-) create mode 100644 contrib/pg_tde/src/encryption/tde_keys.c create mode 100644 contrib/pg_tde/src/include/encryption/tde_keys.h diff --git a/contrib/pg_tde/Makefile b/contrib/pg_tde/Makefile index a8b7458b411f8..24795d0ef6b12 100644 --- a/contrib/pg_tde/Makefile +++ b/contrib/pg_tde/Makefile @@ -29,6 +29,7 @@ endif OBJS = src/encryption/enc_tde.o \ src/encryption/enc_aes.o \ +src/encryption/tde_keys.o \ src/access/pg_tde_tdemap.o \ src/access/pg_tde_xlog.o \ src/access/pg_tde_xlog_smgr.o \ diff --git a/contrib/pg_tde/meson.build b/contrib/pg_tde/meson.build index 21f5b13c445a8..6fbae648da98a 100644 --- a/contrib/pg_tde/meson.build +++ b/contrib/pg_tde/meson.build @@ -10,6 +10,7 @@ pg_tde_sources = files( 'src/common/pg_tde_utils.c', 'src/encryption/enc_aes.c', 'src/encryption/enc_tde.c', + 'src/encryption/tde_keys.c', 'src/keyring/keyring_api.c', 'src/keyring/keyring_curl.c', 'src/keyring/keyring_file.c', diff --git a/contrib/pg_tde/src/access/pg_tde_tdemap.c b/contrib/pg_tde/src/access/pg_tde_tdemap.c index 29ee3267ab6ab..ce36301a23083 100644 --- a/contrib/pg_tde/src/access/pg_tde_tdemap.c +++ b/contrib/pg_tde/src/access/pg_tde_tdemap.c @@ -19,6 +19,7 @@ #include "catalog/tde_principal_key.h" #include "encryption/enc_aes.h" #include "encryption/enc_tde.h" +#include "encryption/tde_keys.h" #include "keyring/keyring_api.h" #ifdef FRONTEND @@ -73,21 +74,67 @@ static void finalize_key_rotation(const char *path_old, const char *path_new); static int pg_tde_open_file_write(const char *tde_filename, const TDESignedPrincipalKeyInfo *signed_key_info, bool truncate, off_t *curr_pos); void -pg_tde_save_smgr_key(RelFileLocator rel, const InternalKey *rel_key_data) +pg_tde_save_smgr_key(RelFileLocator rel, const EncryptedTdeKey *encrypted_key) { TDEPrincipalKey *principal_key; LWLock *lock_pk = tde_lwlock_enc_keys(); + char db_map_path[MAXPGPATH]; + int map_fd; + off_t curr_pos = 0; + TDEMapEntry write_map_entry; + TDESignedPrincipalKeyInfo signed_key_Info; + + write_map_entry.spcOid = rel.spcOid; + write_map_entry.relNumber = rel.relNumber; + write_map_entry.type = TDE_KEY_TYPE_SMGR; + memcpy(write_map_entry.enc_key.key, encrypted_key->key_data, TDE_KEY_SIZE); + memcpy(write_map_entry.enc_key.base_iv, encrypted_key->key_iv, TDE_KEY_IV_SIZE); + write_map_entry.enc_key.type = TDE_KEY_TYPE_SMGR; + write_map_entry.enc_key.start_lsn = 0; + memcpy(write_map_entry.entry_iv, encrypted_key->iv, TDE_KEY_ENCRYPTION_IV_SIZE); + memcpy(write_map_entry.aead_tag, encrypted_key->aead_tag, TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE); + + pg_tde_set_db_file_path(rel.dbOid, db_map_path); LWLockAcquire(lock_pk, LW_EXCLUSIVE); + principal_key = GetPrincipalKey(rel.dbOid, LW_EXCLUSIVE); if (principal_key == NULL) - { ereport(ERROR, errmsg("principal key not configured"), errhint("create one using pg_tde_set_key before using encrypted tables")); + + pg_tde_sign_principal_key_info(&signed_key_Info, principal_key); + + /* Open and validate file for basic correctness. */ + map_fd = pg_tde_open_file_write(db_map_path, &signed_key_Info, false, &curr_pos); + + /* + * Read until we find an empty slot. Otherwise, read until end. This seems + * to be less frequent than vacuum. So let's keep this function here + * rather than overloading the vacuum process. + */ + while (1) + { + TDEMapEntry read_map_entry; + off_t prev_pos = curr_pos; + + if (!pg_tde_read_one_map_entry(map_fd, &read_map_entry, &curr_pos)) + { + curr_pos = prev_pos; + break; + } + + if (read_map_entry.type == MAP_ENTRY_EMPTY) + { + curr_pos = prev_pos; + break; + } } - pg_tde_write_key_map_entry(&rel, rel_key_data, principal_key); + pg_tde_write_one_map_entry(map_fd, &write_map_entry, &curr_pos, db_map_path); + + CloseTransientFile(map_fd); LWLockRelease(lock_pk); } @@ -268,7 +315,7 @@ pg_tde_initialize_map_entry(TDEMapEntry *map_entry, const TDEPrincipalKey *princ AesGcmEncrypt(principal_key->keyData, map_entry->entry_iv, MAP_ENTRY_IV_SIZE, - (unsigned char *) map_entry, offsetof(TDEMapEntry, enc_key), + (uint8 *) rlocator, sizeof(RelFileLocator), rel_key_data->key, INTERNAL_KEY_LEN, map_entry->enc_key.key, map_entry->aead_tag, MAP_ENTRY_AEAD_TAG_SIZE); @@ -725,13 +772,19 @@ tde_decrypt_rel_key(TDEPrincipalKey *principal_key, TDEMapEntry *map_entry) { InternalKey *rel_key_data = palloc_object(InternalKey); + RelFileLocator rlocator = { + .dbOid = principal_key->keyInfo.databaseId, + .relNumber = map_entry->relNumber, + .spcOid = map_entry->spcOid, + }; + Assert(principal_key); *rel_key_data = map_entry->enc_key; if (!AesGcmDecrypt(principal_key->keyData, map_entry->entry_iv, MAP_ENTRY_IV_SIZE, - (unsigned char *) map_entry, offsetof(TDEMapEntry, enc_key), + (uint8 *) &rlocator, sizeof(RelFileLocator), map_entry->enc_key.key, INTERNAL_KEY_LEN, rel_key_data->key, map_entry->aead_tag, MAP_ENTRY_AEAD_TAG_SIZE)) @@ -925,16 +978,15 @@ pg_tde_has_smgr_key(RelFileLocator rel) } /* - * Reads the map entry of the relation and decrypts the key. + * Reads the map entry of the relation and returns the encrypted key. */ -InternalKey * +EncryptedTdeKey * pg_tde_get_smgr_key(RelFileLocator rel) { TDEMapEntry map_entry; - TDEPrincipalKey *principal_key; - LWLock *lock_pk = tde_lwlock_enc_keys(); char db_map_path[MAXPGPATH]; - InternalKey *rel_key; + EncryptedTdeKey *encrypted_key; + bool found_map_entry; Assert(rel.relNumber != InvalidRelFileNumber); @@ -943,37 +995,20 @@ pg_tde_get_smgr_key(RelFileLocator rel) if (access(db_map_path, F_OK) == -1) return NULL; - LWLockAcquire(lock_pk, LW_SHARED); + LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED); + found_map_entry = pg_tde_find_map_entry(&rel, TDE_KEY_TYPE_SMGR, db_map_path, &map_entry); + LWLockRelease(tde_lwlock_enc_keys()); - if (!pg_tde_find_map_entry(&rel, TDE_KEY_TYPE_SMGR, db_map_path, &map_entry)) - { - LWLockRelease(lock_pk); + if (!found_map_entry) return NULL; - } - - /* - * Get/generate a principal key, create the key for relation and get the - * encrypted key with bytes to write - * - * We should hold the lock until the internal key is loaded to be sure the - * retrieved key was encrypted with the obtained principal key. Otherwise, - * the next may happen: - GetPrincipalKey returns key "PKey_1". - Some - * other process rotates the Principal key and re-encrypt an Internal key - * with "PKey_2". - We read the Internal key and decrypt it with "PKey_1" - * (that's what we've got). As the result we return an invalid Internal - * key. - */ - principal_key = GetPrincipalKey(rel.dbOid, LW_SHARED); - if (principal_key == NULL) - ereport(ERROR, - errmsg("principal key not configured"), - errhint("create one using pg_tde_set_key before using encrypted tables")); - - rel_key = tde_decrypt_rel_key(principal_key, &map_entry); - LWLockRelease(lock_pk); + encrypted_key = palloc0_object(EncryptedTdeKey); + memcpy(encrypted_key->key_data, map_entry.enc_key.key, TDE_KEY_SIZE); + memcpy(encrypted_key->key_iv, map_entry.enc_key.base_iv, TDE_KEY_ENCRYPTION_IV_SIZE); + memcpy(encrypted_key->iv, map_entry.entry_iv, TDE_KEY_IV_SIZE); + memcpy(encrypted_key->aead_tag, map_entry.aead_tag, TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE); - return rel_key; + return encrypted_key; } /* diff --git a/contrib/pg_tde/src/encryption/tde_keys.c b/contrib/pg_tde/src/encryption/tde_keys.c new file mode 100644 index 0000000000000..b72fa72cef2d5 --- /dev/null +++ b/contrib/pg_tde/src/encryption/tde_keys.c @@ -0,0 +1,168 @@ +#include "postgres.h" + +#include +#include +#include + +#include "utils/resowner.h" + +#include "encryption/enc_aes.h" +#include "encryption/tde_keys.h" + +#ifdef FRONTEND +#include "pg_tde_fe.h" +#endif + +static void ResourceOwnerReleaseDecryptedTdeKey(Datum resource); +static DecryptedTdeKey *tde_keys_alloc_decrypted_key(void); + +static const ResourceOwnerDesc tde_keys_resource_owner_desc = +{ + .name = "pg_tde decrypted key", + .release_phase = RESOURCE_RELEASE_BEFORE_LOCKS, + .release_priority = RELEASE_PRIO_FIRST, + .ReleaseResource = ResourceOwnerReleaseDecryptedTdeKey, + .DebugPrint = NULL, +}; + +EncryptedTdeKey * +tde_keys_encrypt_key(const DecryptedTdeKey *decrypted_key, + const uint8 *encryption_key, + const uint8 *additional_authentication_data, + int additional_authentication_data_size) +{ + EncryptedTdeKey *encrypted_key = palloc0_object(EncryptedTdeKey); + + memcpy(encrypted_key->key_iv, decrypted_key->iv, TDE_KEY_IV_SIZE); + + if (!RAND_bytes(encrypted_key->iv, TDE_KEY_ENCRYPTION_IV_SIZE)) + ereport(ERROR, + errmsg("could not generate iv for key encryption: %s", + ERR_error_string(ERR_get_error(), NULL))); + + AesGcmEncrypt(encryption_key, + + encrypted_key->iv, + TDE_KEY_ENCRYPTION_IV_SIZE, + + additional_authentication_data, + additional_authentication_data_size, + + decrypted_key->data, + TDE_KEY_SIZE, + + encrypted_key->key_data, + + encrypted_key->aead_tag, + TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE); + + return encrypted_key; +} + +DecryptedTdeKey * +tde_keys_decrypt_key(EncryptedTdeKey *encrypted_key, + const uint8 *decryption_key, + const uint8 *additional_authentication_data, + int additional_authentication_data_size) +{ + DecryptedTdeKey *decrypted_key; + + Assert(encrypted_key); + Assert(decryption_key); + + decrypted_key = tde_keys_alloc_decrypted_key(); + + if (!AesGcmDecrypt(decryption_key, + + encrypted_key->iv, + TDE_KEY_ENCRYPTION_IV_SIZE, + + additional_authentication_data, + additional_authentication_data_size, + + encrypted_key->key_data, + TDE_KEY_SIZE, + + decrypted_key->data, + + encrypted_key->aead_tag, + TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE)) + { + tde_keys_free_decrypted_key(decrypted_key); + return NULL; + } + + memcpy(decrypted_key->iv, encrypted_key->key_iv, TDE_KEY_IV_SIZE); + + return decrypted_key; +} + +void +tde_keys_free_decrypted_key(DecryptedTdeKey *decrypted_key) +{ + Assert(decrypted_key->owner); + + ResourceOwnerForget(decrypted_key->owner, + PointerGetDatum(decrypted_key), + &tde_keys_resource_owner_desc); + OPENSSL_secure_clear_free(decrypted_key, sizeof(DecryptedTdeKey)); +} + +DecryptedTdeKey * +tde_keys_generate_decrypted_key(void) +{ + DecryptedTdeKey *decrypted_key = tde_keys_alloc_decrypted_key(); + + if (!RAND_bytes(decrypted_key->data, TDE_KEY_SIZE)) + ereport(ERROR, + errmsg("could not generate key: %s", + ERR_error_string(ERR_get_error(), NULL))); + + if (!RAND_bytes(decrypted_key->iv, TDE_KEY_IV_SIZE)) + ereport(ERROR, + errmsg("could not generate iv: %s", + ERR_error_string(ERR_get_error(), NULL))); + + return decrypted_key; +} + +static void +ResourceOwnerReleaseDecryptedTdeKey(Datum resource) +{ + OPENSSL_secure_clear_free(DatumGetPointer(resource), sizeof(DecryptedTdeKey)); +} + +static DecryptedTdeKey * +tde_keys_alloc_decrypted_key(void) +{ + DecryptedTdeKey *decrypted_key; + + ResourceOwnerEnlarge(CurrentResourceOwner); + + decrypted_key = OPENSSL_secure_zalloc(sizeof(DecryptedTdeKey)); + + decrypted_key->owner = CurrentResourceOwner; + ResourceOwnerRemember(decrypted_key->owner, + PointerGetDatum(decrypted_key), + &tde_keys_resource_owner_desc); + + return decrypted_key; +} + +EncryptedTdeKey * +tde_keys_new_encrypted_key(const uint8 *encryption_key, + const uint8 *additional_authentication_data, + int additional_authentication_data_size) +{ + DecryptedTdeKey *decrypted_key; + EncryptedTdeKey *encrypted_key; + + decrypted_key = tde_keys_generate_decrypted_key(); + encrypted_key = tde_keys_encrypt_key(decrypted_key, + encryption_key, + additional_authentication_data, + additional_authentication_data_size); + tde_keys_free_decrypted_key(decrypted_key); + + return encrypted_key; +} diff --git a/contrib/pg_tde/src/include/access/pg_tde_tdemap.h b/contrib/pg_tde/src/include/access/pg_tde_tdemap.h index f7d99f735c890..867fa42430285 100644 --- a/contrib/pg_tde/src/include/access/pg_tde_tdemap.h +++ b/contrib/pg_tde/src/include/access/pg_tde_tdemap.h @@ -5,6 +5,7 @@ #include "storage/relfilelocator.h" #include "catalog/tde_principal_key.h" #include "common/pg_tde_utils.h" +#include "encryption/tde_keys.h" typedef enum { @@ -84,9 +85,9 @@ pg_tde_set_db_file_path(Oid dbOid, char *path) join_path_components(path, pg_tde_get_data_dir(), psprintf(PG_TDE_MAP_FILENAME, dbOid)); } -extern void pg_tde_save_smgr_key(RelFileLocator rel, const InternalKey *key); +extern void pg_tde_save_smgr_key(RelFileLocator rel, const EncryptedTdeKey *key); extern bool pg_tde_has_smgr_key(RelFileLocator rel); -extern InternalKey *pg_tde_get_smgr_key(RelFileLocator rel); +extern EncryptedTdeKey *pg_tde_get_smgr_key(RelFileLocator rel); extern void pg_tde_free_key_map_entry(RelFileLocator rel); extern int pg_tde_count_relations(Oid dbOid); diff --git a/contrib/pg_tde/src/include/encryption/tde_keys.h b/contrib/pg_tde/src/include/encryption/tde_keys.h new file mode 100644 index 0000000000000..c1a52425f59f2 --- /dev/null +++ b/contrib/pg_tde/src/include/encryption/tde_keys.h @@ -0,0 +1,39 @@ +#ifndef TDE_KEYS_H +#define TDE_KEYS_H + +#include "postgres.h" + +#include "utils/resowner.h" + +#include "catalog/tde_principal_key.h" + +#define TDE_KEY_SIZE 16 +#define TDE_KEY_IV_SIZE 16 +#define TDE_KEY_ENCRYPTION_IV_SIZE 16 +#define TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE 16 + +typedef struct EncryptedTdeKey +{ + uint8 key_data[TDE_KEY_SIZE]; + uint8 key_iv[TDE_KEY_IV_SIZE]; + + /* IV and tag used when encrypting the key itself */ + uint8 iv[TDE_KEY_ENCRYPTION_IV_SIZE]; + uint8 aead_tag[TDE_KEY_ENCRYPTION_AEAD_TAG_SIZE]; +} EncryptedTdeKey; + +typedef struct DecryptedTdeKey +{ + uint8 data[TDE_KEY_SIZE]; + uint8 iv[TDE_KEY_IV_SIZE]; + + ResourceOwner owner; +} DecryptedTdeKey; + +extern EncryptedTdeKey *tde_keys_encrypt_key(const DecryptedTdeKey *decrypted_key, const uint8 *encryption_key, const uint8 *additional_authentication_data, int additional_authentication_data_size); +extern DecryptedTdeKey *tde_keys_decrypt_key(EncryptedTdeKey *encrypted_key, const uint8 *decryption_key, const uint8 *additional_authentication_data, int additional_authentication_data_size); +extern void tde_keys_free_decrypted_key(DecryptedTdeKey *decrypted_key); +extern DecryptedTdeKey *tde_keys_generate_decrypted_key(void); +extern EncryptedTdeKey *tde_keys_new_encrypted_key(const uint8 *encryption_key, const uint8 *additional_authentication_data, int additional_authentication_data_size); + +#endif diff --git a/contrib/pg_tde/src/smgr/pg_tde_smgr.c b/contrib/pg_tde/src/smgr/pg_tde_smgr.c index b6dbcbb4c915f..339f61952c2d4 100644 --- a/contrib/pg_tde/src/smgr/pg_tde_smgr.c +++ b/contrib/pg_tde/src/smgr/pg_tde_smgr.c @@ -10,6 +10,7 @@ #include "access/pg_tde_xlog.h" #include "encryption/enc_aes.h" #include "encryption/enc_tde.h" +#include "encryption/tde_keys.h" #include "pg_tde_event_capture.h" #include "smgr/pg_tde_smgr.h" @@ -46,13 +47,13 @@ typedef struct TDESMgrRelation struct _MdfdVec *md_seg_fds[MAX_FORKNUM + 1]; TDEMgrRelationEncryptionStatus encryption_status; - InternalKey relKey; + EncryptedTdeKey relKey; } TDESMgrRelation; typedef struct { RelFileLocator rel; - InternalKey key; + EncryptedTdeKey key; } TempRelKeyEntry; #define INIT_TEMP_RELS 16 @@ -64,25 +65,39 @@ static HTAB *TempRelKeys = NULL; static SMgrId OurSMgrId = MaxSMgrId; -static void tde_smgr_save_temp_key(const RelFileLocator *newrlocator, const InternalKey *key); -static InternalKey *tde_smgr_get_temp_key(const RelFileLocator *rel); +static void tde_smgr_save_temp_key(const RelFileLocator *newrlocator, const EncryptedTdeKey *key); +static EncryptedTdeKey *tde_smgr_get_temp_key(const RelFileLocator *rel); static bool tde_smgr_has_temp_key(const RelFileLocator *rel); static void tde_smgr_remove_temp_key(const RelFileLocator *rel); static void CalcBlockIv(ForkNumber forknum, BlockNumber bn, const unsigned char *base_iv, unsigned char *iv); -static InternalKey * +static EncryptedTdeKey * tde_smgr_create_key(const RelFileLocatorBackend *smgr_rlocator) { - InternalKey *key = palloc_object(InternalKey); + EncryptedTdeKey *encrypted_key; + TDEPrincipalKey *principal_key; - pg_tde_generate_internal_key(key, TDE_KEY_TYPE_SMGR); + LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED); + + principal_key = GetPrincipalKey(smgr_rlocator->locator.dbOid, LW_SHARED); + if (principal_key == NULL) + ereport(ERROR, + errmsg("principal key not configured"), + errhint("create one using pg_tde_set_key before using encrypted tables")); + + encrypted_key = tde_keys_new_encrypted_key(principal_key->keyData, + (uint8 *) &smgr_rlocator->locator, + sizeof(RelFileLocator)); + + + LWLockRelease(tde_lwlock_enc_keys()); if (RelFileLocatorBackendIsTemp(*smgr_rlocator)) - tde_smgr_save_temp_key(&smgr_rlocator->locator, key); + tde_smgr_save_temp_key(&smgr_rlocator->locator, encrypted_key); else - pg_tde_save_smgr_key(smgr_rlocator->locator, key); + pg_tde_save_smgr_key(smgr_rlocator->locator, encrypted_key); - return key; + return encrypted_key; } static void @@ -100,14 +115,17 @@ tde_smgr_log_create_key(const RelFileLocatorBackend *smgr_rlocator) void tde_smgr_create_key_redo(const RelFileLocator *rlocator) { - InternalKey key; + EncryptedTdeKey *encrypted_key; + RelFileLocatorBackend smgr_rlocator = { + .locator = *rlocator, + .backend = INVALID_PROC_NUMBER, + }; if (pg_tde_has_smgr_key(*rlocator)) return; - pg_tde_generate_internal_key(&key, TDE_KEY_TYPE_SMGR); - - pg_tde_save_smgr_key(*rlocator, &key); + encrypted_key = tde_smgr_create_key(&smgr_rlocator); + pfree(encrypted_key); } static void @@ -139,7 +157,7 @@ tde_smgr_is_encrypted(const RelFileLocatorBackend *smgr_rlocator) return pg_tde_has_smgr_key(smgr_rlocator->locator); } -static InternalKey * +static EncryptedTdeKey * tde_smgr_get_key(const RelFileLocatorBackend *smgr_rlocator) { if (RelFileLocatorBackendIsTemp(*smgr_rlocator)) @@ -148,6 +166,56 @@ tde_smgr_get_key(const RelFileLocatorBackend *smgr_rlocator) return pg_tde_get_smgr_key(smgr_rlocator->locator); } +static DecryptedTdeKey * +tde_smgr_get_decrypted_key(TDESMgrRelation *tdereln) +{ + DecryptedTdeKey *decrypted_key; + TDEPrincipalKey *principal_key; + + LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED); + + if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) + { + EncryptedTdeKey *encrypted_key = tde_smgr_get_key(&tdereln->reln.smgr_rlocator); + + tdereln->relKey = *encrypted_key; + tdereln->encryption_status = RELATION_KEY_AVAILABLE; + pfree(encrypted_key); + } + + principal_key = GetPrincipalKey(tdereln->reln.smgr_rlocator.locator.dbOid, LW_SHARED); + if (principal_key == NULL) + ereport(ERROR, + errmsg("principal key not configured"), + errhint("create one using pg_tde_set_key before using encrypted tables")); + + decrypted_key = tde_keys_decrypt_key(&tdereln->relKey, + principal_key->keyData, + (uint8 *) &tdereln->reln.smgr_rlocator.locator, + sizeof(RelFileLocator)); + + LWLockRelease(tde_lwlock_enc_keys()); + + if (!decrypted_key) + { + /* + * If the principal key has been rotated we need to load the encrypted + * key from file again. + */ + tdereln->encryption_status = RELATION_KEY_NOT_AVAILABLE; + decrypted_key = tde_smgr_get_decrypted_key(tdereln); + + if (!decrypted_key) + { + /* The problem was not a rotated principal key apparently. */ + ereport(ERROR, + errmsg("Failed to decrypt key, incorrect principal key or corrupted key file")); + } + } + + return decrypted_key; +} + static void tde_smgr_remove_key(const RelFileLocatorBackend *smgr_rlocator) { @@ -211,15 +279,7 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, { unsigned char *local_blocks = palloc_aligned(BLCKSZ * nblocks, PG_IO_ALIGN_SIZE, 0); void **local_buffers = palloc_array(void *, nblocks); - - if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) - { - InternalKey *int_key = tde_smgr_get_key(&reln->smgr_rlocator); - - tdereln->relKey = *int_key; - tdereln->encryption_status = RELATION_KEY_AVAILABLE; - pfree(int_key); - } + DecryptedTdeKey *key = tde_smgr_get_decrypted_key(tdereln); for (int i = 0; i < nblocks; ++i) { @@ -228,9 +288,9 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, local_buffers[i] = &local_blocks[i * BLCKSZ]; - CalcBlockIv(forknum, bn, tdereln->relKey.base_iv, iv); + CalcBlockIv(forknum, bn, key->iv, iv); - AesEncrypt(tdereln->relKey.key, iv, ((unsigned char **) buffers)[i], BLCKSZ, local_buffers[i]); + AesEncrypt(key->data, iv, ((unsigned char **) buffers)[i], BLCKSZ, local_buffers[i]); } mdwritev(reln, forknum, blocknum, @@ -238,6 +298,7 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, pfree(local_blocks); pfree(local_buffers); + tde_keys_free_decrypted_key(key); } } @@ -282,23 +343,16 @@ tde_mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, { unsigned char *local_blocks = palloc_aligned(BLCKSZ, PG_IO_ALIGN_SIZE, 0); unsigned char iv[16]; + DecryptedTdeKey *key = tde_smgr_get_decrypted_key(tdereln); - if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) - { - InternalKey *int_key = tde_smgr_get_key(&reln->smgr_rlocator); - - tdereln->relKey = *int_key; - tdereln->encryption_status = RELATION_KEY_AVAILABLE; - pfree(int_key); - } - - CalcBlockIv(forknum, blocknum, tdereln->relKey.base_iv, iv); + CalcBlockIv(forknum, blocknum, key->iv, iv); - AesEncrypt(tdereln->relKey.key, iv, ((unsigned char *) buffer), BLCKSZ, local_blocks); + AesEncrypt(key->data, iv, ((unsigned char *) buffer), BLCKSZ, local_blocks); mdextend(reln, forknum, blocknum, local_blocks, skipFsync); pfree(local_blocks); + tde_keys_free_decrypted_key(key); } } @@ -307,19 +361,14 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, void **buffers, BlockNumber nblocks) { TDESMgrRelation *tdereln = (TDESMgrRelation *) reln; + DecryptedTdeKey *key; mdreadv(reln, forknum, blocknum, buffers, nblocks); if (tdereln->encryption_status == RELATION_NOT_ENCRYPTED) return; - else if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) - { - InternalKey *int_key = tde_smgr_get_key(&reln->smgr_rlocator); - tdereln->relKey = *int_key; - tdereln->encryption_status = RELATION_KEY_AVAILABLE; - pfree(int_key); - } + key = tde_smgr_get_decrypted_key(tdereln); for (int i = 0; i < nblocks; ++i) { @@ -347,17 +396,19 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, if (allZero) continue; - CalcBlockIv(forknum, bn, tdereln->relKey.base_iv, iv); + CalcBlockIv(forknum, bn, key->iv, iv); - AesDecrypt(tdereln->relKey.key, iv, ((unsigned char **) buffers)[i], BLCKSZ, ((unsigned char **) buffers)[i]); + AesDecrypt(key->data, iv, ((unsigned char **) buffers)[i], BLCKSZ, ((unsigned char **) buffers)[i]); } + + tde_keys_free_decrypted_key(key); } static void tde_mdcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo) { TDESMgrRelation *tdereln = (TDESMgrRelation *) reln; - InternalKey *key; + EncryptedTdeKey *key; /* Copied from mdcreate() in md.c */ if (isRedo && tdereln->md_num_open_segs[forknum] > 0) @@ -481,7 +532,7 @@ RegisterStorageMgr(void) } static void -tde_smgr_save_temp_key(const RelFileLocator *newrlocator, const InternalKey *key) +tde_smgr_save_temp_key(const RelFileLocator *newrlocator, const EncryptedTdeKey *key) { TempRelKeyEntry *entry; bool found; @@ -506,7 +557,7 @@ tde_smgr_save_temp_key(const RelFileLocator *newrlocator, const InternalKey *key entry->key = *key; } -static InternalKey * +static EncryptedTdeKey * tde_smgr_get_temp_key(const RelFileLocator *rel) { TempRelKeyEntry *entry; @@ -518,7 +569,7 @@ tde_smgr_get_temp_key(const RelFileLocator *rel) if (entry) { - InternalKey *key = palloc_object(InternalKey); + EncryptedTdeKey *key = palloc_object(EncryptedTdeKey); *key = entry->key; return key; From baf9640f91b6ee6c85d64de8c92827a7b59110ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20=C3=85strand?= Date: Sat, 21 Jun 2025 14:39:25 +0200 Subject: [PATCH 2/2] fixup! DO NOT MERGE Protect decrypted relation keys --- contrib/pg_tde/src/smgr/pg_tde_smgr.c | 57 +++++++++++++-------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/contrib/pg_tde/src/smgr/pg_tde_smgr.c b/contrib/pg_tde/src/smgr/pg_tde_smgr.c index 339f61952c2d4..e8201981ed407 100644 --- a/contrib/pg_tde/src/smgr/pg_tde_smgr.c +++ b/contrib/pg_tde/src/smgr/pg_tde_smgr.c @@ -172,48 +172,47 @@ tde_smgr_get_decrypted_key(TDESMgrRelation *tdereln) DecryptedTdeKey *decrypted_key; TDEPrincipalKey *principal_key; - LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED); - - if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) + for (int i = 0; i < 2; i++) { - EncryptedTdeKey *encrypted_key = tde_smgr_get_key(&tdereln->reln.smgr_rlocator); + if (tdereln->encryption_status == RELATION_KEY_NOT_AVAILABLE) + { + EncryptedTdeKey *encrypted_key = tde_smgr_get_key(&tdereln->reln.smgr_rlocator); - tdereln->relKey = *encrypted_key; - tdereln->encryption_status = RELATION_KEY_AVAILABLE; - pfree(encrypted_key); - } + tdereln->relKey = *encrypted_key; + tdereln->encryption_status = RELATION_KEY_AVAILABLE; + pfree(encrypted_key); + } - principal_key = GetPrincipalKey(tdereln->reln.smgr_rlocator.locator.dbOid, LW_SHARED); - if (principal_key == NULL) - ereport(ERROR, - errmsg("principal key not configured"), - errhint("create one using pg_tde_set_key before using encrypted tables")); + LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED); - decrypted_key = tde_keys_decrypt_key(&tdereln->relKey, - principal_key->keyData, - (uint8 *) &tdereln->reln.smgr_rlocator.locator, - sizeof(RelFileLocator)); + principal_key = GetPrincipalKey(tdereln->reln.smgr_rlocator.locator.dbOid, LW_SHARED); + if (principal_key == NULL) + ereport(ERROR, + errmsg("principal key not configured"), + errhint("create one using pg_tde_set_key before using encrypted tables")); - LWLockRelease(tde_lwlock_enc_keys()); + decrypted_key = tde_keys_decrypt_key(&tdereln->relKey, + principal_key->keyData, + (uint8 *) &tdereln->reln.smgr_rlocator.locator, + sizeof(RelFileLocator)); + + LWLockRelease(tde_lwlock_enc_keys()); - if (!decrypted_key) - { /* * If the principal key has been rotated we need to load the encrypted * key from file again. */ - tdereln->encryption_status = RELATION_KEY_NOT_AVAILABLE; - decrypted_key = tde_smgr_get_decrypted_key(tdereln); + if (decrypted_key) + return decrypted_key; + else + tdereln->encryption_status = RELATION_KEY_NOT_AVAILABLE; - if (!decrypted_key) - { - /* The problem was not a rotated principal key apparently. */ - ereport(ERROR, - errmsg("Failed to decrypt key, incorrect principal key or corrupted key file")); - } } - return decrypted_key; + /* The problem was not a rotated principal key apparently. */ + ereport(ERROR, + errmsg("Failed to decrypt key, incorrect principal key or corrupted key file")); + } static void