Skip to content
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

Allow mTLS for mysql secrets engine #9181

Merged
merged 10 commits into from
Jun 17, 2020
Merged

Conversation

Valarissa
Copy link
Contributor

@Valarissa Valarissa commented Jun 10, 2020

This change enables the usage of mTLS with the mysql secrets engine.

It requires the following values to be passed to the MySQLConnectionProducer in order to enable mTLS:

	TLSCertificateKeyData []byte `json:"tls_certificate_key" mapstructure:"tls_certificate_key" structs:"-"`
	TLSCAData             []byte `json:"tls_ca"              mapstructure:"tls_ca"              structs:"-"`

These provide a combined Cert and Key PEM block and the CA PEM block of the CA that signed the server and client certs.

An automated test is provided in the code presented, it is skipped by default due to our CI not being able to create and mount files to a docker image.

Feel free to ask any questions to help review this.

Test output:

go test -run "clientTLS" -v ./plugins/database/mysql/
=== RUN   TestInit_clientTLS
[mysql] 2020/06/12 15:16:30 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:30 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:30 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:31 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:31 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:31 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:32 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:32 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:32 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:33 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:33 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:33 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:35 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:35 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:35 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:37 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:37 packets.go:36: unexpected EOF
[mysql] 2020/06/12 15:16:37 packets.go:36: unexpected EOF
--- PASS: TestInit_clientTLS (12.43s)
PASS
ok  	github.com/hashicorp/vault/plugins/database/mysql	12.722s

@Valarissa Valarissa requested a review from a team June 10, 2020 15:59
Copy link
Contributor

@pcman312 pcman312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall! There's a handful of small things. You should also run gofmt on the package as there are a few small formatting issues.

helper/testhelpers/certhelpers/cert_helpers.go Outdated Show resolved Hide resolved
helper/testhelpers/certhelpers/cert_helpers.go Outdated Show resolved Hide resolved
helper/testhelpers/certhelpers/cert_helpers.go Outdated Show resolved Hide resolved
helper/testhelpers/certhelpers/cert_helpers.go Outdated Show resolved Hide resolved
plugins/database/mysql/connection_producer.go Outdated Show resolved Hide resolved
plugins/database/mysql/connection_producer_test.go Outdated Show resolved Hide resolved
plugins/database/mysql/connection_producer_test.go Outdated Show resolved Hide resolved
plugins/database/mysql/connection_producer_test.go Outdated Show resolved Hide resolved

for _, opt := range opts {
err := opt(&builder)
if err != nil {
Copy link
Contributor

@catsby catsby Jun 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style nit: If we're only evaluating a local variable, it's common to see these statements on one line like so:

if err := opt(&builder); err != nil {
  // reference err here
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting style, noted. Used elsewhere, unsure if I changed this one.

@Valarissa Valarissa force-pushed the allow_mtls_for_mysql_secrets branch from c638808 to 5acc198 Compare June 16, 2020 22:15
@Valarissa Valarissa merged commit 601d0eb into master Jun 17, 2020
@pacificandreascc
Copy link

+func (c *mySQLConnectionProducer) Connection(ctx context.Context) (interface{}, error) { + if !c.Initialized { + return nil, connutil.ErrNotInitialized + } + + // If we already have a DB, test it and return + if c.db != nil { + if err := c.db.PingContext(ctx); err == nil { + return c.db, nil + } + // If the ping was unsuccessful, close it and ignore errors as we'll be + // reestablishing anyways + c.db.Close() + } + + connURL, err := c.finalizeConnectionURL()

@Valarissa Valarissa deleted the allow_mtls_for_mysql_secrets branch June 17, 2020 22:11
catsby added a commit that referenced this pull request Jun 19, 2020
* master: (31 commits)
  changelog++
  changelog++
  Ui/replication status discoverability (#8705)
  Update CHANGELOG.md
  Counter that increments on every secret engine lease creation. (#9244)
  Add password_policy field to Azure docs (#9249)
  Replaced ClusterMetricSink's cluster name with an atomic.Value. (#9252)
  Fix database creds rotation panic for nil resp (#9258)
  changelog++
  changelog++
  Move sdk/helper/random -> helper/random (#9226)
  UI: Disallow kv2 with too large 'max versions' value (#9242)
  Allow mTLS for mysql secrets engine (#9181)
  docs: add sample revocation for mongodb (#9245)
  Add new Telemetry config options (#9238)
  Add a simple sealed gauge, updated when seal status changes (#9177)
  Test Shamir-to-Transit and Transit-to-Shamir Seal Migration for post-1.4 Vault. (#9214)
  Configure metrics wrapper with the "global" object, not just the fanout. (#9099)
  changelog++
  Add backend type to audit logs (#9167)
  ...
andaley pushed a commit that referenced this pull request Jul 17, 2020
* Extract certificate helpers for use in non-mongodb packages
* Created mTLS/X509 test for MySQL secrets engine.
* Ensure mysql username and passwords aren't url encoded
* Skip mTLS test for circleCI
Valarissa pushed a commit that referenced this pull request Aug 17, 2020
* Update documentation for MySQL Secrets Engine

Update documentation for MySQL Database Secrets Engine to reflect changes introduced with #9181

* Empty Commit to re-trigger tests

Co-authored-by: Lauren Voswinkel <lvoswinkel@hashicorp.com>
Valarissa pushed a commit that referenced this pull request Aug 17, 2020
* Update documentation for MySQL Secrets Engine

Update documentation for MySQL Database Secrets Engine to reflect changes introduced with #9181

* Empty Commit to re-trigger tests

Co-authored-by: Lauren Voswinkel <lvoswinkel@hashicorp.com>
Valarissa pushed a commit that referenced this pull request Aug 18, 2020
* Update documentation for MySQL Secrets Engine

Update documentation for MySQL Database Secrets Engine to reflect changes introduced with #9181

* Empty Commit to re-trigger tests

Co-authored-by: Lauren Voswinkel <lvoswinkel@hashicorp.com>

Co-authored-by: arnis <8789226+0x63lv@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants