From b30ec82d2dbf5aaca8798080e24d56325b7f6812 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Thu, 24 Jun 2021 14:31:17 -0400 Subject: [PATCH] tlsutil: unexport and remove indirection Unexport outgoingALPNRPCConfig since it is only used internally Remove the MutualTLSCapable->mutualTLSCapable indirection, we only need the exported method. Inline enableAgentTLSForChecks to make it more clear what it does, since it only has a single caller and is wrapping a single field lookup. --- tlsutil/config.go | 34 ++++++++++++++-------------------- tlsutil/config_test.go | 22 +++++++--------------- 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/tlsutil/config.go b/tlsutil/config.go index 69b358fed..4d493e3ce 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -565,12 +565,9 @@ func (c *Configurator) outgoingRPCTLSDisabled() bool { return true } +// MutualTLSCapable returns true if Configurator has a CA and a local TLS +// certificate configured. func (c *Configurator) MutualTLSCapable() bool { - return c.mutualTLSCapable() -} - -// This function acquires a read lock because it reads from the config. -func (c *Configurator) mutualTLSCapable() bool { c.lock.RLock() defer c.lock.RUnlock() return c.caPool != nil && (c.autoTLS.cert != nil || c.manual.cert != nil) @@ -622,13 +619,6 @@ func (c *Configurator) verifyIncomingHTTPS() bool { return c.base.verifyIncomingHTTPS() } -// This function acquires a read lock because it reads from the config. -func (c *Configurator) enableAgentTLSForChecks() bool { - c.lock.RLock() - defer c.lock.RUnlock() - return c.base.EnableAgentTLSForChecks -} - // This function acquires a read lock because it reads from the config. func (c *Configurator) serverNameOrNodeName() string { c.lock.RLock() @@ -720,7 +710,11 @@ func (c *Configurator) IncomingHTTPSConfig() *tls.Config { func (c *Configurator) OutgoingTLSConfigForCheck(skipVerify bool, serverName string) *tls.Config { c.log("OutgoingTLSConfigForCheck") - if !c.enableAgentTLSForChecks() { + c.lock.RLock() + useAgentTLS := c.base.EnableAgentTLSForChecks + c.lock.RUnlock() + + if !useAgentTLS { return &tls.Config{ InsecureSkipVerify: skipVerify, ServerName: serverName, @@ -748,14 +742,14 @@ func (c *Configurator) OutgoingRPCConfig() *tls.Config { return c.commonTLSConfig(false) } -// OutgoingALPNRPCConfig generates a *tls.Config for outgoing RPC connections +// outgoingALPNRPCConfig generates a *tls.Config for outgoing RPC connections // directly using TLS with ALPN instead of the older byte-prefixed protocol. // If there is a CA or VerifyOutgoing is set, a *tls.Config will be provided, // otherwise we assume that no TLS should be used which completely disables the // ALPN variation. -func (c *Configurator) OutgoingALPNRPCConfig() *tls.Config { - c.log("OutgoingALPNRPCConfig") - if !c.mutualTLSCapable() { +func (c *Configurator) outgoingALPNRPCConfig() *tls.Config { + c.log("outgoingALPNRPCConfig") + if !c.MutualTLSCapable() { return nil // ultimately this will hard-fail as TLS is required } @@ -784,11 +778,11 @@ func (c *Configurator) UseTLS(dc string) bool { return !c.outgoingRPCTLSDisabled() && c.getAreaForPeerDatacenterUseTLS(dc) } -// OutgoingALPNRPCWrapper wraps the result of OutgoingALPNRPCConfig in an +// OutgoingALPNRPCWrapper wraps the result of outgoingALPNRPCConfig in an // ALPNWrapper. It configures all of the negotiation plumbing. func (c *Configurator) OutgoingALPNRPCWrapper() ALPNWrapper { c.log("OutgoingALPNRPCWrapper") - if !c.mutualTLSCapable() { + if !c.MutualTLSCapable() { return nil } @@ -893,7 +887,7 @@ func (c *Configurator) wrapALPNTLSClient(dc, nodeName, alpnProto string, conn ne return nil, fmt.Errorf("cannot dial using ALPN-RPC without a target alpn protocol") } - config := c.OutgoingALPNRPCConfig() + config := c.outgoingALPNRPCConfig() if config == nil { return nil, fmt.Errorf("cannot dial via a mesh gateway when outgoing TLS is disabled") } diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index 42116c985..d55aed56c 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -768,7 +768,7 @@ func TestConfigurator_MutualTLSCapable(t *testing.T) { c, err := NewConfigurator(config, nil) require.NoError(t, err) - require.False(t, c.mutualTLSCapable()) + require.False(t, c.MutualTLSCapable()) }) t.Run("ca and no keys", func(t *testing.T) { @@ -779,7 +779,7 @@ func TestConfigurator_MutualTLSCapable(t *testing.T) { c, err := NewConfigurator(config, nil) require.NoError(t, err) - require.False(t, c.mutualTLSCapable()) + require.False(t, c.MutualTLSCapable()) }) t.Run("ca and manual key", func(t *testing.T) { @@ -792,7 +792,7 @@ func TestConfigurator_MutualTLSCapable(t *testing.T) { c, err := NewConfigurator(config, nil) require.NoError(t, err) - require.True(t, c.mutualTLSCapable()) + require.True(t, c.MutualTLSCapable()) }) loadFile := func(t *testing.T, path string) string { @@ -811,7 +811,7 @@ func TestConfigurator_MutualTLSCapable(t *testing.T) { caPEM := loadFile(t, "../test/hostname/CertAuth.crt") require.NoError(t, c.UpdateAutoTLSCA([]string{caPEM})) - require.False(t, c.mutualTLSCapable()) + require.False(t, c.MutualTLSCapable()) }) t.Run("autoencrypt ca and autoencrypt key", func(t *testing.T) { @@ -827,7 +827,7 @@ func TestConfigurator_MutualTLSCapable(t *testing.T) { require.NoError(t, c.UpdateAutoTLSCA([]string{caPEM})) require.NoError(t, c.UpdateAutoTLSCert(certPEM, keyPEM)) - require.True(t, c.mutualTLSCapable()) + require.True(t, c.MutualTLSCapable()) }) } @@ -858,14 +858,6 @@ func TestConfigurator_VerifyIncomingHTTPS(t *testing.T) { require.Equal(t, c.base.VerifyIncomingHTTPS, verify) } -func TestConfigurator_EnableAgentTLSForChecks(t *testing.T) { - c := Configurator{base: &Config{ - EnableAgentTLSForChecks: true, - }} - enabled := c.enableAgentTLSForChecks() - require.Equal(t, c.base.EnableAgentTLSForChecks, enabled) -} - func TestConfigurator_IncomingRPCConfig(t *testing.T) { c, err := NewConfigurator(Config{ VerifyIncomingRPC: true, @@ -1068,7 +1060,7 @@ func TestConfigurator_OutgoingRPCConfig(t *testing.T) { func TestConfigurator_OutgoingALPNRPCConfig(t *testing.T) { c := &Configurator{base: &Config{}} - require.Nil(t, c.OutgoingALPNRPCConfig()) + require.Nil(t, c.outgoingALPNRPCConfig()) c, err := NewConfigurator(Config{ VerifyOutgoing: false, // ignored, assumed true @@ -1078,7 +1070,7 @@ func TestConfigurator_OutgoingALPNRPCConfig(t *testing.T) { }, nil) require.NoError(t, err) - tlsConf := c.OutgoingALPNRPCConfig() + tlsConf := c.outgoingALPNRPCConfig() require.NotNil(t, tlsConf) require.Equal(t, tls.RequireAndVerifyClientCert, tlsConf.ClientAuth) require.False(t, tlsConf.InsecureSkipVerify)