Skip to content

Commit 023928c

Browse files
committed
Ensure decrypted internal keys never swapped
We have already had a locked memory for internal keys. But we read and created keys into usually palloced memory before copying them to the locked memory. This commit changes the approach. Now, the user must allocate (acquire) a locked memory first and use it as a buffer for decryption/creating the key. This locked memory is kinda generic, so the user requests an amount of bytes (rather than objects). Although currently, during the key search, we assume that everything the memory contains only `RelKeyCacheRec` objects. With two exceptions: 1) server creates its own locked page to store the WAL write key; 2) `pg_tde_perform_rotate_key` cheekily allocates a space there for the keys re-encryption but releases this memory when it's done. In future, if we switch to the full _map files encryption, we can gulp (mmap) the whole file into that memory. Fixes PG-1445
1 parent d8cc666 commit 023928c

File tree

8 files changed

+201
-171
lines changed

8 files changed

+201
-171
lines changed

contrib/pg_tde/src/access/pg_tde_tdemap.c

+135-143
Large diffs are not rendered by default.

contrib/pg_tde/src/access/pg_tde_xlog_encrypt.c

+41-22
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,7 @@ typedef struct EncryptionStateData
7070

7171
static EncryptionStateData *EncryptionState = NULL;
7272

73-
/* TODO: can be swapped out to the disk */
74-
static InternalKey EncryptionKey =
75-
{
76-
.rel_type = MAP_ENTRY_EMPTY,
77-
.start_lsn = InvalidXLogRecPtr,
78-
};
73+
static InternalKey *EncryptionKey = NULL;
7974
static void *EncryptionCryptCtx = NULL;
8075

8176
static int XLOGChooseNumBuffers(void);
@@ -163,48 +158,72 @@ TDEXLogWriteEncryptedPages(int fd, const void *buf, size_t count, off_t offset,
163158
TimeLineID tli, XLogSegNo segno)
164159
{
165160
char iv_prefix[16] = {0,};
166-
InternalKey *key = &EncryptionKey;
167161
char *enc_buff = EncryptionState->segBuf;
168162

169163
Assert(count <= TDEXLogEncryptBuffSize());
170164

171165
#ifdef TDE_XLOG_DEBUG
172166
elog(DEBUG1, "write encrypted WAL, size: %lu, offset: %ld [%lX], seg: %X/%X, key_start_lsn: %X/%X",
173-
count, offset, offset, LSN_FORMAT_ARGS(segno), LSN_FORMAT_ARGS(key->start_lsn));
167+
count, offset, offset, LSN_FORMAT_ARGS(segno), LSN_FORMAT_ARGS(EncryptionKey->start_lsn));
174168
#endif
175169

176170
SetXLogPageIVPrefix(tli, segno, iv_prefix);
177171
PG_TDE_ENCRYPT_DATA(iv_prefix, offset,
178172
(char *) buf, count,
179-
enc_buff, key, &EncryptionCryptCtx);
173+
enc_buff, EncryptionKey, &EncryptionCryptCtx);
180174

181175
return pg_pwrite(fd, enc_buff, count, offset);
182176
}
183177

184-
#endif /* !FRONTEND */
185-
186-
void
187-
TDEXLogSmgrInit(void)
178+
static void
179+
tde_init_wal_encryption_key(void)
188180
{
189-
#ifndef FRONTEND
190-
/* TODO: move to the separate func, it's not an SMGR init */
191181
InternalKey *key = pg_tde_read_last_wal_key();
192182

193183
/* TDOO: clean-up this mess */
194184
if ((!key && EncryptXLog) || (key &&
195185
((key->rel_type & TDE_KEY_TYPE_WAL_ENCRYPTED && !EncryptXLog) ||
196186
(key->rel_type & TDE_KEY_TYPE_WAL_UNENCRYPTED && EncryptXLog))))
197187
{
198-
pg_tde_create_wal_key(&EncryptionKey, &GLOBAL_SPACE_RLOCATOR(XLOG_TDE_OID),
199-
(EncryptXLog ? TDE_KEY_TYPE_WAL_ENCRYPTED : TDE_KEY_TYPE_WAL_UNENCRYPTED));
188+
key = pg_tde_create_wal_key(&GLOBAL_SPACE_RLOCATOR(XLOG_TDE_OID),
189+
(EncryptXLog ? TDE_KEY_TYPE_WAL_ENCRYPTED : TDE_KEY_TYPE_WAL_UNENCRYPTED));
200190
}
201191
else if (key)
202192
{
203-
EncryptionKey = *key;
204-
pfree(key);
205-
pg_atomic_write_u64(&EncryptionState->enc_key_lsn, EncryptionKey.start_lsn);
193+
pg_atomic_write_u64(&EncryptionState->enc_key_lsn, key->start_lsn);
194+
}
195+
196+
197+
/*
198+
* The relation cache might be moved to the new address space when growing
199+
* later on. It happens when there are a lot of tde_heap relations during
200+
* the recovery. So copy the key to the own locked mem. We could rewrite
201+
* the EncryptionKey pointer after the grow, but it'd be more complicated
202+
* and error-prone. We can sacrifice a couple of Kb.
203+
*
204+
* TODO: genralize and use `tde_alloc_safe_mem` instead?
205+
*/
206+
if (key)
207+
{
208+
long pageSize = 0;
209+
210+
#ifndef _SC_PAGESIZE
211+
pageSize = getpagesize();
212+
#else
213+
pageSize = sysconf(_SC_PAGESIZE);
214+
#endif
215+
EncryptionKey = (InternalKey *) MemoryContextAllocAligned(TopMemoryContext, pageSize, pageSize, MCXT_ALLOC_ZERO);
216+
memcpy(EncryptionKey, key, sizeof(InternalKey));
206217
}
218+
}
207219

