Skip to content

Commit

Permalink
Handle recreated relations in smgr encryption
Browse files Browse the repository at this point in the history 
Previously operations that created a new relation file for some
reason (truncate for example) resulted in a new unencrypted file,
even if the original file was encrypted.

This commit changes this behavior to properly create a new internal
key for the new internal file in this case.

This only works in pair with the related core change in the postgres
repository.
  • Loading branch information
@dutow
dutow committed Nov 12, 2024
1 parent 70d83ff commit e823274
Show file tree
Hide file tree
Showing 7 changed files with 162 additions and 37 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/postgresql-17-src-meson.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ jobs:
with:
name: Regressions diff and postgresql log
path: |
src/build/testrun/pg_tde/regress/
src/build/testrun/pg_tde/
src/contrib/pg_tde/t/results/
retention-days: 3
1 change: 1 addition & 0 deletions src/libkmip
Submodule libkmip added at f3f21c
21 changes: 16 additions & 5 deletions src/smgr/pg_tde_smgr.c
View file
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)
tde_smgr_get_key(SMgrRelation reln, RelFileLocator* old_locator)
{
TdeCreateEvent *event;
RelKeyData *rkd;
Expand Down Expand Up @@ -85,6 +85,17 @@ tde_smgr_get_key(SMgrRelation reln)
return pg_tde_create_smgr_key(&reln->smgr_rlocator.locator);
}

/* check if we had a key for the old locator, if there's one */
if(old_locator != NULL)
{
RelKeyData *rkd2 = GetRelationKey(*old_locator, TDE_KEY_TYPE_SMGR, true);
if(rkd2!=NULL)
{
// create a new key for the new file
return pg_tde_create_key_map_entry(&reln->smgr_rlocator.locator, TDE_KEY_TYPE_SMGR);
}
}

return NULL;
}

Expand Down Expand Up @@ -213,7 +224,7 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
}

static void
tde_mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo)
tde_mdcreate(RelFileLocator relold, SMgrRelation reln, ForkNumber forknum, bool isRedo)
{
TDESMgrRelation tdereln = (TDESMgrRelation) reln;

Expand All @@ -227,7 +238,7 @@ tde_mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo)
* Later calls then decide to encrypt or not based on the existence of the
* key
*/
RelKeyData *key = tde_smgr_get_key(reln);
RelKeyData *key = tde_smgr_get_key(reln, &relold);

if (key)
{
Expand All @@ -239,7 +250,7 @@ tde_mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo)
tdereln->encrypted_relation = false;
}

return mdcreate(reln, forknum, isRedo);
return mdcreate(relold, reln, forknum, isRedo);
}

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

