8000 PG-1543 Always cache server principal key · percona/postgres@5586521 · GitHub
[go: up one dir, main page]
Skip to content

Commit 5586521

Browse files
jeltzjeltz
committed
PG-1543 Always cache server principal key
This fixes a bug when pg_tde_verify_server_key() was broken if the server key was rotated in the same server lifetime as it was originally created since we wrote to the cache when we created to key the first time but not when we rotated it. An alternative would be to fix that bug and never cache the server key but by always caching it like we do with all other keys, including the default key, we simplify the code paths and make this kind of bug less likely in the future. This also discovered a previously harmless mistake where we grabbed the wrong lock level when trying to write a new WAL key.
1 parent b833868 commit 5586521

File tree

5 files changed

+25
-28
lines changed

5 files changed

+25
-28
lines changed

‎contrib/pg_tde/src/access/pg_tde_tdemap.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,9 @@ pg_tde_create_wal_key(InternalKey *rel_key_data, const RelFileLocator *newrlocat
261261
{
262262
TDEPrincipalKey *principal_key;
263263

264-
LWLockAcquire(tde_lwlock_enc_keys(), LW_SHARED);
264+
LWLockAcquire(tde_lwlock_enc_keys(), LW_EXCLUSIVE);
265265

266-
principal_key = GetPrincipalKey(newrlocator->dbOid, LW_SHARED);
266+
principal_key = GetPrincipalKey(newrlocator->dbOid, LW_EXCLUSIVE);
267267
if (principal_key == NULL)
268268
{
269269
ereport(ERROR,

‎contrib/pg_tde/src/catalog/tde_principal_key.c

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,8 @@ set_principal_key_with_keyring(const char *key_name, const char *provider_name,
328328
/* key rotation */
329329
pg_tde_perform_rotate_key(curr_principal_key, new_principal_key, true);
330330

331-
if (!TDEisInGlobalSpace(curr_principal_key->keyInfo.databaseId))
332-
{
333-
clear_principal_key_cache(curr_principal_key->keyInfo.databaseId);
334-
push_principal_key_to_cache(new_principal_key);
335-
}
331+
clear_principal_key_cache(curr_principal_key->keyInfo.databaseId);
332+
push_principal_key_to_cache(new_principal_key);
336333
}
337334

338335
LWLockRelease(lock_files);
@@ -390,11 +387,8 @@ xl_tde_perform_rotate_key(XLogPrincipalKeyRotate *xlrec)
390387

391388
pg_tde_perform_rotate_key(curr_principal_key, new_principal_key, false);
392389

393-
if (!TDEisInGlobalSpace(curr_principal_key->keyInfo.databaseId))
394-
{
395-
clear_principal_key_cache(curr_principal_key->keyInfo.databaseId);
396-
push_principal_key_to_cache(new_principal_key);
397-
}
390+
clear_principal_key_cache(curr_principal_key->keyInfo.databaseId);
391+
push_principal_key_to_cache(new_principal_key);
398392

399393
LWLockRelease(tde_lwlock_enc_keys());
400394

@@ -800,14 +794,11 @@ GetPrincipalKeyNoDefault(Oid dbOid, LWLockMode lockMode)
800794

801795
#ifndef FRONTEND
802796
Assert(LWLockHeldByMeInMode(tde_lwlock_enc_keys(), lockMode));
803-
/* We don't store global space key in cache */
804-
if (!TDEisInGlobalSpace(dbOid))
805-
{
806-
principalKey = get_principal_key_from_cache(dbOid);
807797

808-
if (likely(principalKey))
809-
return principalKey;
810-
}
798+
principalKey = get_principal_key_from_cache(dbOid);
799+
800+
if (likely(principalKey))
801+
return principalKey;
811802

812803
if (lockMode != LW_EXCLUSIVE)
813804
{
@@ -819,8 +810,7 @@ GetPrincipalKeyNoDefault(Oid dbOid, LWLockMode lockMode)
819810
principalKey = get_principal_key_from_keyring(dbOid);
820811

821812
#ifndef FRONTEND
822-
/* We don't store global space key in cache */
823-
if (principalKey && !TDEisInGlobalSpace(dbOid))
813+
if (principalKey)
824814
{
825815
push_principal_key_to_cache(principalKey);
826816

@@ -986,11 +976,8 @@ pg_tde_rotate_default_key_for_database(TDEPrincipalKey *oldKey, TDEPrincipalKey
986976
/* key rotation */
987977
pg_tde_perform_rotate_key(oldKey, newKey, true);
988978

989-
if (!TDEisInGlobalSpace(newKey->keyInfo.databaseId))
990-
{
991-
clear_principal_key_cache(oldKey->keyInfo.databaseId);
992-
push_principal_key_to_cache(newKey);
993-
}
979+
clear_principal_key_cache(oldKey->keyInfo.databaseId);
980+
push_principal_key_to_cache(newKey);
994981

995982
pfree(newKey);
996983
}

‎contrib/pg_tde/src/include/catalog/tde_global_space.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,4 @@
3333
_obj_oid \
3434
}
3535

36-
#define TDEisInGlobalSpace(dbOid) (dbOid == GLOBAL_DATA_TDE_OID)
37-
3836
#endif /* TDE_GLOBAL_CATALOG_H */

‎contrib/pg_tde/t/009_wal_encrypt.pl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,14 @@
2323
"SELECT pg_tde_add_global_key_provider_file('file-keyring-010', '/tmp/pg_tde_test_keyring010.per');"
2424
);
2525

26+
PGTDE::psql($node, 'postgres', 'SELECT pg_tde_verify_server_key();');
27+
2628
PGTDE::psql($node, 'postgres',
2729
"SELECT pg_tde_set_server_key_using_global_key_provider('server-key', 'file-keyring-010');"
2830
);
2931

32+
PGTDE::psql($node, 'postgres', 'SELECT pg_tde_verify_server_key();');
33+
3034
PGTDE::psql($node, 'postgres', 'ALTER SYSTEM SET pg_tde.wal_encrypt = on;');
3135

3236
PGTDE::append_to_result_file("-- server restart with wal encryption");

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,20 @@ SELECT pg_tde_add_global_key_provider_file('file-keyring-010', '/tmp/pg_tde_test
55
-1
66
(1 row)
77

8+
SELECT pg_tde_verify_server_key();
9+
psql:<stdin>:1: ERROR: principal key not configured for current database
810
SELECT pg_tde_set_server_key_using_global_key_provider('server-key', 'file-keyring-010');
911
pg_tde_set_server_key_using_global_key_provider
1012
-------------------------------------------------
1113

1214
(1 row)
1315

16+
SELECT pg_tde_verify_server_key();
17+
pg_tde_verify_server_key
18+
--------------------------
19+
20+
(1 row)
21+
1422
ALTER SYSTEM SET pg_tde.wal_encrypt = on;
1523
-- server restart with wal encryption
1624
SHOW pg_tde.wal_encrypt;

0 commit comments

Comments
 (0)
0