Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7aa2928

Browse files
AndersAstrandAndersAstrand
committedApr 22, 2025·
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 9024375 commit 7aa2928

14 files changed

+110
-6
lines changed
 

‎contrib/pg_tde/documentation/docs/architecture.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,6 @@ To add a database specific provider:
208208
pg_tde_add_database_key_provider_<TYPE>('provider_name', ... details ...)
209209
```
210210

211-
Note that in these functions do not verify the parameters.
212-
For that, see `pg_tde_verify_key`.
213-
214211
### Changing providers
215212

216213
To change a value of a global provider:

‎contrib/pg_tde/expected/key_provider.out

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,4 +160,7 @@ SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
160160
-1 | file-keyring
161161
(1 row)
162162

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

‎contrib/pg_tde/expected/key_provider_1.out

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,7 @@ SELECT id, provider_name FROM pg_tde_list_all_global_key_providers();
164164
-2 | file-keyring
165165
(2 rows)
166166

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

‎contrib/pg_tde/expected/kmip_test.out

Lines changed: 3 additions & 0 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 3 additions & 0 deletions
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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,8 @@ check_provider_record(KeyringProviderRecord *provider_record)
472472
errmsg("Invalid provider options."));
473473
}
474474

475+
KeyringValidate(provider);
476+
475477
pfree(provider);
476478
}
477479

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

Lines changed: 2 additions & 0 deletions
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

Lines changed: 13 additions & 0 deletions
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

Lines changed: 19 additions & 1 deletion
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

Lines changed: 15 additions & 1 deletion
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,15 @@ get_key_by_name(GenericKeyring *keyring, const char *key_name, KeyringReturnCode
204206

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

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

Lines changed: 38 additions & 1 deletion
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,41 @@ 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+
* Validate connection by listing available keys at the root level of the
315+
* mount point
316+
*/
317+
snprintf(url, VAULT_URL_MAX_LEN, "%s/v1/%s/metadata/?list=true",
318+
vault_keyring->vault_url, vault_keyring->vault_mount_path);
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 the mount point doesn't have any secrets yet, we'll get a 404. */
328+
if (httpCode != 200 && httpCode != 404)
329+
{
330+
ereport(ERROR,
331+
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
332+
errmsg("Listing secrets of \"%s\" at mountpoint \"%s\" failed",
333+
vault_keyring->vault_url, vault_keyring->vault_mount_path));
334+
}
335+
336+
if (str.ptr != NULL)
337+
pfree(str.ptr);
338+
}
339+
303340
/*
304341
* JSON parser routines
305342
*

0 commit comments

Comments
 (0)
Please sign in to comment.