config: remove duplicate TLSConfig fields from agent/consul.Config

tlsutil.Config already presents an excellent structure for this
configuration. Copying the runtime config fields to agent/consul.Config
makes code harder to trace, and provides no advantage.

Instead of copying the fields around, use the tlsutil.Config struct
directly instead.

This is one small step in removing the many layers of duplicate
configuration.
This commit is contained in:
Daniel Nephin 2021-07-09 18:17:42 -04:00
parent 5ff191ad99
commit 3c60a46376
9 changed files with 101 additions and 181 deletions

View File

@ -1198,22 +1198,12 @@ func newConsulConfig(runtimeCfg *config.RuntimeConfig, logger hclog.Logger) (*co
}
cfg.Build = fmt.Sprintf("%s%s:%s", runtimeCfg.Version, runtimeCfg.VersionPrerelease, revision)
cfg.TLSConfig = runtimeCfg.ToTLSUtilConfig()
// Copy the TLS configuration
cfg.VerifyIncoming = runtimeCfg.VerifyIncoming || runtimeCfg.VerifyIncomingRPC
if runtimeCfg.CAPath != "" || runtimeCfg.CAFile != "" {
cfg.UseTLS = true
}
cfg.VerifyOutgoing = runtimeCfg.VerifyOutgoing
cfg.VerifyServerHostname = runtimeCfg.VerifyServerHostname
cfg.CAFile = runtimeCfg.CAFile
cfg.CAPath = runtimeCfg.CAPath
cfg.CertFile = runtimeCfg.CertFile
cfg.KeyFile = runtimeCfg.KeyFile
cfg.ServerName = runtimeCfg.ServerName
cfg.Domain = runtimeCfg.DNSDomain
cfg.TLSMinVersion = runtimeCfg.TLSMinVersion
cfg.TLSCipherSuites = runtimeCfg.TLSCipherSuites
cfg.TLSPreferServerCipherSuites = runtimeCfg.TLSPreferServerCipherSuites
cfg.DefaultQueryTime = runtimeCfg.DefaultQueryTime
cfg.MaxQueryTime = runtimeCfg.MaxQueryTime

View File

