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 Oct 13, 2024
1 parent 38f6e5c commit 6bb4c11
Show file tree
Hide file tree
Showing 5 changed files with 139 additions and 64 deletions.
25 changes: 18 additions & 7 deletions src/smgr/pg_tde_smgr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#ifdef PERCONA_EXT

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 @@ -55,6 +55,17 @@ tde_smgr_get_key(SMgrRelation reln)
return pg_tde_create_key_map_entry(&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);
if(rkd2!=NULL)
{
// create a new key for the new file
return pg_tde_create_key_map_entry(&reln->smgr_rlocator.locator);
}
}

return NULL;
}

Expand All @@ -64,7 +75,7 @@ tde_mdwritev(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
{
AesInit();

RelKeyData* rkd = tde_smgr_get_key(reln);
RelKeyData* rkd = tde_smgr_get_key(reln, NULL);

if(rkd == NULL)
{
Expand Down Expand Up @@ -106,7 +117,7 @@ tde_mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,

AesInit();

rkd = tde_smgr_get_key(reln);
rkd = tde_smgr_get_key(reln, NULL);

if(rkd == NULL)
{
Expand Down Expand Up @@ -143,7 +154,7 @@ tde_mdreadv(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,

mdreadv(reln, forknum, blocknum, buffers, nblocks);

rkd = tde_smgr_get_key(reln);
rkd = tde_smgr_get_key(reln, NULL);

if(rkd == NULL)
return;
Expand Down Expand Up @@ -176,13 +187,13 @@ 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)
{
// This is the only function that gets called during actual CREATE TABLE/INDEX (EVENT TRIGGER)
// so we create the key here by loading it
// Later calls then decide to encrypt or not based on the existence of the key
tde_smgr_get_key(reln);
return mdcreate(reln, forknum, isRedo);
tde_smgr_get_key(reln, &relold);
return mdcreate(relold, reln, forknum, isRedo);
}


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
96 changes: 51 additions & 45 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,62 +91,59 @@
$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc3 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();
######################### test_enc4 (create tde_heap + truncate)

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc ORDER BY id ASC;', extra_params => ['-a']);
$stdout = $node->safe_psql('postgres', 'CREATE TABLE test_enc4(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

# 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 $strings = 'TABLEFILE FOUND: ';
$strings .= `(ls $tablefile >/dev/null && echo yes) || echo no`;
PGTDE::append_to_file($strings);

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


$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', 'CHECKPOINT;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

# 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\');');
$stdout = $node->safe_psql('postgres', 'TRUNCATE test_enc4;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$strings = 'TABLEFILE2 FOUND: ';
$strings .= `(ls $tablefile2 >/dev/null && echo yes) || echo no`;
PGTDE::append_to_file($strings);
$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc4 (k) VALUES (\'foobar\'),(\'barfoo\');', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$strings = 'CONTAINS FOO (should be empty): ';
$strings .= `strings $tablefile2 | grep foo`;
PGTDE::append_to_file($strings);
$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc4 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();

sub verify_table
{
PGTDE::append_to_file('###########################');

my ($table) = @_;

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

$strings = 'TABLEFILE3 FOUND: ';
$strings .= `(ls $tablefile3 >/dev/null && echo yes) || echo no`;
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);

$strings = 'CONTAINS FOO (should be empty): ';
$strings .= `strings $tablefile3 | grep foo`;
PGTDE::append_to_file($strings);
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');

$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 @@ -149,6 +152,9 @@
$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc3;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc4;', 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 6bb4c11

Please sign in to comment.