220+
#endif /* !FRONTEND */
221+
222+
void
223+
TDEXLogInit(void)
224+
{
225+
#ifndef FRONTEND
226+
tde_init_wal_encryption_key();
208227
pg_tde_set_db_file_path(GLOBAL_SPACE_RLOCATOR(XLOG_TDE_OID).dbOid, EncryptionState->db_map_path);
209228

210229
#endif
@@ -222,15 +241,15 @@ tdeheap_xlog_seg_write(int fd, const void *buf, size_t count, off_t offset,
222241
*
223242
* This func called with WALWriteLock held, so no need in any extra sync.
224243
*/
225-
if (EncryptionKey.rel_type & TDE_KEY_TYPE_GLOBAL &&
244+
if (EncryptionKey && EncryptionKey->rel_type & TDE_KEY_TYPE_GLOBAL &&
226245
pg_atomic_read_u64(&EncryptionState->enc_key_lsn) == 0)
227246
{
228247
XLogRecPtr lsn;
229248

230249
XLogSegNoOffsetToRecPtr(segno, offset, wal_segment_size, lsn);
231250

232251
pg_tde_wal_last_key_set_lsn(lsn, EncryptionState->db_map_path);
233-
EncryptionKey.start_lsn = lsn;
252+
EncryptionKey->start_lsn = lsn;
234253
pg_atomic_write_u64(&EncryptionState->enc_key_lsn, lsn);
235254
}
236255

contrib/pg_tde/src/include/access/pg_tde_tdemap.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ extern WALKeyCacheRec *pg_tde_get_wal_cache_keys(void);
8989
extern void pg_tde_wal_last_key_set_lsn(XLogRecPtr lsn, const char *keyfile_path);
9090

9191
extern InternalKey *pg_tde_create_smgr_key(const RelFileLocatorBackend *newrlocator);
92-
extern void pg_tde_create_wal_key(InternalKey *rel_key_data, const RelFileLocator *newrlocator, uint32 flags);
92+
extern InternalKey *pg_tde_create_wal_key(const RelFileLocator *newrlocator, uint32 flags);
9393
extern void pg_tde_free_key_map_entry(const RelFileLocator *rlocator, off_t offset);
9494
extern void pg_tde_write_key_map_entry_redo(const TDEMapEntry *write_map_entry, TDEPrincipalKeyInfo *principal_key_info);
9595

contrib/pg_tde/src/include/access/pg_tde_xlog_encrypt.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,6 @@
1313

1414
extern Size TDEXLogEncryptStateSize(void);
1515
extern void TDEXLogShmemInit(void);
16-
extern void TDEXLogSmgrInit(void);
16+
extern void TDEXLogInit(void);
1717

1818
#endif /* PG_TDE_XLOGENCRYPT_H */

contrib/pg_tde/src/pg_tde.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ tde_shmem_startup(void)
9292
AesInit();
9393

9494
TDEXLogShmemInit();
95-
TDEXLogSmgrInit();
95+
TDEXLogInit();
9696
}
9797

9898
void

contrib/pg_tde/t/009_wal_encrypt.pl

+19-1
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,28 @@
5555
$stdout = $node->safe_psql('postgres', 'CHECKPOINT;', extra_params => ['-a']);
5656
PGTDE::append_to_file($stdout);
5757

58+
PGTDE::append_to_file("-- server restart with wal encryption and recovery with a lot of tde_heap relations");
59+
$node->safe_psql('postgres',
60+
q{
61+
SELECT pg_tde_add_key_provider_file('file-keyring-010-2','/tmp/pg_tde_test_keyring010_2.per');
62+
SELECT pg_tde_set_principal_key('test-db-principal-key','file-keyring-010-2');
63+
64+
do $$
65+
DECLARE idx integer;
66+
begin
67+
for idx in 0..700 loop
68+
EXECUTE format('CREATE TABLE t%s (c1 int) USING tde_heap', idx);
69+
end loop;
70+
end; $$;
71+
});
72+
73+
$rt_value = $node->restart();
74+
ok($rt_value == 1, "Restart Server");
75+
5876
# TODO: add WAL content testing after the wal rework
5977

6078
# DROP EXTENSION
61-
$stdout = $node->safe_psql('postgres', 'DROP EXTENSION pg_tde;', extra_params => ['-a']);
79+
$stdout = $node->safe_psql('postgres', 'DROP EXTENSION pg_tde CASCADE;', extra_params => ['-a']);
6280
PGTDE::append_to_file($stdout);
6381
# Stop the server
6482
$node->stop();

contrib/pg_tde/t/expected/009_wal_encrypt.out

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ SHOW pg_tde.wal_encrypt;
88
on
99
CREATE TABLE test_wal(id SERIAL,k INTEGER,PRIMARY KEY (id));
1010
CHECKPOINT;
11-
DROP EXTENSION pg_tde;
11+
-- server restart with wal encryption and recovery with a lot of tde_heap relations
12+
DROP EXTENSION pg_tde CASCADE;

src/bin/pg_waldump/pg_waldump.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ main(int argc, char **argv)
11561156
if (kringdir != NULL)
11571157
{
11581158
pg_tde_fe_init(kringdir);
1159-
TDEXLogSmgrInit();
1159+
TDEXLogInit();
11601160
}
11611161
#endif
11621162

0 commit comments

Comments
 (0)