Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PG-1254, PG-1253, PG-1250: Improve diagnostic messages #382

Merged
merged 1 commit into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion expected/vault_v2_test.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,27 @@
\i sql/vault_v2_test.inc
CREATE EXTENSION pg_tde;
\getenv root_token ROOT_TOKEN
SELECT pg_tde_add_key_provider_vault_v2('vault-v2',:'root_token','http://127.0.0.1:8200','secret',NULL);
SELECT pg_tde_add_key_provider_vault_v2('vault-incorrect',:'root_token','http://127.0.0.1:8200','DUMMY-TOKEN',NULL);
pg_tde_add_key_provider_vault_v2
----------------------------------
1
(1 row)

-- FAILS
SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-incorrect');
psql:sql/vault_v2_test.inc:7: ERROR: Failed to store key on keyring. Please check the keyring configuration.
CREATE TABLE test_enc(
id SERIAL,
k INTEGER DEFAULT '0' NOT NULL,
PRIMARY KEY (id)
) USING :tde_am;
psql:sql/vault_v2_test.inc:13: ERROR: failed to retrieve principal key. Create one using pg_tde_set_principal_key before using encrypted tables.
SELECT pg_tde_add_key_provider_vault_v2('vault-v2',:'root_token','http://127.0.0.1:8200','secret',NULL);
pg_tde_add_key_provider_vault_v2
----------------------------------
2
(1 row)

SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-v2');
pg_tde_set_principal_key
--------------------------
Expand Down
17 changes: 16 additions & 1 deletion expected/vault_v2_test_basic.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,27 @@
\i sql/vault_v2_test.inc
CREATE EXTENSION pg_tde;
\getenv root_token ROOT_TOKEN
SELECT pg_tde_add_key_provider_vault_v2('vault-v2',:'root_token','http://127.0.0.1:8200','secret',NULL);
SELECT pg_tde_add_key_provider_vault_v2('vault-incorrect',:'root_token','http://127.0.0.1:8200','DUMMY-TOKEN',NULL);
pg_tde_add_key_provider_vault_v2
----------------------------------
1
(1 row)

-- FAILS
SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-incorrect');
psql:sql/vault_v2_test.inc:7: ERROR: Failed to store key on keyring. Please check the keyring configuration.
CREATE TABLE test_enc(
id SERIAL,
k INTEGER DEFAULT '0' NOT NULL,
PRIMARY KEY (id)
) USING :tde_am;
psql:sql/vault_v2_test.inc:13: ERROR: failed to retrieve principal key. Create one using pg_tde_set_principal_key before using encrypted tables.
SELECT pg_tde_add_key_provider_vault_v2('vault-v2',:'root_token','http://127.0.0.1:8200','secret',NULL);
pg_tde_add_key_provider_vault_v2
----------------------------------
2
(1 row)

SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-v2');
pg_tde_set_principal_key
--------------------------
Expand Down
11 changes: 11 additions & 0 deletions sql/vault_v2_test.inc
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
CREATE EXTENSION pg_tde;

\getenv root_token ROOT_TOKEN

SELECT pg_tde_add_key_provider_vault_v2('vault-incorrect',:'root_token','http://127.0.0.1:8200','DUMMY-TOKEN',NULL);
-- FAILS
SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-incorrect');

CREATE TABLE test_enc(
id SERIAL,
k INTEGER DEFAULT '0' NOT NULL,
PRIMARY KEY (id)
) USING :tde_am;

SELECT pg_tde_add_key_provider_vault_v2('vault-v2',:'root_token','http://127.0.0.1:8200','secret',NULL);
SELECT pg_tde_set_principal_key('vault-v2-principal-key','vault-v2');

