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

Validate hostnames when using TLS in Cassandra #11365

Merged
merged 8 commits into from
Apr 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions builtin/logical/cassandra/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ func TestBackend_basic(t *testing.T) {
t.Fatal(err)
}

cleanup, hostname := cassandra.PrepareTestContainer(t, "latest")
copyFromTo := map[string]string{
"test-fixtures/cassandra.yaml": "/etc/cassandra/cassandra.yaml",
}
host, cleanup := cassandra.PrepareTestContainer(t,
cassandra.CopyFromTo(copyFromTo),
)
defer cleanup()

logicaltest.Test(t, logicaltest.TestCase{
LogicalBackend: b,
Steps: []logicaltest.TestStep{
testAccStepConfig(t, hostname),
testAccStepConfig(t, host.ConnectionURL()),
testAccStepRole(t),
testAccStepReadCreds(t, "test"),
},
Expand All @@ -41,13 +46,17 @@ func TestBackend_roleCrud(t *testing.T) {
t.Fatal(err)
}

cleanup, hostname := cassandra.PrepareTestContainer(t, "latest")
copyFromTo := map[string]string{
"test-fixtures/cassandra.yaml": "/etc/cassandra/cassandra.yaml",
}
host, cleanup := cassandra.PrepareTestContainer(t,
cassandra.CopyFromTo(copyFromTo))
defer cleanup()

logicaltest.Test(t, logicaltest.TestCase{
LogicalBackend: b,
Steps: []logicaltest.TestStep{
testAccStepConfig(t, hostname),
testAccStepConfig(t, host.ConnectionURL()),
testAccStepRole(t),
testAccStepRoleWithOptions(t),
testAccStepReadRole(t, "test", testRole),
Expand Down
3 changes: 3 additions & 0 deletions changelog/11365.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
secrets/database/cassandra: Fixed issue where hostnames were not being validated when using TLS
```
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ require (
github.com/go-ole/go-ole v1.2.4 // indirect
github.com/go-sql-driver/mysql v1.5.0
github.com/go-test/deep v1.0.7
github.com/gocql/gocql v0.0.0-20200624222514-34081eda590e
github.com/gocql/gocql v0.0.0-20210401103645-80ab1e13e309
github.com/golang/protobuf v1.4.2
github.com/google/go-github v17.0.0+incompatible
github.com/google/go-metrics-stackdriver v0.2.0
Expand Down
6 changes: 4 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,8 @@ github.com/gobuffalo/packd v0.1.0/go.mod h1:M2Juc+hhDXf/PnmBANFCqx4DM3wRbgDvnVWe
github.com/gobuffalo/packr/v2 v2.0.9/go.mod h1:emmyGweYTm6Kdper+iywB6YK5YzuKchGtJQZ0Odn4pQ=
github.com/gobuffalo/packr/v2 v2.2.0/go.mod h1:CaAwI0GPIAv+5wKLtv8Afwl+Cm78K/I/VCm/3ptBN+0=
github.com/gobuffalo/syncx v0.0.0-20190224160051-33c29581e754/go.mod h1:HhnNqWY95UYwwW3uSASeV7vtgYkT2t16hJgV3AEPUpw=
github.com/gocql/gocql v0.0.0-20200624222514-34081eda590e h1:SroDcndcOU9BVAduPf/PXihXoR2ZYTQYLXbupbqxAyQ=
github.com/gocql/gocql v0.0.0-20200624222514-34081eda590e/go.mod h1:DL0ekTmBSTdlNF25Orwt/JMzqIq3EJ4MVa/J/uK64OY=
github.com/gocql/gocql v0.0.0-20210401103645-80ab1e13e309 h1:8MHuCGYDXh0skFrLumkCMlt9C29hxhqNx39+Haemeqw=
github.com/gocql/gocql v0.0.0-20210401103645-80ab1e13e309/go.mod h1:DL0ekTmBSTdlNF25Orwt/JMzqIq3EJ4MVa/J/uK64OY=
github.com/godbus/dbus v0.0.0-20190422162347-ade71ed3457e/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4=
github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
github.com/gogo/googleapis v1.1.0/go.mod h1:gf4bu3Q80BeJ6H1S1vYPm8/ELATdvryBaNFGgqEef3s=
Expand Down Expand Up @@ -1084,6 +1084,7 @@ github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6So
github.com/rogpeppe/go-internal v1.1.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.2.2/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
github.com/rogpeppe/go-internal v1.6.2 h1:aIihoIOHCiLZHxyoNQ+ABL4NKhFTgKLBdMLyEAh98m0=
github.com/rogpeppe/go-internal v1.6.2/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc=
github.com/rs/zerolog v1.4.0/go.mod h1:YbFCdg8HfsridGWAh22vktObvhZbQsZXe4/zB0OKkWU=
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
Expand Down Expand Up @@ -1631,6 +1632,7 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f h1:BLraFXnmrev5lT+xlilqcH8XK9/i0At2xKjWk4p6zsU=
gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/cheggaaa/pb.v1 v1.0.25/go.mod h1:V/YB90LKu/1FcN3WVnfiiE5oMCibMjukxqG/qStrOgw=
gopkg.in/errgo.v2 v2.1.0 h1:0vLT13EuvQ0hNvakwLuFZ/jYrLp5F3kcWHXdRggjCE8=
gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI=
gopkg.in/fsnotify.v1 v1.4.7 h1:xOHLXZwVvI9hhs+cLKq5+I5onOuwQLhQwiu63xxlHs4=
gopkg.in/fsnotify.v1 v1.4.7/go.mod h1:Tz8NjZHkW78fSQdbUxIjBTcgA1z1m8ZHf0WmKUhAMys=
Expand Down
98 changes: 76 additions & 22 deletions helper/testhelpers/cassandra/cassandrahelper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,86 @@ package cassandra

import (
"context"
"errors"
"fmt"
"net"
"os"
"path/filepath"
"testing"
"time"

"github.com/gocql/gocql"
"github.com/hashicorp/vault/helper/testhelpers/docker"
)

func PrepareTestContainer(t *testing.T, version string) (func(), string) {
type containerConfig struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! I figured since I needed to add another optional field that it would be better to switch to the functional arguments pattern for ease of use downstream.

version string
copyFromTo map[string]string
sslOpts *gocql.SslOptions
}

type ContainerOpt func(*containerConfig)

func Version(version string) ContainerOpt {
return func(cfg *containerConfig) {
cfg.version = version
}
}

func CopyFromTo(copyFromTo map[string]string) ContainerOpt {
return func(cfg *containerConfig) {
cfg.copyFromTo = copyFromTo
}
}

func SslOpts(sslOpts *gocql.SslOptions) ContainerOpt {
return func(cfg *containerConfig) {
cfg.sslOpts = sslOpts
}
}

type Host struct {
Name string
Port string
}

func (h Host) ConnectionURL() string {
return net.JoinHostPort(h.Name, h.Port)
}

func PrepareTestContainer(t *testing.T, opts ...ContainerOpt) (Host, func()) {
t.Helper()
if os.Getenv("CASSANDRA_HOSTS") != "" {
return func() {}, os.Getenv("CASSANDRA_HOSTS")
host, port, err := net.SplitHostPort(os.Getenv("CASSANDRA_HOSTS"))
if err != nil {
t.Fatalf("Failed to split host & port from CASSANDRA_HOSTS (%s): %s", os.Getenv("CASSANDRA_HOSTS"), err)
}
h := Host{
Name: host,
Port: port,
}
return h, func() {}
}

if version == "" {
version = "3.11"
containerCfg := &containerConfig{
version: "3.11",
}

var copyFromTo map[string]string
cwd, _ := os.Getwd()
fixturePath := fmt.Sprintf("%s/test-fixtures/", cwd)
if _, err := os.Stat(fixturePath); err != nil {
if !errors.Is(err, os.ErrNotExist) {
// If it doesn't exist, no biggie
t.Fatal(err)
}
} else {
copyFromTo = map[string]string{
fixturePath: "/etc/cassandra",
for _, opt := range opts {
opt(containerCfg)
}

copyFromTo := map[string]string{}
for from, to := range containerCfg.copyFromTo {
absFrom, err := filepath.Abs(from)
if err != nil {
t.Fatalf("Unable to get absolute path for file %s", from)
}
copyFromTo[absFrom] = to
}

runner, err := docker.NewServiceRunner(docker.RunOptions{
ImageRepo: "cassandra",
ImageTag: version,
ImageTag: containerCfg.version,
Ports: []string{"9042/tcp"},
CopyFromTo: copyFromTo,
Env: []string{"CASSANDRA_BROADCAST_ADDRESS=127.0.0.1"},
Expand All @@ -58,32 +101,43 @@ func PrepareTestContainer(t *testing.T, version string) (func(), string) {
clusterConfig.ProtoVersion = 4
clusterConfig.Port = port

clusterConfig.SslOpts = containerCfg.sslOpts

session, err := clusterConfig.CreateSession()
if err != nil {
return nil, fmt.Errorf("error creating session: %s", err)
}
defer session.Close()

// Create keyspace
q := session.Query(`CREATE KEYSPACE "vault" WITH REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };`)
if err := q.Exec(); err != nil {
query := session.Query(`CREATE KEYSPACE "vault" WITH REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };`)
if err := query.Exec(); err != nil {
t.Fatalf("could not create cassandra keyspace: %v", err)
}

// Create table
q = session.Query(`CREATE TABLE "vault"."entries" (
query = session.Query(`CREATE TABLE "vault"."entries" (
bucket text,
key text,
value blob,
PRIMARY KEY (bucket, key)
) WITH CLUSTERING ORDER BY (key ASC);`)
if err := q.Exec(); err != nil {
if err := query.Exec(); err != nil {
t.Fatalf("could not create cassandra table: %v", err)
}
return cfg, nil
})
if err != nil {
t.Fatalf("Could not start docker cassandra: %s", err)
}
return svc.Cleanup, svc.Config.Address()

host, port, err := net.SplitHostPort(svc.Config.Address())
if err != nil {
t.Fatalf("Failed to split host & port from address (%s): %s", svc.Config.Address(), err)
}
h := Host{
Name: host,
Port: port,
}
return h, svc.Cleanup
}
34 changes: 16 additions & 18 deletions physical/cassandra/cassandra.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@ import (
"strings"
"time"

"github.com/hashicorp/errwrap"
log "github.com/hashicorp/go-hclog"

metrics "github.com/armon/go-metrics"
"github.com/gocql/gocql"
"github.com/hashicorp/errwrap"
log "github.com/hashicorp/go-hclog"
"github.com/hashicorp/vault/sdk/helper/certutil"
"github.com/hashicorp/vault/sdk/physical"
)
Expand Down Expand Up @@ -180,20 +179,18 @@ func setupCassandraTLS(conf map[string]string, cluster *gocql.ClusterConfig) err
if err != nil {
return err
}
} else {
if pemJSONPath, ok := conf["pem_json_file"]; ok {
pemJSONData, err := ioutil.ReadFile(pemJSONPath)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error reading json bundle from %q: {{err}}", pemJSONPath), err)
}
pemJSON, err := certutil.ParsePKIJSON([]byte(pemJSONData))
if err != nil {
return err
}
tlsConfig, err = pemJSON.GetTLSConfig(certutil.TLSClient)
if err != nil {
return err
}
} else if pemJSONPath, ok := conf["pem_json_file"]; ok {
pemJSONData, err := ioutil.ReadFile(pemJSONPath)
if err != nil {
return errwrap.Wrapf(fmt.Sprintf("error reading json bundle from %q: {{err}}", pemJSONPath), err)
}
pemJSON, err := certutil.ParsePKIJSON([]byte(pemJSONData))
if err != nil {
return err
}
tlsConfig, err = pemJSON.GetTLSConfig(certutil.TLSClient)
if err != nil {
return err
}
}

Expand Down Expand Up @@ -225,7 +222,8 @@ func setupCassandraTLS(conf map[string]string, cluster *gocql.ClusterConfig) err
}

cluster.SslOpts = &gocql.SslOptions{
Config: tlsConfig.Clone(),
Config: tlsConfig,
EnableHostVerification: !tlsConfig.InsecureSkipVerify,
}
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions physical/cassandra/cassandra_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ func TestCassandraBackend(t *testing.T) {
t.Skip("skipping race test in CI pending https://github.com/gocql/gocql/pull/1474")
}

cleanup, hosts := cassandra.PrepareTestContainer(t, "")
host, cleanup := cassandra.PrepareTestContainer(t)
defer cleanup()

// Run vault tests
logger := logging.NewVaultLogger(log.Debug)
b, err := NewCassandraBackend(map[string]string{
"hosts": hosts,
"hosts": host.ConnectionURL(),
"protocol_version": "3",
}, logger)
if err != nil {
Expand Down
Loading