-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 API to express like a --ssl-mode=PREFERRED
MySQL client
#1370
Changes from 6 commits
ad744fc
636b5b0
2a5ac9b
372d17b
9e2588b
a58b468
4ced115
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,13 +46,14 @@ type Config struct { | |
ServerPubKey string // Server public key name | ||
pubKey *rsa.PublicKey // Server public key | ||
TLSConfig string // TLS configuration name | ||
tls *tls.Config // TLS configuration | ||
TLS *tls.Config // TLS configuration, its priority is higher than TLSConfig | ||
Timeout time.Duration // Dial timeout | ||
ReadTimeout time.Duration // I/O read timeout | ||
WriteTimeout time.Duration // I/O write timeout | ||
|
||
AllowAllFiles bool // Allow all files to be used with LOAD DATA LOCAL INFILE | ||
AllowCleartextPasswords bool // Allows the cleartext client side plugin | ||
AllowFallbackToNoTLS bool // Allows fallback to unencrypted connection if server does not support TLS | ||
AllowNativePasswords bool // Allows the native password authentication method | ||
AllowOldPasswords bool // Allows the old insecure password method | ||
CheckConnLiveness bool // Check connections for liveness before using them | ||
|
@@ -77,8 +78,8 @@ func NewConfig() *Config { | |
|
||
func (cfg *Config) Clone() *Config { | ||
cp := *cfg | ||
if cp.tls != nil { | ||
cp.tls = cfg.tls.Clone() | ||
if cp.TLS != nil { | ||
cp.TLS = cfg.TLS.Clone() | ||
} | ||
if len(cp.Params) > 0 { | ||
cp.Params = make(map[string]string, len(cfg.Params)) | ||
|
@@ -119,24 +120,29 @@ func (cfg *Config) normalize() error { | |
cfg.Addr = ensureHavePort(cfg.Addr) | ||
} | ||
|
||
switch cfg.TLSConfig { | ||
case "false", "": | ||
// don't set anything | ||
case "true": | ||
cfg.tls = &tls.Config{} | ||
case "skip-verify", "preferred": | ||
cfg.tls = &tls.Config{InsecureSkipVerify: true} | ||
default: | ||
cfg.tls = getTLSConfigClone(cfg.TLSConfig) | ||
if cfg.tls == nil { | ||
return errors.New("invalid value / unknown config name: " + cfg.TLSConfig) | ||
if cfg.TLS == nil { | ||
switch cfg.TLSConfig { | ||
case "false", "": | ||
// don't set anything | ||
case "true": | ||
cfg.TLS = &tls.Config{} | ||
case "skip-verify": | ||
cfg.TLS = &tls.Config{InsecureSkipVerify: true} | ||
Check failure Code scanning / CodeQL Disabled TLS certificate check
InsecureSkipVerify should not be used in production code.
|
||
case "preferred": | ||
cfg.TLS = &tls.Config{InsecureSkipVerify: true} | ||
|
||
cfg.AllowFallbackToNoTLS = true | ||
default: | ||
cfg.TLS = getTLSConfigClone(cfg.TLSConfig) | ||
if cfg.TLS == nil { | ||
return errors.New("invalid value / unknown config name: " + cfg.TLSConfig) | ||
} | ||
} | ||
} | ||
|
||
if cfg.tls != nil && cfg.tls.ServerName == "" && !cfg.tls.InsecureSkipVerify { | ||
if cfg.TLS != nil && cfg.TLS.ServerName == "" && !cfg.TLS.InsecureSkipVerify { | ||
host, _, err := net.SplitHostPort(cfg.Addr) | ||
if err == nil { | ||
cfg.tls.ServerName = host | ||
cfg.TLS.ServerName = host | ||
} | ||
} | ||
|
||
|
@@ -204,6 +210,10 @@ func (cfg *Config) FormatDSN() string { | |
writeDSNParam(&buf, &hasParam, "allowCleartextPasswords", "true") | ||
} | ||
|
||
if cfg.AllowFallbackToNoTLS { | ||
writeDSNParam(&buf, &hasParam, "allowFallbackToNoTLS", "true") | ||
} | ||
|
||
if !cfg.AllowNativePasswords { | ||
writeDSNParam(&buf, &hasParam, "allowNativePasswords", "false") | ||
} | ||
|
@@ -391,6 +401,14 @@ func parseDSNParams(cfg *Config, params string) (err error) { | |
return errors.New("invalid bool value: " + value) | ||
} | ||
|
||
// Allow fallback to unencrypted connection if server does not support TLS | ||
case "allowFallbackToNoTLS": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I should give more detailed description on "when will the driver fallback to no TLS", does it happen on TLS identity verify error, not supported TLS version or server not enable TLS? I want a fallback for the last cases, and maybe the name should contain "TLSHandshake" or something. Will learn the TLS concepts to find a good name later. Welcome to give me some advice also cc @dveeden |
||
var isBool bool | ||
cfg.AllowFallbackToNoTLS, isBool = readBool(value) | ||
if !isBool { | ||
return errors.New("invalid bool value: " + value) | ||
} | ||
|
||
// Use native password authentication | ||
case "allowNativePasswords": | ||
var isBool bool | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -222,9 +222,9 @@ func (mc *mysqlConn) readHandshakePacket() (data []byte, plugin string, err erro | |
if mc.flags&clientProtocol41 == 0 { | ||
return nil, "", ErrOldProtocol | ||
} | ||
if mc.flags&clientSSL == 0 && mc.cfg.tls != nil { | ||
if mc.cfg.TLSConfig == "preferred" { | ||
mc.cfg.tls = nil | ||
if mc.flags&clientSSL == 0 && mc.cfg.TLS != nil { | ||
if mc.cfg.AllowFallbackToNoTLS { | ||
mc.cfg.TLS = nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why delete There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the case case "preferred":
cfg.TLS = &tls.Config{InsecureSkipVerify: true}
cfg.AllowFallbackToNoTLS = true I want to only use one variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. I got it. Merging this PR is beneficial for some users with old versions of MySQL. 🎉 LGTM. |
||
} else { | ||
return nil, "", ErrNoTLS | ||
} | ||
|
@@ -292,7 +292,7 @@ func (mc *mysqlConn) writeHandshakeResponsePacket(authResp []byte, plugin string | |
} | ||
|
||
// To enable TLS / SSL | ||
if mc.cfg.tls != nil { | ||
if mc.cfg.TLS != nil { | ||
clientFlags |= clientSSL | ||
} | ||
|
||
|
@@ -356,14 +356,14 @@ func (mc *mysqlConn) writeHandshakeResponsePacket(authResp []byte, plugin string | |
|
||
// SSL Connection Request Packet | ||
// http://dev.mysql.com/doc/internals/en/connection-phase-packets.html#packet-Protocol::SSLRequest | ||
if mc.cfg.tls != nil { | ||
if mc.cfg.TLS != nil { | ||
// Send TLS / SSL request packet | ||
if err := mc.writePacket(data[:(4+4+1+23)+4]); err != nil { | ||
return err | ||
} | ||
|
||
// Switch to TLS | ||
tlsConn := tls.Client(mc.netConn, mc.cfg.tls) | ||
tlsConn := tls.Client(mc.netConn, mc.cfg.TLS) | ||
if err := tlsConn.Handshake(); err != nil { | ||
return err | ||
} | ||
|
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.
In this version, does it still need to be public?
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.
Yes it does not need to be public, we can still use the old way to register a TLS config and use key name to find it.
Let the maintainer to decide if we can add a short path 😄