PG-1506: Add separate parameters for kmip client cert and key #306
Conversation
|
Is there a reason to not make the key path mandatory? If someone wants to have them in the same file they could point both the certificate path and the key path to the same file. Seems like that would mean less complex logic. And all tools I can think of on top of my head keep these two paths separate and mandatory to specify both. |
|
Mongodb uses this for example, both mongo enterprise and ours. I kept both options here as it isn't more difficult, and still allows the previous behavior. |
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.
Codecov ReportAttention: Patch coverage is
❌ Your project status has failed because the head coverage (80.28%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## TDE_REL_17_STABLE #306 +/- ##
=====================================================
+ Coverage 80.24% 80.28% +0.04%
=====================================================
Files 22 22
Lines 2546 2552 +6
Branches 399 398 -1
=====================================================
+ Hits 2043 2049 +6
Misses 423 423
Partials 80 80
🚀 New features to boost your workflow:
|
|
I think the previous behavior is mostly confusing and means one more code path to test and document and since we are not yet GA I do not see any reason to support it. |
|
I would agree if supporting it would be a big difference - but really the only thing we need to support it is the default values in the sql functions and the additional condition in the kmip keyring code, that's a single line. I also added some conditions for handling if the value is NULL, but realistically that shouldn't happen even with the current code.
if you google it, some people keep them in the same file, and this is a valid/supported usecase by openssl and other ssl software. On the other hand, users just could repeat the same filename twice, or we could add an overload to the functions that does exactly that, but I don't think that would be simpler than this. |
Updates merged from : PG-1506: Add separate parameters for kmip client cert and key #306
| @@ -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. | |||
There was a problem hiding this comment.
Added small clarification after path to the client key: Optional, if not specified the certificate file has to contain both the certificate and the key.
|
To me it seems quite arbitrary to make this parameter optional. Now is the time to do backwards incompatible changes since this isn't in use anywhere yet, so making the "old" way work doesn't really mean anything valuable I think. |
Updated with suggested cleaner text Co-authored-by: Anastasia Alexandrova <anastasia.alexandrova@percona.com>
Modified the structure and overall presentation for the users to make it more accessible and future-proof as we improve pg_tde moving forward. Split big chapters into smaller chunks to improve SEO, renamed a couple of files for better visibility online and cleaned a bit of text. Integrated changes from: - #314 - #306 - Commit f636e82
|
Replaced by #333 |
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.