@ -133,7 +133,7 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
altCSR, _ := connect.TestCSR(t, &altCSRID)
_, s, _ := testACLServerWithConfig(t, func(c *Config) {
c.Domain = "consul"
c.TLSConfig.Domain = "consul"
c.AutoConfigAuthzEnabled = true
c.AutoConfigAuthzAuthMethod = structs.ACLAuthMethod{
Name: "Auth Config Authorizer",
@ -165,14 +165,14 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
err = ioutil.WriteFile(keyfile, []byte(key), 0600)
require.NoError(t, err)
c.CAFile = cafile
c.CertFile = certfile
c.KeyFile = keyfile
c.VerifyOutgoing = true
c.VerifyIncoming = true
c.VerifyServerHostname = true
c.TLSMinVersion = "tls12"
c.TLSPreferServerCipherSuites = true
c.TLSConfig.CAFile = cafile
c.TLSConfig.CertFile = certfile
c.TLSConfig.KeyFile = keyfile
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyIncoming = true
c.TLSConfig.VerifyServerHostname = true
c.TLSConfig.TLSMinVersion = "tls12"
c.TLSConfig.PreferServerCipherSuites = true
c.ConnectEnabled = true
c.AutoEncryptAllowTLS = true
@ -184,11 +184,12 @@ func TestAutoConfigInitialConfiguration(t *testing.T) {
c.SerfLANConfig.MemberlistConfig.Keyring = keyring
}, false)
// TODO: use s.config.TLSConfig directly instead of creating a new one?
conf := tlsutil.Config{
CAFile: s.config.CAFile,
VerifyServerHostname: s.config.VerifyServerHostname,
VerifyOutgoing: s.config.VerifyOutgoing,
Domain: s.config.Domain,
CAFile: s.config.TLSConfig.CAFile,
VerifyServerHostname: s.config.TLSConfig.VerifyServerHostname,
VerifyOutgoing: s.config.TLSConfig.VerifyOutgoing,
Domain: s.config.TLSConfig.Domain,
}
codec, err := insecureRPCClient(s, conf)
require.NoError(t, err)

View File

@ -58,10 +58,10 @@ func TestAutoEncryptSign(t *testing.T) {
dir, s := testServerWithConfig(t, func(c *Config) {
c.AutoEncryptAllowTLS = true
c.Bootstrap = true
c.CAFile = root
c.VerifyOutgoing = true
c.CertFile = cert
c.KeyFile = key
c.TLSConfig.CAFile = root
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.CertFile = cert
c.TLSConfig.KeyFile = key
})
defer os.RemoveAll(dir)
defer s.Shutdown()

View File

@ -437,8 +437,8 @@ func TestClient_RPC_ConsulServerPing(t *testing.T) {
func TestClient_RPC_TLS(t *testing.T) {
t.Parallel()
_, conf1 := testServerConfig(t)
conf1.VerifyIncoming = true
conf1.VerifyOutgoing = true
conf1.TLSConfig.VerifyIncoming = true
conf1.TLSConfig.VerifyOutgoing = true
configureTLS(conf1)
s1, err := newServer(t, conf1)
if err != nil {
@ -447,7 +447,7 @@ func TestClient_RPC_TLS(t *testing.T) {
defer s1.Shutdown()
_, conf2 := testClientConfig(t)
conf2.VerifyOutgoing = true
conf2.TLSConfig.VerifyOutgoing = true
configureTLS(conf2)
c1 := newClient(t, conf2)
@ -494,7 +494,7 @@ func newDefaultDeps(t *testing.T, c *Config) Deps {
Output: testutil.NewLogBuffer(t),
})
tls, err := tlsutil.NewConfigurator(c.ToTLSUtilConfig(), logger)
tls, err := tlsutil.NewConfigurator(c.TLSConfig, logger)
require.NoError(t, err, "failed to create tls configuration")
r := router.NewRouter(logger, c.Datacenter, fmt.Sprintf("%s.%s", c.NodeName, c.Datacenter), nil)
@ -633,8 +633,8 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) {
t.Parallel()
_, conf1 := testServerConfig(t)
conf1.VerifyIncoming = true
conf1.VerifyOutgoing = true
conf1.TLSConfig.VerifyIncoming = true
conf1.TLSConfig.VerifyOutgoing = true
configureTLS(conf1)
s1, err := newServer(t, conf1)
if err != nil {
@ -643,7 +643,7 @@ func TestClient_SnapshotRPC_TLS(t *testing.T) {
defer s1.Shutdown()
_, conf2 := testClientConfig(t)
conf2.VerifyOutgoing = true
conf2.TLSConfig.VerifyOutgoing = true
configureTLS(conf2)
c1 := newClient(t, conf2)

View File

@ -104,9 +104,6 @@ type Config struct {
// Node name is the name we use to advertise. Defaults to hostname.
NodeName string
// Domain is the DNS domain for the records. Defaults to "consul."
Domain string
// RaftConfig is the configuration used for Raft in the local DC
RaftConfig *raft.Config
@ -161,58 +158,12 @@ type Config struct {
// ProtocolVersionMin and ProtocolVersionMax.
ProtocolVersion uint8
// VerifyIncoming is used to verify the authenticity of incoming connections.
// This means that TCP requests are forbidden, only allowing for TLS. TLS connections
// must match a provided certificate authority. This can be used to force client auth.
VerifyIncoming bool
// VerifyOutgoing is used to force verification of the authenticity of outgoing connections.
// This means that TLS requests are used, and TCP requests are not made. TLS connections
// must match a provided certificate authority.
VerifyOutgoing bool
TLSConfig tlsutil.Config
// UseTLS is used to enable TLS for outgoing connections to other TLS-capable Consul
// servers. This doesn't imply any verification, it only enables TLS if possible.
UseTLS bool
// VerifyServerHostname is used to enable hostname verification of servers. This
// ensures that the certificate presented is valid for server.<datacenter>.<domain>.
// This prevents a compromised client from being restarted as a server, and then
// intercepting request traffic as well as being added as a raft peer. This should be
// enabled by default with VerifyOutgoing, but for legacy reasons we cannot break
// existing clients.
VerifyServerHostname bool
// CAFile is a path to a certificate authority file. This is used with VerifyIncoming
// or VerifyOutgoing to verify the TLS connection.
CAFile string
// CAPath is a path to a directory of certificate authority files. This is used with
// VerifyIncoming or VerifyOutgoing to verify the TLS connection.
CAPath string
// CertFile is used to provide a TLS certificate that is used for serving TLS connections.
// Must be provided to serve TLS connections.
CertFile string
// KeyFile is used to provide a TLS key that is used for serving TLS connections.
// Must be provided to serve TLS connections.
KeyFile string
// ServerName is used with the TLS certificate to ensure the name we
// provide matches the certificate
ServerName string
// TLSMinVersion is used to set the minimum TLS version used for TLS connections.
TLSMinVersion string
// TLSCipherSuites is used to specify the list of supported ciphersuites.
TLSCipherSuites []uint16
// TLSPreferServerCipherSuites specifies whether to prefer the server's ciphersuite
// over the client ciphersuites.
TLSPreferServerCipherSuites bool
// RejoinAfterLeave controls our interaction with Serf.
// When set to false (default), a leave causes a Consul to not rejoin
// the cluster until an explicit join is received. If this is set to
@ -483,26 +434,6 @@ type Config struct {
*EnterpriseConfig
}
// ToTLSUtilConfig is only used by tests, usually the config is being passed
// down from the agent.
func (c *Config) ToTLSUtilConfig() tlsutil.Config {
return tlsutil.Config{
VerifyIncoming: c.VerifyIncoming,
VerifyOutgoing: c.VerifyOutgoing,
VerifyServerHostname: c.VerifyServerHostname,
CAFile: c.CAFile,
CAPath: c.CAPath,
CertFile: c.CertFile,
KeyFile: c.KeyFile,
NodeName: c.NodeName,
Domain: c.Domain,
ServerName: c.ServerName,
TLSMinVersion: c.TLSMinVersion,
CipherSuites: c.TLSCipherSuites,
PreferServerCipherSuites: c.TLSPreferServerCipherSuites,
}
}
// CheckProtocolVersion validates the protocol version.
func (c *Config) CheckProtocolVersion() error {
if c.ProtocolVersion < ProtocolVersionMin {
@ -582,8 +513,6 @@ func DefaultConfig() *Config {
RPCRateLimit: rate.Inf,
RPCMaxBurst: 1000,
TLSMinVersion: "tls10",
// TODO (slackpad) - Until #3744 is done, we need to keep these
// in sync with agent/config/default.go.
AutopilotConfig: &structs.AutopilotConfig{

View File

@ -443,12 +443,12 @@ func TestRPC_TLSHandshakeTimeout(t *testing.T) {
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.RPCHandshakeTimeout = 10 * time.Millisecond
c.UseTLS = true
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Alice.crt"
c.KeyFile = "../../test/hostname/Alice.key"
c.VerifyServerHostname = true
c.VerifyOutgoing = true
c.VerifyIncoming = true
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Alice.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Alice.key"
c.TLSConfig.VerifyServerHostname = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyIncoming = true
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
@ -540,13 +540,13 @@ func TestRPC_PreventsTLSNesting(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.UseTLS = true
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Alice.crt"
c.KeyFile = "../../test/hostname/Alice.key"
c.VerifyServerHostname = true
c.VerifyOutgoing = true
c.VerifyIncoming = false // saves us getting client cert setup
c.Domain = "consul"
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Alice.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Alice.key"
c.TLSConfig.VerifyServerHostname = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyIncoming = false // saves us getting client cert setup
c.TLSConfig.Domain = "consul"
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
@ -696,13 +696,13 @@ func TestRPC_RPCMaxConnsPerClient(t *testing.T) {
c.RPCMaxConnsPerClient = 2
if tc.tlsEnabled {
c.UseTLS = true
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Alice.crt"
c.KeyFile = "../../test/hostname/Alice.key"
c.VerifyServerHostname = true
c.VerifyOutgoing = true
c.VerifyIncoming = false // saves us getting client cert setup
c.Domain = "consul"
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Alice.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Alice.key"
c.TLSConfig.VerifyServerHostname = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyIncoming = false // saves us getting client cert setup
c.TLSConfig.Domain = "consul"
}
})
defer os.RemoveAll(dir1)

View File

@ -327,8 +327,8 @@ func NewServer(config *Config, flat Deps) (*Server, error) {
return nil, err
}
// Check if TLS is enabled
if config.CAFile != "" || config.CAPath != "" {
// TODO: this is duplicated in newConsulConfig, do it in only on place
if config.TLSConfig.CAFile != "" || config.TLSConfig.CAPath != "" {
config.UseTLS = true
}

View File

@ -87,9 +87,9 @@ func testServerACLConfig(cb func(*Config)) func(*Config) {
}
func configureTLS(config *Config) {
config.CAFile = "../../test/ca/root.cer"
config.CertFile = "../../test/key/ourdomain.cer"
config.KeyFile = "../../test/key/ourdomain.key"
config.TLSConfig.CAFile = "../../test/ca/root.cer"
config.TLSConfig.CertFile = "../../test/key/ourdomain.cer"
config.TLSConfig.KeyFile = "../../test/key/ourdomain.key"
}
var id int64
@ -643,18 +643,18 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) {
gwAddr := ipaddr.FormatAddressPort("127.0.0.1", gwPort[0])
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.Domain = "consul"
c.TLSConfig.Domain = "consul"
c.NodeName = "bob"
c.Datacenter = "dc1"
c.PrimaryDatacenter = "dc1"
c.Bootstrap = true
// tls
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Bob.crt"
c.KeyFile = "../../test/hostname/Bob.key"
c.VerifyIncoming = true
c.VerifyOutgoing = true
c.VerifyServerHostname = true
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Bob.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Bob.key"
c.TLSConfig.VerifyIncoming = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyServerHostname = true
// wanfed
c.ConnectMeshGatewayWANFederationEnabled = true
})
@ -662,18 +662,18 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) {
defer s1.Shutdown()
dir2, s2 := testServerWithConfig(t, func(c *Config) {
c.Domain = "consul"
c.TLSConfig.Domain = "consul"
c.NodeName = "betty"
c.Datacenter = "dc2"
c.PrimaryDatacenter = "dc1"
c.Bootstrap = true
// tls
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Betty.crt"
c.KeyFile = "../../test/hostname/Betty.key"
c.VerifyIncoming = true
c.VerifyOutgoing = true
c.VerifyServerHostname = true
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Betty.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Betty.key"
c.TLSConfig.VerifyIncoming = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyServerHostname = true
// wanfed
c.ConnectMeshGatewayWANFederationEnabled = true
})
@ -681,18 +681,18 @@ func TestServer_JoinWAN_viaMeshGateway(t *testing.T) {
defer s2.Shutdown()
dir3, s3 := testServerWithConfig(t, func(c *Config) {
c.Domain = "consul"
c.TLSConfig.Domain = "consul"
c.NodeName = "bonnie"
c.Datacenter = "dc3"
c.PrimaryDatacenter = "dc1"
c.Bootstrap = true
// tls
c.CAFile = "../../test/hostname/CertAuth.crt"
c.CertFile = "../../test/hostname/Bonnie.crt"
c.KeyFile = "../../test/hostname/Bonnie.key"
c.VerifyIncoming = true
c.VerifyOutgoing = true
c.VerifyServerHostname = true
c.TLSConfig.CAFile = "../../test/hostname/CertAuth.crt"
c.TLSConfig.CertFile = "../../test/hostname/Bonnie.crt"
c.TLSConfig.KeyFile = "../../test/hostname/Bonnie.key"
c.TLSConfig.VerifyIncoming = true
c.TLSConfig.VerifyOutgoing = true
c.TLSConfig.VerifyServerHostname = true
// wanfed
c.ConnectMeshGatewayWANFederationEnabled = true
})
@ -1076,8 +1076,8 @@ func TestServer_JoinLAN_TLS(t *testing.T) {
t.Parallel()
_, conf1 := testServerConfig(t)
conf1.VerifyIncoming = true
conf1.VerifyOutgoing = true
conf1.TLSConfig.VerifyIncoming = true
conf1.TLSConfig.VerifyOutgoing = true
configureTLS(conf1)
s1, err := newServer(t, conf1)
if err != nil {
@ -1088,8 +1088,8 @@ func TestServer_JoinLAN_TLS(t *testing.T) {
_, conf2 := testServerConfig(t)
conf2.Bootstrap = false
conf2.VerifyIncoming = true
conf2.VerifyOutgoing = true
conf2.TLSConfig.VerifyIncoming = true
conf2.TLSConfig.VerifyOutgoing = true
configureTLS(conf2)
s2, err := newServer(t, conf2)
if err != nil {
@ -1346,9 +1346,9 @@ func TestServer_TLSToNoTLS(t *testing.T) {
// Add a second server with TLS configured
dir2, s2 := testServerWithConfig(t, func(c *Config) {
c.Bootstrap = false
c.CAFile = "../../test/client_certs/rootca.crt"
c.CertFile = "../../test/client_certs/server.crt"
c.KeyFile = "../../test/client_certs/server.key"
c.TLSConfig.CAFile = "../../test/client_certs/rootca.crt"
c.TLSConfig.CertFile = "../../test/client_certs/server.crt"
c.TLSConfig.KeyFile = "../../test/client_certs/server.key"
})
defer os.RemoveAll(dir2)
defer s2.Shutdown()
@ -1378,10 +1378,10 @@ func TestServer_TLSForceOutgoingToNoTLS(t *testing.T) {
// Add a second server with TLS and VerifyOutgoing set
dir2, s2 := testServerWithConfig(t, func(c *Config) {
c.Bootstrap = false
c.CAFile = "../../test/client_certs/rootca.crt"
c.CertFile = "../../test/client_certs/server.crt"
c.KeyFile = "../../test/client_certs/server.key"
c.VerifyOutgoing = true
c.TLSConfig.CAFile = "../../test/client_certs/rootca.crt"
c.TLSConfig.CertFile = "../../test/client_certs/server.crt"
c.TLSConfig.KeyFile = "../../test/client_certs/server.key"
c.TLSConfig.VerifyOutgoing = true
})
defer os.RemoveAll(dir2)
defer s2.Shutdown()
@ -1400,10 +1400,10 @@ func TestServer_TLSToFullVerify(t *testing.T) {
t.Parallel()
// Set up a server with TLS and VerifyIncoming set
dir1, s1 := testServerWithConfig(t, func(c *Config) {
c.CAFile = "../../test/client_certs/rootca.crt"
c.CertFile = "../../test/client_certs/server.crt"
c.KeyFile = "../../test/client_certs/server.key"
c.VerifyOutgoing = true
c.TLSConfig.CAFile = "../../test/client_certs/rootca.crt"
c.TLSConfig.CertFile = "../../test/client_certs/server.crt"
c.TLSConfig.KeyFile = "../../test/client_certs/server.key"
c.TLSConfig.VerifyOutgoing = true
})
defer os.RemoveAll(dir1)
defer s1.Shutdown()
@ -1413,9 +1413,9 @@ func TestServer_TLSToFullVerify(t *testing.T) {
// Add a second server with TLS configured
dir2, s2 := testServerWithConfig(t, func(c *Config) {
c.Bootstrap = false
c.CAFile = "../../test/client_certs/rootca.crt"
c.CertFile = "../../test/client_certs/server.crt"
c.KeyFile = "../../test/client_certs/server.key"
c.TLSConfig.CAFile = "../../test/client_certs/rootca.crt"
c.TLSConfig.CertFile = "../../test/client_certs/server.crt"
c.TLSConfig.KeyFile = "../../test/client_certs/server.key"
})
defer os.RemoveAll(dir2)
defer s2.Shutdown()

View File

@ -26,8 +26,8 @@ func TestSubscribeBackend_IntegrationWithServer_TLSEnabled(t *testing.T) {
t.Parallel()
_, conf1 := testServerConfig(t)
conf1.VerifyIncoming = true
conf1.VerifyOutgoing = true
conf1.TLSConfig.VerifyIncoming = true
conf1.TLSConfig.VerifyOutgoing = true
conf1.RPCConfig.EnableStreaming = true
configureTLS(conf1)
server, err := newServer(t, conf1)
@ -147,11 +147,11 @@ func TestSubscribeBackend_IntegrationWithServer_TLSReload(t *testing.T) {
// Set up a server with initially bad certificates.
_, conf1 := testServerConfig(t)
conf1.VerifyIncoming = true
conf1.VerifyOutgoing = true
conf1.CAFile = "../../test/ca/root.cer"
conf1.CertFile = "../../test/key/ssl-cert-snakeoil.pem"
conf1.KeyFile = "../../test/key/ssl-cert-snakeoil.key"
conf1.TLSConfig.VerifyIncoming = true
conf1.TLSConfig.VerifyOutgoing = true
conf1.TLSConfig.CAFile = "../../test/ca/root.cer"
conf1.TLSConfig.CertFile = "../../test/key/ssl-cert-snakeoil.pem"
conf1.TLSConfig.KeyFile = "../../test/key/ssl-cert-snakeoil.key"
conf1.RPCConfig.EnableStreaming = true
server, err := newServer(t, conf1)
@ -178,7 +178,7 @@ func TestSubscribeBackend_IntegrationWithServer_TLSReload(t *testing.T) {
require.Error(t, err)
// Reload the server with valid certs
newConf := server.config.ToTLSUtilConfig()
newConf := server.config.TLSConfig
newConf.CertFile = "../../test/key/ourdomain.cer"
newConf.KeyFile = "../../test/key/ourdomain.key"
server.tlsConfigurator.Update(newConf)
@ -192,7 +192,7 @@ func TestSubscribeBackend_IntegrationWithServer_TLSReload(t *testing.T) {
}
func clientConfigVerifyOutgoing(config *Config) {
config.VerifyOutgoing = true
config.TLSConfig.VerifyOutgoing = true
}
// retryFailedConn forces the ClientConn to reset its backoff timer and retry the connection,