From e823274f7ba2ff086ba15a2dd4852d209b10d2cc Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 12 Oct 2024 21:59:40 +0100 Subject: [PATCH] Handle recreated relations in smgr encryption 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. --- .github/workflows/postgresql-17-src-meson.yml | 3 +- src/libkmip | 1 + src/smgr/pg_tde_smgr.c | 21 +++-- src16/access/pg_tdeam_handler.c | 19 +++++ src17/access/pg_tdeam_handler.c | 19 +++++ t/008_tde_heap.pl | 83 ++++++++++++++----- t/expected/008_tde_heap.out | 53 +++++++++--- 7 files changed, 162 insertions(+), 37 deletions(-) create mode 160000 src/libkmip diff --git a/.github/workflows/postgresql-17-src-meson.yml b/.github/workflows/postgresql-17-src-meson.yml index 4f1dd45a..2520de64 100644 --- a/.github/workflows/postgresql-17-src-meson.yml +++ b/.github/workflows/postgresql-17-src-meson.yml @@ -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 diff --git a/src/libkmip b/src/libkmip new file mode 160000 index 00000000..f3f21ceb --- /dev/null +++ b/src/libkmip @@ -0,0 +1 @@ +Subproject commit f3f21ceb32bef8ce8fb36e25c6ae4831f7689e02 diff --git a/src/smgr/pg_tde_smgr.c b/src/smgr/pg_tde_smgr.c index 952d911a..f7e77fa6 100644 --- a/src/smgr/pg_tde_smgr.c +++ b/src/smgr/pg_tde_smgr.c @@ -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; @@ -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; } @@ -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; @@ -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) { @@ -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); } /* @@ -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) { diff --git a/src16/access/pg_tdeam_handler.c b/src16/access/pg_tdeam_handler.c index a725a5b9..e7db6daf 100644 --- a/src16/access/pg_tdeam_handler.c +++ b/src16/access/pg_tdeam_handler.c @@ -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 @@ -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 @@ -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); } @@ -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, @@ -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 diff --git a/src17/access/pg_tdeam_handler.c b/src17/access/pg_tdeam_handler.c index 6a09e325..01cbbed9 100644 --- a/src17/access/pg_tdeam_handler.c +++ b/src17/access/pg_tdeam_handler.c @@ -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 @@ -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 @@ -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); } @@ -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, @@ -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 diff --git a/t/008_tde_heap.pl b/t/008_tde_heap.pl index 120d6dbf..6db26aa2 100644 --- a/t/008_tde_heap.pl +++ b/t/008_tde_heap.pl @@ -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); @@ -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); @@ -85,13 +91,36 @@ $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 @@ -99,31 +128,40 @@ $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); @@ -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']); @@ -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"); diff --git a/t/expected/008_tde_heap.out b/t/expected/008_tde_heap.out index 0e99453b..5b16269b 100644 --- a/t/expected/008_tde_heap.out +++ b/t/expected/008_tde_heap.out @@ -1,8 +1,8 @@ CREATE EXTENSION pg_tde; -- server restart -CREATE TABLE test_enc(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap; -INSERT INTO test_enc (k) VALUES ('foobar'),('barfoo'); -SELECT * FROM test_enc ORDER BY id ASC; +CREATE TABLE test_enc1(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap; +INSERT INTO test_enc1 (k) VALUES ('foobar'),('barfoo'); +SELECT * FROM test_enc1 ORDER BY id ASC; 1|foobar 2|barfoo CREATE TABLE test_enc2(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)); @@ -11,21 +11,53 @@ ALTER TABLE test_enc2 SET ACCESS METHOD tde_heap; SELECT * FROM test_enc2 ORDER BY id ASC; 1|foobar 2|barfoo -SET default_table_access_method = "tde_heap"; SET default_table_access_method = "tde_heap"; CREATE TABLE test_enc3(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)); INSERT INTO test_enc3 (k) VALUES ('foobar'),('barfoo'); SELECT * FROM test_enc3 ORDER BY id ASC; 1|foobar 2|barfoo -CREATE TABLE test_enc4(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING heap; INSERT INTO test_enc4 (k) VALUES ('foobar'),('barfoo'); -SET default_table_access_method = "tde_heap"; ALTER TABLE test_enc4 SET ACCESS METHOD DEFAULT; +SELECT * FROM test_enc4 ORDER BY id ASC; +1|foobar +2|barfoo +CREATE TABLE test_enc5(id SERIAL,k VARCHAR(32),PRIMARY KEY (id)) USING tde_heap; +INSERT INTO test_enc5 (k) VALUES ('foobar'),('barfoo'); +CHECKPOINT; +TRUNCATE test_enc5; +INSERT INTO test_enc5 (k) VALUES ('foobar'),('barfoo'); +SELECT * FROM test_enc5 ORDER BY id ASC; +3|foobar +4|barfoo -- server restart -SELECT * FROM test_enc ORDER BY id ASC; +########################### +SELECT * FROM test_enc1 ORDER BY id ASC; 1|foobar 2|barfoo -TABLEFILE FOUND: yes - +TABLEFILE FOR test_enc1 FOUND: yes +CONTAINS FOO (should be empty): +########################### +SELECT * FROM test_enc2 ORDER BY id ASC; +1|foobar +2|barfoo +TABLEFILE FOR test_enc2 FOUND: yes +CONTAINS FOO (should be empty): +########################### +SELECT * FROM test_enc3 ORDER BY id ASC; +1|foobar +2|barfoo +TABLEFILE FOR test_enc3 FOUND: yes +CONTAINS FOO (should be empty): +########################### +SELECT * FROM test_enc4 ORDER BY id ASC; +1|foobar +2|barfoo +TABLEFILE FOR test_enc4 FOUND: yes +CONTAINS FOO (should be empty): +########################### +SELECT * FROM test_enc5 ORDER BY id ASC; +3|foobar +4|barfoo +TABLEFILE FOR test_enc5 FOUND: yes CONTAINS FOO (should be empty): TABLEFILE2 FOUND: yes @@ -36,8 +68,9 @@ CONTAINS FOO (should be empty): TABLEFILE4 FOUND: yes CONTAINS FOO (should be empty): -DROP TABLE test_enc; +DROP TABLE test_enc1; DROP TABLE test_enc2; DROP TABLE test_enc3; DROP TABLE test_enc4; +DROP TABLE test_enc5; DROP EXTENSION pg_tde;