Expand Down
16 changes: 10 additions & 6 deletions src/access/pg_tde_tdemap.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ pg_tde_create_key_map_entry(const RelFileLocator *newrlocator, uint32 entry_type
if (!RAND_bytes(int_key.key, INTERNAL_KEY_LEN))
{
LWLockRelease(lock_pk);
ereport(FATAL,
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
errmsg("could not generate internal key for relation \"%s\": %s",
"TODO", ERR_error_string(ERR_get_error(), NULL))));
Expand Down Expand Up @@ -439,8 +439,9 @@ pg_tde_write_one_map_entry(int fd, const RelFileLocator *rlocator, uint32 flags,
{
char db_map_path[MAXPGPATH] = {0};

// TODO: this seems like a bad idea?
pg_tde_set_db_file_paths(rlocator->dbOid, db_map_path, NULL);
ereport(FATAL,
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write tde map file \"%s\": %m",
db_map_path)));
Expand All @@ -449,6 +450,7 @@ pg_tde_write_one_map_entry(int fd, const RelFileLocator *rlocator, uint32 flags,
{
char db_map_path[MAXPGPATH] = {0};

// TODO: this seems like a bad idea?
pg_tde_set_db_file_paths(rlocator->dbOid, db_map_path, NULL);
ereport(data_sync_elevel(ERROR),
(errcode_for_file_access(),
Expand Down Expand Up @@ -500,7 +502,8 @@ pg_tde_write_one_keydata(int fd, int32 key_index, RelKeyData *enc_rel_key_data)
/* TODO: pgstat_report_wait_start / pgstat_report_wait_end */
if (pg_pwrite(fd, &enc_rel_key_data->internal_key, INTERNAL_KEY_DAT_LEN, curr_pos) != INTERNAL_KEY_DAT_LEN)
{
ereport(FATAL,
// TODO: what now? File is corrupted
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write tde key data file: %m")));
}
Expand Down Expand Up @@ -1022,10 +1025,11 @@ pg_tde_process_map_entry(const RelFileLocator *rlocator, uint32 key_type, char *

if (curr_pos == -1)
{
ereport(FATAL,
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not seek in tde map file \"%s\": %m",
db_map_path)));
return curr_pos;
}
}
else
Expand Down Expand Up @@ -1117,7 +1121,7 @@ tde_decrypt_rel_key(TDEPrincipalKey *principal_key, RelKeyData *enc_rel_key_data
* Open and Validate File Header [pg_tde.*]:
* header: {Format Version, Principal Key Name}
*
* Returns the file descriptor in case of a success. Otherwise, fatal error
* Returns the file descriptor in case of a success. Otherwise, error
* is raised.
*
* Also, it sets the is_new_file to true if the file is just created. This is
Expand Down Expand Up @@ -1160,7 +1164,7 @@ pg_tde_open_file(char *tde_filename, TDEPrincipalKeyInfo *principal_key_info, bo
/*
* Open a TDE file [pg_tde.*]:
*
* Returns the file descriptor in case of a success. Otherwise, fatal error
* Returns the file descriptor in case of a success. Otherwise, error
* is raised except when ignore_missing is true and the file does not exit.
*/
static int
Expand Down
8 changes: 5 additions & 3 deletions src/catalog/tde_principal_key.c
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ set_principal_key_with_keyring(const char *key_name, GenericKeyring *keyring,
keyInfo = load_latest_versioned_key_name(&principalKey->keyInfo, keyring, ensure_new_key);

if (keyInfo == NULL)
keyInfo = KeyringGenerateNewKeyAndStore(keyring, principalKey->keyInfo.keyId.versioned_name, INTERNAL_KEY_LEN, false);
keyInfo = KeyringGenerateNewKeyAndStore(keyring, principalKey->keyInfo.keyId.versioned_name, INTERNAL_KEY_LEN, true);

if (keyInfo == NULL)
{
Expand Down Expand Up @@ -424,7 +424,7 @@ RotatePrincipalKey(TDEPrincipalKey *current_key, const char *new_key_name, const
keyInfo = load_latest_versioned_key_name(&new_principal_key.keyInfo, keyring, ensure_new_key);

if (keyInfo == NULL)
keyInfo = KeyringGenerateNewKeyAndStore(keyring, new_principal_key.keyInfo.keyId.versioned_name, INTERNAL_KEY_LEN, false);
keyInfo = KeyringGenerateNewKeyAndStore(keyring, new_principal_key.keyInfo.keyId.versioned_name, INTERNAL_KEY_LEN, true);

if (keyInfo == NULL)
{
Expand Down Expand Up @@ -496,9 +496,10 @@ load_latest_versioned_key_name(TDEPrincipalKeyInfo *principal_key_info, GenericK
*/
if (kr_ret != KEYRING_CODE_SUCCESS && kr_ret != KEYRING_CODE_RESOURCE_NOT_AVAILABLE)
{
ereport(FATAL,
ereport(ERROR,
(errmsg("failed to retrieve principal key from keyring provider :\"%s\"", keyring->provider_name),
errdetail("Error code: %d", kr_ret)));
return NULL;
}
if (keyInfo == NULL)
{
Expand Down Expand Up @@ -531,6 +532,7 @@ load_latest_versioned_key_name(TDEPrincipalKeyInfo *principal_key_info, GenericK
{
ereport(ERROR,
(errmsg("failed to retrieve principal key. %d versions already exist", MAX_PRINCIPAL_KEY_VERSION_NUM)));
return NULL;
}
}
return NULL; /* Just to keep compiler quite */
Expand Down
2 changes: 2 additions & 0 deletions src/keyring/keyring_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ KeyringGenerateNewKeyAndStore(GenericKeyring *keyring, const char *key_name, uns
if (KeyringStoreKey(keyring, key, throw_error) != KEYRING_CODE_SUCCESS)
{
pfree(key);
ereport(ereport_level,
(errmsg("Failed to store key on keyring. Please check the keyring configuration.")));
return NULL;
}
return key;
Expand Down
1 change: 0 additions & 1 deletion src/pg_tde_event_capture.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ pg_tde_ddl_command_start_capture(PG_FUNCTION_ARGS)
trigdata = (EventTriggerData *) fcinfo->context;
parsetree = trigdata->parsetree;

elog(LOG, "EVENT TRIGGER (%s) %s", trigdata->event, nodeToString(parsetree));
reset_current_tde_create_event();

if (IsA(parsetree, IndexStmt))
Expand Down
Loading