PG-1217: Do not try to create keys for existing files (#356)

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.
pull/209/head
Zsolt Parragi 10 months ago committed by GitHub
parent ce9398db39
commit 0e19bd0b43
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 12
      src/smgr/pg_tde_smgr.c

@ -38,7 +38,7 @@ tde_is_encryption_required(TDESMgrRelation tdereln, ForkNumber forknum)
} }
static RelKeyData * 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; TdeCreateEvent *event;
RelKeyData *rkd; RelKeyData *rkd;
@ -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 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); 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 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 */ /* For now keep it simple and create separate key for indexes */
/* /*
@ -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 */ /* 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); RelKeyData *rkd2 = GetSMGRRelationKey(*old_locator);
if(rkd2!=NULL) if(rkd2!=NULL)
@ -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 * Later calls then decide to encrypt or not based on the existence of the
* key * key
*/ */
RelKeyData *key = tde_smgr_get_key(reln, &relold); RelKeyData *key = tde_smgr_get_key(reln, &relold, true);
if (key) if (key)
{ {
@ -260,7 +260,7 @@ static void
tde_mdopen(SMgrRelation reln) tde_mdopen(SMgrRelation reln)
{ {
TDESMgrRelation tdereln = (TDESMgrRelation) reln; TDESMgrRelation tdereln = (TDESMgrRelation) reln;
RelKeyData *key = tde_smgr_get_key(reln, NULL); RelKeyData *key = tde_smgr_get_key(reln, NULL, false);
if (key) if (key)
{ {

Loading…
Cancel
Save