-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-4206 KMIP support #881
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
Conversation
225ff3d
to
b4bde18
Compare
4eb0140
to
da7d78d
Compare
MONGOC_TEST_KMIP_TLS_CERTIFICATE_KEY_FILE to test runner
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of work in here! I especially appreciate the careful documentation. Please take a look at just a few things in here.
src/libmongoc/doc/mongoc_auto_encryption_opts_set_kms_providers.rst
Outdated
Show resolved
Hide resolved
src/libmongoc/tests/client_side_encryption_prose/corpus/corpus-key-kmip.json
Show resolved
Hide resolved
src/libmongoc/doc/mongoc_client_encryption_opts_set_kms_providers.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
…safe to clean up in `_parse_one_tls_opts`.
…on a zero struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than some minor formatting, LGTM. 👍
Co-authored-by: Ezra Chung <[email protected]>
Co-authored-by: Ezra Chung <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strong improvements! Just a couple of things you may want to consider, but +1 from me.
Co-authored-by: Ezra Chung <[email protected]>
Summary
Implements the specification changes of DRIVERS-1353. Please see the comments of DRIVERS-1353 for the linked specification change commits.
mongoc_auto_encryption_opts_set_tls_opts
andmongoc_client_encryption_opts_set_tls_opts
to set custom TLS options on KMS connections.Rejected alternatives
Passing TLS options with mongoc_ssl_opt_t
The specification is not prescriptive in the API for passing TLS options:
I considered an alternative API for passing TLS options:
It is more consistent with
mongoc_client_set_ssl_opts
. But I chose to reject it because:mongoc_ssl_opt_t
does not permit configuring all TLS options supported in the URI specification (tlsDisableOCSPEndpointCheck
,tlsDisableCertificateRevocationCheck
).mongoc_ssl_opt_t
includes insecure options (allow_invalid_hostname
andweak_cert_validation
) which are prohibited in the specification, as well as C specific optionsca_dir
andcrl_file
, which are not required and are not supported on all TLS implementations.mongoc_uri_get_ssl
is deprecated formongoc_uri_get_tls
.Caveats