From d5e6eac5cecc1cb75b801cf327ac15a12940a11a Mon Sep 17 00:00:00 2001 From: Karol Kokoszka Date: Tue, 23 Apr 2024 19:42:27 +0200 Subject: [PATCH] fix(config-cache): replace map with explicit CQL and Alternator TLS config Fixes #3815 --- pkg/service/configcache/nodeconfig.go | 45 ++++++++++++++ pkg/service/configcache/service.go | 72 ++------------------- pkg/service/configcache/tlsconfig.go | 90 +++++++++++++++++++++++++++ pkg/service/healthcheck/service.go | 6 +- 4 files changed, 142 insertions(+), 71 deletions(-) create mode 100644 pkg/service/configcache/nodeconfig.go create mode 100644 pkg/service/configcache/tlsconfig.go diff --git a/pkg/service/configcache/nodeconfig.go b/pkg/service/configcache/nodeconfig.go new file mode 100644 index 000000000..fb641a16a --- /dev/null +++ b/pkg/service/configcache/nodeconfig.go @@ -0,0 +1,45 @@ +// Copyright (C) 2024 ScyllaDB + +package configcache + +import ( + "github.com/pkg/errors" + "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" + "github.com/scylladb/scylla-manager/v3/pkg/service/cluster" + "github.com/scylladb/scylla-manager/v3/pkg/store" +) + +// NodeConfig keeps the node current node configuration together with the TLS details per different type of connection. +type NodeConfig struct { + *scyllaclient.NodeInfo + + cqlTLSConfig *TLSConfigWithAddress + alternatorTLSConfig *TLSConfigWithAddress +} + +// NewNodeConfig creates and initializes new node configuration struct containing TLS configuration of CQL and Alternator. +func NewNodeConfig(c *cluster.Cluster, nodeInfo *scyllaclient.NodeInfo, secretsStore store.Store, host string) (config NodeConfig, err error) { + cqlTLS, err := newCQLTLSConfigIfEnabled(c, nodeInfo, secretsStore, host) + if err != nil { + return NodeConfig{}, errors.Wrap(err, "building node config") + } + alternatorTLS, err := newAlternatorTLSConfigIfEnabled(c, nodeInfo, secretsStore, host) + if err != nil { + return NodeConfig{}, errors.Wrap(err, "building node config") + } + return NodeConfig{ + NodeInfo: nodeInfo, + cqlTLSConfig: cqlTLS, + alternatorTLSConfig: alternatorTLS, + }, nil +} + +// CQLTLSConfig is a getter of TLS configuration for CQL session. +func (nc NodeConfig) CQLTLSConfig() *TLSConfigWithAddress { + return nc.cqlTLSConfig +} + +// AlternatorTLSConfig is a getter of TLS configuration for Alternator session. +func (nc NodeConfig) AlternatorTLSConfig() *TLSConfigWithAddress { + return nc.alternatorTLSConfig +} diff --git a/pkg/service/configcache/service.go b/pkg/service/configcache/service.go index 2bc204744..1de688834 100644 --- a/pkg/service/configcache/service.go +++ b/pkg/service/configcache/service.go @@ -4,15 +4,12 @@ package configcache import ( "context" - "crypto/tls" "sync" "time" "github.com/pkg/errors" "github.com/scylladb/go-log" "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" - "github.com/scylladb/scylla-manager/v3/pkg/secrets" - "github.com/scylladb/scylla-manager/v3/pkg/service" "github.com/scylladb/scylla-manager/v3/pkg/service/cluster" "github.com/scylladb/scylla-manager/v3/pkg/store" "github.com/scylladb/scylla-manager/v3/pkg/util/uuid" @@ -30,17 +27,6 @@ const ( updateFrequency = 5 * time.Minute ) -type tlsConfigWithAddress struct { - *tls.Config - Address string -} - -// NodeConfig keeps the node current node configuration together with the TLS details per different type of connection. -type NodeConfig struct { - *scyllaclient.NodeInfo - TLSConfig map[ConnectionType]*tlsConfigWithAddress -} - // ConfigCacher is the interface defining the cache behavior. type ConfigCacher interface { // Read returns either the host configuration that is currently stored in the cache, @@ -229,63 +215,13 @@ func (svc *Service) retrieveNodeConfig(ctx context.Context, host string, client nodeInfoResp, err := client.NodeInfo(ctx, host) if err != nil { - return config, errors.Wrap(err, "fetch node info") + return config, errors.Wrap(err, "retrieve cluster host configuration") } - config.NodeInfo = nodeInfoResp - config.TLSConfig = make(map[ConnectionType]*tlsConfigWithAddress, 2) - for _, p := range []ConnectionType{CQL, Alternator} { - var tlsEnabled, clientCertAuth bool - var address string - if p == CQL { - address = nodeInfoResp.CQLAddr(host) - tlsEnabled, clientCertAuth = nodeInfoResp.CQLTLSEnabled() - tlsEnabled = tlsEnabled && !c.ForceTLSDisabled - if tlsEnabled && !c.ForceNonSSLSessionPort { - address = nodeInfoResp.CQLSSLAddr(host) - } - } else if p == Alternator { - tlsEnabled, clientCertAuth = nodeInfoResp.AlternatorTLSEnabled() - address = nodeInfoResp.AlternatorAddr(host) - } - if tlsEnabled { - tlsConfig, err := svc.tlsConfig(c.ID, clientCertAuth) - if err != nil && !errors.Is(err, service.ErrNotFound) { - return config, errors.Wrap(err, "fetch TLS config") - } - if clientCertAuth && errors.Is(err, service.ErrNotFound) { - return config, errors.Wrap(err, "client encryption is enabled, but certificate is missing") - } - config.TLSConfig[p] = &tlsConfigWithAddress{ - Config: tlsConfig, - Address: address, - } - } else { - delete(config.TLSConfig, p) - } + config, err = NewNodeConfig(c, nodeInfoResp, svc.secretsStore, host) + if err != nil { + return config, errors.Wrap(err, "retrieve cluster host configuration") } return config, nil } - -func (svc *Service) tlsConfig(clusterID uuid.UUID, clientCertAuth bool) (*tls.Config, error) { - cfg := tls.Config{ - InsecureSkipVerify: true, - } - - if clientCertAuth { - id := &secrets.TLSIdentity{ - ClusterID: clusterID, - } - if err := svc.secretsStore.Get(id); err != nil { - return nil, errors.Wrap(err, "get SSL user cert from secrets store") - } - keyPair, err := tls.X509KeyPair(id.Cert, id.PrivateKey) - if err != nil { - return nil, errors.Wrap(err, "invalid SSL user key pair") - } - cfg.Certificates = []tls.Certificate{keyPair} - } - - return &cfg, nil -} diff --git a/pkg/service/configcache/tlsconfig.go b/pkg/service/configcache/tlsconfig.go new file mode 100644 index 000000000..6e229ac1f --- /dev/null +++ b/pkg/service/configcache/tlsconfig.go @@ -0,0 +1,90 @@ +// Copyright (C) 2024 ScyllaDB + +package configcache + +import ( + "crypto/tls" + + "github.com/pkg/errors" + "github.com/scylladb/scylla-manager/v3/pkg/scyllaclient" + "github.com/scylladb/scylla-manager/v3/pkg/secrets" + "github.com/scylladb/scylla-manager/v3/pkg/service" + "github.com/scylladb/scylla-manager/v3/pkg/service/cluster" + "github.com/scylladb/scylla-manager/v3/pkg/store" +) + +// TLSConfigWithAddress is a concatenation of tls.Config and Address. +type TLSConfigWithAddress struct { + *tls.Config + Address string +} + +func newCQLTLSConfigIfEnabled(c *cluster.Cluster, nodeInfo *scyllaclient.NodeInfo, secretsStore store.Store, + host string, +) (*TLSConfigWithAddress, error) { + cqlTLSEnabled, cqlClientCertAuth := nodeInfo.CQLTLSEnabled() + if !cqlTLSEnabled || c.ForceTLSDisabled { + return nil, nil // nolint: nilnil + } + cqlAddress := nodeInfo.CQLAddr(host) + if !c.ForceNonSSLSessionPort { + cqlAddress = nodeInfo.CQLSSLAddr(host) + } + tlsConfig := &tls.Config{ + InsecureSkipVerify: true, + } + if cqlClientCertAuth { + cert, err := prepareCertificates(c, secretsStore) + if err != nil { + return nil, errors.Wrap(err, "unable to create TLS configuration for CQL session") + } + tlsConfig.Certificates = []tls.Certificate{cert} + } + return &TLSConfigWithAddress{ + Address: cqlAddress, + Config: tlsConfig, + }, nil +} + +func newAlternatorTLSConfigIfEnabled(c *cluster.Cluster, nodeInfo *scyllaclient.NodeInfo, secretsStore store.Store, + host string, +) (*TLSConfigWithAddress, error) { + alternatorTLSEnabled, alternatorClientCertAuth := nodeInfo.AlternatorTLSEnabled() + if !alternatorTLSEnabled { + return nil, nil // nolint: nilnil + } + alternatorAddress := nodeInfo.AlternatorAddr(host) + + tlsConfig := &tls.Config{ + InsecureSkipVerify: true, + } + if alternatorClientCertAuth { + cert, err := prepareCertificates(c, secretsStore) + if err != nil { + return nil, errors.Wrap(err, "unable to create TLS configuration for Alternator session") + } + tlsConfig.Certificates = []tls.Certificate{cert} + } + return &TLSConfigWithAddress{ + Address: alternatorAddress, + Config: tlsConfig, + }, nil +} + +func prepareCertificates(c *cluster.Cluster, secretsStore store.Store) (cert tls.Certificate, err error) { + id := &secrets.TLSIdentity{ + ClusterID: c.ID, + } + if err := secretsStore.Get(id); err != nil { + if !errors.Is(err, service.ErrNotFound) { + return tls.Certificate{}, errors.Wrap(err, "fetch TLS config") + } + return tls.Certificate{}, errors.Wrap(err, "client encryption is enabled, but certificate is missing") + } + + keyPair, err := tls.X509KeyPair(id.Cert, id.PrivateKey) + if err != nil { + return tls.Certificate{}, errors.Wrap(err, "invalid SSL user key pair") + } + return keyPair, nil +} diff --git a/pkg/service/healthcheck/service.go b/pkg/service/healthcheck/service.go index ce3ccf1c6..2cbdf4907 100644 --- a/pkg/service/healthcheck/service.go +++ b/pkg/service/healthcheck/service.go @@ -219,7 +219,7 @@ func (s *Service) parallelCQLPingFunc(ctx context.Context, clusterID uuid.UUID, s.logger.Error(ctx, "Unable to fetch node information", "error", err) o.SSL = false } else { - o.SSL = ni.TLSConfig[configcache.CQL] != nil + o.SSL = ni.CQLTLSConfig() != nil } return nil @@ -294,7 +294,7 @@ func (s *Service) pingAlternator(ctx context.Context, clusterID uuid.UUID, host Timeout: timeout, } - tlsConfig := ni.TLSConfig[configcache.Alternator] + tlsConfig := ni.AlternatorTLSConfig() if tlsConfig != nil { config.TLSConfig = tlsConfig.Clone() } @@ -321,7 +321,7 @@ func (s *Service) pingCQL(ctx context.Context, clusterID uuid.UUID, host string, Timeout: timeout, } - tlsConfig := ni.TLSConfig[configcache.CQL] + tlsConfig := ni.CQLTLSConfig() if tlsConfig != nil { config.Addr = tlsConfig.Address config.TLSConfig = tlsConfig.Clone()