Skip to content

Commit

Permalink
Improve SSL support
Browse files Browse the repository at this point in the history
Related issue: github#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.
  • Loading branch information
Rafe Kettler committed Feb 22, 2019
1 parent a8fae98 commit b4233df
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 20 deletions.
8 changes: 8 additions & 0 deletions doc/command-line-flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,14 @@ Allows `gh-ost` to connect to the MySQL servers using encrypted connections, but

`--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`.

### ssl-cert

`--ssl-cert=/path/to/ssl-cert.crt`: SSL public key certificate file (in PEM format).

### ssl-key

`--ssl-key=/path/to/ssl-key.key`: SSL private key file (in PEM format).

### test-on-replica

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)
Expand Down
4 changes: 3 additions & 1 deletion go/base/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ type MigrationContext struct {
UseTLS bool
TLSAllowInsecure bool
TLSCACertificate string
TLSCertificate string
TLSKey string
CliMasterUser string
CliMasterPassword string

Expand Down Expand Up @@ -702,7 +704,7 @@ func (this *MigrationContext) ApplyCredentials() {

func (this *MigrationContext) SetupTLS() error {
if this.UseTLS {
return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSAllowInsecure)
return this.InspectorConnectionConfig.UseTLS(this.TLSCACertificate, this.TLSCertificate, this.TLSKey, this.TLSAllowInsecure)
}
return nil
}
Expand Down
8 changes: 8 additions & 0 deletions go/cmd/gh-ost/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ func main() {

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

flag.StringVar(&migrationContext.DatabaseName, "database", "", "database name (mandatory)")
Expand Down Expand Up @@ -204,6 +206,12 @@ func main() {
if migrationContext.TLSCACertificate != "" && !migrationContext.UseTLS {
log.Fatalf("--ssl-ca requires --ssl")
}
if migrationContext.TLSCertificate != "" && !migrationContext.UseTLS {
log.Fatalf("--ssl-crt requires --ssl")
}
if migrationContext.TLSKey != "" && !migrationContext.UseTLS {
log.Fatalf("--ssl-key requires --ssl")
}
if migrationContext.TLSAllowInsecure && !migrationContext.UseTLS {
log.Fatalf("--ssl-allow-insecure requires --ssl")
}
Expand Down
47 changes: 29 additions & 18 deletions go/mysql/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (
"github.com/go-sql-driver/mysql"
)

const (
TLS_CONFIG_KEY = "ghost"
)

// ConnectionConfig is the minimal configuration required to connect to a MySQL server
type ConnectionConfig struct {
Key InstanceKey
Expand Down Expand Up @@ -57,34 +61,41 @@ func (this *ConnectionConfig) Equals(other *ConnectionConfig) bool {
return this.Key.Equals(&other.Key) || this.ImpliedKey.Equals(other.ImpliedKey)
}

func (this *ConnectionConfig) UseTLS(caCertificatePath string, allowInsecure bool) error {
func (this *ConnectionConfig) UseTLS(caCertificatePath, clientCertificate, clientKey string, allowInsecure bool) error {
var rootCertPool *x509.CertPool
var certs []tls.Certificate
var err error

if !allowInsecure {
if caCertificatePath == "" {
rootCertPool, err = x509.SystemCertPool()
if err != nil {
return err
}
} else {
rootCertPool = x509.NewCertPool()
pem, err := ioutil.ReadFile(caCertificatePath)
if err != nil {
return err
}
if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
return errors.New("could not add ca certificate to cert pool")
}
if caCertificatePath == "" {
rootCertPool, err = x509.SystemCertPool()
if err != nil {
return err
}
} else {
rootCertPool = x509.NewCertPool()
pem, err := ioutil.ReadFile(caCertificatePath)
if err != nil {
return err
}
if ok := rootCertPool.AppendCertsFromPEM(pem); !ok {
return errors.New("could not add ca certificate to cert pool")
}
}
if clientCertificate != "" || clientKey != "" {
cert, err := tls.LoadX509KeyPair(clientCertificate, clientKey)
if err != nil {
return err
}
certs = []tls.Certificate{cert}
}

this.tlsConfig = &tls.Config{
Certificates: certs,
RootCAs: rootCertPool,
InsecureSkipVerify: allowInsecure,
}

return mysql.RegisterTLSConfig(this.Key.StringCode(), this.tlsConfig)
return mysql.RegisterTLSConfig(TLS_CONFIG_KEY, this.tlsConfig)
}

func (this *ConnectionConfig) TLSConfig() *tls.Config {
Expand All @@ -103,7 +114,7 @@ func (this *ConnectionConfig) GetDBUri(databaseName string) string {
// simplify construction of the DSN below.
tlsOption := "false"
if this.tlsConfig != nil {
tlsOption = this.Key.StringCode()
tlsOption = TLS_CONFIG_KEY
}
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)
}
2 changes: 1 addition & 1 deletion go/mysql/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,5 +80,5 @@ func TestGetDBUriWithTLSSetup(t *testing.T) {
c.tlsConfig = &tls.Config{}

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

0 comments on commit b4233df

Please sign in to comment.