Skip to content

Commit caea669

Browse files
AndersAstrandAndersAstrand
committed
PG-1419 Validate key provider access
This adds some validation to make sure we can access the key provider when it's created to make the user experience a little nicer. The actual access validation is very rudimentary for now but can easily be expanded.
1 parent d711f37 commit caea669

13 files changed

+103
-6
lines changed

‎contrib/pg_tde/expected/key_provider.out

+3
Original file line numberDiff line numberDiff line change
@@ -162,4 +162,7 @@ SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
162162
----+---------------
163163
(0 rows)
164164

165+
-- Creating a file key provider fails if we can't open or create the file
166+
SELECT pg_tde_add_database_key_provider_file('will-not-work','/cant-create-file-in-root.per');
167+
ERROR: Failed to open keyring file /cant-create-file-in-root.per :Permission denied
165168
DROP EXTENSION pg_tde;

‎contrib/pg_tde/expected/key_provider_1.out

+3
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,7 @@ SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
166166
-1 | reg_file-global
167167
(1 row)
168168

169+
-- Creating a file key provider fails if we can't open or create the file
170+
SELECT pg_tde_add_database_key_provider_file('will-not-work','/cant-create-file-in-root.per');
171+
ERROR: Failed to open keyring file /cant-create-file-in-root.per :Permission denied
169172
DROP EXTENSION pg_tde;

‎contrib/pg_tde/expected/kmip_test.out

+3
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,7 @@ SELECT pg_tde_verify_key();
3434
(1 row)
3535

3636
DROP TABLE test_enc;
37+
-- Creating provider fails if we can't connect to kmip server
38+
SELECT pg_tde_add_database_key_provider_kmip('will-not-work','127.0.0.1', 61, '/tmp/server_certificate.pem', '/tmp/client_key_jane_doe.pem');
39+
ERROR: SSL error: BIO_do_connect failed
3740
DROP EXTENSION pg_tde;

‎contrib/pg_tde/expected/vault_v2_test.out

+3
Original file line numberDiff line numberDiff line change
@@ -51,4 +51,7 @@ SELECT pg_tde_verify_key();
5151
(1 row)
5252

5353
DROP TABLE test_enc;
54+
-- Creating provider fails if we can't connect to vault
55+
SELECT pg_tde_add_database_key_provider_vault_v2('will-not-work', :'root_token', 'http://127.0.0.1:61', 'secret', NULL);
56+
ERROR: HTTP(S) request to keyring provider "will-not-work" failed
5457
DROP EXTENSION pg_tde;

‎contrib/pg_tde/sql/key_provider.sql

+3
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,7 @@ SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
5252
SELECT pg_tde_delete_global_key_provider('file-keyring2');
5353
SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
5454

55+
-- Creating a file key provider fails if we can't open or create the file
56+
SELECT pg_tde_add_database_key_provider_file('will-not-work','/cant-create-file-in-root.per');
57+
5558
DROP EXTENSION pg_tde;

‎contrib/pg_tde/sql/kmip_test.sql

+3
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,7 @@ SELECT pg_tde_verify_key();
1919

2020
DROP TABLE test_enc;
2121

22+
-- Creating provider fails if we can't connect to kmip server
23+
SELECT pg_tde_add_database_key_provider_kmip('will-not-work','127.0.0.1', 61, '/tmp/server_certificate.pem', '/tmp/client_key_jane_doe.pem');
24+
2225
DROP EXTENSION pg_tde;

‎contrib/pg_tde/sql/vault_v2_test.sql

+3
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,7 @@ SELECT pg_tde_verify_key();
3131

3232
DROP TABLE test_enc;
3333

34+
-- Creating provider fails if we can't connect to vault
35+
SELECT pg_tde_add_database_key_provider_vault_v2('will-not-work', :'root_token', 'http://127.0.0.1:61', 'secret', NULL);
36+
3437
DROP EXTENSION pg_tde;

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

+5-3
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,8 @@ check_provider_record(KeyringProviderRecord *provider_record)
473473
errmsg("Invalid provider options."));
474474
}
475475

476+
KeyringValidate(provider);
477+
476478
pfree(provider);
477479
}
478480

