Skip to content

Commit b4da7cf

Browse files
author
Rafe Kettler
committed
Improve SSL support
Related issue: #521 - Add --ssl-cert and --ssl-key options to specify SSL public/private key files - Allow combining --ssl-allow-insecure with other --ssl* flags. `mysql.RegisterTLSConfig` allows combining the corresponding parameters in the `tls.Config` it receives, so gh-ost should allow this. I found being able to pass --ssl-allow-insecure along with --ssl-ca, --ssl-cert, and --ssl-key useful in testing. - Use the same TLS config everywhere. Since the CLI only supports a single set of --ssl* configuration parameters, this should be fine -- `mysql.RegisterTLSConfig` documentation indicates the TLS config given will not be modified, so it can safely be used in many goroutines provided we also do not modify it. The previous implementation did not work when the TLS config was duplicated, which happens when gh-ost walks up the replication chain trying to find the master. This is because, when the config is duplicated, we must call `RegisterTLSConfig` again with the new config. This config is exactly the same, so it's easiest to side-step the issue by registering the TLS config once and using it everywhere.
1 parent a8fae98 commit b4da7cf

File tree

5 files changed

+49
-20
lines changed

5 files changed

+49
-20
lines changed

doc/command-line-flags.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,14 @@ Allows `gh-ost` to connect to the MySQL servers using encrypted connections, but
189189

190190
`--ssl-ca=/path/to/ca-cert.pem`: ca certificate file (in PEM format) to use for server certificate verification. If specified, the default system ca cert pool will not be used for verification, only the ca cert provided here. Requires `--ssl`.
191191

192+
### ssl-cert
193+
194+
`--ssl-cert=/path/to/ssl-cert.crt`: SSL public key certificate file (in PEM format).
195+
196+
### ssl-key
197+
198+
`--ssl-key=/path/to/ssl-key.key`: SSL private key file (in PEM format).
199+
192200
### test-on-replica
193201

194202
Issue the migration on a replica; do not modify data on master. Useful for validating, testing and benchmarking. See [`testing-on-replica`](testing-on-replica.md)

go/base/context.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ type MigrationContext struct {
102102
UseTLS bool
103103
TLSAllowInsecure bool
104104
TLSCACertificate string
105+
TLSCertificate string
106+
TLSKey string
105107
CliMasterUser string
106108
CliMasterPassword string
107109

@@ -702,7 +704,7 @@ func (this *MigrationContext) ApplyCredentials() {
702704

703705
func (this *MigrationContext) SetupTLS() error {
704706
if this.UseTLS {
705-
return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSAllowInsecure)
707+
return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSCertificate, this.TLSKey, this.TLSAllowInsecure)
706708
}
707709
return nil
708710
}

go/cmd/gh-ost/main.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ func main() {
5757

5858
flag.BoolVar(&migrationContext.UseTLS, "ssl", false, "Enable SSL encrypted connections to MySQL hosts")
5959
flag.StringVar(&migrationContext.TLSCACertificate, "ssl-ca", "", "CA certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl")
60+
flag.StringVar(&migrationContext.TLSCertificate, "ssl-cert", "", "Certificate in PEM format for TLS connections to MySQL hosts. Requires --ssl")
61+
flag.StringVar(&migrationContext.TLSKey, "ssl-key", "", "Key in PEM format for TLS connections to MySQL hosts. Requires --ssl")
6062
flag.BoolVar(&migrationContext.TLSAllowInsecure, "ssl-allow-insecure", false, "Skips verification of MySQL hosts' certificate chain and host name. Requires --ssl")
6163

6264
flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)")
@@ -204,6 +206,12 @@ func main() {
204206
if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS {
205207
log.Fatalf("--ssl-ca requires --ssl")
206208
}
209+
if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS {
210+
log.Fatalf("--ssl-cert requires --ssl")
211+
}
212+
if migrationContext.TLSKey != "" && !migrationContext.UseTLS {
213+
log.Fatalf("--ssl-key requires --ssl")
214+
}
207215
if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS {
208216
log.Fatalf("--ssl-allow-insecure requires --ssl")
209217
}

go/mysql/connection.go

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import (
1616
"github.com/go-sql-driver/mysql"
1717
)
1818

19+
const (
20+
TLS_CONFIG_KEY = "ghost"
21+
)
22+
1923
// ConnectionConfig is the minimal configuration required to connect to a MySQL server
2024
type ConnectionConfig struct {
2125
Key InstanceKey
@@ -57,34 +61,41 @@ func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool {
5761
return this.Key.Equals(&other.Key) || this.ImpliedKey.Equals(other.ImpliedKey)
5862
}
5963

60-
func (this *ConnectionConfig) UseTLS(caCertificatePath string, allowInsecure bool) error {
64+
func (this *ConnectionConfig) UseTLS(caCertificatePath, clientCertificate, clientKey string, allowInsecure bool) error {
6165
var rootCertPool *x509.CertPool
66+
var certs []tls.Certificate
6267
var err error
6368

64-
if !allowInsecure {
65-
if caCertificatePath == "" {
66-
rootCertPool, err = x509.SystemCertPool()
67-
if err != nil {
68-
return err
69-
}
70-
} else {
71-
rootCertPool = x509.NewCertPool()
72-
pem, err := ioutil.ReadFile(caCertificatePath)
73-
if err != nil {
74-
return err
75-
}
76-
if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
77-
return errors.New("could not add ca certificate to cert pool")
78-
}
69+
if caCertificatePath == "" {
70+
rootCertPool, err = x509.SystemCertPool()
71+
if err != nil {
72+
return err
73+
}
74+
} else {
75+
rootCertPool = x509.NewCertPool()
76+
pem, err := ioutil.ReadFile(caCertificatePath)
77+
if err != nil {
78+
return err
79+
}
80+
if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
81+
return errors.New("could not add ca certificate to cert pool")
82+
}
83+
}
84+
if clientCertificate != "" || clientKey != "" {
85+
cert, err := tls.LoadX509KeyPair(clientCertificate, clientKey)
86+
if err != nil {
87+
return err
7988
}
89+
certs = []tls.Certificate{cert}
8090
}
8191

8292
this.tlsConfig = &tls.Config{
93+
Certificates: certs,
8394
RootCAs: rootCertPool,
8495
InsecureSkipVerify: allowInsecure,
8596
}
8697

87-
return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig)
98+
return mysql.RegisterTLSConfig(TLS_CONFIG_KEY, this.tlsConfig)
8899
}
89100

90101
func (this *ConnectionConfig) TLSConfig() *tls.Config {
@@ -103,7 +114,7 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string {
103114
// simplify construction of the DSN below.
104115
tlsOption := "false"
105116
if this.tlsConfig != nil {
106-
tlsOption = this.Key.StringCode()
117+
tlsOption = TLS_CONFIG_KEY
107118
}
108119
return fmt.Sprintf("%s:%s@tcp(%s:%d)/%s?interpolateParams=%t&autocommit=true&charset=utf8mb4,utf8,latin1&tls=%s", this.User, this.Password, hostname, this.Key.Port, databaseName, interpolateParams, tlsOption)
109120
}

go/mysql/connection_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,5 @@ func TestGetDBUriWithTLSSetup(t *testing.T) {
8080
c.tlsConfig = &tls.Config{}
8181

8282
uri := c.GetDBUri("test")
83-
test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=myhost:3306")
83+
test.S(t).ExpectEquals(uri, "gromit:penguin@tcp(myhost:3306)/test?interpolateParams=true&autocommit=true&charset=utf8mb4,utf8,latin1&tls=ghost")
8484
}

0 commit comments

Comments
 (0)