- Notifications
You must be signed in to change notification settings - Fork 456
CDRIVER-3927 add KMS TLS tests for client side encryption #879
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
Given ENABLE_CLIENT_SIDE_ENCRYPTION requires MONGOC_ENABLE_SSL, client side encryption tests should not have to be compiled if SSL is not enabled.
kevinAlbs left a comment
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.
Great work! I appreciate the detailed description. Splitting up the work into meaningful commits made reviewing much easier.
kevinAlbs left a comment
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.
LGTM! Nice detective work in finding CDRIVER-4181. I think the additional error details are likely to help users diagnose TLS configuration problems.
rcsanchez97 left a comment
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.
LGTM, modulo the one minor nit.
eramongodb left a comment
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.
Investigating certificate registration failure on RHEL 8.1. Will merge once addressed.
| @kevinAlbs Per external discussion, this PR will now disable CSE tests on RHEL upon merge. This change will be reverted on resolution of BUILD-14068, which will obviate the need to manually register the CA certificate via Evergreen build scripts. This followup task is recorded as CDRIVER-4183. |
This PR comes in three parts:
The TLS handshake implementation for OpenSSL and Secure Channel require some modification to report descriptive certificate verification errors. The TLS handshake implementation for Secure Transport and LibreSSL do not require any modification to obtain descriptive error messages.
Client Side Encryption tests were unconditionally being included by the test suite even when SSL is not enabled, despite CSE requiring SSL to be enabled. Therefore, the relevant source file and test functions are now moved into appropriate conditionals so they are only built and run when
MONGOC_ENABLE_SSLis defined by the build system.Existing unit test helper functions are modified to enable reuse of AWS KMS provider setup helpers for KMS TLS tests separately from other KMS providers. The test function
test_kms_tls_cert_valid(), which confirms a valid TLS handshake on valid server certificate, is not required by CDRIVER-3927, but nevertheless useful to confirm proper CA certificate registration in the environment. It explicitly callsmongoc_stream_tls_handshake_block()and relevant setup functions rathermongoc_client_encryption_create_datakey()to avoid unnecessary errors on datakey creation due to running against the mock KMS server.Evergreen config scripts are updated to start mock KMS servers in the background only for tasks with Client Side Encryption enabled. The run_tests.sh script also requires an update to ensure proper CA certificate registration on Windows and Red Hat Linux, which use different commands and directories.