@@ -562,9 +564,9 @@ save_new_key_provider_info(KeyringProviderRecord *provider, Oid databaseId, bool
562564
close(fd);
563565

564566
/*
565-
* TODO: Should we use overflow aware integer math here? (and then also not
566-
* blindly do abs() on something that might be INT_MIN above). It would be
567-
* overkill to do that, wouldn't it?
567+
* TODO: Should we use overflow aware integer math here? (and then also
568+
* not blindly do abs() on something that might be INT_MIN above). It
569+
* would be overkill to do that, wouldn't it?
568570
*/
569571
new_provider_id = max_provider_id + 1;
570572
provider->provider_id = (databaseId == GLOBAL_DATA_TDE_OID ? -new_provider_id : new_provider_id);

‎contrib/pg_tde/src/include/keyring/keyring_api.h

+2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ typedef struct TDEKeyringRoutine
6161
{
6262
KeyInfo *(*keyring_get_key) (GenericKeyring *keyring, const char *key_name, KeyringReturnCodes *returnCode);
6363
void (*keyring_store_key) (GenericKeyring *keyring, KeyInfo *key);
64+
void (*keyring_validate) (GenericKeyring *keyring);
6465
} TDEKeyringRoutine;
6566

6667
typedef struct FileKeyring
@@ -91,5 +92,6 @@ extern void RegisterKeyProviderType(const TDEKeyringRoutine *routine, ProviderTy
9192

9293
extern KeyInfo *KeyringGetKey(GenericKeyring *keyring, const char *key_name, KeyringReturnCodes *returnCode);
9394
extern KeyInfo *KeyringGenerateNewKeyAndStore(GenericKeyring *keyring, const char *key_name, unsigned key_len);
95+
extern void KeyringValidate(GenericKeyring *keyring);
9496

9597
#endif /* KEYRING_API_H */

‎contrib/pg_tde/src/keyring/keyring_api.c

+13
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ RegisterKeyProviderType(const TDEKeyringRoutine *routine, ProviderType type)
7474
Assert(routine != NULL);
7575
Assert(routine->keyring_get_key != NULL);
7676
Assert(routine->keyring_store_key != NULL);
77+
Assert(routine->keyring_validate != NULL);
7778

7879
kp = find_key_provider_type(type);
7980
if (kp)
@@ -148,3 +149,15 @@ KeyringGenerateNewKeyAndStore(GenericKeyring *keyring, const char *key_name, uns
148149

149150
return key;
150151
}
152+
153+
void
154+
KeyringValidate(GenericKeyring *keyring)
155+
{
156+
RegisteredKeyProviderType *kp = find_key_provider_type(keyring->type);
157+
158+
if (kp == NULL)
159+
ereport(ERROR,
160+
errmsg("Key provider of type %d not registered", keyring->type));
161+
162+
kp->routine->keyring_validate(keyring);
163+
}

‎contrib/pg_tde/src/keyring/keyring_file.c

+19-1
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@
2828

2929
static KeyInfo *get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCodes *return_code);
3030
static void set_key_by_name(GenericKeyring *keyring, KeyInfo *key);
31+
static void validate(GenericKeyring *keyring);
3132

3233
const TDEKeyringRoutine keyringFileRoutine = {
3334
.keyring_get_key = get_key_by_name,
34-
.keyring_store_key = set_key_by_name
35+
.keyring_store_key = set_key_by_name,
36+
.keyring_validate = validate,
3537
};
3638

3739
void
@@ -143,3 +145,19 @@ set_key_by_name(GenericKeyring *keyring, KeyInfo *key)
143145
}
144146
close(fd);
145147
}
148+
149+
static void
150+
validate(GenericKeyring *keyring)
151+
{
152+
FileKeyring *file_keyring = (FileKeyring *) keyring;
153+
int fd = BasicOpenFile(file_keyring->file_name, O_CREAT | O_RDWR | PG_BINARY);
154+
155+
if (fd < 0)
156+
{
157+
ereport(ERROR,
158+
errcode_for_file_access(),
159+
errmsg("Failed to open keyring file %s :%m", file_keyring->file_name));
160+
}
161+
162+
close(fd);
163+
}

‎contrib/pg_tde/src/keyring/keyring_kmip.c

+14-1
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,12 @@
2626

2727
static void set_key_by_name(GenericKeyring *keyring, KeyInfo *key);
2828
static KeyInfo *get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCodes *return_code);
29+
static void validate(GenericKeyring *keyring);
2930

3031
const TDEKeyringRoutine keyringKmipRoutine = {
3132
.keyring_get_key = get_key_by_name,
32-
.keyring_store_key = set_key_by_name
33+
.keyring_store_key = set_key_by_name,
34+
.keyring_validate = validate,
3335
};
3436

3537
void
@@ -204,3 +206,14 @@ get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCode
204206

205207
return key;
206208
}
209+
210+
static void validate(GenericKeyring *keyring)
211+
{
212+
KmipKeyring *kmip_keyring = (KmipKeyring *) keyring;
213+
KmipCtx ctx;
214+
215+
kmipSslConnect(&ctx, kmip_keyring, true);
216+
217+
BIO_free_all(ctx.bio);
218+
SSL_CTX_free(ctx.ssl);
219+
}

‎contrib/pg_tde/src/keyring/keyring_vault.c

+29-1
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,12 @@ static bool curl_perform(VaultV2Keyring *keyring, const char *url, CurlString *o
7070

7171
static void set_key_by_name(GenericKeyring *keyring, KeyInfo *key);
7272
static KeyInfo *get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCodes *return_code);
73+
static void validate(GenericKeyring *keyring);
7374

7475
const TDEKeyringRoutine keyringVaultV2Routine = {
7576
.keyring_get_key = get_key_by_name,
76-
.keyring_store_key = set_key_by_name
77+
.keyring_store_key = set_key_by_name,
78+
.keyring_validate = validate,
7779
};
7880

7981
void
@@ -300,6 +302,32 @@ get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCode
300302
return key;
301303
}
302304

305+
static void
306+
validate(GenericKeyring *keyring)
307+
{
308+
VaultV2Keyring *vault_keyring = (VaultV2Keyring *) keyring;
309+
char url[VAULT_URL_MAX_LEN];
310+
CurlString str;
311+
long httpCode = 0;
312+
313+
/*
314+
* Just try to connect to the mount URL with an empty key name to see if we
315+
* get any response. There might be better ways to validate vault
316+
* connectivity we want to look into in the future.
317+
*/
318+
get_keyring_vault_url(vault_keyring, "", url, sizeof(url));
319+
320+
if (!curl_perform(vault_keyring, url, &str, &httpCode, NULL))
321+
{
322+
ereport(ERROR,
323+
errmsg("HTTP(S) request to keyring provider \"%s\" failed",
324+
vault_keyring->keyring.provider_name));
325+
}
326+
327+
if (str.ptr != NULL)
328+
pfree(str.ptr);
329+
}
330+
303331
/*
304332
* JSON parser routines
305333
*

0 commit comments

Comments
 (0)