Add tls support #2
Conversation
- Adding a command line option for users to enforce tls/ssl connections for the applier, inspector, and binlog reader. - The user can optionally request server certificate verification through a command line option to specify the ca cert via a file path. - Fixes an existing bug appending the timeout option to the singleton applier connection.
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
|
RM +1 dependencies won't be scanned for this repo. this is a temporary fork/fix for tls, once the PR into the original fork is merged that will be used again. |
|
Don't merge this yet @brandonbodnar-wk because once #3 is merged the builds will pass on this. |
| if ok := rootCertPool.AppendCertsFromPEM(pem); !ok { | ||
| return errors.New("could not add ca certificate to cert pool") | ||
| } | ||
| } |
There was a problem hiding this comment.
@brandonbodnar-wk Can you tweak the logic here so that if it uses ssl, it verifies the cert by default?
There was a problem hiding this comment.
(Like how curl only skips cert verification if you pass a --insecure flag.)
There was a problem hiding this comment.
@matthewbalvanz-wf That is not the default behavior for mysql. This (calling --ssl in on the command line) attempts to emulate the behavior of --ssl-mode=REQUIRED in 5.7, which is the same as --enable-ssl for mysql 5.6. That setting only requires that the connection by encrypted with no guarantees about the certificate provided by the server is from a valid CA (self-signed certificates are okay).
Those (like us) who want additional guarantees that the cert is signed by a specific CA authority, can use the additional --ssl-ca flag to behave like --ssl-mode=VERIFY_CA from the mysql command line client.
There was a problem hiding this comment.
The caCertificatePath is set using that flag.
There was a problem hiding this comment.
Hmm I see what you're saying @brandonbodnar-wk , but it seems best to make that --ssl flag emulate VERIFY_CA instead, unless an --ssl-insecure flag is specifically set. The rest of the infosec team agrees that having ssl without verification by default is not a good idea. Here's a PR that achieves what I'm thinking.
Can you check that out and merge it if it looks good? Once that's merged then I'll +1 this PR and approve it in the RM console.
There was a problem hiding this comment.
Thanks @matthewbalvanz-wf. I've made a few tweaks to that PR and merged it here.
|
Security +1 |
|
RM +1 |
Upstream PR at github#705
Description
This PR adds initial support for enforcing TLS/SSL connections for the databases involved in the schema changes. In this initial pass, a gh-ost user can enforce ssl be used for connections using the
--sslflag on the command line. If the user wishes to enforce checking validity of the database's certificate, the user can also specify a CA certificate in pem format with the--ssl-caoption.Adding these two options is sufficient for our use case right now in connecting to AWS RDS instances. While there are plenty more options that can be specified by the native mysql client regarding ssl, this can provide a good starting point for other work by those with more complex needs.
Testing
We tested this updated code against an Aurora2 cluster running a MySQL 5.7 compatible engine.
gh-ost.--sslflag, along with specifying the--ssl-cacertificate for AWS RDS. The job executed flawlessly.