From 566975e449ad71be07822b45ded7fa735a817646 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sun, 4 May 2025 21:28:17 +0100 Subject: [PATCH 1/2] PG-1506: Add separate parameters for kmip client cert and key The old behavior still exists, as the new parameter is optional. If only a certificate is specified, the keyring expects both the key and certificate to be in the same file. If both parameters are specified, it looks for the key and cert in separate files. --- .../documentation/docs/command-line-tools.md | 2 +- contrib/pg_tde/documentation/docs/setup.md | 7 +-- contrib/pg_tde/expected/kmip_test.out | 18 +++++++ contrib/pg_tde/pg_tde--1.0-rc.sql | 48 ++++++++++++------- contrib/pg_tde/pykmip-server.conf | 2 +- contrib/pg_tde/sql/kmip_test.sql | 4 ++ contrib/pg_tde/src/catalog/tde_keyring.c | 14 ++++-- .../src/catalog/tde_keyring_parse_opts.c | 7 +++ .../pg_tde/src/include/keyring/keyring_api.h | 1 + contrib/pg_tde/src/keyring/keyring_kmip.c | 5 +- .../pg_tde/src/pg_tde_change_key_provider.c | 4 +- 11 files changed, 83 insertions(+), 29 deletions(-) diff --git a/contrib/pg_tde/documentation/docs/command-line-tools.md b/contrib/pg_tde/documentation/docs/command-line-tools.md index a737505c4861c..c894523957241 100644 --- a/contrib/pg_tde/documentation/docs/command-line-tools.md +++ b/contrib/pg_tde/documentation/docs/command-line-tools.md @@ -37,7 +37,7 @@ Depending on the provider type, the additional parameters are: ``` pg_tde_change_key_provider [-D ] file pg_tde_change_key_provider [-D ] vault [] -pg_tde_change_key_provider [-D ] kmip [] +pg_tde_change_key_provider [-D ] kmip [] ``` ## pg_waldump diff --git a/contrib/pg_tde/documentation/docs/setup.md b/contrib/pg_tde/documentation/docs/setup.md index 9e8dbcb51b019..d6ea0bb223626 100644 --- a/contrib/pg_tde/documentation/docs/setup.md +++ b/contrib/pg_tde/documentation/docs/setup.md @@ -53,7 +53,7 @@ Load the `pg_tde` at startup time. The extension requires additional shared memo For testing purposes, you can use the PyKMIP server which enables you to set up required certificates. To use a real KMIP server, make sure to obtain the valid certificates issued by the key management appliance. ```sql - SELECT pg_tde_add_global_key_provider_kmip('provider-name','kmip-IP', 5696, '/path_to/server_certificate.pem', '/path_to/client_key.pem'); + SELECT pg_tde_add_global_key_provider_kmip('provider-name','kmip-IP', 5696, '/path_to/server_certificate.pem', '/path_to/client_cert.pem', 'path_to/client_key.pem'); ``` where: @@ -62,14 +62,15 @@ Load the `pg_tde` at startup time. The extension requires additional shared memo * `kmip-IP` is the IP address of a domain name of the KMIP server * `port` is the port to communicate with the KMIP server. Typically used port is 5696. * `server-certificate` is the path to the certificate file for the KMIP server. - * `client key` is the path to the client key. + * `client-cert` is the path to the client certificate. + * `client-key` is the path to the client key. This is optional, if not specified, the certificate key has to contain both the certifcate and the key. :material-information: Warning: Note that pg_tde_add_global_key_provider_kmip currently accepts only a combined client key + client certificate for the last parameter of this function named as `client key`. :material-information: Warning: This example is for testing purposes only: ``` - SELECT pg_tde_add_global_key_provider_kmip('kmip','127.0.0.1', 5696, '/tmp/server_certificate.pem', '/tmp/client_key_jane_doe.pem'); + SELECT pg_tde_add_global_key_provider_kmip('kmip','127.0.0.1', 5696, '/tmp/server_certificate.pem', '/tmp/client_cert_jane_doe.pem', '/tmp/client_key_jane_doe.pem'); ``` === "With HashiCorp Vault" diff --git a/contrib/pg_tde/expected/kmip_test.out b/contrib/pg_tde/expected/kmip_test.out index 3d905bbaeedb3..8caa0aec45a9c 100644 --- a/contrib/pg_tde/expected/kmip_test.out +++ b/contrib/pg_tde/expected/kmip_test.out @@ -5,6 +5,12 @@ SELECT pg_tde_add_database_key_provider_kmip('kmip-prov','127.0.0.1', 5696, '/tm 1 (1 row) +SELECT pg_tde_add_database_key_provider_kmip('kmip-prov2','127.0.0.1', 5696, '/tmp/server_certificate.pem', '/tmp/client_certificate_jane_smith.pem', '/tmp/client_key_jane_smith.pem'); + pg_tde_add_database_key_provider_kmip +--------------------------------------- + 2 +(1 row) + SELECT pg_tde_set_key_using_database_key_provider('kmip-key','kmip-prov'); pg_tde_set_key_using_database_key_provider -------------------------------------------- @@ -33,6 +39,18 @@ SELECT pg_tde_verify_key(); (1 row) +SELECT pg_tde_set_key_using_database_key_provider('kmip-key2','kmip-prov2'); + pg_tde_set_key_using_database_key_provider +-------------------------------------------- + +(1 row) + +SELECT pg_tde_verify_key(); + pg_tde_verify_key +------------------- + +(1 row) + DROP TABLE test_enc; -- Creating provider fails if we can't connect to kmip server 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'); diff --git a/contrib/pg_tde/pg_tde--1.0-rc.sql b/contrib/pg_tde/pg_tde--1.0-rc.sql index a8cf95b7c0564..8979365799764 100644 --- a/contrib/pg_tde/pg_tde--1.0-rc.sql +++ b/contrib/pg_tde/pg_tde--1.0-rc.sql @@ -69,7 +69,8 @@ CREATE FUNCTION pg_tde_add_database_key_provider_kmip(provider_name TEXT, kmip_host TEXT, kmip_port INT, kmip_ca_path TEXT, - kmip_cert_path TEXT) + kmip_cert_path TEXT, + kmip_key_path TEXT DEFAULT '') RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -80,14 +81,16 @@ BEGIN ATOMIC 'host' VALUE COALESCE(kmip_host, ''), 'port' VALUE kmip_port, 'caPath' VALUE COALESCE(kmip_ca_path, ''), - 'certPath' VALUE COALESCE(kmip_cert_path, ''))); + 'certPath' VALUE COALESCE(kmip_cert_path, ''), + 'keyPath' VALUE COALESCE(kmip_key_path, ''))); END; CREATE FUNCTION pg_tde_add_database_key_provider_kmip(provider_name TEXT, kmip_host JSON, kmip_port JSON, kmip_ca_path JSON, - kmip_cert_path JSON) + kmip_cert_path JSON, + kmip_key_path JSON) RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -98,7 +101,8 @@ BEGIN ATOMIC 'host' VALUE kmip_host, 'port' VALUE kmip_port, 'caPath' VALUE kmip_ca_path, - 'certPath' VALUE kmip_cert_path)); + 'certPath' VALUE kmip_cert_path, + 'keyPath' VALUE kmip_key_path)); END; @@ -186,7 +190,8 @@ CREATE FUNCTION pg_tde_add_global_key_provider_kmip(provider_name TEXT, kmip_host TEXT, kmip_port INT, kmip_ca_path TEXT, - kmip_cert_path TEXT) + kmip_cert_path TEXT, + kmip_key_path TEXT DEFAULT '') RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -197,14 +202,16 @@ BEGIN ATOMIC 'host' VALUE COALESCE(kmip_host, ''), 'port' VALUE kmip_port, 'caPath' VALUE COALESCE(kmip_ca_path, ''), - 'certPath' VALUE COALESCE(kmip_cert_path, ''))); + 'certPath' VALUE COALESCE(kmip_cert_path, ''), + 'keyPath' VALUE COALESCE(kmip_key_path, ''))); END; CREATE FUNCTION pg_tde_add_global_key_provider_kmip(provider_name TEXT, kmip_host JSON, kmip_port JSON, kmip_ca_path JSON, - kmip_cert_path JSON) + kmip_cert_path JSON, + kmip_key_path JSON) RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -215,7 +222,8 @@ BEGIN ATOMIC 'host' VALUE kmip_host, 'port' VALUE kmip_port, 'caPath' VALUE kmip_ca_path, - 'certPath' VALUE kmip_cert_path)); + 'certPath' VALUE kmip_cert_path, + 'keyPath' VALUE kmip_key_path)); END; -- Key Provider Management @@ -284,7 +292,8 @@ CREATE FUNCTION pg_tde_change_database_key_provider_kmip(provider_name TEXT, kmip_host TEXT, kmip_port INT, kmip_ca_path TEXT, - kmip_cert_path TEXT) + kmip_cert_path TEXT, + kmip_key_path TEXT DEFAULT '') RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -295,14 +304,16 @@ BEGIN ATOMIC 'host' VALUE COALESCE(kmip_host, ''), 'port' VALUE kmip_port, 'caPath' VALUE COALESCE(kmip_ca_path, ''), - 'certPath' VALUE COALESCE(kmip_cert_path, ''))); + 'certPath' VALUE COALESCE(kmip_cert_path, ''), + 'keyPath' VALUE COALESCE(kmip_key_path, ''))); END; CREATE FUNCTION pg_tde_change_database_key_provider_kmip(provider_name TEXT, kmip_host JSON, kmip_port JSON, kmip_ca_path JSON, - kmip_cert_path JSON) + kmip_cert_path JSON, + kmip_key_path JSON) RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -313,7 +324,8 @@ BEGIN ATOMIC 'host' VALUE kmip_host, 'port' VALUE kmip_port, 'caPath' VALUE kmip_ca_path, - 'certPath' VALUE kmip_cert_path)); + 'certPath' VALUE kmip_cert_path, + 'keyPath' VALUE kmip_key_path)); END; -- Global Tablespace Key Provider Management @@ -382,7 +394,8 @@ CREATE FUNCTION pg_tde_change_global_key_provider_kmip(provider_name TEXT, kmip_host TEXT, kmip_port INT, kmip_ca_path TEXT, - kmip_cert_path TEXT) + kmip_cert_path TEXT, + kmip_key_path TEXT DEFAULT NULL) RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -393,14 +406,16 @@ BEGIN ATOMIC 'host' VALUE COALESCE(kmip_host, ''), 'port' VALUE kmip_port, 'caPath' VALUE COALESCE(kmip_ca_path, ''), - 'certPath' VALUE COALESCE(kmip_cert_path, ''))); + 'certPath' VALUE COALESCE(kmip_cert_path, ''), + 'keyPath' VALUE COALESCE(kmip_key_path, ''))); END; CREATE FUNCTION pg_tde_change_global_key_provider_kmip(provider_name TEXT, kmip_host JSON, kmip_port JSON, kmip_ca_path JSON, - kmip_cert_path JSON) + kmip_cert_path JSON, + kmip_key_path JSON) RETURNS INT LANGUAGE SQL BEGIN ATOMIC @@ -411,7 +426,8 @@ BEGIN ATOMIC 'host' VALUE kmip_host, 'port' VALUE kmip_port, 'caPath' VALUE kmip_ca_path, - 'certPath' VALUE kmip_cert_path)); + 'certPath' VALUE kmip_cert_path, + 'keyPath' VALUE kmip_key_path)); END; CREATE FUNCTION pg_tde_is_encrypted(relation regclass) diff --git a/contrib/pg_tde/pykmip-server.conf b/contrib/pg_tde/pykmip-server.conf index ffea4f118ecdb..d25c57b7b64b8 100644 --- a/contrib/pg_tde/pykmip-server.conf +++ b/contrib/pg_tde/pykmip-server.conf @@ -6,7 +6,7 @@ key_path=/tmp/server_key.pem ca_path=/tmp/root_certificate.pem auth_suite=TLS1.2 policy_path=/tmp/policies -enable_tls_client_auth=True +enable_tls_client_auth=False tls_cipher_suites= TLS_RSA_WITH_AES_128_CBC_SHA256 TLS_RSA_WITH_AES_256_CBC_SHA256 diff --git a/contrib/pg_tde/sql/kmip_test.sql b/contrib/pg_tde/sql/kmip_test.sql index d47467e5fda16..ccaec42b39641 100644 --- a/contrib/pg_tde/sql/kmip_test.sql +++ b/contrib/pg_tde/sql/kmip_test.sql @@ -1,6 +1,7 @@ CREATE EXTENSION pg_tde; SELECT pg_tde_add_database_key_provider_kmip('kmip-prov','127.0.0.1', 5696, '/tmp/server_certificate.pem', '/tmp/client_key_jane_doe.pem'); +SELECT pg_tde_add_database_key_provider_kmip('kmip-prov2','127.0.0.1', 5696, '/tmp/server_certificate.pem', '/tmp/client_certificate_jane_smith.pem', '/tmp/client_key_jane_smith.pem'); SELECT pg_tde_set_key_using_database_key_provider('kmip-key','kmip-prov'); CREATE TABLE test_enc( @@ -17,6 +18,9 @@ SELECT * from test_enc; SELECT pg_tde_verify_key(); +SELECT pg_tde_set_key_using_database_key_provider('kmip-key2','kmip-prov2'); +SELECT pg_tde_verify_key(); + DROP TABLE test_enc; -- Creating provider fails if we can't connect to kmip server diff --git a/contrib/pg_tde/src/catalog/tde_keyring.c b/contrib/pg_tde/src/catalog/tde_keyring.c index 2ebac43a28894..e02f12e191769 100644 --- a/contrib/pg_tde/src/catalog/tde_keyring.c +++ b/contrib/pg_tde/src/catalog/tde_keyring.c @@ -930,10 +930,16 @@ debug_print_kerying(GenericKeyring *keyring) elog(debug_level, "Vault Keyring CA Path: %s", ((VaultV2Keyring *) keyring)->vault_ca_path); break; case KMIP_KEY_PROVIDER: - elog(debug_level, "KMIP Keyring Host: %s", ((KmipKeyring *) keyring)->kmip_host); - elog(debug_level, "KMIP Keyring Port: %s", ((KmipKeyring *) keyring)->kmip_port); - elog(debug_level, "KMIP Keyring CA Path: %s", ((KmipKeyring *) keyring)->kmip_ca_path); - elog(debug_level, "KMIP Keyring Cert Path: %s", ((KmipKeyring *) keyring)->kmip_cert_path); + { + KmipKeyring *kkeyring = (KmipKeyring *) keyring; + char *key_path = kkeyring->kmip_key_path == NULL ? "" : kkeyring->kmip_key_path; + + elog(debug_level, "KMIP Keyring Host: %s", kkeyring->kmip_host); + elog(debug_level, "KMIP Keyring Port: %s", kkeyring->kmip_port); + elog(debug_level, "KMIP Keyring CA Path: %s", kkeyring->kmip_ca_path); + elog(debug_level, "KMIP Keyring Cert Path: %s", kkeyring->kmip_cert_path); + elog(debug_level, "KMIP Keyring Key Path: %s", key_path); + } break; case UNKNOWN_KEY_PROVIDER: elog(debug_level, "Unknown Keyring "); diff --git a/contrib/pg_tde/src/catalog/tde_keyring_parse_opts.c b/contrib/pg_tde/src/catalog/tde_keyring_parse_opts.c index e811040d82976..0cda48ea050af 100644 --- a/contrib/pg_tde/src/catalog/tde_keyring_parse_opts.c +++ b/contrib/pg_tde/src/catalog/tde_keyring_parse_opts.c @@ -73,6 +73,7 @@ typedef enum JsonKeyringField JK_KMIP_PORT, JK_KMIP_CA_PATH, JK_KMIP_CERT_PATH, + JK_KMIP_KEY_PATH, /* must be the last */ JK_FIELDS_TOTAL @@ -99,6 +100,7 @@ static const char *JK_FIELD_NAMES[JK_FIELDS_TOTAL] = { [JK_KMIP_PORT] = "port", [JK_KMIP_CA_PATH] = "caPath", [JK_KMIP_CERT_PATH] = "certPath", + [JK_KMIP_KEY_PATH] = "keyPath", }; #define MAX_JSON_DEPTH 64 @@ -362,6 +364,8 @@ json_kring_object_field_start(void *state, char *fname, bool isnull) *field = JK_KMIP_CA_PATH; else if (strcmp(fname, JK_FIELD_NAMES[JK_KMIP_CERT_PATH]) == 0) *field = JK_KMIP_CERT_PATH; + else if (strcmp(fname, JK_FIELD_NAMES[JK_KMIP_KEY_PATH]) == 0) + *field = JK_KMIP_KEY_PATH; else { *field = JK_FIELD_UNKNOWN; @@ -458,6 +462,9 @@ json_kring_assign_scalar(JsonKeyringState *parse, JsonKeyringField field, char * case JK_KMIP_CERT_PATH: kmip->kmip_cert_path = value; break; + case JK_KMIP_KEY_PATH: + kmip->kmip_key_path = value; + break; default: elog(ERROR, "json keyring: unexpected scalar field %d", field); diff --git a/contrib/pg_tde/src/include/keyring/keyring_api.h b/contrib/pg_tde/src/include/keyring/keyring_api.h index 89be8282f8500..ef322709823c5 100644 --- a/contrib/pg_tde/src/include/keyring/keyring_api.h +++ b/contrib/pg_tde/src/include/keyring/keyring_api.h @@ -86,6 +86,7 @@ typedef struct KmipKeyring char *kmip_port; char *kmip_ca_path; char *kmip_cert_path; + char *kmip_key_path; } KmipKeyring; extern void RegisterKeyProviderType(const TDEKeyringRoutine *routine, ProviderType type); diff --git a/contrib/pg_tde/src/keyring/keyring_kmip.c b/contrib/pg_tde/src/keyring/keyring_kmip.c index 86b438a920886..8bb65186afba2 100644 --- a/contrib/pg_tde/src/keyring/keyring_kmip.c +++ b/contrib/pg_tde/src/keyring/keyring_kmip.c @@ -51,6 +51,7 @@ kmipSslConnect(KmipCtx *ctx, KmipKeyring *kmip_keyring, bool throw_error) { SSL *ssl = NULL; int level = throw_error ? ERROR : WARNING; + char *key_path = kmip_keyring->kmip_key_path == NULL || strlen(kmip_keyring->kmip_key_path) == 0 ? kmip_keyring->kmip_cert_path : kmip_keyring->kmip_key_path; ctx->ssl = SSL_CTX_new(SSLv23_method()); @@ -61,10 +62,10 @@ kmipSslConnect(KmipCtx *ctx, KmipKeyring *kmip_keyring, bool throw_error) return false; } - if (SSL_CTX_use_PrivateKey_file(ctx->ssl, kmip_keyring->kmip_cert_path, SSL_FILETYPE_PEM) != 1) + if (SSL_CTX_use_PrivateKey_file(ctx->ssl, key_path, SSL_FILETYPE_PEM) != 1) { SSL_CTX_free(ctx->ssl); - ereport(level, errmsg("SSL error: Loading the client key failed")); + ereport(level, errmsg("SSL error: Loading the client key failed: %s", key_path)); return false; } diff --git a/contrib/pg_tde/src/pg_tde_change_key_provider.c b/contrib/pg_tde/src/pg_tde_change_key_provider.c index 87db2c5d819b9..175332d3c0dbd 100644 --- a/contrib/pg_tde/src/pg_tde_change_key_provider.c +++ b/contrib/pg_tde/src/pg_tde_change_key_provider.c @@ -206,7 +206,7 @@ main(int argc, char *argv[]) { provider_found = true; - if (argc - argstart != 7 && argc - argstart != 8) + if (argc - argstart != 8 && argc - argstart != 9) { help(); puts("\n"); @@ -214,7 +214,7 @@ main(int argc, char *argv[]) exit(1); } - if (!build_json(json, 5, "type", "kmip", "host", argv[4 + argstart], "port", argv[5 + argstart], "caPath", (argc - argstart > 7 ? argv[7 + argstart] : ""), "certPath", argv[6 + argstart])) + if (!build_json(json, 6, "type", "kmip", "host", argv[4 + argstart], "port", argv[5 + argstart], "caPath", (argc - argstart > 8 ? argv[8 + argstart] : ""), "certPath", argv[6 + argstart], "keyPath", argv[7 + argstart])) { exit(1); } From da6499440410a4f859c769e1d5e457bb6ed64442 Mon Sep 17 00:00:00 2001 From: Dragos Andriciuc Date: Tue, 6 May 2025 17:48:21 +0300 Subject: [PATCH 2/2] Update contrib/pg_tde/documentation/docs/setup.md Updated with suggested cleaner text Co-authored-by: Anastasia Alexandrova --- contrib/pg_tde/documentation/docs/setup.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/pg_tde/documentation/docs/setup.md b/contrib/pg_tde/documentation/docs/setup.md index d6ea0bb223626..99e0bf9fdf98d 100644 --- a/contrib/pg_tde/documentation/docs/setup.md +++ b/contrib/pg_tde/documentation/docs/setup.md @@ -63,7 +63,7 @@ Load the `pg_tde` at startup time. The extension requires additional shared memo * `port` is the port to communicate with the KMIP server. Typically used port is 5696. * `server-certificate` is the path to the certificate file for the KMIP server. * `client-cert` is the path to the client certificate. - * `client-key` is the path to the client key. This is optional, if not specified, the certificate key has to contain both the certifcate and the key. + * `client-key` (optional) is the path to the client key. If not specified, the certificate key has to contain both the certifcate and the key. :material-information: Warning: Note that pg_tde_add_global_key_provider_kmip currently accepts only a combined client key + client certificate for the last parameter of this function named as `client key`.