-
Notifications
You must be signed in to change notification settings - Fork 99
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
Add tls support for mysql client #186
base: master
Are you sure you want to change the base?
Conversation
|
Co-authored-by: Daniël van Eeden <[email protected]>
Co-authored-by: Daniël van Eeden <[email protected]>
Co-authored-by: Daniël van Eeden <[email protected]>
for i, addr := range targets { | ||
hash.Write([]byte(addr)) | ||
switch driver { | ||
case mysqlDriver: | ||
var tlsName string = "preferred" | ||
if len(sslCA) > 0 { |
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.
if len(sslCA) > 0 { | |
if len(sslCA) > 0 || len(sslKey) > 0 || len(sslCert) > 0 { |
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.
Refer https://docs.pingcap.com/tidb/stable/enable-tls-between-clients-and-servers#enable-authentication
By default, the server-to-client authentication is optional. Even if the client does not present its certificate of identification during the TLS handshake, the TLS connection can be still established.
If sslKey and sslCert are not provided, we will only authenticate the TiDB server from the MySQL client.
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.
I think valid configurations are:
- Only
sslCA
provided sslCA
,sslCert
,sslKey
all provided.
Not valid:
sslCert
and/orsslKey
set, but nosslCA
.sslCert
set, but notsslKey
sslKey
set, but notsslCert
So I think we should set the tlsName
to custom if any of sslCert
, sslKey
or sslCA
is set and then later on verify if it is valid (which might already be done)
Co-authored-by: Daniël van Eeden <[email protected]>
Co-authored-by: Daniël van Eeden <[email protected]>
Co-authored-by: Daniël van Eeden <[email protected]>
…ent-2495711184 github.com/spf13/cobra.(*Command).ExecuteC(0xc0003a9348)
Co-authored-by: Daniël van Eeden <[email protected]>
Co-authored-by: Daniël van Eeden <[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.
Please make sure this handles the situation correctly if sslKey
or sslCert
is set, but not sslCA
. Otherwise LGTM.
Co-authored-by: Daniël van Eeden <[email protected]>
@dveeden Please sign the CLA. |
Close: #185
Add tls config for connecting mysql with tls enabled
Manually test: