Skip to content

Commit

Permalink
Standarize TLS configuration
Browse files Browse the repository at this point in the history 
Signed-off-by: Jonah Back <jonah_back@intuit.com>
  • Loading branch information
@backjo
Jonah Back authored and backjo committed Jan 26, 2020
1 parent 4b58b94 commit bf32eb2
Show file tree
Hide file tree
Showing 13 changed files with 140 additions and 257 deletions.
8 changes: 4 additions & 4 deletions cmd/agent/app/reporter/grpc/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (
)

const (
gRPCPrefix = "reporter.grpc."
collectorHostPort = gRPCPrefix + "host-port"
retry = gRPCPrefix + "retry.max"
gRPCPrefix = "reporter.grpc"
collectorHostPort = gRPCPrefix + ".host-port"
retry = gRPCPrefix + ".retry.max"
defaultMaxRetry = 3
discoveryMinPeers = gRPCPrefix + "discovery.min-peers"
discoveryMinPeers = gRPCPrefix + ".discovery.min-peers"
)

var tlsFlagsConfig = tlscfg.ClientFlagsConfig{
Expand Down
2 changes: 1 addition & 1 deletion cmd/collector/app/builder/builder_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const (
)

var tlsFlagsConfig = tlscfg.ServerFlagsConfig{
Prefix: "collector.grpc.",
Prefix: "collector.grpc",
ShowEnabled: true,
ShowClientCA: true,
}
Expand Down
17 changes: 4 additions & 13 deletions pkg/cassandra/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/jaegertracing/jaeger/pkg/cassandra"
gocqlw "github.com/jaegertracing/jaeger/pkg/cassandra/gocql"
"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
)

// Configuration describes the configuration properties needed to connect to a Cassandra cluster
Expand All @@ -44,7 +45,7 @@ type Configuration struct {
Authenticator Authenticator `yaml:"authenticator"`
DisableAutoDiscovery bool `yaml:"disable_auto_discovery"`
EnableDependenciesV2 bool `yaml:"enable_dependencies_v2"`
TLS TLS
TLS tlscfg.Options
}

// Authenticator holds the authentication properties needed to connect to a Cassandra cluster
Expand All @@ -59,16 +60,6 @@ type BasicAuthenticator struct {
Password string `yaml:"password"`
}

// TLS Config
type TLS struct {
Enabled bool
ServerName string
CertPath string
KeyPath string
CaPath string
EnableHostVerification bool
}

// ApplyDefaults copies settings from source unless its own value is non-zero.
func (c *Configuration) ApplyDefaults(source *Configuration) {
if c.ConnectionsPerHost == 0 {
Expand Down Expand Up @@ -160,8 +151,8 @@ func (c *Configuration) NewCluster() *gocql.ClusterConfig {
},
CertPath: c.TLS.CertPath,
KeyPath: c.TLS.KeyPath,
CaPath: c.TLS.CaPath,
EnableHostVerification: c.TLS.EnableHostVerification,
CaPath: c.TLS.CAPath,
EnableHostVerification: !c.TLS.SkipHostVerify,
}
}
// If tunneling connection to C*, disable cluster autodiscovery features.
Expand Down
29 changes: 22 additions & 7 deletions pkg/config/tlscfg/flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ import (
)

const (
tlsEnabled = "tls"
tlsCA = "tls.ca"
tlsCert = "tls.cert"
tlsKey = "tls.key"
tlsServerName = "tls.server-name"
tlsClientCA = "tls.client-ca"
tlsClientCAOld = "tls.client.ca"
tlsPrefix = ".tls"
tlsEnabledOld = tlsPrefix
tlsEnabled = tlsPrefix + ".enabled"
tlsCA = tlsPrefix + ".ca"
tlsCert = tlsPrefix + ".cert"
tlsKey = tlsPrefix + ".key"
tlsServerName = tlsPrefix + ".server-name"
tlsClientCA = tlsPrefix + ".client-ca"
tlsClientCAOld = tlsPrefix + ".client.ca"
tlsSkipHostVerify = tlsPrefix + ".skip-host-verify"
)

// ClientFlagsConfig describes which CLI flags for TLS client should be generated.
Expand All @@ -48,19 +51,22 @@ type ServerFlagsConfig struct {
func (c ClientFlagsConfig) AddFlags(flags *flag.FlagSet) {
if c.ShowEnabled {
flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS when talking to the remote server(s)")
flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled)
}
flags.String(c.Prefix+tlsCA, "", "Path to a TLS CA (Certification Authority) file used to verify the remote server(s) (by default will use the system truststore)")
flags.String(c.Prefix+tlsCert, "", "Path to a TLS Certificate file, used to identify this process to the remote server(s)")
flags.String(c.Prefix+tlsKey, "", "Path to a TLS Private Key file, used to identify this process to the remote server(s)")
if c.ShowServerName {
flags.String(c.Prefix+tlsServerName, "", "Override the TLS server name we expect in the certificate of the remove server(s)")
}
flags.Bool(c.Prefix+tlsSkipHostVerify, false, "(insecure) Skip server's certificate chain and host name verification")
}

// AddFlags adds flags for TLS to the FlagSet.
func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) {
if c.ShowEnabled {
flags.Bool(c.Prefix+tlsEnabled, false, "Enable TLS on the server")
flags.Bool(c.Prefix+tlsEnabledOld, false, "(deprecated) see --"+c.Prefix+tlsEnabled)
}
flags.String(c.Prefix+tlsCert, "", "Path to a TLS Certificate file, used to identify this server to clients")
flags.String(c.Prefix+tlsKey, "", "Path to a TLS Private Key file, used to identify this server to clients")
Expand All @@ -73,13 +79,18 @@ func (c ClientFlagsConfig) InitFromViper(v *viper.Viper) Options {
var p Options
if c.ShowEnabled {
p.Enabled = v.GetBool(c.Prefix + tlsEnabled)

if !p.Enabled {
p.Enabled = v.GetBool(c.Prefix + tlsEnabledOld)
}
}
p.CAPath = v.GetString(c.Prefix + tlsCA)
p.CertPath = v.GetString(c.Prefix + tlsCert)
p.KeyPath = v.GetString(c.Prefix + tlsKey)
if c.ShowServerName {
p.ServerName = v.GetString(c.Prefix + tlsServerName)
}
p.SkipHostVerify = v.GetBool(c.Prefix + tlsSkipHostVerify)
return p
}

Expand All @@ -88,6 +99,10 @@ func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options {
var p Options
if c.ShowEnabled {
p.Enabled = v.GetBool(c.Prefix + tlsEnabled)

if !p.Enabled {
p.Enabled = v.GetBool(c.Prefix + tlsEnabledOld)
}
}
p.CertPath = v.GetString(c.Prefix + tlsCert)
p.KeyPath = v.GetString(c.Prefix + tlsKey)
Expand Down
60 changes: 38 additions & 22 deletions pkg/config/tlscfg/flags_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,35 +26,51 @@ import (

func TestClientFlags(t *testing.T) {
cmdLine := []string{
"--prefix.tls=true",
"--prefix.tls.ca=ca-file",
"--prefix.tls.cert=cert-file",
"--prefix.tls.key=key-file",
"--prefix.tls.server-name=HAL1",
"--prefix.tls.skip-host-verify=true",
}

v := viper.New()
command := cobra.Command{}
flagSet := &flag.FlagSet{}
flagCfg := ClientFlagsConfig{
Prefix: "prefix.",
ShowEnabled: true,
ShowServerName: true,
tests := []struct {
option string
}{
{
option: "--prefix.tls=true",
},
{
option: "--prefix.tls.enabled=true",
},
}
flagCfg.AddFlags(flagSet)
command.PersistentFlags().AddGoFlagSet(flagSet)
v.BindPFlags(command.PersistentFlags())

err := command.ParseFlags(cmdLine)
require.NoError(t, err)
tlsOpts := flagCfg.InitFromViper(v)
assert.Equal(t, Options{
Enabled: true,
CAPath: "ca-file",
CertPath: "cert-file",
KeyPath: "key-file",
ServerName: "HAL1",
}, tlsOpts)
for _, test := range tests {
t.Run(test.option, func(t *testing.T) {
v := viper.New()
command := cobra.Command{}
flagSet := &flag.FlagSet{}
flagCfg := ClientFlagsConfig{
Prefix: "prefix",
ShowEnabled: true,
ShowServerName: true,
}
flagCfg.AddFlags(flagSet)
command.PersistentFlags().AddGoFlagSet(flagSet)
v.BindPFlags(command.PersistentFlags())

err := command.ParseFlags(append(cmdLine, test.option))
require.NoError(t, err)
tlsOpts := flagCfg.InitFromViper(v)
assert.Equal(t, Options{
Enabled: true,
CAPath: "ca-file",
CertPath: "cert-file",
KeyPath: "key-file",
ServerName: "HAL1",
SkipHostVerify: true,
}, tlsOpts)
})
}
}

func TestServerFlags(t *testing.T) {
Expand Down Expand Up @@ -85,7 +101,7 @@ func TestServerFlags(t *testing.T) {
command := cobra.Command{}
flagSet := &flag.FlagSet{}
flagCfg := ServerFlagsConfig{
Prefix: "prefix.",
Prefix: "prefix",
ShowEnabled: true,
ShowClientCA: true,
}
Expand Down
15 changes: 9 additions & 6 deletions pkg/config/tlscfg/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ import (

// Options describes the configuration properties for TLS Connections.
type Options struct {
Enabled bool
CAPath string
CertPath string
KeyPath string
ServerName string // only for client-side TLS config
ClientCAPath string // only for server-side TLS config for client auth
Enabled bool
CAPath string
CertPath string
KeyPath string
ServerName string // only for client-side TLS config
ClientCAPath string // only for server-side TLS config for client auth
SkipHostVerify bool
}

var systemCertPool = x509.SystemCertPool // to allow overriding in unit test
Expand All @@ -46,6 +47,8 @@ func (p Options) Config() (*tls.Config, error) {
tlsCfg := &tls.Config{
RootCAs: certPool,
ServerName: p.ServerName,
// #nosec G402
InsecureSkipVerify: p.SkipHostVerify,
}

if (p.CertPath == "" && p.KeyPath != "") || (p.CertPath != "" && p.KeyPath == "") {
Expand Down
61 changes: 6 additions & 55 deletions pkg/es/config/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package config
import (
"context"
"crypto/tls"
"crypto/x509"
"io/ioutil"
"net/http"
"path/filepath"
Expand All @@ -32,6 +31,7 @@ import (
"github.com/uber/jaeger-lib/metrics"
"go.uber.org/zap"

"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
"github.com/jaegertracing/jaeger/pkg/es"
"github.com/jaegertracing/jaeger/pkg/es/wrapper"
"github.com/jaegertracing/jaeger/storage/spanstore"
Expand Down Expand Up @@ -60,21 +60,12 @@ type Configuration struct {
AllTagsAsFields bool
TagDotReplacement string
Enabled bool
TLS TLSConfig
TLS tlscfg.Options
UseReadWriteAliases bool
CreateIndexTemplates bool
Version uint
}

// TLSConfig describes the configuration properties to connect tls enabled ElasticSearch cluster
type TLSConfig struct {
Enabled bool
SkipHostVerify bool
CertPath string
KeyPath string
CaPath string
}

// ClientBuilder creates new es.Client
type ClientBuilder interface {
NewClient(logger *zap.Logger, metricsFactory metrics.Factory) (es.Client, error)
Expand Down Expand Up @@ -292,7 +283,7 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
}
options = append(options, elastic.SetHttpClient(httpClient))
if c.TLS.Enabled {
ctlsConfig, err := c.TLS.createTLSConfig()
ctlsConfig, err := c.TLS.Config()
if err != nil {
return nil, err
}
Expand All @@ -305,13 +296,12 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
// #nosec G402
TLSClientConfig: &tls.Config{InsecureSkipVerify: c.TLS.SkipHostVerify},
}
if c.TLS.CaPath != "" {
ctls := &TLSConfig{CaPath: c.TLS.CaPath}
ca, err := ctls.loadCertificate()
if c.TLS.CAPath != "" {
config, err := c.TLS.Config()
if err != nil {
return nil, err
}
httpTransport.TLSClientConfig.RootCAs = ca
httpTransport.TLSClientConfig = config
}

token := ""
Expand Down Expand Up @@ -340,45 +330,6 @@ func (c *Configuration) getConfigOptions(logger *zap.Logger) ([]elastic.ClientOp
return options, nil
}

// createTLSConfig creates TLS Configuration to connect with ES Cluster.
func (tlsConfig *TLSConfig) createTLSConfig() (*tls.Config, error) {
rootCerts, err := tlsConfig.loadCertificate()
if err != nil {
return nil, err
}
clientPrivateKey, err := tlsConfig.loadPrivateKey()
if err != nil {
return nil, err
}
// #nosec
return &tls.Config{
RootCAs: rootCerts,
Certificates: []tls.Certificate{*clientPrivateKey},
InsecureSkipVerify: tlsConfig.SkipHostVerify,
}, nil

}

// loadCertificate is used to load root certification
func (tlsConfig *TLSConfig) loadCertificate() (*x509.CertPool, error) {
caCert, err := ioutil.ReadFile(tlsConfig.CaPath)
if err != nil {
return nil, err
}
certificates := x509.NewCertPool()
certificates.AppendCertsFromPEM(caCert)
return certificates, nil
}

// loadPrivateKey is used to load the private certificate and key for TLS
func (tlsConfig *TLSConfig) loadPrivateKey() (*tls.Certificate, error) {
privateKey, err := tls.LoadX509KeyPair(tlsConfig.CertPath, tlsConfig.KeyPath)
if err != nil {
return nil, err
}
return &privateKey, nil
}

// TokenAuthTransport
type tokenAuthTransport struct {
token string
Expand Down
Loading

0 comments on commit bf32eb2

Please sign in to comment.