Support for encryption of MySQL connections#5122
Conversation
| private String trustCertificateKeyStoreType; | ||
|
|
||
| @JsonProperty | ||
| private String trustCertificateKeyStorePassword; |
There was a problem hiding this comment.
for passwords use same mechanism as the db password so that users can provide non plain text passwords.
| |`druid.metadata.mysql.ssl.trustCertificateKeyStorePassword`|Password for the trust store.|none|no| | ||
| |`druid.metadata.mysql.ssl.clientCertificateKeyStoreUrl`|The file path URL to the client certificate key store.|none|no| | ||
| |`druid.metadata.mysql.ssl.clientCertificateKeyStoreType`|The type of the key store where the client certificate is stored.|none|no| | ||
| |`druid.metadata.mysql.ssl.clientCertificateKeyStorePassword`|Password for the client key store.|none|no| |
There was a problem hiding this comment.
is it possible in some cases to have same values for “trustCertificateKeyStoreUrl” and “clientCertificateKeyStoreUrl” .. their types and passwords ? in that case we could default client* properties to be same as trust* properties
There was a problem hiding this comment.
ignore it, i think in all cases they would be independent.
| log.info("SSL is enabled on this MySQL connection. "); | ||
|
|
||
| // Server certificate verification is enabled by default | ||
| datasource.addConnectionProperty("verifyServerCertificate", String.valueOf(true)); |
There was a problem hiding this comment.
we should have this configurable
| |`druid.metadata.mysql.ssl.clientCertificateKeyStorePassword`|The [Password Provider](../operations/password-provider.html) or String password for the client key store.|none|no| | ||
| |`druid.metadata.mysql.ssl.enabledSSLCipherSuites`|Overrides the existing cipher suites with these cipher suites.|none|no| | ||
| |`druid.metadata.mysql.ssl.enabledTLSProtocols`|Overrides the TLS protocols with these protocols.|none|no| | ||
| |`druid.metadata.mysql.ssl.verifyServerCertificate`|Enables server certificate verification.|true|no| |
There was a problem hiding this comment.
can you please specify configurations that don't remain mandatory when this is set to false.
|
👍 |
| datasource.addConnectionProperty("verifyServerCertificate", String.valueOf(connectorConfig.isVerifyServerCertificate())); | ||
| if (connectorConfig.isVerifyServerCertificate()) { | ||
| log.info("Server certificate verification is enabled. "); | ||
| if (connectorConfig.getTrustCertificateKeyStoreUrl() != null) { |
There was a problem hiding this comment.
Since this is required when verifyServerCertificate is set to true, what happens when user does not set this and it is null. Will there be some explicit exception/warning in the logs which would make it obvious for user what's the problem ? otherwise it would be better to explicitly check and throw exception.
There was a problem hiding this comment.
The user does receive a generic error message that the TrustManager was not configured correctly, but it does not tell them the reason was because it was null.
There was a problem hiding this comment.
According to the Mysql docs, the user can leave these properties as null, in which case the default values would be used. The password can also be set to null. In that case, I'm not sure if we need to enforce a null check here. I have clarified this in the documentation. Please review.
| ); | ||
| } | ||
| if (connectorConfig.getTrustCertificateKeyStoreType() != null) { | ||
| datasource.addConnectionProperty( |
| connectorConfig.getTrustCertificateKeyStoreType() | ||
| ); | ||
| } | ||
| if (connectorConfig.getTrustCertificateKeyStorePassword() != null) { |
|
@a2l007 can this be finished up? |
|
@drcrallen : @a2l007 is on vacation this week. this will be wrapped up next week. thanks for reviewing. |
* Encrypting MySQL connections * Update docs * Make verifyServerCertificate a configurable parameter * Change password parameter and doc update * Make server certificate verification disabled by default * Update tostring * Update docs * Add check for trust store passwords * Add warning for null password
* Encrypting MySQL connections * Update docs * Make verifyServerCertificate a configurable parameter * Change password parameter and doc update * Make server certificate verification disabled by default * Update tostring * Update docs * Add check for trust store passwords * Add warning for null password
5 participants
With the MySQL versions 5.5.45+, 5.6.26+ and 5.7.6+, it is required that SSL connection must be explicitly enabled or disabled by default. Since this cannot be done with our existing code, MySQL would keep spewing warning messages everytime about explicitly enabling/disabling SSL on the connection.
This PR includes changes to support SSL/TLS encryption on MySQL connections. By default SSL is explicitly disabled on the connection so that existing users would not receive the SSL warning. Once useSSL is enabled, it would verify the server certificate by default and thus the trust store information would have to be provided along with the client keystore information for the TLS/SSL configuration.