Skip to content

Commit

Permalink
PG-1072: Fix major event trigger issues
Browse files Browse the repository at this point in the history
* Create table always checked the principal key and tried to create
  it, event when we didn't try to create a tde_heap table
* Alter table wasn't handled, and because of this changing a table
  to tde_heap access method didn't result in an encrypted table.
* defaut_table_access_method wasn't handled, and because of this,
  creating a table using that also resulted in a non encrypted
  table.
  • Loading branch information
dutow committed Oct 9, 2024
1 parent 3e4c889 commit 88cddbd
Show file tree
Hide file tree
Showing 4 changed files with 267 additions and 19 deletions.
27 changes: 17 additions & 10 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ sql_tests = [
'vault_v2_test_basic',
]

tap_tests = [
't/001_basic.pl',
't/002_rotate_key.pl',
't/003_remote_config.pl',
't/004_file_config.pl',
't/005_multiple_extensions.pl',
't/006_remote_vault_config.pl',
't/007_access_control.pl',
]

if get_variable('percona_ext', false)
sql_tests += [
'toast_decrypt',
Expand All @@ -116,8 +126,14 @@ if get_variable('percona_ext', false)
'insert_update_delete',
'vault_v2_test',
]

tap_tests += [
't/008_tde_heap.pl',
]
endif



tests += {
'name': 'pg_tde',
'sd': meson.current_source_dir(),
Expand All @@ -128,14 +144,5 @@ tests += {
'runningcheck': false,
},
'tap': {
'tests': [
't/001_basic.pl',
't/002_rotate_key.pl',
't/003_remote_config.pl',
't/004_file_config.pl',
't/005_multiple_extensions.pl',
't/006_remote_vault_config.pl',
't/007_access_control.pl'
],
},
'tests': tap_tests },
}
64 changes: 55 additions & 9 deletions src/pg_tde_event_capture.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "commands/tablespace.h"
#include "catalog/tde_principal_key.h"
#include "miscadmin.h"
#include "access/tableam.h"

/* Global variable that gets set at ddl start and cleard out at ddl end*/
TdeCreateEvent tdeCurrentCreateEvent = {.relation = NULL};
Expand Down Expand Up @@ -106,24 +107,69 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
tdeCurrentCreateEvent.eventType = TDE_TABLE_CREATE_EVENT;
tdeCurrentCreateEvent.relation = stmt->relation;

if (stmt->accessMethod && !strcmp(stmt->accessMethod, "tde_heap"))
if (stmt->accessMethod && strcmp(stmt->accessMethod, "tde_heap") == 0)
{
tdeCurrentCreateEvent.encryptMode = true;
} else if ((stmt->accessMethod == NULL || stmt->accessMethod[0] ==0) && strcmp(default_table_access_method, "tde_heap") == 0)
{
tdeCurrentCreateEvent.encryptMode = true;
}

tablespace_oid = stmt->tablespacename != NULL ? get_tablespace_oid(stmt->tablespacename, false)
: MyDatabaseTableSpace;
LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED);
principal_key = GetPrincipalKey(MyDatabaseId, tablespace_oid, LW_SHARED);
LWLockRelease(tde_lwlock_enc_keys());
if (principal_key == NULL)
if(tdeCurrentCreateEvent.encryptMode)
{
ereport(ERROR,
(errmsg("failed to retrieve principal key. Create one using pg_tde_set_principal_key before using encrypted tables.")));
tablespace_oid = stmt->tablespacename != NULL ? get_tablespace_oid(stmt->tablespacename, false)
: MyDatabaseTableSpace;
LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED);
principal_key = GetPrincipalKey(MyDatabaseId, tablespace_oid, LW_SHARED);
LWLockRelease(tde_lwlock_enc_keys());
if (principal_key == NULL)
{
ereport(ERROR,
(errmsg("failed to retrieve principal key. Create one using pg_tde_set_principal_key before using encrypted tables.")));

}
}

}
else if (IsA(parsetree, AlterTableStmt))
{
fprintf(stderr, "ALTEEEER\n");
fprintf(stderr, "ALTEEEER\n");
LOCKMODE lockmode = AccessShareLock; /* TODO. Verify lock mode? */
AlterTableStmt *stmt = (AlterTableStmt *) parsetree;
TDEPrincipalKey * principal_key;
Oid relationId = RangeVarGetRelid(stmt->relation, NoLock, true);
Relation rel = table_open(relationId, lockmode);
Oid tablespace_oid = rel->rd_locator.spcOid;
ListCell *lcmd;
table_close(rel, lockmode);

foreach(lcmd, stmt->cmds)
{
AlterTableCmd *cmd = (AlterTableCmd *) lfirst(lcmd);
if(cmd->subtype == AT_SetAccessMethod && strcmp(cmd->name, "tde_heap")==0) {
fprintf(stderr, "TO ENCRYPTED\n");
tdeCurrentCreateEvent.encryptMode = true;
tdeCurrentCreateEvent.eventType = TDE_TABLE_CREATE_EVENT;
tdeCurrentCreateEvent.relation = stmt->relation;
}
}

// TODO: also check for tablespace change, if current or new AM is tde_heap!

if (tdeCurrentCreateEvent.encryptMode)
{
LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED);
principal_key = GetPrincipalKey(MyDatabaseId, tablespace_oid, LW_SHARED);
LWLockRelease(tde_lwlock_enc_keys());
if (principal_key == NULL)
{
ereport(ERROR,
(errmsg("failed to retrieve principal key. Create one using pg_tde_set_principal_key before using encrypted tables.")));

}
}
}
#endif
PG_RETURN_NULL();
}
Expand Down
159 changes: 159 additions & 0 deletions t/008_tde_heap.pl
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
#!/usr/bin/perl

