From 7f2f26e60b78f9eb31bccfc2ab1a3d3b04c8e85d Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Sat, 26 Apr 2025 12:29:26 +0200 Subject: [PATCH] PG-1540 Add support for CREATE and ALTER SEQUENCE to the event trigger When creating a table with a serial/identity column the sequence will get the same encryption status as the owning table but when doing the same using CREATE SEQUENCE and ALTER SEQUENCE the sequence ended up not encrypted so this add support for those. A design choice is that sequences not owned by any table are never encrypted which is unclear if it is a good choice or not. --- contrib/pg_tde/expected/recreate_storage.out | 95 +++++++++++++++ contrib/pg_tde/sql/recreate_storage.sql | 43 +++++++ contrib/pg_tde/src/pg_tde_event_capture.c | 122 ++++++++++++++++++- 3 files changed, 258 insertions(+), 2 deletions(-) diff --git a/contrib/pg_tde/expected/recreate_storage.out b/contrib/pg_tde/expected/recreate_storage.out index 58c3b3a0557..0b28c4a72cb 100644 --- a/contrib/pg_tde/expected/recreate_storage.out +++ b/contrib/pg_tde/expected/recreate_storage.out @@ -104,6 +104,7 @@ SELECT pg_tde_is_encrypted('rewritemetoo2'); CREATE TABLE encrypted_table ( id SERIAL, + id2 INT, data TEXT, created_at DATE NOT NULL, PRIMARY KEY (id, created_at) @@ -135,6 +136,100 @@ SELECT pg_tde_is_encrypted('encrypted_table_id_seq'); t (1 row) +CREATE TABLE plain_table ( + id2 INT +) USING heap; +-- Starts independent and becomes encrypted +CREATE SEQUENCE independent_seq; +SELECT pg_tde_is_encrypted('independent_seq'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +ALTER SEQUENCE independent_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('independent_seq'); + pg_tde_is_encrypted +--------------------- + t +(1 row) + +-- Starts independent and stays plain +CREATE SEQUENCE independent_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('independent_seq2'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +ALTER SEQUENCE independent_seq2 OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('independent_seq2'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +-- Starts owned by an encrypted table and becomes owned by a plain table +CREATE SEQUENCE encrypted_table_id2_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq'); + pg_tde_is_encrypted +--------------------- + t +(1 row) + +ALTER SEQUENCE encrypted_table_id2_seq OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +-- Starts owned by an encrypted table and becomes independent +CREATE SEQUENCE encrypted_table_id2_seq2 OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq2'); + pg_tde_is_encrypted +--------------------- + t +(1 row) + +ALTER SEQUENCE encrypted_table_id2_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq2'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +-- Starts owned by a plain table and becomes owned by an encrypted table +CREATE SEQUENCE plain_table_id2_seq OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +ALTER SEQUENCE plain_table_id2_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq'); + pg_tde_is_encrypted +--------------------- + t +(1 row) + +-- Starts owned by a plain table and becomes independent +CREATE SEQUENCE plain_table_id2_seq2 OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq2'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +ALTER SEQUENCE plain_table_id2_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('plain_table_id2_seq2'); + pg_tde_is_encrypted +--------------------- + f +(1 row) + +DROP TABLE plain_table; DROP EXTENSION pg_tde CASCADE; NOTICE: drop cascades to 7 other objects DETAIL: drop cascades to table t1 diff --git a/contrib/pg_tde/sql/recreate_storage.sql b/contrib/pg_tde/sql/recreate_storage.sql index 3296a871527..8b3e5565966 100644 --- a/contrib/pg_tde/sql/recreate_storage.sql +++ b/contrib/pg_tde/sql/recreate_storage.sql @@ -41,10 +41,12 @@ SELECT pg_tde_is_encrypted('rewritemetoo2'); CREATE TABLE encrypted_table ( id SERIAL, + id2 INT, data TEXT, created_at DATE NOT NULL, PRIMARY KEY (id, created_at) ) USING tde_heap; + CREATE INDEX idx_date ON encrypted_table (created_at); SELECT pg_tde_is_encrypted('encrypted_table'); CLUSTER encrypted_table USING idx_date; @@ -54,5 +56,46 @@ SELECT pg_tde_is_encrypted('encrypted_table_id_seq'); ALTER SEQUENCE encrypted_table_id_seq RESTART; SELECT pg_tde_is_encrypted('encrypted_table_id_seq'); +CREATE TABLE plain_table ( + id2 INT +) USING heap; + +-- Starts independent and becomes encrypted +CREATE SEQUENCE independent_seq; +SELECT pg_tde_is_encrypted('independent_seq'); +ALTER SEQUENCE independent_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('independent_seq'); + +-- Starts independent and stays plain +CREATE SEQUENCE independent_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('independent_seq2'); +ALTER SEQUENCE independent_seq2 OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('independent_seq2'); + +-- Starts owned by an encrypted table and becomes owned by a plain table +CREATE SEQUENCE encrypted_table_id2_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq'); +ALTER SEQUENCE encrypted_table_id2_seq OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq'); + +-- Starts owned by an encrypted table and becomes independent +CREATE SEQUENCE encrypted_table_id2_seq2 OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq2'); +ALTER SEQUENCE encrypted_table_id2_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('encrypted_table_id2_seq2'); + +-- Starts owned by a plain table and becomes owned by an encrypted table +CREATE SEQUENCE plain_table_id2_seq OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq'); +ALTER SEQUENCE plain_table_id2_seq OWNED BY encrypted_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq'); + +-- Starts owned by a plain table and becomes independent +CREATE SEQUENCE plain_table_id2_seq2 OWNED BY plain_table.id2; +SELECT pg_tde_is_encrypted('plain_table_id2_seq2'); +ALTER SEQUENCE plain_table_id2_seq2 OWNED BY NONE; +SELECT pg_tde_is_encrypted('plain_table_id2_seq2'); + +DROP TABLE plain_table; DROP EXTENSION pg_tde CASCADE; RESET default_table_access_method; diff --git a/contrib/pg_tde/src/pg_tde_event_capture.c b/contrib/pg_tde/src/pg_tde_event_capture.c index da349043c8f..04c859358a8 100644 --- a/contrib/pg_tde/src/pg_tde_event_capture.c +++ b/contrib/pg_tde/src/pg_tde_event_capture.c @@ -42,6 +42,7 @@ typedef struct Node *parsetree; TDEEncryptMode encryptMode; Oid rebuildSequencesFor; + Oid rebuildSequence; } TdeDdlEvent; static FullTransactionId ddlEventStackTid = {}; @@ -295,8 +296,8 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS) if (rel->rd_rel->relam == get_tde_table_am_oid()) { /* - * We are altering an encrypted table ALTER TABLE can - * create possibly new files set the global state. + * We are altering an encrypted table and ALTER TABLE can + * possibly create new files so set the global state. */ event.encryptMode = TDE_ENCRYPT_MODE_ENCRYPT; checkPrincipalKeyConfigured(); @@ -309,6 +310,108 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS) checkEncryptionStatus(); } } + else if (IsA(parsetree, CreateSeqStmt)) + { + CreateSeqStmt *stmt = (CreateSeqStmt *) parsetree; + ListCell *option; + List *owned_by = NIL; + TdeDdlEvent event = {.parsetree = parsetree}; + + foreach(option, stmt->options) + { + DefElem *defel = (DefElem *) lfirst(option); + + if (strcmp(defel->defname, "owned_by") == 0) + { + owned_by = defGetQualifiedName(defel); + break; + } + } + + if (list_length(owned_by) > 1) + { + List *relname; + RangeVar *rel; + Relation tablerel; + + relname = list_copy_head(owned_by, list_length(owned_by) - 1); + + /* Open and lock rel to ensure it won't go away meanwhile */ + rel = makeRangeVarFromNameList(relname); + tablerel = relation_openrv(rel, AccessShareLock); + + if (tablerel->rd_rel->relam == get_tde_table_am_oid()) + { + event.encryptMode = TDE_ENCRYPT_MODE_ENCRYPT; + checkPrincipalKeyConfigured(); + } + else + event.encryptMode = TDE_ENCRYPT_MODE_PLAIN; + + /* Hold lock until end of transaction */ + relation_close(tablerel, NoLock); + } + else + event.encryptMode = TDE_ENCRYPT_MODE_PLAIN; + + push_event_stack(&event); + } + else if (IsA(parsetree, AlterSeqStmt)) + { + AlterSeqStmt *stmt = (AlterSeqStmt *) parsetree; + ListCell *option; + List *owned_by = NIL; + TdeDdlEvent event = {.parsetree = parsetree}; + Oid relid = RangeVarGetRelid(stmt->sequence, AccessShareLock, true); + + if (relid != InvalidOid) + { + foreach(option, stmt->options) + { + DefElem *defel = (DefElem *) lfirst(option); + + if (strcmp(defel->defname, "owned_by") == 0) + { + owned_by = defGetQualifiedName(defel); + break; + } + } + + if (list_length(owned_by) > 1) + { + List *relname; + RangeVar *rel; + Relation tablerel; + + event.rebuildSequence = relid; + + relname = list_copy_head(owned_by, list_length(owned_by) - 1); + + /* Open and lock rel to ensure it won't go away meanwhile */ + rel = makeRangeVarFromNameList(relname); + tablerel = relation_openrv(rel, AccessShareLock); + + if (tablerel->rd_rel->relam == get_tde_table_am_oid()) + { + event.encryptMode = TDE_ENCRYPT_MODE_ENCRYPT; + checkPrincipalKeyConfigured(); + } + else + event.encryptMode = TDE_ENCRYPT_MODE_PLAIN; + + /* Hold lock until end of transaction */ + relation_close(tablerel, NoLock); + } + else if (list_length(owned_by) == 1) + { + event.rebuildSequence = relid; + + event.encryptMode = TDE_ENCRYPT_MODE_PLAIN; + } + + push_event_stack(&event); + } + } PG_RETURN_VOID(); } @@ -358,6 +461,21 @@ pg_tde_ddl_command_end_capture(PG_FUNCTION_ARGS) } } + if (event->rebuildSequence != InvalidOid) + { + /* + * Seqeunces are not rewritten when just changing owner so force a + * rewrite. There is a small risk of extra overhead if someone changes + * sequence owner and something else at the same time. + */ + Relation rel = relation_open(event->rebuildSequence, NoLock); + char persistence = rel->rd_rel->relpersistence; + + relation_close(rel, NoLock); + + SequenceChangePersistence(event->rebuildSequence, persistence); + } + ddlEventStack = list_delete_last(ddlEventStack); pfree(event);