if (key)
{
Expand Down
19 changes: 19 additions & 0 deletions src16/access/pg_tdeam_handler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,9 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
MultiXactId *minmulti)
{
SMgrRelation srel;
#ifdef PERCONA_EXT
RelFileLocator oldlocator = rel->rd_locator;
#endif

/*
* Initialize to the minimum XID that could put tuples in the table. We
Expand All @@ -617,7 +620,11 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
*/
*minmulti = GetOldestMultiXactId();

#ifdef PERCONA_EXT
srel = RelationCreateStorage(oldlocator, *newrlocator, persistence, true);
#else
srel = RelationCreateStorage(*newrlocator, persistence, true);
#endif

/*
* If required, set up an init fork for an unlogged table so that it can
Expand All @@ -633,7 +640,11 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
rel->rd_rel->relkind == RELKIND_MATVIEW ||
rel->rd_rel->relkind == RELKIND_TOASTVALUE);
#ifdef PERCONA_EXT
smgrcreate(oldlocator, srel, INIT_FORKNUM, false);
#else
smgrcreate(srel, INIT_FORKNUM, false);
#endif
log_smgrcreate(newrlocator, INIT_FORKNUM);
smgrimmedsync(srel, INIT_FORKNUM);
}
Expand Down Expand Up @@ -680,7 +691,11 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator)
* NOTE: any conflict in relfilenumber value will be caught in
* RelationCreateStorage().
*/
#ifdef PERCONA_EXT
RelationCreateStorage(rel->rd_locator, *newrlocator, rel->rd_rel->relpersistence, true);
#else
RelationCreateStorage(*newrlocator, rel->rd_rel->relpersistence, true);
#endif

/* copy main fork */
RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM,
Expand All @@ -692,7 +707,11 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator)
{
if (smgrexists(RelationGetSmgr(rel), forkNum))
{
#ifdef PERCONA_EXT
smgrcreate(rel->rd_locator, dstrel, forkNum, false);
#else
smgrcreate(dstrel, forkNum, false);
#endif

/*
* WAL log creation if the relation is persistent, or this is the
Expand Down
19 changes: 19 additions & 0 deletions src17/access/pg_tdeam_handler.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,9 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
MultiXactId *minmulti)
{
SMgrRelation srel;
#ifdef PERCONA_EXT
RelFileLocator oldlocator = rel->rd_locator;
#endif

/*
* Initialize to the minimum XID that could put tuples in the table. We
Expand All @@ -618,7 +621,11 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
*/
*minmulti = GetOldestMultiXactId();

#ifdef PERCONA_EXT
srel = RelationCreateStorage(oldlocator, *newrlocator, persistence, true);
#else
srel = RelationCreateStorage(*newrlocator, persistence, true);
#endif

/*
* If required, set up an init fork for an unlogged table so that it can
Expand All @@ -631,7 +638,11 @@ pg_tdeam_relation_set_new_filelocator(Relation rel,
Assert(rel->rd_rel->relkind == RELKIND_RELATION ||
rel->rd_rel->relkind == RELKIND_MATVIEW ||
rel->rd_rel->relkind == RELKIND_TOASTVALUE);
#ifdef PERCONA_EXT
smgrcreate(oldlocator, srel, INIT_FORKNUM, false);
#else
smgrcreate(srel, INIT_FORKNUM, false);
#endif
log_smgrcreate(newrlocator, INIT_FORKNUM);
}

Expand Down Expand Up @@ -675,7 +686,11 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator)
* NOTE: any conflict in relfilenumber value will be caught in
* RelationCreateStorage().
*/
#ifdef PERCONA_EXT
dstrel = RelationCreateStorage(rel->rd_locator, *newrlocator, rel->rd_rel->relpersistence, true);
#else
dstrel = RelationCreateStorage(*newrlocator, rel->rd_rel->relpersistence, true);
#endif

/* copy main fork */
RelationCopyStorage(RelationGetSmgr(rel), dstrel, MAIN_FORKNUM,
Expand All @@ -687,7 +702,11 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator)
{
if (smgrexists(RelationGetSmgr(rel), forkNum))
{
#ifdef PERCONA_EXT
smgrcreate(rel->rd_locator, dstrel, forkNum, false);
#else
smgrcreate(dstrel, forkNum, false);
#endif

/*
* WAL log creation if the relation is persistent, or this is the
Expand Down
83 changes: 62 additions & 21 deletions t/008_tde_heap.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,22 @@
$rt_value = $node->psql('postgres', "SELECT pg_tde_add_key_provider_file('file-vault','/tmp/pg_tde_test_keyring.per');", extra_params => ['-a']);
$rt_value = $node->psql('postgres', "SELECT pg_tde_set_principal_key('test-db-principal-key','file-vault');", extra_params => ['-a']);

$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap;', extra_params => ['-a']);


######################### test_enc1 (simple create table w tde_heap)


$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc1(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc1 (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc ORDER BY id ASC;', extra_params => ['-a']);
$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc1 ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

######################### test_enc2 (create heap + alter to tde_heap)

$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc2(id SERIAL,k VARCHAR(32),PRIMARY KEY (id));', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

Expand All @@ -73,8 +80,7 @@
$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc2 ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'SET default_table_access_method = "tde_heap";', extra_params => ['-a']);
PGTDE::append_to_file($stdout);
######################### test_enc3 (default_table_access_method)

$stdout = $node->safe_psql('postgres', 'SET default_table_access_method = "tde_heap"; CREATE TABLE test_enc3(id SERIAL,k VARCHAR(32),PRIMARY KEY (id));', extra_params => ['-a']);
PGTDE::append_to_file($stdout);
Expand All @@ -85,45 +91,77 @@
$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc3 ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

######################### test_enc4 (create heap + alter default)

$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc4(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING heap;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc4 (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'SET default_table_access_method = "tde_heap"; ALTER TABLE test_enc4 SET ACCESS METHOD DEFAULT;', extra_params => ['-a']);

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc4 ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);


######################### test_enc5 (create tde_heap + truncate)

$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc5(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc5 (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'CHECKPOINT;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'TRUNCATE test_enc5;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc5 (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc5 ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

# Restart the server
PGTDE::append_to_file("-- server restart");
$rt_value = $node->stop();
$rt_value = $node->start();

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);
sub verify_table
{
PGTDE::append_to_file('###########################');

# Verify that we can't see the data in the file
my $tablefile = $node->safe_psql('postgres', 'SHOW data_directory;');
$tablefile .= '/';
$tablefile .= $node->safe_psql('postgres', 'SELECT pg_relation_filepath(\'test_enc\');');
my ($table) = @_;

my $strings = 'TABLEFILE FOUND: ';
$strings .= `(ls $tablefile >/dev/null && echo yes) || echo no`;
PGTDE::append_to_file($strings);
my $tablefile = $node->safe_psql('postgres', 'SHOW data_directory;');
$tablefile .= '/';
$tablefile .= $node->safe_psql('postgres', 'SELECT pg_relation_filepath(\''.$table.'\');');

$strings = 'CONTAINS FOO (should be empty): ';
$strings .= `strings $tablefile | grep foo`;
PGTDE::append_to_file($strings);
$stdout = $node->safe_psql('postgres', 'SELECT * FROM ' . $table . ' ORDER BY id ASC;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

my $strings = 'TABLEFILE FOR ' . $table . ' FOUND: ';
$strings .= `(ls $tablefile >/dev/null && echo -n yes) || echo -n no`;
PGTDE::append_to_file($strings);

$strings = 'CONTAINS FOO (should be empty): ';
$strings .= `strings $tablefile | grep foo`;
PGTDE::append_to_file($strings);
}

verify_table('test_enc1');
verify_table('test_enc2');
verify_table('test_enc3');
verify_table('test_enc4');
verify_table('test_enc5');

# Verify that we can't see the data in the file
my $tablefile2 = $node->safe_psql('postgres', 'SHOW data_directory;');
$tablefile2 .= '/';
$tablefile2 .= $node->safe_psql('postgres', 'SELECT pg_relation_filepath(\'test_enc2\');');

$strings = 'TABLEFILE2 FOUND: ';
my $strings = 'TABLEFILE2 FOUND: ';
$strings .= `(ls $tablefile2 >/dev/null && echo yes) || echo no`;
PGTDE::append_to_file($strings);

Expand Down Expand Up @@ -165,7 +203,7 @@



$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc;', extra_params => ['-a']);
$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc1;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc2;', extra_params => ['-a']);
Expand All @@ -177,6 +215,9 @@
$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc4;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc5;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

# DROP EXTENSION
$stdout = $node->safe_psql('postgres', 'DROP EXTENSION pg_tde;', extra_params => ['-a']);
ok($cmdret == 0, "DROP PGTDE EXTENSION");
Expand Down
Loading

0 comments on commit e823274

Please sign in to comment.