use strict;
use warnings;
use File::Basename;
use File::Compare;
use File::Copy;
use Test::More;
use lib 't';
use pgtde;

# Get file name and CREATE out file name and dirs WHERE requried
PGTDE::setup_files_dir(basename($0));

# CREATE new PostgreSQL node and do initdb
my $node = PGTDE->pgtde_init_pg();
my $pgdata = $node->data_dir;

# UPDATE postgresql.conf to include/load pg_tde library
open my $conf, '>>', "$pgdata/postgresql.conf";
print $conf "shared_preload_libraries = 'pg_tde'\n";
close $conf;

# Start server
my $rt_value = $node->start;
ok($rt_value == 1, "Start Server");

# CREATE EXTENSION and change out file permissions
my ($cmdret, $stdout, $stderr) = $node->psql('postgres', 'CREATE EXTENSION pg_tde;', extra_params => ['-a']);
ok($cmdret == 0, "CREATE PGTDE EXTENSION");
PGTDE::append_to_file($stdout);


$rt_value = $node->psql('postgres', 'CREATE TABLE test_enc(id SERIAL,k INTEGER,PRIMARY KEY (id)) USING tde_heap;', extra_params => ['-a']);
ok($rt_value == 3, "Failing query");


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

$rt_value = $node->start();
ok($rt_value == 1, "Restart Server");

$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']);
PGTDE::append_to_file($stdout);

$stdout = $node->safe_psql('postgres', 'INSERT INTO test_enc (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']);
PGTDE::append_to_file($stdout);

$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);

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

$stdout = $node->safe_psql('postgres', 'ALTER TABLE test_enc2 SET ACCESS METHOD tde_heap;', extra_params => ['-a']);
PGTDE::append_to_file($stdout);

$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);

$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);

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

$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();

$stdout = $node->safe_psql('postgres', 'SELECT * FROM test_enc ORDER BY id ASC;', 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);




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

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




# 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\');');

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

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



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

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

$stdout = $node->safe_psql('postgres', 'DROP TABLE test_enc3;', 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");
PGTDE::append_to_file($stdout);
# Stop the server
$node->stop();

# compare the expected and out file
my $compare = PGTDE->compare_results();

# Test/check if expected and result/out file match. If Yes, test passes.
is($compare,0,"Compare Files: $PGTDE::expected_filename_with_path and $PGTDE::out_filename_with_path files.");

# Done testing for this testcase file.
done_testing();
36 changes: 36 additions & 0 deletions t/expected/008_tde_heap.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
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;
1|foobar
2|barfoo
CREATE TABLE test_enc2(id SERIAL,k VARCHAR(32),PRIMARY KEY (id));
INSERT INTO test_enc2 (k) VALUES ('foobar'),('barfoo');
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
-- server restart
SELECT * FROM test_enc ORDER BY id ASC;
1|foobar
2|barfoo
TABLEFILE FOUND: yes

CONTAINS FOO (should be empty):
TABLEFILE2 FOUND: yes

CONTAINS FOO (should be empty):
TABLEFILE3 FOUND: yes

CONTAINS FOO (should be empty):
DROP TABLE test_enc;
DROP TABLE test_enc2;
DROP TABLE test_enc3;
DROP EXTENSION pg_tde;

0 comments on commit 88cddbd

Please sign in to comment.