Skip to content

Commit

Permalink
PG-1217: Do not try to create keys for existing files (#356)
Browse files Browse the repository at this point in the history
Issue: the storage manager code uses the same key retrieval logic
for mdopen and mdcreate, and this logic creates a new key if we
are inside a DDL command that can create files.

This means that for ALTERs, if the table is first opened (mdopen)
during the ALTER, it creates a key for it, and then it tries to
read data from it using decryption, but the data is not actually
encrypted.

Fix: only create keys for new files, not existing ones.
  • Loading branch information
dutow authored Nov 20, 2024
1 parent ce9398d commit 0e19bd0
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions src/smgr/pg_tde_smgr.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tde_is_encryption_required(TDESMgrRelation tdereln, ForkNumber forknum)
}

static RelKeyData *
tde_smgr_get_key(SMgrRelation reln, RelFileLocator* old_locator)
tde_smgr_get_key(SMgrRelation reln, RelFileLocator* old_locator, bool can_create)
{
TdeCreateEvent *event;
RelKeyData *rkd;
Expand Down Expand Up @@ -69,13 +69,13 @@ tde_smgr_get_key(SMgrRelation reln, RelFileLocator* old_locator)
}

/* if this is a CREATE TABLE, we have to generate the key */
if (event->encryptMode == true && event->eventType == TDE_TABLE_CREATE_EVENT)
if (event->encryptMode == true && event->eventType == TDE_TABLE_CREATE_EVENT && can_create)
{
return pg_tde_create_smgr_key(&reln->smgr_rlocator.locator);
}

/* if this is a CREATE INDEX, we have to load the key based on the table */
if (event->encryptMode == true && event->eventType == TDE_INDEX_CREATE_EVENT)
if (event->encryptMode == true && event->eventType == TDE_INDEX_CREATE_EVENT && can_create)
{
/* For now keep it simple and create separate key for indexes */
/*
Expand All @@ -86,7 +86,7 @@ tde_smgr_get_key(SMgrRelation reln, RelFileLocator* old_locator)
}

/* check if we had a key for the old locator, if there's one */
if(old_locator != NULL)
if(old_locator != NULL && can_create)
{
RelKeyData *rkd2 = GetSMGRRelationKey(*old_locator);
if(rkd2!=NULL)
Expand Down Expand Up @@ -240,7 +240,7 @@ tde_mdcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool
* Later calls then decide to encrypt or not based on the existence of the
* key
*/
RelKeyData *key = tde_smgr_get_key(reln, &relold);
RelKeyData *key = tde_smgr_get_key(reln, &relold, true);

if (key)
{
Expand All @@ -260,7 +260,7 @@ static void
tde_mdopen(SMgrRelation reln)
{
TDESMgrRelation tdereln = (TDESMgrRelation) reln;
RelKeyData *key = tde_smgr_get_key(reln, NULL);
RelKeyData *key = tde_smgr_get_key(reln, NULL, false);

if (key)
{
Expand Down

0 comments on commit 0e19bd0

Please sign in to comment.